Re: [PATCH v9] spi: New driver for Altera SPI

On 15/02/11 20:04, Thomas Chou wrote:

Dear Ryan,

On 02/14/2011 10:20 AM, Ryan Mallon wrote:

quoted

On 02/14/2011 03:10 PM, Thomas Chou wrote:

quoted

This patch adds a new SPI driver to support the Altera SOPC Builder
SPI component. It uses the bitbanging library.

Signed-off-by: Thomas Chou<redacted>
---

quoted

+struct altera_spi {
+    /* bitbang has to be first */
+    struct spi_bitbang bitbang;

Is this still true? I had a quick look and can't see anything which
relies on spi_bitbang being the first entry. Things like this should be
using container_of so that position in the struct is irrelevant.

~Ryan

Yes, sadly true. This is due to the implementation of the bitbanging
library, spi_bitbang.c, which assumes the struct spi_bitbang is the
first of drvdata. Though it could be changed in the future (beyond this
little driver), every bitbanging library user has to follow this for now.

Hmm, I see how it works now. Shouldn't the conversion be done like this:

  struct spi_bitbang *bitbang = spi_master_get_devdata(master);
  struct altera_spi *altera = container_of(bitbang,
				 struct altera_spi, bitbang);

Which doesn't require the bitbang field to be the first in the struct
and makes it more clear what is actually going on. The above could be
wrapped into a to_altera_spi macro.

~Ryan