Re: [PATCH V4 03/11] clk: imx: scu: add two cells binding support

[Stephen Boyd <sboyd@kernel.org>]

Quoting Dong Aisheng (2019-09-09 03:23:25)
> Hi Stephen,
> 
> Thanks for the review.
> 
> On Sat, Sep 7, 2019 at 5:29 PM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Dong Aisheng (2019-08-20 04:13:17)
> > > diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
> > > index 5e2903e..1ad3f2a 100644
> > > --- a/drivers/clk/imx/clk-imx8qxp.c
> > > +++ b/drivers/clk/imx/clk-imx8qxp.c
> > > @@ -134,7 +134,12 @@ static int imx8qxp_clk_probe(struct platform_device *pdev)
> > >                                 i, PTR_ERR(clks[i]));
> > >         }
> > >
> > > -       return of_clk_add_hw_provider(ccm_node, of_clk_hw_onecell_get, clk_data);
> > > +       if (clock_cells == 2)
> >
> > Can you just read this from the DT node again instead of having a global
> > variable called "clock_cells" for this?
> >
> 
> I tried thinking about it.
> One problem is that we also need this information in the exist clk
> registration API to
> keep the backwards compatibility:
> e.g.
>  static inline struct clk_hw *imx_clk_scu(const char *name, u32 rsrc_id,
>                                          u8 clk_type)
>  {
> -       return __imx_clk_scu(name, NULL, 0, rsrc_id, clk_type);
> +       if (clock_cells == 2)
> +               return imx_clk_scu_alloc_dev(name, NULL, 0, rsrc_id, clk_type);
> +       else
> +               return __imx_clk_scu(name, NULL, 0, rsrc_id, clk_type);
>  }
> 
> Parsing it for all clocks seems not good.

Can you parse it once for the clock controller and then pass it to the
registration function as the number of cells? I dislike the global and
the name of the global.

> 
> In the future, i planned to totally remove the legacy binding support which
> is a premature one and missing continued support.
> Then we will also remove this unneeded clock_cells.

Ok sure.