All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] clk: x86: Do not gate clocks enabled by the firmware
@ 2017-07-10 20:38 Carlo Caione
  2017-07-10 22:31 ` Enric Balletbo Serra
  0 siblings, 1 reply; 13+ messages in thread
From: Carlo Caione @ 2017-07-10 20:38 UTC (permalink / raw)
  To: mturquette, linux-clk, dvhart, pierre-louis.bossart, sboyd,
	linux, eballetbo, andriy.shevchenko
  Cc: Carlo Caione

From: Carlo Caione <carlo@endlessm.com>

Read the enable register to determine if the clock is already in use by
the firmware. In this case avoid gating the clock.

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 drivers/clk/x86/clk-pmc-atom.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/x86/clk-pmc-atom.c b/drivers/clk/x86/clk-pmc-atom.c
index 2b60577703ef..46a8b6216fca 100644
--- a/drivers/clk/x86/clk-pmc-atom.c
+++ b/drivers/clk/x86/clk-pmc-atom.c
@@ -185,6 +185,9 @@ static struct clk_plt *plt_clk_register(struct platform_device *pdev, int id,
 	pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE;
 	spin_lock_init(&pclk->lock);
 
+	if (plt_clk_is_enabled(&pclk->hw))
+		init.flags |= CLK_IGNORE_UNUSED;
+
 	ret = devm_clk_hw_register(&pdev->dev, &pclk->hw);
 	if (ret) {
 		pclk = ERR_PTR(ret);
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] clk: x86: Do not gate clocks enabled by the firmware
  2017-07-10 20:38 [PATCH] clk: x86: Do not gate clocks enabled by the firmware Carlo Caione
@ 2017-07-10 22:31 ` Enric Balletbo Serra
  2017-07-10 23:54   ` Darren Hart
  0 siblings, 1 reply; 13+ messages in thread
From: Enric Balletbo Serra @ 2017-07-10 22:31 UTC (permalink / raw)
  To: Carlo Caione
  Cc: Michael Turquette, linux-clk, Darren Hart, Pierre-Louis Bossart,
	Stephen Boyd, linux, Andy Shevchenko, Carlo Caione

Hi,

2017-07-10 22:38 GMT+02:00 Carlo Caione <carlo@caione.org>:
> From: Carlo Caione <carlo@endlessm.com>
>
> Read the enable register to determine if the clock is already in use by
> the firmware. In this case avoid gating the clock.
>
> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> ---
>  drivers/clk/x86/clk-pmc-atom.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/clk/x86/clk-pmc-atom.c b/drivers/clk/x86/clk-pmc-atom.c
> index 2b60577703ef..46a8b6216fca 100644
> --- a/drivers/clk/x86/clk-pmc-atom.c
> +++ b/drivers/clk/x86/clk-pmc-atom.c
> @@ -185,6 +185,9 @@ static struct clk_plt *plt_clk_register(struct platform_device *pdev, int id,
>         pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE;
>         spin_lock_init(&pclk->lock);
>
> +       if (plt_clk_is_enabled(&pclk->hw))
> +               init.flags |= CLK_IGNORE_UNUSED;
> +
>         ret = devm_clk_hw_register(&pdev->dev, &pclk->hw);
>         if (ret) {
>                 pclk = ERR_PTR(ret);
> --
> 2.13.2
>

It also fixes the issue introduced in commit 282a4e4 ("platform/x86:
Enable Atom PMC platform clocks") that causes no audio on Baytrail.

Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Thanks Carlo to send this.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] clk: x86: Do not gate clocks enabled by the firmware
  2017-07-10 22:31 ` Enric Balletbo Serra
@ 2017-07-10 23:54   ` Darren Hart
  2017-07-11  1:29     ` Stephen Boyd
  2017-07-24 13:49     ` Pierre-Louis Bossart
  0 siblings, 2 replies; 13+ messages in thread
From: Darren Hart @ 2017-07-10 23:54 UTC (permalink / raw)
  To: Enric Balletbo Serra
  Cc: Carlo Caione, Michael Turquette, linux-clk, Pierre-Louis Bossart,
	Stephen Boyd, linux, Andy Shevchenko, Carlo Caione

On Tue, Jul 11, 2017 at 12:31:28AM +0200, Enric Balletbo Serra wrote:
> Hi,
> 
> 2017-07-10 22:38 GMT+02:00 Carlo Caione <carlo@caione.org>:
> > From: Carlo Caione <carlo@endlessm.com>
> >
> > Read the enable register to determine if the clock is already in use by
> > the firmware. In this case avoid gating the clock.
> >
> > Signed-off-by: Carlo Caione <carlo@endlessm.com>
> > ---
> >  drivers/clk/x86/clk-pmc-atom.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/clk/x86/clk-pmc-atom.c b/drivers/clk/x86/clk-pmc-atom.c
> > index 2b60577703ef..46a8b6216fca 100644
> > --- a/drivers/clk/x86/clk-pmc-atom.c
> > +++ b/drivers/clk/x86/clk-pmc-atom.c
> > @@ -185,6 +185,9 @@ static struct clk_plt *plt_clk_register(struct platform_device *pdev, int id,
> >         pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE;
> >         spin_lock_init(&pclk->lock);
> >
> > +       if (plt_clk_is_enabled(&pclk->hw))
> > +               init.flags |= CLK_IGNORE_UNUSED;
> > +
> >         ret = devm_clk_hw_register(&pdev->dev, &pclk->hw);
> >         if (ret) {
> >                 pclk = ERR_PTR(ret);
> > --
> > 2.13.2
> >
> 
> It also fixes the issue introduced in commit 282a4e4 ("platform/x86:
> Enable Atom PMC platform clocks") that causes no audio on Baytrail.
> 
> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Excellent, thank you Enric. Pierre, any objections to this going in to
4.13 now and stable back to 4.12?

-- 
Darren Hart
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] clk: x86: Do not gate clocks enabled by the firmware
  2017-07-10 23:54   ` Darren Hart
@ 2017-07-11  1:29     ` Stephen Boyd
  2017-07-11  1:38       ` Darren Hart
  2017-07-11  8:09       ` Carlo Caione
  2017-07-24 13:49     ` Pierre-Louis Bossart
  1 sibling, 2 replies; 13+ messages in thread
From: Stephen Boyd @ 2017-07-11  1:29 UTC (permalink / raw)
  To: Darren Hart, Carlo Caione
  Cc: Enric Balletbo Serra, Michael Turquette, linux-clk,
	Pierre-Louis Bossart, linux, Andy Shevchenko, Carlo Caione

On 07/10, Darren Hart wrote:
> On Tue, Jul 11, 2017 at 12:31:28AM +0200, Enric Balletbo Serra wrote:
> > 2017-07-10 22:38 GMT+02:00 Carlo Caione <carlo@caione.org>:
> > >         pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE;
> > >         spin_lock_init(&pclk->lock);
> > >
> > > +       if (plt_clk_is_enabled(&pclk->hw))
> > > +               init.flags |= CLK_IGNORE_UNUSED;

Can you add a comment to the effect of why we're adding ignore
unused here? That will make it clearer when reading the code a
year from now why we're not turning these clks off by default and
also allow us to recall why it isn't marked as a critical clk.

Probably, we want some sort of handoff mechanism here so that the
clk is left on until a driver comes into the picture and acquires
a handle to this clk?

