linux-kernel - Re: [RESEND PATCH v7 1/1] platform: Add driver for RAVE Supervisory Processor

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 24 Oct 2017 11:34:19 -0700
From:   Andrey Smirnov <andrew.smirnov@...il.com>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     Johan Hovold <johan@...nel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Chris Healy <cphealy@...il.com>,
        Lucas Stach <l.stach@...gutronix.de>,
        Nikita Yushchenko <nikita.yoush@...entembedded.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Pavel Machek <pavel@....cz>
Subject: Re: [RESEND PATCH v7 1/1] platform: Add driver for RAVE Supervisory Processor

On Tue, Oct 24, 2017 at 3:28 AM, Lee Jones <lee.jones@...aro.org> wrote:
> On Fri, 13 Oct 2017, Andrey Smirnov wrote:
>
>> On Fri, Oct 13, 2017 at 9:17 AM, Lee Jones <lee.jones@...aro.org> wrote:
>> > On Fri, 13 Oct 2017, Andrey Smirnov wrote:
>> >
>> >> On Fri, Oct 13, 2017 at 12:27 AM, Johan Hovold <johan@...nel.org> wrote:
>> >> > On Thu, Oct 12, 2017 at 11:13:21PM -0700, Andrey Smirnov wrote:
>> >> >> Add a driver for RAVE Supervisory Processor, an MCU implementing
>> >> >> varoius bits of housekeeping functionality (watchdoging, backlight
>> >> >> control, LED control, etc) on RAVE family of products by Zodiac
>> >> >> Inflight Innovations.
>> >> >>
>> >> >> This driver implementes core MFD/serdev device as well as
>> >> >> communication subroutines necessary for commanding the device.
>> >> >
>> >> > I only skimmed this, but have a few of comments below.
>> >> >
>> >> >> Cc: linux-kernel@...r.kernel.org
>> >> >> Cc: cphealy@...il.com
>> >> >> Cc: Lucas Stach <l.stach@...gutronix.de>
>> >> >> Cc: Nikita Yushchenko <nikita.yoush@...entembedded.com>
>> >> >> Cc: Lee Jones <lee.jones@...aro.org>
>> >> >> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>> >> >> Cc: Pavel Machek <pavel@....cz>
>> >> >> Tested-by: Chris Healy <cphealy@...il.com>
>> >> >> Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>
>> >> >> Signed-off-by: Andrey Smirnov <andrew.smirnov@...il.com>
>> >> >> ---
>> >> >>  drivers/platform/Kconfig        |   2 +
>> >> >>  drivers/platform/Makefile       |   1 +
>> >> >>  drivers/platform/rave/Kconfig   |  26 ++
>> >> >>  drivers/platform/rave/Makefile  |   1 +
>> >> >>  drivers/platform/rave/rave-sp.c | 677 ++++++++++++++++++++++++++++++++++++++++
>> >> >
>> >> > First of all, why do these live in drivers/platform and why don't use
>> >> > the mfd subsystem to implement this driver (instead of rolling your own
>> >> > mfd-implementation)?
>> >> >
>> >>
>> >> Because when I submitted this driver to MFD Lee Jones told me that it
>> >> didn't belong to that subsystem and should be added to the kernel in
>> >> "drivers/platform".
>> >
>> > When I first reviewed this driver, it looked far too complex to be an
>> > MFD driver.
>>
>> With exception of removed sysfs/debugfs entries (and minor code
>> changes) the driver is still the same as when you reviewed it.
>>
>> > Most of the code doesn't deal with what I'd expect to be
>> > handled in MFD.  Why do you have ~600 lines of protocol handling?
>> >
>>
>> Because there are three ever-so-slightly different ICDs that the
>> firmware of the device implements depending on the variant/generation
>> of the overall platform it is a part of.
>
> Okay, it sounds like this might fit into MFD after all.
>
> Sorry for the fuss.
>
> Do you want me to review the first submission, or will you submit
> again?
>

No worries. I'll move the code back to "drivers/mfd" in v9 of this
patchset and we can go from there.

Thanks,
Andrey Smirnov