ARM: dts: imx: fix clocks for cspi

PATCH In review Jonas Andersson 5 participants 10 messages May 23, 2013 – Jun 25, 2013

Subsystems (2)

Device tree bindings Arm i.MX and Layerscape

lore

Share & tools

lore

Jonas Andersson

author

2013-05-23 11:38 UTC #0 lore raw

PATCH 1/1 ARM: dts: imx: fix clocks for cspi

The CSPI controller has only one clock, but the driver spi-imx.c needs clock "per" to calculate bitrate divisor.

Signed-off-by Jonas Andersson <[email protected]>

---
 arch/arm/boot/dts/imx25.dtsi | 12 ++++++------
 arch/arm/boot/dts/imx27.dtsi |  6 +++---
 arch/arm/boot/dts/imx51.dtsi |  2 +-
 arch/arm/boot/dts/imx53.dtsi |  2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)

arch/arm/boot/dts/imx25.dtsi +6 -6

index d2550e0..7011539 100644
@@ -141,8 +141,8 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";
 				reg = <0x43fa4000 0x4000>;
-				clocks = <&clks 62>;
-				clock-names = "ipg";
+				clocks = <&clks 62>, <&clks 62>;
+				clock-names = "ipg", "per";
 				interrupts = <14>;
 				status = "disabled";
 			};

@@ -182,8 +182,8 @@
 				compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";
 				reg = <0x50004000 0x4000>;
 				interrupts = <0>;
-				clocks = <&clks 80>;
-				clock-names = "ipg";
+				clocks = <&clks 80>, <&clks 80>;
+				clock-names = "ipg", "per";
 				status = "disabled";
 			};

@@ -210,8 +210,8 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";
 				reg = <0x50010000 0x4000>;
-				clocks = <&clks 79>;
-				clock-names = "ipg";
+				clocks = <&clks 79>, <&clks 79>;
+				clock-names = "ipg", "per";
 				interrupts = <13>;
 				status = "disabled";
 			};

arch/arm/boot/dts/imx27.dtsi +3 -3

index ff4bd48..75bd113 100644
@@ -131,7 +131,7 @@
 				compatible = "fsl,imx27-cspi";
 				reg = <0x1000e000 0x1000>;
 				interrupts = <16>;
-				clocks = <&clks 53>, <&clks 0>;
+				clocks = <&clks 53>, <&clks 53>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};

@@ -142,7 +142,7 @@
 				compatible = "fsl,imx27-cspi";
 				reg = <0x1000f000 0x1000>;
 				interrupts = <15>;
-				clocks = <&clks 52>, <&clks 0>;
+				clocks = <&clks 52>, <&clks 52>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};

@@ -223,7 +223,7 @@
 				compatible = "fsl,imx27-cspi";
 				reg = <0x10017000 0x1000>;
 				interrupts = <6>;
-				clocks = <&clks 51>, <&clks 0>;
+				clocks = <&clks 51>, <&clks 51>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};

arch/arm/boot/dts/imx51.dtsi +1 -1

index 21bb786..53fdde6 100644
@@ -631,7 +631,7 @@
 				compatible = "fsl,imx51-cspi", "fsl,imx35-cspi";
 				reg = <0x83fc0000 0x4000>;
 				interrupts = <38>;
-				clocks = <&clks 55>, <&clks 0>;
+				clocks = <&clks 55>, <&clks 55>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};

arch/arm/boot/dts/imx53.dtsi +1 -1

index 845982e..eb83aa0 100644
@@ -714,7 +714,7 @@
 				compatible = "fsl,imx53-cspi", "fsl,imx35-cspi";
 				reg = <0x63fc0000 0x4000>;
 				interrupts = <38>;
-				clocks = <&clks 55>, <&clks 0>;
+				clocks = <&clks 55>, <&clks 55>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};

Signature · 1 line

1.8.1.4

Sascha Hauer

maintainer

2013-05-23 11:57 UTC #1 lore raw

PATCH 1/1 ARM: dts: imx: fix clocks for cspi

Quoted 3 lines · Jonas Andersson

