On 05/02/2021 02:31, Doug Anderson wrote:
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.
I have been observing strange behaviour from the SPI CAN interface. This
change allowed me to narrow down the issue (with the GPI support for
SPI) we had in our tree.
[skipped the explanation.
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.
That was my original idea, with qup-spi-nocs config being in sm8250.dtsi
and spi0_cs (defining only CS) belonging to the board dts.
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
--
With best wishes
Dmitry
