| Message ID | 20241030072229.1013235-6-wenst@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | platform/chrome: Introduce DT hardware prober | expand |
On 24-10-30 15:22, 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. > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > Reviewed-by: Douglas Anderson <dianders@chromium.org> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > diff --git a/include/linux/i2c-of-prober.h b/include/linux/i2c-of-prober.h > index df95aa6ad90e..a232f3cbb252 100644 > --- a/include/linux/i2c-of-prober.h > +++ b/include/linux/i2c-of-prober.h > @@ -85,6 +85,7 @@ int i2c_of_probe_component(struct device *dev, const struct i2c_of_probe_cfg *cf > * > * The following helpers are provided: > * * i2c_of_probe_simple_enable() > + * * i2c_of_probe_simple_cleanup_early() > * * i2c_of_probe_simple_cleanup() > */ > > @@ -92,14 +93,31 @@ int i2c_of_probe_component(struct device *dev, const struct i2c_of_probe_cfg *cf > * struct i2c_of_probe_simple_opts - Options for simple I2C component prober callbacks > * @res_node_compatible: Compatible string of device node to retrieve resources from. > * @supply_name: Name of regulator supply. > + * @gpio_name: Name of GPIO. NULL if no GPIO line is used. Empty string ("") if GPIO > + * line is unnamed. > * @post_power_on_delay_ms: Delay after regulators are powered on. Passed to msleep(). > + * @post_gpio_config_delay_ms: Delay after GPIO is configured. Passed to msleep(). > + * @gpio_assert_to_enable: %true if GPIO should be asserted, i.e. set to logical high, > + * to enable the component. > + * > + * This describes power sequences common for the class of components supported by the > + * simple component prober: > + * * @gpio_name is configured to the non-active setting according to @gpio_assert_to_enable. > + * * @supply_name regulator supply is enabled. > + * * Wait for @post_power_on_delay_ms to pass. > + * * @gpio_name is configured to the active setting according to @gpio_assert_to_enable. > + * * Wait for @post_gpio_config_delay_ms to pass. > */ > struct i2c_of_probe_simple_opts { > const char *res_node_compatible; > const char *supply_name; > + const char *gpio_name; > unsigned int post_power_on_delay_ms; > + unsigned int post_gpio_config_delay_ms; > + bool gpio_assert_to_enable; This header is missing '#include <linux/types.h>' for bool type. Otherwise compilation may fail with following error: ``` ./include/linux/i2c-of-prober.h:117:9: error: unknown type name ‘bool’ 117 | bool gpio_assert_to_enable; | ^~~~ ./include/linux/i2c-of-prober.h:12:1: note: ‘bool’ is defined in header ‘<stdbool.h>’; this is probably fixable by adding ‘#include <stdbool.h>’ 11 | #include <linux/kconfig.h> +++ |+#include <stdbool.h> 12 | ``` > }; > > +struct gpio_desc; > struct regulator; > > struct i2c_of_probe_simple_ctx { > @@ -107,9 +125,11 @@ struct i2c_of_probe_simple_ctx { > const struct i2c_of_probe_simple_opts *opts; > /* private: internal fields for helpers. */ > struct regulator *supply; > + struct gpio_desc *gpiod; > }; > > int i2c_of_probe_simple_enable(struct device *dev, struct device_node *bus_node, void *data); > +void i2c_of_probe_simple_cleanup_early(struct device *dev, void *data); > void i2c_of_probe_simple_cleanup(struct device *dev, void *data); > > extern struct i2c_of_probe_ops i2c_of_probe_simple_ops; > -- > 2.47.0.163.g1226f6d8fa-goog >
On Wed, Nov 6, 2024 at 12:14 AM Andrey Skvortsov <andrej.skvortzov@gmail.com> wrote: > > On 24-10-30 15:22, 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. > > > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > > Reviewed-by: Douglas Anderson <dianders@chromium.org> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > --- > > > diff --git a/include/linux/i2c-of-prober.h b/include/linux/i2c-of-prober.h > > index df95aa6ad90e..a232f3cbb252 100644 > > --- a/include/linux/i2c-of-prober.h > > +++ b/include/linux/i2c-of-prober.h > > @@ -85,6 +85,7 @@ int i2c_of_probe_component(struct device *dev, const struct i2c_of_probe_cfg *cf > > * > > * The following helpers are provided: > > * * i2c_of_probe_simple_enable() > > + * * i2c_of_probe_simple_cleanup_early() > > * * i2c_of_probe_simple_cleanup() > > */ > > > > @@ -92,14 +93,31 @@ int i2c_of_probe_component(struct device *dev, const struct i2c_of_probe_cfg *cf > > * struct i2c_of_probe_simple_opts - Options for simple I2C component prober callbacks > > * @res_node_compatible: Compatible string of device node to retrieve resources from. > > * @supply_name: Name of regulator supply. > > + * @gpio_name: Name of GPIO. NULL if no GPIO line is used. Empty string ("") if GPIO > > + * line is unnamed. > > * @post_power_on_delay_ms: Delay after regulators are powered on. Passed to msleep(). > > + * @post_gpio_config_delay_ms: Delay after GPIO is configured. Passed to msleep(). > > + * @gpio_assert_to_enable: %true if GPIO should be asserted, i.e. set to logical high, > > + * to enable the component. > > + * > > + * This describes power sequences common for the class of components supported by the > > + * simple component prober: > > + * * @gpio_name is configured to the non-active setting according to @gpio_assert_to_enable. > > + * * @supply_name regulator supply is enabled. > > + * * Wait for @post_power_on_delay_ms to pass. > > + * * @gpio_name is configured to the active setting according to @gpio_assert_to_enable. > > + * * Wait for @post_gpio_config_delay_ms to pass. > > */ > > struct i2c_of_probe_simple_opts { > > const char *res_node_compatible; > > const char *supply_name; > > + const char *gpio_name; > > unsigned int post_power_on_delay_ms; > > + unsigned int post_gpio_config_delay_ms; > > + bool gpio_assert_to_enable; > > This header is missing '#include <linux/types.h>' for bool > type. Otherwise compilation may fail with following error: > > ``` > ./include/linux/i2c-of-prober.h:117:9: error: unknown type name ‘bool’ > 117 | bool gpio_assert_to_enable; > | ^~~~ > ./include/linux/i2c-of-prober.h:12:1: note: ‘bool’ is defined in header ‘<stdbool.h>’; this is probably fixable by adding ‘#include <stdbool.h>’ > 11 | #include <linux/kconfig.h> > +++ |+#include <stdbool.h> > 12 | > ``` Thanks for catching this. I'll send an update. > > }; > > > > +struct gpio_desc; > > struct regulator; > > > > struct i2c_of_probe_simple_ctx { > > @@ -107,9 +125,11 @@ struct i2c_of_probe_simple_ctx { > > const struct i2c_of_probe_simple_opts *opts; > > /* private: internal fields for helpers. */ > > struct regulator *supply; > > + struct gpio_desc *gpiod; > > }; > > > > int i2c_of_probe_simple_enable(struct device *dev, struct device_node *bus_node, void *data); > > +void i2c_of_probe_simple_cleanup_early(struct device *dev, void *data); > > void i2c_of_probe_simple_cleanup(struct device *dev, void *data); > > > > extern struct i2c_of_probe_ops i2c_of_probe_simple_ops; > > -- > > 2.47.0.163.g1226f6d8fa-goog > > > > -- > Best regards, > Andrey Skvortsov
diff --git a/drivers/i2c/i2c-core-of-prober.c b/drivers/i2c/i2c-core-of-prober.c index 76df75c51cae..b9ca785f8b17 100644 --- a/drivers/i2c/i2c-core-of-prober.c +++ b/drivers/i2c/i2c-core-of-prober.c @@ -10,6 +10,7 @@ #include <linux/device.h> #include <linux/dev_printk.h> #include <linux/err.h> +#include <linux/gpio/consumer.h> #include <linux/i2c.h> #include <linux/i2c-of-prober.h> #include <linux/module.h> @@ -31,7 +32,6 @@ * address responds. * * TODO: - * - Support handling common GPIOs. * - Support I2C muxes */ @@ -246,6 +246,62 @@ static void i2c_of_probe_simple_disable_regulator(struct device *dev, struct i2c regulator_disable(ctx->supply); } +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; + + 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_put_gpiod(struct i2c_of_probe_simple_ctx *ctx) +{ + gpiod_put(ctx->gpiod); + ctx->gpiod = NULL; +} + +static int i2c_of_probe_simple_set_gpio(struct device *dev, struct i2c_of_probe_simple_ctx *ctx) +{ + int ret; + + if (!ctx->gpiod) + return 0; + + dev_dbg(dev, "Configuring GPIO\n"); + + ret = gpiod_direction_output(ctx->gpiod, ctx->opts->gpio_assert_to_enable); + if (ret) + return ret; + + if (ctx->opts->post_gpio_config_delay_ms) + msleep(ctx->opts->post_gpio_config_delay_ms); + + return 0; +} + +static void i2c_of_probe_simple_disable_gpio(struct device *dev, struct i2c_of_probe_simple_ctx *ctx) +{ + gpiod_set_value(ctx->gpiod, !ctx->opts->gpio_assert_to_enable); +} + /** * i2c_of_probe_simple_enable - Simple helper for I2C OF prober to get and enable resources * @dev: Pointer to the &struct device of the caller, only used for dev_printk() messages @@ -253,7 +309,11 @@ static void i2c_of_probe_simple_disable_regulator(struct device *dev, struct i2c * @data: Pointer to &struct i2c_of_probe_simple_ctx helper context. * * If &i2c_of_probe_simple_opts->supply_name is given, request the named regulator supply. + * If &i2c_of_probe_simple_opts->gpio_name is given, request the named GPIO. Or if it is + * the empty string, request the unnamed GPIO. * If a regulator supply was found, enable that regulator. + * If a GPIO line was found, configure the GPIO line to output and set value + * according to given options. * * Return: %0 on success or no-op, or a negative error number on failure. */ @@ -282,12 +342,24 @@ int i2c_of_probe_simple_enable(struct device *dev, struct device_node *bus_node, if (ret) goto out_put_node; - ret = i2c_of_probe_simple_enable_regulator(dev, ctx); + ret = i2c_of_probe_simple_get_gpiod(dev, node, ctx); if (ret) goto out_put_supply; + ret = i2c_of_probe_simple_enable_regulator(dev, ctx); + if (ret) + goto out_put_gpiod; + + ret = i2c_of_probe_simple_set_gpio(dev, ctx); + if (ret) + goto out_disable_regulator; + return 0; +out_disable_regulator: + i2c_of_probe_simple_disable_regulator(dev, ctx); +out_put_gpiod: + i2c_of_probe_simple_put_gpiod(ctx); out_put_supply: i2c_of_probe_simple_put_supply(ctx); out_put_node: @@ -296,17 +368,40 @@ int i2c_of_probe_simple_enable(struct device *dev, struct device_node *bus_node, } EXPORT_SYMBOL_NS_GPL(i2c_of_probe_simple_enable, I2C_OF_PROBER); +/** + * i2c_of_probe_simple_cleanup_early - \ + * Simple helper for I2C OF prober to release GPIOs before component is enabled + * @dev: Pointer to the &struct device of the caller; unused. + * @data: Pointer to &struct i2c_of_probe_simple_ctx helper context. + * + * GPIO descriptors are exclusive and have to be released before the + * actual driver probes so that the latter can acquire them. + */ +void i2c_of_probe_simple_cleanup_early(struct device *dev, void *data) +{ + struct i2c_of_probe_simple_ctx *ctx = data; + + i2c_of_probe_simple_put_gpiod(ctx); +} +EXPORT_SYMBOL_NS_GPL(i2c_of_probe_simple_cleanup_early, I2C_OF_PROBER); + /** * i2c_of_probe_simple_cleanup - Clean up and release resources for I2C OF prober simple helpers * @dev: Pointer to the &struct device of the caller, only used for dev_printk() messages * @data: Pointer to &struct i2c_of_probe_simple_ctx helper context. * + * * If a GPIO line was found and not yet released, set its value to the opposite of that + * set in i2c_of_probe_simple_enable() and release it. * * If a regulator supply was found, disable that regulator and release it. */ 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 i2c_of_probe_simple_cleanup_early was called. */ + i2c_of_probe_simple_disable_gpio(dev, ctx); + i2c_of_probe_simple_put_gpiod(ctx); + i2c_of_probe_simple_disable_regulator(dev, ctx); i2c_of_probe_simple_put_supply(ctx); } @@ -314,6 +409,7 @@ EXPORT_SYMBOL_NS_GPL(i2c_of_probe_simple_cleanup, I2C_OF_PROBER); struct i2c_of_probe_ops i2c_of_probe_simple_ops = { .enable = i2c_of_probe_simple_enable, + .cleanup_early = i2c_of_probe_simple_cleanup_early, .cleanup = i2c_of_probe_simple_cleanup, }; EXPORT_SYMBOL_NS_GPL(i2c_of_probe_simple_ops, I2C_OF_PROBER); diff --git a/include/linux/i2c-of-prober.h b/include/linux/i2c-of-prober.h index df95aa6ad90e..a232f3cbb252 100644 --- a/include/linux/i2c-of-prober.h +++ b/include/linux/i2c-of-prober.h @@ -85,6 +85,7 @@ int i2c_of_probe_component(struct device *dev, const struct i2c_of_probe_cfg *cf * * The following helpers are provided: * * i2c_of_probe_simple_enable() + * * i2c_of_probe_simple_cleanup_early() * * i2c_of_probe_simple_cleanup() */ @@ -92,14 +93,31 @@ int i2c_of_probe_component(struct device *dev, const struct i2c_of_probe_cfg *cf * struct i2c_of_probe_simple_opts - Options for simple I2C component prober callbacks * @res_node_compatible: Compatible string of device node to retrieve resources from. * @supply_name: Name of regulator supply. + * @gpio_name: Name of GPIO. NULL if no GPIO line is used. Empty string ("") if GPIO + * line is unnamed. * @post_power_on_delay_ms: Delay after regulators are powered on. Passed to msleep(). + * @post_gpio_config_delay_ms: Delay after GPIO is configured. Passed to msleep(). + * @gpio_assert_to_enable: %true if GPIO should be asserted, i.e. set to logical high, + * to enable the component. + * + * This describes power sequences common for the class of components supported by the + * simple component prober: + * * @gpio_name is configured to the non-active setting according to @gpio_assert_to_enable. + * * @supply_name regulator supply is enabled. + * * Wait for @post_power_on_delay_ms to pass. + * * @gpio_name is configured to the active setting according to @gpio_assert_to_enable. + * * Wait for @post_gpio_config_delay_ms to pass. */ struct i2c_of_probe_simple_opts { const char *res_node_compatible; const char *supply_name; + const char *gpio_name; unsigned int post_power_on_delay_ms; + unsigned int post_gpio_config_delay_ms; + bool gpio_assert_to_enable; }; +struct gpio_desc; struct regulator; struct i2c_of_probe_simple_ctx { @@ -107,9 +125,11 @@ struct i2c_of_probe_simple_ctx { const struct i2c_of_probe_simple_opts *opts; /* private: internal fields for helpers. */ struct regulator *supply; + struct gpio_desc *gpiod; }; int i2c_of_probe_simple_enable(struct device *dev, struct device_node *bus_node, void *data); +void i2c_of_probe_simple_cleanup_early(struct device *dev, void *data); void i2c_of_probe_simple_cleanup(struct device *dev, void *data); extern struct i2c_of_probe_ops i2c_of_probe_simple_ops;