linux-kernel - Re: [PATCH V1 04/10] watchdog: da9061: watchdog driver (RFC)

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-next>] [day] [month] [year] [list]

Message-ID: <6ED8E3B22081A4459DAC7699F3695FB7018CCE242C@SW-EX-MBX02.diasemi.com>
Date:   Thu, 6 Oct 2016 16:28:14 +0000
From:   Steve Twiss <stwiss.opensource@...semi.com>
To:     Guenter Roeck <linux@...ck-us.net>,
        Wim Van Sebroeck <wim@...ana.be>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Eduardo Valentin <edubezval@...il.com>,
        Zhang Rui <rui.zhang@...el.com>
CC:     DEVICETREE <devicetree@...r.kernel.org>,
        LINUX-INPUT <linux-input@...r.kernel.org>,
        LINUX-PM <linux-pm@...r.kernel.org>,
        Lee Jones <lee.jones@...aro.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Rob Herring <robh+dt@...nel.org>,
        Support Opensource <Support.Opensource@...semi.com>,
        Zhang Rui <rui.zhang@...el.com>,
        LINUX-KERNEL <linux-kernel@...r.kernel.org>,
        LINUX-WATCHDOG <linux-watchdog@...r.kernel.org>
Subject: Re: [PATCH V1 04/10] watchdog: da9061: watchdog driver (RFC)

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?

The same question applies to thermal and onkey as well.

Regards,
Steve