| Message ID | 20240112163608.528453-6-krzysztof.kozlowski@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | reset: gpio: ASoC: shared GPIO resets | expand |
On Fr, 2024-01-12 at 17:36 +0100, Krzysztof Kozlowski wrote: > From: Chris Packham <chris.packham@alliedtelesis.co.nz> > > Some hardware designs with multiple PCA954x devices use a reset GPIO > connected to all the muxes. Support this configuration by making use of > the reset controller framework which can deal with the shared reset > GPIOs. Fall back to the old GPIO descriptor method if the reset > controller framework is not enabled. > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > Acked-by: Peter Rosin <peda@axentia.se> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Link: https://lore.kernel.org/r/20240108041913.7078-1-chris.packham@alliedtelesis.co.nz > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > If previous patches are fine, then this commit is independent and could > be taken via I2C. > > Cc: Chris Packham <chris.packham@alliedtelesis.co.nz> > Cc: Bartosz Golaszewski <brgl@bgdev.pl> > Cc: Sean Anderson <sean.anderson@seco.com> > --- > drivers/i2c/muxes/i2c-mux-pca954x.c | 46 ++++++++++++++++++++++++----- > 1 file changed, 38 insertions(+), 8 deletions(-) > > diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c > index 2219062104fb..1702e8d49b91 100644 > --- a/drivers/i2c/muxes/i2c-mux-pca954x.c > +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c > @@ -49,6 +49,7 @@ > #include <linux/pm.h> > #include <linux/property.h> > #include <linux/regulator/consumer.h> > +#include <linux/reset.h> > #include <linux/slab.h> > #include <linux/spinlock.h> > #include <dt-bindings/mux/mux.h> > @@ -102,6 +103,9 @@ struct pca954x { > unsigned int irq_mask; > raw_spinlock_t lock; > struct regulator *supply; > + > + struct gpio_desc *reset_gpio; > + struct reset_control *reset_cont; > }; > > /* Provide specs for the MAX735x, PCA954x and PCA984x types we know about */ > @@ -477,6 +481,35 @@ static int pca954x_init(struct i2c_client *client, struct pca954x *data) > return ret; > } > > +static int pca954x_get_reset(struct device *dev, struct pca954x *data) > +{ > + data->reset_cont = devm_reset_control_get_optional_shared(dev, NULL); > + if (IS_ERR(data->reset_cont)) > + return dev_err_probe(dev, PTR_ERR(data->reset_cont), > + "Failed to get reset\n"); > + else if (data->reset_cont) > + return 0; > + > + /* > + * fallback to legacy reset-gpios > + */ devm_reset_control_get_optional_shared() won't return NULL if the "reset-gpios" property is found in the device tree, so the GPIO fallback is dead code. > + data->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(data->reset_gpio)) { > + return dev_err_probe(dev, PTR_ERR(data->reset_gpio), > + "Failed to get reset gpio"); > + } > + > + return 0; > +} > + regards Philipp
On Di, 2024-01-16 at 19:58 +0000, Chris Packham wrote: > On 17/01/24 04:18, Philipp Zabel wrote: > > On Fr, 2024-01-12 at 17:36 +0100, Krzysztof Kozlowski wrote: > > > From: Chris Packham <chris.packham@alliedtelesis.co.nz> > > > > > > Some hardware designs with multiple PCA954x devices use a reset GPIO > > > connected to all the muxes. Support this configuration by making use of > > > the reset controller framework which can deal with the shared reset > > > GPIOs. Fall back to the old GPIO descriptor method if the reset > > > controller framework is not enabled. > > > > > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > > > Acked-by: Peter Rosin <peda@axentia.se> > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > > Link: https://scanmail.trustwave.com/?c=20988&d=8p6m5Tfi2yYJWYV9xYGcYnz7UYxB6WTGTPkmGu7b8A&u=https%3a%2f%2flore%2ekernel%2eorg%2fr%2f20240108041913%2e7078-1-chris%2epackham%40alliedtelesis%2eco%2enz > > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > > > > > --- > > > > > > If previous patches are fine, then this commit is independent and could > > > be taken via I2C. > > > > > > Cc: Chris Packham <chris.packham@alliedtelesis.co.nz> > > > Cc: Bartosz Golaszewski <brgl@bgdev.pl> > > > Cc: Sean Anderson <sean.anderson@seco.com> > > > --- > > > drivers/i2c/muxes/i2c-mux-pca954x.c | 46 ++++++++++++++++++++++++----- > > > 1 file changed, 38 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c > > > index 2219062104fb..1702e8d49b91 100644 > > > --- a/drivers/i2c/muxes/i2c-mux-pca954x.c > > > +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c > > > @@ -49,6 +49,7 @@ > > > #include <linux/pm.h> > > > #include <linux/property.h> > > > #include <linux/regulator/consumer.h> > > > +#include <linux/reset.h> > > > #include <linux/slab.h> > > > #include <linux/spinlock.h> > > > #include <dt-bindings/mux/mux.h> > > > @@ -102,6 +103,9 @@ struct pca954x { > > > unsigned int irq_mask; > > > raw_spinlock_t lock; > > > struct regulator *supply; > > > + > > > + struct gpio_desc *reset_gpio; > > > + struct reset_control *reset_cont; > > > }; > > > > > > /* Provide specs for the MAX735x, PCA954x and PCA984x types we know about */ > > > @@ -477,6 +481,35 @@ static int pca954x_init(struct i2c_client *client, struct pca954x *data) > > > return ret; > > > } > > > > > > +static int pca954x_get_reset(struct device *dev, struct pca954x *data) > > > +{ > > > + data->reset_cont = devm_reset_control_get_optional_shared(dev, NULL); > > > + if (IS_ERR(data->reset_cont)) > > > + return dev_err_probe(dev, PTR_ERR(data->reset_cont), > > > + "Failed to get reset\n"); > > > + else if (data->reset_cont) > > > + return 0; > > > + > > > + /* > > > + * fallback to legacy reset-gpios > > > + */ > > devm_reset_control_get_optional_shared() won't return NULL if the > > "reset-gpios" property is found in the device tree, so the GPIO > > fallback is dead code. > > Hmm, I was attempting to handle the case where CONFIG_RESET_GPIO wasn't > set [...] > [...] it looks like we'd get -EPROBE_DEFER. I could change to check > for that or just remove the GPIO fallback entirely. Any preference? I hadn't considered this. If CONFIG_RESET_GPIO=n, devm_reset_control_get_optional_shared() probably shouldn't return -EPROBE_DEFER. If we change that, the GPIO fallback here can stay as is. The alternative would be to drop the fallback and select RESET_GPIO. Using -EPROBE_DEFER for fallback detection is no good, as there could be a valid probe deferral if reset-gpio is compiled as a module that will be loaded later. regards Philipp
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c index 2219062104fb..1702e8d49b91 100644 --- a/drivers/i2c/muxes/i2c-mux-pca954x.c +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c @@ -49,6 +49,7 @@ #include <linux/pm.h> #include <linux/property.h> #include <linux/regulator/consumer.h> +#include <linux/reset.h> #include <linux/slab.h> #include <linux/spinlock.h> #include <dt-bindings/mux/mux.h> @@ -102,6 +103,9 @@ struct pca954x { unsigned int irq_mask; raw_spinlock_t lock; struct regulator *supply; + + struct gpio_desc *reset_gpio; + struct reset_control *reset_cont; }; /* Provide specs for the MAX735x, PCA954x and PCA984x types we know about */ @@ -477,6 +481,35 @@ static int pca954x_init(struct i2c_client *client, struct pca954x *data) return ret; } +static int pca954x_get_reset(struct device *dev, struct pca954x *data) +{ + data->reset_cont = devm_reset_control_get_optional_shared(dev, NULL); + if (IS_ERR(data->reset_cont)) + return dev_err_probe(dev, PTR_ERR(data->reset_cont), + "Failed to get reset\n"); + else if (data->reset_cont) + return 0; + + /* + * fallback to legacy reset-gpios + */ + data->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(data->reset_gpio)) { + return dev_err_probe(dev, PTR_ERR(data->reset_gpio), + "Failed to get reset gpio"); + } + + return 0; +} + +static void pca954x_reset_deassert(struct pca954x *data) +{ + if (data->reset_cont) + reset_control_deassert(data->reset_cont); + else + gpiod_set_value_cansleep(data->reset_gpio, 0); +} + /* * I2C init/probing/exit functions */ @@ -485,7 +518,6 @@ static int pca954x_probe(struct i2c_client *client) const struct i2c_device_id *id = i2c_client_get_device_id(client); struct i2c_adapter *adap = client->adapter; struct device *dev = &client->dev; - struct gpio_desc *gpio; struct i2c_mux_core *muxc; struct pca954x *data; int num; @@ -513,15 +545,13 @@ static int pca954x_probe(struct i2c_client *client) return dev_err_probe(dev, ret, "Failed to enable vdd supply\n"); - /* Reset the mux if a reset GPIO is specified. */ - gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); - if (IS_ERR(gpio)) { - ret = PTR_ERR(gpio); + ret = pca954x_get_reset(dev, data); + if (ret) goto fail_cleanup; - } - if (gpio) { + + if (data->reset_cont || data->reset_gpio) { udelay(1); - gpiod_set_value_cansleep(gpio, 0); + pca954x_reset_deassert(data); /* Give the chip some time to recover. */ udelay(1); }