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

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