On Tue, Mar 30, 2021 at 6:33 AM Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > > Hi Alistair, > > On Thu, Mar 25, 2021 at 09:52:27PM -0400, Alistair Francis wrote: > > From: Alistair Francis <alistair@xxxxxxxxxxxxx> > > > > Signed-off-by: Alistair Francis <alistair@xxxxxxxxxxxxx> > > --- > > v4: > > - Initial commit > > > > drivers/input/touchscreen/wacom_i2c.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/drivers/input/touchscreen/wacom_i2c.c b/drivers/input/touchscreen/wacom_i2c.c > > index 84c7ccb737bd..28004b1180c9 100644 > > --- a/drivers/input/touchscreen/wacom_i2c.c > > +++ b/drivers/input/touchscreen/wacom_i2c.c > > @@ -55,6 +55,7 @@ struct wacom_features { > > struct wacom_i2c { > > struct i2c_client *client; > > struct input_dev *input; > > + struct reset_control *rstc; > > struct touchscreen_properties props; > > u8 data[WACOM_QUERY_SIZE]; > > bool prox; > > @@ -175,6 +176,8 @@ static int wacom_i2c_open(struct input_dev *dev) > > struct wacom_i2c *wac_i2c = input_get_drvdata(dev); > > struct i2c_client *client = wac_i2c->client; > > > > + reset_control_reset(wac_i2c->rstc); > > Why does this device need to be reset on every open compared to doing it > in probe? > > > + > > enable_irq(client->irq); > > > > return 0; > > @@ -193,6 +196,7 @@ static int wacom_i2c_probe(struct i2c_client *client, > > { > > struct wacom_i2c *wac_i2c; > > struct input_dev *input; > > + struct reset_control *rstc; > > struct wacom_features features = { 0 }; > > int error; > > > > @@ -201,6 +205,12 @@ static int wacom_i2c_probe(struct i2c_client *client, > > return -EIO; > > } > > > > + rstc = devm_reset_control_get_optional_exclusive(&client->dev, NULL); > > + if (IS_ERR(rstc)) { > > + dev_err(&client->dev, "Failed to get reset control before init\n"); > > + return PTR_ERR(rstc); > > + } > > I think majority users will have this controller reset line connected to > a GPIO. I briefly looked into reset controller code and I do not see it > supporting this case. How is this device connected on your board? That's a good question. I am going to drop this patch as I'm not convinced it's required. Alistair > > > + > > error = wacom_query_device(client, &features); > > if (error) > > return error; > > @@ -214,6 +224,7 @@ static int wacom_i2c_probe(struct i2c_client *client, > > > > wac_i2c->client = client; > > wac_i2c->input = input; > > + wac_i2c->rstc = rstc; > > > > input->name = "Wacom I2C Digitizer"; > > input->id.bustype = BUS_I2C; > > -- > > 2.31.0 > > > > Thanks. > > -- > Dmitry