The CSPI controller has only one clock, but the driver spi-imx.c needs clock "per" to calculate bitrate divisor.
Signed-off-by Jonas Andersson <[email protected]>

Acked-by Sascha Hauer <[email protected]>

Sascha

Quoted 105 lines

---
 arch/arm/boot/dts/imx25.dtsi | 12 ++++++------
 arch/arm/boot/dts/imx27.dtsi |  6 +++---
 arch/arm/boot/dts/imx51.dtsi |  2 +-
 arch/arm/boot/dts/imx53.dtsi |  2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)

arch/arm/boot/dts/imx25.dtsi +6 -6

index d2550e0..7011539 100644
@@ -141,8 +141,8 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";
 				reg = <0x43fa4000 0x4000>;
-				clocks = <&clks 62>;
-				clock-names = "ipg";
+				clocks = <&clks 62>, <&clks 62>;
+				clock-names = "ipg", "per";
 				interrupts = <14>;
 				status = "disabled";
 			};

@@ -182,8 +182,8 @@
 				compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";
 				reg = <0x50004000 0x4000>;
 				interrupts = <0>;
-				clocks = <&clks 80>;
-				clock-names = "ipg";
+				clocks = <&clks 80>, <&clks 80>;
+				clock-names = "ipg", "per";
 				status = "disabled";
 			};

@@ -210,8 +210,8 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";
 				reg = <0x50010000 0x4000>;
-				clocks = <&clks 79>;
-				clock-names = "ipg";
+				clocks = <&clks 79>, <&clks 79>;
+				clock-names = "ipg", "per";
 				interrupts = <13>;
 				status = "disabled";
 			};

arch/arm/boot/dts/imx27.dtsi +3 -3

index ff4bd48..75bd113 100644
@@ -131,7 +131,7 @@
 				compatible = "fsl,imx27-cspi";
 				reg = <0x1000e000 0x1000>;
 				interrupts = <16>;
-				clocks = <&clks 53>, <&clks 0>;
+				clocks = <&clks 53>, <&clks 53>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};

@@ -142,7 +142,7 @@
 				compatible = "fsl,imx27-cspi";
 				reg = <0x1000f000 0x1000>;
 				interrupts = <15>;
-				clocks = <&clks 52>, <&clks 0>;
+				clocks = <&clks 52>, <&clks 52>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};

@@ -223,7 +223,7 @@
 				compatible = "fsl,imx27-cspi";
 				reg = <0x10017000 0x1000>;
 				interrupts = <6>;
-				clocks = <&clks 51>, <&clks 0>;
+				clocks = <&clks 51>, <&clks 51>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};

arch/arm/boot/dts/imx51.dtsi +1 -1

index 21bb786..53fdde6 100644
@@ -631,7 +631,7 @@
 				compatible = "fsl,imx51-cspi", "fsl,imx35-cspi";
 				reg = <0x83fc0000 0x4000>;
 				interrupts = <38>;
-				clocks = <&clks 55>, <&clks 0>;
+				clocks = <&clks 55>, <&clks 55>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};

arch/arm/boot/dts/imx53.dtsi +1 -1

index 845982e..eb83aa0 100644
@@ -714,7 +714,7 @@
 				compatible = "fsl,imx53-cspi", "fsl,imx35-cspi";
 				reg = <0x63fc0000 0x4000>;
 				interrupts = <38>;
-				clocks = <&clks 55>, <&clks 0>;
+				clocks = <&clks 55>, <&clks 55>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};

Signature · 1 line

1.8.1.4

Signature · 4 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Alexander Shiyan

2013-05-23 12:01 UTC #2 lore raw

In reply to Jonas Andersson

Quoted 15 lines

The CSPI controller has only one clock, but the driver spi-imx.c needs clock "per" to calculate bitrate divisor.
Signed-off-by Jonas Andersson <[email protected]>
---
 arch/arm/boot/dts/imx25.dtsi | 12 ++++++------
 arch/arm/boot/dts/imx27.dtsi |  6 +++---
 arch/arm/boot/dts/imx51.dtsi |  2 +-
 arch/arm/boot/dts/imx53.dtsi |  2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)
