2021-10-27 21:32:28

by Nikita Travkin

[permalink] [raw]
Subject: [PATCH 3/6] Input: zinitix - Handle proper supply names

From: Linus Walleij <[email protected]>

The supply names of the Zinitix touchscreen were a bit confused, the new
bindings rectifies this.

To deal with old and new devicetrees, first check if we have "vddo" and in
case that exists assume the old supply names. Else go and look for the new
ones.

We cannot just get the regulators since we would get an OK and a dummy
regulator: we need to check explicitly for the old supply name.

Use struct device *dev as a local variable instead of the I2C client since
the device is what we are actually obtaining the resources from.

Cc: Mark Brown <[email protected]>
Cc: Michael Srba <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Linus Walleij <[email protected]>
[Slightly changed the legacy regulator detection]
Signed-off-by: Nikita Travkin <[email protected]>
---
This patch was previously submitted here:
https://lore.kernel.org/linux-input/[email protected]/

Changes since the original patch:
- Adress the review comments by Dmitry:
Drop explict OF check and use of_find_property()
---
drivers/input/touchscreen/zinitix.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
index 1e70b8d2a8d7..d4e06a88a883 100644
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -252,16 +252,27 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)

static int zinitix_init_regulators(struct bt541_ts_data *bt541)
{
- struct i2c_client *client = bt541->client;
+ struct device *dev = &bt541->client->dev;
int error;

- bt541->supplies[0].supply = "vdd";
- bt541->supplies[1].supply = "vddo";
- error = devm_regulator_bulk_get(&client->dev,
+ /*
+ * Some older device trees have erroneous names for the regulators,
+ * so check if "vddo" is present and in that case use these names
+ * and warn. Else use the proper supply names on the component.
+ */
+ if (of_find_property(dev->of_node, "vddo-supply", NULL)) {
+ bt541->supplies[0].supply = "vdd";
+ bt541->supplies[1].supply = "vddo";
+ } else {
+ /* Else use the proper supply names */
+ bt541->supplies[0].supply = "vcca";
+ bt541->supplies[1].supply = "vdd";
+ }
+ error = devm_regulator_bulk_get(dev,
ARRAY_SIZE(bt541->supplies),
bt541->supplies);
if (error < 0) {
- dev_err(&client->dev, "Failed to get regulators: %d\n", error);
+ dev_err(dev, "Failed to get regulators: %d\n", error);
return error;
}

--
2.30.2


2021-10-27 21:39:11

by Luca Weiss

[permalink] [raw]
Subject: Re: [PATCH 3/6] Input: zinitix - Handle proper supply names

Hi Nikita,

On Mittwoch, 27. Oktober 2021 20:13:47 CEST Nikita Travkin wrote:
> From: Linus Walleij <[email protected]>
>
> The supply names of the Zinitix touchscreen were a bit confused, the new
> bindings rectifies this.
>
> To deal with old and new devicetrees, first check if we have "vddo" and in
> case that exists assume the old supply names. Else go and look for the new
> ones.
>
> We cannot just get the regulators since we would get an OK and a dummy
> regulator: we need to check explicitly for the old supply name.
>
> Use struct device *dev as a local variable instead of the I2C client since
> the device is what we are actually obtaining the resources from.
>
> Cc: Mark Brown <[email protected]>
> Cc: Michael Srba <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Linus Walleij <[email protected]>
> [Slightly changed the legacy regulator detection]
> Signed-off-by: Nikita Travkin <[email protected]>
> ---
> This patch was previously submitted here:
> https://lore.kernel.org/linux-input/20210625113435.2539282-2-linus.walleij@l
> inaro.org/
>
> Changes since the original patch:
> - Adress the review comments by Dmitry:
> Drop explict OF check and use of_find_property()
> ---
> drivers/input/touchscreen/zinitix.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/input/touchscreen/zinitix.c
> b/drivers/input/touchscreen/zinitix.c index 1e70b8d2a8d7..d4e06a88a883
> 100644
> --- a/drivers/input/touchscreen/zinitix.c
> +++ b/drivers/input/touchscreen/zinitix.c
> @@ -252,16 +252,27 @@ static int zinitix_init_touch(struct bt541_ts_data
> *bt541)
>
> static int zinitix_init_regulators(struct bt541_ts_data *bt541)
> {
> - struct i2c_client *client = bt541->client;
> + struct device *dev = &bt541->client->dev;
> int error;
>
> - bt541->supplies[0].supply = "vdd";
> - bt541->supplies[1].supply = "vddo";
> - error = devm_regulator_bulk_get(&client->dev,
> + /*
> + * Some older device trees have erroneous names for the regulators,
> + * so check if "vddo" is present and in that case use these names
> + * and warn. Else use the proper supply names on the component.
> + */

Nitpick, but:
"and in that case use these names and warn."
I don't see any dev_warn or anything that would 'warn'. If you send a v2 it
might be nice to fix that.

Regards
Luca

> + if (of_find_property(dev->of_node, "vddo-supply", NULL)) {
> + bt541->supplies[0].supply = "vdd";
> + bt541->supplies[1].supply = "vddo";
> + } else {
> + /* Else use the proper supply names */
> + bt541->supplies[0].supply = "vcca";
> + bt541->supplies[1].supply = "vdd";
> + }
> + error = devm_regulator_bulk_get(dev,
> ARRAY_SIZE(bt541-
>supplies),
> bt541->supplies);
> if (error < 0) {
> - dev_err(&client->dev, "Failed to get regulators: %d\n",
error);
> + dev_err(dev, "Failed to get regulators: %d\n", error);
> return error;
> }