From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Artur Rojek <contact@artur-rojek.eu>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Jonathan Cameron <jic23@kernel.org>,
Paul Cercueil <paul@crapouillou.net>,
Heiko Stuebner <heiko@sntech.de>,
linux-input <linux-input@vger.kernel.org>,
devicetree <devicetree@vger.kernel.org>,
linux-iio <linux-iio@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [RESEND PATCH v5 3/5] IIO: Ingenic JZ47xx: Add touchscreen mode.
Date: Fri, 17 Apr 2020 23:59:03 +0300 [thread overview]
Message-ID: <CAHp75Vcwnu8tw92nMYc_5-x_iX+FY8_OhtaJkSYNehmNUDkHGQ@mail.gmail.com> (raw)
In-Reply-To: <20200417202859.35427-3-contact@artur-rojek.eu>
On Fri, Apr 17, 2020 at 11:21 PM Artur Rojek <contact@artur-rojek.eu> wrote:
>
> The SADC component in JZ47xx SoCs provides support for touchscreen
> operations (pen position and pen down pressure) in single-ended and
> differential modes.
>
> Of the known hardware to use this controller, GCW Zero and Anbernic RG-350
> utilize the touchscreen mode by having their joystick(s) attached to the
> X/Y positive/negative input pins.
> GCW Zero comes with a single joystick and is sufficiently handled with the
> currently implemented single-ended mode. Support for boards with two
> joysticks, where one is hooked up to Xn/Yn and the other to Xp/Yp channels
> will need to be provided in the future.
>
> The touchscreen component of SADC takes a significant time to stabilize
> after first receiving the clock and a delay of 50ms has been empirically
> proven to be a safe value before data sampling can begin.
>
> All the boards which probe this driver have the interrupt provided from
> devicetree, with no need to handle a case where the irq was not provided.
Device Tree
IRQ
...
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 12,
> + .storagebits = 16
It's slightly better to leave comma in such cases.
> + },
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 12,
> + .storagebits = 16
Ditto.
> + },
...
> .indexed = 1,
> .channel = INGENIC_ADC_AUX,
> + .scan_index = -1
Ditto. You see above? Isn't it nice that you didn't touch that line?
So, perhaps next developer can leverage this subtle kind of things.
> .indexed = 1,
> .channel = INGENIC_ADC_BATTERY,
> + .scan_index = -1
Ditto.
> .indexed = 1,
> .channel = INGENIC_ADC_AUX2,
> + .scan_index = -1
Ditto.
...
> +static int ingenic_adc_buffer_enable(struct iio_dev *iio_dev)
> +{
> + struct ingenic_adc *adc = iio_priv(iio_dev);
> +
> + clk_enable(adc->clk);
Error check?
> + /* It takes significant time for the touchscreen hw to stabilize. */
> + msleep(50);
> + ingenic_adc_set_config(adc, JZ_ADC_REG_CFG_TOUCH_OPS_MASK,
> + JZ_ADC_REG_CFG_SAMPLE_NUM(4) |
> + JZ_ADC_REG_CFG_PULL_UP(4));
> + writew(80, adc->base + JZ_ADC_REG_ADWAIT);
> + writew(2, adc->base + JZ_ADC_REG_ADSAME);
> + writeb((u8)~JZ_ADC_IRQ_TOUCH, adc->base + JZ_ADC_REG_CTRL);
Why casting?
> + writel(0, adc->base + JZ_ADC_REG_ADTCH);
> + ingenic_adc_enable(adc, 2, true);
> +
> + return 0;
> +}
> + irq = platform_get_irq(pdev, 0);
Before it worked w/o IRQ, here is a regression you introduced.
> + if (irq < 0) {
> + dev_err(dev, "Failed to get irq: %d\n", irq);
Redundant message.
> + return irq;
> + }
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2020-04-17 20:59 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-17 20:28 [RESEND PATCH v5 1/5] IIO: Ingenic JZ47xx: Add xlate cb to retrieve correct channel idx Artur Rojek
2020-04-17 20:28 ` [RESEND PATCH v5 2/5] dt-bindings: iio/adc: Add touchscreen idx for JZ47xx SoC ADC Artur Rojek
2020-04-17 20:28 ` [RESEND PATCH v5 3/5] IIO: Ingenic JZ47xx: Add touchscreen mode Artur Rojek
2020-04-17 20:59 ` Andy Shevchenko [this message]
2020-04-17 21:04 ` Paul Cercueil
2020-04-17 21:13 ` Andy Shevchenko
2020-04-17 21:18 ` Paul Cercueil
2020-04-17 21:42 ` Andy Shevchenko
2020-04-17 21:45 ` Paul Cercueil
2020-04-17 21:52 ` Andy Shevchenko
2020-04-17 21:56 ` Paul Cercueil
2020-04-19 12:54 ` Ezequiel Garcia
2020-04-19 13:23 ` Paul Cercueil
2020-04-19 13:31 ` Artur Rojek
2020-04-19 12:19 ` Artur Rojek
2020-04-17 20:28 ` [RESEND PATCH v5 4/5] dt-bindings: input: Add docs for ADC driven joystick Artur Rojek
2020-04-17 20:28 ` [RESEND PATCH v5 5/5] input: joystick: Add ADC attached joystick driver Artur Rojek
2020-04-17 21:10 ` Andy Shevchenko
2020-04-17 21:23 ` Paul Cercueil
2020-04-17 21:49 ` Andy Shevchenko
2020-04-17 22:48 ` Paul Cercueil
2020-04-18 11:57 ` Andy Shevchenko
2020-04-18 12:10 ` Paul Cercueil
2020-04-18 12:42 ` Andy Shevchenko
2020-04-18 13:24 ` Paul Cercueil
2020-04-18 14:22 ` Jonathan Cameron
2020-04-18 17:25 ` Paul Cercueil
2020-04-18 18:20 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAHp75Vcwnu8tw92nMYc_5-x_iX+FY8_OhtaJkSYNehmNUDkHGQ@mail.gmail.com \
--to=andy.shevchenko@gmail.com \
--cc=contact@artur-rojek.eu \
--cc=devicetree@vger.kernel.org \
--cc=dmitry.torokhov@gmail.com \
--cc=heiko@sntech.de \
--cc=jic23@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=paul@crapouillou.net \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).