Re: [PATCH 3/3] net: microchip_t1s: conditional collision detection

From: "Ramón Nordin Rodriguez" <ramon.nordin.rodriguez@ferroamp.se>
To: Parthiban.Veerasooran@microchip.com
Cc: andrew@lunn.ch, hkallweit1@gmail.com, linux@armlinux.org.uk,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] net: microchip_t1s: conditional collision detection
Date: Mon, 27 Nov 2023 17:32:41 +0100	[thread overview]
Message-ID: <ZWTEqXAwxIK1pSHo@debian> (raw)
In-Reply-To: <142ce54c-108c-45b4-b886-ce3ca45df9fe@microchip.com>

On Mon, Nov 27, 2023 at 04:00:18PM +0000, Parthiban.Veerasooran@microchip.com wrote:
> Hi,
> 
> This implementation was introduced in the below patch itself.
> 
> https://lore.kernel.org/netdev/20230426205049.xlfqluzwcvlm6ihh@soft-dev3-1/T/#m9a52b6c03b7fa637f70aed306b50b442590e24a3
> 

But the change was dropped in that patchset right? It's not present in
netdev-next.

> As it is recommended to do it in a separate patch and also the 
> datasheets of LAN867X Rev.B1 and LAN865X Rev.B0 internal PHY have these 
> register is reserved, we were working for a feasible solution to 
> describe this for customer and mainline. By the time many other things 
> messed up and couldn't reach the mainline on time.
> 

Far as I can tell 'collision detect' is described in the following
sections of respective datasheet:

* 11.5.51 - LAN8650
* 5.4.48  - LAN8670

The rest of the bits are reserved though. The change I propose only
manipulate the documented (bit 15) collision bit.

Is your point that the lan8670 datasheet is only valid for rev.c and not
rev.b?

Andrew suggested on the cover letter that it be interesting to look at
completly disabling collision detect, any strings you can pull at
Microchip to investigate that?
Also any input on my suggested testing methodology is more than welcome.

> We also implemented LAN867X Rev.C1 support already in the driver and 
> published in our product site and in the process of preparing mainline 
> patches. But unfortunately it took little more time to make it.
> 
> https://ww1.microchip.com/downloads/aemDocuments/documents/AIS/ProductDocuments/CodeExamples/EVB-LAN8670-USB_Linux_Driver_1v0.zip

I'm aware, we've been using a derivative of that work at ferroamp for
development. But it's been driving me nuts, being the 't1s guy' at work,
and maintaining out of tree drivers for weird dev boxes.

It's not my intention to beat you to the punch, I just want a mainlined
driver so that I can spend less of my time on plumbing.

> 
> Anyway, thank you for the support. Good luck!

Likewise!
R