On 10 February 2014 18:48, Laszlo Papp <lp...@kde.org> wrote:
> On Mon, Feb 10, 2014 at 12:02 PM, Sachin Kamat <sachin.ka...@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 <lp...@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 <lp...@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.p...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
