diff mbox series

[v10,3/4] arm64: dts: qcom: sc7280: Add touchscreen and touchpad support for evoker

Message ID 20221104141515.v10.3.I3ac715e729f6f9b5a3e3001b155df4f9d14e6186@changeid (mailing list archive)
State Superseded
Headers show
Series Add LTE SKU for sc7280-evoker family | expand

Commit Message

Sheng-Liang Pan Nov. 4, 2022, 6:19 a.m. UTC
Change touchpad and touchscreen node for evoker
Touchpad: SA461D-1011
Touchscreen: GT7986U

Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
---

Changes in v8:
- updated patch subjects

Changes in v7:
- add compiatable for gt7986

Changes in v6:
- add removed pinctrl and align touchscreen label with herobrine board

Changes in v5:
- new patch for Touchscreen/trackpad in v5

 .../boot/dts/qcom/sc7280-herobrine-evoker.dtsi    | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

Comments

Konrad Dybcio Nov. 4, 2022, 9:35 a.m. UTC | #1
On 04/11/2022 07:19, Sheng-Liang Pan wrote:
> Change touchpad and touchscreen node for evoker
> Touchpad: SA461D-1011
> Touchscreen: GT7986U

What's the reasoning? Were they changed post r0? Is r0 support 
effectively dropped?

The changes look ok, but I feel like this needs more of a comment in the 
commit msg.


Konrad

>
> Signed-off-by: Sheng-Liang Pan <sheng-liang.pan@quanta.corp-partner.google.com>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v8:
> - updated patch subjects
>
> Changes in v7:
> - add compiatable for gt7986
>
> Changes in v6:
> - add removed pinctrl and align touchscreen label with herobrine board
>
> Changes in v5:
> - new patch for Touchscreen/trackpad in v5
>
>   .../boot/dts/qcom/sc7280-herobrine-evoker.dtsi    | 15 +++++++--------
>   1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi
> index a6015491c6082..706dd82a70138 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi
> @@ -23,16 +23,15 @@ ap_tp_i2c: &i2c0 {
>   	status = "okay";
>   	clock-frequency = <400000>;
>   
> -	trackpad: trackpad@2c {
> -		compatible = "hid-over-i2c";
> -		reg = <0x2c>;
> +	trackpad: trackpad@15 {
> +		compatible = "elan,ekth3000";
> +		reg = <0x15>;
>   		pinctrl-names = "default";
>   		pinctrl-0 = <&tp_int_odl>;
>   
>   		interrupt-parent = <&tlmm>;
>   		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
>   
> -		hid-descr-addr = <0x20>;
>   		vcc-supply = <&pp3300_z1>;
>   
>   		wakeup-source;
> @@ -43,9 +42,9 @@ ts_i2c: &i2c13 {
>   	status = "okay";
>   	clock-frequency = <400000>;
>   
> -	ap_ts: touchscreen@10 {
> -		compatible = "elan,ekth6915";
> -		reg = <0x10>;
> +	ap_ts: touchscreen@5d {
> +		compatible = "goodix,gt7986u", "goodix,gt7375p";
> +		reg = <0x5d>;
>   		pinctrl-names = "default";
>   		pinctrl-0 = <&ts_int_conn>, <&ts_rst_conn>;
>   
> @@ -54,7 +53,7 @@ ap_ts: touchscreen@10 {
>   
>   		reset-gpios = <&tlmm 54 GPIO_ACTIVE_LOW>;
>   
> -		vcc33-supply = <&ts_avdd>;
> +		vdd-supply = <&ts_avdd>;
>   	};
>   };
>
Doug Anderson Nov. 4, 2022, 2:25 p.m. UTC | #2
Hi,

On Fri, Nov 4, 2022 at 2:35 AM Konrad Dybcio
<konrad.dybcio@somainline.org> wrote:
>
>
> On 04/11/2022 07:19, Sheng-Liang Pan wrote:
> > Change touchpad and touchscreen node for evoker
> > Touchpad: SA461D-1011
> > Touchscreen: GT7986U
>
> What's the reasoning? Were they changed post r0? Is r0 support
> effectively dropped?
>
> The changes look ok, but I feel like this needs more of a comment in the
> commit msg.

As I understand it r0's toucscreen/touchpad were not right to start
with. We are moving towards getting things upstream sooner and that
means that hardware hasn't always been fully tested out.

I certainly wouldn't object to a better commit message here, but in
this case there are no real world users (yet) and thus nobody is
really affected by this churn. ...so IMO if the series needs to be
spun for some other reason then the commit message could be updated,
but I wouldn't object to it landing as-is either.

-Doug
Konrad Dybcio Nov. 4, 2022, 2:28 p.m. UTC | #3
On 04/11/2022 15:25, Doug Anderson wrote:
> Hi,
>
> On Fri, Nov 4, 2022 at 2:35 AM Konrad Dybcio
> <konrad.dybcio@somainline.org> wrote:
>>
>> On 04/11/2022 07:19, Sheng-Liang Pan wrote:
>>> Change touchpad and touchscreen node for evoker
>>> Touchpad: SA461D-1011
>>> Touchscreen: GT7986U
>> What's the reasoning? Were they changed post r0? Is r0 support
>> effectively dropped?
>>
>> The changes look ok, but I feel like this needs more of a comment in the
>> commit msg.
> As I understand it r0's toucscreen/touchpad were not right to start
> with. We are moving towards getting things upstream sooner and that
> means that hardware hasn't always been fully tested out.
>
> I certainly wouldn't object to a better commit message here, but in
> this case there are no real world users (yet) and thus nobody is
> really affected by this churn. ...so IMO if the series needs to be
> spun for some other reason then the commit message could be updated,
> but I wouldn't object to it landing as-is either.

If there are no real (read: not-an-internal-devboard) devices using it, 
then I

agree, it's fine to merge as-is.


Konrad

>
> -Doug
Doug Anderson Nov. 4, 2022, 2:32 p.m. UTC | #4
Hi,

On Fri, Nov 4, 2022 at 7:28 AM Konrad Dybcio
<konrad.dybcio@somainline.org> wrote:
>
>
> On 04/11/2022 15:25, Doug Anderson wrote:
> > Hi,
> >
> > On Fri, Nov 4, 2022 at 2:35 AM Konrad Dybcio
> > <konrad.dybcio@somainline.org> wrote:
> >>
> >> On 04/11/2022 07:19, Sheng-Liang Pan wrote:
> >>> Change touchpad and touchscreen node for evoker
> >>> Touchpad: SA461D-1011
> >>> Touchscreen: GT7986U
> >> What's the reasoning? Were they changed post r0? Is r0 support
> >> effectively dropped?
> >>
> >> The changes look ok, but I feel like this needs more of a comment in the
> >> commit msg.
> > As I understand it r0's toucscreen/touchpad were not right to start
> > with. We are moving towards getting things upstream sooner and that
> > means that hardware hasn't always been fully tested out.
> >
> > I certainly wouldn't object to a better commit message here, but in
> > this case there are no real world users (yet) and thus nobody is
> > really affected by this churn. ...so IMO if the series needs to be
> > spun for some other reason then the commit message could be updated,
> > but I wouldn't object to it landing as-is either.
>
> If there are no real (read: not-an-internal-devboard) devices using it,
> then I
>
> agree, it's fine to merge as-is.

I can confirm that. There are no evoker devices in the wild.

-Doug
Konrad Dybcio Nov. 4, 2022, 2:36 p.m. UTC | #5
On 04/11/2022 15:32, Doug Anderson wrote:
> Hi,
> 
> On Fri, Nov 4, 2022 at 7:28 AM Konrad Dybcio
> <konrad.dybcio@somainline.org> wrote:
>>
>>
>> On 04/11/2022 15:25, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Fri, Nov 4, 2022 at 2:35 AM Konrad Dybcio
>>> <konrad.dybcio@somainline.org> wrote:
>>>>
>>>> On 04/11/2022 07:19, Sheng-Liang Pan wrote:
>>>>> Change touchpad and touchscreen node for evoker
>>>>> Touchpad: SA461D-1011
>>>>> Touchscreen: GT7986U
>>>> What's the reasoning? Were they changed post r0? Is r0 support
>>>> effectively dropped?
>>>>
>>>> The changes look ok, but I feel like this needs more of a comment in the
>>>> commit msg.
>>> As I understand it r0's toucscreen/touchpad were not right to start
>>> with. We are moving towards getting things upstream sooner and that
>>> means that hardware hasn't always been fully tested out.
>>>
>>> I certainly wouldn't object to a better commit message here, but in
>>> this case there are no real world users (yet) and thus nobody is
>>> really affected by this churn. ...so IMO if the series needs to be
>>> spun for some other reason then the commit message could be updated,
>>> but I wouldn't object to it landing as-is either.
>>
>> If there are no real (read: not-an-internal-devboard) devices using it,
>> then I
>>
>> agree, it's fine to merge as-is.
> 
> I can confirm that. There are no evoker devices in the wild.
> 
> -Doug
In this case:

Reviewed-by: Konrad Dybcio <konrad.dybcio@somainline.org>

Please include explanations like these in the commit message next time.

Konrad
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi
index a6015491c6082..706dd82a70138 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-evoker.dtsi
@@ -23,16 +23,15 @@  ap_tp_i2c: &i2c0 {
 	status = "okay";
 	clock-frequency = <400000>;
 
-	trackpad: trackpad@2c {
-		compatible = "hid-over-i2c";
-		reg = <0x2c>;
+	trackpad: trackpad@15 {
+		compatible = "elan,ekth3000";
+		reg = <0x15>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&tp_int_odl>;
 
 		interrupt-parent = <&tlmm>;
 		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
 
-		hid-descr-addr = <0x20>;
 		vcc-supply = <&pp3300_z1>;
 
 		wakeup-source;
@@ -43,9 +42,9 @@  ts_i2c: &i2c13 {
 	status = "okay";
 	clock-frequency = <400000>;
 
-	ap_ts: touchscreen@10 {
-		compatible = "elan,ekth6915";
-		reg = <0x10>;
+	ap_ts: touchscreen@5d {
+		compatible = "goodix,gt7986u", "goodix,gt7375p";
+		reg = <0x5d>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&ts_int_conn>, <&ts_rst_conn>;
 
@@ -54,7 +53,7 @@  ap_ts: touchscreen@10 {
 
 		reset-gpios = <&tlmm 54 GPIO_ACTIVE_LOW>;
 
-		vcc33-supply = <&ts_avdd>;
+		vdd-supply = <&ts_avdd>;
 	};
 };