Hello Stephen, Thanks for your review. On 4/20/2023 3:07 AM, Stephen Boyd wrote:
Quoting Taniya Das (2023-04-19 06:30:10)From: Imran Shaik <quic_imrashai@xxxxxxxxxxx> Add support to handle the invert logic for branch2 clocks. Invert branch halt would indicate the clock ON when CLK_OFF bit is '1' and OFF when CLK_OFF bit is '0'. Signed-off-by: Imran Shaik <quic_imrashai@xxxxxxxxxxx> Signed-off-by: Taniya Das <quic_tdas@xxxxxxxxxxx> --- drivers/clk/qcom/clk-branch.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/clk/qcom/clk-branch.c b/drivers/clk/qcom/clk-branch.c index f869fc6aaed6..4b24d45be771 100644 --- a/drivers/clk/qcom/clk-branch.c +++ b/drivers/clk/qcom/clk-branch.c @@ -48,6 +48,7 @@ static bool clk_branch2_check_halt(const struct clk_branch *br, bool enabling) { u32 val; u32 mask; + bool invert = (br->halt_check == BRANCH_HALT_ENABLE);mask = BRANCH_NOC_FSM_STATUS_MASK << BRANCH_NOC_FSM_STATUS_SHIFT;mask |= BRANCH_CLK_OFF; @@ -56,9 +57,16 @@ static bool clk_branch2_check_halt(const struct clk_branch *br, bool enabling)if (enabling) {val &= mask; + + if (invert) + return (val & BRANCH_CLK_OFF) == BRANCH_CLK_OFF; + return (val & BRANCH_CLK_OFF) == 0 || val == BRANCH_NOC_FSM_STATUS_ON;Do these clks have a NOC_FSM_STATUS bit? I think it would be better to make a local variable for the val we're looking for, and then test for that. We may need a mask as well, but the idea is to not duplicate the test and return from multiple places.
Clocks which has invert status doesn't have NOC_FSM_STATUS bit. Will remove the multiple returns in next patch.
} else { + if (invert) + return (val & BRANCH_CLK_OFF) == 0; + return val & BRANCH_CLK_OFF; }While at it, I'd get rid of this else and de-indent the code because if we're 'enabling' we'll return from the function regardless.
Yes, Stephen, will take care in the next patch. -- Thanks & Regards, Taniya Das.
