linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Luca Weiss" <luca.weiss@fairphone.com>
To: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>,
	<linux-input@vger.kernel.org>
Cc: <~postmarketos/upstreaming@lists.sr.ht>,
	<phone-devel@vger.kernel.org>,
	"Dmitry Torokhov" <dmitry.torokhov@gmail.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/3] dt-bindings: input: Add bindings for Awinic AW8695 haptics
Date: Mon, 11 Apr 2022 10:15:04 +0200	[thread overview]
Message-ID: <CJ79EIW9Z89J.YZTZ6AU91TGE@otso> (raw)
In-Reply-To: <1a45984a-752b-6bad-0320-f0946d83f2b9@linaro.org>

Hi Krzysztof,

thanks for the review, replies are inline!

On Fri Apr 8, 2022 at 5:05 PM CEST, Krzysztof Kozlowski wrote:
> On 08/04/2022 13:53, Luca Weiss wrote:
> > Add a document describing the bindings for the AW8695 LRA Haptic Driver.
>
> (...)
>
> > +  reset-gpios:
> > +    maxItems: 1
> > +    description: GPIO connected to RSTN pin (active high)
> > +
> > +  awinic,f0-preset:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Default value for the f0 of LRA
> > +
> > +  awinic,f0-coefficient:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Coefficient between actual f0 and the value in the registers
> > +
> > +  awinic,f0-calibration-percent:
> > +    maxItems: 1
> > +    description: Limit of f0 deviation from awinic,f0-preset
> > +
> > +  awinic,drive-level:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Level of drive waveform in normal driving
> > +
> > +  awinic,f0-detection-play-time:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Drive waveform play times in the first period in the f0 detection
>
> Use standard unit suffixes for known units (e.g. time).

While the datasheet doesn't mention any time unit, the value is used to
calculate the f0_trace_ms variable (which is milliseconds) but the
result also depends on the awinic,f0-preset value, so it's not a raw
time value.

>
> > +
> > +  awinic,f0-detection-wait-time:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Waveform wait times in the f0 detection
>
> Ditto.
>
> > +
> > +  awinic,f0-detection-repeat:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Repeat times in the f0 detection
> > +
> > +  awinic,f0-detection-trace:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Drive waveform play times in the second period and later in the f0 detection
> > +
> > +  awinic,boost-debug:
> > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > +    minItems: 3
> > +    maxItems: 3
> > +    description: Values for BSTDBG1-3 registers
>
> Do not encode device programming model (registers) into the binding. You
> need to define it as a property related to hardware itself, not its
> registers (e.g. boost value in mV).

Unfortunately I couldn't figure the meaning for this and the two values
below.

The datasheet doesn't mention these registers at all and the downstream
driver doesn't do anything meaningful with them (other than setting them)
nor has any comment to indicate what they do.
In the datasheet there's only BSTDBG4 mentioned where bits [5:1] mean
PVDD output voltage setting so for these registers it could really be
anthing :(

Maybe someone with more knowledge about LRAs might be able to decipher
what tset and r_spare is at least? I unfortunately didn't manage.

Regards
Luca

>
> > +
> > +  awinic,tset:
> > +    $ref: /schemas/types.yaml#/definitions/uint8
> > +    description: Value for TSET register
>
> Ditto.
>
> > +
> > +  awinic,r-spare:
> > +    $ref: /schemas/types.yaml#/definitions/uint8
> > +    description: Value for R_SPARE register
>
> Ditto.
>
>
> Best regards,
> Krzysztof


  reply	other threads:[~2022-04-11  8:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-08 11:53 [PATCH 1/3] dt-bindings: input: Add bindings for Awinic AW8695 haptics Luca Weiss
2022-04-08 11:53 ` [PATCH 2/3] Input - aw8695: Add driver for " Luca Weiss
2022-04-08 17:25   ` kernel test robot
2022-04-09 21:15   ` Jeff LaBundy
2022-04-11  9:57     ` Luca Weiss
2022-04-12  2:44       ` Jeff LaBundy
2022-04-08 11:53 ` [PATCH 3/3] arm64: dts: qcom: sm7225-fairphone-fp4: Add " Luca Weiss
2022-04-08 19:49   ` kernel test robot
2022-04-08 15:05 ` [PATCH 1/3] dt-bindings: input: Add bindings for Awinic " Krzysztof Kozlowski
2022-04-11  8:15   ` Luca Weiss [this message]
2022-04-11 12:52     ` Krzysztof Kozlowski
2022-04-11 15:11       ` Luca Weiss

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=CJ79EIW9Z89J.YZTZ6AU91TGE@otso \
    --to=luca.weiss@fairphone.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=phone-devel@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=~postmarketos/upstreaming@lists.sr.ht \
    /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).