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? > 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. Just commenting nits, so: Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>