On Jan 15, 2017 14:27, Jonathan Cameron wrote:
> A few more bits an pieces.
>
> Thanks,
Thanks for the review!
> Jonathan
> > ---
> > +static int max5481_write_cmd(struct max5481_data *data, u8 cmd, u16 val)
> > +{
> > + struct spi_device *spi = data->spi;
> > +
> > + data->msg[0] = cmd;
> > +
> > + switch (cmd) {
> > + case MAX5481_WRITE_WIPER:
> > + data->msg[1] = val >> 2;
> > + data->msg[2] = (val & 0x3) << 6;
> > + return spi_write(spi, data->msg, ARRAY_SIZE(data->msg));
> array_size will give you the number of elements in the array. Here that
> is fine, but inconsistent with the use of sizeof(data->msg[0]) below.
Yes, you are right.
Do you think that plain 3 and 1 below will be OK in this case? This is the way
the protocol is defined.
Or maybe ARRAY_SIZE above is OK, but below I will just write 1?
> > +
> > + case MAX5481_COPY_AB_TO_NV:
> > + case MAX5481_COPY_NV_TO_AB:
> > + return spi_write(spi, data->msg, sizeof(data->msg[0]));
> > +
> > + default:
> > + return -EIO;
> > + }
> > +}
> > +
> > (...)
> > +static int max5481_probe(struct spi_device *spi)
> > +{
> > + struct iio_dev *indio_dev;
> > + struct max5481_data *data;
> > + const struct spi_device_id *id = spi_get_device_id(spi);
> > + int ret;
> > +
> > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
> > + if (!indio_dev)
> > + return -ENOMEM;
> > +
> > + dev_set_drvdata(&spi->dev, indio_dev);
> > + data = iio_priv(indio_dev);
> > +
> > + data->spi = spi;
> To use the of data you have below this will need different handling,
> specifically the use of of_match_device to get access to the data.
> See something like adc/max1363 for how to do this.
OK thanks!
> > + data->cfg = &max5481_cfg[id->driver_data];
> > +
> > + indio_dev->name = id->name;
> > + indio_dev->dev.parent = &spi->dev;
> > + indio_dev->modes = INDIO_DIRECT_MODE;
> > +
> > + /* variant specific configuration */
> > + indio_dev->info = &max5481_info;
> > + indio_dev->channels = max5481_channels;
> > + indio_dev->num_channels = ARRAY_SIZE(max5481_channels);
> > +
> > + /* restore wiper from NV */
> > + ret = max5481_write_cmd(data, MAX5481_COPY_NV_TO_AB, 0);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return iio_device_register(indio_dev);
> > +}
> > (...)
> > +MODULE_AUTHOR("Maury Anderson <maury.anderson@xxxxxxxxxxxxxxxxxxx>");
> > +MODULE_DESCRIPTION("max5481 SPI driver");
> > +MODULE_LICENSE("GPL v2");
> >
> 1
Do you mean:
MODULE_LICENSE("GPL");
?
--
Slawomir Stepien
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
