linux-kernel - Re: [PATCH net-next v2 2/2] net: phy: microchip

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] [day] [month] [year] [list]

Message-ID: <d04abb91-b77f-4d29-a89c-c00ffa68595c@lunn.ch>
Date: Tue, 17 Dec 2024 11:34:30 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Tarun.Alle@...rochip.com
Cc: Arun.Ramadoss@...rochip.com, UNGLinuxDriver@...rochip.com,
	hkallweit1@...il.com, linux@...linux.org.uk, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2 2/2] net: phy: microchip_t1: Auto-negotiation
 support for LAN887x

> We confirmed that there are no customers who are directly using the net-next. 
> Hence, we are setting this to default auto-neg which is also chip default. But if
> any regressions on T1PHYs are dependent,  we will address this default setting.

So this needs to be communicated, to avoid this sort of back and forth
with emails. It is not the first time we have changed a default like
this, after asking the early adopters if it will be an issue, but we
need to make it clear we have done our due diligence before making a
breaking change.

> > I think we also need some more details about the autoneg in the commit
> > message. When used against a standards conforming 100M PHY, negotiation
> > will fail by default, because this PHY is not conformant with 100M, or 1G
> > autoneg.
> 
> I should have given the same errata details in the commit message. Will take care.
> 
> > I don't like you are going to cause regressions, especially when you have decided
> > regressions are worth it for a half broken autoneg.
> > 
> > I actually think it should default to fixed, as it is today. Maybe with the option to
> > enable the broken autoneg. This is different to all PHYs we have today, but we try
> > hard to avoid regressions.
> > 
> > What are the plans for this PHY? Will there be a new revision soon which fixes
> > the broken autoneg? Maybe you should forget about autoneg for this revision
> > of this PHY, it is too broken, and wait for the next revision which actually
> > conforms to the standard?
> > 
> 
> I understand your point and I agree with you. We can drop this patch for this chip 
> revision as we have plans for new revision.

I would probably drop this patch if the new revision is coming soon.

	Andrew