[U-Boot] [PATCH v6 01/11] syscon: dm: Add a new method to get a regmap from DTS

[Simon Glass]

On 25 May 2018 at 07:51, Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
> syscon_regmap_lookup_by_phandle() can be used to get the regmap of a syscon
> device from a reference in the DTS. It operates similarly to the linux
> version of the namesake function.
>
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot at ti.com>
>
> ---
>
> Changes in v6: None
> Changes in v5: None
> Changes in v4:
> - Fix word missing in commit log
>
> Changes in v3:
> - in syscon_regmap_lookup_by_phandle(), use dev_dbg() instead of printf()
> - added unit test for syscon_regmap_lookup_by_phandle()
>
> Changes in v2: None
>
>  arch/sandbox/dts/test.dts    |  6 ++++--
>  drivers/core/syscon-uclass.c | 23 +++++++++++++++++++++++
>  include/syscon.h             | 13 +++++++++++++
>  test/dm/syscon.c             | 29 +++++++++++++++++++++++++++++
>  4 files changed, 69 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

Sorry noticed a nit and a few questions below.

>
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index 5a0f187..9812e30 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -327,6 +327,8 @@
>
>                 test4 {
>                         compatible = "denx,u-boot-probe-test";
> +                       first-syscon = <&syscon0>;
> +                       second-sys-ctrl = <&another_system_controller>;
>                 };
>         };
>
> @@ -396,12 +398,12 @@
>                 };
>         };
>
> -       syscon at 0 {
> +       syscon0: syscon at 0 {
>                 compatible = "sandbox,syscon0";
>                 reg = <0x10 4>;
>         };
>
> -       syscon at 1 {
> +       another_system_controller: syscon at 1 {
>                 compatible = "sandbox,syscon1";
>                 reg = <0x20 5
>                         0x28 6
> diff --git a/drivers/core/syscon-uclass.c b/drivers/core/syscon-uclass.c
> index 303e166..661cf61 100644
> --- a/drivers/core/syscon-uclass.c
> +++ b/drivers/core/syscon-uclass.c
> @@ -53,6 +53,29 @@ static int syscon_pre_probe(struct udevice *dev)
>  #endif
>  }
>
> +struct regmap *syscon_regmap_lookup_by_phandle(struct udevice *dev,
> +                                              const char *name)
> +{
> +       struct udevice *syscon;
> +       struct regmap *r;
> +       int err;
> +
> +       err = uclass_get_device_by_phandle(UCLASS_SYSCON, dev,
> +                                          name, &syscon);
> +       if (err) {
> +               dev_dbg(dev, "unable to find syscon device\n");
> +               return ERR_PTR(err);
> +       }
> +
> +       r = syscon_get_regmap(syscon);
> +       if (!r) {
> +               dev_dbg(dev, "unable to find regmap\n");
> +               return ERR_PTR(-ENODEV);
> +       }
> +
> +       return r;
> +}
> +
>  int syscon_get_by_driver_data(ulong driver_data, struct udevice **devp)
>  {
>         struct udevice *dev;
> diff --git a/include/syscon.h b/include/syscon.h
> index 2aa73e5..3df96e3 100644
> --- a/include/syscon.h
> +++ b/include/syscon.h
> @@ -74,6 +74,19 @@ int syscon_get_by_driver_data(ulong driver_data, struct udevice **devp);
>  struct regmap *syscon_get_regmap_by_driver_data(ulong driver_data);
>
>  /**
> + * syscon_regmap_lookup_by_phandle() - Look up a controller by a phandle
> + *
> + * This operates by looking up the given name in the device (device
> + * tree property) of the device using the system controller.
> + *
> + * @dev:       Device using the system controller
> + * @name:      Name of property referring to the system controller
> + * @return     A pointer to the regmap if found, ERR_PTR(-ve) on error
> + */
> +struct regmap *syscon_regmap_lookup_by_phandle(struct udevice *dev,
> +                                              const char *name);
> +
> +/**
>   * syscon_get_first_range() - get the first memory range from a syscon regmap
>   *
>   * @driver_data:       Driver data value to look up
> diff --git a/test/dm/syscon.c b/test/dm/syscon.c
> index 77c7928..b958bbe 100644
> --- a/test/dm/syscon.c
> +++ b/test/dm/syscon.c
> @@ -6,6 +6,7 @@
>  #include <common.h>
>  #include <dm.h>
>  #include <syscon.h>
> +#include <regmap.h>
>  #include <asm/test.h>
>  #include <dm/test.h>
>  #include <test/ut.h>
> @@ -43,3 +44,31 @@ static int dm_test_syscon_by_driver_data(struct unit_test_state *uts)
>         return 0;
>  }
>  DM_TEST(dm_test_syscon_by_driver_data, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> +
> +/* Test system controller by phandle */
> +static int dm_test_syscon_by_phandle(struct unit_test_state *uts)
> +{
> +       struct udevice *dev;
> +       struct regmap *map;
> +
> +       ut_assertok(uclass_get_device_by_name(UCLASS_TEST_PROBE, "test4",
> +                                             &dev));
> +       if (!dev || IS_ERR(dev))
> +               return -ENODEV;

If the above function returns 0, then dev is valid. Also an error is
never returned in 'dev'. So you should drop these two lines.

> +
> +       ut_assertok_ptr(syscon_regmap_lookup_by_phandle(dev, "first-syscon"));
> +       map = syscon_regmap_lookup_by_phandle(dev, "first-syscon");
> +       if (map && !IS_ERR(map))

Don't you want to ut_assert() that? And below?

> +               ut_asserteq(1, map->range_count);
> +
> +       ut_assertok_ptr(syscon_regmap_lookup_by_phandle(dev,
> +                                                       "second-sys-ctrl"));
> +       map = syscon_regmap_lookup_by_phandle(dev, "second-sys-controller");
> +       if (map && !IS_ERR(map))
> +               ut_asserteq(4, map->range_count);
> +
> +       ut_assert(IS_ERR(syscon_regmap_lookup_by_phandle(dev, "not-present")));
> +
> +       return 0;
> +}
> +DM_TEST(dm_test_syscon_by_phandle, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> --
> 2.7.4
>

Regards,
Simon