[PATCH v4 1/2] clk: rockchip: rk3288: Handle clock tree for rk3288w

Ezequiel Garcia ezequiel at collabora.com
Fri Jul 3 11:02:52 EDT 2020


On Fri, 2020-07-03 at 16:11 +0200, Heiko Stuebner wrote:
> Hi Jagan,
> 
> Am Montag, 29. Juni 2020, 21:11:03 CEST schrieb Jagan Teki:
> > On Tue, Jun 2, 2020 at 1:37 PM Mylène Josserand
> > <mylene.josserand at collabora.com> wrote:
> > > The revision rk3288w has a different clock tree about "hclk_vio"
> > > clock, according to the BSP kernel code.
> > > 
> > > This patch handles this difference by detecting which device-tree
> > > we are using. If it is a "rockchip,rk3288-cru", let's register
> > > the clock tree as it was before. If the device-tree node is
> > > "rockchip,rk3288w-cru", we will apply the difference with this
> > > version of this SoC.
> > > 
> > > Noticed that this new device-tree compatible must be handled in
> > > bootloader such as u-boot.
> > > 
> > > Signed-off-by: Mylène Josserand <mylene.josserand at collabora.com>
> > > ---
> > >  drivers/clk/rockchip/clk-rk3288.c | 20 ++++++++++++++++++--
> > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
> > > index cc2a177bbdbf..204976e2d0cb 100644
> > > --- a/drivers/clk/rockchip/clk-rk3288.c
> > > +++ b/drivers/clk/rockchip/clk-rk3288.c
> > > @@ -425,8 +425,6 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
> > >         COMPOSITE(0, "aclk_vio0", mux_pll_src_cpll_gpll_usb480m_p, CLK_IGNORE_UNUSED,
> > >                         RK3288_CLKSEL_CON(31), 6, 2, MFLAGS, 0, 5, DFLAGS,
> > >                         RK3288_CLKGATE_CON(3), 0, GFLAGS),
> > > -       DIV(0, "hclk_vio", "aclk_vio0", 0,
> > > -                       RK3288_CLKSEL_CON(28), 8, 5, DFLAGS),
> > >         COMPOSITE(0, "aclk_vio1", mux_pll_src_cpll_gpll_usb480m_p, CLK_IGNORE_UNUSED,
> > >                         RK3288_CLKSEL_CON(31), 14, 2, MFLAGS, 8, 5, DFLAGS,
> > >                         RK3288_CLKGATE_CON(3), 2, GFLAGS),
> > > @@ -819,6 +817,16 @@ static struct rockchip_clk_branch rk3288_clk_branches[] __initdata = {
> > >         INVERTER(0, "pclk_isp", "pclk_isp_in", RK3288_CLKSEL_CON(29), 3, IFLAGS),
> > >  };
> > > 
> > > +static struct rockchip_clk_branch rk3288w_hclkvio_branch[] __initdata = {
> > > +       DIV(0, "hclk_vio", "aclk_vio1", 0,
> > > +                       RK3288_CLKSEL_CON(28), 8, 5, DFLAGS),
> > > +};
> > > +
> > > +static struct rockchip_clk_branch rk3288_hclkvio_branch[] __initdata = {
> > > +       DIV(0, "hclk_vio", "aclk_vio0", 0,
> > > +                       RK3288_CLKSEL_CON(28), 8, 5, DFLAGS),
> > > +};
> > > +
> > >  static const char *const rk3288_critical_clocks[] __initconst = {
> > >         "aclk_cpu",
> > >         "aclk_peri",
> > > @@ -936,6 +944,14 @@ static void __init rk3288_clk_init(struct device_node *np)
> > >                                    RK3288_GRF_SOC_STATUS1);
> > >         rockchip_clk_register_branches(ctx, rk3288_clk_branches,
> > >                                   ARRAY_SIZE(rk3288_clk_branches));
> > > +
> > > +       if (of_device_is_compatible(np, "rockchip,rk3288w-cru"))
> > > +               rockchip_clk_register_branches(ctx, rk3288w_hclkvio_branch,
> > > +                                              ARRAY_SIZE(rk3288w_hclkvio_branch));
> > > +       else
> > > +               rockchip_clk_register_branches(ctx, rk3288_hclkvio_branch,
> > > +                                              ARRAY_SIZE(rk3288_hclkvio_branch));
> > > +
> > 
> > Sorry for the late query on this. I am a bit unclear about this
> > compatible change, does Linux expect to replace rockchip,rk3288-cru
> > with rockchip,rk3288w-cru in bootloader if the chip is RK3288w? or
> > append the existing cru compatible node with rockchip,rk3288w-cru?
> > because replace new cru node make clock never probe since the
> > CLK_OF_DECLARE checking rockchip,rk3288-cru
> 
> I guess right now we'd expect "rockchip,rk3288w-cru", "rockchip,rk3288-cru",
> 
> Thinking again about this, I'm wondering if we should switch to having
> only one per variant ... like on the two rk3188 variants,
> so declaring separate rk3288-cru and rk3288w-cru of-clks with shared
> common code.
> 

If we want to take this route (which I think makes sense), we should
do that sooner than later, so we don't release two different implementations
with two different requirements.

This change should be quite simple, no?

Thanks,
Ezequiel






More information about the linux-arm-kernel mailing list