diff mbox series

[net,3/4] docs: net: dsa: update user MDIO bus documentation

Message ID 20231208193518.2018114-4-vladimir.oltean@nxp.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series Add some history to the DSA documentation | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net
netdev/ynl success SINGLE THREAD; Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success No Fixes tags, but series doesn't touch code
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 2 maintainers not CCed: florian.fainelli@broadcom.com linux-doc@vger.kernel.org
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 48 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Vladimir Oltean Dec. 8, 2023, 7:35 p.m. UTC
There are people who are trying to push the ds->user_mii_bus feature
past its sell-by date. I think part of the problem is the fact that the
documentation presents it as this great functionality.

Adapt it to 2023, where we have phy-handle to render it useless, at
least with OF.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 Documentation/networking/dsa/dsa.rst | 36 ++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 7 deletions(-)

Comments

Linus Walleij Dec. 8, 2023, 10:22 p.m. UTC | #1
On Fri, Dec 8, 2023 at 8:36 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:

> There are people who are trying to push the ds->user_mii_bus feature
> past its sell-by date. I think part of the problem is the fact that the
> documentation presents it as this great functionality.
>
> Adapt it to 2023, where we have phy-handle to render it useless, at
> least with OF.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Florian Fainelli Dec. 8, 2023, 10:36 p.m. UTC | #2
On 12/8/23 11:35, Vladimir Oltean wrote:
> There are people who are trying to push the ds->user_mii_bus feature
> past its sell-by date. I think part of the problem is the fact that the
> documentation presents it as this great functionality.
> 
> Adapt it to 2023, where we have phy-handle to render it useless, at
> least with OF.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>   Documentation/networking/dsa/dsa.rst | 36 ++++++++++++++++++++++------
>   1 file changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
> index 676c92136a0e..2cd91358421e 100644
> --- a/Documentation/networking/dsa/dsa.rst
> +++ b/Documentation/networking/dsa/dsa.rst
> @@ -397,19 +397,41 @@ perspective::
>   User MDIO bus
>   -------------
>   
> -In order to be able to read to/from a switch PHY built into it, DSA creates an
> -user MDIO bus which allows a specific switch driver to divert and intercept
> -MDIO reads/writes towards specific PHY addresses. In most MDIO-connected
> -switches, these functions would utilize direct or indirect PHY addressing mode
> -to return standard MII registers from the switch builtin PHYs, allowing the PHY
> -library and/or to return link status, link partner pages, auto-negotiation
> -results, etc.
> +The framework creates an MDIO bus for user ports (``ds->user_mii_bus``) when
> +both methods ``ds->ops->phy_read()`` and ``ds->ops->phy_write()`` are present.
> +However, this pointer may also be populated by the switch driver during the
> +``ds->ops->setup()`` method, with an MDIO bus managed by the driver.
> +
> +Its role is to permit user ports to connect to a PHY (usually internal) when
> +the more general ``phy-handle`` property is unavailable (either because the
> +MDIO bus is missing from the OF description, or because probing uses
> +``platform_data``).
> +
> +In most MDIO-connected switches, these functions would utilize direct or
> +indirect PHY addressing mode to return standard MII registers from the switch
> +builtin PHYs, allowing the PHY library and/or to return link status, link
> +partner pages, auto-negotiation results, etc.

The "and/or" did not read really well with the reset of the sentence, 
maybe just drop those two words?

>   
>   For Ethernet switches which have both external and internal MDIO buses, the
>   user MII bus can be utilized to mux/demux MDIO reads and writes towards either
>   internal or external MDIO devices this switch might be connected to: internal
>   PHYs, external PHYs, or even external switches.
>   
> +When using OF, the ``ds->user_mii_bus`` can be seen as a legacy feature, rather
> +than core functionality. Since 2014, the DSA OF bindings support the
> +``phy-handle`` property, which is a universal mechanism to reference a PHY,
> +be it internal or external.
> +
> +New switch drivers are encouraged to require the more universal ``phy-handle``
> +property even for user ports with internal PHYs. This allows device trees to
> +interoperate with simpler variants of the drivers such as those from U-Boot,
> +which do not have the (redundant) fallback logic for ``ds->user_mii_bus``.
> +
> +The only use case for ``ds->user_mii_bus`` in new drivers would be for probing
> +on non-OF through ``platform_data``. In the distant future where this will be
> +possible through software nodes, there will be no need for ``ds->user_mii_bus``
> +in new drivers at all.

That works for me, with the above addressed:

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Vladimir Oltean Dec. 9, 2023, 1:22 a.m. UTC | #3
On Fri, Dec 08, 2023 at 02:36:40PM -0800, Florian Fainelli wrote:
> On 12/8/23 11:35, Vladimir Oltean wrote:
> > diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
> > index 676c92136a0e..2cd91358421e 100644
> > --- a/Documentation/networking/dsa/dsa.rst
> > +++ b/Documentation/networking/dsa/dsa.rst
> > @@ -397,19 +397,41 @@ perspective::
> >   User MDIO bus
> >   -------------
> > -In order to be able to read to/from a switch PHY built into it, DSA creates an
> > -user MDIO bus which allows a specific switch driver to divert and intercept
> > -MDIO reads/writes towards specific PHY addresses. In most MDIO-connected
> > -switches, these functions would utilize direct or indirect PHY addressing mode
> > -to return standard MII registers from the switch builtin PHYs, allowing the PHY
> > -library and/or to return link status, link partner pages, auto-negotiation
> > -results, etc.
> > +The framework creates an MDIO bus for user ports (``ds->user_mii_bus``) when
> > +both methods ``ds->ops->phy_read()`` and ``ds->ops->phy_write()`` are present.
> > +However, this pointer may also be populated by the switch driver during the
> > +``ds->ops->setup()`` method, with an MDIO bus managed by the driver.
> > +
> > +Its role is to permit user ports to connect to a PHY (usually internal) when
> > +the more general ``phy-handle`` property is unavailable (either because the
> > +MDIO bus is missing from the OF description, or because probing uses
> > +``platform_data``).
> > +
> > +In most MDIO-connected switches, these functions would utilize direct or
> > +indirect PHY addressing mode to return standard MII registers from the switch
> > +builtin PHYs, allowing the PHY library and/or to return link status, link
> > +partner pages, auto-negotiation results, etc.
> 
> The "and/or" did not read really well with the reset of the sentence, maybe
> just drop those two words?

Fun fact, this is the second sentence from the existing text, moved
as-is a bit further down. Git blame on it says:
77760e94928f ("Documentation: networking: add a DSA document")

I do have the slight feeling that the paragraph is pitching sliced
bread (sorry). I did want to remove it completely, but I also wanted to
preserve a phrase about the direct/indirect thing.

How about replacing it with this?

Typically, the user MDIO bus accesses internal PHYs indirectly, by
reading and writing to the MDIO controller registers located in the
switch address space. Sometimes, especially if the switch is controlled
over MDIO by the host, its internal PHYs may also be accessible on the
same MDIO bus as the switch IP, but at a different MDIO address. In that
case, a direct access method for the internal PHYs is to implement the
MDIO access operations as diversions towards the parent MDIO bus of the
switch, at different MDIO addresses.

Conceivably, the direct access method could be extended to also target
external PHYs situated on the same MDIO bus as the switch, or on a
different MDIO bus entirely, referenced through ``platform_data``.
Linus Walleij Dec. 9, 2023, 9:49 p.m. UTC | #4
On Sat, Dec 9, 2023 at 2:22 AM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:

