Skip to contentSkip to conversation

bus: imx-weim

PATCH Applied Sven Van Asbroeck 2 participants 5 messages Dec 17, 2018 – Jan 11, 2019

Subsystems (2)
Device tree bindings Arm i.MX and Layerscape
lore
Share & tools
lore

Sven Van Asbroeck

author
#0 lore raw

v4 cover bus: imx-weim

Support multiple address ranges per child node on imx-weim. While we're at it, insert some code which guards against common config conflicts.

v4:
pick up patch after one-week delay Shawn Guo:
changed devicetree patch position in the set clarified a confusing #define'd constant name minor coding style fixes

v3:
added devicetree binding docs

v2:
corrected acme@... in commit message example

Sven Van Asbroeck (3):
dt-bindings: bus: imx-weim: document multiple address ranges per child
node
bus: imx-weim: support multiple address ranges per child node
bus: imx-weim: guard against timing configuration conflicts

 .../devicetree/bindings/bus/imx-weim.txt      | 32 ++++++++-
 drivers/bus/imx-weim.c                        | 70 +++++++++++++++----
 2 files changed, 86 insertions(+), 16 deletions(-)
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH v4 1/3 dt-bindings: bus: imx-weim: document multiple address ranges per child node

The imx-weim driver was patched to allow correct WEIM configuration when multiple address ranges are used in a child node. Update the dt-bindings to reflect this.

Reviewed-by Rob Herring <[email protected]>
Signed-off-by Sven Van Asbroeck <[email protected]>
---
 .../devicetree/bindings/bus/imx-weim.txt      | 32 +++++++++++++++++--
 1 file changed, 29 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/bus/imx-weim.txt +29 -3
index 683eaf3aed79..dda7d6d66479 100644
@@ -47,9 +47,9 @@ Optional properties:
 Timing property for child nodes. It is mandatory, not optional.
 
  - fsl,weim-cs-timing:	The timing array, contains timing values for the
-			child node. We can get the CS index from the child
-			node's "reg" property. The number of registers depends
-			on the selected chip.
+			child node. We get the CS indexes from the address
+			ranges in the child node's "reg" property.
+			The number of registers depends on the selected chip:
 			For i.MX1, i.MX21 ("fsl,imx1-weim") there are two
 			registers: CSxU, CSxL.
 			For i.MX25, i.MX27, i.MX31 and i.MX35 ("fsl,imx27-weim")

@@ -80,3 +80,29 @@ Example for an imx6q-sabreauto board, the NOR flash connected to the WEIM:
 					0x0000c000 0x1404a38e 0x00000000>;
 		};
 	};
+
+Example for an imx6q-based board, a multi-chipselect device connected to WEIM:
+
+In this case, both chip select 0 and 1 will be configured with the same timing
+array values.
+
+	weim: weim@21b8000 {
+		compatible = "fsl,imx6q-weim";
+		reg = <0x021b8000 0x4000>;
+		clocks = <&clks 196>;
+		#address-cells = <2>;
+		#size-cells = <1>;
+		ranges = <0 0 0x08000000 0x02000000
+			  1 0 0x0a000000 0x02000000
+			  2 0 0x0c000000 0x02000000
+			  3 0 0x0e000000 0x02000000>;
+		fsl,weim-cs-gpr = <&gpr>;
+
+		acme@0 {
+			compatible = "acme,whatever";
+			reg = <0 0 0x100>, <0 0x400000 0x800>,
+				<1 0x400000 0x800>;
+			fsl,weim-cs-timing = <0x024400b1 0x00001010 0x20081100
+				0x00000000 0xa0000240 0x00000000>;
+		};
+	};
Signature · 1 line
2.17.1
author
#2 lore raw

PATCH v4 2/3 bus: imx-weim: support multiple address ranges per child node

In reply to Sven Van Asbroeck

Ensure that timing values for the child node are applied to all chip selects in the child's address ranges.

Note that this does not support multiple timing settings per child; this can be added in the future if required.

Example:

