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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <160695172591.2717324.17788035024164242534@swboyd.mtv.corp.google.com>
Date:   Wed, 02 Dec 2020 15:28:45 -0800
From:   Stephen Boyd <swboyd@...omium.org>
To:     Alexandru M Stan <amstan@...omium.org>
Cc:     Mark Brown <broonie@...nel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-arm-msm@...r.kernel.org, linux-spi@...r.kernel.org,
        Akash Asthana <akashast@...eaurora.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Douglas Anderson <dianders@...omium.org>
Subject: Re: [PATCH] spi: spi-geni-qcom: Use the new method of gpio CS control

Quoting Alexandru M Stan (2020-12-02 14:18:20)
> On Wed, Dec 2, 2020 at 1:49 PM Stephen Boyd <swboyd@...omium.org> 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@...eaurora.org>
> > Cc: Bjorn Andersson <bjorn.andersson@...aro.org>
> > Reviewed-by: Douglas Anderson <dianders@...omium.org>
> > Acked-by: Alexandru M Stan <amstan@...omium.org>
> 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@...omium.org>
> > ---
> >  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.