[PATCH v2 0/5] MIPS: Allow using multi-cluster with a broken HCI.

[PATCH v2 0/5] MIPS: Allow using multi-cluster with a broken HCI.

[Gregory CLEMENT]

Hello,

Some CM3.5 reports indicate that Hardware Cache Initialization is
complete, but in reality it's not the case. They also incorrectly show
that Hardware Cache Initialization is supported. Unfortunately, it is
not possible to detect this issue at runtime and the information has
to be passed by the device tree.

I initially proposed to add this information as a CPU property, but as
rightfully pointed out by Jiaxun, it should be more of a Coherence
Manager property. A few months ago, Jiaxun proposed a series to pass
the address of the CM through the device tree when it was not possible
to get it at runtime. This series introduced the binding for the CM
that I reused. However, there were some parts of this series that were
commented on and needed to be addressed, and I don't have the hardware
for it. Therefore, I've kept only the binding and relaxed the need for
the reg property for the eyeq6 case.

My initial proposal was integrated into the series set by Aleksandar
here [1]. And the series adding the CM binding was here: [2]. The
patches 1,2,3, and 5 have no dependencies while patch 4 should depend
on this series [1]. Actually, those five patches should replace
patches 10, 11, and 12.

Gregory

[1]: https://lore.kernel.org/all/20241028175935.51250-1-arikalo@gmail.com/
[2]: https://lore.kernel.org/all/20240612-cm_probe-v2-5-a5b55440563c@flygoat.com/

Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
Changes in v2:
- Use compatible string instead of property
- Link to v1: https://lore.kernel.org/r/20241115-cluster-hci-broken-v1-0-00636800611d@bootlin.com

---
Gregory CLEMENT (4):
      dt-bindings: mips: mips-cm: Add a new compatible string for EyeQ6
      MIPS: cm: Detect CM quirks from device tree
      MIPS: CPS: Support broken HCI for multicluster
      MIPS: mobileye: dts: eyeq6h: Enable cluster support

Jiaxun Yang (1):
      dt-bindings: mips: Document mti,mips-cm

 .../devicetree/bindings/mips/mti,mips-cm.yaml      | 58 ++++++++++++++++++++++
 arch/mips/boot/dts/mobileye/eyeq6h.dtsi            |  4 ++
 arch/mips/include/asm/mips-cm.h                    | 22 ++++++++
 arch/mips/kernel/mips-cm.c                         | 14 ++++++
 arch/mips/kernel/smp-cps.c                         |  5 +-
 5 files changed, 102 insertions(+), 1 deletion(-)
---
base-commit: ffc2ac99f7a0dccb6389ea5e318a95965d882dba
change-id: 20241115-cluster-hci-broken-840a78f72aae

Best regards,
-- 
Grégory CLEMENT, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v2 1/5] dt-bindings: mips: Document mti,mips-cm

[Gregory CLEMENT]

From: Jiaxun Yang <jiaxun.yang@flygoat.com>

Add devicetree binding documentation for MIPS Coherence Manager.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 .../devicetree/bindings/mips/mti,mips-cm.yaml      | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..9f500804737d23e19f50a9326168686c05d3a54e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mips/mti,mips-cm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MIPS Coherence Manager
+
+description: |
+  Defines a location of the MIPS Coherence Manager registers.
+
+maintainers:
+  - Jiaxun Yang <jiaxun.yang@flygoat.com>
+
+properties:
+  compatible:
+    const: mti,mips-cm
+
+  reg:
+    description:
+      Base address and size of an unoccupied region in system's MMIO address
+      space, which will be used to map the MIPS CM global control registers
+      block. It is conventionally decided by the system integrator.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    coherency-manager@1fbf8000 {
+      compatible = "mti,mips-cm";
+      reg = <0x1bde8000 0x8000>;
+    };
+...

-- 
2.45.2

[PATCH v2 2/5] dt-bindings: mips: mips-cm: Add a new compatible string for EyeQ6

[Gregory CLEMENT]

