* [PATCH v2 07/10] arm64: dts: microchip: sparx5_pcb134: drop LED unit addresses
From: Krzysztof Kozlowski @ 2024-04-05 19:04 UTC (permalink / raw)
To: Conor Dooley, Nicolas Ferre, Claudiu Beznea, Rob Herring,
Krzysztof Kozlowski, Lars Povlsen, Steen Hegelund, Daniel Machon,
UNGLinuxDriver, Bjarni Jonasson, David S. Miller,
linux-arm-kernel, devicetree, linux-kernel
Cc: horatiu.vultur, steen.hegelund, Krzysztof Kozlowski
In-Reply-To: <20240405190419.74162-1-krzk@kernel.org>
GPIO leds should not have unit addresses (no "reg" property), as
reported by dtc W=1 warnings:
sparx5_pcb134_board.dtsi:18.9-21.5: Warning (unit_address_vs_reg): /leds/led@0: node has a unit name, but no reg or ranges property
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Changes in v2:
1. None
---
.../dts/microchip/sparx5_pcb134_board.dtsi | 96 +++++++++----------
1 file changed, 48 insertions(+), 48 deletions(-)
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
index cafec6ef0d0f..f165a409bc1d 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
@@ -15,234 +15,234 @@ gpio-restart {
leds {
compatible = "gpio-leds";
- led@0 {
+ led-0 {
label = "twr0:green";
gpios = <&sgpio_out0 8 0 GPIO_ACTIVE_LOW>;
};
- led@1 {
+ led-1 {
label = "twr0:yellow";
gpios = <&sgpio_out0 8 1 GPIO_ACTIVE_LOW>;
};
- led@2 {
+ led-2 {
label = "twr1:green";
gpios = <&sgpio_out0 9 0 GPIO_ACTIVE_LOW>;
};
- led@3 {
+ led-3 {
label = "twr1:yellow";
gpios = <&sgpio_out0 9 1 GPIO_ACTIVE_LOW>;
};
- led@4 {
+ led-4 {
label = "twr2:green";
gpios = <&sgpio_out0 10 0 GPIO_ACTIVE_LOW>;
};
- led@5 {
+ led-5 {
label = "twr2:yellow";
gpios = <&sgpio_out0 10 1 GPIO_ACTIVE_LOW>;
};
- led@6 {
+ led-6 {
label = "twr3:green";
gpios = <&sgpio_out0 11 0 GPIO_ACTIVE_LOW>;
};
- led@7 {
+ led-7 {
label = "twr3:yellow";
gpios = <&sgpio_out0 11 1 GPIO_ACTIVE_LOW>;
};
- led@8 {
+ led-8 {
label = "eth12:green";
gpios = <&sgpio_out0 12 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@9 {
+ led-9 {
label = "eth12:yellow";
gpios = <&sgpio_out0 12 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@10 {
+ led-10 {
label = "eth13:green";
gpios = <&sgpio_out0 13 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@11 {
+ led-11 {
label = "eth13:yellow";
gpios = <&sgpio_out0 13 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@12 {
+ led-12 {
label = "eth14:green";
gpios = <&sgpio_out0 14 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@13 {
+ led-13 {
label = "eth14:yellow";
gpios = <&sgpio_out0 14 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@14 {
+ led-14 {
label = "eth15:green";
gpios = <&sgpio_out0 15 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@15 {
+ led-15 {
label = "eth15:yellow";
gpios = <&sgpio_out0 15 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@16 {
+ led-16 {
label = "eth48:green";
gpios = <&sgpio_out1 16 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@17 {
+ led-17 {
label = "eth48:yellow";
gpios = <&sgpio_out1 16 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@18 {
+ led-18 {
label = "eth49:green";
gpios = <&sgpio_out1 17 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@19 {
+ led-19 {
label = "eth49:yellow";
gpios = <&sgpio_out1 17 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@20 {
+ led-20 {
label = "eth50:green";
gpios = <&sgpio_out1 18 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@21 {
+ led-21 {
label = "eth50:yellow";
gpios = <&sgpio_out1 18 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@22 {
+ led-22 {
label = "eth51:green";
gpios = <&sgpio_out1 19 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@23 {
+ led-23 {
label = "eth51:yellow";
gpios = <&sgpio_out1 19 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@24 {
+ led-24 {
label = "eth52:green";
gpios = <&sgpio_out1 20 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@25 {
+ led-25 {
label = "eth52:yellow";
gpios = <&sgpio_out1 20 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@26 {
+ led-26 {
label = "eth53:green";
gpios = <&sgpio_out1 21 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@27 {
+ led-27 {
label = "eth53:yellow";
gpios = <&sgpio_out1 21 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@28 {
+ led-28 {
label = "eth54:green";
gpios = <&sgpio_out1 22 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@29 {
+ led-29 {
label = "eth54:yellow";
gpios = <&sgpio_out1 22 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@30 {
+ led-30 {
label = "eth55:green";
gpios = <&sgpio_out1 23 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@31 {
+ led-31 {
label = "eth55:yellow";
gpios = <&sgpio_out1 23 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@32 {
+ led-32 {
label = "eth56:green";
gpios = <&sgpio_out1 24 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@33 {
+ led-33 {
label = "eth56:yellow";
gpios = <&sgpio_out1 24 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@34 {
+ led-34 {
label = "eth57:green";
gpios = <&sgpio_out1 25 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@35 {
+ led-35 {
label = "eth57:yellow";
gpios = <&sgpio_out1 25 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@36 {
+ led-36 {
label = "eth58:green";
gpios = <&sgpio_out1 26 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@37 {
+ led-37 {
label = "eth58:yellow";
gpios = <&sgpio_out1 26 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@38 {
+ led-38 {
label = "eth59:green";
gpios = <&sgpio_out1 27 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@39 {
+ led-39 {
label = "eth59:yellow";
gpios = <&sgpio_out1 27 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@40 {
+ led-40 {
label = "eth60:green";
gpios = <&sgpio_out1 28 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@41 {
+ led-41 {
label = "eth60:yellow";
gpios = <&sgpio_out1 28 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@42 {
+ led-42 {
label = "eth61:green";
gpios = <&sgpio_out1 29 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@43 {
+ led-43 {
label = "eth61:yellow";
gpios = <&sgpio_out1 29 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@44 {
+ led-44 {
label = "eth62:green";
gpios = <&sgpio_out1 30 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@45 {
+ led-45 {
label = "eth62:yellow";
gpios = <&sgpio_out1 30 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@46 {
+ led-46 {
label = "eth63:green";
gpios = <&sgpio_out1 31 0 GPIO_ACTIVE_HIGH>;
default-state = "off";
};
- led@47 {
+ led-47 {
label = "eth63:yellow";
gpios = <&sgpio_out1 31 1 GPIO_ACTIVE_HIGH>;
default-state = "off";
--
2.34.1
^ permalink raw reply related
* [PATCH v2 06/10] arm64: dts: microchip: sparx5_pcb135: align I2C mux node name with bindings
From: Krzysztof Kozlowski @ 2024-04-05 19:04 UTC (permalink / raw)
To: Conor Dooley, Nicolas Ferre, Claudiu Beznea, Rob Herring,
Krzysztof Kozlowski, Lars Povlsen, Steen Hegelund, Daniel Machon,
UNGLinuxDriver, Bjarni Jonasson, David S. Miller,
linux-arm-kernel, devicetree, linux-kernel
Cc: horatiu.vultur, steen.hegelund, Krzysztof Kozlowski
In-Reply-To: <20240405190419.74162-1-krzk@kernel.org>
DT schema expects node names to match certain. This fixes dtbs_check
warnings like:
sparx5_pcb135_emmc.dtb: i2c0-imux@0: $nodename:0: 'i2c0-imux@0' does not match '^(i2c-?)?mux'
and dtc W=1 warnings:
sparx5_pcb135_board.dtsi:132.25-137.4: Warning (simple_bus_reg): /axi@600000000/i2c0-imux@0: missing or empty reg/ranges property
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Changes in v2:
1. None
---
arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi b/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
index bf51a6e11cf1..860975ffe0a1 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
@@ -129,7 +129,7 @@ &sgpio2 {
};
&axi {
- i2c0_imux: i2c0-imux@0 {
+ i2c0_imux: i2c-mux {
compatible = "i2c-mux-pinctrl";
#address-cells = <1>;
#size-cells = <0>;
--
2.34.1
^ permalink raw reply related
* [PATCH v2 05/10] arm64: dts: microchip: sparx5_pcb134: align I2C mux node name with bindings
From: Krzysztof Kozlowski @ 2024-04-05 19:04 UTC (permalink / raw)
To: Conor Dooley, Nicolas Ferre, Claudiu Beznea, Rob Herring,
Krzysztof Kozlowski, Lars Povlsen, Steen Hegelund, Daniel Machon,
UNGLinuxDriver, Bjarni Jonasson, David S. Miller,
linux-arm-kernel, devicetree, linux-kernel
Cc: horatiu.vultur, steen.hegelund, Krzysztof Kozlowski
In-Reply-To: <20240405190419.74162-1-krzk@kernel.org>
DT schema expects node names to match certain. This fixes dtbs_check
warnings like:
sparx5_pcb134_emmc.dtb: i2c0-emux@0: $nodename:0: 'i2c0-emux@0' does not match '^(i2c-?)?mux'
and dtc W=1 warnings:
sparx5_pcb134_board.dtsi:398.25-403.4: Warning (unique_unit_address_if_enabled): /axi@600000000/i2c0-imux@0: duplicate unit-address (also used in node /axi@600000000/i2c0-emux@0)
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Changes in v2:
1. None
---
arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
index e816e6e9d62d..cafec6ef0d0f 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
@@ -395,13 +395,13 @@ i2cmux_11: i2cmux-11-pins {
};
&axi {
- i2c0_imux: i2c0-imux@0 {
+ i2c0_imux: i2c-mux-0 {
compatible = "i2c-mux-pinctrl";
#address-cells = <1>;
#size-cells = <0>;
i2c-parent = <&i2c0>;
};
- i2c0_emux: i2c0-emux@0 {
+ i2c0_emux: i2c-mux-1 {
compatible = "i2c-mux-gpio";
#address-cells = <1>;
#size-cells = <0>;
--
2.34.1
^ permalink raw reply related
* [PATCH v2 04/10] arm64: dts: microchip: sparx5_pcb135: add missing I2C mux unit addresses
From: Krzysztof Kozlowski @ 2024-04-05 19:04 UTC (permalink / raw)
To: Conor Dooley, Nicolas Ferre, Claudiu Beznea, Rob Herring,
Krzysztof Kozlowski, Lars Povlsen, Steen Hegelund, Daniel Machon,
UNGLinuxDriver, Bjarni Jonasson, David S. Miller,
linux-arm-kernel, devicetree, linux-kernel
Cc: horatiu.vultur, steen.hegelund, Krzysztof Kozlowski
In-Reply-To: <20240405190419.74162-1-krzk@kernel.org>
The children of I2C mux should be named "i2c", according to DT schema
and bindings, and they should have unit address.
This fixes dtbs_check warnings like:
sparx5_pcb135.dtb: i2c0-imux@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'i2c_sfp1', 'i2c_sfp2', 'i2c_sfp3', 'i2c_sfp4' were unexpected)
and dtc W=1 warnings:
sparx5_pcb135_board.dtsi:172.23-180.4: Warning (simple_bus_reg): /axi@600000000/sfp-eth60: missing or empty reg/ranges property
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Changes in v2:
1. None
---
arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi b/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
index 82ce007d9959..bf51a6e11cf1 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
@@ -146,22 +146,22 @@ &i2c0_imux {
pinctrl-2 = <&i2cmux_s31>;
pinctrl-3 = <&i2cmux_s32>;
pinctrl-4 = <&i2cmux_pins_i>;
- i2c_sfp1: i2c_sfp1 {
+ i2c_sfp1: i2c@0 {
reg = <0x0>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp2: i2c_sfp2 {
+ i2c_sfp2: i2c@1 {
reg = <0x1>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp3: i2c_sfp3 {
+ i2c_sfp3: i2c@2 {
reg = <0x2>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp4: i2c_sfp4 {
+ i2c_sfp4: i2c@3 {
reg = <0x3>;
#address-cells = <1>;
#size-cells = <0>;
--
2.34.1
^ permalink raw reply related
* [PATCH v2 03/10] arm64: dts: microchip: sparx5_pcb134: add missing I2C mux unit addresses
From: Krzysztof Kozlowski @ 2024-04-05 19:04 UTC (permalink / raw)
To: Conor Dooley, Nicolas Ferre, Claudiu Beznea, Rob Herring,
Krzysztof Kozlowski, Lars Povlsen, Steen Hegelund, Daniel Machon,
UNGLinuxDriver, Bjarni Jonasson, David S. Miller,
linux-arm-kernel, devicetree, linux-kernel
Cc: horatiu.vultur, steen.hegelund, Krzysztof Kozlowski
In-Reply-To: <20240405190419.74162-1-krzk@kernel.org>
The children of I2C mux should be named "i2c", according to DT schema
and bindings, and they should have unit address.
This fixes dtbs_check warnings like:
sparx5_pcb134_emmc.dtb: i2c0-imux@0: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'i2c_sfp1', ...
and dtc W=1 warnings:
sparx5_pcb134_board.dtsi:548.23-555.4: Warning (simple_bus_reg): /axi@600000000/sfp-eth12: missing or empty reg/ranges property
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Changes in v2:
1. None
---
.../dts/microchip/sparx5_pcb134_board.dtsi | 40 +++++++++----------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
index f3e226de5e5e..e816e6e9d62d 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
@@ -427,62 +427,62 @@ &i2c0_imux {
pinctrl-10 = <&i2cmux_10>;
pinctrl-11 = <&i2cmux_11>;
pinctrl-12 = <&i2cmux_pins_i>;
- i2c_sfp1: i2c_sfp1 {
+ i2c_sfp1: i2c@0 {
reg = <0x0>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp2: i2c_sfp2 {
+ i2c_sfp2: i2c@1 {
reg = <0x1>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp3: i2c_sfp3 {
+ i2c_sfp3: i2c@2 {
reg = <0x2>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp4: i2c_sfp4 {
+ i2c_sfp4: i2c@3 {
reg = <0x3>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp5: i2c_sfp5 {
+ i2c_sfp5: i2c@4 {
reg = <0x4>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp6: i2c_sfp6 {
+ i2c_sfp6: i2c@5 {
reg = <0x5>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp7: i2c_sfp7 {
+ i2c_sfp7: i2c@6 {
reg = <0x6>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp8: i2c_sfp8 {
+ i2c_sfp8: i2c@7 {
reg = <0x7>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp9: i2c_sfp9 {
+ i2c_sfp9: i2c@8 {
reg = <0x8>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp10: i2c_sfp10 {
+ i2c_sfp10: i2c@9 {
reg = <0x9>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp11: i2c_sfp11 {
+ i2c_sfp11: i2c@a {
reg = <0xa>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp12: i2c_sfp12 {
+ i2c_sfp12: i2c@b {
reg = <0xb>;
#address-cells = <1>;
#size-cells = <0>;
@@ -495,42 +495,42 @@ &gpio 60 GPIO_ACTIVE_HIGH
&gpio 61 GPIO_ACTIVE_HIGH
&gpio 54 GPIO_ACTIVE_HIGH>;
idle-state = <0x8>;
- i2c_sfp13: i2c_sfp13 {
+ i2c_sfp13: i2c@0 {
reg = <0x0>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp14: i2c_sfp14 {
+ i2c_sfp14: i2c@1 {
reg = <0x1>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp15: i2c_sfp15 {
+ i2c_sfp15: i2c@2 {
reg = <0x2>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp16: i2c_sfp16 {
+ i2c_sfp16: i2c@3 {
reg = <0x3>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp17: i2c_sfp17 {
+ i2c_sfp17: i2c@4 {
reg = <0x4>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp18: i2c_sfp18 {
+ i2c_sfp18: i2c@5 {
reg = <0x5>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp19: i2c_sfp19 {
+ i2c_sfp19: i2c@6 {
reg = <0x6>;
#address-cells = <1>;
#size-cells = <0>;
};
- i2c_sfp20: i2c_sfp20 {
+ i2c_sfp20: i2c@7 {
reg = <0x7>;
#address-cells = <1>;
#size-cells = <0>;
--
2.34.1
^ permalink raw reply related
* [PATCH v2 02/10] arm64: dts: microchip: sparx5: correct serdes unit address
From: Krzysztof Kozlowski @ 2024-04-05 19:04 UTC (permalink / raw)
To: Conor Dooley, Nicolas Ferre, Claudiu Beznea, Rob Herring,
Krzysztof Kozlowski, Lars Povlsen, Steen Hegelund, Daniel Machon,
UNGLinuxDriver, Bjarni Jonasson, David S. Miller,
linux-arm-kernel, devicetree, linux-kernel
Cc: horatiu.vultur, steen.hegelund, Krzysztof Kozlowski
In-Reply-To: <20240405190419.74162-1-krzk@kernel.org>
Unit address should match "reg" property, as reported by dtc W=1
warnings:
sparx5.dtsi:463.27-468.5: Warning (simple_bus_reg): /axi@600000000/serdes@10808000: simple-bus unit address format error, expected "610808000"
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Changes in v2:
1. None
---
arch/arm64/boot/dts/microchip/sparx5.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi
index 5d820da8c69d..c3029e0abacc 100644
--- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
@@ -460,7 +460,7 @@ mdio3: mdio@61101031c {
reg = <0x6 0x1101031c 0x24>;
};
- serdes: serdes@10808000 {
+ serdes: serdes@610808000 {
compatible = "microchip,sparx5-serdes";
#phy-cells = <1>;
clocks = <&sys_clk>;
--
2.34.1
^ permalink raw reply related
* [PATCH v2 01/10] arm64: dts: microchip: sparx5: fix mdio reg
From: Krzysztof Kozlowski @ 2024-04-05 19:04 UTC (permalink / raw)
To: Conor Dooley, Nicolas Ferre, Claudiu Beznea, Rob Herring,
Krzysztof Kozlowski, Lars Povlsen, Steen Hegelund, Daniel Machon,
UNGLinuxDriver, Bjarni Jonasson, David S. Miller,
linux-arm-kernel, devicetree, linux-kernel
Cc: horatiu.vultur, steen.hegelund, Krzysztof Kozlowski
Correct the reg address of mdio node to match unit address. Assume the
reg is not correct and unit address was correct, because there is
already node using the existing reg 0x110102d4.
sparx5.dtsi:443.25-451.5: Warning (simple_bus_reg): /axi@600000000/mdio@6110102f8: simple-bus unit address format error, expected "6110102d4"
Fixes: d0f482bb06f9 ("arm64: dts: sparx5: Add the Sparx5 switch node")
Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com>
Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
Not tested on hardware
Changes in v2:
1. Add tags.
---
arch/arm64/boot/dts/microchip/sparx5.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi
index 24075cd91420..5d820da8c69d 100644
--- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
@@ -447,7 +447,7 @@ mdio2: mdio@6110102f8 {
pinctrl-names = "default";
#address-cells = <1>;
#size-cells = <0>;
- reg = <0x6 0x110102d4 0x24>;
+ reg = <0x6 0x110102f8 0x24>;
};
mdio3: mdio@61101031c {
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2 2/2] Input: edt-ft5x06 - add ft5426
From: Andreas Kemnade @ 2024-04-05 18:56 UTC (permalink / raw)
To: Andy Shevchenko
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
o.rempel, u.kleine-koenig, hdegoede, ye.xingchen, p.puschmann,
linux-input, devicetree, linux-kernel, caleb.connolly
In-Reply-To: <CAHp75VckoDheCN-KQ0KcSk9rE_-cXFUujurtA4sK6KAixDttQQ@mail.gmail.com>
On Fri, 5 Apr 2024 20:21:19 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Fri, Apr 5, 2024 at 7:28 PM Andreas Kemnade <andreas@kemnade.info> wrote:
> > On Fri, 5 Apr 2024 18:13:45 +0300
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > On Fri, Apr 5, 2024 at 1:20 AM Andreas Kemnade <andreas@kemnade.info> wrote:
>
> ...
>
> > > > @@ -1484,6 +1484,7 @@ static const struct of_device_id edt_ft5x06_of_match[] = {
> > > > { .compatible = "edt,edt-ft5206", .data = &edt_ft5x06_data },
> > > > { .compatible = "edt,edt-ft5306", .data = &edt_ft5x06_data },
> > > > { .compatible = "edt,edt-ft5406", .data = &edt_ft5x06_data },
> > > > + { .compatible = "focaltech,ft5426", .data = &edt_ft5506_data },
> > >
> > > Why a different vendor prefix?
> > > In case you need to use this one, keep the list sorted, currently this
> > > splits the edt,* ones.
> > >
> > How do I know whether to use evervision or edt instead?
>
> Ask DT people, the vendor-prefixes lists both...
>
> > I sorted by the numbers. Looking at datasheets for other controllers I see
> > https://www.displayfuture.com/Display/datasheet/controller/FT5x06.pdf
> > it only mentions FocalTech Systems Co., Ltd.
>
> But does the driver use that? AFAICS it uses edt. Perhaps it's due to
> a business split, not to my knowledge anyway.
>
well, the fact is that there were several tried to add duplicates to this
driver to the kernel using focaltech prefixes e.g.
https://lore.kernel.org/linux-input/47209259-9e57-f263-bf48-10f233c63b69@redhat.com/
My guess it is somehow about owner of the firmware in the chip vs the chip itself.
Regards,
Andreas
^ permalink raw reply
* Re: [RFC PATCH 2/2] PCI: Add Qualcomm PCIe ECAM root complex driver
From: Bjorn Helgaas @ 2024-04-05 18:30 UTC (permalink / raw)
To: Mayank Rana
Cc: linux-pci, lpieralisi, kw, robh, bhelgaas, andersson,
manivannan.sadhasivam, krzysztof.kozlowski+dt, conor+dt,
devicetree, linux-arm-msm, quic_ramkri, quic_nkela, quic_shazhuss,
quic_msarkar, quic_nitegupt
In-Reply-To: <1712257884-23841-3-git-send-email-quic_mrana@quicinc.com>
On Thu, Apr 04, 2024 at 12:11:24PM -0700, Mayank Rana wrote:
> On some of Qualcomm platform, firmware configures PCIe controller into
> ECAM mode allowing static memory allocation for configuration space of
> supported bus range. Firmware also takes care of bringing up PCIe PHY
> and performing required operation to bring PCIe link into D0. Firmware
I think link state would be L0, not D0.
> also manages system resources (e.g. clocks/regulators/resets/ bus voting).
> Hence add Qualcomm PCIe ECAM root complex driver which enumerates PCIe
> root complex and connected PCIe devices. Firmware won't be enumerating
> or powering up PCIe root complex until this driver invokes power domain
> based notification to bring PCIe link into D0/D3cold mode.
Again.
> +config PCIE_QCOM_ECAM
> + tristate "QCOM PCIe ECAM host controller"
> + depends on ARCH_QCOM && PCI
> + depends on OF
> + select PCI_MSI
> + select PCI_HOST_COMMON
> + select IRQ_DOMAIN
> + help
> + Say 'Y' here if you want to use ECAM shift mode compatible Qualcomm
> + PCIe root host controller. The controller is programmed using firmware
> + to support ECAM compatible memory address space.
Instead of adding this at the end, place this entry so the entire list
remains sorted by vendor name.
Other related entries are "Qualcomm PCIe controller ..." (not "QCOM").
Use "ECAM PCIe controller (host mode)" (not "PCIe ECAM host
controller") so it matches similar entries.
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
Does this actually work? I expected "#define dev_fmt" since you're
using dev_err(), etc below.
> +#include <linux/irqchip/chained_irq.h>
Can this be reworked so it doesn't use chained IRQs? I admit to not
being an IRQ expert, but I have the impression that it's better to
avoid the chained IRQ model when possible. See
https://lore.kernel.org/all/20231108153133.GA393726@bhelgaas/
> +#define MSI_DB_ADDR 0xa0000000
Where does this come from and why is it hard-coded here? Looks like a
magic address that maybe should come from DT?
> + * struct qcom_msi_irq - MSI IRQ information
> + * @client: pointer to MSI client struct
> + * @grp: group the irq belongs to
s/irq/IRQ/ in comments for consistency (other occurrences below).
Same for s/pcie/PCIe/ and s/msi/MSI/.
> +static void qcom_msi_mask_irq(struct irq_data *data)
> +{
> + struct irq_data *parent_data;
> + struct qcom_msi_irq *msi_irq;
> + struct qcom_msi_grp *msi_grp;
> + struct qcom_msi *msi;
> + unsigned long flags;
> +
> + parent_data = data->parent_data;
> + if (!parent_data)
> + return;
Drop this test; I think it only detects logic errors in the driver or
memory corruptions, and we want to find out about those.
> +static void qcom_msi_unmask_irq(struct irq_data *data)
> +{
> + struct irq_data *parent_data;
> + struct qcom_msi_irq *msi_irq;
> + struct qcom_msi_grp *msi_grp;
> + struct qcom_msi *msi;
> + unsigned long flags;
> +
> + parent_data = data->parent_data;
> + if (!parent_data)
> + return;
Drop.
> +static struct irq_chip qcom_msi_irq_chip = {
> + .name = "qcom_pci_msi",
> + .irq_enable = qcom_msi_unmask_irq,
> + .irq_disable = qcom_msi_mask_irq,
> + .irq_mask = qcom_msi_mask_irq,
> + .irq_unmask = qcom_msi_unmask_irq,
Name these so they match the struct member, e.g., the name should
contain "irq_mask", not "mask_irq") so grep finds them easily.
> +static struct msi_domain_ops qcom_msi_domain_ops = {
> + .msi_prepare = qcom_msi_domain_prepare,
Rename so function name includes the struct member name.
> +static int qcom_msi_irq_set_affinity(struct irq_data *data,
> + const struct cpumask *mask, bool force)
> +{
> + struct irq_data *parent_data = irq_get_irq_data(irqd_to_hwirq(data));
> + int ret = 0;
> +
> + if (!parent_data)
> + return -ENODEV;
> +
> + /* set affinity for MSI HW IRQ */
Unnecessary comment.
> + if (parent_data->chip->irq_set_affinity)
> + ret = parent_data->chip->irq_set_affinity(parent_data, mask, force);
> +
> + return ret;
Drop "ret" and return directly, e.g.,
if (parent_data->chip->irq_set_affinity)
return parent_data->chip->irq_set_affinity(...);
return 0;
> +static void qcom_msi_irq_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> +{
> + struct irq_data *parent_data = irq_get_irq_data(irqd_to_hwirq(data));
> + struct qcom_msi_irq *msi_irq = irq_data_get_irq_chip_data(data);
> + struct qcom_msi_client *client = msi_irq->client;
> +
> + if (!parent_data)
> + return;
Drop.
> +static void qcom_msi_irq_domain_free(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs)
> +{
> + struct irq_data *data = irq_domain_get_irq_data(domain, virq);
> + struct qcom_msi_client *client;
> + struct qcom_msi_irq *msi_irq;
> + struct qcom_msi *msi;
> +
> + if (!data)
> + return;
Drop.
> +static int qcom_msi_irq_setup(struct qcom_msi *msi)
> +{
> + struct qcom_msi_grp *msi_grp;
> + struct qcom_msi_irq *msi_irq;
> + int i, index, ret;
> + unsigned int irq;
> +
> + /* setup each MSI group. nr_hwirqs == nr_grps */
> + for (i = 0; i < msi->nr_hwirqs; i++) {
> + irq = irq_of_parse_and_map(msi->dev->of_node, i);
> + if (!irq) {
> + dev_err(msi->dev,
> + "MSI: failed to parse/map interrupt\n");
Possibly include "i" to identify the offending entry.
^ permalink raw reply
* Re: [RESEND PATCH v9 2/4] dt-bindings: stm32: update DT bingding for stm32mp25
From: Krzysztof Kozlowski @ 2024-04-05 18:22 UTC (permalink / raw)
To: Gabriel FERNANDEZ, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin,
Alexandre Torgue, Philipp Zabel
Cc: linux-clk, devicetree, linux-stm32, linux-arm-kernel,
linux-kernel
In-Reply-To: <285f2f64-58b0-4dd0-9f1a-89306a79d572@foss.st.com>
On 05/04/2024 14:54, Gabriel FERNANDEZ wrote:
>
> On 4/5/24 09:12, Krzysztof Kozlowski wrote:
>> On 02/04/2024 14:53, gabriel.fernandez@foss.st.com wrote:
>>> From: Gabriel Fernandez <gabriel.fernandez@foss.st.com>
>>>
>>> Now RCC driver use '.index' of clk_parent_data struct to define a parent.
>>> The majority of parents are SCMI clocks, then dt-bindings must be fixed.
>>>
>>> Fixes: b5be49db3d47 ("dt-bindings: stm32: add clocks and reset binding for stm32mp25 platform")
>> And except what Rob said, this does not look as a fix. How ABI break
>> could be a fix and what is even to fix here? Please describe the
>> observable bug, how it manifests itself and what is exactly the fix for
>> that bug.
> As I replied to Rob, there are no RCC STM32MP25 drivers already upstreamed.
>
> However, in my series, the DT binding was merged even though Stephen
> made some
>
> important remarks that needed to be taken into account.
>
> That's why I proposed a fix to update the documentation.
>
> To be sure, how would you like me to proceed?
You can send v3 and get exactly the same questions. Your commit msg must
answer to all such unusual questions. If maintainer asks something that
you need to explain, it is a hint for you that your commit msg is
inadequate.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/1] arm64: dts: imx8qxp-mek: add cm40_i2c, wm8960/wm8962 and sai[0,1,4,5]
From: Krzysztof Kozlowski @ 2024-04-05 18:21 UTC (permalink / raw)
To: Frank Li
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list
In-Reply-To: <ZhAO4YWuB8r8k+m8@lizhi-Precision-Tower-5810>
On 05/04/2024 16:46, Frank Li wrote:
> On Fri, Apr 05, 2024 at 08:41:59AM +0200, Krzysztof Kozlowski wrote:
>> On 04/04/2024 18:19, Frank Li wrote:
>>> imx8qxp-mek use two kind audio codec, wm8960 and wm8962. Using dummy gpio
>>> i2c bus mux to connect both i2c devices. One will probe failure and other
>>> will probe success when devices driver check whoami. So one dtb can cover
>>> both board configuration.
>>
>> I don't understand it. Either you add real device or not. If one board
>> has two devices, then why do you need to check for failures?
>>
>> Anyway, don't add fake stuff to DTS.
>
> NAK can't resolve the problem. It should be common problem for long time
> cycle boards. Some chipes will be out life cycle. such as some sensor. So
> chips on boards have been replace by some pin to pin compatible sensor. For
> example:
> old boards: use sensor A with address 0x1a
> new bench: use sensor B with address 0x1b.
>
> You can treat it as two kind boards, RevA or RevB. But most user want to
> use one dtb to handle such small differences. For this case, it should be
> simple. Just add a super set.
> i2c
> {
> sensorA@1a
> {
> }
> sensorB@1b
> {
> }
> }
>
> It also depend on whoami check by i2c devices. Only A or B will probe.
>
> wm8960 and wm8962 are more complex example. wm8960 is out of life. But
> wm8962 and wm8960 have the same i2c address. The current i2c frame can't
> allow the same i2c address in one i2c bus.
>
> You are feel to NAK my method, but I hope you also provide constructive
> solution to help resolve the problem.
Yes, we resolved it long time ago. Your bootloader can (usually easily)
detect revision of the board and load appropriate DTS or DTS+DTSO.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: omap-mcpdm: Convert to DT schema
From: Krzysztof Kozlowski @ 2024-04-05 18:19 UTC (permalink / raw)
To: Mithil
Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, alsa-devel, devicetree, linux-kernel
In-Reply-To: <CAGzNGR=2-us8GRB3RNi4_24QZ9rNBC7Lx0PFsWwbvxuRKk5ngw@mail.gmail.com>
On 05/04/2024 19:21, Mithil wrote:
> On Fri, Apr 5, 2024 at 10:38 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 05/04/2024 18:29, Mithil wrote:
>>> On Fri, Apr 5, 2024 at 9:27 PM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 05/04/2024 16:48, Mithil wrote:
>>>>> So sorry about the 2nd patch being sent as a new mail, here is a new
>>>>> patch with the changes as suggested
>>>>>
>>>>>> Please use subject prefixes matching the subsystem
>>>>> Changed the patch name to match the folder history.
>>>>
>>>> Nothing improved. What the history tells you?
>>>>
>>>
>>> Referred to "ASoC: dt-bindings: rt1015: Convert to dtschema"
>>> Not really sure what else I should change.
>>
>> But the subject you wrote here is "dt-bindings: omap-mcpdm: Convert to
>> DT schema"?
>>
>> Where is the ASoC?
>>
> I did change it, will send the patch again.
>
>>
>> reg is not correct. Please point me to files doing that way, so I can
>> fix them.
>>
>> You need items with description.
>>
> Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml
> I referred here for the description, but will add items for the 2 regs
I don't see at all the code you are using. It's entirely different!
Where in this file is that type of "reg" property?
>
>>> Interrupts and hwmods use maxItems now.
>>
>> hwmods lost description, why?
> Seems self explanatory.
Really? Not to me. I have no clue what this is. Also, you need
description for (almost) every non-standard, vendor property.
>
>>> Changed nodename to be generic in example as well.
>>
>> "mcpdm" does not feel generic. What is mcpdm? Google finds nothing.
>> Maybe just "pdm"?
>>
> Multichannel PDM Controller. Kept it like that since the node is also
You said you "changed nodename". So from what did you change to what?
> called as mcpdm in the devicetree. Calling it pdm might cause
Poor DTS is not the example...
> confusion.
So far I am confused. Often name of SoC block is specific, not generic.
Anyway, that's not important part, so if you claim mcpdm is generic name
of a class of devices, I am fine.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v5 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY
From: Yang Xiwen @ 2024-04-05 17:53 UTC (permalink / raw)
To: Vinod Koul
Cc: Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jiancheng Xue, Shawn Guo, Philipp Zabel, linux-phy,
devicetree, linux-kernel, Kishon Vijay Abraham I, David Yang
In-Reply-To: <ZhAsXUjifTD6HeKx@matsya>
On 4/6/2024 12:52 AM, Vinod Koul wrote:
> On 05-03-24, 21:32, Yang Xiwen via B4 Relay wrote:
>> From: Yang Xiwen <forbidden405@outlook.com>
> That is quite an email id!
>
>> Direct MMIO resgiter access is used by Hi3798MV200. For other models,
>> of_iomap() returns NULL due to insufficient length. So they are
> so how is that fixed... Pls describe the change...
The commit log will be rewritten in next revision. I'll try to emphasize
the PHY and its configuration interface briefly. Though i don't have
access to the datasheets and TRM so most things can not be verified.
For CV200 and MV100 INNO PHY, the configuration interface is attached to
PERICTRL(Peripheral Control Block). So we just use a register called
PERI_USB3 to configure the PHY. The bus reset, clock are all controlled
in PERI_USB3 register. To read/write to a register of the PHY, a special
sequence of register writes and reads are needed, which was implemented
in this driver.
But for MV200 INNO PHY, the configuration interface is attached directly
to system bus(MMIO). The bus clocks and resets are controlled via Clock
Reset Generator(CRG). Now we have to control them with the help of linux
clk and reset framework because they are provided by other modules.
>> unaffected.
>>
>> Also Hi3798MV200 INNO PHY has an extra reset required to be deasserted,
>> switch to reset_control_array_*() APIs for that.
The commit msg is misleading here. There is no extra reset actually. The
reset also exist for existing users. The initial author just decided to
manage it in the hisi_inno_phy_write_reg() routine(without using
reset_control_* APIs) and omit it in the binding.
> That probably should be a different patch
I guess so. From my point of view, the whole patch is to introduce the
support for Hi3798MV200 variant of the INNO PHY. So i've decided to
squash the two changes into one single commit.
>
>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
>> ---
>> drivers/phy/hisilicon/phy-hisi-inno-usb2.c | 66 ++++++++++++++++++------------
>> 1 file changed, 40 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
>> index b7e740eb4752..df154cd99ed8 100644
>> --- a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
>> +++ b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
>> @@ -10,6 +10,7 @@
>> #include <linux/io.h>
>> #include <linux/module.h>
>> #include <linux/of.h>
>> +#include <linux/of_address.h>
>> #include <linux/phy/phy.h>
>> #include <linux/platform_device.h>
>> #include <linux/reset.h>
>> @@ -24,6 +25,7 @@
>>
>> #define PHY_TYPE_0 0
>> #define PHY_TYPE_1 1
>> +#define PHY_TYPE_MMIO 2
>>
>> #define PHY_TEST_DATA GENMASK(7, 0)
>> #define PHY_TEST_ADDR_OFFSET 8
>> @@ -43,6 +45,7 @@
>> #define PHY_CLK_ENABLE BIT(2)
>>
>> struct hisi_inno_phy_port {
>> + void __iomem *base;
>> struct reset_control *utmi_rst;
>> struct hisi_inno_phy_priv *priv;
>> };
>> @@ -50,7 +53,7 @@ struct hisi_inno_phy_port {
>> struct hisi_inno_phy_priv {
>> void __iomem *mmio;
>> struct clk *ref_clk;
>> - struct reset_control *por_rst;
>> + struct reset_control *rsts;
>> unsigned int type;
>> struct hisi_inno_phy_port ports[INNO_PHY_PORT_NUM];
>> };
>> @@ -62,26 +65,31 @@ static void hisi_inno_phy_write_reg(struct hisi_inno_phy_priv *priv,
>> u32 val;
>> u32 value;
>>
>> - if (priv->type == PHY_TYPE_0)
>> - val = (data & PHY_TEST_DATA) |
>> - ((addr << PHY_TEST_ADDR_OFFSET) & PHY0_TEST_ADDR) |
>> - ((port << PHY0_TEST_PORT_OFFSET) & PHY0_TEST_PORT) |
>> - PHY0_TEST_WREN | PHY0_TEST_RST;
>> - else
>> - val = (data & PHY_TEST_DATA) |
>> - ((addr << PHY_TEST_ADDR_OFFSET) & PHY1_TEST_ADDR) |
>> - ((port << PHY1_TEST_PORT_OFFSET) & PHY1_TEST_PORT) |
>> - PHY1_TEST_WREN | PHY1_TEST_RST;
>> - writel(val, reg);
>> -
>> - value = val;
>> - if (priv->type == PHY_TYPE_0)
>> - value |= PHY0_TEST_CLK;
>> - else
>> - value |= PHY1_TEST_CLK;
>> - writel(value, reg);
>> -
>> - writel(val, reg);
>> + if (priv->ports[port].base)
>> + /* FIXME: fill stride in priv */
> when?
I'm not sure. Maybe until some other users with stride other than 3? I
don't have much knowledge about other SoCs.
Maybe replace the FIXME here with some additional information.
>
>> + writel(data, (u32 *)priv->ports[port].base + addr);
>> + else {
>> + if (priv->type == PHY_TYPE_0)
>> + val = (data & PHY_TEST_DATA) |
>> + ((addr << PHY_TEST_ADDR_OFFSET) & PHY0_TEST_ADDR) |
>> + ((port << PHY0_TEST_PORT_OFFSET) & PHY0_TEST_PORT) |
>> + PHY0_TEST_WREN | PHY0_TEST_RST;
>> + else
>> + val = (data & PHY_TEST_DATA) |
>> + ((addr << PHY_TEST_ADDR_OFFSET) & PHY1_TEST_ADDR) |
>> + ((port << PHY1_TEST_PORT_OFFSET) & PHY1_TEST_PORT) |
>> + PHY1_TEST_WREN | PHY1_TEST_RST;
>> + writel(val, reg);
>> +
>> + value = val;
>> + if (priv->type == PHY_TYPE_0)
>> + value |= PHY0_TEST_CLK;
>> + else
>> + value |= PHY1_TEST_CLK;
>> + writel(value, reg);
>> +
>> + writel(val, reg);
> val and value are very helpful variables, do consider naming them
> better!
I'll consider renaming them in the next revision. Maybe val and val2?
They are just some temp vars to store register values.
>
>> + }
>> }
>>
>> static void hisi_inno_phy_setup(struct hisi_inno_phy_priv *priv)
>> @@ -104,7 +112,7 @@ static int hisi_inno_phy_init(struct phy *phy)
>> return ret;
>> udelay(REF_CLK_STABLE_TIME);
>>
>> - reset_control_deassert(priv->por_rst);
>> + reset_control_deassert(priv->rsts);
>> udelay(POR_RST_COMPLETE_TIME);
>>
>> /* Set up phy registers */
>> @@ -122,7 +130,7 @@ static int hisi_inno_phy_exit(struct phy *phy)
>> struct hisi_inno_phy_priv *priv = port->priv;
>>
>> reset_control_assert(port->utmi_rst);
>> - reset_control_assert(priv->por_rst);
>> + reset_control_assert(priv->rsts);
>> clk_disable_unprepare(priv->ref_clk);
>>
>> return 0;
>> @@ -158,15 +166,16 @@ static int hisi_inno_phy_probe(struct platform_device *pdev)
>> if (IS_ERR(priv->ref_clk))
>> return PTR_ERR(priv->ref_clk);
>>
>> - priv->por_rst = devm_reset_control_get_exclusive(dev, NULL);
>> - if (IS_ERR(priv->por_rst))
>> - return PTR_ERR(priv->por_rst);
>> + priv->rsts = devm_reset_control_array_get_exclusive(dev);
>> + if (IS_ERR(priv->rsts))
>> + return PTR_ERR(priv->rsts);
>>
>> priv->type = (uintptr_t) of_device_get_match_data(dev);
>>
>> for_each_child_of_node(np, child) {
>> struct reset_control *rst;
>> struct phy *phy;
>> + void __iomem *base;
>>
>> rst = of_reset_control_get_exclusive(child, NULL);
>> if (IS_ERR(rst)) {
>> @@ -174,7 +183,10 @@ static int hisi_inno_phy_probe(struct platform_device *pdev)
>> return PTR_ERR(rst);
>> }
>>
>> + base = of_iomap(child, 0);
>> +
>> priv->ports[i].utmi_rst = rst;
>> + priv->ports[i].base = base;
>> priv->ports[i].priv = priv;
>>
>> phy = devm_phy_create(dev, child, &hisi_inno_phy_ops);
>> @@ -205,6 +217,8 @@ static const struct of_device_id hisi_inno_phy_of_match[] = {
>> .data = (void *) PHY_TYPE_0 },
>> { .compatible = "hisilicon,hi3798mv100-usb2-phy",
>> .data = (void *) PHY_TYPE_1 },
>> + { .compatible = "hisilicon,hi3798mv200-usb2-phy",
>> + .data = (void *) PHY_TYPE_MMIO },
>> { },
>> };
>> MODULE_DEVICE_TABLE(of, hisi_inno_phy_of_match);
>>
>> --
>> 2.43.0
--
Regards,
Yang Xiwen
^ permalink raw reply
* Re: [RFC PATCH 0/2] Add Qualcomm PCIe ECAM root complex driver
From: Mayank Rana @ 2024-04-05 17:45 UTC (permalink / raw)
To: Krzysztof Kozlowski, linux-pci, lpieralisi, kw, robh, bhelgaas,
andersson, manivannan.sadhasivam, krzysztof.kozlowski+dt,
conor+dt, devicetree
Cc: linux-arm-msm, quic_ramkri, quic_nkela, quic_shazhuss,
quic_msarkar, quic_nitegupt
In-Reply-To: <ab967c4c-363b-4530-b11e-6de7f3fa0426@linaro.org>
Hi Krzysztof
On 4/4/2024 11:50 PM, Krzysztof Kozlowski wrote:
> On 05/04/2024 01:02, Mayank Rana wrote:
>> Hi Krzysztof
>>
>> On 4/4/2024 12:33 PM, Krzysztof Kozlowski wrote:
>>> On 04/04/2024 21:11, Mayank Rana wrote:
>>>> On some of Qualcomm platform, firmware takes care of system resources
>>>> related to PCIe PHY and controller as well bringing up PCIe link and
>>>> having static iATU configuration for PCIe controller to work into
>>>> ECAM compliant mode. Hence add Qualcomm PCIe ECAM root complex driver.
>>>>
>>>> Tested:
>>>> - Validated NVME functionality with PCIe0 and PCIe1 on SA877p-ride platform
>>>>
>>>
>>> RFC means code is not ready, right? Please get internal review done and
>>> send it when it is ready. I am not sure if you expect any reviews. Some
>>> people send RFC and do not expect reviews. Some expect. I have no clue
>>> and I do not want to waste my time. Please clarify what you expect from
>>> maintainers regarding this contribution.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>> Thanks for initial comments.
>> yes, this is work in progress. There are still more functionalities
>> planned to be added as part of this driver. Although purpose of sending
>> initial change here to get feedback and review comments in terms of
>> usage of generic Qualcomm PCIe ECAM driver, and usage of MSI
>> functionality with it. I missed mentioning this as part of cover letter.
>> So please help to review and provide feedback.
>
> Thanks for explanation. Work in progress as not ready to be merged? Then
> I am sorry, I am not going to provide review of unfinished work. I have
> many more *finished* patches to review first. You can help with these
> too....
>
> Best regards,
> Krzysztof
Ok. I am looking forward to send finished work on this once ready.
Thank you.
Regards,
Mayank
^ permalink raw reply
* Re: [RFC PATCH 2/2] PCI: Add Qualcomm PCIe ECAM root complex driver
From: Mayank Rana @ 2024-04-05 17:41 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: linux-pci, lpieralisi, kw, robh, bhelgaas, andersson,
krzysztof.kozlowski+dt, conor+dt, devicetree, linux-arm-msm,
quic_ramkri, quic_nkela, quic_shazhuss, quic_msarkar,
quic_nitegupt
In-Reply-To: <20240405052918.GA2953@thinkpad>
Hi Mani
On 4/4/2024 10:30 PM, Manivannan Sadhasivam wrote:
> On Thu, Apr 04, 2024 at 12:11:24PM -0700, Mayank Rana wrote:
>> On some of Qualcomm platform, firmware configures PCIe controller into
>> ECAM mode allowing static memory allocation for configuration space of
>> supported bus range. Firmware also takes care of bringing up PCIe PHY
>> and performing required operation to bring PCIe link into D0. Firmware
>> also manages system resources (e.g. clocks/regulators/resets/ bus voting).
>> Hence add Qualcomm PCIe ECAM root complex driver which enumerates PCIe
>> root complex and connected PCIe devices. Firmware won't be enumerating
>> or powering up PCIe root complex until this driver invokes power domain
>> based notification to bring PCIe link into D0/D3cold mode.
>>
>
> Is this an in-house PCIe IP of Qualcomm or the same DWC IP that is used in other
> SoCs?
>
> - Mani
Driver is validated on SA8775p-ride platform using PCIe DWC IP for
now.Although this driver doesn't need to know used PCIe controller and
PHY IP as well programming sequence as that would be taken care by firmware.
>> This driver also support MSI functionality using PCIe controller based
>> MSI controller as GIC ITS based MSI functionality is not available on
>> some of platform.
>>
>> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com>
>> ---
>> drivers/pci/controller/Kconfig | 12 +
>> drivers/pci/controller/Makefile | 1 +
>> drivers/pci/controller/pcie-qcom-ecam.c | 575 ++++++++++++++++++++++++++++++++
>> 3 files changed, 588 insertions(+)
>> create mode 100644 drivers/pci/controller/pcie-qcom-ecam.c
>>
>> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
>> index e534c02..abbd9f2 100644
>> --- a/drivers/pci/controller/Kconfig
>> +++ b/drivers/pci/controller/Kconfig
>> @@ -353,6 +353,18 @@ config PCIE_XILINX_CPM
>> Say 'Y' here if you want kernel support for the
>> Xilinx Versal CPM host bridge.
>>
>> +config PCIE_QCOM_ECAM
>> + tristate "QCOM PCIe ECAM host controller"
>> + depends on ARCH_QCOM && PCI
>> + depends on OF
>> + select PCI_MSI
>> + select PCI_HOST_COMMON
>> + select IRQ_DOMAIN
>> + help
>> + Say 'Y' here if you want to use ECAM shift mode compatible Qualcomm
>> + PCIe root host controller. The controller is programmed using firmware
>> + to support ECAM compatible memory address space.
>> +
>> source "drivers/pci/controller/cadence/Kconfig"
>> source "drivers/pci/controller/dwc/Kconfig"
>> source "drivers/pci/controller/mobiveil/Kconfig"
>> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
>> index f2b19e6..2f1ee1e 100644
>> --- a/drivers/pci/controller/Makefile
>> +++ b/drivers/pci/controller/Makefile
>> @@ -40,6 +40,7 @@ obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
>> obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
>> obj-$(CONFIG_PCIE_APPLE) += pcie-apple.o
>> obj-$(CONFIG_PCIE_MT7621) += pcie-mt7621.o
>> +obj-$(CONFIG_PCIE_QCOM_ECAM) += pcie-qcom-ecam.o
>>
>> # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
>> obj-y += dwc/
>> diff --git a/drivers/pci/controller/pcie-qcom-ecam.c b/drivers/pci/controller/pcie-qcom-ecam.c
>> new file mode 100644
>> index 00000000..5b4c68b
>> --- /dev/null
>> +++ b/drivers/pci/controller/pcie-qcom-ecam.c
>> @@ -0,0 +1,575 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Qualcomm PCIe ECAM root host controller driver
>> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>> +#include <linux/irq.h>
>> +#include <linux/irqchip/chained_irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/msi.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/pci.h>
>> +#include <linux/pci-ecam.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_domain.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/slab.h>
>> +#include <linux/types.h>
>> +
>> +#define PCIE_MSI_CTRL_BASE (0x820)
>> +#define PCIE_MSI_CTRL_SIZE (0x68)
>> +#define PCIE_MSI_CTRL_ADDR_OFFS (0x0)
>> +#define PCIE_MSI_CTRL_UPPER_ADDR_OFFS (0x4)
>> +#define PCIE_MSI_CTRL_INT_N_EN_OFFS(n) (0x8 + 0xc * (n))
>> +#define PCIE_MSI_CTRL_INT_N_MASK_OFFS(n) (0xc + 0xc * (n))
>> +#define PCIE_MSI_CTRL_INT_N_STATUS_OFFS(n) (0x10 + 0xc * (n))
>> +
>> +#define MSI_DB_ADDR 0xa0000000
>> +#define MSI_IRQ_PER_GRP (32)
>> +
>> +/**
>> + * struct qcom_msi_irq - MSI IRQ information
>> + * @client: pointer to MSI client struct
>> + * @grp: group the irq belongs to
>> + * @grp_index: index in group
>> + * @hwirq: hwirq number
>> + * @virq: virq number
>> + * @pos: position in MSI bitmap
>> + */
>> +struct qcom_msi_irq {
>> + struct qcom_msi_client *client;
>> + struct qcom_msi_grp *grp;
>> + unsigned int grp_index;
>> + unsigned int hwirq;
>> + unsigned int virq;
>> + u32 pos;
>> +};
>> +
>> +/**
>> + * struct qcom_msi_grp - MSI group information
>> + * @int_en_reg: memory-mapped interrupt enable register address
>> + * @int_mask_reg: memory-mapped interrupt mask register address
>> + * @int_status_reg: memory-mapped interrupt status register address
>> + * @mask: tracks masked/unmasked MSI
>> + * @irqs: structure to MSI IRQ information
>> + */
>> +struct qcom_msi_grp {
>> + void __iomem *int_en_reg;
>> + void __iomem *int_mask_reg;
>> + void __iomem *int_status_reg;
>> + u32 mask;
>> + struct qcom_msi_irq irqs[MSI_IRQ_PER_GRP];
>> +};
>> +
>> +/**
>> + * struct qcom_msi - PCIe controller based MSI controller information
>> + * @clients: list for tracking clients
>> + * @dev: platform device node
>> + * @nr_hwirqs: total number of hardware IRQs
>> + * @nr_virqs: total number of virqs
>> + * @nr_grps: total number of groups
>> + * @grps: pointer to all groups information
>> + * @bitmap: tracks used/unused MSI
>> + * @mutex: for modifying MSI client list and bitmap
>> + * @inner_domain: parent domain; gen irq related
>> + * @msi_domain: child domain; pcie related
>> + * @msi_db_addr: MSI doorbell address
>> + * @cfg_lock: lock for configuring MSI controller registers
>> + * @pcie_msi_cfg: memory-mapped MSI controller register space
>> + */
>> +struct qcom_msi {
>> + struct list_head clients;
>> + struct device *dev;
>> + int nr_hwirqs;
>> + int nr_virqs;
>> + int nr_grps;
>> + struct qcom_msi_grp *grps;
>> + unsigned long *bitmap;
>> + struct mutex mutex;
>> + struct irq_domain *inner_domain;
>> + struct irq_domain *msi_domain;
>> + phys_addr_t msi_db_addr;
>> + spinlock_t cfg_lock;
>> + void __iomem *pcie_msi_cfg;
>> +};
>> +
>> +/**
>> + * struct qcom_msi_client - structure for each client of MSI controller
>> + * @node: list to track number of MSI clients
>> + * @msi: client specific MSI controller based resource pointer
>> + * @dev: client's dev of pci_dev
>> + * @nr_irqs: number of irqs allocated for client
>> + * @msi_addr: MSI doorbell address
>> + */
>> +struct qcom_msi_client {
>> + struct list_head node;
>> + struct qcom_msi *msi;
>> + struct device *dev;
>> + unsigned int nr_irqs;
>> + phys_addr_t msi_addr;
>> +};
>> +
>> +static void qcom_msi_handler(struct irq_desc *desc)
>> +{
>> + struct irq_chip *chip = irq_desc_get_chip(desc);
>> + struct qcom_msi_grp *msi_grp;
>> + u32 status;
>> + int i;
>> +
>> + chained_irq_enter(chip, desc);
>> +
>> + msi_grp = irq_desc_get_handler_data(desc);
>> + status = readl_relaxed(msi_grp->int_status_reg);
>> + status ^= (msi_grp->mask & status);
>> + writel(status, msi_grp->int_status_reg);
>> +
>> + for (i = 0; status; i++, status >>= 1)
>> + if (status & 0x1)
>> + generic_handle_irq(msi_grp->irqs[i].virq);
>> +
>> + chained_irq_exit(chip, desc);
>> +}
>> +
>> +static void qcom_msi_mask_irq(struct irq_data *data)
>> +{
>> + struct irq_data *parent_data;
>> + struct qcom_msi_irq *msi_irq;
>> + struct qcom_msi_grp *msi_grp;
>> + struct qcom_msi *msi;
>> + unsigned long flags;
>> +
>> + parent_data = data->parent_data;
>> + if (!parent_data)
>> + return;
>> +
>> + msi_irq = irq_data_get_irq_chip_data(parent_data);
>> + msi = msi_irq->client->msi;
>> + msi_grp = msi_irq->grp;
>> +
>> + spin_lock_irqsave(&msi->cfg_lock, flags);
>> + pci_msi_mask_irq(data);
>> + msi_grp->mask |= BIT(msi_irq->grp_index);
>> + writel(msi_grp->mask, msi_grp->int_mask_reg);
>> + spin_unlock_irqrestore(&msi->cfg_lock, flags);
>> +}
>> +
>> +static void qcom_msi_unmask_irq(struct irq_data *data)
>> +{
>> + struct irq_data *parent_data;
>> + struct qcom_msi_irq *msi_irq;
>> + struct qcom_msi_grp *msi_grp;
>> + struct qcom_msi *msi;
>> + unsigned long flags;
>> +
>> + parent_data = data->parent_data;
>> + if (!parent_data)
>> + return;
>> +
>> + msi_irq = irq_data_get_irq_chip_data(parent_data);
>> + msi = msi_irq->client->msi;
>> + msi_grp = msi_irq->grp;
>> +
>> + spin_lock_irqsave(&msi->cfg_lock, flags);
>> + msi_grp->mask &= ~BIT(msi_irq->grp_index);
>> + writel(msi_grp->mask, msi_grp->int_mask_reg);
>> + pci_msi_unmask_irq(data);
>> + spin_unlock_irqrestore(&msi->cfg_lock, flags);
>> +}
>> +
>> +static struct irq_chip qcom_msi_irq_chip = {
>> + .name = "qcom_pci_msi",
>> + .irq_enable = qcom_msi_unmask_irq,
>> + .irq_disable = qcom_msi_mask_irq,
>> + .irq_mask = qcom_msi_mask_irq,
>> + .irq_unmask = qcom_msi_unmask_irq,
>> +};
>> +
>> +static int qcom_msi_domain_prepare(struct irq_domain *domain, struct device *dev,
>> + int nvec, msi_alloc_info_t *arg)
>> +{
>> + struct qcom_msi *msi = domain->parent->host_data;
>> + struct qcom_msi_client *client;
>> +
>> + client = kzalloc(sizeof(*client), GFP_KERNEL);
>> + if (!client)
>> + return -ENOMEM;
>> +
>> + client->msi = msi;
>> + client->dev = dev;
>> + client->msi_addr = msi->msi_db_addr;
>> + mutex_lock(&msi->mutex);
>> + list_add_tail(&client->node, &msi->clients);
>> + mutex_unlock(&msi->mutex);
>> +
>> + /* zero out struct for pcie msi framework */
>> + memset(arg, 0, sizeof(*arg));
>> + return 0;
>> +}
>> +
>> +static struct msi_domain_ops qcom_msi_domain_ops = {
>> + .msi_prepare = qcom_msi_domain_prepare,
>> +};
>> +
>> +static struct msi_domain_info qcom_msi_domain_info = {
>> + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
>> + MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX),
>> + .ops = &qcom_msi_domain_ops,
>> + .chip = &qcom_msi_irq_chip,
>> +};
>> +
>> +static int qcom_msi_irq_set_affinity(struct irq_data *data,
>> + const struct cpumask *mask, bool force)
>> +{
>> + struct irq_data *parent_data = irq_get_irq_data(irqd_to_hwirq(data));
>> + int ret = 0;
>> +
>> + if (!parent_data)
>> + return -ENODEV;
>> +
>> + /* set affinity for MSI HW IRQ */
>> + if (parent_data->chip->irq_set_affinity)
>> + ret = parent_data->chip->irq_set_affinity(parent_data, mask, force);
>> +
>> + return ret;
>> +}
>> +
>> +static void qcom_msi_irq_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>> +{
>> + struct irq_data *parent_data = irq_get_irq_data(irqd_to_hwirq(data));
>> + struct qcom_msi_irq *msi_irq = irq_data_get_irq_chip_data(data);
>> + struct qcom_msi_client *client = msi_irq->client;
>> +
>> + if (!parent_data)
>> + return;
>> +
>> + msg->address_lo = lower_32_bits(client->msi_addr);
>> + msg->address_hi = upper_32_bits(client->msi_addr);
>> + msg->data = msi_irq->pos;
>> +}
>> +
>> +static struct irq_chip qcom_msi_bottom_irq_chip = {
>> + .name = "qcom_msi",
>> + .irq_set_affinity = qcom_msi_irq_set_affinity,
>> + .irq_compose_msi_msg = qcom_msi_irq_compose_msi_msg,
>> +};
>> +
>> +static int qcom_msi_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
>> + unsigned int nr_irqs, void *args)
>> +{
>> + struct device *dev = ((msi_alloc_info_t *)args)->desc->dev;
>> + struct qcom_msi_client *tmp, *client = NULL;
>> + struct qcom_msi *msi = domain->host_data;
>> + int i, ret = 0;
>> + int pos;
>> +
>> + mutex_lock(&msi->mutex);
>> + list_for_each_entry(tmp, &msi->clients, node) {
>> + if (tmp->dev == dev) {
>> + client = tmp;
>> + break;
>> + }
>> + }
>> +
>> + if (!client) {
>> + dev_err(msi->dev, "failed to find MSI client dev\n");
>> + ret = -ENODEV;
>> + goto out;
>> + }
>> +
>> + pos = bitmap_find_next_zero_area(msi->bitmap, msi->nr_virqs, 0,
>> + nr_irqs, nr_irqs - 1);
>> + if (pos > msi->nr_virqs) {
>> + ret = -ENOSPC;
>> + goto out;
>> + }
>> +
>> + bitmap_set(msi->bitmap, pos, nr_irqs);
>> + for (i = 0; i < nr_irqs; i++) {
>> + u32 grp = pos / MSI_IRQ_PER_GRP;
>> + u32 index = pos % MSI_IRQ_PER_GRP;
>> + struct qcom_msi_irq *msi_irq = &msi->grps[grp].irqs[index];
>> +
>> + msi_irq->virq = virq + i;
>> + msi_irq->client = client;
>> + irq_domain_set_info(domain, msi_irq->virq,
>> + msi_irq->hwirq,
>> + &qcom_msi_bottom_irq_chip, msi_irq,
>> + handle_simple_irq, NULL, NULL);
>> + client->nr_irqs++;
>> + pos++;
>> + }
>> +out:
>> + mutex_unlock(&msi->mutex);
>> + return ret;
>> +}
>> +
>> +static void qcom_msi_irq_domain_free(struct irq_domain *domain, unsigned int virq,
>> + unsigned int nr_irqs)
>> +{
>> + struct irq_data *data = irq_domain_get_irq_data(domain, virq);
>> + struct qcom_msi_client *client;
>> + struct qcom_msi_irq *msi_irq;
>> + struct qcom_msi *msi;
>> +
>> + if (!data)
>> + return;
>> +
>> + msi_irq = irq_data_get_irq_chip_data(data);
>> + client = msi_irq->client;
>> + msi = client->msi;
>> +
>> + mutex_lock(&msi->mutex);
>> + bitmap_clear(msi->bitmap, msi_irq->pos, nr_irqs);
>> +
>> + client->nr_irqs -= nr_irqs;
>> + if (!client->nr_irqs) {
>> + list_del(&client->node);
>> + kfree(client);
>> + }
>> + mutex_unlock(&msi->mutex);
>> +
>> + irq_domain_free_irqs_parent(domain, virq, nr_irqs);
>> +}
>> +
>> +static const struct irq_domain_ops msi_domain_ops = {
>> + .alloc = qcom_msi_irq_domain_alloc,
>> + .free = qcom_msi_irq_domain_free,
>> +};
>> +
>> +static int qcom_msi_alloc_domains(struct qcom_msi *msi)
>> +{
>> + msi->inner_domain = irq_domain_add_linear(NULL, msi->nr_virqs,
>> + &msi_domain_ops, msi);
>> + if (!msi->inner_domain) {
>> + dev_err(msi->dev, "failed to create IRQ inner domain\n");
>> + return -ENOMEM;
>> + }
>> +
>> + msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->dev->of_node),
>> + &qcom_msi_domain_info, msi->inner_domain);
>> + if (!msi->msi_domain) {
>> + dev_err(msi->dev, "failed to create MSI domain\n");
>> + irq_domain_remove(msi->inner_domain);
>> + return -ENOMEM;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int qcom_msi_irq_setup(struct qcom_msi *msi)
>> +{
>> + struct qcom_msi_grp *msi_grp;
>> + struct qcom_msi_irq *msi_irq;
>> + int i, index, ret;
>> + unsigned int irq;
>> +
>> + /* setup each MSI group. nr_hwirqs == nr_grps */
>> + for (i = 0; i < msi->nr_hwirqs; i++) {
>> + irq = irq_of_parse_and_map(msi->dev->of_node, i);
>> + if (!irq) {
>> + dev_err(msi->dev,
>> + "MSI: failed to parse/map interrupt\n");
>> + ret = -ENODEV;
>> + goto free_irqs;
>> + }
>> +
>> + msi_grp = &msi->grps[i];
>> + msi_grp->int_en_reg = msi->pcie_msi_cfg +
>> + PCIE_MSI_CTRL_INT_N_EN_OFFS(i);
>> + msi_grp->int_mask_reg = msi->pcie_msi_cfg +
>> + PCIE_MSI_CTRL_INT_N_MASK_OFFS(i);
>> + msi_grp->int_status_reg = msi->pcie_msi_cfg +
>> + PCIE_MSI_CTRL_INT_N_STATUS_OFFS(i);
>> +
>> + for (index = 0; index < MSI_IRQ_PER_GRP; index++) {
>> + msi_irq = &msi_grp->irqs[index];
>> +
>> + msi_irq->grp = msi_grp;
>> + msi_irq->grp_index = index;
>> + msi_irq->pos = (i * MSI_IRQ_PER_GRP) + index;
>> + msi_irq->hwirq = irq;
>> + }
>> +
>> + irq_set_chained_handler_and_data(irq, qcom_msi_handler, msi_grp);
>> + }
>> +
>> + return 0;
>> +
>> +free_irqs:
>> + for (--i; i >= 0; i--) {
>> + irq = msi->grps[i].irqs[0].hwirq;
>> +
>> + irq_set_chained_handler_and_data(irq, NULL, NULL);
>> + irq_dispose_mapping(irq);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static void qcom_msi_config(struct irq_domain *domain)
>> +{
>> + struct qcom_msi *msi;
>> + int i;
>> +
>> + msi = domain->parent->host_data;
>> +
>> + /* program termination address */
>> + writel(msi->msi_db_addr, msi->pcie_msi_cfg + PCIE_MSI_CTRL_ADDR_OFFS);
>> + writel(0, msi->pcie_msi_cfg + PCIE_MSI_CTRL_UPPER_ADDR_OFFS);
>> +
>> + /* restore mask and enable all interrupts for each group */
>> + for (i = 0; i < msi->nr_grps; i++) {
>> + struct qcom_msi_grp *msi_grp = &msi->grps[i];
>> +
>> + writel(msi_grp->mask, msi_grp->int_mask_reg);
>> + writel(~0, msi_grp->int_en_reg);
>> + }
>> +}
>> +
>> +static void qcom_msi_deinit(struct qcom_msi *msi)
>> +{
>> + irq_domain_remove(msi->msi_domain);
>> + irq_domain_remove(msi->inner_domain);
>> +}
>> +
>> +static struct qcom_msi *qcom_msi_init(struct device *dev)
>> +{
>> + struct qcom_msi *msi;
>> + u64 addr;
>> + int ret;
>> +
>> + msi = devm_kzalloc(dev, sizeof(*msi), GFP_KERNEL);
>> + if (!msi)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + msi->dev = dev;
>> + mutex_init(&msi->mutex);
>> + spin_lock_init(&msi->cfg_lock);
>> + INIT_LIST_HEAD(&msi->clients);
>> +
>> + msi->msi_db_addr = MSI_DB_ADDR;
>> + msi->nr_hwirqs = of_irq_count(dev->of_node);
>> + if (!msi->nr_hwirqs) {
>> + dev_err(msi->dev, "no hwirqs found\n");
>> + return ERR_PTR(-ENODEV);
>> + }
>> +
>> + if (of_property_read_reg(dev->of_node, 0, &addr, NULL) < 0) {
>> + dev_err(msi->dev, "failed to get reg address\n");
>> + return ERR_PTR(-ENODEV);
>> + }
>> +
>> + dev_dbg(msi->dev, "hwirq:%d pcie_msi_cfg:%llx\n", msi->nr_hwirqs, addr);
>> + msi->pcie_msi_cfg = devm_ioremap(dev, addr + PCIE_MSI_CTRL_BASE, PCIE_MSI_CTRL_SIZE);
>> + if (!msi->pcie_msi_cfg)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + msi->nr_virqs = msi->nr_hwirqs * MSI_IRQ_PER_GRP;
>> + msi->nr_grps = msi->nr_hwirqs;
>> + msi->grps = devm_kcalloc(dev, msi->nr_grps, sizeof(*msi->grps), GFP_KERNEL);
>> + if (!msi->grps)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + msi->bitmap = devm_kcalloc(dev, BITS_TO_LONGS(msi->nr_virqs),
>> + sizeof(*msi->bitmap), GFP_KERNEL);
>> + if (!msi->bitmap)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + ret = qcom_msi_alloc_domains(msi);
>> + if (ret)
>> + return ERR_PTR(ret);
>> +
>> + ret = qcom_msi_irq_setup(msi);
>> + if (ret) {
>> + qcom_msi_deinit(msi);
>> + return ERR_PTR(ret);
>> + }
>> +
>> + qcom_msi_config(msi->msi_domain);
>> + return msi;
>> +}
>> +
>> +static int qcom_pcie_ecam_suspend_noirq(struct device *dev)
>> +{
>> + return pm_runtime_put_sync(dev);
>> +}
>> +
>> +static int qcom_pcie_ecam_resume_noirq(struct device *dev)
>> +{
>> + return pm_runtime_get_sync(dev);
>> +}
>> +
>> +static int qcom_pcie_ecam_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct qcom_msi *msi;
>> + int ret;
>> +
>> + ret = devm_pm_runtime_enable(dev);
>> + if (ret)
>> + return ret;
>> +
>> + ret = pm_runtime_resume_and_get(dev);
>> + if (ret < 0) {
>> + dev_err(dev, "fail to enable pcie controller: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + msi = qcom_msi_init(dev);
>> + if (IS_ERR(msi)) {
>> + pm_runtime_put_sync(dev);
>> + return PTR_ERR(msi);
>> + }
>> +
>> + ret = pci_host_common_probe(pdev);
>> + if (ret) {
>> + dev_err(dev, "pci_host_common_probe() failed:%d\n", ret);
>> + qcom_msi_deinit(msi);
>> + pm_runtime_put_sync(dev);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static const struct dev_pm_ops qcom_pcie_ecam_pm_ops = {
>> + NOIRQ_SYSTEM_SLEEP_PM_OPS(qcom_pcie_ecam_suspend_noirq,
>> + qcom_pcie_ecam_resume_noirq)
>> +};
>> +
>> +static const struct pci_ecam_ops qcom_pcie_ecam_ops = {
>> + .pci_ops = {
>> + .map_bus = pci_ecam_map_bus,
>> + .read = pci_generic_config_read,
>> + .write = pci_generic_config_write,
>> + }
>> +};
>> +
>> +static const struct of_device_id qcom_pcie_ecam_of_match[] = {
>> + {
>> + .compatible = "qcom,pcie-ecam-rc",
>> + .data = &qcom_pcie_ecam_ops,
>> + },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, qcom_pcie_ecam_of_match);
>> +
>> +static struct platform_driver qcom_pcie_ecam_driver = {
>> + .probe = qcom_pcie_ecam_probe,
>> + .driver = {
>> + .name = "qcom-pcie-ecam-rc",
>> + .suppress_bind_attrs = true,
>> + .of_match_table = qcom_pcie_ecam_of_match,
>> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
>> + .pm = &qcom_pcie_ecam_pm_ops,
>> + },
>> +};
>> +module_platform_driver(qcom_pcie_ecam_driver);
>> +
>> +MODULE_DESCRIPTION("Qualcomm PCIe ECAM root complex driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.7.4
>>
>
^ permalink raw reply
* Re: [PATCH 0/5] Add parsing for Zimop ISA extension
From: Deepak Gupta @ 2024-04-05 17:33 UTC (permalink / raw)
To: Andrew Jones
Cc: Clément Léger, Jonathan Corbet, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Conor Dooley, Rob Herring,
Krzysztof Kozlowski, Anup Patel, Shuah Khan, Atish Patra,
linux-doc, linux-riscv, linux-kernel, devicetree, kvm, kvm-riscv,
linux-kselftest
In-Reply-To: <20240405-091c6c174f023d74b434059d@orel>
On Fri, Apr 5, 2024 at 8:26 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote:
> > The Zimop ISA extension was ratified recently. This series adds support
> > for parsing it from riscv,isa, hwprobe export and kvm support for
> > Guest/VM.
>
> I'm not sure we need this. Zimop by itself isn't useful, so I don't know
> if we need to advertise it at all. When an extension comes along that
> redefines some MOPs, then we'll advertise that extension, but the fact
> Zimop is used for that extension is really just an implementation detail.
Only situation I see this can be useful is this:--
An implementer, implemented Zimops in CPU solely for the purpose that they can
run mainline distro & packages on their hardware and don't want to leverage any
feature which are built on top of Zimop.
As an example zicfilp and zicfiss are dependent on zimops. glibc can
do following
1) check elf header if binary was compiled with zicfiss and zicfilp,
if yes goto step 2, else goto step 6.
2) check if zicfiss/zicfilp is available in hw via hwprobe, if yes
goto step 5. else goto step 3
3) check if zimop is available via hwprobe, if yes goto step 6, else goto step 4
4) This binary won't be able to run successfully on this platform,
issue exit syscall. <-- termination
5) issue prctl to enable shadow stack and landing pad for current task
<-- enable feature
6) let the binary run <-- let the binary run because no harm can be done
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: omap-mcpdm: Convert to DT schema
From: Mithil @ 2024-04-05 17:21 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, alsa-devel, devicetree, linux-kernel
In-Reply-To: <c9084453-65f1-43b0-88df-5b73052ccb72@linaro.org>
On Fri, Apr 5, 2024 at 10:38 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 05/04/2024 18:29, Mithil wrote:
> > On Fri, Apr 5, 2024 at 9:27 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 05/04/2024 16:48, Mithil wrote:
> >>> So sorry about the 2nd patch being sent as a new mail, here is a new
> >>> patch with the changes as suggested
> >>>
> >>>> Please use subject prefixes matching the subsystem
> >>> Changed the patch name to match the folder history.
> >>
> >> Nothing improved. What the history tells you?
> >>
> >
> > Referred to "ASoC: dt-bindings: rt1015: Convert to dtschema"
> > Not really sure what else I should change.
>
> But the subject you wrote here is "dt-bindings: omap-mcpdm: Convert to
> DT schema"?
>
> Where is the ASoC?
>
I did change it, will send the patch again.
>
> reg is not correct. Please point me to files doing that way, so I can
> fix them.
>
> You need items with description.
>
Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml
I referred here for the description, but will add items for the 2 regs
> > Interrupts and hwmods use maxItems now.
>
> hwmods lost description, why?
Seems self explanatory.
> > Changed nodename to be generic in example as well.
>
> "mcpdm" does not feel generic. What is mcpdm? Google finds nothing.
> Maybe just "pdm"?
>
Multichannel PDM Controller. Kept it like that since the node is also
called as mcpdm in the devicetree. Calling it pdm might cause
confusion.
Best Regards,
Mithil
^ permalink raw reply
* Re: [PATCH v2 2/2] Input: edt-ft5x06 - add ft5426
From: Andy Shevchenko @ 2024-04-05 17:21 UTC (permalink / raw)
To: Andreas Kemnade
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
o.rempel, u.kleine-koenig, hdegoede, ye.xingchen, p.puschmann,
linux-input, devicetree, linux-kernel, caleb.connolly
In-Reply-To: <20240405182832.4e457695@aktux>
On Fri, Apr 5, 2024 at 7:28 PM Andreas Kemnade <andreas@kemnade.info> wrote:
> On Fri, 5 Apr 2024 18:13:45 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Fri, Apr 5, 2024 at 1:20 AM Andreas Kemnade <andreas@kemnade.info> wrote:
...
> > > @@ -1484,6 +1484,7 @@ static const struct of_device_id edt_ft5x06_of_match[] = {
> > > { .compatible = "edt,edt-ft5206", .data = &edt_ft5x06_data },
> > > { .compatible = "edt,edt-ft5306", .data = &edt_ft5x06_data },
> > > { .compatible = "edt,edt-ft5406", .data = &edt_ft5x06_data },
> > > + { .compatible = "focaltech,ft5426", .data = &edt_ft5506_data },
> >
> > Why a different vendor prefix?
> > In case you need to use this one, keep the list sorted, currently this
> > splits the edt,* ones.
> >
> How do I know whether to use evervision or edt instead?
Ask DT people, the vendor-prefixes lists both...
> I sorted by the numbers. Looking at datasheets for other controllers I see
> https://www.displayfuture.com/Display/datasheet/controller/FT5x06.pdf
> it only mentions FocalTech Systems Co., Ltd.
But does the driver use that? AFAICS it uses edt. Perhaps it's due to
a business split, not to my knowledge anyway.
> So how the vendor prefixes are derived?
Rob, Krzysztof?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: (subset) [PATCH v2 0/7] arm64: qcom-sm8[456]50: properly describe the PCIe Gen4x2 PHY AUX clock
From: Vinod Koul @ 2024-04-05 17:09 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Kishon Vijay Abraham I,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel,
Krzysztof Kozlowski, Dmitry Baryshkov
In-Reply-To: <20240322-topic-sm8x50-upstream-pcie-1-phy-aux-clk-v2-0-3ec0a966d52f@linaro.org>
On Fri, 22 Mar 2024 10:42:37 +0100, Neil Armstrong wrote:
> The PCIe Gen4x2 PHY found in the SM8[456]50 SoCs have a second clock named
> "PHY_AUX_CLK" which is an input of the Global Clock Controller (GCC) which
> is muxed & gated then returned to the PHY as an input.
>
> Document the clock IDs to select the PIPE clock or the AUX clock,
> also enforce a second clock-output-names and a #clock-cells value of 1
> for the PCIe Gen4x2 PHY found in the SM8[456]50 SoCs.
>
> [...]
Applied, thanks!
[1/7] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: document PHY AUX clock on SM8[456]50 SoCs
commit: 72bea132f3680ee51e7ed2cee62892b6f5121909
[2/7] phy: qcom: qmp-pcie: refactor clock register code
commit: 677b45114b4430a43d2602296617efc4d3f2ab7a
[3/7] phy: qcom: qmp-pcie: register second optional PHY AUX clock
commit: 583ca9ccfa806605ae1391aafa3f78a8a2cc0b48
[4/7] phy: qcom: qmp-pcie: register PHY AUX clock for SM8[456]50 4x2 PCIe PHY
commit: 5cee04a8369049b92d52995e320abff18dfeda44
Best regards,
--
~Vinod
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: omap-mcpdm: Convert to DT schema
From: Krzysztof Kozlowski @ 2024-04-05 17:08 UTC (permalink / raw)
To: Mithil
Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, alsa-devel, devicetree, linux-kernel
In-Reply-To: <CAGzNGRktm5gMj=bhtX2RAzcn1v5ref+nV-HV3Fct56FzAzxjWA@mail.gmail.com>
On 05/04/2024 18:29, Mithil wrote:
> On Fri, Apr 5, 2024 at 9:27 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 05/04/2024 16:48, Mithil wrote:
>>> So sorry about the 2nd patch being sent as a new mail, here is a new
>>> patch with the changes as suggested
>>>
>>>> Please use subject prefixes matching the subsystem
>>> Changed the patch name to match the folder history.
>>
>> Nothing improved. What the history tells you?
>>
>
> Referred to "ASoC: dt-bindings: rt1015: Convert to dtschema"
> Not really sure what else I should change.
But the subject you wrote here is "dt-bindings: omap-mcpdm: Convert to
DT schema"?
Where is the ASoC?
>
>>>
>>>> Is it your full name?
>>> Fixed it, my apologies.
>>>
>>>> Filename like compatible.
>>> Fixed.
>>
>> Still not, compatible is omap4.
>>
>
> Sorry, seems like I was sending the old file again.
> Will fix this.
>
>>>
>>>> Please open existing bindings and look how it is done there.
>>> Changed it, is it fine now?
>>
>> You mean v2? I have no clue to what you are responding here, but no, v2
>> did not improve much.
>>
>
> Again, could you guide me to what needs to be done?
> Description for reg should be fine as this is how it is done in other
> files as well.
reg is not correct. Please point me to files doing that way, so I can
fix them.
You need items with description.
> Interrupts and hwmods use maxItems now.
hwmods lost description, why?
> Changed nodename to be generic in example as well.
"mcpdm" does not feel generic. What is mcpdm? Google finds nothing.
Maybe just "pdm"?
Anyway, this patch has exactly the same name as v1, so what did you
improve? Your v2 is almost the same as v1.
> Those were the suggested changes previously.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v5 5/5] phy: hisilicon: hisi-inno-phy: add support for Hi3798MV200 INNO PHY
From: Vinod Koul @ 2024-04-05 16:52 UTC (permalink / raw)
To: forbidden405
Cc: Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jiancheng Xue, Shawn Guo, Philipp Zabel, linux-phy,
devicetree, linux-kernel, Kishon Vijay Abraham I, David Yang
In-Reply-To: <20240305-inno-phy-v5-5-dc1cb130ea08@outlook.com>
On 05-03-24, 21:32, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen <forbidden405@outlook.com>
That is quite an email id!
>
> Direct MMIO resgiter access is used by Hi3798MV200. For other models,
> of_iomap() returns NULL due to insufficient length. So they are
so how is that fixed... Pls describe the change...
> unaffected.
>
> Also Hi3798MV200 INNO PHY has an extra reset required to be deasserted,
> switch to reset_control_array_*() APIs for that.
That probably should be a different patch
>
> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
> ---
> drivers/phy/hisilicon/phy-hisi-inno-usb2.c | 66 ++++++++++++++++++------------
> 1 file changed, 40 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
> index b7e740eb4752..df154cd99ed8 100644
> --- a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
> +++ b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
> @@ -10,6 +10,7 @@
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_address.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> #include <linux/reset.h>
> @@ -24,6 +25,7 @@
>
> #define PHY_TYPE_0 0
> #define PHY_TYPE_1 1
> +#define PHY_TYPE_MMIO 2
>
> #define PHY_TEST_DATA GENMASK(7, 0)
> #define PHY_TEST_ADDR_OFFSET 8
> @@ -43,6 +45,7 @@
> #define PHY_CLK_ENABLE BIT(2)
>
> struct hisi_inno_phy_port {
> + void __iomem *base;
> struct reset_control *utmi_rst;
> struct hisi_inno_phy_priv *priv;
> };
> @@ -50,7 +53,7 @@ struct hisi_inno_phy_port {
> struct hisi_inno_phy_priv {
> void __iomem *mmio;
> struct clk *ref_clk;
> - struct reset_control *por_rst;
> + struct reset_control *rsts;
> unsigned int type;
> struct hisi_inno_phy_port ports[INNO_PHY_PORT_NUM];
> };
> @@ -62,26 +65,31 @@ static void hisi_inno_phy_write_reg(struct hisi_inno_phy_priv *priv,
> u32 val;
> u32 value;
>
> - if (priv->type == PHY_TYPE_0)
> - val = (data & PHY_TEST_DATA) |
> - ((addr << PHY_TEST_ADDR_OFFSET) & PHY0_TEST_ADDR) |
> - ((port << PHY0_TEST_PORT_OFFSET) & PHY0_TEST_PORT) |
> - PHY0_TEST_WREN | PHY0_TEST_RST;
> - else
> - val = (data & PHY_TEST_DATA) |
> - ((addr << PHY_TEST_ADDR_OFFSET) & PHY1_TEST_ADDR) |
> - ((port << PHY1_TEST_PORT_OFFSET) & PHY1_TEST_PORT) |
> - PHY1_TEST_WREN | PHY1_TEST_RST;
> - writel(val, reg);
> -
> - value = val;
> - if (priv->type == PHY_TYPE_0)
> - value |= PHY0_TEST_CLK;
> - else
> - value |= PHY1_TEST_CLK;
> - writel(value, reg);
> -
> - writel(val, reg);
> + if (priv->ports[port].base)
> + /* FIXME: fill stride in priv */
when?
> + writel(data, (u32 *)priv->ports[port].base + addr);
> + else {
> + if (priv->type == PHY_TYPE_0)
> + val = (data & PHY_TEST_DATA) |
> + ((addr << PHY_TEST_ADDR_OFFSET) & PHY0_TEST_ADDR) |
> + ((port << PHY0_TEST_PORT_OFFSET) & PHY0_TEST_PORT) |
> + PHY0_TEST_WREN | PHY0_TEST_RST;
> + else
> + val = (data & PHY_TEST_DATA) |
> + ((addr << PHY_TEST_ADDR_OFFSET) & PHY1_TEST_ADDR) |
> + ((port << PHY1_TEST_PORT_OFFSET) & PHY1_TEST_PORT) |
> + PHY1_TEST_WREN | PHY1_TEST_RST;
> + writel(val, reg);
> +
> + value = val;
> + if (priv->type == PHY_TYPE_0)
> + value |= PHY0_TEST_CLK;
> + else
> + value |= PHY1_TEST_CLK;
> + writel(value, reg);
> +
> + writel(val, reg);
val and value are very helpful variables, do consider naming them
better!
> + }
> }
>
> static void hisi_inno_phy_setup(struct hisi_inno_phy_priv *priv)
> @@ -104,7 +112,7 @@ static int hisi_inno_phy_init(struct phy *phy)
> return ret;
> udelay(REF_CLK_STABLE_TIME);
>
> - reset_control_deassert(priv->por_rst);
> + reset_control_deassert(priv->rsts);
> udelay(POR_RST_COMPLETE_TIME);
>
> /* Set up phy registers */
> @@ -122,7 +130,7 @@ static int hisi_inno_phy_exit(struct phy *phy)
> struct hisi_inno_phy_priv *priv = port->priv;
>
> reset_control_assert(port->utmi_rst);
> - reset_control_assert(priv->por_rst);
> + reset_control_assert(priv->rsts);
> clk_disable_unprepare(priv->ref_clk);
>
> return 0;
> @@ -158,15 +166,16 @@ static int hisi_inno_phy_probe(struct platform_device *pdev)
> if (IS_ERR(priv->ref_clk))
> return PTR_ERR(priv->ref_clk);
>
> - priv->por_rst = devm_reset_control_get_exclusive(dev, NULL);
> - if (IS_ERR(priv->por_rst))
> - return PTR_ERR(priv->por_rst);
> + priv->rsts = devm_reset_control_array_get_exclusive(dev);
> + if (IS_ERR(priv->rsts))
> + return PTR_ERR(priv->rsts);
>
> priv->type = (uintptr_t) of_device_get_match_data(dev);
>
> for_each_child_of_node(np, child) {
> struct reset_control *rst;
> struct phy *phy;
> + void __iomem *base;
>
> rst = of_reset_control_get_exclusive(child, NULL);
> if (IS_ERR(rst)) {
> @@ -174,7 +183,10 @@ static int hisi_inno_phy_probe(struct platform_device *pdev)
> return PTR_ERR(rst);
> }
>
> + base = of_iomap(child, 0);
> +
> priv->ports[i].utmi_rst = rst;
> + priv->ports[i].base = base;
> priv->ports[i].priv = priv;
>
> phy = devm_phy_create(dev, child, &hisi_inno_phy_ops);
> @@ -205,6 +217,8 @@ static const struct of_device_id hisi_inno_phy_of_match[] = {
> .data = (void *) PHY_TYPE_0 },
> { .compatible = "hisilicon,hi3798mv100-usb2-phy",
> .data = (void *) PHY_TYPE_1 },
> + { .compatible = "hisilicon,hi3798mv200-usb2-phy",
> + .data = (void *) PHY_TYPE_MMIO },
> { },
> };
> MODULE_DEVICE_TABLE(of, hisi_inno_phy_of_match);
>
> --
> 2.43.0
--
~Vinod
^ permalink raw reply
* Re: [PATCH v1 4/4] arm64: dts: freescale: imx8mm-verdin-dahlia: support sleep-moci
From: Francesco Dolcini @ 2024-04-05 16:48 UTC (permalink / raw)
To: Stefan Eichenberger
Cc: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, francesco.dolcini, devicetree, imx, linux-arm-kernel,
linux-kernel, Stefan Eichenberger
In-Reply-To: <20240405160720.5977-5-eichest@gmail.com>
On Fri, Apr 05, 2024 at 06:07:20PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> Previously, we had the sleep-moci pin set to always on. However, the
> Dahlia carrier board supports disabling the sleep-moci when the system
> is suspended to power down peripherals that support it. This reduces
> overall power consumption. This commit adds support for this feature by
> disabling the reg_force_sleep_moci regulator and adding two new
> regulators for the USB hub and PCIe that can be turned off when the
> system is suspended.
>
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
^ permalink raw reply
* Re: [PATCH v1 3/4] arm64: dts: freescale: imx8mm-verdin: replace sleep-moci hog with regulator
From: Francesco Dolcini @ 2024-04-05 16:48 UTC (permalink / raw)
To: Stefan Eichenberger
Cc: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, francesco.dolcini, devicetree, imx, linux-arm-kernel,
linux-kernel, Stefan Eichenberger
In-Reply-To: <20240405160720.5977-4-eichest@gmail.com>
On Fri, Apr 05, 2024 at 06:07:19PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> The Verdin family has a signal called sleep-moci which can be used to
> turn off peripherals on the carrier board when the SoM goes into
> suspend. So far we have hogged this signal, which means the peripherals
> are always on and it is not possible to add peripherals that depend on
> the sleep-moci to be on. With this change, we replace the hog with a
> regulator so that peripherals can add their own regulators that use the
> same gpio. Carrier boards that allow peripherals to be powered off in
> suspend can disable this regulator and implement their own regulator to
> control the sleep-moci.
>
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
^ permalink raw reply
* Re: [PATCH v1 2/4] arm64: dts: freescale: imx8mp-verdin-dahlia: support sleep-moci
From: Francesco Dolcini @ 2024-04-05 16:48 UTC (permalink / raw)
To: Stefan Eichenberger
Cc: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, francesco.dolcini, devicetree, imx, linux-arm-kernel,
linux-kernel, Stefan Eichenberger
In-Reply-To: <20240405160720.5977-3-eichest@gmail.com>
On Fri, Apr 05, 2024 at 06:07:18PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> Previously, we had the sleep-moci pin set to always on. However, the
> Dahlia carrier board supports disabling the sleep-moci when the system
> is suspended to power down peripherals that support it. This reduces
> overall power consumption. This commit adds support for this feature by
> disabling the reg_force_sleep_moci regulator and adding two new
> regulators for the USB hub and PCIe that can be turned off when the
> system is suspended.
>
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
^ permalink raw reply
* Re: [PATCH v1 1/4] arm64: dts: freescale: imx8mp-verdin: replace sleep-moci hog with regulator
From: Francesco Dolcini @ 2024-04-05 16:48 UTC (permalink / raw)
To: Stefan Eichenberger
Cc: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, francesco.dolcini, devicetree, imx, linux-arm-kernel,
linux-kernel, Stefan Eichenberger
In-Reply-To: <20240405160720.5977-2-eichest@gmail.com>
On Fri, Apr 05, 2024 at 06:07:17PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> The Verdin family has a signal called sleep-moci which can be used to
> turn off peripherals on the carrier board when the SoM goes into
> suspend. So far we have hogged this signal, which means the peripherals
> are always on and it is not possible to add peripherals that depend on
> the sleep-moci to be on. With this change, we replace the hog with a
> regulator so that peripherals can add their own regulators that use the
> same gpio. Carrier boards that allow peripherals to be powered off in
> suspend can disable this regulator and implement their own regulator to
> control the sleep-moci.
>
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox