Re: [Uclinux-dist-devel] [PATCH] spi/bfin_sport_spi: new driver to emulate a SPI bus using Blackfin SPORT peripheral

[Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>]

On Mon, Jan 10, 2011 at 10:22, Grant Likely wrote:
> On Mon, Jan 10, 2011 at 09:39:40AM -0500, Mike Frysinger wrote:
>> On Mon, Nov 1, 2010 at 01:56, Grant Likely wrote:
>> > On Fri, Oct 22, 2010 at 02:53:35AM -0400, Mike Frysinger wrote:
>> >> +     /* SPI framework hookup */
>> >> +     struct spi_master *master;
>> >> +
>> >> +     /* Regs base of SPI controller */
>> >> +     volatile struct sport_register __iomem *regs;
>> >
>> > Drop the volatile.  Registers must never be dereferenced directly
>> > anyway.
>> >
>> >> +static void bfin_sport_spi_enable(struct master_data *drv_data)
>> >> +{
>> >> +     drv_data->regs->tcr1 |= TSPEN;
>> >> +     drv_data->regs->rcr1 |= TSPEN;
>> >
>> > Illegal direct dereference; use ioread/iowrite instead.  Ditto through
>> > rest of file.
>>
>> that's not correct.  ioread/iowrite are fine if the devices were
>> generically mapped through say the asynchronous memory bus, but these
>> registers arent.  they're mapped directly into the Blackfin hardware
>> system bus and have different semantics than the async memory bus.
>
> It is not okay to use direct register dereference in Linux for well
> documented reasons (see volatile-considered-harmful.txt).

and that document by & large talks about how people abuse it for
normal storage in place of proper barriers which has no bearing here

> particular, accessors prevent either the compiler or the CPU from
> reordering or optimizing out accesses to registers in a predictable
> way.  volatile is strongly discouraged.  I may have the specific
> accessor routine name wrong for the blackfin use case, but direct
> dereference is definitely not okay.

the volatile markings on the registers prevents the same thing for the
same reason.  this usage might be discouraged, but it is certainly not
"illegal" or "wrong".  it's simply "not the preferred method".
-mike
_______________________________________________
Uclinux-dist-devel mailing list
Uclinux-dist-devel@blackfin.uclinux.org
https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel