setting miscdevice.parent after misc_register()
in [Kernel]
|
Prev: [PATCH] KVM: VMX: fix rcu usage warning
Next: [PATCH v4 2/3] dm: export a table+mapped device to the ioctl interface
From: Akinobu Mita on 7 Jun 2010 23:20 In drivers/watchdog/at32ap700x_wdt.c, and drivers/watchdog/sch311x_wdt.c miscdevice.parent is set after misc_register(). drivers/watchdog/at32ap700x_wdt.c: ret = misc_register(&wdt->miscdev); ... wdt->miscdev.parent = &pdev->dev; But it does nothing. (the miscdevice will not be a child of that parent device in sysfs) So should it be set before misc_register() or just keep it as-is? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Hans-Christian Egtvedt on 8 Jun 2010 03:10 On Tue, 8 Jun 2010 12:11:44 +0900 Akinobu Mita <akinobu.mita(a)gmail.com> wrote: > In drivers/watchdog/at32ap700x_wdt.c, and drivers/watchdog/sch311x_wdt.c > miscdevice.parent is set after misc_register(). > > drivers/watchdog/at32ap700x_wdt.c: > > ret = misc_register(&wdt->miscdev); > ... > wdt->miscdev.parent = &pdev->dev; > > But it does nothing. (the miscdevice will not be a child of that parent > device in sysfs) > > So should it be set before misc_register() or just keep it as-is? AFAICT I think it is unsafe to have misc_register() before the last parts of the probe function. The misc_register() should be the last call in _probe(). Something ala the following... From 8439126716c3cf887c6453c9543cca5b58ac9c81 Mon Sep 17 00:00:00 2001 From: Hans-Christian Egtvedt <hans-christian.egtvedt(a)atmel.com> Date: Tue, 8 Jun 2010 08:44:32 +0200 Subject: [PATCH] at32ap700x_wdt: register misc device last in probe() function This patch reworks the probe() function in the at32ap700x_wdt driver, this to make sure the miscdev is properly initialized and the driver is ready to be accessed. Signed-off-by: Hans-Christian Egtvedt <hans-christian.egtvedt(a)atmel.com> --- drivers/watchdog/at32ap700x_wdt.c | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/watchdog/at32ap700x_wdt.c b/drivers/watchdog/at32ap700x_wdt.c index 1cddf92..750bc52 100644 --- a/drivers/watchdog/at32ap700x_wdt.c +++ b/drivers/watchdog/at32ap700x_wdt.c @@ -346,9 +346,13 @@ static int __init at32_wdt_probe(struct platform_device *pdev) } else { wdt->users = 0; } - wdt->miscdev.minor = WATCHDOG_MINOR; - wdt->miscdev.name = "watchdog"; - wdt->miscdev.fops = &at32_wdt_fops; + + wdt->miscdev.minor = WATCHDOG_MINOR; + wdt->miscdev.name = "watchdog"; + wdt->miscdev.fops = &at32_wdt_fops; + wdt->miscdev.parent = &pdev->dev; + + platform_set_drvdata(pdev, wdt); if (at32_wdt_settimeout(timeout)) { at32_wdt_settimeout(TIMEOUT_DEFAULT); @@ -360,17 +364,17 @@ static int __init at32_wdt_probe(struct platform_device *pdev) ret = misc_register(&wdt->miscdev); if (ret) { dev_dbg(&pdev->dev, "failed to register wdt miscdev\n"); - goto err_iounmap; + goto err_register; } - platform_set_drvdata(pdev, wdt); - wdt->miscdev.parent = &pdev->dev; dev_info(&pdev->dev, "AT32AP700X WDT at 0x%p, timeout %d sec (nowayout=%d)\n", wdt->regs, wdt->timeout, nowayout); return 0; +err_register: + platform_set_drvdata(pdev, NULL); err_iounmap: iounmap(wdt->regs); err_free: -- 1.6.3.3 -- Best regards, Hans-Christian Egtvedt -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Wim Van Sebroeck on 8 Jun 2010 15:40
Hi Akinobu, > In drivers/watchdog/at32ap700x_wdt.c, and drivers/watchdog/sch311x_wdt.c > miscdevice.parent is set after misc_register(). > > drivers/watchdog/at32ap700x_wdt.c: > > ret = misc_register(&wdt->miscdev); > ... > wdt->miscdev.parent = &pdev->dev; > > But it does nothing. (the miscdevice will not be a child of that parent > device in sysfs) > > So should it be set before misc_register() or just keep it as-is? It should be fixed by setting it before the misc_register call. I'll do the necessary. Thanks, Wim. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |