Skip to contentSkip to conversation

MIPS: mips-cm: CM regions are disabled at boot

PATCH In review Niklas Svensson 3 participants 4 messages Nov 20, 2014 MIPS

lore
Share & tools
lore

Niklas Svensson

author
#0 lore raw

PATCH 1/1 MIPS: mips-cm: CM regions are disabled at boot

Each CM_REGION_TARGET is set to disabled at boot,
so there is no need to disable the matching
CM_GCR_REG explicitly.

Signed-off-by Niklas Svensson <[email protected]>
---
 arch/mips/kernel/mips-cm.c | 10 ----------
 1 file changed, 10 deletions(-)
arch/mips/kernel/mips-cm.c +0 -10
index f76f7a0..a4bbfd9 100644
@@ -104,16 +104,6 @@ int mips_cm_probe(void)
 	base_reg |= CM_GCR_BASE_CMDEFTGT_MEM;
 	write_gcr_base(base_reg);
 
-	/* disable CM regions */
-	write_gcr_reg0_base(CM_GCR_REGn_BASE_BASEADDR_MSK);
-	write_gcr_reg0_mask(CM_GCR_REGn_MASK_ADDRMASK_MSK);
-	write_gcr_reg1_base(CM_GCR_REGn_BASE_BASEADDR_MSK);
-	write_gcr_reg1_mask(CM_GCR_REGn_MASK_ADDRMASK_MSK);
-	write_gcr_reg2_base(CM_GCR_REGn_BASE_BASEADDR_MSK);
-	write_gcr_reg2_mask(CM_GCR_REGn_MASK_ADDRMASK_MSK);
-	write_gcr_reg3_base(CM_GCR_REGn_BASE_BASEADDR_MSK);
-	write_gcr_reg3_mask(CM_GCR_REGn_MASK_ADDRMASK_MSK);
-
 	/* probe for an L2-only sync region */
 	mips_cm_probe_l2sync();
Signature · 1 line
2.1.3
#1 lore raw
Quoted 1 line · Niklas Svensson

Each CM_REGION_TARGET is set to disabled at boot,

That part is true...

Quoted 2 lines

so there is no need to disable the matching
CM_GCR_REG explicitly.

...however there is no guarantee that the bootloader, or another kernel, or some other piece of code didn't program the registers differently before Linux starts. So I believe this patch to be incorrect.

Thanks,
Paul

Quoted 30 lines
Signed-off-by Niklas Svensson <[email protected]>
---
 arch/mips/kernel/mips-cm.c | 10 ----------
 1 file changed, 10 deletions(-)
arch/mips/kernel/mips-cm.c +0 -10
index f76f7a0..a4bbfd9 100644
@@ -104,16 +104,6 @@ int mips_cm_probe(void)
 	base_reg |= CM_GCR_BASE_CMDEFTGT_MEM;
 	write_gcr_base(base_reg);
 
-	/* disable CM regions */
-	write_gcr_reg0_base(CM_GCR_REGn_BASE_BASEADDR_MSK);
-	write_gcr_reg0_mask(CM_GCR_REGn_MASK_ADDRMASK_MSK);
-	write_gcr_reg1_base(CM_GCR_REGn_BASE_BASEADDR_MSK);
-	write_gcr_reg1_mask(CM_GCR_REGn_MASK_ADDRMASK_MSK);
-	write_gcr_reg2_base(CM_GCR_REGn_BASE_BASEADDR_MSK);
-	write_gcr_reg2_mask(CM_GCR_REGn_MASK_ADDRMASK_MSK);
-	write_gcr_reg3_base(CM_GCR_REGn_BASE_BASEADDR_MSK);
-	write_gcr_reg3_mask(CM_GCR_REGn_MASK_ADDRMASK_MSK);
-
 	/* probe for an L2-only sync region */
 	mips_cm_probe_l2sync();
Signature · 1 line
2.1.3
author
#2 lore raw
Quoted 11 lines · Paul Burton
Quoted 1 line · Niklas Svensson

Each CM_REGION_TARGET is set to disabled at boot,

That part is true...

Quoted 2 lines

so there is no need to disable the matching
CM_GCR_REG explicitly.

...however there is no guarantee that the bootloader, or another kernel, or some other piece of code didn't program the registers differently before Linux starts. So I believe this patch to be incorrect.

That is the reason for this patch. This will overwrite settings written by the bootloader.

Say if the bootloader sets up an iocu, then these writes will clear those settings.

Quoted 34 lines

Thanks,
Paul

Quoted 30 lines
Signed-off-by Niklas Svensson <[email protected]>
---
 arch/mips/kernel/mips-cm.c | 10 ----------
 1 file changed, 10 deletions(-)
arch/mips/kernel/mips-cm.c +0 -10
index f76f7a0..a4bbfd9 100644
@@ -104,16 +104,6 @@ int mips_cm_probe(void)
 	base_reg |= CM_GCR_BASE_CMDEFTGT_MEM;
 	write_gcr_base(base_reg);
 
-	/* disable CM regions */
-	write_gcr_reg0_base(CM_GCR_REGn_BASE_BASEADDR_MSK);
-	write_gcr_reg0_mask(CM_GCR_REGn_MASK_ADDRMASK_MSK);
-	write_gcr_reg1_base(CM_GCR_REGn_BASE_BASEADDR_MSK);
-	write_gcr_reg1_mask(CM_GCR_REGn_MASK_ADDRMASK_MSK);
-	write_gcr_reg2_base(CM_GCR_REGn_BASE_BASEADDR_MSK);
-	write_gcr_reg2_mask(CM_GCR_REGn_MASK_ADDRMASK_MSK);
-	write_gcr_reg3_base(CM_GCR_REGn_BASE_BASEADDR_MSK);
-	write_gcr_reg3_mask(CM_GCR_REGn_MASK_ADDRMASK_MSK);
-
 	/* probe for an L2-only sync region */
 	mips_cm_probe_l2sync();
Signature · 1 line
2.1.3
#3 lore raw
Quoted 9 lines · Niklas Svensson
Quoted 8 lines · Paul Burton
Quoted 1 line · Niklas Svensson

Each CM_REGION_TARGET is set to disabled at boot,

That part is true...

Quoted 2 lines

so there is no need to disable the matching
CM_GCR_REG explicitly.

Quoted 8 lines
Quoted 3 lines

...however there is no guarantee that the bootloader, or another kernel, or some other piece of code didn't program the registers differently before Linux starts. So I believe this patch to be incorrect.

That is the reason for this patch. This will overwrite settings written by the bootloader.

Say if the bootloader sets up an iocu, then these writes will clear those settings.

The kernel has to be firmware agnostic, can't know how these registers have been initialized as long as there is no proper boot protocol for the handover to the kernel which includes this status. At this point I think a solution that hands over the required information as part of the DT is probably the way to go.

Ralf