[PATCH 1/2] net: dsa: fix phydev->speed being uninitialized for the CPU port fixed PHY

[Vladimir Oltean]

On Fri, Mar 25, 2022 at 02:03:56PM -0700, Tim Harvey wrote:
> On Fri, Mar 25, 2022 at 11:07 AM Vladimir Oltean
> <vladimir.oltean at nxp.com> wrote:
> >
> > Hi Tim,
> >
> > On Fri, Mar 25, 2022 at 09:53:20AM -0700, Tim Harvey wrote:
> > > Vladimir,
> > >
> > > I came across this while looking for the best place to configure cpu
> > > port interface mode (ie rgmii id) for the mv88e61xx dsa driver I'm
> > > working on. Note that this patch causes port_probe to be called on the
> > > cpu port 'before' the master device has been probed. I'm not sure if
> > > this is intended or not?
> > >
> > > In my case I was looking to configure the cpu port interface mode when
> > > the port was probed but I can't do that because it happens before the
> > > switch is probed because of some things that need to happen there.
> > > Best Regards,
> > >
> > > Tim
> >
> > You're past the DM_MDIO probing issues now? Glad to hear that.
> > Probing the DSA CPU port before the master wasn't necessarily the
> > intention, but then again, I'm not sure that there's a strict ordering
> > guarantee between the two that needs to be satisfied?
> >
> > What do you need exactly? We could probably always reverse the
> > device_probe(master) call with the probing of the CPU port if the
> > ordering turns out to be a real problem. I can regression-test the
> > change on my boards, but I'd like to understand the need you have, maybe
> > even document it so that future changes are aware of it.
> 
> Yes, I've got the mdio probing working now. The mv88e61xx driver
> supports several chips that have different register offsets that need
> to be known before indirect read/write and port read/write can be
> used. That detection happens early on so I have it in the dsa_probe. I
> would prefer to configure the cpu port interface mode (mac mode, link
> speed/duplex etc) once instead of doing it every time the cpu port
> gets enabled so I want to put that in the dsa_probe but at that time I
> don't have the phy_device to determine interface mode (I suppose I
> could get it from dt?). I noticed dsa_class calls ops->port_probe for
> the cpu port early so that seemed like a great place to do all that,
> but then I found my switch dsa_probe hadn't been called yet so I
> haven't identified the switch and set the register offsets yet.
> 
> I have worked around the fact that the port_probe is called for the
> cpu port before the switch is probed I just wasn't sure if something
> in this patch should be changed in case others fall into this trap in
> the future. dsa_port_probe probes the master before calling
> ops->port_probe with the comment 'We depend on the master device for
> proper operation' so I just figured the same should be done for the
> pre_probe.

Sorry, that was quite a blunder, we should definitely ensure that
U_BOOT_DRIVER :: probe gets called before dsa_ops :: port_probe.
I've made a change moving the port_probe call for the CPU port to
dsa_post_probe(), tested it in the sandbox, and it appears to work.

> I hope to send a series in the next few days but I do have a few items
> I still need to fix:
> 
> 1. my board currently uses the mv88e61xx_hw_reset weak override to
> configure LEDs, GPIO's using direct register writes as I need one of
> the GPIO's to be configured as a 125MHz RGMII_RCLK and configure MAC
> interface mode(rgmii delays). I've moved the mac interface config into
> the driver based on the dt props (phy-mode and fixed-link subnode) but
> am still not sure how to go about dealing with the 'very custom' LED
> and GPIO config without the hassle of defining new dt bindings. I was
> hoping to use board_phy_config() but when that is called for the
> switch the phy_id is a generic PHY_FIXED_ID and when called for the
> ports the phydev->bus uses indirect register writes which can't be
> used to configure the gpios.

How are these configs handled in Linux?

> 2. While my switch mdio bus is probing now as well as my fec_dm_mdio
> bus I'm not clear how to properly get the struct mii_dev * associated
> with the fec_dm_mdio from the dsa switch. Currently I'm using
> miiphy_get_dev_by_name("mdio") which is horrible. How do I get the
> mii_bus or even the udevice of an dm_mdio bus associated with a dm_eth
> device?

Hmm, isn't the MDIO bus udevice the dev->parent of the switch?
Assuming that the reason you need this is for MDIO input/output towards
the switch. Although my suggestion would be to wrap this I/O behind
dm_mdio_read(struct udevice *dev /* MDIO child device */) and
dm_mdio_write(struct udevice *dev), rather than poking inside struct
udevice from the mv88e61xx driver.