Re: [U-Boot] [PATCH 04/11] dm: test: Add tests for the generic PHY uclass

Hi Jean-Jacques,

On 14 April 2017 at 05:08, Jean-Jacques Hiblot jjhiblot@ti.com wrote:

Those tests check:

• the ability for a phy-user to get a phy device a reference in the device-tree
• the ability to perform operations on the phy (init,deinit,on,off)
• the behavior of the uclass when optional operations are not implemented

Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com

arch/sandbox/dts/test.dts | 9 +++++ configs/sandbox_defconfig | 2 + configs/sandbox_noblk_defconfig | 2 + configs/sandbox_spl_defconfig | 3 ++ drivers/phy/Kconfig | 9 +++++ drivers/phy/Makefile | 1 + drivers/phy/sandbox-phy.c | 78 ++++++++++++++++++++++++++++++++++++ test/dm/Makefile | 1 + test/dm/generic_phy.c | 89 +++++++++++++++++++++++++++++++++++++++++ 9 files changed, 194 insertions(+) create mode 100644 drivers/phy/sandbox-phy.c create mode 100644 test/dm/generic_phy.c

This looks really good, just some nits.

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index fff175d..918c899 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -59,6 +59,15 @@ ping-add = <3>; };

•   gen_phy: gen_phy {
  

•           compatible = "sandbox,phy";
  

•   };
  

•   gen_phy_user: gen_phy_user {
  

•           compatible = "simple-bus";
  

•           phy = <&gen_phy>;
  

•   };
  

