[PATCH v2 1/7] drivers: bus: imx-weim: Remove private driver data

[PATCH v2 1/7] drivers: bus: imx-weim: Remove private driver data

[Alexander Shiyan]

Driver uses only probe function so no reason to keep variables
in private driver data.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/bus/imx-weim.c | 41 ++++++++++++++---------------------------
 1 file changed, 14 insertions(+), 27 deletions(-)

diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index 349f14e..0c0e6fe 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -12,11 +12,6 @@
 #include <linux/io.h>
 #include <linux/of_device.h>
 
-struct imx_weim {
-	void __iomem *base;
-	struct clk *clk;
-};
-
 static const struct of_device_id weim_id_table[] = {
 	{ .compatible = "fsl,imx6q-weim", },
 	{}
@@ -27,10 +22,8 @@ MODULE_DEVICE_TABLE(of, weim_id_table);
 #define CS_REG_RANGE  0x18
 
 /* Parse and set the timing for this device. */
-static int
-weim_timing_setup(struct platform_device *pdev, struct device_node *np)
+static int weim_timing_setup(struct device_node *np, void __iomem *base)
 {
-	struct imx_weim *weim = platform_get_drvdata(pdev);
 	u32 value[CS_TIMING_LEN];
 	u32 cs_idx;
 	int ret;
@@ -52,11 +45,11 @@ weim_timing_setup(struct platform_device *pdev, struct device_node *np)
 
 	/* set the timing for WEIM */
 	for (i = 0; i < CS_TIMING_LEN; i++)
-		writel(value[i], weim->base + cs_idx * CS_REG_RANGE + i * 4);
+		writel(value[i], base + cs_idx * CS_REG_RANGE + i * 4);
 	return 0;
 }
 
-static int weim_parse_dt(struct platform_device *pdev)
+static int weim_parse_dt(struct platform_device *pdev, void __iomem *base)
 {
 	struct device_node *child;
 	int ret;
@@ -65,7 +58,7 @@ static int weim_parse_dt(struct platform_device *pdev)
 		if (!child->name)
 			continue;
 
-		ret = weim_timing_setup(pdev, child);
+		ret = weim_timing_setup(child, base);
 		if (ret) {
 			dev_err(&pdev->dev, "%s set timing failed.\n",
 				child->full_name);
@@ -82,38 +75,32 @@ static int weim_parse_dt(struct platform_device *pdev)
 
 static int weim_probe(struct platform_device *pdev)
 {
-	struct imx_weim *weim;
 	struct resource *res;
+	struct clk *clk;
+	void __iomem *base;
 	int ret = -EINVAL;
 
-	weim = devm_kzalloc(&pdev->dev, sizeof(*weim), GFP_KERNEL);
-	if (!weim) {
-		ret = -ENOMEM;
-		goto weim_err;
-	}
-	platform_set_drvdata(pdev, weim);
-
 	/* get the resource */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	weim->base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(weim->base)) {
-		ret = PTR_ERR(weim->base);
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base)) {
+		ret = PTR_ERR(base);
 		goto weim_err;
 	}
 
 	/* get the clock */
-	weim->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(weim->clk))
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk))
 		goto weim_err;
 
-	ret = clk_prepare_enable(weim->clk);
+	ret = clk_prepare_enable(clk);
 	if (ret)
 		goto weim_err;
 
 	/* parse the device node */
-	ret = weim_parse_dt(pdev);
+	ret = weim_parse_dt(pdev, base);
 	if (ret) {
-		clk_disable_unprepare(weim->clk);
+		clk_disable_unprepare(clk);
 		goto weim_err;
 	}
 
-- 
1.8.1.5

[PATCH v2 2/7] drivers: bus: imx-weim: Simplify error path

[Alexander Shiyan]

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/bus/imx-weim.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index 0c0e6fe..0f4b081 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -78,36 +78,30 @@ static int weim_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct clk *clk;
 	void __iomem *base;
