From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752269AbdJMQfB (ORCPT ); Fri, 13 Oct 2017 12:35:01 -0400 Received: from mail-qk0-f193.google.com ([209.85.220.193]:51093 "EHLO mail-qk0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751502AbdJMQfA (ORCPT ); Fri, 13 Oct 2017 12:35:00 -0400 X-Google-Smtp-Source: ABhQp+TfaT22R9/VEWnJTh4kccQfg8enr948R/UG0o3S1EkqwcTDjXPMoFJOE5Nsy6vcsUvFtzFtXJJpJ0O9d0PmSAs= MIME-Version: 1.0 In-Reply-To: <20171013161700.2anaqwthq4rnco6f@dell> References: <20171013061321.31252-1-andrew.smirnov@gmail.com> <20171013061321.31252-2-andrew.smirnov@gmail.com> <20171013072731.GC29243@localhost> <20171013161700.2anaqwthq4rnco6f@dell> From: Andrey Smirnov Date: Fri, 13 Oct 2017 09:34:58 -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 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. Thanks, Andrey Smirnov