[PATCH net-next V3] dt-bindings: net: ethernet-controller: Add ptp-hardware-clock

[PATCH net-next V3] dt-bindings: net: ethernet-controller: Add ptp-hardware-clock

[Sarath Babu Naidu Gaddam]

There is currently no standard property to pass PTP device index
information to ethernet driver when they are independent.

ptp-hardware-clock property will contain phandle to PTP clock node.

Its a generic (optional) property name to link to PTP phandle to
Ethernet node. Any future or current ethernet drivers that need
a reference to the PHC used on their system can simply use this
generic property name instead of using custom property
implementation in their device tree nodes."

Signed-off-by: Sarath Babu Naidu Gaddam <sarath.babu.naidu.gaddam@amd.com>
Acked-by: Richard Cochran <richardcochran@gmail.com>
---

Freescale driver currently has this implementation but it will be
good to agree on a generic (optional) property name to link to PTP
phandle to Ethernet node. In future or any current ethernet driver
wants to use this method of reading the PHC index,they can simply use
this generic name and point their own PTP clock node, instead of
creating separate property names in each ethernet driver DT node.

axiethernet driver uses this method when PTP support is integrated.

Example:
    fman0: fman@1a00000 {
        ptp-hardware-clock = <&ptp_timer0>;
    }

    ptp_timer0: ptp-timer@1afe000 {
        compatible = "fsl,fman-ptp-timer";
        reg = <0x0 0x1afe000 0x0 0x1000>;
    }

DT information:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/qoriq-fman3-0.dtsi#n23

Freescale driver:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c#n407

Changes in V3:
1) Updated commit description.
2) Add Acked-by: Richard Cochran.

Changes in V2:
1) Changed the ptimer-handle to ptp-hardware-clock based on
   Richard Cochran's comment.
2) Updated commit description.
---
 .../devicetree/bindings/net/ethernet-controller.yaml         | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index 00be387984ac..a97ab25b07a5 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -161,6 +161,11 @@ properties:
       - auto
       - in-band-status
 
+  ptp-hardware-clock:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Specifies a reference to a node representing a IEEE1588 timer.
+
   fixed-link:
     oneOf:
       - $ref: /schemas/types.yaml#/definitions/uint32-array
-- 
2.25.1

[PATCH net-next V3] dt-bindings: net: ethernet-controller: Add ptp-hardware-clock

[Sarath Babu Naidu Gaddam]

There is currently no standard property to pass PTP device index
information to ethernet driver when they are independent.

ptp-hardware-clock property will contain phandle to PTP clock node.

Its a generic (optional) property name to link to PTP phandle to
Ethernet node. Any future or current ethernet drivers that need
a reference to the PHC used on their system can simply use this
generic property name instead of using custom property
implementation in their device tree nodes."

Signed-off-by: Sarath Babu Naidu Gaddam <sarath.babu.naidu.gaddam@amd.com>
Acked-by: Richard Cochran <richardcochran@gmail.com>
---

Freescale driver currently has this implementation but it will be
good to agree on a generic (optional) property name to link to PTP
phandle to Ethernet node. In future or any current ethernet driver
wants to use this method of reading the PHC index,they can simply use
this generic name and point their own PTP clock node, instead of
creating separate property names in each ethernet driver DT node.

axiethernet driver uses this method when PTP support is integrated.

Example:
    fman0: fman@1a00000 {
        ptp-hardware-clock = <&ptp_timer0>;
    }

    ptp_timer0: ptp-timer@1afe000 {
        compatible = "fsl,fman-ptp-timer";
        reg = <0x0 0x1afe000 0x0 0x1000>;
    }

DT information:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/qoriq-fman3-0.dtsi#n23

Freescale driver:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c#n407

Changes in V3:
1) Updated commit description.
2) Add Acked-by: Richard Cochran.

Changes in V2:
1) Changed the ptimer-handle to ptp-hardware-clock based on
   Richard Cochran's comment.
2) Updated commit description.
---
 .../devicetree/bindings/net/ethernet-controller.yaml         | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index 00be387984ac..a97ab25b07a5 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -161,6 +161,11 @@ properties:
       - auto
       - in-band-status
 
+  ptp-hardware-clock:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Specifies a reference to a node representing a IEEE1588 timer.
+
   fixed-link:
     oneOf:
       - $ref: /schemas/types.yaml#/definitions/uint32-array
-- 
2.25.1

Re: [PATCH net-next V3] dt-bindings: net: ethernet-controller: Add ptp-hardware-clock

[Jakub Kicinski]

