[PATCH 1/3] spi: sh-hspi: Add missing call to pm_runtime_disable() in failure path

[PATCH 1/3] spi: sh-hspi: Add missing call to pm_runtime_disable() in failure path

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 drivers/spi/spi-sh-hspi.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-sh-hspi.c b/drivers/spi/spi-sh-hspi.c
index 3b170b9f3471..9009456bdf4d 100644
--- a/drivers/spi/spi-sh-hspi.c
+++ b/drivers/spi/spi-sh-hspi.c
@@ -278,11 +278,13 @@ static int hspi_probe(struct platform_device *pdev)
 	ret = devm_spi_register_master(&pdev->dev, master);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "spi_register_master error.\n");
-		goto error1;
+		goto error2;
 	}
 
 	return 0;
 
+ error2:
+	pm_runtime_disable(&pdev->dev);
  error1:
 	clk_put(clk);
  error0:
-- 
1.7.9.5

[PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
---
 drivers/spi/spi-sh-msiof.c |   25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 739eb2f12ecc..e850d03e7190 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -448,9 +448,6 @@ static int sh_msiof_prepare_message(struct spi_master *master,
 	struct sh_msiof_spi_priv *p = spi_master_get_devdata(master);
 	const struct spi_device *spi = msg->spi;
 
-	pm_runtime_get_sync(&p->pdev->dev);
-	clk_enable(p->clk);
-
 	/* Configure pins before asserting CS */
 	sh_msiof_spi_set_pin_regs(p, !!(spi->mode & SPI_CPOL),
 				  !!(spi->mode & SPI_CPHA),
@@ -460,16 +457,6 @@ static int sh_msiof_prepare_message(struct spi_master *master,
 	return 0;
 }
 
-static int sh_msiof_unprepare_message(struct spi_master *master,
-				      struct spi_message *msg)
-{
-	struct sh_msiof_spi_priv *p = spi_master_get_devdata(master);
-
-	clk_disable(p->clk);
-	pm_runtime_put(&p->pdev->dev);
-	return 0;
-}
-
 static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
 				  void (*tx_fifo)(struct sh_msiof_spi_priv *,
 						  const void *, int, int),
@@ -742,12 +729,6 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 		goto err1;
 	}
 
-	ret = clk_prepare(p->clk);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "unable to prepare clock\n");
-		goto err1;
-	}
-
 	p->pdev = pdev;
 	pm_runtime_enable(&pdev->dev);
 
@@ -768,8 +749,8 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 	master->num_chipselect = p->info->num_chipselect;
 	master->setup = sh_msiof_spi_setup;
 	master->prepare_message = sh_msiof_prepare_message;
-	master->unprepare_message = sh_msiof_unprepare_message;
 	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(8, 32);
+	master->auto_runtime_pm = true;
 	master->transfer_one = sh_msiof_transfer_one;
 
 	ret = devm_spi_register_master(&pdev->dev, master);
@@ -782,7 +763,6 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 
  err2:
 	pm_runtime_disable(&pdev->dev);
-	clk_unprepare(p->clk);
  err1:
 	spi_master_put(master);
 	return ret;
@@ -790,10 +770,7 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)
 
 static int sh_msiof_spi_remove(struct platform_device *pdev)
 {
-	struct sh_msiof_spi_priv *p = platform_get_drvdata(pdev);
-
 	pm_runtime_disable(&pdev->dev);
-	clk_unprepare(p->clk);
 	return 0;
 }
 
-- 
1.7.9.5

[PATCH 3/3] spi: rspi: Add runtime PM support, using spi core auto_runtime_pm

[Geert Uytterhoeven]

From: Geert Uytterhoeven <geert+renesas-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>

Signed-off-by: Geert Uytterhoeven <geert+renesas-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
---
 drivers/spi/spi-rspi.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
index 92bec7e91046..1fb0ad213324 100644
--- a/drivers/spi/spi-rspi.c
+++ b/drivers/spi/spi-rspi.c
@@ -33,6 +33,7 @@
 #include <linux/dmaengine.h>
 #include <linux/dma-mapping.h>
 #include <linux/of_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/sh_dma.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/rspi.h>
@@ -1111,7 +1112,7 @@ static int rspi_remove(struct platform_device *pdev)
 	struct rspi_data *rspi = platform_get_drvdata(pdev);
 
 	rspi_release_dma(rspi);
-	clk_disable_unprepare(rspi->clk);
+	pm_runtime_disable(&pdev->dev);
 
 	return 0;
 }
