From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932573AbdJXK2U (ORCPT ); Tue, 24 Oct 2017 06:28:20 -0400 Received: from mail-wr0-f195.google.com ([209.85.128.195]:50407 "EHLO mail-wr0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932403AbdJXK2Q (ORCPT ); Tue, 24 Oct 2017 06:28:16 -0400 X-Google-Smtp-Source: ABhQp+QLzte2zc3NUxUKjwXnIkj7yPQxmF/Lr9f9/Q5Dn4IhDRP5Q98b+g131LwtadZB7H1CO6j3HA== Date: Tue, 24 Oct 2017 11:28:02 +0100 From: Lee Jones To: Andrey Smirnov Cc: Johan Hovold , linux-kernel , Chris Healy , Lucas Stach , Nikita Yushchenko , Greg Kroah-Hartman , Pavel Machek Subject: Re: [RESEND PATCH v7 1/1] platform: Add driver for RAVE Supervisory Processor Message-ID: <20171024102802.u6j3valav7idapi5@dell> References: <20171013061321.31252-1-andrew.smirnov@gmail.com> <20171013061321.31252-2-andrew.smirnov@gmail.com> <20171013072731.GC29243@localhost> <20171013161700.2anaqwthq4rnco6f@dell> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 13 Oct 2017, Andrey Smirnov wrote: > On Fri, Oct 13, 2017 at 9:17 AM, Lee Jones wrote: > > On Fri, 13 Oct 2017, Andrey Smirnov wrote: > > > >> On Fri, Oct 13, 2017 at 12:27 AM, Johan Hovold 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: cphealy@gmail.com > >> >> Cc: Lucas Stach > >> >> Cc: Nikita Yushchenko > >> >> Cc: Lee Jones > >> >> Cc: Greg Kroah-Hartman > >> >> Cc: Pavel Machek > >> >> Tested-by: Chris Healy > >> >> Reviewed-by: Andy Shevchenko > >> >> Signed-off-by: Andrey Smirnov > >> >> --- > >> >> 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? -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog