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.olt...@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.