| Message ID | 20200815163514.11631-2-hauke@hauke-m.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] MIPS: DTS: img: marduk: Add SPI NAND flash | expand |
Hello! On 15.08.2020 19:35, Hauke Mehrtens wrote: > Add Cascoda CA8210 6LoWPAN controller to device tree. > > Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> > --- > arch/mips/boot/dts/img/pistachio_marduk.dts | 22 +++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts b/arch/mips/boot/dts/img/pistachio_marduk.dts > index ea11a21b133b..633a41954cc0 100644 > --- a/arch/mips/boot/dts/img/pistachio_marduk.dts > +++ b/arch/mips/boot/dts/img/pistachio_marduk.dts > @@ -75,6 +75,28 @@ > VDD-supply = <&internal_dac_supply>; > }; > > +&spfi0 { > + status = "okay"; > + pinctrl-0 = <&spim0_pins>, <&spim0_cs0_alt_pin>, <&spim0_cs2_alt_pin>, <&spim0_cs3_alt_pin>, <&spim0_cs4_alt_pin>; > + pinctrl-names = "default"; > + > + cs-gpios = <&gpio1 14 GPIO_ACTIVE_HIGH>, <&gpio0 2 GPIO_ACTIVE_HIGH>, > + <&gpio1 12 GPIO_ACTIVE_HIGH>, <&gpio1 13 GPIO_ACTIVE_HIGH>; > + > + ca8210: ca8210@0 { The device nodes are supposed to have the generic names... > + status = "okay"; > + compatible = "cascoda,ca8210"; > + reg = <0>; > + spi-max-frequency = <4000000>; Only 4MHz? > + spi-cpol; > + reset-gpio = <&gpio0 12 GPIO_ACTIVE_HIGH>; > + irq-gpio = <&gpio2 12 GPIO_ACTIVE_HIGH>; > + extclock-enable; > + extclock-freq = <16000000>; > + extclock-gpio = <2>; Hm, strange spec for a GPIO prop, shouldn't they all be alike? > + }; > +}; > + > &spfi1 { > status = "okay"; > MBR, Sergei
On 8/16/20 11:14 AM, Sergei Shtylyov wrote: > Hello! Thank you for the review Sergei. > On 15.08.2020 19:35, Hauke Mehrtens wrote: > >> Add Cascoda CA8210 6LoWPAN controller to device tree. >> >> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> >> --- >> arch/mips/boot/dts/img/pistachio_marduk.dts | 22 +++++++++++++++++++++ >> 1 file changed, 22 insertions(+) >> >> diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts >> b/arch/mips/boot/dts/img/pistachio_marduk.dts >> index ea11a21b133b..633a41954cc0 100644 >> --- a/arch/mips/boot/dts/img/pistachio_marduk.dts >> +++ b/arch/mips/boot/dts/img/pistachio_marduk.dts >> @@ -75,6 +75,28 @@ >> VDD-supply = <&internal_dac_supply>; >> }; >> +&spfi0 { >> + status = "okay"; >> + pinctrl-0 = <&spim0_pins>, <&spim0_cs0_alt_pin>, >> <&spim0_cs2_alt_pin>, <&spim0_cs3_alt_pin>, <&spim0_cs4_alt_pin>; >> + pinctrl-names = "default"; >> + >> + cs-gpios = <&gpio1 14 GPIO_ACTIVE_HIGH>, <&gpio0 2 >> GPIO_ACTIVE_HIGH>, >> + <&gpio1 12 GPIO_ACTIVE_HIGH>, <&gpio1 13 GPIO_ACTIVE_HIGH>; >> + >> + ca8210: ca8210@0 { > > The device nodes are supposed to have the generic names... I am not referencing this, so I can also remove it: ca8210@0 { Otherwise, would this be ok: ieee802154: ca8210@0 { >> + status = "okay"; This status = "okay"; can be removed. >> + compatible = "cascoda,ca8210"; >> + reg = <0>; >> + spi-max-frequency = <4000000>; > > Only 4MHz? The vendor device tree also use 4MHz: https://github.com/CreatorDev/openwrt/blob/ci40/target/linux/pistachio/dts/pistachio/pistachio_marduk.dts#L34 The device tree binding says "Maximum clock speed, should be *less than* 4000000", it could even make sense to switch this to 3MHz. https://www.kernel.org/doc/Documentation/devicetree/bindings/net/ieee802154/ca8210.txt I do not have the datasheet for the cascoda,ca8210. >> + spi-cpol; >> + reset-gpio = <&gpio0 12 GPIO_ACTIVE_HIGH>; >> + irq-gpio = <&gpio2 12 GPIO_ACTIVE_HIGH>; >> + extclock-enable; >> + extclock-freq = <16000000>; >> + extclock-gpio = <2>; > > Hm, strange spec for a GPIO prop, shouldn't they all be alike? I am also getting this compile warning: arch/mips/boot/dts/img/pistachio_marduk.dts:135.3-23: Warning (gpios_property): /spi@18100f00/ca8210@0:extclock-gpio: cell 0 is not a phandle reference arch/mips/boot/dts/img/pistachio_marduk.dts:126.19-136.4: Warning (gpios_property): /spi@18100f00/ca8210@0: Missing property '#gpio-cells' in node /clk@18144000 or bad phandle (referred from extclock-gpio[0]) I do not know how to fix this. The binding is defined here: https://www.kernel.org/doc/Documentation/devicetree/bindings/net/ieee802154/ca8210.txt Hauke
On 8/16/20 1:59 PM, Hauke Mehrtens wrote: > Thank you for the review Sergei. You're welcome. :-) [...] >>> Add Cascoda CA8210 6LoWPAN controller to device tree. >>> >>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> >>> --- >>> arch/mips/boot/dts/img/pistachio_marduk.dts | 22 +++++++++++++++++++++ >>> 1 file changed, 22 insertions(+) >>> >>> diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts >>> b/arch/mips/boot/dts/img/pistachio_marduk.dts >>> index ea11a21b133b..633a41954cc0 100644 >>> --- a/arch/mips/boot/dts/img/pistachio_marduk.dts >>> +++ b/arch/mips/boot/dts/img/pistachio_marduk.dts >>> @@ -75,6 +75,28 @@ >>> VDD-supply = <&internal_dac_supply>; >>> }; >>> +&spfi0 { >>> + status = "okay"; >>> + pinctrl-0 = <&spim0_pins>, <&spim0_cs0_alt_pin>, >>> <&spim0_cs2_alt_pin>, <&spim0_cs3_alt_pin>, <&spim0_cs4_alt_pin>; >>> + pinctrl-names = "default"; >>> + >>> + cs-gpios = <&gpio1 14 GPIO_ACTIVE_HIGH>, <&gpio0 2 >>> GPIO_ACTIVE_HIGH>, >>> + <&gpio1 12 GPIO_ACTIVE_HIGH>, <&gpio1 13 GPIO_ACTIVE_HIGH>; >>> + >>> + ca8210: ca8210@0 { >> >> The device nodes are supposed to have the generic names... > > I am not referencing this, so I can also remove it: > ca8210@0 { I said name, not label. :-) > Otherwise, would this be ok: > ieee802154: ca8210@0 { > >>> + status = "okay"; > > This status = "okay"; can be removed. Yes, and probably even shpould be removed... >>> + compatible = "cascoda,ca8210"; >>> + reg = <0>; >>> + spi-max-frequency = <4000000>; >> >> Only 4MHz? > > The vendor device tree also use 4MHz: > https://github.com/CreatorDev/openwrt/blob/ci40/target/linux/pistachio/dts/pistachio/pistachio_marduk.dts#L34 > The device tree binding says "Maximum clock speed, should be *less than* > 4000000", it could even make sense to switch this to 3MHz. > https://www.kernel.org/doc/Documentation/devicetree/bindings/net/ieee802154/ca8210.txt It just sounds too low SPI frequency, those a usually one order higher... > I do not have the datasheet for the cascoda,ca8210. > >>> + spi-cpol; >>> + reset-gpio = <&gpio0 12 GPIO_ACTIVE_HIGH>; >>> + irq-gpio = <&gpio2 12 GPIO_ACTIVE_HIGH>; >>> + extclock-enable; >>> + extclock-freq = <16000000>; >>> + extclock-gpio = <2>; >> >> Hm, strange spec for a GPIO prop, shouldn't they all be alike? > > I am also getting this compile warning: > arch/mips/boot/dts/img/pistachio_marduk.dts:135.3-23: Warning > (gpios_property): /spi@18100f00/ca8210@0:extclock-gpio: cell 0 is not a > phandle reference > arch/mips/boot/dts/img/pistachio_marduk.dts:126.19-136.4: Warning > (gpios_property): /spi@18100f00/ca8210@0: Missing property '#gpio-cells' > in node /clk@18144000 or bad phandle (referred from extclock-gpio[0]) Expected. :-) > I do not know how to fix this. > The binding is defined here: > https://www.kernel.org/doc/Documentation/devicetree/bindings/net/ieee802154/ca8210.txt Have to fix the bindings somehow, "extclock-gpio" was a bad name... > Hauke MBR, Sergei
diff --git a/arch/mips/boot/dts/img/pistachio_marduk.dts b/arch/mips/boot/dts/img/pistachio_marduk.dts index ea11a21b133b..633a41954cc0 100644 --- a/arch/mips/boot/dts/img/pistachio_marduk.dts +++ b/arch/mips/boot/dts/img/pistachio_marduk.dts @@ -75,6 +75,28 @@ VDD-supply = <&internal_dac_supply>; }; +&spfi0 { + status = "okay"; + pinctrl-0 = <&spim0_pins>, <&spim0_cs0_alt_pin>, <&spim0_cs2_alt_pin>, <&spim0_cs3_alt_pin>, <&spim0_cs4_alt_pin>; + pinctrl-names = "default"; + + cs-gpios = <&gpio1 14 GPIO_ACTIVE_HIGH>, <&gpio0 2 GPIO_ACTIVE_HIGH>, + <&gpio1 12 GPIO_ACTIVE_HIGH>, <&gpio1 13 GPIO_ACTIVE_HIGH>; + + ca8210: ca8210@0 { + status = "okay"; + compatible = "cascoda,ca8210"; + reg = <0>; + spi-max-frequency = <4000000>; + spi-cpol; + reset-gpio = <&gpio0 12 GPIO_ACTIVE_HIGH>; + irq-gpio = <&gpio2 12 GPIO_ACTIVE_HIGH>; + extclock-enable; + extclock-freq = <16000000>; + extclock-gpio = <2>; + }; +}; + &spfi1 { status = "okay";
Add Cascoda CA8210 6LoWPAN controller to device tree. Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> --- arch/mips/boot/dts/img/pistachio_marduk.dts | 22 +++++++++++++++++++++ 1 file changed, 22 insertions(+)