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

[Johan Hovold]

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@xxxxxxxxxx> 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@xxxxxxxxxx> 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