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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 15 Feb 2011 22:12:54 +1300
From:	Ryan Mallon <ryan@...ewatersys.com>
To:	Thomas Chou <thomas@...ron.com.tw>
CC:	David Brownell <dbrownell@...rs.sourceforge.net>,
	Grant Likely <grant.likely@...retlab.ca>,
	linux-kernel@...r.kernel.org, nios2-dev@...c.et.ntust.edu.tw,
	devicetree-discuss@...ts.ozlabs.org,
	spi-devel-general@...ts.sourceforge.net
Subject: 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:
>> On 02/14/2011 03:10 PM, Thomas Chou wrote:
>>> 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<thomas@...ron.com.tw>
>>> ---
>>
>>> +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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/