From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751414AbdJMQRG (ORCPT ); Fri, 13 Oct 2017 12:17:06 -0400 Received: from mail-wr0-f169.google.com ([209.85.128.169]:54944 "EHLO mail-wr0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751004AbdJMQRE (ORCPT ); Fri, 13 Oct 2017 12:17:04 -0400 X-Google-Smtp-Source: AOwi7QCwN5DjMjuAOFyPJDTXRR7fxNkRgqJuiwAxavWyXhob0Iu18o4S7cDQjr1K9UrxI2Lv7mWs9Q== Date: Fri, 13 Oct 2017 17:17:00 +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: <20171013161700.2anaqwthq4rnco6f@dell> References: <20171013061321.31252-1-andrew.smirnov@gmail.com> <20171013061321.31252-2-andrew.smirnov@gmail.com> <20171013072731.GC29243@localhost> 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 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. 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? -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog