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
