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

[Gaddam, Sarath Babu Naidu]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Sent: Thursday, November 10, 2022 7:36 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 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