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
