linux-kernel - Re: [PATCH 1/5] iTCO_wdt: Expose watchdog properties using platform data

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: <20150729110959.GH2773@codeblueprint.co.uk>
Date:	Wed, 29 Jul 2015 12:09:59 +0100
From:	Matt Fleming <matt@...eblueprint.co.uk>
To:	Jean Delvare <jdelvare@...e.de>
Cc:	Wim Van Sebroeck <wim@...ana.be>, linux-kernel@...r.kernel.org,
	linux-watchdog@...r.kernel.org,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Wolfram Sang <wsa@...-dreams.de>,
	Matt Fleming <matt.fleming@...el.com>,
	Peter Tyser <ptyser@...-inc.com>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	Lee Jones <lee.jones@...aro.org>
Subject: Re: [PATCH 1/5] iTCO_wdt: Expose watchdog properties using platform
 data

On Wed, 29 Jul, at 09:29:34AM, Jean Delvare wrote:
> Hi Matt,
> 
> On Mon, 27 Jul 2015 14:38:08 +0100, Matt Fleming wrote:
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index 241fafde42cb..5336fe2ff689 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -797,7 +797,7 @@ config ITCO_WDT
> >  	tristate "Intel TCO Timer/Watchdog"
> >  	depends on (X86 || IA64) && PCI
> >  	select WATCHDOG_CORE
> > -	select LPC_ICH
> > +	depends on LPC_ICH || I2C_I801
> >  	---help---
> >  	  Hardware driver for the intel TCO timer based watchdog devices.
> >  	  These drivers are included in the Intel 82801 I/O Controller
> 
> I don't like that. Depending on the order of the subsystems in the
> Kconfig tree, the user may not see the option and has no clue that the
> option exists. When he/she later enables LPC_ICH or I2C_I801, the
> option becomes visible but he/she has no reason to return here to
> enable it.
 
Good point!

> I can think of several ways to address the issue. One of them is to
> select both drivers, as this is what most users will want anyway:
> 
> 	select LPC_ICH if !EXPERT
> 	select I2C_I801 if !EXPERT
> 
> Another possibility is to make config ITCO_WDT always visible, and add
> sub-options ITCO_WDT_LPC and ITCO_WDT_I2C which depend on it. Selecting
> either would select the driver it depends on:
> 
> config ITCO_WDT_LPC
> 	bool "Intel TCO Timer/Watchdog on LPC"
> 	depends on ITCO_WDT
> 	select LPC_ICH
> 
> config ITCO_WDT_I2C
> 	bool "Intel TCO Timer/Watchdog on I2C"
> 	depends on ITCO_WDT
> 	select I2C_I801
> 
> I did not test that, some care might be needed due to tristate vs.
> boolean.
> 
> I personally prefer the first approach. It may not be as clean as the
> second approach but it should be good enough in practice and avoids
> cluttering Kconfig with even more options.

Yeah, I'm with you on that. The first approach seems superior to me
because it doesn't require users to know which bus the TCO watchdog
device lives on for their platforms.

-- 
Matt Fleming, Intel Open Source Technology Center
--
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/