Netdev List
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Aleksei Sviridkin <f@lex.la>
Cc: andrew+netdev@lunn.ch, hkallweit1@gmail.com,
	linux@armlinux.org.uk, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, horms@kernel.org,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	ericwouds@gmail.com, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH net-next v2 0/7] net: mdio: an MDIO device driver for the Airoha EN8811H
Date: Wed, 9 Sep 2026 21:01:14 +0200	[thread overview]
Message-ID: <44f163ed-8362-4aad-99c9-f8709dfbed76@lunn.ch> (raw)
In-Reply-To: <20260908155707.4164559-1-f@lex.la>

On Tue, Sep 08, 2026 at 03:57:00PM +0000, Aleksei Sviridkin wrote:
> The Airoha EN8811H answers its PHY ID from power-on, but it is an MD32
> microcontroller until the host loads firmware into its volatile RAM.
> Today one driver owns both roles: the PHY driver downloads the firmware
> from .probe(), which works only when the files are readable by then and
> leaves the reset line in the hands of a PHY node whose detach wipes what
> was loaded.

I think because you are sending this as RFC, the AI bots are ignoring
it. I also think it is not getting build testing etc.

> What I am asking:
> 
>  1. The binding lets software timing pick the topology: the same chip
>     on the same board is a plain PHY node when its firmware is in the
>     bootloader, and an MCU with a child bus when the firmware arrives
>     with the rootfs. Move the files into an initramfs and the
>     recommended description changes without a wire moving. The hardware
>     argument that does hold is narrower - the reset line belongs to the
>     MD32 core rather than to the PHY the firmware creates, and a PHY
>     node that owns it wipes the firmware on detach - and it argues for
>     the MCU node, not for the child bus. If the child bus has to be
>     justified by hardware too, I do not have that argument, and the
>     honest options are to describe the chip one way always, or to say
>     plainly in the binding that this is a driver model choice. Which
>     would you rather see?

One way to argue it is that the MDIO bus it connected to the MCU. The
MCU, once booted, forwards the MDIO bus through to the PHY part of the
package.

>  3. Removing the parent MDIO controller reaches this driver's remove
>     with the child PHY attached, and nothing pins the parent's owner.
>     Closing it wants a way for a bus to tell its PHYs' consumers to let
>     go, which is a phy-core change. The failed-resume path above wants
>     the same channel for "re-initialise" rather than "let go", so that
>     is two uses for one mechanism. Is it worth doing here?

Consider a normal setup, what happens when you remove the MDIO
controller when the PHY is directly connected?  And look at what MDIO
mux drivers do, which is not so different a use case.

>  4. MDIO_AIROHA_EN8811H sits in the "MDIO controller drivers" menu,
>     though it drives a device on a bus and not a controller. The
>     mdio-mux entries set the precedent, but say if you want it
>     elsewhere.

It is fine. I would also add a select or depends on the PHY driver
Kconfig stanza to link it to the MDIO_AIROHA_EN8811H driver.

>  5. The child bus accepts exactly one address, the one this node
>     occupies on the parent, and I do not think the schema can say so.
>     What it does say now is that a child named ethernet-phy@N is
>     validated as one, and that it must not carry reset-gpios or its
>     timings - the whole reason this node exists is that a PHY-node
>     reset is what phy_detach() asserts, and it erases the firmware.
>     What it cannot say is that a differently named child is refused:
>     mdio.yaml's own patternProperties evaluates any node@hex and its
>     additionalProperties: true admits the rest, so unevaluatedProperties
>     has nothing left to reject. Is prose enough for the address, and is
>     there an idiom I am missing for the rest?

Prose is enough. And C code to catch DT authors who get it wrong. But
you also need to be careful to not break backwards compatibility when
the PHY driver is used alone.

>  6. include/linux/mdio/mdio-airoha-en8811h.h is named for the driver
>     that consumes it, while its four neighbours in that directory name
>     the driver that implements them - here the implementation is
>     drivers/net/phy/air_phy_lib.c. Where should this header live, and
>     should the MAINTAINERS entry that now claims it also claim the
>     library files it declares?

The location is fine. MAINTAINERS need to list it. What you might want
to do is list it twice, once in the PHY driver MAINTAINERS record, and
again in the MDIO driver section. Any changes need to be coordinated
in both, so all Maintainers should get notified.

    Andrew

---
pw-bot: cr

  parent reply	other threads:[~2026-09-09 19:01 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-08 15:57 [RFC PATCH net-next v2 0/7] net: mdio: an MDIO device driver for the Airoha EN8811H Aleksei Sviridkin
2026-09-08 15:57 ` [RFC PATCH net-next v2 1/7] dt-bindings: net: add Airoha EN8811H PHY MCU Aleksei Sviridkin
2026-09-08 15:57 ` [RFC PATCH net-next v2 2/7] net: phy: add mdiodev_lock(), mdiodev_lock_nested() and mdiodev_unlock() Aleksei Sviridkin
2026-09-08 15:57 ` [RFC PATCH net-next v2 3/7] net: phy: air: type the buckpbus core on the mdio device Aleksei Sviridkin
2026-09-08 15:57 ` [RFC PATCH net-next v2 4/7] net: phy: air: move the EN8811H firmware download into the library Aleksei Sviridkin
2026-09-08 15:57 ` [RFC PATCH net-next v2 5/7] net: phy: air: skip the download when the MD32 is already running Aleksei Sviridkin
2026-09-08 15:57 ` [RFC PATCH net-next v2 6/7] net: mdio: add Airoha EN8811H MDIO device driver Aleksei Sviridkin
2026-09-08 15:57 ` [RFC PATCH net-next v2 7/7] net: mdio: en8811h: add the nested bus Aleksei Sviridkin
2026-09-09 19:01 ` Andrew Lunn [this message]
2026-09-10 15:38   ` [RFC PATCH net-next v2 0/7] net: mdio: an MDIO device driver for the Airoha EN8811H Aleksei Sviridkin
2026-09-10 20:49   ` Aleksei Sviridkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=44f163ed-8362-4aad-99c9-f8709dfbed76@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=andrew+netdev@lunn.ch \
    --cc=conor+dt@kernel.org \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=edumazet@google.com \
    --cc=ericwouds@gmail.com \
    --cc=f@lex.la \
    --cc=hkallweit1@gmail.com \
    --cc=horms@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox