On Wed, Dec 13, 2023 at 04:26:56PM +0800, Jie Luo wrote:
>
>
> On 12/13/2023 12:06 AM, Conor Dooley wrote:
> > On Tue, Dec 12, 2023 at 07:51:50PM +0800, Luo Jie wrote:
> > > Update the yaml file for the new DTS properties.
> > >
> > > 1. cmn-reference-clock for the CMN PLL source clock select.
> > > 2. clock-frequency for MDIO clock frequency config.
> > > 3. add uniphy AHB & SYS GCC clocks.
> > > 4. add reset-gpios for MDIO bus level reset.
> > >
> > > Signed-off-by: Luo Jie <quic_luoj@xxxxxxxxxxx>
> > > ---
> > > .../bindings/net/qcom,ipq4019-mdio.yaml | 157 +++++++++++++++++-
> > > 1 file changed, 153 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> > > index 3407e909e8a7..9546a6ad7841 100644
> > > --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> > > +++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> > > @@ -20,6 +20,8 @@ properties:
> > > - enum:
> > > - qcom,ipq6018-mdio
> > > - qcom,ipq8074-mdio
> > > + - qcom,ipq9574-mdio
> > > + - qcom,ipq5332-mdio
> > > - const: qcom,ipq4019-mdio
> > > "#address-cells":
> > > @@ -30,19 +32,71 @@ properties:
> > > reg:
> > > minItems: 1
> > > - maxItems: 2
> > > + maxItems: 5
> > > description:
> > > - the first Address and length of the register set for the MDIO controller.
> > > - the second Address and length of the register for ethernet LDO, this second
> > > - address range is only required by the platform IPQ50xx.
> > > + the first Address and length of the register set for the MDIO controller,
> > > + the optional second, third and fourth address and length of the register
> > > + for ethernet LDO, these three address range are required by the platform
> > > + IPQ50xx/IPQ5332/IPQ9574, the last address and length is for the CMN clock
> > > + to select the reference clock.
> > > +
> > > + reg-names:
> > > + minItems: 1
> > > + maxItems: 5
> > > clocks:
> > > + minItems: 1
> > > items:
> > > - description: MDIO clock source frequency fixed to 100MHZ
> > > + - description: UNIPHY0 AHB clock source frequency fixed to 100MHZ
> > > + - description: UNIPHY1 AHB clock source frequency fixed to 100MHZ
> > > + - description: UNIPHY0 SYS clock source frequency fixed to 24MHZ
> > > + - description: UNIPHY1 SYS clock source frequency fixed to 24MHZ
> > > clock-names:
> > > + minItems: 1
> > > items:
> > > - const: gcc_mdio_ahb_clk
> > > + - const: gcc_uniphy0_ahb_clk
> > > + - const: gcc_uniphy1_ahb_clk
> > > + - const: gcc_uniphy0_sys_clk
> > > + - const: gcc_uniphy1_sys_clk
> >
> > > + cmn-reference-clock:
> > > + oneOf:
> > > + - items:
> > > + - enum:
> > > + - 0 # CMN PLL reference internal 48MHZ
> > > + - 1 # CMN PLL reference external 25MHZ
> > > + - 2 # CMN PLL reference external 31250KHZ
> > > + - 3 # CMN PLL reference external 40MHZ
> > > + - 4 # CMN PLL reference external 48MHZ
> > > + - 5 # CMN PLL reference external 50MHZ
> > > + - 6 # CMN PLL reference internal 96MHZ
> >
> > Why is this not represented by an element of the clocks property?
>
> This property is for the reference clock source selection of CMN PLL,
> CMN PLL generates the different clock rates for the different Ethernet
> blocks, this CMN PLL configuration is not located in the GCC, so the
> clock framework can't be used, which is the general hardware register
> instead of RCG register for GCC.
I don't see how the clock being provided by the "GCC" (whatever that is)
or by some other clock controller or fixed clock makes a difference.
Why can't the other clock provider be represented in the devicetree?
> > > + clock-frequency:
> > > + oneOf:
> > > + - items:
> > > + - enum:
> > > + - 12500000
> > > + - 6250000
> > > + - 3125000
> > > + - 1562500
> > > + - 781250
> > > + - 390625
> > > + description:
> > > + The MDIO bus clock that must be output by the MDIO bus hardware,
> > > + only the listed frequecies above can be configured, other frequency
> > > + will cause malfunction. If absent, the default hardware value is used.
> >
> > Likewise.
> >
> > Your commit message contains a bullet point list of what you are doing,
> > but there's no explanation here for why custom properties are required
> > to provide clock information.
> This property clock-frequency is optional to configure the MDIO working
> clock rate, and this is the MDIO general DT property, since the hardware
> default clock rate is 390625HZ, there is requirement for higher clock rate
> in the normal working case, i will update this information in the
> next patch set.
I'm just realising that this particular one is not a custom property,
the unusual `oneOf: - items: - enum:` structure here threw me. This can
just be
clock-frequency:
enum:
- 12500000
- 6250000
- 3125000
- 1562500
- 781250
- 390625
but you're missing a default, given your commit about the last element
in that list being one.
Thanks,
Conor.
Attachment:
signature.asc
Description: PGP signature
