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
Regards, Simon