@@ -1242,16 +1243,13 @@ static int rspi_probe(struct platform_device *pdev)
 		goto error1;
 	}
 
-	ret = clk_prepare_enable(rspi->clk);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "unable to prepare/enable clock\n");
-		goto error1;
-	}
+	pm_runtime_enable(&pdev->dev);
 
 	init_waitqueue_head(&rspi->wait);
 
 	master->bus_num = pdev->id;
 	master->setup = rspi_setup;
+	master->auto_runtime_pm = true;
 	master->transfer_one = ops->transfer_one;
 	master->prepare_message = rspi_prepare_message;
 	master->unprepare_message = rspi_unprepare_message;
@@ -1312,7 +1310,7 @@ static int rspi_probe(struct platform_device *pdev)
 error3:
 	rspi_release_dma(rspi);
 error2:
-	clk_disable_unprepare(rspi->clk);
+	pm_runtime_disable(&pdev->dev);
 error1:
 	spi_master_put(master);
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 169 bytes --]

On Tue, Mar 11, 2014 at 10:59:11AM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] spi: rspi: Add runtime PM support, using spi core auto_runtime_pm

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

On Tue, Mar 11, 2014 at 10:59:12AM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Applied, thanks, though...

> -	ret = clk_prepare_enable(rspi->clk);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "unable to prepare/enable clock\n");
> -		goto error1;
> -	}
> +	pm_runtime_enable(&pdev->dev);

...due to the runtime PM API being configurable you're supposed to start
off with the device runtime enabled (this applies to some of the other
patches too).  I'm not sure that's terribly realistic for these drivers
though.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] spi: rspi: Add runtime PM support, using spi core auto_runtime_pm

[Geert Uytterhoeven]

Hi Mark,

On Tue, Mar 11, 2014 at 11:47 AM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Mar 11, 2014 at 10:59:12AM +0100, Geert Uytterhoeven wrote:
>> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>
> Applied, thanks, though...
>
>> -     ret = clk_prepare_enable(rspi->clk);
>> -     if (ret < 0) {
>> -             dev_err(&pdev->dev, "unable to prepare/enable clock\n");
>> -             goto error1;
>> -     }
>> +     pm_runtime_enable(&pdev->dev);
>
> ...due to the runtime PM API being configurable you're supposed to start
> off with the device runtime enabled (this applies to some of the other
> patches too).  I'm not sure that's terribly realistic for these drivers
> though.

Can you please elaborate what should be fixed?

If I disable CONFIG_PM_RUNTIME, the kernel prints:

    Runtime PM disabled, clock forced on.

and the clock is enabled all the time (verified by looking at the clock
registers)?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/3] spi: rspi: Add runtime PM support, using spi core auto_runtime_pm

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 430 bytes --]

On Tue, Mar 11, 2014 at 02:10:31PM +0100, Geert Uytterhoeven wrote:

> Can you please elaborate what should be fixed?

> If I disable CONFIG_PM_RUNTIME, the kernel prints:

>     Runtime PM disabled, clock forced on.

> and the clock is enabled all the time (verified by looking at the clock
> registers)?

That's very SH specific and doesn't apply in the general case (I would
not be surprised if future SH updates broke it...).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] spi: rspi: Add runtime PM support, using spi core auto_runtime_pm

[Geert Uytterhoeven]

Hi Mark,

On Tue, Mar 11, 2014 at 2:26 PM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Mar 11, 2014 at 02:10:31PM +0100, Geert Uytterhoeven wrote:
>
>> Can you please elaborate what should be fixed?
>
>> If I disable CONFIG_PM_RUNTIME, the kernel prints:
>
>>     Runtime PM disabled, clock forced on.
>
>> and the clock is enabled all the time (verified by looking at the clock
>> registers)?
>
> That's very SH specific and doesn't apply in the general case (I would
> not be surprised if future SH updates broke it...).

Note that this is from drivers/base/power/clock_ops.c

So what should I do instead?

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/3] spi: rspi: Add runtime PM support, using spi core auto_runtime_pm

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 842 bytes --]

On Tue, Mar 11, 2014 at 02:35:15PM +0100, Geert Uytterhoeven wrote:
> On Tue, Mar 11, 2014 at 2:26 PM, Mark Brown <broonie@kernel.org> wrote:
> > On Tue, Mar 11, 2014 at 02:10:31PM +0100, Geert Uytterhoeven wrote:

> >> Can you please elaborate what should be fixed?

> >> If I disable CONFIG_PM_RUNTIME, the kernel prints:

