Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: Wim Van Sebroeck <wim@linux-watchdog.org>,
	Guenter Roeck <linux@roeck-us.net>
Cc: linux-watchdog@vger.kernel.org, kernel@pengutronix.de,
	Ahmad Fatoum <a.fatoum@pengutronix.de>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v1 6/8] watchdog: f71808e_wdt: consolidate variant handling into single array
Date: Thu, 11 Jun 2020 21:17:47 +0200
Message-ID: <20200611191750.28096-7-a.fatoum@pengutronix.de> (raw)
In-Reply-To: <20200611191750.28096-1-a.fatoum@pengutronix.de>

Driver has two big switches: one to map hardware ID to an enum constant
one more to map the enum constant to the variant's pin configuration
routine.

Drop the enum and give every variant an array entry identifying it.
This arguably simplifies the code by making it a little more compact.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/f71808e_wdt.c | 263 +++++++++++++++------------------
 1 file changed, 121 insertions(+), 142 deletions(-)

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index 9f7823819ed1..9299ad4d4a52 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -110,22 +110,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 +120,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 f71808e_variant {
+	u16 id;
+	const char *wdt_name; /* NULL if chip lacks watchdog timer */
+	void (*pinconf)(struct watchdog_data *wd);
+};
+
 struct watchdog_data {
 	unsigned short	sioaddr;
-	enum chips	type;
+	const struct f71808e_variant *variant;
 	unsigned long	opened;
 	struct mutex	lock;
 	char		expect_close;
@@ -156,6 +148,12 @@ static struct watchdog_data watchdog = {
 	.lock = __MUTEX_INITIALIZER(watchdog.lock),
 };
 
+static inline bool has_f81865_wdo_conf(struct watchdog_data *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 +245,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 +307,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 +320,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 +353,6 @@ static int watchdog_start(void)
 				F71808FG_FLAG_WD_PULSE);
 	}
 
-exit_superio:
 	superio_exit(watchdog.sioaddr);
 exit_unlock:
 	mutex_unlock(&watchdog.lock);
@@ -679,7 +606,7 @@ static int __init watchdog_init(int sioaddr)
 
 	snprintf(watchdog.ident.identity,
 		sizeof(watchdog.ident.identity), "%s watchdog",
-		f71808e_names[watchdog.type]);
+		watchdog.variant->wdt_name);
 
 	err = superio_enter(sioaddr);
 	if (err)
@@ -772,73 +699,123 @@ static int __init watchdog_init(int sioaddr)
 	return err;
 }
 
-static int __init f71808e_find(int sioaddr)
+static void f71808fg_pinconf(struct watchdog_data *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 watchdog_data *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 watchdog_data *wd)
+{
+	/* GPIO14 --> WDTRST# */
+	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT1, 4);
+}
+static void f71882fg_pinconf(struct watchdog_data *wd)
+{
+	/* Set pin 56 to WDTRST# */
+	superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
+}
+static void f71889fg_pinconf(struct watchdog_data *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 watchdog_data *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 watchdog_data *wd)
+{
+	/* Set pin 70 to WDTRST# */
+	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 5);
+}
+static void f81866_pinconf(struct watchdog_data *wd)
+{
+	/*
+	 * GPIO1 Control Register when 27h BIT3:2 = 01 & BIT0 = 0.
+	 * The PIN 70(GPIO15/WDTRST) is controlled by 2Ch:
+	 *     BIT5: 0 -> WDTRST#
+	 *           1 -> GPIO15
+	 */
+	u8 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);
+}
+
+struct f71808e_variant f71808e_variants[] = {
+	{ SIO_F71808_ID,  "f71808fg", f71808fg_pinconf },
+	{ SIO_F71862_ID,  "f71862fg", f71862fg_pinconf },
+	{ SIO_F71868_ID,  "f71868",   f71868_pinconf },
+	{ SIO_F71869_ID,  "f71869",   f71868_pinconf },
+	{ SIO_F71869A_ID, "f71869",   f71868_pinconf },
+	{ SIO_F71882_ID,  "f71882fg", f71882fg_pinconf },
+	{ SIO_F71889_ID,  "f71889fg", f71889fg_pinconf },
+	{ SIO_F81803_ID,  "f81803",   f81803_pinconf },
+	{ SIO_F81865_ID,  "f81865",   f81865_pinconf },
+	{ SIO_F81866_ID,  "f81866",   f81866_pinconf },
+	/* Confirmed (by datasheet) not to have a watchdog. */
+	{ SIO_F71858_ID,  NULL,       NULL },
+	{ /* sentinel */ }
+};
+
+static struct f71808e_variant __init *f71808e_find(int sioaddr)
 {
+	struct f71808e_variant *variant;
 	u16 devid;
 	int 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;
+		superio_exit(sioaddr);
+		return ERR_PTR(-ENODEV);
 	}
 
 	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 (variant = f71808e_variants; variant->id; variant++) {
+		if (variant->id == devid)
+			break;
+	}
+
+	if (!variant->wdt_name) {
+		if (!variant->id)
+			pr_info("Unrecognized Fintek device: %04x\n", devid);
+		superio_exit(sioaddr);
+		return ERR_PTR(-ENODEV);
 	}
 
 	pr_info("Found %s watchdog chip, revision %d\n",
-		f71808e_names[watchdog.type],
-		(int)superio_inb(sioaddr, SIO_REG_DEVREV));
-exit:
+		variant->wdt_name,
+		superio_inb(sioaddr, SIO_REG_DEVREV));
+
 	superio_exit(sioaddr);
-	return err;
+	return variant;
 }
 
 static int __init f71808e_init(void)
 {
 	static const unsigned short addrs[] = { 0x2e, 0x4e };
-	int err = -ENODEV;
+	struct f71808e_variant *variant;
 	int i;
 
 	if (f71862fg_pin != 63 && f71862fg_pin != 56) {
@@ -847,12 +824,14 @@ static int __init f71808e_init(void)
 	}
 
 	for (i = 0; i < ARRAY_SIZE(addrs); i++) {
-		err = f71808e_find(addrs[i]);
-		if (err == 0)
+		variant = f71808e_find(addrs[i]);
+		if (!IS_ERR(variant))
 			break;
 	}
 	if (i == ARRAY_SIZE(addrs))
-		return err;
+		return PTR_ERR(variant);
+
+	watchdog.variant = variant;
 
 	return watchdog_init(addrs[i]);
 }
-- 
2.27.0


  parent reply index

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-11 19:17 [PATCH v1 0/8] watchdog: f71808e_wdt: migrate to kernel Ahmad Fatoum
2020-06-11 19:17 ` [PATCH v1 1/8] docs: watchdog: codify ident.options as superset of possible status flags Ahmad Fatoum
2020-06-30 20:49   ` Guenter Roeck
2020-06-11 19:17 ` [PATCH v1 2/8] watchdog: f71808e_wdt: indicate WDIOF_CARDRESET support in watchdog_info.options Ahmad Fatoum
2020-06-30 20:51   ` Guenter Roeck
2020-06-11 19:17 ` [PATCH v1 3/8] watchdog: f71808e_wdt: remove use of wrong watchdog_info option Ahmad Fatoum
2020-06-30 21:07   ` Guenter Roeck
2020-06-11 19:17 ` [PATCH v1 4/8] watchdog: f71808e_wdt: clear watchdog timeout occurred flag Ahmad Fatoum
2020-06-30 21:09   ` Guenter Roeck
2020-06-11 19:17 ` [PATCH v1 5/8] watchdog: f71808e_wdt: do stricter parameter validation Ahmad Fatoum
2020-06-30 21:11   ` Guenter Roeck
2020-06-11 19:17 ` Ahmad Fatoum [this message]
2020-06-30 21:18   ` [PATCH v1 6/8] watchdog: f71808e_wdt: consolidate variant handling into single array Guenter Roeck
2020-06-11 19:17 ` [PATCH v1 7/8] watchdog: f71808e_wdt: migrate to new kernel watchdog API Ahmad Fatoum
2020-06-30 21:26   ` Guenter Roeck
2020-06-11 19:17 ` [PATCH v1 8/8] watchdog: f71808e_wdt: rename variant-independent identifiers appropriately Ahmad Fatoum
2020-06-12  4:57   ` kernel test robot
2020-06-12  4:57   ` [RFC PATCH] watchdog: f71808e_wdt: fintek_variants[] can be static kernel test robot
2020-06-30 21:29   ` [PATCH v1 8/8] watchdog: f71808e_wdt: rename variant-independent identifiers appropriately Guenter Roeck
2020-07-13  8:59     ` Ahmad Fatoum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200611191750.28096-7-a.fatoum@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=kernel@pengutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=wim@linux-watchdog.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org
	public-inbox-index linux-watchdog

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git