Hi Paweł,
On Sun, 25 Jul 2021 at 08:15, Paweł Jarosz paweljarosz3691@gmail.com wrote:
Hi Simon,
sorry for late response i was offline a bit
W dniu 13.07.2021 o 22:17, Simon Glass pisze:
Hi Paweł,
On Tue, 13 Jul 2021 at 12:59, Paweł Jarosz paweljarosz3691@gmail.com wrote:
Add clock driver for rk3066 platform.
Signed-off-by: Paweł Jarosz paweljarosz3691@gmail.com Acked-by: Philipp Tomsich philipp.tomsich@vrull.eu
Changes since v1:
- updated to shifted masks
- moved clk init to tpl
Changes since v2:
- none
Changes since v3:
- none
Changes since v4:
- updated to current codebase
- fixed compilation errors
Changes since v5:
- various style changes
- added clk_enable/clk_disable support for nand and mmc clocks
- updated maintainer email
- renamed uint32_t to u32
- used #if IS_ENABLED macro instead #ifdef
.../include/asm/arch-rockchip/cru_rk3066.h | 203 +++++ drivers/clk/rockchip/Makefile | 1 + drivers/clk/rockchip/clk_rk3066.c | 704 ++++++++++++++++++ 3 files changed, 908 insertions(+) create mode 100644 arch/arm/include/asm/arch-rockchip/cru_rk3066.h create mode 100644 drivers/clk/rockchip/clk_rk3066.c
[..]
+static int rk3066_clk_of_to_plat(struct udevice *dev) +{ +#if !CONFIG_IS_ENABLED(OF_PLATDATA)
struct rk3066_clk_priv *priv = dev_get_priv(dev);priv->cru = dev_read_addr_ptr(dev);+#endif
return 0;+}
+static int rk3066_clk_probe(struct udevice *dev) +{
struct rk3066_clk_priv *priv = dev_get_priv(dev);priv->grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);if (IS_ERR(priv->grf))return PTR_ERR(priv->grf);+#if IS_ENABLED(CONFIG_TPL_BUILD)
Do you need that? The line below should take care of it.
Yep. Later rkclk_init and rkclk_configure_cpu should be only executed in TPL.
But CONFIG_IS_ENABLED(OF_PLATDATA) should be enough to check that, right, since it is only true in TPL? You the TPL check seems to be redundant.
+#if CONFIG_IS_ENABLED(OF_PLATDATA)
struct rk3066_clk_plat *plat = dev_get_plat(dev);priv->cru = map_sysmem(plat->dtd.reg[0], plat->dtd.reg[1]);+#endif
rkclk_init(priv->cru, priv->grf);/* Init CPU frequency */rkclk_configure_cpu(priv->cru, priv->grf, APLL_SAFE_HZ);+#endif
return 0;+}
+static int rk3066_clk_bind(struct udevice *dev) +{
int ret;struct udevice *sys_child;struct sysreset_reg *priv;/* The reset driver does not have a device node, so bind it here */ret = device_bind_driver(dev, "rockchip_sysreset", "sysreset",&sys_child);if (ret) {debug("Warning: No sysreset driver: ret=%d\n", ret);} else {priv = malloc(sizeof(struct sysreset_reg));priv->glb_srst_fst_value = offsetof(struct rk3066_cru,cru_glb_srst_fst_value);priv->glb_srst_snd_value = offsetof(struct rk3066_cru,cru_glb_srst_snd_value);dev_set_priv(sys_child, priv);}+#if CONFIG_IS_ENABLED(RESET_ROCKCHIP)
Can you use if() instead of #if ?
Yes, but what is the difference? Sorry ... I don't know what to ask google to get the answer.
Is it in this case doing the same thing and just looking better?
We are avoiding the preprocessor these days in favour of compile-time checks, since it increases build coverage and makes the code easier to read (there are many other things here). Also patman should warn you about this?
Regards, Simon