arch/arm/boot/dts/imx25.dtsi +0 -0
index d2550e0..7011539 100644

arch/arm/boot/dts/imx25.dtsi line 148

Quoted 10 lines

arch/arm/boot/dts/imx25.dtsi +2 -2

@@ -141,8 +141,8 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";
 				reg = <0x43fa4000 0x4000>;
-				clocks = <&clks 62>;
-				clock-names = "ipg";
+				clocks = <&clks 62>, <&clks 62>;
+				clock-names = "ipg", "per";
 				interrupts = <14>;
 				status = "disabled";
 			};

Why you do not use "dummy" clock for "per" here? This variant will request/enable one clock two times.

---

Alexander Shiyan

2013-05-23 12:10 UTC #3 lore raw

Quoted 10 lines

Quoted 10 lines
The CSPI controller has only one clock, but the driver spi-imx.c needs clock "per" to calculate bitrate divisor.
Signed-off-by Jonas Andersson <[email protected]>
---
 arch/arm/boot/dts/imx25.dtsi | 12 ++++++------
 arch/arm/boot/dts/imx27.dtsi |  6 +++---
 arch/arm/boot/dts/imx51.dtsi |  2 +-
 arch/arm/boot/dts/imx53.dtsi |  2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)

Quoted 15 lines

Quoted 5 lines

arch/arm/boot/dts/imx25.dtsi +0 -0
index d2550e0..7011539 100644

arch/arm/boot/dts/imx25.dtsi line 148

Quoted 10 lines

arch/arm/boot/dts/imx25.dtsi +2 -2

@@ -141,8 +141,8 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";
 				reg = <0x43fa4000 0x4000>;
-				clocks = <&clks 62>;
-				clock-names = "ipg";
+				clocks = <&clks 62>, <&clks 62>;
+				clock-names = "ipg", "per";
 				interrupts = <14>;
 				status = "disabled";
 			};

Sorry, typo.

Quoted 1 line

Why you do not use "dummy" clock for "per" here?

*for "ipg"

Quoted 1 line

This variant will request/enable one clock two times.

---

Sascha Hauer

maintainer

2013-05-23 12:36 UTC #4 lore raw

PATCH 1/1 ARM: dts: imx: fix clocks for cspi

Quoted 28 lines · Alexander Shiyan

Quoted 10 lines
Quoted 10 lines
The CSPI controller has only one clock, but the driver spi-imx.c needs clock "per" to calculate bitrate divisor.
Signed-off-by Jonas Andersson <[email protected]>
---
 arch/arm/boot/dts/imx25.dtsi | 12 ++++++------
 arch/arm/boot/dts/imx27.dtsi |  6 +++---
 arch/arm/boot/dts/imx51.dtsi |  2 +-
 arch/arm/boot/dts/imx53.dtsi |  2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)
Quoted 15 lines
Quoted 5 lines
arch/arm/boot/dts/imx25.dtsi +0 -0
index d2550e0..7011539 100644
arch/arm/boot/dts/imx25.dtsi line 148
Quoted 10 lines
arch/arm/boot/dts/imx25.dtsi +2 -2
@@ -141,8 +141,8 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";
 				reg = <0x43fa4000 0x4000>;
-				clocks = <&clks 62>;
-				clock-names = "ipg";
+				clocks = <&clks 62>, <&clks 62>;
+				clock-names = "ipg", "per";
 				interrupts = <14>;
 				status = "disabled";
 			};
Sorry, typo.

Quoted 3 lines

Quoted 1 line
Why you do not use "dummy" clock for "per" here?
*for "ipg"

Because the same clock is used for the register clock and the baudrate clock. You have to enable the ipg clock to access registers.

Quoted 2 lines

Quoted 2 lines
This variant will request/enable one clock two times.

Indeed, but what's the problem with this?

Sascha

Signature · 4 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Alexander Shiyan

2013-05-23 14:28 UTC #5 lore raw

Quoted 29 lines · Alexander Shiyan

