Re: [PATCH v2 1/4] dt-bindings: platform: introduce EC for Dell XPS 13 9345

[Aleksandrs Vinarskis]

On Monday, April 6th, 2026 at 10:15, Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> wrote:

> On 05/04/2026 21:50, Aleksandrs Vinarskis wrote:
> > 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.

I managed to find the discussion from Krzysztof I was referring to [1].
My interpretation (please correct me if its wrong):
- Dell did not make MEC5200, so 'dell,mec5200-foo-bar' is not correct.
- Since same IC may (and is) used on other platform, compatible and filename
of the driver should be specific to the platform.

[1] https://lore.kernel.org/all/57515c0f-caa0-4651-96cf-dde75a13937f@xxxxxxxxxx/

> >
> >>
> >>> @@ -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
> 
> To me including the laptop model in the IC name, when our best
> information is that this same chip is used on both Thena variants isn't
> very logical.
> 
> The thing is the IC not the platform it resides on.
> 
> But if you think the firmware is specific to each platform - something
> like dell-mec5200-ec, dell,mec5200-xps13-9345-ec makes sense to me.

Krzysztof could you please advise whats the best way to go about these
compatibles (and driver name, if you have thoughts there)?

Thanks,
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
> >>>
> >>
> >>
> 
>