[v4,05/17] dm: Add support for simple-pm-bus

This type of bus is used in Linux to designate busses which have power
domains and/or clocks which need to be enabled before their child devices
can be used.  Because power domains are automatically enabled before
probing in u-boot, we just need to enable any clocks present.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

Changes in v4:
- Split the bus off into its own driver
- Add test
- Fix line spacing in Kconfig
- Lint

Changes in v3:
- New

 arch/sandbox/dts/test.dts                     |  6 ++
 arch/sandbox/include/asm/clk.h                |  1 +
 configs/sandbox_defconfig                     |  1 +
 .../bus/simple-pm-bus.txt                     | 44 +++++++++++++++
 drivers/core/Kconfig                          |  7 +++
 drivers/core/Makefile                         |  1 +
 drivers/core/simple-pm-bus.c                  | 55 +++++++++++++++++++
 test/dm/Makefile                              |  1 +
 test/dm/simple-pm-bus.c                       | 44 +++++++++++++++
 9 files changed, 160 insertions(+)
 create mode 100644 doc/device-tree-bindings/bus/simple-pm-bus.txt
 create mode 100644 drivers/core/simple-pm-bus.c
 create mode 100644 test/dm/simple-pm-bus.c

Hi Sean,

On Mon, 10 Feb 2020 at 23:05, Sean Anderson <seanga2@gmail.com> wrote:
>
> This type of bus is used in Linux to designate busses which have power

buses

> domains and/or clocks which need to be enabled before their child devices
> can be used.  Because power domains are automatically enabled before
> probing in u-boot, we just need to enable any clocks present.

U-Boot

>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---

With the above and below fixed:

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

