diff mbox series

[V2] clk: tegra: Fix Tegra PWM parent clock

Message ID 20221007084524.10712-1-jonathanh@nvidia.com (mailing list archive)
State Superseded, archived
Headers show
Series [V2] clk: tegra: Fix Tegra PWM parent clock | expand

Commit Message

Jon Hunter Oct. 7, 2022, 8:45 a.m. UTC
Commit 8c193f4714df ("pwm: tegra: Optimize period calculation") updated
the period calculation in the Tegra PWM driver and now returns an error
if the period requested is less than minimum period supported. This is
breaking PWM support on various Tegra platforms. For example, on the
Tegra210 Jetson Nano platform this is breaking the PWM fan support and
probing the PWM fan driver now fails ...

 pwm-fan pwm-fan: Failed to configure PWM: -22
 pwm-fan: probe of pwm-fan failed with error -22

The problem is that the default parent clock for the PWM on Tegra210 is
a 32kHz clock and is unable to support the requested PWM period.

Fix PWM support on Tegra20, Tegra30, Tegra124 and Tegra210 by updating
the parent clock for the PWM to be the PLL_P.

Fixes: 8c193f4714df ("pwm: tegra: Optimize period calculation")
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---

I have tested this on Tegra210 and boot tested on the Tegra20/30/124
platforms but please confirm if this fixes all the issues seen on these
platforms.

 drivers/clk/tegra/clk-tegra124.c | 1 +
 drivers/clk/tegra/clk-tegra20.c  | 1 +
 drivers/clk/tegra/clk-tegra210.c | 1 +
 drivers/clk/tegra/clk-tegra30.c  | 1 +
 4 files changed, 4 insertions(+)

Comments

Thierry Reding Oct. 7, 2022, 10:21 a.m. UTC | #1
On Fri, Oct 07, 2022 at 09:45:24AM +0100, Jon Hunter wrote:
> Commit 8c193f4714df ("pwm: tegra: Optimize period calculation") updated
> the period calculation in the Tegra PWM driver and now returns an error
> if the period requested is less than minimum period supported. This is
> breaking PWM support on various Tegra platforms. For example, on the
> Tegra210 Jetson Nano platform this is breaking the PWM fan support and
> probing the PWM fan driver now fails ...
> 
>  pwm-fan pwm-fan: Failed to configure PWM: -22
>  pwm-fan: probe of pwm-fan failed with error -22
> 
> The problem is that the default parent clock for the PWM on Tegra210 is
> a 32kHz clock and is unable to support the requested PWM period.
> 
> Fix PWM support on Tegra20, Tegra30, Tegra124 and Tegra210 by updating
> the parent clock for the PWM to be the PLL_P.
> 
> Fixes: 8c193f4714df ("pwm: tegra: Optimize period calculation")
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> 
> I have tested this on Tegra210 and boot tested on the Tegra20/30/124
> platforms but please confirm if this fixes all the issues seen on these
> platforms.
> 
>  drivers/clk/tegra/clk-tegra124.c | 1 +
>  drivers/clk/tegra/clk-tegra20.c  | 1 +
>  drivers/clk/tegra/clk-tegra210.c | 1 +
>  drivers/clk/tegra/clk-tegra30.c  | 1 +
>  4 files changed, 4 insertions(+)

Thomas, can you check this on Nyan where you were observing display to
be completely broken? In case you don't have this in your mailbox, see
here:

	http://patchwork.ozlabs.org/project/linux-tegra/patch/20221007084524.10712-1-jonathanh@nvidia.com/

Thierry

