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

From: Svyatoslav Ryhel

Date: Tue Nov 03 2026 - 07:09:54 EST


вт, 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?

> > +}
>
> --
> Lee Jones