Messages in this thread | | | Date | Thu, 12 Nov 2020 11:41:36 +0000 | From | Charles Keepax <> | Subject | Re: [PATCH v1] spi: fix client driver breakages when using GPIO descriptors |
| |
On Wed, Nov 11, 2020 at 11:24:14AM -0500, Sven Van Asbroeck wrote: > On Wed, Nov 11, 2020 at 10:48 AM Mark Brown <broonie@kernel.org> wrote: > > > > Applied to > > > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next > > Thank you ! > > Now that our minds are still focused on this subject, should > commit 138c9c32f090 ("spi: spidev: Fix CS polarity if GPIO descriptors > are used") > be reverted? > > This fixed spidev to deal with SPI_CS_HIGH on gpiod. > But after our fix, its behaviour will probably be broken again. > > Another candidate for revert is > commit ada9e3fcc175 ("spi: dw: Correct handling of native chipselect") > although I don't understand that code well enough to be sure. > > Adding Charles Keepax.
Looks like the code has changed a fair amount since my patch. The important detail from it was trying to clarify the semantics of the controller->set_cs callback. That function is called with a boolean argument and that argument could have two possible meanings:
1) True means apply a high logic level to the chip select line. 2) True mean apply chip select.
Under interpretation 2) the chip select line would be set to a different logic level depending on if the device is active high or active low.
If I remember correctly at the point of my patch the core had just changed between the two a couple of times but now consistently did 1) (and looks like it still does), my patch intended to updated the spi-dw driver to match that, as my SPI had stopped working. I think it then turned out, my patch broke some other use-cases and that the bit in the IP basically had 2) semantics in hardware. Which is what this patch fixed:
commit 9aea644ca17b ("spi: dw: Fix native CS being unset")
After that patch my patch is mostly replaced so I don't think it would make any sense to revert my patch at this point, and I don't think your patch will break the spi-dw driver. I don't have easy access to the hardware right now to test, but I will give it is quick run when that option becomes available to me again.
Your fix looks good to me, but I suspect you do need to fix the spidev stuff although I have haven't looked at that in detail.
Thanks, Charles
| |