> 
> diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
> index a9d4efcef2d4..6c46592d794e 100644
> --- a/drivers/clk/tegra/clk-tegra124.c
> +++ b/drivers/clk/tegra/clk-tegra124.c
> @@ -1330,6 +1330,7 @@ static struct tegra_clk_init_table common_init_table[] __initdata = {
>  	{ TEGRA124_CLK_I2S3_SYNC, TEGRA124_CLK_CLK_MAX, 24576000, 0 },
>  	{ TEGRA124_CLK_I2S4_SYNC, TEGRA124_CLK_CLK_MAX, 24576000, 0 },
>  	{ TEGRA124_CLK_VIMCLK_SYNC, TEGRA124_CLK_CLK_MAX, 24576000, 0 },
> +	{ TEGRA124_CLK_PWM, TEGRA124_CLK_PLL_P, 408000000, 0 },
>  	/* must be the last entry */
>  	{ TEGRA124_CLK_CLK_MAX, TEGRA124_CLK_CLK_MAX, 0, 0 },
>  };
> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
> index 8a4514f6d503..422d78247553 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -1044,6 +1044,7 @@ static struct tegra_clk_init_table init_table[] = {
>  	{ TEGRA20_CLK_GR2D, TEGRA20_CLK_PLL_C, 300000000, 0 },
>  	{ TEGRA20_CLK_GR3D, TEGRA20_CLK_PLL_C, 300000000, 0 },
>  	{ TEGRA20_CLK_VDE, TEGRA20_CLK_PLL_C, 300000000, 0 },
> +	{ TEGRA20_CLK_PWM, TEGRA20_CLK_PLL_P, 48000000, 0 },
>  	/* must be the last entry */
>  	{ TEGRA20_CLK_CLK_MAX, TEGRA20_CLK_CLK_MAX, 0, 0 },
>  };
> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
> index 499f999e91e1..a3488aaac3f7 100644
> --- a/drivers/clk/tegra/clk-tegra210.c
> +++ b/drivers/clk/tegra/clk-tegra210.c
> @@ -3597,6 +3597,7 @@ static struct tegra_clk_init_table init_table[] __initdata = {
>  	{ TEGRA210_CLK_VIMCLK_SYNC, TEGRA210_CLK_CLK_MAX, 24576000, 0 },
>  	{ TEGRA210_CLK_HDA, TEGRA210_CLK_PLL_P, 51000000, 0 },
>  	{ TEGRA210_CLK_HDA2CODEC_2X, TEGRA210_CLK_PLL_P, 48000000, 0 },
> +	{ TEGRA210_CLK_PWM, TEGRA210_CLK_PLL_P, 48000000, 0 },
>  	/* This MUST be the last entry. */
>  	{ TEGRA210_CLK_CLK_MAX, TEGRA210_CLK_CLK_MAX, 0, 0 },
>  };
> diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
> index 168c07d5a5f2..60f1534711f1 100644
> --- a/drivers/clk/tegra/clk-tegra30.c
> +++ b/drivers/clk/tegra/clk-tegra30.c
> @@ -1237,6 +1237,7 @@ static struct tegra_clk_init_table init_table[] = {
>  	{ TEGRA30_CLK_VIMCLK_SYNC, TEGRA30_CLK_CLK_MAX, 24000000, 0 },
>  	{ TEGRA30_CLK_HDA, TEGRA30_CLK_PLL_P, 102000000, 0 },
>  	{ TEGRA30_CLK_HDA2CODEC_2X, TEGRA30_CLK_PLL_P, 48000000, 0 },
> +	{ TEGRA30_CLK_PWM, TEGRA30_CLK_PLL_P, 48000000, 0 },
>  	/* must be the last entry */
>  	{ TEGRA30_CLK_CLK_MAX, TEGRA30_CLK_CLK_MAX, 0, 0 },
>  };
> -- 
> 2.25.1
>
Thomas Graichen Oct. 9, 2022, 6:50 a.m. UTC | #2
On Fri, Oct 7, 2022 at 12:21 PM Thierry Reding <thierry.reding@gmail.com> wrote:
>
> On Fri, Oct 07, 2022 at 09:45:24AM +0100, Jon Hunter wrote:
> > Commit 8c193f4714df ("pwm: tegra: Optimize period calculation") updated
> > the period calculation in the Tegra PWM driver and now returns an error
> > if the period requested is less than minimum period supported. This is
> > breaking PWM support on various Tegra platforms. For example, on the
> > Tegra210 Jetson Nano platform this is breaking the PWM fan support and
> > probing the PWM fan driver now fails ...
> >
> >  pwm-fan pwm-fan: Failed to configure PWM: -22
> >  pwm-fan: probe of pwm-fan failed with error -22
> >
> > The problem is that the default parent clock for the PWM on Tegra210 is
> > a 32kHz clock and is unable to support the requested PWM period.
> >
> > Fix PWM support on Tegra20, Tegra30, Tegra124 and Tegra210 by updating
> > the parent clock for the PWM to be the PLL_P.
> >
> > Fixes: 8c193f4714df ("pwm: tegra: Optimize period calculation")
> > Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> > ---
> >
> > I have tested this on Tegra210 and boot tested on the Tegra20/30/124
> > platforms but please confirm if this fixes all the issues seen on these
> > platforms.
> >
> >  drivers/clk/tegra/clk-tegra124.c | 1 +
> >  drivers/clk/tegra/clk-tegra20.c  | 1 +
> >  drivers/clk/tegra/clk-tegra210.c | 1 +
> >  drivers/clk/tegra/clk-tegra30.c  | 1 +
> >  4 files changed, 4 insertions(+)
>
> Thomas, can you check this on Nyan where you were observing display to
> be completely broken? In case you don't have this in your mailbox, see
> here:
>
>         http://patchwork.ozlabs.org/project/linux-tegra/patch/20221007084524.10712-1-jonathanh@nvidia.com/
>
> Thierry

hi thierry,

i can confirm that this also fixes the problem on the tegra124 nyan chromebook.
backlight is working fine and can also be adjusted properly with this
patch applied
(and my revert of the breaking pwm optimization commit removed beforehand)

thanks a lot ... thus

Tested-by: Thomas Graichen <thomas.graichen@gmail.com>

best wishes - thomas

> > diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
> > index a9d4efcef2d4..6c46592d794e 100644
> > --- a/drivers/clk/tegra/clk-tegra124.c
> > +++ b/drivers/clk/tegra/clk-tegra124.c
> > @@ -1330,6 +1330,7 @@ static struct tegra_clk_init_table common_init_table[] __initdata = {
> >       { TEGRA124_CLK_I2S3_SYNC, TEGRA124_CLK_CLK_MAX, 24576000, 0 },
> >       { TEGRA124_CLK_I2S4_SYNC, TEGRA124_CLK_CLK_MAX, 24576000, 0 },
> >       { TEGRA124_CLK_VIMCLK_SYNC, TEGRA124_CLK_CLK_MAX, 24576000, 0 },
> > +     { TEGRA124_CLK_PWM, TEGRA124_CLK_PLL_P, 408000000, 0 },
> >       /* must be the last entry */
> >       { TEGRA124_CLK_CLK_MAX, TEGRA124_CLK_CLK_MAX, 0, 0 },
> >  };
> > diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
> > index 8a4514f6d503..422d78247553 100644
> > --- a/drivers/clk/tegra/clk-tegra20.c
> > +++ b/drivers/clk/tegra/clk-tegra20.c
> > @@ -1044,6 +1044,7 @@ static struct tegra_clk_init_table init_table[] = {
> >       { TEGRA20_CLK_GR2D, TEGRA20_CLK_PLL_C, 300000000, 0 },
> >       { TEGRA20_CLK_GR3D, TEGRA20_CLK_PLL_C, 300000000, 0 },
> >       { TEGRA20_CLK_VDE, TEGRA20_CLK_PLL_C, 300000000, 0 },
> > +     { TEGRA20_CLK_PWM, TEGRA20_CLK_PLL_P, 48000000, 0 },
> >       /* must be the last entry */
> >       { TEGRA20_CLK_CLK_MAX, TEGRA20_CLK_CLK_MAX, 0, 0 },
> >  };
> > diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
> > index 499f999e91e1..a3488aaac3f7 100644
> > --- a/drivers/clk/tegra/clk-tegra210.c
> > +++ b/drivers/clk/tegra/clk-tegra210.c
> > @@ -3597,6 +3597,7 @@ static struct tegra_clk_init_table init_table[] __initdata = {
> >       { TEGRA210_CLK_VIMCLK_SYNC, TEGRA210_CLK_CLK_MAX, 24576000, 0 },
> >       { TEGRA210_CLK_HDA, TEGRA210_CLK_PLL_P, 51000000, 0 },
> >       { TEGRA210_CLK_HDA2CODEC_2X, TEGRA210_CLK_PLL_P, 48000000, 0 },
> > +     { TEGRA210_CLK_PWM, TEGRA210_CLK_PLL_P, 48000000, 0 },
> >       /* This MUST be the last entry. */
> >       { TEGRA210_CLK_CLK_MAX, TEGRA210_CLK_CLK_MAX, 0, 0 },
> >  };
> > diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
> > index 168c07d5a5f2..60f1534711f1 100644
> > --- a/drivers/clk/tegra/clk-tegra30.c
> > +++ b/drivers/clk/tegra/clk-tegra30.c
> > @@ -1237,6 +1237,7 @@ static struct tegra_clk_init_table init_table[] = {
> >       { TEGRA30_CLK_VIMCLK_SYNC, TEGRA30_CLK_CLK_MAX, 24000000, 0 },
> >       { TEGRA30_CLK_HDA, TEGRA30_CLK_PLL_P, 102000000, 0 },
> >       { TEGRA30_CLK_HDA2CODEC_2X, TEGRA30_CLK_PLL_P, 48000000, 0 },
> > +     { TEGRA30_CLK_PWM, TEGRA30_CLK_PLL_P, 48000000, 0 },
> >       /* must be the last entry */
> >       { TEGRA30_CLK_CLK_MAX, TEGRA30_CLK_CLK_MAX, 0, 0 },
> >  };
> > --
> > 2.25.1
> >
Uwe Kleine-König Oct. 9, 2022, 8:18 a.m. UTC | #3
Hello,

