thermal: rcar: Add missing clock handling

From: Geert Uytterhoeven <[email protected]>

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

Signed-off-by Geert Uytterhoeven <[email protected]>

---
 arch/arm/boot/dts/r8a7791.dtsi |    1 +
 1 file changed, 1 insertion(+)

arch/arm/boot/dts/r8a7791.dtsi +1 -0

index d8dbf7e7b6a9..3233041d8a44 100644
@@ -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 {

Hi Geert,

Thank you for the patch.

Quoted 5 lines · On Tuesday 07 January 2014 19:57:13 Geert Uytterhoeven wrote:

From: Geert Uytterhoeven <[email protected]>

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

Signed-off-by Geert Uytterhoeven <[email protected]>

Acked-by Laurent Pinchart <[email protected]>

From: Wolfram Sang <[email protected]>

Signed-off-by Wolfram Sang <[email protected]>

---
 include/dt-bindings/clock/r8a7791-clock.h | 2 ++
 1 file changed, 2 insertions(+)

(quoted diff) +0 -0

b/include/dt-bindings/clock/r8a7791-clock.h index 30f82f2..9aa60f4 100644

include/dt-bindings/clock/r8a7791-clock.h +0 -0

 /* MSTP5 */
 #define R8A7791_CLK_THERMAL		22

Hi Wolfram,

Thank you for the patch.

Quoted 7 lines · On Sunday 23 February 2014 13:40:21 Wolfram Sang wrote:

From: Wolfram Sang <[email protected]>

Signed-off-by Wolfram Sang <[email protected]>

---
 include/dt-bindings/clock/r8a7791-clock.h | 2 ++
 1 file changed, 2 insertions(+)

include/dt-bindings/clock/r8a7791-clock.h line 59

Quoted 10 lines

(quoted diff) +0 -0

b/include/dt-bindings/clock/r8a7791-clock.h index 30f82f2..9aa60f4 100644

include/dt-bindings/clock/r8a7791-clock.h +2 -0

@@ -55,6 +55,8 @@
 #define R8A7791_CLK_CMT1		29
 #define R8A7791_CLK_USBDMAC0		30
 #define R8A7791_CLK_USBDMAC1		31
+#define R8A7791_CLK_IIC0		18
+#define R8A7791_CLK_IIC1		23

Could you please keep the entries sorted by value ?

From: Geert Uytterhoeven <[email protected]>

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

Signed-off-by Geert Uytterhoeven <[email protected]>

---
 arch/arm/boot/dts/r8a7790.dtsi |    1 +
 1 file changed, 1 insertion(+)

arch/arm/boot/dts/r8a7790.dtsi +1 -0

index f48487c2a970..b53834e6a55f 100644
@@ -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 {

Hi Geert,

Thank you for the patch.

Quoted 5 lines · On Tuesday 07 January 2014 19:57:14 Geert Uytterhoeven wrote:

From: Geert Uytterhoeven <[email protected]>

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

Signed-off-by Geert Uytterhoeven <[email protected]>

Acked-by Laurent Pinchart <[email protected]>

From: Geert Uytterhoeven <[email protected]>

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 <[email protected]>

---
  drivers/thermal/rcar_thermal.c |   40 +++++++++++++++++++++++++++++++++-------
  1 file changed, 33 insertions(+), 7 deletions(-)

drivers/thermal/rcar_thermal.c +0 -0

index 88f92e1a9944..a5629500723a 100644

drivers/thermal/rcar_thermal.c +0 -0

@@ -378,23 +380,38 @@ static int rcar_thermal_probe(struct platform_device *pdev)

drivers/thermal/rcar_thermal.c +6 -0

@@ -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);
+	}
+

Quoted 15 lines · Geert Uytterhoeven

drivers/thermal/rcar_thermal.c +10 -0

@@ -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;

drivers/thermal/rcar_thermal.c static int rcar_thermal_probe(struct platform_device *pdev) · line 398

Quoted 6 lines

drivers/thermal/rcar_thermal.c +4 -0

+	}
+
+	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

drivers/thermal/rcar_thermal.c +6 -0

+	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);
+	}
+

Quoted 12 lines

drivers/thermal/rcar_thermal.c +12 -0

+     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;
+     }
+

drivers/thermal/rcar_thermal.c static int rcar_thermal_probe(struct platform_device *pdev) · line 398

Quoted 4 lines

drivers/thermal/rcar_thermal.c +2 -0

+     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 ?

Quoted 12 lines · Kuninori Morimoto

Quoted 12 lines

drivers/thermal/rcar_thermal.c +12 -0

+     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;
+     }
+

Quoted 8 lines

drivers/thermal/rcar_thermal.c static int rcar_thermal_probe(struct platform_device *pdev) · line 398

Quoted 4 lines

drivers/thermal/rcar_thermal.c +2 -0

+     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...

Quoted 3 lines

Quoted 3 lines

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 ?

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.

Hi Morimoto-san,

Quoted 23 lines · Geert Uytterhoeven

Quoted 20 lines · Kuninori Morimoto

Quoted 6 lines

drivers/thermal/rcar_thermal.c +6 -0

+     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);
+     }
+

drivers/thermal/rcar_thermal.c static int rcar_thermal_probe(struct platform_device *pdev) · line 398

Quoted 10 lines

drivers/thermal/rcar_thermal.c +8 -0

+     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 ?

Quoted 1 line

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.

From: Geert Uytterhoeven <[email protected]>

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 <[email protected]>

---
  drivers/thermal/rcar_thermal.c |   40 +++++++++++++++++++++++++++++++++-------
  1 file changed, 33 insertions(+), 7 deletions(-)

drivers/thermal/rcar_thermal.c +17 -2

index 88f92e1a9944..a5629500723a 100644
@@ -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,

Quoted 32 lines

drivers/thermal/rcar_thermal.c +15 -1

@@ -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
                 */
 

Quoted 12 lines

drivers/thermal/rcar_thermal.c +5 -4

-               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;

Quoted 1 line

                }

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

From: Geert Uytterhoeven <[email protected]>

Signed-off-by Geert Uytterhoeven <[email protected]>

Cc Jiri Kosina <[email protected]>

---
 drivers/thermal/rcar_thermal.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/thermal/rcar_thermal.c +1 -1

index a5629500723a..32d591087307 100644
@@ -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++) {

Signature · 1 line

1.7.9.5

From: Geert Uytterhoeven <[email protected]>

Signed-off-by Geert Uytterhoeven <[email protected]>

Cc Jiri Kosina <[email protected]>

---
 drivers/thermal/rcar_thermal.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/thermal/rcar_thermal.c +1 -1

index a5629500723a..32d591087307 100644
@@ -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++) {

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
--
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

Quoted 3 lines · Geert Uytterhoeven

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