On Wed, 8 Mar 2023 11:14:08 +0530 Sarath Babu Naidu Gaddam wrote:
> There is currently no standard property to pass PTP device index
> information to ethernet driver when they are independent.
> 
> ptp-hardware-clock property will contain phandle to PTP clock node.
> 
> Its a generic (optional) property name to link to PTP phandle to
> Ethernet node. Any future or current ethernet drivers that need
> a reference to the PHC used on their system can simply use this
> generic property name instead of using custom property
> implementation in their device tree nodes."
> 
> Signed-off-by: Sarath Babu Naidu Gaddam <sarath.babu.naidu.gaddam@amd.com>
> Acked-by: Richard Cochran <richardcochran@gmail.com>

Rob, Krzysztof, any thoughts on this one?
Looks like the v2 discussion was a bit inconclusive.

Re: [PATCH net-next V3] dt-bindings: net: ethernet-controller: Add ptp-hardware-clock

[Krzysztof Kozlowski]

On 13/03/2023 23:35, Jakub Kicinski wrote:
> On Wed, 8 Mar 2023 11:14:08 +0530 Sarath Babu Naidu Gaddam wrote:
>> There is currently no standard property to pass PTP device index
>> information to ethernet driver when they are independent.
>>
>> ptp-hardware-clock property will contain phandle to PTP clock node.
>>
>> Its a generic (optional) property name to link to PTP phandle to
>> Ethernet node. Any future or current ethernet drivers that need
>> a reference to the PHC used on their system can simply use this
>> generic property name instead of using custom property
>> implementation in their device tree nodes."
>>
>> Signed-off-by: Sarath Babu Naidu Gaddam <sarath.babu.naidu.gaddam@amd.com>
>> Acked-by: Richard Cochran <richardcochran@gmail.com>
> 
> Rob, Krzysztof, any thoughts on this one?
> Looks like the v2 discussion was a bit inconclusive.

Anyway this did not implement changes I requested, so NAK.

Best regards,
Krzysztof

Re: [PATCH net-next V3] dt-bindings: net: ethernet-controller: Add ptp-hardware-clock

[Krzysztof Kozlowski]

On 08/03/2023 06:44, Sarath Babu Naidu Gaddam wrote:
> There is currently no standard property to pass PTP device index
> information to ethernet driver when they are independent.
> 
> ptp-hardware-clock property will contain phandle to PTP clock node.
> 
> Its a generic (optional) property name to link to PTP phandle to
> Ethernet node. Any future or current ethernet drivers that need
> a reference to the PHC used on their system can simply use this
> generic property name instead of using custom property
> implementation in their device tree nodes."
> 
> Signed-off-by: Sarath Babu Naidu Gaddam <sarath.babu.naidu.gaddam@amd.com>
> Acked-by: Richard Cochran <richardcochran@gmail.com>
> ---
> 
> Freescale driver currently has this implementation but it will be
> good to agree on a generic (optional) property name to link to PTP
> phandle to Ethernet node. In future or any current ethernet driver
> wants to use this method of reading the PHC index,they can simply use
> this generic name and point their own PTP clock node, instead of
> creating separate property names in each ethernet driver DT node.

Again, I would like to see an user of this. I asked about this last time
and nothing was provided.

So basically you send the same thing hoping this time will be accepted...

> 
> axiethernet driver uses this method when PTP support is integrated.
> 
> Example:
>     fman0: fman@1a00000 {
>         ptp-hardware-clock = <&ptp_timer0>;
>     }
> 
>     ptp_timer0: ptp-timer@1afe000 {
>         compatible = "fsl,fman-ptp-timer";
>         reg = <0x0 0x1afe000 0x0 0x1000>;
>     }
> 
> DT information:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/qoriq-fman3-0.dtsi#n23
> 
> Freescale driver:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c#n407
> 
> Changes in V3:
> 1) Updated commit description.
> 2) Add Acked-by: Richard Cochran.
> 
> Changes in V2:
> 1) Changed the ptimer-handle to ptp-hardware-clock based on
>    Richard Cochran's comment.
> 2) Updated commit description.
> ---
>  .../devicetree/bindings/net/ethernet-controller.yaml         | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> index 00be387984ac..a97ab25b07a5 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> @@ -161,6 +161,11 @@ properties:
>        - auto
>        - in-band-status
>  
> +  ptp-hardware-clock:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Specifies a reference to a node representing a IEEE1588 timer.


https://lore.kernel.org/all/cfbde0da-9939-e976-52c1-88577de7d4cb@linaro.org/

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.

Best regards,
Krzysztof

RE: [PATCH net-next V3] dt-bindings: net: ethernet-controller: Add ptp-hardware-clock

[Gaddam, Sarath Babu Naidu]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Tuesday, March 14, 2023 9:16 PM
> To: Gaddam, Sarath Babu Naidu
> <sarath.babu.naidu.gaddam@amd.com>; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> robh+dt@kernel.org; richardcochran@gmail.com
> Cc: krzysztof.kozlowski+dt@linaro.org; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> yangbo.lu@nxp.com; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; Sarangi, Anirudha
> <anirudha.sarangi@amd.com>; Katakam, Harini
> <harini.katakam@amd.com>; git (AMD-Xilinx) <git@amd.com>
> Subject: Re: [PATCH net-next V3] dt-bindings: net: ethernet-controller:
> Add ptp-hardware-clock
> 
> On 08/03/2023 06:44, Sarath Babu Naidu Gaddam wrote:
> > There is currently no standard property to pass PTP device index
> > information to ethernet driver when they are independent.
> >
> > ptp-hardware-clock property will contain phandle to PTP clock node.
> >
> > Its a generic (optional) property name to link to PTP phandle to
> > Ethernet node. Any future or current ethernet drivers that need a
> > reference to the PHC used on their system can simply use this generic
> > property name instead of using custom property implementation in
> their
> > device tree nodes."
> >
> > Signed-off-by: Sarath Babu Naidu Gaddam
> > <sarath.babu.naidu.gaddam@amd.com>
> > Acked-by: Richard Cochran <richardcochran@gmail.com>
> > ---
> >
> > Freescale driver currently has this implementation but it will be good
> > to agree on a generic (optional) property name to link to PTP phandle
> > to Ethernet node. In future or any current ethernet driver wants to
> > use this method of reading the PHC index,they can simply use this
> > generic name and point their own PTP clock node, instead of creating
> > separate property names in each ethernet driver DT node.
> 
> Again, I would like to see an user of this. I asked about this last time and
> nothing was provided.
> 
> So basically you send the same thing hoping this time will be accepted...

Apologies for miscommunication. As of now, we see only freescale driver
has this type of implementation but with different binding name. we do 
not have the hardware to test this. However, I am going to upstream PTP
support in AMD/Xilinx axiethernet driver that will be user of this binding.

 I will pick up on this binding again along with consumer driver and address
 your review comments.     

Thanks for the review.

Thanks,
Sarath

Re: [PATCH net-next V3] dt-bindings: net: ethernet-controller: Add ptp-hardware-clock

[Krzysztof Kozlowski]

On 16/03/2023 11:53, Gaddam, Sarath Babu Naidu wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: Tuesday, March 14, 2023 9:16 PM
>> To: Gaddam, Sarath Babu Naidu
>> <sarath.babu.naidu.gaddam@amd.com>; davem@davemloft.net;
>> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
>> robh+dt@kernel.org; richardcochran@gmail.com
>> Cc: krzysztof.kozlowski+dt@linaro.org; netdev@vger.kernel.org;
>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
>> yangbo.lu@nxp.com; Pandey, Radhey Shyam
>> <radhey.shyam.pandey@amd.com>; Sarangi, Anirudha
>> <anirudha.sarangi@amd.com>; Katakam, Harini
>> <harini.katakam@amd.com>; git (AMD-Xilinx) <git@amd.com>
>> Subject: Re: [PATCH net-next V3] dt-bindings: net: ethernet-controller:
>> Add ptp-hardware-clock
>>
>> On 08/03/2023 06:44, Sarath Babu Naidu Gaddam wrote:
>>> There is currently no standard property to pass PTP device index
>>> information to ethernet driver when they are independent.
>>>
>>> ptp-hardware-clock property will contain phandle to PTP clock node.
>>>
>>> Its a generic (optional) property name to link to PTP phandle to
>>> Ethernet node. Any future or current ethernet drivers that need a
>>> reference to the PHC used on their system can simply use this generic
>>> property name instead of using custom property implementation in
>> their
>>> device tree nodes."
>>>
>>> Signed-off-by: Sarath Babu Naidu Gaddam
>>> <sarath.babu.naidu.gaddam@amd.com>
>>> Acked-by: Richard Cochran <richardcochran@gmail.com>
>>> ---
>>>
>>> Freescale driver currently has this implementation but it will be good
>>> to agree on a generic (optional) property name to link to PTP phandle
>>> to Ethernet node. In future or any current ethernet driver wants to
>>> use this method of reading the PHC index,they can simply use this
>>> generic name and point their own PTP clock node, instead of creating
>>> separate property names in each ethernet driver DT node.
>>
>> Again, I would like to see an user of this. I asked about this last time and
>> nothing was provided.
>>
>> So basically you send the same thing hoping this time will be accepted...
> 
> Apologies for miscommunication. As of now, we see only freescale driver
> has this type of implementation but with different binding name. we do 

Then the freescale binding (and driver) should be adjusted to use new
property. Deprecate then the old one.



Best regards,
Krzysztof