On Tue, Oct 08, 2024 at 03:34:25PM +0800, Chen-Yu Tsai wrote: > Add GPIO support to the simple helpers for the I2C OF component prober. > Components that the prober intends to probe likely require their > regulator supplies be enabled, and GPIOs be toggled to enable them or > bring them out of reset before they will respond to probe attempts. > Regulator supplies were handled in the previous patch. > > The assumption is that the same class of components to be probed are > always connected in the same fashion with the same regulator supply > and GPIO. The names may vary due to binding differences, but the > physical layout does not change. > > This supports at most one GPIO pin. The user must specify the GPIO name, > the polarity, and the amount of time to wait after the GPIO is toggled. > Devices with more than one GPIO pin likely require specific power > sequencing beyond what generic code can easily support. ... > +static int i2c_of_probe_simple_get_gpiod(struct device *dev, struct device_node *node, > + struct i2c_of_probe_simple_ctx *ctx) > +{ > + struct fwnode_handle *fwnode = of_fwnode_handle(node); > + struct gpio_desc *gpiod; > + const char *con_id; > + > + /* NULL signals no GPIO needed */ > + if (!ctx->opts->gpio_name) > + return 0; > + > + /* An empty string signals an unnamed GPIO */ > + if (!ctx->opts->gpio_name[0]) > + con_id = NULL; > + else > + con_id = ctx->opts->gpio_name; Can it use positive conditional? if (ctx->opts->gpio_name[0]) con_id = ctx->opts->gpio_name; else con_id = NULL; > + gpiod = fwnode_gpiod_get_index(fwnode, con_id, 0, GPIOD_ASIS, "i2c-of-prober"); > + if (IS_ERR(gpiod)) > + return PTR_ERR(gpiod); > + > + ctx->gpiod = gpiod; > + > + return 0; > +} ... > +static void i2c_of_probe_simple_disable_gpio(struct device *dev, struct i2c_of_probe_simple_ctx *ctx) > +{ > + if (!ctx->gpiod) > + return; Do you need this check for the future patches? > + /* Ignore error if GPIO is not in output direction */ > + gpiod_set_value(ctx->gpiod, !ctx->opts->gpio_assert_to_enable); > +} ... > struct regulator; > +struct gpio_desc; Ordered? -- With Best Regards, Andy Shevchenko