On Mon, Apr 21, 2014 at 01:05:08PM -0700, Guenter Roeck wrote: > On Mon, Apr 21, 2014 at 12:51:27PM -0700, David Cohen wrote: > > On Mon, Apr 21, 2014 at 11:16:53AM -0700, Guenter Roeck wrote: > > > Hi, > > > > > > with the recent submission of an Intel MID watchdog driver, we now have > > > > > > config INTEL_SCU_WATCHDOG > > > bool "Intel SCU Watchdog for Mobile Platforms" > > > depends on X86_INTEL_MID > > > ---help--- > > > Hardware driver for the watchdog time built into the Intel SCU > > > for Intel Mobile Platforms. > > > > > > > > > config INTEL_MID_WATCHDOG > > > tristate "Intel MID Watchdog Timer" > > > depends on X86_INTEL_MID > > > --help-- > > > Watchdog timer driver built into the Intel SCU for Intel MID Platforms. > > > > > > This driver currently supports only the watchdog evolution > > > implementation in SCU, available for Merrifield generation. > > > > > > Furthermore, in the SCU watchdog driver initialization code, we have > > > > > > ... > > > * If it isn't an intel MID device then it doesn't have this watchdog > > > */ > > > if (!intel_mid_identify_cpu()) > > > return -ENODEV; > > > > > > I must admit I find this very confusing. Both watchdogs have the same > > > dependencies. The MID watchdog driver only instantiates for Tangier, > > > while the SCU watchdog driver instantiates for all MID CPUs (specfically > > > including Tangier, but also Penwell and Cloverview and, from earlier > > > commit logs, Moorsetown). > > > > > > I understand I was told earlier that the supported devices would be > > > "sufficiently different" to warrant separate drivers, but this doesn't > > > really make sense to me, at least not without further explanation. > > > > > > Can someone please clarify which driver is supposed to work on which device, > > > and why there are now two watchdog drivers for Tangier (or at least two > > > watchdog drivers which instantiate on Tangier) ? Does the SCU driver work > > > on Tangier, or does it not ? If not, why is there no code in the SCU driver > > > which would cause it to fail to instantiate on Tangier, and why is this > > > limitation not mentioned in Kconfig ? If yes, why do we need separate drivers > > > in the first place ? > > > > Let me try to give extra info: > > > > INTEL_SCU_WATCHDOG supports Penwell and Cloverview. But it is using an > > obsolete interface and would need too much if's to support Tangier (and > > newer ones). And it is mostly unsupported nowadays. > > > > Internally we had a new watchdog for Tangier (and newer platforms) > > called INTEL_SCU_WATCHDOG_EVO. Somebody else tried to upstream it but > > got nacked due to obsolete interface too. I started to rewrite this > > watchdog to comply with community comments [1]. > > > > While rewriting the _EVO one, I felt it would be better to not have 2 > > wdt drivers for Intel MID, but only one. This new INTEL_MID_WATCHDOG > > supports only Tangier now but I am trying to make it generic enough to > > bring the legacy platforms too. Then we can get rid of INTEL_SCU_WATCHDOG. > > > > Does that sound feasible? Maybe I need to write comments to > > INTEL_SCU_WATCHDOG's Kconfig entry about its limitation. > > > Yes, I think that would make sense. I also think the SCU watchdog > driver should fail to load if it is instantiated on Tangier, > if that is not supported. Or, rather, it should only instantiate on > Penwell and Cloverview if those are the only supported platforms > (but what about Moorestown ?). > > Can you submit a patch to do that ? I am about to submit a patch a bit different. I'm extending the arch/x86/platform/intel-mid/device_libs/platform_wdt.c code to penwell and cloverview. INTEL_SCU_WATCHDOG will no longer probe itself, but register a platform driver instead. Then the platform code will register the correct device (SCU or MID). Since watchdog is not part of SFI tables, this is the option I found to avoid the self-probe approach from Intel MID wdt drivers. BR, David > > Thanks, > Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
