> -----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