[PATCH 0/4] thermal: rcar: Add missing clock handling

[PATCH 0/4] thermal: rcar: Add missing clock handling

[Geert Uytterhoeven]

    Hi all,

This patch series fixes a problem when the thermal-rcar device is
instantiated from the device tree on the r8a7791-based Koelsch development
board. The driver fails during initialization with:

    rcar_thermal e61f0000.thermal: thermal sensor was broken

This does not happen when using legacy platform devices.

This series was tested using Koelsch reference (DT) and non-reference
(legacy) kernel configurations. It was not tested on r8a7790/Lager
(which is very similar).

    [1/4] ARM: shmobile: r8a7791: Add thermal clock in device tree
    [2/4] ARM: shmobile: r8a7790: Add thermal clock in device tree
    [3/4] thermal: rcar: Add missing clock handling
    [4/4] thermal: rcar: Spelling s/delaye/delay/g

Thanks for your comments!

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

[PATCH 1/4] ARM: shmobile: r8a7791: Add thermal clock in device tree

[Geert Uytterhoeven]

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

Add the missing thermal MSTP clock to the thermal device node.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
 arch/arm/boot/dts/r8a7791.dtsi |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index d8dbf7e7b6a9..3233041d8a44 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -151,6 +151,7 @@
 		reg = <0 0xe61f0000 0 0x14>, <0 0xe61f0100 0 0x38>;
 		interrupt-parent = <&gic>;
 		interrupts = <0 69 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp5_clks R8A7791_CLK_THERMAL>;
 	};
 
 	timer {
-- 
1.7.9.5

[PATCH 1/4] ARM: shmobile: r8a7791: Add thermal clock in device tree

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

On Tuesday 07 January 2014 19:57:13 Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> Add the missing thermal MSTP clock to the thermal device node.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  arch/arm/boot/dts/r8a7791.dtsi |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
> index d8dbf7e7b6a9..3233041d8a44 100644
> --- a/arch/arm/boot/dts/r8a7791.dtsi
> +++ b/arch/arm/boot/dts/r8a7791.dtsi
> @@ -151,6 +151,7 @@
>  		reg = <0 0xe61f0000 0 0x14>, <0 0xe61f0100 0 0x38>;
>  		interrupt-parent = <&gic>;
>  		interrupts = <0 69 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&mstp5_clks R8A7791_CLK_THERMAL>;
>  	};
> 
>  	timer {
-- 
Regards,

Laurent Pinchart

[PATCH 1/4] ARM: shmobile: r8a7791: Add thermal clock in device tree

[Simon Horman]

On Wed, Jan 08, 2014 at 01:20:09AM +0100, Laurent Pinchart wrote:
> Hi Geert,
> 
> Thank you for the patch.
> 
> On Tuesday 07 January 2014 19:57:13 Geert Uytterhoeven wrote:
> > From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> > 
> > Add the missing thermal MSTP clock to the thermal device node.
> > 
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks, I will queue this up.

[PATCH 2/4] ARM: shmobile: r8a7790: Add thermal clock in device tree

[Geert Uytterhoeven]

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

Add the missing thermal MSTP clock to the thermal device node.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
 arch/arm/boot/dts/r8a7790.dtsi |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index f48487c2a970..b53834e6a55f 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -168,6 +168,7 @@
 		reg = <0 0xe61f0000 0 0x14>, <0 0xe61f0100 0 0x38>;
 		interrupt-parent = <&gic>;
 		interrupts = <0 69 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp5_clks R8A7790_CLK_THERMAL>;
 	};
 
 	timer {
-- 
1.7.9.5

[PATCH 2/4] ARM: shmobile: r8a7790: Add thermal clock in device tree

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

On Tuesday 07 January 2014 19:57:14 Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> Add the missing thermal MSTP clock to the thermal device node.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  arch/arm/boot/dts/r8a7790.dtsi |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index f48487c2a970..b53834e6a55f 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -168,6 +168,7 @@
>  		reg = <0 0xe61f0000 0 0x14>, <0 0xe61f0100 0 0x38>;
>  		interrupt-parent = <&gic>;
>  		interrupts = <0 69 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&mstp5_clks R8A7790_CLK_THERMAL>;
>  	};
> 
>  	timer {
-- 
Regards,

Laurent Pinchart

[PATCH 2/4] ARM: shmobile: r8a7790: Add thermal clock in device tree

[Simon Horman]

On Wed, Jan 08, 2014 at 01:20:08AM +0100, Laurent Pinchart wrote:
> Hi Geert,
> 
> Thank you for the patch.
> 
> On Tuesday 07 January 2014 19:57:14 Geert Uytterhoeven wrote:
> > From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> > 
> > Add the missing thermal MSTP clock to the thermal device node.
> > 
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks, I will queue this up.

[PATCH 3/4] thermal: rcar: Add missing clock handling

[Geert Uytterhoeven]

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

When using DT to instantiate the rcar-thermal device, it prints the
following error:

    rcar_thermal e61f0000.thermal: thermal sensor was broken

Explicitly request and enable the thermal clock to fix this.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
 drivers/thermal/rcar_thermal.c |   40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index 88f92e1a9944..a5629500723a 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -17,6 +17,7 @@
  *  with this program; if not, write to the Free Software Foundation, Inc.,
  *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
  */
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/irq.h>
@@ -53,6 +54,7 @@ struct rcar_thermal_common {
 	struct device *dev;
 	struct list_head head;
 	spinlock_t lock;
+	struct clk *clk;
 };
 
 struct rcar_thermal_priv {
@@ -378,23 +380,38 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 	spin_lock_init(&common->lock);
 	common->dev = dev;
 
+	common->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(common->clk)) {
+		dev_err(&pdev->dev, "cannot get clock\n");
+		return PTR_ERR(common->clk);
+	}
+
+	ret = clk_prepare(common->clk);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "unable to prepare clock\n");
+		return ret;
+	}
+
+	clk_enable(common->clk);
+
 	pm_runtime_enable(dev);
 	pm_runtime_get_sync(dev);
 
 	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (irq) {
-		int ret;
+		int ret2;
 
 		/*
 		 * platform has IRQ support.
 		 * Then, drier use common register
 		 */
 
-		ret = devm_request_irq(dev, irq->start, rcar_thermal_irq, 0,
-				       dev_name(dev), common);
-		if (ret) {
+		ret2 = devm_request_irq(dev, irq->start, rcar_thermal_irq, 0,
+					dev_name(dev), common);
+		if (ret2) {
 			dev_err(dev, "irq request failed\n ");
-			return ret;
+			ret = ret2;
+			goto error_unpm;
 		}
 
 		/*
@@ -402,8 +419,10 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 		 */
 		res = platform_get_resource(pdev, IORESOURCE_MEM, mres++);
 		common->base = devm_ioremap_resource(dev, res);
-		if (IS_ERR(common->base))
-			return PTR_ERR(common->base);
+		if (IS_ERR(common->base)) {
+			ret = PTR_ERR(common->base);
+			goto error_unpm;
+		}
 
 		/* enable temperature comparation */
 		rcar_thermal_common_write(common, ENR, 0x00030303);
@@ -465,9 +484,13 @@ error_unregister:
 			rcar_thermal_irq_disable(priv);
 	}
 
+error_unpm:
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
 
+	clk_disable(common->clk);
+	clk_unprepare(common->clk);
+
 	return ret;
 }
 