Quoted 28 lines
Quoted 10 lines
Quoted 10 lines
The CSPI controller has only one clock, but the driver spi-imx.c needs clock "per" to calculate bitrate divisor.
Signed-off-by Jonas Andersson <[email protected]>
---
 arch/arm/boot/dts/imx25.dtsi | 12 ++++++------
 arch/arm/boot/dts/imx27.dtsi |  6 +++---
 arch/arm/boot/dts/imx51.dtsi |  2 +-
 arch/arm/boot/dts/imx53.dtsi |  2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)
Quoted 15 lines
Quoted 5 lines
arch/arm/boot/dts/imx25.dtsi +0 -0
index d2550e0..7011539 100644
arch/arm/boot/dts/imx25.dtsi line 148
Quoted 10 lines
arch/arm/boot/dts/imx25.dtsi +2 -2
@@ -141,8 +141,8 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";
 				reg = <0x43fa4000 0x4000>;
-				clocks = <&clks 62>;
-				clock-names = "ipg";
+				clocks = <&clks 62>, <&clks 62>;
+				clock-names = "ipg", "per";
 				interrupts = <14>;
 				status = "disabled";
 			};
Sorry, typo.

Quoted 6 lines

Quoted 3 lines
Quoted 1 line
Why you do not use "dummy" clock for "per" here?
*for "ipg"
Because the same clock is used for the register clock and the baudrate clock. You have to enable the ipg clock to access registers.

I am not see any "ipg" clock usage in the spi driver. If "ipg" clock is a dependency for "per" clock, it should be registered in ccm as parent. Is not it?

---

Sascha Hauer

maintainer

2013-05-23 17:33 UTC #6 lore raw

PATCH 1/1 ARM: dts: imx: fix clocks for cspi

Quoted 29 lines · Alexander Shiyan

Quoted 29 lines · Alexander Shiyan
Quoted 28 lines
Quoted 10 lines
Quoted 10 lines
The CSPI controller has only one clock, but the driver spi-imx.c needs clock "per" to calculate bitrate divisor.
Signed-off-by Jonas Andersson <[email protected]>
---
 arch/arm/boot/dts/imx25.dtsi | 12 ++++++------
 arch/arm/boot/dts/imx27.dtsi |  6 +++---
 arch/arm/boot/dts/imx51.dtsi |  2 +-
 arch/arm/boot/dts/imx53.dtsi |  2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)
Quoted 15 lines
Quoted 5 lines
arch/arm/boot/dts/imx25.dtsi +0 -0
index d2550e0..7011539 100644
arch/arm/boot/dts/imx25.dtsi line 148
Quoted 10 lines
arch/arm/boot/dts/imx25.dtsi +2 -2
@@ -141,8 +141,8 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";
 				reg = <0x43fa4000 0x4000>;
-				clocks = <&clks 62>;
-				clock-names = "ipg";
+				clocks = <&clks 62>, <&clks 62>;
+				clock-names = "ipg", "per";
 				interrupts = <14>;
 				status = "disabled";
 			};
Sorry, typo.

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Why you do not use "dummy" clock for "per" here?
*for "ipg"
Because the same clock is used for the register clock and the baudrate clock. You have to enable the ipg clock to access registers.
I am not see any "ipg" clock usage in the spi driver.

It's the register clock. The driver enables it in order to access the registers.

Quoted 2 lines

If "ipg" clock is
a dependency for "per" clock, it should be registered in ccm as parent.

Look, the eCSPI unit has two clock inputs, one for accessing the registers (ipg) and one for generating the SPI bit clock (per). Now the CSPI unit only has a single clock input, so the 100% correct way would be to specify only a single clock for this unit. However, since we handle both units with the same driver the simplest way to cope with it is to provide the very same clock twice, once for the register access and once for generating the SPI bit clock.
And yes, we have to specify the real clock twice and can't replace one with a dummy clock, because if we specify a dummy clock for the ipg clock, then the driver couldn't access the registers even if it enabled the clock. If we would replace the SPI bit clock with a dummy clock then the device couldn't send data even if the driver enabled the bit clock. (That of course only becomes relevant when the driver actually en/disables the clocks during runtime and not only once during probe).

Sascha

Signature · 4 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Alexander Shiyan

2013-05-24 04:16 UTC #7 lore raw

