* [PATCH v4 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID
@ 2020-11-20 10:11 ` Alice Guo
0 siblings, 0 replies; 30+ messages in thread
From: Alice Guo @ 2020-11-20 10:11 UTC (permalink / raw)
To: robh+dt, shawnguo, s.hauer, krzk
Cc: linux-imx, peng.fan, devicetree, linux-kernel, linux-arm-kernel
Add DT Binding doc for the Unique ID of i.MX 8M series.
v2: remove the subject prefix "LF-2571-1"
v3: put it into Documentation/devicetree/bindings/arm/fsl.yaml
modify the description of nvmem-cells
use "make ARCH=arm64 dtbs_check" to test it and fix errors
v4: use allOf to limit new version DTS files for i.MX8M to include
"fsl,imx8mm/n/p/q-soc", nvmem-cells and nvmem-cells-names
Signed-off-by: Alice Guo <alice.guo@nxp.com>
---
.../devicetree/bindings/arm/fsl.yaml | 51 +++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 67980dcef66d..d8048323a290 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -918,6 +918,57 @@ properties:
- fsl,s32v234-evb # S32V234-EVB2 Customer Evaluation Board
- const: fsl,s32v234
+ soc:
+ type: object
+ properties:
+ compatible:
+ oneOf:
+ - description: new version compatible for i.MX8M SoCs
+ items:
+ - enum:
+ - fsl,imx8mm-soc
+ - fsl,imx8mn-soc
+ - fsl,imx8mp-soc
+ - fsl,imx8mq-soc
+ - const: simple-bus
+
+ - description: old version compatible for i.MX8M SoCs
+ items:
+ - const: simple-bus
+
+ nvmem-cells:
+ maxItems: 1
+ description: Phandle to the SOC Unique ID provided by a nvmem node
+
+ nvmem-cells-names:
+ const: soc_unique_id
+
+ allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8mm
+ - fsl,imx8mn
+ - fsl,imx8mp
+ - fsl,imx8mq
+
+ then:
+ properties:
+ compatible:
+ items:
+ - enum:
+ - fsl,imx8mm-soc
+ - fsl,imx8mn-soc
+ - fsl,imx8mp-soc
+ - fsl,imx8mq-soc
+ - const: simple-bus
+
+ required:
+ - nvmem-cells
+ - nvmem-cells-names
+
additionalProperties: true
...
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v4 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID
@ 2020-11-20 10:11 ` Alice Guo
0 siblings, 0 replies; 30+ messages in thread
From: Alice Guo @ 2020-11-20 10:11 UTC (permalink / raw)
To: robh+dt, shawnguo, s.hauer, krzk
Cc: devicetree, peng.fan, linux-imx, linux-arm-kernel, linux-kernel
Add DT Binding doc for the Unique ID of i.MX 8M series.
v2: remove the subject prefix "LF-2571-1"
v3: put it into Documentation/devicetree/bindings/arm/fsl.yaml
modify the description of nvmem-cells
use "make ARCH=arm64 dtbs_check" to test it and fix errors
v4: use allOf to limit new version DTS files for i.MX8M to include
"fsl,imx8mm/n/p/q-soc", nvmem-cells and nvmem-cells-names
Signed-off-by: Alice Guo <alice.guo@nxp.com>
---
.../devicetree/bindings/arm/fsl.yaml | 51 +++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
index 67980dcef66d..d8048323a290 100644
--- a/Documentation/devicetree/bindings/arm/fsl.yaml
+++ b/Documentation/devicetree/bindings/arm/fsl.yaml
@@ -918,6 +918,57 @@ properties:
- fsl,s32v234-evb # S32V234-EVB2 Customer Evaluation Board
- const: fsl,s32v234
+ soc:
+ type: object
+ properties:
+ compatible:
+ oneOf:
+ - description: new version compatible for i.MX8M SoCs
+ items:
+ - enum:
+ - fsl,imx8mm-soc
+ - fsl,imx8mn-soc
+ - fsl,imx8mp-soc
+ - fsl,imx8mq-soc
+ - const: simple-bus
+
+ - description: old version compatible for i.MX8M SoCs
+ items:
+ - const: simple-bus
+
+ nvmem-cells:
+ maxItems: 1
+ description: Phandle to the SOC Unique ID provided by a nvmem node
+
+ nvmem-cells-names:
+ const: soc_unique_id
+
+ allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8mm
+ - fsl,imx8mn
+ - fsl,imx8mp
+ - fsl,imx8mq
+
+ then:
+ properties:
+ compatible:
+ items:
+ - enum:
+ - fsl,imx8mm-soc
+ - fsl,imx8mn-soc
+ - fsl,imx8mp-soc
+ - fsl,imx8mq-soc
+ - const: simple-bus
+
+ required:
+ - nvmem-cells
+ - nvmem-cells-names
+
additionalProperties: true
...
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v4 2/4] arm64: dts: imx8m: add SoC ID compatible
2020-11-20 10:11 ` Alice Guo
@ 2020-11-20 10:11 ` Alice Guo
-1 siblings, 0 replies; 30+ messages in thread
From: Alice Guo @ 2020-11-20 10:11 UTC (permalink / raw)
To: robh+dt, shawnguo, s.hauer, krzk
Cc: linux-imx, peng.fan, devicetree, linux-kernel, linux-arm-kernel
Add compatible string to .dtsi files for binding of imx8_soc_info and
device.
v2: remove the subject prefix "LF-2571-2"
v3: none
v4: change subject and commit message, add Reviewed-by
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Alice Guo <alice.guo@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 +-
arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
arch/arm64/boot/dts/freescale/imx8mp.dtsi | 2 +-
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index c824f2615fe8..d457ce815e68 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -257,7 +257,7 @@
};
soc@0 {
- compatible = "simple-bus";
+ compatible = "fsl,imx8mm-soc", "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index a06d2a6268e6..6d3a809a00fd 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -241,7 +241,7 @@
};
soc@0 {
- compatible = "simple-bus";
+ compatible = "fsl,imx8mn-soc", "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index ecccfbb4f5ad..ec6ac523ecfc 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -218,7 +218,7 @@
};
soc@0 {
- compatible = "simple-bus";
+ compatible = "fsl,imx8mp-soc", "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index a841a023e8e0..9b6d9307e5d7 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -286,7 +286,7 @@
};
soc@0 {
- compatible = "simple-bus";
+ compatible = "fsl,imx8mq-soc", "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v4 2/4] arm64: dts: imx8m: add SoC ID compatible
@ 2020-11-20 10:11 ` Alice Guo
0 siblings, 0 replies; 30+ messages in thread
From: Alice Guo @ 2020-11-20 10:11 UTC (permalink / raw)
To: robh+dt, shawnguo, s.hauer, krzk
Cc: devicetree, peng.fan, linux-imx, linux-arm-kernel, linux-kernel
Add compatible string to .dtsi files for binding of imx8_soc_info and
device.
v2: remove the subject prefix "LF-2571-2"
v3: none
v4: change subject and commit message, add Reviewed-by
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Signed-off-by: Alice Guo <alice.guo@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 2 +-
arch/arm64/boot/dts/freescale/imx8mn.dtsi | 2 +-
arch/arm64/boot/dts/freescale/imx8mp.dtsi | 2 +-
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index c824f2615fe8..d457ce815e68 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -257,7 +257,7 @@
};
soc@0 {
- compatible = "simple-bus";
+ compatible = "fsl,imx8mm-soc", "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index a06d2a6268e6..6d3a809a00fd 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -241,7 +241,7 @@
};
soc@0 {
- compatible = "simple-bus";
+ compatible = "fsl,imx8mn-soc", "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index ecccfbb4f5ad..ec6ac523ecfc 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -218,7 +218,7 @@
};
soc@0 {
- compatible = "simple-bus";
+ compatible = "fsl,imx8mp-soc", "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index a841a023e8e0..9b6d9307e5d7 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -286,7 +286,7 @@
};
soc@0 {
- compatible = "simple-bus";
+ compatible = "fsl,imx8mq-soc", "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v4 3/4] arm64: dts: imx8m: add NVMEM provider and consumer to read soc unique ID
2020-11-20 10:11 ` Alice Guo
@ 2020-11-20 10:11 ` Alice Guo
-1 siblings, 0 replies; 30+ messages in thread
From: Alice Guo @ 2020-11-20 10:11 UTC (permalink / raw)
To: robh+dt, shawnguo, s.hauer, krzk
Cc: linux-imx, peng.fan, devicetree, linux-kernel, linux-arm-kernel
In order to be able to use NVMEM APIs to read soc unique ID, add the
nvmem data cell and name for nvmem-cells to the "soc" node, and add a
nvmem node which provides soc unique ID to efuse@30350000.
v2: remove the subject prefix "LF-2571-3"
v3: convert register addresses and sizes to hex
v4: delete "stuff" in subject and commit message, add detailed
description
Signed-off-by: Alice Guo <alice.guo@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 6 ++++++
arch/arm64/boot/dts/freescale/imx8mn.dtsi | 6 ++++++
arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 ++++++
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 6 ++++++
4 files changed, 24 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index d457ce815e68..0e0edd5db07b 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -261,6 +261,8 @@
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
+ nvmem-cells = <&imx8mm_uid>;
+ nvmem-cell-names = "soc_unique_id";
aips1: bus@30000000 {
compatible = "fsl,aips-bus", "simple-bus";
@@ -518,6 +520,10 @@
#address-cells = <1>;
#size-cells = <1>;
+ imx8mm_uid: unique_id@410 {
+ reg = <0x4 0x8>;
+ };
+
cpu_speed_grade: speed-grade@10 {
reg = <0x10 4>;
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index 6d3a809a00fd..ff12194b60a1 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -245,6 +245,8 @@
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
+ nvmem-cells = <&imx8mn_uid>;
+ nvmem-cell-names = "soc_unique_id";
aips1: bus@30000000 {
compatible = "fsl,aips-bus", "simple-bus";
@@ -388,6 +390,10 @@
#address-cells = <1>;
#size-cells = <1>;
+ imx8mn_uid: unique_id@410 {
+ reg = <0x4 0x8>;
+ };
+
cpu_speed_grade: speed-grade@10 {
reg = <0x10 4>;
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index ec6ac523ecfc..8fbcded6a091 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -222,6 +222,8 @@
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
+ nvmem-cells = <&imx8mp_uid>;
+ nvmem-cell-names = "soc_unique_id";
aips1: bus@30000000 {
compatible = "fsl,aips-bus", "simple-bus";
@@ -328,6 +330,10 @@
#address-cells = <1>;
#size-cells = <1>;
+ imx8mp_uid: unique_id@420 {
+ reg = <0x8 0x8>;
+ };
+
cpu_speed_grade: speed-grade@10 {
reg = <0x10 4>;
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 9b6d9307e5d7..6db5cba9c07d 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -291,6 +291,8 @@
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
dma-ranges = <0x40000000 0x0 0x40000000 0xc0000000>;
+ nvmem-cells = <&imx8mq_uid>;
+ nvmem-cell-names = "soc_unique_id";
bus@30000000 { /* AIPS1 */
compatible = "fsl,aips-bus", "simple-bus";
@@ -555,6 +557,10 @@
#address-cells = <1>;
#size-cells = <1>;
+ imx8mq_uid: soc_uid@410 {
+ reg = <0x4 0x8>;
+ };
+
cpu_speed_grade: speed-grade@10 {
reg = <0x10 4>;
};
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v4 3/4] arm64: dts: imx8m: add NVMEM provider and consumer to read soc unique ID
@ 2020-11-20 10:11 ` Alice Guo
0 siblings, 0 replies; 30+ messages in thread
From: Alice Guo @ 2020-11-20 10:11 UTC (permalink / raw)
To: robh+dt, shawnguo, s.hauer, krzk
Cc: devicetree, peng.fan, linux-imx, linux-arm-kernel, linux-kernel
In order to be able to use NVMEM APIs to read soc unique ID, add the
nvmem data cell and name for nvmem-cells to the "soc" node, and add a
nvmem node which provides soc unique ID to efuse@30350000.
v2: remove the subject prefix "LF-2571-3"
v3: convert register addresses and sizes to hex
v4: delete "stuff" in subject and commit message, add detailed
description
Signed-off-by: Alice Guo <alice.guo@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 6 ++++++
arch/arm64/boot/dts/freescale/imx8mn.dtsi | 6 ++++++
arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 ++++++
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 6 ++++++
4 files changed, 24 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index d457ce815e68..0e0edd5db07b 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -261,6 +261,8 @@
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
+ nvmem-cells = <&imx8mm_uid>;
+ nvmem-cell-names = "soc_unique_id";
aips1: bus@30000000 {
compatible = "fsl,aips-bus", "simple-bus";
@@ -518,6 +520,10 @@
#address-cells = <1>;
#size-cells = <1>;
+ imx8mm_uid: unique_id@410 {
+ reg = <0x4 0x8>;
+ };
+
cpu_speed_grade: speed-grade@10 {
reg = <0x10 4>;
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index 6d3a809a00fd..ff12194b60a1 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -245,6 +245,8 @@
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
+ nvmem-cells = <&imx8mn_uid>;
+ nvmem-cell-names = "soc_unique_id";
aips1: bus@30000000 {
compatible = "fsl,aips-bus", "simple-bus";
@@ -388,6 +390,10 @@
#address-cells = <1>;
#size-cells = <1>;
+ imx8mn_uid: unique_id@410 {
+ reg = <0x4 0x8>;
+ };
+
cpu_speed_grade: speed-grade@10 {
reg = <0x10 4>;
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index ec6ac523ecfc..8fbcded6a091 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -222,6 +222,8 @@
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
+ nvmem-cells = <&imx8mp_uid>;
+ nvmem-cell-names = "soc_unique_id";
aips1: bus@30000000 {
compatible = "fsl,aips-bus", "simple-bus";
@@ -328,6 +330,10 @@
#address-cells = <1>;
#size-cells = <1>;
+ imx8mp_uid: unique_id@420 {
+ reg = <0x8 0x8>;
+ };
+
cpu_speed_grade: speed-grade@10 {
reg = <0x10 4>;
};
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 9b6d9307e5d7..6db5cba9c07d 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -291,6 +291,8 @@
#size-cells = <1>;
ranges = <0x0 0x0 0x0 0x3e000000>;
dma-ranges = <0x40000000 0x0 0x40000000 0xc0000000>;
+ nvmem-cells = <&imx8mq_uid>;
+ nvmem-cell-names = "soc_unique_id";
bus@30000000 { /* AIPS1 */
compatible = "fsl,aips-bus", "simple-bus";
@@ -555,6 +557,10 @@
#address-cells = <1>;
#size-cells = <1>;
+ imx8mq_uid: soc_uid@410 {
+ reg = <0x4 0x8>;
+ };
+
cpu_speed_grade: speed-grade@10 {
reg = <0x10 4>;
};
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v4 4/4] soc: imx8m: change to use platform driver
2020-11-20 10:11 ` Alice Guo
@ 2020-11-20 10:11 ` Alice Guo
-1 siblings, 0 replies; 30+ messages in thread
From: Alice Guo @ 2020-11-20 10:11 UTC (permalink / raw)
To: robh+dt, shawnguo, s.hauer, krzk
Cc: linux-imx, peng.fan, devicetree, linux-kernel, linux-arm-kernel
Directly reading ocotp register depends on that bootloader enables ocotp
clk, which is not always effective, so change to use nvmem API. Using
nvmem API requires to support driver defer probe and thus change
soc-imx8m.c to use platform driver.
The other reason is that directly reading ocotp register causes kexec
kernel hang because the 1st kernel running will disable unused clks
after kernel boots up, and then ocotp clk will be disabled even if
bootloader enables it. When kexec kernel, ocotp clk needs to be enabled
before reading ocotp registers, and nvmem API with platform driver
supported can accomplish this.
v2: remove the subject prefix "LF-2571-4"
v3: Keep the original way which uses device_initcall to read soc unique
ID, and add the other way which uses module_platform_driver and
nvmem API, so that it will not break the old version DTBs.
v4: delete "__maybe_unused"
delete MODULE_DEVICE_TABLE(of, imx8m_soc_match);
rename match table, "fsl,imx8mm/n/q/p" is actually a machine
compabile and "fsl,imx8mm/n/q/p-soc" is a compabile of soc@0
delete "flag" and change to determine whether the pointer is NULL
ues of_find_matching_node_and_match()
delete of_match_ptr()
Signed-off-by: Alice Guo <alice.guo@nxp.com>
---
drivers/soc/imx/soc-imx8m.c | 85 +++++++++++++++++++++++++++++++------
1 file changed, 73 insertions(+), 12 deletions(-)
diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
index cc57a384d74d..1b0a34e545ae 100644
--- a/drivers/soc/imx/soc-imx8m.c
+++ b/drivers/soc/imx/soc-imx8m.c
@@ -5,6 +5,8 @@
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
#include <linux/of_address.h>
#include <linux/slab.h>
#include <linux/sys_soc.h>
@@ -29,7 +31,7 @@
struct imx8_soc_data {
char *name;
- u32 (*soc_revision)(void);
+ u32 (*soc_revision)(struct device *dev);
};
static u64 soc_uid;
@@ -50,7 +52,7 @@ static u32 imx8mq_soc_revision_from_atf(void)
static inline u32 imx8mq_soc_revision_from_atf(void) { return 0; };
#endif
-static u32 __init imx8mq_soc_revision(void)
+static u32 __init imx8mq_soc_revision(struct device *dev)
{
struct device_node *np;
void __iomem *ocotp_base;
@@ -75,9 +77,19 @@ static u32 __init imx8mq_soc_revision(void)
rev = REV_B1;
}
- soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
- soc_uid <<= 32;
- soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
+ if (dev) {
+ int ret = 0;
+
+ ret = nvmem_cell_read_u64(dev, "soc_unique_id", &soc_uid);
+ if (ret) {
+ iounmap(ocotp_base);
+ return ret;
+ }
+ } else {
+ soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
+ soc_uid <<= 32;
+ soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
+ }
iounmap(ocotp_base);
of_node_put(np);
@@ -107,7 +119,7 @@ static void __init imx8mm_soc_uid(void)
of_node_put(np);
}
-static u32 __init imx8mm_soc_revision(void)
+static u32 __init imx8mm_soc_revision(struct device *dev)
{
struct device_node *np;
void __iomem *anatop_base;
@@ -125,7 +137,15 @@ static u32 __init imx8mm_soc_revision(void)
iounmap(anatop_base);
of_node_put(np);
- imx8mm_soc_uid();
+ if (dev) {
+ int ret = 0;
+
+ ret = nvmem_cell_read_u64(dev, "soc_unique_id", &soc_uid);
+ if (ret)
+ return ret;
+ } else {
+ imx8mm_soc_uid();
+ }
return rev;
}
@@ -150,7 +170,7 @@ static const struct imx8_soc_data imx8mp_soc_data = {
.soc_revision = imx8mm_soc_revision,
};
-static __maybe_unused const struct of_device_id imx8_soc_match[] = {
+static const struct of_device_id imx8_machine_match[] = {
{ .compatible = "fsl,imx8mq", .data = &imx8mq_soc_data, },
{ .compatible = "fsl,imx8mm", .data = &imx8mm_soc_data, },
{ .compatible = "fsl,imx8mn", .data = &imx8mn_soc_data, },
@@ -158,12 +178,20 @@ static __maybe_unused const struct of_device_id imx8_soc_match[] = {
{ }
};
+static const struct of_device_id imx8_soc_match[] = {
+ { .compatible = "fsl,imx8mq-soc", .data = &imx8mq_soc_data, },
+ { .compatible = "fsl,imx8mm-soc", .data = &imx8mm_soc_data, },
+ { .compatible = "fsl,imx8mn-soc", .data = &imx8mn_soc_data, },
+ { .compatible = "fsl,imx8mp-soc", .data = &imx8mp_soc_data, },
+ { }
+};
+
#define imx8_revision(soc_rev) \
soc_rev ? \
kasprintf(GFP_KERNEL, "%d.%d", (soc_rev >> 4) & 0xf, soc_rev & 0xf) : \
"unknown"
-static int __init imx8_soc_init(void)
+static int imx8_soc_info(struct platform_device *pdev)
{
struct soc_device_attribute *soc_dev_attr;
struct soc_device *soc_dev;
@@ -182,7 +210,10 @@ static int __init imx8_soc_init(void)
if (ret)
goto free_soc;
- id = of_match_node(imx8_soc_match, of_root);
+ if (pdev)
+ id = of_match_node(imx8_soc_match, pdev->dev.of_node);
+ else
+ id = of_match_node(imx8_machine_match, of_root);
if (!id) {
ret = -ENODEV;
goto free_soc;
@@ -191,8 +222,16 @@ static int __init imx8_soc_init(void)
data = id->data;
if (data) {
soc_dev_attr->soc_id = data->name;
- if (data->soc_revision)
- soc_rev = data->soc_revision();
+ if (data->soc_revision) {
+ if (pdev) {
+ soc_rev = data->soc_revision(&pdev->dev);
+ ret = soc_rev;
+ if (ret < 0)
+ goto free_soc;
+ } else {
+ soc_rev = data->soc_revision(NULL);
+ }
+ }
}
soc_dev_attr->revision = imx8_revision(soc_rev);
@@ -230,4 +269,26 @@ static int __init imx8_soc_init(void)
kfree(soc_dev_attr);
return ret;
}
+
+static int __init imx8_soc_init(void)
+{
+ int ret = 0;
+
+ if (of_find_matching_node_and_match(NULL, imx8_soc_match, NULL))
+ return 0;
+
+ ret = imx8_soc_info(NULL);
+ return ret;
+}
device_initcall(imx8_soc_init);
+
+static struct platform_driver imx8_soc_info_driver = {
+ .probe = imx8_soc_info,
+ .driver = {
+ .name = "imx8_soc_info",
+ .of_match_table = imx8_soc_match,
+ },
+};
+
+module_platform_driver(imx8_soc_info_driver);
+MODULE_LICENSE("GPL v2");
--
2.17.1
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v4 4/4] soc: imx8m: change to use platform driver
@ 2020-11-20 10:11 ` Alice Guo
0 siblings, 0 replies; 30+ messages in thread
From: Alice Guo @ 2020-11-20 10:11 UTC (permalink / raw)
To: robh+dt, shawnguo, s.hauer, krzk
Cc: devicetree, peng.fan, linux-imx, linux-arm-kernel, linux-kernel
Directly reading ocotp register depends on that bootloader enables ocotp
clk, which is not always effective, so change to use nvmem API. Using
nvmem API requires to support driver defer probe and thus change
soc-imx8m.c to use platform driver.
The other reason is that directly reading ocotp register causes kexec
kernel hang because the 1st kernel running will disable unused clks
after kernel boots up, and then ocotp clk will be disabled even if
bootloader enables it. When kexec kernel, ocotp clk needs to be enabled
before reading ocotp registers, and nvmem API with platform driver
supported can accomplish this.
v2: remove the subject prefix "LF-2571-4"
v3: Keep the original way which uses device_initcall to read soc unique
ID, and add the other way which uses module_platform_driver and
nvmem API, so that it will not break the old version DTBs.
v4: delete "__maybe_unused"
delete MODULE_DEVICE_TABLE(of, imx8m_soc_match);
rename match table, "fsl,imx8mm/n/q/p" is actually a machine
compabile and "fsl,imx8mm/n/q/p-soc" is a compabile of soc@0
delete "flag" and change to determine whether the pointer is NULL
ues of_find_matching_node_and_match()
delete of_match_ptr()
Signed-off-by: Alice Guo <alice.guo@nxp.com>
---
drivers/soc/imx/soc-imx8m.c | 85 +++++++++++++++++++++++++++++++------
1 file changed, 73 insertions(+), 12 deletions(-)
diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
index cc57a384d74d..1b0a34e545ae 100644
--- a/drivers/soc/imx/soc-imx8m.c
+++ b/drivers/soc/imx/soc-imx8m.c
@@ -5,6 +5,8 @@
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
#include <linux/of_address.h>
#include <linux/slab.h>
#include <linux/sys_soc.h>
@@ -29,7 +31,7 @@
struct imx8_soc_data {
char *name;
- u32 (*soc_revision)(void);
+ u32 (*soc_revision)(struct device *dev);
};
static u64 soc_uid;
@@ -50,7 +52,7 @@ static u32 imx8mq_soc_revision_from_atf(void)
static inline u32 imx8mq_soc_revision_from_atf(void) { return 0; };
#endif
-static u32 __init imx8mq_soc_revision(void)
+static u32 __init imx8mq_soc_revision(struct device *dev)
{
struct device_node *np;
void __iomem *ocotp_base;
@@ -75,9 +77,19 @@ static u32 __init imx8mq_soc_revision(void)
rev = REV_B1;
}
- soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
- soc_uid <<= 32;
- soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
+ if (dev) {
+ int ret = 0;
+
+ ret = nvmem_cell_read_u64(dev, "soc_unique_id", &soc_uid);
+ if (ret) {
+ iounmap(ocotp_base);
+ return ret;
+ }
+ } else {
+ soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
+ soc_uid <<= 32;
+ soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
+ }
iounmap(ocotp_base);
of_node_put(np);
@@ -107,7 +119,7 @@ static void __init imx8mm_soc_uid(void)
of_node_put(np);
}
-static u32 __init imx8mm_soc_revision(void)
+static u32 __init imx8mm_soc_revision(struct device *dev)
{
struct device_node *np;
void __iomem *anatop_base;
@@ -125,7 +137,15 @@ static u32 __init imx8mm_soc_revision(void)
iounmap(anatop_base);
of_node_put(np);
- imx8mm_soc_uid();
+ if (dev) {
+ int ret = 0;
+
+ ret = nvmem_cell_read_u64(dev, "soc_unique_id", &soc_uid);
+ if (ret)
+ return ret;
+ } else {
+ imx8mm_soc_uid();
+ }
return rev;
}
@@ -150,7 +170,7 @@ static const struct imx8_soc_data imx8mp_soc_data = {
.soc_revision = imx8mm_soc_revision,
};
-static __maybe_unused const struct of_device_id imx8_soc_match[] = {
+static const struct of_device_id imx8_machine_match[] = {
{ .compatible = "fsl,imx8mq", .data = &imx8mq_soc_data, },
{ .compatible = "fsl,imx8mm", .data = &imx8mm_soc_data, },
{ .compatible = "fsl,imx8mn", .data = &imx8mn_soc_data, },
@@ -158,12 +178,20 @@ static __maybe_unused const struct of_device_id imx8_soc_match[] = {
{ }
};
+static const struct of_device_id imx8_soc_match[] = {
+ { .compatible = "fsl,imx8mq-soc", .data = &imx8mq_soc_data, },
+ { .compatible = "fsl,imx8mm-soc", .data = &imx8mm_soc_data, },
+ { .compatible = "fsl,imx8mn-soc", .data = &imx8mn_soc_data, },
+ { .compatible = "fsl,imx8mp-soc", .data = &imx8mp_soc_data, },
+ { }
+};
+
#define imx8_revision(soc_rev) \
soc_rev ? \
kasprintf(GFP_KERNEL, "%d.%d", (soc_rev >> 4) & 0xf, soc_rev & 0xf) : \
"unknown"
-static int __init imx8_soc_init(void)
+static int imx8_soc_info(struct platform_device *pdev)
{
struct soc_device_attribute *soc_dev_attr;
struct soc_device *soc_dev;
@@ -182,7 +210,10 @@ static int __init imx8_soc_init(void)
if (ret)
goto free_soc;
- id = of_match_node(imx8_soc_match, of_root);
+ if (pdev)
+ id = of_match_node(imx8_soc_match, pdev->dev.of_node);
+ else
+ id = of_match_node(imx8_machine_match, of_root);
if (!id) {
ret = -ENODEV;
goto free_soc;
@@ -191,8 +222,16 @@ static int __init imx8_soc_init(void)
data = id->data;
if (data) {
soc_dev_attr->soc_id = data->name;
- if (data->soc_revision)
- soc_rev = data->soc_revision();
+ if (data->soc_revision) {
+ if (pdev) {
+ soc_rev = data->soc_revision(&pdev->dev);
+ ret = soc_rev;
+ if (ret < 0)
+ goto free_soc;
+ } else {
+ soc_rev = data->soc_revision(NULL);
+ }
+ }
}
soc_dev_attr->revision = imx8_revision(soc_rev);
@@ -230,4 +269,26 @@ static int __init imx8_soc_init(void)
kfree(soc_dev_attr);
return ret;
}
+
+static int __init imx8_soc_init(void)
+{
+ int ret = 0;
+
+ if (of_find_matching_node_and_match(NULL, imx8_soc_match, NULL))
+ return 0;
+
+ ret = imx8_soc_info(NULL);
+ return ret;
+}
device_initcall(imx8_soc_init);
+
+static struct platform_driver imx8_soc_info_driver = {
+ .probe = imx8_soc_info,
+ .driver = {
+ .name = "imx8_soc_info",
+ .of_match_table = imx8_soc_match,
+ },
+};
+
+module_platform_driver(imx8_soc_info_driver);
+MODULE_LICENSE("GPL v2");
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4 4/4] soc: imx8m: change to use platform driver
2020-11-20 10:11 ` Alice Guo
@ 2020-11-20 10:46 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2020-11-20 10:46 UTC (permalink / raw)
To: Alice Guo
Cc: robh+dt, shawnguo, s.hauer, linux-imx, peng.fan, devicetree,
linux-kernel, linux-arm-kernel
On Fri, Nov 20, 2020 at 06:11:12PM +0800, Alice Guo wrote:
> Directly reading ocotp register depends on that bootloader enables ocotp
> clk, which is not always effective, so change to use nvmem API. Using
> nvmem API requires to support driver defer probe and thus change
> soc-imx8m.c to use platform driver.
>
> The other reason is that directly reading ocotp register causes kexec
> kernel hang because the 1st kernel running will disable unused clks
> after kernel boots up, and then ocotp clk will be disabled even if
> bootloader enables it. When kexec kernel, ocotp clk needs to be enabled
> before reading ocotp registers, and nvmem API with platform driver
> supported can accomplish this.
>
> v2: remove the subject prefix "LF-2571-4"
> v3: Keep the original way which uses device_initcall to read soc unique
> ID, and add the other way which uses module_platform_driver and
> nvmem API, so that it will not break the old version DTBs.
> v4: delete "__maybe_unused"
> delete MODULE_DEVICE_TABLE(of, imx8m_soc_match);
> rename match table, "fsl,imx8mm/n/q/p" is actually a machine
> compabile and "fsl,imx8mm/n/q/p-soc" is a compabile of soc@0
> delete "flag" and change to determine whether the pointer is NULL
> ues of_find_matching_node_and_match()
> delete of_match_ptr()
Put all the patch version changelogs after --- separator, so they will
not go to the commit.
>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> ---
> drivers/soc/imx/soc-imx8m.c | 85 +++++++++++++++++++++++++++++++------
> 1 file changed, 73 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
> index cc57a384d74d..1b0a34e545ae 100644
> --- a/drivers/soc/imx/soc-imx8m.c
> +++ b/drivers/soc/imx/soc-imx8m.c
> @@ -5,6 +5,8 @@
>
> #include <linux/init.h>
> #include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> #include <linux/of_address.h>
> #include <linux/slab.h>
> #include <linux/sys_soc.h>
> @@ -29,7 +31,7 @@
>
> struct imx8_soc_data {
> char *name;
> - u32 (*soc_revision)(void);
> + u32 (*soc_revision)(struct device *dev);
> };
>
> static u64 soc_uid;
> @@ -50,7 +52,7 @@ static u32 imx8mq_soc_revision_from_atf(void)
> static inline u32 imx8mq_soc_revision_from_atf(void) { return 0; };
> #endif
>
> -static u32 __init imx8mq_soc_revision(void)
> +static u32 __init imx8mq_soc_revision(struct device *dev)
> {
> struct device_node *np;
> void __iomem *ocotp_base;
> @@ -75,9 +77,19 @@ static u32 __init imx8mq_soc_revision(void)
> rev = REV_B1;
> }
>
> - soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
> - soc_uid <<= 32;
> - soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
> + if (dev) {
> + int ret = 0;
> +
> + ret = nvmem_cell_read_u64(dev, "soc_unique_id", &soc_uid);
> + if (ret) {
> + iounmap(ocotp_base);
What about other cleanup parts? of_node_put?
> + return ret;
> + }
> + } else {
> + soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
> + soc_uid <<= 32;
> + soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
> + }
>
> iounmap(ocotp_base);
> of_node_put(np);
> @@ -107,7 +119,7 @@ static void __init imx8mm_soc_uid(void)
> of_node_put(np);
> }
>
> -static u32 __init imx8mm_soc_revision(void)
> +static u32 __init imx8mm_soc_revision(struct device *dev)
> {
> struct device_node *np;
> void __iomem *anatop_base;
> @@ -125,7 +137,15 @@ static u32 __init imx8mm_soc_revision(void)
> iounmap(anatop_base);
> of_node_put(np);
>
> - imx8mm_soc_uid();
> + if (dev) {
> + int ret = 0;
> +
> + ret = nvmem_cell_read_u64(dev, "soc_unique_id", &soc_uid);
> + if (ret)
> + return ret;
> + } else {
> + imx8mm_soc_uid();
> + }
>
> return rev;
> }
> @@ -150,7 +170,7 @@ static const struct imx8_soc_data imx8mp_soc_data = {
> .soc_revision = imx8mm_soc_revision,
> };
>
> -static __maybe_unused const struct of_device_id imx8_soc_match[] = {
> +static const struct of_device_id imx8_machine_match[] = {
> { .compatible = "fsl,imx8mq", .data = &imx8mq_soc_data, },
> { .compatible = "fsl,imx8mm", .data = &imx8mm_soc_data, },
> { .compatible = "fsl,imx8mn", .data = &imx8mn_soc_data, },
> @@ -158,12 +178,20 @@ static __maybe_unused const struct of_device_id imx8_soc_match[] = {
> { }
> };
>
> +static const struct of_device_id imx8_soc_match[] = {
> + { .compatible = "fsl,imx8mq-soc", .data = &imx8mq_soc_data, },
> + { .compatible = "fsl,imx8mm-soc", .data = &imx8mm_soc_data, },
> + { .compatible = "fsl,imx8mn-soc", .data = &imx8mn_soc_data, },
> + { .compatible = "fsl,imx8mp-soc", .data = &imx8mp_soc_data, },
> + { }
> +};
> +
> #define imx8_revision(soc_rev) \
> soc_rev ? \
> kasprintf(GFP_KERNEL, "%d.%d", (soc_rev >> 4) & 0xf, soc_rev & 0xf) : \
> "unknown"
>
> -static int __init imx8_soc_init(void)
> +static int imx8_soc_info(struct platform_device *pdev)
> {
> struct soc_device_attribute *soc_dev_attr;
> struct soc_device *soc_dev;
> @@ -182,7 +210,10 @@ static int __init imx8_soc_init(void)
> if (ret)
> goto free_soc;
>
> - id = of_match_node(imx8_soc_match, of_root);
> + if (pdev)
> + id = of_match_node(imx8_soc_match, pdev->dev.of_node);
> + else
> + id = of_match_node(imx8_machine_match, of_root);
> if (!id) {
> ret = -ENODEV;
> goto free_soc;
> @@ -191,8 +222,16 @@ static int __init imx8_soc_init(void)
> data = id->data;
> if (data) {
> soc_dev_attr->soc_id = data->name;
> - if (data->soc_revision)
> - soc_rev = data->soc_revision();
> + if (data->soc_revision) {
> + if (pdev) {
> + soc_rev = data->soc_revision(&pdev->dev);
> + ret = soc_rev;
> + if (ret < 0)
> + goto free_soc;
> + } else {
> + soc_rev = data->soc_revision(NULL);
> + }
> + }
> }
>
> soc_dev_attr->revision = imx8_revision(soc_rev);
> @@ -230,4 +269,26 @@ static int __init imx8_soc_init(void)
> kfree(soc_dev_attr);
> return ret;
> }
> +
> +static int __init imx8_soc_init(void)
> +{
> + int ret = 0;
Skip the initialization.
You need to document why the initcal is still there. Please write that
it is purely for backward compatibility.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 4/4] soc: imx8m: change to use platform driver
@ 2020-11-20 10:46 ` Krzysztof Kozlowski
0 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2020-11-20 10:46 UTC (permalink / raw)
To: Alice Guo
Cc: devicetree, peng.fan, s.hauer, linux-kernel, robh+dt, linux-imx,
shawnguo, linux-arm-kernel
On Fri, Nov 20, 2020 at 06:11:12PM +0800, Alice Guo wrote:
> Directly reading ocotp register depends on that bootloader enables ocotp
> clk, which is not always effective, so change to use nvmem API. Using
> nvmem API requires to support driver defer probe and thus change
> soc-imx8m.c to use platform driver.
>
> The other reason is that directly reading ocotp register causes kexec
> kernel hang because the 1st kernel running will disable unused clks
> after kernel boots up, and then ocotp clk will be disabled even if
> bootloader enables it. When kexec kernel, ocotp clk needs to be enabled
> before reading ocotp registers, and nvmem API with platform driver
> supported can accomplish this.
>
> v2: remove the subject prefix "LF-2571-4"
> v3: Keep the original way which uses device_initcall to read soc unique
> ID, and add the other way which uses module_platform_driver and
> nvmem API, so that it will not break the old version DTBs.
> v4: delete "__maybe_unused"
> delete MODULE_DEVICE_TABLE(of, imx8m_soc_match);
> rename match table, "fsl,imx8mm/n/q/p" is actually a machine
> compabile and "fsl,imx8mm/n/q/p-soc" is a compabile of soc@0
> delete "flag" and change to determine whether the pointer is NULL
> ues of_find_matching_node_and_match()
> delete of_match_ptr()
Put all the patch version changelogs after --- separator, so they will
not go to the commit.
>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> ---
> drivers/soc/imx/soc-imx8m.c | 85 +++++++++++++++++++++++++++++++------
> 1 file changed, 73 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-imx8m.c
> index cc57a384d74d..1b0a34e545ae 100644
> --- a/drivers/soc/imx/soc-imx8m.c
> +++ b/drivers/soc/imx/soc-imx8m.c
> @@ -5,6 +5,8 @@
>
> #include <linux/init.h>
> #include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/nvmem-consumer.h>
> #include <linux/of_address.h>
> #include <linux/slab.h>
> #include <linux/sys_soc.h>
> @@ -29,7 +31,7 @@
>
> struct imx8_soc_data {
> char *name;
> - u32 (*soc_revision)(void);
> + u32 (*soc_revision)(struct device *dev);
> };
>
> static u64 soc_uid;
> @@ -50,7 +52,7 @@ static u32 imx8mq_soc_revision_from_atf(void)
> static inline u32 imx8mq_soc_revision_from_atf(void) { return 0; };
> #endif
>
> -static u32 __init imx8mq_soc_revision(void)
> +static u32 __init imx8mq_soc_revision(struct device *dev)
> {
> struct device_node *np;
> void __iomem *ocotp_base;
> @@ -75,9 +77,19 @@ static u32 __init imx8mq_soc_revision(void)
> rev = REV_B1;
> }
>
> - soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
> - soc_uid <<= 32;
> - soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
> + if (dev) {
> + int ret = 0;
> +
> + ret = nvmem_cell_read_u64(dev, "soc_unique_id", &soc_uid);
> + if (ret) {
> + iounmap(ocotp_base);
What about other cleanup parts? of_node_put?
> + return ret;
> + }
> + } else {
> + soc_uid = readl_relaxed(ocotp_base + OCOTP_UID_HIGH);
> + soc_uid <<= 32;
> + soc_uid |= readl_relaxed(ocotp_base + OCOTP_UID_LOW);
> + }
>
> iounmap(ocotp_base);
> of_node_put(np);
> @@ -107,7 +119,7 @@ static void __init imx8mm_soc_uid(void)
> of_node_put(np);
> }
>
> -static u32 __init imx8mm_soc_revision(void)
> +static u32 __init imx8mm_soc_revision(struct device *dev)
> {
> struct device_node *np;
> void __iomem *anatop_base;
> @@ -125,7 +137,15 @@ static u32 __init imx8mm_soc_revision(void)
> iounmap(anatop_base);
> of_node_put(np);
>
> - imx8mm_soc_uid();
> + if (dev) {
> + int ret = 0;
> +
> + ret = nvmem_cell_read_u64(dev, "soc_unique_id", &soc_uid);
> + if (ret)
> + return ret;
> + } else {
> + imx8mm_soc_uid();
> + }
>
> return rev;
> }
> @@ -150,7 +170,7 @@ static const struct imx8_soc_data imx8mp_soc_data = {
> .soc_revision = imx8mm_soc_revision,
> };
>
> -static __maybe_unused const struct of_device_id imx8_soc_match[] = {
> +static const struct of_device_id imx8_machine_match[] = {
> { .compatible = "fsl,imx8mq", .data = &imx8mq_soc_data, },
> { .compatible = "fsl,imx8mm", .data = &imx8mm_soc_data, },
> { .compatible = "fsl,imx8mn", .data = &imx8mn_soc_data, },
> @@ -158,12 +178,20 @@ static __maybe_unused const struct of_device_id imx8_soc_match[] = {
> { }
> };
>
> +static const struct of_device_id imx8_soc_match[] = {
> + { .compatible = "fsl,imx8mq-soc", .data = &imx8mq_soc_data, },
> + { .compatible = "fsl,imx8mm-soc", .data = &imx8mm_soc_data, },
> + { .compatible = "fsl,imx8mn-soc", .data = &imx8mn_soc_data, },
> + { .compatible = "fsl,imx8mp-soc", .data = &imx8mp_soc_data, },
> + { }
> +};
> +
> #define imx8_revision(soc_rev) \
> soc_rev ? \
> kasprintf(GFP_KERNEL, "%d.%d", (soc_rev >> 4) & 0xf, soc_rev & 0xf) : \
> "unknown"
>
> -static int __init imx8_soc_init(void)
> +static int imx8_soc_info(struct platform_device *pdev)
> {
> struct soc_device_attribute *soc_dev_attr;
> struct soc_device *soc_dev;
> @@ -182,7 +210,10 @@ static int __init imx8_soc_init(void)
> if (ret)
> goto free_soc;
>
> - id = of_match_node(imx8_soc_match, of_root);
> + if (pdev)
> + id = of_match_node(imx8_soc_match, pdev->dev.of_node);
> + else
> + id = of_match_node(imx8_machine_match, of_root);
> if (!id) {
> ret = -ENODEV;
> goto free_soc;
> @@ -191,8 +222,16 @@ static int __init imx8_soc_init(void)
> data = id->data;
> if (data) {
> soc_dev_attr->soc_id = data->name;
> - if (data->soc_revision)
> - soc_rev = data->soc_revision();
> + if (data->soc_revision) {
> + if (pdev) {
> + soc_rev = data->soc_revision(&pdev->dev);
> + ret = soc_rev;
> + if (ret < 0)
> + goto free_soc;
> + } else {
> + soc_rev = data->soc_revision(NULL);
> + }
> + }
> }
>
> soc_dev_attr->revision = imx8_revision(soc_rev);
> @@ -230,4 +269,26 @@ static int __init imx8_soc_init(void)
> kfree(soc_dev_attr);
> return ret;
> }
> +
> +static int __init imx8_soc_init(void)
> +{
> + int ret = 0;
Skip the initialization.
You need to document why the initcal is still there. Please write that
it is purely for backward compatibility.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID
2020-11-20 10:11 ` Alice Guo
@ 2020-11-20 10:50 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2020-11-20 10:50 UTC (permalink / raw)
To: Alice Guo
Cc: robh+dt, shawnguo, s.hauer, linux-imx, peng.fan, devicetree,
linux-kernel, linux-arm-kernel
On Fri, Nov 20, 2020 at 06:11:09PM +0800, Alice Guo wrote:
> Add DT Binding doc for the Unique ID of i.MX 8M series.
>
> v2: remove the subject prefix "LF-2571-1"
> v3: put it into Documentation/devicetree/bindings/arm/fsl.yaml
> modify the description of nvmem-cells
> use "make ARCH=arm64 dtbs_check" to test it and fix errors
> v4: use allOf to limit new version DTS files for i.MX8M to include
> "fsl,imx8mm/n/p/q-soc", nvmem-cells and nvmem-cells-names
>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> ---
> .../devicetree/bindings/arm/fsl.yaml | 51 +++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 67980dcef66d..d8048323a290 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -918,6 +918,57 @@ properties:
> - fsl,s32v234-evb # S32V234-EVB2 Customer Evaluation Board
> - const: fsl,s32v234
>
> + soc:
> + type: object
> + properties:
> + compatible:
> + oneOf:
> + - description: new version compatible for i.MX8M SoCs
> + items:
> + - enum:
> + - fsl,imx8mm-soc
> + - fsl,imx8mn-soc
> + - fsl,imx8mp-soc
> + - fsl,imx8mq-soc
> + - const: simple-bus
> +
> + - description: old version compatible for i.MX8M SoCs
> + items:
> + - const: simple-bus
> +
> + nvmem-cells:
> + maxItems: 1
> + description: Phandle to the SOC Unique ID provided by a nvmem node
> +
> + nvmem-cells-names:
> + const: soc_unique_id
> +
> + allOf:
Nothing changed here comparing to previous version. Still does not
work.
The allOf should not be part of soc node because the "if" below won't
match. Instead, it should be against root node.
Best regards,
Krzysztof
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - fsl,imx8mm
> + - fsl,imx8mn
> + - fsl,imx8mp
> + - fsl,imx8mq
> +
> + then:
> + properties:
> + compatible:
> + items:
> + - enum:
> + - fsl,imx8mm-soc
> + - fsl,imx8mn-soc
> + - fsl,imx8mp-soc
> + - fsl,imx8mq-soc
> + - const: simple-bus
> +
> + required:
> + - nvmem-cells
> + - nvmem-cells-names
> +
> additionalProperties: true
>
> ...
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID
@ 2020-11-20 10:50 ` Krzysztof Kozlowski
0 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2020-11-20 10:50 UTC (permalink / raw)
To: Alice Guo
Cc: devicetree, peng.fan, s.hauer, linux-kernel, robh+dt, linux-imx,
shawnguo, linux-arm-kernel
On Fri, Nov 20, 2020 at 06:11:09PM +0800, Alice Guo wrote:
> Add DT Binding doc for the Unique ID of i.MX 8M series.
>
> v2: remove the subject prefix "LF-2571-1"
> v3: put it into Documentation/devicetree/bindings/arm/fsl.yaml
> modify the description of nvmem-cells
> use "make ARCH=arm64 dtbs_check" to test it and fix errors
> v4: use allOf to limit new version DTS files for i.MX8M to include
> "fsl,imx8mm/n/p/q-soc", nvmem-cells and nvmem-cells-names
>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> ---
> .../devicetree/bindings/arm/fsl.yaml | 51 +++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 67980dcef66d..d8048323a290 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -918,6 +918,57 @@ properties:
> - fsl,s32v234-evb # S32V234-EVB2 Customer Evaluation Board
> - const: fsl,s32v234
>
> + soc:
> + type: object
> + properties:
> + compatible:
> + oneOf:
> + - description: new version compatible for i.MX8M SoCs
> + items:
> + - enum:
> + - fsl,imx8mm-soc
> + - fsl,imx8mn-soc
> + - fsl,imx8mp-soc
> + - fsl,imx8mq-soc
> + - const: simple-bus
> +
> + - description: old version compatible for i.MX8M SoCs
> + items:
> + - const: simple-bus
> +
> + nvmem-cells:
> + maxItems: 1
> + description: Phandle to the SOC Unique ID provided by a nvmem node
> +
> + nvmem-cells-names:
> + const: soc_unique_id
> +
> + allOf:
Nothing changed here comparing to previous version. Still does not
work.
The allOf should not be part of soc node because the "if" below won't
match. Instead, it should be against root node.
Best regards,
Krzysztof
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - fsl,imx8mm
> + - fsl,imx8mn
> + - fsl,imx8mp
> + - fsl,imx8mq
> +
> + then:
> + properties:
> + compatible:
> + items:
> + - enum:
> + - fsl,imx8mm-soc
> + - fsl,imx8mn-soc
> + - fsl,imx8mp-soc
> + - fsl,imx8mq-soc
> + - const: simple-bus
> +
> + required:
> + - nvmem-cells
> + - nvmem-cells-names
> +
> additionalProperties: true
>
> ...
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 3/4] arm64: dts: imx8m: add NVMEM provider and consumer to read soc unique ID
2020-11-20 10:11 ` Alice Guo
@ 2020-11-20 10:53 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2020-11-20 10:53 UTC (permalink / raw)
To: Alice Guo
Cc: robh+dt, shawnguo, s.hauer, linux-imx, peng.fan, devicetree,
linux-kernel, linux-arm-kernel
On Fri, Nov 20, 2020 at 06:11:11PM +0800, Alice Guo wrote:
> In order to be able to use NVMEM APIs to read soc unique ID, add the
> nvmem data cell and name for nvmem-cells to the "soc" node, and add a
> nvmem node which provides soc unique ID to efuse@30350000.
>
> v2: remove the subject prefix "LF-2571-3"
> v3: convert register addresses and sizes to hex
> v4: delete "stuff" in subject and commit message, add detailed
> description
>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> ---
> arch/arm64/boot/dts/freescale/imx8mm.dtsi | 6 ++++++
> arch/arm64/boot/dts/freescale/imx8mn.dtsi | 6 ++++++
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 ++++++
> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 6 ++++++
> 4 files changed, 24 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index d457ce815e68..0e0edd5db07b 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -261,6 +261,8 @@
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x0 0x0 0x0 0x3e000000>;
> + nvmem-cells = <&imx8mm_uid>;
> + nvmem-cell-names = "soc_unique_id";
>
> aips1: bus@30000000 {
> compatible = "fsl,aips-bus", "simple-bus";
> @@ -518,6 +520,10 @@
> #address-cells = <1>;
> #size-cells = <1>;
>
> + imx8mm_uid: unique_id@410 {
Any reason why device node uses underscore, not a hyphen/dash? Other
fields are proper (it's also naming convention of DT spec and dtc W=2
will point it).
Best regards,
Krzysztof
> + reg = <0x4 0x8>;
> + };
> +
> cpu_speed_grade: speed-grade@10 {
> reg = <0x10 4>;
> };
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 3/4] arm64: dts: imx8m: add NVMEM provider and consumer to read soc unique ID
@ 2020-11-20 10:53 ` Krzysztof Kozlowski
0 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2020-11-20 10:53 UTC (permalink / raw)
To: Alice Guo
Cc: devicetree, peng.fan, s.hauer, linux-kernel, robh+dt, linux-imx,
shawnguo, linux-arm-kernel
On Fri, Nov 20, 2020 at 06:11:11PM +0800, Alice Guo wrote:
> In order to be able to use NVMEM APIs to read soc unique ID, add the
> nvmem data cell and name for nvmem-cells to the "soc" node, and add a
> nvmem node which provides soc unique ID to efuse@30350000.
>
> v2: remove the subject prefix "LF-2571-3"
> v3: convert register addresses and sizes to hex
> v4: delete "stuff" in subject and commit message, add detailed
> description
>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> ---
> arch/arm64/boot/dts/freescale/imx8mm.dtsi | 6 ++++++
> arch/arm64/boot/dts/freescale/imx8mn.dtsi | 6 ++++++
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 6 ++++++
> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 6 ++++++
> 4 files changed, 24 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index d457ce815e68..0e0edd5db07b 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -261,6 +261,8 @@
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x0 0x0 0x0 0x3e000000>;
> + nvmem-cells = <&imx8mm_uid>;
> + nvmem-cell-names = "soc_unique_id";
>
> aips1: bus@30000000 {
> compatible = "fsl,aips-bus", "simple-bus";
> @@ -518,6 +520,10 @@
> #address-cells = <1>;
> #size-cells = <1>;
>
> + imx8mm_uid: unique_id@410 {
Any reason why device node uses underscore, not a hyphen/dash? Other
fields are proper (it's also naming convention of DT spec and dtc W=2
will point it).
Best regards,
Krzysztof
> + reg = <0x4 0x8>;
> + };
> +
> cpu_speed_grade: speed-grade@10 {
> reg = <0x10 4>;
> };
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [EXT] Re: [PATCH v4 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID
2020-11-20 10:50 ` Krzysztof Kozlowski
@ 2020-11-23 4:45 ` Alice Guo
-1 siblings, 0 replies; 30+ messages in thread
From: Alice Guo @ 2020-11-23 4:45 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: robh+dt, shawnguo, s.hauer, dl-linux-imx, Peng Fan, devicetree,
linux-kernel, linux-arm-kernel
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 2020年11月20日 18:51
> To: Alice Guo <alice.guo@nxp.com>
> Cc: robh+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> dl-linux-imx <linux-imx@nxp.com>; Peng Fan <peng.fan@nxp.com>;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org
> Subject: [EXT] Re: [PATCH v4 1/4] dt-bindings: soc: imx8m: add DT Binding doc
> for soc unique ID
>
> Caution: EXT Email
>
> On Fri, Nov 20, 2020 at 06:11:09PM +0800, Alice Guo wrote:
> > Add DT Binding doc for the Unique ID of i.MX 8M series.
> >
> > v2: remove the subject prefix "LF-2571-1"
> > v3: put it into Documentation/devicetree/bindings/arm/fsl.yaml
> > modify the description of nvmem-cells
> > use "make ARCH=arm64 dtbs_check" to test it and fix errors
> > v4: use allOf to limit new version DTS files for i.MX8M to include
> > "fsl,imx8mm/n/p/q-soc", nvmem-cells and nvmem-cells-names
> >
> > Signed-off-by: Alice Guo <alice.guo@nxp.com>
> > ---
> > .../devicetree/bindings/arm/fsl.yaml | 51
> +++++++++++++++++++
> > 1 file changed, 51 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml
> > b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index 67980dcef66d..d8048323a290 100644
> > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > @@ -918,6 +918,57 @@ properties:
> > - fsl,s32v234-evb # S32V234-EVB2 Customer
> Evaluation Board
> > - const: fsl,s32v234
> >
> > + soc:
> > + type: object
> > + properties:
> > + compatible:
> > + oneOf:
> > + - description: new version compatible for i.MX8M SoCs
> > + items:
> > + - enum:
> > + - fsl,imx8mm-soc
> > + - fsl,imx8mn-soc
> > + - fsl,imx8mp-soc
> > + - fsl,imx8mq-soc
> > + - const: simple-bus
> > +
> > + - description: old version compatible for i.MX8M SoCs
> > + items:
> > + - const: simple-bus
> > +
> > + nvmem-cells:
> > + maxItems: 1
> > + description: Phandle to the SOC Unique ID provided by a nvmem
> > + node
> > +
> > + nvmem-cells-names:
> > + const: soc_unique_id
> > +
> > + allOf:
>
> Nothing changed here comparing to previous version. Still does not work.
>
> The allOf should not be part of soc node because the "if" below won't match.
> Instead, it should be against root node.
>
> Best regards,
> Krzysztof
I'm sorry to disturb you. I don't very clear about the grammar rules of yaml, so don't know how to make allOf be part of root node.
If allof is part of root node, how does it specify the soc node? The following is my change which is definitely wrong.
soc:
properties:
compatible:
- description: new version compatible for i.MX8M SoCs
items:
- enum:
- fsl,imx8mm-soc
- fsl,imx8mn-soc
- fsl,imx8mp-soc
- fsl,imx8mq-soc
- const: simple-bus
nvmem-cells:
maxItems: 1
description: Phandle to the SOC Unique ID provided by a nvmem node
nvmem-cells-names:
const: soc_unique_id
allOf:
- if:
properties:
compatible:
contains:
enum:
- fsl,imx8mm
- fsl,imx8mn
- fsl,imx8mp
- fsl,imx8mq
then:
required:
- soc
Please give me suggestion. Thank you.
Best regards,
Alice Guo
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - fsl,imx8mm
> > + - fsl,imx8mn
> > + - fsl,imx8mp
> > + - fsl,imx8mq
> > +
> > + then:
> > + properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - fsl,imx8mm-soc
> > + - fsl,imx8mn-soc
> > + - fsl,imx8mp-soc
> > + - fsl,imx8mq-soc
> > + - const: simple-bus
> > +
> > + required:
> > + - nvmem-cells
> > + - nvmem-cells-names
> > +
> > additionalProperties: true
> >
> > ...
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [EXT] Re: [PATCH v4 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID
@ 2020-11-23 4:45 ` Alice Guo
0 siblings, 0 replies; 30+ messages in thread
From: Alice Guo @ 2020-11-23 4:45 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: devicetree, Peng Fan, s.hauer, linux-kernel, robh+dt,
dl-linux-imx, shawnguo, linux-arm-kernel
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 2020年11月20日 18:51
> To: Alice Guo <alice.guo@nxp.com>
> Cc: robh+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> dl-linux-imx <linux-imx@nxp.com>; Peng Fan <peng.fan@nxp.com>;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org
> Subject: [EXT] Re: [PATCH v4 1/4] dt-bindings: soc: imx8m: add DT Binding doc
> for soc unique ID
>
> Caution: EXT Email
>
> On Fri, Nov 20, 2020 at 06:11:09PM +0800, Alice Guo wrote:
> > Add DT Binding doc for the Unique ID of i.MX 8M series.
> >
> > v2: remove the subject prefix "LF-2571-1"
> > v3: put it into Documentation/devicetree/bindings/arm/fsl.yaml
> > modify the description of nvmem-cells
> > use "make ARCH=arm64 dtbs_check" to test it and fix errors
> > v4: use allOf to limit new version DTS files for i.MX8M to include
> > "fsl,imx8mm/n/p/q-soc", nvmem-cells and nvmem-cells-names
> >
> > Signed-off-by: Alice Guo <alice.guo@nxp.com>
> > ---
> > .../devicetree/bindings/arm/fsl.yaml | 51
> +++++++++++++++++++
> > 1 file changed, 51 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml
> > b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index 67980dcef66d..d8048323a290 100644
> > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > @@ -918,6 +918,57 @@ properties:
> > - fsl,s32v234-evb # S32V234-EVB2 Customer
> Evaluation Board
> > - const: fsl,s32v234
> >
> > + soc:
> > + type: object
> > + properties:
> > + compatible:
> > + oneOf:
> > + - description: new version compatible for i.MX8M SoCs
> > + items:
> > + - enum:
> > + - fsl,imx8mm-soc
> > + - fsl,imx8mn-soc
> > + - fsl,imx8mp-soc
> > + - fsl,imx8mq-soc
> > + - const: simple-bus
> > +
> > + - description: old version compatible for i.MX8M SoCs
> > + items:
> > + - const: simple-bus
> > +
> > + nvmem-cells:
> > + maxItems: 1
> > + description: Phandle to the SOC Unique ID provided by a nvmem
> > + node
> > +
> > + nvmem-cells-names:
> > + const: soc_unique_id
> > +
> > + allOf:
>
> Nothing changed here comparing to previous version. Still does not work.
>
> The allOf should not be part of soc node because the "if" below won't match.
> Instead, it should be against root node.
>
> Best regards,
> Krzysztof
I'm sorry to disturb you. I don't very clear about the grammar rules of yaml, so don't know how to make allOf be part of root node.
If allof is part of root node, how does it specify the soc node? The following is my change which is definitely wrong.
soc:
properties:
compatible:
- description: new version compatible for i.MX8M SoCs
items:
- enum:
- fsl,imx8mm-soc
- fsl,imx8mn-soc
- fsl,imx8mp-soc
- fsl,imx8mq-soc
- const: simple-bus
nvmem-cells:
maxItems: 1
description: Phandle to the SOC Unique ID provided by a nvmem node
nvmem-cells-names:
const: soc_unique_id
allOf:
- if:
properties:
compatible:
contains:
enum:
- fsl,imx8mm
- fsl,imx8mn
- fsl,imx8mp
- fsl,imx8mq
then:
required:
- soc
Please give me suggestion. Thank you.
Best regards,
Alice Guo
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - fsl,imx8mm
> > + - fsl,imx8mn
> > + - fsl,imx8mp
> > + - fsl,imx8mq
> > +
> > + then:
> > + properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - fsl,imx8mm-soc
> > + - fsl,imx8mn-soc
> > + - fsl,imx8mp-soc
> > + - fsl,imx8mq-soc
> > + - const: simple-bus
> > +
> > + required:
> > + - nvmem-cells
> > + - nvmem-cells-names
> > +
> > additionalProperties: true
> >
> > ...
> > --
> > 2.17.1
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [EXT] Re: [PATCH v4 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID
2020-11-23 4:45 ` Alice Guo
@ 2020-11-23 9:04 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2020-11-23 9:04 UTC (permalink / raw)
To: Alice Guo
Cc: robh+dt, shawnguo, s.hauer, dl-linux-imx, Peng Fan, devicetree,
linux-kernel, linux-arm-kernel
On Mon, Nov 23, 2020 at 04:45:13AM +0000, Alice Guo wrote:
>
>
> > -----Original Message-----
> > From: Krzysztof Kozlowski <krzk@kernel.org>
> > Sent: 2020年11月20日 18:51
> > To: Alice Guo <alice.guo@nxp.com>
> > Cc: robh+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > dl-linux-imx <linux-imx@nxp.com>; Peng Fan <peng.fan@nxp.com>;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org
> > Subject: [EXT] Re: [PATCH v4 1/4] dt-bindings: soc: imx8m: add DT Binding doc
> > for soc unique ID
> >
> > Caution: EXT Email
> >
> > On Fri, Nov 20, 2020 at 06:11:09PM +0800, Alice Guo wrote:
> > > Add DT Binding doc for the Unique ID of i.MX 8M series.
> > >
> > > v2: remove the subject prefix "LF-2571-1"
> > > v3: put it into Documentation/devicetree/bindings/arm/fsl.yaml
> > > modify the description of nvmem-cells
> > > use "make ARCH=arm64 dtbs_check" to test it and fix errors
> > > v4: use allOf to limit new version DTS files for i.MX8M to include
> > > "fsl,imx8mm/n/p/q-soc", nvmem-cells and nvmem-cells-names
> > >
> > > Signed-off-by: Alice Guo <alice.guo@nxp.com>
> > > ---
> > > .../devicetree/bindings/arm/fsl.yaml | 51
> > +++++++++++++++++++
> > > 1 file changed, 51 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml
> > > b/Documentation/devicetree/bindings/arm/fsl.yaml
> > > index 67980dcef66d..d8048323a290 100644
> > > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > > @@ -918,6 +918,57 @@ properties:
> > > - fsl,s32v234-evb # S32V234-EVB2 Customer
> > Evaluation Board
> > > - const: fsl,s32v234
> > >
> > > + soc:
> > > + type: object
> > > + properties:
> > > + compatible:
> > > + oneOf:
> > > + - description: new version compatible for i.MX8M SoCs
> > > + items:
> > > + - enum:
> > > + - fsl,imx8mm-soc
> > > + - fsl,imx8mn-soc
> > > + - fsl,imx8mp-soc
> > > + - fsl,imx8mq-soc
> > > + - const: simple-bus
> > > +
> > > + - description: old version compatible for i.MX8M SoCs
> > > + items:
> > > + - const: simple-bus
> > > +
> > > + nvmem-cells:
> > > + maxItems: 1
> > > + description: Phandle to the SOC Unique ID provided by a nvmem
> > > + node
> > > +
> > > + nvmem-cells-names:
> > > + const: soc_unique_id
> > > +
> > > + allOf:
> >
> > Nothing changed here comparing to previous version. Still does not work.
> >
> > The allOf should not be part of soc node because the "if" below won't match.
> > Instead, it should be against root node.
> >
> > Best regards,
> > Krzysztof
>
> I'm sorry to disturb you. I don't very clear about the grammar rules of yaml, so don't know how to make allOf be part of root node.
> If allof is part of root node, how does it specify the soc node? The following is my change which is definitely wrong.
> soc:
> properties:
> compatible:
> - description: new version compatible for i.MX8M SoCs
> items:
> - enum:
> - fsl,imx8mm-soc
> - fsl,imx8mn-soc
> - fsl,imx8mp-soc
> - fsl,imx8mq-soc
> - const: simple-bus
>
> nvmem-cells:
> maxItems: 1
> description: Phandle to the SOC Unique ID provided by a nvmem node
>
> nvmem-cells-names:
> const: soc_unique_id
>
> allOf:
> - if:
> properties:
> compatible:
> contains:
> enum:
> - fsl,imx8mm
> - fsl,imx8mn
> - fsl,imx8mp
> - fsl,imx8mq
>
> then:
> required:
> - soc
>
> Please give me suggestion. Thank you.
This should work:
940 allOf:
941 - if:
942 properties:
943 compatible:
944 contains:
945 enum:
946 - fsl,imx8mm
947 - fsl,imx8mn
948 - fsl,imx8mp
949 - fsl,imx8mq
950
951 then:
952 patternProperties:
953 "^soc@[0-9a-f]+$":
954 properties:
955 compatible:
956 items:
957 - enum:
958 - fsl,imx8mm-soc
959 - fsl,imx8mn-soc
960 - fsl,imx8mp-soc
961 - fsl,imx8mq-soc
962 - const: simple-bus
963
964 required:
965 - nvmem-cells
966 - nvmem-cells-names
And probablt "soc" should also be added to required, just below the
"then:".
You need to test it and find the proper solution.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [EXT] Re: [PATCH v4 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID
@ 2020-11-23 9:04 ` Krzysztof Kozlowski
0 siblings, 0 replies; 30+ messages in thread
From: Krzysztof Kozlowski @ 2020-11-23 9:04 UTC (permalink / raw)
To: Alice Guo
Cc: devicetree, Peng Fan, s.hauer, linux-kernel, robh+dt,
dl-linux-imx, shawnguo, linux-arm-kernel
On Mon, Nov 23, 2020 at 04:45:13AM +0000, Alice Guo wrote:
>
>
> > -----Original Message-----
> > From: Krzysztof Kozlowski <krzk@kernel.org>
> > Sent: 2020年11月20日 18:51
> > To: Alice Guo <alice.guo@nxp.com>
> > Cc: robh+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> > dl-linux-imx <linux-imx@nxp.com>; Peng Fan <peng.fan@nxp.com>;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org
> > Subject: [EXT] Re: [PATCH v4 1/4] dt-bindings: soc: imx8m: add DT Binding doc
> > for soc unique ID
> >
> > Caution: EXT Email
> >
> > On Fri, Nov 20, 2020 at 06:11:09PM +0800, Alice Guo wrote:
> > > Add DT Binding doc for the Unique ID of i.MX 8M series.
> > >
> > > v2: remove the subject prefix "LF-2571-1"
> > > v3: put it into Documentation/devicetree/bindings/arm/fsl.yaml
> > > modify the description of nvmem-cells
> > > use "make ARCH=arm64 dtbs_check" to test it and fix errors
> > > v4: use allOf to limit new version DTS files for i.MX8M to include
> > > "fsl,imx8mm/n/p/q-soc", nvmem-cells and nvmem-cells-names
> > >
> > > Signed-off-by: Alice Guo <alice.guo@nxp.com>
> > > ---
> > > .../devicetree/bindings/arm/fsl.yaml | 51
> > +++++++++++++++++++
> > > 1 file changed, 51 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml
> > > b/Documentation/devicetree/bindings/arm/fsl.yaml
> > > index 67980dcef66d..d8048323a290 100644
> > > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > > @@ -918,6 +918,57 @@ properties:
> > > - fsl,s32v234-evb # S32V234-EVB2 Customer
> > Evaluation Board
> > > - const: fsl,s32v234
> > >
> > > + soc:
> > > + type: object
> > > + properties:
> > > + compatible:
> > > + oneOf:
> > > + - description: new version compatible for i.MX8M SoCs
> > > + items:
> > > + - enum:
> > > + - fsl,imx8mm-soc
> > > + - fsl,imx8mn-soc
> > > + - fsl,imx8mp-soc
> > > + - fsl,imx8mq-soc
> > > + - const: simple-bus
> > > +
> > > + - description: old version compatible for i.MX8M SoCs
> > > + items:
> > > + - const: simple-bus
> > > +
> > > + nvmem-cells:
> > > + maxItems: 1
> > > + description: Phandle to the SOC Unique ID provided by a nvmem
> > > + node
> > > +
> > > + nvmem-cells-names:
> > > + const: soc_unique_id
> > > +
> > > + allOf:
> >
> > Nothing changed here comparing to previous version. Still does not work.
> >
> > The allOf should not be part of soc node because the "if" below won't match.
> > Instead, it should be against root node.
> >
> > Best regards,
> > Krzysztof
>
> I'm sorry to disturb you. I don't very clear about the grammar rules of yaml, so don't know how to make allOf be part of root node.
> If allof is part of root node, how does it specify the soc node? The following is my change which is definitely wrong.
> soc:
> properties:
> compatible:
> - description: new version compatible for i.MX8M SoCs
> items:
> - enum:
> - fsl,imx8mm-soc
> - fsl,imx8mn-soc
> - fsl,imx8mp-soc
> - fsl,imx8mq-soc
> - const: simple-bus
>
> nvmem-cells:
> maxItems: 1
> description: Phandle to the SOC Unique ID provided by a nvmem node
>
> nvmem-cells-names:
> const: soc_unique_id
>
> allOf:
> - if:
> properties:
> compatible:
> contains:
> enum:
> - fsl,imx8mm
> - fsl,imx8mn
> - fsl,imx8mp
> - fsl,imx8mq
>
> then:
> required:
> - soc
>
> Please give me suggestion. Thank you.
This should work:
940 allOf:
941 - if:
942 properties:
943 compatible:
944 contains:
945 enum:
946 - fsl,imx8mm
947 - fsl,imx8mn
948 - fsl,imx8mp
949 - fsl,imx8mq
950
951 then:
952 patternProperties:
953 "^soc@[0-9a-f]+$":
954 properties:
955 compatible:
956 items:
957 - enum:
958 - fsl,imx8mm-soc
959 - fsl,imx8mn-soc
960 - fsl,imx8mp-soc
961 - fsl,imx8mq-soc
962 - const: simple-bus
963
964 required:
965 - nvmem-cells
966 - nvmem-cells-names
And probablt "soc" should also be added to required, just below the
"then:".
You need to test it and find the proper solution.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* regression due to soc_device_match not handling defer (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
2020-11-20 10:11 ` Alice Guo
@ 2021-03-29 9:08 ` Dominique MARTINET
-1 siblings, 0 replies; 30+ messages in thread
From: Dominique MARTINET @ 2021-03-29 9:08 UTC (permalink / raw)
To: Alice Guo, Shawn Guo, Krzysztof Kozlowski
Cc: robh+dt, devicetree, peng.fan, linux-imx, linux-arm-kernel, linux-kernel
Hi,
First thanks for the patch, it fixes the kexec hang I was looking at...
Unfortunately it means the soc is now init much later and other piece of
drivers that depend on querying the soc will fail, I am getting a BUG in
the caam crypto driver from 7d981405d0fd ("soc: imx8m: change to use
platform driver") + ce58459d8c7f ("arm64: dts: imx8m: add SoC ID
compatible") on the imx8mp evk as follow:
[ 2.575505] caam 30900000.crypto: caam pkc algorithms registered in /proc/crypto
[ 2.588986] caam 30900000.crypto: registering rng-caam
[ 2.594168] caam_jr 30901000.jr: job ring error: irqstate: 00000103
[ 2.600492] ------------[ cut here ]------------
[ 2.605109] kernel BUG at drivers/crypto/caam/jr.c:187!
[ 2.610338] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[ 2.615829] Modules linked in:
[ 2.618895] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.12.0-rc5 #8
[ 2.625168] Hardware name: NXP i.MX8MPlus EVK board (DT)
[ 2.630482] pstate: 60000085 (nZCv daIf -PAN -UAO -TCO BTYPE=--)
[ 2.636493] pc : caam_jr_interrupt+0x150/0x154
[ 2.640944] lr : caam_jr_interrupt+0x150/0x154
[ 2.645396] sp : ffff800010003e90
[ 2.648709] x29: ffff800010003e90 x28: ffff800011f82e80
[ 2.654035] x27: ffff800011cd2000 x26: ffff0000c1988400
[ 2.659353] x25: ffff800011cd2000 x24: ffff800011f789e0
[ 2.664674] x23: 000000000000002e x22: ffff800012261000
[ 2.669994] x21: ffff0000c1979410 x20: ffff0000c1988a80
[ 2.675315] x19: 0000000000000103 x18: 0000000000000000
[ 2.680637] x17: 0000000000000007 x16: 000000000000000e
[ 2.685958] x15: 0000000000000030 x14: ffffffffffffffff
[ 2.691279] x13: ffff800090003aa7 x12: ffff800010003aaf
[ 2.696601] x11: 0000000000000003 x10: 0101010101010101
[ 2.701921] x9 : ffff8000100039d0 x8 : ffff800011fa3830
[ 2.707242] x7 : ffff800011ffb830 x6 : ffff800011ffb830
[ 2.712560] x5 : 0000000000000000 x4 : 0000000000000000
[ 2.717881] x3 : 0000000000000000 x2 : 0000000000000000
[ 2.723203] x1 : 0000000000000000 x0 : ffff800011f82e80
[ 2.728528] Call trace:
[ 2.730976] caam_jr_interrupt+0x150/0x154
[ 2.735080] __handle_irq_event_percpu+0x6c/0x280
[ 2.739791] handle_irq_event+0x70/0x160
[ 2.743716] handle_fasteoi_irq+0xb0/0x200
[ 2.747822] __handle_domain_irq+0x8c/0xf0
[ 2.751924] gic_handle_irq+0xd8/0x160
[ 2.755683] el1_irq+0xb4/0x180
[ 2.758829] arch_cpu_idle+0x18/0x30
[ 2.762412] default_idle_call+0x4c/0x1d0
[ 2.766431] do_idle+0x238/0x2b0
[ 2.769664] cpu_startup_entry+0x30/0x7c
[ 2.773595] rest_init+0xe4/0xf4
[ 2.776832] arch_call_rest_init+0x1c/0x28
[ 2.780937] start_kernel+0x570/0x5a8
[ 2.784602] 0x0
[ 2.786452] Code: aa1503e0 90005c41 91108021 940da95d (d4210000)
[ 2.792560] ---[ end trace 968b8515172abc2e ]---
[ 2.797181] Kernel panic - not syncing: Oops - BUG: Fatal exception in interrupt
[ 2.804580] SMP: stopping secondary CPUs
[ 2.808508] Kernel Offset: disabled
[ 2.811998] CPU features: 0x00240002,2000200c
[ 2.816360] Memory Limit: none
[ 2.819419] ---[ end Kernel panic - not syncing: Oops - BUG: Fatal exception in interrupt ]---
This particular crash can be fixed by making the caam driver delay as
well if the device isn't inited yet, e.g. this works:
--------
diff --git a/drivers/base/soc.c b/drivers/base/soc.c
index 0af5363a582c..f179f9e55b49 100644
--- a/drivers/base/soc.c
+++ b/drivers/base/soc.c
@@ -36,6 +36,7 @@ static DEVICE_ATTR(family, 0444, soc_info_show, NULL);
static DEVICE_ATTR(serial_number, 0444, soc_info_show, NULL);
static DEVICE_ATTR(soc_id, 0444, soc_info_show, NULL);
static DEVICE_ATTR(revision, 0444, soc_info_show, NULL);
+static int init_done;
struct device *soc_device_to_device(struct soc_device *soc_dev)
{
@@ -157,6 +158,7 @@ struct soc_device *soc_device_register(struct soc_device_attribute *soc_dev_attr
return ERR_PTR(ret);
}
+ init_done = true;
return soc_dev;
out3:
@@ -243,6 +245,9 @@ const struct soc_device_attribute *soc_device_match(
{
int ret = 0;
+ if (!init_done)
+ return ERR_PTR(-EPROBE_DEFER);
+
if (!matches)
return NULL;
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index ca0361b2dbb0..d08f8cc4131f 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -635,6 +635,9 @@ static int caam_probe(struct platform_device *pdev)
nprop = pdev->dev.of_node;
imx_soc_match = soc_device_match(caam_imx_soc_table);
+ if (IS_ERR(imx_soc_match))
+ return PTR_ERR(imx_soc_match);
+
caam_imx = (bool)imx_soc_match;
if (imx_soc_match) {
-------
But it obviously doesn't cover the ~50 (!) other soc_device_match users
in the code base which would all need to start handling potential error
return code.
(I also had problems with other drivers when trying to backport the
patch to the 5.4.70_2.3.0 imx kernel but I just gave up on that one)
I think having all drivers handle potential EPROBE_DEFER errors is the
best way forward, how do you propose to move on?
I can do some but have no way of testing most of these so am a bit
reluctant to...
Thanks,
--
Dominique
^ permalink raw reply related [flat|nested] 30+ messages in thread
* regression due to soc_device_match not handling defer (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
@ 2021-03-29 9:08 ` Dominique MARTINET
0 siblings, 0 replies; 30+ messages in thread
From: Dominique MARTINET @ 2021-03-29 9:08 UTC (permalink / raw)
To: Alice Guo, Shawn Guo, Krzysztof Kozlowski
Cc: robh+dt, devicetree, peng.fan, linux-imx, linux-arm-kernel, linux-kernel
Hi,
First thanks for the patch, it fixes the kexec hang I was looking at...
Unfortunately it means the soc is now init much later and other piece of
drivers that depend on querying the soc will fail, I am getting a BUG in
the caam crypto driver from 7d981405d0fd ("soc: imx8m: change to use
platform driver") + ce58459d8c7f ("arm64: dts: imx8m: add SoC ID
compatible") on the imx8mp evk as follow:
[ 2.575505] caam 30900000.crypto: caam pkc algorithms registered in /proc/crypto
[ 2.588986] caam 30900000.crypto: registering rng-caam
[ 2.594168] caam_jr 30901000.jr: job ring error: irqstate: 00000103
[ 2.600492] ------------[ cut here ]------------
[ 2.605109] kernel BUG at drivers/crypto/caam/jr.c:187!
[ 2.610338] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[ 2.615829] Modules linked in:
[ 2.618895] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.12.0-rc5 #8
[ 2.625168] Hardware name: NXP i.MX8MPlus EVK board (DT)
[ 2.630482] pstate: 60000085 (nZCv daIf -PAN -UAO -TCO BTYPE=--)
[ 2.636493] pc : caam_jr_interrupt+0x150/0x154
[ 2.640944] lr : caam_jr_interrupt+0x150/0x154
[ 2.645396] sp : ffff800010003e90
[ 2.648709] x29: ffff800010003e90 x28: ffff800011f82e80
[ 2.654035] x27: ffff800011cd2000 x26: ffff0000c1988400
[ 2.659353] x25: ffff800011cd2000 x24: ffff800011f789e0
[ 2.664674] x23: 000000000000002e x22: ffff800012261000
[ 2.669994] x21: ffff0000c1979410 x20: ffff0000c1988a80
[ 2.675315] x19: 0000000000000103 x18: 0000000000000000
[ 2.680637] x17: 0000000000000007 x16: 000000000000000e
[ 2.685958] x15: 0000000000000030 x14: ffffffffffffffff
[ 2.691279] x13: ffff800090003aa7 x12: ffff800010003aaf
[ 2.696601] x11: 0000000000000003 x10: 0101010101010101
[ 2.701921] x9 : ffff8000100039d0 x8 : ffff800011fa3830
[ 2.707242] x7 : ffff800011ffb830 x6 : ffff800011ffb830
[ 2.712560] x5 : 0000000000000000 x4 : 0000000000000000
[ 2.717881] x3 : 0000000000000000 x2 : 0000000000000000
[ 2.723203] x1 : 0000000000000000 x0 : ffff800011f82e80
[ 2.728528] Call trace:
[ 2.730976] caam_jr_interrupt+0x150/0x154
[ 2.735080] __handle_irq_event_percpu+0x6c/0x280
[ 2.739791] handle_irq_event+0x70/0x160
[ 2.743716] handle_fasteoi_irq+0xb0/0x200
[ 2.747822] __handle_domain_irq+0x8c/0xf0
[ 2.751924] gic_handle_irq+0xd8/0x160
[ 2.755683] el1_irq+0xb4/0x180
[ 2.758829] arch_cpu_idle+0x18/0x30
[ 2.762412] default_idle_call+0x4c/0x1d0
[ 2.766431] do_idle+0x238/0x2b0
[ 2.769664] cpu_startup_entry+0x30/0x7c
[ 2.773595] rest_init+0xe4/0xf4
[ 2.776832] arch_call_rest_init+0x1c/0x28
[ 2.780937] start_kernel+0x570/0x5a8
[ 2.784602] 0x0
[ 2.786452] Code: aa1503e0 90005c41 91108021 940da95d (d4210000)
[ 2.792560] ---[ end trace 968b8515172abc2e ]---
[ 2.797181] Kernel panic - not syncing: Oops - BUG: Fatal exception in interrupt
[ 2.804580] SMP: stopping secondary CPUs
[ 2.808508] Kernel Offset: disabled
[ 2.811998] CPU features: 0x00240002,2000200c
[ 2.816360] Memory Limit: none
[ 2.819419] ---[ end Kernel panic - not syncing: Oops - BUG: Fatal exception in interrupt ]---
This particular crash can be fixed by making the caam driver delay as
well if the device isn't inited yet, e.g. this works:
--------
diff --git a/drivers/base/soc.c b/drivers/base/soc.c
index 0af5363a582c..f179f9e55b49 100644
--- a/drivers/base/soc.c
+++ b/drivers/base/soc.c
@@ -36,6 +36,7 @@ static DEVICE_ATTR(family, 0444, soc_info_show, NULL);
static DEVICE_ATTR(serial_number, 0444, soc_info_show, NULL);
static DEVICE_ATTR(soc_id, 0444, soc_info_show, NULL);
static DEVICE_ATTR(revision, 0444, soc_info_show, NULL);
+static int init_done;
struct device *soc_device_to_device(struct soc_device *soc_dev)
{
@@ -157,6 +158,7 @@ struct soc_device *soc_device_register(struct soc_device_attribute *soc_dev_attr
return ERR_PTR(ret);
}
+ init_done = true;
return soc_dev;
out3:
@@ -243,6 +245,9 @@ const struct soc_device_attribute *soc_device_match(
{
int ret = 0;
+ if (!init_done)
+ return ERR_PTR(-EPROBE_DEFER);
+
if (!matches)
return NULL;
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index ca0361b2dbb0..d08f8cc4131f 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -635,6 +635,9 @@ static int caam_probe(struct platform_device *pdev)
nprop = pdev->dev.of_node;
imx_soc_match = soc_device_match(caam_imx_soc_table);
+ if (IS_ERR(imx_soc_match))
+ return PTR_ERR(imx_soc_match);
+
caam_imx = (bool)imx_soc_match;
if (imx_soc_match) {
-------
But it obviously doesn't cover the ~50 (!) other soc_device_match users
in the code base which would all need to start handling potential error
return code.
(I also had problems with other drivers when trying to backport the
patch to the 5.4.70_2.3.0 imx kernel but I just gave up on that one)
I think having all drivers handle potential EPROBE_DEFER errors is the
best way forward, how do you propose to move on?
I can do some but have no way of testing most of these so am a bit
reluctant to...
Thanks,
--
Dominique
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 30+ messages in thread
* RE: [EXT] regression due to soc_device_match not handling defer (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
2021-03-29 9:08 ` Dominique MARTINET
@ 2021-03-30 2:41 ` Alice Guo (OSS)
-1 siblings, 0 replies; 30+ messages in thread
From: Alice Guo (OSS) @ 2021-03-30 2:41 UTC (permalink / raw)
To: Dominique MARTINET, Shawn Guo, Krzysztof Kozlowski
Cc: robh+dt, devicetree, Peng Fan, dl-linux-imx, linux-arm-kernel,
linux-kernel
Hi,
Thanks for reporting this issue, I'll check and add a fix to handle defer probe.
Best regards,
Alice Guo
> -----Original Message-----
> From: Dominique MARTINET <dominique.martinet@atmark-techno.com>
> Sent: 2021年3月29日 17:09
> To: Alice Guo <alice.guo@nxp.com>; Shawn Guo <shawnguo@kernel.org>;
> Krzysztof Kozlowski <krzk@kernel.org>
> Cc: robh+dt@kernel.org; devicetree@vger.kernel.org; Peng Fan
> <peng.fan@nxp.com>; dl-linux-imx <linux-imx@nxp.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [EXT] regression due to soc_device_match not handling defer (Was:
> [PATCH v4 4/4] soc: imx8m: change to use platform driver)
>
> Caution: EXT Email
>
> Hi,
>
> First thanks for the patch, it fixes the kexec hang I was looking at...
>
> Unfortunately it means the soc is now init much later and other piece of drivers
> that depend on querying the soc will fail, I am getting a BUG in the caam crypto
> driver from 7d981405d0fd ("soc: imx8m: change to use platform driver") +
> ce58459d8c7f ("arm64: dts: imx8m: add SoC ID
> compatible") on the imx8mp evk as follow:
>
> [ 2.575505] caam 30900000.crypto: caam pkc algorithms registered in
> /proc/crypto
> [ 2.588986] caam 30900000.crypto: registering rng-caam
> [ 2.594168] caam_jr 30901000.jr: job ring error: irqstate: 00000103
> [ 2.600492] ------------[ cut here ]------------
> [ 2.605109] kernel BUG at drivers/crypto/caam/jr.c:187!
> [ 2.610338] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> [ 2.615829] Modules linked in:
> [ 2.618895] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.12.0-rc5 #8
> [ 2.625168] Hardware name: NXP i.MX8MPlus EVK board (DT)
> [ 2.630482] pstate: 60000085 (nZCv daIf -PAN -UAO -TCO BTYPE=--)
> [ 2.636493] pc : caam_jr_interrupt+0x150/0x154
> [ 2.640944] lr : caam_jr_interrupt+0x150/0x154
> [ 2.645396] sp : ffff800010003e90
> [ 2.648709] x29: ffff800010003e90 x28: ffff800011f82e80
> [ 2.654035] x27: ffff800011cd2000 x26: ffff0000c1988400
> [ 2.659353] x25: ffff800011cd2000 x24: ffff800011f789e0
> [ 2.664674] x23: 000000000000002e x22: ffff800012261000
> [ 2.669994] x21: ffff0000c1979410 x20: ffff0000c1988a80
> [ 2.675315] x19: 0000000000000103 x18: 0000000000000000
> [ 2.680637] x17: 0000000000000007 x16: 000000000000000e
> [ 2.685958] x15: 0000000000000030 x14: ffffffffffffffff
> [ 2.691279] x13: ffff800090003aa7 x12: ffff800010003aaf
> [ 2.696601] x11: 0000000000000003 x10: 0101010101010101
> [ 2.701921] x9 : ffff8000100039d0 x8 : ffff800011fa3830
> [ 2.707242] x7 : ffff800011ffb830 x6 : ffff800011ffb830
> [ 2.712560] x5 : 0000000000000000 x4 : 0000000000000000
> [ 2.717881] x3 : 0000000000000000 x2 : 0000000000000000
> [ 2.723203] x1 : 0000000000000000 x0 : ffff800011f82e80
> [ 2.728528] Call trace:
> [ 2.730976] caam_jr_interrupt+0x150/0x154
> [ 2.735080] __handle_irq_event_percpu+0x6c/0x280
> [ 2.739791] handle_irq_event+0x70/0x160
> [ 2.743716] handle_fasteoi_irq+0xb0/0x200
> [ 2.747822] __handle_domain_irq+0x8c/0xf0
> [ 2.751924] gic_handle_irq+0xd8/0x160
> [ 2.755683] el1_irq+0xb4/0x180
> [ 2.758829] arch_cpu_idle+0x18/0x30
> [ 2.762412] default_idle_call+0x4c/0x1d0
> [ 2.766431] do_idle+0x238/0x2b0
> [ 2.769664] cpu_startup_entry+0x30/0x7c
> [ 2.773595] rest_init+0xe4/0xf4
> [ 2.776832] arch_call_rest_init+0x1c/0x28
> [ 2.780937] start_kernel+0x570/0x5a8
> [ 2.784602] 0x0
> [ 2.786452] Code: aa1503e0 90005c41 91108021 940da95d (d4210000)
> [ 2.792560] ---[ end trace 968b8515172abc2e ]---
> [ 2.797181] Kernel panic - not syncing: Oops - BUG: Fatal exception in
> interrupt
> [ 2.804580] SMP: stopping secondary CPUs
> [ 2.808508] Kernel Offset: disabled
> [ 2.811998] CPU features: 0x00240002,2000200c
> [ 2.816360] Memory Limit: none
> [ 2.819419] ---[ end Kernel panic - not syncing: Oops - BUG: Fatal exception
> in interrupt ]---
>
>
> This particular crash can be fixed by making the caam driver delay as well if the
> device isn't inited yet, e.g. this works:
> --------
> diff --git a/drivers/base/soc.c b/drivers/base/soc.c index
> 0af5363a582c..f179f9e55b49 100644
> --- a/drivers/base/soc.c
> +++ b/drivers/base/soc.c
> @@ -36,6 +36,7 @@ static DEVICE_ATTR(family, 0444,
> soc_info_show, NULL);
> static DEVICE_ATTR(serial_number, 0444, soc_info_show, NULL);
> static DEVICE_ATTR(soc_id, 0444, soc_info_show, NULL);
> static DEVICE_ATTR(revision, 0444, soc_info_show, NULL);
> +static int init_done;
>
> struct device *soc_device_to_device(struct soc_device *soc_dev) { @@
> -157,6 +158,7 @@ struct soc_device *soc_device_register(struct
> soc_device_attribute *soc_dev_attr
> return ERR_PTR(ret);
> }
>
> + init_done = true;
> return soc_dev;
>
> out3:
> @@ -243,6 +245,9 @@ const struct soc_device_attribute
> *soc_device_match( {
> int ret = 0;
>
> + if (!init_done)
> + return ERR_PTR(-EPROBE_DEFER);
> +
> if (!matches)
> return NULL;
>
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index
> ca0361b2dbb0..d08f8cc4131f 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -635,6 +635,9 @@ static int caam_probe(struct platform_device *pdev)
> nprop = pdev->dev.of_node;
>
> imx_soc_match = soc_device_match(caam_imx_soc_table);
> + if (IS_ERR(imx_soc_match))
> + return PTR_ERR(imx_soc_match);
> +
> caam_imx = (bool)imx_soc_match;
>
> if (imx_soc_match) {
> -------
>
> But it obviously doesn't cover the ~50 (!) other soc_device_match users in the
> code base which would all need to start handling potential error return code.
>
> (I also had problems with other drivers when trying to backport the patch to the
> 5.4.70_2.3.0 imx kernel but I just gave up on that one)
>
>
> I think having all drivers handle potential EPROBE_DEFER errors is the best way
> forward, how do you propose to move on?
> I can do some but have no way of testing most of these so am a bit reluctant
> to...
>
> Thanks,
> --
> Dominique
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [EXT] regression due to soc_device_match not handling defer (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
@ 2021-03-30 2:41 ` Alice Guo (OSS)
0 siblings, 0 replies; 30+ messages in thread
From: Alice Guo (OSS) @ 2021-03-30 2:41 UTC (permalink / raw)
To: Dominique MARTINET, Shawn Guo, Krzysztof Kozlowski
Cc: robh+dt, devicetree, Peng Fan, dl-linux-imx, linux-arm-kernel,
linux-kernel
Hi,
Thanks for reporting this issue, I'll check and add a fix to handle defer probe.
Best regards,
Alice Guo
> -----Original Message-----
> From: Dominique MARTINET <dominique.martinet@atmark-techno.com>
> Sent: 2021年3月29日 17:09
> To: Alice Guo <alice.guo@nxp.com>; Shawn Guo <shawnguo@kernel.org>;
> Krzysztof Kozlowski <krzk@kernel.org>
> Cc: robh+dt@kernel.org; devicetree@vger.kernel.org; Peng Fan
> <peng.fan@nxp.com>; dl-linux-imx <linux-imx@nxp.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [EXT] regression due to soc_device_match not handling defer (Was:
> [PATCH v4 4/4] soc: imx8m: change to use platform driver)
>
> Caution: EXT Email
>
> Hi,
>
> First thanks for the patch, it fixes the kexec hang I was looking at...
>
> Unfortunately it means the soc is now init much later and other piece of drivers
> that depend on querying the soc will fail, I am getting a BUG in the caam crypto
> driver from 7d981405d0fd ("soc: imx8m: change to use platform driver") +
> ce58459d8c7f ("arm64: dts: imx8m: add SoC ID
> compatible") on the imx8mp evk as follow:
>
> [ 2.575505] caam 30900000.crypto: caam pkc algorithms registered in
> /proc/crypto
> [ 2.588986] caam 30900000.crypto: registering rng-caam
> [ 2.594168] caam_jr 30901000.jr: job ring error: irqstate: 00000103
> [ 2.600492] ------------[ cut here ]------------
> [ 2.605109] kernel BUG at drivers/crypto/caam/jr.c:187!
> [ 2.610338] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> [ 2.615829] Modules linked in:
> [ 2.618895] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.12.0-rc5 #8
> [ 2.625168] Hardware name: NXP i.MX8MPlus EVK board (DT)
> [ 2.630482] pstate: 60000085 (nZCv daIf -PAN -UAO -TCO BTYPE=--)
> [ 2.636493] pc : caam_jr_interrupt+0x150/0x154
> [ 2.640944] lr : caam_jr_interrupt+0x150/0x154
> [ 2.645396] sp : ffff800010003e90
> [ 2.648709] x29: ffff800010003e90 x28: ffff800011f82e80
> [ 2.654035] x27: ffff800011cd2000 x26: ffff0000c1988400
> [ 2.659353] x25: ffff800011cd2000 x24: ffff800011f789e0
> [ 2.664674] x23: 000000000000002e x22: ffff800012261000
> [ 2.669994] x21: ffff0000c1979410 x20: ffff0000c1988a80
> [ 2.675315] x19: 0000000000000103 x18: 0000000000000000
> [ 2.680637] x17: 0000000000000007 x16: 000000000000000e
> [ 2.685958] x15: 0000000000000030 x14: ffffffffffffffff
> [ 2.691279] x13: ffff800090003aa7 x12: ffff800010003aaf
> [ 2.696601] x11: 0000000000000003 x10: 0101010101010101
> [ 2.701921] x9 : ffff8000100039d0 x8 : ffff800011fa3830
> [ 2.707242] x7 : ffff800011ffb830 x6 : ffff800011ffb830
> [ 2.712560] x5 : 0000000000000000 x4 : 0000000000000000
> [ 2.717881] x3 : 0000000000000000 x2 : 0000000000000000
> [ 2.723203] x1 : 0000000000000000 x0 : ffff800011f82e80
> [ 2.728528] Call trace:
> [ 2.730976] caam_jr_interrupt+0x150/0x154
> [ 2.735080] __handle_irq_event_percpu+0x6c/0x280
> [ 2.739791] handle_irq_event+0x70/0x160
> [ 2.743716] handle_fasteoi_irq+0xb0/0x200
> [ 2.747822] __handle_domain_irq+0x8c/0xf0
> [ 2.751924] gic_handle_irq+0xd8/0x160
> [ 2.755683] el1_irq+0xb4/0x180
> [ 2.758829] arch_cpu_idle+0x18/0x30
> [ 2.762412] default_idle_call+0x4c/0x1d0
> [ 2.766431] do_idle+0x238/0x2b0
> [ 2.769664] cpu_startup_entry+0x30/0x7c
> [ 2.773595] rest_init+0xe4/0xf4
> [ 2.776832] arch_call_rest_init+0x1c/0x28
> [ 2.780937] start_kernel+0x570/0x5a8
> [ 2.784602] 0x0
> [ 2.786452] Code: aa1503e0 90005c41 91108021 940da95d (d4210000)
> [ 2.792560] ---[ end trace 968b8515172abc2e ]---
> [ 2.797181] Kernel panic - not syncing: Oops - BUG: Fatal exception in
> interrupt
> [ 2.804580] SMP: stopping secondary CPUs
> [ 2.808508] Kernel Offset: disabled
> [ 2.811998] CPU features: 0x00240002,2000200c
> [ 2.816360] Memory Limit: none
> [ 2.819419] ---[ end Kernel panic - not syncing: Oops - BUG: Fatal exception
> in interrupt ]---
>
>
> This particular crash can be fixed by making the caam driver delay as well if the
> device isn't inited yet, e.g. this works:
> --------
> diff --git a/drivers/base/soc.c b/drivers/base/soc.c index
> 0af5363a582c..f179f9e55b49 100644
> --- a/drivers/base/soc.c
> +++ b/drivers/base/soc.c
> @@ -36,6 +36,7 @@ static DEVICE_ATTR(family, 0444,
> soc_info_show, NULL);
> static DEVICE_ATTR(serial_number, 0444, soc_info_show, NULL);
> static DEVICE_ATTR(soc_id, 0444, soc_info_show, NULL);
> static DEVICE_ATTR(revision, 0444, soc_info_show, NULL);
> +static int init_done;
>
> struct device *soc_device_to_device(struct soc_device *soc_dev) { @@
> -157,6 +158,7 @@ struct soc_device *soc_device_register(struct
> soc_device_attribute *soc_dev_attr
> return ERR_PTR(ret);
> }
>
> + init_done = true;
> return soc_dev;
>
> out3:
> @@ -243,6 +245,9 @@ const struct soc_device_attribute
> *soc_device_match( {
> int ret = 0;
>
> + if (!init_done)
> + return ERR_PTR(-EPROBE_DEFER);
> +
> if (!matches)
> return NULL;
>
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index
> ca0361b2dbb0..d08f8cc4131f 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -635,6 +635,9 @@ static int caam_probe(struct platform_device *pdev)
> nprop = pdev->dev.of_node;
>
> imx_soc_match = soc_device_match(caam_imx_soc_table);
> + if (IS_ERR(imx_soc_match))
> + return PTR_ERR(imx_soc_match);
> +
> caam_imx = (bool)imx_soc_match;
>
> if (imx_soc_match) {
> -------
>
> But it obviously doesn't cover the ~50 (!) other soc_device_match users in the
> code base which would all need to start handling potential error return code.
>
> (I also had problems with other drivers when trying to backport the patch to the
> 5.4.70_2.3.0 imx kernel but I just gave up on that one)
>
>
> I think having all drivers handle potential EPROBE_DEFER errors is the best way
> forward, how do you propose to move on?
> I can do some but have no way of testing most of these so am a bit reluctant
> to...
>
> Thanks,
> --
> Dominique
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [EXT] regression due to soc_device_match not handling defer (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
2021-03-30 2:41 ` Alice Guo (OSS)
@ 2021-04-15 1:27 ` Dominique MARTINET
-1 siblings, 0 replies; 30+ messages in thread
From: Dominique MARTINET @ 2021-04-15 1:27 UTC (permalink / raw)
To: Alice Guo (OSS)
Cc: Shawn Guo, Krzysztof Kozlowski, robh+dt, devicetree, Peng Fan,
dl-linux-imx, linux-arm-kernel, linux-kernel
Alice Guo (OSS) wrote on Tue, Mar 30, 2021 at 02:41:23AM +0000:
> Thanks for reporting this issue, I'll check and add a fix to handle defer probe.
I haven't seen any follow up on this, have you had a chance to take a
look?
If this won't make it for 5.12 (in a couple of week probably?) would it
make sense to revert 7d981405d0fd ("soc: imx8m: change to use platform
driver") for now?
While looking at the code earlier I also have an unrelated, late-review
on the patch itself:
> +static u32 __init imx8mq_soc_revision(struct device *dev)
> [...]
> @@ -191,8 +223,16 @@ static int __init imx8_soc_init(void)
> data = id->data;
> if (data) {
> soc_dev_attr->soc_id = data->name;
> - if (data->soc_revision)
> - soc_rev = data->soc_revision();
> + if (data->soc_revision) {
> + if (pdev) {
> + soc_rev = data->soc_revision(&pdev->dev);
> + ret = soc_rev;
> + if (ret < 0)
I appreciate current soc_revision are "small enough" (looking at
include/soc/imx/revision.h we're talking < 256) so this actually works,
but would it make sense to either make soc_rev signed, or to have
soc_revision() return a s64, or have the revision filled in another *u32
argument to make sure the error is an error and not just a large rev?
This is most definitely fine for now but that kind of code patterns can
lead to weird errors down the road.
Thanks,
--
Dominique
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [EXT] regression due to soc_device_match not handling defer (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
@ 2021-04-15 1:27 ` Dominique MARTINET
0 siblings, 0 replies; 30+ messages in thread
From: Dominique MARTINET @ 2021-04-15 1:27 UTC (permalink / raw)
To: Alice Guo (OSS)
Cc: Shawn Guo, Krzysztof Kozlowski, robh+dt, devicetree, Peng Fan,
dl-linux-imx, linux-arm-kernel, linux-kernel
Alice Guo (OSS) wrote on Tue, Mar 30, 2021 at 02:41:23AM +0000:
> Thanks for reporting this issue, I'll check and add a fix to handle defer probe.
I haven't seen any follow up on this, have you had a chance to take a
look?
If this won't make it for 5.12 (in a couple of week probably?) would it
make sense to revert 7d981405d0fd ("soc: imx8m: change to use platform
driver") for now?
While looking at the code earlier I also have an unrelated, late-review
on the patch itself:
> +static u32 __init imx8mq_soc_revision(struct device *dev)
> [...]
> @@ -191,8 +223,16 @@ static int __init imx8_soc_init(void)
> data = id->data;
> if (data) {
> soc_dev_attr->soc_id = data->name;
> - if (data->soc_revision)
> - soc_rev = data->soc_revision();
> + if (data->soc_revision) {
> + if (pdev) {
> + soc_rev = data->soc_revision(&pdev->dev);
> + ret = soc_rev;
> + if (ret < 0)
I appreciate current soc_revision are "small enough" (looking at
include/soc/imx/revision.h we're talking < 256) so this actually works,
but would it make sense to either make soc_rev signed, or to have
soc_revision() return a s64, or have the revision filled in another *u32
argument to make sure the error is an error and not just a large rev?
This is most definitely fine for now but that kind of code patterns can
lead to weird errors down the road.
Thanks,
--
Dominique
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [EXT] regression due to soc_device_match not handling defer (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
2021-04-15 1:27 ` Dominique MARTINET
@ 2021-04-15 1:33 ` Peng Fan
-1 siblings, 0 replies; 30+ messages in thread
From: Peng Fan @ 2021-04-15 1:33 UTC (permalink / raw)
To: Dominique MARTINET, Alice Guo (OSS)
Cc: Shawn Guo, Krzysztof Kozlowski, robh+dt, devicetree,
dl-linux-imx, linux-arm-kernel, linux-kernel
> (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
>
> Alice Guo (OSS) wrote on Tue, Mar 30, 2021 at 02:41:23AM +0000:
> > Thanks for reporting this issue, I'll check and add a fix to handle defer probe.
>
> I haven't seen any follow up on this, have you had a chance to take a look?
We are trying to find a proper solution for this.
The proper method might be make soc_device_match return probe defer,
and take early soc attr into consideration, but I am not sure this would win
maintainer's vote.
> If this won't make it for 5.12 (in a couple of week probably?) would it make
> sense to revert 7d981405d0fd ("soc: imx8m: change to use platform
> driver") for now?
Please no. We are targeting android GKI, make driver as modules.
And reverting to original method will also break kexec.
I am on IRC #linux-imx, we could take more if you would like to.
Thanks,
Peng.
>
>
>
> While looking at the code earlier I also have an unrelated, late-review on the
> patch itself:
>
> > +static u32 __init imx8mq_soc_revision(struct device *dev)
> > [...]
> > @@ -191,8 +223,16 @@ static int __init imx8_soc_init(void)
> > data = id->data;
> > if (data) {
> > soc_dev_attr->soc_id = data->name;
> > - if (data->soc_revision)
> > - soc_rev = data->soc_revision();
> > + if (data->soc_revision) {
> > + if (pdev) {
> > + soc_rev =
> data->soc_revision(&pdev->dev);
> > + ret = soc_rev;
> > + if (ret < 0)
>
> I appreciate current soc_revision are "small enough" (looking at
> include/soc/imx/revision.h we're talking < 256) so this actually works, but
> would it make sense to either make soc_rev signed, or to have
> soc_revision() return a s64, or have the revision filled in another *u32
> argument to make sure the error is an error and not just a large rev?
>
> This is most definitely fine for now but that kind of code patterns can lead to
> weird errors down the road.
>
> Thanks,
> --
> Dominique
^ permalink raw reply [flat|nested] 30+ messages in thread
* RE: [EXT] regression due to soc_device_match not handling defer (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
@ 2021-04-15 1:33 ` Peng Fan
0 siblings, 0 replies; 30+ messages in thread
From: Peng Fan @ 2021-04-15 1:33 UTC (permalink / raw)
To: Dominique MARTINET, Alice Guo (OSS)
Cc: Shawn Guo, Krzysztof Kozlowski, robh+dt, devicetree,
dl-linux-imx, linux-arm-kernel, linux-kernel
> (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
>
> Alice Guo (OSS) wrote on Tue, Mar 30, 2021 at 02:41:23AM +0000:
> > Thanks for reporting this issue, I'll check and add a fix to handle defer probe.
>
> I haven't seen any follow up on this, have you had a chance to take a look?
We are trying to find a proper solution for this.
The proper method might be make soc_device_match return probe defer,
and take early soc attr into consideration, but I am not sure this would win
maintainer's vote.
> If this won't make it for 5.12 (in a couple of week probably?) would it make
> sense to revert 7d981405d0fd ("soc: imx8m: change to use platform
> driver") for now?
Please no. We are targeting android GKI, make driver as modules.
And reverting to original method will also break kexec.
I am on IRC #linux-imx, we could take more if you would like to.
Thanks,
Peng.
>
>
>
> While looking at the code earlier I also have an unrelated, late-review on the
> patch itself:
>
> > +static u32 __init imx8mq_soc_revision(struct device *dev)
> > [...]
> > @@ -191,8 +223,16 @@ static int __init imx8_soc_init(void)
> > data = id->data;
> > if (data) {
> > soc_dev_attr->soc_id = data->name;
> > - if (data->soc_revision)
> > - soc_rev = data->soc_revision();
> > + if (data->soc_revision) {
> > + if (pdev) {
> > + soc_rev =
> data->soc_revision(&pdev->dev);
> > + ret = soc_rev;
> > + if (ret < 0)
>
> I appreciate current soc_revision are "small enough" (looking at
> include/soc/imx/revision.h we're talking < 256) so this actually works, but
> would it make sense to either make soc_rev signed, or to have
> soc_revision() return a s64, or have the revision filled in another *u32
> argument to make sure the error is an error and not just a large rev?
>
> This is most definitely fine for now but that kind of code patterns can lead to
> weird errors down the road.
>
> Thanks,
> --
> Dominique
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [EXT] regression due to soc_device_match not handling defer (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
2021-04-15 1:33 ` Peng Fan
@ 2021-06-24 10:36 ` Lucas Stach
-1 siblings, 0 replies; 30+ messages in thread
From: Lucas Stach @ 2021-06-24 10:36 UTC (permalink / raw)
To: Peng Fan, Dominique MARTINET, Alice Guo (OSS)
Cc: Shawn Guo, Krzysztof Kozlowski, robh+dt, devicetree,
dl-linux-imx, linux-arm-kernel, linux-kernel
Hi all,
Am Donnerstag, dem 15.04.2021 um 01:33 +0000 schrieb Peng Fan:
> > (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
> >
> > Alice Guo (OSS) wrote on Tue, Mar 30, 2021 at 02:41:23AM +0000:
> > > Thanks for reporting this issue, I'll check and add a fix to handle defer probe.
> >
> > I haven't seen any follow up on this, have you had a chance to take a look?
>
> We are trying to find a proper solution for this.
>
> The proper method might be make soc_device_match return probe defer,
> and take early soc attr into consideration, but I am not sure this would win
> maintainer's vote.
>
> > If this won't make it for 5.12 (in a couple of week probably?) would it make
> > sense to revert 7d981405d0fd ("soc: imx8m: change to use platform
> > driver") for now?
>
> Please no. We are targeting android GKI, make driver as modules.
> And reverting to original method will also break kexec.
>
> I am on IRC #linux-imx, we could take more if you would like to.
It seems this stalled. This regression totally breaks the kernel boot
on all i.MX8M devices including the CAAM. 5.13 is about to be released,
as the second upstream kernel release after 5.12 without a fix for this
issue. What's the plan here?
If there is no good solution small enough to be ported to the stable
kernels in sight, I think the only sensible option here is to revert
this change.
Regards,
Lucas
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [EXT] regression due to soc_device_match not handling defer (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
@ 2021-06-24 10:36 ` Lucas Stach
0 siblings, 0 replies; 30+ messages in thread
From: Lucas Stach @ 2021-06-24 10:36 UTC (permalink / raw)
To: Peng Fan, Dominique MARTINET, Alice Guo (OSS)
Cc: Shawn Guo, Krzysztof Kozlowski, robh+dt, devicetree,
dl-linux-imx, linux-arm-kernel, linux-kernel
Hi all,
Am Donnerstag, dem 15.04.2021 um 01:33 +0000 schrieb Peng Fan:
> > (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
> >
> > Alice Guo (OSS) wrote on Tue, Mar 30, 2021 at 02:41:23AM +0000:
> > > Thanks for reporting this issue, I'll check and add a fix to handle defer probe.
> >
> > I haven't seen any follow up on this, have you had a chance to take a look?
>
> We are trying to find a proper solution for this.
>
> The proper method might be make soc_device_match return probe defer,
> and take early soc attr into consideration, but I am not sure this would win
> maintainer's vote.
>
> > If this won't make it for 5.12 (in a couple of week probably?) would it make
> > sense to revert 7d981405d0fd ("soc: imx8m: change to use platform
> > driver") for now?
>
> Please no. We are targeting android GKI, make driver as modules.
> And reverting to original method will also break kexec.
>
> I am on IRC #linux-imx, we could take more if you would like to.
It seems this stalled. This regression totally breaks the kernel boot
on all i.MX8M devices including the CAAM. 5.13 is about to be released,
as the second upstream kernel release after 5.12 without a fix for this
issue. What's the plan here?
If there is no good solution small enough to be ported to the stable
kernels in sight, I think the only sensible option here is to revert
this change.
Regards,
Lucas
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [EXT] regression due to soc_device_match not handling defer (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
2021-06-24 10:36 ` Lucas Stach
@ 2021-06-29 2:39 ` Peng Fan (OSS)
-1 siblings, 0 replies; 30+ messages in thread
From: Peng Fan (OSS) @ 2021-06-29 2:39 UTC (permalink / raw)
To: Lucas Stach, Peng Fan, Dominique MARTINET, Alice Guo (OSS)
Cc: Shawn Guo, Krzysztof Kozlowski, robh+dt, devicetree,
dl-linux-imx, linux-arm-kernel, linux-kernel
Hi Lucas,
On 2021/6/24 18:36, Lucas Stach wrote:
> Hi all,
>
> Am Donnerstag, dem 15.04.2021 um 01:33 +0000 schrieb Peng Fan:
>>> (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
>>>
>>> Alice Guo (OSS) wrote on Tue, Mar 30, 2021 at 02:41:23AM +0000:
>>>> Thanks for reporting this issue, I'll check and add a fix to handle defer probe.
>>>
>>> I haven't seen any follow up on this, have you had a chance to take a look?
>>
>> We are trying to find a proper solution for this.
>>
>> The proper method might be make soc_device_match return probe defer,
>> and take early soc attr into consideration, but I am not sure this would win
>> maintainer's vote.
>>
>>> If this won't make it for 5.12 (in a couple of week probably?) would it make
>>> sense to revert 7d981405d0fd ("soc: imx8m: change to use platform
>>> driver") for now?
>>
>> Please no. We are targeting android GKI, make driver as modules.
>> And reverting to original method will also break kexec.
>>
>> I am on IRC #linux-imx, we could take more if you would like to.
>
> It seems this stalled. This regression totally breaks the kernel boot
> on all i.MX8M devices including the CAAM. 5.13 is about to be released,
> as the second upstream kernel release after 5.12 without a fix for this
> issue. What's the plan here?
>
> If there is no good solution small enough to be ported to the stable
> kernels in sight, I think the only sensible option here is to revert
> this change.
I not have time to touch this, maybe you could submit a revert for this.
Thanks,
Peng.
>
> Regards,
> Lucas
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [EXT] regression due to soc_device_match not handling defer (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
@ 2021-06-29 2:39 ` Peng Fan (OSS)
0 siblings, 0 replies; 30+ messages in thread
From: Peng Fan (OSS) @ 2021-06-29 2:39 UTC (permalink / raw)
To: Lucas Stach, Peng Fan, Dominique MARTINET, Alice Guo (OSS)
Cc: Shawn Guo, Krzysztof Kozlowski, robh+dt, devicetree,
dl-linux-imx, linux-arm-kernel, linux-kernel
Hi Lucas,
On 2021/6/24 18:36, Lucas Stach wrote:
> Hi all,
>
> Am Donnerstag, dem 15.04.2021 um 01:33 +0000 schrieb Peng Fan:
>>> (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver)
>>>
>>> Alice Guo (OSS) wrote on Tue, Mar 30, 2021 at 02:41:23AM +0000:
>>>> Thanks for reporting this issue, I'll check and add a fix to handle defer probe.
>>>
>>> I haven't seen any follow up on this, have you had a chance to take a look?
>>
>> We are trying to find a proper solution for this.
>>
>> The proper method might be make soc_device_match return probe defer,
>> and take early soc attr into consideration, but I am not sure this would win
>> maintainer's vote.
>>
>>> If this won't make it for 5.12 (in a couple of week probably?) would it make
>>> sense to revert 7d981405d0fd ("soc: imx8m: change to use platform
>>> driver") for now?
>>
>> Please no. We are targeting android GKI, make driver as modules.
>> And reverting to original method will also break kexec.
>>
>> I am on IRC #linux-imx, we could take more if you would like to.
>
> It seems this stalled. This regression totally breaks the kernel boot
> on all i.MX8M devices including the CAAM. 5.13 is about to be released,
> as the second upstream kernel release after 5.12 without a fix for this
> issue. What's the plan here?
>
> If there is no good solution small enough to be ported to the stable
> kernels in sight, I think the only sensible option here is to revert
> this change.
I not have time to touch this, maybe you could submit a revert for this.
Thanks,
Peng.
>
> Regards,
> Lucas
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2021-06-29 2:41 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 10:11 [PATCH v4 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID Alice Guo
2020-11-20 10:11 ` Alice Guo
2020-11-20 10:11 ` [PATCH v4 2/4] arm64: dts: imx8m: add SoC ID compatible Alice Guo
2020-11-20 10:11 ` Alice Guo
2020-11-20 10:11 ` [PATCH v4 3/4] arm64: dts: imx8m: add NVMEM provider and consumer to read soc unique ID Alice Guo
2020-11-20 10:11 ` Alice Guo
2020-11-20 10:53 ` Krzysztof Kozlowski
2020-11-20 10:53 ` Krzysztof Kozlowski
2020-11-20 10:11 ` [PATCH v4 4/4] soc: imx8m: change to use platform driver Alice Guo
2020-11-20 10:11 ` Alice Guo
2020-11-20 10:46 ` Krzysztof Kozlowski
2020-11-20 10:46 ` Krzysztof Kozlowski
2021-03-29 9:08 ` regression due to soc_device_match not handling defer (Was: [PATCH v4 4/4] soc: imx8m: change to use platform driver) Dominique MARTINET
2021-03-29 9:08 ` Dominique MARTINET
2021-03-30 2:41 ` [EXT] " Alice Guo (OSS)
2021-03-30 2:41 ` Alice Guo (OSS)
2021-04-15 1:27 ` Dominique MARTINET
2021-04-15 1:27 ` Dominique MARTINET
2021-04-15 1:33 ` Peng Fan
2021-04-15 1:33 ` Peng Fan
2021-06-24 10:36 ` Lucas Stach
2021-06-24 10:36 ` Lucas Stach
2021-06-29 2:39 ` Peng Fan (OSS)
2021-06-29 2:39 ` Peng Fan (OSS)
2020-11-20 10:50 ` [PATCH v4 1/4] dt-bindings: soc: imx8m: add DT Binding doc for soc unique ID Krzysztof Kozlowski
2020-11-20 10:50 ` Krzysztof Kozlowski
2020-11-23 4:45 ` [EXT] " Alice Guo
2020-11-23 4:45 ` Alice Guo
2020-11-23 9:04 ` Krzysztof Kozlowski
2020-11-23 9:04 ` Krzysztof Kozlowski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.