Subject: Re: [PATCH v3] net: phy: add Maxlinear GPY115/21x/24x driver

From: Andrew Lunn <[email-masked]>
Date: Thu, 24 Jun 2021 03:06:00 +0200

On Wed, Jun 23, 2021 at 11:09:23PM +0200, Martin Blumenstingl wrote:
> Hi Liang,
> 
> On Wed, Jun 23, 2021 at 12:56 PM Liang Xu <[email-masked]> wrote:
> [...]
> > Hi Martin,
> >
> > 1) The legacy PHY GPY111 does not share the same register format and address for WoL and LED registers. Therefore code saving with a common driver is not possible.
> > 2) The interrupt handling routine would be different when new features are added in driver, such as PTP offload, MACsec offload. These will be added step by step as enhancement patches afte initial version of this driver upstreamed.
> I think that would leave only few shared registers with the older PHYs
> (and thus the intel-xway driver). Due to the lack of a public
> datasheet however I have no way to confirm this.
> So with this information added to the patch description having a
> different driver is fine for me
> 
> Maybe Andrew can also share his opinion - based on this new
> information - as he previously also suggested to extend the intel-xway
> driver instead of creating a new one

If the potential sharing is minimal, then a new driver makes sense. So
please do expand the patch description with a good justification.

> > 3) The new PHY generation comes with a reasonable pre-configuration for the LED registers which does not need any modification usually.
> >    In case a customer needs a specific configuration (e.g. traffic pulsing only, no link status) he needs to configure this via MDIO. There is also another option for a fixed preconfiguration with the support of an external flash. However, this is out of scope of the driver.
> older PHYs also do this, but not all boards use a reasonable LED setup
> 
> > 4) Many old products are mostly used on embedded devices which do not support WoL. Therefore there was no request yet to supported by the legacy XWAY driver.
> my understanding of Andrew's argument is: "if the register layout is
> the same for old and new PHY then why would we do some extra effort to
> not add WoL support for the old PHYs"
> Based on your information above the register layout is different, so
> that means two different implementations are needed anyways.

It would be nice to add WoL support for the older devices. They might
not be maxlinears latest and greatest, but there could be more of them
actually in use at the moment than this new PHY.

	 Andrew