| Message ID | 1394614210-15698-7-git-send-email-maxime.coquelin@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/12/2014 09:50 AM, Maxime COQUELIN wrote: > B2120 HDK is the reference board for STiH407 SoC. > It has the following characteristics: > - 1GB DDR3 > - 8GB eMMC / SD-Card slot > - 32MB NOR Flash > - 1 x Gbit Ethernet > - 1 x USB 3.0 port > - 1 x Mini-PCIe > - 1 x SATA > - 1 x HDMI output > - 1 x HDMI input > - 1 x SPDIF > > This patch only introduces basic functionnalities, such as I2C and UART. > > Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com> > Acked-by: Lee Jones <lee.jones@linaro.org> > Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com> > Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com> > --- > arch/arm/boot/dts/Makefile | 3 +- > arch/arm/boot/dts/stih407-b2120.dts | 78 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 80 insertions(+), 1 deletion(-) > create mode 100644 arch/arm/boot/dts/stih407-b2120.dts > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > index 12455cf..f760a88 100644 > --- a/arch/arm/boot/dts/Makefile > +++ b/arch/arm/boot/dts/Makefile > @@ -317,7 +317,8 @@ dtb-$(CONFIG_ARCH_SPEAR6XX)+= spear600-evb.dtb > dtb-$(CONFIG_ARCH_STI)+= stih415-b2000.dtb \ > stih416-b2000.dtb \ > stih415-b2020.dtb \ > - stih416-b2020.dtb > + stih416-b2020.dtb \ > + stih407-b2120.dtb > dtb-$(CONFIG_ARCH_SUNXI) += \ > sun4i-a10-a1000.dtb \ > sun4i-a10-cubieboard.dtb \ > diff --git a/arch/arm/boot/dts/stih407-b2120.dts b/arch/arm/boot/dts/stih407-b2120.dts > new file mode 100644 > index 0000000..9c97da4 > --- /dev/null > +++ b/arch/arm/boot/dts/stih407-b2120.dts > @@ -0,0 +1,78 @@ > +/* > + * Copyright (C) 2014 STMicroelectronics (R&D) Limited. > + * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > +/dts-v1/; > +#include "stih407.dtsi" > +/ { > + model = "STiH407 B2120"; > + compatible = "st,stih407", "st,stih407-b2120"; > + > + chosen { > + bootargs = "console=ttyAS0,115200"; > + linux,stdout-path = &sbc_serial0; > + }; > + > + memory { > + device_type = "memory"; > + reg = <0x40000000 0x80000000>; > + }; > + > + aliases { > + ttyAS0 = &sbc_serial0; > + }; > + > + soc { > + sbc_serial0: serial@9530000 { > + status = "okay"; > + }; > + > + leds { > + compatible = "gpio-leds"; > + red { > + #gpio-cells = <2>; > + label = "Front Panel LED"; > + gpios = <&PIO4 1 0>; > + linux,default-trigger = "heartbeat"; > + }; > + green { > + #gpio-cells = <2>; > + gpios = <&PIO1 3 0>; > + default-state = "off"; > + }; > + }; > + > + i2c@9842000 { > + status = "okay"; > + }; > + > + i2c@9843000 { > + status = "okay"; > + }; > + > + i2c@9844000 { > + status = "okay"; > + }; > + > + i2c@9845000 { > + status = "okay"; > + }; > + > + i2c@9540000 { > + status = "okay"; > + }; > + > + /* SSC11 to HDMI */ > + i2c@9541000 { > + status = "okay"; > + /* HDMI V1.3a supports Standard mode only */ > + clock-frequency = <100000>; > + st,i2c-min-scl-pulse-width-us = <0>; > + st,i2c-min-sda-pulse-width-us = <5>; > + }; > + }; > +}; Acked-by: Patrice Chotard <patrice.chotard@st.com>
Hi, Just a quick drive-by review since I was looking at these patches in the pull request you sent. On Wed, Mar 12, 2014 at 1:50 AM, Maxime COQUELIN <maxime.coquelin@st.com> wrote: > B2120 HDK is the reference board for STiH407 SoC. > It has the following characteristics: > - 1GB DDR3 > - 8GB eMMC / SD-Card slot > - 32MB NOR Flash > - 1 x Gbit Ethernet > - 1 x USB 3.0 port > - 1 x Mini-PCIe > - 1 x SATA > - 1 x HDMI output > - 1 x HDMI input > - 1 x SPDIF > > This patch only introduces basic functionnalities, such as I2C and UART. > > Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com> > Acked-by: Lee Jones <lee.jones@linaro.org> > Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com> > Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com> > --- > arch/arm/boot/dts/Makefile | 3 +- > arch/arm/boot/dts/stih407-b2120.dts | 78 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 80 insertions(+), 1 deletion(-) > create mode 100644 arch/arm/boot/dts/stih407-b2120.dts > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > index 12455cf..f760a88 100644 > --- a/arch/arm/boot/dts/Makefile > +++ b/arch/arm/boot/dts/Makefile > @@ -317,7 +317,8 @@ dtb-$(CONFIG_ARCH_SPEAR6XX)+= spear600-evb.dtb > dtb-$(CONFIG_ARCH_STI)+= stih415-b2000.dtb \ > stih416-b2000.dtb \ > stih415-b2020.dtb \ > - stih416-b2020.dtb > + stih416-b2020.dtb \ > + stih407-b2120.dtb These should be in alphanumerical order. 407 comes before 415. You've been out of order with others as well. > dtb-$(CONFIG_ARCH_SUNXI) += \ > sun4i-a10-a1000.dtb \ > sun4i-a10-cubieboard.dtb \ > diff --git a/arch/arm/boot/dts/stih407-b2120.dts b/arch/arm/boot/dts/stih407-b2120.dts > new file mode 100644 > index 0000000..9c97da4 > --- /dev/null > +++ b/arch/arm/boot/dts/stih407-b2120.dts > @@ -0,0 +1,78 @@ > +/* > + * Copyright (C) 2014 STMicroelectronics (R&D) Limited. > + * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > +/dts-v1/; > +#include "stih407.dtsi" > +/ { > + model = "STiH407 B2120"; > + compatible = "st,stih407", "st,stih407-b2120"; This should go from specific to generic, so the order needs to be the other way. Please check other dts files for the same (I didn't). > + > + chosen { > + bootargs = "console=ttyAS0,115200"; > + linux,stdout-path = &sbc_serial0; > + }; > + > + memory { > + device_type = "memory"; > + reg = <0x40000000 0x80000000>; > + }; > + > + aliases { > + ttyAS0 = &sbc_serial0; > + }; > + > + soc { > + sbc_serial0: serial@9530000 { > + status = "okay"; > + }; You might want to consider reference-based syntax here instead, so you don't have to mimic the hierarchy. That'd be (at the root level of the file, below this secion: &sbc_serial0: { status = "okay"; }; Same for others. -Olof
> > B2120 HDK is the reference board for STiH407 SoC. > > It has the following characteristics: > > - 1GB DDR3 > > - 8GB eMMC / SD-Card slot > > - 32MB NOR Flash > > - 1 x Gbit Ethernet > > - 1 x USB 3.0 port > > - 1 x Mini-PCIe > > - 1 x SATA > > - 1 x HDMI output > > - 1 x HDMI input > > - 1 x SPDIF > > > > This patch only introduces basic functionnalities, such as I2C and UART. > > > > Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com> > > Acked-by: Lee Jones <lee.jones@linaro.org> > > Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com> > > Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com> > > --- > > arch/arm/boot/dts/Makefile | 3 +- > > arch/arm/boot/dts/stih407-b2120.dts | 78 +++++++++++++++++++++++++++++++++++++ > > 2 files changed, 80 insertions(+), 1 deletion(-) > > create mode 100644 arch/arm/boot/dts/stih407-b2120.dts [...] > > +/ { > > + model = "STiH407 B2120"; > > + compatible = "st,stih407", "st,stih407-b2120"; > > This should go from specific to generic, so the order needs to be the other way. I did have a patch-set that changed all of these. Wonder where that went! [...] > > + soc { > > + sbc_serial0: serial@9530000 { > > + status = "okay"; > > + }; > > You might want to consider reference-based syntax here instead, so you > don't have to mimic the hierarchy. That'd be (at the root level of the > file, below this secion: > > &sbc_serial0: { > status = "okay"; > }; I'm personally not keen on this scheme. It's sometimes helpful to know the hierarchy and I don't think it's a large overhead to format the subordinate DTS files in this way. Please consider not enforcing this.
On Tue, May 20, 2014 at 12:20 AM, Lee Jones <lee.jones@linaro.org> wrote: >> > B2120 HDK is the reference board for STiH407 SoC. >> > It has the following characteristics: >> > - 1GB DDR3 >> > - 8GB eMMC / SD-Card slot >> > - 32MB NOR Flash >> > - 1 x Gbit Ethernet >> > - 1 x USB 3.0 port >> > - 1 x Mini-PCIe >> > - 1 x SATA >> > - 1 x HDMI output >> > - 1 x HDMI input >> > - 1 x SPDIF >> > >> > This patch only introduces basic functionnalities, such as I2C and UART. >> > >> > Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com> >> > Acked-by: Lee Jones <lee.jones@linaro.org> >> > Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com> >> > Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com> >> > --- >> > arch/arm/boot/dts/Makefile | 3 +- >> > arch/arm/boot/dts/stih407-b2120.dts | 78 +++++++++++++++++++++++++++++++++++++ >> > 2 files changed, 80 insertions(+), 1 deletion(-) >> > create mode 100644 arch/arm/boot/dts/stih407-b2120.dts > > [...] > >> > +/ { >> > + model = "STiH407 B2120"; >> > + compatible = "st,stih407", "st,stih407-b2120"; >> >> This should go from specific to generic, so the order needs to be the other way. > > I did have a patch-set that changed all of these. Wonder where that went! Cool. > > [...] > >> > + soc { >> > + sbc_serial0: serial@9530000 { >> > + status = "okay"; >> > + }; >> >> You might want to consider reference-based syntax here instead, so you >> don't have to mimic the hierarchy. That'd be (at the root level of the >> file, below this secion: >> >> &sbc_serial0: { >> status = "okay"; >> }; > > I'm personally not keen on this scheme. It's sometimes helpful to know > the hierarchy and I don't think it's a large overhead to format the > subordinate DTS files in this way. > > Please consider not enforcing this. Definitely not enforcing it, and I didn't use to like it either but it has some real upsides. In particular, it saves a lot of grief when you're changing something like the unit-id of a node in .dtsi and forget to do the same update in the dts. -Olof
> >> > + soc { > >> > + sbc_serial0: serial@9530000 { > >> > + status = "okay"; > >> > + }; > >> > >> You might want to consider reference-based syntax here instead, so you > >> don't have to mimic the hierarchy. That'd be (at the root level of the > >> file, below this secion: > >> > >> &sbc_serial0: { > >> status = "okay"; > >> }; > > > > I'm personally not keen on this scheme. It's sometimes helpful to know > > the hierarchy and I don't think it's a large overhead to format the > > subordinate DTS files in this way. > > > > Please consider not enforcing this. > > Definitely not enforcing it, and I didn't use to like it either but it > has some real upsides. > > In particular, it saves a lot of grief when you're changing something > like the unit-id of a node in .dtsi and forget to do the same update > in the dts. I'm not entirely sure what a unit-id is, but I can see that there would be benefits to using the referenced-based syntax as you call it. If any of those benefits hold true here I won't push back, but I would personally like to see us default to the hierarchical scheme.
On 05/20/2014 09:43 AM, Lee Jones wrote: >>>>> + soc { >>>>> + sbc_serial0: serial@9530000 { >>>>> + status = "okay"; >>>>> + }; >>>> >>>> You might want to consider reference-based syntax here instead, so you >>>> don't have to mimic the hierarchy. That'd be (at the root level of the >>>> file, below this secion: >>>> >>>> &sbc_serial0: { >>>> status = "okay"; >>>> }; >>> >>> I'm personally not keen on this scheme. It's sometimes helpful to know >>> the hierarchy and I don't think it's a large overhead to format the >>> subordinate DTS files in this way. >>> >>> Please consider not enforcing this. >> >> Definitely not enforcing it, and I didn't use to like it either but it >> has some real upsides. >> >> In particular, it saves a lot of grief when you're changing something >> like the unit-id of a node in .dtsi and forget to do the same update >> in the dts. > > I'm not entirely sure what a unit-id is, but I can see that there > would be benefits to using the referenced-based syntax as you call > it. If any of those benefits hold true here I won't push back, but I > would personally like to see us default to the hierarchical scheme. +1, I would prefer to keep the hierarchical scheme.
Hi Olof, On 05/20/2014 08:18 AM, Olof Johansson wrote: > Hi, > > Just a quick drive-by review since I was looking at these patches in > the pull request you sent. > > > Thanks for the review. I will send a new series taking your comments into account, except the one about reference-based syntax. [...] >> >> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile >> index 12455cf..f760a88 100644 >> --- a/arch/arm/boot/dts/Makefile >> +++ b/arch/arm/boot/dts/Makefile >> @@ -317,7 +317,8 @@ dtb-$(CONFIG_ARCH_SPEAR6XX)+= spear600-evb.dtb >> dtb-$(CONFIG_ARCH_STI)+= stih415-b2000.dtb \ >> stih416-b2000.dtb \ >> stih415-b2020.dtb \ >> - stih416-b2020.dtb >> + stih416-b2020.dtb \ >> + stih407-b2120.dtb > > These should be in alphanumerical order. 407 comes before 415. You've > been out of order with others as well. Ok, it will be re-ordered in the next series. > >> dtb-$(CONFIG_ARCH_SUNXI) += \ >> sun4i-a10-a1000.dtb \ >> sun4i-a10-cubieboard.dtb \ >> diff --git a/arch/arm/boot/dts/stih407-b2120.dts b/arch/arm/boot/dts/stih407-b2120.dts >> new file mode 100644 >> index 0000000..9c97da4 >> --- /dev/null >> +++ b/arch/arm/boot/dts/stih407-b2120.dts >> @@ -0,0 +1,78 @@ >> +/* >> + * Copyright (C) 2014 STMicroelectronics (R&D) Limited. >> + * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> +/dts-v1/; >> +#include "stih407.dtsi" >> +/ { >> + model = "STiH407 B2120"; >> + compatible = "st,stih407", "st,stih407-b2120"; > > This should go from specific to generic, so the order needs to be the other way. > > Please check other dts files for the same (I didn't). > Ok, I will change that for this dts and also the other ones. Thanks, Maxime
On Tue, May 20, 2014 at 12:43 AM, Lee Jones <lee.jones@linaro.org> wrote: >> >> > + soc { >> >> > + sbc_serial0: serial@9530000 { >> >> > + status = "okay"; >> >> > + }; >> >> >> >> You might want to consider reference-based syntax here instead, so you >> >> don't have to mimic the hierarchy. That'd be (at the root level of the >> >> file, below this secion: >> >> >> >> &sbc_serial0: { >> >> status = "okay"; >> >> }; >> > >> > I'm personally not keen on this scheme. It's sometimes helpful to know >> > the hierarchy and I don't think it's a large overhead to format the >> > subordinate DTS files in this way. >> > >> > Please consider not enforcing this. >> >> Definitely not enforcing it, and I didn't use to like it either but it >> has some real upsides. >> >> In particular, it saves a lot of grief when you're changing something >> like the unit-id of a node in .dtsi and forget to do the same update >> in the dts. > > I'm not entirely sure what a unit-id is, but I can see that there > would be benefits to using the referenced-based syntax as you call > it. If any of those benefits hold true here I won't push back, but I > would personally like to see us default to the hierarchical scheme. Sorry, I meant unit-address. I.e. the portion that goes behind the @ in the node name. -Olof
> >> >> > + soc { > >> >> > + sbc_serial0: serial@9530000 { > >> >> > + status = "okay"; > >> >> > + }; > >> >> > >> >> You might want to consider reference-based syntax here instead, so you > >> >> don't have to mimic the hierarchy. That'd be (at the root level of the > >> >> file, below this secion: > >> >> > >> >> &sbc_serial0: { > >> >> status = "okay"; > >> >> }; > >> > > >> > I'm personally not keen on this scheme. It's sometimes helpful to know > >> > the hierarchy and I don't think it's a large overhead to format the > >> > subordinate DTS files in this way. > >> > > >> > Please consider not enforcing this. > >> > >> Definitely not enforcing it, and I didn't use to like it either but it > >> has some real upsides. > >> > >> In particular, it saves a lot of grief when you're changing something > >> like the unit-id of a node in .dtsi and forget to do the same update > >> in the dts. > > > > I'm not entirely sure what a unit-id is, but I can see that there > > would be benefits to using the referenced-based syntax as you call > > it. If any of those benefits hold true here I won't push back, but I > > would personally like to see us default to the hierarchical scheme. > > Sorry, I meant unit-address. I.e. the portion that goes behind the @ > in the node name. Ah yes, makes sense now, thanks.
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 12455cf..f760a88 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -317,7 +317,8 @@ dtb-$(CONFIG_ARCH_SPEAR6XX)+= spear600-evb.dtb dtb-$(CONFIG_ARCH_STI)+= stih415-b2000.dtb \ stih416-b2000.dtb \ stih415-b2020.dtb \ - stih416-b2020.dtb + stih416-b2020.dtb \ + stih407-b2120.dtb dtb-$(CONFIG_ARCH_SUNXI) += \ sun4i-a10-a1000.dtb \ sun4i-a10-cubieboard.dtb \ diff --git a/arch/arm/boot/dts/stih407-b2120.dts b/arch/arm/boot/dts/stih407-b2120.dts new file mode 100644 index 0000000..9c97da4 --- /dev/null +++ b/arch/arm/boot/dts/stih407-b2120.dts @@ -0,0 +1,78 @@ +/* + * Copyright (C) 2014 STMicroelectronics (R&D) Limited. + * Author: Giuseppe Cavallaro <peppe.cavallaro@st.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +/dts-v1/; +#include "stih407.dtsi" +/ { + model = "STiH407 B2120"; + compatible = "st,stih407", "st,stih407-b2120"; + + chosen { + bootargs = "console=ttyAS0,115200"; + linux,stdout-path = &sbc_serial0; + }; + + memory { + device_type = "memory"; + reg = <0x40000000 0x80000000>; + }; + + aliases { + ttyAS0 = &sbc_serial0; + }; + + soc { + sbc_serial0: serial@9530000 { + status = "okay"; + }; + + leds { + compatible = "gpio-leds"; + red { + #gpio-cells = <2>; + label = "Front Panel LED"; + gpios = <&PIO4 1 0>; + linux,default-trigger = "heartbeat"; + }; + green { + #gpio-cells = <2>; + gpios = <&PIO1 3 0>; + default-state = "off"; + }; + }; + + i2c@9842000 { + status = "okay"; + }; + + i2c@9843000 { + status = "okay"; + }; + + i2c@9844000 { + status = "okay"; + }; + + i2c@9845000 { + status = "okay"; + }; + + i2c@9540000 { + status = "okay"; + }; + + /* SSC11 to HDMI */ + i2c@9541000 { + status = "okay"; + /* HDMI V1.3a supports Standard mode only */ + clock-frequency = <100000>; + st,i2c-min-scl-pulse-width-us = <0>; + st,i2c-min-sda-pulse-width-us = <5>; + }; + }; +};