[U-Boot] [PATCH] clk: convert API to match reset/mailbox style
Simon Glass
sjg at chromium.org
Wed Jun 8 04:43:29 CEST 2016
Hi Stephen,
On 23 May 2016 at 10:47, Stephen Warren <swarren at wwwdotorg.org> wrote:
> From: Stephen Warren <swarren at 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 at chromium.org>
>
> Cc: Mateusz Kulikowski <mateusz.kulikowski at gmail.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
> Cc: Purna Chandra Mandal <purna.mandal at microchip.com>
> Cc: Masahiro Yamada <yamada.masahiro at socionext.com>
> Signed-off-by: Stephen Warren <swarren at 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
More information about the U-Boot
mailing list