linux-kernel - Re: [PATCH v3] watchdog: Add Broadcom BCM2835 watchdog timer driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 27 Mar 2013 17:36:19 +0100
From:	Lubomir Rintel <lkundrak@...sk>
To:	Stephen Warren <swarren@...dotorg.org>
Cc:	linux-kernel@...r.kernel.org, Wim Van Sebroeck <wim@...ana.be>,
	Guenter Roeck <linux@...ck-us.net>,
	linux-rpi-kernel@...ts.infradead.org,
	linux-watchdog@...r.kernel.org
Subject: Re: [PATCH v3] watchdog: Add Broadcom BCM2835 watchdog timer driver

On Tue, 2013-03-26 at 21:40 -0600, Stephen Warren wrote:
> On 03/26/2013 11:50 AM, Lubomir Rintel wrote:
> > This adds a driver for watchdog timer hardware present on Broadcom BCM2835 SoC,
> > used in Raspberry Pi and Roku 2 devices.
> 
> Since this patch defines a new DT binding, you should send it to
> devicetree-discuss@...ts.ozlabs.org too.

Okay.

...
> > + * Interface to the Broadcom BCM2835 watchdog timer hardware is based on
> > + * "bcm2708_wdog" driver written by Luke Diamand that was obtained from branch
> > + * "rpi-3.6.y" of git://github.com/raspberrypi/linux.git
> 
> I see that the patch isn't S-o-b Luke in the downstream kernel. However,
> it is S-o-b Dom Cobley (popcornmix), and they both work for Broadcom, so
> I think that's OK.

Okay, I'll add it.

...
> > +	platform_set_drvdata(pdev, wdt);
> > +	watchdog_set_drvdata(&bcm2835_wdt_wdd, wdt);
> 
> Do you really need both of those? I would have thought just one would
> have been enough.

I think so; I would like to get rid of the platform_*() one, but we need
that in remove hook. All of the existing drivers in drivers/watchdog/
tree either use global data to represent a single device or use two of
the *_set_drvdata() functions -- the watchdog_*() one and dev_*(),
platform_*(), amba_*() or the like.

> I'd be tempted to put the platform_set_drvdata() call right after the
> devm_kzalloc() of wdt, but it's not a big deal either way.

Done.

I'm actually rather thankful for style fixes such as this, since I'm
only getting familiar with what's customary to do there.

Updated patch will follow shortly.

-- 
Lubomir Rintel <lkundrak@...sk>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/