Re: [PATCH v8 6/8] i2c: of-prober: Add GPIO support to simple helpers

[Chen-Yu Tsai]

On Wed, Oct 16, 2024 at 2:00 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
>
> Hi,
>
> On Tue, Oct 8, 2024 at 12:35 AM Chen-Yu Tsai <wenst@xxxxxxxxxxxx> wrote:
> >
> > +static void i2c_of_probe_simple_disable_gpio(struct device *dev, struct i2c_of_probe_simple_ctx *ctx)
> > +{
> > +       if (!ctx->gpiod)
> > +               return;
> > +
> > +       /* Ignore error if GPIO is not in output direction */
> > +       gpiod_set_value(ctx->gpiod, !ctx->opts->gpio_assert_to_enable);
>
> I'm not sure I understand the comment. Does disable() ever get called
> when set() wasn't called beforehand? How could it not be in output
> direction?

You're right. I think it was carried over (in my mind) from when it was
still four callbacks.

> >  void i2c_of_probe_simple_cleanup(struct device *dev, void *data)
> >  {
> >         struct i2c_of_probe_simple_ctx *ctx = data;
> >
> > +       /* GPIO operations here are no-ops if a component was found and enabled. */
>
> Instead of the above, maybe:
>
> GPIO operations here are no-ops if i2c_of_probe_simple_cleanup_early()
> was called.

Makes sense. Will change.

> Just commenting nits, so:
>
> Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>

I assume this stands even after Andy's suggestion to drop the gpiod check
is applied.

Thanks
ChenYu