@@ -486,6 +509,9 @@ static int rcar_thermal_remove(struct platform_device *pdev)
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
 
+	clk_disable(common->clk);
+	clk_unprepare(common->clk);
+
 	return 0;
 }
 
-- 
1.7.9.5

[PATCH 3/4] thermal: rcar: Add missing clock handling

[Sergei Shtylyov]

Hello.

On 07-01-2014 22:57, Geert Uytterhoeven wrote:

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

> When using DT to instantiate the rcar-thermal device, it prints the
> following error:

>      rcar_thermal e61f0000.thermal: thermal sensor was broken

> Explicitly request and enable the thermal clock to fix this.

> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> ---
>   drivers/thermal/rcar_thermal.c |   40 +++++++++++++++++++++++++++++++++-------
>   1 file changed, 33 insertions(+), 7 deletions(-)

> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index 88f92e1a9944..a5629500723a 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
[...]
> @@ -378,23 +380,38 @@ static int rcar_thermal_probe(struct platform_device *pdev)
[...]
> +	ret = clk_prepare(common->clk);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "unable to prepare clock\n");
> +		return ret;
> +	}
> +
> +	clk_enable(common->clk);
> +

    Why not just clk_prepare_enable()?

[...]
> @@ -465,9 +484,13 @@ error_unregister:
>   			rcar_thermal_irq_disable(priv);
>   	}
>
> +error_unpm:
>   	pm_runtime_put_sync(dev);
>   	pm_runtime_disable(dev);
>
> +	clk_disable(common->clk);
> +	clk_unprepare(common->clk);
> +

    Why not just clk_disable_unprepare()?