Quoted 44 lines

Quoted 29 lines
Quoted 29 lines · Alexander Shiyan
Quoted 28 lines
Quoted 10 lines
Quoted 10 lines
The CSPI controller has only one clock, but the driver spi-imx.c needs clock "per" to calculate bitrate divisor.
Signed-off-by Jonas Andersson <[email protected]>
---
 arch/arm/boot/dts/imx25.dtsi | 12 ++++++------
 arch/arm/boot/dts/imx27.dtsi |  6 +++---
 arch/arm/boot/dts/imx51.dtsi |  2 +-
 arch/arm/boot/dts/imx53.dtsi |  2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)
Quoted 15 lines
Quoted 5 lines
arch/arm/boot/dts/imx25.dtsi +0 -0
index d2550e0..7011539 100644
arch/arm/boot/dts/imx25.dtsi line 148
Quoted 10 lines
arch/arm/boot/dts/imx25.dtsi +2 -2
@@ -141,8 +141,8 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";
 				reg = <0x43fa4000 0x4000>;
-				clocks = <&clks 62>;
-				clock-names = "ipg";
+				clocks = <&clks 62>, <&clks 62>;
+				clock-names = "ipg", "per";
 				interrupts = <14>;
 				status = "disabled";
 			};
Sorry, typo.
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Why you do not use "dummy" clock for "per" here?
*for "ipg"
Because the same clock is used for the register clock and the baudrate clock. You have to enable the ipg clock to access registers.
I am not see any "ipg" clock usage in the spi driver.
It's the register clock. The driver enables it in order to access the registers.
Quoted 2 lines
If "ipg" clock is
a dependency for "per" clock, it should be registered in ccm as parent.

Quoted 14 lines

Look, the eCSPI unit has two clock inputs, one for accessing the registers (ipg) and one for generating the SPI bit clock (per). Now the CSPI unit only has a single clock input, so the 100% correct way would be to specify only a single clock for this unit. However, since we handle both units with the same driver the simplest way to cope with it is to provide the very same clock twice, once for the register access and once for generating the SPI bit clock.
And yes, we have to specify the real clock twice and can't replace one with a dummy clock, because if we specify a dummy clock for the ipg clock, then the driver couldn't access the registers even if it enabled the clock. If we would replace the SPI bit clock with a dummy clock then the device couldn't send data even if the driver enabled the bit clock. (That of course only becomes relevant when the driver actually en/disables the clocks during runtime and not only once during probe).

Well, I completely forgot about the clock for registers. Everything looks right then.
However, it would be nice to add a few words about the clocks in the Documentation/devicetree/bindings/spi/fsl-imx~spi.txt Thanks for the clarification.

---

Michael Grzeschik

2013-06-25 15:05 UTC #8 lore raw

PATCH 1/1 ARM: dts: imx: fix clocks for cspi

Hi,

Quoted 59 lines · Alexander Shiyan

Quoted 44 lines
Quoted 29 lines
Quoted 29 lines · Alexander Shiyan
Quoted 28 lines
Quoted 10 lines
Quoted 10 lines
The CSPI controller has only one clock, but the driver spi-imx.c needs clock "per" to calculate bitrate divisor.
Signed-off-by Jonas Andersson <[email protected]>
---
 arch/arm/boot/dts/imx25.dtsi | 12 ++++++------
 arch/arm/boot/dts/imx27.dtsi |  6 +++---
 arch/arm/boot/dts/imx51.dtsi |  2 +-
 arch/arm/boot/dts/imx53.dtsi |  2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)
Quoted 15 lines
Quoted 5 lines
arch/arm/boot/dts/imx25.dtsi +0 -0
index d2550e0..7011539 100644
arch/arm/boot/dts/imx25.dtsi line 148
Quoted 10 lines
arch/arm/boot/dts/imx25.dtsi +2 -2
@@ -141,8 +141,8 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx25-cspi", "fsl,imx35-cspi";
 				reg = <0x43fa4000 0x4000>;
-				clocks = <&clks 62>;
-				clock-names = "ipg";
+				clocks = <&clks 62>, <&clks 62>;
+				clock-names = "ipg", "per";
 				interrupts = <14>;
 				status = "disabled";
 			};
