[PATCH v3] mfd: MAX6650/6651 support

[PATCH v3] mfd: MAX6650/6651 support

[Laszlo Papp]

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..4ad39f4 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
+	help
+	  Say yes here to support for Maxim Semiconductor MAX6650/MAX6651. This is
+	  a fan speed regulator and monitor IC. This driver provides 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..cd39b5a
--- /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>
+
+#include <linux/mfd/max665x-private.h>
+
+static struct mfd_cell max665x_devs[] = {
+	{ .name = "max6651-gpio", },
+	{ .name = "max6650", }, /* hwmon driver */
+	{},
+};
+
+const struct regmap_config max665x_regmap_config = {
+	.reg_bits = 5,
+};
+
+static int max665x_probe(struct i2c_client *i2c,
+			    const struct i2c_device_id *id)
+{
+	struct max665x_dev *max665x;
+	int ret;
+
+	max665x = devm_kzalloc(&i2c->dev, sizeof(*max665x), GFP_KERNEL);
+	if (!max665x)
+		return -ENOMEM;
+
+	i2c_set_clientdata(i2c, max665x);
+	max665x->dev = &i2c->dev;
+	max665x->i2c = i2c;
+	max665x->map = devm_regmap_init_i2c(i2c, &max665x_regmap_config);
+
+	mutex_init(&max665x->iolock);
+
+	ret = mfd_add_devices(max665x->dev, -1, max665x_devs,
+			ARRAY_SIZE(max665x_devs),
+			NULL, 0, NULL);
+
+	if (ret < 0)
+		dev_err(max665x->dev, "failed to register child devices\n");
+
+	return ret;
+}
+
+static int max665x_remove(struct i2c_client *i2c)
+{
+	struct max665x_dev *max665x = i2c_get_clientdata(i2c);
+
+	mfd_remove_devices(max665x->dev);
+
+	return 0;
+}
+
+static const struct i2c_device_id max665x_id[] = {
+	{ "max6650", TYPE_MAX6650 },
+	{ "max6651", TYPE_MAX6651 },
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, max665x_id);
+
+static struct i2c_driver max665x_driver = {
+	.driver = {
+		.name = "max665x",
+		.owner = THIS_MODULE,
+	},
+	.probe = max665x_probe,
+	.remove = max665x_remove,
+	.id_table = max665x_id,
+};
+
+module_i2c_driver(max665x_driver);
+
+MODULE_AUTHOR("Laszlo Papp <laszlo.papp@polatis.com>");
+MODULE_DESCRIPTION("MAX6650-MAX6651 MFD");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/max665x-private.h b/include/linux/mfd/max665x-private.h
new file mode 100644
index 0000000..eed5fdf
--- /dev/null
+++ b/include/linux/mfd/max665x-private.h
@@ -0,0 +1,42 @@
+/*
+ * max665x-private.h - Driver for the Maxim 6650/6651
+ *
+ * Copyright 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef __LINUX_MFD_MAX665X_PRIVATE_H
+#define __LINUX_MFD_MAX665X_PRIVATE_H
+
+#include <linux/i2c.h>
+#include <linux/export.h>
+#include <linux/regmap.h>
+
+struct max665x_dev {
+	struct device *dev;
+	struct mutex iolock;
+
+	struct i2c_client *i2c;
+	struct regmap     *map;
+
+	int type;
+};
+
+enum max665x_types {
+	TYPE_MAX6650,
+	TYPE_MAX6651,
+};
+
+#endif
-- 
1.8.5.4

Re: [PATCH v3] mfd: MAX6650/6651 support

[Lee Jones]

> 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..4ad39f4 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
> +	help
> +	  Say yes here to support for Maxim Semiconductor MAX6650/MAX6651. This is
> +	  a fan speed regulator and monitor IC. This driver provides 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..cd39b5a
> --- /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>
> +
> +#include <linux/mfd/max665x-private.h>
> +
> +static struct mfd_cell max665x_devs[] = {
> +	{ .name = "max6651-gpio", },
> +	{ .name = "max6650", }, /* hwmon driver */

What happened to renaming the hwmon driver, so we can have
"max6650-hwmon" here?

<snip>