>   	return ret;
>   }
>
> @@ -486,6 +509,9 @@ static int rcar_thermal_remove(struct platform_device *pdev)
>   	pm_runtime_put_sync(dev);
>   	pm_runtime_disable(dev);
>
> +	clk_disable(common->clk);
> +	clk_unprepare(common->clk);
> +

    Likewise.

WBR, Sergei

[PATCH 3/4] thermal: rcar: Add missing clock handling

[Gerhard Sittig]

On Tue, Jan 07, 2014 at 19:57 +0100, Geert Uytterhoeven wrote:
> 
> @@ -378,23 +380,38 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>  	spin_lock_init(&common->lock);
>  	common->dev = dev;
>  
> +	common->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(common->clk)) {
> +		dev_err(&pdev->dev, "cannot get clock\n");
> +		return PTR_ERR(common->clk);
> +	}
> +
> +	ret = clk_prepare(common->clk);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "unable to prepare clock\n");
> +		return ret;
> +	}
> +
> +	clk_enable(common->clk);
> +
>  	pm_runtime_enable(dev);
>  	pm_runtime_get_sync(dev);

clk_enable() can fail, too, so you should check its return value


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de

[PATCH 3/4] thermal: rcar: Add missing clock handling

[Ben Dooks]

On 07/01/14 20:57, Gerhard Sittig wrote:
> On Tue, Jan 07, 2014 at 19:57 +0100, Geert Uytterhoeven wrote:
>>
>> @@ -378,23 +380,38 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>>   	spin_lock_init(&common->lock);
>>   	common->dev = dev;
>>
>> +	common->clk = devm_clk_get(&pdev->dev, NULL);
>> +	if (IS_ERR(common->clk)) {
>> +		dev_err(&pdev->dev, "cannot get clock\n");
>> +		return PTR_ERR(common->clk);
>> +	}
>> +
>> +	ret = clk_prepare(common->clk);
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "unable to prepare clock\n");
>> +		return ret;
>> +	}
>> +
>> +	clk_enable(common->clk);
>> +
>>   	pm_runtime_enable(dev);
>>   	pm_runtime_get_sync(dev);
>
> clk_enable() can fail, too, so you should check its return value

Also, if you enable the pm-runtime then the device clock is actually
handled by the common pm-clock framework.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

[PATCH 3/4] thermal: rcar: Add missing clock handling

[Kuninori Morimoto]

Hi Geert

> +	common->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(common->clk)) {
> +		dev_err(&pdev->dev, "cannot get clock\n");
> +		return PTR_ERR(common->clk);
> +	}
> +
> +	ret = clk_prepare(common->clk);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "unable to prepare clock\n");
> +		return ret;
> +	}
> +
> +	clk_enable(common->clk);
> +
>  	pm_runtime_enable(dev);
>  	pm_runtime_get_sync(dev);

You can use "dev" instead of "&pdev->dev" :)

And this patch seems strange for me.
pm_runtime_xxx() is doing same things.
If it didn't work, wrong place is not driver, clock side ?

[PATCH 3/4] thermal: rcar: Add missing clock handling

[Geert Uytterhoeven]

On Wed, Jan 8, 2014 at 2:08 AM, Kuninori Morimoto
<kuninori.morimoto.gx@gmail.com> wrote:
>> +     common->clk = devm_clk_get(&pdev->dev, NULL);
>> +     if (IS_ERR(common->clk)) {
>> +             dev_err(&pdev->dev, "cannot get clock\n");
>> +             return PTR_ERR(common->clk);
>> +     }
>> +
>> +     ret = clk_prepare(common->clk);
>> +     if (ret < 0) {
>> +             dev_err(&pdev->dev, "unable to prepare clock\n");
>> +             return ret;
>> +     }
>> +
>> +     clk_enable(common->clk);
>> +
>>       pm_runtime_enable(dev);
>>       pm_runtime_get_sync(dev);
>
> And this patch seems strange for me.
> pm_runtime_xxx() is doing same things.
> If it didn't work, wrong place is not driver, clock side ?

That's an interesting observation...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

[PATCH 3/4] thermal: rcar: Add missing clock handling

[Geert Uytterhoeven]

Hi Morimoto-san,

On Wed, Jan 8, 2014 at 11:23 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Wed, Jan 8, 2014 at 2:08 AM, Kuninori Morimoto
> <kuninori.morimoto.gx@gmail.com> wrote:
>>> +     common->clk = devm_clk_get(&pdev->dev, NULL);
>>> +     if (IS_ERR(common->clk)) {
>>> +             dev_err(&pdev->dev, "cannot get clock\n");
>>> +             return PTR_ERR(common->clk);
>>> +     }
>>> +
>>> +     ret = clk_prepare(common->clk);
>>> +     if (ret < 0) {
>>> +             dev_err(&pdev->dev, "unable to prepare clock\n");
>>> +             return ret;
>>> +     }
>>> +
>>> +     clk_enable(common->clk);
>>> +
>>>       pm_runtime_enable(dev);
>>>       pm_runtime_get_sync(dev);
>>
>> And this patch seems strange for me.
>> pm_runtime_xxx() is doing same things.
>> If it didn't work, wrong place is not driver, clock side ?
>
> That's an interesting observation...

You were right. After applying both of "ARM: shmobile: compile drivers/sh
for CONFIG_ARCH_SHMOBILE_MULTI" and "power: clock_ops.c: fixup clk
prepare/unprepare count" from Ben Dooks the issue went away.

So my patch can be dropped.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

[PATCH 3/4] thermal: rcar: Add missing clock handling

[Kuninori Morimoto]

Hi Geert

> >> And this patch seems strange for me.
> >> pm_runtime_xxx() is doing same things.
> >> If it didn't work, wrong place is not driver, clock side ?
(snipO
> You were right. After applying both of "ARM: shmobile: compile drivers/sh
> for CONFIG_ARCH_SHMOBILE_MULTI" and "power: clock_ops.c: fixup clk
> prepare/unprepare count" from Ben Dooks the issue went away.

Nice !
Thank you for your help :)

Best regards
---
Kuninori Morimoto

[PATCH 3/4] thermal: rcar: Add missing clock handling

[Simon Horman]

On Mon, Jan 13, 2014 at 09:57:08AM +0100, Geert Uytterhoeven wrote:
> Hi Morimoto-san,
> 
> On Wed, Jan 8, 2014 at 11:23 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Wed, Jan 8, 2014 at 2:08 AM, Kuninori Morimoto
> > <kuninori.morimoto.gx@gmail.com> wrote:
> >>> +     common->clk = devm_clk_get(&pdev->dev, NULL);
> >>> +     if (IS_ERR(common->clk)) {
> >>> +             dev_err(&pdev->dev, "cannot get clock\n");
> >>> +             return PTR_ERR(common->clk);
> >>> +     }
> >>> +
> >>> +     ret = clk_prepare(common->clk);
> >>> +     if (ret < 0) {
> >>> +             dev_err(&pdev->dev, "unable to prepare clock\n");
> >>> +             return ret;
> >>> +     }
> >>> +
> >>> +     clk_enable(common->clk);
> >>> +
> >>>       pm_runtime_enable(dev);
> >>>       pm_runtime_get_sync(dev);
> >>
> >> And this patch seems strange for me.
> >> pm_runtime_xxx() is doing same things.
> >> If it didn't work, wrong place is not driver, clock side ?
> >
> > That's an interesting observation...
> 
> You were right. After applying both of "ARM: shmobile: compile drivers/sh
> for CONFIG_ARCH_SHMOBILE_MULTI" and "power: clock_ops.c: fixup clk
> prepare/unprepare count" from Ben Dooks the issue went away.

There seems to be some lively discussion around Ben's patch.

> So my patch can be dropped.

I have marked this patch as Rejected accordingly.

[PATCH 3/4] thermal: rcar: Add missing clock handling

[Ben Dooks]

