Hi Morimoto-san,
On Wed, May 26, 2021 at 12:48 AM Kuninori Morimoto
<kuninori.morimoto.gx@xxxxxxxxxxx> wrote:
> > I'm not such a big fan of creating dummy clocks.
> > And what if a future SoC lacks two CLOCKIN pins? Then you'll try to
> > register a second dummy clock with the same name, which will fail,
> > presumably?
>
> I think current code will reuse same null_clk for these.
Oh right, I missed the static clk_hw pointer.
What if you unload the snd-soc-rcar.ko module?
> > This should only be done when the clock does not exist, not in case
> > of other errors (e.g. -EPROBE_DEFER, which isn't handled yet)?
> >
> > As devm_clk_get_optional() already checks for existence, you could use:
> >
> > struct clk *clk = devm_clk_get_optional(dev, clk_name[i]);
> > if (!clk)
> > clk = rsnd_adg_null_clk_get(priv);
>
> Ah, indeed.
> Thanks. I will fix it.
>
> > But in light of the above (avoiding dummy clocks), it might be more
> > robust to make sure all code can handle adg->clk[i] = NULL?
>
> The reason why I don't use adg->clk[i] = NULL is it is using this macro
>
> #define for_each_rsnd_clk(pos, adg, i) \
> for (i = 0; \
> (i < CLKMAX) && \
> ((pos) = adg->clk[i]); \
> i++)
>
> The loop will stop at (A) if it was
>
> adg->clk[0] = audio_clk_a;
> adg->clk[1] = audio_clk_b;
> (A) adg->clk[2] = NULL
> adg->clk[3] = audio_clk_i;
If you use this macro everywhere, that is easily handled by the
following variant:
#define for_each_rsnd_clk(pos, adg, i) \
for (i = 0; (pos) = adg->clk[i], i < CLKMAX; i++) \
if (pos) { \
continue; \
} else
There are several existing examples of such a construct.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
