[v3,09/15] ASoC: tegra: Add fallback for audio mclk
diff mbox series

Message ID 1575600535-26877-10-git-send-email-skomatineni@nvidia.com
State Changes Requested
Headers show
Series
  • Move PMC clocks into Tegra PMC driver
Related show

Commit Message

Sowjanya Komatineni Dec. 6, 2019, 2:48 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.

So, this patch adds fallback to extern1 in case of retrieving mclk fails
to be backward compatible of new device tree with older kernels.

Cc: stable@vger.kernel.org

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 sound/soc/tegra/tegra_asoc_utils.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Sowjanya Komatineni Dec. 6, 2019, 5:49 p.m. UTC | #1
Thanks Greg.

Sorry, Will send this patch separately (out of this series) with stable 
tag to get this applied to stable kernels once review is done for this 
series.

On 12/5/19 6:48 PM, 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.
>
> So, this patch adds fallback to extern1 in case of retrieving mclk fails
> to be backward compatible of new device tree with older kernels.
>
> Cc: stable@vger.kernel.org
>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>   sound/soc/tegra/tegra_asoc_utils.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/tegra/tegra_asoc_utils.c b/sound/soc/tegra/tegra_asoc_utils.c
> index 8e3a3740df7c..f7408d5240c0 100644
> --- a/sound/soc/tegra/tegra_asoc_utils.c
> +++ b/sound/soc/tegra/tegra_asoc_utils.c
> @@ -211,8 +211,14 @@ 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;
> +		data->clk_cdev1 = clk_get_sys("clk_out_1", "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_err(data->dev, "Falling back to extern1\n");
>   	}
>   
>   	/*
Greg KH Dec. 6, 2019, 5:56 p.m. UTC | #2
On Fri, Dec 06, 2019 at 09:49:49AM -0800, Sowjanya Komatineni wrote:
> Thanks Greg.
> 
> Sorry, Will send this patch separately (out of this series) with stable tag
> to get this applied to stable kernels once review is done for this series.

Why not just properly add the tag in the original patch when it gets
merged?  That's how everyone else does it :)

This isn't new, it's been happening this way for 12+ years now...

thanks,

greg k-h
Mark Brown Dec. 9, 2019, 4:40 p.m. UTC | #3
On Thu, Dec 05, 2019 at 06:48:49PM -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.
> 
> So, this patch adds fallback to extern1 in case of retrieving mclk fails
> to be backward compatible of new device tree with older kernels.
> 
> Cc: stable@vger.kernel.org

Why would this need to be a stable fix?  Presumably people with stable
kernels are using the old device tree anyway?
Dmitry Osipenko Dec. 9, 2019, 8:31 p.m. UTC | #4
09.12.2019 19:40, Mark Brown пишет:
> On Thu, Dec 05, 2019 at 06:48:49PM -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.
>>
>> So, this patch adds fallback to extern1 in case of retrieving mclk fails
>> to be backward compatible of new device tree with older kernels.
>>
>> Cc: stable@vger.kernel.org
> 
> Why would this need to be a stable fix?  Presumably people with stable
> kernels are using the old device tree anyway?
> 

Presumably, yes.

At least Rob Herring is asking to maintain backwards compatibility
because some ditros are using newer device-trees with stable kernels.
I'm personally also tending to use the newer DTB with older kernel
version whenever there is a need to check something using stable kernel.
Perhaps losing sound is not very important, but will be nicer if that
doesn't happen.
Mark Brown Dec. 9, 2019, 8:47 p.m. UTC | #5
On Mon, Dec 09, 2019 at 11:31:59PM +0300, Dmitry Osipenko wrote:
> 09.12.2019 19:40, Mark Brown пишет:

> > Why would this need to be a stable fix?  Presumably people with stable
> > kernels are using the old device tree anyway?

> At least Rob Herring is asking to maintain backwards compatibility
> because some ditros are using newer device-trees with stable kernels.

You're talking about forwards compatibility not backwards here.  Are
those distros actually using LTS kernels?

> I'm personally also tending to use the newer DTB with older kernel
> version whenever there is a need to check something using stable kernel.
> Perhaps losing sound is not very important, but will be nicer if that
> doesn't happen.

That really does sound like a "you broke it, you get all the pieces"
situation TBH...  I'd be a lot more comfortable if the stable kernels
were sticking to bugfix only though I do appreciate that they're not
really that any more.
Dmitry Osipenko Dec. 10, 2019, 6:24 p.m. UTC | #6
09.12.2019 23:47, Mark Brown пишет:
> On Mon, Dec 09, 2019 at 11:31:59PM +0300, Dmitry Osipenko wrote:
>> 09.12.2019 19:40, Mark Brown пишет:
> 
>>> Why would this need to be a stable fix?  Presumably people with stable
>>> kernels are using the old device tree anyway?
> 
>> At least Rob Herring is asking to maintain backwards compatibility
>> because some ditros are using newer device-trees with stable kernels.
> 
> You're talking about forwards compatibility not backwards here.  Are
> those distros actually using LTS kernels?

I think openSUSE Leap could be one of those distros that use LTS kernel
with newer device-trees, but that's not 100%. Maybe Rob could help
clarifying that.

>> I'm personally also tending to use the newer DTB with older kernel
>> version whenever there is a need to check something using stable kernel.
>> Perhaps losing sound is not very important, but will be nicer if that
>> doesn't happen.
> 
> That really does sound like a "you broke it, you get all the pieces"
> situation TBH...  I'd be a lot more comfortable if the stable kernels
> were sticking to bugfix only though I do appreciate that they're not
> really that any more.

In some cases it could be painful to maintain device-tree compatibility
for platforms like NVIDIA Tegra SoCs because hardware wasn't modeled
correctly from the start.

I agree that people should use relevant device-trees. It's quite a lot
of hassle to care about compatibility for platforms that are permanently
in a development state. It could be more reasonable to go through the
pain if kernel required a full-featured device tree for every SoC from
the start.

But maybe Tegra / device-tree maintainers have a different opinion.
IIUC, device-trees are meant to be stable and software-agnostic, at
least that was the case not so long time ago and I don't think that this
premise changed.
Mark Brown Dec. 10, 2019, 6:59 p.m. UTC | #7
On Tue, Dec 10, 2019 at 09:24:43PM +0300, Dmitry Osipenko wrote:

> In some cases it could be painful to maintain device-tree compatibility
> for platforms like NVIDIA Tegra SoCs because hardware wasn't modeled
> correctly from the start.

> I agree that people should use relevant device-trees. It's quite a lot
> of hassle to care about compatibility for platforms that are permanently
> in a development state. It could be more reasonable to go through the
> pain if kernel required a full-featured device tree for every SoC from
> the start.

We absolutely should support the newer kernel with older device tree
case, what's less clear to me is the new device tree with old kernel
which is applying LTS updates case.  That does seem incredibly
specialist - I'd honestly never heard of people doing that before this
thread.
Dmitry Osipenko Dec. 12, 2019, 2:17 a.m. UTC | #8
10.12.2019 21:59, Mark Brown пишет:
> On Tue, Dec 10, 2019 at 09:24:43PM +0300, Dmitry Osipenko wrote:
> 
>> In some cases it could be painful to maintain device-tree compatibility
>> for platforms like NVIDIA Tegra SoCs because hardware wasn't modeled
>> correctly from the start.
> 
>> I agree that people should use relevant device-trees. It's quite a lot
>> of hassle to care about compatibility for platforms that are permanently
>> in a development state. It could be more reasonable to go through the
>> pain if kernel required a full-featured device tree for every SoC from
>> the start.
> 
> We absolutely should support the newer kernel with older device tree
> case, what's less clear to me is the new device tree with old kernel
> which is applying LTS updates case.  That does seem incredibly
> specialist - I'd honestly never heard of people doing that before this
> thread.
> 

There was a precedent in the past [1].

[1] https://lkml.org/lkml/2018/8/20/497

Patch
diff mbox series

diff --git a/sound/soc/tegra/tegra_asoc_utils.c b/sound/soc/tegra/tegra_asoc_utils.c
index 8e3a3740df7c..f7408d5240c0 100644
--- a/sound/soc/tegra/tegra_asoc_utils.c
+++ b/sound/soc/tegra/tegra_asoc_utils.c
@@ -211,8 +211,14 @@  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;
+		data->clk_cdev1 = clk_get_sys("clk_out_1", "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_err(data->dev, "Falling back to extern1\n");
 	}
 
 	/*