> How about replacing it with this?
>
> Typically, the user MDIO bus accesses internal PHYs indirectly, by
> reading and writing to the MDIO controller registers located in the
> switch address space. Sometimes, especially if the switch is controlled
> over MDIO by the host, its internal PHYs may also be accessible on the
> same MDIO bus as the switch IP, but at a different MDIO address. In that
> case, a direct access method for the internal PHYs is to implement the
> MDIO access operations as diversions towards the parent MDIO bus of the
> switch, at different MDIO addresses.
>
> Conceivably, the direct access method could be extended to also target
> external PHYs situated on the same MDIO bus as the switch, or on a
> different MDIO bus entirely, referenced through ``platform_data``.

This is clear and simple to understand, go with it.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Alvin Šipraga Dec. 10, 2023, 1:48 p.m. UTC | #5
On Fri, Dec 08, 2023 at 09:35:17PM +0200, Vladimir Oltean wrote:
> There are people who are trying to push the ds->user_mii_bus feature
> past its sell-by date. I think part of the problem is the fact that the
> documentation presents it as this great functionality.
> 
> Adapt it to 2023, where we have phy-handle to render it useless, at
> least with OF.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  Documentation/networking/dsa/dsa.rst | 36 ++++++++++++++++++++++------
>  1 file changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
> index 676c92136a0e..2cd91358421e 100644
> --- a/Documentation/networking/dsa/dsa.rst
> +++ b/Documentation/networking/dsa/dsa.rst
> @@ -397,19 +397,41 @@ perspective::
>  User MDIO bus
>  -------------
>  
> -In order to be able to read to/from a switch PHY built into it, DSA creates an
> -user MDIO bus which allows a specific switch driver to divert and intercept
> -MDIO reads/writes towards specific PHY addresses. In most MDIO-connected
> -switches, these functions would utilize direct or indirect PHY addressing mode
> -to return standard MII registers from the switch builtin PHYs, allowing the PHY
> -library and/or to return link status, link partner pages, auto-negotiation
> -results, etc.
> +The framework creates an MDIO bus for user ports (``ds->user_mii_bus``) when
> +both methods ``ds->ops->phy_read()`` and ``ds->ops->phy_write()`` are present.
> +However, this pointer may also be populated by the switch driver during the
> +``ds->ops->setup()`` method, with an MDIO bus managed by the driver.
> +
> +Its role is to permit user ports to connect to a PHY (usually internal) when
> +the more general ``phy-handle`` property is unavailable (either because the
> +MDIO bus is missing from the OF description, or because probing uses
> +``platform_data``).
> +
> +In most MDIO-connected switches, these functions would utilize direct or
> +indirect PHY addressing mode to return standard MII registers from the switch
> +builtin PHYs, allowing the PHY library and/or to return link status, link
> +partner pages, auto-negotiation results, etc.
>  
>  For Ethernet switches which have both external and internal MDIO buses, the
>  user MII bus can be utilized to mux/demux MDIO reads and writes towards either
>  internal or external MDIO devices this switch might be connected to: internal
>  PHYs, external PHYs, or even external switches.
>  
> +When using OF, the ``ds->user_mii_bus`` can be seen as a legacy feature, rather
> +than core functionality. Since 2014, the DSA OF bindings support the
> +``phy-handle`` property, which is a universal mechanism to reference a PHY,
> +be it internal or external.
> +
> +New switch drivers are encouraged to require the more universal ``phy-handle``
> +property even for user ports with internal PHYs. This allows device trees to
> +interoperate with simpler variants of the drivers such as those from U-Boot,
> +which do not have the (redundant) fallback logic for ``ds->user_mii_bus``.

Considering this policy, should we not emphasize that ds->user_mii_bus
and ds->ops->phy_{read,write}() ought to be left unpopulated by new
drivers, with the remark that if a driver wants to set up an MDIO bus,
it should store the corresponding struct mii_bus pointer in its own
driver private data? Just to make things crystal clear.

Regardless I think this is good!

Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

