From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751591AbdJXSeZ (ORCPT ); Tue, 24 Oct 2017 14:34:25 -0400 Received: from mail-qk0-f196.google.com ([209.85.220.196]:55400 "EHLO mail-qk0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751280AbdJXSeU (ORCPT ); Tue, 24 Oct 2017 14:34:20 -0400 X-Google-Smtp-Source: ABhQp+RLHp0iYsAiH6PfQ1V1hv2oqA6OUCrouZs/hfztbsy1z6JLAqz9Mv5kxPoRrymD6D7vhL9RJr4LHF+awNm10nM= MIME-Version: 1.0 In-Reply-To: <20171024102802.u6j3valav7idapi5@dell> References: <20171013061321.31252-1-andrew.smirnov@gmail.com> <20171013061321.31252-2-andrew.smirnov@gmail.com> <20171013072731.GC29243@localhost> <20171013161700.2anaqwthq4rnco6f@dell> <20171024102802.u6j3valav7idapi5@dell> From: Andrey Smirnov Date: Tue, 24 Oct 2017 11:34:19 -0700 Message-ID: Subject: Re: [RESEND PATCH v7 1/1] platform: Add driver for RAVE Supervisory Processor To: Lee Jones Cc: Johan Hovold , linux-kernel , Chris Healy , Lucas Stach , Nikita Yushchenko , Greg Kroah-Hartman , Pavel Machek Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 24, 2017 at 3:28 AM, Lee Jones wrote: > 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? > 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