Hi Mike, On Mon, Jul 03, 2023 at 10:45:36AM +0200, Mike Looijmans wrote: > Add power supply regulator support to the exc3000 devices. > > Signed-off-by: Mike Looijmans <mike.looijmans@xxxxxxxx> > > --- > > drivers/input/touchscreen/exc3000.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/input/touchscreen/exc3000.c b/drivers/input/touchscreen/exc3000.c > index 4af4c1e5d0da..3e50af8a4a2d 100644 > --- a/drivers/input/touchscreen/exc3000.c > +++ b/drivers/input/touchscreen/exc3000.c > @@ -18,6 +18,7 @@ > #include <linux/interrupt.h> > #include <linux/module.h> > #include <linux/of.h> > +#include <linux/regulator/consumer.h> > #include <linux/sizes.h> > #include <linux/timer.h> > #include <asm/unaligned.h> > @@ -360,6 +361,12 @@ static int exc3000_probe(struct i2c_client *client) > if (IS_ERR(data->reset)) > return PTR_ERR(data->reset); > > + /* For proper reset sequence, enable power while reset asserted */ > + error = devm_regulator_get_enable_optional(&client->dev, "vdd"); > + if (error && error != -ENODEV) > + dev_err_probe(&client->dev, error, > + "failed to request vdd regulator\n"); If there is a regulator described in the firmware we should not continue with initializing the device if we fail to grab/enable it. Think about what happens if you get -EPROBE_DEFER here. You should return here. Also, why are you using the _optional() variant? VDD is not an optional for the controller. regulator_get_optional() is needed when you need to alter the behavior of the device/driver depending on the presence of an optional supply, whereas here it should work fine with a sub supply that will be created if you simply call devm_regulator_get_enable() and there is not regulator mentioned in DT/ACPI for the board. Thanks. -- Dmitry
