[PATCH v2 1/2] watchdog: f71808e_wdt: refactor to platform device/driver pair
From: Ahmad Fatoum
Date: Tue Oct 20 2020 - 02:21:42 EST
Driver so far wasn't ported to the driver model and set up its
miscdevice out of the init after probing the I/O ports for a watchdog
with correct vendor and device revision.
Keep the device detection part at init time, but move watchdog setup
to a platform driver probe function.
While at it, refactor some of the driver code we have to now touch
anyway:
- platform_device_id is used instead of the two big switches mapping
hardware ID to an enum and then mapping it to a pinconf function
- we rename f71808e_ and watchdog_data to fintek_wdt, to avoid mix up
of the generic parts up with the device specific parts
Suggested-by: Guenter Roeck <linux@xxxxxxxxxxxx>
Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
---
drivers/watchdog/f71808e_wdt.c | 377 +++++++++++++++++++--------------
1 file changed, 215 insertions(+), 162 deletions(-)
diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index f60beec1bbae..4ff7a2509125 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -9,12 +9,15 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/err.h>
+#include <linux/slab.h>
#include <linux/fs.h>
#include <linux/init.h>
#include <linux/io.h>
#include <linux/ioport.h>
#include <linux/miscdevice.h>
#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/mod_devicetable.h>
#include <linux/mutex.h>
#include <linux/notifier.h>
#include <linux/reboot.h>
@@ -110,22 +113,6 @@ module_param(start_withtimeout, uint, 0);
MODULE_PARM_DESC(start_withtimeout, "Start watchdog timer on module load with"
" given initial timeout. Zero (default) disables this feature.");
-enum chips { f71808fg, f71858fg, f71862fg, f71868, f71869, f71882fg, f71889fg,
- f81803, f81865, f81866};
-
-static const char *f71808e_names[] = {
- "f71808fg",
- "f71858fg",
- "f71862fg",
- "f71868",
- "f71869",
- "f71882fg",
- "f71889fg",
- "f81803",
- "f81865",
- "f81866",
-};
-
/* Super-I/O Function prototypes */
static inline int superio_inb(int base, int reg);
static inline int superio_inw(int base, int reg);
@@ -136,9 +123,17 @@ static inline int superio_enter(int base);
static inline void superio_select(int base, int ld);
static inline void superio_exit(int base);
-struct watchdog_data {
+struct fintek_wdt;
+
+struct fintek_wdt_variant {
+ u16 id;
+ void (*pinconf)(struct fintek_wdt *wd);
+ const char *identity_override;
+};
+
+struct fintek_wdt {
unsigned short sioaddr;
- enum chips type;
+ const struct fintek_wdt_variant *variant;
unsigned long opened;
struct mutex lock;
char expect_close;
@@ -152,10 +147,15 @@ struct watchdog_data {
char caused_reboot; /* last reboot was by the watchdog */
};
-static struct watchdog_data watchdog = {
+static struct fintek_wdt watchdog = {
.lock = __MUTEX_INITIALIZER(watchdog.lock),
};
+static inline bool has_f81865_wdo_conf(struct fintek_wdt *wd)
+{
+ return wd->variant->id == SIO_F81865_ID || wd->variant->id == SIO_F81866_ID;
+}
+
/* Super I/O functions */
static inline int superio_inb(int base, int reg)
{
@@ -247,7 +247,7 @@ static int watchdog_set_pulse_width(unsigned int pw)
int err = 0;
unsigned int t1 = 25, t2 = 125, t3 = 5000;
- if (watchdog.type == f71868) {
+ if (watchdog.variant->id == SIO_F71868_ID) {
t1 = 30;
t2 = 150;
t3 = 6000;
@@ -309,7 +309,6 @@ static int watchdog_keepalive(void)
static int watchdog_start(void)
{
int err;
- u8 tmp;
/* Make sure we don't die as soon as the watchdog is enabled below */
err = watchdog_keepalive();
@@ -323,81 +322,12 @@ static int watchdog_start(void)
superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
/* Watchdog pin configuration */
- switch (watchdog.type) {
- case f71808fg:
- /* Set pin 21 to GPIO23/WDTRST#, then to WDTRST# */
- superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT2, 3);
- superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT3, 3);
- break;
-
- case f71862fg:
- if (f71862fg_pin == 63) {
- /* SPI must be disabled first to use this pin! */
- superio_clear_bit(watchdog.sioaddr, SIO_REG_ROM_ADDR_SEL, 6);
- superio_set_bit(watchdog.sioaddr, SIO_REG_MFUNCT3, 4);
- } else if (f71862fg_pin == 56) {
- superio_set_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 1);
- }
- break;
-
- case f71868:
- case f71869:
- /* GPIO14 --> WDTRST# */
- superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 4);
- break;
-
- case f71882fg:
- /* Set pin 56 to WDTRST# */
- superio_set_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 1);
- break;
-
- case f71889fg:
- /* set pin 40 to WDTRST# */
- superio_outb(watchdog.sioaddr, SIO_REG_MFUNCT3,
- superio_inb(watchdog.sioaddr, SIO_REG_MFUNCT3) & 0xcf);
- break;
-
- case f81803:
- /* Enable TSI Level register bank */
- superio_clear_bit(watchdog.sioaddr, SIO_REG_CLOCK_SEL, 3);
- /* Set pin 27 to WDTRST# */
- superio_outb(watchdog.sioaddr, SIO_REG_TSI_LEVEL_SEL, 0x5f &
- superio_inb(watchdog.sioaddr, SIO_REG_TSI_LEVEL_SEL));
- break;
-
- case f81865:
- /* Set pin 70 to WDTRST# */
- superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT3, 5);
- break;
-
- case f81866:
- /*
- * GPIO1 Control Register when 27h BIT3:2 = 01 & BIT0 = 0.
- * The PIN 70(GPIO15/WDTRST) is controlled by 2Ch:
- * BIT5: 0 -> WDTRST#
- * 1 -> GPIO15
- */
- tmp = superio_inb(watchdog.sioaddr, SIO_F81866_REG_PORT_SEL);
- tmp &= ~(BIT(3) | BIT(0));
- tmp |= BIT(2);
- superio_outb(watchdog.sioaddr, SIO_F81866_REG_PORT_SEL, tmp);
-
- superio_clear_bit(watchdog.sioaddr, SIO_F81866_REG_GPIO1, 5);
- break;
-
- default:
- /*
- * 'default' label to shut up the compiler and catch
- * programmer errors
- */
- err = -ENODEV;
- goto exit_superio;
- }
+ watchdog.variant->pinconf(&watchdog);
superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
superio_set_bit(watchdog.sioaddr, SIO_REG_ENABLE, 0);
- if (watchdog.type == f81865 || watchdog.type == f81866)
+ if (has_f81865_wdo_conf(&watchdog))
superio_set_bit(watchdog.sioaddr, F81865_REG_WDO_CONF,
F81865_FLAG_WDOUT_EN);
else
@@ -425,7 +355,6 @@ static int watchdog_start(void)
F71808FG_FLAG_WD_PULSE);
}
-exit_superio:
superio_exit(watchdog.sioaddr);
exit_unlock:
mutex_unlock(&watchdog.lock);
@@ -665,21 +594,39 @@ static struct notifier_block watchdog_notifier = {
.notifier_call = watchdog_notify_sys,
};
-static int __init watchdog_init(int sioaddr)
+static int fintek_wdt_probe(struct platform_device *pdev)
{
+ const struct platform_device_id *id;
+ unsigned short sioaddr;
+ struct resource *res;
int wdt_conf, err = 0;
+ const char *identity;
+
+ if (watchdog_miscdev.parent)
+ return -EBUSY;
+
+ watchdog_miscdev.parent = &pdev->dev;
+
+ res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ if (!res)
+ return -ENXIO;
+
+ id = platform_get_device_id(pdev);
+ watchdog.variant = (void *)id->driver_data;
+ if (!watchdog.variant)
+ return -ENODEV;
/* No need to lock watchdog.lock here because no entry points
* into the module have been registered yet.
*/
- watchdog.sioaddr = sioaddr;
+ watchdog.sioaddr = res->start;
watchdog.ident.options = WDIOF_MAGICCLOSE
| WDIOF_KEEPALIVEPING
| WDIOF_CARDRESET;
- snprintf(watchdog.ident.identity,
- sizeof(watchdog.ident.identity), "%s watchdog",
- f71808e_names[watchdog.type]);
+ identity = watchdog.variant->identity_override ?: id->name;
+
+ memcpy(watchdog.ident.identity, identity, sizeof(watchdog.ident.identity));
err = superio_enter(sioaddr);
if (err)
@@ -772,73 +719,176 @@ static int __init watchdog_init(int sioaddr)
return err;
}
-static int __init f71808e_find(int sioaddr)
+static void f71808fg_pinconf(struct fintek_wdt *wd)
{
+ /* Set pin 21 to GPIO23/WDTRST#, then to WDTRST# */
+ superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT2, 3);
+ superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 3);
+}
+
+static void f71862fg_pinconf(struct fintek_wdt *wd)
+{
+ if (f71862fg_pin == 63) {
+ /* SPI must be disabled first to use this pin! */
+ superio_clear_bit(wd->sioaddr, SIO_REG_ROM_ADDR_SEL, 6);
+ superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT3, 4);
+ } else if (f71862fg_pin == 56) {
+ superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
+ }
+}
+
+static void f71868_pinconf(struct fintek_wdt *wd)
+{
+ /* GPIO14 --> WDTRST# */
+ superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT1, 4);
+}
+
+static void f71882fg_pinconf(struct fintek_wdt *wd)
+{
+ /* Set pin 56 to WDTRST# */
+ superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
+}
+
+static void f71889fg_pinconf(struct fintek_wdt *wd)
+{
+ /* set pin 40 to WDTRST# */
+ superio_outb(wd->sioaddr, SIO_REG_MFUNCT3,
+ superio_inb(wd->sioaddr, SIO_REG_MFUNCT3) & 0xcf);
+}
+
+static void f81803_pinconf(struct fintek_wdt *wd)
+{
+ /* Enable TSI Level register bank */
+ superio_clear_bit(wd->sioaddr, SIO_REG_CLOCK_SEL, 3);
+ /* Set pin 27 to WDTRST# */
+ superio_outb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL, 0x5f &
+ superio_inb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL));
+}
+
+static void f81865_pinconf(struct fintek_wdt *wd)
+{
+ /* Set pin 70 to WDTRST# */
+ superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 5);
+}
+
+static void f81866_pinconf(struct fintek_wdt *wd)
+{
+ u8 tmp;
+ /*
+ * GPIO1 Control Register when 27h BIT3:2 = 01 & BIT0 = 0.
+ * The PIN 70(GPIO15/WDTRST) is controlled by 2Ch:
+ * BIT5: 0 -> WDTRST#
+ * 1 -> GPIO15
+ */
+ tmp = superio_inb(wd->sioaddr, SIO_F81866_REG_PORT_SEL);
+ tmp &= ~(BIT(3) | BIT(0));
+ tmp |= BIT(2);
+ superio_outb(wd->sioaddr, SIO_F81866_REG_PORT_SEL, tmp);
+
+ superio_clear_bit(wd->sioaddr, SIO_F81866_REG_GPIO1, 5);
+}
+
+#define FINTEK_DRV_DATA(...) \
+ (kernel_ulong_t)(&(struct fintek_wdt_variant) { __VA_ARGS__ })
+
+/*
+ * Array is used by both platform bus to map name to driver data and by
+ * init below to map id in driver_data to name for device creation.
+ */
+static const struct platform_device_id fintek_wdt_id_table[] = {
+ { "f71808fg watchdog", FINTEK_DRV_DATA(SIO_F71808_ID, f71808fg_pinconf) },
+ { "f71862fg watchdog", FINTEK_DRV_DATA(SIO_F71862_ID, f71862fg_pinconf) },
+ { "f71868 watchdog", FINTEK_DRV_DATA(SIO_F71868_ID, f71868_pinconf) },
+ { "f71869 watchdog", FINTEK_DRV_DATA(SIO_F71869_ID, f71868_pinconf) },
+ /* For backwards-compatibility, we define a different watchdog identity name */
+ { "f71869a watchdog", FINTEK_DRV_DATA(SIO_F71869A_ID, f71868_pinconf,
+ .identity_override = "f71869 watchdog") },
+ { "f71882fg watchdog", FINTEK_DRV_DATA(SIO_F71882_ID, f71882fg_pinconf) },
+ { "f71889fg watchdog", FINTEK_DRV_DATA(SIO_F71889_ID, f71889fg_pinconf) },
+ { "f81803 watchdog", FINTEK_DRV_DATA(SIO_F81803_ID, f81803_pinconf) },
+ { "f81865 watchdog", FINTEK_DRV_DATA(SIO_F81865_ID, f81865_pinconf) },
+ { "f81866 watchdog", FINTEK_DRV_DATA(SIO_F81866_ID, f81866_pinconf) },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, fintek_wdt_id_table);
+
+/* Confirmed (by datasheet) not to have a watchdog.
+ * We list this here anyway to maintain backwards-compatibility in not warning about them
+ */
+static const u16 fintek_no_wdt_table[] = { SIO_F71858_ID, 0 };
+
+static int fintek_wdt_remove(struct platform_device *pdev)
+{
+ if (watchdog_is_running()) {
+ pr_warn("Watchdog timer still running, stopping it\n");
+ watchdog_stop();
+ }
+ misc_deregister(&watchdog_miscdev);
+ unregister_reboot_notifier(&watchdog_notifier);
+ return 0;
+}
+
+static struct platform_driver fintek_wdt_driver = {
+ .probe = fintek_wdt_probe,
+ .remove = fintek_wdt_remove,
+ .id_table = fintek_wdt_id_table,
+ .driver = {
+ .name = DRVNAME,
+ },
+};
+
+static struct platform_device *fintek_wdt_pdev;
+
+static __init const char *fintek_wdt_find(int sioaddr)
+{
+ const char *name = ERR_PTR(-ENODEV);
+ const struct platform_device_id *id;
+ struct fintek_wdt_variant *variant;
u16 devid;
- int err = superio_enter(sioaddr);
+ const u16 *pdevid;
+ int err;
+
+ err = superio_enter(sioaddr);
if (err)
- return err;
+ return ERR_PTR(err);
devid = superio_inw(sioaddr, SIO_REG_MANID);
if (devid != SIO_FINTEK_ID) {
pr_debug("Not a Fintek device\n");
- err = -ENODEV;
- goto exit;
+ goto out;
}
devid = force_id ? force_id : superio_inw(sioaddr, SIO_REG_DEVID);
- switch (devid) {
- case SIO_F71808_ID:
- watchdog.type = f71808fg;
- break;
- case SIO_F71862_ID:
- watchdog.type = f71862fg;
- break;
- case SIO_F71868_ID:
- watchdog.type = f71868;
- break;
- case SIO_F71869_ID:
- case SIO_F71869A_ID:
- watchdog.type = f71869;
- break;
- case SIO_F71882_ID:
- watchdog.type = f71882fg;
- break;
- case SIO_F71889_ID:
- watchdog.type = f71889fg;
- break;
- case SIO_F71858_ID:
- /* Confirmed (by datasheet) not to have a watchdog. */
- err = -ENODEV;
- goto exit;
- case SIO_F81803_ID:
- watchdog.type = f81803;
- break;
- case SIO_F81865_ID:
- watchdog.type = f81865;
- break;
- case SIO_F81866_ID:
- watchdog.type = f81866;
- break;
- default:
- pr_info("Unrecognized Fintek device: %04x\n",
- (unsigned int)devid);
- err = -ENODEV;
- goto exit;
+ for (id = fintek_wdt_id_table; id->driver_data; id++) {
+ variant = (void *)id->driver_data;
+ if (variant->id == devid) {
+ name = id->name;
+ pr_info("Found %s chip, revision %d\n",
+ name, superio_inb(sioaddr, SIO_REG_DEVREV));
+ goto out;
+ }
+ }
+
+ for (pdevid = fintek_no_wdt_table; *pdevid; pdevid++) {
+ if (*pdevid == devid)
+ goto out;
}
- pr_info("Found %s watchdog chip, revision %d\n",
- f71808e_names[watchdog.type],
- (int)superio_inb(sioaddr, SIO_REG_DEVREV));
-exit:
+ pr_info("Unrecognized Fintek device: %04x\n", devid);
+out:
superio_exit(sioaddr);
- return err;
+ return name;
}
-static int __init f71808e_init(void)
+static struct resource wdt_res = {
+ .name = "superio port",
+ .flags = IORESOURCE_IO,
+};
+
+static int __init fintek_wdt_init(void)
{
- static const unsigned short addrs[] = { 0x2e, 0x4e };
- int err = -ENODEV;
+ unsigned short addrs[] = { 0x2e, 0x4e };
+ const char *wdt_name;
int i;
if (f71862fg_pin != 63 && f71862fg_pin != 56) {
@@ -846,30 +896,33 @@ static int __init f71808e_init(void)
return -EINVAL;
}
+ platform_driver_register(&fintek_wdt_driver);
+
for (i = 0; i < ARRAY_SIZE(addrs); i++) {
- err = f71808e_find(addrs[i]);
- if (err == 0)
+ wdt_name = fintek_wdt_find(addrs[i]);
+ if (!IS_ERR(wdt_name))
break;
}
+
if (i == ARRAY_SIZE(addrs))
- return err;
+ return PTR_ERR(wdt_name);
+
+ wdt_res.start = addrs[i];
+ wdt_res.end = wdt_res.start + 1;
- return watchdog_init(addrs[i]);
+ fintek_wdt_pdev = platform_device_register_simple(wdt_name, -1, &wdt_res, 1);
+ return PTR_ERR_OR_ZERO(fintek_wdt_pdev);
}
-static void __exit f71808e_exit(void)
+static void __exit fintek_wdt_exit(void)
{
- if (watchdog_is_running()) {
- pr_warn("Watchdog timer still running, stopping it\n");
- watchdog_stop();
- }
- misc_deregister(&watchdog_miscdev);
- unregister_reboot_notifier(&watchdog_notifier);
+ platform_device_unregister(fintek_wdt_pdev);
+ platform_driver_unregister(&fintek_wdt_driver);
}
MODULE_DESCRIPTION("F71808E Watchdog Driver");
MODULE_AUTHOR("Giel van Schijndel <me@xxxxxxxxx>");
MODULE_LICENSE("GPL");
-module_init(f71808e_init);
-module_exit(f71808e_exit);
+module_init(fintek_wdt_init);
+module_exit(fintek_wdt_exit);
--
2.28.0