Re: [PATCH v12 2/7] mfd: Add driver for ASUS Transformer embedded controller

[Lee Jones]

On Tue, 03 Nov 2026, Svyatoslav Ryhel wrote:

> вт, 3 лист. 2026 р. о 13:43 Lee Jones <lee@xxxxxxxxxx> пише:
> >
> > --- checkpatch.pl (4 issues) ---
> > Line 607: WARNING: DT compatible string "asus,sl101-ec-dock" appears un-documented -- check ./Documentation/devicetree/bindings/
> > Line 610: WARNING: DT compatible string "asus,tf101-ec-dock" appears un-documented -- check ./Documentation/devicetree/bindings/
> > Line 613: WARNING: DT compatible string "asus,tf201-ec-pad" appears un-documented -- check ./Documentation/devicetree/bindings/
> > Line 616: WARNING: DT compatible string "asus,tf600t-ec-pad" appears un-documented -- check ./Documentation/devicetree/bindings/
> > ----------------------------------
> >
> > On Mon, 19 Oct 2026, Svyatoslav Ryhel wrote:
> > > From: Michał Mirosław <mirq-linux@xxxxxxxxxxxx>
> > >
> > > Support Nuvoton NPCE795-based ECs as used in Asus Transformer TF201,
> > > TF300T, TF300TG, TF300TL and TF700T pad and dock, as well as TF101 dock
> > > and TF600T, P1801-T and TF701T pad. This is a glue driver handling
> > > detection and common operations for EC's functions.
> > >
> > > Co-developed-by: Svyatoslav Ryhel <clamor95@xxxxxxxxx>
> > > Signed-off-by: Michał Mirosław <mirq-linux@xxxxxxxxxxxx>
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@xxxxxxxxx>
> > > ---
> > >  drivers/mfd/Kconfig                     |  16 +
> > >  drivers/mfd/Makefile                    |   1 +
> > >  drivers/mfd/asus-transformer-ec.c       | 569 ++++++++++++++++++++++++
> > >  include/linux/mfd/asus-transformer-ec.h | 100 +++++
> > >  4 files changed, 686 insertions(+)
> > >  create mode 100644 drivers/mfd/asus-transformer-ec.c
> > >  create mode 100644 include/linux/mfd/asus-transformer-ec.h
> >
> > [...]
> >
> > > +static int asus_ec_reset(struct asus_ec_data *ddata)
> > > +{
> > > +     int retry, ret;
> > > +
> > > +     guard(mutex)(&ddata->ecreq_lock);
> > > +
> > > +     for (retry = 0; retry < ASUSEC_RETRY_MAX; retry++) {
> >
> > As mentioned on v8, 'for (int retry = 0; ...)' is generally preferred for
> > throw-away loop variables.
> >
> 
> I must have overlooked this, appologies.
> 
> > > +             ret = i2c_smbus_write_word_data(ddata->client, ASUSEC_WRITE_BUF,
> > > +                                             ASUSEC_RESET);
> > > +             if (!ret)
> > > +                     return 0;
> > > +
> > > +             msleep(ASUSEC_ACCESS_TIMEOUT);
> > > +     }
> > > +
> > > +     return ret;
> > > +}
> >
> > [...]
> >
> > > +static int asus_ec_probe(struct i2c_client *client)
> > > +{
> > > +     struct device *dev = &client->dev;
> > > +     struct asus_ec_data *ddata;
> > > +     const struct mfd_cell *cells;
> > > +     unsigned int num_cells;
> > > +     unsigned long irqflags;
> > > +     int ret;
> > > +
> > > +     if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
> > > +             return dev_err_probe(dev, -ENXIO,
> > > +                     "I2C bus is missing required SMBus block mode support\n");
> > > +
> > > +     ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> > > +     if (!ddata)
> > > +             return -ENOMEM;
> > > +
> > > +     ddata->info = device_get_match_data(dev);
> > > +     if (!ddata->info)
> > > +             return -ENODEV;
> > > +
> > > +     switch (ddata->info->variant) {
> > > +     case ASUSEC_SL101_DOCK:
> > > +             cells = asus_ec_sl101_dock_mfd_devices;
> > > +             num_cells = ARRAY_SIZE(asus_ec_sl101_dock_mfd_devices);
> > > +             break;
> > > +     case ASUSEC_TF101_DOCK:
> > > +             cells = asus_ec_tf101_dock_mfd_devices;
> > > +             num_cells = ARRAY_SIZE(asus_ec_tf101_dock_mfd_devices);
> > > +             break;
> > > +     case ASUSEC_TF201_PAD:
> > > +             cells = asus_ec_tf201_pad_mfd_devices;
> > > +             num_cells = ARRAY_SIZE(asus_ec_tf201_pad_mfd_devices);
> > > +             break;
> > > +     case ASUSEC_TF600T_PAD:
> > > +             cells = asus_ec_tf600t_pad_mfd_devices;
> > > +             num_cells = ARRAY_SIZE(asus_ec_tf600t_pad_mfd_devices);
> > > +             break;
> > > +     default:
> > > +             return dev_err_probe(dev, -EINVAL,
> > > +                                  "unknown device variant %d\n",
> > > +                                  ddata->info->variant);
> > > +     }
> > > +
> > > +     i2c_set_clientdata(client, ddata);
> > > +     ddata->client = client;
> > > +
> > > +     ddata->ec.dockram = devm_asus_dockram_get(dev);
> > > +     if (IS_ERR(ddata->ec.dockram))
> > > +             return dev_err_probe(dev, PTR_ERR(ddata->ec.dockram),
> > > +                                  "failed to get dockram\n");
> > > +
> > > +     ddata->ecreq_gpio = devm_gpiod_get(dev, "request", GPIOD_OUT_LOW);
> > > +     if (IS_ERR(ddata->ecreq_gpio))
> > > +             return dev_err_probe(dev, PTR_ERR(ddata->ecreq_gpio),
> > > +                                  "failed to get EC request GPIO\n");
> > > +
> > > +     BLOCKING_INIT_NOTIFIER_HEAD(&ddata->ec.notify_list);
> > > +     mutex_init(&ddata->ecreq_lock);
> > > +
> > > +     asus_ec_signal_request(ddata);
> > > +
> > > +     ret = asus_ec_detect(ddata);
> > > +     if (ret)
> > > +             return dev_err_probe(dev, ret, "failed to detect EC version\n");
> > > +
> > > +     ret = asus_ec_init(ddata);
> > > +     if (ret)
> > > +             return dev_err_probe(dev, ret, "failed to init EC\n");
> > > +
> > > +     /*
> > > +      * Systems using device tree should set up interrupt via DTS,
> > > +      * the rest will use the default low interrupt.
> > > +      */
> > > +     irqflags = dev->of_node ? 0 : IRQF_TRIGGER_LOW;
> > > +
> > > +     ret = devm_request_threaded_irq(dev, client->irq, NULL,
> > > +                                     &asus_ec_interrupt,
> > > +                                     IRQF_ONESHOT | irqflags,
> > > +                                     client->name, ddata);
> > > +     if (ret)
> > > +             return dev_err_probe(dev, ret, "failed to register IRQ\n");
> > > +
> > > +     /* Parent I2C controller uses DMA, ASUS EC and child devices do not */
> > > +     client->dev.coherent_dma_mask = 0;
> > > +     client->dev.dma_mask = &client->dev.coherent_dma_mask;
> > > +
> > > +     return devm_mfd_add_devices(dev, 0, cells, num_cells, NULL, 0, NULL);
> >
> > Why not use 'PLATFORM_DEVID_NONE' or 'PLATFORM_DEVID_AUTO' here instead of a raw
> > '0' and hard-coded cell IDs in 'asus_ec_tf101_dock_mfd_devices'?
> >
> 
> There can be more then 1 EC in the tablet. PLATFORM_DEVID_NONE will
> not work in this case and PLATFORM_DEVID_AUTO should work, but one of
> EC is detachable (dock one) so devices there may need a fixed id. I
> can test if PLATFORM_DEVID_AUTO works, but may this be sent as a
> followup?

Sure.

-- 
Lee Jones