diff mbox series

[v2,2/3] dt-bindings: net: phy: Document support for external PHY clk

Message ID 20230602182659.307876-3-detlev.casanova@collabora.com
State Superseded, archived
Headers show
Series None | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Detlev Casanova June 2, 2023, 6:26 p.m. UTC
Ethern PHYs can have external an clock that needs to be activated before
probing the PHY.

Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 Documentation/devicetree/bindings/net/ethernet-phy.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Andrew Lunn June 2, 2023, 6:42 p.m. UTC | #1
On Fri, Jun 02, 2023 at 02:26:58PM -0400, Detlev Casanova wrote:
> Ethern PHYs can have external an clock that needs to be activated before
> probing the PHY.

`Ethernet PHYs can have an external clock.`

We need to be careful with 'activated before probing the PHY'. phylib
itself will not activate the clock. You must be putting the IDs into
the compatible string, so the correct driver is loaded, and its probe
function is called. The probe itself enables the clock, so it is not
before probe, but during probe.

I'm picky about this because we have issues with enumerating the MDIO
bus to find PHYs. Some boards needs the PHY taking out of reset,
regulators enabled, clocks enabled etc, before the PHY will respond on
the bus. It is hard for the core to do this, before the probe. So we
recommend putting IDs in the compatible, so the driver probe function
to do any additional setup needed.

> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
>  Documentation/devicetree/bindings/net/ethernet-phy.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index 4f574532ee13..c1241c8a3b77 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -93,6 +93,12 @@ properties:
>        the turn around line low at end of the control phase of the
>        MDIO transaction.
>  
> +  clocks:
> +    maxItems: 1
> +    description:
> +      External clock connected to the PHY. If not specified it is assumed
> +      that the PHY uses a fixed crystal or an internal oscillator.

This text is good.

    Andrew

---
pw-bot: cr
Detlev Casanova June 2, 2023, 7:26 p.m. UTC | #2
On Friday, June 2, 2023 2:42:38 P.M. EDT Andrew Lunn wrote:
> On Fri, Jun 02, 2023 at 02:26:58PM -0400, Detlev Casanova wrote:
> > Ethern PHYs can have external an clock that needs to be activated before
> > probing the PHY.
> 
> `Ethernet PHYs can have an external clock.`
> 
> We need to be careful with 'activated before probing the PHY'. phylib
> itself will not activate the clock. You must be putting the IDs into
> the compatible string, so the correct driver is loaded, and its probe
> function is called. The probe itself enables the clock, so it is not
> before probe, but during probe.
> 
> I'm picky about this because we have issues with enumerating the MDIO
> bus to find PHYs. Some boards needs the PHY taking out of reset,
> regulators enabled, clocks enabled etc, before the PHY will respond on
> the bus. It is hard for the core to do this, before the probe. So we
> recommend putting IDs in the compatible, so the driver probe function
> to do any additional setup needed.

That makes sense, In my head, "probing" == calling phy_write/read() functions. 
But I get how this could be confused with the _probe() function. (And I just 
realised that there are typos)

What about "Ethernet PHYs can have an external clock that needs to be 
activated before communicating with the PHY" ?

> > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > ---
> > 
> >  Documentation/devicetree/bindings/net/ethernet-phy.yaml | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > b/Documentation/devicetree/bindings/net/ethernet-phy.yaml index
> > 4f574532ee13..c1241c8a3b77 100644
> > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > 
> > @@ -93,6 +93,12 @@ properties:
> >        the turn around line low at end of the control phase of the
> >        MDIO transaction.
> > 
> > +  clocks:
> > +    maxItems: 1
> > +    description:
> > +      External clock connected to the PHY. If not specified it is assumed
> > +      that the PHY uses a fixed crystal or an internal oscillator.
> 
> This text is good.

Detlev
Andrew Lunn June 2, 2023, 7:43 p.m. UTC | #3
> What about "Ethernet PHYs can have an external clock that needs to be 
> activated before communicating with the PHY" ?

That good.

Thanks
	Andrew
Krzysztof Kozlowski June 3, 2023, 10:27 a.m. UTC | #4
On 02/06/2023 20:26, Detlev Casanova wrote:
> Ethern PHYs can have external an clock that needs to be activated before
> probing the PHY.
> 
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
>  Documentation/devicetree/bindings/net/ethernet-phy.yaml | 6 ++++++

With fixes from Andrew:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 4f574532ee13..c1241c8a3b77 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -93,6 +93,12 @@  properties:
       the turn around line low at end of the control phase of the
       MDIO transaction.
 
+  clocks:
+    maxItems: 1
+    description:
+      External clock connected to the PHY. If not specified it is assumed
+      that the PHY uses a fixed crystal or an internal oscillator.
+
   enet-phy-lane-swap:
     $ref: /schemas/types.yaml#/definitions/flag
     description: