[RFC PATCH] dt-bindings: net: ethernet-controller: Add ptimer_handle

[RFC PATCH] dt-bindings: net: ethernet-controller: Add ptimer_handle

[Sarath Babu Naidu Gaddam]

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

ptimer_handle property will contain phandle to PTP timer node.

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 timer node, instead of
creating seperate property names in each ethernet driver DT node.

axiethernet driver uses this method when PTP support is integrated.

Example:
	fman0: fman@1a00000 {
		ptimer-handle = <&ptp_timer0>;
	}

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

Signed-off-by: Sarath Babu Naidu Gaddam <sarath.babu.naidu.gaddam@amd.com>
---
We want binding to be reviewed/accepted and then make changes in freescale
binding documentation to use this generic binding.

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

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
tree/Documentation/devicetree/bindings/net/fsl-fman.txt#n320

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

Changes in V2:
1)Added links to reference code snippets.
2)Updated commit msg.

Comments, suggestions and thoughts to have a common name are very welcome...!
---
 .../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 4b3c590fcebf..7e726d620c6a 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -155,6 +155,11 @@ properties:
       - auto
       - in-band-status
 
+  ptimer_handle:
+    $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: [RFC PATCH] dt-bindings: net: ethernet-controller: Add ptimer_handle

[Andrew Lunn]

On Thu, Sep 29, 2022 at 06:12:49AM -0600, Sarath Babu Naidu Gaddam wrote:
> There is currently no standard property to pass PTP device index
> information to ethernet driver when they are independent.

You should Cc: the PTP maintainer....

    Andrew

RE: [RFC PATCH] dt-bindings: net: ethernet-controller: Add ptimer_handle

[Gaddam, Sarath Babu Naidu]

> -----Original Message-----
> From: Sarath Babu Naidu Gaddam <sarath.babu.naidu.gaddam@amd.com>
> Sent: Thursday, September 29, 2022 5:43 PM
> To: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; robh+dt@kernel.org
> 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>; Gaddam, Sarath Babu Naidu
> <sarath.babu.naidu.gaddam@amd.com>
> Subject: [RFC PATCH] dt-bindings: net: ethernet-controller: Add
> ptimer_handle
> 
> There is currently no standard property to pass PTP device index information
> to ethernet driver when they are independent.
> 
> ptimer_handle property will contain phandle to PTP timer node.
> 
> 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 timer node, instead of creating seperate property names in each
> ethernet driver DT node.
> 
> axiethernet driver uses this method when PTP support is integrated.
> 
> Example:
> 	fman0: fman@1a00000 {
> 		ptimer-handle = <&ptp_timer0>;
> 	}
> 
> 	ptp_timer0: ptp-timer@1afe000 {
> 		compatible = "fsl,fman-ptp-timer";
> 		reg = <0x0 0x1afe000 0x0 0x1000>;
> 	}
> 
> Signed-off-by: Sarath Babu Naidu Gaddam
> <sarath.babu.naidu.gaddam@amd.com>
> ---
> We want binding to be reviewed/accepted and then make changes in
> freescale binding documentation to use this generic binding.
> 
> 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
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
> tree/Documentation/devicetree/bindings/net/fsl-fman.txt#n320
> 
> Freescale driver:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
> tree/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c#n467
> 
> Changes in V2:
> 1)Added links to reference code snippets.
> 2)Updated commit msg.
> 
> Comments, suggestions and thoughts to have a common name are very
> welcome...!
> ---
>  .../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 4b3c590fcebf..7e726d620c6a 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
> @@ -155,6 +155,11 @@ properties:
>        - auto
>        - in-band-status
> 
> +  ptimer_handle:
> +    $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

+Richard Cochran for PTP; apologies for missing earlier.

Thanks,
Sarath

Re: [RFC PATCH] dt-bindings: net: ethernet-controller: Add ptimer_handle

[Rob Herring]

On Thu, Sep 29, 2022 at 06:12:49AM -0600, Sarath Babu Naidu Gaddam wrote:
> There is currently no standard property to pass PTP device index
> information to ethernet driver when they are independent.
> 
> ptimer_handle property will contain phandle to PTP timer node.

ptimer_handle or ptimer-handle? One matches conventions.

