* [PATCH v1 23/49] ARM: dts: imx6qdl-nitrogen6_max: Use #pwm-cells = <3> for imx27-pwm device
From: Uwe Kleine-König @ 2024-04-05 21:42 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer
Cc: Pengutronix Kernel Team, Fabio Estevam, devicetree, imx,
linux-arm-kernel, linux-kernel, Alexander Stein
In-Reply-To: <cover.1712352665.git.u.kleine-koenig@pengutronix.de>
The binding dictates using 3 pwm-cells. Adhere to that.
This fixes the following dtbs_check warnings:
arch/arm/boot/dts/nxp/imx/imx6q-nitrogen6_max.dtb: pwm@2080000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6q-nitrogen6_max.dtb: pwm@2084000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6q-nitrogen6_max.dtb: pwm@208c000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6qp-nitrogen6_max.dtb: pwm@2080000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6qp-nitrogen6_max.dtb: pwm@2084000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6qp-nitrogen6_max.dtb: pwm@208c000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
arch/arm/boot/dts/nxp/imx/imx6qdl-nitrogen6_max.dtsi | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/nxp/imx/imx6qdl-nitrogen6_max.dtsi b/arch/arm/boot/dts/nxp/imx/imx6qdl-nitrogen6_max.dtsi
index 32a110a35b02..33174febf410 100644
--- a/arch/arm/boot/dts/nxp/imx/imx6qdl-nitrogen6_max.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx6qdl-nitrogen6_max.dtsi
@@ -183,7 +183,7 @@ led-ttymxc4-rs232 {
backlight_lcd: backlight-lcd {
compatible = "pwm-backlight";
- pwms = <&pwm1 0 5000000>;
+ pwms = <&pwm1 0 5000000 0>;
brightness-levels = <0 4 8 16 32 64 128 255>;
default-brightness-level = <7>;
power-supply = <®_3p3v>;
@@ -192,7 +192,7 @@ backlight_lcd: backlight-lcd {
backlight_lvds0: backlight-lvds0 {
compatible = "pwm-backlight";
- pwms = <&pwm4 0 5000000>;
+ pwms = <&pwm4 0 5000000 0>;
brightness-levels = <0 4 8 16 32 64 128 255>;
default-brightness-level = <7>;
power-supply = <®_3p3v>;
@@ -201,7 +201,7 @@ backlight_lvds0: backlight-lvds0 {
backlight_lvds1: backlight-lvds1 {
compatible = "pwm-backlight";
- pwms = <&pwm2 0 5000000>;
+ pwms = <&pwm2 0 5000000 0>;
brightness-levels = <0 4 8 16 32 64 128 255>;
default-brightness-level = <7>;
power-supply = <®_3p3v>;
@@ -735,14 +735,12 @@ &pcie {
};
&pwm1 {
- #pwm-cells = <2>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pwm1>;
status = "okay";
};
&pwm2 {
- #pwm-cells = <2>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pwm2>;
status = "okay";
@@ -755,7 +753,6 @@ &pwm3 {
};
&pwm4 {
- #pwm-cells = <2>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pwm4>;
status = "okay";
--
2.43.0
^ permalink raw reply related
* [PATCH v1 43/49] ARM: dts: imx6ul-ccimx6ulsbcpro: Use #pwm-cells = <3> for imx27-pwm device
From: Uwe Kleine-König @ 2024-04-05 21:42 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer
Cc: Pengutronix Kernel Team, Fabio Estevam, devicetree, imx,
linux-arm-kernel, linux-kernel, Alexander Stein
In-Reply-To: <cover.1712352665.git.u.kleine-koenig@pengutronix.de>
The binding dictates using 3 pwm-cells. Adhere to that.
This fixes the following dtbs_check warning:
arch/arm/boot/dts/nxp/imx/imx6ul-ccimx6ulsbcpro.dtb: pwm@20f0000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
arch/arm/boot/dts/nxp/imx/imx6ul-ccimx6ulsbcpro.dts | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/nxp/imx/imx6ul-ccimx6ulsbcpro.dts b/arch/arm/boot/dts/nxp/imx/imx6ul-ccimx6ulsbcpro.dts
index 1762bc47e18d..ed61ae8524fa 100644
--- a/arch/arm/boot/dts/nxp/imx/imx6ul-ccimx6ulsbcpro.dts
+++ b/arch/arm/boot/dts/nxp/imx/imx6ul-ccimx6ulsbcpro.dts
@@ -18,7 +18,7 @@ / {
lcd_backlight: backlight {
compatible = "pwm-backlight";
- pwms = <&pwm5 0 50000>;
+ pwms = <&pwm5 0 50000 0>;
brightness-levels = <0 4 8 16 32 64 128 255>;
default-brightness-level = <6>;
status = "okay";
@@ -168,7 +168,6 @@ &pwm4 {
};
&pwm5 {
- #pwm-cells = <2>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pwm5>;
status = "okay";
--
2.43.0
^ permalink raw reply related
* [PATCH v1 28/49] ARM: dts: imx6qdl-sabrelite: Use #pwm-cells = <3> for imx27-pwm device
From: Uwe Kleine-König @ 2024-04-05 21:42 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer
Cc: Pengutronix Kernel Team, Fabio Estevam, devicetree, imx,
linux-arm-kernel, linux-kernel, Alexander Stein
In-Reply-To: <cover.1712352665.git.u.kleine-koenig@pengutronix.de>
The binding dictates using 3 pwm-cells. Adhere to that.
This fixes the following dtbs_check warnings:
arch/arm/boot/dts/nxp/imx/imx6dl-sabrelite.dtb: pwm@2080000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6dl-sabrelite.dtb: pwm@2088000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6dl-sabrelite.dtb: pwm@208c000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6q-sabrelite.dtb: pwm@2080000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6q-sabrelite.dtb: pwm@2088000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6q-sabrelite.dtb: pwm@208c000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
arch/arm/boot/dts/nxp/imx/imx6qdl-sabrelite.dtsi | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/nxp/imx/imx6qdl-sabrelite.dtsi b/arch/arm/boot/dts/nxp/imx/imx6qdl-sabrelite.dtsi
index 84c8a9531e18..9c502bf77d0b 100644
--- a/arch/arm/boot/dts/nxp/imx/imx6qdl-sabrelite.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx6qdl-sabrelite.dtsi
@@ -99,7 +99,7 @@ mipi_xclk: mipi_xclk {
#clock-cells = <0>;
clock-frequency = <22000000>;
clock-output-names = "mipi_pwm3";
- pwms = <&pwm3 0 45>; /* 1 / 45 ns = 22 MHz */
+ pwms = <&pwm3 0 45 0>; /* 1 / 45 ns = 22 MHz */
status = "okay";
};
@@ -162,7 +162,7 @@ sound {
backlight_lcd: backlight-lcd {
compatible = "pwm-backlight";
- pwms = <&pwm1 0 5000000>;
+ pwms = <&pwm1 0 5000000 0>;
brightness-levels = <0 4 8 16 32 64 128 255>;
default-brightness-level = <7>;
power-supply = <®_3p3v>;
@@ -171,7 +171,7 @@ backlight_lcd: backlight-lcd {
backlight_lvds: backlight-lvds {
compatible = "pwm-backlight";
- pwms = <&pwm4 0 5000000>;
+ pwms = <&pwm4 0 5000000 0>;
brightness-levels = <0 4 8 16 32 64 128 255>;
default-brightness-level = <7>;
power-supply = <®_3p3v>;
@@ -654,21 +654,18 @@ &pcie {
};
&pwm1 {
- #pwm-cells = <2>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pwm1>;
status = "okay";
};
&pwm3 {
- #pwm-cells = <2>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pwm3>;
status = "okay";
};
&pwm4 {
- #pwm-cells = <2>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pwm4>;
status = "okay";
--
2.43.0
^ permalink raw reply related
* [PATCH v1 00/49] ARM: dts: imx: Use #pwm-cells = <3> for imx27-pwm device
From: Uwe Kleine-König @ 2024-04-05 21:41 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Russell King
Cc: Pengutronix Kernel Team, Fabio Estevam, devicetree, imx,
linux-arm-kernel, linux-kernel, Alexander Stein
Hello,
this series addresses many warnings of the type:
arch/arm/boot/dts/nxp/imx/imx6ul-pico-dwarf.dtb: pwm@2088000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
that is emitted when building with CHECK_DTBS=1.
This completes the conversion started with
fa28d8212ede ("ARM: dts: imx: default to #pwm-cells = <3> in the SoC dtsi files")
4c6f19ab2aed ("dt-bindings: pwm: imx-pwm: Unify #pwm-cells for all compatibles")
Best regards
Uwe
Uwe Kleine-König (49):
ARM: dts: imx51-ts4800: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx53-m53evk: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx53-ppd: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx53-kp: Drop redundant settings in pwm nodes
ARM: dts: imx53-tqma: Use #pwm-cells = <3> for imx27-pwm devices
ARM: dts: imx6dl-aristainetos_4: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6dl-aristainetos_7: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6dl-mamoj: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6q-ba16: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6q-bosch-acc: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-apf6dev: Use #pwm-cells = <3> for imx27-pwm devices
ARM: dts: imx6qdl-aristainetos2: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-cubox-i: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-emcon: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-gw52xx: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-gw53xx: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-gw54xx: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-gw560x: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-gw5903: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-gw5904: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-icore: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-nit6xlite: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-nitrogen6_max: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-nitrogen6_som2: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-nitrogen6x: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-phytec-mira: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-sabreauto: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-sabrelite: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-sabresd: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-savageboard: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6qdl-skov-cpu: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6q-kp: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6q-novena: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6q-pistachio: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6q-prti6q: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6q-var-dt6customboard: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6sl-evk: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6sll-evk: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6sx-nitrogen6sx: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6sx-sdb: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6sx-softing-vining-2000: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6ul-14x14-evk: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6ul-ccimx6ulsbcpro: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6ul-geam: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6ul-imx6ull-opos6uldev: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6ul-isiot: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6ul-kontron-bl-43: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6ul-kontron-bl-common: Use #pwm-cells = <3> for imx27-pwm device
ARM: dts: imx6ul-pico: Use #pwm-cells = <3> for imx27-pwm device
arch/arm/boot/dts/nxp/imx/imx51-ts4800.dts | 3 +--
arch/arm/boot/dts/nxp/imx/imx53-kp-ddc.dts | 2 +-
arch/arm/boot/dts/nxp/imx/imx53-kp.dtsi | 10 +---------
arch/arm/boot/dts/nxp/imx/imx53-m53evk.dts | 3 +--
arch/arm/boot/dts/nxp/imx/imx53-mba53.dts | 2 +-
arch/arm/boot/dts/nxp/imx/imx53-ppd.dts | 6 ++----
arch/arm/boot/dts/nxp/imx/imx53-tqma53.dtsi | 8 --------
arch/arm/boot/dts/nxp/imx/imx6dl-aristainetos_4.dts | 3 +--
arch/arm/boot/dts/nxp/imx/imx6dl-aristainetos_7.dts | 3 +--
arch/arm/boot/dts/nxp/imx/imx6dl-mamoj.dts | 3 +--
arch/arm/boot/dts/nxp/imx/imx6q-ba16.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6q-bosch-acc.dts | 10 +++-------
arch/arm/boot/dts/nxp/imx/imx6q-kp.dtsi | 6 ++----
arch/arm/boot/dts/nxp/imx/imx6q-novena.dts | 3 +--
arch/arm/boot/dts/nxp/imx/imx6q-pistachio.dts | 3 +--
arch/arm/boot/dts/nxp/imx/imx6q-prti6q.dts | 3 +--
.../boot/dts/nxp/imx/imx6q-var-dt6customboard.dts | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-apf6dev.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-aristainetos2.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-cubox-i.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-emcon.dtsi | 9 +++------
arch/arm/boot/dts/nxp/imx/imx6qdl-gw52xx.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-gw53xx.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-gw54xx.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-gw560x.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-gw5903.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-gw5904.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-icore.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-nit6xlite.dtsi | 6 ++----
arch/arm/boot/dts/nxp/imx/imx6qdl-nitrogen6_max.dtsi | 9 +++------
.../arm/boot/dts/nxp/imx/imx6qdl-nitrogen6_som2.dtsi | 6 ++----
arch/arm/boot/dts/nxp/imx/imx6qdl-nitrogen6x.dtsi | 6 ++----
arch/arm/boot/dts/nxp/imx/imx6qdl-phytec-mira.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-sabreauto.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-sabrelite.dtsi | 9 +++------
arch/arm/boot/dts/nxp/imx/imx6qdl-sabresd.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-savageboard.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6qdl-skov-cpu.dtsi | 1 -
arch/arm/boot/dts/nxp/imx/imx6sl-evk.dts | 4 +---
arch/arm/boot/dts/nxp/imx/imx6sll-evk.dts | 4 +---
arch/arm/boot/dts/nxp/imx/imx6sx-nitrogen6sx.dts | 4 +---
arch/arm/boot/dts/nxp/imx/imx6sx-sdb.dtsi | 4 +---
.../boot/dts/nxp/imx/imx6sx-softing-vining-2000.dts | 12 +++---------
arch/arm/boot/dts/nxp/imx/imx6ul-14x14-evk.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6ul-ccimx6ulsbcpro.dts | 3 +--
arch/arm/boot/dts/nxp/imx/imx6ul-geam.dts | 3 +--
.../boot/dts/nxp/imx/imx6ul-imx6ull-opos6uldev.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6ul-isiot.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6ul-kontron-bl-43.dts | 3 +--
.../boot/dts/nxp/imx/imx6ul-kontron-bl-common.dtsi | 3 +--
arch/arm/boot/dts/nxp/imx/imx6ul-pico.dtsi | 3 +--
51 files changed, 64 insertions(+), 150 deletions(-)
base-commit: 4cece764965020c22cff7665b18a012006359095
--
2.43.0
^ permalink raw reply
* [PATCH v1 13/49] ARM: dts: imx6qdl-cubox-i: Use #pwm-cells = <3> for imx27-pwm device
From: Uwe Kleine-König @ 2024-04-05 21:42 UTC (permalink / raw)
To: Russell King, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Shawn Guo, Sascha Hauer
Cc: Pengutronix Kernel Team, Fabio Estevam, devicetree, imx,
linux-arm-kernel, linux-kernel, Alexander Stein
In-Reply-To: <cover.1712352665.git.u.kleine-koenig@pengutronix.de>
The binding dictates using 3 pwm-cells. Adhere to that.
This fixes the following dtbs_check warnings:
arch/arm/boot/dts/nxp/imx/imx6dl-cubox-i.dtb: pwm@2080000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6dl-cubox-i-emmc-som-v15.dtb: pwm@2080000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6dl-cubox-i-som-v15.dtb: pwm@2080000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6q-cubox-i.dtb: pwm@2080000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6q-cubox-i-emmc-som-v15.dtb: pwm@2080000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
arch/arm/boot/dts/nxp/imx/imx6q-cubox-i-som-v15.dtb: pwm@2080000: #pwm-cells:0:0: 3 was expected
from schema : http://devicetree.org/schemas/pwm/imx-pwm.yaml#
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
arch/arm/boot/dts/nxp/imx/imx6qdl-cubox-i.dtsi | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/nxp/imx/imx6qdl-cubox-i.dtsi b/arch/arm/boot/dts/nxp/imx/imx6qdl-cubox-i.dtsi
index 1e530d892b76..761566ae3cf5 100644
--- a/arch/arm/boot/dts/nxp/imx/imx6qdl-cubox-i.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx6qdl-cubox-i.dtsi
@@ -64,7 +64,7 @@ led-1 {
active-low;
label = "imx6:red:front";
max-brightness = <248>;
- pwms = <&pwm1 0 50000>;
+ pwms = <&pwm1 0 50000 0>;
};
};
@@ -233,7 +233,6 @@ MX6QDL_PAD_EIM_DA8__GPIO3_IO08 0x17059
};
&pwm1 {
- #pwm-cells = <2>;
status = "okay";
};
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v6 07/11] clk: k210: Deprecate SOC_CANAAN and use SOC_CANAAN_K210
From: Stephen Boyd @ 2024-04-05 21:47 UTC (permalink / raw)
To: Yangyu Chen, linux-riscv
Cc: Conor Dooley, Damien Le Moal, Rob Herring, Krzysztof Kozlowski,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Guo Ren,
Michael Turquette, Linus Walleij, Philipp Zabel, linux-gpio,
linux-clk, devicetree, linux-kernel, Yangyu Chen
In-Reply-To: <tencent_F06FC1196D1D47235C8898CF10ED4632BE07@qq.com>
Quoting Yangyu Chen (2024-03-23 05:12:19)
> Since SOC_FOO should be deprecated from patch [1], and cleanup for other
> SoCs is already on the mailing list [2,3,4], we remove the use of
> SOC_CANAAN and introduced SOC_CANAAN_K210 for K210-specific drivers,
>
> Thus, we replace its drivers depends on SOC_CANAAN_K210 and default select
> when it has the symbol SOC_CANAAN_K210.
>
> [1] https://lore.kernel.org/linux-riscv/20221121221414.109965-1-conor@kernel.org/
> [2] https://lore.kernel.org/linux-riscv/20240305-praying-clad-c4fbcaa7ed0a@spud/
> [3] https://lore.kernel.org/linux-riscv/20240305-fled-undrilled-41dc0c46bb29@spud/
> [4] https://lore.kernel.org/linux-riscv/20240305-stress-earflap-d7ddb8655a4d@spud/
>
> Signed-off-by: Yangyu Chen <cyy@cyyself.name>
> ---
Acked-by: Stephen Boyd <sboyd@kernel.org>
^ permalink raw reply
* Re: [PATCH 2/6] soc: qcom: smem: Add pcode/fcode getters
From: kernel test robot @ 2024-04-05 22:31 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Rob Clark, Abhinav Kumar,
Dmitry Baryshkov, Sean Paul, Marijn Suijten, David Airlie,
Daniel Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: llvm, oe-kbuild-all, linux-arm-msm, linux-kernel, dri-devel,
freedreno, devicetree, Neil Armstrong, Konrad Dybcio
In-Reply-To: <20240405-topic-smem_speedbin-v1-2-ce2b864251b1@linaro.org>
Hi Konrad,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd]
url: https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/soc-qcom-Move-some-socinfo-defines-to-the-header-expand-them/20240405-164231
base: 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd
patch link: https://lore.kernel.org/r/20240405-topic-smem_speedbin-v1-2-ce2b864251b1%40linaro.org
patch subject: [PATCH 2/6] soc: qcom: smem: Add pcode/fcode getters
config: arm-defconfig (https://download.01.org/0day-ci/archive/20240406/202404060648.DOjOYUSf-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240406/202404060648.DOjOYUSf-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404060648.DOjOYUSf-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/soc/qcom/smem.c:807: warning: Function parameter or struct member 'code' not described in 'qcom_smem_get_feature_code'
>> drivers/soc/qcom/smem.c:807: warning: Excess function parameter 'id' description in 'qcom_smem_get_feature_code'
>> drivers/soc/qcom/smem.c:840: warning: Function parameter or struct member 'code' not described in 'qcom_smem_get_product_code'
>> drivers/soc/qcom/smem.c:840: warning: Excess function parameter 'id' description in 'qcom_smem_get_product_code'
vim +807 drivers/soc/qcom/smem.c
797
798 /**
799 * qcom_smem_get_feature_code() - return the feature code
800 * @id: On success, we return the feature code here.
801 *
802 * Look up the feature code identifier from SMEM and return it.
803 *
804 * Return: 0 on success, negative errno on failure.
805 */
806 int qcom_smem_get_feature_code(u32 *code)
> 807 {
808 struct socinfo *info;
809 u32 raw_code;
810
811 info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL);
812 if (IS_ERR(info))
813 return PTR_ERR(info);
814
815 /* This only makes sense for socinfo >= 16 */
816 if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
817 return -EINVAL;
818
819 raw_code = __le32_to_cpu(info->feature_code);
820
821 /* Ensure the value makes sense */
822 if (raw_code >= SOCINFO_FC_INT_RESERVE)
823 raw_code = SOCINFO_FC_UNKNOWN;
824
825 *code = raw_code;
826
827 return 0;
828 }
829 EXPORT_SYMBOL_GPL(qcom_smem_get_feature_code);
830
831 /**
832 * qcom_smem_get_product_code() - return the product code
833 * @id: On success, we return the product code here.
834 *
835 * Look up feature code identifier from SMEM and return it.
836 *
837 * Return: 0 on success, negative errno on failure.
838 */
839 int qcom_smem_get_product_code(u32 *code)
> 840 {
841 struct socinfo *info;
842 u32 raw_code;
843
844 info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL);
845 if (IS_ERR(info))
846 return PTR_ERR(info);
847
848 /* This only makes sense for socinfo >= 16 */
849 if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
850 return -EINVAL;
851
852 raw_code = __le32_to_cpu(info->pcode);
853
854 /* Ensure the value makes sense */
855 if (raw_code >= SOCINFO_FC_INT_RESERVE)
856 raw_code = SOCINFO_FC_UNKNOWN;
857
858 *code = raw_code;
859
860 return 0;
861 }
862 EXPORT_SYMBOL_GPL(qcom_smem_get_product_code);
863
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH v2 0/3] dt-bindings: kbuild: Rework build rules and dependencies
From: Rob Herring @ 2024-04-05 22:56 UTC (permalink / raw)
To: Krzysztof Kozlowski, Conor Dooley, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier
Cc: Dmitry Baryshkov, Marijn Suijten, devicetree, linux-kernel,
linux-kbuild
This series reworks the DT binding build rules and dependencies. It
fixes a problem with if_changed_rule Masahiro reported some time back[1]
and improves the dependency handling for the DT validation targets.
Relative to v1, I've dropped all but 1 one of the top-level targets
added in v1. The only top-level target added it for building the
processed schema used by multiple targets.
Rob
v1: https://lore.kernel.org/all/20220824203934.2855320-1-robh@kernel.org/
[1] https://lore.kernel.org/all/20220817152027.16928-1-masahiroy@kernel.org/
Signed-off-by: Rob Herring <robh@kernel.org>
---
Rob Herring (3):
dt-bindings: kbuild: Simplify examples target patsubst
dt-bindings: kbuild: Split targets out to separate rules
dt-bindings: kbuild: Add separate target/dependency for processed-schema.json
Documentation/devicetree/bindings/Makefile | 34 ++++++++++++++++++------------
Makefile | 24 ++++++++++-----------
scripts/Makefile.lib | 2 +-
3 files changed, 33 insertions(+), 27 deletions(-)
---
base-commit: 4cece764965020c22cff7665b18a012006359095
change-id: 20240405-dt-kbuild-rework-f356ab890d45
Best regards,
--
Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH v2 1/3] dt-bindings: kbuild: Simplify examples target patsubst
From: Rob Herring @ 2024-04-05 22:56 UTC (permalink / raw)
To: Krzysztof Kozlowski, Conor Dooley, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier
Cc: Dmitry Baryshkov, Marijn Suijten, devicetree, linux-kernel,
linux-kbuild
In-Reply-To: <20240405-dt-kbuild-rework-v2-0-3a035caee357@kernel.org>
Instead of stripping off the $(srctree) multiple times do it once up
front, but keep the src/obj path as it is going to be needed in
subsequent commit.
Rename the variable to CHK_DT_EXAMPLES to better reflect what it
contains.
Signed-off-by: Rob Herring <robh@kernel.org>
---
v2: New patch
---
Documentation/devicetree/bindings/Makefile | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
index 5e08e3a6a97b..95f1436ebcd0 100644
--- a/Documentation/devicetree/bindings/Makefile
+++ b/Documentation/devicetree/bindings/Makefile
@@ -32,7 +32,7 @@ find_cmd = $(find_all_cmd) | \
sed 's|^$(srctree)/||' | \
grep -F -e "$(subst :," -e ",$(DT_SCHEMA_FILES))" | \
sed 's|^|$(srctree)/|'
-CHK_DT_DOCS := $(shell $(find_cmd))
+CHK_DT_EXAMPLES := $(patsubst $(srctree)/%.yaml,%.example.dtb, $(shell $(find_cmd)))
quiet_cmd_yamllint = LINT $(src)
cmd_yamllint = ($(find_cmd) | \
@@ -68,8 +68,8 @@ $(obj)/processed-schema.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version
$(call if_changed_rule,chkdt)
always-y += processed-schema.json
-always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dts, $(CHK_DT_DOCS))
-always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dtb, $(CHK_DT_DOCS))
+always-$(CHECK_DT_BINDING) += $(patsubst $(obj)/%,%, $(CHK_DT_EXAMPLES))
+always-$(CHECK_DT_BINDING) += $(patsubst $(obj)/%.dtb,%.dts, $(CHK_DT_EXAMPLES))
# Hack: avoid 'Argument list too long' error for 'make clean'. Remove most of
# build artifacts here before they are processed by scripts/Makefile.clean
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/3] dt-bindings: kbuild: Split targets out to separate rules
From: Rob Herring @ 2024-04-05 22:56 UTC (permalink / raw)
To: Krzysztof Kozlowski, Conor Dooley, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier
Cc: Dmitry Baryshkov, Marijn Suijten, devicetree, linux-kernel,
linux-kbuild
In-Reply-To: <20240405-dt-kbuild-rework-v2-0-3a035caee357@kernel.org>
Masahiro pointed out the use of if_changed_rule is incorrect and command
line changes are not correctly accounted for.
To fix this, split up the DT binding validation target,
dt_binding_check, into multiple rules for each step: yamllint, schema
validtion with meta-schema, and building the processed schema.
One change in behavior is the yamllint or schema validation will be
re-run again when there are warnings present.
Reported-by: Masahiro Yamada <masahiroy@kernel.org>
Link: https://lore.kernel.org/all/20220817152027.16928-1-masahiroy@kernel.org/
Signed-off-by: Rob Herring <robh@kernel.org>
---
v2:
- Separated rework of build rules to fix if_changed_rule usage from
addition of top-level build rules.
---
Documentation/devicetree/bindings/Makefile | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
index 95f1436ebcd0..3779405269ab 100644
--- a/Documentation/devicetree/bindings/Makefile
+++ b/Documentation/devicetree/bindings/Makefile
@@ -37,11 +37,13 @@ CHK_DT_EXAMPLES := $(patsubst $(srctree)/%.yaml,%.example.dtb, $(shell $(find_cm
quiet_cmd_yamllint = LINT $(src)
cmd_yamllint = ($(find_cmd) | \
xargs -n200 -P$$(nproc) \
- $(DT_SCHEMA_LINT) -f parsable -c $(srctree)/$(src)/.yamllint >&2) || true
+ $(DT_SCHEMA_LINT) -f parsable -c $(srctree)/$(src)/.yamllint >&2) \
+ && touch $@ || true
-quiet_cmd_chk_bindings = CHKDT $@
+quiet_cmd_chk_bindings = CHKDT $(src)
cmd_chk_bindings = ($(find_cmd) | \
- xargs -n200 -P$$(nproc) $(DT_DOC_CHECKER) -u $(srctree)/$(src)) || true
+ xargs -n200 -P$$(nproc) $(DT_DOC_CHECKER) -u $(srctree)/$(src)) \
+ && touch $@ || true
quiet_cmd_mk_schema = SCHEMA $@
cmd_mk_schema = f=$$(mktemp) ; \
@@ -49,12 +51,6 @@ quiet_cmd_mk_schema = SCHEMA $@
$(DT_MK_SCHEMA) -j $(DT_MK_SCHEMA_FLAGS) @$$f > $@ ; \
rm -f $$f
-define rule_chkdt
- $(if $(DT_SCHEMA_LINT),$(call cmd,yamllint),)
- $(call cmd,chk_bindings)
- $(call cmd,mk_schema)
-endef
-
DT_DOCS = $(patsubst $(srctree)/%,%,$(shell $(find_all_cmd)))
override DTC_FLAGS := \
@@ -64,8 +60,15 @@ override DTC_FLAGS := \
-Wno-unique_unit_address \
-Wunique_unit_address_if_enabled
-$(obj)/processed-schema.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version FORCE
- $(call if_changed_rule,chkdt)
+$(obj)/processed-schema.json: $(DT_DOCS) check_dtschema_version FORCE
+ $(call if_changed,mk_schema)
+
+always-$(CHECK_DT_BINDING) += .dt-binding.checked .yamllint.checked
+$(obj)/.yamllint.checked: $(DT_DOCS) $(src)/.yamllint FORCE
+ $(if $(DT_SCHEMA_LINT),$(call if_changed,yamllint),)
+
+$(obj)/.dt-binding.checked: $(DT_DOCS) FORCE
+ $(call if_changed,chk_bindings)
always-y += processed-schema.json
always-$(CHECK_DT_BINDING) += $(patsubst $(obj)/%,%, $(CHK_DT_EXAMPLES))
--
2.43.0
^ permalink raw reply related
* [PATCH v2 3/3] dt-bindings: kbuild: Add separate target/dependency for processed-schema.json
From: Rob Herring @ 2024-04-05 22:56 UTC (permalink / raw)
To: Krzysztof Kozlowski, Conor Dooley, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier
Cc: Dmitry Baryshkov, Marijn Suijten, devicetree, linux-kernel,
linux-kbuild
In-Reply-To: <20240405-dt-kbuild-rework-v2-0-3a035caee357@kernel.org>
Running dtbs_check and dt_compatible_check targets really only depend
on processed-schema.json, but the dependency is 'dt_binding_check'. That
was sort worked around with the CHECK_DT_BINDING variable in order to
skip some of the work that 'dt_binding_check' does. It still runs the
full checks of the schemas which is not necessary and adds 10s of
seconds to the build time. That's significant when checking only a few
DTBs and with recent changes that have improved the validation time by
6-7x.
Add a new target, dt_binding_schema, which just builds
processed-schema.json and can be used as the dependency for other
targets. The scripts_dtc dependency isn't needed either as the examples
aren't built for it.
Signed-off-by: Rob Herring <robh@kernel.org>
---
v2:
- Just split out building processed-schema.json and drop running
yamllint, schema validation, or checking examples separately.
- Fix multiple targets in parallel build. (Thanks Masahiro!)
---
Documentation/devicetree/bindings/Makefile | 9 ++++++---
Makefile | 24 ++++++++++++------------
scripts/Makefile.lib | 2 +-
3 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
index 3779405269ab..8cdda477987f 100644
--- a/Documentation/devicetree/bindings/Makefile
+++ b/Documentation/devicetree/bindings/Makefile
@@ -63,7 +63,7 @@ override DTC_FLAGS := \
$(obj)/processed-schema.json: $(DT_DOCS) check_dtschema_version FORCE
$(call if_changed,mk_schema)
-always-$(CHECK_DT_BINDING) += .dt-binding.checked .yamllint.checked
+targets += .dt-binding.checked .yamllint.checked
$(obj)/.yamllint.checked: $(DT_DOCS) $(src)/.yamllint FORCE
$(if $(DT_SCHEMA_LINT),$(call if_changed,yamllint),)
@@ -71,8 +71,8 @@ $(obj)/.dt-binding.checked: $(DT_DOCS) FORCE
$(call if_changed,chk_bindings)
always-y += processed-schema.json
-always-$(CHECK_DT_BINDING) += $(patsubst $(obj)/%,%, $(CHK_DT_EXAMPLES))
-always-$(CHECK_DT_BINDING) += $(patsubst $(obj)/%.dtb,%.dts, $(CHK_DT_EXAMPLES))
+targets += $(patsubst $(obj)/%,%, $(CHK_DT_EXAMPLES))
+targets += $(patsubst $(obj)/%.dtb,%.dts, $(CHK_DT_EXAMPLES))
# Hack: avoid 'Argument list too long' error for 'make clean'. Remove most of
# build artifacts here before they are processed by scripts/Makefile.clean
@@ -81,3 +81,6 @@ clean-files = $(shell find $(obj) \( -name '*.example.dts' -o \
dt_compatible_check: $(obj)/processed-schema.json
$(Q)$(srctree)/scripts/dtc/dt-extract-compatibles $(srctree) | xargs dt-check-compatible -v -s $<
+
+PHONY += dt_binding_check
+dt_binding_check: $(obj)/.dt-binding.checked $(obj)/.yamllint.checked $(CHK_DT_EXAMPLES)
diff --git a/Makefile b/Makefile
index 763b6792d3d5..1356e48caa2b 100644
--- a/Makefile
+++ b/Makefile
@@ -1398,12 +1398,12 @@ dtbs: dtbs_prepare
# dtbs_install depend on it as dtbs_install may run as root.
dtbs_prepare: include/config/kernel.release scripts_dtc
-ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),)
+ifneq ($(filter dt_binding_check dtbs_check, $(MAKECMDGOALS)),)
export CHECK_DTBS=y
endif
ifneq ($(CHECK_DTBS),)
-dtbs_prepare: dt_binding_check
+dtbs_prepare: dt_binding_schemas
endif
dtbs_check: dtbs
@@ -1421,16 +1421,15 @@ PHONY += scripts_dtc
scripts_dtc: scripts_basic
$(Q)$(MAKE) $(build)=scripts/dtc
-ifneq ($(filter dt_binding_check, $(MAKECMDGOALS)),)
-export CHECK_DT_BINDING=y
-endif
+PHONY += dt_binding_check dt_binding_schemas
+dt_binding_check: dt_binding_schemas scripts_dtc
+ $(Q)$(MAKE) $(build)=Documentation/devicetree/bindings $@
-PHONY += dt_binding_check
-dt_binding_check: scripts_dtc
+dt_binding_schemas:
$(Q)$(MAKE) $(build)=Documentation/devicetree/bindings
PHONY += dt_compatible_check
-dt_compatible_check: dt_binding_check
+dt_compatible_check: dt_binding_schemas
$(Q)$(MAKE) $(build)=Documentation/devicetree/bindings $@
# ---------------------------------------------------------------------------
@@ -1626,10 +1625,11 @@ help:
@echo ''
@$(if $(dtstree), \
echo 'Devicetree:'; \
- echo '* dtbs - Build device tree blobs for enabled boards'; \
- echo ' dtbs_install - Install dtbs to $(INSTALL_DTBS_PATH)'; \
- echo ' dt_binding_check - Validate device tree binding documents'; \
- echo ' dtbs_check - Validate device tree source files';\
+ echo '* dtbs - Build device tree blobs for enabled boards'; \
+ echo ' dtbs_install - Install dtbs to $(INSTALL_DTBS_PATH)'; \
+ echo ' dt_binding_check - Validate device tree binding documents and examples'; \
+ echo ' dt_binding_schema - Build processed device tree binding schemas'; \
+ echo ' dtbs_check - Validate device tree source files';\
echo '')
@echo 'Userspace tools targets:'
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 3179747cbd2c..d1d51e38b55d 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -410,7 +410,7 @@ $(multi-dtb-y): FORCE
$(call if_changed,fdtoverlay)
$(call multi_depend, $(multi-dtb-y), .dtb, -dtbs)
-ifneq ($(CHECK_DTBS)$(CHECK_DT_BINDING),)
+ifneq ($(CHECK_DTBS),)
DT_CHECKER ?= dt-validate
DT_CHECKER_FLAGS ?= $(if $(DT_SCHEMA_FILES),-l $(DT_SCHEMA_FILES),-m)
DT_BINDING_DIR := Documentation/devicetree/bindings
--
2.43.0
^ permalink raw reply related
* Re: [GIT PULL] Devicetree fixes for v6.9, part 1
From: pr-tracker-bot @ 2024-04-06 0:10 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Torvalds, Saravana Kannan, Krzysztof Kozlowski,
Conor Dooley, linux-kernel, devicetree
In-Reply-To: <20240405204017.GA1394619-robh@kernel.org>
The pull request you sent on Fri, 5 Apr 2024 15:40:17 -0500:
> git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git tags/devicetree-fixes-for-6.9-1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/84985eb2c084676f974698cb19fb5a166650886a
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [RFC PATCH 2/2] PCI: Add Qualcomm PCIe ECAM root complex driver
From: Mayank Rana @ 2024-04-06 0:43 UTC (permalink / raw)
To: Bjorn Helgaas
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: <20240405183014.GA1964459@bhelgaas>
Hi Bjorn
Thanks for reviewing change.
On 4/5/2024 11:30 AM, Bjorn Helgaas 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
>
> I think link state would be L0, not D0.
ACK
>> 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.
ACK. will repharse it.
>> +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.
Ok. will rephrase and move as suggested.
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> Does this actually work? I expected "#define dev_fmt" since you're
> using dev_err(), etc below.
Yes. look like it is not needed anymore for this driver as very limited
pr_* usage.
>> +#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/
Ok. will review shared information, and try to rework upon this.
>> +#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?
Yes it is DB address to generate MSI, and it is not tied with directly
with any
hardware/platform. Hence hardcoding here.
>> + * 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/.
ACK
>> +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.
ACK
>> +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.
ACK
>> +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.
ACK
>> +static struct msi_domain_ops qcom_msi_domain_ops = {
>> + .msi_prepare = qcom_msi_domain_prepare,
>
> Rename so function name includes the struct member name.
ACK
>> +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.
ACK
>> + 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;
ACK
>> +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.
ACK
>> +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.
ACK
>> +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.
ACK
Regards,
Mayank
^ permalink raw reply
* Re: [PATCH v8 1/7] dt-bindings: spmi: Add X1E80100 SPMI PMIC ARB schema
From: David Collins @ 2024-04-06 1:28 UTC (permalink / raw)
To: Abel Vesa, Stephen Boyd, Matthias Brugger, Bjorn Andersson,
Konrad Dybcio, Dmitry Baryshkov, Neil Armstrong,
AngeloGioacchino Del Regno, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Srini Kandagatla, Johan Hovold, linux-kernel, linux-arm-kernel,
linux-arm-msm, linux-mediatek, devicetree, Krzysztof Kozlowski
In-Reply-To: <20240402-spmi-multi-master-support-v8-1-ce6f2d14a058@linaro.org>
On 4/2/24 05:07, Abel Vesa wrote:
> + reg:
> + items:
> + - description: core registers
> + - description: tx-channel per virtual slave regosters
Minor: s/regosters/registers/
Thanks,
David Collins
^ permalink raw reply
* Re: [PATCH v8 6/7] spmi: pmic-arb: Register controller for bus instead of arbiter
From: David Collins @ 2024-04-06 1:29 UTC (permalink / raw)
To: Abel Vesa, Stephen Boyd, Matthias Brugger, Bjorn Andersson,
Konrad Dybcio, Dmitry Baryshkov, Neil Armstrong,
AngeloGioacchino Del Regno, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Srini Kandagatla, Johan Hovold, linux-kernel, linux-arm-kernel,
linux-arm-msm, linux-mediatek, devicetree
In-Reply-To: <20240402-spmi-multi-master-support-v8-6-ce6f2d14a058@linaro.org>
On 4/2/24 05:07, Abel Vesa wrote:
> +struct spmi_pmic_arb_bus {
> + struct spmi_pmic_arb *pmic_arb;
> + struct irq_domain *domain;
> + void __iomem *intr;
> + void __iomem *cnfg;
> + struct spmi_controller *spmic;
> + u16 base_apid;
> + int apid_count;
> + u32 *mapping_table;
> + DECLARE_BITMAP(mapping_table_valid, PMIC_ARB_MAX_PERIPHS);
> + u16 *ppid_to_apid;
> + u16 last_apid;
> + struct apid_data *apid_data;
> + u16 min_apid;
> + u16 max_apid;
> + int irq;
> +};
...
> struct spmi_pmic_arb {
> void __iomem *rd_base;
> void __iomem *wr_base;
> - void __iomem *intr;
> - void __iomem *cnfg;
> void __iomem *core;
> resource_size_t core_size;
> raw_spinlock_t lock;
Can you please move "lock" from "struct spmi_pmic_arb" into "struct
spmi_pmic_arb_bus" and update its usage in the functions below? The two
SPMI buses within PMIC Arbiter v7 operate entirely independently and
write to separate sets of registers. As-is, transactions on one bus
would unnecessarily block transactions on the other, leading to a
performance penalty.
> u8 channel;
> - int irq;
> u8 ee;
> - u32 bus_instance;
> - u16 min_apid;
> - u16 max_apid;
> - u16 base_apid;
> - int apid_count;
> - u32 *mapping_table;
> - DECLARE_BITMAP(mapping_table_valid, PMIC_ARB_MAX_PERIPHS);
> - struct irq_domain *domain;
> - struct spmi_controller *spmic;
> const struct pmic_arb_ver_ops *ver_ops;
> - u16 *ppid_to_apid;
> - u16 last_apid;
> - struct apid_data *apid_data;
> int max_periphs;
> + struct spmi_pmic_arb_bus *bus;
> };
Thanks,
David Collins
^ permalink raw reply
* Re: [PATCH v8 7/7] spmi: pmic-arb: Add multi bus support
From: David Collins @ 2024-04-06 1:29 UTC (permalink / raw)
To: Abel Vesa, Stephen Boyd, Matthias Brugger, Bjorn Andersson,
Konrad Dybcio, Dmitry Baryshkov, Neil Armstrong,
AngeloGioacchino Del Regno, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Srini Kandagatla, Johan Hovold, linux-kernel, linux-arm-kernel,
linux-arm-msm, linux-mediatek, devicetree
In-Reply-To: <20240402-spmi-multi-master-support-v8-7-ce6f2d14a058@linaro.org>
On 4/2/24 05:07, Abel Vesa wrote:
> +static void spmi_pmic_arb_deregister_buses(struct spmi_pmic_arb *pmic_arb)
> +{
> + int i;
> +
> + for (i = 0; i < PMIC_ARB_MAX_BUSES; i++) {
s/PMIC_ARB_MAX_BUSES/pmic_arb->buses_available/
This is needed to avoid a NULL pointer dereference in the case that a
single SPMI bus is specified in DT and then removed at runtime.
> + struct spmi_pmic_arb_bus *bus = pmic_arb->buses[i];
> +
> + irq_set_chained_handler_and_data(bus->irq,
> + NULL, NULL);
> + irq_domain_remove(bus->domain);
> + }
> +}
Thanks,
David Collins
^ permalink raw reply
* Re: [PATCH 2/6] soc: qcom: smem: Add pcode/fcode getters
From: Dmitry Baryshkov @ 2024-04-06 2:21 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <20240405-topic-smem_speedbin-v1-2-ce2b864251b1@linaro.org>
On Fri, Apr 05, 2024 at 10:41:30AM +0200, Konrad Dybcio wrote:
> Introduce getters for SoC product and feature codes and export them.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> drivers/soc/qcom/smem.c | 66 +++++++++++++++++++++++++++++++++++++++++++
> include/linux/soc/qcom/smem.h | 2 ++
> 2 files changed, 68 insertions(+)
>
> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> index 7191fa0c087f..e89b4d26877a 100644
> --- a/drivers/soc/qcom/smem.c
> +++ b/drivers/soc/qcom/smem.c
> @@ -795,6 +795,72 @@ int qcom_smem_get_soc_id(u32 *id)
> }
> EXPORT_SYMBOL_GPL(qcom_smem_get_soc_id);
>
> +/**
> + * qcom_smem_get_feature_code() - return the feature code
> + * @id: On success, we return the feature code here.
> + *
> + * Look up the feature code identifier from SMEM and return it.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int qcom_smem_get_feature_code(u32 *code)
> +{
> + struct socinfo *info;
> + u32 raw_code;
> +
> + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL);
> + if (IS_ERR(info))
> + return PTR_ERR(info);
> +
> + /* This only makes sense for socinfo >= 16 */
> + if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
> + return -EINVAL;
> +
> + raw_code = __le32_to_cpu(info->feature_code);
> +
> + /* Ensure the value makes sense */
> + if (raw_code >= SOCINFO_FC_INT_RESERVE)
> + raw_code = SOCINFO_FC_UNKNOWN;
> +
> + *code = raw_code;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(qcom_smem_get_feature_code);
> +
> +/**
> + * qcom_smem_get_product_code() - return the product code
> + * @id: On success, we return the product code here.
> + *
> + * Look up feature code identifier from SMEM and return it.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +int qcom_smem_get_product_code(u32 *code)
> +{
> + struct socinfo *info;
> + u32 raw_code;
> +
> + info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL);
> + if (IS_ERR(info))
> + return PTR_ERR(info);
> +
> + /* This only makes sense for socinfo >= 16 */
> + if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
> + return -EINVAL;
> +
> + raw_code = __le32_to_cpu(info->pcode);
> +
> + /* Ensure the value makes sense */
> + if (raw_code >= SOCINFO_FC_INT_RESERVE)
> + raw_code = SOCINFO_FC_UNKNOWN;
This looks like a c&p from the previous function. Should we be comparing
the raw_code with a SOCINFO_PC_ constant?
> +
> + *code = raw_code;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(qcom_smem_get_product_code);
> +
> static int qcom_smem_get_sbl_version(struct qcom_smem *smem)
> {
> struct smem_header *header;
> diff --git a/include/linux/soc/qcom/smem.h b/include/linux/soc/qcom/smem.h
> index a36a3b9d4929..aef8c9fc6c08 100644
> --- a/include/linux/soc/qcom/smem.h
> +++ b/include/linux/soc/qcom/smem.h
> @@ -13,5 +13,7 @@ int qcom_smem_get_free_space(unsigned host);
> phys_addr_t qcom_smem_virt_to_phys(void *p);
>
> int qcom_smem_get_soc_id(u32 *id);
> +int qcom_smem_get_feature_code(u32 *code);
> +int qcom_smem_get_product_code(u32 *code);
>
> #endif
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 1/6] soc: qcom: Move some socinfo defines to the header, expand them
From: Dmitry Baryshkov @ 2024-04-06 2:22 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <20240405-topic-smem_speedbin-v1-1-ce2b864251b1@linaro.org>
On Fri, Apr 05, 2024 at 10:41:29AM +0200, Konrad Dybcio wrote:
> In preparation for parsing the chip "feature code" (FC) and "product
> code" (PC) (essentially the parameters that let us conclusively
> characterize the sillicon we're running on, including various speed
> bins), move the socinfo version defines to the public header and
> include some more FC/PC defines.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> drivers/soc/qcom/socinfo.c | 8 --------
> include/linux/soc/qcom/socinfo.h | 36 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
> index 277c07a6603d..cf4616a468f2 100644
> --- a/drivers/soc/qcom/socinfo.c
> +++ b/drivers/soc/qcom/socinfo.c
> @@ -21,14 +21,6 @@
>
> #include <dt-bindings/arm/qcom,ids.h>
>
> -/*
> - * SoC version type with major number in the upper 16 bits and minor
> - * number in the lower 16 bits.
> - */
> -#define SOCINFO_MAJOR(ver) (((ver) >> 16) & 0xffff)
> -#define SOCINFO_MINOR(ver) ((ver) & 0xffff)
> -#define SOCINFO_VERSION(maj, min) ((((maj) & 0xffff) << 16)|((min) & 0xffff))
> -
> /* Helper macros to create soc_id table */
> #define qcom_board_id(id) QCOM_ID_ ## id, __stringify(id)
> #define qcom_board_id_named(id, name) QCOM_ID_ ## id, (name)
> diff --git a/include/linux/soc/qcom/socinfo.h b/include/linux/soc/qcom/socinfo.h
> index e78777bb0f4a..ba7f683bd32c 100644
> --- a/include/linux/soc/qcom/socinfo.h
> +++ b/include/linux/soc/qcom/socinfo.h
> @@ -3,6 +3,8 @@
> #ifndef __QCOM_SOCINFO_H__
> #define __QCOM_SOCINFO_H__
>
> +#include <linux/types.h>
> +
> /*
> * SMEM item id, used to acquire handles to respective
> * SMEM region.
> @@ -12,6 +14,14 @@
> #define SMEM_SOCINFO_BUILD_ID_LENGTH 32
> #define SMEM_SOCINFO_CHIP_ID_LENGTH 32
>
> +/*
> + * SoC version type with major number in the upper 16 bits and minor
> + * number in the lower 16 bits.
> + */
> +#define SOCINFO_MAJOR(ver) (((ver) >> 16) & 0xffff)
> +#define SOCINFO_MINOR(ver) ((ver) & 0xffff)
> +#define SOCINFO_VERSION(maj, min) ((((maj) & 0xffff) << 16)|((min) & 0xffff))
> +
> /* Socinfo SMEM item structure */
> struct socinfo {
> __le32 fmt;
> @@ -74,4 +84,30 @@ struct socinfo {
> __le32 boot_core;
> };
>
> +/* Internal feature codes */
> +enum feature_code {
> + /* External feature codes */
> + SOCINFO_FC_UNKNOWN = 0x0,
> + SOCINFO_FC_AA,
> + SOCINFO_FC_AB,
> + SOCINFO_FC_AC,
> + SOCINFO_FC_AD,
> + SOCINFO_FC_AE,
> + SOCINFO_FC_AF,
> + SOCINFO_FC_AG,
> + SOCINFO_FC_AH,
> + SOCINFO_FC_EXT_RESERVE,
> +};
> +
> +/* Internal feature codes */
> +/* Valid values: 0 <= n <= 0xf */
> +#define SOCINFO_FC_Yn(n) (0xf1 + n)
> +#define SOCINFO_FC_INT_RESERVE SOCINFO_FC_Yn(0x10)
> +
> +/* Product codes */
> +#define SOCINFO_PC_UNKNOWN 0
> +/* Valid values: 0 <= n <= 8, the rest is reserved */
> +#define SOCINFO_PCn(n) (n + 1)
> +#define SOCINFO_PC_RESERVE (BIT(31) - 1)
Please move these defines into the next patch.
> +
> #endif
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v2 06/18] PCI: endpoint: test: Implement link_down event operation
From: Manivannan Sadhasivam @ 2024-04-06 2:24 UTC (permalink / raw)
To: Niklas Cassel
Cc: Damien Le Moal, Lorenzo Pieralisi, Kishon Vijay Abraham I,
Shawn Lin, Krzysztof Wilczyński, Bjorn Helgaas,
Heiko Stuebner, linux-pci, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, devicetree, linux-rockchip, linux-arm-kernel,
Rick Wertenbroek, Wilfred Mallawa
In-Reply-To: <Zg//LiMg0Wh7LfT8@x1-carbon>
On Fri, Apr 05, 2024 at 03:39:58PM +0200, Niklas Cassel wrote:
> On Wed, Apr 03, 2024 at 01:18:23PM +0530, Manivannan Sadhasivam wrote:
> > On Sat, Mar 30, 2024 at 01:19:16PM +0900, Damien Le Moal wrote:
> > > Implement the link_down event operation to stop the command execution
> > > delayed work when the endpoint controller notifies a link down event.
> > >
> > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> >
> > This patch is already part of another series I posted [1] and under review. So
> > this can be dropped.
> >
> > - Mani
> >
> > [1] https://lore.kernel.org/linux-pci/20240401-pci-epf-rework-v2-9-970dbe90b99d@linaro.org/
>
> Mani, your patch does not use _sync(),
> so I don't think that we can simply drop this patch.
>
Agree, I was planning to update it in my next version anyway.
- Mani
>
> Kind regards,
> Niklas
>
> >
> > > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > drivers/pci/endpoint/functions/pci-epf-test.c | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > index ab40c3182677..e6d4e1747c9f 100644
> > > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > @@ -824,9 +824,19 @@ static int pci_epf_test_link_up(struct pci_epf *epf)
> > > return 0;
> > > }
> > >
> > > +static int pci_epf_test_link_down(struct pci_epf *epf)
> > > +{
> > > + struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> > > +
> > > + cancel_delayed_work_sync(&epf_test->cmd_handler);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static const struct pci_epc_event_ops pci_epf_test_event_ops = {
> > > .core_init = pci_epf_test_core_init,
> > > .link_up = pci_epf_test_link_up,
> > > + .link_down = pci_epf_test_link_down,
> > > };
> > >
> > > static int pci_epf_test_alloc_space(struct pci_epf *epf)
> > > --
> > > 2.44.0
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH 3/6] drm/msm/adreno: Allow specifying default speedbin value
From: Dmitry Baryshkov @ 2024-04-06 2:56 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <20240405-topic-smem_speedbin-v1-3-ce2b864251b1@linaro.org>
On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
> From: Neil Armstrong <neil.armstrong@linaro.org>
>
> Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
> the highest. Falling back to it when things go wrong is largely
> suboptimal, as more often than not, the top frequencies are not
> supposed to work on other bins.
Isn't it better to just return an error here instead of trying to guess
which speedbin to use?
If that's not the case, I think the commit should be expanded with
actually setting default_speedbin for the existing GPUs.
>
> Let the developer specify the intended "lowest common denominator" bin
> in struct adreno_info. If not specified, partial struct initialization
> will ensure it's set to zero, retaining previous behavior.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> [Konrad: clean up, add commit message]
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 0674aca0f8a3..4cbdfabbcee5 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -2915,7 +2915,7 @@ static int a6xx_set_supported_hw(struct device *dev, const struct adreno_info *i
> DRM_DEV_ERROR(dev,
> "missing support for speed-bin: %u. Some OPPs may not be supported by hardware\n",
> speedbin);
> - supp_hw = BIT(0); /* Default */
> + supp_hw = BIT(info->default_speedbin); /* Default */
> }
>
> ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1);
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 77526892eb8c..460b399be37b 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -110,6 +110,7 @@ struct adreno_info {
> * {SHRT_MAX, 0} sentinal.
> */
> struct adreno_speedbin *speedbins;
> + unsigned int default_speedbin;
> };
>
> #define ADRENO_CHIP_IDS(tbl...) (uint32_t[]) { tbl, 0 }
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v6 00/16] power: sequencing: implement the subsystem and add first users
From: Xilin Wu @ 2024-04-06 3:03 UTC (permalink / raw)
To: Bartosz Golaszewski, Marcel Holtmann, Luiz Augusto von Dentz,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Kalle Valo,
Bjorn Andersson, Konrad Dybcio, Liam Girdwood, Mark Brown,
Catalin Marinas, Will Deacon, Bjorn Helgaas, Saravana Kannan,
Geert Uytterhoeven, Arnd Bergmann, Neil Armstrong,
Marek Szyprowski, Alex Elder, Srini Kandagatla,
Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
Lukas Wunner, Dmitry Baryshkov
Cc: linux-bluetooth, netdev, devicetree, linux-kernel, linux-wireless,
linux-arm-msm, linux-arm-kernel, linux-pci, linux-pm,
Bartosz Golaszewski
In-Reply-To: <20240325131624.26023-1-brgl@bgdev.pl>
On 2024/3/25 21:16, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski<bartosz.golaszewski@linaro.org>
>
> Note: I dropped most of the the review and test tags on purpose, the code
> changed significantly and warrants a new round of reviews and tests.
>
> ===
>
> Problem statement #1: Dynamic bus chicken-and-egg problem.
>
> Certain on-board PCI devices need to be powered up before they are can be
> detected but their PCI drivers won't get bound until the device is
> powered-up so enabling the relevant resources in the PCI device driver
> itself is impossible.
>
> Problem statement #2: Sharing inter-dependent resources between devices.
>
> Certain devices that use separate drivers (often on different busses)
> share resources (regulators, clocks, etc.). Typically these resources
> are reference-counted but in some cases there are additional interactions
> between them to consider, for example specific power-up sequence timings.
>
> ===
>
> The reason for tackling both of these problems in a single series is the
> fact the the platform I'm working on - Qualcomm RB5 - deals with both and
> both need to be addressed in order to enable WLAN and Bluetooth support
> upstream.
>
> The on-board WLAN/BT package - QCA6391 - has a Power Management Unit that
> takes inputs from the host and exposes LDO outputs consumed by the BT and
> WLAN modules which can be powered-up and down independently. However
> a delay of 100ms must be respected between enabling the BT- and
> WLAN-enable GPIOs.
>
> A similar design with a discreet PMU is also employed in other models of
> the WCN family of chips although we can often do without the delays. With
> this series we add support for the WCN7850 as well.
>
> ===
>
> We introduce a new subsystem here - the power sequencing framework. The
> qcom-wcn driver that we add is its first user. It implements the power-up
> sequences for QCA6390 and WCN7850 chips. However - we only use it to
> power-up the bluetooth module in the former. We use it to driver the WLAN
> modules in both. The reason for this is that for WCN7850 we have
> comprehensive bindings already upstream together with existing DT users.
> Porting them to using the pwrseq subsystem can be done separately and in
> an incremental manner once the subsystem itself is upstream. We will also
> have to ensure backward DT compatibility. To avoid overcomplicating this
> series, let's leave it out for now.
>
> ===
>
> This series is logically split into several sections. I'll go
> patch-by-patch and explain each step.
>
> Patches 1/16-5/16:
>
> These contain all relevant DT bindings changes. We add new documents for
> the QCA6390 & WCN7850 PMUs and ATH12K devices as well as extend the bindings
> for the Qualcomm Bluetooth and ATH11K modules with regulators used by them
> in QCA6390.
>
> Patches 6/16-8/16:
>
> These contain changes to device-tree sources for the three platforms we
> work with in this series. We model the PMUs of the WLAN/BT chips as
> top-level platform devices on the device tree. In order to limit the scope
> of this series and not introduce an excessive amount of confusion with
> deprecating DT bindings, we leave the Bluetooth nodes on sm8650 and sm8550
> as is (meaning: they continue to consumer the GPIOs and power inputs from
> the host). As the WCN7850 module doesn't require any specific timings, we can
> incrementally change that later.
>
> In both cases we add WLAN nodes that consume the power outputs of the PMU.
> For QCA6390 we also make the Bluetooth node of the RB5 consume the outputs
> of the PMU - we can do it as the bindings for this chip did not define any
> supply handles prior to this series meaning we are able to get this correct
> right away.
>
> Patches 9/16-12/16:
>
> These contain the bulk of the PCI changes for this series. We introduce
> a simple framework for powering up PCI devices before detecting them on
> the bus.
>
> The general approach is as follows: PCI devices that need special
> treatment before they can be powered up, scanned and bound to their PCI
> drivers must be described on the device-tree as child nodes of the PCI
> port node. These devices will be instantiated on the platform bus. They
> will in fact be generic platform devices with the compatible of the form
> used for PCI devices already upstream ("pci<vendor ID>,<device ID">). We
> add a new directory under drivers/pci/pwrctl/ that contains PCI pwrctl
> drivers. These drivers are platform drivers that will now be matched
> against the devices instantiated from port children just like any other
> platform pairs.
>
> Both the power control platform device *AND* the associated PCI device
> reuse the same OF node and have access to the same properties. The goal
> of the platform driver is to request and bring up any required resources
> and let the pwrctl framework know that it's now OK to rescan the bus and
> detect the devices. When the device is bound, we are notified about it
> by the PCI bus notifier event and can establish a device link between the
> power control device and the PCI device so that any future extension for
> power-management will already be able to work with the correct hierachy.
>
> The reusing of the OF node is the reason for the small changes to the PCI
> OF core: as the bootloader can possibly leave the relevant regulators on
> before booting linux, the PCI device can be detected before its platform
> abstraction is probed. In this case, we find that device first and mark
> its OF node as reused. The pwrctl framework handles the opposite case
> (when the PCI device is detected only after the platform driver
> successfully enabled it).
>
> Patch 13/16 - 14/16:
>
> These add a relatively simple power sequencing subsystem and the first
> driver using it: the pwrseq module for the PMUs on the WCN family of chips.
>
> I'm proposing to add a subsystem that allows different devices to use a shared
> power sequence split into consumer-specific as well as common "units".
>
> A power sequence provider driver registers a set of units with pwrseq
> core. Each unit can be enabled and disabled and contains an optional list
> of other units which must be enabled before it itself can be. A unit
> represents a discreet chunk of the power sequence.
>
> It also registers a list of targets: a target is an abstraction wrapping
> a unit which allows consumers to tell pwrseq which unit they want to
> reach. Real-life example is the driver we're adding here: there's a set
> of common regulators, two PCIe-specific ones and two enable GPIOs: one
> for Bluetooth and one for WLAN.
>
> The Bluetooth driver requests a descriptor to the power sequencer and
> names the target it wants to reach:
>
> pwrseq = devm_pwrseq_get(dev, "bluetooth");
>
> The pwrseq core then knows that when the driver calls:
>
> pwrseq_power_on(pwrseq);
>
> It must enable the "bluetooth-enable" unit but it depends on the
> "regulators-common" unit so this one is enabled first. The provider
> driver is also in charge of assuring an appropriate delay between
> enabling the BT and WLAN enable GPIOs. The WLAN-specific resources are
> handled by the "wlan-enable" unit and so are not enabled until the WLAN
> driver requests the "wlan" target to be powered on.
>
> Another thing worth discussing is the way we associate the consumer with
> the relevant power sequencer. DT maintainers have expressed a discontent
> with the existing mmc pwrseq bindings and have NAKed an earlier
> initiative to introduce global pwrseq bindings to the kernel[1].
>
> In this approach, we model the existing regulators and GPIOs in DT but
> the pwrseq subsystem requires each provider to provide a .match()
> callback. Whenever a consumer requests a power sequencer handle, we
> iterate over the list of pwrseq drivers and call .match() for each. It's
> up to the driver to verify in a platform-specific way whether it deals
> with its consumer and let the core pwrseq code know.
>
> The advantage of this over reusing the regulator or reset subsystem is
> that it's more generalized and can handle resources of all kinds as well
> as deal with any kind of power-on sequences: for instance, Qualcomm has
> a PCI switch they want a driver for but this switch requires enabling
> some resources first (PCI pwrctl) and then configuring the device over
> I2C (which can be handled by the pwrseq provider).
>
> Patch 15:
>
> This patch makes the Qualcomm Bluetooth driver get and use the power
> sequencer for QCA6390.
>
> Patch 16:
>
> While tiny, this patch is possibly the highlight of the entire series.
> It uses the two abstraction layers we introduced before to create an
> elegant power sequencing PCI power control driver and supports the ath11k
> module on QCA6390 and ath12k on WCN7850.
>
> With this series we can now enable BT and WLAN on several new Qualcomm
> boards upstream.
>
> Tested on RB5, sm8650-qrd and sm8550-qrd.
>
> Changelog:
>
> Since v5:
> - unify the approach to modelling the WCN WLAN/BT chips by always exposing
> the PMU node on the device tree and making the WLAN and BT nodes become
> consumers of its power outputs; this includes a major rework of the DT
> sources, bindings and driver code; there's no more a separate PCI
> pwrctl driver for WCN7850, instead its power-up sequence was moved
> into the pwrseq driver common for all WCN chips
> - don't set load_uA from new regulator consumers
> - fix reported kerneldoc issues
> - drop voltage ranges for PMU outputs from DT
> - many minor tweaks and reworks
>
> v1: Original RFC:
>
> https://lore.kernel.org/lkml/20240104130123.37115-1-brgl@bgdev.pl/T/
>
> v2: First real patch series (should have been PATCH v2) adding what I
> referred to back then as PCI power sequencing:
>
> https://lore.kernel.org/linux-arm-kernel/2024021413-grumbling-unlivable-c145@gregkh/T/
>
> v3: RFC for the DT representation of the PMU supplying the WLAN and BT
> modules inside the QCA6391 package (was largely separate from the
> series but probably should have been called PATCH or RFC v3):
>
> https://lore.kernel.org/all/CAMRc=Mc+GNoi57eTQg71DXkQKjdaoAmCpB=h2ndEpGnmdhVV-Q@mail.gmail.com/T/
>
> v4: Second attempt at the full series with changed scope (introduction of
> the pwrseq subsystem, should have been RFC v4)
>
> https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/
>
> v5: Two different ways of handling QCA6390 and WCN7850:
>
> https://lore.kernel.org/lkml/20240216203215.40870-1-brgl@bgdev.pl/
>
> Bartosz Golaszewski (16):
> regulator: dt-bindings: describe the PMU module of the QCA6390 package
> regulator: dt-bindings: describe the PMU module of the WCN7850 package
> dt-bindings: net: bluetooth: qualcomm: describe regulators for QCA6390
> dt-bindings: net: wireless: qcom,ath11k: describe the ath11k on
> QCA6390
> dt-bindings: net: wireless: describe the ath12k PCI module
> arm64: dts: qcom: sm8550-qrd: add the Wifi node
> arm64: dts: qcom: sm8650-qrd: add the Wifi node
> arm64: dts: qcom: qrb5165-rb5: add the Wifi node
> PCI: hold the rescan mutex when scanning for the first time
> PCI/pwrctl: reuse the OF node for power controlled devices
> PCI/pwrctl: create platform devices for child OF nodes of the port
> node
> PCI/pwrctl: add PCI power control core code
> power: sequencing: implement the pwrseq core
> power: pwrseq: add a driver for the PMU module on the QCom WCN
> chipsets
> Bluetooth: qca: use the power sequencer for QCA6390
> PCI/pwrctl: add a PCI power control driver for power sequenced devices
>
> .../net/bluetooth/qualcomm-bluetooth.yaml | 17 +
> .../net/wireless/qcom,ath11k-pci.yaml | 46 +
> .../bindings/net/wireless/qcom,ath12k.yaml | 100 ++
> .../bindings/regulator/qcom,qca6390-pmu.yaml | 185 +++
> MAINTAINERS | 8 +
> arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 103 +-
> arch/arm64/boot/dts/qcom/sm8250.dtsi | 10 +
> arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 97 ++
> arch/arm64/boot/dts/qcom/sm8550.dtsi | 10 +
> arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 89 ++
> arch/arm64/boot/dts/qcom/sm8650.dtsi | 10 +
> drivers/bluetooth/hci_qca.c | 74 +-
> drivers/pci/Kconfig | 1 +
> drivers/pci/Makefile | 1 +
> drivers/pci/bus.c | 9 +-
> drivers/pci/of.c | 14 +-
> drivers/pci/probe.c | 2 +
> drivers/pci/pwrctl/Kconfig | 17 +
> drivers/pci/pwrctl/Makefile | 6 +
> drivers/pci/pwrctl/core.c | 136 +++
> drivers/pci/pwrctl/pci-pwrctl-pwrseq.c | 89 ++
> drivers/pci/remove.c | 2 +
> drivers/power/Kconfig | 1 +
> drivers/power/Makefile | 1 +
> drivers/power/sequencing/Kconfig | 28 +
> drivers/power/sequencing/Makefile | 6 +
> drivers/power/sequencing/core.c | 1065 +++++++++++++++++
> drivers/power/sequencing/pwrseq-qcom-wcn.c | 336 ++++++
> include/linux/pci-pwrctl.h | 51 +
> include/linux/pwrseq/consumer.h | 56 +
> include/linux/pwrseq/provider.h | 75 ++
> 31 files changed, 2614 insertions(+), 31 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml
> create mode 100644 Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml
> create mode 100644 drivers/pci/pwrctl/Kconfig
> create mode 100644 drivers/pci/pwrctl/Makefile
> create mode 100644 drivers/pci/pwrctl/core.c
> create mode 100644 drivers/pci/pwrctl/pci-pwrctl-pwrseq.c
> create mode 100644 drivers/power/sequencing/Kconfig
> create mode 100644 drivers/power/sequencing/Makefile
> create mode 100644 drivers/power/sequencing/core.c
> create mode 100644 drivers/power/sequencing/pwrseq-qcom-wcn.c
> create mode 100644 include/linux/pci-pwrctl.h
> create mode 100644 include/linux/pwrseq/consumer.h
> create mode 100644 include/linux/pwrseq/provider.h
I tested the patchset on SM8550 and it does give me working WiFi. However I
seethe following warnings during boot.
[ 5.973011] mhi mhi0: Requested to power ON
[ 6.597591] mhi mhi0: Power on setup success
[ 6.597631] sysfs: cannot create duplicate filename '/devices/platform/soc@0/1c00000.pcie/pci0000:00/0000:00:00.0/resource0'
[ 6.597634] CPU: 7 PID: 154 Comm: kworker/u32:5 Tainted: G S 6.9.0-rc1-next-20240328-g955237c9980c #1
[ 6.597635] Hardware name: AYN Odin 2 (DT)
[ 6.597637] Workqueue: async async_run_entry_fn
[ 6.597645] Call trace:
[ 6.597646] dump_backtrace+0xa0/0x128
[ 6.597649] show_stack+0x20/0x38
[ 6.597650] dump_stack_lvl+0x74/0x90
[ 6.597653] dump_stack+0x18/0x28
[ 6.597654] sysfs_warn_dup+0x6c/0x90
[ 6.597658] sysfs_add_bin_file_mode_ns+0xdc/0x100
[ 6.597660] sysfs_create_bin_file+0x7c/0xb8
[ 6.597662] pci_create_attr+0xb4/0x1a8
[ 6.597665] pci_create_resource_files+0x64/0xd0
[ 6.597667] pci_create_sysfs_dev_files+0x24/0x40
[ 6.597669] pci_bus_add_device+0x54/0x138
[ 6.597670] pci_bus_add_devices+0x40/0x98
[ 6.597672] pci_host_probe+0x70/0xf0
[ 6.597673] dw_pcie_host_init+0x248/0x658
[ 6.597676] qcom_pcie_probe+0x234/0x330
[ 6.597677] platform_probe+0x70/0xd8
[ 6.597680] really_probe+0xc8/0x3a0
[ 6.597681] __driver_probe_device+0x84/0x170
[ 6.597682] driver_probe_device+0x44/0x120
[ 6.597683] __device_attach_driver+0xc4/0x168
[ 6.597684] bus_for_each_drv+0x8c/0xf0
[ 6.597686] __device_attach_async_helper+0xb4/0x118
[ 6.597687] async_run_entry_fn+0x40/0x178
[ 6.597689] process_one_work+0x16c/0x410
[ 6.597691] worker_thread+0x284/0x3a0
[ 6.597693] kthread+0x118/0x128
[ 6.597693] ret_from_fork+0x10/0x20
[ 6.597698] ------------[ cut here ]------------
[ 6.597698] proc_dir_entry '0000:00/00.0' already registered
[ 6.597710] WARNING: CPU: 7 PID: 154 at fs/proc/generic.c:375 proc_register+0x138/0x1d0
[ 6.597713] Modules linked in:
[ 6.597714] CPU: 7 PID: 154 Comm: kworker/u32:5 Tainted: G S 6.9.0-rc1-next-20240328-g955237c9980c #1
[ 6.597715] Hardware name: AYN Odin 2 (DT)
[ 6.597716] Workqueue: async async_run_entry_fn
[ 6.597718] pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[ 6.597719] pc : proc_register+0x138/0x1d0
[ 6.597721] lr : proc_register+0x138/0x1d0
[ 6.597723] sp : ffff800081e3b9a0
[ 6.597723] x29: ffff800081e3b9a0 x28: 0000000000000000 x27: ffffddb2a28eabe0
[ 6.597725] x26: ffff3425c9ada5c0 x25: ffffddb2a2d4eef0 x24: ffff3425c9ada540
[ 6.597726] x23: 0000000000000004 x22: ffff3425c7b1822c x21: 0000000000000004
[ 6.597727] x20: ffff3425c7b18180 x19: ffff3425c9adaec8 x18: ffffffffffffffff
[ 6.597729] x17: 3040636f732f6d72 x16: 6f6674616c702f73 x15: ffff800081e3b910
[ 6.597730] x14: 0000000000000000 x13: 0a64657265747369 x12: 6765722079646165
[ 6.597731] x11: fffffffffff00000 x10: ffffddb2a27c4fb0 x9 : ffffddb29f5d7528
[ 6.597733] x8 : 00000000ffff7fff x7 : ffffddb2a27c4fb0 x6 : 80000000ffff8000
[ 6.597734] x5 : 0000000000000358 x4 : 0000000000000000 x3 : 00000000ffffffff
[ 6.597736] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff3425c5ce0000
[ 6.597737] Call trace:
[ 6.597737] proc_register+0x138/0x1d0
[ 6.597739] proc_create_data+0x48/0x78
[ 6.597741] pci_proc_attach_device+0x84/0x118
[ 6.597743] pci_bus_add_device+0x5c/0x138
[ 6.597744] pci_bus_add_devices+0x40/0x98
[ 6.597745] pci_host_probe+0x70/0xf0
[ 6.597746] dw_pcie_host_init+0x248/0x658
[ 6.597748] qcom_pcie_probe+0x234/0x330
[ 6.597749] platform_probe+0x70/0xd8
[ 6.597750] really_probe+0xc8/0x3a0
[ 6.597751] __driver_probe_device+0x84/0x170
[ 6.597752] driver_probe_device+0x44/0x120
[ 6.597753] __device_attach_driver+0xc4/0x168
[ 6.597754] bus_for_each_drv+0x8c/0xf0
[ 6.597756] __device_attach_async_helper+0xb4/0x118
[ 6.597757] async_run_entry_fn+0x40/0x178
[ 6.597759] process_one_work+0x16c/0x410
[ 6.597760] worker_thread+0x284/0x3a0
[ 6.597761] kthread+0x118/0x128
[ 6.597762] ret_from_fork+0x10/0x20
[ 6.597763] ---[ end trace 0000000000000000 ]---
This probably only occurs when the relevant drivers on compiled as built-in.
Similar behavior has been noticed before as well:
https://lore.kernel.org/lkml/20240201155532.49707-1-brgl@bgdev.pl/T/#mdeeca9bc8e19458787d53738298abcfff443068a
Thanks,
Xilin
^ permalink raw reply
* Re: [PATCH] arm64: dts: imx8mp: Align both CSI2 pixel clock
From: Marek Vasut @ 2024-04-06 2:58 UTC (permalink / raw)
To: Adam Ford, Laurent Pinchart
Cc: linux-arm-kernel, Conor Dooley, Fabio Estevam,
Krzysztof Kozlowski, Paul Elder, Pengutronix Kernel Team,
Rob Herring, Sascha Hauer, Shawn Guo, devicetree, imx
In-Reply-To: <CAHCN7xKX7v4tmhjvoPLirEoUG91jpu-8R2DV9eE=mnWt=3FffA@mail.gmail.com>
On 4/5/24 11:04 PM, Adam Ford wrote:
> On Fri, Apr 5, 2024 at 3:43 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
>>
>> Hi Marek,
>>
>> (CC'ing Adam)
>>
>> Thank you for the patch.
>>
>> On Fri, Apr 05, 2024 at 10:22:26PM +0200, Marek Vasut wrote:
>>> Configure both CSI2 assigned-clock-rates the same way.
>>> There does not seem to be any reason for keeping the
>>> two CSI2 pixel clock set to different frequencies.
>>
>> There's an issue when using two cameras concurrently. This has been
>> discussed some time ago on the linux-media mailing list, see [1]. Adam
>> knows more than I do on this topic.
>>
>> [1] https://lore.kernel.org/linux-media/CAHCN7x+kymRGO2kxvN2=zLiqRjfTc3hdf3VdNVkWjsW3La0bnA@mail.gmail.com/
>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> ---
>>> Cc: Conor Dooley <conor+dt@kernel.org>
>>> Cc: Fabio Estevam <festevam@gmail.com>
>>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> Cc: Paul Elder <paul.elder@ideasonboard.com>
>>> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>>> Cc: Shawn Guo <shawnguo@kernel.org>
>>> Cc: devicetree@vger.kernel.org
>>> Cc: imx@lists.linux.dev
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> ---
>>> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> index 1bb96e96639f2..2e9ce0c3a9815 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
>>> @@ -1703,7 +1703,7 @@ mipi_csi_1: csi@32e50000 {
>>> <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>;
>>> assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>,
>>> <&clk IMX8MP_CLK_24M>;
>>> - assigned-clock-rates = <266000000>;
>>> + assigned-clock-rates = <500000000>;
>
> I am traveling, so I don't have the technical documents in front of
> me, but I beleive this is an over-drive speed, and 400MHz would be the
> single clock, standard rate. I created an imx8mm-overdrive and
> imx8mn-overdrive dtsi file to let users who operate in overdrive mode
> to update their clocks in one place.
>
> I also think this goes down if the user is running two cameras instead
> of one. I re-read the old thread, and it's coming back to me, but
> until I can get settled into my hotel in Germany, I won't have time to
> review. I think the original idea was to use the lowest, conservative
> value with the idea that people can tweak their clock settings if
> they're only running one and if they are running in over-drive mode.
MX8MPCEC does indeed read 400 MHz regular, 500 MHz overdrive.
Shall we align both CSI2 ports to 400 MHz ? Currently they are one 500
MHz and the other 266 MHz .
^ permalink raw reply
* Re: [PATCH 4/6] drm/msm/adreno: Implement SMEM-based speed bin
From: Dmitry Baryshkov @ 2024-04-06 3:23 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <20240405-topic-smem_speedbin-v1-4-ce2b864251b1@linaro.org>
On Fri, Apr 05, 2024 at 10:41:32AM +0200, Konrad Dybcio wrote:
> On recent (SM8550+) Snapdragon platforms, the GPU speed bin data is
> abstracted through SMEM, instead of being directly available in a fuse.
>
> Add support for SMEM-based speed binning, which includes getting
> "feature code" and "product code" from said source and parsing them
> to form something that lets us match OPPs against.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 8 +++---
> drivers/gpu/drm/msm/adreno/adreno_device.c | 2 ++
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 39 +++++++++++++++++++++++++++---
> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 12 ++++++---
> 4 files changed, 51 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 4cbdfabbcee5..6776fd80f7a6 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -2890,13 +2890,15 @@ static u32 fuse_to_supp_hw(const struct adreno_info *info, u32 fuse)
> return UINT_MAX;
> }
>
> -static int a6xx_set_supported_hw(struct device *dev, const struct adreno_info *info)
> +static int a6xx_set_supported_hw(struct adreno_gpu *adreno_gpu,
> + struct device *dev,
> + const struct adreno_info *info)
> {
> u32 supp_hw;
> u32 speedbin;
> int ret;
>
> - ret = adreno_read_speedbin(dev, &speedbin);
> + ret = adreno_read_speedbin(adreno_gpu, dev, &speedbin);
> /*
> * -ENOENT means that the platform doesn't support speedbin which is
> * fine
> @@ -3056,7 +3058,7 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
>
> a6xx_llc_slices_init(pdev, a6xx_gpu, is_a7xx);
>
> - ret = a6xx_set_supported_hw(&pdev->dev, config->info);
> + ret = a6xx_set_supported_hw(adreno_gpu, &pdev->dev, config->info);
> if (ret) {
> a6xx_destroy(&(a6xx_gpu->base.base));
> return ERR_PTR(ret);
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index c3703a51287b..901ef767e491 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -6,6 +6,8 @@
> * Copyright (c) 2014,2017 The Linux Foundation. All rights reserved.
> */
>
> +#include <linux/soc/qcom/socinfo.h>
> +
> #include "adreno_gpu.h"
>
> bool hang_debug = false;
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 074fb498706f..0e4ff532ac3c 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -21,6 +21,9 @@
> #include "msm_gem.h"
> #include "msm_mmu.h"
>
> +#include <linux/soc/qcom/smem.h>
> +#include <linux/soc/qcom/socinfo.h>
> +
> static u64 address_space_size = 0;
> MODULE_PARM_DESC(address_space_size, "Override for size of processes private GPU address space");
> module_param(address_space_size, ullong, 0600);
> @@ -1057,9 +1060,37 @@ void adreno_gpu_ocmem_cleanup(struct adreno_ocmem *adreno_ocmem)
> adreno_ocmem->hdl);
> }
>
> -int adreno_read_speedbin(struct device *dev, u32 *speedbin)
> +int adreno_read_speedbin(struct adreno_gpu *adreno_gpu,
> + struct device *dev, u32 *speedbin)
> {
> - return nvmem_cell_read_variable_le_u32(dev, "speed_bin", speedbin);
> + u32 fcode, pcode;
> + int ret;
> +
> + /* Try reading the speedbin via a nvmem cell first */
> + ret = nvmem_cell_read_variable_le_u32(dev, "speed_bin", speedbin);
> + if (!ret && ret != -EINVAL)
This is always false.
> + return ret;
> +
> + ret = qcom_smem_get_feature_code(&fcode);
> + if (ret) {
> + dev_err(dev, "Couldn't get feature code from SMEM!\n");
> + return ret;
This brings in QCOM_SMEM dependency (which is not mentioned in the
Kconfig). Please keep iMX5 hardware in mind, so the dependency should be
optional. Respective functions should be stubbed in the header.
> + }
> +
> + ret = qcom_smem_get_product_code(&pcode);
> + if (ret) {
> + dev_err(dev, "Couldn't get product code from SMEM!\n");
> + return ret;
> + }
> +
> + /* Don't consider fcode for external feature codes */
> + if (fcode <= SOCINFO_FC_EXT_RESERVE)
> + fcode = SOCINFO_FC_UNKNOWN;
> +
> + *speedbin = FIELD_PREP(ADRENO_SKU_ID_PCODE, pcode) |
> + FIELD_PREP(ADRENO_SKU_ID_FCODE, fcode);
What about just asking the qcom_smem for the 'gpu_bin' and hiding gory
details there? It almost feels that handling raw PCODE / FCODE here is
too low-level and a subject to change depending on the socinfo format.
> +
> + return ret;
> }
>
> int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
> @@ -1098,9 +1129,9 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
> devm_pm_opp_set_clkname(dev, "core");
> }
>
> - if (adreno_read_speedbin(dev, &speedbin) || !speedbin)
> + if (adreno_read_speedbin(adreno_gpu, dev, &speedbin) || !speedbin)
> speedbin = 0xffff;
> - adreno_gpu->speedbin = (uint16_t) (0xffff & speedbin);
the &= 0xffff should probably go to the adreno_read_speedbin / nvmem
case. WDYT?
> + adreno_gpu->speedbin = speedbin;
>
> gpu_name = devm_kasprintf(dev, GFP_KERNEL, "%"ADRENO_CHIPID_FMT,
> ADRENO_CHIPID_ARGS(config->chip_id));
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 460b399be37b..1770a9e20484 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -81,7 +81,12 @@ extern const struct adreno_reglist a612_hwcg[], a615_hwcg[], a630_hwcg[], a640_h
> extern const struct adreno_reglist a660_hwcg[], a690_hwcg[], a702_hwcg[], a730_hwcg[], a740_hwcg[];
>
> struct adreno_speedbin {
> - uint16_t fuse;
> + /* <= 16-bit for NVMEM fuses, 32b for SOCID values */
> + uint32_t fuse;
> +#define ADRENO_SKU_ID_PCODE GENMASK(31, 16)
> +#define ADRENO_SKU_ID_FCODE GENMASK(15, 0)
> +#define ADRENO_SKU_ID(pcode, fcode) (pcode << 16 | fcode)
> +
> uint16_t speedbin;
> };
>
> @@ -137,7 +142,7 @@ struct adreno_gpu {
> struct msm_gpu base;
> const struct adreno_info *info;
> uint32_t chip_id;
> - uint16_t speedbin;
> + uint32_t speedbin;
> const struct adreno_gpu_funcs *funcs;
>
> /* interesting register offsets to dump: */
> @@ -520,7 +525,8 @@ int adreno_fault_handler(struct msm_gpu *gpu, unsigned long iova, int flags,
> struct adreno_smmu_fault_info *info, const char *block,
> u32 scratch[4]);
>
> -int adreno_read_speedbin(struct device *dev, u32 *speedbin);
> +int adreno_read_speedbin(struct adreno_gpu *adreno_gpu,
> + struct device *dev, u32 *speedbin);
>
> /*
> * For a5xx and a6xx targets load the zap shader that is used to pull the GPU
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 5/6] drm/msm/adreno: Add speedbin data for SM8550 / A740
From: Dmitry Baryshkov @ 2024-04-06 3:25 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <20240405-topic-smem_speedbin-v1-5-ce2b864251b1@linaro.org>
On Fri, Apr 05, 2024 at 10:41:33AM +0200, Konrad Dybcio wrote:
> Add speebin data for A740, as found on SM8550 and derivative SoCs.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> drivers/gpu/drm/msm/adreno/adreno_device.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 901ef767e491..c976a485aef2 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -570,6 +570,20 @@ static const struct adreno_info gpulist[] = {
> .zapfw = "a740_zap.mdt",
> .hwcg = a740_hwcg,
> .address_space_size = SZ_16G,
> + .speedbins = ADRENO_SPEEDBINS(
I think this deserves either a comment or some info in the commit
message.
> + { ADRENO_SKU_ID(SOCINFO_PC_UNKNOWN, SOCINFO_FC_AC), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PC_UNKNOWN, SOCINFO_FC_AF), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(1), SOCINFO_FC_UNKNOWN), 1 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(2), SOCINFO_FC_Yn(0x0)), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(2), SOCINFO_FC_Yn(0x2)), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(4), SOCINFO_FC_Yn(0x0)), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(4), SOCINFO_FC_Yn(0x2)), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0x0)), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0x1)), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0xd)), 0 },
> + { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0xe)), 0 },
> + ),
> + .default_speedbin = 1,
> }, {
> .chip_ids = ADRENO_CHIP_IDS(0x43051401), /* "C520v2" */
> .family = ADRENO_7XX_GEN3,
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 0/6] Add SMEM-based speedbin matching
From: Dmitry Baryshkov @ 2024-04-06 3:28 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, linux-kernel,
dri-devel, freedreno, devicetree, Neil Armstrong
In-Reply-To: <20240405-topic-smem_speedbin-v1-0-ce2b864251b1@linaro.org>
On Fri, Apr 05, 2024 at 10:41:28AM +0200, Konrad Dybcio wrote:
> Newer (SM8550+) SoCs don't seem to have a nice speedbin fuse anymore,
> but instead rely on a set of combinations of "feature code" (FC) and
> "product code" (PC) identifiers to match the bins. This series adds
> support for that.
>
> I suppose a qcom/for-soc immutable branch would be in order if we want
> to land this in the upcoming cycle.
>
> FWIW I preferred the fuses myself..
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> Konrad Dybcio (5):
> soc: qcom: Move some socinfo defines to the header, expand them
> soc: qcom: smem: Add pcode/fcode getters
> drm/msm/adreno: Implement SMEM-based speed bin
> drm/msm/adreno: Add speedbin data for SM8550 / A740
> arm64: dts: qcom: sm8550: Wire up GPU speed bin & more OPPs
>
> Neil Armstrong (1):
> drm/msm/adreno: Allow specifying default speedbin value
Generic comment: as you are reworking speed bins implementaiton, could
you please take a broader look. A5xx just reads nvmem manually. A6xx
uses adreno_read_speedbin(). And then we call adreno_read_speedbin
second time from from adreno_gpu_init(). Can we get to the point where
the function is called only once for all the platforms which implements
speed binning?
--
With best wishes
Dmitry
^ 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