| Message ID | 20170413133242.5068-7-andrew.smirnov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andrey, On Thu, Apr 13, 2017 at 10:32 AM, Andrey Smirnov <andrew.smirnov@gmail.com> wrote: > diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts > index 5be01a1..e0ff276 100644 > --- a/arch/arm/boot/dts/imx7d-sdb.dts > +++ b/arch/arm/boot/dts/imx7d-sdb.dts > @@ -52,6 +52,30 @@ > reg = <0x80000000 0x80000000>; > }; > > + spi4 { Here you use spi4 label... > + compatible = "spi-gpio"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_spi1>; and here spi1, which is a bit confusing. > + status = "okay"; > + gpio-sck = <&gpio1 13 0>; > + gpio-mosi = <&gpio1 9 0>; > + cs-gpios = <&gpio1 12 0>; You could use GPIO_ACTIVE_HIGH flag for better readability.
On Thu, Apr 13, 2017 at 06:32:40AM -0700, Andrey Smirnov wrote: > Add node for U38, a 74LV595PW serial-in shift register that acts as a > GPIO expander on the board. > > Cc: yurovsky@gmail.com > Cc: Sascha Hauer <kernel@pengutronix.de> > Cc: Fabio Estevam <fabio.estevam@nxp.com> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Russell King <linux@armlinux.org.uk> > Cc: devicetree@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> > --- > arch/arm/boot/dts/imx7d-sdb.dts | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts > index 5be01a1..e0ff276 100644 > --- a/arch/arm/boot/dts/imx7d-sdb.dts > +++ b/arch/arm/boot/dts/imx7d-sdb.dts > @@ -52,6 +52,30 @@ > reg = <0x80000000 0x80000000>; > }; > > + spi4 { > + compatible = "spi-gpio"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_spi1>; > + status = "okay"; The 'status' is not needed in this case. > + gpio-sck = <&gpio1 13 0>; > + gpio-mosi = <&gpio1 9 0>; > + cs-gpios = <&gpio1 12 0>; > + num-chipselects = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + gpio_spi: gpio_spi@0 { gpio-expander might be a better node name? > + compatible = "fairchild,74hc595"; > + gpio-controller; > + #gpio-cells = <2>; > + reg = <0>; > + registers-number = <1>; > + /* Enable PERI_3V3, SENSOR_RST_B and HDMI_RST*/ > + registers-default = /bits/ 8 <0x74>; I do not see this property is documented or supported by kernel. > + spi-max-frequency = <100000>; > + }; > + }; > + > regulators { > compatible = "simple-bus"; > #address-cells = <1>; > @@ -642,5 +666,13 @@ > fsl,pins = < > MX7D_PAD_LPSR_GPIO1_IO01__PWM1_OUT 0x110b0 > >; > + > + pinctrl_spi1: spi1grp { > + fsl,pins = < > + MX7D_PAD_GPIO1_IO09__GPIO1_IO9 0x59 > + MX7D_PAD_GPIO1_IO12__GPIO1_IO12 0x59 > + MX7D_PAD_GPIO1_IO13__GPIO1_IO13 0x59 > + >; > + }; > }; > }; > -- > 2.9.3 >
On Thu, Apr 13, 2017 at 3:20 PM, Fabio Estevam <festevam@gmail.com> wrote: > Hi Andrey, > > On Thu, Apr 13, 2017 at 10:32 AM, Andrey Smirnov > <andrew.smirnov@gmail.com> wrote: > >> diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts >> index 5be01a1..e0ff276 100644 >> --- a/arch/arm/boot/dts/imx7d-sdb.dts >> +++ b/arch/arm/boot/dts/imx7d-sdb.dts >> @@ -52,6 +52,30 @@ >> reg = <0x80000000 0x80000000>; >> }; >> >> + spi4 { > > Here you use spi4 label... > >> + compatible = "spi-gpio"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_spi1>; > > and here spi1, which is a bit confusing. > Agreed. It is confusing, will fix in v2. >> + status = "okay"; >> + gpio-sck = <&gpio1 13 0>; >> + gpio-mosi = <&gpio1 9 0>; >> + cs-gpios = <&gpio1 12 0>; > > You could use GPIO_ACTIVE_HIGH flag for better readability. Missed that, will fix in v2. Thanks, Andrey Smirnov
On Thu, Apr 13, 2017 at 8:47 PM, Shawn Guo <shawnguo@kernel.org> wrote: > On Thu, Apr 13, 2017 at 06:32:40AM -0700, Andrey Smirnov wrote: >> Add node for U38, a 74LV595PW serial-in shift register that acts as a >> GPIO expander on the board. >> >> Cc: yurovsky@gmail.com >> Cc: Sascha Hauer <kernel@pengutronix.de> >> Cc: Fabio Estevam <fabio.estevam@nxp.com> >> Cc: Rob Herring <robh+dt@kernel.org> >> Cc: Mark Rutland <mark.rutland@arm.com> >> Cc: Russell King <linux@armlinux.org.uk> >> Cc: devicetree@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> Cc: linux-arm-kernel@lists.infradead.org >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> >> --- >> arch/arm/boot/dts/imx7d-sdb.dts | 32 ++++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts >> index 5be01a1..e0ff276 100644 >> --- a/arch/arm/boot/dts/imx7d-sdb.dts >> +++ b/arch/arm/boot/dts/imx7d-sdb.dts >> @@ -52,6 +52,30 @@ >> reg = <0x80000000 0x80000000>; >> }; >> >> + spi4 { >> + compatible = "spi-gpio"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_spi1>; >> + status = "okay"; > > The 'status' is not needed in this case. > Missed that, will fix in v2. >> + gpio-sck = <&gpio1 13 0>; >> + gpio-mosi = <&gpio1 9 0>; >> + cs-gpios = <&gpio1 12 0>; >> + num-chipselects = <1>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + gpio_spi: gpio_spi@0 { > > gpio-expander might be a better node name? > Yeah, I agree. I'll change it to extended_io: gpio-expander@0 ("Extended IO" is how this part is called out on the schematic) >> + compatible = "fairchild,74hc595"; >> + gpio-controller; >> + #gpio-cells = <2>; >> + reg = <0>; >> + registers-number = <1>; >> + /* Enable PERI_3V3, SENSOR_RST_B and HDMI_RST*/ >> + registers-default = /bits/ 8 <0x74>; > > I do not see this property is documented or supported by kernel. My bad, some downstream properties leakage. Will remove in v2. Thanks, Andrey Smirnov
On Fri, Apr 14, 2017 at 11:47:00AM +0800, Shawn Guo wrote: > On Thu, Apr 13, 2017 at 06:32:40AM -0700, Andrey Smirnov wrote: > > Add node for U38, a 74LV595PW serial-in shift register that acts as a > > GPIO expander on the board. > > > > Cc: yurovsky@gmail.com > > Cc: Sascha Hauer <kernel@pengutronix.de> > > Cc: Fabio Estevam <fabio.estevam@nxp.com> > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: Mark Rutland <mark.rutland@arm.com> > > Cc: Russell King <linux@armlinux.org.uk> > > Cc: devicetree@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > Cc: linux-arm-kernel@lists.infradead.org > > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> > > --- > > arch/arm/boot/dts/imx7d-sdb.dts | 32 ++++++++++++++++++++++++++++++++ > > 1 file changed, 32 insertions(+) > > > > diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts > > index 5be01a1..e0ff276 100644 > > --- a/arch/arm/boot/dts/imx7d-sdb.dts > > +++ b/arch/arm/boot/dts/imx7d-sdb.dts > > @@ -52,6 +52,30 @@ > > reg = <0x80000000 0x80000000>; > > }; > > > > + spi4 { > > + compatible = "spi-gpio"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_spi1>; > > + status = "okay"; > > The 'status' is not needed in this case. > > > + gpio-sck = <&gpio1 13 0>; > > + gpio-mosi = <&gpio1 9 0>; > > + cs-gpios = <&gpio1 12 0>; > > + num-chipselects = <1>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + gpio_spi: gpio_spi@0 { > > gpio-expander might be a better node name? > > > + compatible = "fairchild,74hc595"; > > + gpio-controller; > > + #gpio-cells = <2>; > > + reg = <0>; > > + registers-number = <1>; > > + /* Enable PERI_3V3, SENSOR_RST_B and HDMI_RST*/ > > + registers-default = /bits/ 8 <0x74>; > > I do not see this property is documented or supported by kernel. It's FSL internal invented property to do some trick on register intialization and should be dropped. Regards Dong Aisheng
diff --git a/arch/arm/boot/dts/imx7d-sdb.dts b/arch/arm/boot/dts/imx7d-sdb.dts index 5be01a1..e0ff276 100644 --- a/arch/arm/boot/dts/imx7d-sdb.dts +++ b/arch/arm/boot/dts/imx7d-sdb.dts @@ -52,6 +52,30 @@ reg = <0x80000000 0x80000000>; }; + spi4 { + compatible = "spi-gpio"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_spi1>; + status = "okay"; + gpio-sck = <&gpio1 13 0>; + gpio-mosi = <&gpio1 9 0>; + cs-gpios = <&gpio1 12 0>; + num-chipselects = <1>; + #address-cells = <1>; + #size-cells = <0>; + + gpio_spi: gpio_spi@0 { + compatible = "fairchild,74hc595"; + gpio-controller; + #gpio-cells = <2>; + reg = <0>; + registers-number = <1>; + /* Enable PERI_3V3, SENSOR_RST_B and HDMI_RST*/ + registers-default = /bits/ 8 <0x74>; + spi-max-frequency = <100000>; + }; + }; + regulators { compatible = "simple-bus"; #address-cells = <1>; @@ -642,5 +666,13 @@ fsl,pins = < MX7D_PAD_LPSR_GPIO1_IO01__PWM1_OUT 0x110b0 >; + + pinctrl_spi1: spi1grp { + fsl,pins = < + MX7D_PAD_GPIO1_IO09__GPIO1_IO9 0x59 + MX7D_PAD_GPIO1_IO12__GPIO1_IO12 0x59 + MX7D_PAD_GPIO1_IO13__GPIO1_IO13 0x59 + >; + }; }; };
Add node for U38, a 74LV595PW serial-in shift register that acts as a GPIO expander on the board. Cc: yurovsky@gmail.com Cc: Sascha Hauer <kernel@pengutronix.de> Cc: Fabio Estevam <fabio.estevam@nxp.com> Cc: Rob Herring <robh+dt@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Russell King <linux@armlinux.org.uk> Cc: devicetree@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> --- arch/arm/boot/dts/imx7d-sdb.dts | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)