Message ID | 20211127142327.17692-1-digetx@gmail.com |
---|---|
Headers | show |
Series | NVIDIA Tegra ARM32 device-tree patches for 5.17 (new devices and more) | expand |
On Sat, Nov 27, 2021 at 05:23:16PM +0300, Dmitry Osipenko wrote: > From: Anton Bambura <jenneron@protonmail.com> > > Add more labels in order to use label reference in device-specific > dts files. Labels make device-trees more readable and prevent typos > that are difficult to notice. I believe that you had also a similar patch for Tegra3 (or my [1]). Could you pull it into this series to make the DT's easier to manage? [1] https://rere.qmqm.pl/git/?p=linux;a=commitdiff;h=2c3341163b07d94d209680a22286608d0e4d711a Best Regards Michał Mirosław
On Sat, Nov 27, 2021 at 05:23:18PM +0300, Dmitry Osipenko wrote: > From: Svyatoslav Ryhel <clamor95@gmail.com> > > Add device-tree for ASUS Transformer Prime TF201, which is NVIDIA > Tegra30-based tablet device. > > Co-developed-by: Ion Agorria <ion@agorria.com> > Signed-off-by: Ion Agorria <ion@agorria.com> > Co-developed-by: Maxim Schwalm <maxim.schwalm@gmail.com> > Signed-off-by: Maxim Schwalm <maxim.schwalm@gmail.com> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com> > --- > arch/arm/boot/dts/Makefile | 1 + > .../boot/dts/tegra30-asus-lvds-display.dtsi | 46 + > arch/arm/boot/dts/tegra30-asus-tf201.dts | 623 ++++++ > .../dts/tegra30-asus-transformer-common.dtsi | 1741 +++++++++++++++++ > 4 files changed, 2411 insertions(+) > create mode 100644 arch/arm/boot/dts/tegra30-asus-lvds-display.dtsi > create mode 100644 arch/arm/boot/dts/tegra30-asus-tf201.dts > create mode 100644 arch/arm/boot/dts/tegra30-asus-transformer-common.dtsi I think the common.dtsi is combined from all the DT authors, so they all should be mentioned in the co-developed/signed-off lines. (Or maybe just split the common part out if you want to be more specific in assigning credits?) > + i2c@7000c400 { > + /* Atmel MXT768E touchscreen */ > + touchscreen@4d { [...] > + interrupt-parent = <&gpio>; > + interrupts = <TEGRA_GPIO(H, 4) IRQ_TYPE_EDGE_FALLING>; [...] Nit: This could use 'interrupts-extended' as it's more readable, I think. (There are similar cases in other DTs.) > + uartb: serial@70006040 { > + compatible = "nvidia,tegra30-hsuart"; [...] I wonder why isn't this in the tegra30.dtsi? The DT shouldn't select a driver - it should only describe the hardware. Best Regards Michał Mirosław
28.11.2021 04:32, Michał Mirosław пишет: > On Sat, Nov 27, 2021 at 05:23:16PM +0300, Dmitry Osipenko wrote: >> From: Anton Bambura <jenneron@protonmail.com> >> >> Add more labels in order to use label reference in device-specific >> dts files. Labels make device-trees more readable and prevent typos >> that are difficult to notice. > > I believe that you had also a similar patch for Tegra3 (or my [1]). > Could you pull it into this series to make the DT's easier to manage? > > [1] https://rere.qmqm.pl/git/?p=linux;a=commitdiff;h=2c3341163b07d94d209680a22286608d0e4d711a I had a thought about including it in to this series, but we don't have a user for that patch right now, so I decided not to include it. I'll reconsider to include it into v4, now that you're asking about it. It won't hurt to have that patch, but I really would want somebody to add a similar patch for T20 and then convert all DTs to use those labels uniformly, this will take some time and effort to type these patches.
28.11.2021 04:51, Michał Mirosław пишет: > On Sat, Nov 27, 2021 at 05:23:18PM +0300, Dmitry Osipenko wrote: >> From: Svyatoslav Ryhel <clamor95@gmail.com> >> >> Add device-tree for ASUS Transformer Prime TF201, which is NVIDIA >> Tegra30-based tablet device. >> >> Co-developed-by: Ion Agorria <ion@agorria.com> >> Signed-off-by: Ion Agorria <ion@agorria.com> >> Co-developed-by: Maxim Schwalm <maxim.schwalm@gmail.com> >> Signed-off-by: Maxim Schwalm <maxim.schwalm@gmail.com> >> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com> >> --- >> arch/arm/boot/dts/Makefile | 1 + >> .../boot/dts/tegra30-asus-lvds-display.dtsi | 46 + >> arch/arm/boot/dts/tegra30-asus-tf201.dts | 623 ++++++ >> .../dts/tegra30-asus-transformer-common.dtsi | 1741 +++++++++++++++++ >> 4 files changed, 2411 insertions(+) >> create mode 100644 arch/arm/boot/dts/tegra30-asus-lvds-display.dtsi >> create mode 100644 arch/arm/boot/dts/tegra30-asus-tf201.dts >> create mode 100644 arch/arm/boot/dts/tegra30-asus-transformer-common.dtsi > > I think the common.dtsi is combined from all the DT authors, so they all > should be mentioned in the co-developed/signed-off lines. (Or maybe just > split the common part out if you want to be more specific in assigning > credits?) I'll factor out the common part into separate patch and will make you the author of that patch since it really was derived from yours original TF300T DT. I actually wanted to do that previously, it's good that you reminded about it. >> + i2c@7000c400 { >> + /* Atmel MXT768E touchscreen */ >> + touchscreen@4d { > [...] >> + interrupt-parent = <&gpio>; >> + interrupts = <TEGRA_GPIO(H, 4) IRQ_TYPE_EDGE_FALLING>; > [...] > > Nit: This could use 'interrupts-extended' as it's more readable, I think. > (There are similar cases in other DTs.) I don't have objections, although the classic variant is also good to me. I don't think that it makes sense to change only the Transformer DTs to use 'interrupts-extended'. I'd suggest to create a followup patches that will cleanup and improve all Tegra device-trees together, making them all more uniform and nicer to look. >> + uartb: serial@70006040 { >> + compatible = "nvidia,tegra30-hsuart"; > [...] > > I wonder why isn't this in the tegra30.dtsi? The DT shouldn't select a > driver - it should only describe the hardware. The "nvidia,tegra30-hsuart" implies that UART port is capable of performing the high speed transfers, which should be board dependent. Meanwhile "nvidia,tegra30-uart" should work for all hardware variants. That's how I understand it. The comment in the Tegra dtsi really shouldn't refer to the drivers. Changing that comment should be material for a separate patch. It could become a part of the DT cleanup patches together with the 'interrupts-extended' changes.