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

[Doug Anderson]

Hi,

On Thu, Sep 15, 2022 at 9:04 PM Yunlong Jia <ecs.beijing2022@xxxxxxxxx> 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@xxxxxxxxx>
>
> ---
>
>  .../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.