The CM3.5 used on EyeQ6 reports that Hardware Cache Initialization is
complete, but in reality it's not the case. It also incorrectly
indicates that Hardware Cache Initialization is supported. This new
compatible string allows warning about this broken feature that cannot
be detected at runtime.

Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 .../devicetree/bindings/mips/mti,mips-cm.yaml      | 24 ++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
index 9f500804737d23e19f50a9326168686c05d3a54e..4713673f0cfc7785bb183917ee382a815ebfe9e1 100644
--- a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
+++ b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
@@ -14,7 +14,12 @@ maintainers:
 
 properties:
   compatible:
-    const: mti,mips-cm
+    oneOf:
+      - const: mti,mips-cm
+      - const: mti,eyeq6-cm
+        description:
+          On EyeQ6 the HCI (Hardware Cache Initialization) information for
+          the L2 cache in multi-cluster configuration is broken.
 
   reg:
     description:
@@ -25,14 +30,29 @@ properties:
 
 required:
   - compatible
-  - reg
 
 additionalProperties: false
 
+if:
+  properties:
+    compatible:
+      contains:
+        const: mti,eyeq6-cm
+then:
+  properties:
+    reg: false
+else:
+  required:
+    - reg
+
 examples:
   - |
     coherency-manager@1fbf8000 {
       compatible = "mti,mips-cm";
       reg = <0x1bde8000 0x8000>;
     };
+
+    coherency-manager {
+      compatible = "mti,eyeq6-cm";
+    };
 ...

-- 
2.45.2

[PATCH v2 3/5] MIPS: cm: Detect CM quirks from device tree

[Gregory CLEMENT]

Some information that should be retrieved at runtime for the Coherence
Manager can be either absent or wrong. This patch allows checking if
some of this information is available from the device tree and updates
the internal variable accordingly.

For now, only the compatible string associated with the broken HCI is
being retrieved.

Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 arch/mips/include/asm/mips-cm.h | 22 ++++++++++++++++++++++
 arch/mips/kernel/mips-cm.c      | 14 ++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/arch/mips/include/asm/mips-cm.h b/arch/mips/include/asm/mips-cm.h
index 1afa85db1fb37d1017fbe7d6b7a2b7d2470e8257..3bfe0633b57639bfb05b7692e4bb83ba7c0b2523 100644
--- a/arch/mips/include/asm/mips-cm.h
+++ b/arch/mips/include/asm/mips-cm.h
@@ -59,6 +59,16 @@ extern phys_addr_t mips_cm_l2sync_phys_base(void);
  */
 extern int mips_cm_is64;
 
+/*
+ * mips_cm_is_l2_hci_broken  - determine if HCI is broken
+ *
+ * Some CM reports show that Hardware Cache Initialization is
+ * complete, but in reality it's not the case. They also incorrectly
+ * indicate that Hardware Cache Initialization is supported. This
+ * flags allows warning about this broken feature.
+ */
+extern bool mips_cm_is_l2_hci_broken;
+
 /**
  * mips_cm_error_report - Report CM cache errors
  */
@@ -97,6 +107,18 @@ static inline bool mips_cm_present(void)
 #endif
 }
 
+/**
+ * mips_cm_update_property - update property from the device tree
+ *
+ * Retrieve the properties from the device tree if a CM node exist and
+ * update the internal variable based on this.
+ */
+#ifdef CONFIG_MIPS_CM
+extern void mips_cm_update_property(void);
+#else
+static void mips_cm_update_property(void) {}
+#endif
+
 /**
  * mips_cm_has_l2sync - determine whether an L2-only sync region is present
  *
diff --git a/arch/mips/kernel/mips-cm.c b/arch/mips/kernel/mips-cm.c
index 9854bc2b6895d4db67d216586f65e4810661d29b..e9e6fd6ab8bd342e34e97b0e6c71445f84ee76f7 100644
--- a/arch/mips/kernel/mips-cm.c
+++ b/arch/mips/kernel/mips-cm.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/errno.h>
+#include <linux/of.h>
 #include <linux/percpu.h>
 #include <linux/spinlock.h>
 
@@ -14,6 +15,7 @@
 void __iomem *mips_gcr_base;
 void __iomem *mips_cm_l2sync_base;
 int mips_cm_is64;
+bool mips_cm_is_l2_hci_broken;
 
 static char *cm2_tr[8] = {
 	"mem",	"gcr",	"gic",	"mmio",
@@ -237,6 +239,18 @@ static void mips_cm_probe_l2sync(void)
 	mips_cm_l2sync_base = ioremap(addr, MIPS_CM_L2SYNC_SIZE);
 }
 
+void mips_cm_update_property(void)
+{
+	struct device_node *cm_node;
+
+	cm_node = of_find_compatible_node(of_root, NULL, "mti,eyeq6-cm");
+	if (!cm_node)
+		return;
+	pr_info("HCI (Hardware Cache Init for the L2 cache) in GCR_L2_RAM_CONFIG from the CM3 is broken");
+	mips_cm_is_l2_hci_broken = true;
+	of_node_put(cm_node);
+}
+
 int mips_cm_probe(void)
 {
 	phys_addr_t addr;

-- 
2.45.2

[PATCH v2 4/5] MIPS: CPS: Support broken HCI for multicluster

[Gregory CLEMENT]

Some CM3.5 devices incorrectly report that hardware cache
initialization has completed, and also claim to support hardware cache
initialization when they don't actually do so. This commit fixes this
issue by retrieving the correct information from the device tree and
allowing the system to bypass the hardware cache initialization
step. Instead, it relies on manual operation. As a result, multi-user
support is now possible for these CPUs.

Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 arch/mips/kernel/smp-cps.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/smp-cps.c b/arch/mips/kernel/smp-cps.c
index b20ea4048429e1aab2bffbada793ee594bee1e05..e85bd087467e8caf0640ad247ee5f8eb65107591 100644
--- a/arch/mips/kernel/smp-cps.c
+++ b/arch/mips/kernel/smp-cps.c
@@ -333,6 +333,9 @@ static void __init cps_prepare_cpus(unsigned int max_cpus)
 					   sizeof(*mips_cps_cluster_bootcfg),
 					   GFP_KERNEL);
 
+	if (nclusters > 1)
+		mips_cm_update_property();
+
 	for (cl = 0; cl < nclusters; cl++) {
 		/* Allocate core boot configuration structs */
 		ncores = mips_cps_numcores(cl);
@@ -394,7 +397,7 @@ static void init_cluster_l2(void)
 {
 	u32 l2_cfg, l2sm_cop, result;
 
-	while (1) {
+	while (!mips_cm_is_l2_hci_broken) {
 		l2_cfg = read_gcr_redir_l2_ram_config();
 
 		/* If HCI is not supported, use the state machine below */

-- 
2.45.2

[PATCH v2 5/5] MIPS: mobileye: dts: eyeq6h: Enable cluster support

[Gregory CLEMENT]

The CM3.5 device used in EyeQ6H SoCs incorrectly reports the status
for Hardware Cache Initialization (HCI). This commit adds the
compatible string for the CM to acknowledge this issue, which enables
the use of the second CPU cluster.

Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
---
 arch/mips/boot/dts/mobileye/eyeq6h.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/mips/boot/dts/mobileye/eyeq6h.dtsi b/arch/mips/boot/dts/mobileye/eyeq6h.dtsi
index 1db3c3cda2e395025075387bcb66ea0737fd37f6..c6087bd8ca1d47855b8d93aa7e75e1fed219587b 100644
--- a/arch/mips/boot/dts/mobileye/eyeq6h.dtsi
+++ b/arch/mips/boot/dts/mobileye/eyeq6h.dtsi
@@ -32,6 +32,10 @@ cpu_intc: interrupt-controller {
 		#interrupt-cells = <1>;
 	};
 
+	coherency-manager {
+		compatible = "mti,eyeq6-cm";
+	};
+
 	soc: soc {
 		compatible = "simple-bus";
 		#address-cells = <2>;

-- 
2.45.2

Re: [PATCH v2 5/5] MIPS: mobileye: dts: eyeq6h: Enable cluster support

[Jiaxun Yang]

在2025年1月16日一月 上午10:59，Gregory CLEMENT写道：
> The CM3.5 device used in EyeQ6H SoCs incorrectly reports the status
> for Hardware Cache Initialization (HCI). This commit adds the
> compatible string for the CM to acknowledge this issue, which enables
> the use of the second CPU cluster.
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> ---
>  arch/mips/boot/dts/mobileye/eyeq6h.dtsi | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/mips/boot/dts/mobileye/eyeq6h.dtsi 
> b/arch/mips/boot/dts/mobileye/eyeq6h.dtsi
> index 
> 1db3c3cda2e395025075387bcb66ea0737fd37f6..c6087bd8ca1d47855b8d93aa7e75e1fed219587b 
> 100644
> --- a/arch/mips/boot/dts/mobileye/eyeq6h.dtsi
> +++ b/arch/mips/boot/dts/mobileye/eyeq6h.dtsi
> @@ -32,6 +32,10 @@ cpu_intc: interrupt-controller {
>  		#interrupt-cells = <1>;
>  	};
> 
> +	coherency-manager {
> +		compatible = "mti,eyeq6-cm";

                ^ 
                Maybe compatible = "mti,eyeq6-cm", "mti,mips-cm";

Thanks


-- 
- Jiaxun

Re: [PATCH v2 5/5] MIPS: mobileye: dts: eyeq6h: Enable cluster support

[Gregory CLEMENT]

Hello Jiaxun,

> 在2025年1月16日一月 上午10:59，Gregory CLEMENT写道：
>> The CM3.5 device used in EyeQ6H SoCs incorrectly reports the status
>> for Hardware Cache Initialization (HCI). This commit adds the
>> compatible string for the CM to acknowledge this issue, which enables
>> the use of the second CPU cluster.
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>> ---
>>  arch/mips/boot/dts/mobileye/eyeq6h.dtsi | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/mips/boot/dts/mobileye/eyeq6h.dtsi 
>> b/arch/mips/boot/dts/mobileye/eyeq6h.dtsi
>> index 
>> 1db3c3cda2e395025075387bcb66ea0737fd37f6..c6087bd8ca1d47855b8d93aa7e75e1fed219587b 
>> 100644
>> --- a/arch/mips/boot/dts/mobileye/eyeq6h.dtsi
>> +++ b/arch/mips/boot/dts/mobileye/eyeq6h.dtsi
>> @@ -32,6 +32,10 @@ cpu_intc: interrupt-controller {
>>  		#interrupt-cells = <1>;
>>  	};
>> 
>> +	coherency-manager {
>> +		compatible = "mti,eyeq6-cm";
>
>                 ^ 
>                 Maybe compatible = "mti,eyeq6-cm", "mti,mips-cm";

I explicitly makes the compatible "mti,eyeq6-cm" and "mti,mips-cm"
mutually exclusive. This allows us to require the reg property only for
"mti,mips-cm" and not have to provide it for "mti,eyeq6-cm".

Thanks for your prompt feedback,

Gregory

-- 
Grégory CLEMENT, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 5/5] MIPS: mobileye: dts: eyeq6h: Enable cluster support

[Jiaxun Yang]

在2025年1月16日一月 下午12:39，Gregory CLEMENT写道：
> Hello Jiaxun,
[...]
>>> +	coherency-manager {
>>> +		compatible = "mti,eyeq6-cm";
>>
>>                 ^ 
>>                 Maybe compatible = "mti,eyeq6-cm", "mti,mips-cm";
>
> I explicitly makes the compatible "mti,eyeq6-cm" and "mti,mips-cm"
> mutually exclusive. This allows us to require the reg property only for
> "mti,mips-cm" and not have to provide it for "mti,eyeq6-cm".

IMO reg should also be optional for "mti,mips-cm". There is nothing special
for eyeq6 on address probing and so on.

Thanks
-- 
- Jiaxun

Re: [PATCH v2 1/5] dt-bindings: mips: Document mti,mips-cm

[Rob Herring]

On Thu, Jan 16, 2025 at 11:59:19AM +0100, Gregory CLEMENT wrote:
> From: Jiaxun Yang <jiaxun.yang@flygoat.com>
> 
> Add devicetree binding documentation for MIPS Coherence Manager.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> ---
>  .../devicetree/bindings/mips/mti,mips-cm.yaml      | 38 ++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..9f500804737d23e19f50a9326168686c05d3a54e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mips/mti,mips-cm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MIPS Coherence Manager
> +
> +description: |

Don't need '|'.

> +  Defines a location of the MIPS Coherence Manager registers.

Say what the h/w block does.

> +
> +maintainers:
> +  - Jiaxun Yang <jiaxun.yang@flygoat.com>
> +
> +properties:
> +  compatible:
> +    const: mti,mips-cm

Convince me a genericish compatible is okay here.

> +
> +  reg:
> +    description:
> +      Base address and size of an unoccupied region in system's MMIO address
> +      space, which will be used to map the MIPS CM global control registers
> +      block. It is conventionally decided by the system integrator.

What is decided? The address? That's not relevant.

> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    coherency-manager@1fbf8000 {
> +      compatible = "mti,mips-cm";
> +      reg = <0x1bde8000 0x8000>;
> +    };
> +...
> 
> -- 
> 2.45.2
> 

Re: [PATCH v2 2/5] dt-bindings: mips: mips-cm: Add a new compatible string for EyeQ6

[Rob Herring]

On Thu, Jan 16, 2025 at 11:59:20AM +0100, Gregory CLEMENT wrote:
> The CM3.5 used on EyeQ6 reports that Hardware Cache Initialization is
> complete, but in reality it's not the case. It also incorrectly
> indicates that Hardware Cache Initialization is supported. This new
> compatible string allows warning about this broken feature that cannot
> be detected at runtime.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> ---
>  .../devicetree/bindings/mips/mti,mips-cm.yaml      | 24 ++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
> index 9f500804737d23e19f50a9326168686c05d3a54e..4713673f0cfc7785bb183917ee382a815ebfe9e1 100644
> --- a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
> +++ b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
> @@ -14,7 +14,12 @@ maintainers:
>  
>  properties:
>    compatible:
> -    const: mti,mips-cm
> +    oneOf:
> +      - const: mti,mips-cm
> +      - const: mti,eyeq6-cm

Being a mobileye device, the vendor prefix should be mobileye.

> +        description:
> +          On EyeQ6 the HCI (Hardware Cache Initialization) information for
> +          the L2 cache in multi-cluster configuration is broken.
>  
>    reg:
>      description:
> @@ -25,14 +30,29 @@ properties:
>  
>  required:
>    - compatible
> -  - reg
>  
>  additionalProperties: false
>  
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: mti,eyeq6-cm
> +then:
> +  properties:
> +    reg: false
> +else:
> +  required:
> +    - reg

How does one access this block with no registers? Is this some subset of 
a larger block? If so, need to define that block first.

These 2 blocks don't look related and the only property shared is 
'compatible'. This should be a separate doc.

Rob

Re: [PATCH v2 2/5] dt-bindings: mips: mips-cm: Add a new compatible string for EyeQ6

[Gregory CLEMENT]

Hello Rob,

> On Thu, Jan 16, 2025 at 11:59:20AM +0100, Gregory CLEMENT wrote:
>> The CM3.5 used on EyeQ6 reports that Hardware Cache Initialization is
>> complete, but in reality it's not the case. It also incorrectly
>> indicates that Hardware Cache Initialization is supported. This new
>> compatible string allows warning about this broken feature that cannot
>> be detected at runtime.
>> 
>> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>> ---
>>  .../devicetree/bindings/mips/mti,mips-cm.yaml      | 24 ++++++++++++++++++++--
>>  1 file changed, 22 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
>> index 9f500804737d23e19f50a9326168686c05d3a54e..4713673f0cfc7785bb183917ee382a815ebfe9e1 100644
>> --- a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
>> +++ b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
>> @@ -14,7 +14,12 @@ maintainers:
>>  
>>  properties:
>>    compatible:
>> -    const: mti,mips-cm
>> +    oneOf:
>> +      - const: mti,mips-cm
>> +      - const: mti,eyeq6-cm
>
> Being a mobileye device, the vendor prefix should be mobileye.

I chose mti because actually this block is part of the I6500 and
provided as is by MIPS.

>
>> +        description:
>> +          On EyeQ6 the HCI (Hardware Cache Initialization) information for
>> +          the L2 cache in multi-cluster configuration is broken.
>>  
>>    reg:
>>      description:
>> @@ -25,14 +30,29 @@ properties:
>>  
>>  required:
>>    - compatible
>> -  - reg
>>  
>>  additionalProperties: false
>>  
>> +if:
>> +  properties:
>> +    compatible:
>> +      contains:
>> +        const: mti,eyeq6-cm
>> +then:
>> +  properties:
>> +    reg: false
>> +else:
>> +  required:
>> +    - reg
>
> How does one access this block with no registers? Is this some subset of 
> a larger block? If so, need to define that block first.

CM stands for Coherence Manager. This component is mandatory when you
want to do SMP across MIPS core. This is part of the MIPS architecture,
and the address of the CM is provided by the Coprocessor 0.

"CP0 is incorporated on the CPU chip and supports the virtual memory
system and exception handling. CP0 is also referred to as the System
Control Coprocessor."

So to summarize, in a functional system, this information doesn't have
to be exposed through the device tree, as it is available at runtime
from any MIPS CPU.

>
> These 2 blocks don't look related and the only property shared is 
> 'compatible'. This should be a separate doc.

As mentioned in the cover letter, I reused the work from Jiaxun, who
needed to deal with bogus CM but in a different way. In his use case,
the issue with the CM was that the address in CP0 was wrong. In my case,
this address is correct; it is only one piece of information reported by
the CM that is wrong. I don't mind creating a separate doc if you
still think it is the right thing to do.

Gregory

>
> Rob

-- 
Grégory CLEMENT, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 2/5] dt-bindings: mips: mips-cm: Add a new compatible string for EyeQ6

[Jiaxun Yang]

在2025年1月17日一月 上午9:46，Gregory CLEMENT写道：
[...]

Hi all,

>>
>> These 2 blocks don't look related and the only property shared is 
>> 'compatible'. This should be a separate doc.
>
> As mentioned in the cover letter, I reused the work from Jiaxun, who
> needed to deal with bogus CM but in a different way. In his use case,
> the issue with the CM was that the address in CP0 was wrong. In my case,
> this address is correct; it is only one piece of information reported by
> the CM that is wrong. I don't mind creating a separate doc if you
> still think it is the right thing to do.

Precisely I'm dealing with two kind of systems, the first is systems doesn't
come with CP0.CMCGRBase, and thus rely on DeviceTree for probing the CM. The
second is systems mapping CMGCR at inappropriate locations and we want kernel
to remap it.

We don't want reg property to be mandatory as we are dealing with a huge amount
of legacy systems which mapping CM registers at different locations, while we
have to use a uniformed built-in DT and probe mapping at runtime.

Thanks
- Jiaxun
>
> Gregory
>
>>
>> Rob
>
> -- 
> Grégory CLEMENT, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

-- 
- Jiaxun

Re: [PATCH v2 2/5] dt-bindings: mips: mips-cm: Add a new compatible string for EyeQ6

[Krzysztof Kozlowski]

On Fri, Jan 17, 2025 at 10:46:00AM +0100, Gregory CLEMENT wrote:
> >> -    const: mti,mips-cm
> >> +    oneOf:
> >> +      - const: mti,mips-cm
> >> +      - const: mti,eyeq6-cm
> >
> > Being a mobileye device, the vendor prefix should be mobileye.
> 
> I chose mti because actually this block is part of the I6500 and
> provided as is by MIPS.

But MIPS or MTI did not create eyeq6, so then product does not fit
vendor.

> 
> >
> >> +        description:
> >> +          On EyeQ6 the HCI (Hardware Cache Initialization) information for
> >> +          the L2 cache in multi-cluster configuration is broken.
> >>  
> >>    reg:
> >>      description:
> >> @@ -25,14 +30,29 @@ properties:
> >>  
> >>  required:
> >>    - compatible
> >> -  - reg
> >>  
> >>  additionalProperties: false
> >>  
> >> +if:
> >> +  properties:
> >> +    compatible:
> >> +      contains:
> >> +        const: mti,eyeq6-cm
> >> +then:
> >> +  properties:
> >> +    reg: false
> >> +else:
> >> +  required:
> >> +    - reg
> >
> > How does one access this block with no registers? Is this some subset of 
> > a larger block? If so, need to define that block first.
> 
> CM stands for Coherence Manager. This component is mandatory when you
> want to do SMP across MIPS core. This is part of the MIPS architecture,
> and the address of the CM is provided by the Coprocessor 0.
> 
> "CP0 is incorporated on the CPU chip and supports the virtual memory
> system and exception handling. CP0 is also referred to as the System
> Control Coprocessor."
> 
> So to summarize, in a functional system, this information doesn't have
> to be exposed through the device tree, as it is available at runtime
> from any MIPS CPU.
> 
> >
> > These 2 blocks don't look related and the only property shared is 
> > 'compatible'. This should be a separate doc.
> 
> As mentioned in the cover letter, I reused the work from Jiaxun, who
> needed to deal with bogus CM but in a different way. In his use case,
> the issue with the CM was that the address in CP0 was wrong. In my case,
> this address is correct; it is only one piece of information reported by
> the CM that is wrong. I don't mind creating a separate doc if you
> still think it is the right thing to do.

So the programming interface in general is the same, but in one case the
reg/address detection does not work reliably?

I guess could stay the same doc, but all this should be explained in
binding description.

Best regards,
Krzysztof

Re: [PATCH v2 1/5] dt-bindings: mips: Document mti,mips-cm

[Gregory CLEMENT]

Hi Rob,

> On Thu, Jan 16, 2025 at 11:59:19AM +0100, Gregory CLEMENT wrote:
>> From: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> 
>> Add devicetree binding documentation for MIPS Coherence Manager.
>> 
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>> ---
>>  .../devicetree/bindings/mips/mti,mips-cm.yaml      | 38 ++++++++++++++++++++++
>>  1 file changed, 38 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..9f500804737d23e19f50a9326168686c05d3a54e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mips/mti,mips-cm.yaml
>> @@ -0,0 +1,38 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mips/mti,mips-cm.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: MIPS Coherence Manager
>> +
>> +description: |
>
> Don't need '|'.

OK

>
>> +  Defines a location of the MIPS Coherence Manager registers.
>
> Say what the h/w block does.

Managing coherency ? :)

However, more seriously, I can provide additional details based on what
I have written in my previous emails.

>
>> +
>> +maintainers:
>> +  - Jiaxun Yang <jiaxun.yang@flygoat.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: mti,mips-cm
>
> Convince me a genericish compatible is okay here.

well this componenant is really named Coherence Manager in the MIPS
related document. For example in p4 of
https://training.mips.com/cps_mips/PDF/CPS_Introduction.pdf and also
https://training.mips.com/cps_mips/PDF/Coherency_Manager.pdf.


>
>> +
>> +  reg:
>> +    description:
>> +      Base address and size of an unoccupied region in system's MMIO address
>> +      space, which will be used to map the MIPS CM global control registers
>> +      block. It is conventionally decided by the system integrator.
>
> What is decided? The address? That's not relevant.

Since reusing the description from Jiaxun, I'm unable to speak on his
behalf. From my perspective, it simply implies that the value to be
written should originate from the datasheet of the System-on-Chip (SoC),
as the address is is specific to the integration into a given
SoC.

Gregory

-- 
Grégory CLEMENT, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com