linux-kernel - Re: [PATCH] arm64: dts: qcom: Add touchscreen for pazquel360

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 20 Sep 2022 08:00:50 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Yunlong Jia <ecs.beijing2022@...il.com>,
        Bjorn Andersson <andersson@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Henry Sun <henrysun@...gle.com>,
        Yunlong Jia <yunlong.jia@....com.tw>,
        Bob Moragues <moragues@...omium.org>,
        Andy Gross <agross@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Dmitry Torokhov <dtor@...omium.org>
Subject: Re: [PATCH] arm64: dts: qcom: Add touchscreen for pazquel360

Hi,

On Thu, Sep 15, 2022 at 9:04 PM Yunlong Jia <ecs.beijing2022@...il.com> wrote:
>
> The model used is elan ekth3915, compatible with ekth3500.

If I wanted to be very purist about this I'd probably argue that we
should include both compatibles, the true one and the fallback one,
AKA:

compatible = "elan,ekth3915", "elan,ekth3500";

...which would, of course, require a bindings update as well. In
practice everyone that uses the ekth3500 compatible is ignoring this
and it seems unlikely to make a huge difference. As long as the power
sequencing is fine then the elan driver can discover and account for
any differences at runtime. Thus, I won't insist.

> Signed-off-by: Yunlong Jia <ecs.beijing2022@...il.com>
>
> ---
>
>  .../dts/qcom/sc7180-trogdor-pazquel360.dtsi   | 22 +++++++++++++++++++

The file you're modifying doesn't exist yet. Bjorn landed a bunch of
patches recently but he seems to have missed yours, AKA the two
patches that include:

https://lore.kernel.org/r/20220901024827.v3.2.Iea2d2918adfff2825b87d428b5732717425c196f@changeid/

You should have mentioned this dependency "after the cut". Since you
use patman that would involve using "Commit-notes:" to mention the
dependency.

Another (probably better) option would have just been to send a v4 of
your unlanded series and add the touchscreen in there.

>  1 file changed, 22 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel360.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel360.dtsi
> index 5702325d0c7b..14ea94ce90c1 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel360.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel360.dtsi
> @@ -14,6 +14,28 @@
>         realtek,dmic-clk-rate-hz = <2048000>;
>  };
>
> +ap_ts_pen_1v8: &i2c4 {
> +       status = "okay";
> +       clock-frequency = <400000>;
> +
> +       ap_ts: touchscreen@10 {
> +               compatible = "elan,ekth3500";
> +               reg = <0x10>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&ts_int_l>, <&ts_reset_l>;
> +
> +               interrupt-parent = <&tlmm>;
> +               interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
> +
> +               hid-descr-addr = <0x0001>;
> +
> +               vcc33-supply = <&pp3300_ts>;
> +               vccio-supply = <&pp1800_l10a>;
> +               reset-gpios = <&tlmm 8 GPIO_ACTIVE_LOW>;
> +       };
> +};
> +
> +
>  &keyboard_controller {

nit: Only one blank line between nodes here, not two. Please spin the
patch for this. Once you do then feel free to add my Reviewed-by tag.

So in summary you have two options and I'm OK with either:

1. Spin a v4 of your original "pazquel360" patch series and just
squash this one into it.

2. Leave the original "pazquel360" patch series alone and send a v2 of
this patch series. In your v2 you'd remove the extra blank line and
use "Commit-notes:" to mention your dependency on the other series.

I suspect Bjorn would prefer #1 but he's free to correct me.