Re: [PATCH net-next v1] lan78xx: lan7801 MAC support with lan8841

[<Rengarajan.S@microchip.com>]

Hi Andrew,

Thanks for the comments and apologies for the delay in reply. Please
find my comments inline.

On Thu, 2024-06-20 at 15:49 +0200, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Thu, Jun 20, 2024 at 05:48:31AM +0000, Rengarajan.S@microchip.com
> wrote:
> > Hi Andrew,
> > 
> > Apologies for the delay in reply. Thanks for reviewing the patch.
> > Please find my comments inline.
> > 
> > On Thu, 2024-06-13 at 22:46 +0200, Andrew Lunn wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > > know the content is safe
> > > 
> > > On Tue, Jun 11, 2024 at 03:12:33PM +0530, Rengarajan S wrote:
> > > > Add lan7801 MAC only support with lan8841. The PHY fixup is
> > > > registered
> > > > for lan8841 and the initializations are done using
> > > > lan8835_fixup
> > > > since
> > > > the register configs are similar for both lann8841 and lan8835.
> > > 
> > > What exactly does this fixup do?
> > 
> > Fixup related to the phy handle and manage the configuration and
> > status
> > registers of a particular phy. In this patch it is used to handle
> > the
> > configuration registers of LAN8841 which are similar to registers
> > in
> > LAN8835.
> 
> Details please, not hand waving. What does the errata say? Why is
> this
> specific to your USB dongle, and not all cases where this PHY is
> used?

Although, there is no specific errata available for adding fixup
specific to lan78xx USB dongle, we have added the fixup for handing
specific configurations to ensure the PHY operates correctly with the
MAC. In this case while transmitting from MAC to PHY the device does
not add the delay locally at its TX input pins. It expects the TXC
delay to be provided by on-chip MAC. Since the delay calculated in this
case is specific to the lan78xx USB dongle it is not possible to use
this fixup for interfacing with generic MAC.

Similarly for ksz9131 we add fixed 2ns delay, and the additional delay
is provided by pad skew registers. In this case too the delay value
calculated is specific to lan78xx USB and is not applicable for other
MACs.

> 
> > > Looking at it, what protects it from being used on some other
> > > device
> > > which also happens to use the same PHY? Is there something to
> > > guarantee:
> > > 
> > > struct lan78xx_net *dev = netdev_priv(phydev->attached_dev);
> > > 
> > > really is a lan78xx_net * ?
> > 
> > In this case fixup is called through lan78xx only when interfacing
> > the
> > phy with lan78xx MAC. Since this will not be called on interfacing
> > with
> > other devices, it prevents them from accessing the registers.
> 
> Please give me a details explanation why this fixup will not be
> applied to other instances of this PHY in the system.

As stated above, the TXC delay calculated for the PHY is specific to
the lan78xx on-chip MAC. This delay ensures that both the phy and MAC
clock delay timing is met. Any other MACs connected will need a
different delay values to be synchronized with MAC and hence these
instances will get failed.

> 
>         Andrew