> >>     Runtime PM disabled, clock forced on.

> >> and the clock is enabled all the time (verified by looking at the clock
> >> registers)?

> > That's very SH specific and doesn't apply in the general case (I would
> > not be surprised if future SH updates broke it...).

> Note that this is from drivers/base/power/clock_ops.c

> So what should I do instead?

Oh, is this manipulating the clock managed by the power domains?  If
that's the case don't worry about it, it's fine.  I'd thought it was
another clock.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

Does this require drivers/sh/pm_runtime.c to be compiled in ?

On Tuesday 11 March 2014 10:59:11 Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> Cc: Magnus Damm <magnus.damm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/spi/spi-sh-msiof.c |   25 +------------------------
>  1 file changed, 1 insertion(+), 24 deletions(-)
> 
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index 739eb2f12ecc..e850d03e7190 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -448,9 +448,6 @@ static int sh_msiof_prepare_message(struct spi_master
> *master, struct sh_msiof_spi_priv *p = spi_master_get_devdata(master);
>  	const struct spi_device *spi = msg->spi;
> 
> -	pm_runtime_get_sync(&p->pdev->dev);
> -	clk_enable(p->clk);
> -
>  	/* Configure pins before asserting CS */
>  	sh_msiof_spi_set_pin_regs(p, !!(spi->mode & SPI_CPOL),
>  				  !!(spi->mode & SPI_CPHA),
> @@ -460,16 +457,6 @@ static int sh_msiof_prepare_message(struct spi_master
> *master, return 0;
>  }
> 
> -static int sh_msiof_unprepare_message(struct spi_master *master,
> -				      struct spi_message *msg)
> -{
> -	struct sh_msiof_spi_priv *p = spi_master_get_devdata(master);
> -
> -	clk_disable(p->clk);
> -	pm_runtime_put(&p->pdev->dev);
> -	return 0;
> -}
> -
>  static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
>  				  void (*tx_fifo)(struct sh_msiof_spi_priv *,
>  						  const void *, int, int),
> @@ -742,12 +729,6 @@ static int sh_msiof_spi_probe(struct platform_device
> *pdev) goto err1;
>  	}
> 
> -	ret = clk_prepare(p->clk);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "unable to prepare clock\n");
> -		goto err1;
> -	}
> -
>  	p->pdev = pdev;
>  	pm_runtime_enable(&pdev->dev);
> 
> @@ -768,8 +749,8 @@ static int sh_msiof_spi_probe(struct platform_device
> *pdev) master->num_chipselect = p->info->num_chipselect;
>  	master->setup = sh_msiof_spi_setup;
>  	master->prepare_message = sh_msiof_prepare_message;
> -	master->unprepare_message = sh_msiof_unprepare_message;
>  	master->bits_per_word_mask = SPI_BPW_RANGE_MASK(8, 32);
> +	master->auto_runtime_pm = true;
>  	master->transfer_one = sh_msiof_transfer_one;
> 
>  	ret = devm_spi_register_master(&pdev->dev, master);
> @@ -782,7 +763,6 @@ static int sh_msiof_spi_probe(struct platform_device
> *pdev)
> 
>   err2:
>  	pm_runtime_disable(&pdev->dev);
> -	clk_unprepare(p->clk);
>   err1:
>  	spi_master_put(master);
>  	return ret;
> @@ -790,10 +770,7 @@ static int sh_msiof_spi_probe(struct platform_device
> *pdev)
> 
>  static int sh_msiof_spi_remove(struct platform_device *pdev)
>  {
> -	struct sh_msiof_spi_priv *p = platform_get_drvdata(pdev);
> -
>  	pm_runtime_disable(&pdev->dev);
> -	clk_unprepare(p->clk);
>  	return 0;
>  }

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework

[Geert Uytterhoeven]

Hi Laurent,

On Tue, Mar 11, 2014 at 4:55 PM, Laurent Pinchart
<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> Does this require drivers/sh/pm_runtime.c to be compiled in ?

Let's check...

My koelsch-legacy kernel has drivers/sh/pm_runtime.c compiled in.
My koelsch-reference kernel hasn't.

However, under the -reference kernel many MSTP clocks (incl. MSIOF)
seem to be enabled all the time, while under -legacy they are enabled and
disabled on demand.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework

[Laurent Pinchart]