•   some-bus {
            #address-cells = <1>;
            #size-cells = <0>;
  

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 7f3f5ac..42116cf 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -164,6 +164,8 @@ CONFIG_CONSOLE_ROTATION=y CONFIG_CONSOLE_TRUETYPE=y CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y CONFIG_VIDEO_SANDBOX_SDL=y +CONFIG_GENERIC_PHY=y +CONFIG_PHY_SANDBOX=y CONFIG_CMD_DHRYSTONE=y CONFIG_TPM=y CONFIG_LZ4=y diff --git a/configs/sandbox_noblk_defconfig b/configs/sandbox_noblk_defconfig index 3f8e70d..7a5cd4b 100644 --- a/configs/sandbox_noblk_defconfig +++ b/configs/sandbox_noblk_defconfig @@ -166,6 +166,8 @@ CONFIG_CONSOLE_ROTATION=y CONFIG_CONSOLE_TRUETYPE=y CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y CONFIG_VIDEO_SANDBOX_SDL=y +CONFIG_GENERIC_PHY=y +CONFIG_PHY_SANDBOX=y CONFIG_CMD_DHRYSTONE=y CONFIG_TPM=y CONFIG_LZ4=y diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index ade6714..9b4cf39 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -170,6 +170,9 @@ CONFIG_CONSOLE_ROTATION=y CONFIG_CONSOLE_TRUETYPE=y CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y CONFIG_VIDEO_SANDBOX_SDL=y +CONFIG_GENERIC_PHY=y +CONFIG_SPL_GENERIC_PHY=y +CONFIG_PHY_SANDBOX=y CONFIG_CMD_DHRYSTONE=y CONFIG_TPM=y CONFIG_LZ4=y diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index d66c9e3..032b932 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -29,4 +29,13 @@ config SPL_GENERIC_PHY compatible as possible with the equivalent framework found in the linux kernel.

+config PHY_SANDBOX

•   bool "Sandbox PHY support"
  

•   depends on SANDBOX
  

•   depends on GENERIC_PHY
  

•   help
  

•     This select a dummy sandbox PHY driver. It used only to implement
  

•     unit tests for the generic phy framework
  

endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index b29a8b9..0125844 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -1,2 +1,3 @@ obj-$(CONFIG_$(SPL_)GENERIC_PHY) += phy-uclass.o +obj-$(CONFIG_PHY_SANDBOX) += sandbox-phy.o

diff --git a/drivers/phy/sandbox-phy.c b/drivers/phy/sandbox-phy.c new file mode 100644 index 0000000..1c60308 --- /dev/null +++ b/drivers/phy/sandbox-phy.c @@ -0,0 +1,78 @@ +/*

• • Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/
• • Written by Jean-Jacques Hiblot jjhiblot@ti.com
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <dm.h> +#include <generic-phy.h>

+struct sandbox_phy_priv {

•   int initialized;
  

•   int on;
  

bool?

+};

+static int sandbox_phy_power_on(struct udevice *dev) +{

•   struct sandbox_phy_priv *priv = dev_get_priv(dev);
  

blank line between decls and rest of code

•   if (!priv->initialized)
  

•           return -EIO;
  

•   priv->on = 1;
  

blank line before return true/false better than 0/1 I think

•   return 0;
  

+}

+static int sandbox_phy_power_off(struct udevice *dev) +{

•   struct sandbox_phy_priv *priv = dev_get_priv(dev);
  

•   if (!priv->initialized)
  

•           return -EIO;
  

•   priv->on = 0;
  

•   return 0;
  

+}

+static int sandbox_phy_init(struct udevice *dev) +{

•   struct sandbox_phy_priv *priv = dev_get_priv(dev);
  

•   priv->initialized = 1;
  

•   priv->on = 0;
  

•   return 0;
  

+}

+static int sandbox_phy_exit(struct udevice *dev) +{

•   struct sandbox_phy_priv *priv = dev_get_priv(dev);
  

•   priv->initialized = 0;
  

•   priv->on = 0;
  

•   return 0;
  

+}

+static int sandbox_phy_probe(struct udevice *dev) +{

•   struct sandbox_phy_priv *priv = dev_get_priv(dev);
  

•   priv->initialized = 0;
  

•   priv->on = 0;
  

•   return 0;
  

+}

+static struct generic_phy_ops sandbox_phy_ops = {

•   .power_on = sandbox_phy_power_on,
  

•   .power_off = sandbox_phy_power_off,
  

•   .init = sandbox_phy_init,
  

•   .exit = sandbox_phy_exit,
  

+};

+static const struct udevice_id sandbox_phy_ids[] = {

•   { .compatible = "sandbox,phy" },
  

•   { }
  

+};

+U_BOOT_DRIVER(phy_sandbox) = {

•   .name           = "phy_sandbox",
  

•   .id             = UCLASS_PHY,
  

•   .of_match       = sandbox_phy_ids,
  

•   .ops            = &sandbox_phy_ops,
  

•   .probe          = sandbox_phy_probe,
  

•   .priv_auto_alloc_size = sizeof(struct sandbox_phy_priv),
  

+}; diff --git a/test/dm/Makefile b/test/dm/Makefile index 1885e17..e2b7d43 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -41,4 +41,5 @@ obj-$(CONFIG_TIMER) += timer.o obj-$(CONFIG_DM_VIDEO) += video.o obj-$(CONFIG_ADC) += adc.o obj-$(CONFIG_SPMI) += spmi.o +obj-$(CONFIG_GENERIC_PHY) += generic_phy.o

check ordering

endif diff --git a/test/dm/generic_phy.c b/test/dm/generic_phy.c new file mode 100644 index 0000000..08b862a --- /dev/null +++ b/test/dm/generic_phy.c @@ -0,0 +1,89 @@ +/*

• • Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/
• • Written by Jean-Jacques Hiblot jjhiblot@ti.com
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <dm.h> +#include <dm/test.h> +#include <test/ut.h> +#include <generic-phy.h>

This should go below dm.h

+DECLARE_GLOBAL_DATA_PTR;

+/* Base test of the phy uclass */ +static int dm_test_phy_base(struct unit_test_state *uts) +{

•   struct udevice *dev_method1;
  

•   struct udevice *dev_method2;
  

•   struct udevice *parent;
  

drop extra blank ilne

•   /* Get the device using the phy device*/
  

•   ut_assertok(uclass_get_device_by_name(UCLASS_SIMPLE_BUS,
  

•                                         "gen_phy_user", &parent));
  

•   /* Get the phy device with std uclass function */
  

•   ut_assertok(uclass_get_device_by_name(UCLASS_PHY, "gen_phy",
  

•                                         &dev_method1));
  

•   /*
  

•    * Get the phy device from user device and compare with the one
  

•    * obtained with the previous method.
  

•    */
  

•   dev_method2 = dm_generic_phy_get(parent, "phy");
  

This should be able to use ut_assertok() once the function returns an error code.

•   ut_assertnonnull(dev_method2);
  

•   ut_assertok_ptr(dev_method2);
  

•   ut_asserteq_ptr(dev_method1, dev_method2);
  

•   /* Try to get a non-existing phy */
  

•   ut_asserteq(-ENODEV, uclass_get_device(UCLASS_PHY, 1, &dev_method2));
  

•   dev_method2 = dm_generic_phy_get(parent, "phy_not_existing");
  

•   ut_assert(IS_ERR_OR_NULL(dev_method2));
  

•   return 0;
  

+} +DM_TEST(dm_test_phy_base, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);

+/* Test of the phy uclass using the sandbox phy driver operations */ +static int dm_test_phy_ops(struct unit_test_state *uts) +{

•   struct udevice *dev;
  

•   struct udevice *parent;
  

•   ut_assertok(uclass_get_device_by_name(UCLASS_SIMPLE_BUS,
  

•                                         "gen_phy_user", &parent));
  

•   dev = dm_generic_phy_get(parent, "phy");
  

•   ut_assertnonnull(dev);
  

•   ut_assertok_ptr(dev);
  

•   /* test normal operations */
  

•   ut_assertok(generic_phy_init(dev));
  

•   ut_assertok(generic_phy_power_on(dev));
  

•   ut_assertok(generic_phy_power_off(dev));
  

•   /*
  

•    * test operations after exit().
  

•    * The sandbox phy driver does not allow it.
  

•    */
  

•   ut_assertok(generic_phy_exit(dev));
  

•   ut_assert(generic_phy_power_on(dev) != 0);
  

•   ut_assert(generic_phy_power_off(dev) != 0);
  

•   /*
  

•    * test normal operations again (after re-init)
  

•    */
  

•   ut_assertok(generic_phy_init(dev));
  

•   ut_assertok(generic_phy_power_on(dev));
  

•   ut_assertok(generic_phy_power_off(dev));
  

•   /*
  

•    * test calling unimplemented feature.
  

•    * The call is expected to succeed
  

•    */
  

•   ut_assertok(generic_phy_reset(dev));
  

•   return 0;
  

+}

+DM_TEST(dm_test_phy_ops, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);

1.9.1

Great test, thanks.

Regards, Simon