Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
References: <20210410111356.467340-1-jbrunet@baylibre.com>
 <20210410111356.467340-5-jbrunet@baylibre.com>
 <0c8c3b5d-891a-9bfa-3bbc-4982542df38a@linaro.org>
User-agent: mu4e 1.4.10; emacs 27.2
From:   Jerome Brunet <jbrunet@baylibre.com>
To:     Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
        Mark Brown <broonie@kernel.org>,
        Liam Girdwood <lgirdwood@gmail.com>
Cc:     Stephen Boyd <sboyd@kernel.org>, alsa-devel@alsa-project.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/5] ASoC: lpass: use the clock provider API
In-reply-to: <0c8c3b5d-891a-9bfa-3bbc-4982542df38a@linaro.org>
Message-ID: <1jmtu38znu.fsf@starbuckisacylon.baylibre.com>
Date:   Mon, 12 Apr 2021 14:17:57 +0200
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk


On Mon 12 Apr 2021 at 11:38, Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:

> Thanks Jerome for the patch,
>
>
> On 10/04/2021 12:13, Jerome Brunet wrote:
>> Clock providers should be registered using the clk_hw API.
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>>   sound/soc/codecs/lpass-va-macro.c  | 2 +-
>>   sound/soc/codecs/lpass-wsa-macro.c | 9 +++------
>>   2 files changed, 4 insertions(+), 7 deletions(-)
>> diff --git a/sound/soc/codecs/lpass-va-macro.c
>> b/sound/soc/codecs/lpass-va-macro.c
>> index 5294c57b2cd4..56b887301172 100644
>> --- a/sound/soc/codecs/lpass-va-macro.c
>> +++ b/sound/soc/codecs/lpass-va-macro.c
>> @@ -1343,7 +1343,7 @@ static int va_macro_register_fsgen_output(struct va_macro *va)
>>   	if (ret)
>>   		return ret;
>>   -	return of_clk_add_provider(np, of_clk_src_simple_get, va->hw.clk);
>> +	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, &va->hw);
>
> Now that we convert this to devm, You missed error path and driver remove
> where we delete clk provider. This should be removed as well as part of
> this patch.

Indeed. I should not have switched to devm here - It was not really the
purpose of the patch. Habits I guess.

Do you prefer I stick with devm (with the suggested fix) or revert to the
no-devm way for the v2 ? It makes no difference to me TBH.

>
>
> This applies to both wsa and va macro.
>
> Thanks,
> srini
>>   }
>>     static int va_macro_validate_dmic_sample_rate(u32 dmic_sample_rate,
>> diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
>> index e79a70386b4b..acb95e83c788 100644
>> --- a/sound/soc/codecs/lpass-wsa-macro.c
>> +++ b/sound/soc/codecs/lpass-wsa-macro.c
>> @@ -2337,10 +2337,9 @@ static const struct clk_ops swclk_gate_ops = {
>>   	.recalc_rate = swclk_recalc_rate,
>>   };
>>   -static struct clk *wsa_macro_register_mclk_output(struct wsa_macro
>> *wsa)
>> +static int wsa_macro_register_mclk_output(struct wsa_macro *wsa)
>>   {
>>   	struct device *dev = wsa->dev;
>> -	struct device_node *np = dev->of_node;
>>   	const char *parent_clk_name;
>>   	const char *clk_name = "mclk";
>>   	struct clk_hw *hw;
>> @@ -2358,11 +2357,9 @@ static struct clk *wsa_macro_register_mclk_output(struct wsa_macro *wsa)
>>   	hw = &wsa->hw;
>>   	ret = clk_hw_register(wsa->dev, hw);
>>   	if (ret)
>> -		return ERR_PTR(ret);
>> -
>> -	of_clk_add_provider(np, of_clk_src_simple_get, hw->clk);
>> +		return ret;
>>   -	return NULL;
>> +	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, hw);
>>   }
>>     static const struct snd_soc_component_driver wsa_macro_component_drv
>> = {
>>