On Tuesday 11 March 2014 17:23:37 Geert Uytterhoeven wrote:
> Hi Laurent,
> 
> On Tue, Mar 11, 2014 at 4:55 PM, Laurent Pinchart wrote:
> > Does this require drivers/sh/pm_runtime.c to be compiled in ?
> 
> Let's check...
> 
> My koelsch-legacy kernel has drivers/sh/pm_runtime.c compiled in.
> My koelsch-reference kernel hasn't.
> 
> However, under the -reference kernel many MSTP clocks (incl. MSIOF)
> seem to be enabled all the time, while under -legacy they are enabled and
> disabled on demand.

Is PM_RUNTIME enabled in both cases ?


There's something fishy in there that we should try to fix without too further 
much delay. Ben Dooks has pointed out the problem a couple of months ago, but 
the discussion on the mailing list just died.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework

[Geert Uytterhoeven]

Hi Laurent,

On Tue, Mar 11, 2014 at 5:32 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Tuesday 11 March 2014 17:23:37 Geert Uytterhoeven wrote:
>> On Tue, Mar 11, 2014 at 4:55 PM, Laurent Pinchart wrote:
>> > Does this require drivers/sh/pm_runtime.c to be compiled in ?
>>
>> Let's check...
>>
>> My koelsch-legacy kernel has drivers/sh/pm_runtime.c compiled in.
>> My koelsch-reference kernel hasn't.
>>
>> However, under the -reference kernel many MSTP clocks (incl. MSIOF)
>> seem to be enabled all the time, while under -legacy they are enabled and
>> disabled on demand.
>
> Is PM_RUNTIME enabled in both cases ?

Yes it is.

> There's something fishy in there that we should try to fix without too further
> much delay. Ben Dooks has pointed out the problem a couple of months ago, but
> the discussion on the mailing list just died.

Indeed. I'll look into it next week, unless someone beats me to it.

Note that I dropped Ben's patch to include drivers/sh, as I can't boot into
userspace with it.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework

[Laurent Pinchart]

Hi Geert,

On Tuesday 11 March 2014 18:44:21 Geert Uytterhoeven wrote:
> On Tue, Mar 11, 2014 at 5:32 PM, Laurent Pinchart wrote:
> > On Tuesday 11 March 2014 17:23:37 Geert Uytterhoeven wrote:
> >> On Tue, Mar 11, 2014 at 4:55 PM, Laurent Pinchart wrote:
> >> > Does this require drivers/sh/pm_runtime.c to be compiled in ?
> >> 
> >> Let's check...
> >> 
> >> My koelsch-legacy kernel has drivers/sh/pm_runtime.c compiled in.
> >> My koelsch-reference kernel hasn't.
> >> 
> >> However, under the -reference kernel many MSTP clocks (incl. MSIOF)
> >> seem to be enabled all the time, while under -legacy they are enabled and
> >> disabled on demand.
> > 
> > Is PM_RUNTIME enabled in both cases ?
> 
> Yes it is.
> 
> > There's something fishy in there that we should try to fix without too
> > further much delay. Ben Dooks has pointed out the problem a couple of
> > months ago, but the discussion on the mailing list just died.
> 
> Indeed. I'll look into it next week, unless someone beats me to it.
> 
> Note that I dropped Ben's patch to include drivers/sh, as I can't boot into
> userspace with it.

The patch also had the issue of not being compatible with multiplatform 
kernels, as it resulted in Renesas-specific code being executed for all 
platforms.

If you can find the old mail thread you can just reply to it (CC'ing me) and 
I'll make sure to refresh my memory and explain my concerns again.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework

[Magnus Damm]

On Wed, Mar 12, 2014 at 1:32 AM, Laurent Pinchart
<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> On Tuesday 11 March 2014 17:23:37 Geert Uytterhoeven wrote:
>> Hi Laurent,
>>
>> On Tue, Mar 11, 2014 at 4:55 PM, Laurent Pinchart wrote:
>> > Does this require drivers/sh/pm_runtime.c to be compiled in ?
>>
>> Let's check...
>>
>> My koelsch-legacy kernel has drivers/sh/pm_runtime.c compiled in.
>> My koelsch-reference kernel hasn't.
>>
>> However, under the -reference kernel many MSTP clocks (incl. MSIOF)
>> seem to be enabled all the time, while under -legacy they are enabled and
>> disabled on demand.
>
> Is PM_RUNTIME enabled in both cases ?
>
>
> There's something fishy in there that we should try to fix without too further
> much delay. Ben Dooks has pointed out the problem a couple of months ago, but
> the discussion on the mailing list just died.

Yes, Runtime PM is not working as expected in the multiplatform case,
that is true. I propose that we keep Runtime PM disabled in the
Kconfig for R-Car Gen2 for now to keep things simple.

>From my side I see it as two separate solutions. The short term fix is
to temporarily work around drivers that depend on Runtime PM for clock
control, I propose enabling selected clocks statically using the
function introduced by this series:

[PATCH 00/03] ARM: shmobile: Break out and extend clock workaround
http://www.spinics.net/lists/arm-kernel/msg310278.html

The long term fix I'm not sure sure about, but I trust Geert to figure
it out. =)

