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:	Tue, 26 Mar 2013 21:33:30 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Lubomir Rintel <lkundrak@...sk>
CC:	linux-kernel@...r.kernel.org, Wim Van Sebroeck <wim@...ana.be>,
	linux-rpi-kernel@...ts.infradead.org,
	linux-watchdog@...r.kernel.org
Subject: Re: [PATCH] watchdog: Add Broadcom BCM2708 watchdog timer driver

On 03/24/2013 08:12 AM, Lubomir Rintel wrote:
> On Fri, 2013-03-22 at 20:24 -0600, Stephen Warren wrote:
> 
> Thank you for your response!
> 
>> On 03/22/2013 06:55 AM, Lubomir Rintel wrote:
>>> Signed-off-by: Lubomir Rintel <lkundrak at v3.sk>

>> A couple of general comments:
>>
>> 1)
>>
>> This driver touches the same registers that
>> arch/arm/mach-bcm2835/bcm2835.c uses to implement reboot and "power
>> off". Some co-ordination might be necessary.
>>
>> The implementation of bcm2835_power_off() could easily be moved into
>> this driver, to avoid some of the need for co-ordination.
>>
>> Moving bcm2835_restart() would be more tricky, since the ARM machine
>> descriptor needs a pointer to that function. I guess the kernel probably
>> ensures that none of the code in this watchdog driver is running by the
>> time bcm2835_restart() is called, although perhaps it'd be better to
>> have mach-bcm2835/bcm2835.c and this driver share a lock?
> 
> I need help here, I'm not sure what's the proper way to address this
> (whether to include the actual reboot code in the wdt driver or the
> platform driver).

I assume by "platform driver" you mean the code in
arch/arm/mach-bcm2835? The phrase "platform driver" usually refers to a
struct platform_driver, so that usage is a little unusual. I think you
would usually say "arch code" to refer to mach-bcm2835/, or something
like that!

> Is it okay to have the platform driver depend on watchdog timer?
> Is it okay for the platform driver not to reboot properly if the kernel
> is running without the wdt driver loaded?
> 
> (For now, I'll send a revised patch addressing the other issues so that
> it can be reviewed without addressing this yet.)

I guess what we should do here is merge the driver as you've posted it,
then later we can migrate any code from arch/arm/mach-bcm2835 into the
WDT driver.

IIRC, there certainly are some existing WDT drivers that implement the
reboot hook for their platforms, so it's probably OK to migrate that way
sometime, although indeed the issues you raise do deserve some thought.
--
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