On 18/01/24 00:16, Philipp Zabel wrote: > 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=_ban5W3xRzKSimJ9ijTJ74p10otfq8ZONfjVnBr6Fw&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. I did consider adding `select RESET_GPIO` (or maybe just `imply RESET_GPIO`) initially but decided on the fallback as a way of avoiding surprises for existing users. I'll see if anyone else has a different suggestion but assuming nothing else changes I'll work with Krzystof to get an updated patch for this series.
