Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933485AbcJFStj (ORCPT ); Thu, 6 Oct 2016 14:49:39 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:32812 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751137AbcJFSta (ORCPT ); Thu, 6 Oct 2016 14:49:30 -0400 Date: Thu, 6 Oct 2016 11:49:27 -0700 From: Guenter Roeck To: Steve Twiss Cc: Wim Van Sebroeck , Dmitry Torokhov , Eduardo Valentin , Zhang Rui , DEVICETREE , LINUX-INPUT , LINUX-PM , Lee Jones , Liam Girdwood , Mark Brown , Mark Rutland , Rob Herring , Support Opensource , LINUX-KERNEL , LINUX-WATCHDOG Subject: Re: [PATCH V1 04/10] watchdog: da9061: watchdog driver (RFC) Message-ID: <20161006184927.GB11915@roeck-us.net> References: <6ED8E3B22081A4459DAC7699F3695FB7018CCE242C@SW-EX-MBX02.diasemi.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6ED8E3B22081A4459DAC7699F3695FB7018CCE242C@SW-EX-MBX02.diasemi.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Authenticated_sender: guenter@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: guenter@roeck-us.net X-Authenticated-Sender: bh-25.webhostbox.net: guenter@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2696 Lines: 64 Hi Steve, On Thu, Oct 06, 2016 at 04:28:14PM +0000, Steve Twiss wrote: > Hi Guenter, > > Thanks for your reply. > > On 06 October 2016 14:28, Guenter Roeck, wrote: > > > To: Steve Twiss; LINUX-KERNEL; LINUX-WATCHDOG; Wim Van Sebroeck > > Subject: Re: [PATCH V1 04/10] watchdog: da9061: watchdog driver > > [...] > > > > @@ -180,6 +180,11 @@ static const struct watchdog_info da9062_watchdog_info = { > > > .identity = "DA9062 WDT", > > > }; > > > > > > +static const struct watchdog_info da9061_watchdog_info = { > > > + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING, > > > + .identity = "DA9061 WDT", > > > > This adds a lot of complexity to the driver just to be able to display "DA9061". > > Why not just change the existing identity to "DA9061/DA9062 WDT" ? > > This is true. > I am using the compatible string to pick a different configuration .data block: > > { .compatible = "dlg,da9062-watchdog", .data = &da9062_watchdog_info }, > { .compatible = "dlg,da9061-watchdog", .data = &da9061_watchdog_info }, > > when the only real difference between the DA9061 and DA9062 watchdog driver > is the name. Functionally they are identical in this case. > This was a similar comment in Dialog's internal review on this exact same point. > "Why not just report one thing?" > > My answer to that was because it would allow to distinguish between different chips. > The watchdog driver would report the correct chip type, despite the driver always > being for DA9062. > > This exact same thing would happen with da9063-onkey and da9062-thermal also. > For the ONKEY it is marginally confused by needing to support 63, but for 62 and 61 > it is the same thing. Only the name is different. > > I have TO:'d Dmitry Torokhov; Eduardo Valentin; Zhang Rui, for that reason. > > But, it is just my opinion to keep the "name" different. > This will not be my decision if accepted into the Linux kernel, but I would like to > at least be consistent for DA9061 and DA9062 so ... is this an issue? > Yes, for me it is. The driver is still the same, and I don't see the point of increasing code size and making the driver less readable just to be able to report a slightly different driver identification string. And each time a similar HW is added we would go through the same effort, again for no good reason. FWIW the driver doesn't really need to be updated in the first place. A compatible statement listing both da9061 and da9062 would do it. Plus, on top of that, even your change would not guarantee that the output is correct. A DT entry with lists da9061 compatibility on a da9062 system would report the "wrong" name. And still work. So I really don't see the point. Guenter