[GIT PULL v3] updates to qbman (soc drivers) to support arm/arm64

Mark Rutland mark.rutland at arm.com
Fri Jun 23 09:23:23 PDT 2017


On Fri, Jun 23, 2017 at 04:55:40PM +0100, Russell King - ARM Linux wrote:
> On Fri, Jun 23, 2017 at 04:22:27PM +0100, Mark Rutland wrote:
> > The prior discussion on that front were largely to do with teh
> > shareability of that memory, which is an orthogonal concern.
> > 
> > If these are actually MMIO registers, a Device memory type must be used,
> > rather than a Normal memory type. There are a number of things that
> > could go wrong due to relaxations permitted for Normal memory, such as
> > speculative reads, the potential set of access sizes, memory
> > transactions that the endpoint might not understand, etc.
> 
> Well, we have:
> 
>         dma_wmb();
>         mc->cr->_ncw_verb = myverb | mc->vbit;
>         dpaa_flush(mc->cr);
> 
> and we also have:
> 
>         mc->cr = portal->addr.ce + BM_CL_CR;
>         mc->rridx = (__raw_readb(&mc->cr->_ncw_verb) & BM_MCC_VERB_VBIT) ?
>                     0 : 1;
> 
> and:
> 
>         dpaa_zero(mc->cr);
> 
> which is just another name for a 64-byte memset() which doesn't have
> any flushing.
> 
> Note that this is memory obtained from the ioremap() family of functions,
> so all of these pointers _should_ have __iomem annotations (but don't.)
> However, if this isn't iomem, then it shouldn't be using the ioremap()
> family of functions, and should be using memremap() instead.
> 
> Without really having an understanding of what this chunk of code is
> doing (and abuse like the above makes it harder than necessary) no one
> can really say from just reading through the code... which brings up
> another problem - that of long term maintanability.

Sure; agreed on all counts.

I was (evidently poorly) attempting to clarify what prior discussions
had (and hadn't) covered.

Mark.



More information about the linux-arm-kernel mailing list