diff mbox series

[1/2] dt-bindings: net: phy: Support external PHY xtal

Message ID 20230531150340.522994-1-detlev.casanova@collabora.com
State New
Headers
Series [1/2] dt-bindings: net: phy: Support external PHY xtal |

Commit Message

Detlev Casanova May 31, 2023, 3:03 p.m. UTC 
  Ethernet PHYs can have external an clock that needs to be activated before
probing the PHY.

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

Comments

Andrew Lunn May 31, 2023, 3:16 p.m. UTC | #1 
On Wed, May 31, 2023 at 11:03:39AM -0400, Detlev Casanova wrote:
> Ethernet PHYs can have external an clock that needs to be activated before
> probing the PHY.
> 
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
>  .../devicetree/bindings/net/ethernet-phy.yaml          | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index 4f574532ee13..e83a33c2aa59 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -93,6 +93,16 @@ properties:
>        the turn around line low at end of the control phase of the
>        MDIO transaction.
>  
> +  clock-names:
> +    items:
> +      - const: xtal

I don't think xtal is the best of names here. It generally is used as
an abbreviation for crystal. And the commit message is about there not
being a crystal, but an actual clock.

How is this clock named on the datasheet?

    Andrew
Detlev Casanova May 31, 2023, 6 p.m. UTC | #2 
On Wednesday, May 31, 2023 11:16:46 A.M. EDT Andrew Lunn wrote:
> On Wed, May 31, 2023 at 11:03:39AM -0400, Detlev Casanova wrote:
> > Ethernet PHYs can have external an clock that needs to be activated before
> > probing the PHY.
> > 
> > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > ---
> > 
> >  .../devicetree/bindings/net/ethernet-phy.yaml          | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > b/Documentation/devicetree/bindings/net/ethernet-phy.yaml index
> > 4f574532ee13..e83a33c2aa59 100644
> > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > 
> > @@ -93,6 +93,16 @@ properties:
> >        the turn around line low at end of the control phase of the
> >        MDIO transaction.
> > 
> > +  clock-names:
> > +    items:
> > +      - const: xtal
> 
> I don't think xtal is the best of names here. It generally is used as
> an abbreviation for crystal. And the commit message is about there not
> being a crystal, but an actual clock.
> 
> How is this clock named on the datasheet?

In the case of the PHY I used (RTL8211F), it is EXT_CLK. But this must be 
generic to any (ethernet) PHY, so using ext_clk to match it would not be
good either.

Now this is about having an external clock, so the ext_clk name makes sense in 
this case.

I'm not pushing one name or another, let's use what you feel is more natural.

Detlev.
Florian Fainelli May 31, 2023, 6:05 p.m. UTC | #3 
On 5/31/23 11:00, Detlev Casanova wrote:
> On Wednesday, May 31, 2023 11:16:46 A.M. EDT Andrew Lunn wrote:
>> On Wed, May 31, 2023 at 11:03:39AM -0400, Detlev Casanova wrote:
>>> Ethernet PHYs can have external an clock that needs to be activated before
>>> probing the PHY.
>>>
>>> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
>>> ---
>>>
>>>   .../devicetree/bindings/net/ethernet-phy.yaml          | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
>>> b/Documentation/devicetree/bindings/net/ethernet-phy.yaml index
>>> 4f574532ee13..e83a33c2aa59 100644
>>> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
>>> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
>>>
>>> @@ -93,6 +93,16 @@ properties:
>>>         the turn around line low at end of the control phase of the
>>>         MDIO transaction.
>>>
>>> +  clock-names:
>>> +    items:
>>> +      - const: xtal
>>
>> I don't think xtal is the best of names here. It generally is used as
>> an abbreviation for crystal. And the commit message is about there not
>> being a crystal, but an actual clock.
>>
>> How is this clock named on the datasheet?
> 
> In the case of the PHY I used (RTL8211F), it is EXT_CLK. But this must be
> generic to any (ethernet) PHY, so using ext_clk to match it would not be
> good either.
> 
> Now this is about having an external clock, so the ext_clk name makes sense in
> this case.
> 
> I'm not pushing one name or another, let's use what you feel is more natural.

You can look up clocks by positional index, maybe this is a case where 
there are just too many names that PHY vendors will use that we should 
not be using one specific name in particular, but just define the order 
in which clocks should be specified.
Krzysztof Kozlowski June 1, 2023, 4:52 p.m. UTC | #4 
On 31/05/2023 20:00, Detlev Casanova wrote:
>>> +  clock-names:
>>> +    items:
>>> +      - const: xtal
>>
>> I don't think xtal is the best of names here. It generally is used as
>> an abbreviation for crystal. And the commit message is about there not
>> being a crystal, but an actual clock.
>>
>> How is this clock named on the datasheet?
> 
> In the case of the PHY I used (RTL8211F), it is EXT_CLK. But this must be 
> generic to any (ethernet) PHY, so using ext_clk to match it would not be
> good either.
> 
> Now this is about having an external clock, so the ext_clk name makes sense in 
> this case.
> 
> I'm not pushing one name or another, let's use what you feel is more natural.

Just drop the name.

Best regards,
Krzysztof
Detlev Casanova June 1, 2023, 6:11 p.m. UTC | #5 
On Thursday, June 1, 2023 12:52:18 P.M. EDT Krzysztof Kozlowski wrote:
> On 31/05/2023 20:00, Detlev Casanova wrote:
> >>> +  clock-names:
> >>> +    items:
> >>> +      - const: xtal
> >> 
> >> I don't think xtal is the best of names here. It generally is used as
> >> an abbreviation for crystal. And the commit message is about there not
> >> being a crystal, but an actual clock.
> >> 
> >> How is this clock named on the datasheet?
> > 
> > In the case of the PHY I used (RTL8211F), it is EXT_CLK. But this must be
> > generic to any (ethernet) PHY, so using ext_clk to match it would not be
> > good either.
> > 
> > Now this is about having an external clock, so the ext_clk name makes
> > sense in this case.
> > 
> > I'm not pushing one name or another, let's use what you feel is more
> > natural.
> Just drop the name.

So I can just use devm_clk_get_optional_enabled(dev, NULL) and I'll get the 
first clock defines in the device tree ?

Detlev.
Andrew Lunn June 1, 2023, 6:26 p.m. UTC | #6 
> So I can just use devm_clk_get_optional_enabled(dev, NULL) and I'll get the 
> first clock defines in the device tree ?

Yes:

bcm7xxx.c:	clk = devm_clk_get_optional_enabled(&phydev->mdio.dev, NULL);
micrel.c:	clk = devm_clk_get(&phydev->mdio.dev, "rmii-ref");
smsc.c:		refclk = devm_clk_get_optional_enabled(dev, NULL);

	Andrew
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 4f574532ee13..e83a33c2aa59 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -93,6 +93,16 @@  properties:
       the turn around line low at end of the control phase of the
       MDIO transaction.
 
+  clock-names:
+    items:
+      - const: xtal
+
+  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: