lkml.org 
[lkml]   [2014]   [Feb]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v2] mfd: MAX6650/6651 support
    From
    On 10 February 2014 18:48, Laszlo Papp <lpapp@kde.org> wrote:
    > On Mon, Feb 10, 2014 at 12:02 PM, Sachin Kamat <sachin.kamat@linaro.org> 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 <lpapp@kde.org> 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 <lpapp@kde.org>
    >>> ---
    >>> 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 <laszlo.papp@polatis.com>
    >>> + *
    >>> + * 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 <linux/device.h>
    >>> +#include <linux/mfd/core.h>
    >>> +#include <linux/module.h>
    >>> +#include <linux/i2c.h>
    >>
    >> 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


    \
     
     \ /
      Last update: 2014-02-10 17:01    [W:4.217 / U:0.488 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site