-	int ret = -EINVAL;
+	int ret;
 
 	/* get the resource */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(base)) {
-		ret = PTR_ERR(base);
-		goto weim_err;
-	}
+	if (IS_ERR(base))
+		return PTR_ERR(base);
 
 	/* get the clock */
 	clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(clk))
-		goto weim_err;
+		return PTR_ERR(clk);
 
 	ret = clk_prepare_enable(clk);
 	if (ret)
-		goto weim_err;
+		return ret;
 
 	/* parse the device node */
 	ret = weim_parse_dt(pdev, base);
-	if (ret) {
+	if (ret)
 		clk_disable_unprepare(clk);
-		goto weim_err;
-	}
-
-	dev_info(&pdev->dev, "WEIM driver registered.\n");
-	return 0;
+	else
+		dev_info(&pdev->dev, "Driver registered.\n");
 
-weim_err:
 	return ret;
 }
 
-- 
1.8.1.5

[PATCH v2 3/7] drivers: bus: imx-weim: use module_platform_driver_probe()

[Alexander Shiyan]

Driver should be called only once at startup, so code converted
to using module_platform_driver_probe().

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/bus/imx-weim.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index 0f4b081..f872924 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -22,7 +22,7 @@ MODULE_DEVICE_TABLE(of, weim_id_table);
 #define CS_REG_RANGE  0x18
 
 /* Parse and set the timing for this device. */
-static int weim_timing_setup(struct device_node *np, void __iomem *base)
+static int __init weim_timing_setup(struct device_node *np, void __iomem *base)
 {
 	u32 value[CS_TIMING_LEN];
 	u32 cs_idx;
@@ -49,7 +49,8 @@ static int weim_timing_setup(struct device_node *np, void __iomem *base)
 	return 0;
 }
 
-static int weim_parse_dt(struct platform_device *pdev, void __iomem *base)
+static int __init weim_parse_dt(struct platform_device *pdev,
+				void __iomem *base)
 {
 	struct device_node *child;
 	int ret;
@@ -73,7 +74,7 @@ static int weim_parse_dt(struct platform_device *pdev, void __iomem *base)
 	return ret;
 }
 
-static int weim_probe(struct platform_device *pdev)
+static int __init weim_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	struct clk *clk;
@@ -110,10 +111,9 @@ static struct platform_driver weim_driver = {
 		.name = "imx-weim",
 		.of_match_table = weim_id_table,
 	},
-	.probe   = weim_probe,
 };
+module_platform_driver_probe(weim_driver, weim_probe);
 
-module_platform_driver(weim_driver);
 MODULE_AUTHOR("Freescale Semiconductor Inc.");
 MODULE_DESCRIPTION("i.MX EIM Controller Driver");
 MODULE_LICENSE("GPL");
-- 
1.8.1.5

[PATCH v2 4/7] drivers: bus: imx-weim: Preparation driver to support different CPUs

[Alexander Shiyan]

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/bus/imx-weim.c | 44 ++++++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index f872924..b558aa3 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -12,46 +12,58 @@
 #include <linux/io.h>
 #include <linux/of_device.h>
 
+struct imx_weim_devtype {
+	unsigned int	cs_count;
+	unsigned int	cs_regs_count;
+	unsigned int	cs_stride;
+};
+
+static const struct imx_weim_devtype imx50_weim_devtype = {
+	.cs_count	= 4,
+	.cs_regs_count	= 6,
+	.cs_stride	= 0x18,
+};
+
 static const struct of_device_id weim_id_table[] = {
-	{ .compatible = "fsl,imx6q-weim", },
-	{}
+	/* i.MX50/53/6Q */
+	{ .compatible = "fsl,imx6q-weim", .data = &imx50_weim_devtype, },
+	{ }
 };
 MODULE_DEVICE_TABLE(of, weim_id_table);
 
-#define CS_TIMING_LEN 6
-#define CS_REG_RANGE  0x18
-
 /* Parse and set the timing for this device. */
