On 09/09/19 09:17:03, Stephen Boyd wrote:
> Quoting Jorge Ramirez-Ortiz, Linaro (2019-09-09 07:17:40)
> > On 09/09/19 03:21:16, Stephen Boyd wrote:
> > > Quoting Jorge Ramirez-Ortiz (2019-08-26 09:45:07)
> > > > @@ -76,10 +88,11 @@ static int qcom_apcs_msm8916_clk_probe(struct 
> > > > platform_device *pdev)
> > > >         a53cc->src_shift = 8;
> > > >         a53cc->parent_map = gpll0_a53cc_map;
> > > >  
> > > > -       a53cc->pclk = devm_clk_get(parent, NULL);
> > > > +       a53cc->pclk = of_clk_get(parent->of_node, pll_index);
> > > 
> > > Presumably the PLL was always index 0, so why are we changing it to
> > > index 1 sometimes? Seems unnecessary.
> > > 
> > 
> > it came as a personal preference. hope it is acceptable (I would
> > rather not change it)
> > 
> > apcs-msm8916.c declares the following
> > 
> > [..]
> > static const u32 gpll0_a53cc_map[] = { 4, 5 };
> > static const char *gpll0_a53cc[] = {
> >        "gpll0_vote",
> >         "a53pll",
> >         };
> > [..]
> > 
> > 
> > now will be doing this
> > 
> > --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> > @@ -429,7 +429,8 @@
> >      compatible = "qcom,msm8916-apcs-kpss-global", "syscon";
> >      reg = <0xb011000 0x1000>;
> >      #mbox-cells = <1>;
> > -                   clocks = <&a53pll>;
> > +                 clocks = <&gcc GPLL0_VOTE>, <&a53pll>;
> > +                 clock-names = "aux", "pll";
> >                       #clock-cells = <0>;
> >                };
> >                                                                             
> >                                     
> > 
> > so I chose to keep the consistency between the clocks definition and
> > just change the index before calling of_clk_get.
> > 
> 
> But now the binding is different for the same compatible. I'd prefer we
> keep using devm_clk_get() and use a device pointer here and reorder the
> map and parent arrays instead. The clocks property shouldn't change in a
> way that isn't "additive" so that we maintain backwards compatibility.
> 

but the backwards compatibility is fully maintained - that is the main reason
behind the change. the new stuff is that  instead of hardcoding the
names in the source - like it is being done on the msm8916- we provide
the clocks in the dts node (a cleaner approach with the obvious
benefit of allowing new users to be added without having to modify the
sources).



Reply via email to