Hi Stephen, On 7 June 2016 at 19:43, Simon Glass <s...@chromium.org> wrote: > 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 >
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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot