Re: [PATCH 1/5] mfd: Add support for IO functions of AAEON devices

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

+Cc: Hans (dunno if it's something you would like to be informed of)

On Tue, May 25, 2021 at 8:42 AM <aaeon.asus@xxxxxxxxx> wrote:
>
> From: Kunyang_Fan <kunyang_fan@xxxxxxxx>
>
> This adds the supports for multiple IO functions of the
> AAEON x86 devices and makes use of the WMI interface to
> control the these IO devices including:
>
> - GPIO
> - LED
> - Watchdog
> - HWMON
>
> It also adds the mfd child device drivers to support
> the above IO functions.

Do I miss the cover letter?

...

> +config MFD_AAEON
> +       tristate "AAEON WMI MFD devices"
> +       depends on ASUS_WMI
> +       help
> +         Say yes here to support mltiple IO devices on Single Board Computers

multiple

> +         produced by AAEON.
> +
> +         This driver leverages the ASUS WMI interface to access device
> +         resources.

I'm wondering should it be some kind of WMI framework part to bridge
WMI parts to MFD or so?

...

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.

Please, drop all these duplications in all files. SPDX is enough.

...

> +static int aaeon_wmi_probe(struct wmi_device *wdev, const void *context)
> +{
> +       struct aaeon_wmi_priv *priv;

       struct aaeon_wmi_priv *priv = context;

> +       if (!wmi_has_guid(AAEON_WMI_MGMT_GUID)) {
> +               dev_info(&wdev->dev, "AAEON Management GUID not found\n");
> +               return -ENODEV;
> +       }

Dead code?

> +       priv = (struct aaeon_wmi_priv *)context;

See above.

> +       dev_set_drvdata(&wdev->dev, priv);
> +
> +       return devm_mfd_add_devices(&wdev->dev, 0, priv->cells,
> +                                   priv->ncells, NULL, 0, NULL);
> +}

...

> +static struct wmi_driver aaeon_wmi_driver = {
> +       .driver = {
> +               .name = "mfd-aaeon",
> +       },
> +       .id_table = aaeon_wmi_id_table,
> +       .probe = aaeon_wmi_probe,
> +};

> +

Redundant blank line.

> +module_wmi_driver(aaeon_wmi_driver);

-- 
With Best Regards,
Andy Shevchenko