fsi: sbefifo: Prevent async FFDC collection for Odyssey SBEFIFOs

PATCH Superseded Eddie James 3 participants 12 messages Apr 12, 2024 – Apr 17, 2024

Subsystems (3)

Device tree bindings Arm/Aspeed machine support FSI

lore

Share & tools

lore

Eddie James

author maintainer

2024-04-12 14:43 UTC #0 lore raw

cover fsi: sbefifo: Prevent async FFDC collection for Odyssey SBEFIFOs

Newer SBEs will return any async FFDC in the next transaction, so collecting the FFDC is unnecessary and results in unexpected behavior. Turn off the async collection for newer SBEFIFOs.

Eddie James (3):
dt-bindings: fsi: Document the IBM SBEFIFO engine
fsi: sbefifo: Prevent async FFDC collection for Odyssey SBEFIFOs
ARM: dts: aspeed: Update Odyssey SBEFIFO compatible strings

 .../devicetree/bindings/fsi/ibm,sbefifo.yaml  | 39 +++++++++++
 .../dts/aspeed/aspeed-bmc-ibm-everest.dts     | 64 +++++++++----------
 .../arm/boot/dts/aspeed/ibm-power10-quad.dtsi | 64 +++++++++----------
 drivers/fsi/fsi-sbefifo.c                     | 37 +++++++++++
 4 files changed, 140 insertions(+), 64 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml

Signature · 1 line

2.39.3

Eddie James

author maintainer

2024-04-12 14:43 UTC #1 lore raw

PATCH 1/3 dt-bindings: fsi: Document the IBM SBEFIFO engine

The SBEFIFO engine provides an interface to the POWER processor Self Boto Engine (SBE).

Signed-off-by Eddie James <[email protected]>

---
 .../devicetree/bindings/fsi/ibm,sbefifo.yaml  | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml

Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml new file +39 -0

new file mode 100644
index 000000000000..d70012e42d79
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/fsi/ibm,sbefifo.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: IBM FSI-attached SBEFIFO engine
+
+maintainers:
+  - Eddie James <[email protected]>
+
+description: |
+  This binding describes an FSI CFAM engine called the SBEFIFO. Therefore this
+  node will always be a child of an FSI CFAM node; see fsi.txt for details on
+  FSI slave and CFAM nodes. This SBEFIFO engine provides an interface to the
+  POWER processor Self Boot Engine (SBE).
+
+properties:
+  compatible:
+    enum:
+      - ibm,p9-sbefifo
+      - ibm,ody-sbefifo
+
+  reg:
+    items:
+      - description: FSI slave address
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    sbefifo at 2400 {
+        compatible = "ibm,p9-sbefifo";
+        reg = <0x2400 0x400>;
+    };

Signature · 1 line

2.39.3

Eddie James

author maintainer

2024-04-12 14:43 UTC #5 lore raw

PATCH 2/3 fsi: sbefifo: Prevent async FFDC collection for Odyssey SBEFIFOs

In reply to Eddie James

Newer SBEs will return any async FFDC in the next transaction, so collecting the FFDC is unnecessary and results in unexpected behavior. Turn off the async collection for newer SBEFIFOs.

Signed-off-by Eddie James <[email protected]>

---
 drivers/fsi/fsi-sbefifo.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

drivers/fsi/fsi-sbefifo.c +37 -0

