Subject: Re: [PATCH v2] net: phy: marvell: Honor phy LED set by system firmware on a Dell hardware

From: Andrew Lunn <[email-masked]>
Date: Wed, 16 Feb 2022 08:39:16 +0100

> > > This is an ACPI based platform and we are working on new firmware
> > > property "use-firmware-led" to give driver a hint:
> > > ...
> > >     Scope (_SB.PC00.OTN0)
> > >     {
> > >         Name (_DSD, Package (0x02)  // _DSD: Device-Specific Data
> > >         {
> > >             ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device
> > > Properties for _DSD */,
> > >             Package (0x01)
> > >             {
> > >                 Package (0x02)
> > >                 {
> > >                     "use-firmware-led",
> > >                     One
> > >                 }
> > >             }
> > >         })
> > >     }
> > > ...
> > >
> > > Because the property is under PCI device namespace, I am not sure how
> > > to (cleanly) bring the property from the phylink side to phydev side.
> > > Do you have any suggestion?
> >
> > I'm no ACPI expert, but i think
> > Documentation/firmware-guide/acpi/dsd/phy.rst gives you the basis:
> >
> >     During the MDIO bus driver initialization, PHYs on this bus are probed
> >     using the _ADR object as shown below and are registered on the MDIO bus.
> >
> >       Scope(\_SB.MDI0)
> >       {
> >         Device(PHY1) {
> >           Name (_ADR, 0x1)
> >         } // end of PHY1
> >
> >         Device(PHY2) {
> >           Name (_ADR, 0x2)
> >         } // end of PHY2
> >       }
> >
> > These are the PHYs on the MDIO bus. I _think_ that next to the Name,
> > you can add additional properties, like your "use-firmware-led". This
> > would then be very similar to DT, which is in effect what ACPI is
> > copying. So you need to update this document with your new property,
> > making it clear that this property only applies to boot, not
> > suspend/resume. And fwnode_mdiobus_register_phy() can look for the
> > property and set a flag in the phydev structure indicating that ACPI
> > is totally responsible for LEDs at boot time.
> 
> The problem here is there's no MDIO bus in ACPI namespace, namely no
> "Scope(\_SB.MDI0)" on this platform.

So add it. Basically, copy what DT does. I assume there is a node for
the Ethernet device? And is the MDIO bus driver instantiated by the
Ethernet device? So you can add the MDIO node as a sub node of the
Ethernet device. When you register the MDIO bus using
acpi_mdiobus_register() pass a pointer to this MDIO sub node.

     Andrew