Re: [PATCH] i.MX: vf610: Add support for ZII VF610 Dev Family

[Sascha Hauer <s.hauer@pengutronix.de>]

On Thu, Jan 26, 2017 at 02:38:12PM -0800, Andrey Smirnov wrote:
> On Tue, Jan 24, 2017 at 12:09 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > Hi Andrey,
> >
> > On Sun, Jan 22, 2017 at 09:57:34PM -0800, Andrey Smirnov wrote:
> >> Add support for ZII VF610 Dev based designs such as:
> >>
> >>     - VF610 Dev, revision B
> >>     - VF610 Dev, revision C
> >>     - CFU1, revision A
> >>     - SPU3, revision A
> >>     - SCU4 AIB, revision C
> >>
> >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> >> ---
> >>
> >> Sascha, this patch is rebased on 'next' instead of 'master' so that
> >> you won't have to resolve conflicts with RDU2 patches in 'next'. Let
> >> me know if you'd rather have it rebased on 'master'.
> >
> > It's fine to base on next in this case.
> >
> >> +struct named_signal {
> >> +     unsigned int gpio;
> >> +     const char *name;
> >> +     int value;
> >> +};
> >> +
> >> +static int expose_signals(const struct named_signal *signals,
> >> +                           size_t signal_num)
> >> +{
> >> +     int ret, i;
> >> +
> >> +     for (i = 0; i < signal_num; i++) {
> >> +             const struct named_signal *signal = &signals[i];
> >> +
> >> +             if (signal->value < 0)
> >> +                     ret = gpio_direction_input(signal->gpio);
> >> +             else
> >> +                     ret = gpio_direction_output(signal->gpio, signal->value);
> >> +
> >> +             if (ret) {
> >> +                     pr_err("Failed to configure \"%s\"\n", signal->name);
> >> +                     return ret;
> >> +             }
> >
> > This looks like gpio_request_array(). Could you use this instead?
> 
> Almost. Unfortunately, gpio_request_array doesn't do much with "label'
> portion of a descriptor, except to use it when displaying information
> about GPIOs.
> 
> What I am doing here as well is exposing those GPIO in a very
> primitive way by calling
> 
> export_env_ull(signal->name, signal->gpio);

What I meant was something like:

static int expose_signals(const struct gpio *array, size_t num)
{
	int ret;

	ret = gpio_request_array(gpios);
	if (ret)
		returen ret;

	for (i = 0; i < num; i++) {
		const struct gpio *gpio = &array[i];
		export_env_ull(gpio->label, gpio->gpio);
	}
}

> 
> and so it becomes possible to do things like
> 
> gpio_set_value ${soc_sw_rstn} 0
> 
> instead of having to use numeric value to designate desired GPIO. If
> there's a way this can be done better or if there a good change we can
> make to gpio_request_array(), I am more than happy to change this
> code.

I have no good idea how we could change gpio_request_array.
Unfortunately the gpio labels are not unique, so we can't just export
the variables unconditionally. A new gpio_request_array_export()
function maybe could work, but doesn't look too nice either.

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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox