Re: [RFC PATCH] drivers: bus: add ARM CCI support

[Russell King - ARM Linux <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>]

On Thu, Apr 18, 2013 at 01:54:04PM -0400, Nicolas Pitre wrote:
> On Thu, 18 Apr 2013, Stephen Boyd wrote:
> 
> > On 04/11/13 07:47, Lorenzo Pieralisi wrote:
> > > diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> > > new file mode 100644
> > > index 0000000..81953de
> > > --- /dev/null
> > > +++ b/drivers/bus/arm-cci.c
> > [...]
> > > +static void notrace cci_port_control(unsigned int port, bool enable)
> > > +{
> > > +	void __iomem *base = ports[port].base;
> > > +
> > > +	if (!base)
> > > +		return;
> > > +
> > > +	writel_relaxed(enable, base + CCI_PORT_CTRL);
> > > +	while (readl_relaxed(cci_ctrl_base + CCI_CTRL_STATUS) & 0x1)
> > > +			;
> > 
> > cpu_relax()?
> 
> In some cases there is no more cache coherence when this is called and 
> the hardware might not be in a good state to cope with whatever action 
> people might be tempted to insert into cpu_relax().  After the CCI is 
> disabled it is important to keep a low profile and not touch anything 
> global. With some early hardware revision, even a DMB here was harmful.

It would be useful if there was a comment in the code explaining this.
As it stands, you _will_ get a stream of patches from kernel janitors
itching to add cpu_relax() there.

If you're going to do something different from the norm, it _needs_ to
definitely be commented and explained.