* [PATCH 1/4] watchdog: iTCO_wdt: Use allocated data structures
@ 2017-01-03 14:39 Guenter Roeck
2017-01-03 14:39 ` [PATCH 2/4] watchdog: iTCO_wdt: Use device managed resources Guenter Roeck
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Guenter Roeck @ 2017-01-03 14:39 UTC (permalink / raw)
To: Wim Van Sebroeck
Cc: linux-watchdog, linux-kernel, Matt Fleming, Guenter Roeck
Allocate private data and the watchdog device to to avoid having
to clear it on remove and to enable subsequent simplifications.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/watchdog/iTCO_wdt.c | 402 ++++++++++++++++++++++----------------------
1 file changed, 205 insertions(+), 197 deletions(-)
diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index 06fcb6c8c917..a35a9164ccd0 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -72,22 +72,24 @@
/* Address definitions for the TCO */
/* TCO base address */
-#define TCOBASE (iTCO_wdt_private.tco_res->start)
+#define TCOBASE(p) ((p)->tco_res->start)
/* SMI Control and Enable Register */
-#define SMI_EN (iTCO_wdt_private.smi_res->start)
-
-#define TCO_RLD (TCOBASE + 0x00) /* TCO Timer Reload and Curr. Value */
-#define TCOv1_TMR (TCOBASE + 0x01) /* TCOv1 Timer Initial Value */
-#define TCO_DAT_IN (TCOBASE + 0x02) /* TCO Data In Register */
-#define TCO_DAT_OUT (TCOBASE + 0x03) /* TCO Data Out Register */
-#define TCO1_STS (TCOBASE + 0x04) /* TCO1 Status Register */
-#define TCO2_STS (TCOBASE + 0x06) /* TCO2 Status Register */
-#define TCO1_CNT (TCOBASE + 0x08) /* TCO1 Control Register */
-#define TCO2_CNT (TCOBASE + 0x0a) /* TCO2 Control Register */
-#define TCOv2_TMR (TCOBASE + 0x12) /* TCOv2 Timer Initial Value */
+#define SMI_EN(p) ((p)->smi_res->start)
+
+#define TCO_RLD(p) (TCOBASE(p) + 0x00) /* TCO Timer Reload/Curr. Value */
+#define TCOv1_TMR(p) (TCOBASE(p) + 0x01) /* TCOv1 Timer Initial Value*/
+#define TCO_DAT_IN(p) (TCOBASE(p) + 0x02) /* TCO Data In Register */
+#define TCO_DAT_OUT(p) (TCOBASE(p) + 0x03) /* TCO Data Out Register */
+#define TCO1_STS(p) (TCOBASE(p) + 0x04) /* TCO1 Status Register */
+#define TCO2_STS(p) (TCOBASE(p) + 0x06) /* TCO2 Status Register */
+#define TCO1_CNT(p) (TCOBASE(p) + 0x08) /* TCO1 Control Register */
+#define TCO2_CNT(p) (TCOBASE(p) + 0x0a) /* TCO2 Control Register */
+#define TCOv2_TMR(p) (TCOBASE(p) + 0x12) /* TCOv2 Timer Initial Value*/
/* internal variables */
-static struct { /* this is private data for the iTCO_wdt device */
+struct iTCO_wdt_private {
+ struct watchdog_device wddev;
+
/* TCO version/generation */
unsigned int iTCO_version;
struct resource *tco_res;
@@ -105,7 +107,7 @@ static struct { /* this is private data for the iTCO_wdt device */
struct pci_dev *pdev;
/* whether or not the watchdog has been suspended */
bool suspended;
-} iTCO_wdt_private;
+};
/* module parameters */
#define WATCHDOG_TIMEOUT 30 /* 30 sec default heartbeat */
@@ -135,21 +137,23 @@ MODULE_PARM_DESC(turn_SMI_watchdog_clear_off,
* every 0.6 seconds. v3's internal timer is stored as seconds (some
* datasheets incorrectly state 0.6 seconds).
*/
-static inline unsigned int seconds_to_ticks(int secs)
+static inline unsigned int seconds_to_ticks(struct iTCO_wdt_private *p,
+ int secs)
{
- return iTCO_wdt_private.iTCO_version == 3 ? secs : (secs * 10) / 6;
+ return p->iTCO_version == 3 ? secs : (secs * 10) / 6;
}
-static inline unsigned int ticks_to_seconds(int ticks)
+static inline unsigned int ticks_to_seconds(struct iTCO_wdt_private *p,
+ int ticks)
{
- return iTCO_wdt_private.iTCO_version == 3 ? ticks : (ticks * 6) / 10;
+ return p->iTCO_version == 3 ? ticks : (ticks * 6) / 10;
}
-static inline u32 no_reboot_bit(void)
+static inline u32 no_reboot_bit(struct iTCO_wdt_private *p)
{
u32 enable_bit;
- switch (iTCO_wdt_private.iTCO_version) {
+ switch (p->iTCO_version) {
case 5:
case 3:
enable_bit = 0x00000010;
@@ -167,40 +171,40 @@ static inline u32 no_reboot_bit(void)
return enable_bit;
}
-static void iTCO_wdt_set_NO_REBOOT_bit(void)
+static void iTCO_wdt_set_NO_REBOOT_bit(struct iTCO_wdt_private *p)
{
u32 val32;
/* Set the NO_REBOOT bit: this disables reboots */
- if (iTCO_wdt_private.iTCO_version >= 2) {
- val32 = readl(iTCO_wdt_private.gcs_pmc);
- val32 |= no_reboot_bit();
- writel(val32, iTCO_wdt_private.gcs_pmc);
- } else if (iTCO_wdt_private.iTCO_version == 1) {
- pci_read_config_dword(iTCO_wdt_private.pdev, 0xd4, &val32);
- val32 |= no_reboot_bit();
- pci_write_config_dword(iTCO_wdt_private.pdev, 0xd4, val32);
+ if (p->iTCO_version >= 2) {
+ val32 = readl(p->gcs_pmc);
+ val32 |= no_reboot_bit(p);
+ writel(val32, p->gcs_pmc);
+ } else if (p->iTCO_version == 1) {
+ pci_read_config_dword(p->pdev, 0xd4, &val32);
+ val32 |= no_reboot_bit(p);
+ pci_write_config_dword(p->pdev, 0xd4, val32);
}
}
-static int iTCO_wdt_unset_NO_REBOOT_bit(void)
+static int iTCO_wdt_unset_NO_REBOOT_bit(struct iTCO_wdt_private *p)
{
- u32 enable_bit = no_reboot_bit();
+ u32 enable_bit = no_reboot_bit(p);
u32 val32 = 0;
/* Unset the NO_REBOOT bit: this enables reboots */
- if (iTCO_wdt_private.iTCO_version >= 2) {
- val32 = readl(iTCO_wdt_private.gcs_pmc);
+ if (p->iTCO_version >= 2) {
+ val32 = readl(p->gcs_pmc);
val32 &= ~enable_bit;
- writel(val32, iTCO_wdt_private.gcs_pmc);
+ writel(val32, p->gcs_pmc);
- val32 = readl(iTCO_wdt_private.gcs_pmc);
- } else if (iTCO_wdt_private.iTCO_version == 1) {
- pci_read_config_dword(iTCO_wdt_private.pdev, 0xd4, &val32);
+ val32 = readl(p->gcs_pmc);
+ } else if (p->iTCO_version == 1) {
+ pci_read_config_dword(p->pdev, 0xd4, &val32);
val32 &= ~enable_bit;
- pci_write_config_dword(iTCO_wdt_private.pdev, 0xd4, val32);
+ pci_write_config_dword(p->pdev, 0xd4, val32);
- pci_read_config_dword(iTCO_wdt_private.pdev, 0xd4, &val32);
+ pci_read_config_dword(p->pdev, 0xd4, &val32);
}
if (val32 & enable_bit)
@@ -211,32 +215,33 @@ static int iTCO_wdt_unset_NO_REBOOT_bit(void)
static int iTCO_wdt_start(struct watchdog_device *wd_dev)
{
+ struct iTCO_wdt_private *p = watchdog_get_drvdata(wd_dev);
unsigned int val;
- spin_lock(&iTCO_wdt_private.io_lock);
+ spin_lock(&p->io_lock);
- iTCO_vendor_pre_start(iTCO_wdt_private.smi_res, wd_dev->timeout);
+ iTCO_vendor_pre_start(p->smi_res, wd_dev->timeout);
/* disable chipset's NO_REBOOT bit */
- if (iTCO_wdt_unset_NO_REBOOT_bit()) {
- spin_unlock(&iTCO_wdt_private.io_lock);
+ if (iTCO_wdt_unset_NO_REBOOT_bit(p)) {
+ spin_unlock(&p->io_lock);
pr_err("failed to reset NO_REBOOT flag, reboot disabled by hardware/BIOS\n");
return -EIO;
}
/* Force the timer to its reload value by writing to the TCO_RLD
register */
- if (iTCO_wdt_private.iTCO_version >= 2)
- outw(0x01, TCO_RLD);
- else if (iTCO_wdt_private.iTCO_version == 1)
- outb(0x01, TCO_RLD);
+ if (p->iTCO_version >= 2)
+ outw(0x01, TCO_RLD(p));
+ else if (p->iTCO_version == 1)
+ outb(0x01, TCO_RLD(p));
/* Bit 11: TCO Timer Halt -> 0 = The TCO timer is enabled to count */
- val = inw(TCO1_CNT);
+ val = inw(TCO1_CNT(p));
val &= 0xf7ff;
- outw(val, TCO1_CNT);
- val = inw(TCO1_CNT);
- spin_unlock(&iTCO_wdt_private.io_lock);
+ outw(val, TCO1_CNT(p));
+ val = inw(TCO1_CNT(p));
+ spin_unlock(&p->io_lock);
if (val & 0x0800)
return -1;
@@ -245,22 +250,23 @@ static int iTCO_wdt_start(struct watchdog_device *wd_dev)
static int iTCO_wdt_stop(struct watchdog_device *wd_dev)
{
+ struct iTCO_wdt_private *p = watchdog_get_drvdata(wd_dev);
unsigned int val;
- spin_lock(&iTCO_wdt_private.io_lock);
+ spin_lock(&p->io_lock);
- iTCO_vendor_pre_stop(iTCO_wdt_private.smi_res);
+ iTCO_vendor_pre_stop(p->smi_res);
/* Bit 11: TCO Timer Halt -> 1 = The TCO timer is disabled */
- val = inw(TCO1_CNT);
+ val = inw(TCO1_CNT(p));
val |= 0x0800;
- outw(val, TCO1_CNT);
- val = inw(TCO1_CNT);
+ outw(val, TCO1_CNT(p));
+ val = inw(TCO1_CNT(p));
/* Set the NO_REBOOT bit to prevent later reboots, just for sure */
- iTCO_wdt_set_NO_REBOOT_bit();
+ iTCO_wdt_set_NO_REBOOT_bit(p);
- spin_unlock(&iTCO_wdt_private.io_lock);
+ spin_unlock(&p->io_lock);
if ((val & 0x0800) == 0)
return -1;
@@ -269,67 +275,70 @@ static int iTCO_wdt_stop(struct watchdog_device *wd_dev)
static int iTCO_wdt_ping(struct watchdog_device *wd_dev)
{
- spin_lock(&iTCO_wdt_private.io_lock);
+ struct iTCO_wdt_private *p = watchdog_get_drvdata(wd_dev);
- iTCO_vendor_pre_keepalive(iTCO_wdt_private.smi_res, wd_dev->timeout);
+ spin_lock(&p->io_lock);
+
+ iTCO_vendor_pre_keepalive(p->smi_res, wd_dev->timeout);
/* Reload the timer by writing to the TCO Timer Counter register */
- if (iTCO_wdt_private.iTCO_version >= 2) {
- outw(0x01, TCO_RLD);
- } else if (iTCO_wdt_private.iTCO_version == 1) {
+ if (p->iTCO_version >= 2) {
+ outw(0x01, TCO_RLD(p));
+ } else if (p->iTCO_version == 1) {
/* Reset the timeout status bit so that the timer
* needs to count down twice again before rebooting */
- outw(0x0008, TCO1_STS); /* write 1 to clear bit */
+ outw(0x0008, TCO1_STS(p)); /* write 1 to clear bit */
- outb(0x01, TCO_RLD);
+ outb(0x01, TCO_RLD(p));
}
- spin_unlock(&iTCO_wdt_private.io_lock);
+ spin_unlock(&p->io_lock);
return 0;
}
static int iTCO_wdt_set_timeout(struct watchdog_device *wd_dev, unsigned int t)
{
+ struct iTCO_wdt_private *p = watchdog_get_drvdata(wd_dev);
unsigned int val16;
unsigned char val8;
unsigned int tmrval;
- tmrval = seconds_to_ticks(t);
+ tmrval = seconds_to_ticks(p, t);
/* For TCO v1 the timer counts down twice before rebooting */
- if (iTCO_wdt_private.iTCO_version == 1)
+ if (p->iTCO_version == 1)
tmrval /= 2;
/* from the specs: */
/* "Values of 0h-3h are ignored and should not be attempted" */
if (tmrval < 0x04)
return -EINVAL;
- if (((iTCO_wdt_private.iTCO_version >= 2) && (tmrval > 0x3ff)) ||
- ((iTCO_wdt_private.iTCO_version == 1) && (tmrval > 0x03f)))
+ if ((p->iTCO_version >= 2 && tmrval > 0x3ff) ||
+ (p->iTCO_version == 1 && tmrval > 0x03f))
return -EINVAL;
iTCO_vendor_pre_set_heartbeat(tmrval);
/* Write new heartbeat to watchdog */
- if (iTCO_wdt_private.iTCO_version >= 2) {
- spin_lock(&iTCO_wdt_private.io_lock);
- val16 = inw(TCOv2_TMR);
+ if (p->iTCO_version >= 2) {
+ spin_lock(&p->io_lock);
+ val16 = inw(TCOv2_TMR(p));
val16 &= 0xfc00;
val16 |= tmrval;
- outw(val16, TCOv2_TMR);
- val16 = inw(TCOv2_TMR);
- spin_unlock(&iTCO_wdt_private.io_lock);
+ outw(val16, TCOv2_TMR(p));
+ val16 = inw(TCOv2_TMR(p));
+ spin_unlock(&p->io_lock);
if ((val16 & 0x3ff) != tmrval)
return -EINVAL;
- } else if (iTCO_wdt_private.iTCO_version == 1) {
- spin_lock(&iTCO_wdt_private.io_lock);
- val8 = inb(TCOv1_TMR);
+ } else if (p->iTCO_version == 1) {
+ spin_lock(&p->io_lock);
+ val8 = inb(TCOv1_TMR(p));
val8 &= 0xc0;
val8 |= (tmrval & 0xff);
- outb(val8, TCOv1_TMR);
- val8 = inb(TCOv1_TMR);
- spin_unlock(&iTCO_wdt_private.io_lock);
+ outb(val8, TCOv1_TMR(p));
+ val8 = inb(TCOv1_TMR(p));
+ spin_unlock(&p->io_lock);
if ((val8 & 0x3f) != tmrval)
return -EINVAL;
@@ -341,27 +350,28 @@ static int iTCO_wdt_set_timeout(struct watchdog_device *wd_dev, unsigned int t)
static unsigned int iTCO_wdt_get_timeleft(struct watchdog_device *wd_dev)
{
+ struct iTCO_wdt_private *p = watchdog_get_drvdata(wd_dev);
unsigned int val16;
unsigned char val8;
unsigned int time_left = 0;
/* read the TCO Timer */
- if (iTCO_wdt_private.iTCO_version >= 2) {
- spin_lock(&iTCO_wdt_private.io_lock);
- val16 = inw(TCO_RLD);
+ if (p->iTCO_version >= 2) {
+ spin_lock(&p->io_lock);
+ val16 = inw(TCO_RLD(p));
val16 &= 0x3ff;
- spin_unlock(&iTCO_wdt_private.io_lock);
+ spin_unlock(&p->io_lock);
- time_left = ticks_to_seconds(val16);
- } else if (iTCO_wdt_private.iTCO_version == 1) {
- spin_lock(&iTCO_wdt_private.io_lock);
- val8 = inb(TCO_RLD);
+ time_left = ticks_to_seconds(p, val16);
+ } else if (p->iTCO_version == 1) {
+ spin_lock(&p->io_lock);
+ val8 = inb(TCO_RLD(p));
val8 &= 0x3f;
- if (!(inw(TCO1_STS) & 0x0008))
- val8 += (inb(TCOv1_TMR) & 0x3f);
- spin_unlock(&iTCO_wdt_private.io_lock);
+ if (!(inw(TCO1_STS(p)) & 0x0008))
+ val8 += (inb(TCOv1_TMR(p)) & 0x3f);
+ spin_unlock(&p->io_lock);
- time_left = ticks_to_seconds(val8);
+ time_left = ticks_to_seconds(p, val8);
}
return time_left;
}
@@ -387,166 +397,165 @@ static const struct watchdog_ops iTCO_wdt_ops = {
.get_timeleft = iTCO_wdt_get_timeleft,
};
-static struct watchdog_device iTCO_wdt_watchdog_dev = {
- .info = &ident,
- .ops = &iTCO_wdt_ops,
-};
-
/*
* Init & exit routines
*/
-static void iTCO_wdt_cleanup(void)
+static void iTCO_wdt_cleanup(struct iTCO_wdt_private *p)
{
/* Stop the timer before we leave */
if (!nowayout)
- iTCO_wdt_stop(&iTCO_wdt_watchdog_dev);
+ iTCO_wdt_stop(&p->wddev);
/* Deregister */
- watchdog_unregister_device(&iTCO_wdt_watchdog_dev);
+ watchdog_unregister_device(&p->wddev);
/* release resources */
- release_region(iTCO_wdt_private.tco_res->start,
- resource_size(iTCO_wdt_private.tco_res));
- release_region(iTCO_wdt_private.smi_res->start,
- resource_size(iTCO_wdt_private.smi_res));
- if (iTCO_wdt_private.iTCO_version >= 2) {
- iounmap(iTCO_wdt_private.gcs_pmc);
- release_mem_region(iTCO_wdt_private.gcs_pmc_res->start,
- resource_size(iTCO_wdt_private.gcs_pmc_res));
+ release_region(p->tco_res->start,
+ resource_size(p->tco_res));
+ release_region(p->smi_res->start,
+ resource_size(p->smi_res));
+ if (p->iTCO_version >= 2) {
+ iounmap(p->gcs_pmc);
+ release_mem_region(p->gcs_pmc_res->start,
+ resource_size(p->gcs_pmc_res));
}
-
- iTCO_wdt_private.tco_res = NULL;
- iTCO_wdt_private.smi_res = NULL;
- iTCO_wdt_private.gcs_pmc_res = NULL;
- iTCO_wdt_private.gcs_pmc = NULL;
}
static int iTCO_wdt_probe(struct platform_device *dev)
{
- int ret = -ENODEV;
- unsigned long val32;
struct itco_wdt_platform_data *pdata = dev_get_platdata(&dev->dev);
+ struct iTCO_wdt_private *p;
+ unsigned long val32;
+ int ret;
if (!pdata)
- goto out;
+ return -ENODEV;
- spin_lock_init(&iTCO_wdt_private.io_lock);
+ p = devm_kzalloc(&dev->dev, sizeof(*p), GFP_KERNEL);
+ if (!p)
+ return -ENOMEM;
- iTCO_wdt_private.tco_res =
- platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_TCO);
- if (!iTCO_wdt_private.tco_res)
- goto out;
+ spin_lock_init(&p->io_lock);
- iTCO_wdt_private.smi_res =
- platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_SMI);
- if (!iTCO_wdt_private.smi_res)
- goto out;
+ p->tco_res = platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_TCO);
+ if (!p->tco_res)
+ return -ENODEV;
- iTCO_wdt_private.iTCO_version = pdata->version;
- iTCO_wdt_private.dev = dev;
- iTCO_wdt_private.pdev = to_pci_dev(dev->dev.parent);
+ p->smi_res = platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_SMI);
+ if (!p->smi_res)
+ return -ENODEV;
+
+ p->iTCO_version = pdata->version;
+ p->dev = dev;
+ p->pdev = to_pci_dev(dev->dev.parent);
/*
* Get the Memory-Mapped GCS or PMC register, we need it for the
* NO_REBOOT flag (TCO v2 and v3).
*/
- if (iTCO_wdt_private.iTCO_version >= 2) {
- iTCO_wdt_private.gcs_pmc_res = platform_get_resource(dev,
- IORESOURCE_MEM,
- ICH_RES_MEM_GCS_PMC);
-
- if (!iTCO_wdt_private.gcs_pmc_res)
- goto out;
-
- if (!request_mem_region(iTCO_wdt_private.gcs_pmc_res->start,
- resource_size(iTCO_wdt_private.gcs_pmc_res), dev->name)) {
- ret = -EBUSY;
- goto out;
- }
- iTCO_wdt_private.gcs_pmc = ioremap(iTCO_wdt_private.gcs_pmc_res->start,
- resource_size(iTCO_wdt_private.gcs_pmc_res));
- if (!iTCO_wdt_private.gcs_pmc) {
+ if (p->iTCO_version >= 2) {
+ p->gcs_pmc_res = platform_get_resource(dev,
+ IORESOURCE_MEM,
+ ICH_RES_MEM_GCS_PMC);
+
+ if (!p->gcs_pmc_res)
+ return -ENODEV;
+
+ if (!request_mem_region(p->gcs_pmc_res->start,
+ resource_size(p->gcs_pmc_res),
+ dev->name))
+ return -EBUSY;
+
+ p->gcs_pmc = ioremap(p->gcs_pmc_res->start,
+ resource_size(p->gcs_pmc_res));
+ if (!p->gcs_pmc) {
ret = -EIO;
goto unreg_gcs_pmc;
}
}
/* Check chipset's NO_REBOOT bit */
- if (iTCO_wdt_unset_NO_REBOOT_bit() && iTCO_vendor_check_noreboot_on()) {
+ if (iTCO_wdt_unset_NO_REBOOT_bit(p) &&
+ iTCO_vendor_check_noreboot_on()) {
pr_info("unable to reset NO_REBOOT flag, device disabled by hardware/BIOS\n");
ret = -ENODEV; /* Cannot reset NO_REBOOT bit */
goto unmap_gcs_pmc;
}
/* Set the NO_REBOOT bit to prevent later reboots, just for sure */
- iTCO_wdt_set_NO_REBOOT_bit();
+ iTCO_wdt_set_NO_REBOOT_bit(p);
/* The TCO logic uses the TCO_EN bit in the SMI_EN register */
- if (!request_region(iTCO_wdt_private.smi_res->start,
- resource_size(iTCO_wdt_private.smi_res), dev->name)) {
+ if (!request_region(p->smi_res->start,
+ resource_size(p->smi_res), dev->name)) {
pr_err("I/O address 0x%04llx already in use, device disabled\n",
- (u64)SMI_EN);
+ (u64)SMI_EN(p));
ret = -EBUSY;
goto unmap_gcs_pmc;
}
- if (turn_SMI_watchdog_clear_off >= iTCO_wdt_private.iTCO_version) {
+ if (turn_SMI_watchdog_clear_off >= p->iTCO_version) {
/*
* Bit 13: TCO_EN -> 0
* Disables TCO logic generating an SMI#
*/
- val32 = inl(SMI_EN);
+ val32 = inl(SMI_EN(p));
val32 &= 0xffffdfff; /* Turn off SMI clearing watchdog */
- outl(val32, SMI_EN);
+ outl(val32, SMI_EN(p));
}
- if (!request_region(iTCO_wdt_private.tco_res->start,
- resource_size(iTCO_wdt_private.tco_res), dev->name)) {
+ if (!request_region(p->tco_res->start,
+ resource_size(p->tco_res), dev->name)) {
pr_err("I/O address 0x%04llx already in use, device disabled\n",
- (u64)TCOBASE);
+ (u64)TCOBASE(p));
ret = -EBUSY;
goto unreg_smi;
}
pr_info("Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n",
- pdata->name, pdata->version, (u64)TCOBASE);
+ pdata->name, pdata->version, (u64)TCOBASE(p));
/* Clear out the (probably old) status */
- switch (iTCO_wdt_private.iTCO_version) {
+ switch (p->iTCO_version) {
case 5:
case 4:
- outw(0x0008, TCO1_STS); /* Clear the Time Out Status bit */
- outw(0x0002, TCO2_STS); /* Clear SECOND_TO_STS bit */
+ outw(0x0008, TCO1_STS(p)); /* Clear the Time Out Status bit */
+ outw(0x0002, TCO2_STS(p)); /* Clear SECOND_TO_STS bit */
break;
case 3:
- outl(0x20008, TCO1_STS);
+ outl(0x20008, TCO1_STS(p));
break;
case 2:
case 1:
default:
- outw(0x0008, TCO1_STS); /* Clear the Time Out Status bit */
- outw(0x0002, TCO2_STS); /* Clear SECOND_TO_STS bit */
- outw(0x0004, TCO2_STS); /* Clear BOOT_STS bit */
+ outw(0x0008, TCO1_STS(p)); /* Clear the Time Out Status bit */
+ outw(0x0002, TCO2_STS(p)); /* Clear SECOND_TO_STS bit */
+ outw(0x0004, TCO2_STS(p)); /* Clear BOOT_STS bit */
break;
}
- iTCO_wdt_watchdog_dev.bootstatus = 0;
- iTCO_wdt_watchdog_dev.timeout = WATCHDOG_TIMEOUT;
- watchdog_set_nowayout(&iTCO_wdt_watchdog_dev, nowayout);
- iTCO_wdt_watchdog_dev.parent = &dev->dev;
+ p->wddev.info = &ident,
+ p->wddev.ops = &iTCO_wdt_ops,
+ p->wddev.bootstatus = 0;
+ p->wddev.timeout = WATCHDOG_TIMEOUT;
+ watchdog_set_nowayout(&p->wddev, nowayout);
+ p->wddev.parent = &dev->dev;
+
+ watchdog_set_drvdata(&p->wddev, p);
+ platform_set_drvdata(dev, p);
/* Make sure the watchdog is not running */
- iTCO_wdt_stop(&iTCO_wdt_watchdog_dev);
+ iTCO_wdt_stop(&p->wddev);
/* Check that the heartbeat value is within it's range;
if not reset to the default */
- if (iTCO_wdt_set_timeout(&iTCO_wdt_watchdog_dev, heartbeat)) {
- iTCO_wdt_set_timeout(&iTCO_wdt_watchdog_dev, WATCHDOG_TIMEOUT);
+ if (iTCO_wdt_set_timeout(&p->wddev, heartbeat)) {
+ iTCO_wdt_set_timeout(&p->wddev, WATCHDOG_TIMEOUT);
pr_info("timeout value out of range, using %d\n",
WATCHDOG_TIMEOUT);
}
- ret = watchdog_register_device(&iTCO_wdt_watchdog_dev);
+ ret = watchdog_register_device(&p->wddev);
if (ret != 0) {
pr_err("cannot register watchdog device (err=%d)\n", ret);
goto unreg_tco;
@@ -558,38 +567,34 @@ static int iTCO_wdt_probe(struct platform_device *dev)
return 0;
unreg_tco:
- release_region(iTCO_wdt_private.tco_res->start,
- resource_size(iTCO_wdt_private.tco_res));
+ release_region(p->tco_res->start, resource_size(p->tco_res));
unreg_smi:
- release_region(iTCO_wdt_private.smi_res->start,
- resource_size(iTCO_wdt_private.smi_res));
+ release_region(p->smi_res->start, resource_size(p->smi_res));
unmap_gcs_pmc:
- if (iTCO_wdt_private.iTCO_version >= 2)
- iounmap(iTCO_wdt_private.gcs_pmc);
+ if (p->iTCO_version >= 2)
+ iounmap(p->gcs_pmc);
unreg_gcs_pmc:
- if (iTCO_wdt_private.iTCO_version >= 2)
- release_mem_region(iTCO_wdt_private.gcs_pmc_res->start,
- resource_size(iTCO_wdt_private.gcs_pmc_res));
-out:
- iTCO_wdt_private.tco_res = NULL;
- iTCO_wdt_private.smi_res = NULL;
- iTCO_wdt_private.gcs_pmc_res = NULL;
- iTCO_wdt_private.gcs_pmc = NULL;
-
+ if (p->iTCO_version >= 2)
+ release_mem_region(p->gcs_pmc_res->start,
+ resource_size(p->gcs_pmc_res));
return ret;
}
static int iTCO_wdt_remove(struct platform_device *dev)
{
- if (iTCO_wdt_private.tco_res || iTCO_wdt_private.smi_res)
- iTCO_wdt_cleanup();
+ struct iTCO_wdt_private *p = platform_get_drvdata(dev);
+
+ if (p->tco_res || p->smi_res)
+ iTCO_wdt_cleanup(p);
return 0;
}
static void iTCO_wdt_shutdown(struct platform_device *dev)
{
- iTCO_wdt_stop(NULL);
+ struct iTCO_wdt_private *p = platform_get_drvdata(dev);
+
+ iTCO_wdt_stop(&p->wddev);
}
#ifdef CONFIG_PM_SLEEP
@@ -610,21 +615,24 @@ static inline bool need_suspend(void) { return true; }
static int iTCO_wdt_suspend_noirq(struct device *dev)
{
+ struct iTCO_wdt_private *p = dev_get_drvdata(dev);
int ret = 0;
- iTCO_wdt_private.suspended = false;
- if (watchdog_active(&iTCO_wdt_watchdog_dev) && need_suspend()) {
- ret = iTCO_wdt_stop(&iTCO_wdt_watchdog_dev);
+ p->suspended = false;
+ if (watchdog_active(&p->wddev) && need_suspend()) {
+ ret = iTCO_wdt_stop(&p->wddev);
if (!ret)
- iTCO_wdt_private.suspended = true;
+ p->suspended = true;
}
return ret;
}
static int iTCO_wdt_resume_noirq(struct device *dev)
{
- if (iTCO_wdt_private.suspended)
- iTCO_wdt_start(&iTCO_wdt_watchdog_dev);
+ struct iTCO_wdt_private *p = dev_get_drvdata(dev);
+
+ if (p->suspended)
+ iTCO_wdt_start(&p->wddev);
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/4] watchdog: iTCO_wdt: Use device managed resources
2017-01-03 14:39 [PATCH 1/4] watchdog: iTCO_wdt: Use allocated data structures Guenter Roeck
@ 2017-01-03 14:39 ` Guenter Roeck
2017-01-03 22:41 ` Andy Shevchenko
2017-01-03 14:39 ` [PATCH 3/4] watchdog: iTCO_wdt: Use pdev for platform device and pci_dev for pci device Guenter Roeck
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2017-01-03 14:39 UTC (permalink / raw)
To: Wim Van Sebroeck
Cc: linux-watchdog, linux-kernel, Matt Fleming, Guenter Roeck
Using device managed resources simplifies error handling and cleanup,
and to reduce the likelyhood of errors.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/watchdog/iTCO_wdt.c | 80 ++++++++++-----------------------------------
1 file changed, 17 insertions(+), 63 deletions(-)
diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index a35a9164ccd0..eed1dee6de19 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -401,27 +401,6 @@ static const struct watchdog_ops iTCO_wdt_ops = {
* Init & exit routines
*/
-static void iTCO_wdt_cleanup(struct iTCO_wdt_private *p)
-{
- /* Stop the timer before we leave */
- if (!nowayout)
- iTCO_wdt_stop(&p->wddev);
-
- /* Deregister */
- watchdog_unregister_device(&p->wddev);
-
- /* release resources */
- release_region(p->tco_res->start,
- resource_size(p->tco_res));
- release_region(p->smi_res->start,
- resource_size(p->smi_res));
- if (p->iTCO_version >= 2) {
- iounmap(p->gcs_pmc);
- release_mem_region(p->gcs_pmc_res->start,
- resource_size(p->gcs_pmc_res));
- }
-}
-
static int iTCO_wdt_probe(struct platform_device *dev)
{
struct itco_wdt_platform_data *pdata = dev_get_platdata(&dev->dev);
@@ -458,41 +437,28 @@ static int iTCO_wdt_probe(struct platform_device *dev)
p->gcs_pmc_res = platform_get_resource(dev,
IORESOURCE_MEM,
ICH_RES_MEM_GCS_PMC);
-
- if (!p->gcs_pmc_res)
- return -ENODEV;
-
- if (!request_mem_region(p->gcs_pmc_res->start,
- resource_size(p->gcs_pmc_res),
- dev->name))
- return -EBUSY;
-
- p->gcs_pmc = ioremap(p->gcs_pmc_res->start,
- resource_size(p->gcs_pmc_res));
- if (!p->gcs_pmc) {
- ret = -EIO;
- goto unreg_gcs_pmc;
- }
+ p->gcs_pmc = devm_ioremap_resource(&dev->dev, p->gcs_pmc_res);
+ if (IS_ERR(p->gcs_pmc))
+ return PTR_ERR(p->gcs_pmc);
}
/* Check chipset's NO_REBOOT bit */
if (iTCO_wdt_unset_NO_REBOOT_bit(p) &&
iTCO_vendor_check_noreboot_on()) {
pr_info("unable to reset NO_REBOOT flag, device disabled by hardware/BIOS\n");
- ret = -ENODEV; /* Cannot reset NO_REBOOT bit */
- goto unmap_gcs_pmc;
+ return -ENODEV; /* Cannot reset NO_REBOOT bit */
}
/* Set the NO_REBOOT bit to prevent later reboots, just for sure */
iTCO_wdt_set_NO_REBOOT_bit(p);
/* The TCO logic uses the TCO_EN bit in the SMI_EN register */
- if (!request_region(p->smi_res->start,
- resource_size(p->smi_res), dev->name)) {
+ if (!devm_request_region(&dev->dev, p->smi_res->start,
+ resource_size(p->smi_res),
+ dev->name)) {
pr_err("I/O address 0x%04llx already in use, device disabled\n",
(u64)SMI_EN(p));
- ret = -EBUSY;
- goto unmap_gcs_pmc;
+ return -EBUSY;
}
if (turn_SMI_watchdog_clear_off >= p->iTCO_version) {
/*
@@ -504,12 +470,12 @@ static int iTCO_wdt_probe(struct platform_device *dev)
outl(val32, SMI_EN(p));
}
- if (!request_region(p->tco_res->start,
- resource_size(p->tco_res), dev->name)) {
+ if (!devm_request_region(&dev->dev, p->tco_res->start,
+ resource_size(p->tco_res),
+ dev->name)) {
pr_err("I/O address 0x%04llx already in use, device disabled\n",
(u64)TCOBASE(p));
- ret = -EBUSY;
- goto unreg_smi;
+ return -EBUSY;
}
pr_info("Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n",
@@ -555,37 +521,25 @@ static int iTCO_wdt_probe(struct platform_device *dev)
WATCHDOG_TIMEOUT);
}
- ret = watchdog_register_device(&p->wddev);
+ ret = devm_watchdog_register_device(&dev->dev, &p->wddev);
if (ret != 0) {
pr_err("cannot register watchdog device (err=%d)\n", ret);
- goto unreg_tco;
+ return ret;
}
pr_info("initialized. heartbeat=%d sec (nowayout=%d)\n",
heartbeat, nowayout);
return 0;
-
-unreg_tco:
- release_region(p->tco_res->start, resource_size(p->tco_res));
-unreg_smi:
- release_region(p->smi_res->start, resource_size(p->smi_res));
-unmap_gcs_pmc:
- if (p->iTCO_version >= 2)
- iounmap(p->gcs_pmc);
-unreg_gcs_pmc:
- if (p->iTCO_version >= 2)
- release_mem_region(p->gcs_pmc_res->start,
- resource_size(p->gcs_pmc_res));
- return ret;
}
static int iTCO_wdt_remove(struct platform_device *dev)
{
struct iTCO_wdt_private *p = platform_get_drvdata(dev);
- if (p->tco_res || p->smi_res)
- iTCO_wdt_cleanup(p);
+ /* Stop the timer before we leave */
+ if (!nowayout)
+ iTCO_wdt_stop(&p->wddev);
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/4] watchdog: iTCO_wdt: Use pdev for platform device and pci_dev for pci device
2017-01-03 14:39 [PATCH 1/4] watchdog: iTCO_wdt: Use allocated data structures Guenter Roeck
2017-01-03 14:39 ` [PATCH 2/4] watchdog: iTCO_wdt: Use device managed resources Guenter Roeck
@ 2017-01-03 14:39 ` Guenter Roeck
2017-01-03 22:39 ` Andy Shevchenko
2017-01-03 14:39 ` [PATCH 4/4] watchdog: iTCO_wdt: Simplify module init function Guenter Roeck
2017-01-03 22:44 ` [PATCH 1/4] watchdog: iTCO_wdt: Use allocated data structures Andy Shevchenko
3 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2017-01-03 14:39 UTC (permalink / raw)
To: Wim Van Sebroeck
Cc: linux-watchdog, linux-kernel, Matt Fleming, Guenter Roeck
Use pdev for struct platform_device, pcidev for struct pci_dev, and dev
for struct device variables to improve consistency.
Remove 'struct platform_device *dev;' from struct iTCO_wdt_private since
it was unused.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/watchdog/iTCO_wdt.c | 53 ++++++++++++++++++++++-----------------------
1 file changed, 26 insertions(+), 27 deletions(-)
diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index eed1dee6de19..ad29ae03a30b 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -102,9 +102,8 @@ struct iTCO_wdt_private {
unsigned long __iomem *gcs_pmc;
/* the lock for io operations */
spinlock_t io_lock;
- struct platform_device *dev;
/* the PCI-device */
- struct pci_dev *pdev;
+ struct pci_dev *pcidev;
/* whether or not the watchdog has been suspended */
bool suspended;
};
@@ -181,9 +180,9 @@ static void iTCO_wdt_set_NO_REBOOT_bit(struct iTCO_wdt_private *p)
val32 |= no_reboot_bit(p);
writel(val32, p->gcs_pmc);
} else if (p->iTCO_version == 1) {
- pci_read_config_dword(p->pdev, 0xd4, &val32);
+ pci_read_config_dword(p->pcidev, 0xd4, &val32);
val32 |= no_reboot_bit(p);
- pci_write_config_dword(p->pdev, 0xd4, val32);
+ pci_write_config_dword(p->pcidev, 0xd4, val32);
}
}
@@ -200,11 +199,11 @@ static int iTCO_wdt_unset_NO_REBOOT_bit(struct iTCO_wdt_private *p)
val32 = readl(p->gcs_pmc);
} else if (p->iTCO_version == 1) {
- pci_read_config_dword(p->pdev, 0xd4, &val32);
+ pci_read_config_dword(p->pcidev, 0xd4, &val32);
val32 &= ~enable_bit;
- pci_write_config_dword(p->pdev, 0xd4, val32);
+ pci_write_config_dword(p->pcidev, 0xd4, val32);
- pci_read_config_dword(p->pdev, 0xd4, &val32);
+ pci_read_config_dword(p->pcidev, 0xd4, &val32);
}
if (val32 & enable_bit)
@@ -401,9 +400,10 @@ static const struct watchdog_ops iTCO_wdt_ops = {
* Init & exit routines
*/
-static int iTCO_wdt_probe(struct platform_device *dev)
+static int iTCO_wdt_probe(struct platform_device *pdev)
{
- struct itco_wdt_platform_data *pdata = dev_get_platdata(&dev->dev);
+ struct device *dev = &pdev->dev;
+ struct itco_wdt_platform_data *pdata = dev_get_platdata(dev);
struct iTCO_wdt_private *p;
unsigned long val32;
int ret;
@@ -411,33 +411,32 @@ static int iTCO_wdt_probe(struct platform_device *dev)
if (!pdata)
return -ENODEV;
- p = devm_kzalloc(&dev->dev, sizeof(*p), GFP_KERNEL);
+ p = devm_kzalloc(dev, sizeof(*p), GFP_KERNEL);
if (!p)
return -ENOMEM;
spin_lock_init(&p->io_lock);
- p->tco_res = platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_TCO);
+ p->tco_res = platform_get_resource(pdev, IORESOURCE_IO, ICH_RES_IO_TCO);
if (!p->tco_res)
return -ENODEV;
- p->smi_res = platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_SMI);
+ p->smi_res = platform_get_resource(pdev, IORESOURCE_IO, ICH_RES_IO_SMI);
if (!p->smi_res)
return -ENODEV;
p->iTCO_version = pdata->version;
- p->dev = dev;
- p->pdev = to_pci_dev(dev->dev.parent);
+ p->pcidev = to_pci_dev(dev->parent);
/*
* Get the Memory-Mapped GCS or PMC register, we need it for the
* NO_REBOOT flag (TCO v2 and v3).
*/
if (p->iTCO_version >= 2) {
- p->gcs_pmc_res = platform_get_resource(dev,
+ p->gcs_pmc_res = platform_get_resource(pdev,
IORESOURCE_MEM,
ICH_RES_MEM_GCS_PMC);
- p->gcs_pmc = devm_ioremap_resource(&dev->dev, p->gcs_pmc_res);
+ p->gcs_pmc = devm_ioremap_resource(dev, p->gcs_pmc_res);
if (IS_ERR(p->gcs_pmc))
return PTR_ERR(p->gcs_pmc);
}
@@ -453,9 +452,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
iTCO_wdt_set_NO_REBOOT_bit(p);
/* The TCO logic uses the TCO_EN bit in the SMI_EN register */
- if (!devm_request_region(&dev->dev, p->smi_res->start,
+ if (!devm_request_region(dev, p->smi_res->start,
resource_size(p->smi_res),
- dev->name)) {
+ pdev->name)) {
pr_err("I/O address 0x%04llx already in use, device disabled\n",
(u64)SMI_EN(p));
return -EBUSY;
@@ -470,9 +469,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
outl(val32, SMI_EN(p));
}
- if (!devm_request_region(&dev->dev, p->tco_res->start,
+ if (!devm_request_region(dev, p->tco_res->start,
resource_size(p->tco_res),
- dev->name)) {
+ pdev->name)) {
pr_err("I/O address 0x%04llx already in use, device disabled\n",
(u64)TCOBASE(p));
return -EBUSY;
@@ -505,10 +504,10 @@ static int iTCO_wdt_probe(struct platform_device *dev)
p->wddev.bootstatus = 0;
p->wddev.timeout = WATCHDOG_TIMEOUT;
watchdog_set_nowayout(&p->wddev, nowayout);
- p->wddev.parent = &dev->dev;
+ p->wddev.parent = dev;
watchdog_set_drvdata(&p->wddev, p);
- platform_set_drvdata(dev, p);
+ platform_set_drvdata(pdev, p);
/* Make sure the watchdog is not running */
iTCO_wdt_stop(&p->wddev);
@@ -521,7 +520,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
WATCHDOG_TIMEOUT);
}
- ret = devm_watchdog_register_device(&dev->dev, &p->wddev);
+ ret = devm_watchdog_register_device(dev, &p->wddev);
if (ret != 0) {
pr_err("cannot register watchdog device (err=%d)\n", ret);
return ret;
@@ -533,9 +532,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
return 0;
}
-static int iTCO_wdt_remove(struct platform_device *dev)
+static int iTCO_wdt_remove(struct platform_device *pdev)
{
- struct iTCO_wdt_private *p = platform_get_drvdata(dev);
+ struct iTCO_wdt_private *p = platform_get_drvdata(pdev);
/* Stop the timer before we leave */
if (!nowayout)
@@ -544,9 +543,9 @@ static int iTCO_wdt_remove(struct platform_device *dev)
return 0;
}
-static void iTCO_wdt_shutdown(struct platform_device *dev)
+static void iTCO_wdt_shutdown(struct platform_device *pdev)
{
- struct iTCO_wdt_private *p = platform_get_drvdata(dev);
+ struct iTCO_wdt_private *p = platform_get_drvdata(pdev);
iTCO_wdt_stop(&p->wddev);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/4] watchdog: iTCO_wdt: Simplify module init function
2017-01-03 14:39 [PATCH 1/4] watchdog: iTCO_wdt: Use allocated data structures Guenter Roeck
2017-01-03 14:39 ` [PATCH 2/4] watchdog: iTCO_wdt: Use device managed resources Guenter Roeck
2017-01-03 14:39 ` [PATCH 3/4] watchdog: iTCO_wdt: Use pdev for platform device and pci_dev for pci device Guenter Roeck
@ 2017-01-03 14:39 ` Guenter Roeck
2017-01-03 22:38 ` Andy Shevchenko
2017-01-03 22:44 ` [PATCH 1/4] watchdog: iTCO_wdt: Use allocated data structures Andy Shevchenko
3 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2017-01-03 14:39 UTC (permalink / raw)
To: Wim Van Sebroeck
Cc: linux-watchdog, linux-kernel, Matt Fleming, Guenter Roeck
The 'ret' variable in iTCO_wdt_init_module() does not add any value;
drop it.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/watchdog/iTCO_wdt.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index ad29ae03a30b..fc7712112412 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -612,15 +612,9 @@ static struct platform_driver iTCO_wdt_driver = {
static int __init iTCO_wdt_init_module(void)
{
- int err;
-
pr_info("Intel TCO WatchDog Timer Driver v%s\n", DRV_VERSION);
- err = platform_driver_register(&iTCO_wdt_driver);
- if (err)
- return err;
-
- return 0;
+ return platform_driver_register(&iTCO_wdt_driver);
}
static void __exit iTCO_wdt_cleanup_module(void)
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] watchdog: iTCO_wdt: Simplify module init function
2017-01-03 14:39 ` [PATCH 4/4] watchdog: iTCO_wdt: Simplify module init function Guenter Roeck
@ 2017-01-03 22:38 ` Andy Shevchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2017-01-03 22:38 UTC (permalink / raw)
To: Guenter Roeck
Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel, Matt Fleming
On Tue, Jan 3, 2017 at 4:39 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> The 'ret' variable in iTCO_wdt_init_module() does not add any value;
> drop it.
Perhaps 'err', otherwise:
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/watchdog/iTCO_wdt.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index ad29ae03a30b..fc7712112412 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -612,15 +612,9 @@ static struct platform_driver iTCO_wdt_driver = {
>
> static int __init iTCO_wdt_init_module(void)
> {
> - int err;
> -
> pr_info("Intel TCO WatchDog Timer Driver v%s\n", DRV_VERSION);
>
> - err = platform_driver_register(&iTCO_wdt_driver);
> - if (err)
> - return err;
> -
> - return 0;
> + return platform_driver_register(&iTCO_wdt_driver);
> }
>
> static void __exit iTCO_wdt_cleanup_module(void)
> --
> 2.7.4
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] watchdog: iTCO_wdt: Use pdev for platform device and pci_dev for pci device
2017-01-03 14:39 ` [PATCH 3/4] watchdog: iTCO_wdt: Use pdev for platform device and pci_dev for pci device Guenter Roeck
@ 2017-01-03 22:39 ` Andy Shevchenko
2017-01-03 23:40 ` Guenter Roeck
0 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2017-01-03 22:39 UTC (permalink / raw)
To: Guenter Roeck
Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel, Matt Fleming
On Tue, Jan 3, 2017 at 4:39 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> Use pdev for struct platform_device, pcidev for struct pci_dev, and dev
> for struct device variables to improve consistency.
>
> Remove 'struct platform_device *dev;' from struct iTCO_wdt_private since
> it was unused.
Would pci_dev work?
In any case
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/watchdog/iTCO_wdt.c | 53 ++++++++++++++++++++++-----------------------
> 1 file changed, 26 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index eed1dee6de19..ad29ae03a30b 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -102,9 +102,8 @@ struct iTCO_wdt_private {
> unsigned long __iomem *gcs_pmc;
> /* the lock for io operations */
> spinlock_t io_lock;
> - struct platform_device *dev;
> /* the PCI-device */
> - struct pci_dev *pdev;
> + struct pci_dev *pcidev;
> /* whether or not the watchdog has been suspended */
> bool suspended;
> };
> @@ -181,9 +180,9 @@ static void iTCO_wdt_set_NO_REBOOT_bit(struct iTCO_wdt_private *p)
> val32 |= no_reboot_bit(p);
> writel(val32, p->gcs_pmc);
> } else if (p->iTCO_version == 1) {
> - pci_read_config_dword(p->pdev, 0xd4, &val32);
> + pci_read_config_dword(p->pcidev, 0xd4, &val32);
> val32 |= no_reboot_bit(p);
> - pci_write_config_dword(p->pdev, 0xd4, val32);
> + pci_write_config_dword(p->pcidev, 0xd4, val32);
> }
> }
>
> @@ -200,11 +199,11 @@ static int iTCO_wdt_unset_NO_REBOOT_bit(struct iTCO_wdt_private *p)
>
> val32 = readl(p->gcs_pmc);
> } else if (p->iTCO_version == 1) {
> - pci_read_config_dword(p->pdev, 0xd4, &val32);
> + pci_read_config_dword(p->pcidev, 0xd4, &val32);
> val32 &= ~enable_bit;
> - pci_write_config_dword(p->pdev, 0xd4, val32);
> + pci_write_config_dword(p->pcidev, 0xd4, val32);
>
> - pci_read_config_dword(p->pdev, 0xd4, &val32);
> + pci_read_config_dword(p->pcidev, 0xd4, &val32);
> }
>
> if (val32 & enable_bit)
> @@ -401,9 +400,10 @@ static const struct watchdog_ops iTCO_wdt_ops = {
> * Init & exit routines
> */
>
> -static int iTCO_wdt_probe(struct platform_device *dev)
> +static int iTCO_wdt_probe(struct platform_device *pdev)
> {
> - struct itco_wdt_platform_data *pdata = dev_get_platdata(&dev->dev);
> + struct device *dev = &pdev->dev;
> + struct itco_wdt_platform_data *pdata = dev_get_platdata(dev);
> struct iTCO_wdt_private *p;
> unsigned long val32;
> int ret;
> @@ -411,33 +411,32 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> if (!pdata)
> return -ENODEV;
>
> - p = devm_kzalloc(&dev->dev, sizeof(*p), GFP_KERNEL);
> + p = devm_kzalloc(dev, sizeof(*p), GFP_KERNEL);
> if (!p)
> return -ENOMEM;
>
> spin_lock_init(&p->io_lock);
>
> - p->tco_res = platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_TCO);
> + p->tco_res = platform_get_resource(pdev, IORESOURCE_IO, ICH_RES_IO_TCO);
> if (!p->tco_res)
> return -ENODEV;
>
> - p->smi_res = platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_SMI);
> + p->smi_res = platform_get_resource(pdev, IORESOURCE_IO, ICH_RES_IO_SMI);
> if (!p->smi_res)
> return -ENODEV;
>
> p->iTCO_version = pdata->version;
> - p->dev = dev;
> - p->pdev = to_pci_dev(dev->dev.parent);
> + p->pcidev = to_pci_dev(dev->parent);
>
> /*
> * Get the Memory-Mapped GCS or PMC register, we need it for the
> * NO_REBOOT flag (TCO v2 and v3).
> */
> if (p->iTCO_version >= 2) {
> - p->gcs_pmc_res = platform_get_resource(dev,
> + p->gcs_pmc_res = platform_get_resource(pdev,
> IORESOURCE_MEM,
> ICH_RES_MEM_GCS_PMC);
> - p->gcs_pmc = devm_ioremap_resource(&dev->dev, p->gcs_pmc_res);
> + p->gcs_pmc = devm_ioremap_resource(dev, p->gcs_pmc_res);
> if (IS_ERR(p->gcs_pmc))
> return PTR_ERR(p->gcs_pmc);
> }
> @@ -453,9 +452,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> iTCO_wdt_set_NO_REBOOT_bit(p);
>
> /* The TCO logic uses the TCO_EN bit in the SMI_EN register */
> - if (!devm_request_region(&dev->dev, p->smi_res->start,
> + if (!devm_request_region(dev, p->smi_res->start,
> resource_size(p->smi_res),
> - dev->name)) {
> + pdev->name)) {
> pr_err("I/O address 0x%04llx already in use, device disabled\n",
> (u64)SMI_EN(p));
> return -EBUSY;
> @@ -470,9 +469,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> outl(val32, SMI_EN(p));
> }
>
> - if (!devm_request_region(&dev->dev, p->tco_res->start,
> + if (!devm_request_region(dev, p->tco_res->start,
> resource_size(p->tco_res),
> - dev->name)) {
> + pdev->name)) {
> pr_err("I/O address 0x%04llx already in use, device disabled\n",
> (u64)TCOBASE(p));
> return -EBUSY;
> @@ -505,10 +504,10 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> p->wddev.bootstatus = 0;
> p->wddev.timeout = WATCHDOG_TIMEOUT;
> watchdog_set_nowayout(&p->wddev, nowayout);
> - p->wddev.parent = &dev->dev;
> + p->wddev.parent = dev;
>
> watchdog_set_drvdata(&p->wddev, p);
> - platform_set_drvdata(dev, p);
> + platform_set_drvdata(pdev, p);
>
> /* Make sure the watchdog is not running */
> iTCO_wdt_stop(&p->wddev);
> @@ -521,7 +520,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> WATCHDOG_TIMEOUT);
> }
>
> - ret = devm_watchdog_register_device(&dev->dev, &p->wddev);
> + ret = devm_watchdog_register_device(dev, &p->wddev);
> if (ret != 0) {
> pr_err("cannot register watchdog device (err=%d)\n", ret);
> return ret;
> @@ -533,9 +532,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> return 0;
> }
>
> -static int iTCO_wdt_remove(struct platform_device *dev)
> +static int iTCO_wdt_remove(struct platform_device *pdev)
> {
> - struct iTCO_wdt_private *p = platform_get_drvdata(dev);
> + struct iTCO_wdt_private *p = platform_get_drvdata(pdev);
>
> /* Stop the timer before we leave */
> if (!nowayout)
> @@ -544,9 +543,9 @@ static int iTCO_wdt_remove(struct platform_device *dev)
> return 0;
> }
>
> -static void iTCO_wdt_shutdown(struct platform_device *dev)
> +static void iTCO_wdt_shutdown(struct platform_device *pdev)
> {
> - struct iTCO_wdt_private *p = platform_get_drvdata(dev);
> + struct iTCO_wdt_private *p = platform_get_drvdata(pdev);
>
> iTCO_wdt_stop(&p->wddev);
> }
> --
> 2.7.4
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] watchdog: iTCO_wdt: Use device managed resources
2017-01-03 14:39 ` [PATCH 2/4] watchdog: iTCO_wdt: Use device managed resources Guenter Roeck
@ 2017-01-03 22:41 ` Andy Shevchenko
2017-01-03 23:38 ` Guenter Roeck
2017-01-04 2:44 ` Guenter Roeck
0 siblings, 2 replies; 14+ messages in thread
From: Andy Shevchenko @ 2017-01-03 22:41 UTC (permalink / raw)
To: Guenter Roeck
Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel, Matt Fleming
On Tue, Jan 3, 2017 at 4:39 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> Using device managed resources simplifies error handling and cleanup,
> and to reduce the likelyhood of errors.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Does it make sense to convert to dev_err() at some point?
> ---
> drivers/watchdog/iTCO_wdt.c | 80 ++++++++++-----------------------------------
> 1 file changed, 17 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index a35a9164ccd0..eed1dee6de19 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -401,27 +401,6 @@ static const struct watchdog_ops iTCO_wdt_ops = {
> * Init & exit routines
> */
>
> -static void iTCO_wdt_cleanup(struct iTCO_wdt_private *p)
> -{
> - /* Stop the timer before we leave */
> - if (!nowayout)
> - iTCO_wdt_stop(&p->wddev);
> -
> - /* Deregister */
> - watchdog_unregister_device(&p->wddev);
> -
> - /* release resources */
> - release_region(p->tco_res->start,
> - resource_size(p->tco_res));
> - release_region(p->smi_res->start,
> - resource_size(p->smi_res));
> - if (p->iTCO_version >= 2) {
> - iounmap(p->gcs_pmc);
> - release_mem_region(p->gcs_pmc_res->start,
> - resource_size(p->gcs_pmc_res));
> - }
> -}
> -
> static int iTCO_wdt_probe(struct platform_device *dev)
> {
> struct itco_wdt_platform_data *pdata = dev_get_platdata(&dev->dev);
> @@ -458,41 +437,28 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> p->gcs_pmc_res = platform_get_resource(dev,
> IORESOURCE_MEM,
> ICH_RES_MEM_GCS_PMC);
> -
> - if (!p->gcs_pmc_res)
> - return -ENODEV;
> -
> - if (!request_mem_region(p->gcs_pmc_res->start,
> - resource_size(p->gcs_pmc_res),
> - dev->name))
> - return -EBUSY;
> -
> - p->gcs_pmc = ioremap(p->gcs_pmc_res->start,
> - resource_size(p->gcs_pmc_res));
> - if (!p->gcs_pmc) {
> - ret = -EIO;
> - goto unreg_gcs_pmc;
> - }
> + p->gcs_pmc = devm_ioremap_resource(&dev->dev, p->gcs_pmc_res);
> + if (IS_ERR(p->gcs_pmc))
> + return PTR_ERR(p->gcs_pmc);
> }
>
> /* Check chipset's NO_REBOOT bit */
> if (iTCO_wdt_unset_NO_REBOOT_bit(p) &&
> iTCO_vendor_check_noreboot_on()) {
> pr_info("unable to reset NO_REBOOT flag, device disabled by hardware/BIOS\n");
> - ret = -ENODEV; /* Cannot reset NO_REBOOT bit */
> - goto unmap_gcs_pmc;
> + return -ENODEV; /* Cannot reset NO_REBOOT bit */
> }
>
> /* Set the NO_REBOOT bit to prevent later reboots, just for sure */
> iTCO_wdt_set_NO_REBOOT_bit(p);
>
> /* The TCO logic uses the TCO_EN bit in the SMI_EN register */
> - if (!request_region(p->smi_res->start,
> - resource_size(p->smi_res), dev->name)) {
> + if (!devm_request_region(&dev->dev, p->smi_res->start,
> + resource_size(p->smi_res),
> + dev->name)) {
> pr_err("I/O address 0x%04llx already in use, device disabled\n",
> (u64)SMI_EN(p));
> - ret = -EBUSY;
> - goto unmap_gcs_pmc;
> + return -EBUSY;
> }
> if (turn_SMI_watchdog_clear_off >= p->iTCO_version) {
> /*
> @@ -504,12 +470,12 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> outl(val32, SMI_EN(p));
> }
>
> - if (!request_region(p->tco_res->start,
> - resource_size(p->tco_res), dev->name)) {
> + if (!devm_request_region(&dev->dev, p->tco_res->start,
> + resource_size(p->tco_res),
> + dev->name)) {
> pr_err("I/O address 0x%04llx already in use, device disabled\n",
> (u64)TCOBASE(p));
> - ret = -EBUSY;
> - goto unreg_smi;
> + return -EBUSY;
> }
>
> pr_info("Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n",
> @@ -555,37 +521,25 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> WATCHDOG_TIMEOUT);
> }
>
> - ret = watchdog_register_device(&p->wddev);
> + ret = devm_watchdog_register_device(&dev->dev, &p->wddev);
> if (ret != 0) {
> pr_err("cannot register watchdog device (err=%d)\n", ret);
> - goto unreg_tco;
> + return ret;
> }
>
> pr_info("initialized. heartbeat=%d sec (nowayout=%d)\n",
> heartbeat, nowayout);
>
> return 0;
> -
> -unreg_tco:
> - release_region(p->tco_res->start, resource_size(p->tco_res));
> -unreg_smi:
> - release_region(p->smi_res->start, resource_size(p->smi_res));
> -unmap_gcs_pmc:
> - if (p->iTCO_version >= 2)
> - iounmap(p->gcs_pmc);
> -unreg_gcs_pmc:
> - if (p->iTCO_version >= 2)
> - release_mem_region(p->gcs_pmc_res->start,
> - resource_size(p->gcs_pmc_res));
> - return ret;
> }
>
> static int iTCO_wdt_remove(struct platform_device *dev)
> {
> struct iTCO_wdt_private *p = platform_get_drvdata(dev);
>
> - if (p->tco_res || p->smi_res)
> - iTCO_wdt_cleanup(p);
> + /* Stop the timer before we leave */
> + if (!nowayout)
> + iTCO_wdt_stop(&p->wddev);
>
> return 0;
> }
> --
> 2.7.4
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] watchdog: iTCO_wdt: Use allocated data structures
2017-01-03 14:39 [PATCH 1/4] watchdog: iTCO_wdt: Use allocated data structures Guenter Roeck
` (2 preceding siblings ...)
2017-01-03 14:39 ` [PATCH 4/4] watchdog: iTCO_wdt: Simplify module init function Guenter Roeck
@ 2017-01-03 22:44 ` Andy Shevchenko
3 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2017-01-03 22:44 UTC (permalink / raw)
To: Guenter Roeck
Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel, Matt Fleming
On Tue, Jan 3, 2017 at 4:39 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> Allocate private data and the watchdog device to to avoid having
'too to' ?
> to clear it on remove and to enable subsequent simplifications.
I doubt it will be more than one device per platform, but change is
good by itself to reduce amount of global module variables and a such.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/watchdog/iTCO_wdt.c | 402 ++++++++++++++++++++++----------------------
> 1 file changed, 205 insertions(+), 197 deletions(-)
>
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index 06fcb6c8c917..a35a9164ccd0 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -72,22 +72,24 @@
>
> /* Address definitions for the TCO */
> /* TCO base address */
> -#define TCOBASE (iTCO_wdt_private.tco_res->start)
> +#define TCOBASE(p) ((p)->tco_res->start)
> /* SMI Control and Enable Register */
> -#define SMI_EN (iTCO_wdt_private.smi_res->start)
> -
> -#define TCO_RLD (TCOBASE + 0x00) /* TCO Timer Reload and Curr. Value */
> -#define TCOv1_TMR (TCOBASE + 0x01) /* TCOv1 Timer Initial Value */
> -#define TCO_DAT_IN (TCOBASE + 0x02) /* TCO Data In Register */
> -#define TCO_DAT_OUT (TCOBASE + 0x03) /* TCO Data Out Register */
> -#define TCO1_STS (TCOBASE + 0x04) /* TCO1 Status Register */
> -#define TCO2_STS (TCOBASE + 0x06) /* TCO2 Status Register */
> -#define TCO1_CNT (TCOBASE + 0x08) /* TCO1 Control Register */
> -#define TCO2_CNT (TCOBASE + 0x0a) /* TCO2 Control Register */
> -#define TCOv2_TMR (TCOBASE + 0x12) /* TCOv2 Timer Initial Value */
> +#define SMI_EN(p) ((p)->smi_res->start)
> +
> +#define TCO_RLD(p) (TCOBASE(p) + 0x00) /* TCO Timer Reload/Curr. Value */
> +#define TCOv1_TMR(p) (TCOBASE(p) + 0x01) /* TCOv1 Timer Initial Value*/
> +#define TCO_DAT_IN(p) (TCOBASE(p) + 0x02) /* TCO Data In Register */
> +#define TCO_DAT_OUT(p) (TCOBASE(p) + 0x03) /* TCO Data Out Register */
> +#define TCO1_STS(p) (TCOBASE(p) + 0x04) /* TCO1 Status Register */
> +#define TCO2_STS(p) (TCOBASE(p) + 0x06) /* TCO2 Status Register */
> +#define TCO1_CNT(p) (TCOBASE(p) + 0x08) /* TCO1 Control Register */
> +#define TCO2_CNT(p) (TCOBASE(p) + 0x0a) /* TCO2 Control Register */
> +#define TCOv2_TMR(p) (TCOBASE(p) + 0x12) /* TCOv2 Timer Initial Value*/
>
> /* internal variables */
> -static struct { /* this is private data for the iTCO_wdt device */
> +struct iTCO_wdt_private {
> + struct watchdog_device wddev;
> +
> /* TCO version/generation */
> unsigned int iTCO_version;
> struct resource *tco_res;
> @@ -105,7 +107,7 @@ static struct { /* this is private data for the iTCO_wdt device */
> struct pci_dev *pdev;
> /* whether or not the watchdog has been suspended */
> bool suspended;
> -} iTCO_wdt_private;
> +};
>
> /* module parameters */
> #define WATCHDOG_TIMEOUT 30 /* 30 sec default heartbeat */
> @@ -135,21 +137,23 @@ MODULE_PARM_DESC(turn_SMI_watchdog_clear_off,
> * every 0.6 seconds. v3's internal timer is stored as seconds (some
> * datasheets incorrectly state 0.6 seconds).
> */
> -static inline unsigned int seconds_to_ticks(int secs)
> +static inline unsigned int seconds_to_ticks(struct iTCO_wdt_private *p,
> + int secs)
> {
> - return iTCO_wdt_private.iTCO_version == 3 ? secs : (secs * 10) / 6;
> + return p->iTCO_version == 3 ? secs : (secs * 10) / 6;
> }
>
> -static inline unsigned int ticks_to_seconds(int ticks)
> +static inline unsigned int ticks_to_seconds(struct iTCO_wdt_private *p,
> + int ticks)
> {
> - return iTCO_wdt_private.iTCO_version == 3 ? ticks : (ticks * 6) / 10;
> + return p->iTCO_version == 3 ? ticks : (ticks * 6) / 10;
> }
>
> -static inline u32 no_reboot_bit(void)
> +static inline u32 no_reboot_bit(struct iTCO_wdt_private *p)
> {
> u32 enable_bit;
>
> - switch (iTCO_wdt_private.iTCO_version) {
> + switch (p->iTCO_version) {
> case 5:
> case 3:
> enable_bit = 0x00000010;
> @@ -167,40 +171,40 @@ static inline u32 no_reboot_bit(void)
> return enable_bit;
> }
>
> -static void iTCO_wdt_set_NO_REBOOT_bit(void)
> +static void iTCO_wdt_set_NO_REBOOT_bit(struct iTCO_wdt_private *p)
> {
> u32 val32;
>
> /* Set the NO_REBOOT bit: this disables reboots */
> - if (iTCO_wdt_private.iTCO_version >= 2) {
> - val32 = readl(iTCO_wdt_private.gcs_pmc);
> - val32 |= no_reboot_bit();
> - writel(val32, iTCO_wdt_private.gcs_pmc);
> - } else if (iTCO_wdt_private.iTCO_version == 1) {
> - pci_read_config_dword(iTCO_wdt_private.pdev, 0xd4, &val32);
> - val32 |= no_reboot_bit();
> - pci_write_config_dword(iTCO_wdt_private.pdev, 0xd4, val32);
> + if (p->iTCO_version >= 2) {
> + val32 = readl(p->gcs_pmc);
> + val32 |= no_reboot_bit(p);
> + writel(val32, p->gcs_pmc);
> + } else if (p->iTCO_version == 1) {
> + pci_read_config_dword(p->pdev, 0xd4, &val32);
> + val32 |= no_reboot_bit(p);
> + pci_write_config_dword(p->pdev, 0xd4, val32);
> }
> }
>
> -static int iTCO_wdt_unset_NO_REBOOT_bit(void)
> +static int iTCO_wdt_unset_NO_REBOOT_bit(struct iTCO_wdt_private *p)
> {
> - u32 enable_bit = no_reboot_bit();
> + u32 enable_bit = no_reboot_bit(p);
> u32 val32 = 0;
>
> /* Unset the NO_REBOOT bit: this enables reboots */
> - if (iTCO_wdt_private.iTCO_version >= 2) {
> - val32 = readl(iTCO_wdt_private.gcs_pmc);
> + if (p->iTCO_version >= 2) {
> + val32 = readl(p->gcs_pmc);
> val32 &= ~enable_bit;
> - writel(val32, iTCO_wdt_private.gcs_pmc);
> + writel(val32, p->gcs_pmc);
>
> - val32 = readl(iTCO_wdt_private.gcs_pmc);
> - } else if (iTCO_wdt_private.iTCO_version == 1) {
> - pci_read_config_dword(iTCO_wdt_private.pdev, 0xd4, &val32);
> + val32 = readl(p->gcs_pmc);
> + } else if (p->iTCO_version == 1) {
> + pci_read_config_dword(p->pdev, 0xd4, &val32);
> val32 &= ~enable_bit;
> - pci_write_config_dword(iTCO_wdt_private.pdev, 0xd4, val32);
> + pci_write_config_dword(p->pdev, 0xd4, val32);
>
> - pci_read_config_dword(iTCO_wdt_private.pdev, 0xd4, &val32);
> + pci_read_config_dword(p->pdev, 0xd4, &val32);
> }
>
> if (val32 & enable_bit)
> @@ -211,32 +215,33 @@ static int iTCO_wdt_unset_NO_REBOOT_bit(void)
>
> static int iTCO_wdt_start(struct watchdog_device *wd_dev)
> {
> + struct iTCO_wdt_private *p = watchdog_get_drvdata(wd_dev);
> unsigned int val;
>
> - spin_lock(&iTCO_wdt_private.io_lock);
> + spin_lock(&p->io_lock);
>
> - iTCO_vendor_pre_start(iTCO_wdt_private.smi_res, wd_dev->timeout);
> + iTCO_vendor_pre_start(p->smi_res, wd_dev->timeout);
>
> /* disable chipset's NO_REBOOT bit */
> - if (iTCO_wdt_unset_NO_REBOOT_bit()) {
> - spin_unlock(&iTCO_wdt_private.io_lock);
> + if (iTCO_wdt_unset_NO_REBOOT_bit(p)) {
> + spin_unlock(&p->io_lock);
> pr_err("failed to reset NO_REBOOT flag, reboot disabled by hardware/BIOS\n");
> return -EIO;
> }
>
> /* Force the timer to its reload value by writing to the TCO_RLD
> register */
> - if (iTCO_wdt_private.iTCO_version >= 2)
> - outw(0x01, TCO_RLD);
> - else if (iTCO_wdt_private.iTCO_version == 1)
> - outb(0x01, TCO_RLD);
> + if (p->iTCO_version >= 2)
> + outw(0x01, TCO_RLD(p));
> + else if (p->iTCO_version == 1)
> + outb(0x01, TCO_RLD(p));
>
> /* Bit 11: TCO Timer Halt -> 0 = The TCO timer is enabled to count */
> - val = inw(TCO1_CNT);
> + val = inw(TCO1_CNT(p));
> val &= 0xf7ff;
> - outw(val, TCO1_CNT);
> - val = inw(TCO1_CNT);
> - spin_unlock(&iTCO_wdt_private.io_lock);
> + outw(val, TCO1_CNT(p));
> + val = inw(TCO1_CNT(p));
> + spin_unlock(&p->io_lock);
>
> if (val & 0x0800)
> return -1;
> @@ -245,22 +250,23 @@ static int iTCO_wdt_start(struct watchdog_device *wd_dev)
>
> static int iTCO_wdt_stop(struct watchdog_device *wd_dev)
> {
> + struct iTCO_wdt_private *p = watchdog_get_drvdata(wd_dev);
> unsigned int val;
>
> - spin_lock(&iTCO_wdt_private.io_lock);
> + spin_lock(&p->io_lock);
>
> - iTCO_vendor_pre_stop(iTCO_wdt_private.smi_res);
> + iTCO_vendor_pre_stop(p->smi_res);
>
> /* Bit 11: TCO Timer Halt -> 1 = The TCO timer is disabled */
> - val = inw(TCO1_CNT);
> + val = inw(TCO1_CNT(p));
> val |= 0x0800;
> - outw(val, TCO1_CNT);
> - val = inw(TCO1_CNT);
> + outw(val, TCO1_CNT(p));
> + val = inw(TCO1_CNT(p));
>
> /* Set the NO_REBOOT bit to prevent later reboots, just for sure */
> - iTCO_wdt_set_NO_REBOOT_bit();
> + iTCO_wdt_set_NO_REBOOT_bit(p);
>
> - spin_unlock(&iTCO_wdt_private.io_lock);
> + spin_unlock(&p->io_lock);
>
> if ((val & 0x0800) == 0)
> return -1;
> @@ -269,67 +275,70 @@ static int iTCO_wdt_stop(struct watchdog_device *wd_dev)
>
> static int iTCO_wdt_ping(struct watchdog_device *wd_dev)
> {
> - spin_lock(&iTCO_wdt_private.io_lock);
> + struct iTCO_wdt_private *p = watchdog_get_drvdata(wd_dev);
>
> - iTCO_vendor_pre_keepalive(iTCO_wdt_private.smi_res, wd_dev->timeout);
> + spin_lock(&p->io_lock);
> +
> + iTCO_vendor_pre_keepalive(p->smi_res, wd_dev->timeout);
>
> /* Reload the timer by writing to the TCO Timer Counter register */
> - if (iTCO_wdt_private.iTCO_version >= 2) {
> - outw(0x01, TCO_RLD);
> - } else if (iTCO_wdt_private.iTCO_version == 1) {
> + if (p->iTCO_version >= 2) {
> + outw(0x01, TCO_RLD(p));
> + } else if (p->iTCO_version == 1) {
> /* Reset the timeout status bit so that the timer
> * needs to count down twice again before rebooting */
> - outw(0x0008, TCO1_STS); /* write 1 to clear bit */
> + outw(0x0008, TCO1_STS(p)); /* write 1 to clear bit */
>
> - outb(0x01, TCO_RLD);
> + outb(0x01, TCO_RLD(p));
> }
>
> - spin_unlock(&iTCO_wdt_private.io_lock);
> + spin_unlock(&p->io_lock);
> return 0;
> }
>
> static int iTCO_wdt_set_timeout(struct watchdog_device *wd_dev, unsigned int t)
> {
> + struct iTCO_wdt_private *p = watchdog_get_drvdata(wd_dev);
> unsigned int val16;
> unsigned char val8;
> unsigned int tmrval;
>
> - tmrval = seconds_to_ticks(t);
> + tmrval = seconds_to_ticks(p, t);
>
> /* For TCO v1 the timer counts down twice before rebooting */
> - if (iTCO_wdt_private.iTCO_version == 1)
> + if (p->iTCO_version == 1)
> tmrval /= 2;
>
> /* from the specs: */
> /* "Values of 0h-3h are ignored and should not be attempted" */
> if (tmrval < 0x04)
> return -EINVAL;
> - if (((iTCO_wdt_private.iTCO_version >= 2) && (tmrval > 0x3ff)) ||
> - ((iTCO_wdt_private.iTCO_version == 1) && (tmrval > 0x03f)))
> + if ((p->iTCO_version >= 2 && tmrval > 0x3ff) ||
> + (p->iTCO_version == 1 && tmrval > 0x03f))
> return -EINVAL;
>
> iTCO_vendor_pre_set_heartbeat(tmrval);
>
> /* Write new heartbeat to watchdog */
> - if (iTCO_wdt_private.iTCO_version >= 2) {
> - spin_lock(&iTCO_wdt_private.io_lock);
> - val16 = inw(TCOv2_TMR);
> + if (p->iTCO_version >= 2) {
> + spin_lock(&p->io_lock);
> + val16 = inw(TCOv2_TMR(p));
> val16 &= 0xfc00;
> val16 |= tmrval;
> - outw(val16, TCOv2_TMR);
> - val16 = inw(TCOv2_TMR);
> - spin_unlock(&iTCO_wdt_private.io_lock);
> + outw(val16, TCOv2_TMR(p));
> + val16 = inw(TCOv2_TMR(p));
> + spin_unlock(&p->io_lock);
>
> if ((val16 & 0x3ff) != tmrval)
> return -EINVAL;
> - } else if (iTCO_wdt_private.iTCO_version == 1) {
> - spin_lock(&iTCO_wdt_private.io_lock);
> - val8 = inb(TCOv1_TMR);
> + } else if (p->iTCO_version == 1) {
> + spin_lock(&p->io_lock);
> + val8 = inb(TCOv1_TMR(p));
> val8 &= 0xc0;
> val8 |= (tmrval & 0xff);
> - outb(val8, TCOv1_TMR);
> - val8 = inb(TCOv1_TMR);
> - spin_unlock(&iTCO_wdt_private.io_lock);
> + outb(val8, TCOv1_TMR(p));
> + val8 = inb(TCOv1_TMR(p));
> + spin_unlock(&p->io_lock);
>
> if ((val8 & 0x3f) != tmrval)
> return -EINVAL;
> @@ -341,27 +350,28 @@ static int iTCO_wdt_set_timeout(struct watchdog_device *wd_dev, unsigned int t)
>
> static unsigned int iTCO_wdt_get_timeleft(struct watchdog_device *wd_dev)
> {
> + struct iTCO_wdt_private *p = watchdog_get_drvdata(wd_dev);
> unsigned int val16;
> unsigned char val8;
> unsigned int time_left = 0;
>
> /* read the TCO Timer */
> - if (iTCO_wdt_private.iTCO_version >= 2) {
> - spin_lock(&iTCO_wdt_private.io_lock);
> - val16 = inw(TCO_RLD);
> + if (p->iTCO_version >= 2) {
> + spin_lock(&p->io_lock);
> + val16 = inw(TCO_RLD(p));
> val16 &= 0x3ff;
> - spin_unlock(&iTCO_wdt_private.io_lock);
> + spin_unlock(&p->io_lock);
>
> - time_left = ticks_to_seconds(val16);
> - } else if (iTCO_wdt_private.iTCO_version == 1) {
> - spin_lock(&iTCO_wdt_private.io_lock);
> - val8 = inb(TCO_RLD);
> + time_left = ticks_to_seconds(p, val16);
> + } else if (p->iTCO_version == 1) {
> + spin_lock(&p->io_lock);
> + val8 = inb(TCO_RLD(p));
> val8 &= 0x3f;
> - if (!(inw(TCO1_STS) & 0x0008))
> - val8 += (inb(TCOv1_TMR) & 0x3f);
> - spin_unlock(&iTCO_wdt_private.io_lock);
> + if (!(inw(TCO1_STS(p)) & 0x0008))
> + val8 += (inb(TCOv1_TMR(p)) & 0x3f);
> + spin_unlock(&p->io_lock);
>
> - time_left = ticks_to_seconds(val8);
> + time_left = ticks_to_seconds(p, val8);
> }
> return time_left;
> }
> @@ -387,166 +397,165 @@ static const struct watchdog_ops iTCO_wdt_ops = {
> .get_timeleft = iTCO_wdt_get_timeleft,
> };
>
> -static struct watchdog_device iTCO_wdt_watchdog_dev = {
> - .info = &ident,
> - .ops = &iTCO_wdt_ops,
> -};
> -
> /*
> * Init & exit routines
> */
>
> -static void iTCO_wdt_cleanup(void)
> +static void iTCO_wdt_cleanup(struct iTCO_wdt_private *p)
> {
> /* Stop the timer before we leave */
> if (!nowayout)
> - iTCO_wdt_stop(&iTCO_wdt_watchdog_dev);
> + iTCO_wdt_stop(&p->wddev);
>
> /* Deregister */
> - watchdog_unregister_device(&iTCO_wdt_watchdog_dev);
> + watchdog_unregister_device(&p->wddev);
>
> /* release resources */
> - release_region(iTCO_wdt_private.tco_res->start,
> - resource_size(iTCO_wdt_private.tco_res));
> - release_region(iTCO_wdt_private.smi_res->start,
> - resource_size(iTCO_wdt_private.smi_res));
> - if (iTCO_wdt_private.iTCO_version >= 2) {
> - iounmap(iTCO_wdt_private.gcs_pmc);
> - release_mem_region(iTCO_wdt_private.gcs_pmc_res->start,
> - resource_size(iTCO_wdt_private.gcs_pmc_res));
> + release_region(p->tco_res->start,
> + resource_size(p->tco_res));
> + release_region(p->smi_res->start,
> + resource_size(p->smi_res));
> + if (p->iTCO_version >= 2) {
> + iounmap(p->gcs_pmc);
> + release_mem_region(p->gcs_pmc_res->start,
> + resource_size(p->gcs_pmc_res));
> }
> -
> - iTCO_wdt_private.tco_res = NULL;
> - iTCO_wdt_private.smi_res = NULL;
> - iTCO_wdt_private.gcs_pmc_res = NULL;
> - iTCO_wdt_private.gcs_pmc = NULL;
> }
>
> static int iTCO_wdt_probe(struct platform_device *dev)
> {
> - int ret = -ENODEV;
> - unsigned long val32;
> struct itco_wdt_platform_data *pdata = dev_get_platdata(&dev->dev);
> + struct iTCO_wdt_private *p;
> + unsigned long val32;
> + int ret;
>
> if (!pdata)
> - goto out;
> + return -ENODEV;
>
> - spin_lock_init(&iTCO_wdt_private.io_lock);
> + p = devm_kzalloc(&dev->dev, sizeof(*p), GFP_KERNEL);
> + if (!p)
> + return -ENOMEM;
>
> - iTCO_wdt_private.tco_res =
> - platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_TCO);
> - if (!iTCO_wdt_private.tco_res)
> - goto out;
> + spin_lock_init(&p->io_lock);
>
> - iTCO_wdt_private.smi_res =
> - platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_SMI);
> - if (!iTCO_wdt_private.smi_res)
> - goto out;
> + p->tco_res = platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_TCO);
> + if (!p->tco_res)
> + return -ENODEV;
>
> - iTCO_wdt_private.iTCO_version = pdata->version;
> - iTCO_wdt_private.dev = dev;
> - iTCO_wdt_private.pdev = to_pci_dev(dev->dev.parent);
> + p->smi_res = platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_SMI);
> + if (!p->smi_res)
> + return -ENODEV;
> +
> + p->iTCO_version = pdata->version;
> + p->dev = dev;
> + p->pdev = to_pci_dev(dev->dev.parent);
>
> /*
> * Get the Memory-Mapped GCS or PMC register, we need it for the
> * NO_REBOOT flag (TCO v2 and v3).
> */
> - if (iTCO_wdt_private.iTCO_version >= 2) {
> - iTCO_wdt_private.gcs_pmc_res = platform_get_resource(dev,
> - IORESOURCE_MEM,
> - ICH_RES_MEM_GCS_PMC);
> -
> - if (!iTCO_wdt_private.gcs_pmc_res)
> - goto out;
> -
> - if (!request_mem_region(iTCO_wdt_private.gcs_pmc_res->start,
> - resource_size(iTCO_wdt_private.gcs_pmc_res), dev->name)) {
> - ret = -EBUSY;
> - goto out;
> - }
> - iTCO_wdt_private.gcs_pmc = ioremap(iTCO_wdt_private.gcs_pmc_res->start,
> - resource_size(iTCO_wdt_private.gcs_pmc_res));
> - if (!iTCO_wdt_private.gcs_pmc) {
> + if (p->iTCO_version >= 2) {
> + p->gcs_pmc_res = platform_get_resource(dev,
> + IORESOURCE_MEM,
> + ICH_RES_MEM_GCS_PMC);
> +
> + if (!p->gcs_pmc_res)
> + return -ENODEV;
> +
> + if (!request_mem_region(p->gcs_pmc_res->start,
> + resource_size(p->gcs_pmc_res),
> + dev->name))
> + return -EBUSY;
> +
> + p->gcs_pmc = ioremap(p->gcs_pmc_res->start,
> + resource_size(p->gcs_pmc_res));
> + if (!p->gcs_pmc) {
> ret = -EIO;
> goto unreg_gcs_pmc;
> }
> }
>
> /* Check chipset's NO_REBOOT bit */
> - if (iTCO_wdt_unset_NO_REBOOT_bit() && iTCO_vendor_check_noreboot_on()) {
> + if (iTCO_wdt_unset_NO_REBOOT_bit(p) &&
> + iTCO_vendor_check_noreboot_on()) {
> pr_info("unable to reset NO_REBOOT flag, device disabled by hardware/BIOS\n");
> ret = -ENODEV; /* Cannot reset NO_REBOOT bit */
> goto unmap_gcs_pmc;
> }
>
> /* Set the NO_REBOOT bit to prevent later reboots, just for sure */
> - iTCO_wdt_set_NO_REBOOT_bit();
> + iTCO_wdt_set_NO_REBOOT_bit(p);
>
> /* The TCO logic uses the TCO_EN bit in the SMI_EN register */
> - if (!request_region(iTCO_wdt_private.smi_res->start,
> - resource_size(iTCO_wdt_private.smi_res), dev->name)) {
> + if (!request_region(p->smi_res->start,
> + resource_size(p->smi_res), dev->name)) {
> pr_err("I/O address 0x%04llx already in use, device disabled\n",
> - (u64)SMI_EN);
> + (u64)SMI_EN(p));
> ret = -EBUSY;
> goto unmap_gcs_pmc;
> }
> - if (turn_SMI_watchdog_clear_off >= iTCO_wdt_private.iTCO_version) {
> + if (turn_SMI_watchdog_clear_off >= p->iTCO_version) {
> /*
> * Bit 13: TCO_EN -> 0
> * Disables TCO logic generating an SMI#
> */
> - val32 = inl(SMI_EN);
> + val32 = inl(SMI_EN(p));
> val32 &= 0xffffdfff; /* Turn off SMI clearing watchdog */
> - outl(val32, SMI_EN);
> + outl(val32, SMI_EN(p));
> }
>
> - if (!request_region(iTCO_wdt_private.tco_res->start,
> - resource_size(iTCO_wdt_private.tco_res), dev->name)) {
> + if (!request_region(p->tco_res->start,
> + resource_size(p->tco_res), dev->name)) {
> pr_err("I/O address 0x%04llx already in use, device disabled\n",
> - (u64)TCOBASE);
> + (u64)TCOBASE(p));
> ret = -EBUSY;
> goto unreg_smi;
> }
>
> pr_info("Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n",
> - pdata->name, pdata->version, (u64)TCOBASE);
> + pdata->name, pdata->version, (u64)TCOBASE(p));
>
> /* Clear out the (probably old) status */
> - switch (iTCO_wdt_private.iTCO_version) {
> + switch (p->iTCO_version) {
> case 5:
> case 4:
> - outw(0x0008, TCO1_STS); /* Clear the Time Out Status bit */
> - outw(0x0002, TCO2_STS); /* Clear SECOND_TO_STS bit */
> + outw(0x0008, TCO1_STS(p)); /* Clear the Time Out Status bit */
> + outw(0x0002, TCO2_STS(p)); /* Clear SECOND_TO_STS bit */
> break;
> case 3:
> - outl(0x20008, TCO1_STS);
> + outl(0x20008, TCO1_STS(p));
> break;
> case 2:
> case 1:
> default:
> - outw(0x0008, TCO1_STS); /* Clear the Time Out Status bit */
> - outw(0x0002, TCO2_STS); /* Clear SECOND_TO_STS bit */
> - outw(0x0004, TCO2_STS); /* Clear BOOT_STS bit */
> + outw(0x0008, TCO1_STS(p)); /* Clear the Time Out Status bit */
> + outw(0x0002, TCO2_STS(p)); /* Clear SECOND_TO_STS bit */
> + outw(0x0004, TCO2_STS(p)); /* Clear BOOT_STS bit */
> break;
> }
>
> - iTCO_wdt_watchdog_dev.bootstatus = 0;
> - iTCO_wdt_watchdog_dev.timeout = WATCHDOG_TIMEOUT;
> - watchdog_set_nowayout(&iTCO_wdt_watchdog_dev, nowayout);
> - iTCO_wdt_watchdog_dev.parent = &dev->dev;
> + p->wddev.info = &ident,
> + p->wddev.ops = &iTCO_wdt_ops,
> + p->wddev.bootstatus = 0;
> + p->wddev.timeout = WATCHDOG_TIMEOUT;
> + watchdog_set_nowayout(&p->wddev, nowayout);
> + p->wddev.parent = &dev->dev;
> +
> + watchdog_set_drvdata(&p->wddev, p);
> + platform_set_drvdata(dev, p);
>
> /* Make sure the watchdog is not running */
> - iTCO_wdt_stop(&iTCO_wdt_watchdog_dev);
> + iTCO_wdt_stop(&p->wddev);
>
> /* Check that the heartbeat value is within it's range;
> if not reset to the default */
> - if (iTCO_wdt_set_timeout(&iTCO_wdt_watchdog_dev, heartbeat)) {
> - iTCO_wdt_set_timeout(&iTCO_wdt_watchdog_dev, WATCHDOG_TIMEOUT);
> + if (iTCO_wdt_set_timeout(&p->wddev, heartbeat)) {
> + iTCO_wdt_set_timeout(&p->wddev, WATCHDOG_TIMEOUT);
> pr_info("timeout value out of range, using %d\n",
> WATCHDOG_TIMEOUT);
> }
>
> - ret = watchdog_register_device(&iTCO_wdt_watchdog_dev);
> + ret = watchdog_register_device(&p->wddev);
> if (ret != 0) {
> pr_err("cannot register watchdog device (err=%d)\n", ret);
> goto unreg_tco;
> @@ -558,38 +567,34 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> return 0;
>
> unreg_tco:
> - release_region(iTCO_wdt_private.tco_res->start,
> - resource_size(iTCO_wdt_private.tco_res));
> + release_region(p->tco_res->start, resource_size(p->tco_res));
> unreg_smi:
> - release_region(iTCO_wdt_private.smi_res->start,
> - resource_size(iTCO_wdt_private.smi_res));
> + release_region(p->smi_res->start, resource_size(p->smi_res));
> unmap_gcs_pmc:
> - if (iTCO_wdt_private.iTCO_version >= 2)
> - iounmap(iTCO_wdt_private.gcs_pmc);
> + if (p->iTCO_version >= 2)
> + iounmap(p->gcs_pmc);
> unreg_gcs_pmc:
> - if (iTCO_wdt_private.iTCO_version >= 2)
> - release_mem_region(iTCO_wdt_private.gcs_pmc_res->start,
> - resource_size(iTCO_wdt_private.gcs_pmc_res));
> -out:
> - iTCO_wdt_private.tco_res = NULL;
> - iTCO_wdt_private.smi_res = NULL;
> - iTCO_wdt_private.gcs_pmc_res = NULL;
> - iTCO_wdt_private.gcs_pmc = NULL;
> -
> + if (p->iTCO_version >= 2)
> + release_mem_region(p->gcs_pmc_res->start,
> + resource_size(p->gcs_pmc_res));
> return ret;
> }
>
> static int iTCO_wdt_remove(struct platform_device *dev)
> {
> - if (iTCO_wdt_private.tco_res || iTCO_wdt_private.smi_res)
> - iTCO_wdt_cleanup();
> + struct iTCO_wdt_private *p = platform_get_drvdata(dev);
> +
> + if (p->tco_res || p->smi_res)
> + iTCO_wdt_cleanup(p);
>
> return 0;
> }
>
> static void iTCO_wdt_shutdown(struct platform_device *dev)
> {
> - iTCO_wdt_stop(NULL);
> + struct iTCO_wdt_private *p = platform_get_drvdata(dev);
> +
> + iTCO_wdt_stop(&p->wddev);
> }
>
> #ifdef CONFIG_PM_SLEEP
> @@ -610,21 +615,24 @@ static inline bool need_suspend(void) { return true; }
>
> static int iTCO_wdt_suspend_noirq(struct device *dev)
> {
> + struct iTCO_wdt_private *p = dev_get_drvdata(dev);
> int ret = 0;
>
> - iTCO_wdt_private.suspended = false;
> - if (watchdog_active(&iTCO_wdt_watchdog_dev) && need_suspend()) {
> - ret = iTCO_wdt_stop(&iTCO_wdt_watchdog_dev);
> + p->suspended = false;
> + if (watchdog_active(&p->wddev) && need_suspend()) {
> + ret = iTCO_wdt_stop(&p->wddev);
> if (!ret)
> - iTCO_wdt_private.suspended = true;
> + p->suspended = true;
> }
> return ret;
> }
>
> static int iTCO_wdt_resume_noirq(struct device *dev)
> {
> - if (iTCO_wdt_private.suspended)
> - iTCO_wdt_start(&iTCO_wdt_watchdog_dev);
> + struct iTCO_wdt_private *p = dev_get_drvdata(dev);
> +
> + if (p->suspended)
> + iTCO_wdt_start(&p->wddev);
>
> return 0;
> }
> --
> 2.7.4
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] watchdog: iTCO_wdt: Use device managed resources
2017-01-03 22:41 ` Andy Shevchenko
@ 2017-01-03 23:38 ` Guenter Roeck
2017-01-04 2:44 ` Guenter Roeck
1 sibling, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2017-01-03 23:38 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel, Matt Fleming
On Wed, Jan 04, 2017 at 12:41:56AM +0200, Andy Shevchenko wrote:
> On Tue, Jan 3, 2017 at 4:39 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > Using device managed resources simplifies error handling and cleanup,
> > and to reduce the likelyhood of errors.
> >
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Does it make sense to convert to dev_err() at some point?
>
Sounds like an idea. Let me play with it.
Thanks,
Guenter
> > ---
> > drivers/watchdog/iTCO_wdt.c | 80 ++++++++++-----------------------------------
> > 1 file changed, 17 insertions(+), 63 deletions(-)
> >
> > diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> > index a35a9164ccd0..eed1dee6de19 100644
> > --- a/drivers/watchdog/iTCO_wdt.c
> > +++ b/drivers/watchdog/iTCO_wdt.c
> > @@ -401,27 +401,6 @@ static const struct watchdog_ops iTCO_wdt_ops = {
> > * Init & exit routines
> > */
> >
> > -static void iTCO_wdt_cleanup(struct iTCO_wdt_private *p)
> > -{
> > - /* Stop the timer before we leave */
> > - if (!nowayout)
> > - iTCO_wdt_stop(&p->wddev);
> > -
> > - /* Deregister */
> > - watchdog_unregister_device(&p->wddev);
> > -
> > - /* release resources */
> > - release_region(p->tco_res->start,
> > - resource_size(p->tco_res));
> > - release_region(p->smi_res->start,
> > - resource_size(p->smi_res));
> > - if (p->iTCO_version >= 2) {
> > - iounmap(p->gcs_pmc);
> > - release_mem_region(p->gcs_pmc_res->start,
> > - resource_size(p->gcs_pmc_res));
> > - }
> > -}
> > -
> > static int iTCO_wdt_probe(struct platform_device *dev)
> > {
> > struct itco_wdt_platform_data *pdata = dev_get_platdata(&dev->dev);
> > @@ -458,41 +437,28 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> > p->gcs_pmc_res = platform_get_resource(dev,
> > IORESOURCE_MEM,
> > ICH_RES_MEM_GCS_PMC);
> > -
> > - if (!p->gcs_pmc_res)
> > - return -ENODEV;
> > -
> > - if (!request_mem_region(p->gcs_pmc_res->start,
> > - resource_size(p->gcs_pmc_res),
> > - dev->name))
> > - return -EBUSY;
> > -
> > - p->gcs_pmc = ioremap(p->gcs_pmc_res->start,
> > - resource_size(p->gcs_pmc_res));
> > - if (!p->gcs_pmc) {
> > - ret = -EIO;
> > - goto unreg_gcs_pmc;
> > - }
> > + p->gcs_pmc = devm_ioremap_resource(&dev->dev, p->gcs_pmc_res);
> > + if (IS_ERR(p->gcs_pmc))
> > + return PTR_ERR(p->gcs_pmc);
> > }
> >
> > /* Check chipset's NO_REBOOT bit */
> > if (iTCO_wdt_unset_NO_REBOOT_bit(p) &&
> > iTCO_vendor_check_noreboot_on()) {
> > pr_info("unable to reset NO_REBOOT flag, device disabled by hardware/BIOS\n");
> > - ret = -ENODEV; /* Cannot reset NO_REBOOT bit */
> > - goto unmap_gcs_pmc;
> > + return -ENODEV; /* Cannot reset NO_REBOOT bit */
> > }
> >
> > /* Set the NO_REBOOT bit to prevent later reboots, just for sure */
> > iTCO_wdt_set_NO_REBOOT_bit(p);
> >
> > /* The TCO logic uses the TCO_EN bit in the SMI_EN register */
> > - if (!request_region(p->smi_res->start,
> > - resource_size(p->smi_res), dev->name)) {
> > + if (!devm_request_region(&dev->dev, p->smi_res->start,
> > + resource_size(p->smi_res),
> > + dev->name)) {
> > pr_err("I/O address 0x%04llx already in use, device disabled\n",
> > (u64)SMI_EN(p));
> > - ret = -EBUSY;
> > - goto unmap_gcs_pmc;
> > + return -EBUSY;
> > }
> > if (turn_SMI_watchdog_clear_off >= p->iTCO_version) {
> > /*
> > @@ -504,12 +470,12 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> > outl(val32, SMI_EN(p));
> > }
> >
> > - if (!request_region(p->tco_res->start,
> > - resource_size(p->tco_res), dev->name)) {
> > + if (!devm_request_region(&dev->dev, p->tco_res->start,
> > + resource_size(p->tco_res),
> > + dev->name)) {
> > pr_err("I/O address 0x%04llx already in use, device disabled\n",
> > (u64)TCOBASE(p));
> > - ret = -EBUSY;
> > - goto unreg_smi;
> > + return -EBUSY;
> > }
> >
> > pr_info("Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n",
> > @@ -555,37 +521,25 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> > WATCHDOG_TIMEOUT);
> > }
> >
> > - ret = watchdog_register_device(&p->wddev);
> > + ret = devm_watchdog_register_device(&dev->dev, &p->wddev);
> > if (ret != 0) {
> > pr_err("cannot register watchdog device (err=%d)\n", ret);
> > - goto unreg_tco;
> > + return ret;
> > }
> >
> > pr_info("initialized. heartbeat=%d sec (nowayout=%d)\n",
> > heartbeat, nowayout);
> >
> > return 0;
> > -
> > -unreg_tco:
> > - release_region(p->tco_res->start, resource_size(p->tco_res));
> > -unreg_smi:
> > - release_region(p->smi_res->start, resource_size(p->smi_res));
> > -unmap_gcs_pmc:
> > - if (p->iTCO_version >= 2)
> > - iounmap(p->gcs_pmc);
> > -unreg_gcs_pmc:
> > - if (p->iTCO_version >= 2)
> > - release_mem_region(p->gcs_pmc_res->start,
> > - resource_size(p->gcs_pmc_res));
> > - return ret;
> > }
> >
> > static int iTCO_wdt_remove(struct platform_device *dev)
> > {
> > struct iTCO_wdt_private *p = platform_get_drvdata(dev);
> >
> > - if (p->tco_res || p->smi_res)
> > - iTCO_wdt_cleanup(p);
> > + /* Stop the timer before we leave */
> > + if (!nowayout)
> > + iTCO_wdt_stop(&p->wddev);
> >
> > return 0;
> > }
> > --
> > 2.7.4
> >
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] watchdog: iTCO_wdt: Use pdev for platform device and pci_dev for pci device
2017-01-03 22:39 ` Andy Shevchenko
@ 2017-01-03 23:40 ` Guenter Roeck
2017-01-03 23:48 ` Andy Shevchenko
0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2017-01-03 23:40 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel, Matt Fleming
On Wed, Jan 04, 2017 at 12:39:59AM +0200, Andy Shevchenko wrote:
> On Tue, Jan 3, 2017 at 4:39 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> > Use pdev for struct platform_device, pcidev for struct pci_dev, and dev
> > for struct device variables to improve consistency.
> >
> > Remove 'struct platform_device *dev;' from struct iTCO_wdt_private since
> > it was unused.
>
> Would pci_dev work?
>
Sure, or maybe just 'pci'. Any preference ?
Thanks,
Guenter
> In any case
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> >
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> > drivers/watchdog/iTCO_wdt.c | 53 ++++++++++++++++++++++-----------------------
> > 1 file changed, 26 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> > index eed1dee6de19..ad29ae03a30b 100644
> > --- a/drivers/watchdog/iTCO_wdt.c
> > +++ b/drivers/watchdog/iTCO_wdt.c
> > @@ -102,9 +102,8 @@ struct iTCO_wdt_private {
> > unsigned long __iomem *gcs_pmc;
> > /* the lock for io operations */
> > spinlock_t io_lock;
> > - struct platform_device *dev;
> > /* the PCI-device */
> > - struct pci_dev *pdev;
> > + struct pci_dev *pcidev;
> > /* whether or not the watchdog has been suspended */
> > bool suspended;
> > };
> > @@ -181,9 +180,9 @@ static void iTCO_wdt_set_NO_REBOOT_bit(struct iTCO_wdt_private *p)
> > val32 |= no_reboot_bit(p);
> > writel(val32, p->gcs_pmc);
> > } else if (p->iTCO_version == 1) {
> > - pci_read_config_dword(p->pdev, 0xd4, &val32);
> > + pci_read_config_dword(p->pcidev, 0xd4, &val32);
> > val32 |= no_reboot_bit(p);
> > - pci_write_config_dword(p->pdev, 0xd4, val32);
> > + pci_write_config_dword(p->pcidev, 0xd4, val32);
> > }
> > }
> >
> > @@ -200,11 +199,11 @@ static int iTCO_wdt_unset_NO_REBOOT_bit(struct iTCO_wdt_private *p)
> >
> > val32 = readl(p->gcs_pmc);
> > } else if (p->iTCO_version == 1) {
> > - pci_read_config_dword(p->pdev, 0xd4, &val32);
> > + pci_read_config_dword(p->pcidev, 0xd4, &val32);
> > val32 &= ~enable_bit;
> > - pci_write_config_dword(p->pdev, 0xd4, val32);
> > + pci_write_config_dword(p->pcidev, 0xd4, val32);
> >
> > - pci_read_config_dword(p->pdev, 0xd4, &val32);
> > + pci_read_config_dword(p->pcidev, 0xd4, &val32);
> > }
> >
> > if (val32 & enable_bit)
> > @@ -401,9 +400,10 @@ static const struct watchdog_ops iTCO_wdt_ops = {
> > * Init & exit routines
> > */
> >
> > -static int iTCO_wdt_probe(struct platform_device *dev)
> > +static int iTCO_wdt_probe(struct platform_device *pdev)
> > {
> > - struct itco_wdt_platform_data *pdata = dev_get_platdata(&dev->dev);
> > + struct device *dev = &pdev->dev;
> > + struct itco_wdt_platform_data *pdata = dev_get_platdata(dev);
> > struct iTCO_wdt_private *p;
> > unsigned long val32;
> > int ret;
> > @@ -411,33 +411,32 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> > if (!pdata)
> > return -ENODEV;
> >
> > - p = devm_kzalloc(&dev->dev, sizeof(*p), GFP_KERNEL);
> > + p = devm_kzalloc(dev, sizeof(*p), GFP_KERNEL);
> > if (!p)
> > return -ENOMEM;
> >
> > spin_lock_init(&p->io_lock);
> >
> > - p->tco_res = platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_TCO);
> > + p->tco_res = platform_get_resource(pdev, IORESOURCE_IO, ICH_RES_IO_TCO);
> > if (!p->tco_res)
> > return -ENODEV;
> >
> > - p->smi_res = platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_SMI);
> > + p->smi_res = platform_get_resource(pdev, IORESOURCE_IO, ICH_RES_IO_SMI);
> > if (!p->smi_res)
> > return -ENODEV;
> >
> > p->iTCO_version = pdata->version;
> > - p->dev = dev;
> > - p->pdev = to_pci_dev(dev->dev.parent);
> > + p->pcidev = to_pci_dev(dev->parent);
> >
> > /*
> > * Get the Memory-Mapped GCS or PMC register, we need it for the
> > * NO_REBOOT flag (TCO v2 and v3).
> > */
> > if (p->iTCO_version >= 2) {
> > - p->gcs_pmc_res = platform_get_resource(dev,
> > + p->gcs_pmc_res = platform_get_resource(pdev,
> > IORESOURCE_MEM,
> > ICH_RES_MEM_GCS_PMC);
> > - p->gcs_pmc = devm_ioremap_resource(&dev->dev, p->gcs_pmc_res);
> > + p->gcs_pmc = devm_ioremap_resource(dev, p->gcs_pmc_res);
> > if (IS_ERR(p->gcs_pmc))
> > return PTR_ERR(p->gcs_pmc);
> > }
> > @@ -453,9 +452,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> > iTCO_wdt_set_NO_REBOOT_bit(p);
> >
> > /* The TCO logic uses the TCO_EN bit in the SMI_EN register */
> > - if (!devm_request_region(&dev->dev, p->smi_res->start,
> > + if (!devm_request_region(dev, p->smi_res->start,
> > resource_size(p->smi_res),
> > - dev->name)) {
> > + pdev->name)) {
> > pr_err("I/O address 0x%04llx already in use, device disabled\n",
> > (u64)SMI_EN(p));
> > return -EBUSY;
> > @@ -470,9 +469,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> > outl(val32, SMI_EN(p));
> > }
> >
> > - if (!devm_request_region(&dev->dev, p->tco_res->start,
> > + if (!devm_request_region(dev, p->tco_res->start,
> > resource_size(p->tco_res),
> > - dev->name)) {
> > + pdev->name)) {
> > pr_err("I/O address 0x%04llx already in use, device disabled\n",
> > (u64)TCOBASE(p));
> > return -EBUSY;
> > @@ -505,10 +504,10 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> > p->wddev.bootstatus = 0;
> > p->wddev.timeout = WATCHDOG_TIMEOUT;
> > watchdog_set_nowayout(&p->wddev, nowayout);
> > - p->wddev.parent = &dev->dev;
> > + p->wddev.parent = dev;
> >
> > watchdog_set_drvdata(&p->wddev, p);
> > - platform_set_drvdata(dev, p);
> > + platform_set_drvdata(pdev, p);
> >
> > /* Make sure the watchdog is not running */
> > iTCO_wdt_stop(&p->wddev);
> > @@ -521,7 +520,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> > WATCHDOG_TIMEOUT);
> > }
> >
> > - ret = devm_watchdog_register_device(&dev->dev, &p->wddev);
> > + ret = devm_watchdog_register_device(dev, &p->wddev);
> > if (ret != 0) {
> > pr_err("cannot register watchdog device (err=%d)\n", ret);
> > return ret;
> > @@ -533,9 +532,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
> > return 0;
> > }
> >
> > -static int iTCO_wdt_remove(struct platform_device *dev)
> > +static int iTCO_wdt_remove(struct platform_device *pdev)
> > {
> > - struct iTCO_wdt_private *p = platform_get_drvdata(dev);
> > + struct iTCO_wdt_private *p = platform_get_drvdata(pdev);
> >
> > /* Stop the timer before we leave */
> > if (!nowayout)
> > @@ -544,9 +543,9 @@ static int iTCO_wdt_remove(struct platform_device *dev)
> > return 0;
> > }
> >
> > -static void iTCO_wdt_shutdown(struct platform_device *dev)
> > +static void iTCO_wdt_shutdown(struct platform_device *pdev)
> > {
> > - struct iTCO_wdt_private *p = platform_get_drvdata(dev);
> > + struct iTCO_wdt_private *p = platform_get_drvdata(pdev);
> >
> > iTCO_wdt_stop(&p->wddev);
> > }
> > --
> > 2.7.4
> >
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] watchdog: iTCO_wdt: Use pdev for platform device and pci_dev for pci device
2017-01-03 23:40 ` Guenter Roeck
@ 2017-01-03 23:48 ` Andy Shevchenko
2017-01-03 23:50 ` Andy Shevchenko
0 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2017-01-03 23:48 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel
On Wed, Jan 4, 2017 at 1:40 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Wed, Jan 04, 2017 at 12:39:59AM +0200, Andy Shevchenko wrote:
>> On Tue, Jan 3, 2017 at 4:39 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> > Use pdev for struct platform_device, pcidev for struct pci_dev, and dev
>> > for struct device variables to improve consistency.
>> >
>> > Remove 'struct platform_device *dev;' from struct iTCO_wdt_private since
>> > it was unused.
>>
>> Would pci_dev work?
>>
> Sure, or maybe just 'pci'. Any preference ?
Just slightly prefer pci_dev over others (pcidev, pci), but do not
insist. Up to you.
P.S. Matt is not working for Intel anymore. I would recommend to Cc to
Mika instead.
>
> Thanks,
> Guenter
>
>> In any case
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>
>> >
>> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> > ---
>> > drivers/watchdog/iTCO_wdt.c | 53 ++++++++++++++++++++++-----------------------
>> > 1 file changed, 26 insertions(+), 27 deletions(-)
>> >
>> > diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
>> > index eed1dee6de19..ad29ae03a30b 100644
>> > --- a/drivers/watchdog/iTCO_wdt.c
>> > +++ b/drivers/watchdog/iTCO_wdt.c
>> > @@ -102,9 +102,8 @@ struct iTCO_wdt_private {
>> > unsigned long __iomem *gcs_pmc;
>> > /* the lock for io operations */
>> > spinlock_t io_lock;
>> > - struct platform_device *dev;
>> > /* the PCI-device */
>> > - struct pci_dev *pdev;
>> > + struct pci_dev *pcidev;
>> > /* whether or not the watchdog has been suspended */
>> > bool suspended;
>> > };
>> > @@ -181,9 +180,9 @@ static void iTCO_wdt_set_NO_REBOOT_bit(struct iTCO_wdt_private *p)
>> > val32 |= no_reboot_bit(p);
>> > writel(val32, p->gcs_pmc);
>> > } else if (p->iTCO_version == 1) {
>> > - pci_read_config_dword(p->pdev, 0xd4, &val32);
>> > + pci_read_config_dword(p->pcidev, 0xd4, &val32);
>> > val32 |= no_reboot_bit(p);
>> > - pci_write_config_dword(p->pdev, 0xd4, val32);
>> > + pci_write_config_dword(p->pcidev, 0xd4, val32);
>> > }
>> > }
>> >
>> > @@ -200,11 +199,11 @@ static int iTCO_wdt_unset_NO_REBOOT_bit(struct iTCO_wdt_private *p)
>> >
>> > val32 = readl(p->gcs_pmc);
>> > } else if (p->iTCO_version == 1) {
>> > - pci_read_config_dword(p->pdev, 0xd4, &val32);
>> > + pci_read_config_dword(p->pcidev, 0xd4, &val32);
>> > val32 &= ~enable_bit;
>> > - pci_write_config_dword(p->pdev, 0xd4, val32);
>> > + pci_write_config_dword(p->pcidev, 0xd4, val32);
>> >
>> > - pci_read_config_dword(p->pdev, 0xd4, &val32);
>> > + pci_read_config_dword(p->pcidev, 0xd4, &val32);
>> > }
>> >
>> > if (val32 & enable_bit)
>> > @@ -401,9 +400,10 @@ static const struct watchdog_ops iTCO_wdt_ops = {
>> > * Init & exit routines
>> > */
>> >
>> > -static int iTCO_wdt_probe(struct platform_device *dev)
>> > +static int iTCO_wdt_probe(struct platform_device *pdev)
>> > {
>> > - struct itco_wdt_platform_data *pdata = dev_get_platdata(&dev->dev);
>> > + struct device *dev = &pdev->dev;
>> > + struct itco_wdt_platform_data *pdata = dev_get_platdata(dev);
>> > struct iTCO_wdt_private *p;
>> > unsigned long val32;
>> > int ret;
>> > @@ -411,33 +411,32 @@ static int iTCO_wdt_probe(struct platform_device *dev)
>> > if (!pdata)
>> > return -ENODEV;
>> >
>> > - p = devm_kzalloc(&dev->dev, sizeof(*p), GFP_KERNEL);
>> > + p = devm_kzalloc(dev, sizeof(*p), GFP_KERNEL);
>> > if (!p)
>> > return -ENOMEM;
>> >
>> > spin_lock_init(&p->io_lock);
>> >
>> > - p->tco_res = platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_TCO);
>> > + p->tco_res = platform_get_resource(pdev, IORESOURCE_IO, ICH_RES_IO_TCO);
>> > if (!p->tco_res)
>> > return -ENODEV;
>> >
>> > - p->smi_res = platform_get_resource(dev, IORESOURCE_IO, ICH_RES_IO_SMI);
>> > + p->smi_res = platform_get_resource(pdev, IORESOURCE_IO, ICH_RES_IO_SMI);
>> > if (!p->smi_res)
>> > return -ENODEV;
>> >
>> > p->iTCO_version = pdata->version;
>> > - p->dev = dev;
>> > - p->pdev = to_pci_dev(dev->dev.parent);
>> > + p->pcidev = to_pci_dev(dev->parent);
>> >
>> > /*
>> > * Get the Memory-Mapped GCS or PMC register, we need it for the
>> > * NO_REBOOT flag (TCO v2 and v3).
>> > */
>> > if (p->iTCO_version >= 2) {
>> > - p->gcs_pmc_res = platform_get_resource(dev,
>> > + p->gcs_pmc_res = platform_get_resource(pdev,
>> > IORESOURCE_MEM,
>> > ICH_RES_MEM_GCS_PMC);
>> > - p->gcs_pmc = devm_ioremap_resource(&dev->dev, p->gcs_pmc_res);
>> > + p->gcs_pmc = devm_ioremap_resource(dev, p->gcs_pmc_res);
>> > if (IS_ERR(p->gcs_pmc))
>> > return PTR_ERR(p->gcs_pmc);
>> > }
>> > @@ -453,9 +452,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
>> > iTCO_wdt_set_NO_REBOOT_bit(p);
>> >
>> > /* The TCO logic uses the TCO_EN bit in the SMI_EN register */
>> > - if (!devm_request_region(&dev->dev, p->smi_res->start,
>> > + if (!devm_request_region(dev, p->smi_res->start,
>> > resource_size(p->smi_res),
>> > - dev->name)) {
>> > + pdev->name)) {
>> > pr_err("I/O address 0x%04llx already in use, device disabled\n",
>> > (u64)SMI_EN(p));
>> > return -EBUSY;
>> > @@ -470,9 +469,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
>> > outl(val32, SMI_EN(p));
>> > }
>> >
>> > - if (!devm_request_region(&dev->dev, p->tco_res->start,
>> > + if (!devm_request_region(dev, p->tco_res->start,
>> > resource_size(p->tco_res),
>> > - dev->name)) {
>> > + pdev->name)) {
>> > pr_err("I/O address 0x%04llx already in use, device disabled\n",
>> > (u64)TCOBASE(p));
>> > return -EBUSY;
>> > @@ -505,10 +504,10 @@ static int iTCO_wdt_probe(struct platform_device *dev)
>> > p->wddev.bootstatus = 0;
>> > p->wddev.timeout = WATCHDOG_TIMEOUT;
>> > watchdog_set_nowayout(&p->wddev, nowayout);
>> > - p->wddev.parent = &dev->dev;
>> > + p->wddev.parent = dev;
>> >
>> > watchdog_set_drvdata(&p->wddev, p);
>> > - platform_set_drvdata(dev, p);
>> > + platform_set_drvdata(pdev, p);
>> >
>> > /* Make sure the watchdog is not running */
>> > iTCO_wdt_stop(&p->wddev);
>> > @@ -521,7 +520,7 @@ static int iTCO_wdt_probe(struct platform_device *dev)
>> > WATCHDOG_TIMEOUT);
>> > }
>> >
>> > - ret = devm_watchdog_register_device(&dev->dev, &p->wddev);
>> > + ret = devm_watchdog_register_device(dev, &p->wddev);
>> > if (ret != 0) {
>> > pr_err("cannot register watchdog device (err=%d)\n", ret);
>> > return ret;
>> > @@ -533,9 +532,9 @@ static int iTCO_wdt_probe(struct platform_device *dev)
>> > return 0;
>> > }
>> >
>> > -static int iTCO_wdt_remove(struct platform_device *dev)
>> > +static int iTCO_wdt_remove(struct platform_device *pdev)
>> > {
>> > - struct iTCO_wdt_private *p = platform_get_drvdata(dev);
>> > + struct iTCO_wdt_private *p = platform_get_drvdata(pdev);
>> >
>> > /* Stop the timer before we leave */
>> > if (!nowayout)
>> > @@ -544,9 +543,9 @@ static int iTCO_wdt_remove(struct platform_device *dev)
>> > return 0;
>> > }
>> >
>> > -static void iTCO_wdt_shutdown(struct platform_device *dev)
>> > +static void iTCO_wdt_shutdown(struct platform_device *pdev)
>> > {
>> > - struct iTCO_wdt_private *p = platform_get_drvdata(dev);
>> > + struct iTCO_wdt_private *p = platform_get_drvdata(pdev);
>> >
>> > iTCO_wdt_stop(&p->wddev);
>> > }
>> > --
>> > 2.7.4
>> >
>>
>>
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] watchdog: iTCO_wdt: Use pdev for platform device and pci_dev for pci device
2017-01-03 23:48 ` Andy Shevchenko
@ 2017-01-03 23:50 ` Andy Shevchenko
2017-01-04 1:38 ` Guenter Roeck
0 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2017-01-03 23:50 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel
On Wed, Jan 4, 2017 at 1:48 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Jan 4, 2017 at 1:40 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On Wed, Jan 04, 2017 at 12:39:59AM +0200, Andy Shevchenko wrote:
>>> On Tue, Jan 3, 2017 at 4:39 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>>> > Use pdev for struct platform_device, pcidev for struct pci_dev, and dev
>>> > for struct device variables to improve consistency.
>>> >
>>> > Remove 'struct platform_device *dev;' from struct iTCO_wdt_private since
>>> > it was unused.
>>>
>>> Would pci_dev work?
>>>
>> Sure, or maybe just 'pci'. Any preference ?
>
> Just slightly prefer pci_dev over others (pcidev, pci), but do not
> insist. Up to you.
Or even
struct device *dev;
...
struct pci_dev *pci_dev = to_pci_dev(dev);
in places where it's guaranteed to be PCI.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] watchdog: iTCO_wdt: Use pdev for platform device and pci_dev for pci device
2017-01-03 23:50 ` Andy Shevchenko
@ 2017-01-04 1:38 ` Guenter Roeck
0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2017-01-04 1:38 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel
On 01/03/2017 03:50 PM, Andy Shevchenko wrote:
> On Wed, Jan 4, 2017 at 1:48 AM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Wed, Jan 4, 2017 at 1:40 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>>> On Wed, Jan 04, 2017 at 12:39:59AM +0200, Andy Shevchenko wrote:
>>>> On Tue, Jan 3, 2017 at 4:39 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>>>>> Use pdev for struct platform_device, pcidev for struct pci_dev, and dev
>>>>> for struct device variables to improve consistency.
>>>>>
>>>>> Remove 'struct platform_device *dev;' from struct iTCO_wdt_private since
>>>>> it was unused.
>>>>
>>>> Would pci_dev work?
>>>>
>>> Sure, or maybe just 'pci'. Any preference ?
>>
>> Just slightly prefer pci_dev over others (pcidev, pci), but do not
>> insist. Up to you.
>
> Or even
>
> struct device *dev;
> ...
> struct pci_dev *pci_dev = to_pci_dev(dev);
>
'dev' would have to be a pointer to the parent device. If I consider using
dev_{info,err}, I'll need 'dev' in struct iTCO_wdt_private. I think I'll
stick with pci_dev.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] watchdog: iTCO_wdt: Use device managed resources
2017-01-03 22:41 ` Andy Shevchenko
2017-01-03 23:38 ` Guenter Roeck
@ 2017-01-04 2:44 ` Guenter Roeck
1 sibling, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2017-01-04 2:44 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel, Matt Fleming
On 01/03/2017 02:41 PM, Andy Shevchenko wrote:
> On Tue, Jan 3, 2017 at 4:39 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> Using device managed resources simplifies error handling and cleanup,
>> and to reduce the likelyhood of errors.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Does it make sense to convert to dev_err() at some point?
>
I tried. It gives me
[59886.759774] iTCO_vendor_support: vendor-support=0
[59886.760452] iTCO_wdt: Intel TCO WatchDog Timer Driver v1.11
[59886.760540] iTCO_wdt iTCO_wdt.0.auto: Found a 9 Series TCO device (Version=2, TCOBASE=0x1860)
[59886.761849] iTCO_wdt iTCO_wdt.0.auto: initialized. heartbeat=30 sec (nowayout=0)
which I don't really like too much. Let's stick with pr_{info,err}.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-01-04 2:47 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-03 14:39 [PATCH 1/4] watchdog: iTCO_wdt: Use allocated data structures Guenter Roeck
2017-01-03 14:39 ` [PATCH 2/4] watchdog: iTCO_wdt: Use device managed resources Guenter Roeck
2017-01-03 22:41 ` Andy Shevchenko
2017-01-03 23:38 ` Guenter Roeck
2017-01-04 2:44 ` Guenter Roeck
2017-01-03 14:39 ` [PATCH 3/4] watchdog: iTCO_wdt: Use pdev for platform device and pci_dev for pci device Guenter Roeck
2017-01-03 22:39 ` Andy Shevchenko
2017-01-03 23:40 ` Guenter Roeck
2017-01-03 23:48 ` Andy Shevchenko
2017-01-03 23:50 ` Andy Shevchenko
2017-01-04 1:38 ` Guenter Roeck
2017-01-03 14:39 ` [PATCH 4/4] watchdog: iTCO_wdt: Simplify module init function Guenter Roeck
2017-01-03 22:38 ` Andy Shevchenko
2017-01-03 22:44 ` [PATCH 1/4] watchdog: iTCO_wdt: Use allocated data structures Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).