index 0385476bfb03..706b98b49d08 100644
@@ -125,6 +125,7 @@ struct sbefifo {
 	struct mutex		lock;
 	bool			broken;
 	bool			dead;
+	bool			need_async;
 	bool			async_ffdc;
 	bool			timed_out;
 	u32			timeout_in_cmd_ms;

@@ -141,6 +142,10 @@ struct sbefifo_user {
 	u32			read_timeout_ms;
 };
 
+struct sbefifo_data {
+	bool need_async;
+};
+
 static DEFINE_MUTEX(sbefifo_ffdc_mutex);
 
 static ssize_t timeout_show(struct device *dev, struct device_attribute *attr,

@@ -317,6 +322,10 @@ static int sbefifo_check_sbe_state(struct sbefifo *sbefifo)
 		return -ESHUTDOWN;
 	}
 
+	/* Newer SBEFIFOs don't need async FFDC collection */
+	if (!sbefifo->need_async)
+		return 0;
+
 	/* Is there async FFDC available ? Remember it */
 	if (sbm & CFAM_SBM_SBE_ASYNC_FFDC)
 		sbefifo->async_ffdc = true;

@@ -1031,6 +1040,7 @@ static void sbefifo_free(struct device *dev)
 
 static int sbefifo_probe(struct device *dev)
 {
+	const struct sbefifo_data *md = of_device_get_match_data(dev);
 	struct fsi_device *fsi_dev = to_fsi_dev(dev);
 	struct sbefifo *sbefifo;
 	struct device_node *np;

@@ -1044,6 +1054,11 @@ static int sbefifo_probe(struct device *dev)
 	if (!sbefifo)
 		return -ENOMEM;
 
+	if (md)
+		sbefifo->need_async = md->need_async;
+	else
+		sbefifo->need_async = true;
+
 	/* Grab a reference to the device (parent of our cdev), we'll drop it later */
 	if (!get_device(dev)) {
 		kfree(sbefifo);

@@ -1128,6 +1143,27 @@ static int sbefifo_remove(struct device *dev)
 	return 0;
 }
 
+static const struct sbefifo_data p9_sbefifo_data = {
+	.need_async = true,
+};
+
+static const struct sbefifo_data ody_sbefifo_data = {
+	.need_async = false,
+};
+
+static const struct of_device_id sbefifo_of_ids[] = {
+	{
+		.compatible = "ibm,p9-sbefifo",
+		.data = &p9_sbefifo_data,
+	},
+	{
+		.compatible = "ibm,ody-sbefifo",
+		.data = &ody_sbefifo_data,
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sbefifo_of_ids);
+
 static const struct fsi_device_id sbefifo_ids[] = {
 	{
 		.engine_type = FSI_ENGID_SBE,

@@ -1141,6 +1177,7 @@ static struct fsi_driver sbefifo_drv = {
 	.drv = {
 		.name = DEVICE_NAME,
 		.bus = &fsi_bus_type,
+		.of_match_table = sbefifo_of_ids,
 		.probe = sbefifo_probe,
 		.remove = sbefifo_remove,
 	}

Signature · 1 line

2.39.3

Eddie James

author maintainer

2024-04-12 14:43 UTC #6 lore raw

PATCH 3/3 ARM: dts: aspeed: Update Odyssey SBEFIFO compatible strings

In reply to Eddie James

Set the new compatible string for Odyssey SBEFIFOs so that they don't collect async FFDC.

Signed-off-by Eddie James <[email protected]>

---
 .../dts/aspeed/aspeed-bmc-ibm-everest.dts     | 64 +++++++++----------
 .../arm/boot/dts/aspeed/ibm-power10-quad.dtsi | 64 +++++++++----------
 2 files changed, 64 insertions(+), 64 deletions(-)

arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts +32 ~~-32~~

index 214b2e6a4c6d..3a2bfdf035cb 100644
@@ -2545,7 +2545,7 @@ scom500: scom at 1000 {
 						};
 
 						sbefifo500: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -2577,7 +2577,7 @@ scom501: scom at 1000 {
 						};
 
 						sbefifo501: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -2609,7 +2609,7 @@ scom510: scom at 1000 {
 						};
 
 						sbefifo510: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -2641,7 +2641,7 @@ scom511: scom at 1000 {
 						};
 
 						sbefifo511: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -2673,7 +2673,7 @@ scom512: scom at 1000 {
 						};
 
 						sbefifo512: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -2705,7 +2705,7 @@ scom513: scom at 1000 {
 						};
 
 						sbefifo513: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -2737,7 +2737,7 @@ scom514: scom at 1000 {
 						};
 
 						sbefifo514: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -2769,7 +2769,7 @@ scom515: scom at 1000 {
 						};
 
 						sbefifo515: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -2918,7 +2918,7 @@ scom602: scom at 1000 {
 						};
 
 						sbefifo602: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -2950,7 +2950,7 @@ scom603: scom at 1000 {
 						};
 
 						sbefifo603: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -2982,7 +2982,7 @@ scom610: scom at 1000 {
 						};
 
 						sbefifo610: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3014,7 +3014,7 @@ scom611: scom at 1000 {
 						};
 
 						sbefifo611: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3046,7 +3046,7 @@ scom614: scom at 1000 {
 						};
 
 						sbefifo614: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3078,7 +3078,7 @@ scom615: scom at 1000 {
 						};
 
 						sbefifo615: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3110,7 +3110,7 @@ scom616: scom at 1000 {
 						};
 
 						sbefifo616: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3142,7 +3142,7 @@ scom617: scom at 1000 {
 						};
 
 						sbefifo617: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3291,7 +3291,7 @@ scom700: scom at 1000 {
 						};
 
 						sbefifo700: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3323,7 +3323,7 @@ scom701: scom at 1000 {
 						};
 
 						sbefifo701: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3355,7 +3355,7 @@ scom710: scom at 1000 {
 						};
 
 						sbefifo710: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3387,7 +3387,7 @@ scom711: scom at 1000 {
 						};
 
 						sbefifo711: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3419,7 +3419,7 @@ scom712: scom at 1000 {
 						};
 
 						sbefifo712: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3451,7 +3451,7 @@ scom713: scom at 1000 {
 						};
 
 						sbefifo713: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3483,7 +3483,7 @@ scom714: scom at 1000 {
 						};
 
 						sbefifo714: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3515,7 +3515,7 @@ scom715: scom at 1000 {
 						};
 
 						sbefifo715: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3664,7 +3664,7 @@ scom802: scom at 1000 {
 						};
 
 						sbefifo802: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3696,7 +3696,7 @@ scom803: scom at 1000 {
 						};
 
 						sbefifo803: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3728,7 +3728,7 @@ scom810: scom at 1000 {
 						};
 
 						sbefifo810: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3760,7 +3760,7 @@ scom811: scom at 1000 {
 						};
 
 						sbefifo811: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3792,7 +3792,7 @@ scom814: scom at 1000 {
 						};
 
 						sbefifo814: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3824,7 +3824,7 @@ scom815: scom at 1000 {
 						};
 
 						sbefifo815: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3856,7 +3856,7 @@ scom816: scom at 1000 {
 						};
 
 						sbefifo816: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

@@ -3888,7 +3888,7 @@ scom817: scom at 1000 {
 						};
 
 						sbefifo817: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

arch/arm/boot/dts/aspeed/ibm-power10-quad.dtsi +32 ~~-32~~

256 lines not on this page · raw message

Signature · 1 line

2.39.3

Krzysztof Kozlowski

2024-04-13 07:47 UTC #2 lore raw

PATCH 1/3 dt-bindings: fsi: Document the IBM SBEFIFO engine

In reply to Eddie James

Quoted 2 lines · Eddie James

The SBEFIFO engine provides an interface to the POWER processor Self Boto Engine (SBE).

Boot?

Quoted 16 lines

Signed-off-by Eddie James <[email protected]>

---
 .../devicetree/bindings/fsi/ibm,sbefifo.yaml  | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml

Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml new file +3 -0

new file mode 100644
index 000000000000..d70012e42d79
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---

Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml line 13

Quoted 10 lines

Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml new file +10 -0

+$id: http://devicetree.org/schemas/fsi/ibm,sbefifo.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: IBM FSI-attached SBEFIFO engine
+
+maintainers:
+  - Eddie James <[email protected]>
+
+description: |
+  This binding describes an FSI CFAM engine called the SBEFIFO. Therefore this

Don't describe the binding, but the hardware. Drop redundant "This binding" and say what is the hardware here.

Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml line 17

Quoted 4 lines

Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml new file +4 -0

+  node will always be a child of an FSI CFAM node; see fsi.txt for details on
+  FSI slave and CFAM nodes. This SBEFIFO engine provides an interface to the
+  POWER processor Self Boot Engine (SBE).
+

With these two changes:

Reviewed-by Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

Andrew Jeffery

maintainer

2024-04-15 00:50 UTC #3 lore raw

PATCH 1/3 dt-bindings: fsi: Document the IBM SBEFIFO engine

In reply to Eddie James

Quoted 27 lines · Eddie James

The SBEFIFO engine provides an interface to the POWER processor Self Boto Engine (SBE).

Signed-off-by Eddie James <[email protected]>

---
 .../devicetree/bindings/fsi/ibm,sbefifo.yaml  | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml

Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml new file +12 -0

new file mode 100644
index 000000000000..d70012e42d79
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/fsi/ibm,sbefifo.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: IBM FSI-attached SBEFIFO engine
+
+maintainers:
+  - Eddie James <[email protected]>
+
+description: |

Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml line 22

Quoted 10 lines

Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml new file +10 -0

+  This binding describes an FSI CFAM engine called the SBEFIFO. Therefore this
+  node will always be a child of an FSI CFAM node; see fsi.txt for details on
+  FSI slave and CFAM nodes. This SBEFIFO engine provides an interface to the
+  POWER processor Self Boot Engine (SBE).
+
+properties:
+  compatible:
+    enum:
+      - ibm,p9-sbefifo
+      - ibm,ody-sbefifo

Bit of a nitpick, but: Is there any argument against using ibm,odyssey-sbefifo? Feels less cryptic.

Andrew

Andrew Jeffery

maintainer

2024-04-15 00:54 UTC #7 lore raw

PATCH 3/3 ARM: dts: aspeed: Update Odyssey SBEFIFO compatible strings

In reply to Eddie James

Quoted 14 lines · Eddie James

Set the new compatible string for Odyssey SBEFIFOs so that they don't collect async FFDC.
Signed-off-by Eddie James <[email protected]>
---
 .../dts/aspeed/aspeed-bmc-ibm-everest.dts     | 64 +++++++++----------
 .../arm/boot/dts/aspeed/ibm-power10-quad.dtsi | 64 +++++++++----------
 2 files changed, 64 insertions(+), 64 deletions(-)
arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts +0 -0
index 214b2e6a4c6d..3a2bfdf035cb 100644

arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts scom500: scom at 1000 { · line 2551

Quoted 8 lines

arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts +1 -1

@@ -2545,7 +2545,7 @@ scom500: scom at 1000 {
 						};
 
 						sbefifo500: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
 							reg = <0x2400 0x400>;
 							#address-cells = <1>;
 							#size-cells = <0>;

Bit of a drive-by comment as I'm not really holding any stakes here, but did the hardware actually change?

Would it be better to request the behaviour by a property rather than changing the compatible?

Andrew

Eddie James

author maintainer

2024-04-15 14:59 UTC #8 lore raw

PATCH 3/3 ARM: dts: aspeed: Update Odyssey SBEFIFO compatible strings

Quoted 17 lines · Andrew Jeffery

Quoted 16 lines · Eddie James
Set the new compatible string for Odyssey SBEFIFOs so that they don't collect async FFDC.
Signed-off-by Eddie James <[email protected]>
---
  .../dts/aspeed/aspeed-bmc-ibm-everest.dts     | 64 +++++++++----------
  .../arm/boot/dts/aspeed/ibm-power10-quad.dtsi | 64 +++++++++----------
  2 files changed, 64 insertions(+), 64 deletions(-)
arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts +0 -0
index 214b2e6a4c6d..3a2bfdf035cb 100644
@@ -2545,7 +2545,7 @@ scom500: scom at 1000 {
  						};
 

Quoted 9 lines

arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts scom500: scom at 1000 { · line 2551
Quoted 6 lines
arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts +1 -1
  						sbefifo500: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
  							reg = <0x2400 0x400>;
  							#address-cells = <1>;
  							#size-cells = <0>;
Bit of a drive-by comment as I'm not really holding any stakes here, but did the hardware actually change?

Yes, the hardware did change in the Odyssey version.

Quoted 3 lines

Would it be better to request the behaviour by a property rather than changing the compatible?

I'm not sure! In the past devicetree maintainers have objected to behavioral type properties, like the FSI scan behavior one...

Thanks for the review!

Eddie

Quoted 2 lines

Andrew

Eddie James

author maintainer

2024-04-15 15:03 UTC #4 lore raw

PATCH 1/3 dt-bindings: fsi: Document the IBM SBEFIFO engine

In reply to Andrew Jeffery

Quoted 32 lines · Andrew Jeffery

Quoted 31 lines · Eddie James

The SBEFIFO engine provides an interface to the POWER processor Self Boto Engine (SBE).

Signed-off-by Eddie James <[email protected]>

---
  .../devicetree/bindings/fsi/ibm,sbefifo.yaml  | 39 +++++++++++++++++++
  1 file changed, 39 insertions(+)

create mode 100644 Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml

Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml new file +16 -0

new file mode 100644
index 000000000000..d70012e42d79
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/fsi/ibm,sbefifo.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: IBM FSI-attached SBEFIFO engine
+
+maintainers:
+  - Eddie James <[email protected]>
+
+description: |
+  This binding describes an FSI CFAM engine called the SBEFIFO. Therefore this
+  node will always be a child of an FSI CFAM node; see fsi.txt for details on
+  FSI slave and CFAM nodes. This SBEFIFO engine provides an interface to the
+  POWER processor Self Boot Engine (SBE).

Quoted 8 lines

Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml line 22
Quoted 6 lines
Documentation/devicetree/bindings/fsi/ibm,sbefifo.yaml new file +6 -0
+
+properties:
+  compatible:
+    enum:
+      - ibm,p9-sbefifo
+      - ibm,ody-sbefifo
Bit of a nitpick, but: Is there any argument against using ibm,odyssey-sbefifo? Feels less cryptic.

True, that might be better.

Quoted 2 lines

Andrew

Andrew Jeffery

maintainer

2024-04-16 00:06 UTC #9 lore raw

PATCH 3/3 ARM: dts: aspeed: Update Odyssey SBEFIFO compatible strings

In reply to Eddie James

Quoted 29 lines · Eddie James

Quoted 17 lines · Andrew Jeffery
Quoted 16 lines · Eddie James
Set the new compatible string for Odyssey SBEFIFOs so that they don't collect async FFDC.
Signed-off-by Eddie James <[email protected]>
---
  .../dts/aspeed/aspeed-bmc-ibm-everest.dts     | 64 +++++++++----------
  .../arm/boot/dts/aspeed/ibm-power10-quad.dtsi | 64 +++++++++----------
  2 files changed, 64 insertions(+), 64 deletions(-)
arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts +0 -0
index 214b2e6a4c6d..3a2bfdf035cb 100644
@@ -2545,7 +2545,7 @@ scom500: scom at 1000 {
  						};
 
Quoted 9 lines
arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts scom500: scom at 1000 { · line 2551
Quoted 6 lines
arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts +1 -1
  						sbefifo500: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
  							reg = <0x2400 0x400>;
  							#address-cells = <1>;
  							#size-cells = <0>;
Bit of a drive-by comment as I'm not really holding any stakes here, but did the hardware actually change?

Quoted 1 line

Yes, the hardware did change in the Odyssey version.

Should there not be a new platform DTS then? Seems a bit puzzling, but again this is a bit of a drive-by comment.

Andrew

Eddie James

author maintainer

2024-04-16 14:48 UTC #10 lore raw

PATCH 3/3 ARM: dts: aspeed: Update Odyssey SBEFIFO compatible strings

Quoted 28 lines · Andrew Jeffery

Quoted 27 lines · Eddie James
Quoted 25 lines · Andrew Jeffery
Quoted 14 lines · Eddie James
Set the new compatible string for Odyssey SBEFIFOs so that they don't collect async FFDC.
Signed-off-by Eddie James <[email protected]>
---
   .../dts/aspeed/aspeed-bmc-ibm-everest.dts     | 64 +++++++++----------
   .../arm/boot/dts/aspeed/ibm-power10-quad.dtsi | 64 +++++++++----------
   2 files changed, 64 insertions(+), 64 deletions(-)
arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts +0 -0
index 214b2e6a4c6d..3a2bfdf035cb 100644
arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts scom500: scom at 1000 { · line 2551
Quoted 8 lines
arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts +1 -1
@@ -2545,7 +2545,7 @@ scom500: scom at 1000 {
   						};
 
   						sbefifo500: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
   							reg = <0x2400 0x400>;
   							#address-cells = <1>;
   							#size-cells = <0>;
Bit of a drive-by comment as I'm not really holding any stakes here, but did the hardware actually change?

Quoted 3 lines

Quoted 1 line
Yes, the hardware did change in the Odyssey version.
Should there not be a new platform DTS then? Seems a bit puzzling, but again this is a bit of a drive-by comment.

Ah, I should clarify. The SBE hardware was changed between P9/P10 and Odyssey versions. The Odyssey hasn't changed. The difference in the SBE wasn't apparent when we initially added support for Odyssey, so this is effectively a bug fix on the new hardware. There is no new platform.

Thanks,

Eddie

Quoted 2 lines

Andrew

Andrew Jeffery

maintainer

2024-04-17 07:02 UTC #11 lore raw

PATCH 3/3 ARM: dts: aspeed: Update Odyssey SBEFIFO compatible strings

Quoted 30 lines · Eddie James

Quoted 29 lines · Andrew Jeffery
Quoted 28 lines · Eddie James
Quoted 17 lines · Andrew Jeffery
Quoted 16 lines · Eddie James
Set the new compatible string for Odyssey SBEFIFOs so that they don't collect async FFDC.
Signed-off-by Eddie James <[email protected]>
---
   .../dts/aspeed/aspeed-bmc-ibm-everest.dts     | 64 +++++++++----------
   .../arm/boot/dts/aspeed/ibm-power10-quad.dtsi | 64 +++++++++----------
   2 files changed, 64 insertions(+), 64 deletions(-)
arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts +0 -0
index 214b2e6a4c6d..3a2bfdf035cb 100644
@@ -2545,7 +2545,7 @@ scom500: scom at 1000 {
   						};
 
Quoted 8 lines
arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts scom500: scom at 1000 { · line 2551
Quoted 6 lines
arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-everest.dts +1 -1
   						sbefifo500: sbefifo at 2400 {
-							compatible = "ibm,p9-sbefifo";
+							compatible = "ibm,ody-sbefifo";
   							reg = <0x2400 0x400>;
   							#address-cells = <1>;
   							#size-cells = <0>;
Bit of a drive-by comment as I'm not really holding any stakes here, but did the hardware actually change?
Yes, the hardware did change in the Odyssey version.

Quoted 8 lines

Quoted 2 lines
Should there not be a new platform DTS then? Seems a bit puzzling, but again this is a bit of a drive-by comment.
Ah, I should clarify. The SBE hardware was changed between P9/P10 and Odyssey versions. The Odyssey hasn't changed. The difference in the SBE wasn't apparent when we initially added support for Odyssey, so this is effectively a bug fix on the new hardware. There is no new platform.

That sounds like good info for the commit message. Do you mind updating it?

Andrew