On 02/02/2012 06:48 PM, Wolfram Sang wrote:
> The resource handling in this driver was flaky: IO_ADDRESS instead of
> ioremap (and no unmapping), an unneeded static resource, no central exit
> path for error cases. Fix this by converting the driver to use managed
> resources. Also use dev_*-messages instead of printk while we are here.
>
> Signed-off-by: Wolfram Sang <w.sang@xxxxxxxxxxxxxx>
> ---
> drivers/watchdog/pnx4008_wdt.c | 74 ++++++++++++++-------------------------
> 1 files changed, 27 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/watchdog/pnx4008_wdt.c b/drivers/watchdog/pnx4008_wdt.c
> index 8e210aa..322a392 100644
> --- a/drivers/watchdog/pnx4008_wdt.c
> +++ b/drivers/watchdog/pnx4008_wdt.c
> @@ -89,7 +89,6 @@ static unsigned long wdt_status;
>
> static unsigned long boot_status;
>
> -static struct resource *wdt_mem;
> static void __iomem *wdt_base;
> struct clk *wdt_clk;
>
> @@ -253,61 +252,46 @@ static struct miscdevice pnx4008_wdt_miscdev = {
>
> static int __devinit pnx4008_wdt_probe(struct platform_device *pdev)
> {
> - int ret = 0, size;
> + struct resource *r;
> + int ret = 0;
>
> if (heartbeat < 1 || heartbeat > MAX_HEARTBEAT)
> heartbeat = DEFAULT_HEARTBEAT;
>
> - printk(KERN_INFO MODULE_NAME
> - "PNX4008 Watchdog Timer: heartbeat %d sec\n", heartbeat);
> -
> - wdt_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (wdt_mem == NULL) {
> - printk(KERN_INFO MODULE_NAME
> - "failed to get memory region resouce\n");
> - return -ENOENT;
> - }
> -
> - size = resource_size(wdt_mem);
> -
> - if (!request_mem_region(wdt_mem->start, size, pdev->name)) {
> - printk(KERN_INFO MODULE_NAME "failed to get memory region\n");
> - return -ENOENT;
> - }
> - wdt_base = (void __iomem *)IO_ADDRESS(wdt_mem->start);
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + wdt_base = devm_request_and_ioremap(&pdev->dev, r);
> + if (!wdt_base)
> + return -EADDRINUSE;
>
> wdt_clk = clk_get(&pdev->dev, NULL);
> - if (IS_ERR(wdt_clk)) {
> - ret = PTR_ERR(wdt_clk);
> - release_mem_region(wdt_mem->start, size);
> - wdt_mem = NULL;
> - goto out;
> - }
> + if (IS_ERR(wdt_clk))
> + return PTR_ERR(wdt_clk);
>
> ret = clk_enable(wdt_clk);
> - if (ret) {
> - release_mem_region(wdt_mem->start, size);
> - wdt_mem = NULL;
> - clk_put(wdt_clk);
> - goto out;
> - }
> + if (ret)
> + goto put_clk;
>
> ret = misc_register(&pnx4008_wdt_miscdev);
> if (ret < 0) {
> - printk(KERN_ERR MODULE_NAME "cannot register misc device\n");
> - release_mem_region(wdt_mem->start, size);
> - wdt_mem = NULL;
> - clk_disable(wdt_clk);
> - clk_put(wdt_clk);
> - } else {
> - boot_status = (__raw_readl(WDTIM_RES(wdt_base)) & WDOG_RESET) ?
> - WDIOF_CARDRESET : 0;
> - wdt_disable(); /*disable for now */
> - clk_disable(wdt_clk);
> - set_bit(WDT_DEVICE_INITED, &wdt_status);
> + dev_err(&pdev->dev, "cannot register misc device\n");
> + goto disable_clk;
> }
>
> -out:
> + boot_status = (__raw_readl(WDTIM_RES(wdt_base)) &
> + WDOG_RESET) ? WDIOF_CARDRESET : 0;
> + wdt_disable(); /*disable for now */
> + clk_disable(wdt_clk);
> + set_bit(WDT_DEVICE_INITED, &wdt_status);
> +
> + dev_info(&pdev->dev, "PNX4008 Watchdog Timer: heartbeat %d sec\n",
> + heartbeat);
> +
> + return 0;
> +
> + disable_clk:
> + clk_disable(wdt_clk);
> + put_clk:
> + clk_put(wdt_clk);
> return ret;
> }
>
> @@ -318,10 +302,6 @@ static int __devexit pnx4008_wdt_remove(struct platform_device *pdev)
> clk_disable(wdt_clk);
> clk_put(wdt_clk);
>
> - if (wdt_mem) {
> - release_mem_region(wdt_mem->start, resource_size(wdt_mem));
> - wdt_mem = NULL;
> - }
> return 0;
> }
>
Tested-by: Roland Stigge <stigge@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