Regardless, rushing to fix this "correctly" seems as useful to me as
dead line driven DT development....

Thanks,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework

[Laurent Pinchart]

Hi Magnus,

On Wednesday 12 March 2014 10:23:48 Magnus Damm wrote:
> On Wed, Mar 12, 2014 at 1:32 AM, Laurent Pinchart wrote:
> > On Tuesday 11 March 2014 17:23:37 Geert Uytterhoeven wrote:
> >> Hi Laurent,
> >> 
> >> On Tue, Mar 11, 2014 at 4:55 PM, Laurent Pinchart wrote:
> >> > Does this require drivers/sh/pm_runtime.c to be compiled in ?
> >> 
> >> Let's check...
> >> 
> >> My koelsch-legacy kernel has drivers/sh/pm_runtime.c compiled in.
> >> My koelsch-reference kernel hasn't.
> >> 
> >> However, under the -reference kernel many MSTP clocks (incl. MSIOF)
> >> seem to be enabled all the time, while under -legacy they are enabled and
> >> disabled on demand.
> > 
> > Is PM_RUNTIME enabled in both cases ?
> > 
> > There's something fishy in there that we should try to fix without too
> > further much delay. Ben Dooks has pointed out the problem a couple of
> > months ago, but the discussion on the mailing list just died.
> 
> Yes, Runtime PM is not working as expected in the multiplatform case,
> that is true. I propose that we keep Runtime PM disabled in the
> Kconfig for R-Car Gen2 for now to keep things simple.

Isn't it ? I thought it was only broken with regard to clocks, but I might be 
missing something.

> From my side I see it as two separate solutions. The short term fix is
> to temporarily work around drivers that depend on Runtime PM for clock
> control, I propose enabling selected clocks statically using the
> function introduced by this series:
> 
> [PATCH 00/03] ARM: shmobile: Break out and extend clock workaround
> http://www.spinics.net/lists/arm-kernel/msg310278.html
> 
> The long term fix I'm not sure sure about, but I trust Geert to figure
> it out. =)
> 
> Regardless, rushing to fix this "correctly" seems as useful to me as
> dead line driven DT development....

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework

[Magnus Damm]

Hi Laurent,

On Wed, Mar 12, 2014 at 8:26 PM, Laurent Pinchart
<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> Hi Magnus,
>
> On Wednesday 12 March 2014 10:23:48 Magnus Damm wrote:
>> On Wed, Mar 12, 2014 at 1:32 AM, Laurent Pinchart wrote:
>> > On Tuesday 11 March 2014 17:23:37 Geert Uytterhoeven wrote:
>> >> Hi Laurent,
>> >>
>> >> On Tue, Mar 11, 2014 at 4:55 PM, Laurent Pinchart wrote:
>> >> > Does this require drivers/sh/pm_runtime.c to be compiled in ?
>> >>
>> >> Let's check...
>> >>
>> >> My koelsch-legacy kernel has drivers/sh/pm_runtime.c compiled in.
>> >> My koelsch-reference kernel hasn't.
>> >>
>> >> However, under the -reference kernel many MSTP clocks (incl. MSIOF)
>> >> seem to be enabled all the time, while under -legacy they are enabled and
>> >> disabled on demand.
>> >
>> > Is PM_RUNTIME enabled in both cases ?
>> >
>> > There's something fishy in there that we should try to fix without too
>> > further much delay. Ben Dooks has pointed out the problem a couple of
>> > months ago, but the discussion on the mailing list just died.
>>
>> Yes, Runtime PM is not working as expected in the multiplatform case,
>> that is true. I propose that we keep Runtime PM disabled in the
>> Kconfig for R-Car Gen2 for now to keep things simple.
>
> Isn't it ? I thought it was only broken with regard to clocks, but I might be
> missing something.

Perhaps the correct way to put it is that we have not yet figured out
how to enable Runtime PM in a way that is suitable for multiplatform.
Which means that clocks are behaving differently in the legacy case
and in the multiplatform case.

Thanks,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html