However, 'handle' is redundant and 'ptimer' is vague. 'ptp-timer' 
instead? (Humm, looking at fsl-fman.txt after writing everything here, 
it's already using that name! So why are you making something new?)

However, for anything common, I'd like to see multiple examples and 
users. Do we have any custom bindings for this purpose already (besides 
FSL)? 

Could an ethernet device ever need more than 1 timer? Could a provider 
provide multiple timers? IOW, does this need to follow standard 
provider/consumer pattern of 'foos' and '#foo-cells'?

> 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

What's PHC index?

> this generic name and point their own PTP timer node, instead of
> creating seperate property names in each ethernet driver DT node.
> 
> axiethernet driver uses this method when PTP support is integrated.
> 
> Example:
> 	fman0: fman@1a00000 {
> 		ptimer-handle = <&ptp_timer0>;
> 	}
> 
> 	ptp_timer0: ptp-timer@1afe000 {
> 		compatible = "fsl,fman-ptp-timer";
> 		reg = <0x0 0x1afe000 0x0 0x1000>;
> 	}
> 
> Signed-off-by: Sarath Babu Naidu Gaddam <sarath.babu.naidu.gaddam@amd.com>
> ---
> We want binding to be reviewed/accepted and then make changes in freescale
> binding documentation to use this generic binding.

You can't just change the binding you are using. That's an ABI break.

Rob

RE: [RFC PATCH] dt-bindings: net: ethernet-controller: Add ptimer_handle

[Gaddam, Sarath Babu Naidu]

+Richard Cochran

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Saturday, October 1, 2022 12:52 AM
> To: Gaddam, Sarath Babu Naidu <sarath.babu.naidu.gaddam@amd.com>
> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; 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>
> Subject: Re: [RFC PATCH] dt-bindings: net: ethernet-controller: Add
> ptimer_handle
> 
> On Thu, Sep 29, 2022 at 06:12:49AM -0600, Sarath Babu Naidu Gaddam wrote:
> > There is currently no standard property to pass PTP device index
> > information to ethernet driver when they are independent.
> >
> > ptimer_handle property will contain phandle to PTP timer node.
> 
> ptimer_handle or ptimer-handle? One matches conventions.
> 
> However, 'handle' is redundant and 'ptimer' is vague. 'ptp-timer'
> instead? (Humm, looking at fsl-fman.txt after writing everything here, it's
> already using that name! So why are you making something new?)

Thanks for the review.
I think Freescale driver uses "ptimer-handle" but I also see Freescale DT 
documentation refers to "ptp-timer".
 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
tree/Documentation/devicetree/bindings/net/fsl-fman.txt#n320

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

it will be good to agree on a generic property name to link to PTP phandle
to Ethernet node for any driver to use. 

> 
> However, for anything common, I'd like to see multiple examples and users.
> Do we have any custom bindings for this purpose already (besides FSL)?

AFAIK There are no other examples. We plan to use this in Xilinx axienet.  

> 
> Could an ethernet device ever need more than 1 timer? Could a provider
> provide multiple timers? IOW, does this need to follow standard
> provider/consumer pattern of 'foos' and '#foo-cells'?

No, Even In systems with multiple timers ethernet uses timestamps only
from one associated PTP timer.

> 
> > 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
> 
> What's PHC index?

PHC(PTP Hardware clock) index is a number which is used by ptp4l
application. When a PTP device registers with a kernel, device node
will be created in the /dev.For example, /dev/ptp0, /dev/ptp1.

When PTP and Ethernet are in the same device driver, This PHC index
Information is internally accessible. When they are independent drivers,
PTP DT node should be linked to ethernet node so that PTP timer
information such as PHC index is accessible.   

> 
> > this generic name and point their own PTP timer node, instead of
> > creating seperate property names in each ethernet driver DT node.
> >
> > axiethernet driver uses this method when PTP support is integrated.
> >
> > Example:
> > 	fman0: fman@1a00000 {
> > 		ptimer-handle = <&ptp_timer0>;
> > 	}
> >
> > 	ptp_timer0: ptp-timer@1afe000 {
> > 		compatible = "fsl,fman-ptp-timer";
> > 		reg = <0x0 0x1afe000 0x0 0x1000>;
> > 	}
> >
> > Signed-off-by: Sarath Babu Naidu Gaddam
> > <sarath.babu.naidu.gaddam@amd.com>
> > ---
> > We want binding to be reviewed/accepted and then make changes in
> > freescale binding documentation to use this generic binding.
> 
> You can't just change the binding you are using. That's an ABI break.
> 
Apologies for confusion. I am not proposing a change to the property
name.I am simply trying to document the correct DT property name 
used in Freescale and then move it to generic ethernet-controller.yaml.

Thanks,
Sarath

Re: [RFC PATCH] dt-bindings: net: ethernet-controller: Add ptimer_handle

[Richard Cochran]

On Mon, Oct 03, 2022 at 09:29:00AM +0000, Gaddam, Sarath Babu Naidu wrote:

> PHC(PTP Hardware clock) index is a number which is used by ptp4l
> application. When a PTP device registers with a kernel, device node
> will be created in the /dev.For example, /dev/ptp0, /dev/ptp1.
> 
> When PTP and Ethernet are in the same device driver, This PHC index
> Information is internally accessible. When they are independent drivers,
> PTP DT node should be linked to ethernet node so that PTP timer
> information such as PHC index is accessible.   

Good explanation.  The handle you propose makes sense to me.
Maybe let the name spell it out clearly?

	fman0: fman@1a00000 {
		ptp-hardware-clock = <&phc0>;
	}
	phc0: ptp-timer@1afe000 {
		compatible = "fsl,fman-ptp-timer";
		reg = <0x0 0x1afe000 0x0 0x1000>;
	}

In any case, to me "timer" has a different connotation than "clock".

Thanks,
Richard