All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] MIPS: cm: Probe GCR address from devicetree
@ 2024-05-07  9:01 Jiaxun Yang
  2024-05-07  9:01 ` [PATCH 1/5] MIPS: generic: Do __dt_setup_arch in prom_init Jiaxun Yang
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Jiaxun Yang @ 2024-05-07  9:01 UTC (permalink / raw
  To: Paul Burton, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-mips, linux-kernel, devicetree, Jiaxun Yang

Hi all,

This series enabled mips-cm code to probe GCR address from devicetree.

This feature has been implemented in MIPS's out-of-tree kernel for
a while, and MIPS's u-boot fork on boston will generate required
"mti,mips-cm" node as well.

Please review.
Thanks

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
Jiaxun Yang (5):
      MIPS: generic: Do __dt_setup_arch in prom_init
      MIPS: cm: Prefix probe functions with __init
      MIPS: Move mips_cm_probe after prom_init
      dt-bindings: mips: Document mti,mips-cm
      MIPS: cm: Probe GCR address from DeviceTree

 .../devicetree/bindings/mips/mips-cm.yaml          | 37 ++++++++++++
 arch/mips/generic/init.c                           |  9 ++-
 arch/mips/include/asm/mips-cm.h                    |  4 +-
 arch/mips/kernel/mips-cm.c                         | 66 ++++++++++++++++++----
 arch/mips/kernel/setup.c                           |  2 +-
 5 files changed, 100 insertions(+), 18 deletions(-)
---
base-commit: 2b84edefcad14934796fad37b16512b6a2ca467e
change-id: 20240506-cm_probe-0c667c8b63bf

Best regards,
-- 
Jiaxun Yang <jiaxun.yang@flygoat.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/5] MIPS: generic: Do __dt_setup_arch in prom_init
  2024-05-07  9:01 [PATCH 0/5] MIPS: cm: Probe GCR address from devicetree Jiaxun Yang
@ 2024-05-07  9:01 ` Jiaxun Yang
  2024-05-07  9:01 ` [PATCH 2/5] MIPS: cm: Prefix probe functions with __init Jiaxun Yang
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Jiaxun Yang @ 2024-05-07  9:01 UTC (permalink / raw
  To: Paul Burton, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-mips, linux-kernel, devicetree, Jiaxun Yang

We want fdt parse functions to be available as early as possible,
thus do __dt_setup_arch immediately after we get fdt address in
prom_init.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/mips/generic/init.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/mips/generic/init.c b/arch/mips/generic/init.c
index 1d712eac1617..9fd09061de78 100644
--- a/arch/mips/generic/init.c
+++ b/arch/mips/generic/init.c
@@ -26,8 +26,12 @@ static __initconst const void *mach_match_data;
 
 void __init prom_init(void)
 {
+	fw_init_cmdline();
 	plat_get_fdt();
 	BUG_ON(!fdt);
+	if (mach && mach->fixup_fdt)
+		fdt = mach->fixup_fdt(fdt, mach_match_data);
+	__dt_setup_arch((void *)fdt);
 }
 
 void __init *plat_get_fdt(void)
@@ -101,11 +105,6 @@ void __init plat_fdt_relocated(void *new_location)
 
 void __init plat_mem_setup(void)
 {
-	if (mach && mach->fixup_fdt)
-		fdt = mach->fixup_fdt(fdt, mach_match_data);
-
-	fw_init_cmdline();
-	__dt_setup_arch((void *)fdt);
 }
 
 void __init device_tree_init(void)

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/5] MIPS: cm: Prefix probe functions with __init
  2024-05-07  9:01 [PATCH 0/5] MIPS: cm: Probe GCR address from devicetree Jiaxun Yang
  2024-05-07  9:01 ` [PATCH 1/5] MIPS: generic: Do __dt_setup_arch in prom_init Jiaxun Yang
