On Tue, Oct 24, 2017 at 3:28 AM, Lee Jones <lee.jo...@linaro.org> wrote: > On Fri, 13 Oct 2017, Andrey Smirnov wrote: > >> On Fri, Oct 13, 2017 at 9:17 AM, Lee Jones <lee.jo...@linaro.org> wrote: >> > On Fri, 13 Oct 2017, Andrey Smirnov wrote: >> > >> >> On Fri, Oct 13, 2017 at 12:27 AM, Johan Hovold <jo...@kernel.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@vger.kernel.org >> >> >> Cc: cphe...@gmail.com >> >> >> Cc: Lucas Stach <l.st...@pengutronix.de> >> >> >> Cc: Nikita Yushchenko <nikita.yo...@cogentembedded.com> >> >> >> Cc: Lee Jones <lee.jo...@linaro.org> >> >> >> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org> >> >> >> Cc: Pavel Machek <pa...@ucw.cz> >> >> >> Tested-by: Chris Healy <cphe...@gmail.com> >> >> >> Reviewed-by: Andy Shevchenko <andy.shevche...@gmail.com> >> >> >> Signed-off-by: Andrey Smirnov <andrew.smir...@gmail.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
