Re: [PATCH v2 1/2] clk: imx93: Move IMX93_CLK_END macro to clk driver

[Pengfei Li]

On Wed, Oct 09, 2024 at 02:51:41PM +0300, Abel Vesa wrote:
> On 24-06-27 16:24:25, Pengfei Li wrote:
> > IMX93_CLK_END was previously defined in imx93-clock.h to indicate
> > the number of clocks, but it is not part of the ABI, so it should
> > be moved to clk driver.
> > 
> > Signed-off-by: Pengfei Li <pengfei.li_1@xxxxxxx>
> > ---
> > 
> > Notes:
> >     Change for v2:
> >     - Use pre-processor define to simplify code.
> > 
> >  drivers/clk/imx/clk-imx93.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/clk/imx/clk-imx93.c b/drivers/clk/imx/clk-imx93.c
> > index c6a9bc8ecc1f..c8b65146e76e 100644
> > --- a/drivers/clk/imx/clk-imx93.c
> > +++ b/drivers/clk/imx/clk-imx93.c
> > @@ -15,6 +15,8 @@
> >  
> >  #include "clk.h"
> >  
> > +#define IMX93_CLK_END 202
> > +
> 
> TBH, I don't like this. And I still don't understand the reason.
> 
> Where are the clocks you want to add ?

Hi Abel,

I re-sent a new version of the patch set which includes changes to add the new clocks:
https://lore.kernel.org/all/20241011045736.308338-1-pengfei.li_1@xxxxxxx/

> 
> And why wouldn't it be OK to just keep this in the bindings header even
> if you need to add more clocks ?

Krzysztof has help explained this:
https://lore.kernel.org/all/a07828a4-8040-42cb-8c62-8939cac4d9de@xxxxxxxxxx/.

> 
> For the moment, I don't see the value of this change.
> 
> Sorry, but NACK.
> 
> >  enum clk_sel {
> >  	LOW_SPEED_IO_SEL,
> >  	NON_IO_SEL,
> > -- 
> > 2.34.1
> > 
>