&weim {
	acme@0 {
		compatible = "acme,whatever";
		reg = <0 0 0x100>, <0 0x400000 0x800>,
				<1 0x400000 0x800>;
		fsl,weim-cs-timing = <0x024400b1 0x00001010 0x20081100
				0x00000000 0xa0000240 0x00000000>;
	};
};
Signed-off-by Sven Van Asbroeck <[email protected]>
---
 drivers/bus/imx-weim.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)
drivers/bus/imx-weim.c +25 -10
index d84996a4528e..1a0e0277a404 100644
@@ -46,6 +46,7 @@ static const struct imx_weim_devtype imx51_weim_devtype = {
 };
 
 #define MAX_CS_REGS_COUNT	6
+#define OF_REG_SIZE		3
 
 static const struct of_device_id weim_id_table[] = {
 	/* i.MX1/21 */

@@ -116,26 +117,40 @@ static int __init weim_timing_setup(struct device_node *np, void __iomem *base,
 {
 	u32 cs_idx, value[MAX_CS_REGS_COUNT];
 	int i, ret;
+	int reg_idx, num_regs;
 
 	if (WARN_ON(devtype->cs_regs_count > MAX_CS_REGS_COUNT))
 		return -EINVAL;
 
-	/* get the CS index from this child node's "reg" property. */
-	ret = of_property_read_u32(np, "reg", &cs_idx);
+	ret = of_property_read_u32_array(np, "fsl,weim-cs-timing",
+					 value, devtype->cs_regs_count);
 	if (ret)
 		return ret;
 
-	if (cs_idx >= devtype->cs_count)
+	/*
+	 * the child node's "reg" property may contain multiple address ranges,
+	 * extract the chip select for each.
+	 */
+	num_regs = of_property_count_elems_of_size(np, "reg", OF_REG_SIZE);
+	if (num_regs < 0)
+		return num_regs;
+	if (!num_regs)
 		return -EINVAL;
+	for (reg_idx = 0; reg_idx < num_regs; reg_idx++) {
+		/* get the CS index from this child node's "reg" property. */
+		ret = of_property_read_u32_index(np, "reg",
+					reg_idx * OF_REG_SIZE, &cs_idx);
+		if (ret)
+			break;
 
-	ret = of_property_read_u32_array(np, "fsl,weim-cs-timing",
-					 value, devtype->cs_regs_count);
-	if (ret)
-		return ret;
+		if (cs_idx >= devtype->cs_count)
+			return -EINVAL;
 
-	/* set the timing for WEIM */
-	for (i = 0; i < devtype->cs_regs_count; i++)
-		writel(value[i], base + cs_idx * devtype->cs_stride + i * 4);
+		/* set the timing for WEIM */
+		for (i = 0; i < devtype->cs_regs_count; i++)
+			writel(value[i],
+				base + cs_idx * devtype->cs_stride + i * 4);
+	}
 
 	return 0;
 }
Signature · 1 line
2.17.1
author
#3 lore raw

PATCH v4 3/3 bus: imx-weim: guard against timing configuration conflicts

In reply to Sven Van Asbroeck

When specifying weim child devices, there is a risk that more than one timing setting is specified for the same chip select.

The driver cannot support such a configuration.

In case of conflict, this patch will print a warning to the log, and will ignore the child node in question.

In this example, node acme@1 will be ignored, as it tries to modify timing settings for CS0:

&weim {
	acme@0 {
		compatible = "acme,whatever";
		reg = <0 0 0x100>;
		fsl,weim-cs-timing = <something>;
	};
	acme@1 {
		compatible = "acme,whatnot";
		reg = <0 0x500 0x100>;
		fsl,weim-cs-timing = <something else>;
	};
};

However in this example, the driver will be happy:

&weim {
        acme@0 {
                compatible = "acme,whatever";
                reg = <0 0 0x100>;
                fsl,weim-cs-timing = <something>;
        };
        acme@1 {
                compatible = "acme,whatnot";
                reg = <0 0x500 0x100>;
                fsl,weim-cs-timing = <something>;
        };
};
Signed-off-by Sven Van Asbroeck <[email protected]>
---
 drivers/bus/imx-weim.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)
drivers/bus/imx-weim.c +32 -3
index 1a0e0277a404..db74334ca5ef 100644
@@ -46,8 +46,18 @@ static const struct imx_weim_devtype imx51_weim_devtype = {
 };
 
 #define MAX_CS_REGS_COUNT	6
+#define MAX_CS_COUNT		6
 #define OF_REG_SIZE		3
 
+struct cs_timing {
+	bool is_applied;
+	u32 regs[MAX_CS_REGS_COUNT];
+};
+
+struct cs_timing_state {
+	struct cs_timing cs[MAX_CS_COUNT];
+};
+
 static const struct of_device_id weim_id_table[] = {
 	/* i.MX1/21 */
 	{ .compatible = "fsl,imx1-weim", .data = &imx1_weim_devtype, },

@@ -112,15 +122,20 @@ static int __init imx_weim_gpr_setup(struct platform_device *pdev)
 }
 
 /* Parse and set the timing for this device. */
-static int __init weim_timing_setup(struct device_node *np, void __iomem *base,
-				    const struct imx_weim_devtype *devtype)
+static int __init weim_timing_setup(struct device *dev,
+				    struct device_node *np, void __iomem *base,
+				    const struct imx_weim_devtype *devtype,
+				    struct cs_timing_state *ts)
 {
 	u32 cs_idx, value[MAX_CS_REGS_COUNT];
 	int i, ret;
 	int reg_idx, num_regs;
+	struct cs_timing *cst;
 
 	if (WARN_ON(devtype->cs_regs_count > MAX_CS_REGS_COUNT))
 		return -EINVAL;
+	if (WARN_ON(devtype->cs_count > MAX_CS_COUNT))
+		return -EINVAL;
 
 	ret = of_property_read_u32_array(np, "fsl,weim-cs-timing",
 					 value, devtype->cs_regs_count);

@@ -146,10 +161,23 @@ static int __init weim_timing_setup(struct device_node *np, void __iomem *base,
 		if (cs_idx >= devtype->cs_count)
 			return -EINVAL;
 
+		/* prevent re-configuring a CS that's already been configured */
+		cst = &ts->cs[cs_idx];
+		if (cst->is_applied && memcmp(value, cst->regs,
+					devtype->cs_regs_count * sizeof(u32))) {
+			dev_err(dev, "fsl,weim-cs-timing conflict on %pOF", np);
+			return -EINVAL;
+		}
+
 		/* set the timing for WEIM */
 		for (i = 0; i < devtype->cs_regs_count; i++)
 			writel(value[i],
 				base + cs_idx * devtype->cs_stride + i * 4);
+		if (!cst->is_applied) {
+			cst->is_applied = true;
+			memcpy(cst->regs, value,
+				devtype->cs_regs_count * sizeof(u32));
+		}
 	}
 
 	return 0;

@@ -163,6 +191,7 @@ static int __init weim_parse_dt(struct platform_device *pdev,
 	const struct imx_weim_devtype *devtype = of_id->data;
 	struct device_node *child;
 	int ret, have_child = 0;
+	struct cs_timing_state ts = {};
 
 	if (devtype == &imx50_weim_devtype) {
 		ret = imx_weim_gpr_setup(pdev);

@@ -171,7 +200,7 @@ static int __init weim_parse_dt(struct platform_device *pdev,
 	}
 
 	for_each_available_child_of_node(pdev->dev.of_node, child) {
-		ret = weim_timing_setup(child, base, devtype);
+		ret = weim_timing_setup(&pdev->dev, child, base, devtype, &ts);
 		if (ret)
 			dev_warn(&pdev->dev, "%pOF set timing failed.\n",
 				child);
Signature · 1 line
2.17.1
Applied
#4 lore raw
In reply to Sven Van Asbroeck
Quoted 5 lines · Sven Van Asbroeck

Sven Van Asbroeck (3):
dt-bindings: bus: imx-weim: document multiple address ranges per child
node
bus: imx-weim: support multiple address ranges per child node
bus: imx-weim: guard against timing configuration conflicts

Applied all, thanks.