| Message ID | 20230207115617.12088-3-pshete@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] dt-bindings: pinctrl: tegra234: Add DT binding doc | expand |
On 07/02/2023 12:56, Prathamesh Shete wrote: > This change adds pinmux node for Tegra234. > > Signed-off-by: Prathamesh Shete <pshete@nvidia.com> > --- > arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi > index eaf05ee9acd1..c91b88bc56d1 100644 > --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi > +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi > @@ -701,6 +701,13 @@ > interrupt-controller; > #gpio-cells = <2>; > gpio-controller; > + gpio-ranges = <&pinmux 0 0 164>; > + }; > + > + pinmux: pinmux@2430000 { > + compatible = "nvidia,tegra234-pinmux"; > + reg = <0x2430000 0x19100>; > + status = "okay"; Why? Anything disabled it? > }; > > mc: memory-controller@2c00000 { > @@ -1664,6 +1671,13 @@ > interrupt-controller; > #gpio-cells = <2>; > gpio-controller; > + gpio-range = <&pinmux_aon 0 0 32>; > + }; > + > + pinmux_aon: pinmux@c300000 { > + compatible = "nvidia,tegra234-pinmux-aon"; > + reg = <0xc300000 0x4000>; > + status = "okay"; Also why? > }; > > pwm4: pwm@c340000 { Best regards, Krzysztof
On Tue, Feb 07, 2023 at 04:33:42PM +0100, Krzysztof Kozlowski wrote: > On 07/02/2023 12:56, Prathamesh Shete wrote: > > This change adds pinmux node for Tegra234. > > > > Signed-off-by: Prathamesh Shete <pshete@nvidia.com> > > --- > > arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi > > index eaf05ee9acd1..c91b88bc56d1 100644 > > --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi > > +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi > > @@ -701,6 +701,13 @@ > > interrupt-controller; > > #gpio-cells = <2>; > > gpio-controller; > > + gpio-ranges = <&pinmux 0 0 164>; > > + }; > > + > > + pinmux: pinmux@2430000 { > > + compatible = "nvidia,tegra234-pinmux"; > > + reg = <0x2430000 0x19100>; > > + status = "okay"; > > Why? Anything disabled it? > > > }; > > > > mc: memory-controller@2c00000 { > > @@ -1664,6 +1671,13 @@ > > interrupt-controller; > > #gpio-cells = <2>; > > gpio-controller; > > + gpio-range = <&pinmux_aon 0 0 32>; > > + }; > > + > > + pinmux_aon: pinmux@c300000 { > > + compatible = "nvidia,tegra234-pinmux-aon"; > > + reg = <0xc300000 0x4000>; > > + status = "okay"; > > Also why? These are probably copy-pasted from Tegra194 where these snuck in. I can drop those when applying. I'll also prepare a patch to drop these from the tegra194.dtsi. I wonder if there's a good way to detect these. We'd have to run checks on the DT source files, so that's a bit difficult. I do have an experimental script that tries to capture some common pitfalls on sources but it's quite ugly and slow, but I guess I could add something like this. But perhaps there are better ways? Thierry
On 08/02/2023 12:00, Thierry Reding wrote: > On Tue, Feb 07, 2023 at 04:33:42PM +0100, Krzysztof Kozlowski wrote: >> On 07/02/2023 12:56, Prathamesh Shete wrote: >>> This change adds pinmux node for Tegra234. >>> >>> Signed-off-by: Prathamesh Shete <pshete@nvidia.com> >>> --- >>> arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi >>> index eaf05ee9acd1..c91b88bc56d1 100644 >>> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi >>> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi >>> @@ -701,6 +701,13 @@ >>> interrupt-controller; >>> #gpio-cells = <2>; >>> gpio-controller; >>> + gpio-ranges = <&pinmux 0 0 164>; >>> + }; >>> + >>> + pinmux: pinmux@2430000 { >>> + compatible = "nvidia,tegra234-pinmux"; >>> + reg = <0x2430000 0x19100>; >>> + status = "okay"; >> >> Why? Anything disabled it? >> >>> }; >>> >>> mc: memory-controller@2c00000 { >>> @@ -1664,6 +1671,13 @@ >>> interrupt-controller; >>> #gpio-cells = <2>; >>> gpio-controller; >>> + gpio-range = <&pinmux_aon 0 0 32>; >>> + }; >>> + >>> + pinmux_aon: pinmux@c300000 { >>> + compatible = "nvidia,tegra234-pinmux-aon"; >>> + reg = <0xc300000 0x4000>; >>> + status = "okay"; >> >> Also why? > > These are probably copy-pasted from Tegra194 where these snuck in. I can > drop those when applying. I'll also prepare a patch to drop these from > the tegra194.dtsi. > > I wonder if there's a good way to detect these. We'd have to run checks > on the DT source files, so that's a bit difficult. I do have an > experimental script that tries to capture some common pitfalls on > sources but it's quite ugly and slow, but I guess I could add something > like this. But perhaps there are better ways? One way to easy spot them is to override always by label, thus every node defined like above is a new node. However I think we talked about this and you do not follow this practice, thus there is no way to tell - is the status reasonable or not. Automated tools could help here as well - run fdtdump on DTB and look for status=okay. Best regards, Krzysztof
On 08/02/2023 13:01, Krzysztof Kozlowski wrote: >> I wonder if there's a good way to detect these. We'd have to run checks >> on the DT source files, so that's a bit difficult. I do have an >> experimental script that tries to capture some common pitfalls on >> sources but it's quite ugly and slow, but I guess I could add something >> like this. But perhaps there are better ways? > > One way to easy spot them is to override always by label, thus every > node defined like above is a new node. However I think we talked about > this and you do not follow this practice, thus there is no way to tell - > is the status reasonable or not. > > Automated tools could help here as well - run fdtdump on DTB and look > for status=okay. Eh, obviously it won't work - every node which was disabled in DTSI and enabled in DTS will have the status=okay... Best regards, Krzysztof
On Wed, Feb 08, 2023 at 01:06:02PM +0100, Krzysztof Kozlowski wrote: > On 08/02/2023 13:01, Krzysztof Kozlowski wrote: > >> I wonder if there's a good way to detect these. We'd have to run checks > >> on the DT source files, so that's a bit difficult. I do have an > >> experimental script that tries to capture some common pitfalls on > >> sources but it's quite ugly and slow, but I guess I could add something > >> like this. But perhaps there are better ways? > > > > One way to easy spot them is to override always by label, thus every > > node defined like above is a new node. However I think we talked about > > this and you do not follow this practice, thus there is no way to tell - > > is the status reasonable or not. > > > > Automated tools could help here as well - run fdtdump on DTB and look > > for status=okay. > > Eh, obviously it won't work - every node which was disabled in DTSI and > enabled in DTS will have the status=okay... Yeah, I was originally thinking along the same lines, but when things are overridden that check no longer works. I suppose DTC could be taught to check for this when it merges nodes. Thierry
diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi index eaf05ee9acd1..c91b88bc56d1 100644 --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi @@ -701,6 +701,13 @@ interrupt-controller; #gpio-cells = <2>; gpio-controller; + gpio-ranges = <&pinmux 0 0 164>; + }; + + pinmux: pinmux@2430000 { + compatible = "nvidia,tegra234-pinmux"; + reg = <0x2430000 0x19100>; + status = "okay"; }; mc: memory-controller@2c00000 { @@ -1664,6 +1671,13 @@ interrupt-controller; #gpio-cells = <2>; gpio-controller; + gpio-range = <&pinmux_aon 0 0 32>; + }; + + pinmux_aon: pinmux@c300000 { + compatible = "nvidia,tegra234-pinmux-aon"; + reg = <0xc300000 0x4000>; + status = "okay"; }; pwm4: pwm@c340000 {
This change adds pinmux node for Tegra234. Signed-off-by: Prathamesh Shete <pshete@nvidia.com> --- arch/arm64/boot/dts/nvidia/tegra234.dtsi | 14 ++++++++++++++ 1 file changed, 14 insertions(+)