[PATCH] i.MX6-SabreAuto: EIM: pull PAD_EIM_D18 low for NOR probe

[PATCH] i.MX6-SabreAuto: EIM: pull PAD_EIM_D18 low for NOR probe

[alison]

From: Alison Chaiken <alison_chaiken@mentor.com>

PAD_EIM_D18 must be pulled low at boot in order for the parallel NOR
connected to the EIM switch to probe properly.  Otherwise
cfi_qry_present() may return "U-V-]" rather than "Q-R-Y" if the
PAD_EIM_D18 is high.  Add a nor-gpios property to the nor node in the
SabreAuto device-tree and add a function to the imx-weim probe to set
GPIO5 to drive the pad.

Signed-off-by: Alison Chaiken <alison_chaiken@mentor.com>
---
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi |  1 +
 drivers/bus/imx-weim.c                   | 43 ++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 009abd6..dd5e3bc 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -454,5 +454,6 @@
 		bank-width = <2>;
 		fsl,weim-cs-timing = <0x00620081 0x00000001 0x1c022000
 				0x0000c000 0x1404a38e 0x00000000>;
+		nor-gpios = <&gpio5 4 GPIO_ACTIVE_LOW>;
 	};
 };
diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index 0958b69..b3c2ca6 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -14,6 +14,8 @@
 #include <linux/mfd/syscon.h>
 #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
 #include <linux/regmap.h>
+#include <linux/of_gpio.h>
+#include <linux/gpio.h>
 
 struct imx_weim_devtype {
 	unsigned int	cs_count;
@@ -108,6 +110,40 @@ err:
 	return -EINVAL;
 }
 