On Fri, Oct 07, 2022 at 09:45:24AM +0100, Jon Hunter wrote:
> Commit 8c193f4714df ("pwm: tegra: Optimize period calculation") updated
> the period calculation in the Tegra PWM driver and now returns an error
> if the period requested is less than minimum period supported. This is
> breaking PWM support on various Tegra platforms. For example, on the
> Tegra210 Jetson Nano platform this is breaking the PWM fan support and
> probing the PWM fan driver now fails ...
> 
>  pwm-fan pwm-fan: Failed to configure PWM: -22
>  pwm-fan: probe of pwm-fan failed with error -22
> 
> The problem is that the default parent clock for the PWM on Tegra210 is
> a 32kHz clock and is unable to support the requested PWM period.
> 
> Fix PWM support on Tegra20, Tegra30, Tegra124 and Tegra210 by updating
> the parent clock for the PWM to be the PLL_P.
> 
> Fixes: 8c193f4714df ("pwm: tegra: Optimize period calculation")
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>

First of all, thanks for your fix. Very appreciated.

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Who will take care for this patch? I assume it will be Stephen and
Michael (i.e. the clk maintainers)? Do you consider this patch fine for
application before the next merge window, or do you expect that it might
break other use cases and so should cook in next for some time?

Best regards
Uwe
Jon Hunter Oct. 10, 2022, 10:03 a.m. UTC | #4
On 09/10/2022 09:18, Uwe Kleine-König wrote:
> Hello,
> 
> On Fri, Oct 07, 2022 at 09:45:24AM +0100, Jon Hunter wrote:
>> Commit 8c193f4714df ("pwm: tegra: Optimize period calculation") updated
>> the period calculation in the Tegra PWM driver and now returns an error
>> if the period requested is less than minimum period supported. This is
>> breaking PWM support on various Tegra platforms. For example, on the
>> Tegra210 Jetson Nano platform this is breaking the PWM fan support and
>> probing the PWM fan driver now fails ...
>>
>>   pwm-fan pwm-fan: Failed to configure PWM: -22
>>   pwm-fan: probe of pwm-fan failed with error -22
>>
>> The problem is that the default parent clock for the PWM on Tegra210 is
>> a 32kHz clock and is unable to support the requested PWM period.
>>
>> Fix PWM support on Tegra20, Tegra30, Tegra124 and Tegra210 by updating
>> the parent clock for the PWM to be the PLL_P.
>>
>> Fixes: 8c193f4714df ("pwm: tegra: Optimize period calculation")
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> 
> First of all, thanks for your fix. Very appreciated.

