Ulrik Bech Hald <ubh@xxxxxx> writes:
> This patch enables the IVA and SECURE WDTs in the omap_wdt
> driver for omap34xx family. SECURE will be available as /dev/watchdog1
> HS/EMU devices and IVA will be available as /dev/watchdog3.
> MPU will be available as /dev/watchdog2
> For omap versions earlier than 34xx only MPU watchdog is present and
> will be available as /dev/watchdog for backwards compatibility.
>
> Signed-off-by: Ulrik Bech Hald <ubh@xxxxxx>
> ---
> drivers/watchdog/omap_wdt.c | 42 +++++++++++++++++++++++++++++++++++-------
> 1 files changed, 35 insertions(+), 7 deletions(-)
> mode change 100644 => 100755 drivers/watchdog/omap_wdt.c
>
> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
[...]
> static int omap_wdt_open(struct inode *inode, struct file *file)
> {
> - struct omap_wdt_dev *wdev = platform_get_drvdata(omap_wdt_dev);
> - void __iomem *base = wdev->base;
> + struct omap_wdt_dev *wdev;
> + void __iomem *base;
> +
> + /* by default MPU wdt is present across omap family */
> + wdev = platform_get_drvdata(omap_wdt_dev[1]);
> +
> + /* if multiple wdts, find match between device node and wdt device */
> + if (cpu_is_omap34xx()) {
> + int i;
> + for (i = 0; i < NUM_WDTS; i++) {
> + if (omap_wdt_dev[i]) {
> + wdev = platform_get_drvdata(omap_wdt_dev[i]);
> + if (iminor(inode)
> + == wdev->omap_wdt_miscdev.minor)
> + break;
> + }
> + }
> + }
> +
> + base = wdev->base;
Hmm, this does not look right to me. Any use of cpu_is_* in driver
code usually indicates that something is not quite right. This same
watchdog IP is used in some DaVinci family processors and needs to be
reused there.
I didn't do the miscdev research, but can't you get the pdev from the
miscdev.parent, and from there get the right wdev pointer from the
pdev.drvdata.
Otherwise, drop the cpu_is_* and just do inode checking all the time.
The way you've done it looks error prone to me.
> if (test_and_set_bit(1, (unsigned long *)&(wdev->omap_wdt_users)))
> return -EBUSY;
> @@ -263,6 +283,7 @@ static int __devinit omap_wdt_probe(struct platform_device *pdev)
> struct resource *res, *mem;
> struct omap_wdt_dev *wdev;
> int ret;
> + static char wdt_name[32];
>
> /* reserve static register mappings */
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -271,7 +292,7 @@ static int __devinit omap_wdt_probe(struct platform_device *pdev)
> goto err_get_resource;
> }
>
> - if (omap_wdt_dev) {
> + if (omap_wdt_dev[pdev->id-1]) {
> ret = -EBUSY;
> goto err_busy;
> }
> @@ -318,8 +339,14 @@ static int __devinit omap_wdt_probe(struct platform_device *pdev)
>
> wdev->omap_wdt_miscdev.parent = &pdev->dev;
> wdev->omap_wdt_miscdev.minor = WATCHDOG_MINOR;
> - wdev->omap_wdt_miscdev.name = "watchdog";
> wdev->omap_wdt_miscdev.fops = &omap_wdt_fops;
> + wdev->omap_wdt_miscdev.name = "watchdog";
> + if (cpu_is_omap34xx()) {
> + snprintf(wdt_name, sizeof(wdt_name), "watchdog%d", pdev->id);
> + wdev->omap_wdt_miscdev.name = wdt_name;
> + if (pdev->id != 2)
> + wdev->omap_wdt_miscdev.minor = MISC_DYNAMIC_MINOR;
> + }
Again, red flag: cpu_is_*
It might be more consistent to use pdev->dev->name which is already of
the form "watchdog.%d".
Any reason not to use MISC_DYNAMIC_MINOR for all of them?
Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