-static int __init weim_timing_setup(struct device_node *np, void __iomem *base)
+static int __init weim_timing_setup(struct device_node *np, void __iomem *base,
+				    const struct imx_weim_devtype *devtype)
 {
-	u32 value[CS_TIMING_LEN];
-	u32 cs_idx;
-	int ret;
-	int i;
+	u32 cs_idx, value[devtype->cs_regs_count];
+	int i, ret;
 
 	/* get the CS index from this child node's "reg" property. */
 	ret = of_property_read_u32(np, "reg", &cs_idx);
 	if (ret)
 		return ret;
 
-	/* The weim has four chip selects. */
-	if (cs_idx > 3)
+	if (cs_idx >= devtype->cs_count)
 		return -EINVAL;
 
 	ret = of_property_read_u32_array(np, "fsl,weim-cs-timing",
-					value, CS_TIMING_LEN);
+					 value, devtype->cs_regs_count);
 	if (ret)
 		return ret;
 
 	/* set the timing for WEIM */
-	for (i = 0; i < CS_TIMING_LEN; i++)
-		writel(value[i], base + cs_idx * CS_REG_RANGE + i * 4);
+	for (i = 0; i < devtype->cs_regs_count; i++)
+		writel(value[i], base + cs_idx * devtype->cs_stride + i * 4);
+
 	return 0;
 }
 
 static int __init weim_parse_dt(struct platform_device *pdev,
 				void __iomem *base)
 {
+	const struct of_device_id *of_id = of_match_device(weim_id_table,
+							   &pdev->dev);
+	const struct imx_weim_devtype *devtype = of_id->data;
 	struct device_node *child;
 	int ret;
 
@@ -59,7 +71,7 @@ static int __init weim_parse_dt(struct platform_device *pdev,
 		if (!child->name)
 			continue;
 
-		ret = weim_timing_setup(child, base);
+		ret = weim_timing_setup(child, base, devtype);
 		if (ret) {
 			dev_err(&pdev->dev, "%s set timing failed.\n",
 				child->full_name);
-- 
1.8.1.5

[PATCH v2 5/7] drivers: bus: imx-weim: Add missing platform_friver.owner field

[Alexander Shiyan]

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/bus/imx-weim.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index b558aa3..b6479fb 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -120,8 +120,9 @@ static int __init weim_probe(struct platform_device *pdev)
 
 static struct platform_driver weim_driver = {
 	.driver = {
-		.name = "imx-weim",
-		.of_match_table = weim_id_table,
+		.name		= "imx-weim",
+		.owner		= THIS_MODULE,
+		.of_match_table	= weim_id_table,
 	},
 };
 module_platform_driver_probe(weim_driver, weim_probe);
-- 
1.8.1.5

[PATCH v2 6/7] drivers: bus: imx-weim: Add support for i.MX1/21/25/27/31/35/50/51/53

[Alexander Shiyan]

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 Documentation/devicetree/bindings/bus/imx-weim.txt | 11 +++++++----
 drivers/bus/Kconfig                                |  4 ++--
 drivers/bus/imx-weim.c                             | 23 ++++++++++++++++++++++
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/bus/imx-weim.txt b/Documentation/devicetree/bindings/bus/imx-weim.txt
index cedc2a9..99406e4 100644
--- a/Documentation/devicetree/bindings/bus/imx-weim.txt
+++ b/Documentation/devicetree/bindings/bus/imx-weim.txt
@@ -8,7 +8,7 @@ The actual devices are instantiated from the child nodes of a WEIM node.
 
 Required properties:
 
- - compatible:		Should be set to "fsl,imx6q-weim"
+ - compatible:		Should be set to "fsl,<soc>-weim"
  - reg:			A resource specifier for the register space
 			(see the example below)
  - clocks:		the clock, see the example below.
@@ -21,11 +21,14 @@ Required properties:
 
 Timing property for child nodes. It is mandatory, not optional.
 
- - fsl,weim-cs-timing:	The timing array, contains 6 timing values for the
+ - 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. This property contains the values
-			for the registers EIM_CSnGCR1, EIM_CSnGCR2, EIM_CSnRCR1,
+			node's "reg" property. For example, if i.MX6Q CPU
+			is used, this property contains the values for the
+			registers EIM_CSnGCR1, EIM_CSnGCR2, EIM_CSnRCR1,
 			EIM_CSnRCR2, EIM_CSnWCR1, EIM_CSnWCR2 in this order.
+			For other i.MX CPUs count of register and register
+			names may be different.
 
 Example for an imx6q-sabreauto board, the NOR flash connected to the WEIM:
 
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 1f70e84..4faedc21 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -8,10 +8,10 @@ config IMX_WEIM
 	bool "Freescale EIM DRIVER"
 	depends on ARCH_MXC
 	help
-	  Driver for i.MX6 WEIM controller.
+	  Driver for i.MX WEIM controller.
 	  The WEIM(Wireless External Interface Module) works like a bus.
 	  You can attach many different devices on it, such as NOR, onenand.
-	  But now, we only support the Parallel NOR.
+	  But now, we only support the "of_physmap" driver.
 
 config MVEBU_MBUS
 	bool
diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index b6479fb..77fa1d4 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -17,6 +17,17 @@ struct imx_weim_devtype {
 	unsigned int	cs_regs_count;
 	unsigned int	cs_stride;
 };
+static const struct imx_weim_devtype imx1_weim_devtype = {
+	.cs_count	= 6,
+	.cs_regs_count	= 2,
+	.cs_stride	= 0x08,
+};
+
+static const struct imx_weim_devtype imx25_weim_devtype = {
+	.cs_count	= 6,
+	.cs_regs_count	= 3,
+	.cs_stride	= 0x10,
+};
 
 static const struct imx_weim_devtype imx50_weim_devtype = {
 	.cs_count	= 4,
@@ -24,9 +35,21 @@ static const struct imx_weim_devtype imx50_weim_devtype = {
 	.cs_stride	= 0x18,
 };
 
+static const struct imx_weim_devtype imx51_weim_devtype = {
+	.cs_count	= 6,
+	.cs_regs_count	= 6,
+	.cs_stride	= 0x18,
+};
+
 static const struct of_device_id weim_id_table[] = {
+	/* i.MX1/21 */
+	{ .compatible = "fsl,imx1-weim", .data = &imx1_weim_devtype, },
+	/* i.MX25/27/31/35 */
+	{ .compatible = "fsl,imx25-weim", .data = &imx25_weim_devtype, },
 	/* i.MX50/53/6Q */
 	{ .compatible = "fsl,imx6q-weim", .data = &imx50_weim_devtype, },
+	/* i.MX51 */
+	{ .compatible = "fsl,imx51-weim", .data = &imx51_weim_devtype, },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, weim_id_table);
-- 
1.8.1.5

[PATCH v2 6/7] drivers: bus: imx-weim: Add support for i.MX1/21/25/27/31/35/50/51/53

[Sascha Hauer]

Hi Alexander,

Nice work!

On Wed, Jun 19, 2013 at 11:54:37PM +0400, Alexander Shiyan wrote:
> - - fsl,weim-cs-timing:	The timing array, contains 6 timing values for the
> + - 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. This property contains the values
> -			for the registers EIM_CSnGCR1, EIM_CSnGCR2, EIM_CSnRCR1,
> +			node's "reg" property. For example, if i.MX6Q CPU
> +			is used, this property contains the values for the
> +			registers EIM_CSnGCR1, EIM_CSnGCR2, EIM_CSnRCR1,
>  			EIM_CSnRCR2, EIM_CSnWCR1, EIM_CSnWCR2 in this order.
> +			For other i.MX CPUs count of register and register
> +			names may be different.

I think here we should have a more precise description for each SoC.

>  
>  Example for an imx6q-sabreauto board, the NOR flash connected to the WEIM:
>  
> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> index 1f70e84..4faedc21 100644
> --- a/drivers/bus/Kconfig
> +++ b/drivers/bus/Kconfig
> @@ -8,10 +8,10 @@ config IMX_WEIM
>  	bool "Freescale EIM DRIVER"
>  	depends on ARCH_MXC
>  	help
> -	  Driver for i.MX6 WEIM controller.
> +	  Driver for i.MX WEIM controller.
>  	  The WEIM(Wireless External Interface Module) works like a bus.
>  	  You can attach many different devices on it, such as NOR, onenand.
> -	  But now, we only support the Parallel NOR.
> +	  But now, we only support the "of_physmap" driver.

This comment is wrong. In the early versions of this patch indeed only
parallel NOR was supported, but now there is no limitation on the device
type anymore.

>  
>  config MVEBU_MBUS
>  	bool
> diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
> index b6479fb..77fa1d4 100644
> --- a/drivers/bus/imx-weim.c
> +++ b/drivers/bus/imx-weim.c
> @@ -17,6 +17,17 @@ struct imx_weim_devtype {
>  	unsigned int	cs_regs_count;
>  	unsigned int	cs_stride;
>  };
> +static const struct imx_weim_devtype imx1_weim_devtype = {
> +	.cs_count	= 6,
> +	.cs_regs_count	= 2,
> +	.cs_stride	= 0x08,
> +};
> +
> +static const struct imx_weim_devtype imx25_weim_devtype = {
> +	.cs_count	= 6,
> +	.cs_regs_count	= 3,
> +	.cs_stride	= 0x10,
> +};
>  
>  static const struct imx_weim_devtype imx50_weim_devtype = {
>  	.cs_count	= 4,
> @@ -24,9 +35,21 @@ static const struct imx_weim_devtype imx50_weim_devtype = {
>  	.cs_stride	= 0x18,
>  };
>  
> +static const struct imx_weim_devtype imx51_weim_devtype = {
> +	.cs_count	= 6,
> +	.cs_regs_count	= 6,
> +	.cs_stride	= 0x18,
> +};
> +
>  static const struct of_device_id weim_id_table[] = {
> +	/* i.MX1/21 */
> +	{ .compatible = "fsl,imx1-weim", .data = &imx1_weim_devtype, },
> +	/* i.MX25/27/31/35 */
> +	{ .compatible = "fsl,imx25-weim", .data = &imx25_weim_devtype, },

We usually name the compatible name after the oldest i.MX supporting
this IP, not after the lowest number. So this should be imx27, not
imx25.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re[2]: [PATCH v2 6/7] drivers: bus: imx-weim: Add support for i.MX1/21/25/27/31/35/50/51/53

[Alexander Shiyan]

> Hi Alexander,
> 
> Nice work!
> 
> On Wed, Jun 19, 2013 at 11:54:37PM +0400, Alexander Shiyan wrote:
> > - - fsl,weim-cs-timing:	The timing array, contains 6 timing values for the
> > + - 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. This property contains the values
> > -			for the registers EIM_CSnGCR1, EIM_CSnGCR2, EIM_CSnRCR1,
> > +			node's "reg" property. For example, if i.MX6Q CPU
> > +			is used, this property contains the values for the
> > +			registers EIM_CSnGCR1, EIM_CSnGCR2, EIM_CSnRCR1,
> >  			EIM_CSnRCR2, EIM_CSnWCR1, EIM_CSnWCR2 in this order.
> > +			For other i.MX CPUs count of register and register
> > +			names may be different.
> 
> I think here we should have a more precise description for each SoC.

OK.

> >  Example for an imx6q-sabreauto board, the NOR flash connected to the WEIM:
> >  
> > diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> > index 1f70e84..4faedc21 100644
> > --- a/drivers/bus/Kconfig
> > +++ b/drivers/bus/Kconfig
> > @@ -8,10 +8,10 @@ config IMX_WEIM
> >  	bool "Freescale EIM DRIVER"
> >  	depends on ARCH_MXC
> >  	help
> > -	  Driver for i.MX6 WEIM controller.
> > +	  Driver for i.MX WEIM controller.
> >  	  The WEIM(Wireless External Interface Module) works like a bus.
> >  	  You can attach many different devices on it, such as NOR, onenand.
> > -	  But now, we only support the Parallel NOR.
> > +	  But now, we only support the "of_physmap" driver.
> 
> This comment is wrong. In the early versions of this patch indeed only
> parallel NOR was supported, but now there is no limitation on the device
> type anymore.

I am do not think so. But I will not insist on his opinion.
I checked the operation only for physmap-flash and mtd-ram.

...
> >  static const struct of_device_id weim_id_table[] = {
> > +	/* i.MX1/21 */
> > +	{ .compatible = "fsl,imx1-weim", .data = &imx1_weim_devtype, },
> > +	/* i.MX25/27/31/35 */
> > +	{ .compatible = "fsl,imx25-weim", .data = &imx25_weim_devtype, },
> 
> We usually name the compatible name after the oldest i.MX supporting
> this IP, not after the lowest number. So this should be imx27, not
> imx25.

OK. Seems this fact also avoid [7/7] part. Is not it?

---

[PATCH v2 6/7] drivers: bus: imx-weim: Add support for i.MX1/21/25/27/31/35/50/51/53

[Sascha Hauer]

On Thu, Jun 20, 2013 at 12:16:33AM +0400, Alexander Shiyan wrote:
> > Hi Alexander,
> > 
> > Nice work!
> > 
> > On Wed, Jun 19, 2013 at 11:54:37PM +0400, Alexander Shiyan wrote:
> > > - - fsl,weim-cs-timing:	The timing array, contains 6 timing values for the
> > > + - 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. This property contains the values
> > > -			for the registers EIM_CSnGCR1, EIM_CSnGCR2, EIM_CSnRCR1,
> > > +			node's "reg" property. For example, if i.MX6Q CPU
> > > +			is used, this property contains the values for the
> > > +			registers EIM_CSnGCR1, EIM_CSnGCR2, EIM_CSnRCR1,
> > >  			EIM_CSnRCR2, EIM_CSnWCR1, EIM_CSnWCR2 in this order.
> > > +			For other i.MX CPUs count of register and register
> > > +			names may be different.
> > 
> > I think here we should have a more precise description for each SoC.
> 
> OK.
> 
> > >  Example for an imx6q-sabreauto board, the NOR flash connected to the WEIM:
> > >  
> > > diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> > > index 1f70e84..4faedc21 100644
> > > --- a/drivers/bus/Kconfig
> > > +++ b/drivers/bus/Kconfig
> > > @@ -8,10 +8,10 @@ config IMX_WEIM
> > >  	bool "Freescale EIM DRIVER"
> > >  	depends on ARCH_MXC
> > >  	help
> > > -	  Driver for i.MX6 WEIM controller.
> > > +	  Driver for i.MX WEIM controller.
> > >  	  The WEIM(Wireless External Interface Module) works like a bus.
> > >  	  You can attach many different devices on it, such as NOR, onenand.
> > > -	  But now, we only support the Parallel NOR.
> > > +	  But now, we only support the "of_physmap" driver.
> > 
> > This comment is wrong. In the early versions of this patch indeed only
> > parallel NOR was supported, but now there is no limitation on the device
> > type anymore.
> 
> I am do not think so. But I will not insist on his opinion.
> I checked the operation only for physmap-flash and mtd-ram.

The first version of the driver looked for a child node named 'nor' and
registered only that one. Now it calls of_platform_populate which
registers everything found under the weim node.

> 
> ...
> > >  static const struct of_device_id weim_id_table[] = {
> > > +	/* i.MX1/21 */
> > > +	{ .compatible = "fsl,imx1-weim", .data = &imx1_weim_devtype, },
> > > +	/* i.MX25/27/31/35 */
> > > +	{ .compatible = "fsl,imx25-weim", .data = &imx25_weim_devtype, },
> > 
> > We usually name the compatible name after the oldest i.MX supporting
> > this IP, not after the lowest number. So this should be imx27, not
> > imx25.
> 
> OK. Seems this fact also avoid [7/7] part. Is not it?

I think i.MX50 is older than i.MX6, so 7/7 should be correct.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v2 7/7] drivers: bus: imx-weim: Change compatible string for i.MX6Q

[Alexander Shiyan]

This patch changes compatible string for i.MX6Q to "fsl,imx50-weim"
since this is a lowest CPU type with same bus.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 Documentation/devicetree/bindings/bus/imx-weim.txt | 2 +-
 drivers/bus/imx-weim.c                             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/bus/imx-weim.txt b/Documentation/devicetree/bindings/bus/imx-weim.txt
index 99406e4..b134811 100644
--- a/Documentation/devicetree/bindings/bus/imx-weim.txt
+++ b/Documentation/devicetree/bindings/bus/imx-weim.txt
@@ -33,7 +33,7 @@ Timing property for child nodes. It is mandatory, not optional.
 Example for an imx6q-sabreauto board, the NOR flash connected to the WEIM:
 
 	weim: weim at 021b8000 {
-		compatible = "fsl,imx6q-weim";
+		compatible = "fsl,imx6q-weim", "fsl,imx50-weim";
 		reg = <0x021b8000 0x4000>;
 		clocks = <&clks 196>;
 		#address-cells = <2>;
diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index 77fa1d4..3f665f8 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -47,7 +47,7 @@ static const struct of_device_id weim_id_table[] = {
 	/* i.MX25/27/31/35 */
 	{ .compatible = "fsl,imx25-weim", .data = &imx25_weim_devtype, },
 	/* i.MX50/53/6Q */
-	{ .compatible = "fsl,imx6q-weim", .data = &imx50_weim_devtype, },
+	{ .compatible = "fsl,imx50-weim", .data = &imx50_weim_devtype, },
 	/* i.MX51 */
 	{ .compatible = "fsl,imx51-weim", .data = &imx51_weim_devtype, },
 	{ }
-- 
1.8.1.5

[PATCH v2 7/7] drivers: bus: imx-weim: Change compatible string for i.MX6Q

[Shawn Guo]

On Wed, Jun 19, 2013 at 11:54:38PM +0400, Alexander Shiyan wrote:
> This patch changes compatible string for i.MX6Q to "fsl,imx50-weim"
> since this is a lowest CPU type with same bus.
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  Documentation/devicetree/bindings/bus/imx-weim.txt | 2 +-
>  drivers/bus/imx-weim.c                             | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
I think patches #4, #6 and #7 can just be one patch.

> diff --git a/Documentation/devicetree/bindings/bus/imx-weim.txt b/Documentation/devicetree/bindings/bus/imx-weim.txt
> index 99406e4..b134811 100644
> --- a/Documentation/devicetree/bindings/bus/imx-weim.txt
> +++ b/Documentation/devicetree/bindings/bus/imx-weim.txt
> @@ -33,7 +33,7 @@ Timing property for child nodes. It is mandatory, not optional.
>  Example for an imx6q-sabreauto board, the NOR flash connected to the WEIM:
>  
>  	weim: weim at 021b8000 {
> -		compatible = "fsl,imx6q-weim";
> +		compatible = "fsl,imx6q-weim", "fsl,imx50-weim";
>  		reg = <0x021b8000 0x4000>;
>  		clocks = <&clks 196>;
>  		#address-cells = <2>;
> diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
> index 77fa1d4..3f665f8 100644
> --- a/drivers/bus/imx-weim.c
> +++ b/drivers/bus/imx-weim.c
> @@ -47,7 +47,7 @@ static const struct of_device_id weim_id_table[] = {
>  	/* i.MX25/27/31/35 */
>  	{ .compatible = "fsl,imx25-weim", .data = &imx25_weim_devtype, },
>  	/* i.MX50/53/6Q */
> -	{ .compatible = "fsl,imx6q-weim", .data = &imx50_weim_devtype, },
> +	{ .compatible = "fsl,imx50-weim", .data = &imx50_weim_devtype, },

The compatible "fsl,imx6q-weim" already has a user on the way to 3.11.
Changing the match table without updating the user will break it right
away.  Also, since the series will be 3.12 material, doing this will
result in an incompatible device tree between 3.11 and 3.12.  For this
reason, I suggest we keep using "fsl,imx6q-weim" for matching
imx6q/50/53 type of weim device.

Shawn

>  	/* i.MX51 */
>  	{ .compatible = "fsl,imx51-weim", .data = &imx51_weim_devtype, },
>  	{ }
> -- 
> 1.8.1.5
> 

[PATCH v2 7/7] drivers: bus: imx-weim: Change compatible string for i.MX6Q

[Sascha Hauer]

On Thu, Jun 20, 2013 at 03:10:01PM +0800, Shawn Guo wrote:
> On Wed, Jun 19, 2013 at 11:54:38PM +0400, Alexander Shiyan wrote:
> > This patch changes compatible string for i.MX6Q to "fsl,imx50-weim"
> > since this is a lowest CPU type with same bus.
> > 
> > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > ---
> >  Documentation/devicetree/bindings/bus/imx-weim.txt | 2 +-
> >  drivers/bus/imx-weim.c                             | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> I think patches #4, #6 and #7 can just be one patch.
> 
> > diff --git a/Documentation/devicetree/bindings/bus/imx-weim.txt b/Documentation/devicetree/bindings/bus/imx-weim.txt
> > index 99406e4..b134811 100644
> > --- a/Documentation/devicetree/bindings/bus/imx-weim.txt
> > +++ b/Documentation/devicetree/bindings/bus/imx-weim.txt
> > @@ -33,7 +33,7 @@ Timing property for child nodes. It is mandatory, not optional.
> >  Example for an imx6q-sabreauto board, the NOR flash connected to the WEIM:
> >  
> >  	weim: weim at 021b8000 {
> > -		compatible = "fsl,imx6q-weim";
> > +		compatible = "fsl,imx6q-weim", "fsl,imx50-weim";
> >  		reg = <0x021b8000 0x4000>;
> >  		clocks = <&clks 196>;
> >  		#address-cells = <2>;
> > diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
> > index 77fa1d4..3f665f8 100644
> > --- a/drivers/bus/imx-weim.c
> > +++ b/drivers/bus/imx-weim.c
> > @@ -47,7 +47,7 @@ static const struct of_device_id weim_id_table[] = {
> >  	/* i.MX25/27/31/35 */
> >  	{ .compatible = "fsl,imx25-weim", .data = &imx25_weim_devtype, },
> >  	/* i.MX50/53/6Q */
> > -	{ .compatible = "fsl,imx6q-weim", .data = &imx50_weim_devtype, },
> > +	{ .compatible = "fsl,imx50-weim", .data = &imx50_weim_devtype, },
> 
> The compatible "fsl,imx6q-weim" already has a user on the way to 3.11.
> Changing the match table without updating the user will break it right
> away.  Also, since the series will be 3.12 material, doing this will
> result in an incompatible device tree between 3.11 and 3.12.  For this
> reason, I suggest we keep using "fsl,imx6q-weim" for matching
> imx6q/50/53 type of weim device.

How about:

+	{ .compatible = "fsl,imx6q-weim", .data = &imx50_weim_devtype, },
+	{ .compatible = "fsl,imx50-weim", .data = &imx50_weim_devtype, },

We could then change the in-kernel devicetrees how we need it without
breaking existing devicetrees.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v2 7/7] drivers: bus: imx-weim: Change compatible string for i.MX6Q

[Shawn Guo]

On Thu, Jun 20, 2013 at 10:07:37AM +0200, Sascha Hauer wrote:
> How about:
> 
> +	{ .compatible = "fsl,imx6q-weim", .data = &imx50_weim_devtype, },
> +	{ .compatible = "fsl,imx50-weim", .data = &imx50_weim_devtype, },
> 
> We could then change the in-kernel devicetrees how we need it without
> breaking existing devicetrees.
> 
Yeah, it works for me.

Shawn

[PATCH v2 7/7] drivers: bus: imx-weim: Change compatible string for i.MX6Q

[Fabio Estevam]

On Thu, Jun 20, 2013 at 5:07 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Thu, Jun 20, 2013 at 03:10:01PM +0800, Shawn Guo wrote:

> How about:
>
> +       { .compatible = "fsl,imx6q-weim", .data = &imx50_weim_devtype, },
> +       { .compatible = "fsl,imx50-weim", .data = &imx50_weim_devtype, },

Why don't we just drop the mx50 case, since there is no kernel support for mx50?