Re: [PATCH v1] watchdog: ni903x_wdt: Convert to a platform driver

[Rafael J. Wysocki]

On Tue, Mar 17, 2026 at 4:13 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> On 3/14/26 04:53, Rafael J. Wysocki wrote:
> > From: "Rafael J. Wysocki" <rafael.j.wysocki@xxxxxxxxx>
> >
> > In all cases in which a struct acpi_driver is used for binding a driver
> > to an ACPI device object, a corresponding platform device is created by
> > the ACPI core and that device is regarded as a proper representation of
> > underlying hardware.  Accordingly, a struct platform_driver should be
> > used by driver code to bind to that device.  There are multiple reasons
> > why drivers should not bind directly to ACPI device objects [1].
> >
> > In particular, registering a watchdog device under a struct acpi_device
> > is questionable because it causes the watchdog to be hidden in the ACPI
> > bus sysfs hierarchy and it goes against the general rule that a struct
> > acpi_device can only be a parent of another struct acpi_device.
> >
> > Overall, it is better to bind drivers to platform devices than to their
> > ACPI companions, so convert the ni903x_wdt watchdog ACPI driver to a
> > platform one.
> >
> > While this is not expected to alter functionality, it changes sysfs
> > layout and so it will be visible to user space.
> >
> > Note that after this change it actually makes sense to look for
> > the "timeout-sec" property via device_property_read_u32() under the
> > device passed to watchdog_init_timeout() because it has an fwnode
> > handle (unlike a struct acpi_device which is an fwnode itself).
> >
> > Link: https://lore.kernel.org/all/2396510.ElGaqSPkdT@rafael.j.wysocki/ [1]
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>

Unless anyone has any objections or concerns, I'm going to queue up
this patch for 7.1.

Thanks!

> > ---
> >   drivers/watchdog/ni903x_wdt.c | 27 ++++++++++++++-------------
> >   1 file changed, 14 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/watchdog/ni903x_wdt.c b/drivers/watchdog/ni903x_wdt.c
> > index 045bb72d9a43..8b1b9baa914e 100644
> > --- a/drivers/watchdog/ni903x_wdt.c
> > +++ b/drivers/watchdog/ni903x_wdt.c
> > @@ -8,6 +8,7 @@
> >   #include <linux/interrupt.h>
> >   #include <linux/io.h>
> >   #include <linux/module.h>
> > +#include <linux/platform_device.h>
> >   #include <linux/watchdog.h>
> >
> >   #define NIWD_CONTROL        0x01
> > @@ -177,9 +178,9 @@ static const struct watchdog_ops ni903x_wdd_ops = {
> >       .get_timeleft = ni903x_wdd_get_timeleft,
> >   };
> >
> > -static int ni903x_acpi_add(struct acpi_device *device)
> > +static int ni903x_acpi_probe(struct platform_device *pdev)
> >   {
> > -     struct device *dev = &device->dev;
> > +     struct device *dev = &pdev->dev;
> >       struct watchdog_device *wdd;
> >       struct ni903x_wdt *wdt;
> >       acpi_status status;
> > @@ -189,10 +190,10 @@ static int ni903x_acpi_add(struct acpi_device *device)
> >       if (!wdt)
> >               return -ENOMEM;
> >
> > -     device->driver_data = wdt;
> > +     platform_set_drvdata(pdev, wdt);
> >       wdt->dev = dev;
> >
> > -     status = acpi_walk_resources(device->handle, METHOD_NAME__CRS,
> > +     status = acpi_walk_resources(ACPI_HANDLE(dev), METHOD_NAME__CRS,
> >                                    ni903x_resources, wdt);
> >       if (ACPI_FAILURE(status) || wdt->io_base == 0) {
> >               dev_err(dev, "failed to get resources\n");
> > @@ -224,9 +225,9 @@ static int ni903x_acpi_add(struct acpi_device *device)
> >       return 0;
> >   }
> >
> > -static void ni903x_acpi_remove(struct acpi_device *device)
> > +static void ni903x_acpi_remove(struct platform_device *pdev)
> >   {
> > -     struct ni903x_wdt *wdt = acpi_driver_data(device);
> > +     struct ni903x_wdt *wdt = platform_get_drvdata(pdev);
> >
> >       ni903x_wdd_stop(&wdt->wdd);
> >       watchdog_unregister_device(&wdt->wdd);
> > @@ -238,16 +239,16 @@ static const struct acpi_device_id ni903x_device_ids[] = {
> >   };
> >   MODULE_DEVICE_TABLE(acpi, ni903x_device_ids);
> >
> > -static struct acpi_driver ni903x_acpi_driver = {
> > -     .name = NIWD_NAME,
> > -     .ids = ni903x_device_ids,
> > -     .ops = {
> > -             .add = ni903x_acpi_add,
> > -             .remove = ni903x_acpi_remove,
> > +static struct platform_driver ni903x_acpi_driver = {
> > +     .probe = ni903x_acpi_probe,
> > +     .remove = ni903x_acpi_remove,
> > +     .driver = {
> > +             .name = NIWD_NAME,
> > +             .acpi_match_table = ni903x_device_ids,
> >       },
> >   };
> >
> > -module_acpi_driver(ni903x_acpi_driver);
> > +module_platform_driver(ni903x_acpi_driver);
> >
> >   MODULE_DESCRIPTION("NI 903x Watchdog");
> >   MODULE_AUTHOR("Jeff Westfahl <jeff.westfahl@xxxxxx>");
>