On 02/06/2024 06:12, Bjorn Andersson wrote: > On Fri, May 31, 2024 at 11:59:04AM GMT, Krzysztof Kozlowski wrote: >> On 31/05/2024 11:02, Taniya Das wrote: >>> The gpu clocks and GDSC have been marked critical from the clock driver >>> which is not desired for functionality. Hence remove the CLK_IS_CRITICAL >>> and ALWAYS_ON flags. >>> >>> Fixes: 0afa16afc36d ("clk: qcom: add the GPUCC driver for sa8775p") >>> Signed-off-by: Taniya Das <quic_tdas@xxxxxxxxxxx> >>> --- >>> drivers/clk/qcom/gpucc-sa8775p.c | 27 +++++++++++---------------- >>> 1 file changed, 11 insertions(+), 16 deletions(-) >>> >>> diff --git a/drivers/clk/qcom/gpucc-sa8775p.c b/drivers/clk/qcom/gpucc-sa8775p.c >>> index 1167c42da39d..f965babf4330 100644 >>> --- a/drivers/clk/qcom/gpucc-sa8775p.c >>> +++ b/drivers/clk/qcom/gpucc-sa8775p.c >>> @@ -1,6 +1,6 @@ >>> // SPDX-License-Identifier: GPL-2.0-only >>> /* >>> - * Copyright (c) 2021-2022, Qualcomm Innovation Center, Inc. All rights reserved. >>> + * Copyright (c) 2021-2022, 2024, Qualcomm Innovation Center, Inc. All rights reserved. >> >> That's not a fix. >> >>> * Copyright (c) 2023, Linaro Limited >>> */ >>> >>> @@ -280,7 +280,7 @@ static struct clk_branch gpu_cc_ahb_clk = { >>> &gpu_cc_hub_ahb_div_clk_src.clkr.hw, >>> }, >>> .num_parents = 1, >>> - .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, >>> + .flags = CLK_SET_RATE_PARENT, >> >> I fail to see why this is a fix. They were marked as critical on >> purpose. It was needed, wasn't it? >> >> Provide jsutification for commits, not just sprinkle Fixes tag all around. >> > > This is indeed a fix, as marking clocks CLK_IS_CRITICAL prevents any > power-domain associated with the clock controller from suspending. In > other words, the current behavior is broken. > > @Taniya, "not desired for functionality" does not carry any useful > information explaining why this change is made. Please update the commit > message. Then please provide some sort of bug explanation in the commit msg. I assume the clocks were marked critical to solve some particular problem, e.g. missing parents, so marking this as fix sounds like both incorrect and error-prone when backported. Maybe that's not the case, so this is why there is commit msg to explain such details... Best regards, Krzysztof