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]
Message-ID: <1364402179.17807.4.camel@hobbes.kokotovo>
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