| Message ID | 20221104141515.v10.3.I3ac715e729f6f9b5a3e3001b155df4f9d14e6186@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add LTE SKU for sc7280-evoker family | expand |
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>; > }; > }; >
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
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
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
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 --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>; }; };