Re: [PATCH 3/3] ASoC: rsnd: add null CLOCKIN support
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-05-25 10:33:03
Also in:
alsa-devel
Hi Morimoto-san, On Mon, May 24, 2021 at 8:12 AM Kuninori Morimoto [off-list ref] wrote:
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Some Renesas SoC doesn't have full CLOCKIN. This patch add null_clk, and accepts it. Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Thanks for your patch, which is now commit d6956a7dde6fbf84 ("ASoC:
rsnd: add null CLOCKIN support") in asoc/for-next.
]> --- a/sound/soc/sh/rcar/adg.cquoted hunk ↗ jump to hunk
+++ b/sound/soc/sh/rcar/adg.c@@ -389,6 +389,30 @@ void rsnd_adg_clk_control(struct rsnd_priv *priv, int enable) } } +#define NULL_CLK "rsnd_adg_null" +static struct clk *rsnd_adg_null_clk_get(struct rsnd_priv *priv) +{ + static struct clk_hw *hw; + struct device *dev = rsnd_priv_to_dev(priv); + + if (!hw) { + struct clk_hw *_hw; + int ret; + + _hw = clk_hw_register_fixed_rate_with_accuracy(dev, NULL_CLK, NULL, 0, 0, 0); + if (IS_ERR(_hw)) + return NULL; + + ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get, _hw);
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?
quoted hunk ↗ jump to hunk
+ if (ret < 0) + clk_hw_unregister_fixed_rate(_hw); + + hw = _hw; + } + + return clk_hw_get_clk(hw, NULL_CLK); +} + static void rsnd_adg_get_clkin(struct rsnd_priv *priv, struct rsnd_adg *adg) {@@ -398,7 +422,12 @@ static void rsnd_adg_get_clkin(struct rsnd_priv *priv, for (i = 0; i < CLKMAX; i++) { struct clk *clk = devm_clk_get(dev, clk_name[i]); - adg->clk[i] = IS_ERR(clk) ? NULL : clk; + if (IS_ERR(clk)) + clk = rsnd_adg_null_clk_get(priv);
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);
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?
+ if (IS_ERR(clk))
+ dev_err(dev, "no adg clock (%s)\n", clk_name[i]);
+
+ adg->clk[i] = clk;
}
}
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