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@xxxxxxxxxxxxxxxxxxx> > > > > > > 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@xxxxxxxxxxxxxxxxxxx> > > > Acked-by: Peter Rosin <peda@xxxxxxxxxx> > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > > > 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@xxxxxxxxxx> > > > > > > --- > > > > > > If previous patches are fine, then this commit is independent and could > > > be taken via I2C. > > > > > > Cc: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx> > > > Cc: Bartosz Golaszewski <brgl@xxxxxxxx> > > > Cc: Sean Anderson <sean.anderson@xxxxxxxx> > > > --- > > > 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