> > > +
> > >         ret = devm_clk_hw_register(&pdev->dev, &pclk->hw);
> > >         if (ret) {
> > >                 pclk = ERR_PTR(ret);
> > > --
> > > 2.13.2
> > >
> > 
> > It also fixes the issue introduced in commit 282a4e4 ("platform/x86:
> > Enable Atom PMC platform clocks") that causes no audio on Baytrail.
> > 
> > Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> 
> Excellent, thank you Enric. Pierre, any objections to this going in to
> 4.13 now and stable back to 4.12?
> 

You mean v4.11? That's when commit 282a4e4 was released.
Typically we punt these sorts of things to the next release
because it isn't a regression in the release that's being worked
on (i.e. it wasn't introduced in this merge window), but in this
case it seems like a small enough patch plus it's a bother to
keep it out of the release to make it alright to merge now.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] clk: x86: Do not gate clocks enabled by the firmware
  2017-07-11  1:29     ` Stephen Boyd
@ 2017-07-11  1:38       ` Darren Hart
  2017-07-11  8:09       ` Carlo Caione
  1 sibling, 0 replies; 13+ messages in thread
From: Darren Hart @ 2017-07-11  1:38 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Carlo Caione, Enric Balletbo Serra, Michael Turquette, linux-clk,
	Pierre-Louis Bossart, linux, Andy Shevchenko, Carlo Caione

On Mon, Jul 10, 2017 at 06:29:08PM -0700, Stephen Boyd wrote:
> On 07/10, Darren Hart wrote:
> > On Tue, Jul 11, 2017 at 12:31:28AM +0200, Enric Balletbo Serra wrote:
> > > 2017-07-10 22:38 GMT+02:00 Carlo Caione <carlo@caione.org>:
> > > >         pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE;
> > > >         spin_lock_init(&pclk->lock);
> > > >
> > > > +       if (plt_clk_is_enabled(&pclk->hw))
> > > > +               init.flags |= CLK_IGNORE_UNUSED;
> 
> Can you add a comment to the effect of why we're adding ignore
> unused here? That will make it clearer when reading the code a
> year from now why we're not turning these clks off by default and
> also allow us to recall why it isn't marked as a critical clk.

Yes please, good point.

> 
> Probably, we want some sort of handoff mechanism here so that the
> clk is left on until a driver comes into the picture and acquires
> a handle to this clk?

Presumably optionally - it will stay on permanently if nothing claims
it, as in the case where the firmware sets up for the platform, and
nothing else touches it.

> 
> > > > +
> > > >         ret = devm_clk_hw_register(&pdev->dev, &pclk->hw);
> > > >         if (ret) {
> > > >                 pclk = ERR_PTR(ret);
> > > > --
> > > > 2.13.2
> > > >
> > > 
> > > It also fixes the issue introduced in commit 282a4e4 ("platform/x86:
> > > Enable Atom PMC platform clocks") that causes no audio on Baytrail.
> > > 
> > > Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > 
> > Excellent, thank you Enric. Pierre, any objections to this going in to
> > 4.13 now and stable back to 4.12?
> > 
> 
> You mean v4.11? That's when commit 282a4e4 was released.

Yes indeed.

> Typically we punt these sorts of things to the next release
> because it isn't a regression in the release that's being worked
> on (i.e. it wasn't introduced in this merge window), but in this
> case it seems like a small enough patch plus it's a bother to
> keep it out of the release to make it alright to merge now.
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

-- 
Darren Hart
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] clk: x86: Do not gate clocks enabled by the firmware
  2017-07-11  1:29     ` Stephen Boyd
  2017-07-11  1:38       ` Darren Hart
@ 2017-07-11  8:09       ` Carlo Caione
  1 sibling, 0 replies; 13+ messages in thread
From: Carlo Caione @ 2017-07-11  8:09 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Darren Hart, Carlo Caione, Enric Balletbo Serra,
	Michael Turquette, linux-clk, Pierre-Louis Bossart,
	Linux Upstreaming Team, Andy Shevchenko

On Tue, Jul 11, 2017 at 3:29 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 07/10, Darren Hart wrote:
>> On Tue, Jul 11, 2017 at 12:31:28AM +0200, Enric Balletbo Serra wrote:
>> > 2017-07-10 22:38 GMT+02:00 Carlo Caione <carlo@caione.org>:
>> > >         pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE;
>> > >         spin_lock_init(&pclk->lock);
>> > >
>> > > +       if (plt_clk_is_enabled(&pclk->hw))
>> > > +               init.flags |= CLK_IGNORE_UNUSED;
>
> Can you add a comment to the effect of why we're adding ignore
> unused here? That will make it clearer when reading the code a
> year from now why we're not turning these clks off by default and
> also allow us to recall why it isn't marked as a critical clk.

This is actually a good point. Probably here we want to mark the clock
as CLK_IS_CRITICAL instead of CLK_IGNORE_UNUSED.
Michael, any preference here?

-- 
Carlo Caione  |  +39.340.80.30.096  |  Endless

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] clk: x86: Do not gate clocks enabled by the firmware
  2017-07-10 23:54   ` Darren Hart
  2017-07-11  1:29     ` Stephen Boyd
@ 2017-07-24 13:49     ` Pierre-Louis Bossart
  2017-07-24 14:02       ` Andy Shevchenko
  1 sibling, 1 reply; 13+ messages in thread
From: Pierre-Louis Bossart @ 2017-07-24 13:49 UTC (permalink / raw)
  To: Darren Hart, Enric Balletbo Serra
  Cc: Carlo Caione, Michael Turquette, linux-clk, Stephen Boyd, linux,
	Andy Shevchenko, Carlo Caione

On 7/11/17 1:54 AM, Darren Hart wrote:
> On Tue, Jul 11, 2017 at 12:31:28AM +0200, Enric Balletbo Serra wrote:
>> Hi,
>>
>> 2017-07-10 22:38 GMT+02:00 Carlo Caione <carlo@caione.org>:
>>> From: Carlo Caione <carlo@endlessm.com>
>>>
>>> Read the enable register to determine if the clock is already in use by
>>> the firmware. In this case avoid gating the clock.
>>>
>>> Signed-off-by: Carlo Caione <carlo@endlessm.com>
>>> ---
>>>  drivers/clk/x86/clk-pmc-atom.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/clk/x86/clk-pmc-atom.c b/drivers/clk/x86/clk-pmc-atom.c
>>> index 2b60577703ef..46a8b6216fca 100644
>>> --- a/drivers/clk/x86/clk-pmc-atom.c
>>> +++ b/drivers/clk/x86/clk-pmc-atom.c
>>> @@ -185,6 +185,9 @@ static struct clk_plt *plt_clk_register(struct platform_device *pdev, int id,
>>>         pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE;
>>>         spin_lock_init(&pclk->lock);
>>>
>>> +       if (plt_clk_is_enabled(&pclk->hw))
>>> +               init.flags |= CLK_IGNORE_UNUSED;
>>> +
>>>         ret = devm_clk_hw_register(&pdev->dev, &pclk->hw);
>>>         if (ret) {
>>>                 pclk = ERR_PTR(ret);
>>> --
>>> 2.13.2
>>>
>>
>> It also fixes the issue introduced in commit 282a4e4 ("platform/x86:
>> Enable Atom PMC platform clocks") that causes no audio on Baytrail.
>>
>> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>
> Excellent, thank you Enric. Pierre, any objections to this going in to
> 4.13 now and stable back to 4.12?

I saw an update of this patch being merged while I was away on an 
extended summer break, but out of curiosity what was the problem with no 
audio on Baytrail? the code was added precisely to add support for 19.2 
MHz on Baytrail and finally solve audio issues. I have no reports of 
broken functionality with this 282a4e4 commit... Enric, do you have a 
bugzilla or pointers?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] clk: x86: Do not gate clocks enabled by the firmware
  2017-07-24 13:49     ` Pierre-Louis Bossart
@ 2017-07-24 14:02       ` Andy Shevchenko
  2017-07-24 14:07         ` Pierre-Louis Bossart
  0 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2017-07-24 14:02 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Darren Hart, Enric Balletbo Serra
  Cc: Carlo Caione, Michael Turquette, linux-clk, Stephen Boyd, linux,
	Carlo Caione

On Mon, 2017-07-24 at 15:49 +0200, Pierre-Louis Bossart wrote:
> On 7/11/17 1:54 AM, Darren Hart wrote:
> > On Tue, Jul 11, 2017 at 12:31:28AM +0200, Enric Balletbo Serra
> > wrote:

> > > Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > 
> > Excellent, thank you Enric. Pierre, any objections to this going in
> > to
> > 4.13 now and stable back to 4.12?
> 
> I saw an update of this patch being merged while I was away on an 
> extended summer break, but out of curiosity what was the problem with
> no 
> audio on Baytrail? the code was added precisely to add support for
> 19.2 
> MHz on Baytrail and finally solve audio issues. I have no reports of 
> broken functionality with this 282a4e4 commit... Enric, do you have a 
> bugzilla or pointers?

TL;DR: We should respect firmware settings before blindly shut down
"unused" resources. (We used to have similar stuff in GPIO/pinctrl
driver for direct IRQ pins, where we shut up the pins which are used
directly by IOAPIC)

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] clk: x86: Do not gate clocks enabled by the firmware
  2017-07-24 14:02       ` Andy Shevchenko
@ 2017-07-24 14:07         ` Pierre-Louis Bossart
  2017-07-24 15:02           ` Enric Balletbo Serra
  0 siblings, 1 reply; 13+ messages in thread
From: Pierre-Louis Bossart @ 2017-07-24 14:07 UTC (permalink / raw)
  To: Andy Shevchenko, Darren Hart, Enric Balletbo Serra
  Cc: Carlo Caione, Michael Turquette, linux-clk, Stephen Boyd, linux,
	Carlo Caione

On 7/24/17 4:02 PM, Andy Shevchenko wrote:
> On Mon, 2017-07-24 at 15:49 +0200, Pierre-Louis Bossart wrote:
>> On 7/11/17 1:54 AM, Darren Hart wrote:
>>> On Tue, Jul 11, 2017 at 12:31:28AM +0200, Enric Balletbo Serra
>>> wrote:
>
>>>> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>
>>> Excellent, thank you Enric. Pierre, any objections to this going in
>>> to
>>> 4.13 now and stable back to 4.12?
>>
>> I saw an update of this patch being merged while I was away on an
>> extended summer break, but out of curiosity what was the problem with
>> no
>> audio on Baytrail? the code was added precisely to add support for
>> 19.2
>> MHz on Baytrail and finally solve audio issues. I have no reports of
>> broken functionality with this 282a4e4 commit... Enric, do you have a
>> bugzilla or pointers?
>
> TL;DR: We should respect firmware settings before blindly shut down
> "unused" resources. (We used to have similar stuff in GPIO/pinctrl
> driver for direct IRQ pins, where we shut up the pins which are used
> directly by IOAPIC)

I wasn't arguing against those patches, just curious as to which 
platforms were broken by setting pmc_plat_3 blindly in the driver.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] clk: x86: Do not gate clocks enabled by the firmware
  2017-07-24 14:07         ` Pierre-Louis Bossart
@ 2017-07-24 15:02           ` Enric Balletbo Serra
  2017-07-24 16:26             ` Pierre-Louis Bossart
  0 siblings, 1 reply; 13+ messages in thread
From: Enric Balletbo Serra @ 2017-07-24 15:02 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: Andy Shevchenko, Darren Hart, Carlo Caione, Michael Turquette,
	linux-clk, Stephen Boyd, linux, Carlo Caione

2017-07-24 16:07 GMT+02:00 Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com>:
> On 7/24/17 4:02 PM, Andy Shevchenko wrote:
>>
>> On Mon, 2017-07-24 at 15:49 +0200, Pierre-Louis Bossart wrote:
>>>
>>> On 7/11/17 1:54 AM, Darren Hart wrote:
>>>>
>>>> On Tue, Jul 11, 2017 at 12:31:28AM +0200, Enric Balletbo Serra
>>>> wrote:
>>
>>
>>>>> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>>
>>>>
>>>> Excellent, thank you Enric. Pierre, any objections to this going in
>>>> to
>>>> 4.13 now and stable back to 4.12?
>>>
>>>
>>> I saw an update of this patch being merged while I was away on an
>>> extended summer break, but out of curiosity what was the problem with
>>> no
>>> audio on Baytrail? the code was added precisely to add support for
>>> 19.2
>>> MHz on Baytrail and finally solve audio issues. I have no reports of
>>> broken functionality with this 282a4e4 commit... Enric, do you have a
>>> bugzilla or pointers?
>>
>>
>> TL;DR: We should respect firmware settings before blindly shut down
>> "unused" resources. (We used to have similar stuff in GPIO/pinctrl
>> driver for direct IRQ pins, where we shut up the pins which are used
>> directly by IOAPIC)
>
>
> I wasn't arguing against those patches, just curious as to which platforms
> were broken by setting pmc_plat_3 blindly in the driver.
>

After your patch the clock framework disables the required clocks
because the driver does not request then and audio stopped to
work.This is not the solution as is the codec who *must* request these
clocks. We should implement this in the codec driver.. But now, at
least, if firmware enables the clocks audio continues working on
Baytrail.

Regards,
 Enric

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] clk: x86: Do not gate clocks enabled by the firmware
  2017-07-24 15:02           ` Enric Balletbo Serra
@ 2017-07-24 16:26             ` Pierre-Louis Bossart
  2017-07-24 16:33               ` Enric Balletbo Serra
  0 siblings, 1 reply; 13+ messages in thread
From: Pierre-Louis Bossart @ 2017-07-24 16:26 UTC (permalink / raw)
  To: Enric Balletbo Serra
  Cc: Andy Shevchenko, Darren Hart, Carlo Caione, Michael Turquette,
	linux-clk, Stephen Boyd, linux, Carlo Caione

On 7/24/17 5:02 PM, Enric Balletbo Serra wrote:
> 2017-07-24 16:07 GMT+02:00 Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com>:
>> On 7/24/17 4:02 PM, Andy Shevchenko wrote:
>>>
>>> On Mon, 2017-07-24 at 15:49 +0200, Pierre-Louis Bossart wrote:
>>>>
>>>> On 7/11/17 1:54 AM, Darren Hart wrote:
>>>>>
>>>>> On Tue, Jul 11, 2017 at 12:31:28AM +0200, Enric Balletbo Serra
>>>>> wrote:
>>>
>>>
>>>>>> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>>>
>>>>>
>>>>> Excellent, thank you Enric. Pierre, any objections to this going in
>>>>> to
>>>>> 4.13 now and stable back to 4.12?
>>>>
>>>>
>>>> I saw an update of this patch being merged while I was away on an
>>>> extended summer break, but out of curiosity what was the problem with
>>>> no
>>>> audio on Baytrail? the code was added precisely to add support for
>>>> 19.2
>>>> MHz on Baytrail and finally solve audio issues. I have no reports of
>>>> broken functionality with this 282a4e4 commit... Enric, do you have a
>>>> bugzilla or pointers?
>>>
>>>
>>> TL;DR: We should respect firmware settings before blindly shut down
>>> "unused" resources. (We used to have similar stuff in GPIO/pinctrl
>>> driver for direct IRQ pins, where we shut up the pins which are used
>>> directly by IOAPIC)
>>
>>
>> I wasn't arguing against those patches, just curious as to which platforms
>> were broken by setting pmc_plat_3 blindly in the driver.
>>
>
> After your patch the clock framework disables the required clocks
> because the driver does not request then and audio stopped to
> work.This is not the solution as is the codec who *must* request these
> clocks. We should implement this in the codec driver.. But now, at
> least, if firmware enables the clocks audio continues working on
> Baytrail.

Clocks are not enabled/managed in any of the BIOSes I've seen for 
Baytrail, are you talking about a Chromebook by any chance with the 
legacy non-DPCM byt-max98080 driver?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] clk: x86: Do not gate clocks enabled by the firmware
  2017-07-24 16:26             ` Pierre-Louis Bossart
@ 2017-07-24 16:33               ` Enric Balletbo Serra
  2017-07-24 16:44                 ` Pierre-Louis Bossart
  0 siblings, 1 reply; 13+ messages in thread
From: Enric Balletbo Serra @ 2017-07-24 16:33 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: Andy Shevchenko, Darren Hart, Carlo Caione, Michael Turquette,
	linux-clk, Stephen Boyd, linux, Carlo Caione

Hi,

2017-07-24 18:26 GMT+02:00 Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com>:
> On 7/24/17 5:02 PM, Enric Balletbo Serra wrote:
>>
>> 2017-07-24 16:07 GMT+02:00 Pierre-Louis Bossart
>> <pierre-louis.bossart@linux.intel.com>:
>>>
>>> On 7/24/17 4:02 PM, Andy Shevchenko wrote:
>>>>
>>>>
>>>> On Mon, 2017-07-24 at 15:49 +0200, Pierre-Louis Bossart wrote:
>>>>>
>>>>>
>>>>> On 7/11/17 1:54 AM, Darren Hart wrote:
>>>>>>
>>>>>>
>>>>>> On Tue, Jul 11, 2017 at 12:31:28AM +0200, Enric Balletbo Serra
>>>>>> wrote:
>>>>
>>>>
>>>>
>>>>>>> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Excellent, thank you Enric. Pierre, any objections to this going in
>>>>>> to
>>>>>> 4.13 now and stable back to 4.12?
>>>>>
>>>>>
>>>>>
>>>>> I saw an update of this patch being merged while I was away on an
>>>>> extended summer break, but out of curiosity what was the problem with
>>>>> no
>>>>> audio on Baytrail? the code was added precisely to add support for
>>>>> 19.2
>>>>> MHz on Baytrail and finally solve audio issues. I have no reports of
>>>>> broken functionality with this 282a4e4 commit... Enric, do you have a
>>>>> bugzilla or pointers?
>>>>
>>>>
>>>>
>>>> TL;DR: We should respect firmware settings before blindly shut down
>>>> "unused" resources. (We used to have similar stuff in GPIO/pinctrl
>>>> driver for direct IRQ pins, where we shut up the pins which are used
>>>> directly by IOAPIC)
>>>
>>>
>>>
>>> I wasn't arguing against those patches, just curious as to which
>>> platforms
>>> were broken by setting pmc_plat_3 blindly in the driver.
>>>
>>
>> After your patch the clock framework disables the required clocks
>> because the driver does not request then and audio stopped to
>> work.This is not the solution as is the codec who *must* request these
>> clocks. We should implement this in the codec driver.. But now, at
>> least, if firmware enables the clocks audio continues working on
>> Baytrail.
>
>
> Clocks are not enabled/managed in any of the BIOSes I've seen for Baytrail,
> are you talking about a Chromebook by any chance with the legacy non-DPCM
> byt-max98080 driver?
>

Yes the issue is with a Chromebook with the legacy byt-max98080 driver.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] clk: x86: Do not gate clocks enabled by the firmware
  2017-07-24 16:33               ` Enric Balletbo Serra
@ 2017-07-24 16:44                 ` Pierre-Louis Bossart
  0 siblings, 0 replies; 13+ messages in thread
From: Pierre-Louis Bossart @ 2017-07-24 16:44 UTC (permalink / raw)
  To: Enric Balletbo Serra
  Cc: Andy Shevchenko, Darren Hart, Carlo Caione, Michael Turquette,
	linux-clk, Stephen Boyd, linux, Carlo Caione

On 7/24/17 6:33 PM, Enric Balletbo Serra wrote:
> Hi,
>
> 2017-07-24 18:26 GMT+02:00 Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com>:
>> On 7/24/17 5:02 PM, Enric Balletbo Serra wrote:
>>>
>>> 2017-07-24 16:07 GMT+02:00 Pierre-Louis Bossart
>>> <pierre-louis.bossart@linux.intel.com>:
>>>>
>>>> On 7/24/17 4:02 PM, Andy Shevchenko wrote:
>>>>>
>>>>>
>>>>> On Mon, 2017-07-24 at 15:49 +0200, Pierre-Louis Bossart wrote:
>>>>>>
>>>>>>
>>>>>> On 7/11/17 1:54 AM, Darren Hart wrote:
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Jul 11, 2017 at 12:31:28AM +0200, Enric Balletbo Serra
>>>>>>> wrote:
>>>>>
>>>>>
>>>>>
>>>>>>>> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Excellent, thank you Enric. Pierre, any objections to this going in
>>>>>>> to
>>>>>>> 4.13 now and stable back to 4.12?
>>>>>>
>>>>>>
>>>>>>
>>>>>> I saw an update of this patch being merged while I was away on an
>>>>>> extended summer break, but out of curiosity what was the problem with
>>>>>> no
>>>>>> audio on Baytrail? the code was added precisely to add support for
>>>>>> 19.2
>>>>>> MHz on Baytrail and finally solve audio issues. I have no reports of
>>>>>> broken functionality with this 282a4e4 commit... Enric, do you have a
>>>>>> bugzilla or pointers?
>>>>>
>>>>>
>>>>>
>>>>> TL;DR: We should respect firmware settings before blindly shut down
>>>>> "unused" resources. (We used to have similar stuff in GPIO/pinctrl
>>>>> driver for direct IRQ pins, where we shut up the pins which are used
>>>>> directly by IOAPIC)
>>>>
>>>>
>>>>
>>>> I wasn't arguing against those patches, just curious as to which
>>>> platforms
>>>> were broken by setting pmc_plat_3 blindly in the driver.
>>>>
>>>
>>> After your patch the clock framework disables the required clocks
>>> because the driver does not request then and audio stopped to
>>> work.This is not the solution as is the codec who *must* request these
>>> clocks. We should implement this in the codec driver.. But now, at
>>> least, if firmware enables the clocks audio continues working on
>>> Baytrail.
>>
>>
>> Clocks are not enabled/managed in any of the BIOSes I've seen for Baytrail,
>> are you talking about a Chromebook by any chance with the legacy non-DPCM
>> byt-max98080 driver?
>>
>
> Yes the issue is with a Chromebook with the legacy byt-max98080 driver.

ah, yes I never tested on those (couldn't make the device boot).
We may be able to do more simplifications, in machine drivers I test for 
Baytrail vs. Cherrytrail to manage the clocks at the kernel level, but 
maybe I should just look at what the firmware does and use the kernel 
clock management as a fallback.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-07-24 16:44 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-10 20:38 [PATCH] clk: x86: Do not gate clocks enabled by the firmware Carlo Caione
2017-07-10 22:31 ` Enric Balletbo Serra
2017-07-10 23:54   ` Darren Hart
2017-07-11  1:29     ` Stephen Boyd
2017-07-11  1:38       ` Darren Hart
2017-07-11  8:09       ` Carlo Caione
2017-07-24 13:49     ` Pierre-Louis Bossart
2017-07-24 14:02       ` Andy Shevchenko
2017-07-24 14:07         ` Pierre-Louis Bossart
2017-07-24 15:02           ` Enric Balletbo Serra
2017-07-24 16:26             ` Pierre-Louis Bossart
2017-07-24 16:33               ` Enric Balletbo Serra
2017-07-24 16:44                 ` Pierre-Louis Bossart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.