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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 8 Mar 2022 10:05:08 +0000
From:   <Divya.Koppera@...rochip.com>
To:     <andrew@...n.ch>
CC:     <netdev@...r.kernel.org>, <hkallweit1@...il.com>,
        <linux@...linux.org.uk>, <davem@...emloft.net>, <kuba@...nel.org>,
        <robh+dt@...nel.org>, <devicetree@...r.kernel.org>,
        <richardcochran@...il.com>, <linux-kernel@...r.kernel.org>,
        <UNGLinuxDriver@...rochip.com>, <Madhuri.Sripada@...rochip.com>,
        <Manohar.Puri@...rochip.com>
Subject: RE: [PATCH net-next 2/3] dt-bindings: net: micrel: Configure latency
 values and timestamping check for LAN8814 phy

> -----Original Message-----
> From: Andrew Lunn <andrew@...n.ch>
> Sent: Monday, March 7, 2022 6:39 PM
> To: Divya Koppera - I30481 <Divya.Koppera@...rochip.com>
> Cc: netdev@...r.kernel.org; hkallweit1@...il.com; linux@...linux.org.uk;
> davem@...emloft.net; kuba@...nel.org; robh+dt@...nel.org;
> devicetree@...r.kernel.org; richardcochran@...il.com; linux-
> kernel@...r.kernel.org; UNGLinuxDriver <UNGLinuxDriver@...rochip.com>;
> Madhuri Sripada - I34878 <Madhuri.Sripada@...rochip.com>; Manohar Puri -
> I30488 <Manohar.Puri@...rochip.com>
> 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
> 
> > > > +
> > > > + - 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.
> 
> Thanks for the reply, but you did not answer my question:
> 
>   Does this mean the hardware itself cannot tell you it is missing the
>   needed hardware?
> 
> Don't you have different IDs in register 2 and 3 for those devices with clock
> register and those without?
> 

The purpose of this option is, if both PHY and MAC supports timestamping then always timestamping is done in PHY.
If timestamping need to be done in MAC we need a way to stop PHY timestamping. If this flag is used then timestamping is taken care by MAC.

> > > > + - 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.
> 
> But you do actually have hard coded defaults. Those odd hex values i pointed
> out.
> 
> By different PHYs do you mean different PHY versions? So you can look at
> register 2 and 3, determine what PHY it is, and so from that what defaults
> should be used? Or do you mean different boards with the same PHY?
> 
> In general, the less tunables you have, the better. If the driver can figure it out,
> it is better to not have DT properties. The PHY will then also work with ACPI
> and USB etc, where there is no DT. Implementing the user space API Richard
> pointed out will also allow your PHY to work with none DP systems.
> 

Sorry I answered wrong. Latency values vary depending on the position of PHY in board. 
We have used this PHY in different hardware's, where latency values differs based on PHY positioning. 
So we used latency option in DTS file.
If you have other ideas or I'm wrong please let me know?

> > Yes in our case latency values depends on port speed. It is delay
> > between network medium and PTP timestamp point.
> 
> What are the units. You generally have the units in the property name. So e.g.
> lan8814,latency_tx_1000_ns. If need be, the driver then converts to whatever
> value you place into the register.
> 
> If you do keep them, please make it clear that these values are optional, and
> state what value will be used when the property is not present.
> 

Yes units are Nanoseconds.

>         Andrew