From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Marginean Date: Mon, 8 Jul 2019 18:43:22 +0300 Subject: [U-Boot] [PATCH 4/4 v3] test: dm: add a test for MDIO MUX DM uclass In-Reply-To: References: <20190618145823.606-1-alexandru.marginean@nxp.com> <20190618145823.606-4-alexandru.marginean@nxp.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Bin, On 7/8/2019 10:40 AM, Bin Meng wrote: > On Tue, Jun 18, 2019 at 10:58 PM Alexandru Marginean > wrote: >> >> Adds a test using a makeshift MDIO MUX. The test is based on the existing >> MDIO test. It uses the last emulated PHY register to verify MUX selection. >> >> Signed-off-by: Alex Marginean >> --- >> >> Changes in v2: >> - no change >> Changes in v3: >> - no change, just fighting with the email server >> >> arch/Kconfig | 1 + >> arch/sandbox/dts/test.dts | 21 +++++++- >> drivers/net/Kconfig | 10 ++++ >> drivers/net/Makefile | 1 + >> drivers/net/mdio_mux_sandbox.c | 97 ++++++++++++++++++++++++++++++++++ >> test/dm/Makefile | 1 + >> test/dm/mdio_mux.c | 80 ++++++++++++++++++++++++++++ >> 7 files changed, 210 insertions(+), 1 deletion(-) >> create mode 100644 drivers/net/mdio_mux_sandbox.c >> create mode 100644 test/dm/mdio_mux.c >> >> diff --git a/arch/Kconfig b/arch/Kconfig >> index 1e62a7615d..1a0f1ab8a7 100644 >> --- a/arch/Kconfig >> +++ b/arch/Kconfig >> @@ -122,6 +122,7 @@ config SANDBOX >> imply PCH >> imply PHYLIB >> imply DM_MDIO >> + imply DM_MDIO_MUX >> >> config SH >> bool "SuperH architecture" >> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts >> index dd50a951a8..a05e437abf 100644 >> --- a/arch/sandbox/dts/test.dts >> +++ b/arch/sandbox/dts/test.dts >> @@ -808,7 +808,26 @@ >> dma-names = "m2m", "tx0", "rx0"; >> }; >> >> - mdio-test { >> + /* >> + * keep mdio-mux ahead of mdio, u-boot doesn't do reference count on > > nits: U-Boot > >> + * these devices and we don't want mdio-parent-bus to be released before >> + * the mux. > > I did not get it why there is a ordering issue? Could you please elaborate? I can try. At the end of 'ut dm' classes and devices are removed, which by itself is fine. When MDIO buses are removed there's a reset issued, which is also fine. There is no explicit dependency between the mux and parent MDIO though, not described in the way that the code removing these devices would understand. If parent MDIO is removed first, then removal/reset of mux child MDIOs triggers a new probe of the parent MDIO device to execute reset on child MDIO buses. This leaves parent MDIO probed at the end of the test and test-main doesn't like that as it can't remove the MDIO class with parent MDIO left active. The call sequence looks like this: Parent MDIO dev is removed first: dm_mdio_pre_remove: mdio-test mdio_sandbox_reset: mdio-test mdio_sandbox_remove: mdio-test Then MUX is removed, along with its children: dm_mdio_pre_remove: mdio-ch-test at 0 mmux_reset: mdio-mux-test/mdio-ch-test at 0 mdio_sandbox_probe: mdio-test dm_mdio_post_probe: mdio-test mdio_sandbox_reset: mdio-test dm_mdio_pre_remove: mdio-ch-test at 1 mmux_reset: mdio-mux-test/mdio-ch-test at 1 mdio_sandbox_reset: mdio-test test/dm/test-main.c:72, dm_test_destroy(): 0 == uclass_destroy(uc): Expected 0, got -22 I think I can add a uclass device post_remove API and use that in MUX uclass to remove parent MDIO device, but that looks like an unsafe thing to do. There is no way for the MUX uclass code to know whether any other piece of software still holds a reference to parent MDIO device. I could skip calling remove for child MDIOs during removal of the MUX, to avoid probing parent MDIO again, but that looks like a hack. The fix to the general problem would involve some form of reference counting per device, but it doesn't make sense to add that just for a MDIO MUX test. In practice not removing a device that's no longer used isn't a big deal for u-boot I think. Keeping dts nodes in that order works, but it is also a hack, it's pretty obscure, I'm not super happy with that either. Any suggestions? Thank you! Alex > >> + */ >> + mdio-mux-test { >> + compatible = "sandbox,mdio-mux"; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + mdio-parent-bus = <&mdio>; >> + >> + mdio-ch-test at 0 { >> + reg = <0>; >> + }; >> + mdio-ch-test at 1 { >> + reg = <1>; >> + }; >> + }; >> + > > Test codes looks good to me though. > > Regards, > Bin > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > https://lists.denx.de/listinfo/u-boot >