| Message ID | 20231215204050.2296404-6-cristian.ciocaltea@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable networking support for StarFive JH7100 SoC | expand |
Cristian Ciocaltea wrote: > Add pinmux configuration for DWMAC found on the JH7100 based boards and > enable the related DT node, providing a basic PHY configuration. > > Co-developed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> > Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > .../boot/dts/starfive/jh7100-common.dtsi | 85 +++++++++++++++++++ > 1 file changed, 85 insertions(+) > > diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi > index 42fb61c36068..5cafe8f5c2e7 100644 > --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi > +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi > @@ -72,7 +72,92 @@ wifi_pwrseq: wifi-pwrseq { > }; > }; > > +&gmac { > + pinctrl-names = "default"; > + pinctrl-0 = <&gmac_pins>; > + phy-mode = "rgmii-id"; > + phy-handle = <&phy>; I'm not sure if it's a generic policy or not, but I don't really like adding a reference to a non-existant node here. I'd move this property to the board files where the phy node is actually defined. /Emil
On Sat, Dec 16, 2023 at 11:38:53AM -0800, Emil Renner Berthing wrote: > Cristian Ciocaltea wrote: > > Add pinmux configuration for DWMAC found on the JH7100 based boards and > > enable the related DT node, providing a basic PHY configuration. > > > > Co-developed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> > > Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > > --- > > .../boot/dts/starfive/jh7100-common.dtsi | 85 +++++++++++++++++++ > > 1 file changed, 85 insertions(+) > > > > diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi > > index 42fb61c36068..5cafe8f5c2e7 100644 > > --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi > > +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi > > @@ -72,7 +72,92 @@ wifi_pwrseq: wifi-pwrseq { > > }; > > }; > > > > +&gmac { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&gmac_pins>; > > + phy-mode = "rgmii-id"; > > + phy-handle = <&phy>; > > I'm not sure if it's a generic policy or not, but I don't really like adding a > reference to a non-existant node here. I'd move this property to the board > files where the phy node is actually defined. FWIW, I don't like the reference-in-the-wrong-place thing either.
On 12/16/23 21:38, Emil Renner Berthing wrote: > Cristian Ciocaltea wrote: >> Add pinmux configuration for DWMAC found on the JH7100 based boards and >> enable the related DT node, providing a basic PHY configuration. >> >> Co-developed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> >> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >> --- >> .../boot/dts/starfive/jh7100-common.dtsi | 85 +++++++++++++++++++ >> 1 file changed, 85 insertions(+) >> >> diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi >> index 42fb61c36068..5cafe8f5c2e7 100644 >> --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi >> +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi >> @@ -72,7 +72,92 @@ wifi_pwrseq: wifi-pwrseq { >> }; >> }; >> >> +&gmac { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&gmac_pins>; >> + phy-mode = "rgmii-id"; >> + phy-handle = <&phy>; > > I'm not sure if it's a generic policy or not, but I don't really like adding a > reference to a non-existant node here. I'd move this property to the board > files where the phy node is actually defined. Totally agree, I simply went too far while dropping duplicated code and didn't realize the mistake. Thanks for noticing!
On 12/17/23 23:03, Conor Dooley wrote: > On Sat, Dec 16, 2023 at 11:38:53AM -0800, Emil Renner Berthing wrote: >> Cristian Ciocaltea wrote: >>> Add pinmux configuration for DWMAC found on the JH7100 based boards and >>> enable the related DT node, providing a basic PHY configuration. >>> >>> Co-developed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> >>> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> >>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >>> --- >>> .../boot/dts/starfive/jh7100-common.dtsi | 85 +++++++++++++++++++ >>> 1 file changed, 85 insertions(+) >>> >>> diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi >>> index 42fb61c36068..5cafe8f5c2e7 100644 >>> --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi >>> +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi >>> @@ -72,7 +72,92 @@ wifi_pwrseq: wifi-pwrseq { >>> }; >>> }; >>> >>> +&gmac { >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&gmac_pins>; >>> + phy-mode = "rgmii-id"; >>> + phy-handle = <&phy>; >> >> I'm not sure if it's a generic policy or not, but I don't really like adding a >> reference to a non-existant node here. I'd move this property to the board >> files where the phy node is actually defined. > > FWIW, I don't like the reference-in-the-wrong-place thing either. Yeah, as I already mentioned this was unintentional, will fix in v4. Thanks, Cristian
diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi index 42fb61c36068..5cafe8f5c2e7 100644 --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi @@ -72,7 +72,92 @@ wifi_pwrseq: wifi-pwrseq { }; }; +&gmac { + pinctrl-names = "default"; + pinctrl-0 = <&gmac_pins>; + phy-mode = "rgmii-id"; + phy-handle = <&phy>; + status = "okay"; + + mdio: mdio { + #address-cells = <1>; + #size-cells = <0>; + compatible = "snps,dwmac-mdio"; + }; +}; + &gpio { + gmac_pins: gmac-0 { + gtxclk-pins { + pins = <PAD_FUNC_SHARE(115)>; + bias-pull-up; + drive-strength = <35>; + input-enable; + input-schmitt-enable; + slew-rate = <0>; + }; + miitxclk-pins { + pins = <PAD_FUNC_SHARE(116)>; + bias-pull-up; + drive-strength = <14>; + input-enable; + input-schmitt-disable; + slew-rate = <0>; + }; + tx-pins { + pins = <PAD_FUNC_SHARE(117)>, + <PAD_FUNC_SHARE(119)>, + <PAD_FUNC_SHARE(120)>, + <PAD_FUNC_SHARE(121)>, + <PAD_FUNC_SHARE(122)>, + <PAD_FUNC_SHARE(123)>, + <PAD_FUNC_SHARE(124)>, + <PAD_FUNC_SHARE(125)>, + <PAD_FUNC_SHARE(126)>; + bias-pull-up; + drive-strength = <35>; + input-disable; + input-schmitt-disable; + slew-rate = <0>; + }; + rxclk-pins { + pins = <PAD_FUNC_SHARE(127)>; + bias-pull-up; + drive-strength = <14>; + input-enable; + input-schmitt-disable; + slew-rate = <6>; + }; + rxer-pins { + pins = <PAD_FUNC_SHARE(129)>; + bias-pull-up; + drive-strength = <14>; + input-enable; + input-schmitt-disable; + slew-rate = <0>; + }; + rx-pins { + pins = <PAD_FUNC_SHARE(128)>, + <PAD_FUNC_SHARE(130)>, + <PAD_FUNC_SHARE(131)>, + <PAD_FUNC_SHARE(132)>, + <PAD_FUNC_SHARE(133)>, + <PAD_FUNC_SHARE(134)>, + <PAD_FUNC_SHARE(135)>, + <PAD_FUNC_SHARE(136)>, + <PAD_FUNC_SHARE(137)>, + <PAD_FUNC_SHARE(138)>, + <PAD_FUNC_SHARE(139)>, + <PAD_FUNC_SHARE(140)>, + <PAD_FUNC_SHARE(141)>; + bias-pull-up; + drive-strength = <14>; + input-enable; + input-schmitt-enable; + slew-rate = <0>; + }; + }; + i2c0_pins: i2c0-0 { i2c-pins { pinmux = <GPIOMUX(62, GPO_LOW,