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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 18 Oct 2017 09:27:47 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Andrey Smirnov <andrew.smirnov@...il.com>
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>,
        Lee Jones <lee.jones@...aro.org>,
        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 Mon, Oct 16, 2017 at 09:45:58AM -0700, Andrey Smirnov wrote:
> On Mon, Oct 16, 2017 at 7:14 AM, Johan Hovold <johan@...nel.org> wrote:
> > On Fri, Oct 13, 2017 at 08:56:00AM -0700, 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.

> The reason I mentioned "simple-mfd" was because I assumed it was
> necessary to use of_platform_default_populate(), which, re-reading the
> corresponding code, seems to have been a mistake on my part. However,
> now I am not sure I understand the point you are/were trying to make.
> The way I understand "instead of rolling your own mfd-implementation"
> is "You ignored/missed the precedent and didn't use agreed-upon way of
> creating an MFD from DT and re-invented the code to do so", in which
> case I'd like to know the canonical way I missed.
> 
> Form you response, however, I get a feeling that you are trying to
> convince me that the driver I submitted is an MFD driver. If so, then
> there's no need, we are in agreement here.

Good, then we just need to convince Lee. ;)

I was merely pointing out that, yes, you are implementing an MFD driver,
but you placed it in drivers/platform and did not not use any facilities
provided by the MFD subsystem (hence, the "rolling your own").

Take a look at the dln2 mfd driver which I believe implement something
similar albeit using USB as a transport (and therefore does not use DT
to instantiates the children). One point being that not all mfd drivers
use i2c/spi and regmap.

> > Also note that no drivers under drivers/platforms does anything like
> > this.
> >
> 
> I am aware of that. As I as I mentioned before, the decision to place
> this code into "drivers/platforms" was not mine.

You should have put this in the changelog and commit message since its
far from obvious what happened here.

Johan