Hi, On Tue, May 24, 2022 at 2:13 PM Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx> wrote: > > > On 24/05/2022 22:48, Douglas Anderson wrote: > > This adds the touchscreen to the sc7280-herobrine-villager device > > tree. Note that the touchscreen on villager actually uses the reset > > line and thus we use the more specific "elan,ekth6915" compatible > > which allows us to specify the reset. > > > > The fact that villager's touchscreen uses the reset line can be > > contrasted against the touchscreen for CRD/herobrine-r1. On those > > boards, even though the touchscreen goes to the display, it's not > > hooked up to anything there. > > > > In order to keep the line parked on herobrine/CRD, we'll move the > > pullup from the qcard.dtsi file to the specific boards. This allows us > > to disable the pullup in the villager device tree since the pin is an > > output. > > > > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > > --- > > This uses bindings introduced in the patch ("dt-bindings: HID: > > i2c-hid: elan: Introduce bindings for Elan eKTH6915") [1]. > > > > [1] https://lore.kernel.org/r/20220523142257.v2.1.Iedc61f9ef220a89af6a031200a7850a27a440134@changeid > > > > .../boot/dts/qcom/sc7280-herobrine-crd.dts | 11 ++++++++ > > .../qcom/sc7280-herobrine-herobrine-r1.dts | 11 ++++++++ > > .../dts/qcom/sc7280-herobrine-villager-r0.dts | 25 +++++++++++++++++++ > > arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 1 - > > 4 files changed, 47 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts > > index a4ac33c4fd59..b79d84d7870a 100644 > > --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts > > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine-crd.dts > > @@ -134,6 +134,17 @@ &sdhc_2 { > > status = "okay"; > > }; > > > > +/* PINCTRL - ADDITIONS TO NODES IN PARENT DEVICE TREE FILES */ > > Please drop this line, this isn't msm-3.4. It's immediately obvious that > if a pin is referenced by a label and it is not defined in this file > (because otherwise it wouldn't be both defined and referenced here..), > it comes from a previously included device tree. In general these headings specify a change in sort ordering. Without them then either we intersperse pinctrl overrides with other stuff, which IMO is overall worse or people have no idea why the sort ordering changes. > > @@ -604,7 +604,6 @@ ts_int_conn: ts-int-conn { > > ts_rst_conn: ts-rst-conn { > > pins = "gpio54"; > > function = "gpio"; > > - bias-pull-up; > > If you overwrite it where it should be overwritten, wouldn't it make > more sense to leave bias-pull-up here as a default configuration for > boards that don't have a peculiar routed-but-NC line? Yeah, it'd be nice. ...but because of the way "bias" is specified in the device tree it means an ugly "/delete-property" in places that actually route the line. I believe that, style wise, the preference is to avoid delete-property and move the bias toward board files in cases like this. Bjorn can feel free to override me if he disagrees. -Doug