Hi,
On Thu, Feb 4, 2021 at 3:07 PM Bjorn Andersson
<bjorn.andersson@xxxxxxxxxx> wrote:
>
> On Thu 04 Feb 14:49 CST 2021, Dmitry Baryshkov wrote:
>
> > GENI SPI controller shows several issues if it manages the CS on its own
> > (see 37dd4b777942 ("arm64: dts: qcom: sc7180: Provide pinconf for SPI to
> > use GPIO for CS")) for the details. Provide pinctrl entries for SPI
> > controllers using the same CS pin but in GPIO mode.
I'm curious: were you seeing real problems or are you just trying to
optimize things more? The only known problem (other than performance)
that I'm aware of is that if you get really high interrupt latency
then setting the chip select can appear to "fail" (we timeout waiting
for the interrupt saying that the chip select command was done). The
SPI framework doesn't expect setting the chip select to be something
that can fail so error handling isn't the most amazing in this case,
though at least it shouldn't crash after the most recent fixes I sent
to the SPI driver.
> Doug, can you confirm that this is the final (or at least current)
> verdict?
As far as I know using a GPIO chip select is always superior on
Qualcomm hardware unless you're forced into GPI/GSI mode. In GPI/GSI
mode (I think) you lose full control of the chip select anyway. I
wrote this long-winded explanation recently to explain it to someone
else. Pasting here in case it's useful, though there's overlap w/ the
commit message that Dmitry pointed at.
---
In general the Linux kernel supports using any GPIO as a SPI chip
select. However, it also supports the concept that a SPI controller
may have its own way of controlling chip select. You can freely mix
and match these approaches. For instance maybe you've got 4 devices
on a single SPI "bus" (we never do this on Chrome OS designs but it is
possible). The SPI controller itself might have the ability to
control two chip selects. You could still make things work by hooking
two of the peripherals up to GPIOs and two up to the SPI controller's
native chip selects and it'd all be hunky dory. Or you could choose
to not use any of the builtin chip selects and use all 4 on GPIOs.
...or 3 and 1.
When a SPI controller has a builtin chip select, it can usually be
configured in two ways: something more automatic where the controller
asserts the chip select and deasserts it automatically around
transfers or in a fully manual mode. In general Linux prefers the
fully manual mode. The Linux API to SPI endpoint drivers is super
flexible and allows them to assert/deassert chip select at will and
it's hard to honor that flexibility when it's not manually controlled.
If a SPI chip select is manually controlled it's not at all different
from a GPIO configured in "output mode". Thus using a GPIO instead of
a builtin chip select has no real downsides.
On many SoCs, sc7180 included, pinmuxing allows you to reconfigure
almost any pin as a GPIO. So it turns out that on sc7180 boards it's
just a different way of looking at it to say whether we're hooked up
to a GPIO or we're hooked up to the native chip select logic. Both
are valid ways to look at it.
On Qualcomm SoCs it's incredibly inefficient to control the native SPI
chip select in "manual mode". It involves sending a packet (a
command) to the SPI controller and waiting for it to respond that it
set the chip select. However, it is plenty efficient to control
GPIOs. Thus it is more efficient (with no real downsides) to envision
that the chip select is hooked up to a GPIO instead of the native chip
select.
---
>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> > ---
> > arch/arm64/boot/dts/qcom/sm8250.dtsi | 380 +++++++++++++++++++++++++++
> > 1 file changed, 380 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > index 3cea28058a91..03015174ec06 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> > @@ -3046,6 +3046,25 @@ config {
> > };
> > };
> >
> > + qup_spi0_cs_gpio: qup-spi0-cs-gpio {
>
> There might be others who need the same states, but I would prefer if we
> move this to the device's dts.
This is opposite to what Stephen requested, though it was in a review
on our gerrit and not on lists [1]. :-P
It definitely feels like a 6 of one half dozen of the other. Unless
you're dead set on moving them to the board dts my bias would be
towards keeping consistent with what was done on sc7180. If you
really want this moved to the board file we should do it for sc7180
too.
> > + mux {
>
> Rather than splitting the properties in {mux, cs, config} I think it
> makes more sense to split them in {spi, cs} or something like that.
In general pinconf doesn't belong in the SoC dts file. If there's no
reason to change it seems like this should match what sc7180 did.
[1] https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2406557/1/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
