On Thu, 31 Aug 2017, s.abhi...@gmail.com wrote:

> From: Abhisit Sangjan <s.abhi...@gmail.com>
> 
> TI LMP92001 Analog System Monitor and Controller
> 
> 8-bit GPIOs.
> 12 DACs with 12-bit resolution.
> 
> The GPIOs and DACs are shared port function with Cy function pin to
> take control the pin suddenly from external hardware.
> DAC's referance voltage selectable for Internal/External.
> 
> 16 + 1 ADCs with 12-bit resolution.
> 
> Built-in internal Temperature Sensor on channel 17.
> Window Comparator Function is supported on channel 1-3 and 9-11 for
> monitoring with interrupt signal (pending to implement for interrupt).
> ADC's referance voltage selectable for Internal/External.
> 
> Signed-off-by: Abhisit Sangjan <s.abhi...@gmail.com>
> ---
>  drivers/mfd/Kconfig                |  12 ++
>  drivers/mfd/Makefile               |   4 +
>  drivers/mfd/lmp92001-core.c        | 307 
> +++++++++++++++++++++++++++++++++++++
>  drivers/mfd/lmp92001-debug.c       |  64 ++++++++
>  drivers/mfd/lmp92001-i2c.c         | 211 +++++++++++++++++++++++++
>  include/linux/mfd/lmp92001/core.h  | 135 ++++++++++++++++
>  include/linux/mfd/lmp92001/debug.h |  27 ++++
>  7 files changed, 760 insertions(+)
>  create mode 100644 drivers/mfd/lmp92001-core.c
>  create mode 100644 drivers/mfd/lmp92001-debug.c
>  create mode 100644 drivers/mfd/lmp92001-i2c.c
>  create mode 100644 include/linux/mfd/lmp92001/core.h
>  create mode 100644 include/linux/mfd/lmp92001/debug.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 94ad2c1c3d90..a20389a30cf8 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1528,6 +1528,18 @@ config MFD_LM3533
>         additional drivers must be enabled in order to use the LED,
>         backlight or ambient-light-sensor functionality of the device.
>  
> +config MFD_LMP92001
> +       tristate "TI LMP92001 Analog System Monitor and Controller"
> +       depends on I2C
> +       select MFD_CORE
> +       select REGMAP_I2C
> +       help
> +         Say yes here to enable support for TI LMP92001 Analog System
> +         Monitor and Controller
> +
> +         This driver provide support for 16 ADC, 12 DAC, Voltage Reference,
> +         Analog Temperature Sensor and 8-bit GPIO Port.
> +
>  config MFD_TIMBERDALE
>       tristate "Timberdale FPGA"
>       select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 080793b3fd0e..20d2e65d503a 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -199,6 +199,10 @@ obj-$(CONFIG_MFD_RN5T618)        += rn5t618.o
>  obj-$(CONFIG_MFD_SEC_CORE)   += sec-core.o sec-irq.o
>  obj-$(CONFIG_MFD_SYSCON)     += syscon.o
>  obj-$(CONFIG_MFD_LM3533)     += lm3533-core.o lm3533-ctrlbank.o
> +
> +lmp92001-objs                        := lmp92001-core.o lmp92001-i2c.o 
> lmp92001-debug.o
> +obj-$(CONFIG_MFD_LMP92001)   += lmp92001.o
> +
>  obj-$(CONFIG_MFD_VEXPRESS_SYSREG)    += vexpress-sysreg.o
>  obj-$(CONFIG_MFD_RETU)               += retu-mfd.o
>  obj-$(CONFIG_MFD_AS3711)     += as3711.o
> diff --git a/drivers/mfd/lmp92001-core.c b/drivers/mfd/lmp92001-core.c
> new file mode 100644
> index 000000000000..5ee315ce61bb
> --- /dev/null
> +++ b/drivers/mfd/lmp92001-core.c
> @@ -0,0 +1,307 @@
> +/*
> + * lmp92001-core.c - Device access for TI LMP92001

Please remove the filename from the header comment.

> + * Copyright 2016-2017 Celestica Ltd.
> + *
> + * Author: Abhisit Sangjan <s.abhi...@gmail.com>
> + *
> + * Inspired by wm831x driver.
> + *
> + * 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.

Can you use the short licence?

> + */
> +
> +#include <linux/mfd/lmp92001/core.h>
> +#include <linux/mfd/lmp92001/debug.h>

