[v8,11/22] ASoC: tegra: Add fallback implementation for audio mclk
diff mbox series

Message ID 1578986667-16041-12-git-send-email-skomatineni@nvidia.com
State New
Headers show
Series
  • Move PMC clocks into Tegra PMC driver
Related show

Commit Message

Sowjanya Komatineni Jan. 14, 2020, 7:24 a.m. UTC
mclk is from clk_out_1 which is part of Tegra PMC block and pmc clocks
are moved to Tegra PMC driver with pmc as clock provider and using pmc
clock ids.

New device tree uses clk_out_1 from pmc clock provider as audio mclk.

So, this patch adds implementation for mclk fallback to extern1 when
retrieving mclk returns -ENOENT to be backward compatible of new device
tree with older kernels.

Fixes: 110147c8c513 ("ASoC: tegra: always use clk_get() in utility code")

Tested-by: Dmitry Osipenko <digetx@gmail.com>
Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 sound/soc/tegra/tegra_asoc_utils.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

Comments

Dmitry Osipenko Jan. 19, 2020, 3:08 p.m. UTC | #1
14.01.2020 10:24, Sowjanya Komatineni пишет:
> mclk is from clk_out_1 which is part of Tegra PMC block and pmc clocks
> are moved to Tegra PMC driver with pmc as clock provider and using pmc
> clock ids.
> 
> New device tree uses clk_out_1 from pmc clock provider as audio mclk.
> 
> So, this patch adds implementation for mclk fallback to extern1 when
> retrieving mclk returns -ENOENT to be backward compatible of new device
> tree with older kernels.
> 
> Fixes: 110147c8c513 ("ASoC: tegra: always use clk_get() in utility code")

I don't think that it is correct to use the Fixes tag here because this
patch doesn't fix any real problem of the referenced commit, instead the
Stable CC tag should be used.

