| Message ID | 1469693621-17851-2-git-send-email-preid@electromag.com.au |
|---|---|
| State | New |
| Headers | show |
On Thu, Jul 28, 2016 at 10:13 AM, Phil Reid <preid@electromag.com.au> wrote: > Some i2c gpio devices are connected to a switchale power supply > which needs to be enabled prior to probing the device. This patch > allows the drive to enable the devices vcc regulator prior to probing. > > Signed-off-by: Phil Reid <preid@electromag.com.au> Overall good, I would change the subject to just "add a regulator" and not use _get_optional. The regulators should be added to *all* devices that have VDD/VDDIO etc lines. Whether the platform will supply them with a real backing regulator or just use a dummy or even stubs (of CONFIG_REGULATOR) is not set) is no concern of the individual driver. Just grab them and handle the errors, like you do. > + reg = devm_regulator_get_optional(&client->dev, "vcc"); So use devm_regulator_get() > + if (IS_ERR(reg)) { > + ret = PTR_ERR(reg); > + if (ret != -ENODEV) { > + if (ret != -EPROBE_DEFER) > + dev_err(&client->dev, "reg get err: %d\n", ret); > + return ret; > + } Just bail out if IS_ERR(), one of the possible errors would be deferral but -ENODEV need no special handling. > + } else { > + ret = regulator_enable(reg); > + if (ret) { > + dev_err(&client->dev, "reg en err: %d\n", ret); > + return ret; > + } > + } Then just de-indent the else clause. We should always get something that we can call regulator_enable() on, even if it is a dummy or a stub. There is another problem: you do not call regulator_disable() anywhere. Call it in the remove() function, and on the error path in probe following this call, possibly with a goto-try/catch construction. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c index 5e3be32..12d8e91 100644 --- a/drivers/gpio/gpio-pca953x.c +++ b/drivers/gpio/gpio-pca953x.c @@ -21,6 +21,7 @@ #include <asm/unaligned.h> #include <linux/of_platform.h> #include <linux/acpi.h> +#include <linux/regulator/consumer.h> #define PCA953X_INPUT 0 #define PCA953X_OUTPUT 1 @@ -744,6 +745,7 @@ static int pca953x_probe(struct i2c_client *client, int irq_base = 0; int ret; u32 invert = 0; + struct regulator *reg; chip = devm_kzalloc(&client->dev, sizeof(struct pca953x_chip), GFP_KERNEL); @@ -763,6 +765,22 @@ static int pca953x_probe(struct i2c_client *client, chip->client = client; + reg = devm_regulator_get_optional(&client->dev, "vcc"); + if (IS_ERR(reg)) { + ret = PTR_ERR(reg); + if (ret != -ENODEV) { + if (ret != -EPROBE_DEFER) + dev_err(&client->dev, "reg get err: %d\n", ret); + return ret; + } + } else { + ret = regulator_enable(reg); + if (ret) { + dev_err(&client->dev, "reg en err: %d\n", ret); + return ret; + } + } + if (id) { chip->driver_data = id->driver_data; } else {
Some i2c gpio devices are connected to a switchale power supply which needs to be enabled prior to probing the device. This patch allows the drive to enable the devices vcc regulator prior to probing. Signed-off-by: Phil Reid <preid@electromag.com.au> --- drivers/gpio/gpio-pca953x.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)