Skip to contentSkip to conversation

fsl-ifc: set extended addressing for systems whose bootloader doesn't

PATCH In review David Singleton 2 participants 2 messages Oct 10, 2016 – Oct 17, 2016 Memory controller drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fsl-ifc: set extended addressing for systems whose bootloader doesn't

For 32bit systems whose bootloader doesn't set the extended 36-bit addressing register for flash devices above the 4GB boundary we can set up in the driver.

This patch checks the number of address-cells in the dts file for the fsl-ifc flash controller. If #address-cells is 2 then it's a 36-bit address mapping, so set the extended address register in the ccsr for the upper 0xf address, as specified in the dts file.

The code only sets the extended addressing register if the dts defines 36-bit addressing for the flash devices AND the register was not set by the boot loader.

If the bootloader has set the extended addressing register the code does not update the register.

---
 drivers/memory/fsl_ifc.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
drivers/memory/fsl_ifc.c +35 -0
index 1b182b1..3d7a52e 100644
@@ -78,6 +78,41 @@ EXPORT_SYMBOL(fsl_ifc_find);
 static int fsl_ifc_ctrl_init(struct fsl_ifc_ctrl *ctrl)
 {
 	struct fsl_ifc_global __iomem *ifc = ctrl->gregs;
+	struct device_node *np;
+
+	/*
+	 * enable extended 36-bit addressing
+	 * 24.3.2 Extended Chip Select Property registers (IFC_CSPRn_EXT)
+	 * The extended chip select property register (CSPRn_EXT) contains
+	 * the extended base address, that is, the most significant
+	 * bits (msb) of the base address.
+	 * Set it here for systems where the bootloader doesn't.
+	 */
+	np = of_find_compatible_node(NULL, NULL, "fsl,ifc");
+	if (np) {
+		const u32 *prop;
+
+		prop = of_get_property(np, "#address-cells", NULL);
+		if (prop) {
+			u32 cells;
+			/*
+			 * #address-cells 2 means 36-bit addresses are used
+			 * and the if cspr_ext register is zero, the
+			 * bootloader didn't set it, we'll set it manually
+			 */
+			cells = of_n_addr_cells(np);
+			if ((cells == 2) && !(ifc_in32(&ifc->cspr_cs[0].cspr_ext))) {
+				prop = of_get_property(np, "reg", NULL);
+				if (prop) {
+					u32 extaddr;
+
+					extaddr = *prop; /* get the top nibble for 36-bit */
+					pr_info("fsl-ifc extended 36-bit addressing\n");
+					ifc_out32(extaddr, &ifc->cspr_cs[0].cspr_ext);
+				}
+			}
+		}
+	}
 
 	/*
 	 * Clear all the common status and event registers
Signature · 1 line
2.9.3
#1 lore raw
Quoted 7 lines · David Singleton

For 32bit systems whose bootloader doesn't set the extended 36-bit addressing register for flash devices above the 4GB boundary we can set up in the driver.

This patch checks the number of address-cells in the dts file for the fsl-ifc flash controller. If #address-cells is 2 then it's a 36-bit address mapping,

Not necessarily.

Quoted 2 lines

so set the extended address register
in the ccsr for the upper 0xf address, as specified in the dts file.

The changelog should not mention 0xf as that gives the impression that the patch assumes the upper bits are 0xf.

Quoted 6 lines

The code only sets the extended addressing register if the dts defines 36-bit addressing for the flash devices AND the register was not set by the boot loader.

If the bootloader has set the extended addressing register the code does not update the register.

This is not accurate if the bootloader has set the extended addressing register to zero.

Quoted 7 lines
---
 drivers/memory/fsl_ifc.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

Who are you asking to apply this? If it's me, please CC [email protected] (and CC me at [email protected], not [email protected]). If you want it to go via the mtd tree, CC the relevant maintainer and list (and me).

Quoted 8 lines
drivers/memory/fsl_ifc.c +0 -0
index 1b182b1..3d7a52e 100644
@@ -78,6 +78,41 @@ EXPORT_SYMBOL(fsl_ifc_find);
 static int fsl_ifc_ctrl_init(struct fsl_ifc_ctrl *ctrl)
 {
 	struct fsl_ifc_global __iomem *ifc = ctrl->gregs;

drivers/memory/fsl_ifc.c EXPORT_SYMBOL(fsl_ifc_find); · line 90

Quoted 10 lines
drivers/memory/fsl_ifc.c +10 -0
+	struct device_node *np;
+
+	/*
+	 * enable extended 36-bit addressing
+	 * 24.3.2 Extended Chip Select Property registers (IFC_CSPRn_EXT)
+	 * The extended chip select property register (CSPRn_EXT) contains
+	 * the extended base address, that is, the most significant
+	 * bits (msb) of the base address.
+	 * Set it here for systems where the bootloader doesn't.
+	 */

Quoting manual section numbers is meaningless without referencing a specific document. This comment does not need to be anything more than:

/*
* Some bootloaders neglect to set the extended base address,
* so set it based on the device tree.
*/

drivers/memory/fsl_ifc.c EXPORT_SYMBOL(fsl_ifc_find); · line 91

Quoted 1 line
drivers/memory/fsl_ifc.c +1 -0
+	np = of_find_compatible_node(NULL, NULL, "fsl,ifc");

The function that calls this has the node already. Pass it in instead of searching from scratch.

drivers/memory/fsl_ifc.c EXPORT_SYMBOL(fsl_ifc_find); · line 104

Quoted 13 lines
drivers/memory/fsl_ifc.c +13 -0
+	if (np) {
+		const u32 *prop;
+
+		prop = of_get_property(np, "#address-cells", NULL);
+		if (prop) {
+			u32 cells;
+			/*
+			 * #address-cells 2 means 36-bit addresses are used
+			 * and the if cspr_ext register is zero, the
+			 * bootloader didn't set it, we'll set it manually
+			 */
+			cells = of_n_addr_cells(np);
+			if ((cells == 2) && !(ifc_in32(&ifc->cspr_cs[0].cspr_ext))) {

Unnecessary parens.

Why are you only fixing CS0?

drivers/memory/fsl_ifc.c EXPORT_SYMBOL(fsl_ifc_find); · line 105

Quoted 1 line
drivers/memory/fsl_ifc.c +1 -0
+				prop = of_get_property(np, "reg", NULL);

This is the reg property of the controller, not the node that corresponds to CS0. You cannot assume that CCSR and CS0 have the same upper address bits.

drivers/memory/fsl_ifc.c EXPORT_SYMBOL(fsl_ifc_find); · line 109

Quoted 4 lines
drivers/memory/fsl_ifc.c +4 -0
+				if (prop) {
+					u32 extaddr;
+
+					extaddr = *prop; /* get the top nibble for 36-bit */

cspr_ext is 8 bits on some chips (40-bit addressing). Just remove the comment.

drivers/memory/fsl_ifc.c EXPORT_SYMBOL(fsl_ifc_find); · line 110

Quoted 1 line
drivers/memory/fsl_ifc.c +1 -0
+					pr_info("fsl-ifc extended 36-bit addressing\n");

If you must print something here it should indicate that a workaround is being applied.

-Scott