> Tested-by: Dmitry Osipenko <digetx@gmail.com>
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---[snip]
Dmitry Osipenko Jan. 23, 2020, 11:56 p.m. UTC | #2
14.01.2020 10:24, Sowjanya Komatineni пишет:
> mclk is from clk_out_1 which is part of Tegra PMC block and pmc clocks
> are moved to Tegra PMC driver with pmc as clock provider and using pmc
> clock ids.
> 
> New device tree uses clk_out_1 from pmc clock provider as audio mclk.
> 
> So, this patch adds implementation for mclk fallback to extern1 when
> retrieving mclk returns -ENOENT to be backward compatible of new device
> tree with older kernels.
> 
> Fixes: 110147c8c513 ("ASoC: tegra: always use clk_get() in utility code")
> 
> Tested-by: Dmitry Osipenko <digetx@gmail.com>
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>  sound/soc/tegra/tegra_asoc_utils.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/tegra/tegra_asoc_utils.c b/sound/soc/tegra/tegra_asoc_utils.c
> index 536a578e9512..74d3ffe7e603 100644
> --- a/sound/soc/tegra/tegra_asoc_utils.c
> +++ b/sound/soc/tegra/tegra_asoc_utils.c
> @@ -191,9 +191,21 @@ int tegra_asoc_utils_init(struct tegra_asoc_utils_data *data,
>  
>  	data->clk_cdev1 = clk_get(dev, "mclk");
>  	if (IS_ERR(data->clk_cdev1)) {
> -		dev_err(data->dev, "Can't retrieve clk cdev1\n");
> -		ret = PTR_ERR(data->clk_cdev1);
> -		goto err_put_pll_a_out0;
> +		if (PTR_ERR(data->clk_cdev1) != -ENOENT) {
> +			dev_err(data->dev, "Can't retrieve clk cdev1\n");
> +			ret = PTR_ERR(data->clk_cdev1);
> +			goto err_put_pll_a_out0;
> +		}
> +
> +		/* Fall back to extern1 */
> +		data->clk_cdev1 = clk_get(dev, "extern1");
> +		if (IS_ERR(data->clk_cdev1)) {
> +			dev_err(data->dev, "Can't retrieve clk extern1\n");
> +			ret = PTR_ERR(data->clk_cdev1);
> +			goto err_put_pll_a_out0;
> +		}
> +
> +		dev_info(data->dev, "Falling back to extern1\n");
>  	}
>  
>  	ret = tegra_asoc_utils_set_rate(data, 44100, 256 * 44100);
> 

I tried to double-check if audio works using the updated DT works with
this fallback and unfortunately it is not (maybe I actually missed to
test this case before).. the driver doesn't probe at all because of the
assigned-clocks presence, which makes clk core to fail finding the MCLK
and thus assigned-clocks configuration fails, preventing the driver's
loading.

I'm not sure what could be done about it. Perhaps just to give up on the
compatibility of older kernels with the new DTs, missing audio isn't
really that critical (perhaps).
Thierry Reding Feb. 17, 2020, 9:29 a.m. UTC | #3
On Fri, Jan 24, 2020 at 02:56:45AM +0300, Dmitry Osipenko wrote:
> 14.01.2020 10:24, Sowjanya Komatineni пишет:
> > mclk is from clk_out_1 which is part of Tegra PMC block and pmc clocks
> > are moved to Tegra PMC driver with pmc as clock provider and using pmc
> > clock ids.
> > 
> > New device tree uses clk_out_1 from pmc clock provider as audio mclk.
> > 
> > So, this patch adds implementation for mclk fallback to extern1 when
> > retrieving mclk returns -ENOENT to be backward compatible of new device
> > tree with older kernels.
> > 
> > Fixes: 110147c8c513 ("ASoC: tegra: always use clk_get() in utility code")
> > 
> > Tested-by: Dmitry Osipenko <digetx@gmail.com>
> > Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> > Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> > ---
> >  sound/soc/tegra/tegra_asoc_utils.c | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> > 
> > diff --git a/sound/soc/tegra/tegra_asoc_utils.c b/sound/soc/tegra/tegra_asoc_utils.c
> > index 536a578e9512..74d3ffe7e603 100644
> > --- a/sound/soc/tegra/tegra_asoc_utils.c
> > +++ b/sound/soc/tegra/tegra_asoc_utils.c
> > @@ -191,9 +191,21 @@ int tegra_asoc_utils_init(struct tegra_asoc_utils_data *data,
> >  
> >  	data->clk_cdev1 = clk_get(dev, "mclk");
> >  	if (IS_ERR(data->clk_cdev1)) {
> > -		dev_err(data->dev, "Can't retrieve clk cdev1\n");
> > -		ret = PTR_ERR(data->clk_cdev1);
> > -		goto err_put_pll_a_out0;
> > +		if (PTR_ERR(data->clk_cdev1) != -ENOENT) {
> > +			dev_err(data->dev, "Can't retrieve clk cdev1\n");
> > +			ret = PTR_ERR(data->clk_cdev1);
> > +			goto err_put_pll_a_out0;
> > +		}
> > +
> > +		/* Fall back to extern1 */
> > +		data->clk_cdev1 = clk_get(dev, "extern1");
> > +		if (IS_ERR(data->clk_cdev1)) {
> > +			dev_err(data->dev, "Can't retrieve clk extern1\n");
> > +			ret = PTR_ERR(data->clk_cdev1);
> > +			goto err_put_pll_a_out0;
> > +		}
> > +
> > +		dev_info(data->dev, "Falling back to extern1\n");
> >  	}
> >  
> >  	ret = tegra_asoc_utils_set_rate(data, 44100, 256 * 44100);
> > 
> 
> I tried to double-check if audio works using the updated DT works with
> this fallback and unfortunately it is not (maybe I actually missed to
> test this case before).. the driver doesn't probe at all because of the
> assigned-clocks presence, which makes clk core to fail finding the MCLK
> and thus assigned-clocks configuration fails, preventing the driver's
> loading.
> 
> I'm not sure what could be done about it. Perhaps just to give up on the
> compatibility of older kernels with the new DTs, missing audio isn't
> really that critical (perhaps).

Compatibility of older kernels with new DTs isn't really something that
we strive for. The whole point about ABI stability is to avoid requiring
DT updates with new kernels because a DTB may not be in a writable
location. Practically I'm not aware of such a case on Tegra.

However, the kernel is always assumed to be in a writable location, if
for nothing else but to ensure it can be updated to fix security issues.
So if a new DTB is flashed it can be assumed that users will be able to
also flash a new kernel to work with that DTB.

Thierry
Thierry Reding Feb. 17, 2020, 9:40 a.m. UTC | #4
On Mon, Jan 13, 2020 at 11:24:16PM -0800, Sowjanya Komatineni wrote:
> mclk is from clk_out_1 which is part of Tegra PMC block and pmc clocks
> are moved to Tegra PMC driver with pmc as clock provider and using pmc
> clock ids.
> 
> New device tree uses clk_out_1 from pmc clock provider as audio mclk.
> 
> So, this patch adds implementation for mclk fallback to extern1 when
> retrieving mclk returns -ENOENT to be backward compatible of new device
> tree with older kernels.
> 
> Fixes: 110147c8c513 ("ASoC: tegra: always use clk_get() in utility code")
> 
> Tested-by: Dmitry Osipenko <digetx@gmail.com>
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>  sound/soc/tegra/tegra_asoc_utils.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)

There's some inconsistent spelling of PMC in the above, but other than
that:

Acked-by: Thierry Reding <treding@nvidia.com>
Dmitry Osipenko Feb. 17, 2020, 2:51 p.m. UTC | #5
17.02.2020 12:40, Thierry Reding пишет:
> On Mon, Jan 13, 2020 at 11:24:16PM -0800, Sowjanya Komatineni wrote:
>> mclk is from clk_out_1 which is part of Tegra PMC block and pmc clocks
>> are moved to Tegra PMC driver with pmc as clock provider and using pmc
>> clock ids.
>>
>> New device tree uses clk_out_1 from pmc clock provider as audio mclk.
>>
>> So, this patch adds implementation for mclk fallback to extern1 when
>> retrieving mclk returns -ENOENT to be backward compatible of new device
>> tree with older kernels.
>>
>> Fixes: 110147c8c513 ("ASoC: tegra: always use clk_get() in utility code")
>>
>> Tested-by: Dmitry Osipenko <digetx@gmail.com>
>> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>>  sound/soc/tegra/tegra_asoc_utils.c | 18 +++++++++++++++---
>>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> There's some inconsistent spelling of PMC in the above, but other than
> that:
> 
> Acked-by: Thierry Reding <treding@nvidia.com>
> 

Seems you missed my point.. this patch doesn't work at all, and thus, it
should be dropped.

Patch
diff mbox series

diff --git a/sound/soc/tegra/tegra_asoc_utils.c b/sound/soc/tegra/tegra_asoc_utils.c
index 536a578e9512..74d3ffe7e603 100644
--- a/sound/soc/tegra/tegra_asoc_utils.c
+++ b/sound/soc/tegra/tegra_asoc_utils.c
@@ -191,9 +191,21 @@  int tegra_asoc_utils_init(struct tegra_asoc_utils_data *data,
 
 	data->clk_cdev1 = clk_get(dev, "mclk");
 	if (IS_ERR(data->clk_cdev1)) {
-		dev_err(data->dev, "Can't retrieve clk cdev1\n");
-		ret = PTR_ERR(data->clk_cdev1);
-		goto err_put_pll_a_out0;
+		if (PTR_ERR(data->clk_cdev1) != -ENOENT) {
+			dev_err(data->dev, "Can't retrieve clk cdev1\n");
+			ret = PTR_ERR(data->clk_cdev1);
+			goto err_put_pll_a_out0;
+		}
+
+		/* Fall back to extern1 */
+		data->clk_cdev1 = clk_get(dev, "extern1");
+		if (IS_ERR(data->clk_cdev1)) {
+			dev_err(data->dev, "Can't retrieve clk extern1\n");
+			ret = PTR_ERR(data->clk_cdev1);
+			goto err_put_pll_a_out0;
+		}
+
+		dev_info(data->dev, "Falling back to extern1\n");
 	}
 
 	ret = tegra_asoc_utils_set_rate(data, 44100, 256 * 44100);