[U-Boot] [PATCH 1/4] dm: clk: Add a way to find a clock by its driver

Some SoCs have a single clock device. Provide a way to find it given its driver name. This is handled by the linker so will fail if the name is not found, avoiding strange errors when names change and do not match. It is also faster than a string comparison.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/uclass.c | 20 ++++++++++++++++++++ include/dm/device.h | 4 ++++ include/dm/uclass.h | 17 +++++++++++++++++ 3 files changed, 41 insertions(+)
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index 1141ce1..de602ae 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -311,6 +311,26 @@ static int uclass_find_device_by_phandle(enum uclass_id id, } #endif
+int uclass_get_device_by_driver(enum uclass_id id, + const struct driver *find_drv, + struct udevice **devp) +{ + struct udevice *dev; + struct uclass *uc; + int ret; + + ret = uclass_get(id, &uc); + if (ret) + return ret; + + list_for_each_entry(dev, &uc->dev_head, uclass_node) { + if (dev->driver == find_drv) + return uclass_get_device_tail(dev, 0, devp); + } + + return -ENODEV; +} + int uclass_get_device_tail(struct udevice *dev, int ret, struct udevice **devp) { diff --git a/include/dm/device.h b/include/dm/device.h index c825d47..705849b 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -207,6 +207,10 @@ struct driver { #define U_BOOT_DRIVER(__name) \ ll_entry_declare(struct driver, __name, driver)
+/* Get a pointer to a given driver */ +#define DM_GET_DRIVER(__name) \ + ll_entry_get(struct driver, __name, driver) + /** * dev_get_platdata() - Get the platform data for a device * diff --git a/include/dm/uclass.h b/include/dm/uclass.h index fd368b6..e49fbed 100644 --- a/include/dm/uclass.h +++ b/include/dm/uclass.h @@ -194,6 +194,23 @@ int uclass_get_device_by_phandle(enum uclass_id id, struct udevice *parent, const char *name, struct udevice **devp);
/** + * uclass_get_device_by_driver() - Get a uclass device for a driver + * + * This searches the devices in the uclass for one that uses the given + * driver. Use DM_GET_DRIVER(name) for the @drv argument, where 'name' is + * the driver name - as used in U_BOOT_DRIVER(name). + * + * The device is probed to activate it ready for use. + * + * @id: ID to look up + * @drv: Driver to look for + * @devp: Returns pointer to the first device with that driver + * @return 0 if OK, -ve on error + */ +int uclass_get_device_by_driver(enum uclass_id id, const struct driver *drv, + struct udevice **devp); + +/** * uclass_first_device() - Get the first device in a uclass * * The device returned is probed if necessary, and ready for use

On Rockchip SoCs we typically have a main clock device that uses the Soc clock driver. There is also a fixed clock for the oscillator. Add a function to obtain the core clock.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/arm/include/asm/arch-rockchip/clock.h | 2 ++ arch/arm/mach-rockchip/rk3288/Makefile | 1 + arch/arm/mach-rockchip/rk3288/clk_rk3288.c | 17 +++++++++++++++++ 3 files changed, 20 insertions(+) create mode 100644 arch/arm/mach-rockchip/rk3288/clk_rk3288.c
diff --git a/arch/arm/include/asm/arch-rockchip/clock.h b/arch/arm/include/asm/arch-rockchip/clock.h index 317e512..99cc18c 100644 --- a/arch/arm/include/asm/arch-rockchip/clock.h +++ b/arch/arm/include/asm/arch-rockchip/clock.h @@ -67,4 +67,6 @@ struct rk3288_grf;
void rkclk_configure_cpu(struct rk3288_cru *cru, struct rk3288_grf *grf);
+int rockchip_get_clk(struct udevice **devp); + #endif diff --git a/arch/arm/mach-rockchip/rk3288/Makefile b/arch/arm/mach-rockchip/rk3288/Makefile index 6f62375..82b00a1 100644 --- a/arch/arm/mach-rockchip/rk3288/Makefile +++ b/arch/arm/mach-rockchip/rk3288/Makefile @@ -4,6 +4,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
+obj-y += clk_rk3288.o obj-y += reset_rk3288.o obj-y += sdram_rk3288.o obj-y += syscon_rk3288.o diff --git a/arch/arm/mach-rockchip/rk3288/clk_rk3288.c b/arch/arm/mach-rockchip/rk3288/clk_rk3288.c new file mode 100644 index 0000000..2099e34 --- /dev/null +++ b/arch/arm/mach-rockchip/rk3288/clk_rk3288.c @@ -0,0 +1,17 @@ +/* + * Copyright (C) 2015 Google, Inc + * Written by Simon Glass sjg@chromium.org + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <syscon.h> +#include <asm/arch/clock.h> + +int rockchip_get_clk(struct udevice **devp) +{ + return uclass_get_device_by_driver(UCLASS_CLK, + DM_GET_DRIVER(rockchip_rk3288_cru), devp); +}

On 17 July 2016 at 15:23, Simon Glass sjg@chromium.org wrote:
On Rockchip SoCs we typically have a main clock device that uses the Soc clock driver. There is also a fixed clock for the oscillator. Add a function to obtain the core clock.
Signed-off-by: Simon Glass sjg@chromium.org
arch/arm/include/asm/arch-rockchip/clock.h | 2 ++ arch/arm/mach-rockchip/rk3288/Makefile | 1 + arch/arm/mach-rockchip/rk3288/clk_rk3288.c | 17 +++++++++++++++++ 3 files changed, 20 insertions(+) create mode 100644 arch/arm/mach-rockchip/rk3288/clk_rk3288.c
I'm bringing this in now as it fixes a clock bug and I am out most of this week. But if there are comments I can address them later.
Applied to u-boot-rockchip

The current code picks the first available clock. In U-Boot proper this is the oscillator device, not the SoC clock device. As a result the HDMI display does not work.
Fix this by calling rockchip_get_clk() instead.
Fixes: 135aa950 (clk: convert API to match reset/mailbox style) Signed-off-by: Simon Glass sjg@chromium.org ---
arch/arm/mach-rockchip/board.c | 2 +- arch/arm/mach-rockchip/rk3288-board-spl.c | 2 +- arch/arm/mach-rockchip/rk3288/sdram_rk3288.c | 2 +- drivers/clk/clk_rk3288.c | 2 +- drivers/video/rockchip/rk_vop.c | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c index f662b26..bec756d 100644 --- a/arch/arm/mach-rockchip/board.c +++ b/arch/arm/mach-rockchip/board.c @@ -178,7 +178,7 @@ static int do_clock(cmd_tbl_t *cmdtp, int flag, int argc, int ret, i; struct udevice *dev;
- ret = uclass_get_device(UCLASS_CLK, 0, &dev); + ret = rockchip_get_clk(&dev); if (ret) { printf("clk-uclass not found\n"); return 0; diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c b/arch/arm/mach-rockchip/rk3288-board-spl.c index d82115f..ed14023 100644 --- a/arch/arm/mach-rockchip/rk3288-board-spl.c +++ b/arch/arm/mach-rockchip/rk3288-board-spl.c @@ -187,7 +187,7 @@ void board_init_f(ulong dummy) rockchip_timer_init(); configure_l2ctlr();
- ret = uclass_get_device(UCLASS_CLK, 0, &dev); + ret = rockchip_get_clk(&dev); if (ret) { debug("CLK init failed: %d\n", ret); return; diff --git a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c index b36b6af..eea10cc 100644 --- a/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c +++ b/arch/arm/mach-rockchip/rk3288/sdram_rk3288.c @@ -923,7 +923,7 @@ static int rk3288_dmc_probe(struct udevice *dev) priv->chan[1].pctl = regmap_get_range(plat->map, 2); priv->chan[1].publ = regmap_get_range(plat->map, 3); #endif - ret = uclass_get_device(UCLASS_CLK, 0, &dev_clk); + ret = rockchip_get_clk(&dev_clk); if (ret) return ret; priv->ddr_clk.id = CLK_DDR; diff --git a/drivers/clk/clk_rk3288.c b/drivers/clk/clk_rk3288.c index 679f010..e8463f9 100644 --- a/drivers/clk/clk_rk3288.c +++ b/drivers/clk/clk_rk3288.c @@ -145,7 +145,7 @@ void *rockchip_get_cru(void) struct udevice *dev; int ret;
- ret = uclass_get_device(UCLASS_CLK, 0, &dev); + ret = rockchip_get_clk(&dev); if (ret) return ERR_PTR(ret);
diff --git a/drivers/video/rockchip/rk_vop.c b/drivers/video/rockchip/rk_vop.c index cc26f19..c6d88d9 100644 --- a/drivers/video/rockchip/rk_vop.c +++ b/drivers/video/rockchip/rk_vop.c @@ -238,7 +238,7 @@ int rk_display_init(struct udevice *dev, ulong fbbase, return ret; }
- ret = uclass_get_device(UCLASS_CLK, 0, &dev_clk); + ret = rockchip_get_clk(&dev_clk); if (!ret) { clk.id = DCLK_VOP0 + remote_vop_id; ret = clk_request(dev_clk, &clk);

On Sun, 17 Jul 2016 15:23:17 -0600 Simon Glass sjg@chromium.org wrote:
The current code picks the first available clock. In U-Boot proper this is the oscillator device, not the SoC clock device. As a result the HDMI display does not work.
Fix this by calling rockchip_get_clk() instead.
Fixes: 135aa950 (clk: convert API to match reset/mailbox style) Signed-off-by: Simon Glass sjg@chromium.org
Acked-by: Anatolij Gustschin agust@denx.de

On 17 July 2016 at 16:17, Anatolij Gustschin agust@denx.de wrote:
On Sun, 17 Jul 2016 15:23:17 -0600 Simon Glass sjg@chromium.org wrote:
The current code picks the first available clock. In U-Boot proper this is the oscillator device, not the SoC clock device. As a result the HDMI display does not work.
Fix this by calling rockchip_get_clk() instead.
Fixes: 135aa950 (clk: convert API to match reset/mailbox style) Signed-off-by: Simon Glass sjg@chromium.org
Acked-by: Anatolij Gustschin agust@denx.de
Applied to u-boot-rockchip

This function is called from outside the driver. It should be placed into common SoC code. Move it.
The same could be done for rk3036 but is not attempted here.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/arm/mach-rockchip/rk3288/clk_rk3288.c | 15 +++++++++++++++ drivers/clk/clk_rk3288.c | 15 --------------- 2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/arm/mach-rockchip/rk3288/clk_rk3288.c b/arch/arm/mach-rockchip/rk3288/clk_rk3288.c index 2099e34..b665009 100644 --- a/arch/arm/mach-rockchip/rk3288/clk_rk3288.c +++ b/arch/arm/mach-rockchip/rk3288/clk_rk3288.c @@ -15,3 +15,18 @@ int rockchip_get_clk(struct udevice **devp) return uclass_get_device_by_driver(UCLASS_CLK, DM_GET_DRIVER(rockchip_rk3288_cru), devp); } + +void *rockchip_get_cru(void) +{ + struct rk3288_clk_priv *priv; + struct udevice *dev; + int ret; + + ret = rockchip_get_clk(&dev); + if (ret) + return ERR_PTR(ret); + + priv = dev_get_priv(dev); + + return priv->cru; +} diff --git a/drivers/clk/clk_rk3288.c b/drivers/clk/clk_rk3288.c index e8463f9..b1238fa 100644 --- a/drivers/clk/clk_rk3288.c +++ b/drivers/clk/clk_rk3288.c @@ -139,21 +139,6 @@ static const struct pll_div apll_init_cfg = PLL_DIVISORS(APLL_HZ, 1, 1); static const struct pll_div gpll_init_cfg = PLL_DIVISORS(GPLL_HZ, 2, 2); static const struct pll_div cpll_init_cfg = PLL_DIVISORS(CPLL_HZ, 1, 2);
-void *rockchip_get_cru(void) -{ - struct rk3288_clk_priv *priv; - struct udevice *dev; - int ret; - - ret = rockchip_get_clk(&dev); - if (ret) - return ERR_PTR(ret); - - priv = dev_get_priv(dev); - - return priv->cru; -} - static int rkclk_set_pll(struct rk3288_cru *cru, enum rk_clk_id clk_id, const struct pll_div *div) {

On 17 July 2016 at 15:23, Simon Glass sjg@chromium.org wrote:
Some SoCs have a single clock device. Provide a way to find it given its driver name. This is handled by the linker so will fail if the name is not found, avoiding strange errors when names change and do not match. It is also faster than a string comparison.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/core/uclass.c | 20 ++++++++++++++++++++ include/dm/device.h | 4 ++++ include/dm/uclass.h | 17 +++++++++++++++++ 3 files changed, 41 insertions(+)
Applied to u-boot-rockchip.

On 07/17/2016 03:23 PM, Simon Glass wrote:
Some SoCs have a single clock device. Provide a way to find it given its driver name. This is handled by the linker so will fail if the name is not found, avoiding strange errors when names change and do not match. It is also faster than a string comparison.
The code looks plausible, but the commit subject and description imply this has something to do with clocks, whereas it doesn't; it seems to be a generic/core DM function.

Hi Stephen,
On 18 July 2016 at 10:23, Stephen Warren swarren@wwwdotorg.org wrote:
On 07/17/2016 03:23 PM, Simon Glass wrote:
Some SoCs have a single clock device. Provide a way to find it given its driver name. This is handled by the linker so will fail if the name is not found, avoiding strange errors when names change and do not match. It is also faster than a string comparison.
The code looks plausible, but the commit subject and description imply this has something to do with clocks, whereas it doesn't; it seems to be a generic/core DM function.
Yes you are correct. I updated the commit subject when I applied it:
c57f806 dm: core: Add a way to find a device by its driver
Regards, Simon
participants (3)
-
Anatolij Gustschin
-
Simon Glass
-
Stephen Warren