On 25 May 2018 at 07:51, Jean-Jacques Hiblot <jjhib...@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 <jjhib...@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 <s...@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@0 {
> + syscon0: syscon@0 {
> compatible = "sandbox,syscon0";
> reg = <0x10 4>;
> };
>
> - syscon@1 {
> + another_system_controller: syscon@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
