Re: [PATCH 1/3] gpio: pseudo: common helper functions for pseudo gpio devices

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Mon, Feb 17, 2025 at 5:21 PM Koichiro Den <koichiro.den@xxxxxxxxxxxxx> wrote:
>
> On Tue, Feb 18, 2025 at 01:12:17AM GMT, Koichiro Den wrote:
> > On Mon, Feb 17, 2025 at 04:46:30PM GMT, Bartosz Golaszewski wrote:
> > > On Mon, Feb 17, 2025 at 3:28 PM Koichiro Den <koichiro.den@xxxxxxxxxxxxx> wrote:
> > > >
> > > > Both gpio-sim and gpio-virtuser share a mechanism to instantiate a
> > > > platform device and wait synchronously for probe completion.
> > > > With gpio-aggregator adopting the same approach in a later commit for
> > > > its configfs interface, it's time to factor out the common code.
> > > >
> > > > Add gpio-pseudo.[ch] to house helper functions used by all the pseudo
> > > > GPIO device implementations.
> > > >
> > > > No functional change.
> > > >
> > > > Signed-off-by: Koichiro Den <koichiro.den@xxxxxxxxxxxxx>
> > > > ---
> > >
> >
> > Thanks for the review.
> >
> > > Looking at this patch now, I've realized that there is nothing
> > > GPIO-specific here. It's a mechanism for synchronous platform device
> > > probing. I don't think its place is in drivers/gpio/ if we're making
> > > it a set of library functions. Can I suggest moving it to lib/ and
> > > renaming the module as pdev_sync_probe or something else that's
> > > expressive enough to tell users what it does? You can make me the
> > > maintainer of that module if you wish (feel free to add yourself
> > > too!).
> >
> > I had vaguely envisioned that this might eventually contain some
> > GPIO-specific code for some reason, and also it's just a tiny utility to
> > reduce code duplication, which is why I placed it in the neighborhood,
> > drivers/gpio/. However, of course you’re right, there’s nothing
> > GPIO-specific here, so moving it to lib/ makes sense.
> >
> > I'm not really sure if this method for synchronous platform device probing
> > can be broadly accepted as a general solution, but I have no objections to
> > making the change. I'll move it as you suggested and send v2, setting you
> > as its maintainer.
>
> Regarding this series, I feel that it might make discussions smoother if
> you submit it directly. So if you're okay with it, please go ahead. In
> that case, there's even no need to mention me or CC me - I can track it on
> ML :)

I'm not sure I'm following. Why would I submit it myself? You did most
of the work already. If you want the changes to gpio-aggregator
merged, then I think that it's time to refactor this code before we do
that because repeating it three times is just bad programming. I
probably wouldn't have done it otherwise at this point.

The code looks good other than that, just put it under lib/, rename
functions to pdev_sync_probe_init/register/unregister() and send it to
the list as usual. With that it's good to go. Just make sure to
mention to Andrew Morton the need for this to go through the GPIO
tree, I don't think he'll mind.

Bart