Re: [PATCH 2/4] clk: qcom: branch: Add mem ops support for branch2 clocks

[Imran Shaik]

On 8/23/2023 12:22 AM, Stephen Boyd wrote:
> Quoting Imran Shaik (2023-08-07 22:14:05)
> > diff --git a/drivers/clk/qcom/clk-branch.h b/drivers/clk/qcom/clk-branch.h
> > index 0cf800b9d08d..0ffda6bef00e 100644
> > --- a/drivers/clk/qcom/clk-branch.h
> > +++ b/drivers/clk/qcom/clk-branch.h
> > @@ -24,8 +24,11 @@
> >   struct clk_branch {
> >          u32     hwcg_reg;
> >          u32     halt_reg;
> > +       u32     mem_enable_reg;
> > +       u32     mem_ack_reg;
> >          u8      hwcg_bit;
> >          u8      halt_bit;
> > +       u8      mem_enable_ack_bit;
> >          u8      halt_check;
> >   #define BRANCH_VOTED                   BIT(7) /* Delay on disable */
> >   #define BRANCH_HALT                    0 /* pol: 1 = halt */
>
> I suspect making a wrapper around struct clk_branch would be a better
> approach so that we don't bloat all the other clk_branch structures that
> exist in the qcom clk drivers.
>
>   $ git grep 'struct clk_branch' -- drivers/clk/qcom | wc -l
>     6357
>
> How many of these are going to be using these new registers? It may also
> make sense to do that for hardware clock gating as well, but I'm not
> really sure. Anyway, the idea is
>
> 	struct clk_mem_branch {
> 		u32 enable_reg;
> 		u32 ack_reg;
> 		u8  ack_bit;
> 		struct clk_branch branch;
> 	};
>
> and then a container_of define. Plus, you can put some comment above the
> structure to describe when these clks are used.

Sure, will use the approach mentioned above and push the next series.

Thanks,
Imran