[PATCH v6 2/4] rockchip: rk3066: add clock driver for rk3066 soc
Simon Glass
sjg at chromium.org
Mon Jul 26 16:06:26 CEST 2021
Hi Paweł,
On Sun, 25 Jul 2021 at 08:15, Paweł Jarosz <paweljarosz3691 at 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 at gmail.com> wrote:
> >> Add clock driver for rk3066 platform.
> >>
> >> Signed-off-by: Paweł Jarosz <paweljarosz3691 at gmail.com>
> >> Acked-by: Philipp Tomsich <philipp.tomsich at 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
More information about the U-Boot
mailing list