| Message ID | 20231215074005.26976-15-quic_luoj@quicinc.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | add qca8084 ethernet phy driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 15/12/2023 08:40, Luo Jie wrote: > The following properties are added for qca8084 PHY. > > 1. add the compatible string "ethernet-phy-id004d.d180" since > the PHY device is not accessible during MDIO bus register. > 2. add property "qcom,phy-addr-fixup" for customizing MDIO address. > 3. add property "qcom,phy-work-mode" for specifying qca8084 PHY > work mode. > 4. add the initial clocks and resets. All my previous comments (sent one minute before this patchset :) ) apply. Please respond to them or implement them in v5 (not earlier than after 24h). Best regards, Krzysztof
On 12/15/2023 4:22 PM, Krzysztof Kozlowski wrote: > On 15/12/2023 08:40, Luo Jie wrote: >> The following properties are added for qca8084 PHY. >> >> 1. add the compatible string "ethernet-phy-id004d.d180" since >> the PHY device is not accessible during MDIO bus register. >> 2. add property "qcom,phy-addr-fixup" for customizing MDIO address. >> 3. add property "qcom,phy-work-mode" for specifying qca8084 PHY >> work mode. >> 4. add the initial clocks and resets. > > All my previous comments (sent one minute before this patchset :) ) > apply. Please respond to them or implement them in v5 (not earlier than > after 24h). > > > Best regards, > Krzysztof > Sure, will update the new version after the discussion completed. actually i have query about the dt-bindings doc. The "maxItems: 1" of the property resets is defined in ethernet-phy.yaml that is referenced by qca,ar803x.yaml, but i have 11 reset instances used for qca8084 PHY, it seems i can't overwrite maxItems of reset property to 11 from 1 in qca,ar803x.yaml. is there any method to overwrite the maxItems value? Thanks, Jie
> The "maxItems: 1" of the property resets is defined in ethernet-phy.yaml > that is referenced by qca,ar803x.yaml, but i have 11 reset instances > used for qca8084 PHY 11!?!?? Really? Why? I assume the order and timer matters, otherwise why would you need 11? So the PHY driver needs to handle this, not phylib framework. So you will be adding vendor properties to describe all 11 of them. So ethernet-phy.yaml does not matter. Andrew
> + clocks: > + items: > + - description: APB bridge clock > + - description: AHB clock > + - description: Security control clock > + - description: TLMM clock > + - description: TLMM AHB clock > + - description: CNOC AHB clock > + - description: MDIO AHB clock > + - description: MDIO master AHB clock > + - description: PCS0 system clock > + - description: PCS1 system clock > + - description: EPHY0 system clock > + - description: EPHY1 system clock > + - description: EPHY2 system clock > + - description: EPHY3 system clock What exactly are you describing here? A PHY, or a PHY package? The ethernet-phy.yaml describes a PHY. So does each of your 4 PHYs have 14 clocks? The PHY package as a whole has 14*4 clocks? This seems unlikely. You have some clocks used by the package as a whole, and you have some clocks used by one specific PHY within the package. So you need a hierarchical description of the hardware in DT, to match the actual hierarchical of the hardware. This is exactly what Christian has been working on, and you have persistently ignored what he is doing. You need to work with him. Nothing is going to be merged until you and Christian have one consistent design for the two PHYs you are working on. Andrew --- pw-bot: cr
On 12/15/2023 7:25 PM, Andrew Lunn wrote: >> The "maxItems: 1" of the property resets is defined in ethernet-phy.yaml >> that is referenced by qca,ar803x.yaml, but i have 11 reset instances >> used for qca8084 PHY > > 11!?!?? Really? Why? > > I assume the order and timer matters, otherwise why would you need > 11? So the PHY driver needs to handle this, not phylib framework. So > you will be adding vendor properties to describe all 11 of them. So > ethernet-phy.yaml does not matter. > > Andrew Since these resets need to be configured in the special sequence, and these clocks need to be configured with different clock rate. But the clock instance get, the property name is fixed to "clock-names" according to the function of_parse_clkspec, and the reset property name is also fixed to "reset-names" from function __of_reset_control_get.
On 12/15/2023 8:12 PM, Andrew Lunn wrote: >> + clocks: >> + items: >> + - description: APB bridge clock >> + - description: AHB clock >> + - description: Security control clock >> + - description: TLMM clock >> + - description: TLMM AHB clock >> + - description: CNOC AHB clock >> + - description: MDIO AHB clock >> + - description: MDIO master AHB clock >> + - description: PCS0 system clock >> + - description: PCS1 system clock >> + - description: EPHY0 system clock >> + - description: EPHY1 system clock >> + - description: EPHY2 system clock >> + - description: EPHY3 system clock > > What exactly are you describing here? A PHY, or a PHY package? > > The ethernet-phy.yaml describes a PHY. So does each of your 4 PHYs > have 14 clocks? The PHY package as a whole has 14*4 clocks? > > This seems unlikely. You have some clocks used by the package as a > whole, and you have some clocks used by one specific PHY within the > package. So you need a hierarchical description of the hardware in DT, > to match the actual hierarchical of the hardware. > > This is exactly what Christian has been working on, and you have > persistently ignored what he is doing. You need to work with him. > Nothing is going to be merged until you and Christian have one > consistent design for the two PHYs you are working on. > > > Andrew > > --- > pw-bot: cr Hi Andrew, These clocks are for the whole PHY package including quad PHYs, since these clocks & resets need to be initialized at one point, i put it the previous MDIO driver code, these clocks & resets are configured after GPIO hardware reset, after these clocks and resets sequences configured, each PHY capabilities can be acquired correctly in the PHY probe function. Sorry for missing Christian's patches, i will look his patches and update qca8084 PHY driver correspondingly.
On Fri, Dec 15, 2023 at 08:33:00PM +0800, Jie Luo wrote: > > > On 12/15/2023 8:12 PM, Andrew Lunn wrote: > > > + clocks: > > > + items: > > > + - description: APB bridge clock > > > + - description: AHB clock > > > + - description: Security control clock > > > + - description: TLMM clock > > > + - description: TLMM AHB clock > > > + - description: CNOC AHB clock > > > + - description: MDIO AHB clock > > > + - description: MDIO master AHB clock > > > + - description: PCS0 system clock > > > + - description: PCS1 system clock > > > + - description: EPHY0 system clock > > > + - description: EPHY1 system clock > > > + - description: EPHY2 system clock > > > + - description: EPHY3 system clock > Hi Andrew, > These clocks are for the whole PHY package including quad PHYs, since > these clocks & resets need to be initialized at one point, i put it > the previous MDIO driver code, these clocks & resets are configured > after GPIO hardware reset, after these clocks and resets sequences > configured, each PHY capabilities can be acquired correctly in the PHY > probe function. I really expect the hardware is hierarchical. Its unlikely that EPHY0 is connected to all four PHYs in the package. Its specific to one PHY. So it should be in the DT properties for that one specific PHY. I expect the resets are the same. It seems there is a soft and hard reset per PHY, so i would expect these to be in the node for one PHY. Do the two PCS instances take up two MDIO address? They can be considered devices on the bus, so could have a DT node, and hence you can place the PCS clocks on that node? What exactly do the two MDIO clocks do? I assume these are not for the MDIO bus master, but the MDIO slave block within the PHY package? There is one MDIO slave block shared by the four PHYs. So these are package properties and should be in the package node in DT. Look at all the other clocks and decide, are they package clocks, or specific to one block on the MDIO bus? Do the properties go in the package node, or the per PHY node? Andrew
On Fri, Dec 15, 2023 at 08:16:53PM +0800, Jie Luo wrote: > On 12/15/2023 7:25 PM, Andrew Lunn wrote: > > > The "maxItems: 1" of the property resets is defined in ethernet-phy.yaml > > > that is referenced by qca,ar803x.yaml, but i have 11 reset instances > > > used for qca8084 PHY > > > > 11!?!?? Really? Why? > > > > I assume the order and timer matters, otherwise why would you need > > 11? So the PHY driver needs to handle this, not phylib framework. So > > you will be adding vendor properties to describe all 11 of them. So > > ethernet-phy.yaml does not matter. > > > > Andrew > > Since these resets need to be configured in the special sequence, and > these clocks need to be configured with different clock rate. > > But the clock instance get, the property name is fixed to "clock-names" > according to the function of_parse_clkspec, and the reset property name > is also fixed to "reset-names" from function __of_reset_control_get. I think you need to give more details about this. Where are these 11 resets located? What is the sequence? Why does the PHY driver need to deal with each individual reset? IMHO, a PHY driver should _not_ be dealing with the resets outside of the PHY device itself, and I find it hard to imagine that qca8084 would have 11 external resets. If these are 11 internal resets (to qca8084) then why are you using the reset subsystem, and why do you need to describe them in DT? Surely if they are internal to the PHY, that can be encapsulated within the PHY driver? This is an example of why it is useful to have an _example_ of the use of this binding, because it would answer some of the above questions.
On 12/15/2023 9:31 PM, Andrew Lunn wrote: > On Fri, Dec 15, 2023 at 08:33:00PM +0800, Jie Luo wrote: >> >> >> On 12/15/2023 8:12 PM, Andrew Lunn wrote: >>>> + clocks: >>>> + items: >>>> + - description: APB bridge clock >>>> + - description: AHB clock >>>> + - description: Security control clock >>>> + - description: TLMM clock >>>> + - description: TLMM AHB clock >>>> + - description: CNOC AHB clock >>>> + - description: MDIO AHB clock >>>> + - description: MDIO master AHB clock >>>> + - description: PCS0 system clock >>>> + - description: PCS1 system clock >>>> + - description: EPHY0 system clock >>>> + - description: EPHY1 system clock >>>> + - description: EPHY2 system clock >>>> + - description: EPHY3 system clock > >> Hi Andrew, >> These clocks are for the whole PHY package including quad PHYs, since >> these clocks & resets need to be initialized at one point, i put it >> the previous MDIO driver code, these clocks & resets are configured >> after GPIO hardware reset, after these clocks and resets sequences >> configured, each PHY capabilities can be acquired correctly in the PHY >> probe function. > > I really expect the hardware is hierarchical. Its unlikely that EPHY0 > is connected to all four PHYs in the package. Its specific to one > PHY. So it should be in the DT properties for that one specific PHY. I > expect the resets are the same. It seems there is a soft and hard > reset per PHY, so i would expect these to be in the node for one PHY.be Hi Andrew, i understand your point, i tried putting the related clocks and resets into each device node per PHY, which does not work, since these clocks ans resets need to be initialized at one function pointer after GPIO reset on the qca8084 package. Sorry for these confusions here, let me explain the qca8084 chip more detail here, and i will also update this info on the cover letter. The following is the chip package, the chip can work on the switch mode like the existed upstream code qca8k, where PHY1-PHY4 is connected with MAC1-MAC4 directly; The chip can also work on the PHY mode, where PHY1- PHY4 is connected with PCS1 by 10g-qxgmii; Either switch mode or PHY mode, the PHY4 is optionally connected with PCS0 by SGMII, PCS0 and PCS1 are connected with the SoC(IPQ platform) PCSes. +----------------------------------------------+ | PCS1 PCS0 | | | | MAC0 MAC5 | | | | | | | | MAC1 MAC2 MAC3 MAC4 | | | | PHY1 PHY2 PHY3 PHY4 | +----------------------------------------------+ After the GPIO reset on this package from the MDIO bus level, the chip is in the cold hardware reset status, the clocks and resets mentioned here need to be initialized before the capabilities of PHY can be acquired correctly in the PHY probe function, that is why i put these clocks and resets in the first PHY device tree node. When the chip works on the PHY mode, the MAC listed in the block is not used, the MAC is only used in the switch mode. i know we can also put the related clocks and resets into the each PHY and PCS device node to make the hardware hierarchical, then i can read all initial clocks and resets in the first PHY probe function, but there are still some package level clocks such as APB/AHB/CNOC clocks needed for these initial configuration, so i put these clocks and resets at one PHY device node and only needed to be called by one time. From the clocks and resets name, EPHY0 is specific to the first PHY1, which is providing clock and reset on the first PHY1, other EPHY clock and reset is same for the corresponding PHY. > > Do the two PCS instances take up two MDIO address? They can be > considered devices on the bus, so could have a DT node, and hence you > can place the PCS clocks on that node? Yes, two PCS instances take up two MDIO device with different MDIO address, which also have the DT nodes as the child node of MDIO bus node there is also some specific clocks and resets defined in the PCS DT node for the PCS driver. > > What exactly do the two MDIO clocks do? I assume these are not for the > MDIO bus master, but the MDIO slave block within the PHY package? > There is one MDIO slave block shared by the four PHYs. So these are > package properties and should be in the package node in DT. The MDIO clocks are the qca8084 package level clock, since there are other modules such as GCC and security control located in the qca8084 chip, these module register is also accessed by the MDIO bus, the MDIO AHB clock is this modules access. there is also a MDIO master for the back pressure function in qca8084 chip, the MDIO master AHB clock is for this function, actually this function is for the switch mode, but it is the package level clock, so i put it together here. No, these clocks are not for the IPQ4019 SoC MDIO bus master. Four PHYs are the independent MDIO slave devices. For the switch mode, we can define these package level clocks and resets in the DSA device node. But for the PHY mode, All 4 PHYs is connected with PCS1 by 10g-qxgmii, there is no package level device tree node defined. > > Look at all the other clocks and decide, are they package clocks, or > specific to one block on the MDIO bus? Do the properties go in the > package node, or the per PHY node? > > Andrew The clocks and resets with prefix PCS or EPHY is per PHY/PCS node, other clocks and resets are the qca8084 package level node.
On 12/15/2023 9:42 PM, Russell King (Oracle) wrote: > On Fri, Dec 15, 2023 at 08:16:53PM +0800, Jie Luo wrote: >> On 12/15/2023 7:25 PM, Andrew Lunn wrote: >>>> The "maxItems: 1" of the property resets is defined in ethernet-phy.yaml >>>> that is referenced by qca,ar803x.yaml, but i have 11 reset instances >>>> used for qca8084 PHY >>> >>> 11!?!?? Really? Why? >>> >>> I assume the order and timer matters, otherwise why would you need >>> 11? So the PHY driver needs to handle this, not phylib framework. So >>> you will be adding vendor properties to describe all 11 of them. So >>> ethernet-phy.yaml does not matter. >>> >>> Andrew >> >> Since these resets need to be configured in the special sequence, and >> these clocks need to be configured with different clock rate. >> >> But the clock instance get, the property name is fixed to "clock-names" >> according to the function of_parse_clkspec, and the reset property name >> is also fixed to "reset-names" from function __of_reset_control_get. > > I think you need to give more details about this. > > Where are these 11 resets located? What is the sequence? Why does the > PHY driver need to deal with each individual reset? All these resets and clocks are located in qca8084 chip that includes the GCC module registered as the clock provider, all these clocks and resets are from qca8084 clock provider driver, the clock and reset name prefix with PCS or EPHY is for the individual MDIO device, others are the qca8084 level, but these clocks and resets need to be initialized completely, then the PHY probe function can be initialized correctly with the PHY capabilities acquired after the qca8084 chip level GPIO reset. The sequence code is realized in the function qca8084_clock_config of the patch <net: phy: at803x: add qca808x initial config sequence>, this function is doing as below. 1. set the Ethernet system clock tree as 25M clock rate. 2. reset and enable PCS system clocks. 3. reset and enable the PHY system clocks. 4. loading the efuse, which is not related the clock and reset. > > IMHO, a PHY driver should _not_ be dealing with the resets outside of > the PHY device itself, and I find it hard to imagine that qca8084 > would have 11 external resets. Indeed, these resets are not for a PHY device, some are the chip level resets, and each PHY has the individual resets. > > If these are 11 internal resets (to qca8084) then why are you using the > reset subsystem, and why do you need to describe them in DT? Surely if > they are internal to the PHY, that can be encapsulated within the PHY > driver? These resets and clocks are realized by the qca8k clock controller driver, and i use the clock consumer APIs to initialize these clocks and resets, when qca8084 works on the PHY mode, there are only PHY driver and PCS driver needed to be enabled to make PHY working, and these clocks and resets need to be initialized before the PHY probe function finished correctly, where the phy capabilities is read during the probe function. > > This is an example of why it is useful to have an _example_ of the use > of this binding, because it would answer some of the above questions. > Yes, Russell, i will add an example in the DT doc in the next patch set. The following is the device node used for the current qca8084 PHY code design. mdio: mdio@90000 { ethernet-phy@0 { compatible = "ethernet-phy-id004d.d180"; reg = <1>; qcom,phy-addr-fixup = <1 2 3 4 5 6 7>; qcom,phy-work-mode = <2>; clocks = <&qca8k_nsscc NSS_CC_APB_BRIDGE_CLK>, <&qca8k_nsscc NSS_CC_AHB_CLK>, <&qca8k_nsscc NSS_CC_SEC_CTRL_AHB_CLK>, <&qca8k_nsscc NSS_CC_TLMM_CLK>, <&qca8k_nsscc NSS_CC_TLMM_AHB_CLK>, <&qca8k_nsscc NSS_CC_CNOC_AHB_CLK>, <&qca8k_nsscc NSS_CC_MDIO_AHB_CLK>, <&qca8k_nsscc NSS_CC_MDIO_MASTER_AHB_CLK>, <&qca8k_nsscc NSS_CC_SRDS0_SYS_CLK>, <&qca8k_nsscc NSS_CC_SRDS1_SYS_CLK>, <&qca8k_nsscc NSS_CC_GEPHY0_SYS_CLK>, <&qca8k_nsscc NSS_CC_GEPHY1_SYS_CLK>, <&qca8k_nsscc NSS_CC_GEPHY2_SYS_CLK>, <&qca8k_nsscc NSS_CC_GEPHY3_SYS_CLK>; clock-names = "apb_bridge", "ahb", "sec_ctrl_ahb", "tlmm", "tlmm_ahb", "cnoc_ahb", "mdio_ahb", "mdio_master_ahb", "srds0_sys", "srds1_sys", "gephy0_sys", "gephy1_sys", "gephy2_sys", "gephy3_sys"; resets = <&qca8k_nsscc NSS_CC_SRDS0_SYS_ARES>, <&qca8k_nsscc NSS_CC_SRDS1_SYS_ARES>, <&qca8k_nsscc NSS_CC_GEPHY0_SYS_ARES>, <&qca8k_nsscc NSS_CC_GEPHY1_SYS_ARES>, <&qca8k_nsscc NSS_CC_GEPHY2_SYS_ARES>, <&qca8k_nsscc NSS_CC_GEPHY3_SYS_ARES>, <&qca8k_nsscc NSS_CC_GEPHY0_ARES>, <&qca8k_nsscc NSS_CC_GEPHY1_ARES>, <&qca8k_nsscc NSS_CC_GEPHY2_ARES>, <&qca8k_nsscc NSS_CC_GEPHY3_ARES>, <&qca8k_nsscc NSS_CC_DSP_ARES>; reset-names = "srds0_sys", "srds1_sys", "gephy0_sys", "gephy1_sys", "gephy2_sys", "gephy3_sys", "gephy0_soft", "gephy1_soft", "gephy2_soft", "gephy3_soft", "gephy_dsp"; }; ethernet-phy@1 { compatible = "ethernet-phy-id004d.d180"; reg = <2>; }; ethernet-phy@2 { compatible = "ethernet-phy-id004d.d180"; reg = <3>; }; ethernet-phy@3 { compatible = "ethernet-phy-id004d.d180"; reg = <4>; }; }; Thanks.
> The following is the chip package, the chip can work on the switch mode > like the existed upstream code qca8k, where PHY1-PHY4 is connected with > MAC1-MAC4 directly; Ah, that is new information, and has a big effect on the design. Can the qca8K driver be extended to drive this hardware in switch mode? Andrew
On 12/16/2023 6:21 PM, Andrew Lunn wrote: >> The following is the chip package, the chip can work on the switch mode >> like the existed upstream code qca8k, where PHY1-PHY4 is connected with >> MAC1-MAC4 directly; > > Ah, that is new information, and has a big effect on the design. > > Can the qca8K driver be extended to drive this hardware in switch > mode? > > Andrew Yes, Andrew, the qca8k driver can be extended to drive this hardware in the switch mode, we will push it to the upstream in the near future.
On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote: > > The following is the chip package, the chip can work on the switch mode > > like the existed upstream code qca8k, where PHY1-PHY4 is connected with > > MAC1-MAC4 directly; > > Ah, that is new information, and has a big effect on the design. This QCA8084 that's being proposed in these patches is not a PHY in itself, but is a SoC. I came across this: https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/ It's sounding like what we have here is some PHY IP that is integrated into a larger SoC, and the larger SoC needs to be configured so the PHY IP can work correctly. Given that this package of four PHYs seems to be rather unique, I think we need Jie Luo to provide sufficient information so we can understand: 1) this package of four PHYs itself 2) how this package is integrated into the SoC Specifically, what resets and clocks are controlled from within the package's register space, which are external to the package register space (and thus are provided by other IPs in the SoC). As I've said previously, the lack of DT example doesn't help to further our understanding. The lack of details of what the package encompases also doesn't help us understand the hardware. Unless we can gain that understanding, I feel that Jie Luo's patches are effectively unreviewable and can't be accepted into mainline.
On 12/16/2023 9:51 PM, Russell King (Oracle) wrote: > On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote: >>> The following is the chip package, the chip can work on the switch mode >>> like the existed upstream code qca8k, where PHY1-PHY4 is connected with >>> MAC1-MAC4 directly; >> >> Ah, that is new information, and has a big effect on the design. > > This QCA8084 that's being proposed in these patches is not a PHY in > itself, but is a SoC. I came across this: > > https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/ The chip mentioned in the link you mentioned is SoC, which is not the chip that the qca8084 driver work for. qca8084/qca8386 is just the Ethernet CHIP, not SoC, for the switch mode qca8386, which is most like qca8337 the dsa drive qca8k.c is already in upstream. i qca8084 chip package includes 4 PHYs, 2 PCSs and the common chip level modules such as GCC and security control modules, all these modules are located in the qca8084 chip package, since qca8084 works on PHY mode, so the MACs are not used. qca8084 is connected with the SoC CHIP such as IPQ platform by PCS1 working on 10g-qxgmii mode and the fourth PHY can also optionally be connected with the IPQ SoC PCS by sgmii mode, there is no more interface on qca8084 to connect the external chips. > It's sounding like what we have here is some PHY IP that is integrated > into a larger SoC, and the larger SoC needs to be configured so the > PHY IP can work correctly. qca8084 is not a SoC, it is the Ethernet chip, in this qca8084 package, there are GCC that is driving the PHY working on the various link speed. that is the reason we need to do these package level common clocks and resets initialization before probing PHY correctly. > > Given that this package of four PHYs seems to be rather unique, I think > we need Jie Luo to provide sufficient information so we can understand: > > 1) this package of four PHYs itself Yes, this chip package for all 4 PHYs itself, also including the PCSes and common package level modules such as GCC. > 2) how this package is integrated into the SoC the qca8084 is connected with SoC by PCSes. > > Specifically, what resets and clocks are controlled from within the > package's register space, which are external to the package > register space (and thus are provided by other IPs in the SoC). All clocks and resets mentioned for qca8084 drive including package level and PCS & PHY clocks and resets from the qca8084 internal GCC modules register space, > > As I've said previously, the lack of DT example doesn't help to further > our understanding. The lack of details of what the package encompases > also doesn't help us understand the hardware. Indeed, i will add the qca8084 DT example in the next patch set. BTW, i also replied your earlier comments by providing the DTS defined for the current qca8084 drive code. hope you can have a better understanding with the provided DTS code in earlier reply of this email thread. > > Unless we can gain that understanding, I feel that Jie Luo's patches > are effectively unreviewable and can't be accepted into mainline. >
On Sat, Dec 16, 2023 at 10:41:28PM +0800, Jie Luo wrote: > > > On 12/16/2023 9:51 PM, Russell King (Oracle) wrote: > > On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote: > > > > The following is the chip package, the chip can work on the switch mode > > > > like the existed upstream code qca8k, where PHY1-PHY4 is connected with > > > > MAC1-MAC4 directly; > > > > > > Ah, that is new information, and has a big effect on the design. > > > > This QCA8084 that's being proposed in these patches is not a PHY in > > itself, but is a SoC. I came across this: > > > > https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/ > > The chip mentioned in the link you mentioned is SoC, which is not the > chip that the qca8084 driver work for. > > qca8084/qca8386 is just the Ethernet CHIP, not SoC, for the switch mode > qca8386, which is most like qca8337 the dsa drive qca8k.c is already in > upstream. Hi, sorry for stepping in. I guess here there is a massive confusion with naming and using qca8k. Since it seems the same name is used for PHY and for Switch stuff, I would add PHY and MAC prefix when referring to qca8064. With the previous message I was a bit confused by the use of qca8k and didn't know you were actually referring to the DSA driver. Interesting... this is for the upcoming WiFi 7 platoform right? (ipq9574) All these discussion comes for the problem of using this PHY as an integrated PHY in the qca8386 switch and trying to select the mode in the PHY driver. Considering you would use the same logic of the current DSA qca8k driver with the integrated PHY, the problem doesn't apply or a different implementation should be used (and actually handled later when the actual DSA code will come) I would expect in the integrated mode, the switch to handle the PHY (as it's done by qca8337) with the PHY defined in the switch node and qca8k handling the PHY registration. With the following implementation flags can be passed and PHY can be configured to integrated mode. (or virtual PHY ID can be used for such scope with dedicated functions in the PHY driver) With this in mind the entire integrated problem can put on hold and dropped to be later reimplemented when it's time. (assuming that all the prereq are already here and the very same implementation of qca8k will be used) Anyway, I'm more or less the maintainer of the qca8k.c DSA driver and I would be more than happy to help you guys internally or externally on pushing and make this proceed further. (again assuming this is ipq9574 stuff, it would be good to finally have proper DSA driver instead of leaving the thing unusable as it's the current situation with ipq8074) > > i qca8084 chip package includes 4 PHYs, 2 PCSs and the common chip level > modules such as GCC and security control modules, all these modules are > located in the qca8084 chip package, since qca8084 works on PHY mode, so > the MACs are not used. > > qca8084 is connected with the SoC CHIP such as IPQ platform by PCS1 > working on 10g-qxgmii mode and the fourth PHY can also optionally > be connected with the IPQ SoC PCS by sgmii mode, there is no more > interface on qca8084 to connect the external chips. > > > It's sounding like what we have here is some PHY IP that is integrated > > into a larger SoC, and the larger SoC needs to be configured so the > > PHY IP can work correctly. > > qca8084 is not a SoC, it is the Ethernet chip, in this qca8084 package, > there are GCC that is driving the PHY working on the various link speed. > that is the reason we need to do these package level common clocks and > resets initialization before probing PHY correctly. > > > > > Given that this package of four PHYs seems to be rather unique, I think > > we need Jie Luo to provide sufficient information so we can understand: > > > > 1) this package of four PHYs itself > > Yes, this chip package for all 4 PHYs itself, also including the PCSes > and common package level modules such as GCC. > > > 2) how this package is integrated into the SoC > > the qca8084 is connected with SoC by PCSes. > > > > > Specifically, what resets and clocks are controlled from within the > > package's register space, which are external to the package > > register space (and thus are provided by other IPs in the SoC). > > All clocks and resets mentioned for qca8084 drive including package > level and PCS & PHY clocks and resets from the qca8084 internal GCC > modules register space, > > > > > As I've said previously, the lack of DT example doesn't help to further > > our understanding. The lack of details of what the package encompases > > also doesn't help us understand the hardware. > > Indeed, i will add the qca8084 DT example in the next patch set. > BTW, i also replied your earlier comments by providing the DTS defined > for the current qca8084 drive code. > > hope you can have a better understanding with the provided DTS code in > earlier reply of this email thread. > > > > Unless we can gain that understanding, I feel that Jie Luo's patches > > are effectively unreviewable and can't be accepted into mainline. > >
On Sat, Dec 16, 2023 at 10:41:28PM +0800, Jie Luo wrote: > > > On 12/16/2023 9:51 PM, Russell King (Oracle) wrote: > > On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote: > > > > The following is the chip package, the chip can work on the switch mode > > > > like the existed upstream code qca8k, where PHY1-PHY4 is connected with > > > > MAC1-MAC4 directly; > > > > > > Ah, that is new information, and has a big effect on the design. > > > > This QCA8084 that's being proposed in these patches is not a PHY in > > itself, but is a SoC. I came across this: > > > > https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/ > > The chip mentioned in the link you mentioned is SoC, which is not the > chip that the qca8084 driver work for. So there's two chips called QCA8084 both produced by Qualcomm? I find that hard to believe.
> Yes, Russell, i will add an example in the DT doc in the next patch set. > The following is the device node used for the current qca8084 PHY > code design. If you look at Christians work, this would be expressed differently: > mdio: mdio@90000 { > ethernet-phy-package@1 { > > compatible = "qca,qca8084-package"; > > qcom,phy-work-mode = <2>; > clocks = <&qca8k_nsscc NSS_CC_APB_BRIDGE_CLK>, > <&qca8k_nsscc NSS_CC_AHB_CLK>, > <&qca8k_nsscc NSS_CC_SEC_CTRL_AHB_CLK>, > <&qca8k_nsscc NSS_CC_TLMM_CLK>, > <&qca8k_nsscc NSS_CC_TLMM_AHB_CLK>, > <&qca8k_nsscc NSS_CC_CNOC_AHB_CLK>, > <&qca8k_nsscc NSS_CC_MDIO_AHB_CLK>, > <&qca8k_nsscc NSS_CC_MDIO_MASTER_AHB_CLK>, > <&qca8k_nsscc NSS_CC_SRDS0_SYS_CLK>, > <&qca8k_nsscc NSS_CC_SRDS1_SYS_CLK>; > clock-names = "apb_bridge", > "ahb", > "sec_ctrl_ahb", > "tlmm", > "tlmm_ahb", > "cnoc_ahb", > "mdio_ahb", > "mdio_master_ahb", > "srds0_sys", > "srds1_sys"; > resets = <&qca8k_nsscc NSS_CC_SRDS0_SYS_ARES>, > <&qca8k_nsscc NSS_CC_SRDS1_SYS_ARES>, > <&qca8k_nsscc NSS_CC_DSP_ARES>; > reset-names = "srds0_sys", > "srds1_sys"; > All the properties above are common to the package as a whole. Then follow the four individual PHYs, and the properties which are specific to each one. > > ethernet-phy@0 { > compatible = "ethernet-phy-id004d.d180"; > reg = <0>; > clocks = <qca8k_nsscc NSS_CC_GEPHY0_SYS_CLK>, > clock-names = <"gephy_sys">; > resets = <&qca8k_nsscc NSS_CC_GEPHY0_SYS_ARES>, > <&qca8k_nsscc NSS_CC_GEPHY0_ARES>; > reset-names = "gephy_sys", "gephy_soft"; > }; > > > ethernet-phy@1 { > compatible = "ethernet-phy-id004d.d180"; > reg = <1>; > clocks = <qca8k_nsscc NSS_CC_GEPHY1_SYS_CLK>, > clock-names = <"gephy_sys">; > resets = <&qca8k_nsscc NSS_CC_GEPHY1_SYS_ARES>, > <&qca8k_nsscc NSS_CC_GEPHY1_ARES>; > reset-names = "gephy_sys", "gephy_soft"; > > }; > > ethernet-phy@2 { > compatible = "ethernet-phy-id004d.d180"; > reg = <2>; > clocks = <qca8k_nsscc NSS_CC_GEPHY2_SYS_CLK>, > clock-names = <"gephy_sys">; > resets = <&qca8k_nsscc NSS_CC_GEPHY2_SYS_ARES>, > <&qca8k_nsscc NSS_CC_GEPHY2_ARES>; > reset-names = "gephy_sys", "gephy_soft"; > > }; > > ethernet-phy@3 { > compatible = "ethernet-phy-id004d.d180"; > reg = <3>; > clocks = <qca8k_nsscc NSS_CC_GEPHY3_SYS_CLK>, > clock-names = <"gephy_sys">; > reset-names = "gephy_sys", "gephy_soft"; > }; > }; Andrew
> The following is the chip package, the chip can work on the switch mode > like the existed upstream code qca8k, where PHY1-PHY4 is connected with > MAC1-MAC4 directly; The chip can also work on the PHY mode, where PHY1- > PHY4 is connected with PCS1 by 10g-qxgmii; Either switch mode or PHY mode, > the PHY4 is optionally connected with PCS0 by SGMII, PCS0 and PCS1 > are connected with the SoC(IPQ platform) PCSes. I don't really understand. Are you saying the hardware is actually : +----------------------------------------------+ | PCS1 PCS0 | | | | MAC0 MAC5 | | | | | | +-----+--------------+-------------+ | | | | | | | Switch | | | | | | | +-+---------+---------+---------+--+ | | | | | | | | MAC1 MAC2 MAC3 MAC4 | | | | PHY1 PHY2 PHY3 PHY4 | +----------------------------------------------+ When in PHY mode, the switch is hard coded to map the 4 PCS1 channels straight to MAC1-MAC4 and all switch functionality is disabled. But then in switch mode, the switch can be controlled as a DSA switch? The 10G PCS1 is then a single 10G port, not 4x 2.5G? Is there a product brief for this PHY? That might help us understand this hardware? Andrew
On Sat, Dec 16, 2023 at 06:30:00PM +0100, Andrew Lunn wrote: > > The following is the chip package, the chip can work on the switch mode > > like the existed upstream code qca8k, where PHY1-PHY4 is connected with > > MAC1-MAC4 directly; The chip can also work on the PHY mode, where PHY1- > > PHY4 is connected with PCS1 by 10g-qxgmii; Either switch mode or PHY mode, > > the PHY4 is optionally connected with PCS0 by SGMII, PCS0 and PCS1 > > are connected with the SoC(IPQ platform) PCSes. > > I don't really understand. Are you saying the hardware is actually : > > > +----------------------------------------------+ > | PCS1 PCS0 | > | | > | MAC0 MAC5 | > | | | | > | +-----+--------------+-------------+ | > | | | | > | | Switch | | > | | | | > | +-+---------+---------+---------+--+ | > | | | | | | > | MAC1 MAC2 MAC3 MAC4 | > | | > | PHY1 PHY2 PHY3 PHY4 | > +----------------------------------------------+ > > When in PHY mode, the switch is hard coded to map the 4 PCS1 channels > straight to MAC1-MAC4 and all switch functionality is disabled. But > then in switch mode, the switch can be controlled as a DSA switch? The > 10G PCS1 is then a single 10G port, not 4x 2.5G? > > Is there a product brief for this PHY? That might help us understand > this hardware? Not even digikey give any clues what "QCA8084" is - they list it as "unclassified" and give no documentation and no photo. Basically it seems to be a super secret device.
On 12/17/2023 12:09 AM, Russell King (Oracle) wrote: > On Sat, Dec 16, 2023 at 10:41:28PM +0800, Jie Luo wrote: >> >> >> On 12/16/2023 9:51 PM, Russell King (Oracle) wrote: >>> On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote: >>>>> The following is the chip package, the chip can work on the switch mode >>>>> like the existed upstream code qca8k, where PHY1-PHY4 is connected with >>>>> MAC1-MAC4 directly; >>>> >>>> Ah, that is new information, and has a big effect on the design. >>> >>> This QCA8084 that's being proposed in these patches is not a PHY in >>> itself, but is a SoC. I came across this: >>> >>> https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/ >> >> The chip mentioned in the link you mentioned is SoC, which is not the >> chip that the qca8084 driver work for. > > So there's two chips called QCA8084 both produced by Qualcomm? I find > that hard to believe. > The SoC mentioned in the link you provided is the APQ8084 that is introduced in the link below: https://www.qualcomm.com/products/mobile/snapdragon/smartphones/snapdragon-8-series-mobile-platforms/snapdragon-processors-805 https://hwbot.org/hardware/processor/snapdragon_805_apq8084_pro__(8084_fusion_4.5)_2700mhz/ The driver here is for qca8084, which has the different prefix, and qca8084 is the Ethernet CHIP like qca8081, but qca8084 is multiple ports(quad-phy).
On 12/17/2023 1:30 AM, Andrew Lunn wrote: >> The following is the chip package, the chip can work on the switch mode >> like the existed upstream code qca8k, where PHY1-PHY4 is connected with >> MAC1-MAC4 directly; The chip can also work on the PHY mode, where PHY1- >> PHY4 is connected with PCS1 by 10g-qxgmii; Either switch mode or PHY mode, >> the PHY4 is optionally connected with PCS0 by SGMII, PCS0 and PCS1 >> are connected with the SoC(IPQ platform) PCSes. > > I don't really understand. Are you saying the hardware is actually : > > > +----------------------------------------------+ > | PCS1 PCS0 | > | | > | MAC0 MAC5 | > | | | | > | +-----+--------------+-------------+ | > | | | | > | | Switch | | > | | | | > | +-+---------+---------+---------+--+ | > | | | | | | > | MAC1 MAC2 MAC3 MAC4 | > | | > | PHY1 PHY2 PHY3 PHY4 | > +----------------------------------------------+ > Actually there are two CHIP types, ,let me explain to be more clear. 1. The diagram you describe is actually the switch work mode, which has the different chip name called qca8386, the DSA driver and PHY driver are used, since the general PHY driver can't work for the PHY here. +----------------------------------------------+ | +-----+ | | GCC | | +-----+ PCS1 PCS0 | | | | MAC0 MAC5 | | | | | | +-----+--------------+-------------+ | | | | | | | Switch | | | | | | | +-+---------+---------+---------+--+ | | | | | | | | MAC1 MAC2 MAC3 MAC4 | | | | PHY1 PHY2 PHY3 PHY4 | +----------------------------------------------+ 2. The pure PHY chip called by qca8084 works on the PHY mode 10-qxgmii on quad-phy, or the sgmii mode can be configured on PHY4 optionally. The qca8084 is below, there is no MAC involved on qca8084. +----------------------------------------------+ | PCS1 PCS0 | | | | +-----+ | | GCC | | +-----+ | | | PHY1 PHY2 PHY3 PHY4 | +----------------------------------------------+ On qca8386, the same qca8084 PHY is used, but the qca8084 PHY is connected with internal MAC directly same as qca8337(qca8k dsa driver). On both Ethernet chips qca8386 and qca8084, GCC block is same and with the same clock controller driver that provides the clocks and resets used by the qca8084 PHY driver and qca8386 DSA driver(leverage the existed DSA driver qca8k.c). > When in PHY mode, the switch is hard coded to map the 4 PCS1 channels > straight to MAC1-MAC4 and all switch functionality is disabled. But > then in switch mode, the switch can be controlled as a DSA switch? The > 10G PCS1 is then a single 10G port, not 4x 2.5G? For the qca8084 PHY chip, there is no MAC involved, the PHY is connected with the PCS with 10g-qxgmii, PHY4 is optional connected with sgmii. For the qca8386 switch chip, it is controlled as DSA, the PCS is connected with the SOC(such as IPQ5332) PCS. > > Is there a product brief for this PHY? That might help us understand > this hardware? > Sorry, i also searched it on the internet and Qualcomm website, there is no Doc found, the CHIP is developed recently 1-2 year before, the Doc is not updated to the website. > Andrew
On 12/17/2023 3:08 AM, Russell King (Oracle) wrote: > On Sat, Dec 16, 2023 at 06:30:00PM +0100, Andrew Lunn wrote: >>> The following is the chip package, the chip can work on the switch mode >>> like the existed upstream code qca8k, where PHY1-PHY4 is connected with >>> MAC1-MAC4 directly; The chip can also work on the PHY mode, where PHY1- >>> PHY4 is connected with PCS1 by 10g-qxgmii; Either switch mode or PHY mode, >>> the PHY4 is optionally connected with PCS0 by SGMII, PCS0 and PCS1 >>> are connected with the SoC(IPQ platform) PCSes. >> >> I don't really understand. Are you saying the hardware is actually : >> >> >> +----------------------------------------------+ >> | PCS1 PCS0 | >> | | >> | MAC0 MAC5 | >> | | | | >> | +-----+--------------+-------------+ | >> | | | | >> | | Switch | | >> | | | | >> | +-+---------+---------+---------+--+ | >> | | | | | | >> | MAC1 MAC2 MAC3 MAC4 | >> | | >> | PHY1 PHY2 PHY3 PHY4 | >> +----------------------------------------------+ >> >> When in PHY mode, the switch is hard coded to map the 4 PCS1 channels >> straight to MAC1-MAC4 and all switch functionality is disabled. But >> then in switch mode, the switch can be controlled as a DSA switch? The >> 10G PCS1 is then a single 10G port, not 4x 2.5G? >> >> Is there a product brief for this PHY? That might help us understand >> this hardware? > > Not even digikey give any clues what "QCA8084" is - they list it as > "unclassified" and give no documentation and no photo. Basically it > seems to be a super secret device. > Sorry for the confusion here, maybe the chip is developed recently, which leads to the Doc or introduction is not released in time.
On 12/17/2023 1:17 AM, Andrew Lunn wrote: >> Yes, Russell, i will add an example in the DT doc in the next patch set. >> The following is the device node used for the current qca8084 PHY >> code design. > > If you look at Christians work, this would be expressed differently: > >> mdio: mdio@90000 { >> ethernet-phy-package@1 { >> >> compatible = "qca,qca8084-package"; >> >> qcom,phy-work-mode = <2>; >> clocks = <&qca8k_nsscc NSS_CC_APB_BRIDGE_CLK>, >> <&qca8k_nsscc NSS_CC_AHB_CLK>, >> <&qca8k_nsscc NSS_CC_SEC_CTRL_AHB_CLK>, >> <&qca8k_nsscc NSS_CC_TLMM_CLK>, >> <&qca8k_nsscc NSS_CC_TLMM_AHB_CLK>, >> <&qca8k_nsscc NSS_CC_CNOC_AHB_CLK>, >> <&qca8k_nsscc NSS_CC_MDIO_AHB_CLK>, >> <&qca8k_nsscc NSS_CC_MDIO_MASTER_AHB_CLK>, >> <&qca8k_nsscc NSS_CC_SRDS0_SYS_CLK>, >> <&qca8k_nsscc NSS_CC_SRDS1_SYS_CLK>; >> clock-names = "apb_bridge", >> "ahb", >> "sec_ctrl_ahb", >> "tlmm", >> "tlmm_ahb", >> "cnoc_ahb", >> "mdio_ahb", >> "mdio_master_ahb", >> "srds0_sys", >> "srds1_sys"; >> resets = <&qca8k_nsscc NSS_CC_SRDS0_SYS_ARES>, >> <&qca8k_nsscc NSS_CC_SRDS1_SYS_ARES>, >> <&qca8k_nsscc NSS_CC_DSP_ARES>; >> reset-names = "srds0_sys", >> "srds1_sys"; >> > > All the properties above are common to the package as a whole. > > Then follow the four individual PHYs, and the properties which are > specific to each one. Thanks Andrew for the proposal. For the pure PHY chip qca8084, there is no driver to parse the package level device tree node for common clocks and resets. For the common clocks and resets above, whether we can add a qca8084 common device tree node as the child node of MDIO bus node, and then parse these common properties in the PHY probe function? since the DSA driver is not enabled for the pure PHY chip. > >> >> ethernet-phy@0 { >> compatible = "ethernet-phy-id004d.d180"; >> reg = <0>; >> clocks = <qca8k_nsscc NSS_CC_GEPHY0_SYS_CLK>, >> clock-names = <"gephy_sys">; >> resets = <&qca8k_nsscc NSS_CC_GEPHY0_SYS_ARES>, >> <&qca8k_nsscc NSS_CC_GEPHY0_ARES>; >> reset-names = "gephy_sys", "gephy_soft"; >> }; >> >> >> ethernet-phy@1 { >> compatible = "ethernet-phy-id004d.d180"; >> reg = <1>; >> clocks = <qca8k_nsscc NSS_CC_GEPHY1_SYS_CLK>, >> clock-names = <"gephy_sys">; >> resets = <&qca8k_nsscc NSS_CC_GEPHY1_SYS_ARES>, >> <&qca8k_nsscc NSS_CC_GEPHY1_ARES>; >> reset-names = "gephy_sys", "gephy_soft"; >> >> }; >> >> ethernet-phy@2 { >> compatible = "ethernet-phy-id004d.d180"; >> reg = <2>; >> clocks = <qca8k_nsscc NSS_CC_GEPHY2_SYS_CLK>, >> clock-names = <"gephy_sys">; >> resets = <&qca8k_nsscc NSS_CC_GEPHY2_SYS_ARES>, >> <&qca8k_nsscc NSS_CC_GEPHY2_ARES>; >> reset-names = "gephy_sys", "gephy_soft"; >> >> }; >> >> ethernet-phy@3 { >> compatible = "ethernet-phy-id004d.d180"; >> reg = <3>; >> clocks = <qca8k_nsscc NSS_CC_GEPHY3_SYS_CLK>, >> clock-names = <"gephy_sys">; >> reset-names = "gephy_sys", "gephy_soft"; >> }; >> }; > > Andrew
On 12/17/2023 12:01 AM, Christian Marangi wrote: > On Sat, Dec 16, 2023 at 10:41:28PM +0800, Jie Luo wrote: >> >> >> On 12/16/2023 9:51 PM, Russell King (Oracle) wrote: >>> On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote: >>>>> The following is the chip package, the chip can work on the switch mode >>>>> like the existed upstream code qca8k, where PHY1-PHY4 is connected with >>>>> MAC1-MAC4 directly; >>>> >>>> Ah, that is new information, and has a big effect on the design. >>> >>> This QCA8084 that's being proposed in these patches is not a PHY in >>> itself, but is a SoC. I came across this: >>> >>> https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/ >> >> The chip mentioned in the link you mentioned is SoC, which is not the >> chip that the qca8084 driver work for. >> >> qca8084/qca8386 is just the Ethernet CHIP, not SoC, for the switch mode >> qca8386, which is most like qca8337 the dsa drive qca8k.c is already in >> upstream. > > Hi, > sorry for stepping in. I guess here there is a massive confusion with > naming and using qca8k. > > Since it seems the same name is used for PHY and for Switch stuff, I > would add PHY and MAC prefix when referring to qca8064. Hi Christian, Welcome to join the discussion. Sorry for the confusion, since the switch and PHY are the different chips, the switch chip is named as qca8386, the pure PHY chip is named as qca8084, the pure PHY chip qca8084 is connected with the SoC by PCS working on 10g-qxgmii, the switch chip qca8386 is connected with SoC by PCS working on sgmii. > > With the previous message I was a bit confused by the use of qca8k and > didn't know you were actually referring to the DSA driver. > Interesting... this is for the upcoming WiFi 7 platoform right? (ipq9574) For qca8386, it is the switch chip and leverages the qca8k DSA driver, it is connected on the ipq5332 platform currently. But for qca8084, it is the pure PHY chip, which works on 10g-qxgmii for quad-phy, which is connected on the ipq9574. > > All these discussion comes for the problem of using this PHY as an > integrated PHY in the qca8386 switch and trying to select the mode in > the PHY driver. Yes, for qca8386, the PHY is integrated to switch, since same PHY needs to work with qca8386 and qca8084, so the work mode needs to be configured to distinguish the different work mode as the patch <[PATCH v8 13/14] net: phy: at803x: configure qca8084 work mode>. > > Considering you would use the same logic of the current DSA qca8k driver > with the integrated PHY, the problem doesn't apply or a different > implementation should be used (and actually handled later when the > actual DSA code will come) qca8386 has some differences from qca8337(qca8k dsa driver), qca8337 integrated PHY can work by the general PHY, but the PHY in qca8386 needs the extra PHY driver, and the GCC clocks introduced. > > I would expect in the integrated mode, the switch to handle the PHY (as > it's done by qca8337) with the PHY defined in the switch node and qca8k > handling the PHY registration. With the following implementation flags > can be passed and PHY can be configured to integrated mode. (or virtual > PHY ID can be used for such scope with dedicated functions in the PHY > driver) > Since there is the pure PHY chip qca8084 using the same PHY, which does not enable the DSA driver. So the PHY driver should be standalone, and the common clocks and resets is better to put in the PHY driver, since these are the common configs for the qca8386 and qca8084. > With this in mind the entire integrated problem can put on hold and > dropped to be later reimplemented when it's time. (assuming that all the > prereq are already here and the very same implementation of qca8k will > be used) For the qca8386, it is true very same implementation of qca8k, besides the 2.5G capability and GCC clock involved. > > Anyway, I'm more or less the maintainer of the qca8k.c DSA driver and I > would be more than happy to help you guys internally or externally on > pushing and make this proceed further. (again assuming this is ipq9574 > stuff, it would be good to finally have proper DSA driver instead of > leaving the thing unusable as it's the current situation with ipq8074) Yes, the pure PHY chip qca8084 is connected on ipq9574 platform, and the switch chip qca8386 is connected on ipq5332 platform. Many thanks Christian again, will add you when pushing the qca8386 DSA patches for upstream review. > >> >> i qca8084 chip package includes 4 PHYs, 2 PCSs and the common chip level >> modules such as GCC and security control modules, all these modules are >> located in the qca8084 chip package, since qca8084 works on PHY mode, so >> the MACs are not used. >> >> qca8084 is connected with the SoC CHIP such as IPQ platform by PCS1 >> working on 10g-qxgmii mode and the fourth PHY can also optionally >> be connected with the IPQ SoC PCS by sgmii mode, there is no more >> interface on qca8084 to connect the external chips. >> >>> It's sounding like what we have here is some PHY IP that is integrated >>> into a larger SoC, and the larger SoC needs to be configured so the >>> PHY IP can work correctly. >> >> qca8084 is not a SoC, it is the Ethernet chip, in this qca8084 package, >> there are GCC that is driving the PHY working on the various link speed. >> that is the reason we need to do these package level common clocks and >> resets initialization before probing PHY correctly. >> >>> >>> Given that this package of four PHYs seems to be rather unique, I think >>> we need Jie Luo to provide sufficient information so we can understand: >>> >>> 1) this package of four PHYs itself >> >> Yes, this chip package for all 4 PHYs itself, also including the PCSes >> and common package level modules such as GCC. >> >>> 2) how this package is integrated into the SoC >> >> the qca8084 is connected with SoC by PCSes. >> >>> >>> Specifically, what resets and clocks are controlled from within the >>> package's register space, which are external to the package >>> register space (and thus are provided by other IPs in the SoC). >> >> All clocks and resets mentioned for qca8084 drive including package >> level and PCS & PHY clocks and resets from the qca8084 internal GCC >> modules register space, >> >>> >>> As I've said previously, the lack of DT example doesn't help to further >>> our understanding. The lack of details of what the package encompases >>> also doesn't help us understand the hardware. >> >> Indeed, i will add the qca8084 DT example in the next patch set. >> BTW, i also replied your earlier comments by providing the DTS defined >> for the current qca8084 drive code. >> >> hope you can have a better understanding with the provided DTS code in >> earlier reply of this email thread. >>> >>> Unless we can gain that understanding, I feel that Jie Luo's patches >>> are effectively unreviewable and can't be accepted into mainline. >>> >
> Thanks Andrew for the proposal. > For the pure PHY chip qca8084, there is no driver to parse the package > level device tree node for common clocks and resets. So you still have not look at the work Christian is doing. You must work together with Christian. This driver is not going to be accepted unless you do. > > > ethernet-phy@0 { > > > compatible = "ethernet-phy-id004d.d180"; > > > reg = <0>; > > > clocks = <qca8k_nsscc NSS_CC_GEPHY0_SYS_CLK>, > > > clock-names = <"gephy_sys">; > > > resets = <&qca8k_nsscc NSS_CC_GEPHY0_SYS_ARES>, > > > <&qca8k_nsscc NSS_CC_GEPHY0_ARES>; > > > reset-names = "gephy_sys", "gephy_soft"; Which of these properties exist for the Pure PHY device? Which exist for the integrated switch? And by that, i mean which are actual pins on the PHY device? We need the device tree binding to list which properties are required for each use case. Andrew
On 12/18/2023 5:34 PM, Andrew Lunn wrote: >> Thanks Andrew for the proposal. >> For the pure PHY chip qca8084, there is no driver to parse the package >> level device tree node for common clocks and resets. > > So you still have not look at the work Christian is doing. You must > work together with Christian. This driver is not going to be accepted > unless you do. OK, Andrew, i am looking at Christian's patches on at803x.c, and i will update qca8084 patches based on Christian's patch set. > >>>> ethernet-phy@0 { >>>> compatible = "ethernet-phy-id004d.d180"; >>>> reg = <0>; >>>> clocks = <qca8k_nsscc NSS_CC_GEPHY0_SYS_CLK>, >>>> clock-names = <"gephy_sys">; >>>> resets = <&qca8k_nsscc NSS_CC_GEPHY0_SYS_ARES>, >>>> <&qca8k_nsscc NSS_CC_GEPHY0_ARES>; >>>> reset-names = "gephy_sys", "gephy_soft"; > > Which of these properties exist for the Pure PHY device? Which exist > for the integrated switch? And by that, i mean which are actual pins > on the PHY device? We need the device tree binding to list which > properties are required for each use case. > > Andrew Hi Andrew, For the clocks and resets listed here, only the clock "mdio_master_ahb" is dedicated in qca8386, others are needed on the both chips qca8386 and qca8084. Here is the DTS example for the clocks and resets working on the devices, from the example below, we can get the dedicated clocks and resets for each MDIO device and package level device. The DTS properties in the "qcom,phy-common" should be initialized by the first PHY probe function, and only being initialized one time. phy0: ethernet-phy@0 { compatible = "ethernet-phy-id004d.d180"; reg = <1>; /* Package level configs, applicable on qca8386 and qca8081. */ phy-common-config { qcom,phy-addr-fixup = <1 2 3 4 5 6 7>; qcom,phy-work-mode = <2>; clocks = <&qca8k_nsscc NSS_CC_APB_BRIDGE_CLK>, <&qca8k_nsscc NSS_CC_AHB_CLK>, <&qca8k_nsscc NSS_CC_SEC_CTRL_AHB_CLK>, <&qca8k_nsscc NSS_CC_TLMM_CLK>, <&qca8k_nsscc NSS_CC_TLMM_AHB_CLK>, <&qca8k_nsscc NSS_CC_CNOC_AHB_CLK>, <&qca8k_nsscc NSS_CC_MDIO_AHB_CLK>; clock-names = "apb_bridge", "ahb", "sec_ctrl_ahb", "tlmm", "tlmm_ahb", "cnoc_ahb", "mdio_ahb"; resets = <&qca8k_nsscc NSS_CC_DSP_ARES>; reset-names = "gephy_dsp"; ethernet-ports { /* clocks and resets for pcs0. */ pcs0 { clocks = <&qca8k_nsscc NSS_CC_SRDS0_SYS_CLK>; clock-names = "srds0_sys"; resets = <&qca8k_nsscc NSS_CC_SRDS0_SYS_ARES>; reset-names = "srds0_sys"; }; /* clocks and resets for pcs1. */ pcs1 { clocks = <&qca8k_nsscc NSS_CC_SRDS1_SYS_CLK>; clock-names = "srds1_sys"; resets = <&qca8k_nsscc NSS_CC_SRDS1_SYS_ARES>; reset-names = "srds1_sys"; }; /* clocks and resets for first phy */ phy0 { clocks = <&qca8k_nsscc NSS_CC_GEPHY0_SYS_CLK>; clock-names = "gephy0_sys"; resets = <&qca8k_nsscc NSS_CC_GEPHY0_SYS_ARES>, <&qca8k_nsscc NSS_CC_GEPHY0_ARES>; reset-names = "gephy0_sys", "gephy0_soft"; }; /* clocks and resets for second phy */ phy1 { clocks = <&qca8k_nsscc NSS_CC_GEPHY1_SYS_CLK>; clock-names = "gephy1_sys"; resets = <&qca8k_nsscc NSS_CC_GEPHY1_SYS_ARES>, <&qca8k_nsscc NSS_CC_GEPHY1_ARES>; reset-names = "gephy1_sys", "gephy1_soft"; }; /* clocks and resets for third phy */ phy2 { clocks = <&qca8k_nsscc NSS_CC_GEPHY2_SYS_CLK>; clock-names = "gephy2_sys"; resets = <&qca8k_nsscc NSS_CC_GEPHY2_SYS_ARES>, <&qca8k_nsscc NSS_CC_GEPHY2_ARES>; reset-names = "gephy2_sys", "gephy2_soft"; }; /* clocks and resets for fourth phy */ phy3 { clocks = <&qca8k_nsscc NSS_CC_GEPHY3_SYS_CLK>; clock-names = "gephy3_sys"; resets = <&qca8k_nsscc NSS_CC_GEPHY3_SYS_ARES>, <&qca8k_nsscc NSS_CC_GEPHY3_ARES>; reset-names = "gephy3_sys", "gephy3_soft"; }; }; }; phy1: ethernet-phy@1 { compatible = "ethernet-phy-id004d.d180"; reg = <2>; }; phy2: ethernet-phy@2 { compatible = "ethernet-phy-id004d.d180"; reg = <3>; }; phy3: ethernet-phy@3 { compatible = "ethernet-phy-id004d.d180"; reg = <4>; };
On Mon, Dec 18, 2023 at 11:01:03AM +0800, Jie Luo wrote: > > > On 12/17/2023 12:09 AM, Russell King (Oracle) wrote: > > On Sat, Dec 16, 2023 at 10:41:28PM +0800, Jie Luo wrote: > > > > > > > > > On 12/16/2023 9:51 PM, Russell King (Oracle) wrote: > > > > On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote: > > > > > > The following is the chip package, the chip can work on the switch mode > > > > > > like the existed upstream code qca8k, where PHY1-PHY4 is connected with > > > > > > MAC1-MAC4 directly; > > > > > > > > > > Ah, that is new information, and has a big effect on the design. > > > > > > > > This QCA8084 that's being proposed in these patches is not a PHY in > > > > itself, but is a SoC. I came across this: > > > > > > > > https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/ > > > > > > The chip mentioned in the link you mentioned is SoC, which is not the > > > chip that the qca8084 driver work for. > > > > So there's two chips called QCA8084 both produced by Qualcomm? I find > > that hard to believe. > > > > The SoC mentioned in the link you provided is the APQ8084 that is introduced > in the link below: > https://www.qualcomm.com/products/mobile/snapdragon/smartphones/snapdragon-8-series-mobile-platforms/snapdragon-processors-805 So the one mentioned in the rt-rk article and a load of CVEs is _not_ QCA8084 but is APQ8084. Sounds like a lot of people are getting stuff wrong - which is hardly surprising as there are people that seem to _enjoy_ getting the technical details wrong. I haven't worked out if it's intentional malace, or they're just fundamentally lazy individuals who just like to screw with other people. Sigh.
On Tue, Jan 02, 2024 at 09:57:48AM +0000, Russell King (Oracle) wrote: > On Mon, Dec 18, 2023 at 11:01:03AM +0800, Jie Luo wrote: > > > > > > On 12/17/2023 12:09 AM, Russell King (Oracle) wrote: > > > On Sat, Dec 16, 2023 at 10:41:28PM +0800, Jie Luo wrote: > > > > > > > > > > > > On 12/16/2023 9:51 PM, Russell King (Oracle) wrote: > > > > > On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote: > > > > > > > The following is the chip package, the chip can work on the switch mode > > > > > > > like the existed upstream code qca8k, where PHY1-PHY4 is connected with > > > > > > > MAC1-MAC4 directly; > > > > > > > > > > > > Ah, that is new information, and has a big effect on the design. > > > > > > > > > > This QCA8084 that's being proposed in these patches is not a PHY in > > > > > itself, but is a SoC. I came across this: > > > > > > > > > > https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/ > > > > > > > > The chip mentioned in the link you mentioned is SoC, which is not the > > > > chip that the qca8084 driver work for. > > > > > > So there's two chips called QCA8084 both produced by Qualcomm? I find > > > that hard to believe. > > > > > > > The SoC mentioned in the link you provided is the APQ8084 that is introduced > > in the link below: > > https://www.qualcomm.com/products/mobile/snapdragon/smartphones/snapdragon-8-series-mobile-platforms/snapdragon-processors-805 > > So the one mentioned in the rt-rk article and a load of CVEs is _not_ > QCA8084 but is APQ8084. Sounds like a lot of people are getting stuff > wrong - which is hardly surprising as there are people that seem to > _enjoy_ getting the technical details wrong. I haven't worked out if > it's intentional malace, or they're just fundamentally lazy individuals > who just like to screw with other people. > > Sigh. > Hoping to give some clarification with the naming. - APQ8084 ("Application" SoC for 8084 family) - IPQ8084 ("Internet" SoC version of APQ8084) - QCA8084 (Integrated PHYs in the IPQ8084 SoC) I guess? Considering QCA8084 is only in in IPQ8084 SoC, the confusion with referring to it is in the fact that it's all the same thing, and everything related to APQ is also related to IPQ since they are the same SoC with minor difference (different DSP, presence of NSS cores) I can totally see sencente like "The IPQ8084 PHYs..." referencing the QCA8084 PHY. (Just to put how the naming is confusing there are PMIC with the same exact naming)
On 1/2/2024 6:08 PM, Christian Marangi wrote: > On Tue, Jan 02, 2024 at 09:57:48AM +0000, Russell King (Oracle) wrote: >> On Mon, Dec 18, 2023 at 11:01:03AM +0800, Jie Luo wrote: >>> >>> >>> On 12/17/2023 12:09 AM, Russell King (Oracle) wrote: >>>> On Sat, Dec 16, 2023 at 10:41:28PM +0800, Jie Luo wrote: >>>>> >>>>> >>>>> On 12/16/2023 9:51 PM, Russell King (Oracle) wrote: >>>>>> On Sat, Dec 16, 2023 at 11:21:53AM +0100, Andrew Lunn wrote: >>>>>>>> The following is the chip package, the chip can work on the switch mode >>>>>>>> like the existed upstream code qca8k, where PHY1-PHY4 is connected with >>>>>>>> MAC1-MAC4 directly; >>>>>>> >>>>>>> Ah, that is new information, and has a big effect on the design. >>>>>> >>>>>> This QCA8084 that's being proposed in these patches is not a PHY in >>>>>> itself, but is a SoC. I came across this: >>>>>> >>>>>> https://www.rt-rk.com/android-tv-solution-tv-in-smartphone-pantsstb-based-on-qualcomm-soc-design/ >>>>> >>>>> The chip mentioned in the link you mentioned is SoC, which is not the >>>>> chip that the qca8084 driver work for. >>>> >>>> So there's two chips called QCA8084 both produced by Qualcomm? I find >>>> that hard to believe. >>>> >>> >>> The SoC mentioned in the link you provided is the APQ8084 that is introduced >>> in the link below: >>> https://www.qualcomm.com/products/mobile/snapdragon/smartphones/snapdragon-8-series-mobile-platforms/snapdragon-processors-805 >> >> So the one mentioned in the rt-rk article and a load of CVEs is _not_ >> QCA8084 but is APQ8084. Sounds like a lot of people are getting stuff >> wrong - which is hardly surprising as there are people that seem to >> _enjoy_ getting the technical details wrong. I haven't worked out if >> it's intentional malace, or they're just fundamentally lazy individuals >> who just like to screw with other people. >> >> Sigh. >> > > Hoping to give some clarification with the naming. > - APQ8084 ("Application" SoC for 8084 family) > - IPQ8084 ("Internet" SoC version of APQ8084) > - QCA8084 (Integrated PHYs in the IPQ8084 SoC) > > I guess? > > Considering QCA8084 is only in in IPQ8084 SoC, the confusion with > referring to it is in the fact that it's all the same thing, and > everything related to APQ is also related to IPQ since they are the same > SoC with minor difference (different DSP, presence of NSS cores) > > I can totally see sencente like "The IPQ8084 PHYs..." referencing the > QCA8084 PHY. > > (Just to put how the naming is confusing there are PMIC with the > same exact naming) > There should be NO IPQ8084. Yes, APQ8084 is the application SoC. QCA8084 is the pure PHY chip which has quad-phy. The prefix QCA is the Ethernet device, like qca8081(PHY chip), qca8386( switch chip) and qca8084(PHY chip). The prefix IPQ is the internet processor SoC, like ipq5332.
> Yes, APQ8084 is the application SoC. > QCA8084 is the pure PHY chip which has quad-phy. I think everybody agrees these are terrible names, being so close together but being very different devices. You have the issues of not giving clear explanations of your hardware. This is resulting in a lot of wasted tome for everybody. So please make your explanations very clear. I personally would avoid using APQ8084 or QCA8084 on there own. Always say the application SoC APQ8084, or the PHY chip QCA8084, or the switch embedded within the application processor APQ8084, or the PHYs embedded within the Application processor etc. This is particularly important when talking about clocks and resets, since the PHYs embedded within the application processor are likely to have different clocks and reset controllers to the PHY chip QCA8084. Andrew
On 1/3/2024 10:22 PM, Andrew Lunn wrote: >> Yes, APQ8084 is the application SoC. >> QCA8084 is the pure PHY chip which has quad-phy. > > I think everybody agrees these are terrible names, being so close > together but being very different devices. > > You have the issues of not giving clear explanations of your > hardware. This is resulting in a lot of wasted tome for everybody. S > please make your explanations very clear. I personally would avoid > using APQ8084 or QCA8084 on there own. Always say the application SoC > APQ8084, or the PHY chip QCA8084, or the switch embedded within the > application processor APQ8084, or the PHYs embedded within the > Application processor etc. This is particularly important when talking > about clocks and resets, since the PHYs embedded within the > application processor are likely to have different clocks and reset > controllers to the PHY chip QCA8084. > > Andrew Let me explain it more. APQ8084 is the Snapdragon SoC(for smart phone or other applicaiton) according to the link below. https://www.qualcomm.com/products/mobile/snapdragon/smartphones/snapdragon-8-series-mobile-platforms/snapdragon-processors-805. which has nothing to do with QCA8084 or IPQ SoC we are discussing here. let's remove out the APQ SoC from the discussion here. 1. For IPQ SoC series, there are only ipq4019, ipq5018, ipq6018, ipq8074 documented in the current dt-bindings doc qcom,ipq4019-mdio.yaml and ipq9574, ipq5332 that are being added by the MDIO patch, and one more ipq8064 whose MDIO driver is mdio-ipq8064.c, on more others. 2. For qca8084(pure PHY chip), which is the quad-phy chip, which is just like qca8081 PHY(single port PHY), each port can be linked to maximum speed 2.5G. For qca8386(switch chip), which includes the same PHY CHIP as qca8084 (4 physical ports and two CPU ports), qca8386 switch can work with the current qca8k.c DSA driver with the supplement patches. Both qca8084 and qca8386 includes same network clock controller(let's call it NSSCC, since this clock controller is located in the Ethernet chip qca8084 and qca8386), they have the same clock initial configuration sequence to initialize the Ethernet chip. The Ethernet chip qca8084 and qca8386 are only connected with IPQ SoC, Currently qca8084 is connected with IPQ SoC by 10G-QXGMII mode. the 4 PHYs of qca8386 are connected with the internal MAC of qca8386 by GMII, the maximum speed is also 2.5G. The port4 of qca8084 or qca8386 is optionally be able to connected with IPQ SoC by sgmii.
> 1. For IPQ SoC series, there are only ipq4019, ipq5018, ipq6018, > ipq8074 documented in the current dt-bindings doc qcom,ipq4019-mdio.yaml > and ipq9574, ipq5332 that are being added by the MDIO patch, and one > more ipq8064 whose MDIO driver is mdio-ipq8064.c, on more others. > > 2. For qca8084(pure PHY chip), which is the quad-phy chip, which is just > like qca8081 PHY(single port PHY), each port can be linked to maximum > speed 2.5G. > > For qca8386(switch chip), which includes the same PHY CHIP as qca8084 > (4 physical ports and two CPU ports), qca8386 switch can work with > the current qca8k.c DSA driver with the supplement patches. Is the qca8386 purely a switch plus integrated PHYs? There is no CPU on it? What is the management path? MDIO? > > Both qca8084 and qca8386 includes same network clock controller(let's > call it NSSCC, since this clock controller is located in the > Ethernet chip qca8084 and qca8386), they have the same clock initial > configuration sequence to initialize the Ethernet chip. You said For "qca8084(pure PHY chip)". Here you just called it an Ethernet chip? To me, and Ethernet chip is a MAC, Intel e1000e etc. Do you now see how your explanations are confusing. Is it s pure PHY, or is it an Ethernet chip? O.K. Since we are getting nowhere at the moment, lets take just the pure PHY chip, and ignore the rest for the moment. For any pure PHY, there is generally one clock input, which might be a crystal, or an actual clock. If you look at other DT bindings for PHYs, it is only listed if the clock is expected to come from somewhere else, like a SoC, and it needs to be turned on before the PHY will work. And generally, a pure PHY has one defined clock frequency input. If that is true, there is no need to specify the clock. If multiple clock input frequencies are supported, then you do need to specify the clock, so its possible to work out what frequency it is using. How that clock input is then used internally in the PHY is not described in DT, but the driver can set any dividers, PLLs needed etc. So, for the pure PHY chip, what is the pinout? Is there one clock input? Or 4 clock inputs, one per PHY in the quad package? Typically, where does this/these clocks come from? Is the frequency fixed by the design, or are a number of input frequencies supported? > The Ethernet chip qca8084 and qca8386 are only connected with IPQ SoC, > Currently qca8084 is connected with IPQ SoC by 10G-QXGMII mode. > the 4 PHYs of qca8386 are connected with the internal MAC of qca8386 > by GMII, the maximum speed is also 2.5G. > The port4 of qca8084 or qca8386 is optionally be able to connected > with IPQ SoC by sgmii. To some extent, this does not matter. The DT binding and the driver should not care what the pure PHY is connected to. It has standardised ports, so in theory it could be connected to any vendors MAC. Please be very careful with your wording. Because computers instructions should be unambiguous, it does what it is told, we also expect computer scientists to be unambiguous. Wording is very important. Andrew
On 1/4/2024 9:57 PM, Andrew Lunn wrote: >> 1. For IPQ SoC series, there are only ipq4019, ipq5018, ipq6018, >> ipq8074 documented in the current dt-bindings doc qcom,ipq4019-mdio.yaml >> and ipq9574, ipq5332 that are being added by the MDIO patch, and one >> more ipq8064 whose MDIO driver is mdio-ipq8064.c, on more others. >> >> 2. For qca8084(pure PHY chip), which is the quad-phy chip, which is just >> like qca8081 PHY(single port PHY), each port can be linked to maximum >> speed 2.5G. >> >> For qca8386(switch chip), which includes the same PHY CHIP as qca8084 >> (4 physical ports and two CPU ports), qca8386 switch can work with >> the current qca8k.c DSA driver with the supplement patches. > > Is the qca8386 purely a switch plus integrated PHYs? There is no CPU > on it? What is the management path? MDIO? Yes, qca8386 is a pure switch plus integrated PHYs(same PHY type as qca8084), there is no CPU on qca8386, the management path is MDIO. the access of switch register is by the multiple MDIO operations. > >> >> Both qca8084 and qca8386 includes same network clock controller(let's >> call it NSSCC, since this clock controller is located in the >> Ethernet chip qca8084 and qca8386), they have the same clock initial >> configuration sequence to initialize the Ethernet chip. > > You said For "qca8084(pure PHY chip)". Here you just called it an > Ethernet chip? To me, and Ethernet chip is a MAC, Intel e1000e etc. > Do you now see how your explanations are confusing. Is it s pure PHY, > or is it an Ethernet chip? My bad, sorry for this confusion. qca8084 is a pure PHY, there is no MAC in qca8084. > > O.K. Since we are getting nowhere at the moment, lets take just the > pure PHY chip, and ignore the rest for the moment. > > For any pure PHY, there is generally one clock input, which might be a > crystal, or an actual clock. If you look at other DT bindings for > PHYs, it is only listed if the clock is expected to come from > somewhere else, like a SoC, and it needs to be turned on before the > PHY will work. And generally, a pure PHY has one defined clock > frequency input. If that is true, there is no need to specify the > clock. If multiple clock input frequencies are supported, then you do > need to specify the clock, so its possible to work out what frequency > it is using. How that clock input is then used internally in the PHY > is not described in DT, but the driver can set any dividers, PLLs > needed etc. Yes, Andrew, there is only one clock input to qca8084(same as qca8386), this input clock rate is 50MHZ, which is from the output clock of CMN PLL block that is configured by the MDIO bus driver patch under review. In qca8084(same as qca8386), there is a clock controller, let's call it as NSSCC, the logic of NSSCC is same as qualcomm GCC(located in SoC), the NSSCC provides the clocks to the quad PHYs, the initial clocks for quad PHYs need to be configured before PHY to work. These clocks and resets are provided by the NSSCC provider driver, i need to define these clocks and resets in DT to use it. > > So, for the pure PHY chip, what is the pinout? Is there one clock > input? Or 4 clock inputs, one per PHY in the quad package? Typically, > where does this/these clocks come from? Is the frequency fixed by the > design, or are a number of input frequencies supported? There is one 50M clock input for qca8084(same as qca8386), the input clock is generated from the CMN PLL block that is configured by MDIO driver patch of mdio-ipq4019.c. The frequency of input clock is fixed to 50MHZ. > >> The Ethernet chip qca8084 and qca8386 are only connected with IPQ SoC, >> Currently qca8084 is connected with IPQ SoC by 10G-QXGMII mode. >> the 4 PHYs of qca8386 are connected with the internal MAC of qca8386 >> by GMII, the maximum speed is also 2.5G. >> The port4 of qca8084 or qca8386 is optionally be able to connected >> with IPQ SoC by sgmii. > > To some extent, this does not matter. The DT binding and the driver > should not care what the pure PHY is connected to. It has standardised > ports, so in theory it could be connected to any vendors MAC. Yes, it can be connected with any vendors MAC with the interface mode supported. > > Please be very careful with your wording. Because computers > instructions should be unambiguous, it does what it is told, we also > expect computer scientists to be unambiguous. Wording is very > important. > > Andrew Got it. Thanks Andrew for the comments and suggestions.
> > O.K. Since we are getting nowhere at the moment, lets take just the > > pure PHY chip, and ignore the rest for the moment. > > > > For any pure PHY, there is generally one clock input, which might be a > > crystal, or an actual clock. If you look at other DT bindings for > > PHYs, it is only listed if the clock is expected to come from > > somewhere else, like a SoC, and it needs to be turned on before the > > PHY will work. And generally, a pure PHY has one defined clock > > frequency input. If that is true, there is no need to specify the > > clock. If multiple clock input frequencies are supported, then you do > > need to specify the clock, so its possible to work out what frequency > > it is using. How that clock input is then used internally in the PHY > > is not described in DT, but the driver can set any dividers, PLLs > > needed etc. > > Yes, Andrew, there is only one clock input to qca8084(same as qca8386), > this input clock rate is 50MHZ, which is from the output clock of CMN > PLL block that is configured by the MDIO bus driver patch under review. Lets concentrate on the pure PHY. All it sees is a clock. It does not care where it come from. All you need in the device tree for the pure PHY is a clock consumer. There is one clock input, so its shared by all four instances in the pure PHY package. So you need to use Christians code which extends the PHY DT bindings to allow DT properties for a package of PHYs. What about resets. Is there one reset pin for the pure PHY package, or one per PHY? Go find Christians code, understand it, and propose a DT binding for the pure PHY. Include the clock provider and the reset provider. Forget about the MDIO controller, and the PHY integrated into the switch, etc. Baby steps... > In qca8084(same as qca8386), there is a clock controller, let's call it > as NSSCC, the logic of NSSCC is same as qualcomm GCC(located in SoC), > the NSSCC provides the clocks to the quad PHYs, the initial clocks for > quad PHYs need to be configured before PHY to work. You said above, there is one clock input to the qca8084. Here you use the word clocks, plural. Is there one clock, or multiple clocks? Andrew
On 1/5/2024 9:37 PM, Andrew Lunn wrote: >>> O.K. Since we are getting nowhere at the moment, lets take just the >>> pure PHY chip, and ignore the rest for the moment. >>> >>> For any pure PHY, there is generally one clock input, which might be a >>> crystal, or an actual clock. If you look at other DT bindings for >>> PHYs, it is only listed if the clock is expected to come from >>> somewhere else, like a SoC, and it needs to be turned on before the >>> PHY will work. And generally, a pure PHY has one defined clock >>> frequency input. If that is true, there is no need to specify the >>> clock. If multiple clock input frequencies are supported, then you do >>> need to specify the clock, so its possible to work out what frequency >>> it is using. How that clock input is then used internally in the PHY >>> is not described in DT, but the driver can set any dividers, PLLs >>> needed etc. >> >> Yes, Andrew, there is only one clock input to qca8084(same as qca8386), >> this input clock rate is 50MHZ, which is from the output clock of CMN >> PLL block that is configured by the MDIO bus driver patch under review. > > Lets concentrate on the pure PHY. All it sees is a clock. It does not > care where it come from. All you need in the device tree for the pure > PHY is a clock consumer. Yes. > > There is one clock input, so its shared by all four instances in the > pure PHY package. So you need to use Christians code which extends the > PHY DT bindings to allow DT properties for a package of PHYs. OK, will > > What about resets. Is there one reset pin for the pure PHY package, or > one per PHY? There is only one GPIO hardware reset PIN for the chip qca8084 including all 4 PHYs. > > Go find Christians code, understand it, and propose a DT binding for > the pure PHY. Include the clock provider and the reset > provider. Forget about the MDIO controller, and the PHY integrated > into the switch, etc. Baby steps... Thanks Andrew for pointing me the Christians code, i will keep the driver of qca8084 synced with Christian's code before pushing the next patch set. > >> In qca8084(same as qca8386), there is a clock controller, let's call it >> as NSSCC, the logic of NSSCC is same as qualcomm GCC(located in SoC), >> the NSSCC provides the clocks to the quad PHYs, the initial clocks for >> quad PHYs need to be configured before PHY to work. > > You said above, there is one clock input to the qca8084. Here you use > the word clocks, plural. Is there one clock, or multiple clocks? > > Andrew Yes, Andrew, it is multiple clocks. These multiple clocks are generated(PLL, divider) and used internally by qca8084 CHIP, these clocks are generated by the clock controller of qca8084, let's call the clock controller of qca8084 as NSSCC provider, which generates the clocks to the PHYs, this NSSCC is located in qca8084. The only one input clock of qca8084 is the clock source of the chip qca8084, which is fixed to 50MHZ. The NSSCC of qca8084 generates different clock rates for the different link speed of the PHY, which is the internal block of qca8084.
diff --git a/Documentation/devicetree/bindings/net/qca,ar803x.yaml b/Documentation/devicetree/bindings/net/qca,ar803x.yaml index 3acd09f0da86..febff039a44f 100644 --- a/Documentation/devicetree/bindings/net/qca,ar803x.yaml +++ b/Documentation/devicetree/bindings/net/qca,ar803x.yaml @@ -14,9 +14,6 @@ maintainers: description: | Bindings for Qualcomm Atheros AR803x PHYs -allOf: - - $ref: ethernet-phy.yaml# - properties: qca,clk-out-frequency: description: Clock output frequency in Hertz. @@ -85,6 +82,161 @@ properties: $ref: /schemas/regulator/regulator.yaml unevaluatedProperties: false + qcom,phy-addr-fixup: + $ref: /schemas/types.yaml#/definitions/uint32-array + description: + MDIO address for 4 PHY devices and 3 PCS devices + + qcom,phy-work-mode: + description: PHY device work mode. + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2, 3] + + clocks: + items: + - description: APB bridge clock + - description: AHB clock + - description: Security control clock + - description: TLMM clock + - description: TLMM AHB clock + - description: CNOC AHB clock + - description: MDIO AHB clock + - description: MDIO master AHB clock + - description: PCS0 system clock + - description: PCS1 system clock + - description: EPHY0 system clock + - description: EPHY1 system clock + - description: EPHY2 system clock + - description: EPHY3 system clock + description: PHY initial common clock configs + + clock-names: + items: + - const: apb_bridge + - const: ahb + - const: sec_ctrl_ahb + - const: tlmm + - const: tlmm_ahb + - const: cnoc_ahb + - const: mdio_ahb + - const: mdio_master_ahb + - const: srds0_sys + - const: srds1_sys + - const: gephy0_sys + - const: gephy1_sys + - const: gephy2_sys + - const: gephy3_sys + + resets: + items: + - description: PCS0 system reset + - description: PCS1 system reset + - description: EPHY0 system reset + - description: EPHY1 system reset + - description: EPHY2 system reset + - description: EPHY3 system reset + - description: EPHY0 software reset + - description: EPHY1 software reset + - description: EPHY2 software reset + - description: EPHY3 software reset + - description: Ethernet DSP reset + description: PHY initial common reset configs + + reset-names: + items: + - const: srds0_sys + - const: srds1_sys + - const: gephy0_sys + - const: gephy1_sys + - const: gephy2_sys + - const: gephy3_sys + - const: gephy0_soft + - const: gephy1_soft + - const: gephy2_soft + - const: gephy3_soft + - const: gephy_dsp + +allOf: + - $ref: ethernet-phy.yaml# + + - if: + properties: + compatible: + contains: + enum: + - ethernet-phy-id004d.d180 + then: + properties: + clocks: + items: + - description: APB bridge clock + - description: AHB clock + - description: Security control clock + - description: TLMM clock + - description: TLMM AHB clock + - description: CNOC AHB clock + - description: MDIO AHB clock + - description: MDIO master AHB clock + - description: PCS0 system clock + - description: PCS1 system clock + - description: EPHY0 system clock + - description: EPHY1 system clock + - description: EPHY2 system clock + - description: EPHY3 system clock + clock-names: + items: + - const: apb_bridge + - const: ahb + - const: sec_ctrl_ahb + - const: tlmm + - const: tlmm_ahb + - const: cnoc_ahb + - const: mdio_ahb + - const: mdio_master_ahb + - const: srds0_sys + - const: srds1_sys + - const: gephy0_sys + - const: gephy1_sys + - const: gephy2_sys + - const: gephy3_sys + resets: + items: + - description: PCS0 system reset + - description: PCS1 system reset + - description: EPHY0 system reset + - description: EPHY1 system reset + - description: EPHY2 system reset + - description: EPHY3 system reset + - description: EPHY0 software reset + - description: EPHY1 software reset + - description: EPHY2 software reset + - description: EPHY3 software reset + - description: Ethernet DSP reset + reset-names: + items: + - const: srds0_sys + - const: srds1_sys + - const: gephy0_sys + - const: gephy1_sys + - const: gephy2_sys + - const: gephy3_sys + - const: gephy0_soft + - const: gephy1_soft + - const: gephy2_soft + - const: gephy3_soft + - const: gephy_dsp + required: + - qcom,phy-addr-fixup + - qcom,phy-work-mode + - clocks + - clock-names + - resets + - reset-names + else: + properties: + qcom,phy-addr-fixup: false + qcom,phy-work-mode: false + unevaluatedProperties: false examples:
The following properties are added for qca8084 PHY. 1. add the compatible string "ethernet-phy-id004d.d180" since the PHY device is not accessible during MDIO bus register. 2. add property "qcom,phy-addr-fixup" for customizing MDIO address. 3. add property "qcom,phy-work-mode" for specifying qca8084 PHY work mode. 4. add the initial clocks and resets. Signed-off-by: Luo Jie <quic_luoj@quicinc.com> --- .../devicetree/bindings/net/qca,ar803x.yaml | 158 +++++++++++++++++- 1 file changed, 155 insertions(+), 3 deletions(-)