No problem.

> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks. I have just sent a V3 because I was missing a fix for Tegra114. 
Can you ACK that one as well?

> Who will take care for this patch? I assume it will be Stephen and
> Michael (i.e. the clk maintainers)? Do you consider this patch fine for
> application before the next merge window, or do you expect that it might
> break other use cases and so should cook in next for some time?

We should pick this up as a fix for v6.1. I also plan to request this is 
pulled into stable as well (all going well).

Cheers
Jon
diff mbox series

Patch

diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
index a9d4efcef2d4..6c46592d794e 100644
--- a/drivers/clk/tegra/clk-tegra124.c
+++ b/drivers/clk/tegra/clk-tegra124.c
@@ -1330,6 +1330,7 @@  static struct tegra_clk_init_table common_init_table[] __initdata = {
 	{ TEGRA124_CLK_I2S3_SYNC, TEGRA124_CLK_CLK_MAX, 24576000, 0 },
 	{ TEGRA124_CLK_I2S4_SYNC, TEGRA124_CLK_CLK_MAX, 24576000, 0 },
 	{ TEGRA124_CLK_VIMCLK_SYNC, TEGRA124_CLK_CLK_MAX, 24576000, 0 },
+	{ TEGRA124_CLK_PWM, TEGRA124_CLK_PLL_P, 408000000, 0 },
 	/* must be the last entry */
 	{ TEGRA124_CLK_CLK_MAX, TEGRA124_CLK_CLK_MAX, 0, 0 },
 };
diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 8a4514f6d503..422d78247553 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -1044,6 +1044,7 @@  static struct tegra_clk_init_table init_table[] = {
 	{ TEGRA20_CLK_GR2D, TEGRA20_CLK_PLL_C, 300000000, 0 },
 	{ TEGRA20_CLK_GR3D, TEGRA20_CLK_PLL_C, 300000000, 0 },
 	{ TEGRA20_CLK_VDE, TEGRA20_CLK_PLL_C, 300000000, 0 },
+	{ TEGRA20_CLK_PWM, TEGRA20_CLK_PLL_P, 48000000, 0 },
 	/* must be the last entry */
 	{ TEGRA20_CLK_CLK_MAX, TEGRA20_CLK_CLK_MAX, 0, 0 },
 };
diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index 499f999e91e1..a3488aaac3f7 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -3597,6 +3597,7 @@  static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA210_CLK_VIMCLK_SYNC, TEGRA210_CLK_CLK_MAX, 24576000, 0 },
 	{ TEGRA210_CLK_HDA, TEGRA210_CLK_PLL_P, 51000000, 0 },
 	{ TEGRA210_CLK_HDA2CODEC_2X, TEGRA210_CLK_PLL_P, 48000000, 0 },
+	{ TEGRA210_CLK_PWM, TEGRA210_CLK_PLL_P, 48000000, 0 },
 	/* This MUST be the last entry. */
 	{ TEGRA210_CLK_CLK_MAX, TEGRA210_CLK_CLK_MAX, 0, 0 },
 };
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index 168c07d5a5f2..60f1534711f1 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1237,6 +1237,7 @@  static struct tegra_clk_init_table init_table[] = {
 	{ TEGRA30_CLK_VIMCLK_SYNC, TEGRA30_CLK_CLK_MAX, 24000000, 0 },
 	{ TEGRA30_CLK_HDA, TEGRA30_CLK_PLL_P, 102000000, 0 },
 	{ TEGRA30_CLK_HDA2CODEC_2X, TEGRA30_CLK_PLL_P, 48000000, 0 },
+	{ TEGRA30_CLK_PWM, TEGRA30_CLK_PLL_P, 48000000, 0 },
 	/* must be the last entry */
 	{ TEGRA30_CLK_CLK_MAX, TEGRA30_CLK_CLK_MAX, 0, 0 },
 };