> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Sent: Sunday, October 23, 2022 9:12 PM
> To: Gaddam, Sarath Babu Naidu <sarath.babu.naidu.gaddam@xxxxxxx>;
> davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> pabeni@xxxxxxxxxx; robh+dt@xxxxxxxxxx; richardcochran@xxxxxxxxx
> Cc: krzysztof.kozlowski+dt@xxxxxxxxxx; netdev@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> yangbo.lu@xxxxxxx; Pandey, Radhey Shyam
> <radhey.shyam.pandey@xxxxxxx>; Sarangi, Anirudha
> <anirudha.sarangi@xxxxxxx>; Katakam, Harini
> <harini.katakam@xxxxxxx>; git (AMD-Xilinx) <git@xxxxxxx>
> Subject: Re: [PATCH net-next V2] dt-bindings: net: ethernet-controller: Add
> ptp-hardware-clock
>
> 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@xxxxxxx>
> > ---
> > 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?
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.
>
> 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.
Thanks,
Sarath
