| Message ID | 20221021054111.25852-1-sarath.babu.naidu.gaddam@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | [net-next,V2] dt-bindings: net: ethernet-controller: Add ptp-hardware-clock | expand |
On 21/10/2022 01:41, 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. > > 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>; > } > > 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. No, send entire set. We need to see the users of it. > > 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 Don't wrap links. It's not possible to click them... > > 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) 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 3aef506fa158..d2863c1dd585 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. Drop "Specifies a reference to". It's obvious from the schema. Aren't you expecting here some specific Devicetree node of IEEE1588 timer? IOW, you expect to point to timer, but what this timer must provide? How is this generic? In your commit msg you use multiple times "driver", so are you adding it only to satisfy Linux driver requirements? What about other drivers, e.g. on BSD or U-Boot? Best regards, Krzysztof
On Mon, Oct 24, 2022 at 11:57:23AM -0500, Rob Herring wrote: > On Thu, Oct 20, 2022 at 11:41:10PM -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. > > > > ptp-hardware-clock property will contain phandle to PTP clock 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 clock node, instead of > > creating separate property names in each ethernet driver DT node. > > Seems like this does the same thing as > Documentation/devicetree/bindings/ptp/timestamper.txt. That is different. It goes from: MAC -> time stamp generator The proposed binding goes from: MAC (with built in time stamp generator) -> PTP Hardware Clock (with get/settime etc) Thanks, Richard
On Mon, Oct 24, 2022 at 11:57:23AM -0500, Rob Herring wrote: > On Thu, Oct 20, 2022 at 11:41:10PM -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. > > > > ptp-hardware-clock property will contain phandle to PTP clock 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 clock node, instead of > > creating separate property names in each ethernet driver DT node. > > Seems like this does the same thing as > Documentation/devicetree/bindings/ptp/timestamper.txt. > > Or perhaps what we have in bindings/timestamp/ which unfortunately does > about the same thing. > > The latter one is more flexible and follows standard provider/consumer > patterns. So timestamper.txt should probably be deprecated. I don't see how you can do that. The provider/consumer semantics are completely opposite. The three (including present patch) bindings specify three different relationships. Thanks, Richard
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Thursday, November 10, 2022 7:36 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 V2] dt-bindings: net: ethernet-controller: > Add ptp-hardware-clock > > On 10/11/2022 10:57, Gaddam, Sarath Babu Naidu wrote: > >>> > >>> + ptp-hardware-clock: > >>> + $ref: /schemas/types.yaml#/definitions/phandle > >>> + description: > >>> + Specifies a reference to a node representing a IEEE1588 timer. > >> > >> Drop "Specifies a reference to". It's obvious from the schema. > >> > >> Aren't you expecting here some specific Devicetree node of IEEE1588 > timer? > >> IOW, you expect to point to timer, but what this timer must provide? > >> How is this generic? > > > > Thanks for review comments. > > Format can be as documented by users > Documentation/devicetree/bindings/ptp/ members. The node should be > accessible to derive the index but the format of the PTP clock node is > upto the vendor. > > I am not sure what do you mean here. Anyway description might need > something more specific. Apologies for picking up on this thread after a long time. PTP clock node(timer) is upto the vendor. Driver which needs a reference to this node can be accessed by this new property. I will update the description. > > > > > >> > >> In your commit msg you use multiple times "driver", so are you > adding > >> it only to satisfy Linux driver requirements? What about other > >> drivers, e.g. on BSD or U-Boot? > > > > AFAIK this is for Linux. It is not relevant to uboot as there's no PTP > support there. > > And BSD? Bindings are not for Linux only. Please abstract from any OS > specifics. This new binding is a generic property. It can be used in other drivers also if they want to access the PTP device node in the current driver and It's an optional property. I will change the commit description as below. If this is fine, I will send another version with updated commit description. "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." Thanks, Sarath > Also your messages needs wrapping. Use mailing list reply style. > > Best regards, > Krzysztof
diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml index 3aef506fa158..d2863c1dd585 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
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. 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>; } 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) 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(+)