Re: [PATCH 3/3] ASoC: rsnd: add null CLOCKIN support

Hi Morimoto-san,

On Wed, May 26, 2021 at 12:48 AM Kuninori Morimoto
[off-list ref] wrote:

quoted

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?

quoted

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.

quoted

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@linux-m68k.org

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