@ 2024-05-07  9:01 ` Jiaxun Yang
  2024-05-07  9:01 ` [PATCH 3/5] MIPS: Move mips_cm_probe after prom_init Jiaxun Yang
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Jiaxun Yang @ 2024-05-07  9:01 UTC (permalink / raw
  To: Paul Burton, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-mips, linux-kernel, devicetree, Jiaxun Yang

Those functions are only used at boot time.
Prefix them with __init so they can be discarded after boot.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/mips/include/asm/mips-cm.h | 4 ++--
 arch/mips/kernel/mips-cm.c      | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/mips/include/asm/mips-cm.h b/arch/mips/include/asm/mips-cm.h
index c2930a75b7e4..5292b516d60b 100644
--- a/arch/mips/include/asm/mips-cm.h
+++ b/arch/mips/include/asm/mips-cm.h
@@ -30,7 +30,7 @@ extern void __iomem *mips_cm_l2sync_base;
  * and may be overridden by platforms which determine this address in a
  * different way by defining a function with the same prototype.
  */
-extern phys_addr_t mips_cm_phys_base(void);
+extern phys_addr_t __init mips_cm_phys_base(void);
 
 /**
  * mips_cm_l2sync_phys_base - retrieve the physical base address of the CM
@@ -43,7 +43,7 @@ extern phys_addr_t mips_cm_phys_base(void);
  * determine this address in a different way by defining a function with the
  * same prototype.
  */
-extern phys_addr_t mips_cm_l2sync_phys_base(void);
+extern phys_addr_t  __init mips_cm_l2sync_phys_base(void);
 
 /*
  * mips_cm_is64 - determine CM register width
diff --git a/arch/mips/kernel/mips-cm.c b/arch/mips/kernel/mips-cm.c
index 3a115fab5573..dddc9428fe58 100644
--- a/arch/mips/kernel/mips-cm.c
+++ b/arch/mips/kernel/mips-cm.c
@@ -179,7 +179,7 @@ static char *cm3_causes[32] = {
 static DEFINE_PER_CPU_ALIGNED(spinlock_t, cm_core_lock);
 static DEFINE_PER_CPU_ALIGNED(unsigned long, cm_core_lock_flags);
 
-phys_addr_t __weak mips_cm_phys_base(void)
+phys_addr_t __init __weak mips_cm_phys_base(void)
 {
 	unsigned long cmgcr;
 
@@ -198,7 +198,7 @@ phys_addr_t __weak mips_cm_phys_base(void)
 	return (cmgcr & MIPS_CMGCRF_BASE) << (36 - 32);
 }
 
-phys_addr_t __weak mips_cm_l2sync_phys_base(void)
+phys_addr_t __init __weak mips_cm_l2sync_phys_base(void)
 {
 	u32 base_reg;
 
@@ -214,7 +214,7 @@ phys_addr_t __weak mips_cm_l2sync_phys_base(void)
 	return mips_cm_phys_base() + MIPS_CM_GCR_SIZE;
 }
 
-static void mips_cm_probe_l2sync(void)
+static void __init mips_cm_probe_l2sync(void)
 {
 	unsigned major_rev;
 	phys_addr_t addr;
@@ -237,7 +237,7 @@ static void mips_cm_probe_l2sync(void)
 	mips_cm_l2sync_base = ioremap(addr, MIPS_CM_L2SYNC_SIZE);
 }
 
-int mips_cm_probe(void)
+int __init mips_cm_probe(void)
 {
 	phys_addr_t addr;
 	u32 base_reg;

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/5] MIPS: Move mips_cm_probe after prom_init
  2024-05-07  9:01 [PATCH 0/5] MIPS: cm: Probe GCR address from devicetree Jiaxun Yang
  2024-05-07  9:01 ` [PATCH 1/5] MIPS: generic: Do __dt_setup_arch in prom_init Jiaxun Yang
  2024-05-07  9:01 ` [PATCH 2/5] MIPS: cm: Prefix probe functions with __init Jiaxun Yang
@ 2024-05-07  9:01 ` Jiaxun Yang
  2024-05-08 12:50   ` Serge Semin
  2024-05-07  9:01 ` [PATCH 4/5] dt-bindings: mips: Document mti,mips-cm Jiaxun Yang
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Jiaxun Yang @ 2024-05-07  9:01 UTC (permalink / raw
  To: Paul Burton, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-mips, linux-kernel, devicetree, Jiaxun Yang

Move mips_cm_probe after prom_init so we can use fdt functions
in mips_cm_probe to obtain essential information.

Impat for all systems that may have CM in system:
- geneirc: Adjusted code to accommodate this change
- Lantiq: No impact, CM configuration won't be changed at all
- ralink: Called mips_cm_probe on it's own, in prom_init->prom_soc_init
- malta: No impact, CM address comes from CP0_CMGCR

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/mips/kernel/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 12a1a4ffb602..732579c8f4f8 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -773,8 +773,8 @@ static void __init setup_rng_seed(void)
 void __init setup_arch(char **cmdline_p)
 {
 	cpu_probe();
-	mips_cm_probe();
 	prom_init();
+	mips_cm_probe();
 
 	setup_early_fdc_console();
 #ifdef CONFIG_EARLY_PRINTK

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/5] dt-bindings: mips: Document mti,mips-cm
  2024-05-07  9:01 [PATCH 0/5] MIPS: cm: Probe GCR address from devicetree Jiaxun Yang
                   ` (2 preceding siblings ...)
  2024-05-07  9:01 ` [PATCH 3/5] MIPS: Move mips_cm_probe after prom_init Jiaxun Yang
@ 2024-05-07  9:01 ` Jiaxun Yang
  2024-05-07 16:50   ` Conor Dooley
  2024-05-07  9:01 ` [PATCH 5/5] MIPS: cm: Probe GCR address from DeviceTree Jiaxun Yang
  2024-05-08 12:37 ` [PATCH 0/5] MIPS: cm: Probe GCR address from devicetree Serge Semin
  5 siblings, 1 reply; 15+ messages in thread
From: Jiaxun Yang @ 2024-05-07  9:01 UTC (permalink / raw
  To: Paul Burton, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-mips, linux-kernel, devicetree, Jiaxun Yang

Add devicetree binding documentation for MIPS Coherence Manager.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 .../devicetree/bindings/mips/mips-cm.yaml          | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/Documentation/devicetree/bindings/mips/mips-cm.yaml b/Documentation/devicetree/bindings/mips/mips-cm.yaml
new file mode 100644
index 000000000000..b92b008d7758
--- /dev/null
+++ b/Documentation/devicetree/bindings/mips/mips-cm.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mips/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 memory region, which will be
+      used to map the MIPS CM registers block.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    cm@1fbf8000 {
+      compatible = "mti,mips-cm";
+      reg = <0x1bde8000 0x8000>;
+    };
+...

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/5] MIPS: cm: Probe GCR address from DeviceTree
  2024-05-07  9:01 [PATCH 0/5] MIPS: cm: Probe GCR address from devicetree Jiaxun Yang
                   ` (3 preceding siblings ...)
  2024-05-07  9:01 ` [PATCH 4/5] dt-bindings: mips: Document mti,mips-cm Jiaxun Yang
@ 2024-05-07  9:01 ` Jiaxun Yang
  2024-05-08  4:25   ` kernel test robot
  2024-05-08 12:37 ` [PATCH 0/5] MIPS: cm: Probe GCR address from devicetree Serge Semin
  5 siblings, 1 reply; 15+ messages in thread
From: Jiaxun Yang @ 2024-05-07  9:01 UTC (permalink / raw
  To: Paul Burton, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-mips, linux-kernel, devicetree, Jiaxun Yang

Traditionally, CM GCR address can be probed from CP0_CMGCRBase.

However there are chips in wild that do have CM GCR but CP0_CMGCRBase
is not available from CPU point of view. Thus we need to be able to
probe GCR address from DeviceTree.

It is implemented as:
- If only CP0_CMGCRBase present, trust CP0_CMGCRBase
- If only mti,mips-cm node present, trust mti,mips-cm reg prop
- If both present, remap address space to address specified in dt

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/mips/kernel/mips-cm.c | 58 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 52 insertions(+), 6 deletions(-)

diff --git a/arch/mips/kernel/mips-cm.c b/arch/mips/kernel/mips-cm.c
index dddc9428fe58..9563fc1ad438 100644
--- a/arch/mips/kernel/mips-cm.c
+++ b/arch/mips/kernel/mips-cm.c
@@ -5,6 +5,8 @@
  */
 
 #include <linux/errno.h>
+#include <linux/of_address.h>
+#include <linux/of_fdt.h>
 #include <linux/percpu.h>
 #include <linux/spinlock.h>
 
@@ -179,23 +181,67 @@ static char *cm3_causes[32] = {
 static DEFINE_PER_CPU_ALIGNED(spinlock_t, cm_core_lock);
 static DEFINE_PER_CPU_ALIGNED(unsigned long, cm_core_lock_flags);
 
+static int __init mips_cm_fdt_scan(unsigned long node, const char *uname,
+		int depth, void *data)
+{
+	unsigned long *cmgcr = data;
+
+	if (!of_flat_dt_is_compatible(node, "mti,mips-cm"))
+		return 0;
+
+	*cmgcr = of_flat_dt_translate_address(node);
+	if (*cmgcr == OF_BAD_ADDR)
+		*cmgcr = 0;
+
+	return 0;
+}
+
 phys_addr_t __init __weak mips_cm_phys_base(void)
 {
-	unsigned long cmgcr;
+	unsigned long gcr_reg = 0, gcr_dt = 0;
+
+	if (of_have_populated_dt()) {
+		int err;
+		struct resource res;
+		struct device_node *cm_node;
+
+		cm_node = of_find_compatible_node(of_root, NULL, "mti,mips-cm");
+		if (cm_node) {
+			err = of_address_to_resource(cm_node, 0, &res);
+			of_node_put(cm_node);
+			if (!err)
+				gcr_dt = res.start;
+		}
+	} else {
+		of_scan_flat_dt(mips_cm_fdt_scan, &gcr_dt);
+	}
 
 	/* Check the CMGCRBase register is implemented */
 	if (!(read_c0_config() & MIPS_CONF_M))
-		return 0;
+		return gcr_dt;
 
 	if (!(read_c0_config2() & MIPS_CONF_M))
-		return 0;
+		return gcr_dt;
 
 	if (!(read_c0_config3() & MIPS_CONF3_CMGCR))
-		return 0;
+		return gcr_dt;
 
 	/* Read the address from CMGCRBase */
-	cmgcr = read_c0_cmgcrbase();
-	return (cmgcr & MIPS_CMGCRF_BASE) << (36 - 32);
+	gcr_reg = read_c0_cmgcrbase();
+	gcr_reg = (gcr_reg & MIPS_CMGCRF_BASE) << (36 - 32);
+
+	/* If no of node, return straight away */
+	if (!gcr_dt)
+		return gcr_reg;
+
+	/* If the CMGCRBase mismatches with dt, remap it */
+	if (gcr_reg != gcr_dt) {
+		pr_info("Remapping CMGCRBase from 0x%08lx to 0x%08lx\n",
+			gcr_reg, gcr_dt);
+		change_gcr_base(CM_GCR_BASE_GCRBASE, gcr_dt);
+	}
+
+	return gcr_dt;
 }
 
 phys_addr_t __init __weak mips_cm_l2sync_phys_base(void)

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/5] dt-bindings: mips: Document mti,mips-cm
  2024-05-07  9:01 ` [PATCH 4/5] dt-bindings: mips: Document mti,mips-cm Jiaxun Yang
@ 2024-05-07 16:50   ` Conor Dooley
  2024-05-07 18:16     ` Jiaxun Yang
  0 siblings, 1 reply; 15+ messages in thread
From: Conor Dooley @ 2024-05-07 16:50 UTC (permalink / raw
  To: Jiaxun Yang
  Cc: Paul Burton, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-mips, linux-kernel,
	devicetree

[-- Attachment #1: Type: text/plain, Size: 2156 bytes --]

On Tue, May 07, 2024 at 10:01:52AM +0100, Jiaxun Yang wrote:
> Add devicetree binding documentation for MIPS Coherence Manager.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  .../devicetree/bindings/mips/mips-cm.yaml          | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mips/mips-cm.yaml b/Documentation/devicetree/bindings/mips/mips-cm.yaml
> new file mode 100644
> index 000000000000..b92b008d7758
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mips/mips-cm.yaml

Filename matching the compatible please.

> @@ -0,0 +1,37 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mips/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

Is it actually only available on mips? Google seems to report there
being Coherence Managers on their RISC-V offerings too.

> +  reg:
> +    description: |

The | isn't needed, there's no formatting to preserve.

> +      Base address and size of an unoccupied memory region, which will be
> +      used to map the MIPS CM registers block.

This sounds like it should actually be a memory-region that references
some reserved memory, not a reg, given the description. I think the
commit message here is lacking any information about what the intentions
are for this binding.

> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    cm@1fbf8000 {

And a generic node name here please. I actually don't quite know what to
suggest though, but "coherency-manager" would likely be better than
"cm".

Thanks,
Conor.

> +      compatible = "mti,mips-cm";
> +      reg = <0x1bde8000 0x8000>;
> +    };
> +...
> 
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/5] dt-bindings: mips: Document mti,mips-cm
  2024-05-07 16:50   ` Conor Dooley
@ 2024-05-07 18:16     ` Jiaxun Yang
  2024-05-08 17:01       ` Conor Dooley
  0 siblings, 1 reply; 15+ messages in thread
From: Jiaxun Yang @ 2024-05-07 18:16 UTC (permalink / raw
  To: Conor Dooley
  Cc: paulburton@kernel.org, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-mips@vger.kernel.org,
	linux-kernel, devicetree



在2024年5月7日五月 下午5:50,Conor Dooley写道:
> On Tue, May 07, 2024 at 10:01:52AM +0100, Jiaxun Yang wrote:
>> Add devicetree binding documentation for MIPS Coherence Manager.
>> 
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>  .../devicetree/bindings/mips/mips-cm.yaml          | 37 ++++++++++++++++++++++
>>  1 file changed, 37 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/mips/mips-cm.yaml b/Documentation/devicetree/bindings/mips/mips-cm.yaml
>> new file mode 100644
>> index 000000000000..b92b008d7758
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mips/mips-cm.yaml
Hi Cornor,

Thanks for your comments.

>
> Filename matching the compatible please.
Ok.

>
>> @@ -0,0 +1,37 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mips/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
>
> Is it actually only available on mips? Google seems to report there
> being Coherence Managers on their RISC-V offerings too.

I think for MIPS's RISC-V system, it is only used by SBI and transparent
to kernel, so it won't present in DT. 

Register fields for RISC-V system is totally different with MIPS one, and
there is no driver to be reused. In MIPS system CM code is highly coupled
with arch code, so for RISC-V if we want to expose it to kernel we'll need
a new set of driver and a new binding.

>
>> +  reg:
>> +    description: |
>
> The | isn't needed, there's no formatting to preserve.
Ok.

>
>> +      Base address and size of an unoccupied memory region, which will be
>> +      used to map the MIPS CM registers block.
>
> This sounds like it should actually be a memory-region that references
> some reserved memory, not a reg, given the description. I think the
> commit message here is lacking any information about what the intentions
> are for this binding.
So it's actually a register block that can be remapped to anywhere in
MMIO address space. DeviceTree usually passes firmware's mapping location
to kernel.

There are some other similar bindings like mti,mips-cdmm and mti,mips-cpc,
I just copied phraseology from them, should I try to explain it more here?

>
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    cm@1fbf8000 {
>
> And a generic node name here please. I actually don't quite know what to
> suggest though, but "coherency-manager" would likely be better than
> "cm".
Ok

Thanks!
- Jiaxun
>
> Thanks,
> Conor.
>
>> +      compatible = "mti,mips-cm";
>> +      reg = <0x1bde8000 0x8000>;
>> +    };
>> +...
>> 
>> -- 
>> 2.34.1
>> 
>
> 附件:
> * signature.asc

-- 
- Jiaxun

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/5] MIPS: cm: Probe GCR address from DeviceTree
  2024-05-07  9:01 ` [PATCH 5/5] MIPS: cm: Probe GCR address from DeviceTree Jiaxun Yang
@ 2024-05-08  4:25   ` kernel test robot
  0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2024-05-08  4:25 UTC (permalink / raw
  To: Jiaxun Yang, Paul Burton, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: oe-kbuild-all, linux-mips, linux-kernel, devicetree, Jiaxun Yang

Hi Jiaxun,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 2b84edefcad14934796fad37b16512b6a2ca467e]

url:    https://github.com/intel-lab-lkp/linux/commits/Jiaxun-Yang/MIPS-generic-Do-__dt_setup_arch-in-prom_init/20240507-170413
base:   2b84edefcad14934796fad37b16512b6a2ca467e
patch link:    https://lore.kernel.org/r/20240507-cm_probe-v1-5-11dbfd598f3c%40flygoat.com
patch subject: [PATCH 5/5] MIPS: cm: Probe GCR address from DeviceTree
config: mips-randconfig-r113-20240508 (https://download.01.org/0day-ci/archive/20240508/202405081255.MGvZEuuc-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 0ab4458df0688955620b72cc2c72a32dffad3615)
reproduce: (https://download.01.org/0day-ci/archive/20240508/202405081255.MGvZEuuc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405081255.MGvZEuuc-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> arch/mips/kernel/mips-cm.c:193:13: warning: result of comparison of constant 18446744073709551615 with expression of type 'unsigned long' is always false [-Wtautological-constant-out-of-range-compare]
     193 |         if (*cmgcr == OF_BAD_ADDR)
         |             ~~~~~~ ^  ~~~~~~~~~~~
   1 warning generated.


vim +193 arch/mips/kernel/mips-cm.c

   183	
   184	static int __init mips_cm_fdt_scan(unsigned long node, const char *uname,
   185			int depth, void *data)
   186	{
   187		unsigned long *cmgcr = data;
   188	
   189		if (!of_flat_dt_is_compatible(node, "mti,mips-cm"))
   190			return 0;
   191	
   192		*cmgcr = of_flat_dt_translate_address(node);
 > 193		if (*cmgcr == OF_BAD_ADDR)
   194			*cmgcr = 0;
   195	
   196		return 0;
   197	}
   198	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/5] MIPS: cm: Probe GCR address from devicetree
  2024-05-07  9:01 [PATCH 0/5] MIPS: cm: Probe GCR address from devicetree Jiaxun Yang
                   ` (4 preceding siblings ...)
  2024-05-07  9:01 ` [PATCH 5/5] MIPS: cm: Probe GCR address from DeviceTree Jiaxun Yang
@ 2024-05-08 12:37 ` Serge Semin
  5 siblings, 0 replies; 15+ messages in thread
From: Serge Semin @ 2024-05-08 12:37 UTC (permalink / raw
  To: Jiaxun Yang
  Cc: Paul Burton, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-mips, linux-kernel,
	devicetree

Hi Jiaxun

On Tue, May 07, 2024 at 10:01:48AM +0100, Jiaxun Yang wrote:
> Hi all,
> 
> This series enabled mips-cm code to probe GCR address from devicetree.
> 
> This feature has been implemented in MIPS's out-of-tree kernel for
> a while, and MIPS's u-boot fork on boston will generate required
> "mti,mips-cm" node as well.

Thank you very much for the series. This work has been scheduled in my
TODO list for years. I could have done it earlier but a simple at the
first glance change turned to be tricky. The main concern was the
stage at what CM was probed. I was afraid to break things by changing
the order of the CM-base address getting. Let's discuss this matter in
the respective patch. It might get to be I was wrong to worry.

-Serge(y)

> 
> Please review.
> Thanks
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> Jiaxun Yang (5):
>       MIPS: generic: Do __dt_setup_arch in prom_init
>       MIPS: cm: Prefix probe functions with __init
>       MIPS: Move mips_cm_probe after prom_init
>       dt-bindings: mips: Document mti,mips-cm
>       MIPS: cm: Probe GCR address from DeviceTree
> 
>  .../devicetree/bindings/mips/mips-cm.yaml          | 37 ++++++++++++
>  arch/mips/generic/init.c                           |  9 ++-
>  arch/mips/include/asm/mips-cm.h                    |  4 +-
>  arch/mips/kernel/mips-cm.c                         | 66 ++++++++++++++++++----
>  arch/mips/kernel/setup.c                           |  2 +-
>  5 files changed, 100 insertions(+), 18 deletions(-)
> ---
> base-commit: 2b84edefcad14934796fad37b16512b6a2ca467e
> change-id: 20240506-cm_probe-0c667c8b63bf
> 
> Best regards,
> -- 
> Jiaxun Yang <jiaxun.yang@flygoat.com>
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/5] MIPS: Move mips_cm_probe after prom_init
  2024-05-07  9:01 ` [PATCH 3/5] MIPS: Move mips_cm_probe after prom_init Jiaxun Yang
@ 2024-05-08 12:50   ` Serge Semin
  2024-05-08 16:23     ` Jiaxun Yang
  0 siblings, 1 reply; 15+ messages in thread
From: Serge Semin @ 2024-05-08 12:50 UTC (permalink / raw
  To: Jiaxun Yang
  Cc: Paul Burton, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-mips, linux-kernel,
	devicetree

On Tue, May 07, 2024 at 10:01:51AM +0100, Jiaxun Yang wrote:
> Move mips_cm_probe after prom_init so we can use fdt functions
> in mips_cm_probe to obtain essential information.
> 
> Impat for all systems that may have CM in system:

> - geneirc: Adjusted code to accommodate this change

s/geneirc/generic

> - Lantiq: No impact, CM configuration won't be changed at all
> - ralink: Called mips_cm_probe on it's own, in prom_init->prom_soc_init

> - malta: No impact, CM address comes from CP0_CMGCR

Are you sure about this? This was one of the problematic part I met
back when was trying to implement the feature.
arch/mips/mti-malta/malta-init.c:
prom_init()
+-> mips_cpc_probe()
    +-> mips_cpc_phys_base()
        +-> mips_cm_present(): mips_gcr_base != NULL
        +-> read_gcr_cpc_status()
        +-> read_gcr_cpc_base()
        +-> write_gcr_cpc_base()

So by moving mips_cm_probe() to being executed after prom_init() the
calls-chain above will be broken since the mips_gcr_base will be left
uninitialized. Do I miss something?

Please, note originally the mips_cm_probe() invocation was right
above the Malta's mips_cpc_probe():
3af5a67c86a3 ("MIPS: Fix early CM probing")

-Serge(y)

> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  arch/mips/kernel/setup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index 12a1a4ffb602..732579c8f4f8 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -773,8 +773,8 @@ static void __init setup_rng_seed(void)
>  void __init setup_arch(char **cmdline_p)
>  {
>  	cpu_probe();
> -	mips_cm_probe();
>  	prom_init();
> +	mips_cm_probe();
>  
>  	setup_early_fdc_console();
>  #ifdef CONFIG_EARLY_PRINTK
> 
> -- 
> 2.34.1
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/5] MIPS: Move mips_cm_probe after prom_init
  2024-05-08 12:50   ` Serge Semin
@ 2024-05-08 16:23     ` Jiaxun Yang
  0 siblings, 0 replies; 15+ messages in thread
From: Jiaxun Yang @ 2024-05-08 16:23 UTC (permalink / raw
  To: Serge Semin
  Cc: paulburton@kernel.org, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-mips@vger.kernel.org,
	linux-kernel, devicetree



在2024年5月8日五月 下午1:50,Serge Semin写道:
> On Tue, May 07, 2024 at 10:01:51AM +0100, Jiaxun Yang wrote:
>> Move mips_cm_probe after prom_init so we can use fdt functions
>> in mips_cm_probe to obtain essential information.
>> 
>> Impat for all systems that may have CM in system:
>
>> - geneirc: Adjusted code to accommodate this change
>
> s/geneirc/generic
>
>> - Lantiq: No impact, CM configuration won't be changed at all
>> - ralink: Called mips_cm_probe on it's own, in prom_init->prom_soc_init
>
>> - malta: No impact, CM address comes from CP0_CMGCR
>
> Are you sure about this? This was one of the problematic part I met
> back when was trying to implement the feature.
> arch/mips/mti-malta/malta-init.c:
> prom_init()
> +-> mips_cpc_probe()
>     +-> mips_cpc_phys_base()
>         +-> mips_cm_present(): mips_gcr_base != NULL
>         +-> read_gcr_cpc_status()
>         +-> read_gcr_cpc_base()
>         +-> write_gcr_cpc_base()
>
> So by moving mips_cm_probe() to being executed after prom_init() the
> calls-chain above will be broken since the mips_gcr_base will be left
> uninitialized. Do I miss something?

Hi Serge,

Good catch! This is indeed a problem.
I tried to dig a little bit and found that a possible solution is to
move SMP initialization to device_tree_init(), as what generic platform
did.

I was able to test malta boot with this solution on my heavily patched
QEMU, I can confirm that both CM-SMP and scache are working as expected.

Will share my QEMU patches and configurations in next rev. 

Thanks
- Jiaxun

>
> Please, note originally the mips_cm_probe() invocation was right
> above the Malta's mips_cpc_probe():
> 3af5a67c86a3 ("MIPS: Fix early CM probing")
>
> -Serge(y)
>
>> 
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>  arch/mips/kernel/setup.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
>> index 12a1a4ffb602..732579c8f4f8 100644
>> --- a/arch/mips/kernel/setup.c
>> +++ b/arch/mips/kernel/setup.c
>> @@ -773,8 +773,8 @@ static void __init setup_rng_seed(void)
>>  void __init setup_arch(char **cmdline_p)
>>  {
>>  	cpu_probe();
>> -	mips_cm_probe();
>>  	prom_init();
>> +	mips_cm_probe();
>>  
>>  	setup_early_fdc_console();
>>  #ifdef CONFIG_EARLY_PRINTK
>> 
>> -- 
>> 2.34.1
>> 
>>

-- 
- Jiaxun

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/5] dt-bindings: mips: Document mti,mips-cm
  2024-05-07 18:16     ` Jiaxun Yang
@ 2024-05-08 17:01       ` Conor Dooley
  2024-05-08 20:28         ` Jiaxun Yang
  0 siblings, 1 reply; 15+ messages in thread
From: Conor Dooley @ 2024-05-08 17:01 UTC (permalink / raw
  To: Jiaxun Yang
  Cc: paulburton@kernel.org, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-mips@vger.kernel.org,
	linux-kernel, devicetree

[-- Attachment #1: Type: text/plain, Size: 3284 bytes --]

On Tue, May 07, 2024 at 07:16:25PM +0100, Jiaxun Yang wrote:
> 
> 
> 在2024年5月7日五月 下午5:50,Conor Dooley写道:
> > On Tue, May 07, 2024 at 10:01:52AM +0100, Jiaxun Yang wrote:
> >> Add devicetree binding documentation for MIPS Coherence Manager.
> >> 
> >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> >> ---
> >>  .../devicetree/bindings/mips/mips-cm.yaml          | 37 ++++++++++++++++++++++
> >>  1 file changed, 37 insertions(+)
> >> 
> >> diff --git a/Documentation/devicetree/bindings/mips/mips-cm.yaml b/Documentation/devicetree/bindings/mips/mips-cm.yaml
> >> new file mode 100644
> >> index 000000000000..b92b008d7758
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mips/mips-cm.yaml
> Hi Cornor,
> 
> Thanks for your comments.
> 
> >
> > Filename matching the compatible please.
> Ok.
> 
> >
> >> @@ -0,0 +1,37 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/mips/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
> >
> > Is it actually only available on mips? Google seems to report there
> > being Coherence Managers on their RISC-V offerings too.
> 
> I think for MIPS's RISC-V system, it is only used by SBI and transparent
> to kernel, so it won't present in DT. 

Devicetree isn't just for Linux, things that only the SBI implementation
cares about should also be documented in bindings - or at least I try to
get them to be, where I have enough sway to have it happen..

> Register fields for RISC-V system is totally different with MIPS one, and
> there is no driver to be reused. In MIPS system CM code is highly coupled
> with arch code, so for RISC-V if we want to expose it to kernel we'll need
> a new set of driver and a new binding.

Right, that's a reasonable reason (lol) for having it be declared as
mips-specific.

> >> +  reg:
> >> +    description: |
> >
> > The | isn't needed, there's no formatting to preserve.
> Ok.
> 
> >
> >> +      Base address and size of an unoccupied memory region, which will be
> >> +      used to map the MIPS CM registers block.
> >
> > This sounds like it should actually be a memory-region that references
> > some reserved memory, not a reg, given the description. I think the
> > commit message here is lacking any information about what the intentions
> > are for this binding.
> So it's actually a register block that can be remapped to anywhere in
> MMIO address space. DeviceTree usually passes firmware's mapping location
> to kernel.
> 
> There are some other similar bindings like mti,mips-cdmm and mti,mips-cpc,
> I just copied phraseology from them, should I try to explain it more here?

The description that you've given here is of something that sounded
awfully like mapping into a location in DDR etc, is it actually being
mapped into a non-memory address?

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/5] dt-bindings: mips: Document mti,mips-cm
  2024-05-08 17:01       ` Conor Dooley
@ 2024-05-08 20:28         ` Jiaxun Yang
  2024-05-09 17:24           ` Conor Dooley
  0 siblings, 1 reply; 15+ messages in thread
From: Jiaxun Yang @ 2024-05-08 20:28 UTC (permalink / raw
  To: Conor Dooley
  Cc: paulburton@kernel.org, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-mips@vger.kernel.org,
	linux-kernel, devicetree



在 2024/5/8 18:01, Conor Dooley 写道:
[...]
>> So it's actually a register block that can be remapped to anywhere in
>> MMIO address space. DeviceTree usually passes firmware's mapping location
>> to kernel.
>>
>> There are some other similar bindings like mti,mips-cdmm and mti,mips-cpc,
>> I just copied phraseology from them, should I try to explain it more here?
> The description that you've given here is of something that sounded
> awfully like mapping into a location in DDR etc, is it actually being
> mapped into a non-memory address?
It is an overlay being realized at CPU core level so it can be mapped at any
where, but the firmware convention is to map it to a "non-memory address".

Thanks
- Jiaxun
>
> Thanks,
> Conor.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/5] dt-bindings: mips: Document mti,mips-cm
  2024-05-08 20:28         ` Jiaxun Yang
@ 2024-05-09 17:24           ` Conor Dooley
  0 siblings, 0 replies; 15+ messages in thread
From: Conor Dooley @ 2024-05-09 17:24 UTC (permalink / raw
  To: Jiaxun Yang
  Cc: paulburton@kernel.org, Thomas Bogendoerfer, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-mips@vger.kernel.org,
	linux-kernel, devicetree

[-- Attachment #1: Type: text/plain, Size: 949 bytes --]

On Wed, May 08, 2024 at 09:28:27PM +0100, Jiaxun Yang wrote:
> 
> 
> 在 2024/5/8 18:01, Conor Dooley 写道:
> [...]
> > > So it's actually a register block that can be remapped to anywhere in
> > > MMIO address space. DeviceTree usually passes firmware's mapping location
> > > to kernel.
> > > 
> > > There are some other similar bindings like mti,mips-cdmm and mti,mips-cpc,
> > > I just copied phraseology from them, should I try to explain it more here?
> > The description that you've given here is of something that sounded
> > awfully like mapping into a location in DDR etc, is it actually being
> > mapped into a non-memory address?
> It is an overlay being realized at CPU core level so it can be mapped at any
> where, but the firmware convention is to map it to a "non-memory address".

In that case, a description that even eejits like my can understand
sounds like all you need to understand :)

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2024-05-09 17:24 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-07  9:01 [PATCH 0/5] MIPS: cm: Probe GCR address from devicetree Jiaxun Yang
2024-05-07  9:01 ` [PATCH 1/5] MIPS: generic: Do __dt_setup_arch in prom_init Jiaxun Yang
2024-05-07  9:01 ` [PATCH 2/5] MIPS: cm: Prefix probe functions with __init Jiaxun Yang
2024-05-07  9:01 ` [PATCH 3/5] MIPS: Move mips_cm_probe after prom_init Jiaxun Yang
2024-05-08 12:50   ` Serge Semin
2024-05-08 16:23     ` Jiaxun Yang
2024-05-07  9:01 ` [PATCH 4/5] dt-bindings: mips: Document mti,mips-cm Jiaxun Yang
2024-05-07 16:50   ` Conor Dooley
2024-05-07 18:16     ` Jiaxun Yang
2024-05-08 17:01       ` Conor Dooley
2024-05-08 20:28         ` Jiaxun Yang
2024-05-09 17:24           ` Conor Dooley
2024-05-07  9:01 ` [PATCH 5/5] MIPS: cm: Probe GCR address from DeviceTree Jiaxun Yang
2024-05-08  4:25   ` kernel test robot
2024-05-08 12:37 ` [PATCH 0/5] MIPS: cm: Probe GCR address from devicetree Serge Semin

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.