| Message ID | 1544481988-30032-3-git-send-email-ynezz@true.cz |
|---|---|
| State | Changes Requested |
| Delegated to: | Mathias Kresin |
| Headers | show |
| Series | ath79: Add support for Ubiquiti Nanostation M (XW) | expand |
Hey Petr, find my comments inline. Mathias 10/12/2018 23:46, Petr Štetiar: > Most of the M/XW platform settings can be shared so move those common > DTS bits into include file. > > Tested-by: Joe Ayers <ae6xe@arrl.net> > Signed-off-by: Petr Štetiar <ynezz@true.cz> > --- > target/linux/ath79/dts/ar9342_ubnt_bullet-m-xw.dts | 11 +++-------- > target/linux/ath79/dts/ar9342_ubnt_xw.dtsi | 8 ++++++++ > 2 files changed, 11 insertions(+), 8 deletions(-) > > diff --git a/target/linux/ath79/dts/ar9342_ubnt_bullet-m-xw.dts b/target/linux/ath79/dts/ar9342_ubnt_bullet-m-xw.dts > index ad98075..7e4862e 100644 > --- a/target/linux/ath79/dts/ar9342_ubnt_bullet-m-xw.dts > +++ b/target/linux/ath79/dts/ar9342_ubnt_bullet-m-xw.dts > @@ -15,20 +15,19 @@ > status = "okay"; > > phy-mask = <4>; > + > phy4: ethernet-phy@4 { > - phy-mode = "rgmii"; > reg = <4>; > + phy-mode = "rgmii"; > }; > }; Unrelated noop change. Please drop it. > > ð0 { > status = "okay"; > > - pll-data = <0x06000000 0x00000101 0x00001313>; You drop the pll-data without adding them to the dtsi. This looks wrong and doesn't match the "move from dts to dtsi" from the commit message. > - mtd-mac-address = <&eeprom 0x0>; > - > phy-mode = "rgmii"; > phy-handle = <&phy4>; > + mtd-mac-address = <&eeprom 0x0>; > > gmac-config { > device = <&gmac>; > @@ -36,7 +35,3 @@ > rxdv-delay = <3>; > }; > }; > - > -ð1 { > - status = "disabled"; > -}; > diff --git a/target/linux/ath79/dts/ar9342_ubnt_xw.dtsi b/target/linux/ath79/dts/ar9342_ubnt_xw.dtsi > index 742df11..812810f 100644 > --- a/target/linux/ath79/dts/ar9342_ubnt_xw.dtsi > +++ b/target/linux/ath79/dts/ar9342_ubnt_xw.dtsi > @@ -117,3 +117,11 @@ > mtd-cal-data = <&eeprom 0x1000>; > mtd-mac-address = <&eeprom 0x1002>; > }; > + > +&mdio1 { > + status = "disabled"; > +}; This one is new and therefore doesn't match what is described in the commit message. > + > +ð1 { > + status = "disabled"; > +}; >
Mathias Kresin <dev@kresin.me> [2018-12-12 13:27:17]: Hi Mathias, > > phy-mask = <4>; > > + > > phy4: ethernet-phy@4 { > > - phy-mode = "rgmii"; > > reg = <4>; > > + phy-mode = "rgmii"; > > }; > > }; > > Unrelated noop change. Please drop it. Ok. > > ð0 { > > status = "okay"; > > - pll-data = <0x06000000 0x00000101 0x00001313>; > > You drop the pll-data without adding them to the dtsi. This looks wrong and > doesn't match the "move from dts to dtsi" from the commit message. Yes, my bad, sorry. I've removed the pll-data since those defined in ar934x.dtsi work and matches what is in ar71xx. > > +++ b/target/linux/ath79/dts/ar9342_ubnt_xw.dtsi > > @@ -117,3 +117,11 @@ > > mtd-cal-data = <&eeprom 0x1000>; > > mtd-mac-address = <&eeprom 0x1002>; > > }; > > + > > +&mdio1 { > > + status = "disabled"; > > +}; > > This one is new and therefore doesn't match what is described in the commit > message. Ok, I'll make it clear. Thanks for the review! -- ynezz
diff --git a/target/linux/ath79/dts/ar9342_ubnt_bullet-m-xw.dts b/target/linux/ath79/dts/ar9342_ubnt_bullet-m-xw.dts index ad98075..7e4862e 100644 --- a/target/linux/ath79/dts/ar9342_ubnt_bullet-m-xw.dts +++ b/target/linux/ath79/dts/ar9342_ubnt_bullet-m-xw.dts @@ -15,20 +15,19 @@ status = "okay"; phy-mask = <4>; + phy4: ethernet-phy@4 { - phy-mode = "rgmii"; reg = <4>; + phy-mode = "rgmii"; }; }; ð0 { status = "okay"; - pll-data = <0x06000000 0x00000101 0x00001313>; - mtd-mac-address = <&eeprom 0x0>; - phy-mode = "rgmii"; phy-handle = <&phy4>; + mtd-mac-address = <&eeprom 0x0>; gmac-config { device = <&gmac>; @@ -36,7 +35,3 @@ rxdv-delay = <3>; }; }; - -ð1 { - status = "disabled"; -}; diff --git a/target/linux/ath79/dts/ar9342_ubnt_xw.dtsi b/target/linux/ath79/dts/ar9342_ubnt_xw.dtsi index 742df11..812810f 100644 --- a/target/linux/ath79/dts/ar9342_ubnt_xw.dtsi +++ b/target/linux/ath79/dts/ar9342_ubnt_xw.dtsi @@ -117,3 +117,11 @@ mtd-cal-data = <&eeprom 0x1000>; mtd-mac-address = <&eeprom 0x1002>; }; + +&mdio1 { + status = "disabled"; +}; + +ð1 { + status = "disabled"; +};