Re: [PATCH 4/4] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC

[Konrad Dybcio]

On 4/2/26 2:52 PM, Aleksandrs Vinarskis wrote:
> 
> On Wednesday, April 1st, 2026 at 11:21, Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx> wrote:
> 
>> On 4/1/26 9:33 AM, Aleksandrs Vinarskis wrote:
>>> Describe embedded controller, its interrupt and required thermal zones.
>>> Add EC's reset GPIO to reserved range, as triggering it during device
>>> operation leads to unrecoverable and unusable state.
>>>
>>> Signed-off-by: Aleksandrs Vinarskis <alex@xxxxxxxxxxxxx>
>>> ---
>>
>> [...]
>>
>>> +		io-channels = <&pmk8550_vadc PM8350_ADC7_GPIO3_100K_PU(1)>,
>>> +			      <&pmk8550_vadc PM8350_ADC7_GPIO4_100K_PU(1)>,
>>> +			      <&pmk8550_vadc PM8350_ADC7_AMUX_THM1_100K_PU(1)>,
>>> +			      <&pmk8550_vadc PM8350_ADC7_AMUX_THM2_100K_PU(1)>,
>>> +			      <&pmk8550_vadc PM8350_ADC7_AMUX_THM3_100K_PU(1)>,
>>> +			      <&pmk8550_vadc PM8350_ADC7_AMUX_THM4_100K_PU(1)>,
>>> +			      <&pmk8550_vadc PM8350_ADC7_AMUX_THM5_100K_PU(1)>;
>>> +
>>> +		io-channel-names = "sys_therm0", "sys_therm1", "sys_therm2",
>>> +				   "sys_therm3", "sys_therm4", "sys_therm5",
>>> +				   "sys_therm6";
>>
>> nit: one a line please, without a separating \n between x and x-names
> 
> Will drop \n. One a line as in:
> io-channel-names = "sys_therm0",
>                    "sys_therm1",
>                    "sys_therm2",
>                     ...
> ?

Yes please

[...]

>>>  &tlmm {
>>>  	gpio-reserved-ranges = <44 4>,  /* SPI11 (TPM) */
>>> +			       <65 1>,  /* EC Reset */
>>
>> Is that a "this may not be accessed" or rather "you can, but it has dire
>> consequences"?
> 
> The latter. Triggering EC reset appears to leave it in un-initialized state.
> When analyzing i2c dumps I noticed UEFI sends some data to EC prior to
> Windows driver loading, I am assuming its required for EC configuration.
> When resetting EC from userpsace:
> - Keyboard, Trackpad, touch-row power is out. WiFi connection drops. Dell's
>   UEFI allows disabling many peripherals, EC can 'veto' their resets and/or
>   power supplies. It appears in default reset state it kill some/all outputs
> - Holding power button does not reboot laptop, it looks as if it asserts and
>   holds EC in reset until released. During this time fans spin to max speed.
> - Device can be recovered only by disassembly and battery removal.
> 
>>
>> Would the EC driver/binding benefit from having a reference to that pin?
> 
> It will not be used by the driver, and it would greatly inconvenience user
> if triggered manually. I would make the reset pin as inaccessible as
> possible, but if you say its cleaner to reference it to EC driver and just
> not use it, I could do that as well.

I would assume the EC is powered from some always-on rail, or that it can
at least somehow sustain entry into all the various low power modes and we
won't have to re-initialize it from Linux, but that's only a guess

That said, like you suggest, exposing that pin currently causes more harm
than good and we can always circle back and revert this in the future, should
that become desired, perhaps with the only caveat being that users of old DTs
(i.e. without that description) would not get the ability to reset the EC on
demand

Konrad