>
> Changes in v4:
> - Split the bus off into its own driver
> - Add test
> - Fix line spacing in Kconfig
> - Lint
>
> Changes in v3:
> - New
>
>  arch/sandbox/dts/test.dts                     |  6 ++
>  arch/sandbox/include/asm/clk.h                |  1 +
>  configs/sandbox_defconfig                     |  1 +
>  .../bus/simple-pm-bus.txt                     | 44 +++++++++++++++
>  drivers/core/Kconfig                          |  7 +++
>  drivers/core/Makefile                         |  1 +
>  drivers/core/simple-pm-bus.c                  | 55 +++++++++++++++++++
>  test/dm/Makefile                              |  1 +
>  test/dm/simple-pm-bus.c                       | 44 +++++++++++++++
>  9 files changed, 160 insertions(+)
>  create mode 100644 doc/device-tree-bindings/bus/simple-pm-bus.txt
>  create mode 100644 drivers/core/simple-pm-bus.c
>  create mode 100644 test/dm/simple-pm-bus.c
>
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index 23f2aefd43..648f21239a 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -929,6 +929,12 @@
>                 compatible = "sandbox,mdio";
>         };
>
> +       pm-bus-test {
> +               compatible = "simple-pm-bus";
> +               clocks = <&clk_sandbox 4>;
> +               power-domains = <&pwrdom 1>;
> +       };
> +
>         resetc2: syscon-reset {
>                 compatible = "syscon-reset";
>                 #reset-cells = <1>;
> diff --git a/arch/sandbox/include/asm/clk.h b/arch/sandbox/include/asm/clk.h
> index 1573e4a134..c184c4bffc 100644
> --- a/arch/sandbox/include/asm/clk.h
> +++ b/arch/sandbox/include/asm/clk.h
> @@ -21,6 +21,7 @@ enum sandbox_clk_id {
>         SANDBOX_CLK_ID_I2C,
>         SANDBOX_CLK_ID_UART1,
>         SANDBOX_CLK_ID_UART2,
> +       SANDBOX_CLK_ID_BUS,
>
>         SANDBOX_CLK_ID_COUNT,
>  };
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 19970f1db5..c637b39b96 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -89,6 +89,7 @@ CONFIG_REGMAP=y
>  CONFIG_SYSCON=y
>  CONFIG_DEVRES=y
>  CONFIG_DEBUG_DEVRES=y
> +CONFIG_SIMPLE_PM_BUS=y
>  CONFIG_ADC=y
>  CONFIG_ADC_SANDBOX=y
>  CONFIG_AXI=y
> diff --git a/doc/device-tree-bindings/bus/simple-pm-bus.txt b/doc/device-tree-bindings/bus/simple-pm-bus.txt
> new file mode 100644
> index 0000000000..6f15037131
> --- /dev/null
> +++ b/doc/device-tree-bindings/bus/simple-pm-bus.txt
> @@ -0,0 +1,44 @@
> +Simple Power-Managed Bus
> +========================
> +
> +A Simple Power-Managed Bus is a transparent bus that doesn't need a real
> +driver, as it's typically initialized by the boot loader.
> +
> +However, its bus controller is part of a PM domain, or under the control of a
> +functional clock.  Hence, the bus controller's PM domain and/or clock must be
> +enabled for child devices connected to the bus (either on-SoC or externally)
> +to function.
> +
> +While "simple-pm-bus" follows the "simple-bus" set of properties, as specified
> +in the Devicetree Specification, it is not an extension of "simple-bus".
> +
> +
> +Required properties:
> +  - compatible: Must contain at least "simple-pm-bus".
> +               Must not contain "simple-bus".
> +               It's recommended to let this be preceded by one or more
> +               vendor-specific compatible values.
> +  - #address-cells, #size-cells, ranges: Must describe the mapping between
> +               parent address and child address spaces.
> +
> +Optional platform-specific properties for clock or PM domain control (at least
> +one of them is required):
> +  - clocks: Must contain a reference to the functional clock(s),
> +  - power-domains: Must contain a reference to the PM domain.
> +Please refer to the binding documentation for the clock and/or PM domain
> +providers for more details.
> +
> +
> +Example:
> +
> +       bsc: bus@fec10000 {
> +               compatible = "renesas,bsc-sh73a0", "renesas,bsc",
> +                            "simple-pm-bus";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               ranges = <0 0 0x20000000>;
> +               reg = <0xfec10000 0x400>;
> +               interrupts = <0 39 IRQ_TYPE_LEVEL_HIGH>;
> +               clocks = <&zb_clk>;
> +               power-domains = <&pd_a4s>;
> +       };
> diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
> index 3b95b5387b..0cd687526e 100644
> --- a/drivers/core/Kconfig
> +++ b/drivers/core/Kconfig
> @@ -195,6 +195,13 @@ config SPL_SIMPLE_BUS
>           Supports the 'simple-bus' driver, which is used on some systems
>           in SPL.
>
> +config SIMPLE_PM_BUS
> +       bool "Support simple-pm-bus driver"
> +       depends on DM && OF_CONTROL && CLK && POWER_DOMAIN
> +       help
> +         Supports the 'simple-pm-bus' driver, which is used for busses that
> +         have power domains and/or clocks which need to be enabled before use.
> +
>  config OF_TRANSLATE
>         bool "Translate addresses using fdt_translate_address"
>         depends on DM && OF_CONTROL
> diff --git a/drivers/core/Makefile b/drivers/core/Makefile
> index bce7467da1..284af7caaa 100644
> --- a/drivers/core/Makefile
> +++ b/drivers/core/Makefile
> @@ -6,6 +6,7 @@ obj-y   += device.o fdtaddr.o lists.o root.o uclass.o util.o
>  obj-$(CONFIG_DEVRES) += devres.o
>  obj-$(CONFIG_$(SPL_)DM_DEVICE_REMOVE)  += device-remove.o
>  obj-$(CONFIG_$(SPL_)SIMPLE_BUS)        += simple-bus.o
> +obj-$(CONFIG_SIMPLE_PM_BUS)    += simple-pm-bus.o
>  obj-$(CONFIG_DM)       += dump.o
>  obj-$(CONFIG_$(SPL_TPL_)REGMAP)        += regmap.o
>  obj-$(CONFIG_$(SPL_TPL_)SYSCON)        += syscon-uclass.o
> diff --git a/drivers/core/simple-pm-bus.c b/drivers/core/simple-pm-bus.c
> new file mode 100644
> index 0000000000..3ee54531d1
> --- /dev/null
> +++ b/drivers/core/simple-pm-bus.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com>
> + */
> +

common.h

> +#include <dm.h>
> +#include <clk.h>

clk.h above dm.h

> +
> +/*
> + * Power domains are taken care of by driver_probe, so we just have to enable
> + * clocks
> + */
> +static const int simple_pm_bus_probe(struct udevice *dev)
> +{
> +       int ret;
> +       struct clk_bulk *bulk = dev_get_priv(dev);
> +
> +       ret = clk_get_bulk(dev, bulk);
> +       if (ret)
> +               return ret;
> +
> +       ret = clk_enable_bulk(bulk);
> +       if (ret && ret != -ENOSYS && ret != -ENOTSUPP) {
> +               clk_release_bulk(bulk);
> +               return ret;
> +       }
> +       return 0;
> +}
> +
> +static const int simple_pm_bus_remove(struct udevice *dev)
> +{
> +       int ret;
> +       struct clk_bulk *bulk = dev_get_priv(dev);
> +
> +       ret = clk_release_bulk(bulk);
> +       if (ret && ret != -ENOSYS && ret != -ENOTSUPP)
> +               return ret;
> +       else
> +               return 0;
> +}
> +
> +static const struct udevice_id simple_pm_bus_ids[] = {
> +       { .compatible = "simple-pm-bus" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(simple_pm_bus_drv) = {
> +       .name   = "simple_pm_bus",
> +       .id     = UCLASS_SIMPLE_BUS,
> +       .of_match = simple_pm_bus_ids,
> +       .probe = simple_pm_bus_probe,
> +       .remove = simple_pm_bus_remove,
> +       .priv_auto_alloc_size = sizeof(struct clk_bulk),
> +       .flags  = DM_FLAG_PRE_RELOC,
> +};
> diff --git a/test/dm/Makefile b/test/dm/Makefile
> index 1f0e0febb4..a760ab8b14 100644
> --- a/test/dm/Makefile
> +++ b/test/dm/Makefile
> @@ -71,4 +71,5 @@ obj-$(CONFIG_DM_MDIO) += mdio.o
>  obj-$(CONFIG_DM_MDIO_MUX) += mdio_mux.o
>  obj-$(CONFIG_DM_RNG) += rng.o
>  obj-$(CONFIG_RESET_SYSCON) += syscon-reset.o
> +obj-$(CONFIG_SIMPLE_PM_BUS) += simple-pm-bus.o
>  endif
> diff --git a/test/dm/simple-pm-bus.c b/test/dm/simple-pm-bus.c
> new file mode 100644
> index 0000000000..1b42415ccd
> --- /dev/null
> +++ b/test/dm/simple-pm-bus.c
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com>
> + */
> +

Please fix include ordering

https://www.denx.de/wiki/U-Boot/CodingStyle

> +#include <asm/clk.h>
> +#include <asm/power-domain.h>
> +#include <dm.h>
> +#include <dm/test.h>
> +#include <dm/device-internal.h>
> +#include <test/ut.h>
> +
> +/* These must match the ids in the device tree */
> +#define TEST_CLOCK_ID 4
> +#define TEST_POWER_ID 1
> +
> +static int dm_test_simple_pm_bus(struct unit_test_state *uts)
> +{
> +       struct udevice *power;
> +       struct udevice *clock;
> +       struct udevice *bus;
> +
> +       ut_assertok(uclass_get_device_by_name(UCLASS_POWER_DOMAIN,
> +                                             "power-domain", &power));
> +       ut_assertok(uclass_get_device_by_name(UCLASS_CLK, "clk-sbox",
> +                                             &clock));
> +       ut_asserteq(0, sandbox_power_domain_query(power, TEST_POWER_ID));
> +       ut_asserteq(0, sandbox_clk_query_enable(clock, TEST_CLOCK_ID));
> +
> +       ut_assertok(uclass_get_device_by_name(UCLASS_SIMPLE_BUS, "pm-bus-test",
> +                                             &bus));
> +       ut_asserteq(1, sandbox_power_domain_query(power, TEST_POWER_ID));
> +       ut_asserteq(1, sandbox_clk_query_enable(clock, TEST_CLOCK_ID));
> +
> +       ut_assertok(device_remove(bus, DM_REMOVE_NORMAL));
> +       /* must re-probe since device_remove also removes the power domain */
> +       ut_assertok(uclass_get_device_by_name(UCLASS_POWER_DOMAIN,
> +                                             "power-domain", &power));
> +       ut_asserteq(0, sandbox_power_domain_query(power, TEST_POWER_ID));
> +       ut_asserteq(0, sandbox_clk_query_enable(clock, TEST_CLOCK_ID));
> +
> +       return 0;
> +}
> +DM_TEST(dm_test_simple_pm_bus, DM_TESTF_SCAN_FDT);
> --
> 2.25.0
>

Regards,
Simon

Hi Simon,

On 2/11/20 12:14 PM, Simon Glass wrote:
> Hi Sean,
> 
> On Mon, 10 Feb 2020 at 23:05, Sean Anderson <seanga2@gmail.com> wrote:
>>
>> This type of bus is used in Linux to designate busses which have power
> 
> buses
> 
>> domains and/or clocks which need to be enabled before their child devices
>> can be used.  Because power domains are automatically enabled before
>> probing in u-boot, we just need to enable any clocks present.
> 
> U-Boot
> 
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
> 
> With the above and below fixed:
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 

Will do, thanks.

>> diff --git a/drivers/core/simple-pm-bus.c b/drivers/core/simple-pm-bus.c
>> new file mode 100644
>> index 0000000000..3ee54531d1
>> --- /dev/null
>> +++ b/drivers/core/simple-pm-bus.c
>> @@ -0,0 +1,55 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com>
>> + */
>> +
> 
> common.h
> 
>> +#include <dm.h>
>> +#include <clk.h>
> 
> clk.h above dm.h
> 
>> diff --git a/test/dm/simple-pm-bus.c b/test/dm/simple-pm-bus.c
>> new file mode 100644
>> index 0000000000..1b42415ccd
>> --- /dev/null
>> +++ b/test/dm/simple-pm-bus.c
>> @@ -0,0 +1,44 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com>
>> + */
>> +
> 
> Please fix include ordering
> 
> https://www.denx.de/wiki/U-Boot/CodingStyle

Ah, I wasn't aware that there was a specific ordering; I had been doing
it alphabetically.

--Sean