lkml.org 
[lkml]   [2021]   [May]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v3] net: phy: add driver for Motorcomm yt8511 phy
On Fri, May 14, 2021 at 9:24 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Fri, May 14, 2021 at 07:58:26AM -0400, Peter Geis wrote:
> > Add a driver for the Motorcomm yt8511 phy that will be used in the
> > production Pine64 rk3566-quartz64 development board.
> > It supports gigabit transfer speeds, rgmii, and 125mhz clk output.
>
> Thanks for adding RGMII support.
>
> > +#define PHY_ID_YT8511 0x0000010a
>
> No OUI in the PHY ID?
>
> Humm, the datasheet says it defaults to zero. That is not very
> good. This could be a source of problems in the future, if some other
> manufacture also does not use an OUI.
>
> > +/* RX Delay enabled = 1.8ns 1000T, 8ns 10/100T */
> > +#define YT8511_DELAY_RX BIT(0)
> > +
> > +/* TX Delay is bits 7:4, default 0x5
> > + * Delay = 150ps * N - 250ps, Default = 500ps
> > + */
> > +#define YT8511_DELAY_TX (0x5 << 4)
>
> > +
> > + switch (phydev->interface) {
> > + case PHY_INTERFACE_MODE_RGMII:
> > + val &= ~(YT8511_DELAY_RX | YT8511_DELAY_TX);
> > + break;
>
> This is not correct. YT8511_DELAY_TX will only mask the 2 bits in 0x5,
> not all the bits in 7:4. And since the formula is
>
> Delay = 150ps * N - 250ps
>
> setting N to 0 is not what you want. You probably want N=2, so you end up with 50ps

The manufacturer's driver set this to <0> to disable, but I agree the
datasheet disagrees with this.

>
> > + case PHY_INTERFACE_MODE_RGMII_ID:
> > + val |= YT8511_DELAY_RX | YT8511_DELAY_TX;
> > + break;
> > + case PHY_INTERFACE_MODE_RGMII_RXID:
> > + val &= ~(YT8511_DELAY_TX);
> > + val |= YT8511_DELAY_RX;
>
> The delay should be around 2ns. For RX you only have 1.8ns, which is
> probably good enough. But for TX you have more flexibility. You are
> setting it to the default of 500ps which is too small. I would suggest
> 1.85ns, N=14, so it is the same as RX.

Makes sense, these are the insights I was hoping for!

>
> I also wonder about bits 15:12 of PHY EXT ODH: Delay and driver
> strength CFG register.

The default value *works*, and from an emi perspective we want the
lowest strength single that is reliable.
Unfortunately I don't have the equipment to *test* the actual output
strengths and the datasheet isn't explicitly clear about them either.
This is one of the values I was considering having defined in the devicetree.

>
> Andrew

\
 
 \ /
  Last update: 2021-05-14 16:14    [W:0.069 / U:0.940 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site