Quoting Douglas Anderson (2021-02-18 14:55:09) > At boot time the following happens: > 1. Device core gets ready to probe our SPI driver. > 2. Device core applies SPI controller's "default" pinctrl. > 3. Device core calls the SPI driver's probe() function which will > eventually setup the chip select GPIO as "unasserted". > > Thinking about the above, we can find: > a) For SPI devices that the BIOS inits (Cr50 and EC), the BIOS would > have had them configured as "GENI" pins and not as "GPIO" pins. > b) It turns out that our BIOS also happens to init these pins as > "output" (even though it doesn't need to since they're not muxed as > GPIO) but leaves them at the default state of "low". > c) As soon as we apply the "default" chip select it'll switch the > function to GPIO and stop driving the chip select high (which is > how "GENI" was driving it) and start driving it low. > d) As of commit 9378f46040be ("UPSTREAM: spi: spi-geni-qcom: Use the > new method of gpio CS control"), when the SPI core inits things it > inits the GPIO to be "deasserted". Prior to that commit the GPIO > was left untouched until first use. > e) When the first transaction happens we'll assert the chip select and > then deassert it after done. > > So before the commit to change us to use gpio descriptors we used to > have a _really long_ assertion of chip select before our first > transaction (because it got pulled down and then the first "assert" > was a no-op). That wasn't great but (apparently) didn't cause any > real harm. > > After the commit to change us to use gpio descriptors we end up > glitching the chip select line during probe. It would go low and then > high with no data transferred. The other side ought to be robust > against this, but it certainly could cause some confusion. It's known > to at least cause an error message on the EC console and it's believed > that, under certain timing conditions, it could be getting the EC into > a confused state causing the EC driver to fail to probe. > > Let's fix things to avoid the glitch. We'll add an extra pinctrl > entry that sets the value of the pin to output high (CS deasserted) > before doing anything else. We'll do this in its own pinctrl node > that comes before the normal pinctrl entries to ensure that the order > is correct and that this gets applied before the mux change. > > This change is in the trogdor board file rather than in the SoC dtsi > file because chip select polarity can be different depending on what's > hooked up and it doesn't feel worth it to spam the SoC dtsi file with > both options. The board file would need to pick the right one anyway. > > Fixes: cfbb97fde694 ("arm64: dts: qcom: Switch sc7180-trogdor to control SPI CS via GPIO") > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > --- Reviewed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>