RE: [PATCH net-next 2/3] dt-bindings: net: micrel: Configure latency values and timestamping check for LAN8814 phy

[Divya.Koppera]

Hi Andrew,

Thanks for review and comments. Please find reply inline below.

> -----Original Message-----
> From: Andrew Lunn <andrew@xxxxxxx>
> Sent: Friday, March 4, 2022 6:21 PM
> To: Divya Koppera - I30481 <Divya.Koppera@xxxxxxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; hkallweit1@xxxxxxxxx; linux@xxxxxxxxxxxxxxx;
> davem@xxxxxxxxxxxxx; kuba@xxxxxxxxxx; robh+dt@xxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; richardcochran@xxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; UNGLinuxDriver <UNGLinuxDriver@xxxxxxxxxxxxx>;
> Madhuri Sripada - I34878 <Madhuri.Sripada@xxxxxxxxxxxxx>; Manohar Puri -
> I30488 <Manohar.Puri@xxxxxxxxxxxxx>
> Subject: Re: [PATCH net-next 2/3] dt-bindings: net: micrel: Configure latency
> values and timestamping check for LAN8814 phy
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> On Fri, Mar 04, 2022 at 03:04:17PM +0530, Divya Koppera wrote:
> > Supports configuring latency values and also adds check for phy
> > timestamping feature.
> >
> > Signed-off-by: Divya Koppera<Divya.Koppera@xxxxxxxxxxxxx>
> > ---
> >  .../devicetree/bindings/net/micrel.txt          | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/micrel.txt
> > b/Documentation/devicetree/bindings/net/micrel.txt
> > index 8d157f0295a5..c5ab62c39133 100644
> > --- a/Documentation/devicetree/bindings/net/micrel.txt
> > +++ b/Documentation/devicetree/bindings/net/micrel.txt
> > @@ -45,3 +45,20 @@ Optional properties:
> >
> >       In fiber mode, auto-negotiation is disabled and the PHY can only work in
> >       100base-fx (full and half duplex) modes.
> > +
> > + - lan8814,ignore-ts: If present the PHY will not support timestamping.
> > +
> > +     This option acts as check whether Timestamping is supported by
> > +     hardware or not. LAN8814 phy support hardware tmestamping.
> 
> Does this mean the hardware itself cannot tell you it is missing the needed
> hardware? What happens when you forget to add this flag? Does the driver
> timeout waiting for hardware which does not exists?
> 

If forgot to add this flag, driver will try to register ptp_clock that needs
access to clock related registers, which in turn fails if those registers doesn't exists.

> > + - lan8814,latency_rx_10: Configures Latency value of phy in ingress at 10
> Mbps.
> > +
> > + - lan8814,latency_tx_10: Configures Latency value of phy in egress at 10
> Mbps.
> > +
> > + - lan8814,latency_rx_100: Configures Latency value of phy in ingress at 100
> Mbps.
> > +
> > + - lan8814,latency_tx_100: Configures Latency value of phy in egress at 100
> Mbps.
> > +
> > + - lan8814,latency_rx_1000: Configures Latency value of phy in ingress at
> 1000 Mbps.
> > +
> > + - lan8814,latency_tx_1000: Configures Latency value of phy in egress at
> 1000 Mbps.
> 
> Why does this need to be configured, rather than hard coded? Why would the
> latency for a given speed change? I would of thought though you would take
> the average length of a PTP packet and divide is by the link speed.
> 

This latency values could be different for different phy's. So hardcoding will not work here.
Yes in our case latency values depends on port speed. It is delay between network medium and 
PTP timestamp point.

>      Andrew