Re: [PATCH v6 03/18] clk: qcom: gcc-ipq806x: add PXO_SRC in clk table

From: Dmitry Baryshkov
Date: Wed Apr 13 2022 - 13:32:40 EST


On Wed, 13 Apr 2022 at 20:00, Ansuel Smith <ansuelsmth@xxxxxxxxx> wrote:
>
> On Thu, Mar 24, 2022 at 06:22:29PM -0700, Stephen Boyd wrote:
> > Quoting Ansuel Smith (2022-03-24 18:13:49)
> > > On Thu, Mar 24, 2022 at 06:10:35PM -0700, Stephen Boyd wrote:
> > > > Quoting Ansuel Smith (2022-03-21 16:15:33)
> > > > > PXO_SRC is currently defined in the gcc include and referenced in the
> > > > > ipq8064 DTSI. Correctly provide a clk after gcc probe to fix kernel
> > > > > panic if a driver starts to actually use it.
> > > > >
> > > > > Signed-off-by: Ansuel Smith <ansuelsmth@xxxxxxxxx>
> > > > > ---
> > > >
> > > > What is this patch about? clk providers shouldn't be calling clk_get().
> > > >
> > >
> > > If pxo is passed as a clock in dts and defined as a fixed clock, what
> > > should be used?
> >
> > clk_parent_data
>
> Sorry but I'm not following you. No idea if you missed the cover letter
> where i describe the problem with PXO_SRC.
>
> The problem here is that
> - In DTS we have node that reference <&gcc PXO_SRC>
> But
> - gcc driver NEVER defined PXO_SRC
> As
> - PXO_SRC is actually pxo_board that should be defined as a fixed-clock
> in dts or is defined using qcom_cc_register_board_clk.
>
> So in theory we should just put in PXO_SRC the clk hw of the
> fixed-clock. That is why I'm using clk_get(). I can use __clk_lookup()
> as an alternative but I really can't find a way to get the clock defined
> from DTS or qcom_cc_register_board_clk.
>
> (I have the same exact problem with the cpu qsb clock where is defined
> using fixed-clock API but can also defined directly in DTS and I have to
> use clk_get())
>
> I'm totally missing something so I would love some hint on how to solve
> this.

When we were doing such conversion for other platforms, we pointed
clock consumers to the board clocks directly. There is no need to go
through the gcc to fetch pxo.
Instead you can use a <&pxo_board> in the dts directly. Typically the
sequence is the following:
- Minor cleanup of the clock-controller driver
(ARRAY_SIZE(parent_data), removal of unused clock sources, unused enum
entries, etc)
- update drivers to use both .name and .fw_name in replacement of
parent_names. Use parent_hws where possible.
- update dtsi to reference clocks using clocks/clock-names properties.
Pass board/rpmh/rpm clocks directly to their consumers without
bandaids in the gcc driver.
- (optionally) after several major releases drop parent_data.name
completely. I think we mostly skipped this, since it provides no gain.

This way you don't have to play around clk_get to return PXO_SRC from
gcc clock-controller.

--
With best wishes
Dmitry