Re: [PATCH v3 1/7] dt-bindings: embedded-controller: document ASUS Transformer EC

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 5221 bytes --]

On Mon, Feb 16, 2026 at 08:22:38PM +0200, Svyatoslav Ryhel wrote:
> пн, 16 лют. 2026 р. о 20:04 Conor Dooley <conor@kernel.org> пише:
> >
> > On Sat, Feb 14, 2026 at 08:09:53PM +0200, Svyatoslav Ryhel wrote:
> > > Document embedded controller used in ASUS Transformer device series.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > >  .../asus,transformer-ec.yaml                  | 98 +++++++++++++++++++
> > >  1 file changed, 98 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/embedded-controller/asus,transformer-ec.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/embedded-controller/asus,transformer-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/asus,transformer-ec.yaml
> > > new file mode 100644
> > > index 000000000000..670c4c2d339d
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/embedded-controller/asus,transformer-ec.yaml
> > > @@ -0,0 +1,98 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/embedded-controller/asus,transformer-ec.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: ASUS Transformer's Embedded Controller
> > > +
> > > +description:
> > > +  Several Nuvoton based Embedded Controllers attached to an I2C bus,
> > > +  running a custom ASUS firmware, specific to the ASUS Transformer
> > > +  device series.
> > > +
> > > +maintainers:
> > > +  - Svyatoslav Ryhel <clamor95@gmail.com>
> > > +
> > > +allOf:
> > > +  - $ref: /schemas/power/supply/power-supply.yaml
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - enum:
> > > +          - asus,p1801-t-ec-pad
> > > +          - asus,sl101-ec-dock
> > > +          - asus,tf600t-ec-pad
> > > +          - asus,tf701t-ec-pad
> > > +
> > > +      - items:
> > > +          - enum:
> > > +              - asus,tf101-ec-dock
> > > +              - asus,tf101g-ec-dock
> > > +              - asus,tf201-ec-dock
> > > +              - asus,tf300t-ec-dock
> > > +              - asus,tf300tg-ec-dock
> > > +              - asus,tf300tl-ec-dock
> > > +              - asus,tf700t-ec-dock
> > > +          - const: asus,transformer-ec-dock
> > > +
> > > +      - items:
> > > +          - enum:
> > > +              - asus,tf201-ec-pad
> > > +              - asus,tf300t-ec-pad
> > > +              - asus,tf300tg-ec-pad
> > > +              - asus,tf300tl-ec-pad
> > > +              - asus,tf700t-ec-pad
> > > +          - const: asus,transformer-ec-pad
> >
> > Can you explain somewhere here what the "dock" and "pad" devices are and
> > how they differ? For example, I am currently wondering why tf700t has both
> > "dock" and "pad".
> 
> "pad" is used for the controller in the tablet itself in the pad
> "dock" is used for the controller in the mobile dock keyboard.
> Seems quite obvious.

To someone intimately familiar with the devices maybe. Not to the people
you have to justify your patches to. Your response implies that this is
some sort of thing that's like a surface, but please put an explicit
explanation into the binding somewhere as to what each compatible is
for.

> > Also, why are some of the compatibles permitted standalone? That should
> > be mentioned in your commit message too. Also, other than the sl101, the
> > standalone ones seem to have the same match data in the mfd driver. Why
> > are fallbacks not made use of there?
> >
> 
> Because standalone compatibles describe a unique hw configuration
> which cannot be grouped into something meaningful. asus,p1801-t-ec-pad
> is for EC of Tegra30/Intel based p1801-t AIO, asus,sl101-ec-dock is
> for EC of Tegra20 slider tablet, asus,tf600t-ec-pad is for altered EC
> in Win8 Tegra30 tablet, asus,tf701t-ec-pad is for Tegra114 tablet.
> Different generations, different form-factors.

I don't see any reasons here that eliminate fallback compatibles.
+	{ .compatible = "asus,p1801-t-ec-pad", .data = &asus_ec_pad_charger_data },
+	{ .compatible = "asus,tf600t-ec-pad", .data = &asus_ec_pad_charger_data },
+	{ .compatible = "asus,tf701t-ec-pad", .data = &asus_ec_pad_charger_data },
+	{ }
Three of them use the same match data, so you need to explain why you've
made these three standalone when all the others that share a programming
model got a generic fallback. Fallback usage is based on programming
model, not based on whether the devices are a physically different, so
your explanation must reflect this.

> > Since this transformer series seems to have multiple programming models
> > for "ec-pad" devices, it calls into question your use of the generic
> > fallback compatibles is appropriate and makes it seem like you should be
> > using device compatibles as a fallback.
> 
> That is redundant.

I don't understand how that is a response to what I said.

> > The rest looks okay other than the filename, which doesn't match any of
> > the compatibles that you've got here.
> >
> 
> How should I call it then?

Make it match a compatible, just like any other binding.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]