Thanks Guenter On Fri, Feb 23, 2024, at 3:21 PM, Guenter Roeck wrote: > On 2/23/24 11:58, Mark Pearson wrote: >> On Fri, Jul 21, 2023, at 8:29 AM, David Ober wrote: >>> This modules is to allow for the new ITE 5632 EC chip >>> to support the watchdog for initial use in the Lenovo SE10 >>> >>> Signed-off-by: David Ober <dober6023@xxxxxxxxx> >>> >>> V2 Fix stop to deactivate wdog on unload of module >>> V2 Remove extra logging that is not needed >>> V2 change udelays to usleep_range >>> V2 Changed to now request region on start and release on stop instead >>> of for every ping and read/write >>> V3 add counter to while loops so it will not hang >>> V3 rework code to use platform_device_register_simple >>> V3 rework getting the Chip ID to remove duplicate code and close IO >>> V3 change some extra logging to be debug only >>> --- > [ ... ] >>> +config ITE5632_WDT >>> + tristate "ITE 5632" >>> + select WATCHDOG_CORE >>> + help >>> + If you say yes here you get support for the watchdog >>> + functionality of the ITE 5632 eSIO chip. >>> + >>> + This driver can also be built as a module. If so, the module >>> + will be called ite5632_wdt. >>> + > > [ ... ] > >> >> >> Please let us know if there is anything else needed to get this accepted. Happy to address any feedback. >> > > I am sure I commented on this before. The fact that the Lenovo SE10 uses an > ITE 5632 controller is completely irrelevant. Lenovo could decide tomorrow to > replace the ITE chip with a Nuvoton chip, use the same API to talk with it, > and the watchdog would work perfectly fine. > > This is a driver for the watchdog implemented in the embedded controller > on Lenovo SE10. It is not a watchdog driver for ITE5632. Again, the EC chip > used in that Lenovo system is completely irrelevant, even more so since > this seems to be one of those undocumented ITE chips which officially > don't even exist. Claiming that this would be a watchdog driver for ITE5632 > would be not only misleading but simply wrong. > > It seems that we can not agree on this. That means that, from my perspective, > there is no real path to move forward. Wim will have to decide if and how > to proceed. > My apologies - I hadn't realised that was the issue (my fault for missing it). Appreciate the clarification. Is this as simple as renaming this driver as (for example) a lenovo_se_wdt device, and adding in the appropriate checking during the init that it is only used on Lenovo SE10 platforms? I don't understand the concern if a different chip was used - wouldn't that need a different driver at that point? If it's more subtle than that, is there something you propose instead? Thanks Mark