I don't see a requirement to create a new directory for this driver.

> +static const struct mfd_cell lmp92001_devs[] = {
> +     {
> +             .name = "lmp92001-gpio",
> +             .of_compatible = "ti,lmp92001-gpio",
> +     },
> +     {

Place these two lines on a single one.

> +             .name = "lmp92001-adc",
> +             .of_compatible = "ti,lmp92001-adc",
> +     },
> +     {

... and here.

> +             .name = "lmp92001-dac",
> +             .of_compatible = "ti,lmp92001-dac",
> +     },
> +};
> +
> +static struct reg_default lmp92001_defaults[] = {
> +     { LMP92001_SGEN,  0x40   },
> +     { LMP92001_SHIL,  0x00   },
> +     { LMP92001_SLOL,  0x00   },
> +     { LMP92001_CGEN,  0x00   },
> +     { LMP92001_CDAC,  0x03   },
> +     { LMP92001_CGPO,  0xFF   },
> +     { LMP92001_CINH,  0x00   },
> +     { LMP92001_CINL,  0x00   },
> +     { LMP92001_CAD1,  0x00   },
> +     { LMP92001_CAD2,  0x00   },
> +     { LMP92001_CAD3,  0x00   },
> +     { LMP92001_CTRIG, 0x00   },
> +     { LMP92001_CREF,  0x07   },
> +     { LMP92001_ADC1,  0x0000 },
> +     { LMP92001_ADC2,  0x0000 },
> +     { LMP92001_ADC3,  0x0000 },
> +     { LMP92001_ADC4,  0x0000 },
> +     { LMP92001_ADC5,  0x0000 },
> +     { LMP92001_ADC6,  0x0000 },
> +     { LMP92001_ADC7,  0x0000 },
> +     { LMP92001_ADC8,  0x0000 },
> +     { LMP92001_ADC9,  0x0000 },
> +     { LMP92001_ADC10, 0x0000 },
> +     { LMP92001_ADC11, 0x0000 },
> +     { LMP92001_ADC12, 0x0000 },
> +     { LMP92001_ADC13, 0x0000 },
> +     { LMP92001_ADC14, 0x0000 },
> +     { LMP92001_ADC15, 0x0000 },
> +     { LMP92001_ADC16, 0x0000 },
> +     { LMP92001_LIH1,  0x0FFF },
> +     { LMP92001_LIH2,  0x0FFF },
> +     { LMP92001_LIH3,  0x0FFF },
> +     { LMP92001_LIH9,  0x0FFF },
> +     { LMP92001_LIH10, 0x0FFF },
> +     { LMP92001_LIH11, 0x0FFF },
> +     { LMP92001_LIL1,  0x0000 },
> +     { LMP92001_LIL2,  0x0000 },
> +     { LMP92001_LIL3,  0x0000 },
> +     { LMP92001_LIL9,  0x0000 },
> +     { LMP92001_LIL10, 0x0000 },
> +     { LMP92001_LIL11, 0x0000 },
> +     { LMP92001_DAC1,  0x0000 },
> +     { LMP92001_DAC2,  0x0000 },
> +     { LMP92001_DAC3,  0x0000 },
> +     { LMP92001_DAC4,  0x0000 },
> +     { LMP92001_DAC5,  0x0000 },
> +     { LMP92001_DAC6,  0x0000 },
> +     { LMP92001_DAC7,  0x0000 },
> +     { LMP92001_DAC8,  0x0000 },
> +     { LMP92001_DAC9,  0x0000 },
> +     { LMP92001_DAC10, 0x0000 },
> +     { LMP92001_DAC11, 0x0000 },
> +     { LMP92001_DAC12, 0x0000 },
> +     { LMP92001_DALL,  0x0000 },
> +};
> +
> +static bool lmp92001_reg_readable(struct device *dev, unsigned int reg)
> +{
> +     switch (reg) {
> +     case LMP92001_ID:
> +     case LMP92001_VER:
> +     case LMP92001_SGEN:
> +     case LMP92001_SGPI:
> +     case LMP92001_SHIL:
> +     case LMP92001_SLOL:
> +     case LMP92001_CGEN:
> +     case LMP92001_CDAC:
> +     case LMP92001_CGPO:
> +     case LMP92001_CINH:
> +     case LMP92001_CINL:
> +     case LMP92001_CAD1:
> +     case LMP92001_CAD2:
> +     case LMP92001_CAD3:
> +     case LMP92001_ADC1:
> +     case LMP92001_ADC2:
> +     case LMP92001_ADC3:
> +     case LMP92001_ADC4:
> +     case LMP92001_ADC5:
> +     case LMP92001_ADC6:
> +     case LMP92001_ADC7:
> +     case LMP92001_ADC8:
> +     case LMP92001_ADC9:
> +     case LMP92001_ADC10:
> +     case LMP92001_ADC11:
> +     case LMP92001_ADC12:
> +     case LMP92001_ADC13:
> +     case LMP92001_ADC14:
> +     case LMP92001_ADC15:
> +     case LMP92001_ADC16:
> +     case LMP92001_ADC17:
> +     case LMP92001_LIH1:
> +     case LMP92001_LIH2:
> +     case LMP92001_LIH3:
> +     case LMP92001_LIH9:
> +     case LMP92001_LIH10:
> +     case LMP92001_LIH11:
> +     case LMP92001_LIL1:
> +     case LMP92001_LIL2:
> +     case LMP92001_LIL3:
> +     case LMP92001_LIL9:
> +     case LMP92001_LIL10:
> +     case LMP92001_LIL11:
> +     case LMP92001_CREF:
> +     case LMP92001_DAC1:
> +     case LMP92001_DAC2:
> +     case LMP92001_DAC3:
> +     case LMP92001_DAC4:
> +     case LMP92001_DAC5:
> +     case LMP92001_DAC6:
> +     case LMP92001_DAC7:
> +     case LMP92001_DAC8:
> +     case LMP92001_DAC9:
> +     case LMP92001_DAC10:
> +     case LMP92001_DAC11:
> +     case LMP92001_DAC12:
> +     case LMP92001_BLK0:
> +     case LMP92001_BLK1:
> +     case LMP92001_BLK2:
> +     case LMP92001_BLK3:
> +     case LMP92001_BLK4:
> +     case LMP92001_BLK5:

Any reason why you're not using ranges?

Listing every single register is a drag.

> +             return true;
> +     default:
> +             return false;
> +     }
> +}
> +
> +static bool lmp92001_reg_writeable(struct device *dev, unsigned int reg)
> +{
> +     switch (reg) {
> +     case LMP92001_CGEN:
> +     case LMP92001_CDAC:
> +     case LMP92001_CGPO:
> +     case LMP92001_CINH:
> +     case LMP92001_CINL:
> +     case LMP92001_CAD1:
> +     case LMP92001_CAD2:
> +     case LMP92001_CAD3:
> +     case LMP92001_CTRIG:
> +     case LMP92001_LIH1:
> +     case LMP92001_LIH2:
> +     case LMP92001_LIH3:
> +     case LMP92001_LIH9:
> +     case LMP92001_LIH10:
> +     case LMP92001_LIH11:
> +     case LMP92001_LIL1:
> +     case LMP92001_LIL2:
> +     case LMP92001_LIL3:
> +     case LMP92001_LIL9:
> +     case LMP92001_LIL10:
> +     case LMP92001_LIL11:
> +     case LMP92001_CREF:
> +     case LMP92001_DAC1:
> +     case LMP92001_DAC2:
> +     case LMP92001_DAC3:
> +     case LMP92001_DAC4:
> +     case LMP92001_DAC5:
> +     case LMP92001_DAC6:
> +     case LMP92001_DAC7:
> +     case LMP92001_DAC8:
> +     case LMP92001_DAC9:
> +     case LMP92001_DAC10:
> +     case LMP92001_DAC11:
> +     case LMP92001_DAC12:
> +     case LMP92001_DALL:
> +     case LMP92001_BLK0:
> +     case LMP92001_BLK1:
> +     case LMP92001_BLK4:
> +     case LMP92001_BLK5:
> +             return true;
> +     default:
> +             return false;
> +     }
> +}
> +
> +static bool lmp92001_reg_volatile(struct device *dev, unsigned int reg)
> +{
> +     switch (reg) {
> +     case LMP92001_SGEN:
> +     case LMP92001_SGPI:
> +     case LMP92001_SHIL:
> +     case LMP92001_SLOL:
> +     case LMP92001_CGEN:
> +     case LMP92001_ADC1:
> +     case LMP92001_ADC2:
> +     case LMP92001_ADC3:
> +     case LMP92001_ADC4:
> +     case LMP92001_ADC5:
> +     case LMP92001_ADC6:
> +     case LMP92001_ADC7:
> +     case LMP92001_ADC8:
> +     case LMP92001_ADC9:
> +     case LMP92001_ADC10:
> +     case LMP92001_ADC11:
> +     case LMP92001_ADC12:
> +     case LMP92001_ADC13:
> +     case LMP92001_ADC14:
> +     case LMP92001_ADC15:
> +     case LMP92001_ADC16:
> +     case LMP92001_ADC17:
> +     case LMP92001_BLK2:
> +     case LMP92001_BLK3:
> +             return true;
> +     default:
> +             return false;
> +     }
> +}
> +
> +struct regmap_config lmp92001_regmap_config = {
> +     .reg_bits = 8,
> +     .val_bits = 16,
> +

Remove the line breaks in this struct.

> +     .cache_type = REGCACHE_RBTREE,
> +
> +     .reg_defaults = lmp92001_defaults,
> +     .num_reg_defaults = ARRAY_SIZE(lmp92001_defaults),
> +
> +     .max_register = LMP92001_BLK5,
> +     .readable_reg = lmp92001_reg_readable,
> +     .writeable_reg = lmp92001_reg_writeable,
> +     .volatile_reg = lmp92001_reg_volatile,
> +};
> +
> +int lmp92001_device_init(struct lmp92001 *lmp92001, unsigned long id, int 
> irq)

