From: "Heiko Stübner" <heiko@sntech.de>
To: "Ondřej Jirman" <megi@xff.cz>,
"Peter Robinson" <pbrobinson@gmail.com>,
"Rob Herring" <robh+dt@kernel.org>,
"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
"Heiko Stuebner" <heiko@sntech.de>,
"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 10:34:20 +0200 [thread overview]
Message-ID: <4152389.RUnXabflUD@diego> (raw)
In-Reply-To: <20230405135339.bcdyjdbtynuwf5yz@core>
Hi Peter, Ondrej,
Am Mittwoch, 5. April 2023, 15:53:39 CEST schrieb Ondřej Jirman:
> On Wed, Apr 05, 2023 at 01:38:13PM +0100, Peter Robinson wrote:
> > From: Ondrej Jirman <megi@xff.cz>
> >
> > These are implemented via regular ADC, so regular polling is needed,
> > for these keys to work.
> >
> > Signed-off-by: Martijn Braam <martijn@brixit.nl>
> > Co-developed-by: Kamil Trzciński <ayufan@ayufan.eu>
> > Signed-off-by: Ondrej Jirman <megi@xff.cz>
> > Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
> > ---
> > .../dts/rockchip/rk3399-pinephone-pro.dts | 26 +++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > index a0795a2b1cb1..ecd48040eb0c 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > @@ -10,6 +10,7 @@
> > */
> >
> > /dts-v1/;
> > +#include <dt-bindings/input/gpio-keys.h>
> > #include <dt-bindings/input/linux-event-codes.h>
> > #include "rk3399.dtsi"
> > #include "rk3399-opp.dtsi"
> > @@ -29,6 +30,26 @@ chosen {
> > stdout-path = "serial2:115200n8";
> > };
> >
> > + adc-keys {
> > + compatible = "adc-keys";
> > + io-channels = <&saradc 1>;
> > + io-channel-names = "buttons";
> > + keyup-threshold-microvolt = <1600000>;
> > + poll-interval = <100>;
> > +
> > + button-up {
> > + label = "Volume Up";
> > + linux,code = <KEY_VOLUMEUP>;
> > + press-threshold-microvolt = <100000>;
> > + };
> > +
> > + button-down {
> > + label = "Volume Down";
> > + linux,code = <KEY_VOLUMEDOWN>;
> > + press-threshold-microvolt = <300000>;
>
> I don't know about this... I've tried reading voltage values from:
>
> cd /sys/bus/iio/devices/iio:device0 (path may differ on your kernel)
>
> echo $((`cat in_voltage_scale`*`cat in_voltage1_raw`))
>
> and I get various readings around the value 300 mV on both sides of the
> threshold when pressing the vol down key. So this threshold may not be
> good enough in practice.
>
> Values I get for several different pushes of the button:
>
> 293.5546875
> 328.7109375
> 332.2265625
> 304.1015625
> 297.0703125
> 522.0703125
>
> (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.
Thanks
Heiko
> Otherwise:
>
> Tested-by: Ondrej Jirman <megi@xff.cz>
>
> kind regards,
> o.
>
> > + };
> > + };
> > +
> > gpio-keys {
> > compatible = "gpio-keys";
> > pinctrl-names = "default";
> > @@ -429,6 +450,11 @@ &sdio0 {
> > status = "okay";
> > };
> >
> > +&saradc {
> > + vref-supply = <&vcca1v8_s3>;
> > + status = "okay";
> > +};
> > +
> > &sdmmc {
> > bus-width = <4>;
> > cap-sd-highspeed;
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
WARNING: multiple messages have this Message-ID (diff)
From: "Heiko Stübner" <heiko@sntech.de>
To: "Ondřej Jirman" <megi@xff.cz>,
"Peter Robinson" <pbrobinson@gmail.com>,
"Rob Herring" <robh+dt@kernel.org>,
"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
"Heiko Stuebner" <heiko@sntech.de>,
"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 10:34:20 +0200 [thread overview]
Message-ID: <4152389.RUnXabflUD@diego> (raw)
In-Reply-To: <20230405135339.bcdyjdbtynuwf5yz@core>
Hi Peter, Ondrej,
Am Mittwoch, 5. April 2023, 15:53:39 CEST schrieb Ondřej Jirman:
> On Wed, Apr 05, 2023 at 01:38:13PM +0100, Peter Robinson wrote:
> > From: Ondrej Jirman <megi@xff.cz>
> >
> > These are implemented via regular ADC, so regular polling is needed,
> > for these keys to work.
> >
> > Signed-off-by: Martijn Braam <martijn@brixit.nl>
> > Co-developed-by: Kamil Trzciński <ayufan@ayufan.eu>
> > Signed-off-by: Ondrej Jirman <megi@xff.cz>
> > Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
> > ---
> > .../dts/rockchip/rk3399-pinephone-pro.dts | 26 +++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > index a0795a2b1cb1..ecd48040eb0c 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > @@ -10,6 +10,7 @@
> > */
> >
> > /dts-v1/;
> > +#include <dt-bindings/input/gpio-keys.h>
> > #include <dt-bindings/input/linux-event-codes.h>
> > #include "rk3399.dtsi"
> > #include "rk3399-opp.dtsi"
> > @@ -29,6 +30,26 @@ chosen {
> > stdout-path = "serial2:115200n8";
> > };
> >
> > + adc-keys {
> > + compatible = "adc-keys";
> > + io-channels = <&saradc 1>;
> > + io-channel-names = "buttons";
> > + keyup-threshold-microvolt = <1600000>;
> > + poll-interval = <100>;
> > +
> > + button-up {
> > + label = "Volume Up";
> > + linux,code = <KEY_VOLUMEUP>;
> > + press-threshold-microvolt = <100000>;
> > + };
> > +
> > + button-down {
> > + label = "Volume Down";
> > + linux,code = <KEY_VOLUMEDOWN>;
> > + press-threshold-microvolt = <300000>;
>
> I don't know about this... I've tried reading voltage values from:
>
> cd /sys/bus/iio/devices/iio:device0 (path may differ on your kernel)
>
> echo $((`cat in_voltage_scale`*`cat in_voltage1_raw`))
>
> and I get various readings around the value 300 mV on both sides of the
> threshold when pressing the vol down key. So this threshold may not be
> good enough in practice.
>
> Values I get for several different pushes of the button:
>
> 293.5546875
> 328.7109375
> 332.2265625
> 304.1015625
> 297.0703125
> 522.0703125
>
> (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.
Thanks
Heiko
> Otherwise:
>
> Tested-by: Ondrej Jirman <megi@xff.cz>
>
> kind regards,
> o.
>
> > + };
> > + };
> > +
> > gpio-keys {
> > compatible = "gpio-keys";
> > pinctrl-names = "default";
> > @@ -429,6 +450,11 @@ &sdio0 {
> > status = "okay";
> > };
> >
> > +&saradc {
> > + vref-supply = <&vcca1v8_s3>;
> > + status = "okay";
> > +};
> > +
> > &sdmmc {
> > bus-width = <4>;
> > cap-sd-highspeed;
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: "Heiko Stübner" <heiko@sntech.de>
To: "Ondřej Jirman" <megi@xff.cz>,
"Peter Robinson" <pbrobinson@gmail.com>,
"Rob Herring" <robh+dt@kernel.org>,
"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
"Heiko Stuebner" <heiko@sntech.de>,
"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 10:34:20 +0200 [thread overview]
Message-ID: <4152389.RUnXabflUD@diego> (raw)
In-Reply-To: <20230405135339.bcdyjdbtynuwf5yz@core>
Hi Peter, Ondrej,
Am Mittwoch, 5. April 2023, 15:53:39 CEST schrieb Ondřej Jirman:
> On Wed, Apr 05, 2023 at 01:38:13PM +0100, Peter Robinson wrote:
> > From: Ondrej Jirman <megi@xff.cz>
> >
> > These are implemented via regular ADC, so regular polling is needed,
> > for these keys to work.
> >
> > Signed-off-by: Martijn Braam <martijn@brixit.nl>
> > Co-developed-by: Kamil Trzciński <ayufan@ayufan.eu>
> > Signed-off-by: Ondrej Jirman <megi@xff.cz>
> > Signed-off-by: Peter Robinson <pbrobinson@gmail.com>
> > ---
> > .../dts/rockchip/rk3399-pinephone-pro.dts | 26 +++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > index a0795a2b1cb1..ecd48040eb0c 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> > @@ -10,6 +10,7 @@
> > */
> >
> > /dts-v1/;
> > +#include <dt-bindings/input/gpio-keys.h>
> > #include <dt-bindings/input/linux-event-codes.h>
> > #include "rk3399.dtsi"
> > #include "rk3399-opp.dtsi"
> > @@ -29,6 +30,26 @@ chosen {
> > stdout-path = "serial2:115200n8";
> > };
> >
> > + adc-keys {
> > + compatible = "adc-keys";
> > + io-channels = <&saradc 1>;
> > + io-channel-names = "buttons";
> > + keyup-threshold-microvolt = <1600000>;
> > + poll-interval = <100>;
> > +
> > + button-up {
> > + label = "Volume Up";
> > + linux,code = <KEY_VOLUMEUP>;
> > + press-threshold-microvolt = <100000>;
> > + };
> > +
> > + button-down {
> > + label = "Volume Down";
> > + linux,code = <KEY_VOLUMEDOWN>;
> > + press-threshold-microvolt = <300000>;
>
> I don't know about this... I've tried reading voltage values from:
>
> cd /sys/bus/iio/devices/iio:device0 (path may differ on your kernel)
>
> echo $((`cat in_voltage_scale`*`cat in_voltage1_raw`))
>
> and I get various readings around the value 300 mV on both sides of the
> threshold when pressing the vol down key. So this threshold may not be
> good enough in practice.
>
> Values I get for several different pushes of the button:
>
> 293.5546875
> 328.7109375
> 332.2265625
> 304.1015625
> 297.0703125
> 522.0703125
>
> (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.
Thanks
Heiko
> Otherwise:
>
> Tested-by: Ondrej Jirman <megi@xff.cz>
>
> kind regards,
> o.
>
> > + };
> > + };
> > +
> > gpio-keys {
> > compatible = "gpio-keys";
> > pinctrl-names = "default";
> > @@ -429,6 +450,11 @@ &sdio0 {
> > status = "okay";
> > };
> >
> > +&saradc {
> > + vref-supply = <&vcca1v8_s3>;
> > + status = "okay";
> > +};
> > +
> > &sdmmc {
> > bus-width = <4>;
> > cap-sd-highspeed;
>
next prev parent reply other threads:[~2023-04-17 8:34 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-05 12:38 [PATCH] arm64: dts: rk3399-pinephone-pro: Add support for volume keys Peter Robinson
2023-04-05 12:38 ` Peter Robinson
2023-04-05 12:38 ` Peter Robinson
2023-04-05 13:53 ` Ondřej Jirman
2023-04-05 13:53 ` Ondřej Jirman
2023-04-05 13:53 ` Ondřej Jirman
2023-04-17 8:34 ` Heiko Stübner [this message]
2023-04-17 8:34 ` Heiko Stübner
2023-04-17 8:34 ` Heiko Stübner
2023-04-17 12:37 ` Ondřej Jirman
2023-04-17 12:37 ` Ondřej Jirman
2023-04-17 12:37 ` Ondřej Jirman
2023-04-17 14:36 ` Heiko Stübner
2023-04-17 14:36 ` Heiko Stübner
2023-04-17 14:36 ` Heiko Stübner
2023-04-17 14:48 ` Ondřej Jirman
2023-04-17 14:48 ` Ondřej Jirman
2023-04-17 14:48 ` Ondřej Jirman
2023-04-17 22:43 ` Heiko Stuebner
2023-04-17 22:43 ` Heiko Stuebner
2023-04-17 22:43 ` Heiko Stuebner
2023-04-18 7:42 ` Peter Robinson
2023-04-18 7:42 ` Peter Robinson
2023-04-18 7:42 ` Peter Robinson
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=4152389.RUnXabflUD@diego \
--to=heiko@sntech.de \
--cc=devicetree@vger.kernel.org \
--cc=kc@postmarketos.org \
--cc=kernel@undef.tools \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=martijn@brixit.nl \
--cc=megi@xff.cz \
--cc=pbrobinson@gmail.com \
--cc=robh+dt@kernel.org \
--cc=tom@tom-fitzhenry.me.uk \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.