On 07/01/14 18:57, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>
> When using DT to instantiate the rcar-thermal device, it prints the
> following error:
>
>      rcar_thermal e61f0000.thermal: thermal sensor was broken
>
> Explicitly request and enable the thermal clock to fix this.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> ---
>   drivers/thermal/rcar_thermal.c |   40 +++++++++++++++++++++++++++++++++-------
>   1 file changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index 88f92e1a9944..a5629500723a 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -17,6 +17,7 @@
>    *  with this program; if not, write to the Free Software Foundation, Inc.,
>    *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
>    */
> +#include <linux/clk.h>
>   #include <linux/delay.h>
>   #include <linux/err.h>
>   #include <linux/irq.h>
> @@ -53,6 +54,7 @@ struct rcar_thermal_common {
>   	struct device *dev;
>   	struct list_head head;
>   	spinlock_t lock;
> +	struct clk *clk;
>   };
>
>   struct rcar_thermal_priv {
> @@ -378,23 +380,38 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>   	spin_lock_init(&common->lock);
>   	common->dev = dev;
>
> +	common->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(common->clk)) {
> +		dev_err(&pdev->dev, "cannot get clock\n");
> +		return PTR_ERR(common->clk);
> +	}
> +
> +	ret = clk_prepare(common->clk);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "unable to prepare clock\n");
> +		return ret;
> +	}
> +
> +	clk_enable(common->clk);
> +
>   	pm_runtime_enable(dev);
>   	pm_runtime_get_sync(dev);
>
>   	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>   	if (irq) {
> -		int ret;
> +		int ret2;
>
>   		/*
>   		 * platform has IRQ support.
>   		 * Then, drier use common register
>   		 */
>
> -		ret = devm_request_irq(dev, irq->start, rcar_thermal_irq, 0,
> -				       dev_name(dev), common);
> -		if (ret) {
> +		ret2 = devm_request_irq(dev, irq->start, rcar_thermal_irq, 0,
> +					dev_name(dev), common);
> +		if (ret2) {
>   			dev_err(dev, "irq request failed\n ");
> -			return ret;
> +			ret = ret2;
> +			goto error_unpm;
>   		}

I'd suggest not renaming ret2 and just use the original ret.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

[PATCH 3/4] thermal: rcar: Add missing clock handling

[Geert Uytterhoeven]

On Wed, Jan 8, 2014 at 1:23 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>> @@ -378,23 +380,38 @@ static int rcar_thermal_probe(struct platform_device
>> *pdev)
>>         spin_lock_init(&common->lock);
>>         common->dev = dev;
>>
>> +       common->clk = devm_clk_get(&pdev->dev, NULL);
>> +       if (IS_ERR(common->clk)) {
>> +               dev_err(&pdev->dev, "cannot get clock\n");
>> +               return PTR_ERR(common->clk);
>> +       }
>> +
>> +       ret = clk_prepare(common->clk);
>> +       if (ret < 0) {
>> +               dev_err(&pdev->dev, "unable to prepare clock\n");
>> +               return ret;
>> +       }
>> +
>> +       clk_enable(common->clk);
>> +
>>         pm_runtime_enable(dev);
>>         pm_runtime_get_sync(dev);
>>
>>         irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>>         if (irq) {
>> -               int ret;
>> +               int ret2;
>>
>>                 /*
>>                  * platform has IRQ support.
>>                  * Then, drier use common register
>>                  */
>>
>> -               ret = devm_request_irq(dev, irq->start, rcar_thermal_irq,
>> 0,
>> -                                      dev_name(dev), common);
>> -               if (ret) {
>> +               ret2 = devm_request_irq(dev, irq->start, rcar_thermal_irq,
>> 0,
>> +                                       dev_name(dev), common);
>> +               if (ret2) {
>>                         dev_err(dev, "irq request failed\n ");
>> -                       return ret;
>> +                       ret = ret2;
>> +                       goto error_unpm;
>>                 }
>
> I'd suggest not renaming ret2 and just use the original ret.

I did that because the for loop after that block depends on ret still being
-ENODEV. Upon closer look, I did break that myself by assigning it
the return value of clk_prepare().
So I'll use the original ret, and reset it to -ENODEV before the for loop
(unless we manage to fix it in pm_runtime_*()).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

[PATCH -trivial 4/4] thermal: rcar: Spelling s/delaye/delay/g

[Geert Uytterhoeven]

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

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
Cc: Jiri Kosina <trivial@kernel.org>
---
 drivers/thermal/rcar_thermal.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
index a5629500723a..32d591087307 100644
--- a/drivers/thermal/rcar_thermal.c
+++ b/drivers/thermal/rcar_thermal.c
@@ -427,7 +427,7 @@ static int rcar_thermal_probe(struct platform_device *pdev)
 		/* enable temperature comparation */
 		rcar_thermal_common_write(common, ENR, 0x00030303);
 
-		idle = 0; /* polling delaye is not needed */
+		idle = 0; /* polling delay is not needed */
 	}
 
 	for (i = 0;; i++) {
-- 
1.7.9.5

[PATCH -trivial 4/4] thermal: rcar: Spelling s/delaye/delay/g

[Simon Horman]

On Tue, Jan 07, 2014 at 07:57:16PM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> Cc: Jiri Kosina <trivial@kernel.org>

Acked-by: Simon Horman <horms+renesas@verge.net.au>

> ---
>  drivers/thermal/rcar_thermal.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index a5629500723a..32d591087307 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -427,7 +427,7 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>  		/* enable temperature comparation */
>  		rcar_thermal_common_write(common, ENR, 0x00030303);
>  
> -		idle = 0; /* polling delaye is not needed */
> +		idle = 0; /* polling delay is not needed */
>  	}
>  
>  	for (i = 0;; i++) {
> -- 
> 1.7.9.5
> 

[PATCH -trivial 4/4] thermal: rcar: Spelling s/delaye/delay/g

[Zhang Rui]

On Tue, 2014-01-07 at 19:57 +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> Cc: Jiri Kosina <trivial@kernel.org>

applied.

thanks,
rui
> ---
>  drivers/thermal/rcar_thermal.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c
> index a5629500723a..32d591087307 100644
> --- a/drivers/thermal/rcar_thermal.c
> +++ b/drivers/thermal/rcar_thermal.c
> @@ -427,7 +427,7 @@ static int rcar_thermal_probe(struct platform_device *pdev)
>  		/* enable temperature comparation */
>  		rcar_thermal_common_write(common, ENR, 0x00030303);
>  
> -		idle = 0; /* polling delaye is not needed */
> +		idle = 0; /* polling delay is not needed */
>  	}
>  
>  	for (i = 0;; i++) {

[PATCH 0/4] thermal: rcar: Add missing clock handling

[Valentine]

On 01/07/2014 10:57 PM, Geert Uytterhoeven wrote:
>      Hi all,

Hi Geert,

>
> This patch series fixes a problem when the thermal-rcar device is
> instantiated from the device tree on the r8a7791-based Koelsch development
> board. The driver fails during initialization with:

I think explicitly acquiring the clocks should not be needed.
Could you please try the patches from the following series instead?

http://marc.info/?l=linux-sh&m=138823255807611&w=2

Thanks,
Val.

>
>      rcar_thermal e61f0000.thermal: thermal sensor was broken
>
> This does not happen when using legacy platform devices.
>
> This series was tested using Koelsch reference (DT) and non-reference
> (legacy) kernel configurations. It was not tested on r8a7790/Lager
> (which is very similar).
>
>      [1/4] ARM: shmobile: r8a7791: Add thermal clock in device tree
>      [2/4] ARM: shmobile: r8a7790: Add thermal clock in device tree
>      [3/4] thermal: rcar: Add missing clock handling
>      [4/4] thermal: rcar: Spelling s/delaye/delay/g
>

> Thanks for your comments!
>
> Gr{oetje,eeting}s,
>
> 						Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH 0/4] thermal: rcar: Add missing clock handling

[Geert Uytterhoeven]

Hi Valentine,

On Tue, Jan 7, 2014 at 10:52 PM, Valentine
<valentine.barshak@cogentembedded.com> wrote:
> On 01/07/2014 10:57 PM, Geert Uytterhoeven wrote:
>> This patch series fixes a problem when the thermal-rcar device is
>> instantiated from the device tree on the r8a7791-based Koelsch development
>> board. The driver fails during initialization with:
>
>
> I think explicitly acquiring the clocks should not be needed.
> Could you please try the patches from the following series instead?
>
> http://marc.info/?l=linux-sh&m=138823255807611&w=2

I already had these applied, and the problem persists.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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