Hi Stephen, On 23 May 2016 at 10:47, Stephen Warren <swar...@wwwdotorg.org> wrote: > From: Stephen Warren <swar...@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 <s...@chromium.org> > > Cc: Mateusz Kulikowski <mateusz.kulikow...@gmail.com> > Cc: Daniel Schwierzeck <daniel.schwierz...@gmail.com> > Cc: Purna Chandra Mandal <purna.man...@microchip.com> > Cc: Masahiro Yamada <yamada.masah...@socionext.com> > Signed-off-by: Stephen Warren <swar...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot