[PATCH v3 2/5] arm64: dts: meson: fix PHY deassert timing requirements

2020-12-07 Thread Stefan Agner
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
registers. This fixes an issue seen on ODROID-C2 where the Ethernet
link doesn't come up when using ip link set down/up:
  [ 6630.714855] meson8b-dwmac c941.ethernet eth0: Link is Down
  [ 6630.785775] meson8b-dwmac c941.ethernet eth0: PHY [stmmac-0:00] driver 
[RTL8211F Gigabit Ethernet] (irq=36)
  [ 6630.893071] meson8b-dwmac c941.ethernet: Failed to reset the dma
  [ 6630.893800] meson8b-dwmac c941.ethernet eth0: stmmac_hw_setup: DMA 
engine initialization failed
  [ 6630.902835] meson8b-dwmac c941.ethernet eth0: stmmac_open: Hw setup 
failed

Fixes: f29cabf240ed ("arm64: dts: meson: use the generic Ethernet PHY reset 
GPIO bindings")
Reviewed-by: Martin Blumenstingl 
Signed-off-by: Stefan Agner 
---
 arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts  | 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts   | 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi  | 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi | 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts  | 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts | 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts   | 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxm-q200.dts| 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts| 2 +-
 9 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts 
b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
index 7be3e354093b..de27beafe9db 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
@@ -165,7 +165,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
 
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts 
b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index 70fcfb7b0683..50de1d01e565 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -200,7 +200,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
 
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
index 222ee8069cfa..9b0b81f191f1 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
@@ -126,7 +126,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
 
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
index ad812854a107..a350fee1264d 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
@@ -147,7 +147,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
 
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts 
b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
index b08c4537f260..b2ab05c22090 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
@@ -82,7 +82,7 @@ external_phy: ethernet-phy@0 {
 
/* External PHY reset is shared with internal PHY Led signal */
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
 
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts 
b/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dt

[PATCH v3 5/5] arm64: dts: meson: g12b: w400: fix PHY deassert timing requirements

2020-12-07 Thread Stefan Agner
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
egisters. On similar boards with the same PHY this fixes an issue where
Ethernet link would not come up when using ip link set down/up.

Fixes: 2cd2310fca4c ("arm64: dts: meson-g12b-ugoos-am6: add initial 
device-tree")
Reviewed-by: Martin Blumenstingl 
Signed-off-by: Stefan Agner 
---
 arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
index 2802ddbb83ac..feb088504740 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
@@ -264,7 +264,7 @@ external_phy: ethernet-phy@0 {
max-speed = <1000>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW | 
GPIO_OPEN_DRAIN)>;
 
interrupt-parent = <_intc>;
-- 
2.29.2



[PATCH v3 4/5] arm64: dts: meson: g12a: x96-max: fix PHY deassert timing requirements

2020-12-07 Thread Stefan Agner
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
registers. On similar boards with the same PHY this fixes an issue where
Ethernet link would not come up when using ip link set down/up.

Fixes: ed5e8f689154 ("arm64: dts: meson: g12a: x96-max: fix the Ethernet PHY 
reset line")
Reviewed-by: Martin Blumenstingl 
Signed-off-by: Stefan Agner 
---
 arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts 
b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
index 1b07c8c06eac..463a72d6bb7c 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
@@ -340,7 +340,7 @@ external_phy: ethernet-phy@0 {
eee-broken-1000t;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW | 
GPIO_OPEN_DRAIN)>;
 
interrupt-parent = <_intc>;
-- 
2.29.2



[PATCH v3 1/5] arm64: dts: meson: g12b: odroid-n2: fix PHY deassert timing requirements

2020-12-07 Thread Stefan Agner
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
registers. This fixes an issue where the Ethernet link doesn't come up
when using ip link set down/up:
  [   29.360965] meson8b-dwmac ff3f.ethernet eth0: Link is Down
  [   34.569012] meson8b-dwmac ff3f.ethernet eth0: PHY [0.0:00] driver 
[RTL8211F Gigabit Ethernet] (irq=31)
  [   34.676732] meson8b-dwmac ff3f.ethernet: Failed to reset the dma
  [   34.678874] meson8b-dwmac ff3f.ethernet eth0: stmmac_hw_setup: DMA 
engine initialization failed
  [   34.687850] meson8b-dwmac ff3f.ethernet eth0: stmmac_open: Hw setup 
failed

Fixes: 658e4129bb81 ("arm64: dts: meson: g12b: odroid-n2: add the Ethernet PHY 
reset line")
Reviewed-by: Martin Blumenstingl 
Signed-off-by: Stefan Agner 
---
 arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
index 6982632ae646..39a09661c5f6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
@@ -413,7 +413,7 @@ external_phy: ethernet-phy@0 {
max-speed = <1000>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW | 
GPIO_OPEN_DRAIN)>;
 
interrupt-parent = <_intc>;
-- 
2.29.2



[PATCH v3 3/5] ARM: dts: meson: fix PHY deassert timing requirements

2020-12-07 Thread Stefan Agner
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
registers. On similar boards with the same PHY this fixes an issue where
Ethernet link would not come up when using ip link set down/up.

Fixes: a2c6e82e5341 ("ARM: dts: meson: switch to the generic Ethernet PHY reset 
bindings")
Reviewed-by: Martin Blumenstingl 
Tested-by: Martin Blumenstingl  # on 
Odroid-C1+
Signed-off-by: Stefan Agner 
---
 arch/arm/boot/dts/meson8b-odroidc1.dts| 2 +-
 arch/arm/boot/dts/meson8m2-mxiii-plus.dts | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts 
b/arch/arm/boot/dts/meson8b-odroidc1.dts
index 0c26467de4d0..5963566dbcc9 100644
--- a/arch/arm/boot/dts/meson8b-odroidc1.dts
+++ b/arch/arm/boot/dts/meson8b-odroidc1.dts
@@ -224,7 +224,7 @@ eth_phy: ethernet-phy@0 {
reg = <0>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOH_4 GPIO_ACTIVE_LOW>;
 
interrupt-parent = <_intc>;
diff --git a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts 
b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
index cc498191ddd1..8f4eb1ed4581 100644
--- a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
+++ b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
@@ -81,7 +81,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOH_4 GPIO_ACTIVE_LOW>;
};
};
-- 
2.29.2



Re: [PATCH v2 3/5] ARM: dts: meson: fix PHY deassert timing requirements

2020-12-07 Thread Stefan Agner
On 2020-12-05 14:04, Martin Blumenstingl wrote:
> Hi Stefan,
> 
> On Tue, Dec 1, 2020 at 2:21 PM Stefan Agner  wrote:
>>
>> According to the datasheet (Rev. 1.9) the RTL8211F requires at least
>> 72ms "for internal circuits settling time" before accessing the PHY
>> egisters. On similar boards with the same PHY this fixes an issue where
> there's a typo here: it should be "registers"
> this is the same for the other four patches also

Whoops, will send v3 shortly.

> 
>> Ethernet link would not come up when using ip link set down/up.
> I have never experienced that myself but gotten a few reports about this.
> thank you very much for coming up with info from the datasheet!
> 
> the following stmmac patch [0] has been added recently which may - or
> may not - have any impact also.

Thanks for the hint, wasn't aware of that.

--
Stefan

> 
>> Fixes: a2c6e82e5341 ("ARM: dts: meson: switch to the generic Ethernet PHY 
>> reset bindings")
>> Signed-off-by: Stefan Agner 
> with above typo fixed:
> Reviewed-by: Martin Blumenstingl 
> and also:
> Tested-by: Martin Blumenstingl  #
> on Odroid-C1+
> 
> 
> Best regards,
> Martin
> 
> 
> [0] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/stmicro/stmmac?id=56311a315da7ebc668dbcc2f1c99689cc10796c4


[PATCH v2 4/5] arm64: dts: meson: g12a: x96-max: fix PHY deassert timing requirements

2020-12-01 Thread Stefan Agner
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
egisters. On similar boards with the same PHY this fixes an issue where
Ethernet link would not come up when using ip link set down/up.

Fixes: ed5e8f689154 ("arm64: dts: meson: g12a: x96-max: fix the Ethernet PHY 
reset line")
Signed-off-by: Stefan Agner 
---
 arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts 
b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
index 1b07c8c06eac..463a72d6bb7c 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
@@ -340,7 +340,7 @@ external_phy: ethernet-phy@0 {
eee-broken-1000t;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW | 
GPIO_OPEN_DRAIN)>;
 
interrupt-parent = <_intc>;
-- 
2.29.2



[PATCH v2 5/5] arm64: dts: meson: g12b: w400: fix PHY deassert timing requirements

2020-12-01 Thread Stefan Agner
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
egisters. On similar boards with the same PHY this fixes an issue where
Ethernet link would not come up when using ip link set down/up.

Fixes: 2cd2310fca4c ("arm64: dts: meson-g12b-ugoos-am6: add initial 
device-tree")
Signed-off-by: Stefan Agner 
---
 arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
index 2802ddbb83ac..feb088504740 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
@@ -264,7 +264,7 @@ external_phy: ethernet-phy@0 {
max-speed = <1000>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW | 
GPIO_OPEN_DRAIN)>;
 
interrupt-parent = <_intc>;
-- 
2.29.2



[PATCH v2 3/5] ARM: dts: meson: fix PHY deassert timing requirements

2020-12-01 Thread Stefan Agner
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
egisters. On similar boards with the same PHY this fixes an issue where
Ethernet link would not come up when using ip link set down/up.

Fixes: a2c6e82e5341 ("ARM: dts: meson: switch to the generic Ethernet PHY reset 
bindings")
Signed-off-by: Stefan Agner 
---
 arch/arm/boot/dts/meson8b-odroidc1.dts| 2 +-
 arch/arm/boot/dts/meson8m2-mxiii-plus.dts | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts 
b/arch/arm/boot/dts/meson8b-odroidc1.dts
index 0c26467de4d0..5963566dbcc9 100644
--- a/arch/arm/boot/dts/meson8b-odroidc1.dts
+++ b/arch/arm/boot/dts/meson8b-odroidc1.dts
@@ -224,7 +224,7 @@ eth_phy: ethernet-phy@0 {
reg = <0>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOH_4 GPIO_ACTIVE_LOW>;
 
interrupt-parent = <_intc>;
diff --git a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts 
b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
index cc498191ddd1..8f4eb1ed4581 100644
--- a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
+++ b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
@@ -81,7 +81,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOH_4 GPIO_ACTIVE_LOW>;
};
};
-- 
2.29.2



[PATCH v2 1/5] arm64: dts: meson: g12b: odroid-n2: fix PHY deassert timing requirements

2020-12-01 Thread Stefan Agner
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
egisters. This fixes an issue where the Ethernet link doesn't come up
when using ip link set down/up:
  [   29.360965] meson8b-dwmac ff3f.ethernet eth0: Link is Down
  [   34.569012] meson8b-dwmac ff3f.ethernet eth0: PHY [0.0:00] driver 
[RTL8211F Gigabit Ethernet] (irq=31)
  [   34.676732] meson8b-dwmac ff3f.ethernet: Failed to reset the dma
  [   34.678874] meson8b-dwmac ff3f.ethernet eth0: stmmac_hw_setup: DMA 
engine initialization failed
  [   34.687850] meson8b-dwmac ff3f.ethernet eth0: stmmac_open: Hw setup 
failed

Fixes: 658e4129bb81 ("arm64: dts: meson: g12b: odroid-n2: add the Ethernet PHY 
reset line")
Signed-off-by: Stefan Agner 
---
 arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
index 6982632ae646..39a09661c5f6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
@@ -413,7 +413,7 @@ external_phy: ethernet-phy@0 {
max-speed = <1000>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW | 
GPIO_OPEN_DRAIN)>;
 
interrupt-parent = <_intc>;
-- 
2.29.2



[PATCH v2 2/5] arm64: dts: meson: fix PHY deassert timing requirements

2020-12-01 Thread Stefan Agner
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
egisters. This fixes an issue seen on ODROID-C2 where the Ethernet
link doesn't come up when using ip link set down/up:
  [ 6630.714855] meson8b-dwmac c941.ethernet eth0: Link is Down
  [ 6630.785775] meson8b-dwmac c941.ethernet eth0: PHY [stmmac-0:00] driver 
[RTL8211F Gigabit Ethernet] (irq=36)
  [ 6630.893071] meson8b-dwmac c941.ethernet: Failed to reset the dma
  [ 6630.893800] meson8b-dwmac c941.ethernet eth0: stmmac_hw_setup: DMA 
engine initialization failed
  [ 6630.902835] meson8b-dwmac c941.ethernet eth0: stmmac_open: Hw setup 
failed

Fixes: f29cabf240ed ("arm64: dts: meson: use the generic Ethernet PHY reset 
GPIO bindings")
Signed-off-by: Stefan Agner 
---
 arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts  | 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts   | 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi  | 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi | 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts  | 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts | 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts   | 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxm-q200.dts| 2 +-
 arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts| 2 +-
 9 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts 
b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
index 7be3e354093b..de27beafe9db 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
@@ -165,7 +165,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
 
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts 
b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index 70fcfb7b0683..50de1d01e565 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -200,7 +200,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
 
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
index 222ee8069cfa..9b0b81f191f1 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
@@ -126,7 +126,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
 
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
index ad812854a107..a350fee1264d 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
@@ -147,7 +147,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
 
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts 
b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
index b08c4537f260..b2ab05c22090 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
@@ -82,7 +82,7 @@ external_phy: ethernet-phy@0 {
 
/* External PHY reset is shared with internal PHY Led signal */
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
 
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts 
b/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
index bff8ec2c

Re: [PATCH] arm64: dts: meson: fix PHY deassert timing requirements

2020-12-01 Thread Stefan Agner
On 2020-12-01 09:31, Jerome Brunet wrote:
> On Tue 01 Dec 2020 at 01:25, Stefan Agner  wrote:
> 
>> According to the datasheet (Rev. 1.4, page 30) the RTL8211F requires
>> at least 50ms "for internal circuits settling time" before accessing
>> the PHY registers. This fixes an issue where the Ethernet link doesn't
>> come up when using ip link set down/up:
>>   [   29.360965] meson8b-dwmac ff3f.ethernet eth0: Link is Down
>>   [   34.569012] meson8b-dwmac ff3f.ethernet eth0: PHY [0.0:00] driver 
>> [RTL8211F Gigabit Ethernet] (irq=31)
>>   [   34.676732] meson8b-dwmac ff3f.ethernet: Failed to reset the dma
>>   [   34.678874] meson8b-dwmac ff3f.ethernet eth0: stmmac_hw_setup: DMA 
>> engine initialization failed
>>   [   34.687850] meson8b-dwmac ff3f.ethernet eth0: stmmac_open: Hw setup 
>> failed
>>
>> Fixes: 658e4129bb81 ("arm64: dts: meson: g12b: odroid-n2: add the Ethernet 
>> PHY reset line")
>> Signed-off-by: Stefan Agner 
>> ---
>>  arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi 
>> b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
>> index 6982632ae646..a5652caacb27 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
>> @@ -413,7 +413,7 @@ external_phy: ethernet-phy@0 {
>>  max-speed = <1000>;
>>
>>  reset-assert-us = <1>;
>> -reset-deassert-us = <3>;
>> +reset-deassert-us = <5>;
>>  reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW | 
>> GPIO_OPEN_DRAIN)>;
>>
>>  interrupt-parent = <_intc>;
> 
> Thanks for sharing this is Stefan,
> The title of your patch should probably be modified to show that it
> addresses the odroid n2 only, as it stands.

Yes make sense. Hm, are there other boards with RTL8211F? From the
comments in the DT it seems several other boards use the same PHY. Some
however do not have any reset timing data at all currently it seems.

> 
> I have checked the RTL8211F doc I have, v1.9, and this one mention
> "72ms at least - not including the 1.0V supply rise time" before
> accessing the PHY registers :/ ... so 80ms maybe ?

Uh interesting, so it seems they increased it over documentation
revisions. Yeah agreed 80ms is the safer value then.

FWIW, I did test it with 50ms in a continuous loop for an hour or so
without seeing any failure, but that was room temperature only.

--
Stefan


[PATCH] arm64: dts: meson: fix PHY deassert timing requirements

2020-11-30 Thread Stefan Agner
According to the datasheet (Rev. 1.4, page 30) the RTL8211F requires
at least 50ms "for internal circuits settling time" before accessing
the PHY registers. This fixes an issue where the Ethernet link doesn't
come up when using ip link set down/up:
  [   29.360965] meson8b-dwmac ff3f.ethernet eth0: Link is Down
  [   34.569012] meson8b-dwmac ff3f.ethernet eth0: PHY [0.0:00] driver 
[RTL8211F Gigabit Ethernet] (irq=31)
  [   34.676732] meson8b-dwmac ff3f.ethernet: Failed to reset the dma
  [   34.678874] meson8b-dwmac ff3f.ethernet eth0: stmmac_hw_setup: DMA 
engine initialization failed
  [   34.687850] meson8b-dwmac ff3f.ethernet eth0: stmmac_open: Hw setup 
failed

Fixes: 658e4129bb81 ("arm64: dts: meson: g12b: odroid-n2: add the Ethernet PHY 
reset line")
Signed-off-by: Stefan Agner 
---
 arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
index 6982632ae646..a5652caacb27 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
@@ -413,7 +413,7 @@ external_phy: ethernet-phy@0 {
max-speed = <1000>;
 
reset-assert-us = <1>;
-   reset-deassert-us = <3>;
+   reset-deassert-us = <5>;
reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW | 
GPIO_OPEN_DRAIN)>;
 
interrupt-parent = <_intc>;
-- 
2.29.2



Re: [PATCH] mm/zsmalloc: include sparsemem.h for MAX_PHYSMEM_BITS

2020-11-10 Thread Stefan Agner
[adding Russell King for ARM]

On 2020-11-10 12:21, Arnd Bergmann wrote:
> On Tue, Nov 10, 2020 at 10:58 AM Mike Rapoport  wrote:
>> > >
>> > > asm/sparsemem.h is not available on some architectures.
>> > > It's better to use linux/mmzone.h instead.
> 
> Ah, I missed that, too.
> 
>> > Hm, linux/mmzone.h only includes asm/sparsemem.h when CONFIG_SPARSEMEM
>> > is enabled. However, on ARM at least I can have configurations without
>> > CONFIG_SPARSEMEM and physical address extension on (e.g.
>> > multi_v7_defconfig + CONFIG_LPAE + CONFIG_ZSMALLOC).
>> >
>> > While sparsemem seems to be a good idea with LPAE it really seems not
>> > required (see also https://lore.kernel.org/patchwork/patch/567589/).
>> >
>> > There seem to be also other architectures which define MAX_PHYSMEM_BITS
>> > only when SPARSEMEM is enabled, e.g.
>> > arch/riscv/include/asm/sparsemem.h...
>> >
>> > Not sure how to get out of this.. Maybe make ZSMALLOC dependent on
>> > SPARSEMEM? It feels a bit silly restricting ZSMALLOC selection only due
>> > to a compile time define...
>>
>> I think we can define MAX_POSSIBLE_PHYSMEM_BITS in one of
>> arch/arm/inclide/asm/pgtable-{2,3}level-*.h headers to values supported
>> by !LPAE and LPAE.

Hm I see mm/zsmalloc.c really only needs to know how many bits are
potentially used to calculate how many bits it can use for object
indexing.

> 
> Good idea. I wonder what other architectures need the same though.
> Here are some I found:
> 
> $ git grep -l PHYS_ADDR_T_64BIT arch | grep Kconfig
> arch/arc/Kconfig
> arch/arm/mm/Kconfig
> arch/mips/Kconfig
> arch/powerpc/platforms/Kconfig.cputype
> arch/x86/Kconfig
> 
> arch/arc has a CONFIG_ARC_HAS_PAE40 option
> arch/riscv has 34-bit addressing in rv32 mode
> arch/mips has up to 40 bits with mips32r3 XPA, but I don't know what
> supports that
> 
> arch/powerpc has this:
> config PHYS_64BIT
> bool 'Large physical address support' if E500 || PPC_86xx
> depends on (44x || E500 || PPC_86xx) && !PPC_83xx && !PPC_82xx
> 
> Apparently all three (4xx, e500v2, mpc86xx/e600) do 36-bit physical
> addressing, but each one has a different page table format.
> 
> Microblaze has physical address extensions, but neither those nor
> 64-bit mode have so far made it into the kernel.
> 
> To be on the safe side, we could provoke a compile-time error
> when CONFIG_PHYS_ADDR_T_64BIT is set on a 32-bit
> architecture, but MAX_POSSIBLE_PHYSMEM_BITS is not set.
> 
>> That's what x86 does:
>>
>> $ git grep -w MAX_POSSIBLE_PHYSMEM_BITS arch/
>> arch/x86/include/asm/pgtable-3level_types.h:#define 
>> MAX_POSSIBLE_PHYSMEM_BITS   36
> 
> Doesn't x86 also support a 40-bit addressing mode? I suppose
> those machines that actually used it are long gone.
> 
>> arch/x86/include/asm/pgtable_64_types.h:#define MAX_POSSIBLE_PHYSMEM_BITS
>>52
>>
>> It seems that actual numbers would be 36 for !LPAE and 40 for LPAE, but
>> I'm not sure about that.
> 
> Close enough, yes.
> 
> The 36-bit addressing is on !LPAE is only used for early static mappings,
> so I think we can pretend it's always 32-bit. I checked the ARMv8 reference,
> and it says that ARMv8-Aarch32 actually supports 40 bit physical addressing
> both with non-LPAE superpages (short descriptor format) and LPAE (long
> descriptor format), but Linux only does 36-bit addressing on superpages
> as specified for ARMv6/ARMv7 short descriptors.

Oh so, more than 4GB of memory can be supported by !LPAE systems via
superpages? Wasn't aware of that.

Since only ARM_LPAE selects CONFIG_PHYS_ADDR_T_64BIT it really is safe
to assume 32 bits for non-LPAE systems.

I guess that would mean adding a #define MAX_POSSIBLE_PHYSMEM_BITS 32 to
arch/arm/include/asm/pgtable-2level.h and a MAX_POSSIBLE_PHYSMEM_BITS 40
in arch/arm/include/asm/pgtable-3level.h. Seems straight forward and
would solve the problem I had. I can prepare a patch for ARM, not sure
about the other architectures...

--
Stefan


Re: [PATCH] mm/zsmalloc: include sparsemem.h for MAX_PHYSMEM_BITS

2020-11-10 Thread Stefan Agner
On 2020-11-08 07:46, Mike Rapoport wrote:
> On Sat, Nov 07, 2020 at 04:22:06PM +0100, Stefan Agner wrote:
>> Most architectures define MAX_PHYSMEM_BITS in asm/sparsemem.h and don't
>> include it in asm/pgtable.h. Include asm/sparsemem.h directly to get
>> the MAX_PHYSMEM_BITS define on all architectures.
>>
>> This fixes a crash when accessing zram on 32-bit ARM platform with LPAE and
>> more than 4GB of memory:
>>   Unable to handle kernel NULL pointer dereference at virtual address 
>> 
>>   pgd = a27bd01c
>>   [] *pgd=236a0003, *pmd=1ffa64003
>>   Internal error: Oops: 207 [#1] SMP ARM
>>   Modules linked in: mdio_bcm_unimac(+) brcmfmac cfg80211 brcmutil 
>> raspberrypi_hwmon hci_uart crc32_arm_ce bcm2711_thermal phy_generic genet
>>   CPU: 0 PID: 123 Comm: mkfs.ext4 Not tainted 5.9.6 #1
>>   Hardware name: BCM2711
>>   PC is at zs_map_object+0x94/0x338
>>   LR is at zram_bvec_rw.constprop.0+0x330/0xa64
>>   pc : []lr : []psr: 6013
>>   sp : e376bbe0  ip :   fp : c1e2921c
>>   r10: 0002  r9 : c1dda730  r8 : 
>>   r7 : e8ff7a00  r6 :   r5 : 02f9ffa0  r4 : e371
>>   r3 : 000fdffe  r2 : c1e0ce80  r1 : ebf979a0  r0 : 
>>   Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
>>   Control: 30c5383d  Table: 235c2a80  DAC: fffd
>>   Process mkfs.ext4 (pid: 123, stack limit = 0x495a22e6)
>>   Stack: (0xe376bbe0 to 0xe376c000)
>>   ...
>>   [] (zs_map_object) from [] 
>> (zram_bvec_rw.constprop.0+0x330/0xa64)
>>   [] (zram_bvec_rw.constprop.0) from [] 
>> (zram_submit_bio+0x1a4/0x40c)
>>   [] (zram_submit_bio) from [] 
>> (submit_bio_noacct+0xd0/0x3c8)
>>   [] (submit_bio_noacct) from [] (submit_bio+0x4c/0x190)
>>   [] (submit_bio) from [] (submit_bh_wbc+0x188/0x1b8)
>>   [] (submit_bh_wbc) from [] 
>> (__block_write_full_page+0x340/0x5e4)
>>   [] (__block_write_full_page) from [] 
>> (block_write_full_page+0x128/0x170)
>>   [] (block_write_full_page) from [] 
>> (__writepage+0x14/0x68)
>>   [] (__writepage) from [] 
>> (write_cache_pages+0x1bc/0x494)
>>   [] (write_cache_pages) from [] 
>> (generic_writepages+0x58/0x8c)
>>   [] (generic_writepages) from [] 
>> (do_writepages+0x48/0xec)
>>   [] (do_writepages) from [] 
>> (__filemap_fdatawrite_range+0xf0/0x128)
>>   [] (__filemap_fdatawrite_range) from [] 
>> (file_write_and_wait_range+0x48/0x98)
>>   [] (file_write_and_wait_range) from [] 
>> (blkdev_fsync+0x1c/0x44)
>>   [] (blkdev_fsync) from [] (do_fsync+0x3c/0x70)
>>   [] (do_fsync) from [] (__sys_trace_return+0x0/0x2c)
>>   Exception stack(0xe376bfa8 to 0xe376bff0)
>>   bfa0:   0003d2e0 b6f7b6f0 0003 00046e40 1000 
>> 
>>   bfc0: 0003d2e0 b6f7b6f0  0076   befcbb20 
>> befcbb28
>>   bfe0: b6f4e060 befcbad8 b6f23e0c b6dc4a80
>>   Code: e5927000 e0050391 e0871005 e5918018 (e5983000)
>>
>> Fixes: 61989a80fb3a ("staging: zsmalloc: zsmalloc memory allocation library")
>> Signed-off-by: Stefan Agner 
>> ---
>>  mm/zsmalloc.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
>> index c36fdff9a371..260bd48aacd0 100644
>> --- a/mm/zsmalloc.c
>> +++ b/mm/zsmalloc.c
>> @@ -40,6 +40,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
> 
> asm/sparsemem.h is not available on some architectures.
> It's better to use linux/mmzone.h instead.
> 

Hm, linux/mmzone.h only includes asm/sparsemem.h when CONFIG_SPARSEMEM
is enabled. However, on ARM at least I can have configurations without
CONFIG_SPARSEMEM and physical address extension on (e.g.
multi_v7_defconfig + CONFIG_LPAE + CONFIG_ZSMALLOC).

While sparsemem seems to be a good idea with LPAE it really seems not
required (see also https://lore.kernel.org/patchwork/patch/567589/).

There seem to be also other architectures which define MAX_PHYSMEM_BITS
only when SPARSEMEM is enabled, e.g.
arch/riscv/include/asm/sparsemem.h...

Not sure how to get out of this.. Maybe make ZSMALLOC dependent on
SPARSEMEM? It feels a bit silly restricting ZSMALLOC selection only due
to a compile time define...

--
Stefan

>>  #include 
>>  #include 
>>  #include 
>> --
>> 2.29.1
>>
>>


Re: [PATCH] mm/zsmalloc: include sparsemem.h for MAX_PHYSMEM_BITS

2020-11-07 Thread Stefan Agner
On 2020-11-08 01:56, Andrew Morton wrote:
> On Sat,  7 Nov 2020 16:22:06 +0100 Stefan Agner  wrote:
> 
>> Most architectures define MAX_PHYSMEM_BITS in asm/sparsemem.h and don't
>> include it in asm/pgtable.h. Include asm/sparsemem.h directly to get
>> the MAX_PHYSMEM_BITS define on all architectures.
>>
>> This fixes a crash when accessing zram on 32-bit ARM platform with LPAE and
>> more than 4GB of memory:
>>   Unable to handle kernel NULL pointer dereference at virtual address 
>> 
> 
> Mysterious.  Presumably without this include, some compilation unit is
> picking up the wrong value of MAX_PHYSMEM_BITS?  But I couldn't
> actually see where/how this occurs.  Can you please explain further?

Not sure if I got that right, but from what I understand if
MAX_PHYSMEM_BITS is not set in mm/zsmalloc.c it will set
MAX_PHYSMEM_BITS to BITS_PER_LONG. And this is 32-bit, too short when
LPAE is in use...

--
Stefan


[PATCH] mm/zsmalloc: include sparsemem.h for MAX_PHYSMEM_BITS

2020-11-07 Thread Stefan Agner
Most architectures define MAX_PHYSMEM_BITS in asm/sparsemem.h and don't
include it in asm/pgtable.h. Include asm/sparsemem.h directly to get
the MAX_PHYSMEM_BITS define on all architectures.

This fixes a crash when accessing zram on 32-bit ARM platform with LPAE and
more than 4GB of memory:
  Unable to handle kernel NULL pointer dereference at virtual address 
  pgd = a27bd01c
  [] *pgd=236a0003, *pmd=1ffa64003
  Internal error: Oops: 207 [#1] SMP ARM
  Modules linked in: mdio_bcm_unimac(+) brcmfmac cfg80211 brcmutil 
raspberrypi_hwmon hci_uart crc32_arm_ce bcm2711_thermal phy_generic genet
  CPU: 0 PID: 123 Comm: mkfs.ext4 Not tainted 5.9.6 #1
  Hardware name: BCM2711
  PC is at zs_map_object+0x94/0x338
  LR is at zram_bvec_rw.constprop.0+0x330/0xa64
  pc : []lr : []psr: 6013
  sp : e376bbe0  ip :   fp : c1e2921c
  r10: 0002  r9 : c1dda730  r8 : 
  r7 : e8ff7a00  r6 :   r5 : 02f9ffa0  r4 : e371
  r3 : 000fdffe  r2 : c1e0ce80  r1 : ebf979a0  r0 : 
  Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
  Control: 30c5383d  Table: 235c2a80  DAC: fffd
  Process mkfs.ext4 (pid: 123, stack limit = 0x495a22e6)
  Stack: (0xe376bbe0 to 0xe376c000)
  ...
  [] (zs_map_object) from [] 
(zram_bvec_rw.constprop.0+0x330/0xa64)
  [] (zram_bvec_rw.constprop.0) from [] 
(zram_submit_bio+0x1a4/0x40c)
  [] (zram_submit_bio) from [] 
(submit_bio_noacct+0xd0/0x3c8)
  [] (submit_bio_noacct) from [] (submit_bio+0x4c/0x190)
  [] (submit_bio) from [] (submit_bh_wbc+0x188/0x1b8)
  [] (submit_bh_wbc) from [] 
(__block_write_full_page+0x340/0x5e4)
  [] (__block_write_full_page) from [] 
(block_write_full_page+0x128/0x170)
  [] (block_write_full_page) from [] (__writepage+0x14/0x68)
  [] (__writepage) from [] (write_cache_pages+0x1bc/0x494)
  [] (write_cache_pages) from [] 
(generic_writepages+0x58/0x8c)
  [] (generic_writepages) from [] (do_writepages+0x48/0xec)
  [] (do_writepages) from [] 
(__filemap_fdatawrite_range+0xf0/0x128)
  [] (__filemap_fdatawrite_range) from [] 
(file_write_and_wait_range+0x48/0x98)
  [] (file_write_and_wait_range) from [] 
(blkdev_fsync+0x1c/0x44)
  [] (blkdev_fsync) from [] (do_fsync+0x3c/0x70)
  [] (do_fsync) from [] (__sys_trace_return+0x0/0x2c)
  Exception stack(0xe376bfa8 to 0xe376bff0)
  bfa0:   0003d2e0 b6f7b6f0 0003 00046e40 1000 
  bfc0: 0003d2e0 b6f7b6f0  0076   befcbb20 befcbb28
  bfe0: b6f4e060 befcbad8 b6f23e0c b6dc4a80
  Code: e5927000 e0050391 e0871005 e5918018 (e5983000)

Fixes: 61989a80fb3a ("staging: zsmalloc: zsmalloc memory allocation library")
Signed-off-by: Stefan Agner 
---
 mm/zsmalloc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index c36fdff9a371..260bd48aacd0 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-- 
2.29.1



Re: [PATCH v3 0/3] Support NVIDIA Tegra-based Ouya game console

2020-10-07 Thread Stefan Agner
Hi Peter,

On 2020-10-04 15:31, Peter Geis wrote:
> Good Day,
> 
> This series introduces upstream kernel support for the Ouya game
> console device. Please review and apply. Thank you in advance.

Interesting patchset, maybe I can give my Ouya a second live now :-) Do
you happen to have (a link) to instructions how to flash the device?

Btw, there was also a driver for the Bluetooth controller on the ML
once, maybe a good time to revive that:
https://spinics.net/lists/linux-input/msg56288.html

--
Stefan

> 
> Changelog:
> v3: - Reorder aliases per Dmitry Osipenko's review.
> - Add sdio clocks per Dmitry Osipenko's review.
> - Add missing ti sleep bits per Dmitry Osipenko's review.
> - Enable lp1 sleep mode.
> - Fix bluetooth comment and add missing power supplies.
> 
> v2: - Update pmic and clock handles per Rob Herring's review.
> - Add acks from Rob Herring to patch 2 and 3.
> 
> Peter Geis (3):
>   ARM: tegra: Add device-tree for Ouya
>   dt-bindings: Add vendor prefix for Ouya Inc.
>   dt-bindings: ARM: tegra: Add Ouya game console
> 
>  .../devicetree/bindings/arm/tegra.yaml|3 +
>  .../devicetree/bindings/vendor-prefixes.yaml  |2 +
>  arch/arm/boot/dts/Makefile|3 +-
>  arch/arm/boot/dts/tegra30-ouya.dts| 4511 +
>  4 files changed, 4518 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/tegra30-ouya.dts


Re: [PATCH][next] mtd: rawnand: Replace one-element array with flexible-array member

2020-10-01 Thread Stefan Agner
On 2020-10-01 10:12, Miquel Raynal wrote:
> Hi Jann,
> 
> Jann Horn  wrote on Thu, 1 Oct 2020 00:32:24 +0200:
> 
>> On Wed, Sep 30, 2020 at 11:30 PM Gustavo A. R. Silva
>>  wrote:
>> > On Wed, Sep 30, 2020 at 11:10:43PM +0200, Jann Horn wrote:
>> > > On Wed, Sep 30, 2020 at 11:02 PM Gustavo A. R. Silva
>> > >  wrote:
>> > > > There is a regular need in the kernel to provide a way to declare 
>> > > > having
>> > > > a dynamically sized set of trailing elements in a structure. Kernel 
>> > > > code
>> > > > should always use “flexible array members”[1] for these cases. The 
>> > > > older
>> > > > style of one-element or zero-length arrays should no longer be used[2].
>> > >
>> > > But this is not such a case, right? Isn't this a true fixed-size
>> > > array? It sounds like you're just changing it because it
>> > > pattern-matched on "array of length 1 at the end of a struct".
>> >
>> > Yeah; I should have changed that 'dynamically' part of the text above
>> > a bit. However, as I commented in the text below, in the case that more
>> > CS IDs are needed (let's wait for the maintainers to comment on this...)
>> > in the future, this change makes the code more maintainable, as for
>> > the allocation part, the developer would only have to update the CS_N
>> > macro to the number of CS IDs that are needed.
>>
>> But in that case, shouldn't you change it to "int cs[CS_N]" and get
>> rid of the struct_size() stuff?
> 
> I do agree with Jann, I think it's best to consider this a fixed-size
> array for now. If we ever want to extend the number of supported CS,
> there is much more rework involved anyway.

I agree, too, just assume this is a fixed-size array of 1 element.

In fact, I am not aware of a design which needs multiple chip selects.

--
Stefan


Re: [PATCH v3] drm: mxsfb: check framebuffer pitch

2020-09-16 Thread Stefan Agner
On 2020-09-08 16:16, Stefan Agner wrote:
> The lcdif IP does not support a framebuffer pitch (stride) other than
> framebuffer width. Check for equality and reject the framebuffer
> otherwise.
> 
> This prevents a distorted picture when using 640x800 and running the
> Mesa graphics stack. Mesa tries to use a cache aligned stride, which
> leads at that particular resolution to width != stride. Currently
> Mesa has no fallback behavior, but rejecting this configuration allows
> userspace to handle the issue correctly.
> 
> Fixes: 45d59d704080 ("drm: Add new driver for MXSFB controller")
> Signed-off-by: Stefan Agner 
> Reviewed-by: Laurent Pinchart 

Applied to drm-misc-next.

--
Stefan

> ---
> Changes in v3:
> - Fix typo in commit message
> - Add fixes tag
> 
> Changes in v2:
> - Validate pitch in fb_create callback
> 
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 21 -
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index 8c549c3931af..35122aef037b 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -81,8 +82,26 @@ void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
>   clk_disable_unprepare(mxsfb->clk_axi);
>  }
>  
> +static struct drm_framebuffer *
> +mxsfb_fb_create(struct drm_device *dev, struct drm_file *file_priv,
> + const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> + const struct drm_format_info *info;
> +
> + info = drm_get_format_info(dev, mode_cmd);
> + if (!info)
> + return ERR_PTR(-EINVAL);
> +
> + if (mode_cmd->width * info->cpp[0] != mode_cmd->pitches[0]) {
> + dev_dbg(dev->dev, "Invalid pitch: fb width must match pitch\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + return drm_gem_fb_create(dev, file_priv, mode_cmd);
> +}
> +
>  static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = {
> - .fb_create  = drm_gem_fb_create,
> + .fb_create  = mxsfb_fb_create,
>   .atomic_check   = drm_atomic_helper_check,
>   .atomic_commit  = drm_atomic_helper_commit,
>  };


Re: [PATCH] drm: mxsfb: check framebuffer pitch

2020-09-08 Thread Stefan Agner
On 2020-09-08 10:48, Daniel Vetter wrote:
> On Tue, Sep 08, 2020 at 11:18:25AM +0300, Tomi Valkeinen wrote:
>> Hi,
>>
>> On 08/09/2020 10:55, Stefan Agner wrote:
>> > On 2020-09-07 20:18, Daniel Vetter wrote:
>> >> On Mon, Sep 07, 2020 at 07:17:12PM +0300, Laurent Pinchart wrote:
>> >>> Hi Stefan,
>> >>>
>> >>> Thank you for the patch.
>> >>>
>> >>> On Mon, Sep 07, 2020 at 06:03:43PM +0200, Stefan Agner wrote:
>> >>>> The lcdif IP does not support a framebuffer pitch (stride) other than
>> >>>> the CRTC width. Check for equality and reject the state otherwise.
>> >>>>
>> >>>> This prevents a distorted picture when using 640x800 and running the
>> >>>> Mesa graphics stack. Mesa tires to use a cache aligned stride, which
>> >>>
>> >>> s/tires/tries/
>> >>>
>> >>>> leads at that particular resolution to width != stride. Currently
>> >>>> Mesa has no fallback behavior, but rejecting this configuration allows
>> >>>> userspace to handle the issue correctly.
>> >>>
>> >>> I'm increasingly impressed by how featureful this IP core is :-)
>> >>>
>> >>>> Signed-off-by: Stefan Agner 
>> >>>> ---
>> >>>>  drivers/gpu/drm/mxsfb/mxsfb_kms.c | 22 ++
>> >>>>  1 file changed, 18 insertions(+), 4 deletions(-)
>> >>>>
>> >>>> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c 
>> >>>> b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
>> >>>> index b721b8b262ce..79aa14027f91 100644
>> >>>> --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
>> >>>> +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
>> >>>> @@ -403,14 +403,28 @@ static int mxsfb_plane_atomic_check(struct 
>> >>>> drm_plane *plane,
>> >>>>  {
>> >>>> struct mxsfb_drm_private *mxsfb = 
>> >>>> to_mxsfb_drm_private(plane->dev);
>> >>>> struct drm_crtc_state *crtc_state;
>> >>>> +   unsigned int pitch;
>> >>>> +   int ret;
>> >>>>
>> >>>> crtc_state = drm_atomic_get_new_crtc_state(plane_state->state,
>> >>>>>crtc);
>> >>>>
>> >>>> -   return drm_atomic_helper_check_plane_state(plane_state, 
>> >>>> crtc_state,
>> >>>> -  
>> >>>> DRM_PLANE_HELPER_NO_SCALING,
>> >>>> -  
>> >>>> DRM_PLANE_HELPER_NO_SCALING,
>> >>>> -  false, true);
>> >>>> +   ret = drm_atomic_helper_check_plane_state(plane_state, 
>> >>>> crtc_state,
>> >>>> + 
>> >>>> DRM_PLANE_HELPER_NO_SCALING,
>> >>>> + 
>> >>>> DRM_PLANE_HELPER_NO_SCALING,
>> >>>> + false, true);
>> >>>> +   if (ret || !plane_state->visible)
>> >>>
>> >>> Would it be more explict to check for !plane_state->fb ? Otherwise I'll
>> >>> have to verify that !fb always implies !visible :-)
>> >>>
>> >>>> +   return ret;
>> >>>> +
>> >>>> +   pitch = crtc_state->mode.hdisplay *
>> >>>> +   plane_state->fb->format->cpp[0];
>> >>>
>> >>> This holds on a single line.
>> >>>
>> >>>> +   if (plane_state->fb->pitches[0] != pitch) {
>> >>>> +   dev_err(plane->dev->dev,
>> >>>> +   "Invalid pitch: fb and crtc widths must be the 
>> >>>> same");
>> >>>
>> >>> I'd turn this into a dev_dbg(), printing error messages to the kernel
>> >>> log in response to user-triggered conditions is a bit too verbose and
>> >>> could flood the log.
>> >>>
>> >>> Wouldn't it be best to catch this issue when creating the framebuffer ?
>> >>
>> >> Yeah this should be verified at addfb time. We try to validate as early as
>> >> possible.
>> >> -Daniel
>> >>
>> >
>> > Sounds sensible. From what I can tell fb_create is the proper callback
>> > to implement this at addfb time. Will give this a try.
>> >
>> > FWIW, I got the idea from drivers/gpu/drm/tilcdc/tilcdc_plane.c. Maybe
>> > should be moved to addfb there too?
>>
>> But you don't know the crtc width when creating the framebuffer.
> 
> Hm right this is a different check. What we could check in fb_create for
> both is that the logical fb size matches exactly the pitch. That's not
> sufficient criteria, but it will at least catch some of them already.
> 
> But yeah we'd need both here.

After validating width of framebuffer against pitch, the only thing we
need to check here is that the width matches. From what I can tell,
least for mxsfb, this should be covered by
drm_atomic_helper_check_plane_state's can_position parameter set to
false.

So I think in my case I can get away by only checking the framebuffer.

--
Stefan


> -Daniel
> 
>>
>>  Tomi
>>
>> --
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


[PATCH v3] drm: mxsfb: check framebuffer pitch

2020-09-08 Thread Stefan Agner
The lcdif IP does not support a framebuffer pitch (stride) other than
framebuffer width. Check for equality and reject the framebuffer
otherwise.

This prevents a distorted picture when using 640x800 and running the
Mesa graphics stack. Mesa tries to use a cache aligned stride, which
leads at that particular resolution to width != stride. Currently
Mesa has no fallback behavior, but rejecting this configuration allows
userspace to handle the issue correctly.

Fixes: 45d59d704080 ("drm: Add new driver for MXSFB controller")
Signed-off-by: Stefan Agner 
Reviewed-by: Laurent Pinchart 
---
Changes in v3:
- Fix typo in commit message
- Add fixes tag

Changes in v2:
- Validate pitch in fb_create callback

 drivers/gpu/drm/mxsfb/mxsfb_drv.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c 
b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 8c549c3931af..35122aef037b 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -81,8 +82,26 @@ void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
clk_disable_unprepare(mxsfb->clk_axi);
 }
 
+static struct drm_framebuffer *
+mxsfb_fb_create(struct drm_device *dev, struct drm_file *file_priv,
+   const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+   const struct drm_format_info *info;
+
+   info = drm_get_format_info(dev, mode_cmd);
+   if (!info)
+   return ERR_PTR(-EINVAL);
+
+   if (mode_cmd->width * info->cpp[0] != mode_cmd->pitches[0]) {
+   dev_dbg(dev->dev, "Invalid pitch: fb width must match pitch\n");
+   return ERR_PTR(-EINVAL);
+   }
+
+   return drm_gem_fb_create(dev, file_priv, mode_cmd);
+}
+
 static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = {
-   .fb_create  = drm_gem_fb_create,
+   .fb_create  = mxsfb_fb_create,
.atomic_check   = drm_atomic_helper_check,
.atomic_commit  = drm_atomic_helper_commit,
 };
-- 
2.28.0



Re: [PATCH] drm: mxsfb: check framebuffer pitch

2020-09-08 Thread Stefan Agner
On 2020-09-08 14:33, Laurent Pinchart wrote:
> On Tue, Sep 08, 2020 at 02:29:02PM +0200, Daniel Vetter wrote:
>> On Tue, Sep 8, 2020 at 2:07 PM Stefan Agner  wrote:
>> > On 2020-09-08 10:48, Daniel Vetter wrote:
>> >> On Tue, Sep 08, 2020 at 11:18:25AM +0300, Tomi Valkeinen wrote:
>> >>> On 08/09/2020 10:55, Stefan Agner wrote:
>> >>>> On 2020-09-07 20:18, Daniel Vetter wrote:
>> >>>>> On Mon, Sep 07, 2020 at 07:17:12PM +0300, Laurent Pinchart wrote:
>> >>>>>> Hi Stefan,
>> >>>>>>
>> >>>>>> Thank you for the patch.
>> >>>>>>
>> >>>>>> On Mon, Sep 07, 2020 at 06:03:43PM +0200, Stefan Agner wrote:
>> >>>>>>> The lcdif IP does not support a framebuffer pitch (stride) other than
>> >>>>>>> the CRTC width. Check for equality and reject the state otherwise.
>> >>>>>>>
>> >>>>>>> This prevents a distorted picture when using 640x800 and running the
>> >>>>>>> Mesa graphics stack. Mesa tires to use a cache aligned stride, which
>> >>>>>>
>> >>>>>> s/tires/tries/
>> >>>>>>
>> >>>>>>> leads at that particular resolution to width != stride. Currently
>> >>>>>>> Mesa has no fallback behavior, but rejecting this configuration 
>> >>>>>>> allows
>> >>>>>>> userspace to handle the issue correctly.
>> >>>>>>
>> >>>>>> I'm increasingly impressed by how featureful this IP core is :-)
>> >>>>>>
>> >>>>>>> Signed-off-by: Stefan Agner 
>> >>>>>>> ---
>> >>>>>>>  drivers/gpu/drm/mxsfb/mxsfb_kms.c | 22 ++
>> >>>>>>>  1 file changed, 18 insertions(+), 4 deletions(-)
>> >>>>>>>
>> >>>>>>> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c 
>> >>>>>>> b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
>> >>>>>>> index b721b8b262ce..79aa14027f91 100644
>> >>>>>>> --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
>> >>>>>>> +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
>> >>>>>>> @@ -403,14 +403,28 @@ static int mxsfb_plane_atomic_check(struct 
>> >>>>>>> drm_plane *plane,
>> >>>>>>>  {
>> >>>>>>> struct mxsfb_drm_private *mxsfb = 
>> >>>>>>> to_mxsfb_drm_private(plane->dev);
>> >>>>>>> struct drm_crtc_state *crtc_state;
>> >>>>>>> +   unsigned int pitch;
>> >>>>>>> +   int ret;
>> >>>>>>>
>> >>>>>>> crtc_state = 
>> >>>>>>> drm_atomic_get_new_crtc_state(plane_state->state,
>> >>>>>>>>crtc);
>> >>>>>>>
>> >>>>>>> -   return drm_atomic_helper_check_plane_state(plane_state, 
>> >>>>>>> crtc_state,
>> >>>>>>> -  
>> >>>>>>> DRM_PLANE_HELPER_NO_SCALING,
>> >>>>>>> -  
>> >>>>>>> DRM_PLANE_HELPER_NO_SCALING,
>> >>>>>>> -  false, true);
>> >>>>>>> +   ret = drm_atomic_helper_check_plane_state(plane_state, 
>> >>>>>>> crtc_state,
>> >>>>>>> + 
>> >>>>>>> DRM_PLANE_HELPER_NO_SCALING,
>> >>>>>>> + 
>> >>>>>>> DRM_PLANE_HELPER_NO_SCALING,
>> >>>>>>> + false, true);
>> >>>>>>> +   if (ret || !plane_state->visible)
>> >>>>>>
>> >>>>>> Would it be more explict to check for !plane_state->fb ? Otherwise 
>> >>>>>> I'll
>> >>>>>> have to verify that !fb always implies !visible :-)
>

[PATCH v2] drm: mxsfb: check framebuffer pitch

2020-09-08 Thread Stefan Agner
The lcdif IP does not support a framebuffer pitch (stride) other than
framebuffer width. Check for equality and reject the framebuffer
otherwise.

This prevents a distorted picture when using 640x800 and running the
Mesa graphics stack. Mesa tires to use a cache aligned stride, which
leads at that particular resolution to width != stride. Currently
Mesa has no fallback behavior, but rejecting this configuration allows
userspace to handle the issue correctly.

Signed-off-by: Stefan Agner 
---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c 
b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 8c549c3931af..fa6798d21029 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -81,8 +82,26 @@ void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
clk_disable_unprepare(mxsfb->clk_axi);
 }
 
+static struct drm_framebuffer *
+mxsfb_fb_create(struct drm_device *dev, struct drm_file *file_priv,
+ const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+   const struct drm_format_info *info;
+
+   info = drm_get_format_info(dev, mode_cmd);
+   if (!info)
+   return ERR_PTR(-EINVAL);
+
+   if (mode_cmd->width * info->cpp[0] != mode_cmd->pitches[0]) {
+   dev_dbg(dev->dev, "Invalid pitch: fb width must match pitch\n");
+   return ERR_PTR(-EINVAL);
+   }
+
+   return drm_gem_fb_create(dev, file_priv, mode_cmd);
+}
+
 static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = {
-   .fb_create  = drm_gem_fb_create,
+   .fb_create  = mxsfb_fb_create,
.atomic_check   = drm_atomic_helper_check,
.atomic_commit  = drm_atomic_helper_commit,
 };
-- 
2.28.0



Re: [PATCH] drm: mxsfb: check framebuffer pitch

2020-09-08 Thread Stefan Agner
On 2020-09-07 20:18, Daniel Vetter wrote:
> On Mon, Sep 07, 2020 at 07:17:12PM +0300, Laurent Pinchart wrote:
>> Hi Stefan,
>>
>> Thank you for the patch.
>>
>> On Mon, Sep 07, 2020 at 06:03:43PM +0200, Stefan Agner wrote:
>> > The lcdif IP does not support a framebuffer pitch (stride) other than
>> > the CRTC width. Check for equality and reject the state otherwise.
>> >
>> > This prevents a distorted picture when using 640x800 and running the
>> > Mesa graphics stack. Mesa tires to use a cache aligned stride, which
>>
>> s/tires/tries/
>>
>> > leads at that particular resolution to width != stride. Currently
>> > Mesa has no fallback behavior, but rejecting this configuration allows
>> > userspace to handle the issue correctly.
>>
>> I'm increasingly impressed by how featureful this IP core is :-)
>>
>> > Signed-off-by: Stefan Agner 
>> > ---
>> >  drivers/gpu/drm/mxsfb/mxsfb_kms.c | 22 ++
>> >  1 file changed, 18 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c 
>> > b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
>> > index b721b8b262ce..79aa14027f91 100644
>> > --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
>> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
>> > @@ -403,14 +403,28 @@ static int mxsfb_plane_atomic_check(struct drm_plane 
>> > *plane,
>> >  {
>> >struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(plane->dev);
>> >struct drm_crtc_state *crtc_state;
>> > +  unsigned int pitch;
>> > +  int ret;
>> >
>> >crtc_state = drm_atomic_get_new_crtc_state(plane_state->state,
>> >   >crtc);
>> >
>> > -  return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
>> > - DRM_PLANE_HELPER_NO_SCALING,
>> > - DRM_PLANE_HELPER_NO_SCALING,
>> > - false, true);
>> > +  ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state,
>> > +DRM_PLANE_HELPER_NO_SCALING,
>> > +DRM_PLANE_HELPER_NO_SCALING,
>> > +false, true);
>> > +  if (ret || !plane_state->visible)
>>
>> Would it be more explict to check for !plane_state->fb ? Otherwise I'll
>> have to verify that !fb always implies !visible :-)
>>
>> > +  return ret;
>> > +
>> > +  pitch = crtc_state->mode.hdisplay *
>> > +  plane_state->fb->format->cpp[0];
>>
>> This holds on a single line.
>>
>> > +  if (plane_state->fb->pitches[0] != pitch) {
>> > +  dev_err(plane->dev->dev,
>> > +  "Invalid pitch: fb and crtc widths must be the same");
>>
>> I'd turn this into a dev_dbg(), printing error messages to the kernel
>> log in response to user-triggered conditions is a bit too verbose and
>> could flood the log.
>>
>> Wouldn't it be best to catch this issue when creating the framebuffer ?
> 
> Yeah this should be verified at addfb time. We try to validate as early as
> possible.
> -Daniel
> 

Sounds sensible. From what I can tell fb_create is the proper callback
to implement this at addfb time. Will give this a try.

FWIW, I got the idea from drivers/gpu/drm/tilcdc/tilcdc_plane.c. Maybe
should be moved to addfb there too?

[added Jyri/Tomi]

--
Stefan

>>
>> > +  return -EINVAL;
>> > +  }
>> > +
>> > +  return 0;
>> >  }
>> >
>> >  static void mxsfb_plane_primary_atomic_update(struct drm_plane *plane,
>>
>> --
>> Regards,
>>
>> Laurent Pinchart


[PATCH] drm: mxsfb: check framebuffer pitch

2020-09-07 Thread Stefan Agner
The lcdif IP does not support a framebuffer pitch (stride) other than
the CRTC width. Check for equality and reject the state otherwise.

This prevents a distorted picture when using 640x800 and running the
Mesa graphics stack. Mesa tires to use a cache aligned stride, which
leads at that particular resolution to width != stride. Currently
Mesa has no fallback behavior, but rejecting this configuration allows
userspace to handle the issue correctly.

Signed-off-by: Stefan Agner 
---
 drivers/gpu/drm/mxsfb/mxsfb_kms.c | 22 ++
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c 
b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
index b721b8b262ce..79aa14027f91 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
@@ -403,14 +403,28 @@ static int mxsfb_plane_atomic_check(struct drm_plane 
*plane,
 {
struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(plane->dev);
struct drm_crtc_state *crtc_state;
+   unsigned int pitch;
+   int ret;
 
crtc_state = drm_atomic_get_new_crtc_state(plane_state->state,
   >crtc);
 
-   return drm_atomic_helper_check_plane_state(plane_state, crtc_state,
-  DRM_PLANE_HELPER_NO_SCALING,
-  DRM_PLANE_HELPER_NO_SCALING,
-  false, true);
+   ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true);
+   if (ret || !plane_state->visible)
+   return ret;
+
+   pitch = crtc_state->mode.hdisplay *
+   plane_state->fb->format->cpp[0];
+   if (plane_state->fb->pitches[0] != pitch) {
+   dev_err(plane->dev->dev,
+   "Invalid pitch: fb and crtc widths must be the same");
+   return -EINVAL;
+   }
+
+   return 0;
 }
 
 static void mxsfb_plane_primary_atomic_update(struct drm_plane *plane,
-- 
2.28.0



[PATCH v2] clk: meson: g12a: mark fclk_div2 as critical

2020-08-28 Thread Stefan Agner
On Amlogic Meson G12b platform, similar to fclk_div3, the fclk_div2
seems to be necessary for the system to operate correctly as well.

Typically, the clock also gets chosen by the eMMC peripheral. This
probably masked the problem so far. However, when booting from a SD
card the clock seems to get disabled which leads to a system freeze.

Let's mark this clock as critical, fixing boot from SD card on G12b
platforms.

Fixes: 085a4ea93d54 ("clk: meson: g12a: add peripheral clock controller")
Cc: Marek Szyprowski 
Signed-off-by: Stefan Agner 
Tested-by: Anand Moon 
---
 drivers/clk/meson/g12a.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index fad616cac01e..6d44cadc06af 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -298,6 +298,17 @@ static struct clk_regmap g12a_fclk_div2 = {
_fclk_div2_div.hw
},
.num_parents = 1,
+   /*
+* Similar to fclk_div3, it seems that this clock is used by
+* the resident firmware and is required by the platform to
+* operate correctly.
+* Until the following condition are met, we need this clock to
+* be marked as critical:
+* a) Mark the clock used by a firmware resource, if possible
+* b) CCF has a clock hand-off mechanism to make the sure the
+*clock stays on until the proper driver comes along
+*/
+   .flags = CLK_IS_CRITICAL,
},
 };
 
-- 
2.28.0



[PATCH] clk: meson: g12a: mark fclk_div2 as critical

2020-08-27 Thread Stefan Agner
On Amlogic Meson G12b platform, similar to fclk_div3, the fclk_div2
seems to be necessary for the system to operate correctly as well.

Typically, the clock also gets chosen by the eMMC peripheral. This
probably masked the problem so far. However, when booting from a SD
card the clock seems to get disabled which leads to a system freeze.

Let's mark this clock as critical, fixing boot from SD card on G12b
platforms.

Signed-off-by: Stefan Agner 
---
 drivers/clk/meson/g12a.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index fad616cac01e..2214b974f748 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -298,6 +298,7 @@ static struct clk_regmap g12a_fclk_div2 = {
_fclk_div2_div.hw
},
.num_parents = 1,
+   .flags = CLK_IS_CRITICAL,
},
 };
 
-- 
2.28.0



Re: [PATCH v4] dt-bindings: nvmem: Add syscon to Vybrid OCOTP driver

2020-08-25 Thread Stefan Agner
On 2020-08-25 05:04, Chris Healy wrote:
> From: Chris Healy 
> 
> Add syscon compatibility with Vybrid OCOTP driver binding. This is
> required to access the UID.
> 
> Fixes: 623069946952 ("nvmem: Add DT binding documentation for Vybrid
> OCOTP driver")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Chris Healy 

Reviewed-by: Stefan Agner 

> ---
> Changes in v4:
>  - Point to the appropriate commit for the Fixes: line
>  - Update the Required Properties to add the "syscon" compatible
> ---
>  Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt
> b/Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt
> index 56ed481c3e26..72ba628f6d0b 100644
> --- a/Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt
> +++ b/Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt
> @@ -2,7 +2,7 @@ On-Chip OTP Memory for Freescale Vybrid
>  
>  Required Properties:
>compatible:
> -  - "fsl,vf610-ocotp" for VF5xx/VF6xx
> +  - "fsl,vf610-ocotp", "syscon" for VF5xx/VF6xx
>#address-cells : Should be 1
>#size-cells : Should be 1
>reg : Address and length of OTP controller and fuse map registers
> @@ -11,7 +11,7 @@ Required Properties:
>  Example for Vybrid VF5xx/VF6xx:
>  
>   ocotp: ocotp@400a5000 {
> - compatible = "fsl,vf610-ocotp";
> + compatible = "fsl,vf610-ocotp", "syscon";
>   #address-cells = <1>;
>   #size-cells = <1>;
>   reg = <0x400a5000 0xCF0>;


Re: [PATCH v2] ARM: dts: vfxxx: Add syscon compatible with ocotp

2020-08-21 Thread Stefan Agner
On 2020-08-21 16:13, Chris Healy wrote:
> On Fri, Aug 21, 2020 at 6:21 AM Stefan Agner  wrote:
>>
>> On 2020-08-20 06:10, Chris Healy wrote:
>> > From: Chris Healy 
>> >
>> > Add syscon compatibility with Vybrid ocotp node. This is required to
>> > access the UID.
>>
>> Hm, it seems today the SoC driver uses the specific compatible. It also
>> should expose the UID as soc_id, see drivers/soc/imx/soc-imx.c.
>>
> Yes, until I added syscon, the soc_id was empty and I would get the
> following line in dmesg:  "failed to find vf610-ocotp regmap!
> 

Ah I see, it looks up syscon, so that requires syscon to be in
compatible.

>> Maybe it does make sense exposing it as syscon, but then we should
>> probably also adjust
>> Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt.
>>
> Makes sense.  I will update vf610-ocotp.txt in v3.  Tnx
> 

Ok, thx. With that you can add Reviewed-by: Stefan Agner
 as well.

--
Stefan

>> --
>> Stefan
>>
>> >
>> > Fixes: fa8d20c8dbb77 ("ARM: dts: vfxxx: Add node corresponding to OCOTP")
>> > Cc: sta...@vger.kernel.org
>> > Signed-off-by: Chris Healy 
>> > ---
>> > Changes in v2:
>> >  - Add Fixes line to commit message
>> >
>> >  arch/arm/boot/dts/vfxxx.dtsi | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
>> > index 0fe03aa0367f..2259d11af721 100644
>> > --- a/arch/arm/boot/dts/vfxxx.dtsi
>> > +++ b/arch/arm/boot/dts/vfxxx.dtsi
>> > @@ -495,7 +495,7 @@ edma1: dma-controller@40098000 {
>> >   };
>> >
>> >   ocotp: ocotp@400a5000 {
>> > - compatible = "fsl,vf610-ocotp";
>> > + compatible = "fsl,vf610-ocotp", "syscon";
>> >   reg = <0x400a5000 0x1000>;
>> >   clocks = < VF610_CLK_OCOTP>;
>> >   };


Re: [PATCH] drm: fsl-dcu: enable PIXCLK on LS1021A

2020-08-21 Thread Stefan Agner
Hi Matthias,

On 2020-08-20 12:58, Matthias Schiffer wrote:
> The PIXCLK needs to be enabled in SCFG before accessing the DCU on LS1021A,
> or the access will hang.

Hm, this seems a rather ad-hoc access to SCFG from the DCU. We do
support a pixel clock in the device tree bindings of fsl-dcu, so ideally
we should enable the pixel clock through the clock framework.

On the other hand, I guess that would mean adding a clock driver to flip
a single bit, which seems a bit excessive too.

I'd like a second opinion on that. Adding clk framework maintainers.

--
Stefan

> 
> Signed-off-by: Matthias Schiffer 
> ---
>  drivers/gpu/drm/fsl-dcu/Kconfig   |  1 +
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 25 +++
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  3 +++
>  3 files changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/fsl-dcu/Kconfig b/drivers/gpu/drm/fsl-dcu/Kconfig
> index d7dd8ba90e3a..9e5a35e7c00c 100644
> --- a/drivers/gpu/drm/fsl-dcu/Kconfig
> +++ b/drivers/gpu/drm/fsl-dcu/Kconfig
> @@ -8,6 +8,7 @@ config DRM_FSL_DCU
>   select DRM_PANEL
>   select REGMAP_MMIO
>   select VIDEOMODE_HELPERS
> + select MFD_SYSCON if SOC_LS1021A
>   help
> Choose this option if you have an Freescale DCU chipset.
> If M is selected the module will be called fsl-dcu-drm.
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> index abbc1ddbf27f..8a7556655581 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> @@ -51,6 +51,23 @@ static const struct regmap_config fsl_dcu_regmap_config = {
>   .volatile_reg = fsl_dcu_drm_is_volatile_reg,
>  };
>  
> +static int fsl_dcu_scfg_config_ls1021a(struct device_node *np)
> +{
> + struct regmap *scfg;
> +
> + scfg = syscon_regmap_lookup_by_compatible("fsl,ls1021a-scfg");
> + if (IS_ERR(scfg))
> + return PTR_ERR(scfg);
> +
> + /*
> +  * For simplicity, enable the PIXCLK unconditionally. It might
> +  * be possible to disable the clock in PM or on unload as a future
> +  * improvement.
> +  */
> + return regmap_update_bits(scfg, SCFG_PIXCLKCR, SCFG_PIXCLKCR_PXCEN,
> +   SCFG_PIXCLKCR_PXCEN);
> +}
> +
>  static void fsl_dcu_irq_uninstall(struct drm_device *dev)
>  {
>   struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
> @@ -70,6 +87,14 @@ static int fsl_dcu_load(struct drm_device *dev,
> unsigned long flags)
>   return ret;
>   }
>  
> + if (of_device_is_compatible(fsl_dev->np, "fsl,ls1021a-dcu")) {
> + ret = fsl_dcu_scfg_config_ls1021a(fsl_dev->np);
> + if (ret < 0) {
> + dev_err(dev->dev, "failed to enable pixclk\n");
> + goto done;
> + }
> + }
> +
>   ret = drm_vblank_init(dev, dev->mode_config.num_crtc);
>   if (ret < 0) {
>   dev_err(dev->dev, "failed to initialize vblank\n");
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> index e2049a0e8a92..566396013c04 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> @@ -160,6 +160,9 @@
>  #define FSL_DCU_ARGB 12
>  #define FSL_DCU_YUV422   14
>  
> +#define SCFG_PIXCLKCR0x28
> +#define SCFG_PIXCLKCR_PXCEN  BIT(31)
> +
>  #define VF610_LAYER_REG_NUM  9
>  #define LS1021A_LAYER_REG_NUM10


Re: [PATCH v2] ARM: dts: vfxxx: Add syscon compatible with ocotp

2020-08-21 Thread Stefan Agner
On 2020-08-20 06:10, Chris Healy wrote:
> From: Chris Healy 
> 
> Add syscon compatibility with Vybrid ocotp node. This is required to
> access the UID.

Hm, it seems today the SoC driver uses the specific compatible. It also
should expose the UID as soc_id, see drivers/soc/imx/soc-imx.c.

Maybe it does make sense exposing it as syscon, but then we should
probably also adjust
Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt.

--
Stefan

> 
> Fixes: fa8d20c8dbb77 ("ARM: dts: vfxxx: Add node corresponding to OCOTP")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Chris Healy 
> ---
> Changes in v2:
>  - Add Fixes line to commit message
> 
>  arch/arm/boot/dts/vfxxx.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
> index 0fe03aa0367f..2259d11af721 100644
> --- a/arch/arm/boot/dts/vfxxx.dtsi
> +++ b/arch/arm/boot/dts/vfxxx.dtsi
> @@ -495,7 +495,7 @@ edma1: dma-controller@40098000 {
>   };
>  
>   ocotp: ocotp@400a5000 {
> - compatible = "fsl,vf610-ocotp";
> + compatible = "fsl,vf610-ocotp", "syscon";
>   reg = <0x400a5000 0x1000>;
>   clocks = < VF610_CLK_OCOTP>;
>   };


Re: [PATCH 3/3] arm64: dts: meson: add support for the ODROID-N2+

2020-07-22 Thread Stefan Agner
Hi Christian,

On 2020-07-19 16:10, Christian Hewitt wrote:
> HardKernel ODROID-N2+ uses an Amlogic S922X rev. C chip capable of higher
> clock speeds than the original ODROID-N2. Hardkernel supports the big cpu
> cluster at 2.4GHz and the little cpu cluster at 2.0GHz. Opp points and
> regulator changess are from the HardKernel Linux kernel sources.

According to the ODROID wiki those values are already in the
overclocking range:
https://wiki.odroid.com/odroid-n2/hardware/overclocking

>From what I can tell, for ODROID-N2 upstream Linux so far used defaults
from meson-g12b-s922x.dtsi, which were 1896MHz for the A53 and 1704MHz
for the A73 (so it seems currently the A73 running even 100MHz below
"Stock").

I guess we should pick either Stock or Overclock for the two models.
Unless there is another good reason not to?

--
Stefan

> 
> Suggested-by: Dongjin Kim 
> Signed-off-by: Christian Hewitt 
> ---
>  arch/arm64/boot/dts/amlogic/Makefile  |  1 +
>  .../dts/amlogic/meson-g12b-odroid-n2-plus.dts | 53 +++
>  2 files changed, 54 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2-plus.dts
> 
> diff --git a/arch/arm64/boot/dts/amlogic/Makefile
> b/arch/arm64/boot/dts/amlogic/Makefile
> index 5cac4d1d487d..6dc508b80133 100644
> --- a/arch/arm64/boot/dts/amlogic/Makefile
> +++ b/arch/arm64/boot/dts/amlogic/Makefile
> @@ -8,6 +8,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking-pro.dtb
>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-a311d-khadas-vim3.dtb
>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-s922x-khadas-vim3.dtb
>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2.dtb
> +dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2-plus.dtb
>  dtb-$(CONFIG_ARCH_MESON) += meson-g12b-ugoos-am6.dtb
>  dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-kii-pro.dtb
>  dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-nanopi-k2.dtb
> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2-plus.dts
> b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2-plus.dts
> new file mode 100644
> index ..99e96be509f8
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2-plus.dts
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2019 BayLibre, SAS
> + * Author: Neil Armstrong 
> + */
> +
> +/dts-v1/;
> +
> +#include "meson-g12b-odroid-n2.dtsi"
> +
> +/ {
> + compatible = "hardkernel,odroid-n2-plus", "amlogic,s922x", 
> "amlogic,g12b";
> + model = "Hardkernel ODROID-N2+";
> +
> + vddcpu_a: regulator-vddcpu-a {
> + regulator-min-microvolt = <68>;
> + regulator-max-microvolt = <104>;
> +
> + pwms = <_ab 0 1500 0>;
> + };
> +
> + vddcpu_b: regulator-vddcpu-b {
> + regulator-min-microvolt = <68>;
> + regulator-max-microvolt = <104>;
> +
> + pwms = <_AO_cd 1 1500 0>;
> + };
> +
> + cpu_opp_table_0: opp-table-0 {
> + opp-190800 {
> + opp-hz = /bits/ 64 <190800>;
> + opp-microvolt = <103>;
> + };
> +
> + opp-201600 {
> + opp-hz = /bits/ 64 <201600>;
> + opp-microvolt = <104>;
> + };
> + };
> +
> + cpub_opp_table_1: opp-table-1 {
> + opp-230400 {
> + opp-hz = /bits/ 64 <230400>;
> + opp-microvolt = <103>;
> + };
> +
> + opp-24 {
> + opp-hz = /bits/ 64 <24>;
> + opp-microvolt = <104>;
> + };
> + };
> +};
> +


Re: [PATCH] drm/mxsfb: Make supported modifiers explicit

2020-07-20 Thread Stefan Agner
On 2020-07-18 19:14, Guido Günther wrote:
> Hi,
> On Mon, Mar 23, 2020 at 04:51:05PM +0100, Lucas Stach wrote:
>> Am Montag, den 23.03.2020, 15:52 +0100 schrieb Guido Günther:
>> > In contrast to other display controllers on imx like DCSS and ipuv3
>> > lcdif/mxsfb does not support detiling e.g. vivante tiled layouts.
>> > Since mesa might assume otherwise make it explicit that only
>> > DRM_FORMAT_MOD_LINEAR is supported.
>> >
>> > Signed-off-by: Guido Günther 
>>
>> Reviewed-by: Lucas Stach 
> 
> Can i do anything to get this applied?
> Cheers,
>  -- Guido

Sorry about the delay, I was thinking to apply it with another patchset
which is not ready though.

Pushed this patch to drm-misc-next just now.

--
Stefan

> 
>>
>> > ---
>> >  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 9 +++--
>> >  1 file changed, 7 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c 
>> > b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>> > index 762379530928..fc71e7a7a02e 100644
>> > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
>> > @@ -73,6 +73,11 @@ static const uint32_t mxsfb_formats[] = {
>> >DRM_FORMAT_RGB565
>> >  };
>> >
>> > +static const uint64_t mxsfb_modifiers[] = {
>> > +  DRM_FORMAT_MOD_LINEAR,
>> > +  DRM_FORMAT_MOD_INVALID
>> > +};
>> > +
>> >  static struct mxsfb_drm_private *
>> >  drm_pipe_to_mxsfb_drm_private(struct drm_simple_display_pipe *pipe)
>> >  {
>> > @@ -334,8 +339,8 @@ static int mxsfb_load(struct drm_device *drm, unsigned 
>> > long flags)
>> >}
>> >
>> >ret = drm_simple_display_pipe_init(drm, >pipe, _funcs,
>> > -  mxsfb_formats, ARRAY_SIZE(mxsfb_formats), NULL,
>> > -  mxsfb->connector);
>> > +  mxsfb_formats, ARRAY_SIZE(mxsfb_formats),
>> > +  mxsfb_modifiers, mxsfb->connector);
>> >if (ret < 0) {
>> >dev_err(drm->dev, "Cannot setup simple display pipe\n");
>> >goto err_vblank;
>>


Re: [PATCH] drm/mxsfb: miss err handle in probe

2020-07-20 Thread Stefan Agner
On 2020-06-11 14:23, Bernard Zhao wrote:
> There are three err return values in drm_fbdev_generic_setup.
> In mxsfb_probe we called this function, but didn`t handle the
> return value, this change is to add err handle, maybe make code
> a bit more readable.

This got recently changed, so I guess checking the return value isn't
required anymore:
https://patchwork.freedesktop.org/patch/msgid/20200408082641.590-11-tzimmerm...@suse.de

--
Stefan

> 
> Signed-off-by: Bernard Zhao 
> ---
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index 497cf443a9af..a45f3b85f725 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -415,7 +415,9 @@ static int mxsfb_probe(struct platform_device *pdev)
>   if (ret)
>   goto err_unload;
>  
> - drm_fbdev_generic_setup(drm, 32);
> + ret = drm_fbdev_generic_setup(drm, 32);
> + if (ret)
> + goto err_unload;
>  
>   return 0;


[PATCH v3 2/3] ARM: use VFP assembler mnemonics in register load/store macros

2020-06-26 Thread Stefan Agner
The integrated assembler of Clang 10 and earlier do not allow to access
the VFP registers through the coprocessor load/store instructions:
:4:6: error: invalid operand for instruction
 LDC p11, cr0, [r10],#32*4 @ FLDMIAD r10!, {d0-d15}
 ^

This has been addressed with Clang 11 [0]. However, to support earlier
versions of Clang and for better readability use of VFP assembler
mnemonics still is preferred.

Replace the coprocessor load/store instructions with explicit assembler
mnemonics to accessing the floating point coprocessor registers. Use
assembler directives to select the appropriate FPU version.

This allows to build these macros with GNU assembler as well as with
Clang's built-in assembler.

[0] https://reviews.llvm.org/D59733

Link: https://github.com/ClangBuiltLinux/linux/issues/905
Signed-off-by: Stefan Agner 
---
Changes in v3:
- Reworded commit message, adding hint that Clang 11 won't have this
  restriction any longer

Changes in v2:
- Add link in commit message

 arch/arm/include/asm/vfpmacros.h | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/arch/arm/include/asm/vfpmacros.h b/arch/arm/include/asm/vfpmacros.h
index 628c336e8e3b..947ee5395e1f 100644
--- a/arch/arm/include/asm/vfpmacros.h
+++ b/arch/arm/include/asm/vfpmacros.h
@@ -19,23 +19,25 @@
 
@ read all the working registers back into the VFP
.macro  VFPFLDMIA, base, tmp
+   .fpuvfpv2
 #if __LINUX_ARM_ARCH__ < 6
-   LDC p11, cr0, [\base],#33*4 @ FLDMIAX \base!, {d0-d15}
+   fldmiax \base!, {d0-d15}
 #else
-   LDC p11, cr0, [\base],#32*4 @ FLDMIAD \base!, {d0-d15}
+   vldmia  \base!, {d0-d15}
 #endif
 #ifdef CONFIG_VFPv3
+   .fpuvfpv3
 #if __LINUX_ARM_ARCH__ <= 6
ldr \tmp, =elf_hwcap@ may not have MVFR regs
ldr \tmp, [\tmp, #0]
tst \tmp, #HWCAP_VFPD32
-   ldclne  p11, cr0, [\base],#32*4 @ FLDMIAD \base!, {d16-d31}
+   vldmiane \base!, {d16-d31}
addeq   \base, \base, #32*4 @ step over unused register 
space
 #else
VFPFMRX \tmp, MVFR0 @ Media and VFP Feature 
Register 0
and \tmp, \tmp, #MVFR0_A_SIMD_MASK  @ A_SIMD field
cmp \tmp, #2@ 32 x 64bit registers?
-   ldcleq  p11, cr0, [\base],#32*4 @ FLDMIAD \base!, {d16-d31}
+   vldmiaeq \base!, {d16-d31}
addne   \base, \base, #32*4 @ step over unused register 
space
 #endif
 #endif
@@ -44,22 +46,23 @@
@ write all the working registers out of the VFP
.macro  VFPFSTMIA, base, tmp
 #if __LINUX_ARM_ARCH__ < 6
-   STC p11, cr0, [\base],#33*4 @ FSTMIAX \base!, {d0-d15}
+   fstmiax \base!, {d0-d15}
 #else
-   STC p11, cr0, [\base],#32*4 @ FSTMIAD \base!, {d0-d15}
+   vstmia  \base!, {d0-d15}
 #endif
 #ifdef CONFIG_VFPv3
+   .fpuvfpv3
 #if __LINUX_ARM_ARCH__ <= 6
ldr \tmp, =elf_hwcap@ may not have MVFR regs
ldr \tmp, [\tmp, #0]
tst \tmp, #HWCAP_VFPD32
-   stclne  p11, cr0, [\base],#32*4 @ FSTMIAD \base!, {d16-d31}
+   vstmiane \base!, {d16-d31}
addeq   \base, \base, #32*4 @ step over unused register 
space
 #else
VFPFMRX \tmp, MVFR0 @ Media and VFP Feature 
Register 0
and \tmp, \tmp, #MVFR0_A_SIMD_MASK  @ A_SIMD field
cmp \tmp, #2@ 32 x 64bit registers?
-   stcleq  p11, cr0, [\base],#32*4 @ FSTMIAD \base!, {d16-d31}
+   vstmiaeq \base!, {d16-d31}
addne   \base, \base, #32*4 @ step over unused register 
space
 #endif
 #endif
-- 
2.27.0



[PATCH v3 0/3] ARM: make use of UAL VFP mnemonics when possible

2020-06-26 Thread Stefan Agner
To build the kernel with the integrated assembler of Clang 10 and earlier
the VFP code needs to make use of the unified assembler language (UAL)
VFP mnemonics.

As pointed out by Russell, even for ARMv7 blocking VFP access through
MCR/MRC is not correct. This has been addressed in upstream Clang and
VFP access will be possible through MCR/MRC (see [0]).

At first I tried to replace all co-processor instructions to access the
floating point unit along with the macros. However, due to missing
FPINST/FPINST2 argument support in older binutils versions we have to
keep them around. Once we drop support for binutils 2.24 and older, the
move to UAL VFP mnemonics will be straight forward with this changes
applied.

Tested using Clang with integrated assembler as well as external
(binutils assembler), various gcc/binutils version down to 4.7/2.23.
Disassembled and compared the object files in arch/arm/vfp/ to make
sure this changes leads to the same code. Besides different inlining
behavior I was not able to spot a difference.

In v2 the check for FPINST argument support is now made in Kconfig.

In v3 reworded commit message and addressed review feedback in patch 1.

--
Stefan

[0] https://reviews.llvm.org/D59733

Stefan Agner (3):
  ARM: use .fpu assembler directives instead of assembler arguments
  ARM: use VFP assembler mnemonics in register load/store macros
  ARM: use VFP assembler mnemonics if available

 arch/arm/Kconfig |  2 ++
 arch/arm/Kconfig.assembler   |  6 ++
 arch/arm/include/asm/vfp.h   |  2 ++
 arch/arm/include/asm/vfpmacros.h | 31 ++-
 arch/arm/vfp/Makefile|  2 --
 arch/arm/vfp/vfphw.S | 31 +--
 arch/arm/vfp/vfpinstr.h  | 23 +++
 7 files changed, 72 insertions(+), 25 deletions(-)
 create mode 100644 arch/arm/Kconfig.assembler

-- 
2.27.0



[PATCH v3 3/3] ARM: use VFP assembler mnemonics if available

2020-06-26 Thread Stefan Agner
The integrated assembler of Clang 10 and earlier do not allow to access
the VFP registers through the coprocessor load/store instructions:
arch/arm/vfp/vfpmodule.c:342:2: error: invalid operand for instruction
fmxr(FPEXC, fpexc & 
~(FPEXC_EX|FPEXC_DEX|FPEXC_FP2V|FPEXC_VV|FPEXC_TRAP_MASK));
^
arch/arm/vfp/vfpinstr.h:79:6: note: expanded from macro 'fmxr'
asm("mcr p10, 7, %0, " vfpreg(_vfp_) ", cr0, 0 @ fmxr   " #_vfp_ ", %0" 
\
^
:1:6: note: instantiated into assembly here
mcr p10, 7, r0, cr8, cr0, 0 @ fmxr  FPEXC, r0
^

This has been addressed with Clang 11 [0]. However, to support earlier
versions of Clang and for better readability use of VFP assembler
mnemonics still is preferred.

Ideally we would replace this code with the unified assembler language
mnemonics vmrs/vmsr on call sites along with .fpu assembler directives.
The GNU assembler supports the .fpu directive at least since 2.17 (when
documentation has been added). Since Linux requires binutils 2.21 it is
safe to use .fpu directive. However, binutils does not allow to use
FPINST or FPINST2 as an argument to vmrs/vmsr instructions up to
binutils 2.24 (see binutils commit 16d02dc907c5):
arch/arm/vfp/vfphw.S: Assembler messages:
arch/arm/vfp/vfphw.S:162: Error: operand 0 must be FPSID or FPSCR pr FPEXC -- 
`vmsr FPINST,r6'
arch/arm/vfp/vfphw.S:165: Error: operand 0 must be FPSID or FPSCR pr FPEXC -- 
`vmsr FPINST2,r8'
arch/arm/vfp/vfphw.S:235: Error: operand 1 must be a VFP extension System 
Register -- `vmrs r3,FPINST'
arch/arm/vfp/vfphw.S:238: Error: operand 1 must be a VFP extension System 
Register -- `vmrs r12,FPINST2'

Use as-instr in Kconfig to check if FPINST/FPINST2 can be used. If they
can be used make use of .fpu directives and UAL VFP mnemonics for
register access.

This allows to build vfpmodule.c with Clang and its integrated assembler.

[0] https://reviews.llvm.org/D59733

Link: https://github.com/ClangBuiltLinux/linux/issues/905
Signed-off-by: Stefan Agner 
---
Changes in v3:
- Reworded commit message, adding hint that Clang 11 won't have this
  restriction any longer

Changes in v2:
- Check assembler capabilities in Kconfig instead of Makefile

 arch/arm/Kconfig |  2 ++
 arch/arm/Kconfig.assembler   |  6 ++
 arch/arm/include/asm/vfp.h   |  2 ++
 arch/arm/include/asm/vfpmacros.h | 12 +++-
 arch/arm/vfp/vfphw.S |  1 +
 arch/arm/vfp/vfpinstr.h  | 23 +++
 6 files changed, 41 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/Kconfig.assembler

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2ac74904a3ce..911f55a11c63 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -2097,3 +2097,5 @@ source "drivers/firmware/Kconfig"
 if CRYPTO
 source "arch/arm/crypto/Kconfig"
 endif
+
+source "arch/arm/Kconfig.assembler"
diff --git a/arch/arm/Kconfig.assembler b/arch/arm/Kconfig.assembler
new file mode 100644
index ..5cb31aae1188
--- /dev/null
+++ b/arch/arm/Kconfig.assembler
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config AS_VFP_VMRS_FPINST
+   def_bool $(as-instr,.fpu vfpv2\nvmrs r0$(comma)FPINST)
+   help
+ Supported by binutils >= 2.24 and LLVM integrated assembler.
diff --git a/arch/arm/include/asm/vfp.h b/arch/arm/include/asm/vfp.h
index 7157d2a30a49..19928bfb4f9c 100644
--- a/arch/arm/include/asm/vfp.h
+++ b/arch/arm/include/asm/vfp.h
@@ -9,6 +9,7 @@
 #ifndef __ASM_VFP_H
 #define __ASM_VFP_H
 
+#ifndef CONFIG_AS_VFP_VMRS_FPINST
 #define FPSID  cr0
 #define FPSCR  cr1
 #define MVFR1  cr6
@@ -16,6 +17,7 @@
 #define FPEXC  cr8
 #define FPINST cr9
 #define FPINST2cr10
+#endif
 
 /* FPSID bits */
 #define FPSID_IMPLEMENTER_BIT  (24)
diff --git a/arch/arm/include/asm/vfpmacros.h b/arch/arm/include/asm/vfpmacros.h
index 947ee5395e1f..ba0d4cb5377e 100644
--- a/arch/arm/include/asm/vfpmacros.h
+++ b/arch/arm/include/asm/vfpmacros.h
@@ -8,7 +8,16 @@
 
 #include 
 
-@ Macros to allow building with old toolkits (with no VFP support)
+#ifdef CONFIG_AS_VFP_VMRS_FPINST
+   .macro  VFPFMRX, rd, sysreg, cond
+   vmrs\cond   \rd, \sysreg
+   .endm
+
+   .macro  VFPFMXR, sysreg, rd, cond
+   vmsr\cond   \sysreg, \rd
+   .endm
+#else
+   @ Macros to allow building with old toolkits (with no VFP support)
.macro  VFPFMRX, rd, sysreg, cond
MRC\condp10, 7, \rd, \sysreg, cr0, 0@ FMRX  \rd, \sysreg
.endm
@@ -16,6 +25,7 @@
.macro  VFPFMXR, sysreg, rd, cond
MCR\condp10, 7, \rd, \sysreg, cr0, 0@ FMXR  \sysreg, \rd
.endm
+#endif
 
@ read all the working registers back into the VFP
.macro  VFPFLDMIA, base, tmp
diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S
index 29ed36b99d1d

[PATCH v3 1/3] ARM: use .fpu assembler directives instead of assembler arguments

2020-06-26 Thread Stefan Agner
Explicit FPU selection has been introduced in commit 1a6be26d5b1a
("[ARM] Enable VFP to be built when non-VFP capable CPUs are selected")
to make use of assembler mnemonics for VFP instructions.

However, clang currently does not support passing assembler flags
like this and errors out with:
clang-10: error: the clang compiler does not support '-Wa,-mfpu=softvfp+vfp'

Make use of the .fpu assembler directives to select the floating point
hardware selectively. Also use the new unified assembler language
mnemonics. This allows to build these procedures with Clang.

Link: https://github.com/ClangBuiltLinux/linux/issues/762
Signed-off-by: Stefan Agner 
---
Changes in V3:
- Drop unnecessary comma
- Add .fpu directive also in single precision macros to avoid Clang error
  error: instruction requires: fp registers

Changes in v2:
- Add link in commit message

 arch/arm/vfp/Makefile |  2 --
 arch/arm/vfp/vfphw.S  | 30 --
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/arch/arm/vfp/Makefile b/arch/arm/vfp/Makefile
index 9975b63ac3b0..749901a72d6d 100644
--- a/arch/arm/vfp/Makefile
+++ b/arch/arm/vfp/Makefile
@@ -8,6 +8,4 @@
 # ccflags-y := -DDEBUG
 # asflags-y := -DDEBUG
 
-KBUILD_AFLAGS  :=$(KBUILD_AFLAGS:-msoft-float=-Wa,-mfpu=softvfp+vfp 
-mfloat-abi=soft)
-
 obj-y  += vfpmodule.o entry.o vfphw.o vfpsingle.o vfpdouble.o
diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S
index b2e560290860..29ed36b99d1d 100644
--- a/arch/arm/vfp/vfphw.S
+++ b/arch/arm/vfp/vfphw.S
@@ -258,11 +258,14 @@ vfp_current_hw_state_address:
 
 ENTRY(vfp_get_float)
tbl_branch r0, r3, #3
+   .fpuvfpv2
.irpdr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
-1: mrc p10, 0, r0, c\dr, c0, 0 @ fmrs  r0, s0
+1: vmovr0, s\dr
ret lr
.org1b + 8
-1: mrc p10, 0, r0, c\dr, c0, 4 @ fmrs  r0, s1
+   .endr
+   .irpdr,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31
+1: vmovr0, s\dr
ret lr
.org1b + 8
.endr
@@ -270,11 +273,14 @@ ENDPROC(vfp_get_float)
 
 ENTRY(vfp_put_float)
tbl_branch r1, r3, #3
+   .fpuvfpv2
.irpdr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
-1: mcr p10, 0, r0, c\dr, c0, 0 @ fmsr  r0, s0
+1: vmovs\dr, r0
ret lr
.org1b + 8
-1: mcr p10, 0, r0, c\dr, c0, 4 @ fmsr  r0, s1
+   .endr
+   .irpdr,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31
+1: vmovs\dr, r0
ret lr
.org1b + 8
.endr
@@ -282,15 +288,17 @@ ENDPROC(vfp_put_float)
 
 ENTRY(vfp_get_double)
tbl_branch r0, r3, #3
+   .fpuvfpv2
.irpdr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
-1: fmrrd   r0, r1, d\dr
+1: vmovr0, r1, d\dr
ret lr
.org1b + 8
.endr
 #ifdef CONFIG_VFPv3
@ d16 - d31 registers
-   .irpdr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
-1: mrrcp11, 3, r0, r1, c\dr@ fmrrd r0, r1, d\dr
+   .fpuvfpv3
+   .irpdr,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31
+1: vmovr0, r1, d\dr
ret lr
.org1b + 8
.endr
@@ -304,15 +312,17 @@ ENDPROC(vfp_get_double)
 
 ENTRY(vfp_put_double)
tbl_branch r2, r3, #3
+   .fpuvfpv2
.irpdr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
-1: fmdrr   d\dr, r0, r1
+1: vmovd\dr, r0, r1
ret lr
.org1b + 8
.endr
 #ifdef CONFIG_VFPv3
+   .fpuvfpv3
@ d16 - d31 registers
-   .irpdr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
-1: mcrrp11, 3, r0, r1, c\dr@ fmdrr r0, r1, d\dr
+   .irpdr,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31
+1: vmovd\dr, r0, r1
ret lr
.org1b + 8
.endr
-- 
2.27.0



Re: [PATCH] ARM: dts: vf610: Align L2 cache-controller nodename with dtschema

2020-06-26 Thread Stefan Agner
On 2020-06-26 10:05, Krzysztof Kozlowski wrote:
> Fix dtschema validator warnings like:
> l2-cache@40006000: $nodename:0:
> 'l2-cache@40006000' does not match
> '^(cache-controller|cpu)(@[0-9a-f,]+)*$'
> 
> Signed-off-by: Krzysztof Kozlowski 

Acked-by: Stefan Agner 

--
Stefan

> ---
>  arch/arm/boot/dts/vf610.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
> index 7fd39817f8ab..956182d08e74 100644
> --- a/arch/arm/boot/dts/vf610.dtsi
> +++ b/arch/arm/boot/dts/vf610.dtsi
> @@ -10,7 +10,7 @@
>  };
>  
>   {
> - L2: l2-cache@40006000 {
> + L2: cache-controller@40006000 {
>   compatible = "arm,pl310-cache";
>   reg = <0x40006000 0x1000>;
>   cache-unified;


Re: [PATCH 5/5] rtc: rtc-rc5t583: add ricoh rc5t619 RTC driver

2019-10-21 Thread Stefan Agner
On 2019-10-21 07:41, Andreas Kemnade wrote:
> Add an RTC driver for the RTC device on Ricoh MFD rc5t619,
> which is implemented as a variant of rn5t618
> 
> Signed-off-by: Andreas Kemnade 
> ---
>  drivers/rtc/Kconfig   |  10 +
>  drivers/rtc/Makefile  |   1 +
>  drivers/rtc/rtc-rc5t619.c | 476 
> ++

Parts of this driver look very similar to drivers/rtc/rtc-rc5t583.c. Can
it maybe shared?

--
Stefan

>  3 files changed, 487 insertions(+)
>  create mode 100644 drivers/rtc/rtc-rc5t619.c
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index e72f65b61176..a4dc04c49fec 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -588,6 +588,16 @@ config RTC_DRV_RC5T583
> This driver can also be built as a module. If so, the module
> will be called rtc-rc5t583.
>  
> +config RTC_DRV_RC5T619
> + tristate "RICOH RC5T619 RTC driver"
> + depends on MFD_RN5T618
> + help
> +   If you say yes here you get support for the RTC on the
> +   RICOH RC5T619 chips.
> +
> +   This driver can also be built as a module. If so, the module
> +   will be called rtc-rc5t619.
> +
>  config RTC_DRV_S35390A
>   tristate "Seiko Instruments S-35390A"
>   select BITREVERSE
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 6b09c21dc1b6..1d0673fd0954 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -136,6 +136,7 @@ obj-$(CONFIG_RTC_DRV_PXA) += rtc-pxa.o
>  obj-$(CONFIG_RTC_DRV_R7301)  += rtc-r7301.o
>  obj-$(CONFIG_RTC_DRV_R9701)  += rtc-r9701.o
>  obj-$(CONFIG_RTC_DRV_RC5T583)+= rtc-rc5t583.o
> +obj-$(CONFIG_RTC_DRV_RC5T619)+= rtc-rc5t619.o
>  obj-$(CONFIG_RTC_DRV_RK808)  += rtc-rk808.o
>  obj-$(CONFIG_RTC_DRV_RP5C01) += rtc-rp5c01.o
>  obj-$(CONFIG_RTC_DRV_RS5C313)+= rtc-rs5c313.o
> diff --git a/drivers/rtc/rtc-rc5t619.c b/drivers/rtc/rtc-rc5t619.c
> new file mode 100644
> index ..311788ff0723
> --- /dev/null
> +++ b/drivers/rtc/rtc-rc5t619.c
> @@ -0,0 +1,476 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * drivers/rtc/rtc-ricoh619.c
> + *
> + * Real time clock driver for RICOH R5T619 power management chip.
> + *
> + * Copyright (C) 2019 Andreas Kemnade
> + *
> + * Based on code
> + *  Copyright (C) 2012-2014 RICOH COMPANY,LTD
> + *
> + * Based on code
> + *  Copyright (C) 2011 NVIDIA Corporation
> + */
> +
> +/* #define debug 1 */
> +/* #define verbose_debug 1 */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct rc5t619_rtc {
> + int irq;
> + struct rtc_device   *rtc;
> + struct rn5t618 *rn5t618;
> +};
> +
> +#define CTRL1_ALARM_ENABLED 0x40
> +#define CTRL1_24HR 0x20
> +#define CTRL1_PERIODIC_MASK 0xf
> +
> +#define CTRL2_PON 0x10
> +#define CTRL2_ALARM_STATUS 0x80
> +#define CTRL2_CTFG 0x4
> +#define CTRL2_CTC 0x1
> +
> +static int rc5t619_rtc_periodic_disable(struct device *dev)
> +{
> + struct rc5t619_rtc *rtc = dev_get_drvdata(dev);
> + int err;
> +
> + /* disable function */
> + err = regmap_update_bits(rtc->rn5t618->regmap,
> + RN5T618_RTC_CTRL1, CTRL1_PERIODIC_MASK, 0);
> + if (err < 0)
> + return err;
> +
> + /* clear alarm flag and CTFG */
> + err = regmap_update_bits(rtc->rn5t618->regmap, RN5T618_RTC_CTRL2,
> + CTRL2_ALARM_STATUS | CTRL2_CTFG | CTRL2_CTC, 0);
> + if (err < 0)
> + return err;
> +
> + return 0;
> +}
> +
> +static int rc5t619_rtc_clk_adjust(struct device *dev, uint8_t clk)
> +{
> + struct rc5t619_rtc *rtc = dev_get_drvdata(dev);
> +
> + return regmap_write(rtc->rn5t618->regmap, RN5T618_RTC_ADJUST, clk);
> +}
> +
> +static int rc5t619_rtc_pon_get_clr(struct device *dev, uint8_t *pon_f)
> +{
> + struct rc5t619_rtc *rtc = dev_get_drvdata(dev);
> + int err;
> + unsigned int reg_data;
> +
> + err = regmap_read(rtc->rn5t618->regmap, RN5T618_RTC_CTRL2, _data);
> + if (err < 0)
> + return err;
> +
> + if (reg_data & CTRL2_PON) {
> + *pon_f = 1;
> + /* clear VDET PON */
> + reg_data &= ~(CTRL2_PON | CTRL2_CTC | 0x4a);/* 0101-1011 */
> + reg_data |= 0x20;   /* 0010- */
> + err = regmap_write(rtc->rn5t618->regmap, RN5T618_RTC_CTRL2,
> + reg_data);
> + } else {
> + *pon_f = 0;
> + }
> +
> + return err;
> +}
> +
> +/* 0-12hour, 1-24hour */
> +static int rc5t619_rtc_24hour_mode_set(struct device *dev, int mode)
> +{
> + struct rc5t619_rtc *rtc = dev_get_drvdata(dev);
> +
> + return regmap_update_bits(rtc->rn5t618->regmap, RN5T618_RTC_CTRL1,
> + CTRL1_24HR, mode ? CTRL1_24HR : 0);
> +}
> +
> +
> +static int rc5t619_rtc_read_time(struct device *dev, 

Re: [PATCH v1 2/3] tty: serial: lpuart: Use defines that correspond to correct register

2019-10-16 Thread Stefan Agner
On 2019-10-16 17:18, Philippe Schenker wrote:
> Use UARTMODIR defines instead of UARTMODEM as it is a 32-bit function

This reads a bit strange at first. Also it is helpful for later to state
that this does not make a difference in practise, so how about:

Use define from the 32-bit register description UARTMODIR_* instead of
UARTMODEM_*. The value is the same, so there is no functional change.

Otherwise looks good to me:

Reviewed-by: Stefan Agner 

--
Stefan

> 
> Signed-off-by: Philippe Schenker 
> ---
> 
>  drivers/tty/serial/fsl_lpuart.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index f3271857621c..346b4a070ce9 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -1879,10 +1879,10 @@ lpuart32_set_termios(struct uart_port *port,
> struct ktermios *termios,
>   }
>  
>   if (termios->c_cflag & CRTSCTS) {
> - modem |= UARTMODEM_RXRTSE | UARTMODEM_TXCTSE;
> + modem |= (UARTMODIR_RXRTSE | UARTMODIR_TXCTSE);
>   } else {
>   termios->c_cflag &= ~CRTSCTS;
> - modem &= ~(UARTMODEM_RXRTSE | UARTMODEM_TXCTSE);
> + modem &= ~(UARTMODIR_RXRTSE | UARTMODIR_TXCTSE);
>   }
>  
>   if (termios->c_cflag & CSTOPB)


Re: [PATCH 3/6] drivers: firmware: psci: Register with kernel restart handler

2019-10-16 Thread Stefan Agner
On 2019-10-15 16:51, Thierry Reding wrote:
> From: Guenter Roeck 
> 
> Register with kernel restart handler instead of setting arm_pm_restart
> directly. This enables support for replacing the PSCI restart handler
> with a different handler if necessary for a specific board.
> 
> Select a priority of 129 to indicate a higher than default priority, but
> keep it as low as possible since PSCI reset is known to fail on some
> boards.
> 
> Acked-by: Arnd Bergmann 
> Reviewed-by: Wolfram Sang 
> Tested-by: Wolfram Sang 
> Signed-off-by: Guenter Roeck 
> Acked-by: Lorenzo Pieralisi 
> Signed-off-by: Thierry Reding 

Looks good to me! And helps also in my case, a board which has a broken
PSCI reset capability.

Reviewed-by: Stefan Agner 

--
Stefan

> ---
>  drivers/firmware/psci/psci.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> index 84f4ff351c62..a41c6ba043a2 100644
> --- a/drivers/firmware/psci/psci.c
> +++ b/drivers/firmware/psci/psci.c
> @@ -250,7 +250,8 @@ static int get_set_conduit_method(struct device_node *np)
>   return 0;
>  }
>  
> -static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd)
> +static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> +   void *data)
>  {
>   if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>   psci_system_reset2_supported) {
> @@ -263,8 +264,15 @@ static void psci_sys_reset(enum reboot_mode
> reboot_mode, const char *cmd)
>   } else {
>   invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
>   }
> +
> + return NOTIFY_DONE;
>  }
>  
> +static struct notifier_block psci_sys_reset_nb = {
> + .notifier_call = psci_sys_reset,
> + .priority = 129,
> +};
> +
>  static void psci_sys_poweroff(void)
>  {
>   invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
> @@ -431,7 +439,7 @@ static void __init psci_0_2_set_functions(void)
>  
>   psci_ops.migrate_info_type = psci_migrate_info_type;
>  
> - arm_pm_restart = psci_sys_reset;
> + register_restart_handler(_sys_reset_nb);
>  
>   pm_power_off = psci_sys_poweroff;
>  }


Re: [PATCH] drivers: firmware: psci: use kernel restart handler functionality

2019-10-14 Thread Stefan Agner
Hi Sudeep,


On 2019-10-14 12:07, Sudeep Holla wrote:
> On Sat, Oct 12, 2019 at 11:47:35PM +0200, Stefan Agner wrote:
>> From: Stefan Agner 
>>
>> Use the kernels restart handler to register the PSCI system reset
>> capability. The restart handler use notifier chains along with
>> priorities. This allows to use restart handlers with higher priority
>> (in case available) while still supporting PSCI.
>>
>> Since the ARM handler had priority over the kernels restart handler
>> before this patch, use a slightly elevated priority of 160 to make
>> sure PSCI is used before most of the other handlers are called.
>>
> 
> There's an attempt(rather pull request[1]) to consolidate these into new
> system power/restart handler.

Oh thanks for the pointer! Interesting timing :-)

--
Stefan

> 
> --
> Regards,
> Sudeep
> 
> [1]
> https://lore.kernel.org/linux-arm-kernel/20191002131228.4085560-1-thierry.red...@gmail.com


[PATCH] drivers: firmware: psci: use kernel restart handler functionality

2019-10-12 Thread Stefan Agner
From: Stefan Agner 

Use the kernels restart handler to register the PSCI system reset
capability. The restart handler use notifier chains along with
priorities. This allows to use restart handlers with higher priority
(in case available) while still supporting PSCI.

Since the ARM handler had priority over the kernels restart handler
before this patch, use a slightly elevated priority of 160 to make
sure PSCI is used before most of the other handlers are called.

Signed-off-by: Stefan Agner 
---
 drivers/firmware/psci/psci.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index 84f4ff351c62..d8677b54132f 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -82,6 +82,7 @@ static u32 psci_function_id[PSCI_FN_MAX];
 
 static u32 psci_cpu_suspend_feature;
 static bool psci_system_reset2_supported;
+static struct notifier_block psci_restart_handler;
 
 static inline bool psci_has_ext_power_state(void)
 {
@@ -250,7 +251,8 @@ static int get_set_conduit_method(struct device_node *np)
return 0;
 }
 
-static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd)
+static int psci_sys_reset(struct notifier_block *this,
+   unsigned long reboot_mode, void *cmd)
 {
if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
psci_system_reset2_supported) {
@@ -263,6 +265,8 @@ static void psci_sys_reset(enum reboot_mode reboot_mode, 
const char *cmd)
} else {
invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
}
+
+   return NOTIFY_DONE;
 }
 
 static void psci_sys_poweroff(void)
@@ -411,6 +415,8 @@ static void __init psci_init_smccc(void)
 
 static void __init psci_0_2_set_functions(void)
 {
+   int ret;
+
pr_info("Using standard PSCI v0.2 function IDs\n");
psci_ops.get_version = psci_get_version;
 
@@ -431,7 +437,14 @@ static void __init psci_0_2_set_functions(void)
 
psci_ops.migrate_info_type = psci_migrate_info_type;
 
-   arm_pm_restart = psci_sys_reset;
+   psci_restart_handler.notifier_call = psci_sys_reset;
+   psci_restart_handler.priority = 160;
+
+   ret = register_restart_handler(_restart_handler);
+   if (ret) {
+   pr_err("Cannot register restart handler, %d\n", ret);
+   return;
+   }
 
pm_power_off = psci_sys_poweroff;
 }
-- 
2.23.0



Re: [PATCH] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer

2019-08-29 Thread Stefan Agner
On 2019-08-29 21:34, Nathan Chancellor wrote:
> On Thu, Aug 29, 2019 at 10:55:28AM -0700, Nick Desaulniers wrote:
>> On Wed, Aug 28, 2019 at 11:27 PM Nathan Chancellor
>>  wrote:
>> >
>> > Currently, multi_v7_defconfig + CONFIG_FUNCTION_TRACER fails to build
>> > with clang:
>> >
>> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `_local_bh_enable':
>> > softirq.c:(.text+0x504): undefined reference to `mcount'
>> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `__local_bh_enable_ip':
>> > softirq.c:(.text+0x58c): undefined reference to `mcount'
>> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `do_softirq':
>> > softirq.c:(.text+0x6c8): undefined reference to `mcount'
>> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_enter':
>> > softirq.c:(.text+0x75c): undefined reference to `mcount'
>> > arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_exit':
>> > softirq.c:(.text+0x840): undefined reference to `mcount'
>> > arm-linux-gnueabi-ld: kernel/softirq.o:softirq.c:(.text+0xa50): more 
>> > undefined references to `mcount' follow
>> >
>> > clang can emit a working mcount symbol, __gnu_mcount_nc, when
>> > '-meabi gnu' is passed to it. Until r369147 in LLVM, this was
>> > broken and caused the kernel not to boot because the calling
>> > convention was not correct. Now that it is fixed, add this to
>> > the command line when clang is 10.0.0 or newer so everything
>> > works properly.
>> >
>> > Link: https://github.com/ClangBuiltLinux/linux/issues/35
>> > Link: https://bugs.llvm.org/show_bug.cgi?id=33845
>> > Link: 
>> > https://github.com/llvm/llvm-project/commit/16fa8b09702378bacfa3d07081afe6b353b99e60
>> > Signed-off-by: Nathan Chancellor 
>> > ---
>> >  arch/arm/Makefile | 6 ++
>> >  1 file changed, 6 insertions(+)
>> >
>> > diff --git a/arch/arm/Makefile b/arch/arm/Makefile
>> > index c3624ca6c0bc..7b5a26a866fc 100644
>> > --- a/arch/arm/Makefile
>> > +++ b/arch/arm/Makefile
>> > @@ -112,6 +112,12 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
>> >  CFLAGS_ABI +=-funwind-tables
>> >  endif
>> >
>> > +ifeq ($(CONFIG_CC_IS_CLANG),y)
>> > +ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 10; echo $$?),0)
>> > +CFLAGS_ABI +=-meabi gnu
>> > +endif
>> > +endif
>> > +
>>
>> Thanks for the patch!  I think this is one of the final issues w/ 32b
>> ARM configs when building w/ Clang.
>>
>> I'm not super enthused about the version check.  The flag is indeed
>> not recognized by GCC, but I think it would actually be more concise
>> with $(cc-option) and no compiler or version check.
>>
>> Further, I think that the working __gnu_mcount_nc in Clang would
>> better be represented as marking the arch/arm/KConfig option for
>> CONFIG_FUNCTION_TRACER for dependent on a version of Clang greater
>> than or equal to Clang 10, not conditionally adding this flag. (We
>> should always add the flag when supported, IMO.  __gnu_mcount_nc's
>> calling convention being broken is orthogonal to the choice of
>> __gnu_mcount_nc vs mcount, and it's the former's that should be
>> checked, not the latter as in this patch.
> 
> I will test with or without CONFIG_AEABI like Matthias asked and I will
> implement your Kconfig suggestion if it passes all of my tests. The
> reason that I did it this way is because I didn't want a user to end up
> with a non-booting kernel since -meabi gnu works with older versions of
> clang at build time, the issue happens at boot time but the Kconfig
> suggestion + cc-option should fix that.

I agree with Nathan here, I'd rather prefer the build system to fail
building rather than runtime error.

If we decide we want to have it building despite it not building a
functional kernel, we should at least add a #warning...

--
Stefan

> 
> I should have a v2 out this evening.
> 
> Cheers,
> Nathan


Re: [PATCH] ARM: Emit __gnu_mcount_nc when using Clang 10.0.0 or newer

2019-08-29 Thread Stefan Agner
On 2019-08-29 08:26, Nathan Chancellor wrote:
> Currently, multi_v7_defconfig + CONFIG_FUNCTION_TRACER fails to build
> with clang:
> 
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `_local_bh_enable':
> softirq.c:(.text+0x504): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `__local_bh_enable_ip':
> softirq.c:(.text+0x58c): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `do_softirq':
> softirq.c:(.text+0x6c8): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_enter':
> softirq.c:(.text+0x75c): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o: in function `irq_exit':
> softirq.c:(.text+0x840): undefined reference to `mcount'
> arm-linux-gnueabi-ld: kernel/softirq.o:softirq.c:(.text+0xa50): more
> undefined references to `mcount' follow
> 
> clang can emit a working mcount symbol, __gnu_mcount_nc, when
> '-meabi gnu' is passed to it. Until r369147 in LLVM, this was
> broken and caused the kernel not to boot because the calling
> convention was not correct. Now that it is fixed, add this to
> the command line when clang is 10.0.0 or newer so everything
> works properly.

Cool, finally function tracing with Clang :-)

> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/35
> Link: https://bugs.llvm.org/show_bug.cgi?id=33845
> Link:
> https://github.com/llvm/llvm-project/commit/16fa8b09702378bacfa3d07081afe6b353b99e60
> Signed-off-by: Nathan Chancellor 

Reviewed-by: Stefan Agner 

--
Stefan

> ---
>  arch/arm/Makefile | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index c3624ca6c0bc..7b5a26a866fc 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -112,6 +112,12 @@ ifeq ($(CONFIG_ARM_UNWIND),y)
>  CFLAGS_ABI   +=-funwind-tables
>  endif
>  
> +ifeq ($(CONFIG_CC_IS_CLANG),y)
> +ifeq ($(shell test $(CONFIG_CLANG_VERSION) -ge 10; echo $$?),0)
> +CFLAGS_ABI   +=-meabi gnu
> +endif
> +endif
> +
>  # Accept old syntax despite ".syntax unified"
>  AFLAGS_NOWARN:=$(call 
> as-option,-Wa$(comma)-mno-warn-deprecated,-Wa$(comma)-W)


Re: [PATCH] ASoC: soc-core: remove error due to probe deferral

2019-08-08 Thread Stefan Agner
On 2019-08-08 15:14, Takashi Iwai wrote:
> On Thu, 08 Aug 2019 15:02:17 +0200,
> Mark Brown wrote:
>>
>> On Thu, Aug 08, 2019 at 03:00:06PM +0200, Takashi Iwai wrote:
>> > Mark Brown wrote:
>>
>> > > No, they absolutely should tell the user why they are deferring so the
>> > > user has some information to go on when they're trying to figure out why
>> > > their device isn't instantiating.
>>
>> > But it's no real error that *must* be printed on the console, either.
>> > Maybe downgrading the printk level?
>>
>> Yes, downgrading can be OK though it does bloat the code.
> 
> I guess we can use dev_printk() with the conditional level choice.
> 

How about use dev_info always? We get a dev_err message from
soc_init_dai_link in error cases...

ret = soc_init_dai_link(card, dai_link);
if (ret && ret != -EPROBE_DEFER) {
dev_info(card->dev, "ASoC: failed to init link %s: 
%d\n",
 dai_link->name, ret);
}
if (ret) {
soc_cleanup_platform(card);
mutex_unlock(_mutex);
return ret;
}

--
Stefan


Re: [PATCH] ASoC: soc-core: remove error due to probe deferral

2019-08-08 Thread Stefan Agner
On 2019-08-08 14:44, Mark Brown wrote:
> On Thu, Aug 08, 2019 at 02:36:55PM +0200, Stefan Agner wrote:
>> From: Stefan Agner 
>>
>> Deferred probes shouldn't cause error messages in the boot log. Avoid
>> printing with dev_err() in case EPROBE_DEFER is the return value.
> 
> No, they absolutely should tell the user why they are deferring so the
> user has some information to go on when they're trying to figure out why
> their device isn't instantiating.

Hm, I see, if the driver defers and does not manage in the end, then the
messages are indeed helpful.

But can we lower severity, e.g. to dev_info? In my case it succeeds in
the end, just defers about 6 times. I have 3 links which then leads to
18 error messages which confuse users... From what I can see
soc_init_dai_link() would print dev_err in case there is an actual
error.

--
Stefan


[PATCH] ASoC: soc-core: remove error due to probe deferral

2019-08-08 Thread Stefan Agner
From: Stefan Agner 

Deferred probes shouldn't cause error messages in the boot log. Avoid
printing with dev_err() in case EPROBE_DEFER is the return value.

Signed-off-by: Stefan Agner 
---
 sound/soc/soc-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index fd6eaae6c0ed..98e1e80b5493 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1985,9 +1985,11 @@ static int snd_soc_instantiate_card(struct snd_soc_card 
*card)
mutex_lock(_mutex);
for_each_card_prelinks(card, i, dai_link) {
ret = soc_init_dai_link(card, dai_link);
-   if (ret) {
+   if (ret && ret != -EPROBE_DEFER) {
dev_err(card->dev, "ASoC: failed to init link %s: %d\n",
dai_link->name, ret);
+   }
+   if (ret) {
mutex_unlock(_mutex);
return ret;
}
-- 
2.22.0



[PATCH] arm64: defconfig: enable deprecated ARMv8 instructions emulation

2019-08-08 Thread Stefan Agner
Enable deprecated/obsolete ARMv8 instructions emulation. This allows
to run ARMv6 binaries (e.g. Raspberry Pi) on ARMv8 machines.

Signed-off-by: Stefan Agner 
---
 arch/arm64/configs/defconfig | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 0e58ef02880c..fd5af5582dda 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -68,6 +68,10 @@ CONFIG_KEXEC=y
 CONFIG_CRASH_DUMP=y
 CONFIG_XEN=y
 CONFIG_COMPAT=y
+CONFIG_ARMV8_DEPRECATED=y
+CONFIG_SWP_EMULATION=y
+CONFIG_CP15_BARRIER_EMULATION=y
+CONFIG_SETEND_EMULATION=y
 CONFIG_RANDOMIZE_BASE=y
 CONFIG_HIBERNATION=y
 CONFIG_WQ_POWER_EFFICIENT_DEFAULT=y
-- 
2.22.0



Re: [PATCH v2 07/20] ARM: dts: imx7-colibri: fix 1.8V/UHS support

2019-08-02 Thread Stefan Agner
On 2019-07-31 16:52, Philippe Schenker wrote:
> On Wed, 2019-07-31 at 09:56 -0300, Fabio Estevam wrote:
>> On Wed, Jul 31, 2019 at 9:38 AM Philippe Schenker
>>  wrote:
>> > From: Stefan Agner 
>> >
>> > Add pinmuxing and do not specify voltage restrictions in the
>> > module level device tree.
>>
>> It would be nice to explain the reason for doing this.
> 
> This commit is in preparation of another patch that didn't made into this
> patchset (downstream stuff in there). But I will do another patch on top that
> will use this patch here. That should anyway be in mainline.

I guess what Fabio meant here is explain this patch.

The commit message really could be improved, e.g.:

Add pinmuxing and do not specify voltage restrictions for the usdhc
instance
available on the modules edge connector. This allows to use SD-cards
with
higher transfer modes if supported by the carrier board.

--
Stefan

> 
> Philippe
> 
>>
>> > Signed-off-by: Stefan Agner 
>> > Signed-off-by: Philippe Schenker 
>> > ---
>> >
>> > Changes in v2: None
>> >
>> >  arch/arm/boot/dts/imx7-colibri.dtsi | 23 ++-
>> >  1 file changed, 22 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-
>> > colibri.dtsi
>> > index 16d1a1ed1aff..67f5e0c87fdc 100644
>> > --- a/arch/arm/boot/dts/imx7-colibri.dtsi
>> > +++ b/arch/arm/boot/dts/imx7-colibri.dtsi
>> > @@ -326,7 +326,6 @@
>> >   {
>> > pinctrl-names = "default";
>> > pinctrl-0 = <_usdhc1 _cd_usdhc1>;
>> > -   no-1-8-v;
>> > cd-gpios = < 0 GPIO_ACTIVE_LOW>;
>> > disable-wp;
>> > vqmmc-supply = <_LDO2>;
>> > @@ -671,6 +670,28 @@
>> > >;
>> > };
>> >
>> > +   pinctrl_usdhc1_100mhz: usdhc1grp_100mhz {
>> > +   fsl,pins = <
>> > +   MX7D_PAD_SD1_CMD__SD1_CMD   0x5a
>> > +   MX7D_PAD_SD1_CLK__SD1_CLK   0x1a
>> > +   MX7D_PAD_SD1_DATA0__SD1_DATA0   0x5a
>> > +   MX7D_PAD_SD1_DATA1__SD1_DATA1   0x5a
>> > +   MX7D_PAD_SD1_DATA2__SD1_DATA2   0x5a
>> > +   MX7D_PAD_SD1_DATA3__SD1_DATA3   0x5a
>> > +   >;
>> > +   };
>> > +
>> > +   pinctrl_usdhc1_200mhz: usdhc1grp_200mhz {
>> > +   fsl,pins = <
>> > +   MX7D_PAD_SD1_CMD__SD1_CMD   0x5b
>> > +   MX7D_PAD_SD1_CLK__SD1_CLK   0x1b
>> > +   MX7D_PAD_SD1_DATA0__SD1_DATA0   0x5b
>> > +   MX7D_PAD_SD1_DATA1__SD1_DATA1   0x5b
>> > +   MX7D_PAD_SD1_DATA2__SD1_DATA2   0x5b
>> > +   MX7D_PAD_SD1_DATA3__SD1_DATA3   0x5b
>> > +   >;
>> > +   };
>>
>> You add the entries for 100MHz and 200MHz, but I don't see them being
>> referenced anywhere.


Re: [PATCH] drm/tegra: return with probe defer if GPIO subsystem is not ready

2019-07-26 Thread Stefan Agner
On 2019-07-26 16:46, Dmitry Osipenko wrote:
> 26.07.2019 17:23, Stefan Agner пишет:
>> Hi Thierry, Hi Dave,
>>
>> On 2018-09-07 01:31, Stefan Agner wrote:
>>> On 26.07.2018 06:36, Stefan Agner wrote:
>>>> If the GPIO subsystem is not ready make sure to return -EPROBE_DEFER
>>>> instead of silently continuing without HPD.
>>>>
>>>> Reported-by: Marcel Ziswiler 
>>>> Signed-off-by: Stefan Agner 
>>>
>>> I think this did not get merged yet, any chance to get it in?
>>
>> Any chance to get this in in the next merge window?
> 
> The patch isn't relevant anymore (since v5.3) because the code was
> changed and now propagates the proper error code in a case of error,
> please see [1].
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.3-rc1=bbad640709fd43ff77b8838c409c977c0b28430c

Oh I see, I accidentally checked in Linux 5.2. Sorry for the noise and
thanks for the link!

--
Stefan


[PATCH RESEND v8] PCI: imx6: limit DBI register length

2019-07-26 Thread Stefan Agner
Define the length of the DBI registers and limit config space to its
length. This makes sure that the kernel does not access registers
beyond that point, avoiding the following abort on a i.MX 6Quad:
  # cat /sys/devices/soc0/soc/1ffc000.pcie/pci\:00/\:00\:00.0/config
  [  100.021433] Unhandled fault: imprecise external abort (0x1406) at 
0xb6ea7000
  ...
  [  100.056423] PC is at dw_pcie_read+0x50/0x84
  [  100.060790] LR is at dw_pcie_rd_own_conf+0x44/0x48
  ...

Signed-off-by: Stefan Agner 
---
Changes in v3:
- Rebase on pci/dwc
Changes in v4:
- Rebase on pci/dwc
Changes in v5:
- Rebased ontop of pci/dwc
- Use DBI length of 0x200
Changes in v6:
- Use pci_dev.cfg_size mechanism to limit config space (this made patch 1
  of previous versions of this patchset obsolete).
Changes in v7:
- Restrict fixup to Synopsys/0xabcd
- Apply cfg_size limitation only if dbi_length is specified
Changes in v8:
- Restrict fixup for Synopsys/0xabcd and class PCI bridge
- Check device driver to be pci-imx6

 drivers/pci/controller/dwc/pci-imx6.c | 33 +++
 1 file changed, 33 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
b/drivers/pci/controller/dwc/pci-imx6.c
index 9b5cb5b70389..8b8efa3063f5 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -57,6 +57,7 @@ enum imx6_pcie_variants {
 struct imx6_pcie_drvdata {
enum imx6_pcie_variants variant;
u32 flags;
+   int dbi_length;
 };
 
 struct imx6_pcie {
@@ -1212,6 +1213,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
.variant = IMX6Q,
.flags = IMX6_PCIE_FLAG_IMX6_PHY |
 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE,
+   .dbi_length = 0x200,
},
[IMX6SX] = {
.variant = IMX6SX,
@@ -1254,6 +1256,37 @@ static struct platform_driver imx6_pcie_driver = {
.shutdown = imx6_pcie_shutdown,
 };
 
+static void imx6_pcie_quirk(struct pci_dev *dev)
+{
+   struct pci_bus *bus = dev->bus;
+   struct pcie_port *pp = bus->sysdata;
+
+   /* Bus parent is the PCI bridge, its parent is this platform driver */
+   if (!bus->dev.parent || !bus->dev.parent->parent)
+   return;
+
+   /* Make sure we only quirk devices associated with this driver */
+   if (bus->dev.parent->parent->driver != _pcie_driver.driver)
+   return;
+
+   if (bus->number == pp->root_bus_nr) {
+   struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+   struct imx6_pcie *imx6_pcie = to_imx6_pcie(pci);
+
+   /*
+* Limit config length to avoid the kernel reading beyond
+* the register set and causing an abort on i.MX 6Quad
+*/
+   if (imx6_pcie->drvdata->dbi_length) {
+   dev->cfg_size = imx6_pcie->drvdata->dbi_length;
+   dev_info(>dev, "Limiting cfg_size to %d\n",
+   dev->cfg_size);
+   }
+   }
+}
+DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_VENDOR_ID_SYNOPSYS, 0xabcd,
+   PCI_CLASS_BRIDGE_PCI, 8, imx6_pcie_quirk);
+
 static int __init imx6_pcie_init(void)
 {
 #ifdef CONFIG_ARM
-- 
2.22.0



Re: [PATCH v2 1/1] ARM: dts: colibri: introduce dts with UHS-I support enabled

2019-07-09 Thread Stefan Agner
On 2019-05-14 16:38, Igor Opaniuk wrote:
> Introduce DTS for Colibri iMX6S/DL V1.1x re-design, where UHS-I support was
> added. Provide proper configuration for VGEN3, which allows that rail to
> be automatically switched to 1.8 volts for proper UHS-I operation mode.
> 
> Signed-off-by: Igor Opaniuk 
> ---
> 
> v2:
> - rework hierarchy of dts files, and a separate dtsi for Colibri
>   iMX6S/DL V1.1x re-design, where UHS-I was added
> - add comments about vgen3 power rail
> - fix other minor issues, addressing Marcel's comments.
> 
>  arch/arm/boot/dts/Makefile|   1 +
>  .../boot/dts/imx6dl-colibri-v1.1-eval-v3.dts  | 220 +
>  arch/arm/boot/dts/imx6qdl-colibri-v1.1.dtsi   | 852 ++
>  3 files changed, 1073 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx6dl-colibri-v1.1-eval-v3.dts
>  create mode 100644 arch/arm/boot/dts/imx6qdl-colibri-v1.1.dtsi
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index dab2914fa293..dc4ea05c8e2a 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -401,6 +401,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
>   imx6dl-aristainetos2_4.dtb \
>   imx6dl-aristainetos2_7.dtb \
>   imx6dl-colibri-eval-v3.dtb \
> + imx6dl-colibri-v1.1-eval-v3.dtb \
>   imx6dl-cubox-i.dtb \
>   imx6dl-cubox-i-emmc-som-v15.dtb \
>   imx6dl-cubox-i-som-v15.dtb \
> diff --git a/arch/arm/boot/dts/imx6dl-colibri-v1.1-eval-v3.dts
> b/arch/arm/boot/dts/imx6dl-colibri-v1.1-eval-v3.dts
> new file mode 100644
> index ..8ed7a528e7c7
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6dl-colibri-v1.1-eval-v3.dts
> @@ -0,0 +1,220 @@
> +// SPDX-License-Identifier: GPL-2.0+ OR X11
> +/*
> + * Copyright 2019 Toradex AG
> + */
> +
> +/dts-v1/;
> +
> +#include 
> +#include 
> +#include "imx6dl.dtsi"
> +#include "imx6qdl-colibri-v1.1.dtsi"
> +
> +/ {
> + model = "Toradex Colibri iMX6DL/S V1.1 on Colibri Evaluation Board V3";
> + compatible = "toradex,colibri_imx6dl-eval-v3", "toradex,colibri_imx6dl",
> +  "fsl,imx6dl";

I would also change compatible here so someone could detect the new
version, e.g.

"toradex,colibri_imx6dl-v1.1-eval-v3", "toradex,colibri_imx6dl-v1.1",

I guess if the module is considered backward compatible, we should keep
the old compatible strings as well, like we did on Apalis T30, see
arch/arm/boot/dts/tegra30-apalis-v1.1-eval.dts.

The module level model/compatible get overwritten anyway, I think we
could drop those.

--
Stefan

> +
> + /* Will be filled by the bootloader */
> + memory@1000 {
> + device_type = "memory";
> + reg = <0x1000 0>;
> + };
> +
> + aliases {
> + i2c0 = 
> + i2c1 = 
> + };
> +
> + aliases {
> + rtc0 = _i2c;
> + rtc1 = _rtc;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + /* Fixed crystal dedicated to mcp251x */
> + clk16m: clock-16m {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <1600>;
> + clock-output-names = "clk16m";
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> + pinctrl-names = "default";
> + pinctrl-0 = <_gpio_keys>;
> +
> + wakeup {
> + label = "Wake-Up";
> + gpios = < 22 GPIO_ACTIVE_HIGH>; /* SODIMM 45 */
> + linux,code = ;
> + debounce-interval = <10>;
> + wakeup-source;
> + };
> + };
> +
> + lcd_display: disp0 {
> + compatible = "fsl,imx-parallel-display";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + interface-pix-fmt = "bgr666";
> + pinctrl-names = "default";
> + pinctrl-0 = <_ipu1_lcdif>;
> + status = "okay";
> +
> + port@0 {
> + reg = <0>;
> +
> + lcd_display_in: endpoint {
> + remote-endpoint = <_di0_disp0>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + lcd_display_out: endpoint {
> + remote-endpoint = <_panel_in>;
> + };
> + };
> + };
> +
> + panel: panel {
> + /*
> +  * edt,et057090dhu: EDT 5.7" LCD TFT
> +  * edt,et070080dh6: EDT 7.0" LCD TFT
> +  */
> + compatible = "edt,et057090dhu";
> + backlight = <>;
> +
> + port {
> + lcd_panel_in: endpoint {
> + remote-endpoint = <_display_out>;
> + };
> + };
> + };
> +};
> +
> + {
> + brightness-levels = <0 127 191 223 239 247 251 255>;
> +

Re: [PATCH 1/1] ARM: dts: imx6ull-colibri: enable UHS-I for USDHC1

2019-06-12 Thread Stefan Agner
On 06.06.2019 11:06, Igor Opaniuk wrote:
> From: Igor Opaniuk 
> 
> Allows to use the SD interface at a higher speed mode if the card
> supports it. For this the signaling voltage is switched from 3.3V to
> 1.8V under the usdhc1's drivers control.
> 
> Signed-off-by: Igor Opaniuk 
> ---
>  arch/arm/boot/dts/imx6ul.dtsi  |  4 
>  arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi | 11 +--
>  arch/arm/boot/dts/imx6ull-colibri.dtsi |  6 ++
>  3 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
> index fc388b84bf22..91a0ced44e27 100644
> --- a/arch/arm/boot/dts/imx6ul.dtsi
> +++ b/arch/arm/boot/dts/imx6ul.dtsi
> @@ -857,6 +857,8 @@
>< IMX6UL_CLK_USDHC1>,
>< IMX6UL_CLK_USDHC1>;
>   clock-names = "ipg", "ahb", "per";
> + fsl,tuning-step= <2>;
> + fsl,tuning-start-tap = <20>;
>   bus-width = <4>;
>   status = "disabled";
>   };
> @@ -870,6 +872,8 @@
>< IMX6UL_CLK_USDHC2>;
>   clock-names = "ipg", "ahb", "per";
>   bus-width = <4>;
> + fsl,tuning-step= <2>;
> + fsl,tuning-start-tap = <20>;
>   status = "disabled";
>   };
>  
> diff --git a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> index 006690ea98c0..7dc7770cf52c 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> @@ -145,13 +145,20 @@
>  };
>  
>   {
> - pinctrl-names = "default";
> + pinctrl-names = "default", "state_100mhz", "state_200mhz", "sleep";
>   pinctrl-0 = <_usdhc1 _snvs_usdhc1_cd>;
> - no-1-8-v;
> + pinctrl-1 = <_usdhc1_100mhz _snvs_usdhc1_cd>;
> + pinctrl-2 = <_usdhc1_100mhz _snvs_usdhc1_cd>;

Should that not be pinctrl_usdhc1_200mhz?

--
Stefan

> + pinctrl-3 = <_usdhc1 _snvs_usdhc1_sleep_cd>;
>   cd-gpios = < 0 GPIO_ACTIVE_LOW>;
>   disable-wp;
>   wakeup-source;
>   keep-power-in-suspend;
>   vmmc-supply = <_3v3>;
> + vqmmc-supply = <_sd1_vmmc>;
> + sd-uhs-sdr12;
> + sd-uhs-sdr25;
> + sd-uhs-sdr50;
> + sd-uhs-sdr104;
>   status = "okay";
>  };
> diff --git a/arch/arm/boot/dts/imx6ull-colibri.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri.dtsi
> index 9ad1da159768..d56728f03c35 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri.dtsi
> @@ -545,6 +545,12 @@
>   >;
>   };
>  
> + pinctrl_snvs_usdhc1_sleep_cd: snvs-usdhc1-cd-grp-slp {
> + fsl,pins = <
> + MX6ULL_PAD_SNVS_TAMPER0__GPIO5_IO00 0x0
> + >;
> + };
> +
>   pinctrl_snvs_wifi_pdn: snvs-wifi-pdn-grp {
>   fsl,pins = <
>   MX6ULL_PAD_BOOT_MODE1__GPIO5_IO11   0x14


Re: [PATCH v4 2/2] ARM: OMAP2: drop explicit assembler architecture

2019-06-02 Thread Stefan Agner
On 30.05.2019 22:02, Nick Desaulniers wrote:
> On Mon, May 27, 2019 at 3:41 PM Stefan Agner  wrote:
>>
>> OMAP2 depends on ARCH_MULTI_V6, which makes sure that the kernel is
>> compiled with -march=armv6. The compiler frontend will pass the
>> architecture to the assembler. There is no explicit architecture
>> specification necessary.
>>
>> Signed-off-by: Stefan Agner 
>> Acked-by: Tony Lindgren 
>> ---
>> Changes since v2:
>> - New patch
>>
>> Changes since v3:
>> - Rebase on top of v5.2-rc2
> 
> Hi Stefan, looks like both patches are ack'd.  Are you waiting for an
> explicit reviewed by tag to submit to
> https://www.armlinux.org.uk/developer/patches/?

This should go through arm-soc, it missed the last merge window, see
Olofs message:
https://lore.kernel.org/lkml/20190516214819.dopw4eiumt6is46e@localhost/T/#u

Should be still early enough to make it in this merge window.

--
Stefan


[PATCH v4 1/2] ARM: use arch_extension directive instead of arch argument

2019-05-27 Thread Stefan Agner
The LLVM Target parser currently does not allow to specify the security
extension as part of -march (see also LLVM Bug 40186 [0]). When trying
to use Clang with LLVM's integrated assembler, this leads to build
errors such as this:
  clang-8: error: the clang compiler does not support '-Wa,-march=armv7-a+sec'

Use ".arch_extension sec" to enable the security extension in a more
portable fasion. Also make sure to use ".arch armv7-a" in case a v6/v7
multi-platform kernel is being built.

Note that this is technically not exactly the same as the old code
checked for availabilty of the security extension by calling as-instr.
However, there are already other sites which use ".arch_extension sec"
unconditionally, hence de-facto we need an assembler capable of
".arch_extension sec" already today (arch/arm/mm/proc-v7.S). The
arch extension "sec" is available since binutils 2.21 according to
its documentation [1].

[0] https://bugs.llvm.org/show_bug.cgi?id=40186
[1] https://sourceware.org/binutils/docs-2.21/as/ARM-Options.html

Signed-off-by: Stefan Agner 
Acked-by: Mans Rullgard 
Acked-by: Arnd Bergmann 
Acked-by: Krzysztof Kozlowski 
---
Changes since v1:
- Explicitly specify assembler architecture as armv7-a to avoid
  build issues when bulding v6/v7 multi arch kernel.

Changes since v2:
- Add armv7-a also in mach-tango
- Move .arch armv7-a outside of ifdef'ed area in sleep44xx.S
  to make the kernel compile also without CONFIG_SMP/PM.

Changes since v3:
- Rebase on top of v5.2-rc2

 arch/arm/mach-bcm/Makefile | 3 ---
 arch/arm/mach-bcm/bcm_kona_smc.c   | 2 --
 arch/arm/mach-exynos/Makefile  | 4 
 arch/arm/mach-exynos/exynos-smc.S  | 3 ++-
 arch/arm/mach-exynos/sleep.S   | 3 ++-
 arch/arm/mach-highbank/Makefile| 3 ---
 arch/arm/mach-highbank/smc.S   | 3 ++-
 arch/arm/mach-keystone/Makefile| 3 ---
 arch/arm/mach-keystone/smc.S   | 1 +
 arch/arm/mach-omap2/Makefile   | 8 
 arch/arm/mach-omap2/omap-headsmp.S | 2 ++
 arch/arm/mach-omap2/omap-smc.S | 3 ++-
 arch/arm/mach-omap2/sleep33xx.S| 1 +
 arch/arm/mach-omap2/sleep34xx.S| 2 ++
 arch/arm/mach-omap2/sleep43xx.S| 2 ++
 arch/arm/mach-omap2/sleep44xx.S| 3 +++
 arch/arm/mach-tango/Makefile   | 3 ---
 arch/arm/mach-tango/smc.S  | 2 ++
 18 files changed, 21 insertions(+), 30 deletions(-)

diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
index 8fd23b263c60..b59c813b1af4 100644
--- a/arch/arm/mach-bcm/Makefile
+++ b/arch/arm/mach-bcm/Makefile
@@ -40,9 +40,6 @@ obj-$(CONFIG_ARCH_BCM_MOBILE_L2_CACHE) += kona_l2_cache.o
 
 # Support for secure monitor traps
 obj-$(CONFIG_ARCH_BCM_MOBILE_SMC) += bcm_kona_smc.o
-ifeq ($(call as-instr,.arch_extension sec,as_has_sec),as_has_sec)
-CFLAGS_bcm_kona_smc.o  += -Wa,-march=armv7-a+sec -DREQUIRES_SEC
-endif
 
 # BCM2835
 obj-$(CONFIG_ARCH_BCM2835) += board_bcm2835.o
diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c
index a55a7ecf146a..541e850a736c 100644
--- a/arch/arm/mach-bcm/bcm_kona_smc.c
+++ b/arch/arm/mach-bcm/bcm_kona_smc.c
@@ -125,9 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
__asmeq("%2", "r4")
__asmeq("%3", "r5")
__asmeq("%4", "r6")
-#ifdef REQUIRES_SEC
".arch_extension sec\n"
-#endif
"   smc#0\n"
: "=r" (ip), "=r" (r0)
: "r" (r4), "r" (r5), "r" (r6)
diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index 264dbaa89c3d..5ccf9d7e58d4 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -14,9 +14,5 @@ obj-$(CONFIG_PM_SLEEP)+= suspend.o
 
 obj-$(CONFIG_SMP)  += platsmp.o headsmp.o
 
-plus_sec := $(call as-instr,.arch_extension sec,+sec)
-AFLAGS_exynos-smc.o:=-Wa,-march=armv7-a$(plus_sec)
-AFLAGS_sleep.o :=-Wa,-march=armv7-a$(plus_sec)
-
 obj-$(CONFIG_MCPM) += mcpm-exynos.o
 CFLAGS_mcpm-exynos.o   += -march=armv7-a
diff --git a/arch/arm/mach-exynos/exynos-smc.S 
b/arch/arm/mach-exynos/exynos-smc.S
index d259532ba937..6da31e6a7acb 100644
--- a/arch/arm/mach-exynos/exynos-smc.S
+++ b/arch/arm/mach-exynos/exynos-smc.S
@@ -10,7 +10,8 @@
 /*
  * Function signature: void exynos_smc(u32 cmd, u32 arg1, u32 arg2, u32 arg3)
  */
-
+   .arch armv7-a
+   .arch_extension sec
 ENTRY(exynos_smc)
stmfd   sp!, {r4-r11, lr}
dsb
diff --git a/arch/arm/mach-exynos/sleep.S b/arch/arm/mach-exynos/sleep.S
index 2783c3a0c06a..ed93f91853b8 100644
--- a/arch/arm/mach-exynos/sleep.S
+++ b/arch/arm/mach-exynos/sleep.S
@@ -44,7 +44,8 @@ ENTRY(exynos_cpu_resume)
 ENDPROC(exynos_cpu_resume)
 
.align
-
+   .arch armv7-a
+   .arch_

[PATCH v4 2/2] ARM: OMAP2: drop explicit assembler architecture

2019-05-27 Thread Stefan Agner
OMAP2 depends on ARCH_MULTI_V6, which makes sure that the kernel is
compiled with -march=armv6. The compiler frontend will pass the
architecture to the assembler. There is no explicit architecture
specification necessary.

Signed-off-by: Stefan Agner 
Acked-by: Tony Lindgren 
---
Changes since v2:
- New patch

Changes since v3:
- Rebase on top of v5.2-rc2

 arch/arm/mach-omap2/Makefile | 5 -
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index f1d283995b31..600650551621 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -45,9 +45,6 @@ obj-$(CONFIG_SOC_DRA7XX)  += $(omap-4-5-common) 
$(smp-y) sleep44xx.o
 obj-$(CONFIG_SOC_OMAP2420) += sram242x.o
 obj-$(CONFIG_SOC_OMAP2430) += sram243x.o
 
-AFLAGS_sram242x.o  :=-Wa,-march=armv6
-AFLAGS_sram243x.o  :=-Wa,-march=armv6
-
 # Restart code (OMAP4/5 currently in omap4-common.c)
 obj-$(CONFIG_SOC_OMAP2420) += omap2-restart.o
 obj-$(CONFIG_SOC_OMAP2430) += omap2-restart.o
@@ -89,8 +86,6 @@ obj-$(CONFIG_PM_DEBUG)+= pm-debug.o
 obj-$(CONFIG_POWER_AVS_OMAP)   += sr_device.o
 obj-$(CONFIG_POWER_AVS_OMAP_CLASS3)+= smartreflex-class3.o
 
-AFLAGS_sleep24xx.o :=-Wa,-march=armv6
-
 endif
 
 ifeq ($(CONFIG_CPU_IDLE),y)
-- 
2.21.0



Re: Kconfig dependency issue on function-graph tracer and frame pointer on arm

2019-04-23 Thread Stefan Agner
On 15.04.2019 14:28, Arnd Bergmann wrote:
> On Sun, Apr 14, 2019 at 3:35 PM Russell King - ARM Linux admin
>  wrote:
> 
>> The subsequent hunks remove the defaulting of the choice according to
>> the function graph tracer - this is not a "hint" where the user can
>> still choose either option irrespective of the state of the function
>> graph tracer.  They should only be able to select the frame pointer
>> option in that case.
>>
>> Another way forward would be for someone to put the work in to making
>> the function graph tracer work without frame pointers.
> 
> I think Stefan was already looking into making CONFIG_FUNCTION_TRACER
> work with clang. I don't know what the status of that work is, but I
> think getting
> FUNCTION_GRAPH_TRACER working at the same time would be best.

Function Tracer is currently blocked by buggy mcount implemention on
Clang side. I do have a hacked up version which works with the buggy
Clang implementation, but not something we want to merge IMHO. see also:
https://bugs.llvm.org/show_bug.cgi?id=33845

> 
> I never noticed the Kconfig issue here, because I was using a patch to
> turn off FUNCTION_TRACER on ARM with clang to make it build, and that
> turns off FUNCTION_GRAPH_TRACER in the process.
> 
>> So, how about this:
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 850b4805e2d1..9aed25a6019b 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -73,7 +73,7 @@ config ARM
>> select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || 
>> CPU_V7) && MMU
>> select HAVE_EXIT_THREAD
>> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
>> -   select HAVE_FUNCTION_GRAPH_TRACER if !THUMB2_KERNEL
>> +   select HAVE_FUNCTION_GRAPH_TRACER if !THUMB2_KERNEL && !CC_IS_CLANG
>> select HAVE_FUNCTION_TRACER if !XIP_KERNEL

I think due to the fact above, we should add  && !CC_IS_CLANG here too.

>> select HAVE_GCC_PLUGINS
>> select HAVE_HW_BREAKPOINT if PERF_EVENTS && (CPU_V6 || CPU_V6K || 
>> CPU_V7)
>> diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
>> index 6d6e0330930b..e388af4594a6 100644
>> --- a/arch/arm/Kconfig.debug
>> +++ b/arch/arm/Kconfig.debug
>> @@ -47,8 +47,8 @@ config DEBUG_WX
>>
>>  choice
>> prompt "Choose kernel unwinder"
>> -   default UNWINDER_ARM if AEABI && !FUNCTION_GRAPH_TRACER
>> -   default UNWINDER_FRAME_POINTER if !AEABI || FUNCTION_GRAPH_TRACER
>> +   default UNWINDER_ARM if AEABI
>> +   default UNWINDER_FRAME_POINTER if !AEABI
>> help
>>   This determines which method will be used for unwinding kernel 
>> stack
>>   traces for panics, oopses, bugs, warnings, perf, /proc//stack,
>> @@ -65,7 +65,7 @@ config UNWINDER_FRAME_POINTER
>>
>>  config UNWINDER_ARM
>> bool "ARM EABI stack unwinder"
>> -   depends on AEABI
>> +   depends on AEABI && !FUNCTION_GRAPH_TRACER
>> select ARM_UNWIND
>> help
>>   This option enables stack unwinding support in the kernel
> 
> This looks good to me in the meantime, at least if there is any
> way to get the non-graph FUNCTION_TRACER to build with clang.

Looks sensible to me too.

Note that a similar issue came up a while ago on the mailing list:
https://marc.info/?l=linux-arm-kernel=154739414703313=2
[added Jeremy]

Unfortunately this never really materialized in a mergeable patch.

--
Stefan






Re: [PATCH v3 1/4] ARM: use arch_extension directive instead of arch argument

2019-04-23 Thread Stefan Agner
On 11.04.2019 09:54, Stefan Agner wrote:
> The LLVM Target parser currently does not allow to specify the security
> extension as part of -march (see also LLVM Bug 40186 [0]). When trying
> to use Clang with LLVM's integrated assembler, this leads to build
> errors such as this:
>   clang-8: error: the clang compiler does not support '-Wa,-march=armv7-a+sec'
> 
> Use ".arch_extension sec" to enable the security extension in a more
> portable fasion. Also make sure to use ".arch armv7-a" in case a v6/v7
> multi-platform kernel is being built.
> 
> Note that this is technically not exactly the same as the old code
> checked for availabilty of the security extension by calling as-instr.
> However, there are already other sites which use ".arch_extension sec"
> unconditionally, hence de-facto we need an assembler capable of
> ".arch_extension sec" already today (arch/arm/mm/proc-v7.S). The
> arch extension "sec" is available since binutils 2.21 according to
> its documentation [1].
> 
> [0] https://bugs.llvm.org/show_bug.cgi?id=40186
> [1] https://sourceware.org/binutils/docs-2.21/as/ARM-Options.html
> 
> Signed-off-by: Stefan Agner 
> Acked-by: Mans Rullgard 
> Acked-by: Arnd Bergmann 
> Acked-by: Krzysztof Kozlowski 

Arnd, Tony,

Patch 3 and 4 got merged by Gregory. I think the other two patches are
ready to be merged too. I think they should go in together to avoid
merge conflicts. Tony, if you agree, can you Ack patch 2 so they can get
merged through arm-soc?

--
Stefan

> ---
> Changes since v1:
> - Explicitly specify assembler architecture as armv7-a to avoid
>   build issues when bulding v6/v7 multi arch kernel.
> 
> Changes since v2:
> - Add armv7-a also in mach-tango
> - Move .arch armv7-a outside of ifdef'ed area in sleep44xx.S
>   to make the kernel compile also without CONFIG_SMP/PM.
> 
>  arch/arm/mach-bcm/Makefile | 3 ---
>  arch/arm/mach-bcm/bcm_kona_smc.c   | 2 --
>  arch/arm/mach-exynos/Makefile  | 4 
>  arch/arm/mach-exynos/exynos-smc.S  | 3 ++-
>  arch/arm/mach-exynos/sleep.S   | 3 ++-
>  arch/arm/mach-highbank/Makefile| 3 ---
>  arch/arm/mach-highbank/smc.S   | 3 ++-
>  arch/arm/mach-keystone/Makefile| 3 ---
>  arch/arm/mach-keystone/smc.S   | 1 +
>  arch/arm/mach-omap2/Makefile   | 8 
>  arch/arm/mach-omap2/omap-headsmp.S | 2 ++
>  arch/arm/mach-omap2/omap-smc.S | 3 ++-
>  arch/arm/mach-omap2/sleep33xx.S| 1 +
>  arch/arm/mach-omap2/sleep34xx.S| 2 ++
>  arch/arm/mach-omap2/sleep43xx.S| 2 ++
>  arch/arm/mach-omap2/sleep44xx.S| 3 +++
>  arch/arm/mach-tango/Makefile   | 3 ---
>  arch/arm/mach-tango/smc.S  | 2 ++
>  18 files changed, 21 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
> index 8fd23b263c60..b59c813b1af4 100644
> --- a/arch/arm/mach-bcm/Makefile
> +++ b/arch/arm/mach-bcm/Makefile
> @@ -40,9 +40,6 @@ obj-$(CONFIG_ARCH_BCM_MOBILE_L2_CACHE) += kona_l2_cache.o
>  
>  # Support for secure monitor traps
>  obj-$(CONFIG_ARCH_BCM_MOBILE_SMC) += bcm_kona_smc.o
> -ifeq ($(call as-instr,.arch_extension sec,as_has_sec),as_has_sec)
> -CFLAGS_bcm_kona_smc.o+= -Wa,-march=armv7-a+sec -DREQUIRES_SEC
> -endif
>  
>  # BCM2835
>  obj-$(CONFIG_ARCH_BCM2835)   += board_bcm2835.o
> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c 
> b/arch/arm/mach-bcm/bcm_kona_smc.c
> index a55a7ecf146a..541e850a736c 100644
> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
> @@ -125,9 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32 
> buffer_phys)
>   __asmeq("%2", "r4")
>   __asmeq("%3", "r5")
>   __asmeq("%4", "r6")
> -#ifdef REQUIRES_SEC
>   ".arch_extension sec\n"
> -#endif
>   "   smc#0\n"
>   : "=r" (ip), "=r" (r0)
>   : "r" (r4), "r" (r5), "r" (r6)
> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> index cd00c82a1add..44de9f36fd1b 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -14,9 +14,5 @@ obj-$(CONFIG_PM_SLEEP)  += suspend.o
>  
>  obj-$(CONFIG_SMP)+= platsmp.o headsmp.o
>  
> -plus_sec := $(call as-instr,.arch_extension sec,+sec)
> -AFLAGS_exynos-smc.o  :=-Wa,-march=armv7-a$(plus_sec)
> -AFLAGS_sleep.o   :=-Wa,-march=armv7-a$(plus_sec)
> -
>  obj-$(CONFIG_EXYNOS5420_MCPM)+= mcpm-exynos.o
>  CFLAGS_mcpm-exynos.o += -march=ar

[PATCH] mtd: rawnand: use longest matching pattern

2019-04-19 Thread Stefan Agner
Sometimes the exec_op parser does not choose the optimal pattern if
multiple patterns with optional elements are available. Since the stack
automatically splits operations in multiple exec_op calls, a non-optimal
pattern gets broken up into multiple calls. E.g. an OOB read using the
vf610 driver:
  nand: executing subop:
  nand: ->CMD  [0x00]
  nand: ->ADDR [5 cyc: 00 08 ea 94 02]
  nand: ->CMD  [0x30]
  nand: ->WAITRDY  [max 20 ms]
  nand:   DATA_IN  [64 B]
  nand: executing subop:
  nand:   CMD  [0x00]
  nand:   ADDR [5 cyc: 00 08 ea 94 02]
  nand:   CMD  [0x30]
  nand:   WAITRDY  [max 20 ms]
  nand: ->DATA_IN  [64 B]

However, the vf610 driver has a pattern which can execute the complete
command in a single go...

This patch makes sure that the longest matching pattern is chosen
instead of the first (potentially only partial) match. With this
change the vf610 reads the OOB in a single exec_op call:
  nand: executing subop:
  nand: ->CMD  [0x00]
  nand: ->ADDR [5 cyc: 00 08 c0 1d 00]
  nand: ->CMD  [0x30]
  nand: ->WAITRDY  [max 20 ms]
  nand: ->DATA_IN  [64 B]

Reported-by: Sascha Hauer 
Suggested-by: Boris Brezillon 
Tested-by: Stefan Agner 
Signed-off-by: Stefan Agner 
---
 drivers/mtd/nand/raw/nand_base.c | 50 +++-
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index ddd396e93e32..ebf219c05853 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -2131,6 +2131,22 @@ static void nand_op_parser_trace(const struct 
nand_op_parser_ctx *ctx)
 }
 #endif
 
+static int nand_op_parser_cmp_ctx(const struct nand_op_parser_ctx *a,
+ const struct nand_op_parser_ctx *b)
+{
+   if (a->subop.ninstrs < b->subop.ninstrs)
+   return -1;
+   else if (a->subop.ninstrs > b->subop.ninstrs)
+   return 1;
+
+   if (a->subop.last_instr_end_off < b->subop.last_instr_end_off)
+   return -1;
+   else if (a->subop.last_instr_end_off > b->subop.last_instr_end_off)
+   return 1;
+
+   return 0;
+}
+
 /**
  * nand_op_parser_exec_op - exec_op parser
  * @chip: the NAND chip
@@ -2165,32 +2181,40 @@ int nand_op_parser_exec_op(struct nand_chip *chip,
unsigned int i;
 
while (ctx.subop.instrs < op->instrs + op->ninstrs) {
-   int ret;
+   const struct nand_op_parser_pattern *pattern;
+   struct nand_op_parser_ctx best_ctx;
+   int ret, best_pattern = -1;
 
for (i = 0; i < parser->npatterns; i++) {
-   const struct nand_op_parser_pattern *pattern;
+   struct nand_op_parser_ctx test_ctx = ctx;
 
pattern = >patterns[i];
-   if (!nand_op_parser_match_pat(pattern, ))
+   if (!nand_op_parser_match_pat(pattern, _ctx))
continue;
 
-   nand_op_parser_trace();
-
-   if (check_only)
-   break;
-
-   ret = pattern->exec(chip, );
-   if (ret)
-   return ret;
+   if (best_pattern >= 0 &&
+   nand_op_parser_cmp_ctx(_ctx, _ctx) <= 0)
+   continue;
 
-   break;
+   best_pattern = i;
+   best_ctx = test_ctx;
}
 
-   if (i == parser->npatterns) {
+   if (best_pattern < 0) {
pr_debug("->exec_op() parser: pattern not found!\n");
return -ENOTSUPP;
}
 
+   ctx = best_ctx;
+   nand_op_parser_trace();
+
+   if (!check_only) {
+   pattern = >patterns[best_pattern];
+   ret = pattern->exec(chip, );
+   if (ret)
+   return ret;
+   }
+
/*
 * Update the context structure by pointing to the start of the
 * next subop.
-- 
2.21.0



[PATCH v3 3/4] ARM: mvebu: drop unnecessary label

2019-04-11 Thread Stefan Agner
The label mvebu_boot_wa_start is not necessary and causes a build
issue when building with LLVM's integrated assembler:
AS  arch/arm/mach-mvebu/pmsu_ll.o
  arch/arm/mach-mvebu/pmsu_ll.S:59:1: error: invalid symbol redefinition
  mvebu_boot_wa_start:
  ^

Drop the label.

Signed-off-by: Stefan Agner 
Acked-by: Nicolas Pitre 
---
 arch/arm/mach-mvebu/pmsu_ll.S | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/pmsu_ll.S b/arch/arm/mach-mvebu/pmsu_ll.S
index 88651221dbdd..c1fb713e9306 100644
--- a/arch/arm/mach-mvebu/pmsu_ll.S
+++ b/arch/arm/mach-mvebu/pmsu_ll.S
@@ -56,7 +56,6 @@ ENDPROC(armada_38x_cpu_resume)
 
 /* The following code will be executed from SRAM */
 ENTRY(mvebu_boot_wa_start)
-mvebu_boot_wa_start:
 ARM_BE8(setend be)
adr r0, 1f
ldr r0, [r0]@ load the address of the
-- 
2.21.0



[PATCH v3 2/4] ARM: OMAP2: drop explicit assembler architecture

2019-04-11 Thread Stefan Agner
OMAP2 depends on ARCH_MULTI_V6, which makes sure that the kernel is
compiled with -march=armv6. The compiler frontend will pass the
architecture to the assembler. There is no explicit architecture
specification necessary.

Signed-off-by: Stefan Agner 
---
Changes since v2:
- New patch

 arch/arm/mach-omap2/Makefile | 5 -
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index f1d283995b31..600650551621 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -45,9 +45,6 @@ obj-$(CONFIG_SOC_DRA7XX)  += $(omap-4-5-common) 
$(smp-y) sleep44xx.o
 obj-$(CONFIG_SOC_OMAP2420) += sram242x.o
 obj-$(CONFIG_SOC_OMAP2430) += sram243x.o
 
-AFLAGS_sram242x.o  :=-Wa,-march=armv6
-AFLAGS_sram243x.o  :=-Wa,-march=armv6
-
 # Restart code (OMAP4/5 currently in omap4-common.c)
 obj-$(CONFIG_SOC_OMAP2420) += omap2-restart.o
 obj-$(CONFIG_SOC_OMAP2430) += omap2-restart.o
@@ -89,8 +86,6 @@ obj-$(CONFIG_PM_DEBUG)+= pm-debug.o
 obj-$(CONFIG_POWER_AVS_OMAP)   += sr_device.o
 obj-$(CONFIG_POWER_AVS_OMAP_CLASS3)+= smartreflex-class3.o
 
-AFLAGS_sleep24xx.o :=-Wa,-march=armv6
-
 endif
 
 ifeq ($(CONFIG_CPU_IDLE),y)
-- 
2.21.0



[PATCH v3 1/4] ARM: use arch_extension directive instead of arch argument

2019-04-11 Thread Stefan Agner
The LLVM Target parser currently does not allow to specify the security
extension as part of -march (see also LLVM Bug 40186 [0]). When trying
to use Clang with LLVM's integrated assembler, this leads to build
errors such as this:
  clang-8: error: the clang compiler does not support '-Wa,-march=armv7-a+sec'

Use ".arch_extension sec" to enable the security extension in a more
portable fasion. Also make sure to use ".arch armv7-a" in case a v6/v7
multi-platform kernel is being built.

Note that this is technically not exactly the same as the old code
checked for availabilty of the security extension by calling as-instr.
However, there are already other sites which use ".arch_extension sec"
unconditionally, hence de-facto we need an assembler capable of
".arch_extension sec" already today (arch/arm/mm/proc-v7.S). The
arch extension "sec" is available since binutils 2.21 according to
its documentation [1].

[0] https://bugs.llvm.org/show_bug.cgi?id=40186
[1] https://sourceware.org/binutils/docs-2.21/as/ARM-Options.html

Signed-off-by: Stefan Agner 
Acked-by: Mans Rullgard 
Acked-by: Arnd Bergmann 
Acked-by: Krzysztof Kozlowski 
---
Changes since v1:
- Explicitly specify assembler architecture as armv7-a to avoid
  build issues when bulding v6/v7 multi arch kernel.

Changes since v2:
- Add armv7-a also in mach-tango
- Move .arch armv7-a outside of ifdef'ed area in sleep44xx.S
  to make the kernel compile also without CONFIG_SMP/PM.

 arch/arm/mach-bcm/Makefile | 3 ---
 arch/arm/mach-bcm/bcm_kona_smc.c   | 2 --
 arch/arm/mach-exynos/Makefile  | 4 
 arch/arm/mach-exynos/exynos-smc.S  | 3 ++-
 arch/arm/mach-exynos/sleep.S   | 3 ++-
 arch/arm/mach-highbank/Makefile| 3 ---
 arch/arm/mach-highbank/smc.S   | 3 ++-
 arch/arm/mach-keystone/Makefile| 3 ---
 arch/arm/mach-keystone/smc.S   | 1 +
 arch/arm/mach-omap2/Makefile   | 8 
 arch/arm/mach-omap2/omap-headsmp.S | 2 ++
 arch/arm/mach-omap2/omap-smc.S | 3 ++-
 arch/arm/mach-omap2/sleep33xx.S| 1 +
 arch/arm/mach-omap2/sleep34xx.S| 2 ++
 arch/arm/mach-omap2/sleep43xx.S| 2 ++
 arch/arm/mach-omap2/sleep44xx.S| 3 +++
 arch/arm/mach-tango/Makefile   | 3 ---
 arch/arm/mach-tango/smc.S  | 2 ++
 18 files changed, 21 insertions(+), 30 deletions(-)

diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
index 8fd23b263c60..b59c813b1af4 100644
--- a/arch/arm/mach-bcm/Makefile
+++ b/arch/arm/mach-bcm/Makefile
@@ -40,9 +40,6 @@ obj-$(CONFIG_ARCH_BCM_MOBILE_L2_CACHE) += kona_l2_cache.o
 
 # Support for secure monitor traps
 obj-$(CONFIG_ARCH_BCM_MOBILE_SMC) += bcm_kona_smc.o
-ifeq ($(call as-instr,.arch_extension sec,as_has_sec),as_has_sec)
-CFLAGS_bcm_kona_smc.o  += -Wa,-march=armv7-a+sec -DREQUIRES_SEC
-endif
 
 # BCM2835
 obj-$(CONFIG_ARCH_BCM2835) += board_bcm2835.o
diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c
index a55a7ecf146a..541e850a736c 100644
--- a/arch/arm/mach-bcm/bcm_kona_smc.c
+++ b/arch/arm/mach-bcm/bcm_kona_smc.c
@@ -125,9 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
__asmeq("%2", "r4")
__asmeq("%3", "r5")
__asmeq("%4", "r6")
-#ifdef REQUIRES_SEC
".arch_extension sec\n"
-#endif
"   smc#0\n"
: "=r" (ip), "=r" (r0)
: "r" (r4), "r" (r5), "r" (r6)
diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index cd00c82a1add..44de9f36fd1b 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -14,9 +14,5 @@ obj-$(CONFIG_PM_SLEEP)+= suspend.o
 
 obj-$(CONFIG_SMP)  += platsmp.o headsmp.o
 
-plus_sec := $(call as-instr,.arch_extension sec,+sec)
-AFLAGS_exynos-smc.o:=-Wa,-march=armv7-a$(plus_sec)
-AFLAGS_sleep.o :=-Wa,-march=armv7-a$(plus_sec)
-
 obj-$(CONFIG_EXYNOS5420_MCPM)  += mcpm-exynos.o
 CFLAGS_mcpm-exynos.o   += -march=armv7-a
diff --git a/arch/arm/mach-exynos/exynos-smc.S 
b/arch/arm/mach-exynos/exynos-smc.S
index d259532ba937..6da31e6a7acb 100644
--- a/arch/arm/mach-exynos/exynos-smc.S
+++ b/arch/arm/mach-exynos/exynos-smc.S
@@ -10,7 +10,8 @@
 /*
  * Function signature: void exynos_smc(u32 cmd, u32 arg1, u32 arg2, u32 arg3)
  */
-
+   .arch armv7-a
+   .arch_extension sec
 ENTRY(exynos_smc)
stmfd   sp!, {r4-r11, lr}
dsb
diff --git a/arch/arm/mach-exynos/sleep.S b/arch/arm/mach-exynos/sleep.S
index 2783c3a0c06a..ed93f91853b8 100644
--- a/arch/arm/mach-exynos/sleep.S
+++ b/arch/arm/mach-exynos/sleep.S
@@ -44,7 +44,8 @@ ENTRY(exynos_cpu_resume)
 ENDPROC(exynos_cpu_resume)
 
.align
-
+   .arch armv7-a
+   .arch_extension sec
 ENTRY(exynos_cpu_resume_ns)
  

[PATCH v3 4/4] ARM: mvebu: prefix coprocessor operand with p

2019-04-11 Thread Stefan Agner
In every other instance where mrc is used the coprocessor operand
is prefix with p (e.g. p15). Use the p prefix in this case too.
This fixes a build issue when using LLVM's integrated assembler:
  arch/arm/mach-mvebu/coherency_ll.S:69:6: error: invalid operand for 
instruction
   mrc 15, 0, r3, cr0, cr0, 5
   ^
  arch/arm/mach-mvebu/pmsu_ll.S:19:6: error: invalid operand for instruction
   mrc 15, 0, r0, cr0, cr0, 5 @ get the CPU ID
   ^

Signed-off-by: Stefan Agner 
Acked-by: Nicolas Pitre 
---
 arch/arm/mach-mvebu/coherency_ll.S | 2 +-
 arch/arm/mach-mvebu/pmsu_ll.S  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-mvebu/coherency_ll.S 
b/arch/arm/mach-mvebu/coherency_ll.S
index 8b2fbc8b6bc6..2d962fe48821 100644
--- a/arch/arm/mach-mvebu/coherency_ll.S
+++ b/arch/arm/mach-mvebu/coherency_ll.S
@@ -66,7 +66,7 @@ ENDPROC(ll_get_coherency_base)
  * fabric registers
  */
 ENTRY(ll_get_coherency_cpumask)
-   mrc 15, 0, r3, cr0, cr0, 5
+   mrc p15, 0, r3, cr0, cr0, 5
and r3, r3, #15
mov r2, #(1 << 24)
lsl r3, r2, r3
diff --git a/arch/arm/mach-mvebu/pmsu_ll.S b/arch/arm/mach-mvebu/pmsu_ll.S
index c1fb713e9306..7aae9a25cfeb 100644
--- a/arch/arm/mach-mvebu/pmsu_ll.S
+++ b/arch/arm/mach-mvebu/pmsu_ll.S
@@ -16,7 +16,7 @@
 ENTRY(armada_38x_scu_power_up)
mrc p15, 4, r1, c15, c0 @ get SCU base address
orr r1, r1, #0x8@ SCU CPU Power Status Register
-   mrc 15, 0, r0, cr0, cr0, 5  @ get the CPU ID
+   mrc p15, 0, r0, cr0, cr0, 5 @ get the CPU ID
and r0, r0, #15
add r1, r1, r0
mov r0, #0x0
-- 
2.21.0



[PATCH] ARM: imx: use generic function to exit coherency

2019-04-10 Thread Stefan Agner
The common ARM architecture code provides a generic function to exit
coherency called v7_exit_coherency_flush(). Replace the machine
specific implementation using the generic function.

Tested on a i.MX 6Dual by hotplugging the secondary CPU under load
through sysfs several 1000 times.

Tested-by: Stefan Agner 
Signed-off-by: Stefan Agner 
---
 arch/arm/mach-imx/hotplug.c | 24 ++--
 1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/arch/arm/mach-imx/hotplug.c b/arch/arm/mach-imx/hotplug.c
index b35e99cc5e5b..7a74a9b89a49 100644
--- a/arch/arm/mach-imx/hotplug.c
+++ b/arch/arm/mach-imx/hotplug.c
@@ -12,32 +12,12 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
 #include "common.h"
 
-static inline void cpu_enter_lowpower(void)
-{
-   unsigned int v;
-
-   asm volatile(
-   "mcrp15, 0, %1, c7, c5, 0\n"
-   "   mcr p15, 0, %1, c7, c10, 4\n"
-   /*
-* Turn off coherency
-*/
-   "   mrc p15, 0, %0, c1, c0, 1\n"
-   "   bic %0, %0, %3\n"
-   "   mcr p15, 0, %0, c1, c0, 1\n"
-   "   mrc p15, 0, %0, c1, c0, 0\n"
-   "   bic %0, %0, %2\n"
-   "   mcr p15, 0, %0, c1, c0, 0\n"
- : "=" (v)
- : "r" (0), "Ir" (CR_C), "Ir" (0x40)
- : "cc");
-}
-
 /*
  * platform-specific code to shutdown a CPU
  *
@@ -45,7 +25,7 @@ static inline void cpu_enter_lowpower(void)
  */
 void imx_cpu_die(unsigned int cpu)
 {
-   cpu_enter_lowpower();
+   v7_exit_coherency_flush(louis);
/*
 * We use the cpu jumping argument register to sync with
 * imx_cpu_kill() which is running on cpu0 and waiting for
-- 
2.21.0



Re: [PATCH v2 1/3] ARM: use arch_extension directive instead of arch argument

2019-04-10 Thread Stefan Agner
On 09.04.2019 16:50, Tony Lindgren wrote:
> Hi,
> 
> * Stefan Agner  [190408 20:59]:
>> --- a/arch/arm/mach-omap2/Makefile
>> +++ b/arch/arm/mach-omap2/Makefile
>> @@ -41,11 +41,6 @@ obj-$(CONFIG_SOC_OMAP5)   += 
>> $(omap-4-5-common) $(smp-y) sleep44xx.o
>>  obj-$(CONFIG_SOC_AM43XX)+= $(omap-4-5-common)
>>  obj-$(CONFIG_SOC_DRA7XX)+= $(omap-4-5-common) $(smp-y) 
>> sleep44xx.o
>>
>> -plus_sec := $(call as-instr,.arch_extension sec,+sec)
>> -AFLAGS_omap-headsmp.o   :=-Wa,-march=armv7-a$(plus_sec)
>> -AFLAGS_omap-smc.o   :=-Wa,-march=armv7-a$(plus_sec)
>> -AFLAGS_sleep44xx.o  :=-Wa,-march=armv7-a$(plus_sec)
>> -
>>  # Functions loaded to SRAM
>>  obj-$(CONFIG_SOC_OMAP2420)  += sram242x.o
>>  obj-$(CONFIG_SOC_OMAP2430)  += sram243x.o
>> @@ -95,9 +90,6 @@ obj-$(CONFIG_POWER_AVS_OMAP)   += sr_device.o
>>  obj-$(CONFIG_POWER_AVS_OMAP_CLASS3)+= smartreflex-class3.o
>>
>>  AFLAGS_sleep24xx.o  :=-Wa,-march=armv6
>> -AFLAGS_sleep34xx.o  :=-Wa,-march=armv7-a$(plus_sec)
>> -AFLAGS_sleep33xx.o  :=-Wa,-march=armv7-a$(plus_sec)
>> -AFLAGS_sleep43xx.o  :=-Wa,-march=armv7-a$(plus_sec)
> 
> I think we should also change the AFLAGS_sleep24xx.o above the
> same way but with armv6 flags. This can be build tested with
> omap2plus_defconfig.

>From what I can tell, since those do not add the sec extension they
should work fine for LLVM's integrated assembler. But I agree, for
consistency it would be nice to get rid of them the same way too.

A bit further up, there is also:
AFLAGS_sram242x.o  :=-Wa,-march=armv6
AFLAGS_sram243x.o  :=-Wa,-march=armv6

I think those explicit arch definitions are not even necessary since
ARCH_OMAP2 depends on ARCH_MULTI_V6, which cannot be built with pre v6
architecture. So the minimum architecture we build for will be armv6...

In a quick test omap2plus_defconfig builds fine without those AFLAGS. I
will put it through some more testing and drop those flags in v3.

--
Stefan


> 
> Regards,
> 
> Tony


Re: [PATCH v2 1/3] ARM: use arch_extension directive instead of arch argument

2019-04-10 Thread Stefan Agner
On 09.04.2019 14:25, Måns Rullgård wrote:
> Stefan Agner  writes:
> 
>> The LLVM Target parser currently does not allow to specify the security
>> extension as part of -march (see also LLVM Bug 40186 [0]). When trying
>> to use Clang with LLVM's integrated assembler, this leads to build
>> errors such as this:
>>   clang-8: error: the clang compiler does not support 
>> '-Wa,-march=armv7-a+sec'
>>
>> Use ".arch_extension sec" to enable the security extension in a more
>> portable fasion. Also make sure to use ".arch armv7-a" in case a v6/v7
>> multi-platform kernel is being built.
>>
>> Note that this is technically not exactly the same as the old code
>> checked for availabilty of the security extension by calling as-instr.
>> However, there are already other sites which use ".arch_extension sec"
>> unconditionally, hence de-facto we need an assembler capable of
>> ".arch_extension sec" already today (arch/arm/mm/proc-v7.S). The
>> arch extension "sec" is available since binutils 2.21 according to
>> its documentation [1].
>>
>> [0] https://bugs.llvm.org/show_bug.cgi?id=40186
>> [1] https://sourceware.org/binutils/docs-2.21/as/ARM-Options.html
>>
>> Signed-off-by: Stefan Agner 
>> Acked-by: Mans Rullgard 
>> Acked-by: Arnd Bergmann 
>> Acked-by: Krzysztof Kozlowski 
>> ---
>> Changes since v1:
>> - Explicitly specify assembler architecture as armv7-a to avoid
>>   build issues when bulding v6/v7 multi arch kernel.
>>
>>  arch/arm/mach-bcm/Makefile | 3 ---
>>  arch/arm/mach-bcm/bcm_kona_smc.c   | 2 --
>>  arch/arm/mach-exynos/Makefile  | 4 
>>  arch/arm/mach-exynos/exynos-smc.S  | 3 ++-
>>  arch/arm/mach-exynos/sleep.S   | 3 ++-
>>  arch/arm/mach-highbank/Makefile| 3 ---
>>  arch/arm/mach-highbank/smc.S   | 3 ++-
>>  arch/arm/mach-keystone/Makefile| 3 ---
>>  arch/arm/mach-keystone/smc.S   | 1 +
>>  arch/arm/mach-omap2/Makefile   | 8 
>>  arch/arm/mach-omap2/omap-headsmp.S | 2 ++
>>  arch/arm/mach-omap2/omap-smc.S | 3 ++-
>>  arch/arm/mach-omap2/sleep33xx.S| 1 +
>>  arch/arm/mach-omap2/sleep34xx.S| 2 ++
>>  arch/arm/mach-omap2/sleep43xx.S| 2 ++
>>  arch/arm/mach-omap2/sleep44xx.S| 2 ++
>>  arch/arm/mach-tango/Makefile   | 3 ---
>>  arch/arm/mach-tango/smc.S  | 1 +
>>  18 files changed, 19 insertions(+), 30 deletions(-)
> 
> [...]
> 
>> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c 
>> b/arch/arm/mach-bcm/bcm_kona_smc.c
>> index a55a7ecf146a..541e850a736c 100644
>> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
>> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
>> @@ -125,9 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32 
>> buffer_phys)
>>  __asmeq("%2", "r4")
>>  __asmeq("%3", "r5")
>>  __asmeq("%4", "r6")
>> -#ifdef REQUIRES_SEC
>>  ".arch_extension sec\n"
>> -#endif
>>  "   smc#0\n"
>>  : "=r" (ip), "=r" (r0)
>>  : "r" (r4), "r" (r5), "r" (r6)
> 
> [...]
> 
>> diff --git a/arch/arm/mach-keystone/smc.S b/arch/arm/mach-keystone/smc.S
>> index d15de8179fab..ec03dc499270 100644
>> --- a/arch/arm/mach-keystone/smc.S
>> +++ b/arch/arm/mach-keystone/smc.S
>> @@ -21,6 +21,7 @@
>>   *
>>   * Return: Non zero value on failure
>>   */
>> +.arch_extension sec
>>  ENTRY(keystone_cpu_smc)
>>  stmfd   sp!, {r4-r11, lr}
>>  smc #0
> 
> [...]
> 
>> diff --git a/arch/arm/mach-tango/smc.S b/arch/arm/mach-tango/smc.S
>> index 361a8dc89804..cf2d21e5226c 100644
>> --- a/arch/arm/mach-tango/smc.S
>> +++ b/arch/arm/mach-tango/smc.S
>> @@ -1,6 +1,7 @@
>>  /* SPDX-License-Identifier: GPL-2.0 */
>>  #include 
>>
>> +.arch_extension sec
>>  ENTRY(tango_smc)
>>  push{lr}
>>  mov ip, r1

Actually, mach-tango uses dsb and needs the .arch directive. Will fix in
v3.

--
Stefan

> 
> Is there some reason these three don't need the .arch directive?


Re: [PATCH v1 1/1] ARM: dts: colibri/apalis: convert to SPDX license tags

2019-04-09 Thread Stefan Agner
> - *
> - * Or, alternatively,
> - *
> - *  b) Permission is hereby granted, free of charge, to any person
> - * obtaining a copy of this software and associated documentation
> - * files (the "Software"), to deal in the Software without
> - * restriction, including without limitation the rights to use,
> - * copy, modify, merge, publish, distribute, sublicense, and/or
> - * sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following
> - * conditions:
> - *
> - * The above copyright notice and this permission notice shall be
> - * included in all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> - * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> - * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> - * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> - * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> - * OTHER DEALINGS IN THE SOFTWARE.
>   */
>  
>  #include "vf500.dtsi"
> diff --git a/arch/arm/boot/dts/vf610-colibri-eval-v3.dts
> b/arch/arm/boot/dts/vf610-colibri-eval-v3.dts
> index ef9b4d6209f6..048c8de951cc 100644
> --- a/arch/arm/boot/dts/vf610-colibri-eval-v3.dts
> +++ b/arch/arm/boot/dts/vf610-colibri-eval-v3.dts
> @@ -1,42 +1,6 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR X11)
>  /*
>   * Copyright 2014 Toradex AG
> - *
> - * This file is dual-licensed: you can use it either under the terms
> - * of the GPL or the X11 license, at your option. Note that this dual
> - * licensing only applies to this file, and not this project as a
> - * whole.
> - *
> - *  a) This file is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License
> - * version 2 as published by the Free Software Foundation.
> - *
> - * This file is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - *
> - * Or, alternatively,
> - *
> - *  b) Permission is hereby granted, free of charge, to any person
> - * obtaining a copy of this software and associated documentation
> - * files (the "Software"), to deal in the Software without
> - * restriction, including without limitation the rights to use,
> - * copy, modify, merge, publish, distribute, sublicense, and/or
> - * sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following
> - * conditions:
> - *
> - * The above copyright notice and this permission notice shall be
> - * included in all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> - * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> - * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> - * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> - * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> - * OTHER DEALINGS IN THE SOFTWARE.
>   */
>  
>  /dts-v1/;
> diff --git a/arch/arm/boot/dts/vf610-colibri.dtsi
> b/arch/arm/boot/dts/vf610-colibri.dtsi
> index 05c9a39509b8..415efab39a95 100644
> --- a/arch/arm/boot/dts/vf610-colibri.dtsi
> +++ b/arch/arm/boot/dts/vf610-colibri.dtsi
> @@ -1,42 +1,6 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR X11)
>  /*
> - * Copyright 2014 Toradex AG
> - *
> - * This file is dual-licensed: you can use it either under the terms
> - * of the GPL or the X11 license, at your option. Note that this dual
> - * licensing only applies to this file, and not this project as a
> - * whole.
> - *
> - *  a) This file is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License
> - * version 2 as published by the Free Software Foundation.
> - *
> - * This file is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Pub

Re: [PATCH v2 1/3] ARM: use arch_extension directive instead of arch argument

2019-04-09 Thread Stefan Agner
On 09.04.2019 14:25, Måns Rullgård wrote:
> Stefan Agner  writes:
> 
>> The LLVM Target parser currently does not allow to specify the security
>> extension as part of -march (see also LLVM Bug 40186 [0]). When trying
>> to use Clang with LLVM's integrated assembler, this leads to build
>> errors such as this:
>>   clang-8: error: the clang compiler does not support 
>> '-Wa,-march=armv7-a+sec'
>>
>> Use ".arch_extension sec" to enable the security extension in a more
>> portable fasion. Also make sure to use ".arch armv7-a" in case a v6/v7
>> multi-platform kernel is being built.
>>
>> Note that this is technically not exactly the same as the old code
>> checked for availabilty of the security extension by calling as-instr.
>> However, there are already other sites which use ".arch_extension sec"
>> unconditionally, hence de-facto we need an assembler capable of
>> ".arch_extension sec" already today (arch/arm/mm/proc-v7.S). The
>> arch extension "sec" is available since binutils 2.21 according to
>> its documentation [1].
>>
>> [0] https://bugs.llvm.org/show_bug.cgi?id=40186
>> [1] https://sourceware.org/binutils/docs-2.21/as/ARM-Options.html
>>
>> Signed-off-by: Stefan Agner 
>> Acked-by: Mans Rullgard 
>> Acked-by: Arnd Bergmann 
>> Acked-by: Krzysztof Kozlowski 
>> ---
>> Changes since v1:
>> - Explicitly specify assembler architecture as armv7-a to avoid
>>   build issues when bulding v6/v7 multi arch kernel.
>>
>>  arch/arm/mach-bcm/Makefile | 3 ---
>>  arch/arm/mach-bcm/bcm_kona_smc.c   | 2 --
>>  arch/arm/mach-exynos/Makefile  | 4 
>>  arch/arm/mach-exynos/exynos-smc.S  | 3 ++-
>>  arch/arm/mach-exynos/sleep.S   | 3 ++-
>>  arch/arm/mach-highbank/Makefile| 3 ---
>>  arch/arm/mach-highbank/smc.S   | 3 ++-
>>  arch/arm/mach-keystone/Makefile| 3 ---
>>  arch/arm/mach-keystone/smc.S   | 1 +
>>  arch/arm/mach-omap2/Makefile   | 8 
>>  arch/arm/mach-omap2/omap-headsmp.S | 2 ++
>>  arch/arm/mach-omap2/omap-smc.S | 3 ++-
>>  arch/arm/mach-omap2/sleep33xx.S| 1 +
>>  arch/arm/mach-omap2/sleep34xx.S| 2 ++
>>  arch/arm/mach-omap2/sleep43xx.S| 2 ++
>>  arch/arm/mach-omap2/sleep44xx.S| 2 ++
>>  arch/arm/mach-tango/Makefile   | 3 ---
>>  arch/arm/mach-tango/smc.S  | 1 +
>>  18 files changed, 19 insertions(+), 30 deletions(-)
> 
> [...]
> 
>> diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c 
>> b/arch/arm/mach-bcm/bcm_kona_smc.c
>> index a55a7ecf146a..541e850a736c 100644
>> --- a/arch/arm/mach-bcm/bcm_kona_smc.c
>> +++ b/arch/arm/mach-bcm/bcm_kona_smc.c
>> @@ -125,9 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32 
>> buffer_phys)
>>  __asmeq("%2", "r4")
>>  __asmeq("%3", "r5")
>>  __asmeq("%4", "r6")
>> -#ifdef REQUIRES_SEC
>>  ".arch_extension sec\n"
>> -#endif
>>  "   smc#0\n"
>>  : "=r" (ip), "=r" (r0)
>>  : "r" (r4), "r" (r5), "r" (r6)
> 
> [...]
> 
>> diff --git a/arch/arm/mach-keystone/smc.S b/arch/arm/mach-keystone/smc.S
>> index d15de8179fab..ec03dc499270 100644
>> --- a/arch/arm/mach-keystone/smc.S
>> +++ b/arch/arm/mach-keystone/smc.S
>> @@ -21,6 +21,7 @@
>>   *
>>   * Return: Non zero value on failure
>>   */
>> +.arch_extension sec
>>  ENTRY(keystone_cpu_smc)
>>  stmfd   sp!, {r4-r11, lr}
>>  smc #0
> 
> [...]
> 
>> diff --git a/arch/arm/mach-tango/smc.S b/arch/arm/mach-tango/smc.S
>> index 361a8dc89804..cf2d21e5226c 100644
>> --- a/arch/arm/mach-tango/smc.S
>> +++ b/arch/arm/mach-tango/smc.S
>> @@ -1,6 +1,7 @@
>>  /* SPDX-License-Identifier: GPL-2.0 */
>>  #include 
>>
>> +.arch_extension sec
>>  ENTRY(tango_smc)
>>  push{lr}
>>  mov ip, r1
> 
> Is there some reason these three don't need the .arch directive?

They all do not use a memory barrier instruction (e.g. dmb) which caused
issues on the other files.

--
Stefan


[PATCH v2 1/3] ARM: use arch_extension directive instead of arch argument

2019-04-08 Thread Stefan Agner
The LLVM Target parser currently does not allow to specify the security
extension as part of -march (see also LLVM Bug 40186 [0]). When trying
to use Clang with LLVM's integrated assembler, this leads to build
errors such as this:
  clang-8: error: the clang compiler does not support '-Wa,-march=armv7-a+sec'

Use ".arch_extension sec" to enable the security extension in a more
portable fasion. Also make sure to use ".arch armv7-a" in case a v6/v7
multi-platform kernel is being built.

Note that this is technically not exactly the same as the old code
checked for availabilty of the security extension by calling as-instr.
However, there are already other sites which use ".arch_extension sec"
unconditionally, hence de-facto we need an assembler capable of
".arch_extension sec" already today (arch/arm/mm/proc-v7.S). The
arch extension "sec" is available since binutils 2.21 according to
its documentation [1].

[0] https://bugs.llvm.org/show_bug.cgi?id=40186
[1] https://sourceware.org/binutils/docs-2.21/as/ARM-Options.html

Signed-off-by: Stefan Agner 
Acked-by: Mans Rullgard 
Acked-by: Arnd Bergmann 
Acked-by: Krzysztof Kozlowski 
---
Changes since v1:
- Explicitly specify assembler architecture as armv7-a to avoid
  build issues when bulding v6/v7 multi arch kernel.

 arch/arm/mach-bcm/Makefile | 3 ---
 arch/arm/mach-bcm/bcm_kona_smc.c   | 2 --
 arch/arm/mach-exynos/Makefile  | 4 
 arch/arm/mach-exynos/exynos-smc.S  | 3 ++-
 arch/arm/mach-exynos/sleep.S   | 3 ++-
 arch/arm/mach-highbank/Makefile| 3 ---
 arch/arm/mach-highbank/smc.S   | 3 ++-
 arch/arm/mach-keystone/Makefile| 3 ---
 arch/arm/mach-keystone/smc.S   | 1 +
 arch/arm/mach-omap2/Makefile   | 8 
 arch/arm/mach-omap2/omap-headsmp.S | 2 ++
 arch/arm/mach-omap2/omap-smc.S | 3 ++-
 arch/arm/mach-omap2/sleep33xx.S| 1 +
 arch/arm/mach-omap2/sleep34xx.S| 2 ++
 arch/arm/mach-omap2/sleep43xx.S| 2 ++
 arch/arm/mach-omap2/sleep44xx.S| 2 ++
 arch/arm/mach-tango/Makefile   | 3 ---
 arch/arm/mach-tango/smc.S  | 1 +
 18 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
index 8fd23b263c60..b59c813b1af4 100644
--- a/arch/arm/mach-bcm/Makefile
+++ b/arch/arm/mach-bcm/Makefile
@@ -40,9 +40,6 @@ obj-$(CONFIG_ARCH_BCM_MOBILE_L2_CACHE) += kona_l2_cache.o
 
 # Support for secure monitor traps
 obj-$(CONFIG_ARCH_BCM_MOBILE_SMC) += bcm_kona_smc.o
-ifeq ($(call as-instr,.arch_extension sec,as_has_sec),as_has_sec)
-CFLAGS_bcm_kona_smc.o  += -Wa,-march=armv7-a+sec -DREQUIRES_SEC
-endif
 
 # BCM2835
 obj-$(CONFIG_ARCH_BCM2835) += board_bcm2835.o
diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c
index a55a7ecf146a..541e850a736c 100644
--- a/arch/arm/mach-bcm/bcm_kona_smc.c
+++ b/arch/arm/mach-bcm/bcm_kona_smc.c
@@ -125,9 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
__asmeq("%2", "r4")
__asmeq("%3", "r5")
__asmeq("%4", "r6")
-#ifdef REQUIRES_SEC
".arch_extension sec\n"
-#endif
"   smc#0\n"
: "=r" (ip), "=r" (r0)
: "r" (r4), "r" (r5), "r" (r6)
diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index cd00c82a1add..44de9f36fd1b 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -14,9 +14,5 @@ obj-$(CONFIG_PM_SLEEP)+= suspend.o
 
 obj-$(CONFIG_SMP)  += platsmp.o headsmp.o
 
-plus_sec := $(call as-instr,.arch_extension sec,+sec)
-AFLAGS_exynos-smc.o:=-Wa,-march=armv7-a$(plus_sec)
-AFLAGS_sleep.o :=-Wa,-march=armv7-a$(plus_sec)
-
 obj-$(CONFIG_EXYNOS5420_MCPM)  += mcpm-exynos.o
 CFLAGS_mcpm-exynos.o   += -march=armv7-a
diff --git a/arch/arm/mach-exynos/exynos-smc.S 
b/arch/arm/mach-exynos/exynos-smc.S
index d259532ba937..6da31e6a7acb 100644
--- a/arch/arm/mach-exynos/exynos-smc.S
+++ b/arch/arm/mach-exynos/exynos-smc.S
@@ -10,7 +10,8 @@
 /*
  * Function signature: void exynos_smc(u32 cmd, u32 arg1, u32 arg2, u32 arg3)
  */
-
+   .arch armv7-a
+   .arch_extension sec
 ENTRY(exynos_smc)
stmfd   sp!, {r4-r11, lr}
dsb
diff --git a/arch/arm/mach-exynos/sleep.S b/arch/arm/mach-exynos/sleep.S
index 2783c3a0c06a..ed93f91853b8 100644
--- a/arch/arm/mach-exynos/sleep.S
+++ b/arch/arm/mach-exynos/sleep.S
@@ -44,7 +44,8 @@ ENTRY(exynos_cpu_resume)
 ENDPROC(exynos_cpu_resume)
 
.align
-
+   .arch armv7-a
+   .arch_extension sec
 ENTRY(exynos_cpu_resume_ns)
mrc p15, 0, r0, c0, c0, 0
ldr r1, =CPU_MASK
diff --git a/arch/arm/mach-highbank/Makefile b/arch/arm/mach-highbank/Makefile
index 55840f414d3e..e7741b883d13

[PATCH v2 2/3] ARM: mvebu: drop unnecessary label

2019-04-08 Thread Stefan Agner
The label mvebu_boot_wa_start is not necessary and causes a build
issue when building with LLVM's integrated assembler:
AS  arch/arm/mach-mvebu/pmsu_ll.o
  arch/arm/mach-mvebu/pmsu_ll.S:59:1: error: invalid symbol redefinition
  mvebu_boot_wa_start:
  ^

Drop the label.

Signed-off-by: Stefan Agner 
Acked-by: Nicolas Pitre 
---
 arch/arm/mach-mvebu/pmsu_ll.S | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/pmsu_ll.S b/arch/arm/mach-mvebu/pmsu_ll.S
index 88651221dbdd..c1fb713e9306 100644
--- a/arch/arm/mach-mvebu/pmsu_ll.S
+++ b/arch/arm/mach-mvebu/pmsu_ll.S
@@ -56,7 +56,6 @@ ENDPROC(armada_38x_cpu_resume)
 
 /* The following code will be executed from SRAM */
 ENTRY(mvebu_boot_wa_start)
-mvebu_boot_wa_start:
 ARM_BE8(setend be)
adr r0, 1f
ldr r0, [r0]@ load the address of the
-- 
2.21.0



[PATCH v2 3/3] ARM: mvebu: prefix coprocessor operand with p

2019-04-08 Thread Stefan Agner
In every other instance where mrc is used the coprocessor operand
is prefix with p (e.g. p15). Use the p prefix in this case too.
This fixes a build issue when using LLVM's integrated assembler:
  arch/arm/mach-mvebu/coherency_ll.S:69:6: error: invalid operand for 
instruction
   mrc 15, 0, r3, cr0, cr0, 5
   ^
  arch/arm/mach-mvebu/pmsu_ll.S:19:6: error: invalid operand for instruction
   mrc 15, 0, r0, cr0, cr0, 5 @ get the CPU ID
   ^

Signed-off-by: Stefan Agner 
Acked-by: Nicolas Pitre 
---
 arch/arm/mach-mvebu/coherency_ll.S | 2 +-
 arch/arm/mach-mvebu/pmsu_ll.S  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-mvebu/coherency_ll.S 
b/arch/arm/mach-mvebu/coherency_ll.S
index 8b2fbc8b6bc6..2d962fe48821 100644
--- a/arch/arm/mach-mvebu/coherency_ll.S
+++ b/arch/arm/mach-mvebu/coherency_ll.S
@@ -66,7 +66,7 @@ ENDPROC(ll_get_coherency_base)
  * fabric registers
  */
 ENTRY(ll_get_coherency_cpumask)
-   mrc 15, 0, r3, cr0, cr0, 5
+   mrc p15, 0, r3, cr0, cr0, 5
and r3, r3, #15
mov r2, #(1 << 24)
lsl r3, r2, r3
diff --git a/arch/arm/mach-mvebu/pmsu_ll.S b/arch/arm/mach-mvebu/pmsu_ll.S
index c1fb713e9306..7aae9a25cfeb 100644
--- a/arch/arm/mach-mvebu/pmsu_ll.S
+++ b/arch/arm/mach-mvebu/pmsu_ll.S
@@ -16,7 +16,7 @@
 ENTRY(armada_38x_scu_power_up)
mrc p15, 4, r1, c15, c0 @ get SCU base address
orr r1, r1, #0x8@ SCU CPU Power Status Register
-   mrc 15, 0, r0, cr0, cr0, 5  @ get the CPU ID
+   mrc p15, 0, r0, cr0, cr0, 5 @ get the CPU ID
and r0, r0, #15
add r1, r1, r0
mov r0, #0x0
-- 
2.21.0



Re: [PATCH 1/3] ARM: use arch_extension directive instead of arch argument

2019-04-08 Thread Stefan Agner
On 31.03.2019 19:34, Arnd Bergmann wrote:
> On Sun, Mar 24, 2019 at 3:06 AM Arnd Bergmann  wrote:
>>
>> On Sat, Mar 23, 2019 at 4:52 PM Stefan Agner  wrote:
>> >
>> > The LLVM Target parser currently does not allow to specify the security
>> > extension as part of -march (see also LLVM Bug 40186 [0]). When trying
>> > to use Clang with LLVM's integrated assembler, this leads to a build
>> > errors such as this:
>> >   clang-8: error: the clang compiler does not support 
>> > '-Wa,-march=armv7-a+sec'
>> >
>> > Use ".arch_extension sec" to enable the security extension in a more
>> > portable fasion.
>> >
>> > Note that this is technically not exactly the same as the old code
>> > checked for availabilty of the security extension by calling as-instr.
>> > However, there are already other sites which use ".arch_extension sec"
>> > unconditionally, hence de-facto we need an assembler capable of
>> > ".arch_extension sec" already today (arch/arm/mm/proc-v7.S). The
>> > arch extension "sec" is available since binutils 2.21 according to
>> > its documentation [1].
>> >
>> > [0] https://bugs.llvm.org/show_bug.cgi?id=40186
>> > [1] https://sourceware.org/binutils/docs-2.21/as/ARM-Options.html
>> >
>> > Signed-off-by: Stefan Agner 
>>
>> This sounds like a good idea. I think we have platform specific
>> minimum toolchain versions elsewhere, but I don't see a problem
>> with raising the minimum version for all the armv7ve platforms.
>>
>> I've added this patch to my randconfig test queue, but please
>> send it to a...@kernel.org for inclusion when you have
>> collected more Acks.
>>
>> Do you have a git tree with other patches required for the
>> integrated assembler? I might try that out as well with
>> my randconfig tree. At the moment I'm building with
>> clang-8 and a small number of patches on top.
>>
>> Acked-by: Arnd Bergmann 
> 
> I only now looked at the results and found a problem:
> In a mixed v6/v7 configuration, the arch_extension flag
> is not sufficient, and for armv6+sec, we get failures like
> 
> /git/arm-soc/arch/arm/mach-omap2/sleep44xx.S: Assembler messages:
> /git/arm-soc/arch/arm/mach-omap2/sleep44xx.S:343: Error: selected
> processor does not support `isb' in ARM mode
> /git/arm-soc/arch/arm/mach-omap2/sleep44xx.S:350: Error: selected
> processor does not support `dsb' in ARM mode
> /git/arm-soc/arch/arm/mach-omap2/sleep44xx.S:351: Error: selected
> processor does not support `dmb' in ARM mode
> clang: error: assembler command failed with exit code 1 (use -v to see
> invocation)
> /git/arm-soc/scripts/Makefile.build:369: recipe for target
> 'arch/arm/mach-omap2/sleep44xx.o' failed
> make[3]: *** [arch/arm/mach-omap2/sleep44xx.o] Error 1
> 
> ==> build/arm/0x64728DCE_defconfig/log <==
> /git/arm-soc/arch/arm/mach-omap2/omap-smc.S: Assembler messages:
> /git/arm-soc/arch/arm/mach-omap2/omap-smc.S:31: Error: selected
> processor does not support `dsb' in ARM mode
> /git/arm-soc/arch/arm/mach-omap2/omap-smc.S:53: Error: selected
> processor does not support `dsb' in ARM mode
> /git/arm-soc/arch/arm/mach-omap2/omap-smc.S:54: Error: selected
> processor does not support `dmb' in ARM mode


Hm, I guess I can just use .arch  armv7-a in those cases, as we use in
other places.

Thanks for testing! Will send a v2.

--
Stefan


Re: [PATCH 1/3] ARM: use arch_extension directive instead of arch argument

2019-03-24 Thread Stefan Agner
On 23.03.2019 21:06, Arnd Bergmann wrote:
> On Sat, Mar 23, 2019 at 4:52 PM Stefan Agner  wrote:
>>
>> The LLVM Target parser currently does not allow to specify the security
>> extension as part of -march (see also LLVM Bug 40186 [0]). When trying
>> to use Clang with LLVM's integrated assembler, this leads to a build
>> errors such as this:
>>   clang-8: error: the clang compiler does not support 
>> '-Wa,-march=armv7-a+sec'
>>
>> Use ".arch_extension sec" to enable the security extension in a more
>> portable fasion.
>>
>> Note that this is technically not exactly the same as the old code
>> checked for availabilty of the security extension by calling as-instr.
>> However, there are already other sites which use ".arch_extension sec"
>> unconditionally, hence de-facto we need an assembler capable of
>> ".arch_extension sec" already today (arch/arm/mm/proc-v7.S). The
>> arch extension "sec" is available since binutils 2.21 according to
>> its documentation [1].
>>
>> [0] https://bugs.llvm.org/show_bug.cgi?id=40186
>> [1] https://sourceware.org/binutils/docs-2.21/as/ARM-Options.html
>>
>> Signed-off-by: Stefan Agner 
> 
> This sounds like a good idea. I think we have platform specific
> minimum toolchain versions elsewhere, but I don't see a problem
> with raising the minimum version for all the armv7ve platforms.
> 
> I've added this patch to my randconfig test queue, but please
> send it to a...@kernel.org for inclusion when you have
> collected more Acks.

Cool, will do!

> 
> Do you have a git tree with other patches required for the
> integrated assembler? I might try that out as well with
> my randconfig tree. At the moment I'm building with
> clang-8 and a small number of patches on top.

I do have some more work in progress patches. I made some rough commits
and pushed the tree here:
https://github.com/ClangBuiltLinux/linux/commits/arm-fixes-hacks-to-make-llvm-integrated-as-work

This tree compiles for me and a test boot with qemu seems to work.

It seems that LLVM's integrated assembler is capable of assembling
almost the whole kernel a lot can be worked around/fixed on kernel side.
There are only about a handful of files where I still use the GNU
assembler. Haven't looked closely at these cases yet.

There is one issue which probably need a change in LLVM:
https://github.com/ClangBuiltLinux/linux/issues/306

I proposed this fix:
https://reviews.llvm.org/D59733


> 
> Acked-by: Arnd Bergmann 

Thx.

--
Stefan


[PATCH 3/3] ARM: mvebu: prefix coprocessor operand with p

2019-03-23 Thread Stefan Agner
In every other instance where mrc is used the coprocessor operand
is prefix with p (e.g. p15). Use the p prefix in this case too.
This fixes a build issue when using LLVM's integrated assembler:
  arch/arm/mach-mvebu/coherency_ll.S:69:6: error: invalid operand for 
instruction
   mrc 15, 0, r3, cr0, cr0, 5
   ^
  arch/arm/mach-mvebu/pmsu_ll.S:19:6: error: invalid operand for instruction
   mrc 15, 0, r0, cr0, cr0, 5 @ get the CPU ID
   ^

Signed-off-by: Stefan Agner 
---
 arch/arm/mach-mvebu/coherency_ll.S | 2 +-
 arch/arm/mach-mvebu/pmsu_ll.S  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-mvebu/coherency_ll.S 
b/arch/arm/mach-mvebu/coherency_ll.S
index 8b2fbc8b6bc6..2d962fe48821 100644
--- a/arch/arm/mach-mvebu/coherency_ll.S
+++ b/arch/arm/mach-mvebu/coherency_ll.S
@@ -66,7 +66,7 @@ ENDPROC(ll_get_coherency_base)
  * fabric registers
  */
 ENTRY(ll_get_coherency_cpumask)
-   mrc 15, 0, r3, cr0, cr0, 5
+   mrc p15, 0, r3, cr0, cr0, 5
and r3, r3, #15
mov r2, #(1 << 24)
lsl r3, r2, r3
diff --git a/arch/arm/mach-mvebu/pmsu_ll.S b/arch/arm/mach-mvebu/pmsu_ll.S
index c1fb713e9306..7aae9a25cfeb 100644
--- a/arch/arm/mach-mvebu/pmsu_ll.S
+++ b/arch/arm/mach-mvebu/pmsu_ll.S
@@ -16,7 +16,7 @@
 ENTRY(armada_38x_scu_power_up)
mrc p15, 4, r1, c15, c0 @ get SCU base address
orr r1, r1, #0x8@ SCU CPU Power Status Register
-   mrc 15, 0, r0, cr0, cr0, 5  @ get the CPU ID
+   mrc p15, 0, r0, cr0, cr0, 5 @ get the CPU ID
and r0, r0, #15
add r1, r1, r0
mov r0, #0x0
-- 
2.21.0



[PATCH 2/3] ARM: mvebu: drop unnecessary label

2019-03-23 Thread Stefan Agner
The label mvebu_boot_wa_start is not necessary and causes a build
issue when building with LLVM's integrated assembler:
AS  arch/arm/mach-mvebu/pmsu_ll.o
  arch/arm/mach-mvebu/pmsu_ll.S:59:1: error: invalid symbol redefinition
  mvebu_boot_wa_start:
  ^

Drop the label.

Signed-off-by: Stefan Agner 
---
 arch/arm/mach-mvebu/pmsu_ll.S | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/pmsu_ll.S b/arch/arm/mach-mvebu/pmsu_ll.S
index 88651221dbdd..c1fb713e9306 100644
--- a/arch/arm/mach-mvebu/pmsu_ll.S
+++ b/arch/arm/mach-mvebu/pmsu_ll.S
@@ -56,7 +56,6 @@ ENDPROC(armada_38x_cpu_resume)
 
 /* The following code will be executed from SRAM */
 ENTRY(mvebu_boot_wa_start)
-mvebu_boot_wa_start:
 ARM_BE8(setend be)
adr r0, 1f
ldr r0, [r0]@ load the address of the
-- 
2.21.0



[PATCH 1/3] ARM: use arch_extension directive instead of arch argument

2019-03-23 Thread Stefan Agner
The LLVM Target parser currently does not allow to specify the security
extension as part of -march (see also LLVM Bug 40186 [0]). When trying
to use Clang with LLVM's integrated assembler, this leads to a build
errors such as this:
  clang-8: error: the clang compiler does not support '-Wa,-march=armv7-a+sec'

Use ".arch_extension sec" to enable the security extension in a more
portable fasion.

Note that this is technically not exactly the same as the old code
checked for availabilty of the security extension by calling as-instr.
However, there are already other sites which use ".arch_extension sec"
unconditionally, hence de-facto we need an assembler capable of
".arch_extension sec" already today (arch/arm/mm/proc-v7.S). The
arch extension "sec" is available since binutils 2.21 according to
its documentation [1].

[0] https://bugs.llvm.org/show_bug.cgi?id=40186
[1] https://sourceware.org/binutils/docs-2.21/as/ARM-Options.html

Signed-off-by: Stefan Agner 
---
 arch/arm/mach-bcm/Makefile | 3 ---
 arch/arm/mach-bcm/bcm_kona_smc.c   | 2 --
 arch/arm/mach-exynos/Makefile  | 4 
 arch/arm/mach-exynos/exynos-smc.S  | 2 +-
 arch/arm/mach-exynos/sleep.S   | 2 +-
 arch/arm/mach-highbank/Makefile| 3 ---
 arch/arm/mach-highbank/smc.S   | 2 +-
 arch/arm/mach-keystone/Makefile| 3 ---
 arch/arm/mach-keystone/smc.S   | 1 +
 arch/arm/mach-omap2/Makefile   | 8 
 arch/arm/mach-omap2/omap-headsmp.S | 1 +
 arch/arm/mach-omap2/omap-smc.S | 2 +-
 arch/arm/mach-omap2/sleep34xx.S| 1 +
 arch/arm/mach-omap2/sleep43xx.S| 1 +
 arch/arm/mach-omap2/sleep44xx.S| 1 +
 arch/arm/mach-tango/Makefile   | 3 ---
 arch/arm/mach-tango/smc.S  | 1 +
 17 files changed, 10 insertions(+), 30 deletions(-)

diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
index 8fd23b263c60..b59c813b1af4 100644
--- a/arch/arm/mach-bcm/Makefile
+++ b/arch/arm/mach-bcm/Makefile
@@ -40,9 +40,6 @@ obj-$(CONFIG_ARCH_BCM_MOBILE_L2_CACHE) += kona_l2_cache.o
 
 # Support for secure monitor traps
 obj-$(CONFIG_ARCH_BCM_MOBILE_SMC) += bcm_kona_smc.o
-ifeq ($(call as-instr,.arch_extension sec,as_has_sec),as_has_sec)
-CFLAGS_bcm_kona_smc.o  += -Wa,-march=armv7-a+sec -DREQUIRES_SEC
-endif
 
 # BCM2835
 obj-$(CONFIG_ARCH_BCM2835) += board_bcm2835.o
diff --git a/arch/arm/mach-bcm/bcm_kona_smc.c b/arch/arm/mach-bcm/bcm_kona_smc.c
index a55a7ecf146a..541e850a736c 100644
--- a/arch/arm/mach-bcm/bcm_kona_smc.c
+++ b/arch/arm/mach-bcm/bcm_kona_smc.c
@@ -125,9 +125,7 @@ static int bcm_kona_do_smc(u32 service_id, u32 buffer_phys)
__asmeq("%2", "r4")
__asmeq("%3", "r5")
__asmeq("%4", "r6")
-#ifdef REQUIRES_SEC
".arch_extension sec\n"
-#endif
"   smc#0\n"
: "=r" (ip), "=r" (r0)
: "r" (r4), "r" (r5), "r" (r6)
diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index cd00c82a1add..44de9f36fd1b 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -14,9 +14,5 @@ obj-$(CONFIG_PM_SLEEP)+= suspend.o
 
 obj-$(CONFIG_SMP)  += platsmp.o headsmp.o
 
-plus_sec := $(call as-instr,.arch_extension sec,+sec)
-AFLAGS_exynos-smc.o:=-Wa,-march=armv7-a$(plus_sec)
-AFLAGS_sleep.o :=-Wa,-march=armv7-a$(plus_sec)
-
 obj-$(CONFIG_EXYNOS5420_MCPM)  += mcpm-exynos.o
 CFLAGS_mcpm-exynos.o   += -march=armv7-a
diff --git a/arch/arm/mach-exynos/exynos-smc.S 
b/arch/arm/mach-exynos/exynos-smc.S
index d259532ba937..392f8ba351f4 100644
--- a/arch/arm/mach-exynos/exynos-smc.S
+++ b/arch/arm/mach-exynos/exynos-smc.S
@@ -10,7 +10,7 @@
 /*
  * Function signature: void exynos_smc(u32 cmd, u32 arg1, u32 arg2, u32 arg3)
  */
-
+   .arch_extension sec
 ENTRY(exynos_smc)
stmfd   sp!, {r4-r11, lr}
dsb
diff --git a/arch/arm/mach-exynos/sleep.S b/arch/arm/mach-exynos/sleep.S
index 2783c3a0c06a..6e3207d486b4 100644
--- a/arch/arm/mach-exynos/sleep.S
+++ b/arch/arm/mach-exynos/sleep.S
@@ -44,7 +44,7 @@ ENTRY(exynos_cpu_resume)
 ENDPROC(exynos_cpu_resume)
 
.align
-
+   .arch_extension sec
 ENTRY(exynos_cpu_resume_ns)
mrc p15, 0, r0, c0, c0, 0
ldr r1, =CPU_MASK
diff --git a/arch/arm/mach-highbank/Makefile b/arch/arm/mach-highbank/Makefile
index 55840f414d3e..e7741b883d13 100644
--- a/arch/arm/mach-highbank/Makefile
+++ b/arch/arm/mach-highbank/Makefile
@@ -1,6 +1,3 @@
 obj-y  := highbank.o system.o smc.o
 
-plus_sec := $(call as-instr,.arch_extension sec,+sec)
-AFLAGS_smc.o   :=-Wa,-march=armv7-a$(plus_sec)
-
 obj-$(CONFIG_PM_SLEEP) += pm.o
diff --git a/arch/arm/mach-highbank/smc.S b/arch/arm/mach

Re: [PATCH] regulator: rn5t618: Constify regulator_desc

2019-03-23 Thread Stefan Agner
On 20.03.2019 13:10, Axel Lin wrote:
> The regulator_desc never need to be modified, so define them as const as a
> hint to the compiler that they can go into .rodata.
> 
> Signed-off-by: Axel Lin 

Looks good to me.

Reviewed-by: Stefan Agner 

--
Stefan

> ---
>  drivers/regulator/rn5t618-regulator.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/regulator/rn5t618-regulator.c
> b/drivers/regulator/rn5t618-regulator.c
> index 790a4a73ea2c..a79c0c43b9f8 100644
> --- a/drivers/regulator/rn5t618-regulator.c
> +++ b/drivers/regulator/rn5t618-regulator.c
> @@ -46,7 +46,7 @@ static const struct regulator_ops rn5t618_reg_ops = {
>   .vsel_mask  = (vmask),  \
>   }
>  
> -static struct regulator_desc rn5t567_regulators[] = {
> +static const struct regulator_desc rn5t567_regulators[] = {
>   /* DCDC */
>   REG(DCDC1, DC1CTL, BIT(0), DC1DAC, 0xff, 60, 350, 12500),
>   REG(DCDC2, DC2CTL, BIT(0), DC2DAC, 0xff, 60, 350, 12500),
> @@ -63,7 +63,7 @@ static struct regulator_desc rn5t567_regulators[] = {
>   REG(LDORTC2, LDOEN2, BIT(5), LDORTC2DAC, 0x7f, 90, 350, 25000),
>  };
>  
> -static struct regulator_desc rn5t618_regulators[] = {
> +static const struct regulator_desc rn5t618_regulators[] = {
>   /* DCDC */
>   REG(DCDC1, DC1CTL, BIT(0), DC1DAC, 0xff, 60, 350, 12500),
>   REG(DCDC2, DC2CTL, BIT(0), DC2DAC, 0xff, 60, 350, 12500),
> @@ -79,7 +79,7 @@ static struct regulator_desc rn5t618_regulators[] = {
>   REG(LDORTC2, LDOEN2, BIT(5), LDORTC2DAC, 0x7f, 90, 350, 25000),
>  };
>  
> -static struct regulator_desc rc5t619_regulators[] = {
> +static const struct regulator_desc rc5t619_regulators[] = {
>   /* DCDC */
>   REG(DCDC1, DC1CTL, BIT(0), DC1DAC, 0xff, 60, 350, 12500),
>   REG(DCDC2, DC2CTL, BIT(0), DC2DAC, 0xff, 60, 350, 12500),
> @@ -107,7 +107,7 @@ static int rn5t618_regulator_probe(struct
> platform_device *pdev)
>   struct rn5t618 *rn5t618 = dev_get_drvdata(pdev->dev.parent);
>   struct regulator_config config = { };
>   struct regulator_dev *rdev;
> - struct regulator_desc *regulators;
> + const struct regulator_desc *regulators;
>   int i;
>   int num_regulators = 0;


Re: [PATCH v2 1/2] ARM: drop WASM to work around LLVM issue

2019-03-19 Thread Stefan Agner
On 18.03.2019 19:09, Nick Desaulniers wrote:
> On Sun, Mar 17, 2019 at 4:05 PM Stefan Agner  wrote:
>>
>> Currently LLVM's integrated assembler does not recognize .w form
>> of the pld instructions (LLVM Bug 40972 [0]):
>>
>>   ./arch/arm/include/asm/processor.h:133:5: error: invalid instruction
>>   "pldw.w\t%a0 \n"
>>^
>>   :2:1: note: instantiated into assembly here
>>   pldw.w  [r0]
>>   ^
>>   1 error generated.
>>
>> The W macro for generating wide instructions when targeting Thumb-2
>> is not strictly required for the preload data instructions (pld, pldw)
>> since they are only available as wide instructions. The GNU assembler
>> works with or without the .w appended when compiling an Thumb-2 kernel.
>>
>> Drop the macro to work around LLVM Bug 40972 issue.
>>
>> [0] https://bugs.llvm.org/show_bug.cgi?id=40972
>>
>> Signed-off-by: Stefan Agner 
> 
> Thanks for the bug report and patch.
> Reviewed-by: Nick Desaulniers 
> 
> Just curious, there are only 3 other expansion sites of this macro.
> Are any of those problematic?  Looks like nop.w, sev.w and b.w?
> 

All three sites are in inline assembly, and I did a bunch of successful
Thumb2 builds using the integrated assembler, so I think all those sites
have been assembled by LLVM successfully.

Also confirmed those three instructions with the reproducer example from
the LLVM bug above.

--
Stefan

>> ---
>> Changes in v2:
>> - Reword commit message to reflect the fact that this is a work around
>>   for LLVM.
>>
>>  arch/arm/include/asm/processor.h | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/processor.h 
>> b/arch/arm/include/asm/processor.h
>> index 57fe73ea0f72..5d06f75ffad4 100644
>> --- a/arch/arm/include/asm/processor.h
>> +++ b/arch/arm/include/asm/processor.h
>> @@ -135,8 +135,8 @@ static inline void prefetchw(const void *ptr)
>> __asm__ __volatile__(
>> ".arch_extensionmp\n"
>> __ALT_SMP_ASM(
>> -   WASM(pldw)  "\t%a0",
>> -   WASM(pld)   "\t%a0"
>> +   "pldw\t%a0",
>> +   "pld\t%a0"
>> )
>> :: "p" (ptr));
>>  }
>> --
>> 2.21.0
>>


Re: [PATCH V5 1/5] dt-bindings: pwm: Add i.MX TPM PWM binding

2019-03-18 Thread Stefan Agner
On 18.03.2019 08:41, Anson Huang wrote:
> Add i.MX TPM(Low Power Timer/Pulse Width Modulation Module) PWM binding.
> 
> Signed-off-by: Anson Huang 
> ---
> No changes.
> ---
>  Documentation/devicetree/bindings/pwm/imx-tpm-pwm.txt | 19 
> +++
>  1 file changed, 19 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/imx-tpm-pwm.txt
> 
> diff --git a/Documentation/devicetree/bindings/pwm/imx-tpm-pwm.txt
> b/Documentation/devicetree/bindings/pwm/imx-tpm-pwm.txt
> new file mode 100644
> index 000..d47b8fb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/imx-tpm-pwm.txt
> @@ -0,0 +1,19 @@
> +Freescale i.MX TPM PWM controller
> +
> +Required properties:
> +- compatible : Should be "fsl,imx-tpm-pwm".
> +- reg: Physical base address and length of the controller's registers.
> +- #pwm-cells: Should be 2. See pwm.txt in this directory for a
> description of the cells format.

It seems that the driver supports polarity. Can we use 3 cells so we can
specify the polarity in the device tree?

Also use 3 cells in the base device tree to enable other boards making
use of the polarity (arch/arm/boot/dts/imx7ulp.dtsi).

--
Stefan

> +- clocks : The clock provided by the SoC to drive the PWM.
> +- interrupts: The interrupt for the pwm controller.
> +
> +Example:
> +
> +pwm0: tpm@4025 {
> + compatible = "fsl,imx-tpm-pwm";
> + reg = <0x4025 0x1000>;
> + assigned-clocks = < IMX7ULP_CLK_LPTPM4>;
> + assigned-clock-parents = < IMX7ULP_CLK_SOSC_BUS_CLK>;
> + clocks = < IMX7ULP_CLK_LPTPM4>;
> + #pwm-cells = <2>;
> +};


[PATCH v2 2/2] ARM: drop -mauto-it

2019-03-17 Thread Stefan Agner
The assembler option -mauto-it is no longer a valid option. The last
remaining references have been removed from the documentation in
July 2009 [0].

The currently supported binutils version is 2.20 (released in
September 2009) or higher where gas supports -mimplicit-it=always.
Drop the fallback to -mauto-it and use -mimplicit-it=always only.

[0] 
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=529707530657a333a304c651c808ea630c955223

Signed-off-by: Stefan Agner 
Reviewed-by: Nick Desaulniers 
---
Changes in v2:
- Drop $(comma) since we are no longer use the call to as-option.

 arch/arm/Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index df3ad82d312c..eb9d38e77cf0 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -113,8 +113,7 @@ CFLAGS_ABI  +=-funwind-tables
 endif
 
 ifeq ($(CONFIG_THUMB2_KERNEL),y)
-AFLAGS_AUTOIT  :=$(call 
as-option,-Wa$(comma)-mimplicit-it=always,-Wa$(comma)-mauto-it)
-CFLAGS_ISA :=-mthumb $(AFLAGS_AUTOIT)
+CFLAGS_ISA :=-mthumb -Wa,-mimplicit-it=always
 AFLAGS_ISA :=$(CFLAGS_ISA) -Wa$(comma)-mthumb
 # Work around buggy relocation from gas if requested:
 ifeq ($(CONFIG_THUMB2_AVOID_R_ARM_THM_JUMP11),y)
-- 
2.21.0



[PATCH v2 1/2] ARM: drop WASM to work around LLVM issue

2019-03-17 Thread Stefan Agner
Currently LLVM's integrated assembler does not recognize .w form
of the pld instructions (LLVM Bug 40972 [0]):

  ./arch/arm/include/asm/processor.h:133:5: error: invalid instruction
  "pldw.w\t%a0 \n"
   ^
  :2:1: note: instantiated into assembly here
  pldw.w  [r0]
  ^
  1 error generated.

The W macro for generating wide instructions when targeting Thumb-2
is not strictly required for the preload data instructions (pld, pldw)
since they are only available as wide instructions. The GNU assembler
works with or without the .w appended when compiling an Thumb-2 kernel.

Drop the macro to work around LLVM Bug 40972 issue.

[0] https://bugs.llvm.org/show_bug.cgi?id=40972

Signed-off-by: Stefan Agner 
---
Changes in v2:
- Reword commit message to reflect the fact that this is a work around
  for LLVM.

 arch/arm/include/asm/processor.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/processor.h b/arch/arm/include/asm/processor.h
index 57fe73ea0f72..5d06f75ffad4 100644
--- a/arch/arm/include/asm/processor.h
+++ b/arch/arm/include/asm/processor.h
@@ -135,8 +135,8 @@ static inline void prefetchw(const void *ptr)
__asm__ __volatile__(
".arch_extensionmp\n"
__ALT_SMP_ASM(
-   WASM(pldw)  "\t%a0",
-   WASM(pld)   "\t%a0"
+   "pldw\t%a0",
+   "pld\t%a0"
)
:: "p" (ptr));
 }
-- 
2.21.0



Re: [PATCH 1/2] ARM: drop unnecessary WASM

2019-03-17 Thread Stefan Agner
On 06.03.2019 01:47, Nick Desaulniers wrote:
> On Tue, Mar 5, 2019 at 3:39 PM Robin Murphy  wrote:
>>
>> Hi Stefan,
>>
>> On 2019-03-05 10:18 pm, Stefan Agner wrote:
>> > The W macro for generating wide instructions when targeting Thumb-2
>> > is not required for the preload data instructions (pld, pldw) since
>> > they are only available as wide instructions. The GNU assembler seems
>> > to work with or without the .w appended when compiling an Thumb-2
>> > kernel. However, Clang's integrated assembler does not consider the
>> > .w variants as valid instructions:
>> >
>> >./arch/arm/include/asm/processor.h:133:5: error: invalid instruction
>> >"pldw.w\t%a0 \n"
>> > ^
>> >:2:1: note: instantiated into assembly here
>> >pldw.w  [r0]
>> >^
>> >1 error generated.
>>
>> Have you filed a bug against Clang for that? Something like "pldwal.w"
> 
> Yes; please.  For each deficiency you find, please file a bug.  We're
> working on identifying what's missing from Clang's integrated
> assembler support.  Given the list of issues, it's easier to estimate
> how much effort is needed, which helps us allocate resources towards
> fixing those issues better.
> 
> It would be good to know if pldw.w is valid under UAL or not.

As far as I understand the Arm ARM it is valid under UAL.

Reported a bug in LLVM's bug tracker:
https://bugs.llvm.org/show_bug.cgi?id=40972

Will send a v2 patch mentioning this is really a work around for LLVM
and link to the bug report.

> 
> Hopefully, the ARM kernel team can stress the importance of assembler
> support for their ISA to their LLVM team.

--
Stefan


Re: [PATCH v2 0/4] Patches to allow consistent mmc / mmcblk numbering w/ device tree

2019-03-17 Thread Stefan Agner
On 16.03.2019 16:39, Russell King - ARM Linux admin wrote:
> On Sat, Mar 16, 2019 at 01:33:58PM +0100, Marek Vasut wrote:
>> If you have a FS or partition table there, it does.
>> If you don't, I agree ... that's a problem.
> 
> eMMC boot partitions are called mmcblkXbootY, and unless you have more
> than one eMMC device on the system, they can be found either by looking
> for /dev/mmcblk*boot* or by querying udev.  The advantage of using udev
> is you can discover the physical device behind it by looking at DEVPATH,
> ID_PATH, etc, but you may not have that installed on an embedded device.
> 
> However, as I say, just looking for /dev/mmcblk*boot* is sufficient to
> find the eMMC boot partitions where there is just one eMMC device
> present (which seems to be the standard setup.)
> 
>> > I don't care the slightest what the numbering is, as long as it is
>> > stable.  On some hardware, with an unpatched kernel, the mmc device
>> > numbering changes depending on whether or not an SD card is inserted on
>> > boot.  Getting rid of that behaviour is really all I want.
>>
>> Agreed, that would be an improvement.
> 
> The mmc device numbering was tied to the mmc host numbering a while back
> and the order that the hosts are probed should be completely independent
> of whether a card is inserted or not:
> 
> snprintf(md->disk->disk_name, sizeof(md->disk->disk_name),
>  "mmcblk%u%s", card->host->index, subname ? subname : "");
> 
> snprintf(rpmb_name, sizeof(rpmb_name),
>  "mmcblk%u%s", card->host->index, subname ? subname : "");
> 
> I suspect that Mans is quoting something from the dim and distant past
> to confuse the issue - as shown above, it is now dependent on the host
> numbering order not the order in which cards are inserted.

Commit 9aaf3437aa72 ("mmc: block: Use the mmc host device index as the
mmcblk device index") which came in with v4.6 enables constant mmc block
device numbering. I can confirm that it works nicely, and it improved
the situation a lot.

That being said, we still use a patch downstream which allows
renumbering using an alias. We deal with a bunch of different boards
with different SoC's. I have a couple of SD cards with various rootfs
and use internal eMMC boot quite often as well. Remembering which board
uses which numbering is a pain. Maintaining a patch is just easier...
Furthermore, U-Boot allows reordering and all boards I deal with use mmc
0 for the internal eMMC. The aliases allow consistency.

--
Stefan


Re: [PATCH 1/2] ARM: drop unnecessary WASM

2019-03-05 Thread Stefan Agner
On 06.03.2019 00:39, Robin Murphy wrote:
> Hi Stefan,
> 
> On 2019-03-05 10:18 pm, Stefan Agner wrote:
>> The W macro for generating wide instructions when targeting Thumb-2
>> is not required for the preload data instructions (pld, pldw) since
>> they are only available as wide instructions. The GNU assembler seems
>> to work with or without the .w appended when compiling an Thumb-2
>> kernel. However, Clang's integrated assembler does not consider the
>> .w variants as valid instructions:
>>
>>./arch/arm/include/asm/processor.h:133:5: error: invalid instruction
>>"pldw.w\t%a0 \n"
>> ^
>>:2:1: note: instantiated into assembly here
>>pldw.w  [r0]
>>^
>>1 error generated.
> 
> Have you filed a bug against Clang for that? Something like "pldwal.w"
> may be super-redundant, but it's still perfectly valid syntax. The
> "Standard assembler syntax fields" section of the Arm ARM even calls
> out that "...the .W qualifier has no effect" on ARM/A32 instructions
> since they are inherently 32-bit; that should equally apply for 32-bit
> only Thumb/T32 instructions. There are certainly a few instructions
> which don't allow a condition code (even "AL"), but off-hand I don't
> remember any not having the optional {} field in their syntax.

Good point, bug filed:
https://bugs.llvm.org/show_bug.cgi?id=40972

Will send a v2 and note that this is a work around for Clang and
reference the bug.

--
Stefan

> 
> That said, obviously the patch itself is no problem since the syntax
> *is* redundant here, but it really is just a workaround for an
> assembler bug.
> 
> Robin.
> 
>> Drop the macro to make sure non-wide variants of pld and pldw are
>> emitted in all cases.
>>
>> Signed-off-by: Stefan Agner 
>> ---
>>   arch/arm/include/asm/processor.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/processor.h 
>> b/arch/arm/include/asm/processor.h
>> index 120f4c9bbfde..042d77cf686d 100644
>> --- a/arch/arm/include/asm/processor.h
>> +++ b/arch/arm/include/asm/processor.h
>> @@ -131,8 +131,8 @@ static inline void prefetchw(const void *ptr)
>>  __asm__ __volatile__(
>>  ".arch_extensionmp\n"
>>  __ALT_SMP_ASM(
>> -WASM(pldw)  "\t%a0",
>> -WASM(pld)   "\t%a0"
>> +"pldw\t%a0",
>> +"pld\t%a0"
>>  )
>>  :: "p" (ptr));
>>   }
>>


Re: [PATCH 2/2] ARM: drop -mauto-it

2019-03-05 Thread Stefan Agner
On 05.03.2019 23:21, Nick Desaulniers wrote:
> On Tue, Mar 5, 2019 at 2:17 PM Stefan Agner  wrote:
>>
>> The assembler option -mauto-it is no longer a valid option. It has
>> been removed from the documentation in July 2009, which is well
>> before the release date of the currently supported binutils version
>> 2.20.
> 
> Do you by chance have a link to the relevant commit?
> 

Documentation got removed in
https://github.com/bminor/binutils-gdb/commit/529707530657a333a304c651c808ea630c955223

I think -mauto-it never really made it upstream. Documentation has been
introduced here, but it seems that the option already has been renamed:
https://github.com/bminor/binutils-gdb/commit/e07e6e58be1c5273ed79a25c80ba831e71ac86b1

>>
>> Signed-off-by: Stefan Agner 
>> ---
>>  arch/arm/Makefile | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
>> index 9561325c5201..ebf67bebe73d 100644
>> --- a/arch/arm/Makefile
>> +++ b/arch/arm/Makefile
>> @@ -113,8 +113,7 @@ CFLAGS_ABI  +=-funwind-tables
>>  endif
>>
>>  ifeq ($(CONFIG_THUMB2_KERNEL),y)
>> -AFLAGS_AUTOIT  :=$(call 
>> as-option,-Wa$(comma)-mimplicit-it=always,-Wa$(comma)-mauto-it)
>> -CFLAGS_ISA :=-mthumb $(AFLAGS_AUTOIT)
>> +CFLAGS_ISA :=-mthumb -Wa$(comma)-mimplicit-it=always
> 
> Is $(comma) still needed? I thought it was only needed when a flag
> that would contain commas would be in another set of parens (thus
> making an ambiguity), like cc-ldoption or such?  Can you replace
> `$(comma)` here with `,`?  I suspect it should work.

Could catch, yes comma should work here.

--
Stefan


[PATCH 2/2] ARM: drop -mauto-it

2019-03-05 Thread Stefan Agner
The assembler option -mauto-it is no longer a valid option. It has
been removed from the documentation in July 2009, which is well
before the release date of the currently supported binutils version
2.20.

Signed-off-by: Stefan Agner 
---
 arch/arm/Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 9561325c5201..ebf67bebe73d 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -113,8 +113,7 @@ CFLAGS_ABI  +=-funwind-tables
 endif
 
 ifeq ($(CONFIG_THUMB2_KERNEL),y)
-AFLAGS_AUTOIT  :=$(call 
as-option,-Wa$(comma)-mimplicit-it=always,-Wa$(comma)-mauto-it)
-CFLAGS_ISA :=-mthumb $(AFLAGS_AUTOIT)
+CFLAGS_ISA :=-mthumb -Wa$(comma)-mimplicit-it=always
 AFLAGS_ISA :=$(CFLAGS_ISA) -Wa$(comma)-mthumb
 # Work around buggy relocation from gas if requested:
 ifeq ($(CONFIG_THUMB2_AVOID_R_ARM_THM_JUMP11),y)
-- 
2.20.1



[PATCH 1/2] ARM: drop unnecessary WASM

2019-03-05 Thread Stefan Agner
The W macro for generating wide instructions when targeting Thumb-2
is not required for the preload data instructions (pld, pldw) since
they are only available as wide instructions. The GNU assembler seems
to work with or without the .w appended when compiling an Thumb-2
kernel. However, Clang's integrated assembler does not consider the
.w variants as valid instructions:

  ./arch/arm/include/asm/processor.h:133:5: error: invalid instruction
  "pldw.w\t%a0 \n"
   ^
  :2:1: note: instantiated into assembly here
  pldw.w  [r0]
  ^
  1 error generated.

Drop the macro to make sure non-wide variants of pld and pldw are
emitted in all cases.

Signed-off-by: Stefan Agner 
---
 arch/arm/include/asm/processor.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/processor.h b/arch/arm/include/asm/processor.h
index 120f4c9bbfde..042d77cf686d 100644
--- a/arch/arm/include/asm/processor.h
+++ b/arch/arm/include/asm/processor.h
@@ -131,8 +131,8 @@ static inline void prefetchw(const void *ptr)
__asm__ __volatile__(
".arch_extensionmp\n"
__ALT_SMP_ASM(
-   WASM(pldw)  "\t%a0",
-   WASM(pld)   "\t%a0"
+   "pldw\t%a0",
+   "pld\t%a0"
)
:: "p" (ptr));
 }
-- 
2.20.1



Re: arch/arm/kernel/setup.c fails to compile for NOMMU

2019-03-05 Thread Stefan Agner
Hi Michal,

On 06.04.2018 11:56, Michal Hocko wrote:
> On Fri 25-08-17 08:45:40, Michal Hocko wrote:
>> On Thu 24-08-17 17:17:41, Russell King - ARM Linux wrote:
>> > On Fri, Aug 18, 2017 at 01:24:02PM +0200, Michal Hocko wrote:
>> > > Hi Russel,
>> > > I have a battery of configs for compile testing and for some time I've
>> > > been seeing the following compilation error with nommu config (attached)
>> > >
>> > > arch/arm/kernel/setup.c: In function 'reserve_crashkernel':
>> > > arch/arm/kernel/setup.c:1005:25: error: 'SECTION_SIZE' undeclared (first
>> > > use in this function)
>> > >  crash_size, SECTION_SIZE);
>> > >
>> > > I didn't get to look what is going on here, maybe my config is just too
>> > > artificial but the primary reason is that SECTION_SIZE is not defined in
>> > > pgtable-nommu.h. To be honest I am not familiar with nommu very much and
>> > > it smells like the whole reserve_crashkernel doesn't really make any
>> > > sense on those configs. Could you have a look what is the best fix
>> > > please?
>> >
>> > Hi,
>> >
>> > I suspect that mach-netx has never been tested in nommu configurations
>> > (ditto for many of the older platforms, which pre-date merging nommu
>> > support.)
>> >
>> > Maybe the best solution is to make these old platforms depend on MMU.
>> >
>> > However, I'm wondering whether kexec makes sense for !MMU - that's
>> > probably something that hasn't been tested and doesn't actually work.
>> > So maybe another approach would be to make kexec depend on MMU for
>> > ARM - but I'm afraid I don't really know.
>>
>> Yeah, I've disabled KEXEC in my testing config. All I do care about is
>> to test nommu specific code paths in MM code.
>>
>> > I only have very limited nommu experience.
>>
>> me too
>>
>> So what would you say about the following?
> 
> It's been some time and it seems this has fallen between cracks. Is this
> worth puruing or I should just forget about it and drop it on the floor?

I actually came across this issue during some randconfig testing. Your
change looks good to me:

Reviewed-by: Stefan Agner 

Fixes for the ARM core usually go through Russell's patch tracker, did
you submit you patch there?

See also:
https://www.arm.linux.org.uk/developer/patches/info.php

--
Stefan

>> ---
>> From 2707f3bf00181bbc9dcf6a1f287eb7369141e955 Mon Sep 17 00:00:00 2001
>> From: Michal Hocko 
>> Date: Fri, 25 Aug 2017 08:40:09 +0200
>> Subject: [PATCH] arm: make kexec depend on MMU
>>
>> arm nommu config with KEXEC enabled doesn't compile
>> arch/arm/kernel/setup.c: In function 'reserve_crashkernel':
>> arch/arm/kernel/setup.c:1005:25: error: 'SECTION_SIZE' undeclared (first
>> use in this function)
>>  crash_size, SECTION_SIZE);
>>
>> since 61603016e212 ("ARM: kexec: fix crashkernel= handling") which is
>> over one year without anybody noticing. I have only noticed beause of
>> my testing nommu config which somehow gained CONFIG_KEXEC without
>> an intention. This suggests that nobody is actually using KEXEC
>> on nommu ARM configs. It is even a question whether kexec works with
>> nommu.
>>
>> Make KEXEC depend on MMU to make this clear. If somebody wants to enable
>> there will be probably more things to take care.
>>
>> Signed-off-by: Michal Hocko 
>> ---
>>  arch/arm/Kconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 3f4aa9179337..c8603195d7fc 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -2003,6 +2003,7 @@ config KEXEC
>>  bool "Kexec system call (EXPERIMENTAL)"
>>  depends on (!SMP || PM_SLEEP_SMP)
>>  depends on !CPU_V7M
>> +depends on MMU
>>  select KEXEC_CORE
>>  help
>>kexec is a system call that implements the ability to shutdown your
>> --
>> 2.13.2
>>
>> --
>> Michal Hocko
>> SUSE Labs


[PATCH 2/2] ARM: uaccess: use unified assembler language syntax

2019-03-02 Thread Stefan Agner
Convert the conditional infix to a postfix to make sure this inline
assembly is unified syntax. Since gcc assumes non-unified syntax
when emitting ARM instructions, make sure to define the syntax as
unified.

This allows to use LLVM's integrated assembler.

Additionally, for GCC ".syntax unified" for inline assembly.
When compiling non-Thumb2 GCC always emits a ".syntax divided"
at the beginning of the inline assembly which makes the
assembler fail. Since GCC 5 there is the -masm-syntax-unified
GCC option which make GCC assume unified syntax asm and hence
emits ".syntax unified" even in ARM mode. However, the option
is broken since GCC version 6 (see GCC PR88648 [1]). Work
around by adding ".syntax unified" as part of the inline
assembly.

[0] 
https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html#index-masm-syntax-unified
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88648

Signed-off-by: Stefan Agner 
---
I missed this instance in my previous commits and realized only
after running some randconfig.

 arch/arm/include/asm/uaccess.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index 89a28680934b..18303bcc9990 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -113,10 +113,11 @@ static inline void __user *__uaccess_mask_range_ptr(const 
void __user *ptr,
unsigned long tmp;
 
asm volatile(
+   "   .syntax unified\n"
"   sub %1, %3, #1\n"
"   subs%1, %1, %0\n"
"   addhs   %1, %1, #1\n"
-   "   subhss  %1, %1, %2\n"
+   "   subshs  %1, %1, %2\n"
"   movlo   %0, #0\n"
: "+r" (safe_ptr), "=" (tmp)
: "r" (size), "r" (current_thread_info()->addr_limit)
-- 
2.20.1



[PATCH 1/2] ARM: add TUSERCOND() macro for conditional postfix

2019-03-02 Thread Stefan Agner
Unified assembly syntax requires conditionals to be postfixes.
TUSER() currently only takes a single argument which then gets
appended t (with translation) on every instruction.

This fixes a build error when using LLVM's integrated assembler:
  In file included from kernel/futex.c:72:
  ./arch/arm/include/asm/futex.h:116:3: error: invalid instruction, did you 
mean: strt?
  "2: " TUSER(streq) "%3, [%4]\n"
   ^
  :5:4: note: instantiated into assembly here
  2:  streqt  r2, [r4]
  ^~

Additionally, for GCC ".syntax unified" for inline assembly.
When compiling non-Thumb2 GCC always emits a ".syntax divided"
at the beginning of the inline assembly which makes the
assembler fail. Since GCC 5 there is the -masm-syntax-unified
GCC option which make GCC assume unified syntax asm and hence
emits ".syntax unified" even in ARM mode. However, the option
is broken since GCC version 6 (see GCC PR88648 [1]). Work
around by adding ".syntax unified" as part of the inline
assembly.

[0] 
https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html#index-masm-syntax-unified
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88648

Signed-off-by: Stefan Agner 
---
 arch/arm/include/asm/domain.h | 6 --
 arch/arm/include/asm/futex.h  | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/domain.h b/arch/arm/include/asm/domain.h
index 99d9f630d6b6..1888c2d15da5 100644
--- a/arch/arm/include/asm/domain.h
+++ b/arch/arm/include/asm/domain.h
@@ -133,9 +133,11 @@ static inline void modify_domain(unsigned dom, unsigned 
type)  { }
  * instructions (inline assembly)
  */
 #ifdef CONFIG_CPU_USE_DOMAINS
-#define TUSER(instr)   #instr "t"
+#define TUSER(instr)   TUSERCOND(instr, )
+#define TUSERCOND(instr, cond) #instr "t" #cond
 #else
-#define TUSER(instr)   #instr
+#define TUSER(instr)   TUSERCOND(instr, )
+#define TUSERCOND(instr, cond) #instr #cond
 #endif
 
 #else /* __ASSEMBLY__ */
diff --git a/arch/arm/include/asm/futex.h b/arch/arm/include/asm/futex.h
index 0a46676b4245..83c391b597d4 100644
--- a/arch/arm/include/asm/futex.h
+++ b/arch/arm/include/asm/futex.h
@@ -110,10 +110,11 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user 
*uaddr,
preempt_disable();
__ua_flags = uaccess_save_and_enable();
__asm__ __volatile__("@futex_atomic_cmpxchg_inatomic\n"
+   "   .syntax unified\n"
"1: " TUSER(ldr) "  %1, [%4]\n"
"   teq %1, %2\n"
"   it  eq  @ explicit IT needed for the 2b label\n"
-   "2: " TUSER(streq) "%3, [%4]\n"
+   "2: " TUSERCOND(str, eq) "  %3, [%4]\n"
__futex_atomic_ex_table("%5")
: "+r" (ret), "=" (val)
: "r" (oldval), "r" (newval), "r" (uaddr), "Ir" (-EFAULT)
-- 
2.20.1



Re: [PATCH] Document: dt: binding: imx: Fix PAD_CTL_DSE_X*

2019-02-26 Thread Stefan Agner
On 26.02.2019 13:21, Aisheng Dong wrote:
>> From: Christina Quast [mailto:cqu...@hanoverdisplays.com]
>> Sent: Saturday, February 23, 2019 1:01 AM
>>
>> In the iMX7d datasheet, the PAD_CTL_DSE_X* values are different from the
>> documentation.
>>
> 
> It's a doc problem.
> Latest RM seems got updated.
> 
> As here it's a reference definition in binding doc and device tree
> actually does not
> use it (IMX Pinctrl use raw data to set pad configuration). So it
> won't cause any
> compatibility issue to me.
> 
> Please update the patch title to:
> dt-bindings: pinctrl: imx7d: x
> 
> Otherwise:
> Ack-by: Dong Aisheng 

Btw, I saw that imx7d-sdb.dts (and probably other i.MX 7 boards too) use
three different settings for usdhc pinctrl: 0x59, 0x5a and 0x5b (for
default, 100MHz and 200MHz respectively). One would expect that higher
frequency use higher driver strength (and this is the case for i.MX 6).
But with this new/corrected pad values this means we use x4, x2 and x6
for default, 100MHz and 200MHz respectively. This hardly seems right..?
Probably needs fixing too?

--
Stefan


> 
> Regards
> Dong Aisheng
> 
>> Signed-off-by: Christina Quast 
>> ---
>>  .../devicetree/bindings/pinctrl/fsl,imx7d-pinctrl.txt   | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/fsl,imx7d-pinctrl.txt
>> b/Documentation/devicetree/bindings/pinctrl/fsl,imx7d-pinctrl.txt
>> index 277c3acb..8ac1d0851a0f 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/fsl,imx7d-pinctrl.txt
>> +++ b/Documentation/devicetree/bindings/pinctrl/fsl,imx7d-pinctrl.txt
>> @@ -48,9 +48,9 @@ PAD_CTL_HYS (1 << 3)
>>  PAD_CTL_SRE_SLOW(1 << 2)
>>  PAD_CTL_SRE_FAST(0 << 2)
>>  PAD_CTL_DSE_X1  (0 << 0)
>> -PAD_CTL_DSE_X2  (1 << 0)
>> -PAD_CTL_DSE_X3  (2 << 0)
>> -PAD_CTL_DSE_X4  (3 << 0)
>> +PAD_CTL_DSE_X4  (1 << 0)
>> +PAD_CTL_DSE_X2  (2 << 0)
>> +PAD_CTL_DSE_X6  (3 << 0)
>>
> 
> 
>>  Examples:
>>  While iomuxc-lpsr is intended to be used by dedicated peripherals to take
>> --
>> 2.20.1
>>
>>
>>
>>
>>
>> Please consider the environment before printing this email
>>
>>
>>
>>
>>
>> The information transmitted is intended only for the person or entity to 
>> which
>> it is addressed and may contain confidential and/or privileged material. Any
>> review, retransmission, dissemination or other use of, or taking of any 
>> action in
>> reliance upon, this information by persons or entities other than the 
>> intended
>> recipient is prohibited.
>> If you received this in error, please contact the sender or postmaster
>> (postmas...@hanoverdisplays.com) and delete the material from any
>> computer.
>> Although we routinely screen for viruses, addressees should check this e-mail
>> and any attachment for viruses. We make no warranty as to absence of viruses
>> in this e-mail or any attachments.
>> Our Company's email policy is to permit incidental personal use. If this 
>> email is
>> of a personal nature, it must not be relied upon as expressing the views or
>> opinions of the company.


[PATCH v8] PCI: imx6: limit DBI register length

2019-02-26 Thread Stefan Agner
Define the length of the DBI registers and limit config space to its
length. This makes sure that the kernel does not access registers
beyond that point, avoiding the following abort on a i.MX 6Quad:
  # cat /sys/devices/soc0/soc/1ffc000.pcie/pci\:00/\:00\:00.0/config
  [  100.021433] Unhandled fault: imprecise external abort (0x1406) at 
0xb6ea7000
  ...
  [  100.056423] PC is at dw_pcie_read+0x50/0x84
  [  100.060790] LR is at dw_pcie_rd_own_conf+0x44/0x48
  ...

Signed-off-by: Stefan Agner 
---
Changes in v3:
- Rebase on pci/dwc
Changes in v4:
- Rebase on pci/dwc
Changes in v5:
- Rebased ontop of pci/dwc
- Use DBI length of 0x200
Changes in v6:
- Use pci_dev.cfg_size mechanism to limit config space (this made patch 1
  of previous versions of this patchset obsolete).
Changes in v7:
- Restrict fixup to Synopsys/0xabcd
- Apply cfg_size limitation only if dbi_length is specified
Changes in v8:
- Restrict fixup for Synopsys/0xabcd and class PCI bridge
- Check device driver to be pci-imx6

 drivers/pci/controller/dwc/pci-imx6.c | 33 +++
 1 file changed, 33 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
b/drivers/pci/controller/dwc/pci-imx6.c
index aaa9489e2140..5fbc00f71a94 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -56,6 +56,7 @@ enum imx6_pcie_variants {
 struct imx6_pcie_drvdata {
enum imx6_pcie_variants variant;
u32 flags;
+   int dbi_length;
 };
 
 struct imx6_pcie {
@@ -1242,6 +1243,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
.variant = IMX6Q,
.flags = IMX6_PCIE_FLAG_IMX6_PHY |
 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE,
+   .dbi_length = 0x200,
},
[IMX6SX] = {
.variant = IMX6SX,
@@ -1281,6 +1283,37 @@ static struct platform_driver imx6_pcie_driver = {
.shutdown = imx6_pcie_shutdown,
 };
 
+static void imx6_pcie_quirk(struct pci_dev *dev)
+{
+   struct pci_bus *bus = dev->bus;
+   struct pcie_port *pp = bus->sysdata;
+
+   /* Bus parent is the PCI bridge, its parent is this platform driver */
+   if (!bus->dev.parent || !bus->dev.parent->parent)
+   return;
+
+   /* Make sure we only quirk devices associated with this driver */
+   if (bus->dev.parent->parent->driver != _pcie_driver.driver)
+   return;
+
+   if (bus->number == pp->root_bus_nr) {
+   struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+   struct imx6_pcie *imx6_pcie = to_imx6_pcie(pci);
+
+   /*
+* Limit config length to avoid the kernel reading beyond
+* the register set and causing an abort on i.MX 6Quad
+*/
+   if (imx6_pcie->drvdata->dbi_length) {
+   dev->cfg_size = imx6_pcie->drvdata->dbi_length;
+   dev_info(>dev, "Limiting cfg_size to %d\n",
+   dev->cfg_size);
+   }
+   }
+}
+DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_VENDOR_ID_SYNOPSYS, 0xabcd,
+   PCI_CLASS_BRIDGE_PCI, 8, imx6_pcie_quirk);
+
 static int __init imx6_pcie_init(void)
 {
 #ifdef CONFIG_ARM
-- 
2.21.0



Re: [PATCH v7] PCI: imx6: limit DBI register length

2019-02-26 Thread Stefan Agner
On 25.02.2019 17:52, Trent Piepho wrote:
> On Mon, 2019-02-25 at 16:15 +, Leonard Crestez wrote:
>> On Mon, 2019-02-25 at 17:02 +0100, Stefan Agner wrote:
>> > Define the length of the DBI registers and limit config space to its
>> > length. This makes sure that the kernel does not access registers
>> > beyond that point, avoiding the following abort on a i.MX 6Quad:
>> >
>> > +static void imx6_pcie_quirk(struct pci_dev *dev)
>> > +{
>> > +  struct pci_bus *bus = dev->bus;
>> > +  struct pcie_port *pp = bus->sysdata;
>> > +
>> > +  if (bus->number == pp->root_bus_nr) {
>> > +  struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> > +  struct imx6_pcie *imx6_pcie = to_imx6_pcie(pci);
>> > +
>> > +  /*
>> > +   * Limit config length to avoid the kernel reading beyond
>> > +   * the register set and causing an abort on i.MX 6Quad
>> > +   */
>> > +  if (imx6_pcie->drvdata->dbi_length)
>> > +  dev->cfg_size = imx6_pcie->drvdata->dbi_length;
>> > +  }
>> > +}
>> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SYNOPSYS, 0xabcd, imx6_pcie_quirk);
>>
>> This looks like a default from SYNOPSYS so it likely run on other SOCs
>> using the DesignWare PCI IP and crash because of those unchecked casts.
> 
> Yes, it's used on IMX7d too.  But it's worse than that, there's a USB
> controller core that uses the same vendor and device id,
> PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3.  The quirk for that one uses class ==
> PCI_CLASS_SERIAL_USB_DEVICE to avoid matching this PCI-e IP.  See
> thread "PCI: Check for USB xHCI class for HAPS platform"

Hm, I see, something like this should fix this:

DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_VENDOR_ID_SYNOPSYS, 0xabcd,
PCI_CLASS_BRIDGE_PCI, 8, imx6_pcie_quirk);


(this needs "PCI: Work around Synopsys duplicate Device ID (HAPS USB3,
NXP i.MX)" applied, which is in v5.0-rc8 already).

--
Stefan


Re: [PATCH v7] PCI: imx6: limit DBI register length

2019-02-26 Thread Stefan Agner
On 25.02.2019 21:19, Bjorn Helgaas wrote:
> [+cc Thinh]
> 
> On Mon, Feb 25, 2019 at 10:52 AM Trent Piepho  wrote:
>> On Mon, 2019-02-25 at 16:15 +, Leonard Crestez wrote:
>> > On Mon, 2019-02-25 at 17:02 +0100, Stefan Agner wrote:
>> > > Define the length of the DBI registers and limit config space to its
>> > > length. This makes sure that the kernel does not access registers
>> > > beyond that point, avoiding the following abort on a i.MX 6Quad:
>> > >
>> > > +static void imx6_pcie_quirk(struct pci_dev *dev)
>> > > +{
>> > > +   struct pci_bus *bus = dev->bus;
>> > > +   struct pcie_port *pp = bus->sysdata;
>> > > +
>> > > +   if (bus->number == pp->root_bus_nr) {
>> > > +   struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> > > +   struct imx6_pcie *imx6_pcie = to_imx6_pcie(pci);
>> > > +
>> > > +   /*
>> > > +* Limit config length to avoid the kernel reading beyond
>> > > +* the register set and causing an abort on i.MX 6Quad
>> > > +*/
>> > > +   if (imx6_pcie->drvdata->dbi_length)
>> > > +   dev->cfg_size = imx6_pcie->drvdata->dbi_length;
>> > > +   }
>> > > +}
>> > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SYNOPSYS, 0xabcd, 
>> > > imx6_pcie_quirk);
>> >
>> > This looks like a default from SYNOPSYS so it likely run on other SOCs
>> > using the DesignWare PCI IP and crash because of those unchecked casts.
>>
>> Yes, it's used on IMX7d too.  But it's worse than that, there's a USB
>> controller core that uses the same vendor and device id,
>> PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3.  The quirk for that one uses class ==
>> PCI_CLASS_SERIAL_USB_DEVICE to avoid matching this PCI-e IP.  See
>> thread "PCI: Check for USB xHCI class for HAPS platform"
> 
> If we could get these vendors to allocate their own Vendor/Device IDs,
> maybe we could consider a DECLARE_PCI_FIXUP_EARLY quirk that would fix
> up pdev->vendor and pdev->device?  That might be cleaner than
> cluttering all these quirks with details of this screwup.

According to www.pcilookup.com there is a vendor/product id allocated
for recovery mode (Freescale i.MX 6, 15a2:0054). Is this a real PCI id?
The same Vendor/Device ID is used for USB recovery...

--
Stefan


[PATCH v7] PCI: imx6: limit DBI register length

2019-02-25 Thread Stefan Agner
Define the length of the DBI registers and limit config space to its
length. This makes sure that the kernel does not access registers
beyond that point, avoiding the following abort on a i.MX 6Quad:
  # cat /sys/devices/soc0/soc/1ffc000.pcie/pci\:00/\:00\:00.0/config
  [  100.021433] Unhandled fault: imprecise external abort (0x1406) at 
0xb6ea7000
  ...
  [  100.056423] PC is at dw_pcie_read+0x50/0x84
  [  100.060790] LR is at dw_pcie_rd_own_conf+0x44/0x48
  ...

Signed-off-by: Stefan Agner 
---
Changes in v3:
- Rebase on pci/dwc
Changes in v4:
- Rebase on pci/dwc
Changes in v5:
- Rebased ontop of pci/dwc
- Use DBI length of 0x200
Changes in v6:
- Use pci_dev.cfg_size mechanism to limit config space (this made patch 1
  of previous versions of this patchset obsolete).
Changes in v7:
- Restrict fixup to Synopsys/0xabcd
- Apply cfg_size limitation only if dbi_length is specified

 drivers/pci/controller/dwc/pci-imx6.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
b/drivers/pci/controller/dwc/pci-imx6.c
index aaa9489e2140..4317d2fffb67 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -56,6 +56,7 @@ enum imx6_pcie_variants {
 struct imx6_pcie_drvdata {
enum imx6_pcie_variants variant;
u32 flags;
+   int dbi_length;
 };
 
 struct imx6_pcie {
@@ -912,6 +913,25 @@ static const struct dw_pcie_ops dw_pcie_ops = {
/* No special ops needed, but pcie-designware still expects this struct 
*/
 };
 
+static void imx6_pcie_quirk(struct pci_dev *dev)
+{
+   struct pci_bus *bus = dev->bus;
+   struct pcie_port *pp = bus->sysdata;
+
+   if (bus->number == pp->root_bus_nr) {
+   struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+   struct imx6_pcie *imx6_pcie = to_imx6_pcie(pci);
+
+   /*
+* Limit config length to avoid the kernel reading beyond
+* the register set and causing an abort on i.MX 6Quad
+*/
+   if (imx6_pcie->drvdata->dbi_length)
+   dev->cfg_size = imx6_pcie->drvdata->dbi_length;
+   }
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SYNOPSYS, 0xabcd, imx6_pcie_quirk);
+
 #ifdef CONFIG_PM_SLEEP
 static void imx6_pcie_ltssm_disable(struct device *dev)
 {
@@ -1242,6 +1262,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
.variant = IMX6Q,
.flags = IMX6_PCIE_FLAG_IMX6_PHY |
 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE,
+   .dbi_length = 0x200,
},
[IMX6SX] = {
.variant = IMX6SX,
-- 
2.20.1



Re: [PATCH v6] PCI: imx6: limit DBI register length

2019-02-25 Thread Stefan Agner
On 25.02.2019 15:47, Leonard Crestez wrote:
> On Mon, 2019-02-25 at 15:25 +0100, Stefan Agner wrote:
>> Define the length of the DBI registers and limit config space to its
>> length. This makes sure that the kernel does not access registers
>> beyond that point, avoiding the following abort on a i.MX 6Quad:
>>   # cat /sys/devices/soc0/soc/1ffc000.pcie/pci\:00/\:00\:00.0/config
>>   [  100.021433] Unhandled fault: imprecise external abort (0x1406) at 
>> 0xb6ea7000
>>   ...
>>   [  100.056423] PC is at dw_pcie_read+0x50/0x84
>>   [  100.060790] LR is at dw_pcie_rd_own_conf+0x44/0x48
>>
>> +static void imx6_pcie_quirk(struct pci_dev *dev)
>> +{
>> +struct pci_bus *bus = dev->bus;
>> +struct pcie_port *pp = bus->sysdata;
>> +
>> +if (bus->number == pp->root_bus_nr) {
>> +struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> +struct imx6_pcie *imx6_pcie = to_imx6_pcie(pci);
>> +
>> +/*
>> + * Limit config length to avoid the kernel reading beyond
>> + * the register set and causing an abort on i.MX 6Quad
>> + */
>> +dev->cfg_size = imx6_pcie->drvdata->dbi_length;
>> +}
>> +}
>> +DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, imx6_pcie_quirk);
> 
> Are you sure that this quirk is only applied to imx6-pcie?
> Superficially it looks like it would apply to all PCI roots and this
> could be a problem on configs which enable all drivers, like arm64.

That was inspired by the FIXUP in keystone, which uses
PCI_ANY_ID/PCI_ANY_ID too.

However, keystone also does not have arm64 variants it seems.
Furthermore, the keystone fixup checks PCI ids in code.

> 
> Maybe the linker magic inside DECLARE_PCI_FIXUP deals with that, in
> that case I apologize.

I don't think there is special linker magic, if CONFIG_PCI_IMX6 is
enabled this will get called.

I agree, I should use PCI_VENDOR_ID_SYNOPSYS, 0xabcd here.


Btw, while looking at the patch again I realized that I need to check
imx6_pcie->drvdata->dbi_length for 0 and not apply in this case to avoid
breaking other i.MX designs.

I will send a v7.

--
Stefan


[PATCH v6] PCI: imx6: limit DBI register length

2019-02-25 Thread Stefan Agner
Define the length of the DBI registers and limit config space to its
length. This makes sure that the kernel does not access registers
beyond that point, avoiding the following abort on a i.MX 6Quad:
  # cat /sys/devices/soc0/soc/1ffc000.pcie/pci\:00/\:00\:00.0/config
  [  100.021433] Unhandled fault: imprecise external abort (0x1406) at 
0xb6ea7000
  ...
  [  100.056423] PC is at dw_pcie_read+0x50/0x84
  [  100.060790] LR is at dw_pcie_rd_own_conf+0x44/0x48
  ...

Signed-off-by: Stefan Agner 
---
Changes in v3:
- Rebase on pci/dwc
Changes in v4:
- Rebase on pci/dwc
Changes in v5:
- Rebased ontop of pci/dwc
- Use DBI length of 0x200
Changes in v6:
- Use pci_dev.cfg_size mechanism to limit config space (this made patch 1
  of previous versions of this patchset obsolete).

(Lucas, I dropped your Reviewed-by since changes are substantial...)

 drivers/pci/controller/dwc/pci-imx6.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c 
b/drivers/pci/controller/dwc/pci-imx6.c
index aaa9489e2140..8bd335c52414 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -56,6 +56,7 @@ enum imx6_pcie_variants {
 struct imx6_pcie_drvdata {
enum imx6_pcie_variants variant;
u32 flags;
+   int dbi_length;
 };
 
 struct imx6_pcie {
@@ -912,6 +913,24 @@ static const struct dw_pcie_ops dw_pcie_ops = {
/* No special ops needed, but pcie-designware still expects this struct 
*/
 };
 
+static void imx6_pcie_quirk(struct pci_dev *dev)
+{
+   struct pci_bus *bus = dev->bus;
+   struct pcie_port *pp = bus->sysdata;
+
+   if (bus->number == pp->root_bus_nr) {
+   struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+   struct imx6_pcie *imx6_pcie = to_imx6_pcie(pci);
+
+   /*
+* Limit config length to avoid the kernel reading beyond
+* the register set and causing an abort on i.MX 6Quad
+*/
+   dev->cfg_size = imx6_pcie->drvdata->dbi_length;
+   }
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, imx6_pcie_quirk);
+
 #ifdef CONFIG_PM_SLEEP
 static void imx6_pcie_ltssm_disable(struct device *dev)
 {
@@ -1242,6 +1261,7 @@ static const struct imx6_pcie_drvdata drvdata[] = {
.variant = IMX6Q,
.flags = IMX6_PCIE_FLAG_IMX6_PHY |
 IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE,
+   .dbi_length = 0x200,
},
[IMX6SX] = {
.variant = IMX6SX,
-- 
2.20.1



  1   2   3   4   5   6   7   8   9   10   >