| Message ID | 20210326015229.141-7-alistair@alistair23.me |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,01/10] dt-bindings: Add Wacom to vendor bindings | expand |
Hi Alistair, On Thu, Mar 25, 2021 at 09:52:27PM -0400, Alistair Francis wrote: > From: Alistair Francis <alistair@alistair22.me> > > Signed-off-by: Alistair Francis <alistair@alistair22.me> > --- > 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? > + > 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
On Tue, Mar 30, 2021 at 6:33 AM Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > Hi Alistair, > > On Thu, Mar 25, 2021 at 09:52:27PM -0400, Alistair Francis wrote: > > From: Alistair Francis <alistair@alistair22.me> > > > > Signed-off-by: Alistair Francis <alistair@alistair22.me> > > --- > > 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
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); + 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); + } + 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;