Re: [PATCH v2 6/6] net: phy: DP83640: Add fiber mode enabling/disabling from device tree

[Maxime Chevallier]

Hi Bastien, Andrew,

On Thu, 29 Feb 2024 16:23:59 +0100
Andrew Lunn <andrew@xxxxxxx> wrote:

> On Thu, Feb 29, 2024 at 08:31:55AM +0100, Bastien Curutchet wrote:
> > Hi Andrew,
> > 
> > On 2/27/24 17:18, Andrew Lunn wrote:  
> > > On Tue, Feb 27, 2024 at 10:39:45AM +0100, Bastien Curutchet wrote:  
> > > > The PHY is able to use copper or fiber. The fiber mode can be enabled or
> > > > disabled by hardware strap. If hardware strap is incorrect, PHY can't
> > > > establish link.
> > > > 
> > > > Add a DT attribute 'ti,fiber-mode' that can be use to override the
> > > > hardware strap configuration. If the property is not present, hardware
> > > > strap configuration is left as is.  
> > > How have you tested this? Do you have a RDK with it connected to an
> > > SFP cage?  
> > 
> > I did not test fiber mode as my board uses copper.
> > 
> > My use case is that I need to explicitly disable the fiber mode because the
> > strap hardware is
> > misconfigured and could possibly enable fiber mode from time to time.  
> 
> O.K. So lets refocus this is little. Rather than support fibre mode,
> just support disabling fibre mode. But leave a clear path for somebody
> to add fibre support sometime in the future.
> 
> Looking at the current code, do you think fibre mode actually works
> today? If you think it cannot actually work today in fibre mode, one
> option would be to hard code it to copper mode. Leave the
> configuration between fibre and copper mode to the future when
> somebody actually implements fibre mode.

Looking at the driver and the datasheet, it's hard to say that the
fiber mode can't work in the current state. It's configured either
through straps or an overriding register, and it's enough to get the
scrambler/descrambler automatically setup according to that single
strap. 

So it's hard to say that defaulting to copper won't break anything :(

OTOH there's no SFP support in this PHY, in terms of register config,
some aneg modes won't work in 100BaseFX, which the driver doesn't account for,
So nothing would indicate that the fiber mode was ever used.

There's the DP83822 driver that can accept the "ti,fiber-mode"
property, so adding that would at least be coherent with other DP83xxx
PHYs but it has the opposite logic we want, so doesn't prevent any
possible regression for existing fiber users.

All in all, do you think that defaulting to copper and leaving users an
option to implement "ti,fiber-mode" is an acceptable risk to take ?

Maxime