Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752594AbaBJPW7 (ORCPT ); Mon, 10 Feb 2014 10:22:59 -0500 Received: from mail-ob0-f174.google.com ([209.85.214.174]:48085 "EHLO mail-ob0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752232AbaBJPW4 (ORCPT ); Mon, 10 Feb 2014 10:22:56 -0500 MIME-Version: 1.0 In-Reply-To: References: <1391859219-3102-1-git-send-email-lpapp@kde.org> Date: Mon, 10 Feb 2014 20:52:55 +0530 Message-ID: Subject: Re: [PATCH v2] mfd: MAX6650/6651 support From: Sachin Kamat To: Laszlo Papp , LKML , Lee Jones Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10 February 2014 18:48, Laszlo Papp wrote: > On Mon, Feb 10, 2014 at 12:02 PM, Sachin Kamat wrote: >> Hi Laszlo, >> >> If you are considering re-spinning this patch based on Lee's comments, please >> also consider my comments inline. > > Hi, sure, thanks. > >> On 8 February 2014 17:03, Laszlo Papp wrote: >>> MAX6650/MAX6651 chip is a multi-function device with I2C busses. The >>> chip includes fan-speed regulators and monitors, GPIO, and alarm. >>> >>> This patch is an initial release of a MAX6650/6651 MFD driver that >>> supports to enable the chip with its primary I2C bus that will connect >>> the hwmon, and then the gpio devices for now. >>> >>> Signed-off-by: Laszlo Papp >>> --- >>> drivers/mfd/Kconfig | 11 +++++ >>> drivers/mfd/Makefile | 1 + >>> drivers/mfd/max665x.c | 88 +++++++++++++++++++++++++++++++++++++ >>> include/linux/mfd/max665x-private.h | 42 ++++++++++++++++++ >>> 4 files changed, 142 insertions(+) >>> create mode 100644 drivers/mfd/max665x.c >>> create mode 100644 include/linux/mfd/max665x-private.h >>> >>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >>> index 49bb445..e25be62 100644 >>> --- a/drivers/mfd/Kconfig >>> +++ b/drivers/mfd/Kconfig >>> @@ -368,6 +368,17 @@ config MFD_MAX8907 >>> accessing the device; additional drivers must be enabled in order >>> to use the functionality of the device. >>> >>> +config MFD_MAX665X >>> + bool "Maxim Semiconductor MAX6650/MAX6651 Support" >>> + select MFD_CORE >>> + depends on I2C >>> + select REGMAP_I2C >> >> Move the "depends on" before the select MFD_CORE. > > OK. > >>> + help >>> + Say yes here to support for Maxim Semiconductor MAX6650/MAX6651. >> >> s/here to support/here to add support > > OK. > >>>This is >>> + a fan speed regulator and monitor IC. This driver provies common support >>> + for accessing the device, additional drivers must be enabled in order to >>> + use the functionality of the device. >>> + >>> config MFD_MAX8925 >>> bool "Maxim Semiconductor MAX8925 PMIC Support" >>> depends on I2C=y >>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile >>> index 5aea5ef..63668c5 100644 >>> --- a/drivers/mfd/Makefile >>> +++ b/drivers/mfd/Makefile >>> @@ -111,6 +111,7 @@ obj-$(CONFIG_MFD_DA9055) += da9055.o >>> da9063-objs := da9063-core.o da9063-irq.o da9063-i2c.o >>> obj-$(CONFIG_MFD_DA9063) += da9063.o >>> >>> +obj-$(CONFIG_MFD_MAX665X) += max665x.o >>> obj-$(CONFIG_MFD_MAX14577) += max14577.o >>> obj-$(CONFIG_MFD_MAX77686) += max77686.o max77686-irq.o >>> obj-$(CONFIG_MFD_MAX77693) += max77693.o max77693-irq.o >>> diff --git a/drivers/mfd/max665x.c b/drivers/mfd/max665x.c >>> new file mode 100644 >>> index 0000000..395373fe >>> --- /dev/null >>> +++ b/drivers/mfd/max665x.c >>> @@ -0,0 +1,88 @@ >>> +/* >>> + * Device access for MAX6650-MAX6651 >>> + * >>> + * Copyright(c) 2013 Polatis Ltd. >>> + * >>> + * Author: Laszlo Papp >>> + * >>> + * This program is free software; you can redistribute it and/or modify it >>> + * under the terms of the GNU General Public License as published by the >>> + * Free Software Foundation; either version 2 of the License, or (at your >>> + * option) any later version. >>> + */ >>> + >>> +#include >>> +#include >>> +#include >>> +#include >> >> Please arrange these alphabetically. > > OK, it does not matter for me. Please agree upon something with Lee. :-) > >> >> [snip] >>> + >>> +struct max665x_dev { >>> + struct device *dev; >>> + struct mutex iolock; >>> + >>> + struct i2c_client *i2c; >>> + struct regmap *map; >>> + >>> + int type; >> >> Unnecessary extra lines above could be removed. > > I prefer it that way, but I will remove the two extra lines as you wish. As I already said, these are just nits and only since you were re-spinning the patch, I suggested them. Also, since this is a new file being added, it avoids further patches doing these same things. -- With warm regards, Sachin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/