Re: [PATCH] spi: spi-geni-qcom: Use the new method of gpio CS control

From: Stephen Boyd
Date: Wed Dec 02 2020 - 18:29:45 EST


Quoting Alexandru M Stan (2020-12-02 14:18:20)
> On Wed, Dec 2, 2020 at 1:49 PM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> >
> > Let's set the 'use_gpio_descriptors' field so that we use the new way of
> > requesting the CS GPIOs in the core. This allows us to avoid having to
> > configure the CS pins in "output" mode with an 'output-enable' pinctrl
> > setting.
> >
> > Cc: Akash Asthana <akashast@xxxxxxxxxxxxxx>
> > Cc: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
> > Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> > Acked-by: Alexandru M Stan <amstan@xxxxxxxxxxxx>
> I meant this as a joke in chat. It doesn't really mean anything in any capacity.

Sorry! It can be removed when applying.

>
> > Signed-off-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> > ---
> > drivers/spi/spi-geni-qcom.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> > index 25810a7eef10..c4c88984abc9 100644
> > --- a/drivers/spi/spi-geni-qcom.c
> > +++ b/drivers/spi/spi-geni-qcom.c
> > @@ -636,6 +636,7 @@ static int spi_geni_probe(struct platform_device *pdev)
> > spi->auto_runtime_pm = true;
> > spi->handle_err = handle_fifo_timeout;
> > spi->set_cs = spi_geni_set_cs;
> > + spi->use_gpio_descriptors = true;
> >
> > init_completion(&mas->cs_done);
> > init_completion(&mas->cancel_done);
> >
> > base-commit: b65054597872ce3aefbc6a666385eabdf9e288da
> > --
> > https://chromeos.dev
> >
>
> Unfortunately this patch makes my cros-ec (the main EC that used to
> work even before my debugging) also fail to probe:
> [ 0.839533] cros-ec-spi spi6.0: EC failed to respond in time
> [ 1.040453] cros-ec-spi spi6.0: EC failed to respond in time
> [ 1.040852] cros-ec-spi spi6.0: Cannot identify the EC: error -110
> [ 1.040855] cros-ec-spi spi6.0: cannot register EC, fallback to spidev
> [ 1.040942] cros-ec-spi: probe of spi6.0 failed with error -110
>
> I wasn't closely looking at this part closely when I was using my
> other spi port with spidev, so this is why I haven't noticed it
> before.
> Doug suggests this might be a polarity issue. More scoping to be had.
>

Ah I see. It looks like the cs-gpios polarity is wrong for the DTS on
sc7180. That's a patch that Doug has sent in for the qcom tree, commit
37dd4b777942 ("arm64: dts: qcom: sc7180: Provide pinconf for SPI to use
GPIO for CS") and it is pending for the next release (v5.11). Doug says
he will send in a fix for the DTS side, but this patch is still "good"
as far as I can tell. It moves us to use gpio descriptors and also finds
bugs like this in the DTS file that we would have missed otherwise
because the legacy mode doesn't look at the polarity flags in DT.