[PATCH v2 3/3] arm64: dts: qcom: sdm845-oneplus: add tri-state-key

[PATCH v2 3/3] arm64: dts: qcom: sdm845-oneplus: add tri-state-key

[Gergo Koteles]

The tri-state-key is a sound profile switch found on the OnePlus 6,
Android maps the states to "mute", "vibrate" and "ring". Expose them as
ABS_SND_PROFILE events.
The previous GPIO numbers were wrong. Update them to the correct
ones.

Co-developed-by: Caleb Connolly <caleb@connolly.tech>
Signed-off-by: Caleb Connolly <caleb@connolly.tech>
Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
 .../boot/dts/qcom/sdm845-oneplus-common.dtsi  | 39 ++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
index 64638ea94db7..e45d4fdead82 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
@@ -52,6 +52,43 @@ key-vol-up {
 		};
 	};
 
+	tri-state-key {
+		compatible = "gpio-keys";
+		label = "Tri-state key";
+		pinctrl-names = "default";
+		pinctrl-0 = <&tri_state_key_default>;
+		state-top {
+			label = "Tri-state key top";
+			linux,input-type = <EV_ABS>;
+			linux,code = <ABS_SND_PROFILE>;
+			linux,input-value = <0>;
+			gpios = <&tlmm 126 GPIO_ACTIVE_LOW>;
+			debounce-interval = <50>;
+			linux,can-disable;
+		};
+
+		state-middle {
+			label = "Tri-state key middle";
+			linux,input-type = <EV_ABS>;
+			linux,code = <ABS_SND_PROFILE>;
+			linux,input-value = <1>;
+			gpios = <&tlmm 52 GPIO_ACTIVE_LOW>;
+			debounce-interval = <50>;
+			linux,can-disable;
+
+		};
+
+		state-bottom {
+			label = "Tri-state key bottom";
+			linux,input-type = <EV_ABS>;
+			linux,code = <ABS_SND_PROFILE>;
+			linux,input-value = <2>;
+			gpios = <&tlmm 24 GPIO_ACTIVE_LOW>;
+			debounce-interval = <50>;
+			linux,can-disable;
+		};
+	};
+
 	reserved-memory {
 		/*
 		 * The rmtfs_mem needs to be guarded due to "XPU limitations"
@@ -754,7 +791,7 @@ &tlmm {
 	gpio-reserved-ranges = <0 4>, <81 4>;
 
 	tri_state_key_default: tri-state-key-default-state {
-		pins = "gpio40", "gpio42", "gpio26";
+		pins = "gpio126", "gpio52", "gpio24";
 		function = "gpio";
 		drive-strength = <2>;
 		bias-disable;
-- 
2.39.1

Re: [PATCH v2 3/3] arm64: dts: qcom: sdm845-oneplus: add tri-state-key

[Krzysztof Kozlowski]

On 10/02/2023 00:25, Gergo Koteles wrote:
> The tri-state-key is a sound profile switch found on the OnePlus 6,
> Android maps the states to "mute", "vibrate" and "ring". Expose them as
> ABS_SND_PROFILE events.
> The previous GPIO numbers were wrong. Update them to the correct
> ones.
> 
> Co-developed-by: Caleb Connolly <caleb@connolly.tech>
> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> Signed-off-by: Gergo Koteles <soyer@irl.hu>

Where are other patches? I got only 3/3.

> ---
>  .../boot/dts/qcom/sdm845-oneplus-common.dtsi  | 39 ++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> index 64638ea94db7..e45d4fdead82 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> @@ -52,6 +52,43 @@ key-vol-up {
>  		};
>  	};
>  
> +	tri-state-key {
> +		compatible = "gpio-keys";
> +		label = "Tri-state key";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&tri_state_key_default>;

Missing blank line.

> +		state-top {

Does not look like you tested the DTS against bindings. Please run `make
dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
for instructions).


Best regards,
Krzysztof

Re: [PATCH v2 3/3] arm64: dts: qcom: sdm845-oneplus: add tri-state-key

[Gergo Koteles]

On 2023. 02. 10. 12:33, Krzysztof Kozlowski wrote:
> On 10/02/2023 00:25, Gergo Koteles wrote:
>> The tri-state-key is a sound profile switch found on the OnePlus 6,
>> Android maps the states to "mute", "vibrate" and "ring". Expose them as
>> ABS_SND_PROFILE events.
>> The previous GPIO numbers were wrong. Update them to the correct
>> ones.
>>
>> Co-developed-by: Caleb Connolly <caleb@connolly.tech>
>> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
>> Signed-off-by: Gergo Koteles <soyer@irl.hu>
> 
> Where are other patches? I got only 3/3.
> 
Hi Krzysztof,

Sorry, I missed the --thread option for git format-patch.

>> ---
>>   .../boot/dts/qcom/sdm845-oneplus-common.dtsi  | 39 ++++++++++++++++++-
>>   1 file changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>> index 64638ea94db7..e45d4fdead82 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>> @@ -52,6 +52,43 @@ key-vol-up {
>>   		};
>>   	};
>>   
>> +	tri-state-key {
>> +		compatible = "gpio-keys";
>> +		label = "Tri-state key";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&tri_state_key_default>;
> 
> Missing blank line.
> 

I'll add it to v3.

>> +		state-top {
> 
> Does not look like you tested the DTS against bindings. Please run `make
> dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
> for instructions).
> 

I ran dtbs_check with 
DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/qcom.yaml. It only 
shows warnings for msm8996-oneplus3, but not for sdm845-oneplus phones. 
Is there anything else I need to check?


...
arch/arm64/boot/dts/qcom/msm8996-oneplus3.dtb: /: qcom,board-id: 'oneOf' 
conditional failed, one must be fixed:
	[8, 0, 15801, 15, 8, 0, 15801, 16] is too long
	From schema: /Documentation/devicetree/bindings/arm/qcom.yaml
...
   DTC_CHK arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dtb
   DTC_CHK arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dtb


Thanks,
Gergo

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 3/3] arm64: dts: qcom: sdm845-oneplus: add tri-state-key

[Konrad Dybcio]

On 10.02.2023 14:45, Gergo Koteles wrote:
> On 2023. 02. 10. 12:33, Krzysztof Kozlowski wrote:
>> On 10/02/2023 00:25, Gergo Koteles wrote:
>>> The tri-state-key is a sound profile switch found on the OnePlus 6,
>>> Android maps the states to "mute", "vibrate" and "ring". Expose them as
>>> ABS_SND_PROFILE events.
>>> The previous GPIO numbers were wrong. Update them to the correct
>>> ones.
>>>
>>> Co-developed-by: Caleb Connolly <caleb@connolly.tech>
>>> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
>>> Signed-off-by: Gergo Koteles <soyer@irl.hu>
>>
>> Where are other patches? I got only 3/3.
>>
> Hi Krzysztof,
> 
> Sorry, I missed the --thread option for git format-patch.
> 
>>> ---
>>>   .../boot/dts/qcom/sdm845-oneplus-common.dtsi  | 39 ++++++++++++++++++-
>>>   1 file changed, 38 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>> index 64638ea94db7..e45d4fdead82 100644
>>> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>> @@ -52,6 +52,43 @@ key-vol-up {
>>>           };
>>>       };
>>>   +    tri-state-key {
>>> +        compatible = "gpio-keys";
>>> +        label = "Tri-state key";
>>> +        pinctrl-names = "default";
>>> +        pinctrl-0 = <&tri_state_key_default>;
>>
>> Missing blank line.
>>
> 
> I'll add it to v3.
While at it, please put pinctrl-names after pinctrl-0.

> 
>>> +        state-top {
>>
>> Does not look like you tested the DTS against bindings. Please run `make
>> dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
>> for instructions).
>>
> 
> I ran dtbs_check with DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/qcom.yaml. It only shows warnings for msm8996-oneplus3, but not for sdm845-oneplus phones. Is there anything else I need to check?
You're only checking against a schema file which validates msm-id and
machine compatibles. The goal is to not introduce *any* new warnings.

You want to run:

make (your make args) CHECK_DTBS=1 qcom/sdm845-oneplus-enchilada.dtb

pre and post your patch.

Konrad

> 
> 
> ...
> arch/arm64/boot/dts/qcom/msm8996-oneplus3.dtb: /: qcom,board-id: 'oneOf' conditional failed, one must be fixed:
>     [8, 0, 15801, 15, 8, 0, 15801, 16] is too long
>     From schema: /Documentation/devicetree/bindings/arm/qcom.yaml
> ...
>   DTC_CHK arch/arm64/boot/dts/qcom/sdm845-oneplus-enchilada.dtb
>   DTC_CHK arch/arm64/boot/dts/qcom/sdm845-oneplus-fajita.dtb
> 
> 
> Thanks,
> Gergo
> 
>>
>> Best regards,
>> Krzysztof
>>
> 

Re: [PATCH v2 3/3] arm64: dts: qcom: sdm845-oneplus: add tri-state-key

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 549 bytes --]

Hi!

> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> @@ -52,6 +52,43 @@ key-vol-up {
>  		};
>  	};
>  
> +	tri-state-key {
> +		compatible = "gpio-keys";
> +		label = "Tri-state key";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&tri_state_key_default>;
> +		state-top {
> +			label = "Tri-state key top";

"top/middle" is not too useful. Do we need the label at all? If so,
should it say "loud/vibrations only/mute"?

BR,
								Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH v2 3/3] arm64: dts: qcom: sdm845-oneplus: add tri-state-key

[Caleb Connolly]

On 11/02/2023 16:40, Pavel Machek wrote:
> Hi!
>
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>> @@ -52,6 +52,43 @@ key-vol-up {
>>  		};
>>  	};
>>
>> +	tri-state-key {
>> +		compatible = "gpio-keys";
>> +		label = "Tri-state key";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&tri_state_key_default>;
>> +		state-top {
>> +			label = "Tri-state key top";
>
> "top/middle" is not too useful. Do we need the label at all? If so,
> should it say "loud/vibrations only/mute"?

"mute", "vibrate" and "ring" sound good to me.

Although it would be nice if users can easily map the physical key
position to the action when viewing the input device or remapping the
key in userspace.

Do you have any ideas or recommendations on how to do this?
>
> BR,
> 								Pavel

--
Kind Regards,
Caleb

Re: [PATCH v2 3/3] arm64: dts: qcom: sdm845-oneplus: add tri-state-key

[Gergo Koteles]

Hi,

> 
> 
> On 11/02/2023 16:40, Pavel Machek wrote:
>> Hi!
>>
>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>> @@ -52,6 +52,43 @@ key-vol-up {
>>>   		};
>>>   	};
>>>
>>> +	tri-state-key {
>>> +		compatible = "gpio-keys";
>>> +		label = "Tri-state key";
>>> +		pinctrl-names = "default";
>>> +		pinctrl-0 = <&tri_state_key_default>;
>>> +		state-top {
>>> +			label = "Tri-state key top";
>>
>> "top/middle" is not too useful. Do we need the label at all? If so,
>> should it say "loud/vibrations only/mute"?
> 
> "mute", "vibrate" and "ring" sound good to me.
> 

OnePlus uses the silent/vibrate/ring, iPhone the silent/ring names.
Maybe silent/vibrate/ring are more familiar.

Adding labels can document these modes here.
Should we also document these in input-event-codes.h?
#define ABS_SND_PROFILE		0x22 /* 0 = silent; 1 = vibrate; 2 = ring */


Thanks,
Gergo

> Although it would be nice if users can easily map the physical key
> position to the action when viewing the input device or remapping the
> key in userspace.
> 
> Do you have any ideas or recommendations on how to do this?
>>
>> BR,
>> 								Pavel
> 
> --
> Kind Regards,
> Caleb
> 

Re: [PATCH v2 3/3] arm64: dts: qcom: sdm845-oneplus: add tri-state-key

[Caleb Connolly]

On 16/02/2023 03:32, Gergo Koteles wrote:
> Hi,
>
>>
>>
>> On 11/02/2023 16:40, Pavel Machek wrote:
>>> Hi!
>>>
>>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>> @@ -52,6 +52,43 @@ key-vol-up {
>>>>   		};
>>>>   	};
>>>>
>>>> +	tri-state-key {
>>>> +		compatible = "gpio-keys";
>>>> +		label = "Tri-state key";
>>>> +		pinctrl-names = "default";
>>>> +		pinctrl-0 = <&tri_state_key_default>;
>>>> +		state-top {
>>>> +			label = "Tri-state key top";
>>>
>>> "top/middle" is not too useful. Do we need the label at all? If so,
>>> should it say "loud/vibrations only/mute"?
>>
>> "mute", "vibrate" and "ring" sound good to me.
>>
>
> OnePlus uses the silent/vibrate/ring, iPhone the silent/ring names.
> Maybe silent/vibrate/ring are more familiar.
>
> Adding labels can document these modes here.
> Should we also document these in input-event-codes.h?

Maybe it would be best to define macros for these rather than leave them
as magic numbers
> #define ABS_SND_PROFILE		0x22 /* 0 = silent; 1 = vibrate; 2 = ring */

#define ABS_SND_PROFILE_SILENT	0
#define ABS_SND_PROFILE_VIBRATE	1
#define ABS_SND_PROFILE_RING	2

>
>
> Thanks,
> Gergo
>
>> Although it would be nice if users can easily map the physical key
>> position to the action when viewing the input device or remapping the
>> key in userspace.
>>
>> Do you have any ideas or recommendations on how to do this?
>>>
>>> BR,
>>> 								Pavel
>>
>> --
>> Kind Regards,
>> Caleb
>>
>

--
Kind Regards,
Caleb