On 10.06.2023 14:15, Konrad Dybcio wrote: > > > On 10.06.2023 13:35, Stephan Gerhold wrote: >> On Fri, Jun 09, 2023 at 10:19:09PM +0200, Konrad Dybcio wrote: >>> Before we issue a call to RPM through clk_smd_rpm_enable_scaling() the >>> clock rate requests will not be commited in hardware. This poses a >>> race threat since we're accessing the bus clocks directly from within >>> the interconnect framework. >>> >>> Add a marker to indicate that we're good to go with sending new requests >>> and export it so that it can be referenced from icc. >>> >>> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> >>> --- >>> drivers/clk/qcom/clk-smd-rpm.c | 9 +++++++++ >>> include/linux/soc/qcom/smd-rpm.h | 2 ++ >>> 2 files changed, 11 insertions(+) >>> >>> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c >>> index 937cb1515968..482fe30ee6f0 100644 >>> --- a/drivers/clk/qcom/clk-smd-rpm.c >>> +++ b/drivers/clk/qcom/clk-smd-rpm.c >>> @@ -151,6 +151,7 @@ >>> #define to_clk_smd_rpm(_hw) container_of(_hw, struct clk_smd_rpm, hw) >>> >>> static struct qcom_smd_rpm *rpmcc_smd_rpm; >>> +static bool smd_rpm_clk_scaling; >>> >>> struct clk_smd_rpm { >>> const int rpm_res_type; >>> @@ -385,6 +386,12 @@ static unsigned long clk_smd_rpm_recalc_rate(struct clk_hw *hw, >>> return r->rate; >>> } >>> >>> +bool qcom_smd_rpm_scaling_available(void) >>> +{ >>> + return smd_rpm_clk_scaling; >>> +} >>> +EXPORT_SYMBOL_GPL(qcom_smd_rpm_scaling_available); >>> + >>> static int clk_smd_rpm_enable_scaling(void) >>> { >>> int ret; >>> @@ -410,6 +417,8 @@ static int clk_smd_rpm_enable_scaling(void) >>> return ret; >>> } >>> >>> + smd_rpm_clk_scaling = true; >>> + >> >> If you move the platform_device_register_data(&rpdev->dev, >> "icc_smd_rpm", ...) from drivers/soc/qcom/smd-rpm.c to here you can >> avoid the race completely and drop this API. I think that would be >> cleaner. And it will likely probe much faster because probe deferral >> is slow. :) > Sounds like an idea.. especially since it's pretty much the only > dependency other than SMDRPM itself! It sounds great, but to not break bisecting one has to: 1. change the registration in soc/smd-rpm to store rpm ptr in driver data, in addition to parent driver data 2. change icc/smd-rpm to use the device and not parent data 3. add a platform_device_register_data call in clk-smd-rpm that will always fail because the device is always registered 4. remove the registration from soc/smd-rpm I know you'd love to see me break my [PATCH xx/42] record, but I'd say that deserves its own series and *this* patch could be a good transition middleground :P Hopefully Stephen, Bjorn and Georgi won't send a hitman after me for abusing atomic cross-subsystem merges.. Konrad > > Konrad >> >> Thanks, >> Stephan