> +static const struct i2c_device_id max665x_id[] = {
> +	{ "max6650", TYPE_MAX6650 },
> +	{ "max6651", TYPE_MAX6651 },
> +	{},

Can you remind me where these IDs are being used?

> +};
> +MODULE_DEVICE_TABLE(i2c, max665x_id);
> +
> +static struct i2c_driver max665x_driver = {
> +	.driver = {
> +		.name = "max665x",
> +		.owner = THIS_MODULE,

All new drivers are required to support Device Tree.

<snip>

> +#ifndef __LINUX_MFD_MAX665X_PRIVATE_H
> +#define __LINUX_MFD_MAX665X_PRIVATE_H
> +
> +#include <linux/i2c.h>
> +#include <linux/export.h>

What's this here for?

> +#include <linux/regmap.h>
> +
> +struct max665x_dev {
> +	struct device *dev;
> +	struct mutex iolock;
> +
> +	struct i2c_client *i2c;
> +	struct regmap     *map;
> +
> +	int type;
> +};
> +
> +enum max665x_types {
> +	TYPE_MAX6650,
> +	TYPE_MAX6651,
> +};
> +
> +#endif

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3] mfd: MAX6650/6651 support

[Laszlo Papp]

>> +static struct mfd_cell max665x_devs[] = {
>> +     { .name = "max6651-gpio", },
>> +     { .name = "max6650", }, /* hwmon driver */
>
> What happened to renaming the hwmon driver, so we can have
> "max6650-hwmon" here?

I will add the alias in the next patch (e.g. hwmon change). I can come
back to this and rename then.

>> +static const struct i2c_device_id max665x_id[] = {
>> +     { "max6650", TYPE_MAX6650 },
>> +     { "max6651", TYPE_MAX6651 },
>> +     {},
>
> Can you remind me where these IDs are being used?

Strictly speaking, not in this change, so I can remove it and add
later if needed.

>> +};
>> +MODULE_DEVICE_TABLE(i2c, max665x_id);
>> +
>> +static struct i2c_driver max665x_driver = {
>> +     .driver = {
>> +             .name = "max665x",
>> +             .owner = THIS_MODULE,
>
> All new drivers are required to support Device Tree.

Do you have some recommendation for documentation, examples, etc? I am
new to this.

> <snip>
>
>> +#ifndef __LINUX_MFD_MAX665X_PRIVATE_H
>> +#define __LINUX_MFD_MAX665X_PRIVATE_H
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/export.h>
>
> What's this here for?

Good catch, the export line is a left-over.

Re: [PATCH v3] mfd: MAX6650/6651 support

[Laszlo Papp]

>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, max665x_id);
>>> +
>>> +static struct i2c_driver max665x_driver = {
>>> +     .driver = {
>>> +             .name = "max665x",
>>> +             .owner = THIS_MODULE,
>>
>> All new drivers are required to support Device Tree.
>
> Do you have some recommendation for documentation, examples, etc? I am
> new to this.

I see .of_match_table added in certain mfd drivers, but I am not sure
what I should put into the name or even the compatible member, I see
them boot neglected in various drivers. The structure here does not
seem to be documented either:
http://lxr.free-electrons.com/source/include/linux/mod_devicetable.h#L220

I do not know where to get the information for the .name and/or
.compatible entry for, or if I need anything more.

Re: [PATCH v3] mfd: MAX6650/6651 support

[Lee Jones]

> >> +static struct mfd_cell max665x_devs[] = {
> >> +     { .name = "max6651-gpio", },
> >> +     { .name = "max6650", }, /* hwmon driver */
> >
> > What happened to renaming the hwmon driver, so we can have
> > "max6650-hwmon" here?
> 
> I will add the alias in the next patch (e.g. hwmon change). I can come
> back to this and rename then.

Better add this patch _after_ that change. That will prevent any
inherently awkward to handing 'cross subsystem' patches.

> >> +static const struct i2c_device_id max665x_id[] = {
> >> +     { "max6650", TYPE_MAX6650 },
> >> +     { "max6651", TYPE_MAX6651 },
> >> +     {},
> >
> > Can you remind me where these IDs are being used?
> 
> Strictly speaking, not in this change, so I can remove it and add
> later if needed.

Please remove them if they're not being used.

> >> +};
> >> +MODULE_DEVICE_TABLE(i2c, max665x_id);
> >> +
> >> +static struct i2c_driver max665x_driver = {
> >> +     .driver = {
> >> +             .name = "max665x",
> >> +             .owner = THIS_MODULE,
> >
> > All new drivers are required to support Device Tree.
> 
> Do you have some recommendation for documentation, examples, etc? I am
> new to this.

Documentation/devicetree/usage-model.txt
Documentation/devicetree/bindings/mfd/*

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog