2023-11-28 08:46:00

by Chen-Yu Tsai

[permalink] [raw]
Subject: [RFC PATCH v3 4/5] arm64: dts: mediatek: mt8173-elm-hana: Mark touchscreens and trackpads as fail

Instead of having them all available, mark them all as "fail-needs-probe"
and have the implementation try to probe which one is present.

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
Changes since v2:
- Drop class from status
---
arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi b/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi
index bdcd35cecad9..1d68bc6834e4 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173-elm-hana.dtsi
@@ -15,6 +15,7 @@ touchscreen2: touchscreen@34 {
reg = <0x34>;
interrupt-parent = <&pio>;
interrupts = <88 IRQ_TYPE_LEVEL_LOW>;
+ status = "fail-needs-probe";
};

/*
@@ -28,6 +29,7 @@ touchscreen3: touchscreen@20 {
hid-descr-addr = <0x0020>;
interrupt-parent = <&pio>;
interrupts = <88 IRQ_TYPE_LEVEL_LOW>;
+ status = "fail-needs-probe";
};
};

@@ -44,6 +46,7 @@ trackpad2: trackpad@2c {
reg = <0x2c>;
hid-descr-addr = <0x0020>;
wakeup-source;
+ status = "fail-needs-probe";
};
};

@@ -68,3 +71,11 @@ pins_wp {
};
};
};
+
+&touchscreen {
+ status = "fail-needs-probe";
+};
+
+&trackpad {
+ status = "fail-needs-probe";
+};
--
2.43.0.rc1.413.gea7ed67945-goog


2023-12-02 00:59:04

by Doug Anderson

[permalink] [raw]
Subject: Re: [RFC PATCH v3 4/5] arm64: dts: mediatek: mt8173-elm-hana: Mark touchscreens and trackpads as fail

Hi,

On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <[email protected]> wrote:
>
> @@ -44,6 +46,7 @@ trackpad2: trackpad@2c {
> reg = <0x2c>;
> hid-descr-addr = <0x0020>;
> wakeup-source;
> + status = "fail-needs-probe";

While doing this, you could also remove the hack where the trackpad
IRQ pinctrl is listed under i2c4.

2023-12-04 06:59:52

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [RFC PATCH v3 4/5] arm64: dts: mediatek: mt8173-elm-hana: Mark touchscreens and trackpads as fail

On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <[email protected]> wrote:
>
> Hi,
>
> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <[email protected]> wrote:
> >
> > @@ -44,6 +46,7 @@ trackpad2: trackpad@2c {
> > reg = <0x2c>;
> > hid-descr-addr = <0x0020>;
> > wakeup-source;
> > + status = "fail-needs-probe";
>
> While doing this, you could also remove the hack where the trackpad
> IRQ pinctrl is listed under i2c4.

Sure. I do think we can do away with it though. According to at least one
schematic, the interrupt line has pull-ups on both sides of the voltage
shifter.

BTW, The touchscreen doesn't have pinctrl entries. This has pull-ups on
both sides of the voltage shifter as well.

ChenYu

2023-12-04 16:55:49

by Doug Anderson

[permalink] [raw]
Subject: Re: [RFC PATCH v3 4/5] arm64: dts: mediatek: mt8173-elm-hana: Mark touchscreens and trackpads as fail

Hi,

On Sun, Dec 3, 2023 at 10:59 PM Chen-Yu Tsai <[email protected]> wrote:
>
> On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <[email protected]> wrote:
> >
> > Hi,
> >
> > On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <[email protected]> wrote:
> > >
> > > @@ -44,6 +46,7 @@ trackpad2: trackpad@2c {
> > > reg = <0x2c>;
> > > hid-descr-addr = <0x0020>;
> > > wakeup-source;
> > > + status = "fail-needs-probe";
> >
> > While doing this, you could also remove the hack where the trackpad
> > IRQ pinctrl is listed under i2c4.
>
> Sure. I do think we can do away with it though. According to at least one
> schematic, the interrupt line has pull-ups on both sides of the voltage
> shifter.
>
> BTW, The touchscreen doesn't have pinctrl entries. This has pull-ups on
> both sides of the voltage shifter as well.

I dunno if the convention is different on Mediatek boards, but at
least on boards I've been involved with in the past we've always put
pinctrl entries just to make things explicit. This meant that we
didn't rely on the firmware/bootrom/defaults to leave pulls in any
particular state. ...otherwise those external pull-ups could be
fighting with internal pull-downs, right?

-Doug

Subject: Re: [RFC PATCH v3 4/5] arm64: dts: mediatek: mt8173-elm-hana: Mark touchscreens and trackpads as fail

Il 04/12/23 17:50, Doug Anderson ha scritto:
> Hi,
>
> On Sun, Dec 3, 2023 at 10:59 PM Chen-Yu Tsai <[email protected]> wrote:
>>
>> On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <[email protected]> wrote:
>>>
>>> Hi,
>>>
>>> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <[email protected]> wrote:
>>>>
>>>> @@ -44,6 +46,7 @@ trackpad2: trackpad@2c {
>>>> reg = <0x2c>;
>>>> hid-descr-addr = <0x0020>;
>>>> wakeup-source;
>>>> + status = "fail-needs-probe";
>>>
>>> While doing this, you could also remove the hack where the trackpad
>>> IRQ pinctrl is listed under i2c4.
>>
>> Sure. I do think we can do away with it though. According to at least one
>> schematic, the interrupt line has pull-ups on both sides of the voltage
>> shifter.
>>
>> BTW, The touchscreen doesn't have pinctrl entries. This has pull-ups on
>> both sides of the voltage shifter as well.
>
> I dunno if the convention is different on Mediatek boards, but at
> least on boards I've been involved with in the past we've always put
> pinctrl entries just to make things explicit. This meant that we
> didn't rely on the firmware/bootrom/defaults to leave pulls in any
> particular state. ...otherwise those external pull-ups could be
> fighting with internal pull-downs, right?
>

MediaTek boards aren't special and there's no good reason for those to rely on
firmware/bootrom/defaults - so there is no good reason to avoid declaring any
relevant pinctrl entry.

Cheers,
Angelo

2023-12-06 02:58:33

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [RFC PATCH v3 4/5] arm64: dts: mediatek: mt8173-elm-hana: Mark touchscreens and trackpads as fail

On Tue, Dec 5, 2023 at 6:22 PM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Il 04/12/23 17:50, Doug Anderson ha scritto:
> > Hi,
> >
> > On Sun, Dec 3, 2023 at 10:59 PM Chen-Yu Tsai <[email protected]> wrote:
> >>
> >> On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <[email protected]> wrote:
> >>>
> >>> Hi,
> >>>
> >>> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <[email protected]> wrote:
> >>>>
> >>>> @@ -44,6 +46,7 @@ trackpad2: trackpad@2c {
> >>>> reg = <0x2c>;
> >>>> hid-descr-addr = <0x0020>;
> >>>> wakeup-source;
> >>>> + status = "fail-needs-probe";
> >>>
> >>> While doing this, you could also remove the hack where the trackpad
> >>> IRQ pinctrl is listed under i2c4.
> >>
> >> Sure. I do think we can do away with it though. According to at least one
> >> schematic, the interrupt line has pull-ups on both sides of the voltage
> >> shifter.
> >>
> >> BTW, The touchscreen doesn't have pinctrl entries. This has pull-ups on
> >> both sides of the voltage shifter as well.
> >
> > I dunno if the convention is different on Mediatek boards, but at
> > least on boards I've been involved with in the past we've always put
> > pinctrl entries just to make things explicit. This meant that we
> > didn't rely on the firmware/bootrom/defaults to leave pulls in any
> > particular state. ...otherwise those external pull-ups could be
> > fighting with internal pull-downs, right?
> >
>
> MediaTek boards aren't special and there's no good reason for those to rely on
> firmware/bootrom/defaults - so there is no good reason to avoid declaring any
> relevant pinctrl entry.

I think this should be migrated to use the proper GPIO bindings: the
GPIO_PULL_UP / GPIO_PULL_DOWN / GPIO_BIAS_DISABLE flags.

But that's a different discussion.

ChenYu

Subject: Re: [RFC PATCH v3 4/5] arm64: dts: mediatek: mt8173-elm-hana: Mark touchscreens and trackpads as fail

Il 06/12/23 03:55, Chen-Yu Tsai ha scritto:
> On Tue, Dec 5, 2023 at 6:22 PM AngeloGioacchino Del Regno
> <[email protected]> wrote:
>>
>> Il 04/12/23 17:50, Doug Anderson ha scritto:
>>> Hi,
>>>
>>> On Sun, Dec 3, 2023 at 10:59 PM Chen-Yu Tsai <[email protected]> wrote:
>>>>
>>>> On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <[email protected]> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <[email protected]> wrote:
>>>>>>
>>>>>> @@ -44,6 +46,7 @@ trackpad2: trackpad@2c {
>>>>>> reg = <0x2c>;
>>>>>> hid-descr-addr = <0x0020>;
>>>>>> wakeup-source;
>>>>>> + status = "fail-needs-probe";
>>>>>
>>>>> While doing this, you could also remove the hack where the trackpad
>>>>> IRQ pinctrl is listed under i2c4.
>>>>
>>>> Sure. I do think we can do away with it though. According to at least one
>>>> schematic, the interrupt line has pull-ups on both sides of the voltage
>>>> shifter.
>>>>
>>>> BTW, The touchscreen doesn't have pinctrl entries. This has pull-ups on
>>>> both sides of the voltage shifter as well.
>>>
>>> I dunno if the convention is different on Mediatek boards, but at
>>> least on boards I've been involved with in the past we've always put
>>> pinctrl entries just to make things explicit. This meant that we
>>> didn't rely on the firmware/bootrom/defaults to leave pulls in any
>>> particular state. ...otherwise those external pull-ups could be
>>> fighting with internal pull-downs, right?
>>>
>>
>> MediaTek boards aren't special and there's no good reason for those to rely on
>> firmware/bootrom/defaults - so there is no good reason to avoid declaring any
>> relevant pinctrl entry.
>
> I think this should be migrated to use the proper GPIO bindings: the
> GPIO_PULL_UP / GPIO_PULL_DOWN / GPIO_BIAS_DISABLE flags.
>
> But that's a different discussion.
>

100% agreed.

Cheers,
Angelo

2023-12-06 17:01:21

by Doug Anderson

[permalink] [raw]
Subject: Re: [RFC PATCH v3 4/5] arm64: dts: mediatek: mt8173-elm-hana: Mark touchscreens and trackpads as fail

Hi,

On Wed, Dec 6, 2023 at 2:02 AM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Il 06/12/23 03:55, Chen-Yu Tsai ha scritto:
> > On Tue, Dec 5, 2023 at 6:22 PM AngeloGioacchino Del Regno
> > <[email protected]> wrote:
> >>
> >> Il 04/12/23 17:50, Doug Anderson ha scritto:
> >>> Hi,
> >>>
> >>> On Sun, Dec 3, 2023 at 10:59 PM Chen-Yu Tsai <[email protected]> wrote:
> >>>>
> >>>> On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <[email protected]> wrote:
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <[email protected]> wrote:
> >>>>>>
> >>>>>> @@ -44,6 +46,7 @@ trackpad2: trackpad@2c {
> >>>>>> reg = <0x2c>;
> >>>>>> hid-descr-addr = <0x0020>;
> >>>>>> wakeup-source;
> >>>>>> + status = "fail-needs-probe";
> >>>>>
> >>>>> While doing this, you could also remove the hack where the trackpad
> >>>>> IRQ pinctrl is listed under i2c4.
> >>>>
> >>>> Sure. I do think we can do away with it though. According to at least one
> >>>> schematic, the interrupt line has pull-ups on both sides of the voltage
> >>>> shifter.
> >>>>
> >>>> BTW, The touchscreen doesn't have pinctrl entries. This has pull-ups on
> >>>> both sides of the voltage shifter as well.
> >>>
> >>> I dunno if the convention is different on Mediatek boards, but at
> >>> least on boards I've been involved with in the past we've always put
> >>> pinctrl entries just to make things explicit. This meant that we
> >>> didn't rely on the firmware/bootrom/defaults to leave pulls in any
> >>> particular state. ...otherwise those external pull-ups could be
> >>> fighting with internal pull-downs, right?
> >>>
> >>
> >> MediaTek boards aren't special and there's no good reason for those to rely on
> >> firmware/bootrom/defaults - so there is no good reason to avoid declaring any
> >> relevant pinctrl entry.
> >
> > I think this should be migrated to use the proper GPIO bindings: the
> > GPIO_PULL_UP / GPIO_PULL_DOWN / GPIO_BIAS_DISABLE flags.
> >
> > But that's a different discussion.
> >
>
> 100% agreed.

I guess I'd need to see patches as an example to see how this looks,
but I'm at least slightly skeptical. In this case the GPIO is
indirectly specified via "interrupts". Would you add these flags to
the interrupts specifier, too? There's another potential pull as well
(PIN_CONFIG_BIAS_BUS_HOLD) as well as other pin configuration
(PIN_CONFIG_INPUT_DEBOUNCE, for instance). Do we try to fit all of
these into the GPIO / interrupt specifier?

Certainly I will admit that this is a complicated topic, but it seems
weird to say that we use pinctrl to specify pin configuration / pulls
for all pins except ones that are configured as GPIOs or GPIO
interrupts.

-Doug