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

[Andrey Smirnov <andrew.smirnov@gmail.com>]

On Sun, Jan 29, 2017 at 11:05 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 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);
>         }
> }

Doh, I completely missed the fact that I can avoid declaring a
pointless custom type if I do this. Good idea, will fix in v2.

Thanks,
Andrey

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