| Message ID | 20211027181350.91630-1-nikita@trvn.ru |
|---|---|
| Headers | show |
| Series | Add touch-keys support to the Zinitix touch driver | expand |
Hi Nikita, On Mittwoch, 27. Oktober 2021 20:13:47 CEST Nikita Travkin wrote: > From: Linus Walleij <linus.walleij@linaro.org> > > 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 <broonie@kernel.org> > Cc: Michael Srba <Michael.Srba@seznam.cz> > Cc: phone-devel@vger.kernel.org > Cc: devicetree@vger.kernel.org > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > [Slightly changed the legacy regulator detection] > Signed-off-by: Nikita Travkin <nikita@trvn.ru> > --- > 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; > }
On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote: > Since irq request is the last thing in the driver probe, it happens > later than the input device registration. This means that there is a > small time window where if the open method is called the driver will > attempt to enable not yet available irq. > > Fix that by moving the irq request before the input device registration. > > Fixes: 26822652c85e ("Input: add zinitix touchscreen driver") > Signed-off-by: Nikita Travkin <nikita@trvn.ru> Good catch! Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote: > Zinitix BT532 is another touch controller that seem to implement the > same interface as an already supported BT541. Add it to the driver. > > Signed-off-by: Nikita Travkin <nikita@trvn.ru> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote: > Zinitix touch controllers can use some of the sense lines for virtual > keys (like those found on many phones). Add support for those keys. > > Signed-off-by: Nikita Travkin <nikita@trvn.ru> Nice! Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
Hi Nikita, On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote: > This series adds support for the touch-keys that can be present on some > touchscreen configurations, adds the compatible for bt532 and fixes a > small race condition bug in the driver probe function. > > I also pick up the series that converts the dt bindings to yaml > initially submitted by Linus Walleij in [1]. > I made some minor changes to those patches: > - Fixed dt_schema_check error > - Adressed the review comments from Dmitry on the original series Thanks for picking this up! Have you notices some behaviour like surplus touch events (like many press/release events fall through to the UI) when using this driver? I think it might need some z fuzzing but I am not sure. Yours, Linus Walleij
Hi Linus, Linus Walleij писал(а) 09.11.2021 09:45: > Hi Nikita, > > On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote: > >> This series adds support for the touch-keys that can be present on >> some >> touchscreen configurations, adds the compatible for bt532 and fixes a >> small race condition bug in the driver probe function. >> >> I also pick up the series that converts the dt bindings to yaml >> initially submitted by Linus Walleij in [1]. >> I made some minor changes to those patches: >> - Fixed dt_schema_check error >> - Adressed the review comments from Dmitry on the original series > > Thanks for picking this up! > > Have you notices some behaviour like surplus touch events > (like many press/release events fall through to the UI) > when using this driver? I think it might need some z fuzzing > but I am not sure. > On my device (8 inch tablet with BT532) I saw no problems with touch so far. However another person with a different tablet (10 inch with ZT7554) indeed says that they notice "multiplied" touches that make typing hard so maybe that depends on controller model/firmware... And speaking of that ZT7554: Seems like it's works with the driver and I'd like to add the compatible for it in v2 but I'd also have to add it to the bindings. Looking at how you add all other similar names for BT* there does it make sense to add ZT* as well? Maybe you have some hints where to look for a list of the models? I was planning to send a v2 with all the review fixes near the end of the week but I've noticed a yet another quirky issue with the touch controller: At least on my device, for some reason enabling touchkeys changes the way the controller reports the finger touch events which breaks multi-touch... Assuming that *not* enabling the touchkeys leads to calibration being wrong (controller assigns the touchkey sense lines to the touch area in that case) I now have to resolve this quirk as well... Thanks, Nikita > Yours, > Linus Walleij
On Tue, Nov 9, 2021 at 4:23 PM Nikita Travkin <nikita@trvn.ru> wrote: > [Me] > > Have you notices some behaviour like surplus touch events > > (like many press/release events fall through to the UI) > > when using this driver? I think it might need some z fuzzing > > but I am not sure. > > > > On my device (8 inch tablet with BT532) I saw no problems with touch > so far. However another person with a different tablet (10 inch with > ZT7554) > indeed says that they notice "multiplied" touches that make typing hard > so maybe that depends on controller model/firmware... It may even be depending on specimen. I saw that the vendor driver does contain some debouncing code. > And speaking of that ZT7554: Seems like it's works with the driver > and I'd like to add the compatible for it in v2 but I'd also have to add > it to the bindings. Looking at how you add all other similar names for BT* > there does it make sense to add ZT* as well? Yeah probably, if they are electrically very similar. > Maybe you have some hints where > to look for a list of the models? I usually google ... try to find things like powerpoints with roadmaps from the vendor and things like that. Best thing is if they answer to mail but I don't know if Zinitix are even around anymore. > I've noticed a yet another quirky issue with the touch controller: > At least on my device, for some reason enabling touchkeys changes the > way the > controller reports the finger touch events which breaks multi-touch... > Assuming that *not* enabling the touchkeys leads to calibration being > wrong > (controller assigns the touchkey sense lines to the touch area in that > case) > I now have to resolve this quirk as well... Hm yeah I guess refer to the (messy) vendor driver for hints. Yours, Linus Walleij
Hi Nikita, On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote: > This series adds support for the touch-keys that can be present on some > touchscreen configurations, adds the compatible for bt532 and fixes a > small race condition bug in the driver probe function. This appears unaddressed since October? I see there are just some small nits in patch 5 & 6 to fix, then it is finished. Do you have time to pick it up for kernel v5.17 instead? Make sure to collect all Reviewed-by on this series. Yours, Linus Walleij
Linus Walleij писал(а) 05.01.2022 02:04: > Hi Nikita, > > On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote: > >> This series adds support for the touch-keys that can be present on some >> touchscreen configurations, adds the compatible for bt532 and fixes a >> small race condition bug in the driver probe function. > > This appears unaddressed since October? > I see there are just some small nits in patch 5 & 6 to fix, then > it is finished. > Hi, I was planning to include the fix for the message reporting to the next version as well but then I got rather low on time and could never finish that bit. As it seem to only affect my device, there was not really much stopping me from submitting a next version without that fix other than my "irrational perfectionism" which I should probably learn to recognize better... > Do you have time to pick it up for kernel v5.17 instead? > Make sure to collect all Reviewed-by on this series. > I will try to submit a new version with review fixes and tags shortly. Thanks, Nikita > Yours, > Linus Walleij
This series adds support for the touch-keys that can be present on some touchscreen configurations, adds the compatible for bt532 and fixes a small race condition bug in the driver probe function. I also pick up the series that converts the dt bindings to yaml initially submitted by Linus Walleij in [1]. I made some minor changes to those patches: - Fixed dt_schema_check error - Adressed the review comments from Dmitry on the original series [1] https://lore.kernel.org/linux-input/20210625113435.2539282-1-linus.walleij@linaro.org/ Linus Walleij (2): dt-bindings: input/ts/zinitix: Convert to YAML, fix and extend Input: zinitix - Handle proper supply names Nikita Travkin (4): input: touchscreen: zinitix: Make sure the IRQ is allocated before it gets enabled input: touchscreen: zinitix: Add compatible for bt532 dt-bindings: input: zinitix: Document touch-keys support input: touchscreen: zinitix: Add touchkey support .../input/touchscreen/zinitix,bt400.yaml | 123 ++++++++++++++++++ .../bindings/input/touchscreen/zinitix.txt | 40 ------ drivers/input/touchscreen/zinitix.c | 101 +++++++++++--- 3 files changed, 207 insertions(+), 57 deletions(-) create mode 100644 Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/zinitix.txt