* [PATCH 1/3] spi: sh-hspi: Add missing call to pm_runtime_disable() in failure path @ 2014-03-11 9:59 Geert Uytterhoeven 2014-03-11 9:59 ` [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework Geert Uytterhoeven [not found] ` <1394531952-3905-1-git-send-email-geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> 0 siblings, 2 replies; 17+ messages in thread From: Geert Uytterhoeven @ 2014-03-11 9:59 UTC (permalink / raw) To: Mark Brown Cc: linux-spi, linux-sh, linux-kernel, Geert Uytterhoeven, Kuninori Morimoto 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework 2014-03-11 9:59 [PATCH 1/3] spi: sh-hspi: Add missing call to pm_runtime_disable() in failure path Geert Uytterhoeven @ 2014-03-11 9:59 ` Geert Uytterhoeven [not found] ` <1394531952-3905-2-git-send-email-geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> [not found] ` <1394531952-3905-1-git-send-email-geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> 1 sibling, 1 reply; 17+ messages in thread From: Geert Uytterhoeven @ 2014-03-11 9:59 UTC (permalink / raw) To: Mark Brown Cc: linux-spi, linux-sh, linux-kernel, Geert Uytterhoeven, Magnus Damm 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
[parent not found: <1394531952-3905-2-git-send-email-geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>]
* Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework [not found] ` <1394531952-3905-2-git-send-email-geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> @ 2014-03-11 10:45 ` Mark Brown 2014-03-11 15:55 ` Laurent Pinchart 1 sibling, 0 replies; 17+ messages in thread From: Mark Brown @ 2014-03-11 10:45 UTC (permalink / raw) To: Geert Uytterhoeven Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-sh-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven, Magnus Damm [-- 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 --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework [not found] ` <1394531952-3905-2-git-send-email-geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> 2014-03-11 10:45 ` Mark Brown @ 2014-03-11 15:55 ` Laurent Pinchart 2014-03-11 16:23 ` Geert Uytterhoeven 1 sibling, 1 reply; 17+ messages in thread From: Laurent Pinchart @ 2014-03-11 15:55 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-sh-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven, Magnus Damm 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework 2014-03-11 15:55 ` Laurent Pinchart @ 2014-03-11 16:23 ` Geert Uytterhoeven 2014-03-11 16:32 ` Laurent Pinchart 0 siblings, 1 reply; 17+ messages in thread From: Geert Uytterhoeven @ 2014-03-11 16:23 UTC (permalink / raw) To: Laurent Pinchart Cc: Mark Brown, linux-spi, Linux-sh list, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Geert Uytterhoeven, Magnus Damm 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework 2014-03-11 16:23 ` Geert Uytterhoeven @ 2014-03-11 16:32 ` Laurent Pinchart 2014-03-11 17:44 ` Geert Uytterhoeven 2014-03-12 1:23 ` Magnus Damm 0 siblings, 2 replies; 17+ messages in thread From: Laurent Pinchart @ 2014-03-11 16:32 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Mark Brown, linux-spi, Linux-sh list, linux-kernel@vger.kernel.org, Geert Uytterhoeven, Magnus Damm 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework 2014-03-11 16:32 ` Laurent Pinchart @ 2014-03-11 17:44 ` Geert Uytterhoeven [not found] ` <CAMuHMdXEOygh0--niiLcXFgnRNPi_b0EGPKVBg2Q6Ws01YmNjw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2014-03-12 1:23 ` Magnus Damm 1 sibling, 1 reply; 17+ messages in thread From: Geert Uytterhoeven @ 2014-03-11 17:44 UTC (permalink / raw) To: Laurent Pinchart Cc: Mark Brown, linux-spi, Linux-sh list, linux-kernel@vger.kernel.org, Geert Uytterhoeven, Magnus Damm 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <CAMuHMdXEOygh0--niiLcXFgnRNPi_b0EGPKVBg2Q6Ws01YmNjw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework [not found] ` <CAMuHMdXEOygh0--niiLcXFgnRNPi_b0EGPKVBg2Q6Ws01YmNjw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2014-03-11 17:57 ` Laurent Pinchart 0 siblings, 0 replies; 17+ messages in thread From: Laurent Pinchart @ 2014-03-11 17:57 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Mark Brown, linux-spi, Linux-sh list, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Geert Uytterhoeven, Magnus Damm 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework 2014-03-11 16:32 ` Laurent Pinchart 2014-03-11 17:44 ` Geert Uytterhoeven @ 2014-03-12 1:23 ` Magnus Damm [not found] ` <CANqRtoRR9Q8Q=ACqhnLBpTavn0K5cD1=dBkjkfZ8kb+H68GuOw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 1 sibling, 1 reply; 17+ messages in thread From: Magnus Damm @ 2014-03-12 1:23 UTC (permalink / raw) To: Laurent Pinchart Cc: Geert Uytterhoeven, Mark Brown, linux-spi, Linux-sh list, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Geert Uytterhoeven 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <CANqRtoRR9Q8Q=ACqhnLBpTavn0K5cD1=dBkjkfZ8kb+H68GuOw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework [not found] ` <CANqRtoRR9Q8Q=ACqhnLBpTavn0K5cD1=dBkjkfZ8kb+H68GuOw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2014-03-12 11:26 ` Laurent Pinchart 2014-03-12 11:28 ` Magnus Damm 0 siblings, 1 reply; 17+ messages in thread From: Laurent Pinchart @ 2014-03-12 11:26 UTC (permalink / raw) To: Magnus Damm Cc: Geert Uytterhoeven, Mark Brown, linux-spi, Linux-sh list, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Geert Uytterhoeven 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework 2014-03-12 11:26 ` Laurent Pinchart @ 2014-03-12 11:28 ` Magnus Damm 0 siblings, 0 replies; 17+ messages in thread From: Magnus Damm @ 2014-03-12 11:28 UTC (permalink / raw) To: Laurent Pinchart Cc: Geert Uytterhoeven, Mark Brown, linux-spi, Linux-sh list, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Geert Uytterhoeven 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <1394531952-3905-1-git-send-email-geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>]
* [PATCH 3/3] spi: rspi: Add runtime PM support, using spi core auto_runtime_pm [not found] ` <1394531952-3905-1-git-send-email-geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> @ 2014-03-11 9:59 ` Geert Uytterhoeven 2014-03-11 10:47 ` Mark Brown 0 siblings, 1 reply; 17+ messages in thread From: Geert Uytterhoeven @ 2014-03-11 9:59 UTC (permalink / raw) To: Mark Brown Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-sh-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] spi: rspi: Add runtime PM support, using spi core auto_runtime_pm 2014-03-11 9:59 ` [PATCH 3/3] spi: rspi: Add runtime PM support, using spi core auto_runtime_pm Geert Uytterhoeven @ 2014-03-11 10:47 ` Mark Brown 2014-03-11 13:10 ` Geert Uytterhoeven 0 siblings, 1 reply; 17+ messages in thread From: Mark Brown @ 2014-03-11 10:47 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: linux-spi, linux-sh, linux-kernel, Geert Uytterhoeven [-- 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 --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] spi: rspi: Add runtime PM support, using spi core auto_runtime_pm 2014-03-11 10:47 ` Mark Brown @ 2014-03-11 13:10 ` Geert Uytterhoeven 2014-03-11 13:26 ` Mark Brown 0 siblings, 1 reply; 17+ messages in thread From: Geert Uytterhoeven @ 2014-03-11 13:10 UTC (permalink / raw) To: Mark Brown Cc: linux-spi, Linux-sh list, linux-kernel@vger.kernel.org, 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] spi: rspi: Add runtime PM support, using spi core auto_runtime_pm 2014-03-11 13:10 ` Geert Uytterhoeven @ 2014-03-11 13:26 ` Mark Brown 2014-03-11 13:35 ` Geert Uytterhoeven 0 siblings, 1 reply; 17+ messages in thread From: Mark Brown @ 2014-03-11 13:26 UTC (permalink / raw) To: Geert Uytterhoeven Cc: linux-spi, Linux-sh list, linux-kernel@vger.kernel.org, Geert Uytterhoeven [-- 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 --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] spi: rspi: Add runtime PM support, using spi core auto_runtime_pm 2014-03-11 13:26 ` Mark Brown @ 2014-03-11 13:35 ` Geert Uytterhoeven 2014-03-11 14:18 ` Mark Brown 0 siblings, 1 reply; 17+ messages in thread From: Geert Uytterhoeven @ 2014-03-11 13:35 UTC (permalink / raw) To: Mark Brown Cc: linux-spi, Linux-sh list, linux-kernel@vger.kernel.org, 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] spi: rspi: Add runtime PM support, using spi core auto_runtime_pm 2014-03-11 13:35 ` Geert Uytterhoeven @ 2014-03-11 14:18 ` Mark Brown 0 siblings, 0 replies; 17+ messages in thread From: Mark Brown @ 2014-03-11 14:18 UTC (permalink / raw) To: Geert Uytterhoeven Cc: linux-spi, Linux-sh list, linux-kernel@vger.kernel.org, Geert Uytterhoeven [-- 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 --] ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-03-12 11:28 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-11 9:59 [PATCH 1/3] spi: sh-hspi: Add missing call to pm_runtime_disable() in failure path Geert Uytterhoeven 2014-03-11 9:59 ` [PATCH 2/3] spi: sh-msiof: Convert to spi core auto_runtime_pm framework Geert Uytterhoeven [not found] ` <1394531952-3905-2-git-send-email-geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> 2014-03-11 10:45 ` Mark Brown 2014-03-11 15:55 ` Laurent Pinchart 2014-03-11 16:23 ` Geert Uytterhoeven 2014-03-11 16:32 ` Laurent Pinchart 2014-03-11 17:44 ` Geert Uytterhoeven [not found] ` <CAMuHMdXEOygh0--niiLcXFgnRNPi_b0EGPKVBg2Q6Ws01YmNjw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2014-03-11 17:57 ` Laurent Pinchart 2014-03-12 1:23 ` Magnus Damm [not found] ` <CANqRtoRR9Q8Q=ACqhnLBpTavn0K5cD1=dBkjkfZ8kb+H68GuOw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2014-03-12 11:26 ` Laurent Pinchart 2014-03-12 11:28 ` Magnus Damm [not found] ` <1394531952-3905-1-git-send-email-geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> 2014-03-11 9:59 ` [PATCH 3/3] spi: rspi: Add runtime PM support, using spi core auto_runtime_pm Geert Uytterhoeven 2014-03-11 10:47 ` Mark Brown 2014-03-11 13:10 ` Geert Uytterhoeven 2014-03-11 13:26 ` Mark Brown 2014-03-11 13:35 ` Geert Uytterhoeven 2014-03-11 14:18 ` Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).