iio: potentiometer: add driver for Microchip MCP413X/414X/415X/416X/423X/424X/425X/426X

drivers/iio/potentiometer/mcp4131.c new file +14 -0

+static int mcp4131_read_raw(struct iio_dev *indio_dev,
+                           struct iio_chan_spec const *chan,
+                           int *val, int *val2, long mask)
+{
+       int err;
+       struct mcp4131_data *data = iio_priv(indio_dev);
+       int address = chan->channel;
+
+       switch (mask) {
+       case IIO_CHAN_INFO_RAW:
+               mutex_lock(&data->lock);
+
+               data->buf[0] = (address << MCP4131_WIPER_SHIFT) | MCP4131_READ;
+               data->buf[1] = 0;

drivers/iio/potentiometer/mcp4131.c new file +37 -0

+
+               *val = MCP4131_RAW(data->buf);
+               mutex_unlock(&data->lock);
+
+               return IIO_VAL_INT;
+
+       case IIO_CHAN_INFO_SCALE:
+               *val = 1000 * data->cfg->kohms;
+               *val2 = data->cfg->max_pos;
+               return IIO_VAL_FRACTIONAL;
+       }
+
+       return -EINVAL;
+}
+
+static int mcp4131_write_raw(struct iio_dev *indio_dev,
+                            struct iio_chan_spec const *chan,
+                            int val, int val2, long mask)
+{
+       int err;
+       struct mcp4131_data *data = iio_priv(indio_dev);
+       int address = chan->channel << MCP4131_WIPER_SHIFT;
+
+       switch (mask) {
+       case IIO_CHAN_INFO_RAW:
+               if (val > data->cfg->max_pos || val < 0)
+                       return -EINVAL;
+               break;
+
+       default:
+               return -EINVAL;
+       }
+
+       mutex_lock(&data->lock);
+
+       data->buf[0] = address << MCP4131_WIPER_SHIFT;
+       data->buf[0] |= MCP4131_WRITE | (val >> 8);

Hi Slawomir,

Quoted 7 lines · Slawomir Stepien

The following functionalities are supported:
- write, read from volatile memory

Datasheet: http://ww1.microchip.com/downloads/en/DeviceDoc/22060b.pdf

Signed-off-by Slawomir Stepien <[email protected]>

---

drivers/iio/potentiometer/mcp4131.c line 41

Quoted 5 lines

drivers/iio/potentiometer/mcp4131.c new file +5 -0

+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/err.h>
+
+#include <linux/iio/iio.h>

Give that you use that you have a some OF stuff in your driver you should also include <linux/of.h>. Same goes for <linux/mutex.h>. I am sure this builds fine without those includes, but you should explicitly include stuff that you use.

While you're at it you could also put the includes in alphabetic order.

Quoted 18 lines

drivers/iio/potentiometer/mcp4131.c new file +18 -0

+static int mcp4131_read_raw(struct iio_dev *indio_dev,
+                           struct iio_chan_spec const *chan,
+                           int *val, int *val2, long mask)
+{
+       int err;
+       struct mcp4131_data *data = iio_priv(indio_dev);
+       int address = chan->channel;
+
+       switch (mask) {
+       case IIO_CHAN_INFO_RAW:
+               mutex_lock(&data->lock);
+
+               data->buf[0] = (address << MCP4131_WIPER_SHIFT) | MCP4131_READ;
+               data->buf[1] = 0;
+
+               err = mcp4131_read(data->spi, data->buf, 2);
+               if (err) {
+                       mutex_unlock(&data->lock);

drivers/iio/potentiometer/mcp4131.c line 185

Quoted 6 lines

drivers/iio/potentiometer/mcp4131.c new file +6 -0

+                       return err;
+               }
+
+               /* Error, bad address/command combination */
+               if (!MCP4131_CMDERR(data->buf))
+                       return -EIO;

Missing mutex unlock here.

Quoted 37 lines

drivers/iio/potentiometer/mcp4131.c new file +37 -0

+
+               *val = MCP4131_RAW(data->buf);
+               mutex_unlock(&data->lock);
+
+               return IIO_VAL_INT;
+
+       case IIO_CHAN_INFO_SCALE:
+               *val = 1000 * data->cfg->kohms;
+               *val2 = data->cfg->max_pos;
+               return IIO_VAL_FRACTIONAL;
+       }
+
+       return -EINVAL;
+}
+
+static int mcp4131_write_raw(struct iio_dev *indio_dev,
+                            struct iio_chan_spec const *chan,
+                            int val, int val2, long mask)
+{
+       int err;
+       struct mcp4131_data *data = iio_priv(indio_dev);
+       int address = chan->channel << MCP4131_WIPER_SHIFT;
+
+       switch (mask) {
+       case IIO_CHAN_INFO_RAW:
+               if (val > data->cfg->max_pos || val < 0)
+                       return -EINVAL;
+               break;
+
+       default:
+               return -EINVAL;
+       }
+
+       mutex_lock(&data->lock);
+
+       data->buf[0] = address << MCP4131_WIPER_SHIFT;
+       data->buf[0] |= MCP4131_WRITE | (val >> 8);

drivers/iio/potentiometer/mcp4131.c line 232

Quoted 10 lines

drivers/iio/potentiometer/mcp4131.c new file +10 -0

+       data->buf[1] = val & 0xFF; /* 8 bits here */
+
+       err = spi_write(data->spi, data->buf, 2);
+       if (err) {
+               mutex_unlock(&data->lock);
+               return err;
+       }
+       mutex_unlock(&data->lock);
+
+       return 0;

This last part could be written as:
err = spi_write(data->spi, data->buf, 2);
mutex_unlock(&data->lock);

return err;

Other than the things I noted driver looks good to me.

regards,
Joachim Eastwood