mbox series

[0/2] spi: Correct CS GPIOs polarity when using GPIO descriptors

Message ID 1620270017-52643-1-git-send-email-f.fangjian@huawei.com
Headers show
Series spi: Correct CS GPIOs polarity when using GPIO descriptors | expand

Message

Jay Fang May 6, 2021, 3 a.m. UTC
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.

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(-)

Comments

Andy Shevchenko May 6, 2021, 9:44 a.m. UTC | #1
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.
Linus Walleij May 6, 2021, 11:06 a.m. UTC | #2
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
Mark Brown May 6, 2021, 11:24 a.m. UTC | #3
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).
Andy Shevchenko May 6, 2021, 12:17 p.m. UTC | #4
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.