| Message ID | TYZPR01MB5556FA040B07F48AFE544680C9762@TYZPR01MB5556.apcprd01.prod.exchangelabs.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ipq5018: enable ethernet support | expand |
On Sun, Jan 21, 2024 at 08:42:36PM +0800, Ziyang Huang wrote: > Signed-off-by: Ziyang Huang <hzyitc@outlook.com> > --- > arch/arm64/boot/dts/qcom/ipq5018.dtsi | 120 +++++++++++++++++++++++++- > 1 file changed, 116 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi > index e502a3ecf4b7..b36e5c2136b7 100644 > --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi > +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi > @@ -94,6 +94,63 @@ soc: soc@0 { > #size-cells = <1>; > ranges = <0 0 0 0xffffffff>; > > + mdio0: mdio@88000 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "qcom,ipq5018-mdio", "qcom,qca-mdio"; > + reg = <0x88000 0x64>; > + resets = <&gcc GCC_GEPHY_MDC_SW_ARES>, > + <&gcc GCC_GEPHY_DSP_HW_ARES>; What do these two resets do? An MDIO bus controller is unlikely to have a DSP in it. That is something a PHY is more likely to have. An MDIO bus controller does have an MDC line, but why is it GCC_GEPHY_MDC_SW_ARES not GCC_MDIO_MDC_SW_ARES? So this again makes me think this is a PHY reset, so should be in the PHY node. A device tree binding will help sort this out. Andrew --- pw-bot: cr
在 2024/1/22 0:45, Andrew Lunn 写道: > On Sun, Jan 21, 2024 at 08:42:36PM +0800, Ziyang Huang wrote: >> Signed-off-by: Ziyang Huang <hzyitc@outlook.com> >> --- >> arch/arm64/boot/dts/qcom/ipq5018.dtsi | 120 +++++++++++++++++++++++++- >> 1 file changed, 116 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi >> index e502a3ecf4b7..b36e5c2136b7 100644 >> --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi >> +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi >> @@ -94,6 +94,63 @@ soc: soc@0 { >> #size-cells = <1>; >> ranges = <0 0 0 0xffffffff>; >> >> + mdio0: mdio@88000 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + compatible = "qcom,ipq5018-mdio", "qcom,qca-mdio"; >> + reg = <0x88000 0x64>; >> + resets = <&gcc GCC_GEPHY_MDC_SW_ARES>, >> + <&gcc GCC_GEPHY_DSP_HW_ARES>; > > What do these two resets do? An MDIO bus controller is unlikely to > have a DSP in it. That is something a PHY is more likely to have. An > MDIO bus controller does have an MDC line, but why is it > GCC_GEPHY_MDC_SW_ARES not GCC_MDIO_MDC_SW_ARES? So this again makes me > think this is a PHY reset, so should be in the PHY node. > IPQ5018 has two mdio bus. mdio0 is an internal bus which only connects to internal PHY while mdio1 is outgoing and can be used to connect PHY or switch. So I thought GCC_GEPHY_MDC_SW_ARES is for mdio0 and GCC_MDIO_MDC_SW_ARES is for mdio1. GCC_GEPHY_DSP_HW_ARES is seem like PHY reset. But if we don't deassert it, the phy node can't be scaned. So I add it here like what reset-gpios do - to reset PHY or switch. > A device tree binding will help sort this out. Base on the functions I explained, I can't write the accurate names and number of resets here. > > > Andrew > > --- > pw-bot: cr
On Mon, Jan 22, 2024 at 11:52:30PM +0800, Ziyang Huang wrote: > 在 2024/1/22 0:45, Andrew Lunn 写道: > > On Sun, Jan 21, 2024 at 08:42:36PM +0800, Ziyang Huang wrote: > > > Signed-off-by: Ziyang Huang <hzyitc@outlook.com> > > > --- > > > arch/arm64/boot/dts/qcom/ipq5018.dtsi | 120 +++++++++++++++++++++++++- > > > 1 file changed, 116 insertions(+), 4 deletions(-) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi > > > index e502a3ecf4b7..b36e5c2136b7 100644 > > > --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi > > > @@ -94,6 +94,63 @@ soc: soc@0 { > > > #size-cells = <1>; > > > ranges = <0 0 0 0xffffffff>; > > > + mdio0: mdio@88000 { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + compatible = "qcom,ipq5018-mdio", "qcom,qca-mdio"; > > > + reg = <0x88000 0x64>; > > > + resets = <&gcc GCC_GEPHY_MDC_SW_ARES>, > > > + <&gcc GCC_GEPHY_DSP_HW_ARES>; > > > > What do these two resets do? An MDIO bus controller is unlikely to > > have a DSP in it. That is something a PHY is more likely to have. An > > MDIO bus controller does have an MDC line, but why is it > > GCC_GEPHY_MDC_SW_ARES not GCC_MDIO_MDC_SW_ARES? So this again makes me > > think this is a PHY reset, so should be in the PHY node. > > > > IPQ5018 has two mdio bus. mdio0 is an internal bus which only connects to > internal PHY while mdio1 is outgoing and can be used to connect PHY or > switch. > > So I thought GCC_GEPHY_MDC_SW_ARES is for mdio0 and GCC_MDIO_MDC_SW_ARES is > for mdio1. > > GCC_GEPHY_DSP_HW_ARES is seem like PHY reset. But if we don't deassert it, > the phy node can't be scaned. So I add it here like what reset-gpios do - to > reset PHY or switch. > > > A device tree binding will help sort this out. > > Base on the functions I explained, I can't write the accurate names and > number of resets here. Without a device tree binding, your patch will not be accepted. At minimum, you need to describe your understanding of the hardware, what you think the resets do. Since this is an internal PHY, you know exactly what it is. So you can use ID values in the compatible so the driver loads. The driver can then deal with clocks and resets. With complex setups like this, scanning is not always possible. Andrew
diff --git a/arch/arm64/boot/dts/qcom/ipq5018.dtsi b/arch/arm64/boot/dts/qcom/ipq5018.dtsi index e502a3ecf4b7..b36e5c2136b7 100644 --- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi +++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi @@ -94,6 +94,63 @@ soc: soc@0 { #size-cells = <1>; ranges = <0 0 0 0xffffffff>; + mdio0: mdio@88000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "qcom,ipq5018-mdio", "qcom,qca-mdio"; + reg = <0x88000 0x64>; + resets = <&gcc GCC_GEPHY_MDC_SW_ARES>, + <&gcc GCC_GEPHY_DSP_HW_ARES>; + clocks = <&gcc GCC_MDIO0_AHB_CLK>; + clock-names = "gcc_mdio_ahb_clk"; + status = "disabled"; + + gephy: ethernet-phy@7 { + #clock-cells = <1>; + reg = <7>; + resets = <&gcc GCC_GEPHY_BCR>, + <&gcc GCC_GEPHY_RX_ARES>, + <&gcc GCC_GEPHY_TX_ARES>; + clocks = <&gcc GCC_GEPHY_RX_CLK>, + <&gcc GCC_GEPHY_TX_CLK>; + }; + }; + + mdio1: mdio@90000 { + #address-cells = <1>; + #size-cells = <0>; + compatible = "qcom,ipq5018-mdio"; + reg = <0x90000 0x64>; + clocks = <&gcc GCC_MDIO1_AHB_CLK>; + clock-names = "gcc_mdio_ahb_clk"; + status = "disabled"; + }; + + uniphy0: eth-uniphy@98000 { + compatible = "qcom,ipq5018-eth-uniphy"; + #clock-cells = <1>; + #phy-cells = <0>; + reg = <0x98000 0x800>, + <0x9b000 0x800>, + <0x19475c4 0x4>; + reg-names = "uniphy", + "cmn", + "tcsr"; + clocks = <&gcc GCC_CMN_BLK_AHB_CLK>, + <&gcc GCC_CMN_BLK_SYS_CLK>, + <&gcc GCC_UNIPHY_AHB_CLK>, + <&gcc GCC_UNIPHY_SYS_CLK>, + <&gcc GCC_UNIPHY_RX_CLK>, + <&gcc GCC_UNIPHY_TX_CLK>; + resets = <&gcc GCC_UNIPHY_BCR>, + <&gcc GCC_UNIPHY_AHB_ARES>, + <&gcc GCC_UNIPHY_SYS_ARES>, + <&gcc GCC_UNIPHY_RX_ARES>, + <&gcc GCC_UNIPHY_TX_ARES>; + + status = "disabled"; + }; + tlmm: pinctrl@1000000 { compatible = "qcom,ipq5018-tlmm"; reg = <0x01000000 0x300000>; @@ -120,10 +177,10 @@ gcc: clock-controller@1800000 { <0>, <0>, <0>, - <0>, - <0>, - <0>, - <0>; + <&gephy 0>, + <&gephy 1>, + <&uniphy0 0>, + <&uniphy0 1>; #clock-cells = <1>; #reset-cells = <1>; #power-domain-cells = <1>; @@ -244,6 +301,61 @@ frame@b128000 { status = "disabled"; }; }; + + gmac0: ethernet@39c00000 { + compatible = "qcom,ipq50xx-gmac", "snps,dwmac"; + reg = <0x39C00000 0x10000>; + clocks = <&gcc GCC_GMAC0_SYS_CLK>, + <&gcc GCC_GMAC0_CFG_CLK>, + <&gcc GCC_SNOC_GMAC0_AHB_CLK>, + <&gcc GCC_SNOC_GMAC0_AXI_CLK>, + <&gcc GCC_GMAC0_RX_CLK>, + <&gcc GCC_GMAC0_TX_CLK>, + <&gcc GCC_GMAC0_PTP_CLK>; + clock-names = "sys", + "cfg", + "ahb", + "axi", + "rx", + "tx", + "ptp"; + resets = <&gcc GCC_GMAC0_BCR>; + interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "macirq"; + + phy-handle = <&gephy>; + phy-mode = "sgmii"; + + status = "disabled"; + }; + + gmac1: ethernet@39d00000 { + compatible = "qcom,ipq50xx-gmac", "snps,dwmac"; + reg = <0x39D00000 0x10000>; + reg-names = "stmmaceth"; + clocks = <&gcc GCC_GMAC1_SYS_CLK>, + <&gcc GCC_GMAC1_CFG_CLK>, + <&gcc GCC_SNOC_GMAC1_AHB_CLK>, + <&gcc GCC_SNOC_GMAC1_AXI_CLK>, + <&gcc GCC_GMAC1_RX_CLK>, + <&gcc GCC_GMAC1_TX_CLK>, + <&gcc GCC_GMAC1_PTP_CLK>; + clock-names = "sys", + "cfg", + "ahb", + "axi", + "rx", + "tx", + "ptp"; + resets = <&gcc GCC_GMAC1_BCR>; + interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "macirq"; + + phys = <&uniphy0>; + phy-names = "uniphy"; + + status = "disabled"; + }; }; timer {
Signed-off-by: Ziyang Huang <hzyitc@outlook.com> --- arch/arm64/boot/dts/qcom/ipq5018.dtsi | 120 +++++++++++++++++++++++++- 1 file changed, 116 insertions(+), 4 deletions(-)