> +
> +The only use case for ``ds->user_mii_bus`` in new drivers would be for probing
> +on non-OF through ``platform_data``. In the distant future where this will be
> +possible through software nodes, there will be no need for ``ds->user_mii_bus``
> +in new drivers at all.
> +
>  Data structures
>  ---------------
>  
> -- 
> 2.34.1
>
Vladimir Oltean Dec. 11, 2023, 2:35 p.m. UTC | #6
On Sun, Dec 10, 2023 at 01:48:12PM +0000, Alvin Šipraga wrote:
> On Fri, Dec 08, 2023 at 09:35:17PM +0200, Vladimir Oltean wrote:
> > +When using OF, the ``ds->user_mii_bus`` can be seen as a legacy feature, rather
> > +than core functionality. Since 2014, the DSA OF bindings support the
> > +``phy-handle`` property, which is a universal mechanism to reference a PHY,
> > +be it internal or external.
> > +
> > +New switch drivers are encouraged to require the more universal ``phy-handle``
> > +property even for user ports with internal PHYs. This allows device trees to
> > +interoperate with simpler variants of the drivers such as those from U-Boot,
> > +which do not have the (redundant) fallback logic for ``ds->user_mii_bus``.
> 
> Considering this policy, should we not emphasize that ds->user_mii_bus
> and ds->ops->phy_{read,write}() ought to be left unpopulated by new
> drivers, with the remark that if a driver wants to set up an MDIO bus,
> it should store the corresponding struct mii_bus pointer in its own
> driver private data? Just to make things crystal clear.
> 
> Regardless I think this is good!
> 
> Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

I think something that makes a limited amount of sense is for DSA to
probe on OF, but not describe the MDIO controller in OF. Then, you'd
need ds->user_mii_bus. But new drivers should probably not do that
either; they should look into the MFD model and make the MDIO controller
be separate from (not a child of) the DSA switch. Then use a phy-handle
to it. So for new drivers, even this doesn't make too much sense, and
neither is it best to allocate the mii_bus from driver private code.

