| Message ID | 1620270017-52643-1-git-send-email-f.fangjian@huawei.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | spi: Correct CS GPIOs polarity when using GPIO descriptors | expand |
On 2021/5/6 16:14, Andy Shevchenko wrote: > > > On Thursday, May 6, 2021, Andy Shevchenko <andy.shevchenko@gmail.com <mailto:andy.shevchenko@gmail.com>> wrote: > > > > On Thursday, May 6, 2021, Jay Fang <f.fangjian@huawei.com <mailto:f.fangjian@huawei.com>> wrote: > > This series introduces a generic implementation to solve the conflict > between the 'cs-gpios' flags and the optional SPI slaves flags. So we > don't need to add two similar quirks separately for DT and ACPI. > > > NAK. There is a patch against documentation that clarifies polarity of GPIO for ACPI. I have a fix for that to use generic implementation of CS GPIOs . Thanks. Has the Fix patch been merged ? Commit Id ? > > > https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=bleeding-edge&id=ec3576eac11d66a388b6cba6a7cfb3b45039a712 <https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=bleeding-edge&id=ec3576eac11d66a388b6cba6a7cfb3b45039a712> > > > If your ACPI table uses GPIO CS with polarity low (assumes _DSD() is involved), this is a bug. Fix firmware or do a quirk specific for your platform. > > > > > > Jay Fang (2): > spi: Correct CS GPIOs polarity when using GPIO descriptors > Revert "gpio: of: Handle SPI chipselect legacy bindings" > > drivers/gpio/gpiolib-of.c | 51 ++--------------------------------------------- > drivers/spi/spi.c | 22 ++++++++++++++++++-- > 2 files changed, 22 insertions(+), 51 deletions(-) > > -- > 2.7.4 > > > > -- > With Best Regards, > Andy Shevchenko > > > > > -- > With Best Regards, > Andy Shevchenko > >
On Thu, May 6, 2021 at 12:11 PM Jay Fang <f.fangjian@huawei.com> wrote: > On 2021/5/6 16:14, Andy Shevchenko wrote: > > On Thursday, May 6, 2021, Andy Shevchenko <andy.shevchenko@gmail.com <mailto:andy.shevchenko@gmail.com>> wrote: > > On Thursday, May 6, 2021, Jay Fang <f.fangjian@huawei.com <mailto:f.fangjian@huawei.com>> wrote: > > > > This series introduces a generic implementation to solve the conflict > > between the 'cs-gpios' flags and the optional SPI slaves flags. So we > > don't need to add two similar quirks separately for DT and ACPI. > > NAK. There is a patch against documentation that clarifies polarity of GPIO for ACPI. I have a fix for that to use generic implementation of CS GPIOs . > > Thanks. Has the Fix patch been merged ? Commit Id ? Not yet. I'm planning to send it next week (after v5.13-rc1 is out) as a fix. For your convenience the whole story is available in my publick branch: https://gitlab.com/andy-shev/next/-/tree/topic/spi/reload and yes, I have tested it on real hardware. > > https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=bleeding-edge&id=ec3576eac11d66a388b6cba6a7cfb3b45039a712 <https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=bleeding-edge&id=ec3576eac11d66a388b6cba6a7cfb3b45039a712> > > > > > > If your ACPI table uses GPIO CS with polarity low (assumes _DSD() is involved), this is a bug. Fix firmware or do a quirk specific for your platform.
On Thu, May 6, 2021 at 12:44 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Thu, May 6, 2021 at 12:11 PM Jay Fang <f.fangjian@huawei.com> wrote: > > On 2021/5/6 16:14, Andy Shevchenko wrote: > > > On Thursday, May 6, 2021, Andy Shevchenko <andy.shevchenko@gmail.com <mailto:andy.shevchenko@gmail.com>> wrote: > > > On Thursday, May 6, 2021, Jay Fang <f.fangjian@huawei.com <mailto:f.fangjian@huawei.com>> wrote: > > > > > > This series introduces a generic implementation to solve the conflict > > > between the 'cs-gpios' flags and the optional SPI slaves flags. So we > > > don't need to add two similar quirks separately for DT and ACPI. > > > > NAK. There is a patch against documentation that clarifies polarity of GPIO for ACPI. I have a fix for that to use generic implementation of CS GPIOs . > > > > Thanks. Has the Fix patch been merged ? Commit Id ? It's here https://gitlab.com/andy-shev/next/-/commit/5ccbdbb4787d871722f361d77c5f3cb806811c48 and now I remember that I didn't dare to make it as a fix due to: - recent (non-fix) dependencies - no existing driver uses it for ACPI (at least nothing has been reported) > Not yet. I'm planning to send it next week (after v5.13-rc1 is out) as a fix. > For your convenience the whole story is available in my publick branch: > > https://gitlab.com/andy-shev/next/-/tree/topic/spi/reload > > and yes, I have tested it on real hardware. > > > > https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=bleeding-edge&id=ec3576eac11d66a388b6cba6a7cfb3b45039a712 <https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=bleeding-edge&id=ec3576eac11d66a388b6cba6a7cfb3b45039a712> > > > > > > > > > If your ACPI table uses GPIO CS with polarity low (assumes _DSD() is involved), this is a bug. Fix firmware or do a quirk specific for your platform.
On Thu, May 6, 2021 at 11:45 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > Thanks. Has the Fix patch been merged ? Commit Id ? > > Not yet. I'm planning to send it next week (after v5.13-rc1 is out) as a fix. > For your convenience the whole story is available in my publick branch: > > https://gitlab.com/andy-shev/next/-/tree/topic/spi/reload > > and yes, I have tested it on real hardware. Oh what a nightmare you got into there. Curious that ACPI has SPI CS as always active high, but that just underscore that we *really* need to abstract this out to avoid really complex specialcasing. Interesting that the PXA2xx XScale lives in Merrifield, I didn't know. Reminds med of how the Samsung hardware is alive and well in the recent Apple M1 laptops. Yours, Linus Walleij
On Thu, May 06, 2021 at 01:06:23PM +0200, Linus Walleij wrote: > Interesting that the PXA2xx XScale lives in Merrifield, > I didn't know. Reminds med of how the Samsung > hardware is alive and well in the recent Apple M1 laptops. There's a lot of PXA IP blocks in modern Intel SoCs - half the audio hardware is PXA derived (though mostly connected via DSP rather than directly to the CPU).
On Thu, May 06, 2021 at 02:24:17PM +0300, Andy Shevchenko wrote: > On Thursday, May 6, 2021, Linus Walleij <linus.walleij@linaro.org> wrote: > > Curious that ACPI has SPI CS as always active high, > Here I didn’t get what exactly you are pointing out. GPIOs are active high, > due to historical reasons. Otherwise SPI CS depends on the actual hardware > and may be (most of the cases?) active low. SPI chip selects are almost always active low - the signal is often written nCS or /CS for that reason.
On Thu, May 6, 2021 at 2:35 PM Mark Brown <broonie@kernel.org> wrote: > On Thu, May 06, 2021 at 02:24:17PM +0300, Andy Shevchenko wrote: > > On Thursday, May 6, 2021, Linus Walleij <linus.walleij@linaro.org> wrote: > > > > Curious that ACPI has SPI CS as always active high, > > > Here I didn’t get what exactly you are pointing out. GPIOs are active high, > > due to historical reasons. Otherwise SPI CS depends on the actual hardware > > and may be (most of the cases?) active low. > > SPI chip selects are almost always active low - the signal is often > written nCS or /CS for that reason. Exactly, and it's not altered with ACPI. That's the whole point of keeping it active high for _GPIO_ CS case.