[PATCH v2 07/12] ARM: imx6q: add config-on-boot gpios

[Richard Zhao]

On Tue, May 22, 2012 at 11:17:12AM +0800, Shawn Guo wrote:
> Apart from my comments below, we really need to know Rob and Grant's
> opinion on this.
Exactly. I cced them to draw their attention.
> 
> On Mon, May 21, 2012 at 05:23:52PM +0800, Richard Zhao wrote:
> > Sometimes, boards have gpios that don't own by any driver or owner
> > by a generic driver that don't like hacks. Such gpios is normally
> > output and need setup once on boot. So I introduce the config-on-boot
> > gpios.
> > 
> > Signed-off-by: Richard Zhao <richard.zhao at freescale.com>
> > Cc: Shawn Guo <shawn.guo at linaro.org>
> > Cc: Rob Herring <rob.herring at calxeda.com>
> > Cc: Grant Likely <grant.likely at secretlab.ca>
> > ---
> >  arch/arm/boot/dts/imx6q-sabrelite.dts |    7 ++++++
> >  arch/arm/mach-imx/mach-imx6q.c        |   35 +++++++++++++++++++++++++++++++++
> >  2 files changed, 42 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
> > index e0ec929..1dd2261 100644
> > --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
> > +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
> > @@ -17,6 +17,13 @@
> >  	model = "Freescale i.MX6 Quad SABRE Lite Board";
> >  	compatible = "fsl,imx6q-sabrelite", "fsl,imx6q";
> >  
> > +	config-on-boot {
> > +		output-gpios = <
> > +				&gpio3 22 0>;	/* vbus reset */
> > +		output-gpio-values = <
> > +				1>;		/* vbus reset */
> > +	};
> > +
> 
> So it looks like something not specific to imx board but generic to
> other boards.  If so, we may need to come up with a generic binding
> document for this.
Yes.
> 
> Is this only used to configure output pin?  Will there be any pin that
> needs to be configured as input but not owned by any driver?  Hopefully
> not.
I didn't see the case yet. Input pin normally has a corresponding driver. 
> 
> >  	memory {
> >  		reg = <0x10000000 0x40000000>;
> >  	};
> > diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> > index b47e98b..577cf19 100644
> > --- a/arch/arm/mach-imx/mach-imx6q.c
> > +++ b/arch/arm/mach-imx/mach-imx6q.c
> > @@ -19,6 +19,7 @@
> >  #include <linux/irqdomain.h>
> >  #include <linux/of.h>
> >  #include <linux/of_address.h>
> > +#include <linux/of_gpio.h>
> >  #include <linux/of_irq.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/pinctrl/machine.h>
> > @@ -113,6 +114,38 @@ static void __init imx6q_sabrelite_init(void)
> >  	imx6q_sabrelite_cko1_setup();
> >  }
> >  
> > +static void __init imx6q_config_on_boot(void)
> 
> Do you intend to make this function non-gpio-config only.  Otherwise,
> we may need to have "gpio" in the function name, as well as the
> "config-on-boot" node name.
config-on-boot is not specific for gpio, but may be for others. For
example, regulators. For now, it only has output gpios.
> 
> Also, if we define the binding as generic one, we may need to find a
> common place to implement the function.
Yes. But it's better be called at machine init after populate devices.
> 
> > +{
> > +	struct device_node *np;
> > +	struct property *pp;
> > +	int cnt, len, i;
> > +	int gpio;
> > +
> > +	np = of_find_node_by_path("/config-on-boot");
> > +	if (!np)
> > +		return;
> > +	cnt = of_gpio_named_count(np, "output-gpios");
> > +	pp = of_find_property(np, "output-gpio-values", &len);
> > +	if (!pp || cnt != len / sizeof(u32)) {
> > +		pr_err("Invalid config-on-boot gpios!\n");
> > +		of_node_put(np);
> > +		return;
> > +	}
> > +	for (i = 0; i < cnt; i++) {
> > +		gpio = of_get_named_gpio(np, "output-gpios", i);
> > +		if (gpio_is_valid(gpio))
> > +			gpio_request_one(gpio, GPIOF_OUT_INIT_HIGH,
> > +					"config-on-boot");
> 
> So all the gpios will use the same name.
Yes.
> 
> And doesn't the reset generally need a pulse or edge signal, e.g. pull
> down for a msec and pull up?
strict reset is too complicated for config-on-boot. It may need delay
some time case by case. I don't have the case yet. But someone may
add it if it really needed, probably with addon properties.

Thanks
Richard
> 
> Regards,
> Shawn
> 
> > +	}
> > +
> > +	of_node_put(np);
> > +}
> > +
> > +static void __init imx6q_post_populate(void)
> > +{
> > +	imx6q_config_on_boot();
> > +}
> > +
> >  static void __init imx6q_init_machine(void)
> >  {
> >  	/*
> > @@ -126,6 +159,8 @@ static void __init imx6q_init_machine(void)
> >  
> >  	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> >  
> > +	imx6q_post_populate();
> > +
> >  	imx6q_pm_init();
> >  }
> >  
> > -- 
> > 1.7.5.4
> > 
> > 
>