devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Cc: linux-kernel@vger.kernel.org, Frank Li <Frank.Li@nxp.com>,
	linux-amarula@amarulasolutions.com,
	Conor Dooley <conor.dooley@microchip.com>,
	Conor Dooley <conor+dt@kernel.org>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Javier Carrasco <javier.carrasco@wolfvision.net>,
	Jeff LaBundy <jeff@labundy.com>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	devicetree@vger.kernel.org, linux-input@vger.kernel.org
Subject: Re: [PATCH v5 3/6] dt-bindings: touchscreen: add touchscreen-glitch-threshold-ns property
Date: Mon, 22 Sep 2025 10:07:56 -0500	[thread overview]
Message-ID: <20250922150756.GA4067300-robh@kernel.org> (raw)
In-Reply-To: <CABGWkvr8X5a0ezeu6HDCMfjh+xbg-bQq4cLwzRD2BvoJsvH_BA@mail.gmail.com>

On Sat, Sep 20, 2025 at 11:39:59AM +0200, Dario Binacchi wrote:
> On Fri, Sep 19, 2025 at 10:44 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Fri, Sep 19, 2025 at 05:12:42PM +0200, Dario Binacchi wrote:
> > > On Fri, Sep 19, 2025 at 4:38 PM Rob Herring <robh@kernel.org> wrote:
> > > >
> > > > On Thu, Sep 18, 2025 at 10:37:37PM +0200, Dario Binacchi wrote:
> > > > > On Thu, Sep 18, 2025 at 10:04 PM Rob Herring <robh@kernel.org> wrote:
> > > > > >
> > > > > > On Thu, Sep 18, 2025 at 05:52:31PM +0200, Dario Binacchi wrote:
> > > > > > > Add support for glitch threshold configuration. A detected signal is valid
> > > > > > > only if it lasts longer than the set threshold; otherwise, it is regarded
> > > > > > > as a glitch.
> > > > > > >
> > > > > > > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > > > > > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > > > > >
> > > > > > > ---
> > > > > > >
> > > > > > > Changes in v5:
> > > > > > > - Add Acked-by tag of Conor Dooley
> > > > > > >
> > > > > > > Changes in v2:
> > > > > > > - Added in v2.
> > > > > > >
> > > > > > >  .../devicetree/bindings/input/touchscreen/touchscreen.yaml    | 4 ++++
> > > > > > >  1 file changed, 4 insertions(+)
> > > > > > >
> > > > > > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> > > > > > > index 3e3572aa483a..a60b4d08620d 100644
> > > > > > > --- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> > > > > > > +++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
> > > > > > > @@ -206,6 +206,10 @@ properties:
> > > > > > >
> > > > > > >          unevaluatedProperties: false
> > > > > > >
> > > > > > > +  touchscreen-glitch-threshold-ns:
> > > > > > > +    description: Minimum duration in nanoseconds a signal must remain stable
> > > > > > > +      to be considered valid.
> > > > > >
> > > > > > What's wrong with debounce-delay-ms?
> > > > >
> > > > > Do you mean that I should rename touchscreen-glitch-threshold-ns to
> > > > > debounce-delay-ms?
> > > >
> > > > I mean that's the common property we already have, so use it or explain
> > > > why you aren't using it. I suppose the definition is technically a bit
> > > > different if it's purely a s/w delay vs. h/w monitoring of the signal
> > > > state. I don't think it matters if the interpretation by each driver is
> > > > a bit different.
> > > >
> > > > Maybe msec is not enough resolution for you could be another reason?
> > >
> > > Yes, this is the main reason. As specified in the following patch:
> > >   v5 4/6 dt-bindings: touchscreen: fsl,imx6ul-tsc: support glitch threshold
> > >
> > > Drivers must convert this value to IPG clock cycles and map
> > > it to one of the four discrete thresholds exposed by the
> > > TSC_DEBUG_MODE2 register:
> > >
> > >   0: 8191 IPG cycles
> > >   1: 4095 IPG cycles
> > >   2: 2047 IPG cycles
> > >   3: 1023 IPG cycles
> > >
> > > In my case, the IPG clock runs at 66 MHz, which corresponds to:
> > >
> > > 124 µs for 0
> > > 62 µs for 1
> > > 31 us for 2
> > > 15 us for 3
> > >
> > > So using milliseconds would not fit my use case. A possible trade-off
> > > could be to use debounce-delay-us. Would that be acceptable?
> >
> > I agree it wouldn't map to what the h/w provides, but is what the h/w
> > provides actually useful? There's plenty of h/w designed that's not
> > useful. 15us is quite short for a glitch. Do you have an actual cases
> > where the different values above are needed?
> 
> Considering an IPG clock at 66 MHz, currently at reset the deglitch
> filter is set to 124 µs,
> the driver sets it to 31 µs with a hardcoded value, and in my use case
> I need to set it to 62 µs,

It would be helpful if the commit message explained why. What platform 
needs it and what happens without this support added?

> as you can see in the patch:
> https://lore.kernel.org/all/20250918155240.2536852-6-dario.binacchi@amarulasolutions.com/
> and its handling in
> https://lore.kernel.org/all/20250918155240.2536852-7-dario.binacchi@amarulasolutions.com/
> 
> Another option could be to use a specific binding for the
> fsl,imx6ul-tsc controller, as I did in the
> earlier versions of the series.

No, add debounce-delay-us to the common binding.

Rob

  reply	other threads:[~2025-09-22 15:07 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-18 15:52 [PATCH v5 0/6] Input: imx6ul_tsc - set glitch threshold by dts property Dario Binacchi
2025-09-18 15:52 ` [PATCH v5 3/6] dt-bindings: touchscreen: add touchscreen-glitch-threshold-ns property Dario Binacchi
2025-09-18 20:04   ` Rob Herring
2025-09-18 20:37     ` Dario Binacchi
2025-09-19 14:38       ` Rob Herring
2025-09-19 15:12         ` Dario Binacchi
2025-09-19 20:44           ` Rob Herring
2025-09-20  9:39             ` Dario Binacchi
2025-09-22 15:07               ` Rob Herring [this message]
2025-09-18 15:52 ` [PATCH v5 4/6] dt-bindings: touchscreen: fsl,imx6ul-tsc: support glitch thresold Dario Binacchi
2025-09-18 15:52 ` [PATCH v5 5/6] ARM: dts: imx6ull-engicam-microgea-bmm: set touchscreen glitch threshold Dario Binacchi

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=20250922150756.GA4067300-robh@kernel.org \
    --to=robh@kernel.org \
    --cc=Frank.Li@nxp.com \
    --cc=conor+dt@kernel.org \
    --cc=conor.dooley@microchip.com \
    --cc=dario.binacchi@amarulasolutions.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=javier.carrasco@wolfvision.net \
    --cc=jeff@labundy.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-amarula@amarulasolutions.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.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).