[U-Boot] [PATCH 4/4 v3] test: dm: add a test for MDIO MUX DM uclass

[Alex Marginean]

Hi Bin,

On 7/8/2019 10:40 AM, Bin Meng wrote:
> On Tue, Jun 18, 2019 at 10:58 PM Alexandru Marginean
> <alexandru.marginean at nxp.com> 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 <alexm.osslist at gmail.com>
>> ---
>>
>> 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
>