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.

>> +     SSYNC();
>
> The SSYNC shouldn't be necessary with the correct accessors.

independently incorrect from the ioread/iowrite comment.  there's no
way any register accessors function would ever impose a SSYNC on the
Blackfin architecture.  this causes a *system* wide sync of *all*
registers and as such, is used only when necessary.  if we did as you
proposed and added them to every single read/write, it would kill
performance.

the rest looked fine and should be doable in time
-mike

------------------------------------------------------------------------------
Gaining the trust of online customers is vital for the success of any company
that requires sensitive data to be transmitted over the Web.   Learn how to 
best implement a security strategy that keeps consumers' information secure 
and instills the confidence they need to proceed with transactions.
http://p.sf.net/sfu/oracle-sfdevnl 
_______________________________________________
spi-devel-general mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/spi-devel-general

Reply via email to