Re: [PATCH v2 1/4] dt-bindings: platform: introduce EC for Dell XPS 13 9345
From: Aleksandrs Vinarskis
Date: Sun Apr 05 2026 - 16:50:47 EST
On Sunday, April 5th, 2026 at 02:05, Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> wrote:
> On 04/04/2026 13:55, Aleksandrs Vinarskis wrote:
> > Add bindings for Embedded Controller (EC) in Dell XPS 13 9345 (platform
> > codename 'tributo'). It may be partially or fully compatible with EC
> > found in Snapdragon-based Dell Latitude, Inspiron ('thena').
> >
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Aleksandrs Vinarskis <alex@xxxxxxxxxxxxx>
> > ---
> > .../embedded-controller/dell,xps13-9345-ec.yaml | 91 ++++++++++++++++++++++
> > MAINTAINERS | 5 ++
> > 2 files changed, 96 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..e14dbf2f1a6af8cc7511890fbef08c6c717c0aa6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml
>
> I believe the part name of this embedded controller is the "mec5200" so
> instead of calling it dell,xps13-9345-ec suggest "dell,mec5200"
Correct, its Microchip MEC5200. I remember reading some series discussion
about not naming driver after IC its based on, but rather platform its
used for since driver depends on firmware which is platform specific...
cannot find that discussion now.
>
> > @@ -0,0 +1,91 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/embedded-controller/dell,xps13-9345-ec.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Dell XPS 13 9345 Embedded Controller
> > +
> > +maintainers:
> > + - Aleksandrs Vinarskis <alex@xxxxxxxxxxxxx>
> > +
> > +description:
> > + The Dell XPS 13 9345 has an Embedded Controller (EC) which handles thermal
> > + and power management. It is communicating with SoC over multiple i2c busses.
> > + Among other things, it handles fan speed control, thermal shutdown, peripheral
> > + power supply including trackpad, touch-row, display. For these functions, it
> > + requires frequently updated thermal readings from onboard thermistors.
> > +
> > +properties:
> > + compatible:
> > + const: dell,xps13-9345-ec
>
> Ditto the compat - name it after the IC not the laptop its a "mec5200"
> or "mec5200-ec" - I suspect the -ec postfix is a tautology the ec bit in
> "mec" probably captures.
I'm not sure I agree regarding the compatible, its supposed to be as exact as
possible. "dell,mec5200" will not allow us to differentiate between EC drivers
of "tributo" and "thena" for example.
Suggestion:
- Schema filename to be generalized "dell,mec5200-ec.yaml"
- Driver filename to be generalized "dell-mec5200-ec.c"
- Config to be generalized "EC_DELL_MEC5200"
- Compatible to stay specific "dell,xps13-9345-ec", with fallback to
"dell,mec5200-ec".
I would also keep "-ec" to stay consistent with naming convention of
existing drivers and bindings.
Let me know if this would work for you.
Alex
>
> > +
> > + reg:
> > + const: 0x3b
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + io-channels:
> > + description:
> > + ADC channels connected to the 7 onboard thermistors on PMK8550.
> > + EC requires frequent thermal readings of these channels to perform
> > + automated fan speed control.
> > + items:
> > + - description: ADC channel for sys_therm0
> > + - description: ADC channel for sys_therm1
> > + - description: ADC channel for sys_therm2
> > + - description: ADC channel for sys_therm3
> > + - description: ADC channel for sys_therm4
> > + - description: ADC channel for sys_therm5
> > + - description: ADC channel for sys_therm6
> > +
> > + io-channel-names:
> > + items:
> > + - const: sys_therm0
> > + - const: sys_therm1
> > + - const: sys_therm2
> > + - const: sys_therm3
> > + - const: sys_therm4
> > + - const: sys_therm5
> > + - const: sys_therm6
>
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - io-channels
> > + - io-channel-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + #include <dt-bindings/iio/qcom,spmi-adc7-pm8350.h>
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + embedded-controller@3b {
> > + compatible = "dell,xps13-9345-ec";
> > + reg = <0x3b>;
> > + interrupts-extended = <&tlmm 66 IRQ_TYPE_LEVEL_LOW>;
> > +
> > + 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";
> > + };
> > + };
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 96e0781f2201b41b976dfa69efd44d62c4ff0058..a5d175559f4468dfe363b319a1b08d3425f4d712 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7236,6 +7236,11 @@ S: Maintained
> > F: Documentation/ABI/testing/sysfs-class-firmware-attributes
> > F: drivers/platform/x86/dell/dell-wmi-sysman/
> >
> > +DELL XPS EMBEDDED CONTROLLER DRIVER
> > +M: Aleksandrs Vinarskis <alex@xxxxxxxxxxxxx>
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/embedded-controller/dell,xps13-9345-ec.yaml
> > +
> > DELTA AHE-50DC FAN CONTROL MODULE DRIVER
> > M: Zev Weiss <zev@xxxxxxxxxxxxxxxxx>
> > L: linux-hwmon@xxxxxxxxxxxxxxx
> >
>
>