Sorry, typo.
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Why you do not use "dummy" clock for "per" here?
*for "ipg"
Because the same clock is used for the register clock and the baudrate clock. You have to enable the ipg clock to access registers.
I am not see any "ipg" clock usage in the spi driver.
It's the register clock. The driver enables it in order to access the registers.
Quoted 2 lines
If "ipg" clock is
a dependency for "per" clock, it should be registered in ccm as parent.
Quoted 14 lines
Look, the eCSPI unit has two clock inputs, one for accessing the registers (ipg) and one for generating the SPI bit clock (per). Now the CSPI unit only has a single clock input, so the 100% correct way would be to specify only a single clock for this unit. However, since we handle both units with the same driver the simplest way to cope with it is to provide the very same clock twice, once for the register access and once for generating the SPI bit clock.
And yes, we have to specify the real clock twice and can't replace one with a dummy clock, because if we specify a dummy clock for the ipg clock, then the driver couldn't access the registers even if it enabled the clock. If we would replace the SPI bit clock with a dummy clock then the device couldn't send data even if the driver enabled the bit clock. (That of course only becomes relevant when the driver actually en/disables the clocks during runtime and not only once during probe).

Quoted 5 lines

Well, I completely forgot about the clock for registers. Everything looks right then.
However, it would be nice to add a few words about the clocks in the Documentation/devicetree/bindings/spi/fsl-imx~spi.txt Thanks for the clarification.

the spi device i got, only keeps working if having the per2_gate enabled.

It also seems that we lost that dependency, as the file arch/arm/mach-imx/clk-imx27.c contains this:

clk_register_clkdev(clk[per2_gate], "per", "imx27-cspi.0"); clk_register_clkdev(clk[cspi1_ipg_gate], "ipg", "imx27-cspi.0"); clk_register_clkdev(clk[per2_gate], "per", "imx27-cspi.1"); clk_register_clkdev(clk[cspi2_ipg_gate], "ipg", "imx27-cspi.1"); clk_register_clkdev(clk[per2_gate], "per", "imx27-cspi.2"); clk_register_clkdev(clk[cspi3_ipg_gate], "ipg", "imx27-cspi.2");

So my suggestion is as follows:

From: Michael Grzeschik <[email protected]>
Subject: [PATCH] ARM: dts: imx: fix clocks for cspi

The CSPI controller needs the per2_gate clock to work.

Signed-off-by Michael Grzeschik <[email protected]>

---
 arch/arm/boot/dts/imx27.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

arch/arm/boot/dts/imx27.dtsi +3 -3

index 7f7d4a5..14683c3 100644
@@ -169,7 +169,7 @@
 				compatible = "fsl,imx27-cspi";
 				reg = <0x1000e000 0x1000>;
 				interrupts = <16>;
-				clocks = <&clks 53>, <&clks 53>;
+				clocks = <&clks 53>, <&clks 60>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};

@@ -180,7 +180,7 @@
 				compatible = "fsl,imx27-cspi";
 				reg = <0x1000f000 0x1000>;
 				interrupts = <15>;
-				clocks = <&clks 52>, <&clks 52>;
+				clocks = <&clks 52>, <&clks 60>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};

@@ -345,7 +345,7 @@
 				compatible = "fsl,imx27-cspi";
 				reg = <0x10017000 0x1000>;
 				interrupts = <6>;
-				clocks = <&clks 51>, <&clks 51>;
+				clocks = <&clks 51>, <&clks 60>;
 				clock-names = "ipg", "per";
 				status = "disabled";
 			};

Signature · 10 lines

1.8.3.1

Thanks,
Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Shawn Guo

Applied

2013-05-24 05:42 UTC #9 lore raw

PATCH 1/1 ARM: dts: imx: fix clocks for cspi

In reply to Jonas Andersson

Quoted 3 lines · Jonas Andersson

The CSPI controller has only one clock, but the driver spi-imx.c needs clock "per" to calculate bitrate divisor.
Signed-off-by Jonas Andersson <[email protected]>

Applied, thanks.