+/* set the GPIO to control PAD_EIM_D18 so cfi_qry_present() works properly */
+static int __init nor_gpio_setup(struct device_node *np, struct device *parent)
+{
+	unsigned nor_gpio, level;
+	enum of_gpio_flags of_flags;
+	int ret;
+
+	nor_gpio = of_get_named_gpio_flags(np, "nor-gpios", 0, &of_flags);
+
+	/* this child is not a NOR chip */
+	if (!nor_gpio)
+		return 0;
+
+	if (gpio_is_valid(nor_gpio)) {
+		ret = devm_gpio_request_one(parent, nor_gpio,
+					GPIOF_DIR_OUT, "nor-gpio");
+	} else {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	if (ret < 0)
+		goto out;
+
+	level = ((of_flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1);
+
+	gpio_set_value(nor_gpio, level);
+
+	return 0;
+out:
+	dev_err(parent, "Unable to request EIM_D18 GPIO for NOR.\n");
+	return ret;
+}
+
 /* 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)
@@ -160,6 +196,13 @@ static int __init weim_parse_dt(struct platform_device *pdev,
 				child->full_name);
 			return ret;
 		}
+
+		ret = nor_gpio_setup(child, &pdev->dev);
+		if (ret) {
+			dev_err(&pdev->dev, "%s gpios setup failed.\n",
+				child->full_name);
+			return ret;
+		}
 	}
 
 	ret = of_platform_populate(pdev->dev.of_node,
-- 
2.1.4

Re: [PATCH] i.MX6-SabreAuto: EIM: pull PAD_EIM_D18 low for NOR probe

[Brian Norris]

+ linux-arm-kernel, maintainers, devicetree

Shawn, Sascha: should this driver be listed under the Freescale IMX
MAINTAINERS entry?

On Wed, Jan 14, 2015 at 03:36:49PM -0800, alison-hh6fLRYtCEIS+FvcfC7Uqw@public.gmane.org wrote:
> From: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> 
> PAD_EIM_D18 must be pulled low at boot in order for the parallel NOR
> connected to the EIM switch to probe properly.  Otherwise
> cfi_qry_present() may return "U-V-]" rather than "Q-R-Y" if the
> PAD_EIM_D18 is high.  Add a nor-gpios property to the nor node in the
> SabreAuto device-tree and add a function to the imx-weim probe to set
> GPIO5 to drive the pad.
> 
> Signed-off-by: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> ---
>  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi |  1 +
>  drivers/bus/imx-weim.c                   | 43 ++++++++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> index 009abd6..dd5e3bc 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> @@ -454,5 +454,6 @@
>  		bank-width = <2>;
>  		fsl,weim-cs-timing = <0x00620081 0x00000001 0x1c022000
>  				0x0000c000 0x1404a38e 0x00000000>;
> +		nor-gpios = <&gpio5 4 GPIO_ACTIVE_LOW>;

Such a binding needs to be documented. Also, if it's going to be named
generically like that, it needs to be generically useful and supported
under MTD code, not platform-specific bus code.

So what pin is this, exactly? A write-protect pin? An address pin? A
toaster control, where the toaster is keeping the flash too hot?

>  	};
>  };
> diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
> index 0958b69..b3c2ca6 100644
> --- a/drivers/bus/imx-weim.c
> +++ b/drivers/bus/imx-weim.c
> @@ -14,6 +14,8 @@
>  #include <linux/mfd/syscon.h>
>  #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
>  #include <linux/regmap.h>
> +#include <linux/of_gpio.h>
> +#include <linux/gpio.h>
>  
>  struct imx_weim_devtype {
>  	unsigned int	cs_count;
> @@ -108,6 +110,40 @@ err:
>  	return -EINVAL;
>  }
>  
> +/* set the GPIO to control PAD_EIM_D18 so cfi_qry_present() works properly */
> +static int __init nor_gpio_setup(struct device_node *np, struct device *parent)
> +{
> +	unsigned nor_gpio, level;
> +	enum of_gpio_flags of_flags;
> +	int ret;
> +
> +	nor_gpio = of_get_named_gpio_flags(np, "nor-gpios", 0, &of_flags);
> +
> +	/* this child is not a NOR chip */
> +	if (!nor_gpio)
> +		return 0;
> +
> +	if (gpio_is_valid(nor_gpio)) {
> +		ret = devm_gpio_request_one(parent, nor_gpio,
> +					GPIOF_DIR_OUT, "nor-gpio");
> +	} else {
> +		ret = -ENODEV;
> +		goto out;
> +	}
> +
> +	if (ret < 0)
> +		goto out;
> +
> +	level = ((of_flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1);
> +
> +	gpio_set_value(nor_gpio, level);
> +
> +	return 0;
> +out:
> +	dev_err(parent, "Unable to request EIM_D18 GPIO for NOR.\n");
> +	return ret;
> +}
> +
>  /* 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)
> @@ -160,6 +196,13 @@ static int __init weim_parse_dt(struct platform_device *pdev,
>  				child->full_name);
>  			return ret;
>  		}
> +
> +		ret = nor_gpio_setup(child, &pdev->dev);
> +		if (ret) {
> +			dev_err(&pdev->dev, "%s gpios setup failed.\n",
> +				child->full_name);
> +			return ret;
> +		}
>  	}
>  
>  	ret = of_platform_populate(pdev->dev.of_node,

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i.MX6-SabreAuto: EIM: pull PAD_EIM_D18 low for NOR probe

[Brian Norris]

+ linux-arm-kernel, maintainers, devicetree

Shawn, Sascha: should this driver be listed under the Freescale IMX
MAINTAINERS entry?

On Wed, Jan 14, 2015 at 03:36:49PM -0800, alison@she-devel.com wrote:
> From: Alison Chaiken <alison_chaiken@mentor.com>
> 
> PAD_EIM_D18 must be pulled low at boot in order for the parallel NOR
> connected to the EIM switch to probe properly.  Otherwise
> cfi_qry_present() may return "U-V-]" rather than "Q-R-Y" if the
> PAD_EIM_D18 is high.  Add a nor-gpios property to the nor node in the
> SabreAuto device-tree and add a function to the imx-weim probe to set
> GPIO5 to drive the pad.
> 
> Signed-off-by: Alison Chaiken <alison_chaiken@mentor.com>
> ---
>  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi |  1 +
>  drivers/bus/imx-weim.c                   | 43 ++++++++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> index 009abd6..dd5e3bc 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> @@ -454,5 +454,6 @@
>  		bank-width = <2>;
>  		fsl,weim-cs-timing = <0x00620081 0x00000001 0x1c022000
>  				0x0000c000 0x1404a38e 0x00000000>;
> +		nor-gpios = <&gpio5 4 GPIO_ACTIVE_LOW>;

Such a binding needs to be documented. Also, if it's going to be named
generically like that, it needs to be generically useful and supported
under MTD code, not platform-specific bus code.

So what pin is this, exactly? A write-protect pin? An address pin? A
toaster control, where the toaster is keeping the flash too hot?

>  	};
>  };
> diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
> index 0958b69..b3c2ca6 100644
> --- a/drivers/bus/imx-weim.c
> +++ b/drivers/bus/imx-weim.c
> @@ -14,6 +14,8 @@
>  #include <linux/mfd/syscon.h>
>  #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
>  #include <linux/regmap.h>
> +#include <linux/of_gpio.h>
> +#include <linux/gpio.h>
>  
>  struct imx_weim_devtype {
>  	unsigned int	cs_count;
> @@ -108,6 +110,40 @@ err:
>  	return -EINVAL;
>  }
>  
> +/* set the GPIO to control PAD_EIM_D18 so cfi_qry_present() works properly */
> +static int __init nor_gpio_setup(struct device_node *np, struct device *parent)
> +{
> +	unsigned nor_gpio, level;
> +	enum of_gpio_flags of_flags;
> +	int ret;
> +
> +	nor_gpio = of_get_named_gpio_flags(np, "nor-gpios", 0, &of_flags);
> +
> +	/* this child is not a NOR chip */
> +	if (!nor_gpio)
> +		return 0;
> +
> +	if (gpio_is_valid(nor_gpio)) {
> +		ret = devm_gpio_request_one(parent, nor_gpio,
> +					GPIOF_DIR_OUT, "nor-gpio");
> +	} else {
> +		ret = -ENODEV;
> +		goto out;
> +	}
> +
> +	if (ret < 0)
> +		goto out;
> +
> +	level = ((of_flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1);
> +
> +	gpio_set_value(nor_gpio, level);
> +
> +	return 0;
> +out:
> +	dev_err(parent, "Unable to request EIM_D18 GPIO for NOR.\n");
> +	return ret;
> +}
> +
>  /* 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)
> @@ -160,6 +196,13 @@ static int __init weim_parse_dt(struct platform_device *pdev,
>  				child->full_name);
>  			return ret;
>  		}
> +
> +		ret = nor_gpio_setup(child, &pdev->dev);
> +		if (ret) {
> +			dev_err(&pdev->dev, "%s gpios setup failed.\n",
> +				child->full_name);
> +			return ret;
> +		}
>  	}
>  
>  	ret = of_platform_populate(pdev->dev.of_node,

Brian

[PATCH] i.MX6-SabreAuto: EIM: pull PAD_EIM_D18 low for NOR probe

[Brian Norris]

+ linux-arm-kernel, maintainers, devicetree

Shawn, Sascha: should this driver be listed under the Freescale IMX
MAINTAINERS entry?

On Wed, Jan 14, 2015 at 03:36:49PM -0800, alison at she-devel.com wrote:
> From: Alison Chaiken <alison_chaiken@mentor.com>
> 
> PAD_EIM_D18 must be pulled low at boot in order for the parallel NOR
> connected to the EIM switch to probe properly.  Otherwise
> cfi_qry_present() may return "U-V-]" rather than "Q-R-Y" if the
> PAD_EIM_D18 is high.  Add a nor-gpios property to the nor node in the
> SabreAuto device-tree and add a function to the imx-weim probe to set
> GPIO5 to drive the pad.
> 
> Signed-off-by: Alison Chaiken <alison_chaiken@mentor.com>
> ---
>  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi |  1 +
>  drivers/bus/imx-weim.c                   | 43 ++++++++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> index 009abd6..dd5e3bc 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> @@ -454,5 +454,6 @@
>  		bank-width = <2>;
>  		fsl,weim-cs-timing = <0x00620081 0x00000001 0x1c022000
>  				0x0000c000 0x1404a38e 0x00000000>;
> +		nor-gpios = <&gpio5 4 GPIO_ACTIVE_LOW>;

Such a binding needs to be documented. Also, if it's going to be named
generically like that, it needs to be generically useful and supported
under MTD code, not platform-specific bus code.

So what pin is this, exactly? A write-protect pin? An address pin? A
toaster control, where the toaster is keeping the flash too hot?

>  	};
>  };
> diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
> index 0958b69..b3c2ca6 100644
> --- a/drivers/bus/imx-weim.c
> +++ b/drivers/bus/imx-weim.c
> @@ -14,6 +14,8 @@
>  #include <linux/mfd/syscon.h>
>  #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
>  #include <linux/regmap.h>
> +#include <linux/of_gpio.h>
> +#include <linux/gpio.h>
>  
>  struct imx_weim_devtype {
>  	unsigned int	cs_count;
> @@ -108,6 +110,40 @@ err:
>  	return -EINVAL;
>  }
>  
> +/* set the GPIO to control PAD_EIM_D18 so cfi_qry_present() works properly */
> +static int __init nor_gpio_setup(struct device_node *np, struct device *parent)
> +{
> +	unsigned nor_gpio, level;
> +	enum of_gpio_flags of_flags;
> +	int ret;
> +
> +	nor_gpio = of_get_named_gpio_flags(np, "nor-gpios", 0, &of_flags);
> +
> +	/* this child is not a NOR chip */
> +	if (!nor_gpio)
> +		return 0;
> +
> +	if (gpio_is_valid(nor_gpio)) {
> +		ret = devm_gpio_request_one(parent, nor_gpio,
> +					GPIOF_DIR_OUT, "nor-gpio");
> +	} else {
> +		ret = -ENODEV;
> +		goto out;
> +	}
> +
> +	if (ret < 0)
> +		goto out;
> +
> +	level = ((of_flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1);
> +
> +	gpio_set_value(nor_gpio, level);
> +
> +	return 0;
> +out:
> +	dev_err(parent, "Unable to request EIM_D18 GPIO for NOR.\n");
> +	return ret;
> +}
> +
>  /* 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)
> @@ -160,6 +196,13 @@ static int __init weim_parse_dt(struct platform_device *pdev,
>  				child->full_name);
>  			return ret;
>  		}
> +
> +		ret = nor_gpio_setup(child, &pdev->dev);
> +		if (ret) {
> +			dev_err(&pdev->dev, "%s gpios setup failed.\n",
> +				child->full_name);
> +			return ret;
> +		}
>  	}
>  
>  	ret = of_platform_populate(pdev->dev.of_node,

Brian

Re: [PATCH] i.MX6-SabreAuto: EIM: pull PAD_EIM_D18 low for NOR probe

[Sascha Hauer]

On Wed, Jan 14, 2015 at 04:03:34PM -0800, Brian Norris wrote:
> + linux-arm-kernel, maintainers, devicetree
> 
> Shawn, Sascha: should this driver be listed under the Freescale IMX
> MAINTAINERS entry?

This driver is purely i.MX, so yes, that would be good.

> 
> On Wed, Jan 14, 2015 at 03:36:49PM -0800, alison-hh6fLRYtCEIS+FvcfC7Uqw@public.gmane.org wrote:
> > From: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> > 
> > PAD_EIM_D18 must be pulled low at boot in order for the parallel NOR
> > connected to the EIM switch to probe properly.  Otherwise
> > cfi_qry_present() may return "U-V-]" rather than "Q-R-Y" if the
> > PAD_EIM_D18 is high.  Add a nor-gpios property to the nor node in the
> > SabreAuto device-tree and add a function to the imx-weim probe to set
> > GPIO5 to drive the pad.
> > 
> > Signed-off-by: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi |  1 +
> >  drivers/bus/imx-weim.c                   | 43 ++++++++++++++++++++++++++++++++
> >  2 files changed, 44 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> > index 009abd6..dd5e3bc 100644
> > --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> > @@ -454,5 +454,6 @@
> >  		bank-width = <2>;
> >  		fsl,weim-cs-timing = <0x00620081 0x00000001 0x1c022000
> >  				0x0000c000 0x1404a38e 0x00000000>;
> > +		nor-gpios = <&gpio5 4 GPIO_ACTIVE_LOW>;
> 
> Such a binding needs to be documented. Also, if it's going to be named
> generically like that, it needs to be generically useful and supported
> under MTD code, not platform-specific bus code.
> 
> So what pin is this, exactly? A write-protect pin? An address pin? A
> toaster control, where the toaster is keeping the flash too hot?

Yeah, the gpio should probably be named after the input pin on the flash
where it's connected to. Otherwise people just use this property for any
GPIO they need to put in a certain direction.

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 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i.MX6-SabreAuto: EIM: pull PAD_EIM_D18 low for NOR probe

[Sascha Hauer]

On Wed, Jan 14, 2015 at 04:03:34PM -0800, Brian Norris wrote:
> + linux-arm-kernel, maintainers, devicetree
> 
> Shawn, Sascha: should this driver be listed under the Freescale IMX
> MAINTAINERS entry?

This driver is purely i.MX, so yes, that would be good.

> 
> On Wed, Jan 14, 2015 at 03:36:49PM -0800, alison@she-devel.com wrote:
> > From: Alison Chaiken <alison_chaiken@mentor.com>
> > 
> > PAD_EIM_D18 must be pulled low at boot in order for the parallel NOR
> > connected to the EIM switch to probe properly.  Otherwise
> > cfi_qry_present() may return "U-V-]" rather than "Q-R-Y" if the
> > PAD_EIM_D18 is high.  Add a nor-gpios property to the nor node in the
> > SabreAuto device-tree and add a function to the imx-weim probe to set
> > GPIO5 to drive the pad.
> > 
> > Signed-off-by: Alison Chaiken <alison_chaiken@mentor.com>
> > ---
> >  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi |  1 +
> >  drivers/bus/imx-weim.c                   | 43 ++++++++++++++++++++++++++++++++
> >  2 files changed, 44 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> > index 009abd6..dd5e3bc 100644
> > --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> > @@ -454,5 +454,6 @@
> >  		bank-width = <2>;
> >  		fsl,weim-cs-timing = <0x00620081 0x00000001 0x1c022000
> >  				0x0000c000 0x1404a38e 0x00000000>;
> > +		nor-gpios = <&gpio5 4 GPIO_ACTIVE_LOW>;
> 
> Such a binding needs to be documented. Also, if it's going to be named
> generically like that, it needs to be generically useful and supported
> under MTD code, not platform-specific bus code.
> 
> So what pin is this, exactly? A write-protect pin? An address pin? A
> toaster control, where the toaster is keeping the flash too hot?

Yeah, the gpio should probably be named after the input pin on the flash
where it's connected to. Otherwise people just use this property for any
GPIO they need to put in a certain direction.

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] i.MX6-SabreAuto: EIM: pull PAD_EIM_D18 low for NOR probe

[Sascha Hauer]

On Wed, Jan 14, 2015 at 04:03:34PM -0800, Brian Norris wrote:
> + linux-arm-kernel, maintainers, devicetree
> 
> Shawn, Sascha: should this driver be listed under the Freescale IMX
> MAINTAINERS entry?

This driver is purely i.MX, so yes, that would be good.

> 
> On Wed, Jan 14, 2015 at 03:36:49PM -0800, alison at she-devel.com wrote:
> > From: Alison Chaiken <alison_chaiken@mentor.com>
> > 
> > PAD_EIM_D18 must be pulled low at boot in order for the parallel NOR
> > connected to the EIM switch to probe properly.  Otherwise
> > cfi_qry_present() may return "U-V-]" rather than "Q-R-Y" if the
> > PAD_EIM_D18 is high.  Add a nor-gpios property to the nor node in the
> > SabreAuto device-tree and add a function to the imx-weim probe to set
> > GPIO5 to drive the pad.
> > 
> > Signed-off-by: Alison Chaiken <alison_chaiken@mentor.com>
> > ---
> >  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi |  1 +
> >  drivers/bus/imx-weim.c                   | 43 ++++++++++++++++++++++++++++++++
> >  2 files changed, 44 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> > index 009abd6..dd5e3bc 100644
> > --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> > @@ -454,5 +454,6 @@
> >  		bank-width = <2>;
> >  		fsl,weim-cs-timing = <0x00620081 0x00000001 0x1c022000
> >  				0x0000c000 0x1404a38e 0x00000000>;
> > +		nor-gpios = <&gpio5 4 GPIO_ACTIVE_LOW>;
> 
> Such a binding needs to be documented. Also, if it's going to be named
> generically like that, it needs to be generically useful and supported
> under MTD code, not platform-specific bus code.
> 
> So what pin is this, exactly? A write-protect pin? An address pin? A
> toaster control, where the toaster is keeping the flash too hot?

Yeah, the gpio should probably be named after the input pin on the flash
where it's connected to. Otherwise people just use this property for any
GPIO they need to put in a certain direction.

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 |

[PATCHv2] i.MX6-SabreAuto: WEIM: add steering-gpios to set WEIM for NOR

[alison-hh6fLRYtCEIS+FvcfC7Uqw]

From: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>

The EIM switch of the i.MX6 SabreAuto CPU cards can connect pins
EIM_D18 and EIM_D30 of the CPU to either the parallel NOR or to other
peripherals.  Add a steering-gpios property to the imx6qdl-sabreauto
device-tree to allow boot-time configuration of the EIM.  Use GPIO5,
pin 4 to connect EIM_D18 to the NOR so the do_map_probe() function
correctly detects the CFI.

Support is not added for boot-time steering of EIM_D30 due to present
inability to test.

Alison Chaiken (1):
  i.MX6-SabreAuto: WEIM: add steering-gpios to set WEIM for NOR

 Documentation/devicetree/bindings/bus/imx-weim.txt | 10 +++++
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi           |  1 +
 drivers/bus/imx-weim.c                             | 46 ++++++++++++++++++++++
 3 files changed, 57 insertions(+)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] i.MX6-SabreAuto: WEIM: add steering-gpios to set WEIM for NOR

[alison at she-devel.com]

From: Alison Chaiken <alison_chaiken@mentor.com>

The EIM switch of the i.MX6 SabreAuto CPU cards can connect pins
EIM_D18 and EIM_D30 of the CPU to either the parallel NOR or to other
peripherals.  Add a steering-gpios property to the imx6qdl-sabreauto
device-tree to allow boot-time configuration of the EIM.  Use GPIO5,
pin 4 to connect EIM_D18 to the NOR so the do_map_probe() function
correctly detects the CFI.

Support is not added for boot-time steering of EIM_D30 due to present
inability to test.

Alison Chaiken (1):
  i.MX6-SabreAuto: WEIM: add steering-gpios to set WEIM for NOR

 Documentation/devicetree/bindings/bus/imx-weim.txt | 10 +++++
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi           |  1 +
 drivers/bus/imx-weim.c                             | 46 ++++++++++++++++++++++
 3 files changed, 57 insertions(+)

-- 
2.1.4

[PATCH] i.MX6-SabreAuto: WEIM: add steering-gpios to set WEIM for NOR

[alison-hh6fLRYtCEIS+FvcfC7Uqw]

From: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>

EIM_D18 pin of i.MX6 CPU is connected to the WEIM switch, to which pin
DQ2 of the parallel NOR is also attached.  Use GPIO5, pin 4 to steer
the switch to connect the pin to the parallel NOR at boot.  If the
GPIO is not set LOW, the probe of the NOR fails when cfi_qry_present()
returns "U-V-]" rather than "Q-R-Y" because bit 2 is erroneously high.
Implement control of the GPIO by adding a steering-gpios property to
the nor child node of the WEIM in the SabreAuto device-tree. Also add
a function to the imx-weim probe to set GPIO5 to drive the pad.

Signed-off-by: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
 Documentation/devicetree/bindings/bus/imx-weim.txt | 10 +++++
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi           |  1 +
 drivers/bus/imx-weim.c                             | 46 ++++++++++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/Documentation/devicetree/bindings/bus/imx-weim.txt b/Documentation/devicetree/bindings/bus/imx-weim.txt
index 6630d84..359fb26 100644
--- a/Documentation/devicetree/bindings/bus/imx-weim.txt
+++ b/Documentation/devicetree/bindings/bus/imx-weim.txt
@@ -59,6 +59,15 @@ Timing property for child nodes. It is mandatory, not optional.
 			there are six registers: CSxGCR1, CSxGCR2, CSxRCR1,
 			CSxRCR2, CSxWCR1, CSxWCR2.
 
+Steering property for child nodes is optional.  Steering is needed if
+the bootloader doesn't set the GPIOs driving the EIM switch to connect
+the WEIM to the CPU rather than to the peripherals with which the WEIM
+has a pin conflict.
+
+ - fsl,steering-gpios:  For i.mx6q-sabreauto, the connectivity of CPU
+			pins EIM_D18 and EIM_D30 can be controlled via
+			GPIOs.
+
 Example for an imx6q-sabreauto board, the NOR flash connected to the WEIM:
 
 	weim: weim@021b8000 {
@@ -76,6 +85,7 @@ Example for an imx6q-sabreauto board, the NOR flash connected to the WEIM:
 			#address-cells = <1>;
 			#size-cells = <1>;
 			bank-width = <2>;
+			fsl,steering-gpios = <&gpio5 4 GPIO_ACTIVE_LOW>;
 			fsl,weim-cs-timing = <0x00620081 0x00000001 0x1c022000
 					0x0000c000 0x1404a38e 0x00000000>;
 		};
diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 009abd6..e1429c5 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -454,5 +454,6 @@
 		bank-width = <2>;
 		fsl,weim-cs-timing = <0x00620081 0x00000001 0x1c022000
 				0x0000c000 0x1404a38e 0x00000000>;
+		fsl,steering-gpios = <&gpio5 4 GPIO_ACTIVE_LOW>;
 	};
 };
diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index 0958b69..af3408e 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -14,6 +14,8 @@
 #include <linux/mfd/syscon.h>
 #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
 #include <linux/regmap.h>
+#include <linux/of_gpio.h>
+#include <linux/gpio.h>
 
 struct imx_weim_devtype {
 	unsigned int	cs_count;
@@ -108,6 +110,43 @@ err:
 	return -EINVAL;
 }
 
+/* set GPIOs to steer EIM CPU PADs to WEIM connection so that
+ * cfi_qry_present() of attached NOR chip works properly */
+static int __init steering_gpio_setup(struct device_node *np,
+				struct device *parent)
+{
+	unsigned steering_gpio, level;
+	enum of_gpio_flags of_flags;
+	int ret;
+
+	steering_gpio = of_get_named_gpio_flags(np, "fsl,steering-gpios",
+						0, &of_flags);
+
+	/* this child requires no pin steering */
+	if (!steering_gpio)
+		return 0;
+
+	if (gpio_is_valid(steering_gpio)) {
+		ret = devm_gpio_request_one(parent, steering_gpio,
+					GPIOF_DIR_OUT, "steering-gpio");
+	} else {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	if (ret < 0)
+		goto out;
+
+	level = ((of_flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1);
+
+	gpio_set_value(steering_gpio, level);
+
+	return 0;
+out:
+	dev_err(parent, "Unable to request steering GPIO.\n");
+	return ret;
+}
+
 /* 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)
@@ -160,6 +199,13 @@ static int __init weim_parse_dt(struct platform_device *pdev,
 				child->full_name);
 			return ret;
 		}
+
+		ret = steering_gpio_setup(child, &pdev->dev);
+		if (ret) {
+			dev_err(&pdev->dev, "%s steering gpios setup failed.\n",
+				child->full_name);
+			return ret;
+		}
 	}
 
 	ret = of_platform_populate(pdev->dev.of_node,
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] i.MX6-SabreAuto: WEIM: add steering-gpios to set WEIM for NOR

[alison at she-devel.com]

From: Alison Chaiken <alison_chaiken@mentor.com>

EIM_D18 pin of i.MX6 CPU is connected to the WEIM switch, to which pin
DQ2 of the parallel NOR is also attached.  Use GPIO5, pin 4 to steer
the switch to connect the pin to the parallel NOR at boot.  If the
GPIO is not set LOW, the probe of the NOR fails when cfi_qry_present()
returns "U-V-]" rather than "Q-R-Y" because bit 2 is erroneously high.
Implement control of the GPIO by adding a steering-gpios property to
the nor child node of the WEIM in the SabreAuto device-tree. Also add
a function to the imx-weim probe to set GPIO5 to drive the pad.

Signed-off-by: Alison Chaiken <alison_chaiken@mentor.com>
---
 Documentation/devicetree/bindings/bus/imx-weim.txt | 10 +++++
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi           |  1 +
 drivers/bus/imx-weim.c                             | 46 ++++++++++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/Documentation/devicetree/bindings/bus/imx-weim.txt b/Documentation/devicetree/bindings/bus/imx-weim.txt
index 6630d84..359fb26 100644
--- a/Documentation/devicetree/bindings/bus/imx-weim.txt
+++ b/Documentation/devicetree/bindings/bus/imx-weim.txt
@@ -59,6 +59,15 @@ Timing property for child nodes. It is mandatory, not optional.
 			there are six registers: CSxGCR1, CSxGCR2, CSxRCR1,
 			CSxRCR2, CSxWCR1, CSxWCR2.
 
+Steering property for child nodes is optional.  Steering is needed if
+the bootloader doesn't set the GPIOs driving the EIM switch to connect
+the WEIM to the CPU rather than to the peripherals with which the WEIM
+has a pin conflict.
+
+ - fsl,steering-gpios:  For i.mx6q-sabreauto, the connectivity of CPU
+			pins EIM_D18 and EIM_D30 can be controlled via
+			GPIOs.
+
 Example for an imx6q-sabreauto board, the NOR flash connected to the WEIM:
 
 	weim: weim at 021b8000 {
@@ -76,6 +85,7 @@ Example for an imx6q-sabreauto board, the NOR flash connected to the WEIM:
 			#address-cells = <1>;
 			#size-cells = <1>;
 			bank-width = <2>;
+			fsl,steering-gpios = <&gpio5 4 GPIO_ACTIVE_LOW>;
 			fsl,weim-cs-timing = <0x00620081 0x00000001 0x1c022000
 					0x0000c000 0x1404a38e 0x00000000>;
 		};
diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 009abd6..e1429c5 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -454,5 +454,6 @@
 		bank-width = <2>;
 		fsl,weim-cs-timing = <0x00620081 0x00000001 0x1c022000
 				0x0000c000 0x1404a38e 0x00000000>;
+		fsl,steering-gpios = <&gpio5 4 GPIO_ACTIVE_LOW>;
 	};
 };
diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index 0958b69..af3408e 100644
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -14,6 +14,8 @@
 #include <linux/mfd/syscon.h>
 #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
 #include <linux/regmap.h>
+#include <linux/of_gpio.h>
+#include <linux/gpio.h>
 
 struct imx_weim_devtype {
 	unsigned int	cs_count;
@@ -108,6 +110,43 @@ err:
 	return -EINVAL;
 }
 
+/* set GPIOs to steer EIM CPU PADs to WEIM connection so that
+ * cfi_qry_present() of attached NOR chip works properly */
+static int __init steering_gpio_setup(struct device_node *np,
+				struct device *parent)
+{
+	unsigned steering_gpio, level;
+	enum of_gpio_flags of_flags;
+	int ret;
+
+	steering_gpio = of_get_named_gpio_flags(np, "fsl,steering-gpios",
+						0, &of_flags);
+
+	/* this child requires no pin steering */
+	if (!steering_gpio)
+		return 0;
+
+	if (gpio_is_valid(steering_gpio)) {
+		ret = devm_gpio_request_one(parent, steering_gpio,
+					GPIOF_DIR_OUT, "steering-gpio");
+	} else {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	if (ret < 0)
+		goto out;
+
+	level = ((of_flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1);
+
+	gpio_set_value(steering_gpio, level);
+
+	return 0;
+out:
+	dev_err(parent, "Unable to request steering GPIO.\n");
+	return ret;
+}
+
 /* 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)
@@ -160,6 +199,13 @@ static int __init weim_parse_dt(struct platform_device *pdev,
 				child->full_name);
 			return ret;
 		}
+
+		ret = steering_gpio_setup(child, &pdev->dev);
+		if (ret) {
+			dev_err(&pdev->dev, "%s steering gpios setup failed.\n",
+				child->full_name);
+			return ret;
+		}
 	}
 
 	ret = of_platform_populate(pdev->dev.of_node,
-- 
2.1.4

Re: [PATCH] i.MX6-SabreAuto: WEIM: add steering-gpios to set WEIM for NOR

[Sascha Hauer]

On Thu, Jan 15, 2015 at 10:46:11AM -0800, alison-hh6fLRYtCEIS+FvcfC7Uqw@public.gmane.org wrote:
> From: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> 
> EIM_D18 pin of i.MX6 CPU is connected to the WEIM switch, to which pin
> DQ2 of the parallel NOR is also attached.  Use GPIO5, pin 4 to steer
> the switch to connect the pin to the parallel NOR at boot.  If the
> GPIO is not set LOW, the probe of the NOR fails when cfi_qry_present()
> returns "U-V-]" rather than "Q-R-Y" because bit 2 is erroneously high.
> Implement control of the GPIO by adding a steering-gpios property to
> the nor child node of the WEIM in the SabreAuto device-tree. Also add
> a function to the imx-weim probe to set GPIO5 to drive the pad.
> 
> Signed-off-by: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/bus/imx-weim.txt | 10 +++++
>  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi           |  1 +
>  drivers/bus/imx-weim.c                             | 46 ++++++++++++++++++++++
>  3 files changed, 57 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/bus/imx-weim.txt b/Documentation/devicetree/bindings/bus/imx-weim.txt
> index 6630d84..359fb26 100644
> --- a/Documentation/devicetree/bindings/bus/imx-weim.txt
> +++ b/Documentation/devicetree/bindings/bus/imx-weim.txt
> @@ -59,6 +59,15 @@ Timing property for child nodes. It is mandatory, not optional.
>  			there are six registers: CSxGCR1, CSxGCR2, CSxRCR1,
>  			CSxRCR2, CSxWCR1, CSxWCR2.
>  
> +Steering property for child nodes is optional.  Steering is needed if
> +the bootloader doesn't set the GPIOs driving the EIM switch to connect
> +the WEIM to the CPU rather than to the peripherals with which the WEIM
> +has a pin conflict.
> +
> + - fsl,steering-gpios:  For i.mx6q-sabreauto, the connectivity of CPU
> +			pins EIM_D18 and EIM_D30 can be controlled via
> +			GPIOs.

It seems to be common that some GPIOs which are not regulators or reset
pins must be switched into the right direction for stuff to work. Adding
board specific properties to the device tree to solve this common
problem for a single usecase doesn't sound like a good solution. I
suggest that you take a look at the GPIO hogging mechanism patches from
Benoit Parrot instead: https://lkml.org/lkml/2014/12/19/342

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 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] i.MX6-SabreAuto: WEIM: add steering-gpios to set WEIM for NOR

[Sascha Hauer]

On Thu, Jan 15, 2015 at 10:46:11AM -0800, alison at she-devel.com wrote:
> From: Alison Chaiken <alison_chaiken@mentor.com>
> 
> EIM_D18 pin of i.MX6 CPU is connected to the WEIM switch, to which pin
> DQ2 of the parallel NOR is also attached.  Use GPIO5, pin 4 to steer
> the switch to connect the pin to the parallel NOR at boot.  If the
> GPIO is not set LOW, the probe of the NOR fails when cfi_qry_present()
> returns "U-V-]" rather than "Q-R-Y" because bit 2 is erroneously high.
> Implement control of the GPIO by adding a steering-gpios property to
> the nor child node of the WEIM in the SabreAuto device-tree. Also add
> a function to the imx-weim probe to set GPIO5 to drive the pad.
> 
> Signed-off-by: Alison Chaiken <alison_chaiken@mentor.com>
> ---
>  Documentation/devicetree/bindings/bus/imx-weim.txt | 10 +++++
>  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi           |  1 +
>  drivers/bus/imx-weim.c                             | 46 ++++++++++++++++++++++
>  3 files changed, 57 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/bus/imx-weim.txt b/Documentation/devicetree/bindings/bus/imx-weim.txt
> index 6630d84..359fb26 100644
> --- a/Documentation/devicetree/bindings/bus/imx-weim.txt
> +++ b/Documentation/devicetree/bindings/bus/imx-weim.txt
> @@ -59,6 +59,15 @@ Timing property for child nodes. It is mandatory, not optional.
>  			there are six registers: CSxGCR1, CSxGCR2, CSxRCR1,
>  			CSxRCR2, CSxWCR1, CSxWCR2.
>  
> +Steering property for child nodes is optional.  Steering is needed if
> +the bootloader doesn't set the GPIOs driving the EIM switch to connect
> +the WEIM to the CPU rather than to the peripherals with which the WEIM
> +has a pin conflict.
> +
> + - fsl,steering-gpios:  For i.mx6q-sabreauto, the connectivity of CPU
> +			pins EIM_D18 and EIM_D30 can be controlled via
> +			GPIOs.

It seems to be common that some GPIOs which are not regulators or reset
pins must be switched into the right direction for stuff to work. Adding
board specific properties to the device tree to solve this common
problem for a single usecase doesn't sound like a good solution. I
suggest that you take a look at the GPIO hogging mechanism patches from
Benoit Parrot instead: https://lkml.org/lkml/2014/12/19/342

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 0/2] i.MX6-SabreAuto: DTS: use gpio-hog to enable WEIM-NOR at boot

[alison-hh6fLRYtCEIS+FvcfC7Uqw]

From: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>

The parallel NOR on the i.MX6 SabreAuto boards is connected by the
WEIM simple bus.  WEIM needs gpio5 low at boot in order to properly
set the steering of I2C3.

Rather than override the GPIO5 node in a new DTSI file, it would be better
to add the weim_nor gpio-hog to imx6qdl.dtsi and disable it by default.
However, gpio-hogs are always enabled, so inclusion of a separate, new
file is necessary.

Alison Chaiken (1):
  i.MX6-SabreAuto: DTS: use gpio-hog to enable WEIM-NOR at boot

 arch/arm/boot/dts/imx6-sabreauto-weim-nor.dtsi | 56 ++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6-sabreauto-weim-nor.dtsi

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] i.MX6-SabreAuto: DTS: use gpio-hog to enable WEIM-NOR at boot

[alison-hh6fLRYtCEIS+FvcfC7Uqw]

From: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>

Create an imx6-sabreauto-weim-nor.dtsi file whose inclusion in
a DTS file sets GPIO5 to the level at boot that the WEIM-NOR
device requires.  The GPIO is set via the gpio-hogging mechanism.

Signed-off-by: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/imx6-sabreauto-weim-nor.dtsi | 56 ++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6-sabreauto-weim-nor.dtsi

diff --git a/arch/arm/boot/dts/imx6-sabreauto-weim-nor.dtsi b/arch/arm/boot/dts/imx6-sabreauto-weim-nor.dtsi
new file mode 100644
index 0000000..940a908
--- /dev/null
+++ b/arch/arm/boot/dts/imx6-sabreauto-weim-nor.dtsi
@@ -0,0 +1,56 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ * Copyright 2011 Linaro Ltd.
+ * Copyright (c) 2015 Mentor Graphics Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+/ {
+	soc {
+		aips-bus@02000000 {
+			gpio5: gpio@020ac000 {
+				compatible = "fsl,imx6q-gpio", "fsl,imx35-gpio";
+				reg = <0x020ac000 0x4000>;
+				interrupts = <0 74 IRQ_TYPE_LEVEL_HIGH>,
+					     <0 75 IRQ_TYPE_LEVEL_HIGH>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+
+				weim_nor {
+					 gpio-hog;
+					 gpios = <4 0>;
+					 output-low;
+					 line-name = "weim-nor-gpio";
+				};
+			};
+		};
+	};
+};
+
+&weim {
+	status = "okay";
+};
+
+&i2c3 {
+	status = "disabled";
+};
+
+&uart3 {
+	status = "disabled";
+};
+
+&ecspi1 {
+	status = "disabled";
+};
+
+&usdhc3 {
+	status = "disabled";
+};
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] i.MX6-SabreAuto: DTS: use gpio-hog to enable WEIM-NOR at boot

[Sascha Hauer]

On Sun, Apr 19, 2015 at 02:02:23PM -0700, alison-hh6fLRYtCEIS+FvcfC7Uqw@public.gmane.org wrote:
> From: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> 
> Create an imx6-sabreauto-weim-nor.dtsi file whose inclusion in
> a DTS file sets GPIO5 to the level at boot that the WEIM-NOR
> device requires.  The GPIO is set via the gpio-hogging mechanism.
> 
> Signed-off-by: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> ---
>  arch/arm/boot/dts/imx6-sabreauto-weim-nor.dtsi | 56 ++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx6-sabreauto-weim-nor.dtsi
> 
> diff --git a/arch/arm/boot/dts/imx6-sabreauto-weim-nor.dtsi b/arch/arm/boot/dts/imx6-sabreauto-weim-nor.dtsi
> new file mode 100644
> index 0000000..940a908
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6-sabreauto-weim-nor.dtsi
> @@ -0,0 +1,56 @@
> +/*
> + * Copyright 2012 Freescale Semiconductor, Inc.
> + * Copyright 2011 Linaro Ltd.
> + * Copyright (c) 2015 Mentor Graphics Inc.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +/ {
> +	soc {
> +		aips-bus@02000000 {
> +			gpio5: gpio@020ac000 {
> +				compatible = "fsl,imx6q-gpio", "fsl,imx35-gpio";
> +				reg = <0x020ac000 0x4000>;
> +				interrupts = <0 74 IRQ_TYPE_LEVEL_HIGH>,
> +					     <0 75 IRQ_TYPE_LEVEL_HIGH>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;

Why do you repeat all properties already present in imx6qdl.dtsi here?

Also use &pgio5 like done in the nodes below.

> +
> +				weim_nor {
> +					 gpio-hog;
> +					 gpios = <4 0>;
> +					 output-low;
> +					 line-name = "weim-nor-gpio";
> +				};
> +			};
> +		};
> +	};
> +};
> +
> +&weim {
> +	status = "okay";
> +};
> +
> +&i2c3 {
> +	status = "disabled";
> +};
> +
> +&uart3 {
> +	status = "disabled";
> +};

i2c3 and uart3 are not enabled for this board, why do you have to
disable them here?

> +
> +&ecspi1 {
> +	status = "disabled";
> +};
> +
> +&usdhc3 {
> +	status = "disabled";
> +};

usdhc3 uses other pins than the weim controller. Why do you have to
disable it?

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 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 0/2] i.MX6-SabreAuto: DTS: use gpio-hog to enable WEIM-NOR at boo

[alison-hh6fLRYtCEIS+FvcfC7Uqw]

From: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>

The parallel NOR on the i.MX6 SabreAuto boards is connected by the
WEIM simple bus.  WEIM needs gpio5 low at boot in order to properly
set the steering of I2C3.

Rather than modifying the GPIO5 node in a new DTSI file, it would be better
to add the weim_nor gpio-hog to imx6qdl.dtsi and disable it by default.
However, gpio-hogs are always enabled, so inclusion of a separate, new
file is necessary.

Changes since version 1:

... Refer to gpio5 by its phandle rather than overriding the node
    completely.

... Change the name of the new file for greater consistency with other
    i.MX6 dtsi files.

Alison Chaiken (1):
  i.MX6-SabreAuto: DTS: use gpio-hog to enable WEIM-NOR at boot

 arch/arm/boot/dts/imx6qdl-sabreauto-weim-nor.dtsi | 43 +++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6qdl-sabreauto-weim-nor.dtsi

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 1/2] i.MX6-SabreAuto: DTS: use gpio-hog to enable WEIM-NOR at boot

[alison-hh6fLRYtCEIS+FvcfC7Uqw]

From: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>

Create an imx6qdl-sabreauto-weim-nor.dtsi file whose inclusion in
a DTS file sets GPIO5 to the level at boot that the WEIM-NOR
device requires.  The GPIO is set via the gpio-hogging mechanism.
Devices whose pinmux needs conflict with those of NOR are disabled.

Signed-off-by: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/imx6qdl-sabreauto-weim-nor.dtsi | 43 +++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6qdl-sabreauto-weim-nor.dtsi

diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto-weim-nor.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto-weim-nor.dtsi
new file mode 100644
index 0000000..a126335
--- /dev/null
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto-weim-nor.dtsi
@@ -0,0 +1,43 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ * Copyright 2011 Linaro Ltd.
+ * Copyright (c) 2015 Mentor Graphics Inc.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+&gpio5 {
+	weim_nor {
+		gpio-hog;
+		gpios = <4 0>;
+		output-low;
+		line-name = "weim-nor-gpio";
+	};
+};
+
+&weim {
+	status = "okay";
+};
+
+/* Disable devices that have pinmux conflicts with WEIM */
+
+&i2c3 {
+	status = "disabled";
+};
+
+&uart3 {
+	status = "disabled";
+};
+
+&ecspi1 {
+	status = "disabled";
+};
+
+&usdhc3 {
+	status = "disabled";
+};
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] i.MX6-SabreAuto: DTS: use gpio-hog to enable WEIM-NOR at boot

[Sascha Hauer]

On Thu, Apr 23, 2015 at 08:31:27AM +0200, Sascha Hauer wrote:
> On Sun, Apr 19, 2015 at 02:02:23PM -0700, alison-hh6fLRYtCEIS+FvcfC7Uqw@public.gmane.org wrote:
> > +&i2c3 {
> > +	status = "disabled";
> > +};
> > +
> > +&uart3 {
> > +	status = "disabled";
> > +};
> 
> i2c3 and uart3 are not enabled for this board, why do you have to
> disable them here?
> 
> > +
> > +&ecspi1 {
> > +	status = "disabled";
> > +};
> > +
> > +&usdhc3 {
> > +	status = "disabled";
> > +};
> 
> usdhc3 uses other pins than the weim controller. Why do you have to
> disable it?


Please read the whole mail.

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 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 1/2] i.MX6-SabreAuto: DTS: use gpio-hog to enable WEIM-NOR at boot

[Shawn Guo]

On Fri, Apr 24, 2015 at 01:00:20AM -0700, alison-hh6fLRYtCEIS+FvcfC7Uqw@public.gmane.org wrote:
> From: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> 
> Create an imx6qdl-sabreauto-weim-nor.dtsi file whose inclusion in
> a DTS file sets GPIO5 to the level at boot that the WEIM-NOR
> device requires.  The GPIO is set via the gpio-hogging mechanism.
> Devices whose pinmux needs conflict with those of NOR are disabled.
> 
> Signed-off-by: Alison Chaiken <alison_chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>

- The patch numbering is confusing.  It says this 1/2, but I never saw
  patch 2/2.

- The patch is prefixed in a way different than what we usually do -
  'ARM: dts: ...'

- The commit log is vague.  GPIO5 is not a GPIO pin but a GPIO port.

- The patch is incomplete if I understand WEIM NOR on SabreAuto
  correctly. Not only EIM_D18 but also EIM_D30 needs to be steered,
  right?  And how are the steering pins EIMD18_I2C3_STEER and
  EIMD30_BTUART3_STEER being configured as GPIO in pinctrl?

Shawn

> ---
>  arch/arm/boot/dts/imx6qdl-sabreauto-weim-nor.dtsi | 43 +++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx6qdl-sabreauto-weim-nor.dtsi
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto-weim-nor.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto-weim-nor.dtsi
> new file mode 100644
> index 0000000..a126335
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6qdl-sabreauto-weim-nor.dtsi
> @@ -0,0 +1,43 @@
> +/*
> + * Copyright 2012 Freescale Semiconductor, Inc.
> + * Copyright 2011 Linaro Ltd.
> + * Copyright (c) 2015 Mentor Graphics Inc.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +&gpio5 {
> +	weim_nor {
> +		gpio-hog;
> +		gpios = <4 0>;
> +		output-low;
> +		line-name = "weim-nor-gpio";
> +	};
> +};
> +
> +&weim {
> +	status = "okay";
> +};
> +
> +/* Disable devices that have pinmux conflicts with WEIM */
> +
> +&i2c3 {
> +	status = "disabled";
> +};
> +
> +&uart3 {
> +	status = "disabled";
> +};
> +
> +&ecspi1 {
> +	status = "disabled";
> +};
> +
> +&usdhc3 {
> +	status = "disabled";
> +};
> -- 
> 2.1.4
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html