On Thu 04 Feb 17:31 CST 2021, Doug Anderson wrote:
> 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:
[..]
> > > 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.
>
What I dislike is the fact that we have a huge amount of these unused in
the platform.dtsi, but let's align with what I agreed to on
sc7180...Sorry for the short memory.
>
> > > + 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.
>
Right, but I still would prefer the pinctrl state to be split by
function/pins, rather than pinmux vs pinconf. That way it's natural to
add pinconf properties to the various functional parts (i.e. bias or
drive-strength for the spi pins vs cs).
Do you have any concerns with this?
Regards,
Bjorn
>
> [1] https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2406557/1/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
