[PATCH 0/2] arm64: dts: uniphier: UniPhier DT updates for Linux 4.7-rc1

[PATCH 0/2] arm64: dts: uniphier: UniPhier DT updates for Linux 4.7-rc1

[Masahiro Yamada]

Masahiro Yamada (2):
  arm64: dts: uniphier: fix I2C nodes of PH1-LD20
  arm64: dts: uniphier: change release address of spin-table

 .../boot/dts/socionext/uniphier-ph1-ld20-ref.dts   |  1 -
 .../boot/dts/socionext/uniphier-ph1-ld20.dtsi      | 28 +++++++---------------
 2 files changed, 9 insertions(+), 20 deletions(-)

-- 
1.9.1

[PATCH 1/2] arm64: dts: uniphier: fix I2C nodes of PH1-LD20

[Masahiro Yamada]

The I2C hardware blocks on this SoC are connected as follows:

  I2C0: external connection
  I2C1: external connection
  I2C2: internal connection
  I2C3: external connection
  I2C4: external connection
  I2C5: internal connection
  I2C6: no connection (not accessible)

Delete pinctrl from Ch2, add pinctrl to Ch4, and remove the Ch6 node.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 .../boot/dts/socionext/uniphier-ph1-ld20-ref.dts     |  1 -
 arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi | 20 +++++---------------
 2 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20-ref.dts b/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20-ref.dts
index 727ae5f..b0ed443 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20-ref.dts
+++ b/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20-ref.dts
@@ -70,7 +70,6 @@
 		i2c3 = &i2c3;
 		i2c4 = &i2c4;
 		i2c5 = &i2c5;
-		i2c6 = &i2c6;
 	};
 };
 
diff --git a/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi b/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
index e682a3f..651c9d9 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
@@ -201,15 +201,12 @@
 
 		i2c2: i2c@58782000 {
 			compatible = "socionext,uniphier-fi2c";
-			status = "disabled";
 			reg = <0x58782000 0x80>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			interrupts = <0 43 4>;
-			pinctrl-names = "default";
-			pinctrl-0 = <&pinctrl_i2c2>;
 			clocks = <&i2c_clk>;
-			clock-frequency = <100000>;
+			clock-frequency = <400000>;
 		};
 
 		i2c3: i2c@58783000 {
@@ -227,12 +224,15 @@
 
 		i2c4: i2c@58784000 {
 			compatible = "socionext,uniphier-fi2c";
+			status = "disabled";
 			reg = <0x58784000 0x80>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			interrupts = <0 45 4>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_i2c4>;
 			clocks = <&i2c_clk>;
-			clock-frequency = <400000>;
+			clock-frequency = <100000>;
 		};
 
 		i2c5: i2c@58785000 {
@@ -245,16 +245,6 @@
 			clock-frequency = <400000>;
 		};
 
-		i2c6: i2c@58786000 {
-			compatible = "socionext,uniphier-fi2c";
-			reg = <0x58786000 0x80>;
-			#address-cells = <1>;
-			#size-cells = <0>;
-			interrupts = <0 26 4>;
-			clocks = <&i2c_clk>;
-			clock-frequency = <400000>;
-		};
-
 		system_bus: system-bus@58c00000 {
 			compatible = "socionext,uniphier-system-bus";
 			status = "disabled";
-- 
1.9.1

[PATCH 2/2] arm64: dts: uniphier: change release address of spin-table

[Masahiro Yamada]

The 8-byte register located at 0x59801200 on this SoC is dedicated
for waking up secondary CPUs.  We can use it and save normal memory.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi b/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
index 651c9d9..f73b09e 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
@@ -77,7 +77,7 @@
 			compatible = "arm,cortex-a72", "arm,armv8";
 			reg = <0 0x000>;
 			enable-method = "spin-table";
-			cpu-release-addr = <0 0x80000100>;
+			cpu-release-addr = <0 0x59801200>;
 		};
 
 		cpu1: cpu@1 {
@@ -85,7 +85,7 @@
 			compatible = "arm,cortex-a72", "arm,armv8";
 			reg = <0 0x001>;
 			enable-method = "spin-table";
-			cpu-release-addr = <0 0x80000100>;
+			cpu-release-addr = <0 0x59801200>;
 		};
 
 		cpu2: cpu@100 {
@@ -93,7 +93,7 @@
 			compatible = "arm,cortex-a53", "arm,armv8";
 			reg = <0 0x100>;
 			enable-method = "spin-table";
-			cpu-release-addr = <0 0x80000100>;
+			cpu-release-addr = <0 0x59801200>;
 		};
 
 		cpu3: cpu@101 {
@@ -101,7 +101,7 @@
 			compatible = "arm,cortex-a53", "arm,armv8";
 			reg = <0 0x101>;
 			enable-method = "spin-table";
-			cpu-release-addr = <0 0x80000100>;
+			cpu-release-addr = <0 0x59801200>;
 		};
 	};
 
-- 
1.9.1

Re: [PATCH 2/2] arm64: dts: uniphier: change release address of spin-table

[Mark Rutland]

On Fri, Apr 15, 2016 at 07:30:47PM +0900, Masahiro Yamada wrote:
> The 8-byte register located at 0x59801200 on this SoC is dedicated
> for waking up secondary CPUs.  We can use it and save normal memory.

Generally, it is not safe to use MMIO registers to back spin-table. The
kernel maps the spin table location with cacheable attributes, so there
may be speculative accesses to any registes in the same (64K) page, and
a writeback may be larger than the 8-byte register width (which the
device might not accept, triggering an SError).

Given that, I do not think this is a good idea.

Thanks,
Mark.

> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi b/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
> index 651c9d9..f73b09e 100644
> --- a/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
> +++ b/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi
> @@ -77,7 +77,7 @@
>  			compatible = "arm,cortex-a72", "arm,armv8";
>  			reg = <0 0x000>;
>  			enable-method = "spin-table";
> -			cpu-release-addr = <0 0x80000100>;
> +			cpu-release-addr = <0 0x59801200>;
>  		};
>  
>  		cpu1: cpu@1 {
> @@ -85,7 +85,7 @@
>  			compatible = "arm,cortex-a72", "arm,armv8";
>  			reg = <0 0x001>;
>  			enable-method = "spin-table";
> -			cpu-release-addr = <0 0x80000100>;
> +			cpu-release-addr = <0 0x59801200>;
>  		};
>  
>  		cpu2: cpu@100 {
> @@ -93,7 +93,7 @@
>  			compatible = "arm,cortex-a53", "arm,armv8";
>  			reg = <0 0x100>;
>  			enable-method = "spin-table";
> -			cpu-release-addr = <0 0x80000100>;
> +			cpu-release-addr = <0 0x59801200>;
>  		};
>  
>  		cpu3: cpu@101 {
> @@ -101,7 +101,7 @@
>  			compatible = "arm,cortex-a53", "arm,armv8";
>  			reg = <0 0x101>;
>  			enable-method = "spin-table";
> -			cpu-release-addr = <0 0x80000100>;
> +			cpu-release-addr = <0 0x59801200>;
>  		};
>  	};
>  
> -- 
> 1.9.1
> 

Re: [PATCH 2/2] arm64: dts: uniphier: change release address of spin-table

[Masahiro Yamada]

Hi Mark.

2016-04-15 22:05 GMT+09:00 Mark Rutland <mark.rutland@arm.com>:
> On Fri, Apr 15, 2016 at 07:30:47PM +0900, Masahiro Yamada wrote:
>> The 8-byte register located at 0x59801200 on this SoC is dedicated
>> for waking up secondary CPUs.  We can use it and save normal memory.
>
> Generally, it is not safe to use MMIO registers to back spin-table. The
> kernel maps the spin table location with cacheable attributes, so there
> may be speculative accesses to any registes in the same (64K) page, and
> a writeback may be larger than the 8-byte register width (which the
> device might not accept, triggering an SError).
>
> Given that, I do not think this is a good idea.

I did not know this.  Thanks for your advice!


Arnd, Olof

Please drop this patch.
(I think 1/2 is still OK.)



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/2] arm64: dts: uniphier: change release address of spin-table

[Arnd Bergmann]

On Friday 15 April 2016 22:13:55 Masahiro Yamada wrote:
> 2016-04-15 22:05 GMT+09:00 Mark Rutland <mark.rutland@arm.com>:
> > On Fri, Apr 15, 2016 at 07:30:47PM +0900, Masahiro Yamada wrote:
> >> The 8-byte register located at 0x59801200 on this SoC is dedicated
> >> for waking up secondary CPUs.  We can use it and save normal memory.
> >
> > Generally, it is not safe to use MMIO registers to back spin-table. The
> > kernel maps the spin table location with cacheable attributes, so there
> > may be speculative accesses to any registes in the same (64K) page, and
> > a writeback may be larger than the 8-byte register width (which the
> > device might not accept, triggering an SError).
> >
> > Given that, I do not think this is a good idea.
> 
> I did not know this.  Thanks for your advice!
> 
> 
> Arnd, Olof
> 
> Please drop this patch.
> (I think 1/2 is still OK.)
> 

Should patch 1 be applied as a bugfix for 4.6 instead?

	Arnd

Re: [PATCH 2/2] arm64: dts: uniphier: change release address of spin-table

[Masahiro Yamada]

Hi Arnd,


2016-04-16 3:48 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> On Friday 15 April 2016 22:13:55 Masahiro Yamada wrote:
>> 2016-04-15 22:05 GMT+09:00 Mark Rutland <mark.rutland@arm.com>:
>> > On Fri, Apr 15, 2016 at 07:30:47PM +0900, Masahiro Yamada wrote:
>> >> The 8-byte register located at 0x59801200 on this SoC is dedicated
>> >> for waking up secondary CPUs.  We can use it and save normal memory.
>> >
>> > Generally, it is not safe to use MMIO registers to back spin-table. The
>> > kernel maps the spin table location with cacheable attributes, so there
>> > may be speculative accesses to any registes in the same (64K) page, and
>> > a writeback may be larger than the 8-byte register width (which the
>> > device might not accept, triggering an SError).
>> >
>> > Given that, I do not think this is a good idea.
>>
>> I did not know this.  Thanks for your advice!
>>
>>
>> Arnd, Olof
>>
>> Please drop this patch.
>> (I think 1/2 is still OK.)
>>
>
> Should patch 1 be applied as a bugfix for 4.6 instead?
>

Yes, please!


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 1/2] arm64: dts: uniphier: fix I2C nodes of PH1-LD20

[Arnd Bergmann]

On Friday 15 April 2016 19:30:46 Masahiro Yamada wrote:
> The I2C hardware blocks on this SoC are connected as follows:
> 
>   I2C0: external connection
>   I2C1: external connection
>   I2C2: internal connection
>   I2C3: external connection
>   I2C4: external connection
>   I2C5: internal connection
>   I2C6: no connection (not accessible)
> 
> Delete pinctrl from Ch2, add pinctrl to Ch4, and remove the Ch6 node.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> 


Applied patch 1/2 to the fixes branch now, thanks!

	Arnd