| Message ID | 20230209232556.91554-1-soyer@irl.hu (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add tri-state-key for oneplus6 | expand |
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
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 >
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 >> >
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
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
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 >
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
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;