On Wed, Oct 06, 2021 at 10:51:52AM -0500, Pierre-Louis Bossart wrote:
On 10/6/21 10:04 AM, Andy Shevchenko wrote:
The devm_clk_get_optional() helper returns NULL when devm_clk_get() returns -ENOENT. This makes things slightly cleaner. The added benefit is mostly cosmetic.
...
if (SND_SOC_DAPM_EVENT_ON(event)) {
if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN) {ret = clk_prepare_enable(priv->mclk);if (ret < 0) {dev_err(card->dev,"could not configure MCLK state");return ret;}
ret = clk_prepare_enable(priv->mclk);if (ret < 0) {dev_err(card->dev, "could not configure MCLK state");return ret;I don't get why you removed the test on the BYT_RT5651_MCLK_EN quirk, see below it was designed as a fall-back mode. We don't want to return an error when we know the clock is not present/desired.
Why should we do a unneeded test? When we switch to the optional, there will be no error from these CCF APIs. Besides that it drops indentation level and makes code neat.
...
same here, why was the quirk removed?
Same answer.
...
that part in the probe looks fine, but the changes above are controversial.
I didn't get. How controversial? Why? The whole point of _optional is to get rid of unneeded checks (since they are _anyway_ be called).