On 13/04/2022 20:54, Ansuel Smith wrote:
On Wed, Apr 13, 2022 at 08:32:21PM +0300, Dmitry Baryshkov wrote:
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
Thanks for the list of steps to do this kind of cleanup.
From what I'm reading this series is ""stuck"" in the sense that I first
have to fix the wrong PXO_SRC reference and then I can continue the
conversion work. A bit sad considering most of the time DTS proposal got
ignored :(
Not really. You can leave "pxo" as is. Use { .fw_name = "pxo", .name =
"pxo_board" } as parent_data. Then pass <&pxo_board> as the "pxo" clock
to the consumers. Yes, you will still have the lingering "pxo" / "cxo"
clocks at this step. It's okay, they might be used by other drivers.
After the whole conversion is finished, you can make "pxo"/"cxo"
registration conditional on !of_find_property("clocks") rather than
using clk_get.
As a rule of thumb, you don't have to complete the whole thing in a
single commit. Having smaller commits might be better.
[And yes, I'm looking forward to testing your cpufreq changes on my
apq8064 devices].
--
With best wishes
Dmitry
