David Brownell wrote:
> The patch adds a SPI driver for DaVinci series SOC's.


Hello David and Sandeep

Just some comments to your driver.

Why are you talking about 32 bit transfer? I don't know all devices this driver
is meant for, but at least DM355 has only up to 16bits.

> + * davinci_spi_setup_transfer - This functions will determine transfer method
> + * @spi: spi device on which data transfer to be done
> + * @t: spi transfer in which transfer info is filled
> + *
> + * This function determines data transfer method (8/16/32 bit transfer).

[snip]
See also here:

> +     /*
> +      * Assign function pointer to appropriate transfer method
> +      * 8bit, 16bit or 32bit transfer
> +      */



A question to using a division in "davinci_spi_bufs_pio":

> +     conv = davinci_spi->slave[spi->chip_select].bytes_per_word;
> +     davinci_spi->count = t->len / conv;

I'm not sure, if the compiler manages to detect that "bytes_per_word" can be
only 1 or 2 and uses shifts? Divisions are quite slow.



Did you read the silicon errata for DM355, sprz264d? At least the error checks
should be done special, and CS can cause trouble, too, as described on p.15ff.

As a side note unrelated to SPI: Does anybody know, if the kernel works around
"Concurrent Access to ARM Internal Memory May Result in Access Errors" (p.14 of
sprz264d)?


Best regards
Siegbert


_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to