linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff LaBundy <jeff@labundy.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: 8a790966-d985-c0fc-498e-c17e69a6622e@linaro.org,
	devicetree@vger.kernel.org, dmitry.torokhov@gmail.com,
	jiriv@axis.com, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org,
	u.kleine-koenig@pengutronix.de
Subject: Re: Fwd: [PATCH 1/2] dt-bindings: input: microchip,cap11xx: add advanced sensitivity settings
Date: Mon, 1 May 2023 10:49:21 -0500	[thread overview]
Message-ID: <ZE/fgU/FYbFwkMpf@nixie71> (raw)
In-Reply-To: <403412d0-18b8-8d2f-e044-0e27b06a2d12@linaro.org>

Hi Krzysztof and Jiri,

On Mon, May 01, 2023 at 08:37:49AM +0200, Krzysztof Kozlowski wrote:
> On 28/04/2023 19:09, Jiri Valek - 2N wrote:
> > Hi Krzysztof,
> > and thanks for the review
> > 
> > On 4/15/23 11:10, Krzysztof Kozlowski wrote:
> >> On 15/04/2023 01:38, Jiri Valek - 2N wrote:
> >>> Add support for advanced sensitivity settings and signal guard feature.
> >>>
> >>> Signed-off-by: Jiri Valek - 2N <jiriv@axis.com>
> >>> ---

[...]

> >>> +  microchip,input-treshold:
> >>> +    minItems: 3
> >>> +    maxItems: 8
> >>> +    minimum: 0
> >>> +    maximum: 127
> >>> +    default: 64
> >>> +    description: |
> >>> +      Optional parameter. Specifies the delta threshold that is used to
> >>> +      determine if a touch has been detected.
> >>> +      The number of entries must correspond to the number of channels.
> >>> +
> >>> +  microchip,calib-sensitivity:
> >>> +    minItems: 3
> >>> +    maxItems: 8
> >>> +    enum: [1, 2, 4]
> >>> +    default: 1
> >>> +    description: |
> >>> +      Optional parameter supported only for CAP129x. Specifies an array of
> >>> +      numeric values that controls the gain used by the calibration routine to
> >>> +      enable sensor inputs to be more sensitive for proximity detection.
> >>> +      The number of entries must correspond to the number of channels.
> >>
> >> Most of these properties do not look like hardware properties. Policies
> >> and runtime configuration should not be put into DT. Explain please why
> >> these are board-specific thus suitable for DT.
> > 
> > All these parameters are intended to set HW properties of touch buttons. 
> 
> I know, but some HW properties are software policies. Consider the
> simplest example - audio volume of a speaker. It's a hardware property,
> but it is not for DT. Software should choose audio volume based on
> user's decisions.
> 
> > Each button can have different PCB layout and when you start without 
> > setting these parameters in DT, then touches won't be detected or you 
> > will get false positive readings.
> > E.g. 'signal-guard' change property of analog input from button to some 
> > type of shield.
> > I made all of them optional for backward compatibility.
> > Maybe 'sensitivity-base-shift' is really not necessary to have in DT.
> > I will remove it if you agree.
> 
> Keep only ones which are not policies but depend on physical/electrical
> characteristic of boards.

For capacitive sensing devices such as this, the threshold is specified
based on the size of the electrode and properties of the physical overlay.
Unlike speaker volume, it is a static property that is often times advised
by the module vendor.

Based on a quick scan of this patch, all of the properties proposed here
seem appropriate for DT and are in line with the properties exposed for
comparable devices (e.g. azoteq,thresh).

> 
> Best regards,
> Krzysztof
> 

Kind regards,
Jeff LaBundy

      reply	other threads:[~2023-05-01 15:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <d7f77779-5d28-b78a-da4e-cc237b2a04b9@axis.com>
2023-04-28 17:09 ` Fwd: [PATCH 1/2] dt-bindings: input: microchip,cap11xx: add advanced sensitivity settings Jiri Valek - 2N
2023-05-01  6:37   ` Krzysztof Kozlowski
2023-05-01 15:49     ` Jeff LaBundy [this message]

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=ZE/fgU/FYbFwkMpf@nixie71 \
    --to=jeff@labundy.com \
    --cc=8a790966-d985-c0fc-498e-c17e69a6622e@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jiriv@axis.com \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=u.kleine-koenig@pengutronix.de \
    /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).