Re: [PATCH net-next] net: dsa: sja1105: adapt to a SPI controller with a limited max transfer size

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]

On Thu, May 20, 2021 at 05:06:09PM +0300, Vladimir Oltean wrote:
> On Thu, May 20, 2021 at 02:56:15PM +0100, Mark Brown wrote:
> > On Thu, May 20, 2021 at 04:50:31PM +0300, Vladimir Oltean wrote:

> > > Only that certain SPI controllers, such as the spi-sc18is602 I2C-to-SPI
> > > bridge, cannot keep the chip select asserted for that long.
> > > The spi_max_transfer_size() and spi_max_message_size() functions are how
> > > the controller can impose its hardware limitations upon the SPI
> > > peripheral driver.

> > You should respect both, frankly I don't see any advantage to using
> > cs_change for something like this - just do a bunch of async SPI
> > transfers and you'll get the same effect in terms of being able to keep
> > the queue for the controller primed with more robust support since it's
> > not stressing edge cases.  cs_change is more for doing things that are
> > just very non-standard.

> Sorry, I don't really understand your comment: in which way would it be
> more robust for my use case to use spi_async()?

Your description sounds like the driver is just stitching a bunch of
messages together into a single big message with lots of cs_changes with
the goal of improving performance which is really not using the API at
all idiomatically.  That's at best asking for trouble (it'll certainly
work with fewer controllers), it may even be less performant as you're
less likely to get the benefit of framework enhancements.

> The cs_change logic was already there prior to this patch, I am just
> reiterating how it works. Given the way in which it works (which I think

It seems like you could avoid this issue and most likely other future
issues by making the way the driver uses the API more normal.

> is correct), the most natural way to limit the buffer length is to look
> for the max transfer len.

No, you really do need to pay attention to both - what makes you think
it is safe to just ignore one of them?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]