* [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* 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 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 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
* [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* 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 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
* [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 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
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.