Static?

> +{
> +     int ret;
> +     unsigned int comid, ver;
> +
> +     dev_set_drvdata(lmp92001->dev, lmp92001);
> +
> +     ret = regmap_read(lmp92001->regmap, LMP92001_ID, &comid);
> +     if (ret < 0) {
> +             dev_err(lmp92001->dev, "failed to read Company ID: %d\n", ret);

Company ID?  That's a new one on me.  What is this?

> +             goto exit;
> +     }
> +
> +     ret = regmap_read(lmp92001->regmap, LMP92001_VER, &ver);
> +     if (ret < 0) {
> +             dev_err(lmp92001->dev, "failed to read Version: %d\n", ret);
> +             goto exit;
> +     }
> +
> +     dev_info(lmp92001->dev, "Company ID 0x%.2x, Version 0x%.2x\n",
> +                     comid, ver);
> +
> +     ret = mfd_add_devices(lmp92001->dev, PLATFORM_DEVID_AUTO,
> +                             lmp92001_devs, ARRAY_SIZE(lmp92001_devs),
> +                             NULL, 0, NULL);
> +     if (ret != 0) {

if (ret) {

> +             dev_err(lmp92001->dev, "failed to add children\n");

Try to be user friendly.

"Failed to register child devices" is probably more helpful.

> +             goto exit;

Just return here.

> +     }
> +
> +     ret = lmp92001_debug_init(lmp92001);
> +     if (ret < 0) {

if (ret) {

> +             dev_err(lmp92001->dev, "failed to initial debug fs.\n");

This makes no sense.

"Failed to initialise DebugFS"

Why does this function even exist?

> +             goto exit;

Drop this.

> +     }
> +
> +exit:

Drop this.

> +     return ret;
> +}
> +
> +void lmp92001_device_exit(struct lmp92001 *lmp92001)
> +{
> +     lmp92001_debug_exit(lmp92001);
> +     mfd_remove_devices(lmp92001->dev);

Any reason not to use devm_*?

> +}
> diff --git a/drivers/mfd/lmp92001-debug.c b/drivers/mfd/lmp92001-debug.c
> new file mode 100644
> index 000000000000..b1746dc810b3
> --- /dev/null
> +++ b/drivers/mfd/lmp92001-debug.c

This file probably isn't needed at all.

> @@ -0,0 +1,64 @@
> +/*
> + * lmp92001-debug.c - Debug file system for TI LMP92001

Same here.

> + * Copyright 2016-2017 Celestica Ltd.
> + *
> + * Author: Abhisit Sangjan <s.abhi...@gmail.com>
> + *
> + * Inspired by wm831x driver.
> + *
> + * 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.

And here.

> + */
> +
> +#include <linux/kernel.h>
> +
> +#include <linux/mfd/lmp92001/core.h>
> +
> +static ssize_t lmp92001_id_ver_show(struct device *dev,
> +     struct device_attribute *attr, char *buf)
> +{
> +     struct lmp92001 *lmp92001 = dev_get_drvdata(dev);
> +     int ret;
> +     unsigned int comid, ver;
> +
> +     ret = regmap_read(lmp92001->regmap, LMP92001_ID, &comid);
> +     if (ret < 0) {
> +             dev_err(lmp92001->dev, "failed to read Company ID: %d\n", ret);
> +             return ret;
> +     }
> +
> +     ret = regmap_read(lmp92001->regmap, LMP92001_VER, &ver);
> +     if (ret < 0) {
> +             dev_err(lmp92001->dev, "failed to read Version: %d\n", ret);
> +             return ret;
> +     }

This is duplicated code.  You did this once already.

> +     return sprintf(buf, "Company ID 0x%02x (%d), Version 0x%02x (%d)\n",
> +                     comid, comid, ver, ver);
> +}
> +static DEVICE_ATTR(lmp92001_id_ver, 0444, lmp92001_id_ver_show, NULL);

Why don't you just print this to the kernel log (actually I think you
did already) and remove this function/file entirely.

> +int lmp92001_debug_init(struct lmp92001 *lmp92001)
> +{
> +     int ret;
> +
> +     ret = device_create_file(lmp92001->dev, &dev_attr_lmp92001_id_ver);
> +     if (ret != 0)
> +             dev_err(lmp92001->dev,
> +                     "unique ID attribute is not created: %d\n", ret);
> +
> +     return ret;
> +}
> +
> +void lmp92001_debug_exit(struct lmp92001 *lmp92001)
> +{
> +     device_remove_file(lmp92001->dev, &dev_attr_lmp92001_id_ver);
> +}
> diff --git a/drivers/mfd/lmp92001-i2c.c b/drivers/mfd/lmp92001-i2c.c
> new file mode 100644
> index 000000000000..e22ea88f5820
> --- /dev/null
> +++ b/drivers/mfd/lmp92001-i2c.c
> @@ -0,0 +1,211 @@
> +/*
> + * lmp92001-i2c.c - I2C access for TI LMP92001

Same here.

> + * Copyright 2016-2017 Celestica Ltd.
> + *
> + * Author: Abhisit Sangjan <s.abhi...@gmail.com>
> + *
> + * Inspired by wm831x driver.
> + *
> + * 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.

And here.

> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include <linux/mfd/lmp92001/core.h>
> +
> +static const unsigned short lmp92001_i2c_addresses[] = {
> +     0x40, 0x42, 0x44, 0x46, 0x48, 0x4A, 0x4C, 0x4E, 0x50, I2C_CLIENT_END
> +};
> +
> +/* TODO: To read/write block access, it may need to re-ordering endianness! 
> */

Why don't you just do this right away?

When are you realistically going to implement this?

> +static int lmp92001_reg_read(void *context, unsigned int reg, unsigned int 
> *val)
> +{
> +     struct device *dev = context;
> +     struct i2c_client *i2c = to_i2c_client(dev);
> +     int ret;
> +
> +     if (reg > 0xff)

No magic numbers.  Please define the 0xff.

> +             return -EINVAL;
> +
> +     switch (reg) {
> +     case LMP92001_ID ... LMP92001_CTRIG:
> +     case LMP92001_CREF:
> +             *val = ret = i2c_smbus_read_byte_data(i2c, reg);

Only assign *val if ret is not an error.

> +             break;
> +     case LMP92001_ADC1 ... LMP92001_LIL11:
> +     case LMP92001_DAC1 ... LMP92001_DALL:
> +             *val = ret = i2c_smbus_read_word_swapped(i2c, reg);
> +             break;
> +     case LMP92001_BLK0 ... LMP92001_BLK5:
> +             ret = i2c_smbus_read_block_data(i2c, reg,
> +                                             (u8 *)((uintptr_t)*val));
> +             break;
> +     default:
> +             return -EINVAL;
> +     }
> +
> +     if (ret < 0)
> +             return ret;
> +
> +     return 0;

Remove the 4 lines above and just return ret.

> +}
> +
> +static int lmp92001_reg_write(void *context, unsigned int reg, unsigned int 
> val)
> +{
> +     struct device *dev = context;
> +     struct i2c_client *i2c = to_i2c_client(dev);
> +     int ret;
> +
> +     if (reg > 0xff)
> +             return -EINVAL;
> +
> +     switch (reg) {
> +     case LMP92001_ID ... LMP92001_CTRIG:
> +     case LMP92001_CREF:
> +             ret = i2c_smbus_write_byte_data(i2c, reg, val);
> +             break;
> +     case LMP92001_ADC1 ... LMP92001_LIL11:
> +     case LMP92001_DAC1 ... LMP92001_DALL:
> +             ret = i2c_smbus_write_word_swapped(i2c, reg, val);
> +             break;
> +     /* To call this function/case, must be passed val as pointer */
> +     case LMP92001_BLK0:
> +     case LMP92001_BLK4:
> +             ret = i2c_smbus_write_block_data(i2c, reg, 24,

No magic numbers.  Please define them all.

> +                                             (u8 *)((uintptr_t)val));

Why is this type of casting required?

> +             break;
> +     case LMP92001_BLK1:
> +     case LMP92001_BLK5:
> +             ret = i2c_smbus_write_block_data(i2c, reg, 12,
> +                                             (u8 *)((uintptr_t)val));
> +             break;
> +     case LMP92001_BLK2:
> +             ret = i2c_smbus_write_block_data(i2c, reg, 34,
> +                                             (u8 *)((uintptr_t)val));
> +             break;
> +     case LMP92001_BLK3:
> +             ret = i2c_smbus_write_block_data(i2c, reg, 18,
> +                                             (u8 *)((uintptr_t)val));
> +             break;
> +     default:
> +             return -EINVAL;
> +     }
> +
> +     return ret;
> +}
> +
> +static int lmp92001_i2c_probe(struct i2c_client *i2c,

             ^
Line break here instead.

> +     const struct i2c_device_id *id)
> +{
> +     struct lmp92001 *lmp92001;
> +     int ret;
> +
> +     lmp92001 = devm_kzalloc(&i2c->dev, sizeof(struct lmp92001), GFP_KERNEL);
> +     if (!lmp92001)
> +             return -ENOMEM;
> +
> +     i2c_set_clientdata(i2c, lmp92001);
> +     lmp92001->dev = &i2c->dev;
> +
> +     lmp92001_regmap_config.reg_read = lmp92001_reg_read;
> +     lmp92001_regmap_config.reg_write = lmp92001_reg_write;
> +
> +     lmp92001->regmap = devm_regmap_init(&i2c->dev, NULL, &i2c->dev,
> +                                             &lmp92001_regmap_config);
> +     if (IS_ERR(lmp92001->regmap)) {
> +             ret = PTR_ERR(lmp92001->regmap);
> +             dev_err(lmp92001->dev, "failed to allocate register map: %d\n",
> +                             ret);
> +             return ret;
> +     }
> +
> +     return lmp92001_device_init(lmp92001, id->driver_data, i2c->irq);
> +}
> +
> +static int lmp92001_i2c_remove(struct i2c_client *i2c)
> +{
> +     struct lmp92001 *lmp92001 = i2c_get_clientdata(i2c);
> +
> +     lmp92001_device_exit(lmp92001);
> +
> +     return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id lmp92001_dt_ids[] = {
> +     { .compatible = "ti,lmp92001", },
> +     { },
> +};
> +MODULE_DEVICE_TABLE(of, lmp92001_dt_ids);
> +#endif
> +
> +static const struct i2c_device_id lmp92001_i2c_ids[] = {
> +     { "lmp92001" },
> +     { },
> +};
> +MODULE_DEVICE_TABLE(i2c, lmp92001_i2c_ids);
> +
> +static int lmp92001_i2c_detect(struct i2c_client *i2c,
> +     struct i2c_board_info *info)
> +{
> +     struct i2c_adapter *adapter = i2c->adapter;
> +     s32 comid, ver;
> +
> +     if (!i2c_check_functionality(adapter,
> +                                     I2C_FUNC_SMBUS_BYTE_DATA |
> +                                     I2C_FUNC_SMBUS_WORD_DATA |
> +                                     I2C_FUNC_SMBUS_BLOCK_DATA))
> +             return -ENODEV;
> +
> +     comid = i2c_smbus_read_byte_data(i2c, LMP92001_ID);
> +     ver = i2c_smbus_read_byte_data(i2c, LMP92001_VER);
> +
> +     if (comid != 0x01 || ver != 0x10)

No magic numbers.

> +             return -ENODEV;
> +
> +     return 0;
> +}
> +
> +static struct i2c_driver lmp92001_i2c_driver = {
> +     .driver = {
> +             .name = "lmp92001",
> +             .of_match_table = of_match_ptr(lmp92001_dt_ids),
> +     },
> +     .probe          = lmp92001_i2c_probe,
> +     .remove         = lmp92001_i2c_remove,
> +     .id_table       = lmp92001_i2c_ids,
> +     .detect         = lmp92001_i2c_detect,
> +     .address_list   = lmp92001_i2c_addresses,
> +};
> +
> +static int __init lmp92001_i2c_init(void)
> +{
> +     return i2c_add_driver(&lmp92001_i2c_driver);
> +}
> +subsys_initcall(lmp92001_i2c_init);
> +
> +static void __exit lmp92001_i2c_exit(void)
> +{
> +     i2c_del_driver(&lmp92001_i2c_driver);
> +}
> +module_exit(lmp92001_i2c_exit);
> +
> +MODULE_DESCRIPTION("i2c driver for TI LMP92001");
> +MODULE_AUTHOR("Abhisit Sangjan <s.abhi...@gmail.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/lmp92001/core.h 
> b/include/linux/mfd/lmp92001/core.h
> new file mode 100644
> index 000000000000..81aabf8b8e7a
> --- /dev/null
> +++ b/include/linux/mfd/lmp92001/core.h
> @@ -0,0 +1,135 @@
> +/*
> + * include/linux/mfd/lmp92001/core.h - Core interface for TI LMP92001

Same here.

> + * Copyright 2016-2017 Celestica Ltd.
> + *
> + * Author: Abhisit Sangjan <s.abhi...@gmail.com>
> + *
> + * Inspired by wm831x driver.
> + *
> + * 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.

Same here.

> + */
> +
> +#ifndef __MFD_LMP92001_CORE_H__
> +#define __MFD_LMP92001_CORE_H__
> +
> +#include <linux/device.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +
> +/*
> + * Register values.
> + */
> +/* ID */
> +#define LMP92001_ID  0x0E
> +#define LMP92001_VER 0x0F
> +/* STATUS */
> +#define LMP92001_SGEN        0x10
> +#define LMP92001_SGPI        0x11
> +#define LMP92001_SHIL        0x12
> +#define LMP92001_SLOL        0x13
> +/* CONTROL */
> +#define LMP92001_CGEN        0x14
> +#define LMP92001_CDAC        0x15
> +#define LMP92001_CGPO        0x16
> +#define LMP92001_CINH        0x17
> +#define LMP92001_CINL        0x18
> +#define LMP92001_CAD1        0x19
> +#define LMP92001_CAD2        0x1A
> +#define LMP92001_CAD3        0x1B
> +#define LMP92001_CTRIG       0x1C
> +/* ADC OUTPUT DATA */
> +#define LMP92001_ADC1        0x20
> +#define LMP92001_ADC2        0x21
> +#define LMP92001_ADC3        0x22
> +#define LMP92001_ADC4        0x23
> +#define LMP92001_ADC5        0x24
> +#define LMP92001_ADC6        0x25
> +#define LMP92001_ADC7        0x26
> +#define LMP92001_ADC8        0x27
> +#define LMP92001_ADC9        0x28
> +#define LMP92001_ADC10       0x29
> +#define LMP92001_ADC11       0x2A
> +#define LMP92001_ADC12       0x2B
> +#define LMP92001_ADC13       0x2C
> +#define LMP92001_ADC14       0x2D
> +#define LMP92001_ADC15       0x2E
> +#define LMP92001_ADC16       0x2F
> +#define LMP92001_ADC17       0x30
> +/* ADC WINDOW COMPARATOR LIMITS */
> +#define LMP92001_LIH1        0x40
> +#define LMP92001_LIH2        0x41
> +#define LMP92001_LIH3        0x42
> +#define LMP92001_LIH9        0x43
> +#define LMP92001_LIH10       0x44
> +#define LMP92001_LIH11       0x45
> +#define LMP92001_LIL1        0x46
> +#define LMP92001_LIL2        0x47
> +#define LMP92001_LIL3        0x48
> +#define LMP92001_LIL9        0x49
> +#define LMP92001_LIL10       0x4A
> +#define LMP92001_LIL11       0x4B
> +/* INTERNAL REFERENCE CONTROL */
> +#define LMP92001_CREF        0x66
> +/* DAC INPUT DATA */
> +#define LMP92001_DAC1        0x80
> +#define LMP92001_DAC2        0x81
> +#define LMP92001_DAC3        0x82
> +#define LMP92001_DAC4        0x83
> +#define LMP92001_DAC5        0x84
> +#define LMP92001_DAC6        0x85
> +#define LMP92001_DAC7        0x86
> +#define LMP92001_DAC8        0x87
> +#define LMP92001_DAC9        0x88
> +#define LMP92001_DAC10       0x89
> +#define LMP92001_DAC11       0x8A
> +#define LMP92001_DAC12       0x8B
> +#define LMP92001_DALL        0x90
> +/* MEMORY MAPPED BLOCK COMMANDS */
> +#define LMP92001_BLK0        0xF0
> +#define LMP92001_BLK1        0xF1
> +#define LMP92001_BLK2        0xF2
> +#define LMP92001_BLK3        0xF3
> +#define LMP92001_BLK4        0xF4
> +#define LMP92001_BLK5        0xF5
> +
> +/*
> + * Helpful macros
> + */
> +/* DAC */
> +#define CDAC_OFF     BIT(0)
> +#define CDAC_OLVL    BIT(1)
> +#define CDAC_GANG    BIT(2)
> +/* INT. REFERANCE ENABLE */
> +#define CREF_DEXT    BIT(0)
> +#define CREF_AEXT    BIT(1)
> +/* GENERAL */
> +#define CGEN_STRT    BIT(0)
> +#define CGEN_LCK     BIT(1)
> +#define CGEN_RST     BIT(7)
> +/* STATUS: GENERNAL */
> +#define SGEN_GPI     BIT(0)
> +
> +struct lmp92001 {
> +     struct device *dev;
> +     struct regmap *regmap;
> +
> +     struct mutex adc_lock;
> +     struct mutex dac_lock;
> +};
> +
> +extern struct regmap_config lmp92001_regmap_config;
> +
> +int lmp92001_device_init(struct lmp92001 *lmp92001, unsigned long id, int 
> irq);
> +void lmp92001_device_exit(struct lmp92001 *lmp92001);
> +
> +#endif /* __MFD_LMP92001_CORE_H__ */
> diff --git a/include/linux/mfd/lmp92001/debug.h 
> b/include/linux/mfd/lmp92001/debug.h
> new file mode 100644
> index 000000000000..e6fda7efc917
> --- /dev/null
> +++ b/include/linux/mfd/lmp92001/debug.h
> @@ -0,0 +1,27 @@
> +/*
> + * include/linux/mfd/lmp92001/debug.h - Debug interface for TI 92001
> + *
> + * Copyright 2016-2017 Celestica Ltd.
> + *
> + * Author: Abhisit Sangjan <s.abhi...@gmail.com>
> + *
> + * Inspired by wm831x driver.
> + *
> + * 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 __MFD_LMP92001_DEBUG_H__
> +#define __MFD_LMP92001_DEBUG_H__
> +
> +int lmp92001_debug_init(struct lmp92001 *lmp92001);
> +void lmp92001_debug_exit(struct lmp92001 *lmp92001);
> +
> +#endif /* __MFD_LMP92001_DEBUG_H__ */

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

Reply via email to