Re: [PATCH] arm64: dts: rk3399-pinephone-pro: Add support for volume keys

From: "Ondřej Jirman" <megi@xff.cz>
To: "Heiko Stübner" <heiko@sntech.de>
Cc: Peter Robinson <pbrobinson@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Tom Fitzhenry <tom@tom-fitzhenry.me.uk>,
	Martijn Braam <martijn@brixit.nl>,
	Caleb Connolly <kc@postmarketos.org>,
	Jarrah Gosbell <kernel@undef.tools>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org
Subject: Re: [PATCH] arm64: dts: rk3399-pinephone-pro: Add support for volume keys
Date: Mon, 17 Apr 2023 14:37:16 +0200	[thread overview]
Message-ID: <20230417123716.c23izju4ezjrzytn@core> (raw)
In-Reply-To: <4152389.RUnXabflUD@diego>

Hello Heiko,

On Mon, Apr 17, 2023 at 10:34:20AM +0200, Heiko Stübner wrote:
> Hi Peter, Ondrej,
> 
> Am Mittwoch, 5. April 2023, 15:53:39 CEST schrieb Ondřej Jirman:
> > [...]
> >
> > (I have to press quite hard to get bellow 300 and to get reliable detection
> > of volume down key press)
> > 
> > On development version of the phone, the value returned by sardac is less
> > variable. Basically either 298.828125 or 300.5859375 but it's also on
> > the edge.
> > 
> > I suggest raising the threshold to something like 600 and to do your own
> > testing, to get more data points. Unpressed value is ~1791.2109375 on both
> > phones, so 400 still gets a lot of headroom. And volume up is always < 15
> > in my tests.
> 
> did this get more attention meanwhile?
> 
> I don't have a Pinephone Pro myself, so you'll need to decide between you
> about the value and the concern Ondrej raised here for the value.

It's safe and needed to use a higher value.

SAR ADC input is pulled high to 1.8V unless some key is pressed, so unpressed
value will always be around 1800 on all Pinephones, and pressed value will
depend on contact quality and tolerances. For volume down, SAR ADC input is fed
from a resistor divider of (10kOhm + 2kOhm) from 1.8V power rail. So that gives
2/12*1.8 = 0.3V. We can't have the press detection threshold right at this
voltage, because:

1) these resistors have tolerances that will randomly result in measured voltage
   being above or below the 0.3V on real devices (-1% on 10k and +1% on 2k =
   2*1.01/(10*0.99+2*1.01)*1.8 = 305 mV - already too high even without
   considering switch contact quality), and

2) those piddly membrane switches apparently have their own random resistance
   that is added to the bottom leg of the resistor divider, and depends on
   strenght of the press on some devices (and switches may develop higher
   resistance with age/use).

Schematic: https://megous.com/dl/tmp/1125d9248a8213b3.png

kind regards,
	o.

> Thanks
> Heiko

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel