Re: Multi driver SPI chip select count unenforced.

From: Joe Burmeister <joe.burmeister@devtank.co.uk>
To: Lukas Wunner <lukas@wunner.de>
Cc: Mark Brown <broonie@kernel.org>,
	linux-spi@vger.kernel.org, Phil Elwell <phil@raspberrypi.com>
Subject: Re: Multi driver SPI chip select count unenforced.
Date: Mon, 19 Apr 2021 13:56:30 +0100	[thread overview]
Message-ID: <438c7530-49db-47cc-6451-4defc645b577@devtank.co.uk> (raw)
In-Reply-To: <20210416153019.GA26144@wunner.de>

Thanks Lukas,

I'll tweak and resubmit properly.

Regards,

Joe

On 16/04/2021 16:30, Lukas Wunner wrote:
> On Fri, Apr 16, 2021 at 11:55:45AM +0100, Joe Burmeister wrote:
>> However, after I found the same broken pattern in a few places.
>> You can find suspects by grep'ing in the SPI folder for "->chip_select]".
> You need to cc the maintainers of the affected drivers.
> You can retrieve them with "scripts/get_maintainer.pl drivers/spi/spi-...".
>
> Please don't attach patches but rather submit each one separately.
> Pass a commit range (abcdefg..0123456) to "git format-patch" to
> get numbered patches, then submit them with msmtp or "git send-email".
>
>
>> Subject: [PATCH] Handle SPI device setup callback failure.
> Prepend the subsystem to the subject and drop the period, e.g.:
>
> spi: Handle SPI device setup callback failure
>
> Patch otherwise LGTM.
>
>
>> Subject: [PATCH] Remove BCM2835 SPI chipselect limit.
> Prepend "spi: bcm2835: " in this case and drop the period.
> Use "git log --oneline <filename>" to see what the prefix should look
> like for a particular file.
>
>
>> The limit of 4 chipselects for the BCM2835 was not required and also was
>> not inforced. Without inforcement it was possible to make a device tree
>> over this limit which would trample memory.
>>
>> The chipselect count is now obtained from the device tree and expanded
>> if more devices are added.
> I'd prefer it if you could just raise BCM2835_SPI_NUM_CS to 6
> (or 8 or whatever you need).  Use commit 603e92ff10a8 as a template.
>
> Then submit a separate patch to error out of bcm2835_spi_setup()
> if bcm2835_spi_setup >= BCM2835_SPI_NUM_CS.
>
> Honestly I think the additional code complexity isn't worth it to allow
> for dynamic resizing.  Raising BCM2835_SPI_NUM_CS results in just a
> few additional bytes, that's probably smaller than the increase of the
> text segment due to the additional resizing code.
>
> Nit: I think the correct spelling is "enforce".  (Not an English native
> speaker but my dictionary says so.)
>
> Thanks!
>
> Lukas