What makes no sense whatsoever is commit fe7324b93222 ("net: dsa:
OF-ware slave_mii_bus"). Because DSA provides ds->user_mii_bus to do
something reasonable when the MDIO controller isn't described in OF,
but this change assumes that it _is_ described in OF!

I'm not sure how and where to best put in words "let's not make DSA a
library for everything, just keep it for the switch". I'll think about
it some more.
Luiz Angelo Daros de Luca Dec. 13, 2023, 5:30 a.m. UTC | #7
> > > +When using OF, the ``ds->user_mii_bus`` can be seen as a legacy feature, rather
> > > +than core functionality. Since 2014, the DSA OF bindings support the
> > > +``phy-handle`` property, which is a universal mechanism to reference a PHY,
> > > +be it internal or external.
> > > +
> > > +New switch drivers are encouraged to require the more universal ``phy-handle``
> > > +property even for user ports with internal PHYs. This allows device trees to
> > > +interoperate with simpler variants of the drivers such as those from U-Boot,
> > > +which do not have the (redundant) fallback logic for ``ds->user_mii_bus``.
> >
> > Considering this policy, should we not emphasize that ds->user_mii_bus
> > and ds->ops->phy_{read,write}() ought to be left unpopulated by new
> > drivers, with the remark that if a driver wants to set up an MDIO bus,
> > it should store the corresponding struct mii_bus pointer in its own
> > driver private data? Just to make things crystal clear.
> >
> > Regardless I think this is good!
> >
> > Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
>
> I think something that makes a limited amount of sense is for DSA to
> probe on OF, but not describe the MDIO controller in OF. Then, you'd
> need ds->user_mii_bus. But new drivers should probably not do that
> either; they should look into the MFD model and make the MDIO controller
> be separate from (not a child of) the DSA switch. Then use a phy-handle
> to it. So for new drivers, even this doesn't make too much sense, and
> neither is it best to allocate the mii_bus from driver private code.
>
> What makes no sense whatsoever is commit fe7324b93222 ("net: dsa:
> OF-ware slave_mii_bus"). Because DSA provides ds->user_mii_bus to do
> something reasonable when the MDIO controller isn't described in OF,
> but this change assumes that it _is_ described in OF!
>
> I'm not sure how and where to best put in words "let's not make DSA a
> library for everything, just keep it for the switch". I'll think about
> it some more.

Hello Vladimir,

Sorry for my lack of understanding but I still didn't get it.

You are telling us we should not use user_mii_bus when the user MDIO
is described in the OF. Is it only about the "generic DSA mii" or also
about the custom ones the current drivers have? If it is the latter, I
don't know how to connect the dots between phy_read/write functions
and the port.

We have some drivers that define ds->user_mii_bus (not the "generic
DSA mii") with the MDIO described in OF. Are they wrong?

Alvin asked if we should store the mii_bus internally and not in the
ds->user_mii_bus but I don't think you answered it. Is it about where
we store the bus (for dropping user_mii_bus in the future)?

You now also mention using the MFD model (shouldn't it be cited in the
docs too?) but I couldn't identify a DSA driver example that uses that
model, with mdio outside the switch. Do we have one already? Would the
OF compatible with the MDF model be something like this?

my_mfd {
    compatible "aaa";
    switch {
        compatible = "bbb";
        ports {
            port@0: {
              phy-handle = <&ethphy0>;
            }
        }
    }
    mdio {
         compatible = "ccc";
         ethphy0: ethernet-phy@0 {
         }
    }
}

And for MDIO-connected switches, something like this?

eth0 {
     mdio {
         my_mfd {
            switch{...}
            mdio{...}
         }
     }
}

Is it only a suggestion on how to write a new driver or should we
change the existing ones to fit both models?

Regards,

Luiz
Vladimir Oltean Dec. 13, 2023, 12:06 p.m. UTC | #8
On Wed, Dec 13, 2023 at 02:30:48AM -0300, Luiz Angelo Daros de Luca wrote:
> Hello Vladimir,
> 
> Sorry for my lack of understanding but I still didn't get it.
> 
> You are telling us we should not use user_mii_bus when the user MDIO
> is described in the OF. Is it only about the "generic DSA mii" or also
> about the custom ones the current drivers have? If it is the latter, I
> don't know how to connect the dots between phy_read/write functions
> and the port.

It's about both. It has to do with the role that ds->user_mii_bus has
(see more below), not about who allocates it.

When the user_mii_bus is allocated by the driver, ds->ops->phy_read()
and ds->ops->phy_write() are not needed. They are only needed for DSA
to implement the ops of the generic user_mii_bus - see dsa_user_mii_bus_init().

> We have some drivers that define ds->user_mii_bus (not the "generic
> DSA mii") with the MDIO described in OF. Are they wrong?

This right here is the core ds->user_mii_bus functionality:

	ret = phylink_of_phy_connect(dp->pl, port_dn, phy_flags);
	if (ret == -ENODEV && ds->user_mii_bus) {
		/* We could not connect to a designated PHY or SFP, so try to
		 * use the switch internal MDIO bus instead
		 */
		ret = dsa_user_phy_connect(user_dev, dp->index, phy_flags);
	}

So, the ds->user_mii_bus is only used if we cant't do phylink_of_phy_connect(),
which follows the "phy-handle" reference.

The reasons (that I can see) why we can't do phylink_of_phy_connect() are:
(a) port_dn is NULL (probing on platform_data and not OF)
(b) port_dn exists, but has no phy-handle
(c) port_dn exists and has a phy-handle to a PHY that doesn't respond
    (wrong address, ?!)

Out of those, it only makes practical sense to design for (a) and (b).

If the ds->user_mii_bus has an OF node, it means that we are not in case
(a). We are in case (b).

Normally to be in case (b), it means that the device tree looks like this:

	switch {
		ports {
			port@0 {
				reg = <0>;
			};
		};
	};

aka port@0 is a user port with an internal PHY, not described in OF.

But to combine case (b) with the additional constraint that ds->user_mii_bus
has an OF node means to have this device tree:

	switch {
		ports {
			port@0 {
				reg = <0>;
				// no phy-handle to <&phy0>
			};
		};

		mdio {
			phy0: ethernet-phy@0 {
			};
		};
	};

Which is simply a broken device tree which should not be like that [1].
If the MDIO bus appears in OF, then _all_ its PHYs must appear in OF [2].
And if all PHYs appear in OF, then you must have a phy-handle to
reference them.

[1] There exists an exception, which is the hack added by Florian in
    commit 771089c2a485 ("net: dsa: bcm_sf2: Ensure that MDIO diversion
    is used"). There, he starts with a valid phy-handle in the device
    tree, but the DSA driver removes it. This makes phylink_of_phy_connect()
    fail, and "diverts" the code to dsa_user_phy_connect(), where the
    mii_bus read and write ops are in control of the DSA driver. Hence
    the name "diversion to ds->user_mii_bus". It's a huge hack, make no
    mistake about it.

[2] This is because __of_mdiobus_register() does this:

	/* Mask out all PHYs from auto probing.  Instead the PHYs listed in
	 * the device tree are populated after the bus has been registered */
	mdio->phy_mask = ~0;

> Alvin asked if we should store the mii_bus internally and not in the
> ds->user_mii_bus but I don't think you answered it. Is it about where
> we store the bus (for dropping user_mii_bus in the future)?

The ds->user_mii_bus is not just a dropbox, a pointer for random storage
that the DSA core generously offers... It would have had a void * type
if it was that, and DSA wouldn't have used it.

When a driver populates ds->user_mii_bus, it opts into its core functionality,
which I just described above. If you don't need it, don't use it. It's
as simple as that. Use your own private pointer to a struct mii_bus,
which stays out of dsa_user_phy_connect()'s business.

It's very unlikely that ds->user_mii_bus will be dropped though, unless
we resolve all other situations that need dsa_user_phy_connect() in some
other way. One of those situations is case (b) described above, aka
device trees with no phy-handle, which we don't want to break (for the
old drivers where that used to work).

I cannot make a blanket comment on whether all drivers that populate
ds->user_mii_bus with an OF-aware MDIO bus "are wrong". Probably out
of sheer ignorance, they connected and tangled together 2 logically
isolated code paths by using ds->user_mii_bus as dumb storage.

What was written carelessly now takes an expert to untangle. You now
have as much information as I do, so you can judge for yourself whether
the behavior given by dsa_user_phy_connect() is needed. My only ask is
to stop proliferating this monkey-see-monkey-do. If, on top of that,
we could eliminate the gratuitous uses of ds->user_mii_bus as plain
storage, that would be just great.

> 
> You now also mention using the MFD model (shouldn't it be cited in the
> docs too?) but I couldn't identify a DSA driver example that uses that
> model, with mdio outside the switch. Do we have one already? Would the
> OF compatible with the MDF model be something like this?

I did mention it already in the docs.

"But an internal microprocessor may have a very different view of the switch
address space (which is MMIO), and may have discrete Linux drivers for each
peripheral. In 2023, the ``ocelot_ext`` driver was added, which deviated from
the traditional DSA driver architecture. Rather than probing on the entire
``spi_device``, it created a multi-function device (MFD) top-level driver for
it (associated with the SoC at large), and the switching IP is only one of the
children of the MFD (it is a platform device with regmaps provided by its
parent). The drivers for each peripheral in this switch SoC are the same when
controlled over SPI and when controlled by the internal processor."

> 
> my_mfd {
>     compatible "aaa";
>     switch {
>         compatible = "bbb";
>         ports {
>             port@0: {
>               phy-handle = <&ethphy0>;
>             }
>         }
>     }
>     mdio {
>          compatible = "ccc";
>          ethphy0: ethernet-phy@0 {
>          }
>     }
> }
> 
> And for MDIO-connected switches, something like this?
> 
> eth0 {
>      mdio {
>          my_mfd {
>             switch{...}
>             mdio{...}
>          }
>      }
> }

Follow the clue given by the "ocelot_ext" reference. Search for the "mscc,vsc7512-switch"
compatible string, which leads you to the Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml
which is the schema for the entire SPI-connected SoC.

> 
> Is it only a suggestion on how to write a new driver or should we
> change the existing ones to fit both models?

First and foremost it is for new drivers. Read the actual patch:

"Authors of new switch drivers that use DSA are encouraged to have a wider view
of the peripherals on the chip that they are controlling, and to use the MFD
model to separate the components whenever possible. The general direction for
the DSA core is to focus only on the Ethernet switching IP and its ports.
``CONFIG_NET_DSA_HWMON`` was removed in 2017. Adding new methods to ``struct
dsa_switch_ops`` which are outside of DSA's core focus on Ethernet is strongly
discouraged."

For changing existing drivers, on one hand I would be glad to see effort
put there, but on the other, I need to be realistic and say that I also
tried to convert the sja1105 driver to the MFD model, and it's really,
really hard to be compatible with both device tree formats. So I'm not
holding my breath that I'll ever see conversion patches.
diff mbox series

Patch

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index 676c92136a0e..2cd91358421e 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -397,19 +397,41 @@  perspective::
 User MDIO bus
 -------------
 
-In order to be able to read to/from a switch PHY built into it, DSA creates an
-user MDIO bus which allows a specific switch driver to divert and intercept
-MDIO reads/writes towards specific PHY addresses. In most MDIO-connected
-switches, these functions would utilize direct or indirect PHY addressing mode
-to return standard MII registers from the switch builtin PHYs, allowing the PHY
-library and/or to return link status, link partner pages, auto-negotiation
-results, etc.
+The framework creates an MDIO bus for user ports (``ds->user_mii_bus``) when
+both methods ``ds->ops->phy_read()`` and ``ds->ops->phy_write()`` are present.
+However, this pointer may also be populated by the switch driver during the
+``ds->ops->setup()`` method, with an MDIO bus managed by the driver.
+
+Its role is to permit user ports to connect to a PHY (usually internal) when
+the more general ``phy-handle`` property is unavailable (either because the
+MDIO bus is missing from the OF description, or because probing uses
+``platform_data``).
+
+In most MDIO-connected switches, these functions would utilize direct or
+indirect PHY addressing mode to return standard MII registers from the switch
+builtin PHYs, allowing the PHY library and/or to return link status, link
+partner pages, auto-negotiation results, etc.
 
 For Ethernet switches which have both external and internal MDIO buses, the
 user MII bus can be utilized to mux/demux MDIO reads and writes towards either
 internal or external MDIO devices this switch might be connected to: internal
 PHYs, external PHYs, or even external switches.
 
+When using OF, the ``ds->user_mii_bus`` can be seen as a legacy feature, rather
+than core functionality. Since 2014, the DSA OF bindings support the
+``phy-handle`` property, which is a universal mechanism to reference a PHY,
+be it internal or external.
+
+New switch drivers are encouraged to require the more universal ``phy-handle``
+property even for user ports with internal PHYs. This allows device trees to
+interoperate with simpler variants of the drivers such as those from U-Boot,
+which do not have the (redundant) fallback logic for ``ds->user_mii_bus``.
+
+The only use case for ``ds->user_mii_bus`` in new drivers would be for probing
+on non-OF through ``platform_data``. In the distant future where this will be
+possible through software nodes, there will be no need for ``ds->user_mii_bus``
+in new drivers at all.
+
 Data structures
 ---------------