Re: [PATCH net v3] net: phy: intel-xway: enable integrated led functions

From: Florian Fainelli
Date: Fri Feb 04 2022 - 12:07:02 EST




On 2/3/2022 8:02 AM, Tim Harvey wrote:
On Wed, Feb 2, 2022 at 7:12 PM Florian Fainelli <f.fainelli@xxxxxxxxx> wrote:



On 2/2/2022 5:01 PM, Andrew Lunn wrote:
As a person responsible for boot firmware through kernel for a set of
boards I continue to do the following to keep Linux from mucking with
various PHY configurations:
- remove PHY reset pins from Linux DT's to keep Linux from hard resetting PHY's
- disabling PHY drivers

What are your thoughts about this?

Hi Tim

I don't like the idea that the bootloader is controlling the hardware,
not linux.

This is really trying to take advantage of the boot loader setting
things up in a way that Linux can play dumb by using the Generic PHY
driver and being done with it. This works... until it stops, which
happens very very quickly in general. The perfect counter argument to
using the Generic PHY driver is when your system implements a low power
mode where the PHY loses its power/settings, comes up from suspend and
the strap configuration is insufficient and the boot loader is not part
of the resume path *prior* to Linux. In that case Linux needs to restore
the settings, but it needs a PHY driver for that.

Florian,

That makes sense - I'm always trying to figure out what the advantage
of using some of these PHY drivers really is vs disabling them.


If your concern Tim is with minimizing the amount of time the link gets
dropped and re-established, then there is not really much that can be
done that is compatible with Linux setting things up, short of
minimizing the amount of register writes that do need the "commit phase"
via BMCR.RESET.

No, my reasoning has nothing to do with link time - I have just run
into several cases where some new change in a PHY driver blatantly
either resets the PHY reverting to pin-strapping config which is wrong
(happend to me with DP83867 but replacing the 'reset' to a 'restart'
solved that) or imposes some settings without dt bindings to guide it
(this case with the LEDs) or imposes some settings based on 'new'
dt-bindings which I was simply not aware of (a lesser issue as dt
bindings can be added to resolve it).


I do agree that blindly imposing LED settings that are different than
those you want is not great, and should be remedied. Maybe you can
comment this part out in your downstream tree for a while until the LED
binding shows up (we have never been so close I am told).

or disable the driver in defconfig, or blacklist the module if I want
to do it via rootfs.

Can you point me to something I can look at for these new LED bindings
that are being worked on?


This is the latest attempt AFAICT:

https://lore.kernel.org/netdev/20211112153557.26941-1-ansuelsmth@xxxxxxxxx/
--
Florian