Re: [PATCH v4 6/8] mtd: maps: gpio-addr-flash: Convert to gpiod

[Boris Brezillon <boris.brezillon@bootlin.com>]

Hi Ricardo,

On Wed, 3 Oct 2018 17:56:03 +0200
Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote:

> Hi Boris
> On Wed, Oct 3, 2018 at 5:17 PM Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> >
> > On Wed, 3 Oct 2018 17:11:14 +0200
> > Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> >  
> > > Hi Ricardo,
> > >
> > > On Mon,  1 Oct 2018 14:43:49 +0200
> > > Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote:
> > >  
> > > > @@ -248,14 +252,19 @@ static int gpio_flash_probe(struct platform_device *pdev)
> > > >
> > > >     i = 0;
> > > >     do {
> > > > -           if (devm_gpio_request(&pdev->dev, state->gpio_addrs[i],
> > > > -                                 DRIVER_NAME)) {
> > > > +           unsigned int *gpio_id = (unsigned int *)gpios->start;
> > > > +
> > > > +           if (devm_gpio_request_one(&pdev->dev, gpio_id[i], GPIOD_OUT_LOW,
> > > > +                                   DRIVER_NAME)) {
> > > >                     dev_err(&pdev->dev, "failed to request gpio %d\n",
> > > > -                           state->gpio_addrs[i]);
> > > > +                           gpio_id[i]);
> > > >                     return -EBUSY;
> > > >             }
> > > > -           gpio_direction_output(state->gpio_addrs[i], 0);
> > > > -   } while (++i < state->gpio_count);
> > > > +
> > > > +           state->gpios->desc[i] = gpio_to_desc(gpio_id[i]);
> > > > +           if (!state->gpios->desc[i])
> > > > +                   return -EINVAL;
> > > > +   } while (++i < state->gpios->ndescs);  
> > >
> > > Actually, I was thinking about using devm_gpiod_get_array() here and
> > > defining a gpio lookup table in the board file registering the device.
> > > This way, adding support for DT based parsing is transparent.  
> >
> > It's actually easier than I thought since no one is registering such a
> > device, so all you have to do is call devm_gpiod_get_array() and have a
> > struct gpio_descs pointer in struct async_state.  
> 
> That is what I do in patch 8/8 for DT based parsing.

Well, yes, except you do it differently for the !DT case, while I'm
suggesting to use the same patch for both DT and !DT.

> 
> I am am doing the gpio_to_desc and other to maintain compatibility
> with old platform board files (in and out tree).

I do care about keeping in-tree users functional (which is why I
suggested to declare gpio lookup tables in the first place), but
keeping out-of-tree code functional is on a best-effort basis. If it
breaks because we have to rework an internal API then that's not our
fault. Actually, that's one of the reason we push people to upstream
their code => their maintenance burden is greatly reduced once the code
has reached mainline.

> 
> I think is important to maintain that compatibility, but you decide ;)

And my decision is, keep the code as simple as possible even if it
breaks out of tree users.

Regards,

Boris