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

Hi,

On 27/11/23 10:02 pm, Ramón Nordin Rodriguez wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

On Mon, Nov 27, 2023 at 04:00:18PM +0000, Parthiban.Veerasooran@microchip.com wrote:

quoted

Hi,

This implementation was introduced in the below patch itself.

https://lore.kernel.org/netdev/20230426205049.xlfqluzwcvlm6ihh@soft-dev3-1/T/#m9a52b6c03b7fa637f70aed306b50b442590e24a3 (local)

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

Yes, it was dropped. The reason why I gave this info is, you mentioned 
in the cover letter that it took some time for you to find this in the 
datasheet.

quoted

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?

It is valid for rev.b1 as well but the current datasheet for rev.c1 
doesn't show that info.

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?

Unfortunately I can't commit anything from my side as we are occupied 
with other activities. But definitely I will try my level best if time 
permits. Alternatively you can contact our Microchip customer support 
team if you are interested to do this testing at Microchip.

Also any input on my suggested testing methodology is more than welcome.

quoted

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.

I completely understand. Also it was not my intention too. Just to let 
you know why it is delayed in reaching mainline and a quick reference 
for the existing implementation. Enjoy!

Best regards,
Parthiban V

quoted

Anyway, thank you for the support. Good luck!

Likewise!
R