Hi Stephen,
On 7 June 2016 at 19:43, Simon Glass sjg@chromium.org wrote:
Hi Stephen,
On 23 May 2016 at 10:47, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
The following changes are made to the clock API:
- The concept of "clocks" and "peripheral clocks" are unified; each clock provider now implements a single set of clocks. This provides a simpler conceptual interface to clients, and better aligns with device tree clock bindings.
- Clocks are now identified with a single "struct clk", rather than requiring clients to store the clock provider device and clock identity values separately. For simple clock consumers, this isolates clients from internal details of the clock API.
- clk.h is split into clk_client.h and clk_uclass.h to make it obvious which parts are relevant to consumers and providers. This aligns with the recently added reset and mailbox APIs.
- clk_ops .of_xlate(), .request(), and .free() are added so providers can customize these operations if needed. This also aligns with the recently added reset and mailbox APIs.
- clk_disable() is added.
- All users of the current clock APIs are updated.
- Sandbox clock tests are updated to exercise clock lookup via DT, and clock enable/disable.
- rkclk_get_clk() is removed and replaced with standard APIs.
Buildman shows no clock-related errors for any board for which buildman can download a toolchain.
test/py passes for sandbox (which invokes the dm clk test amongst others).
Sorry for the delay on this. It took more thinking and testing than I had time for before I went away.
It looks really clean to me. Just some minor things I'd like to change.
Acked-by: Simon Glass sjg@chromium.org
Cc: Mateusz Kulikowski mateusz.kulikowski@gmail.com Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Purna Chandra Mandal purna.mandal@microchip.com Cc: Masahiro Yamada yamada.masahiro@socionext.com Signed-off-by: Stephen Warren swarren@nvidia.com
arch/arm/include/asm/arch-rockchip/clock.h | 12 -- arch/arm/mach-rockchip/board.c | 41 +++++- arch/arm/mach-rockchip/rk3288/sdram_rk3288.c | 17 ++- arch/arm/mach-snapdragon/clock-apq8016.c | 10 +- arch/arm/mach-zynq/clk.c | 1 - arch/mips/mach-pic32/cpu.c | 47 +++--- arch/sandbox/dts/test.dts | 17 ++- arch/sandbox/include/asm/clk.h | 39 +++++ arch/sandbox/include/asm/test.h | 9 -- board/microchip/pic32mzda/pic32mzda.c | 23 ++- cmd/clk.c | 2 +- drivers/clk/Makefile | 1 + drivers/clk/clk-uclass.c | 204 +++++++++++++++++++-------- drivers/clk/clk_fixed_rate.c | 13 +- drivers/clk/clk_pic32.c | 32 ++--- drivers/clk/clk_rk3036.c | 83 +++-------- drivers/clk/clk_rk3288.c | 141 ++++-------------- drivers/clk/clk_sandbox.c | 85 ++++++----- drivers/clk/clk_sandbox_test.c | 101 +++++++++++++ drivers/clk/uniphier/clk-uniphier-core.c | 26 ++-- drivers/clk/uniphier/clk-uniphier-mio.c | 1 - drivers/gpio/rk_gpio.c | 1 - drivers/i2c/rk_i2c.c | 8 +- drivers/mmc/msm_sdhci.c | 15 +- drivers/mmc/rockchip_dw_mmc.c | 8 +- drivers/mmc/uniphier-sd.c | 17 +-- drivers/serial/serial_msm.c | 15 +- drivers/serial/serial_pic32.c | 9 +- drivers/spi/rk_spi.c | 8 +- drivers/usb/host/ehci-generic.c | 16 +-- drivers/video/rockchip/rk_edp.c | 13 +- drivers/video/rockchip/rk_hdmi.c | 14 +- drivers/video/rockchip/rk_lvds.c | 1 - drivers/video/rockchip/rk_vop.c | 13 +- include/clk.h | 132 ----------------- include/clk_client.h | 174 +++++++++++++++++++++++ include/clk_uclass.h | 95 +++++++++++++
Can we use clk.h and clk-uclass.h instead? I'm not keen on the _client suffix. Client should be the defauilt.
test/dm/clk.c | 110 ++++++++++----- 38 files changed, 948 insertions(+), 606 deletions(-) create mode 100644 arch/sandbox/include/asm/clk.h create mode 100644 drivers/clk/clk_sandbox_test.c delete mode 100644 include/clk.h create mode 100644 include/clk_client.h create mode 100644 include/clk_uclass.h
diff --git a/arch/arm/include/asm/arch-rockchip/clock.h b/arch/arm/include/asm/arch-rockchip/clock.h index d66b26f18ef3..317e5128ed2b 100644 --- a/arch/arm/include/asm/arch-rockchip/clock.h +++ b/arch/arm/include/asm/arch-rockchip/clock.h @@ -62,18 +62,6 @@ static inline u32 clk_get_divisor(ulong input_rate, uint output_rate) */ void *rockchip_get_cru(void);
-/**
- rkclk_get_clk() - get a pointer to a given clock
- This is an internal function - use outside the clock subsystem indicates
- that work is needed!
- @clk_id: Clock requested
- @devp: Returns a pointer to that clock
- @return 0 if OK, -ve on error
- */
-int rkclk_get_clk(enum rk_clk_id clk_id, struct udevice **devp);
struct rk3288_cru; struct rk3288_grf;
[snip]
diff --git a/arch/sandbox/include/asm/clk.h b/arch/sandbox/include/asm/clk.h new file mode 100644 index 000000000000..7fd2868f9080 --- /dev/null +++ b/arch/sandbox/include/asm/clk.h @@ -0,0 +1,39 @@ +/*
- Copyright (c) 2016, NVIDIA CORPORATION.
- SPDX-License-Identifier: GPL-2.0
- */
+#ifndef __SANDBOX_CLK_H +#define __SANDBOX_CLK_H
+#include <common.h>
+struct udevice;
+enum {
SANDBOX_CLK_ID_SPI,SANDBOX_CLK_ID_I2C,SANDBOX_CLK_ID_COUNT,+};
+enum {
SANDBOX_CLK_TEST_ID_FIXED,SANDBOX_CLK_TEST_ID_SPI,SANDBOX_CLK_TEST_ID_I2C,SANDBOX_CLK_TEST_ID_COUNT,What's the difference between these two enums? Can you add a comment to each?
+};
+ulong sandbox_clk_query_rate(struct udevice *dev, int id); +int sandbox_clk_query_enable(struct udevice *dev, int id);
Can you add function comments for these?
+int sandbox_clk_test_get(struct udevice *dev); +ulong sandbox_clk_test_get_rate(struct udevice *dev, int id); +ulong sandbox_clk_test_set_rate(struct udevice *dev, int id, ulong rate); +int sandbox_clk_test_enable(struct udevice *dev, int id); +int sandbox_clk_test_disable(struct udevice *dev, int id); +int sandbox_clk_test_free(struct udevice *dev);
and these?
+#endif
Also can you please take a look at these build errors?
08: sandbox: gpio: doc: Fix parameter documentation aarch64: + uniphier_ld11 espresso7420 mips: + tplink_wdr4300 blackfin: + cm-bf527 arm: + uniphier_ld4_sld8 s5pc210_universal uniphier_sld3 smdkc100 odroid peach-pit smdkv310 peach-pi smdk5250 smdk5420 origen odroid-xu3 snow s5p_goni spring arndale trats trats2 -arm-unknown-linux-gnueabi-ld.bfd: region `.sram' overflowed by 44 bytes + ret = clk_get_by_index(dev, i, &clk); + ^ +In file included from ../drivers/usb/host/ehci-generic.c:8:0: +../include/clk_client.h:78:5: note: expected 'struct clk *' but argument is of type 'struct clk **' + int clk_get_by_index(struct udevice *dev, int index, struct clk *clk); + ^ + if (clk_enable(&clk)) + ^ +../include/clk_client.h:161:5: note: expected 'struct clk *' but argument is of type 'struct clk **' + int clk_enable(struct clk *clk); + clk_free(&clk); + ^ +../include/clk_client.h:133:5: note: expected 'struct clk *' but argument is of type 'struct clk **' + int clk_free(struct clk *clk); +/usr/local/google/c/big/buildall.c/toolchains/bfin-uclinux/bin/../bfin-uclinux/bin/ld.real: region ram is full (u-boot section .bss) +make[1]: *** [u-boot] Error 1 +../drivers/serial/serial_s5p.c:20:17: fatal error: clk.h: No such file or directory + #include <clk.h> + ^ +compilation terminated. +make[2]: *** [drivers/serial/serial_s5p.o] Error 1 +make[1]: *** [drivers/serial] Error 2 +../drivers/clk/exynos/clk-exynos7420.c:12:17: fatal error: clk.h: No such file or directory +make[4]: *** [drivers/clk/exynos/clk-exynos7420.o] Error 1 +make[3]: *** [drivers/clk/exynos] Error 2 +make[2]: *** [drivers/clk] Error 2 +make[1]: *** [drivers] Error 2 +arm-unknown-linux-gnueabi-ld.bfd: region `.sram' overflowed by 60 bytes w+../drivers/usb/host/ehci-generic.c: In function 'ehci_usb_probe': w+../drivers/usb/host/ehci-generic.c:32:9: warning: passing argument 3 of 'clk_get_by_index' from incompatible pointer type w+../drivers/usb/host/ehci-generic.c:35:7: warning: passing argument 1 of 'clk_enable' from incompatible pointer type w+../drivers/usb/host/ehci-generic.c:37:3: warning: passing argument 1 of 'clk_free' from incompatible pointer type
(also firefly-rk3288)
Regards, Simon