| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <915a5d31-5eb4-54b9-3fc9-ff69836f3d76@topic.nl>
Date: Wed, 24 May 2023 13:05:12 +0200
From: Mike Looijmans <mike.looijmans@...ic.nl>
To: marius.cristea@...rochip.com, jic23@...nel.org, lars@...afoo.de,
robh+dt@...nel.org
CC: linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] iio: adc: adding support for MCP3564 ADC
On 19-05-2023 18:01, marius.cristea@...rochip.com wrote:
> From: Marius Cristea <marius.cristea@...rochip.com>
>
> This is the iio driver for Microchip family of 153.6 ksps,
> Low-Noise 16/24-Bit Delta-Sigma ADCs with an SPI interface.
Oh nice.. I tried to submit a similar driver just yesterday, I'll comment on
yours instead...
>
> Signed-off-by: Marius Cristea <marius.cristea@...rochip.com>
> ---
...
> +
> +static inline u8 mcp356x_reg_write(u8 chip_addr, u8 reg)
> +{
> + return ((chip_addr << 6) | (reg << 2) | MCP356X_WRT_CTRL);
> +}
> +
> +static inline u8 mcp356x_reg_read(u8 chip_addr, u8 reg)
> +{
> + return ((chip_addr << 6) | (reg << 2) | MCP356X_RD_CTRL);
> +}
How about using FIELD_PREP() here?
> +
> +static inline u8 mcp356x_reg_fast_cmd(u8 chip_addr, u8 cmd)
> +{
> + return ((chip_addr << 6) | (cmd << 2));
> +}
> +
> +static int mcp356x_read(struct mcp356x_state *adc, u8 reg, u32 *val, u8 len)
> +{
> + int ret;
> + u8 tmp_reg;
> +
> + tmp_reg = mcp356x_reg_read(adc->dev_addr, reg);
> +
> + ret = spi_write_then_read(adc->spi, &tmp_reg, 1, val, len);
I had issues with spi_write_then_read not communicating properly with the
device. This may be due to the SPI controller (Xilinx IP in FPGA) though.
I had to use spi_sync() to get reliable answers. Also has the added benefit of
giving access to the interrupt flags.
> +
> + be32_to_cpus(val);
> + *val >>= ((4 - len) * 8);
> +
> + return ret;
> +}
> +
> +static int mcp356x_write(struct mcp356x_state *adc, u8 reg, u32 val, u8 len)
> +{
> + val |= (mcp356x_reg_write(adc->dev_addr, reg) << (len * 8));
> + val <<= (3 - len) * 8;
> + cpu_to_be32s(&val);
> +
> + return spi_write(adc->spi, &val, len + 1);
> +}
> +
> +static int mcp356x_fast_cmd(struct mcp356x_state *adc, u8 fast_cmd)
> +{
> + u8 val;
> +
> + val = mcp356x_reg_fast_cmd(adc->dev_addr, fast_cmd);
> +
> + return spi_write(adc->spi, &val, 1);
> +}
> +
> +static int mcp356x_update(struct mcp356x_state *adc, u8 reg, u32 mask, u32 val,
> + u8 len)
> +{
> + u32 tmp;
> + int ret;
> +
> + ret = mcp356x_read(adc, reg, &tmp, len);
> +
> + if (ret == 0) {
> + val &= mask;
> + val |= tmp & ~mask;
> + ret = mcp356x_write(adc, reg, val, len);
> + }
> +
> + return ret;
> +}
This update function is used a lot, and is just a re-implementation of
"regmap" functionality. Since this driver only reads/writes single registers,
I would recommend using regmap with the read/write functions above. That would
also reduce the SPI traffic as regmap can cache the values for you.
...
> +
> +#define MCP3564_CHANNELS(depth) { \
> + MCP356X_V_CHANNEL(0, 0, depth), \
> + MCP356X_V_CHANNEL(1, 1, depth), \
> + MCP356X_V_CHANNEL(2, 2, depth), \
> + MCP356X_V_CHANNEL(3, 3, depth), \
> + MCP356X_V_CHANNEL(4, 4, depth), \
> + MCP356X_V_CHANNEL(5, 5, depth), \
> + MCP356X_V_CHANNEL(6, 6, depth), \
> + MCP356X_V_CHANNEL(7, 7, depth), \
> + MCP356X_T_CHAN(depth), \
> + MCP356X_V_CHANNEL_DIFF(0, 1, 0x01, depth), \
> + MCP356X_V_CHANNEL_DIFF(1, 0, 0x10, depth), \
> + MCP356X_V_CHANNEL_DIFF(0, 2, 0x02, depth), \
> + MCP356X_V_CHANNEL_DIFF(0, 3, 0x03, depth), \
> + MCP356X_V_CHANNEL_DIFF(1, 2, 0x12, depth), \
> + MCP356X_V_CHANNEL_DIFF(1, 3, 0x13, depth), \
> + MCP356X_V_CHANNEL_DIFF(2, 3, 0x23, depth), \
> + MCP356X_V_CHANNEL_DIFF(2, 0, 0x20, depth), \
> + MCP356X_V_CHANNEL_DIFF(3, 0, 0x30, depth), \
> + MCP356X_V_CHANNEL_DIFF(2, 1, 0x21, depth), \
> + MCP356X_V_CHANNEL_DIFF(3, 1, 0x31, depth), \
> + MCP356X_V_CHANNEL_DIFF(3, 2, 0x32, depth), \
> + MCP356X_V_CHANNEL_DIFF(0, 4, 0x04, depth), \
> + MCP356X_V_CHANNEL_DIFF(0, 5, 0x05, depth), \
> + MCP356X_V_CHANNEL_DIFF(0, 6, 0x06, depth), \
> + MCP356X_V_CHANNEL_DIFF(0, 7, 0x07, depth), \
> + MCP356X_V_CHANNEL_DIFF(1, 4, 0x14, depth), \
> + MCP356X_V_CHANNEL_DIFF(1, 5, 0x15, depth), \
> + MCP356X_V_CHANNEL_DIFF(1, 6, 0x16, depth), \
> + MCP356X_V_CHANNEL_DIFF(1, 7, 0x17, depth), \
> + MCP356X_V_CHANNEL_DIFF(2, 4, 0x24, depth), \
> + MCP356X_V_CHANNEL_DIFF(2, 5, 0x25, depth), \
> + MCP356X_V_CHANNEL_DIFF(2, 6, 0x26, depth), \
> + MCP356X_V_CHANNEL_DIFF(2, 7, 0x27, depth), \
> + MCP356X_V_CHANNEL_DIFF(3, 4, 0x34, depth), \
> + MCP356X_V_CHANNEL_DIFF(3, 5, 0x35, depth), \
> + MCP356X_V_CHANNEL_DIFF(3, 6, 0x36, depth), \
> + MCP356X_V_CHANNEL_DIFF(3, 7, 0x37, depth), \
> + MCP356X_V_CHANNEL_DIFF(4, 5, 0x45, depth), \
> + MCP356X_V_CHANNEL_DIFF(4, 6, 0x46, depth), \
> + MCP356X_V_CHANNEL_DIFF(4, 7, 0x47, depth), \
> + MCP356X_V_CHANNEL_DIFF(5, 6, 0x56, depth), \
> + MCP356X_V_CHANNEL_DIFF(5, 7, 0x57, depth), \
> + MCP356X_V_CHANNEL_DIFF(6, 7, 0x67, depth), \
> + MCP356X_V_CHANNEL_DIFF(4, 0, 0x40, depth), \
> + MCP356X_V_CHANNEL_DIFF(5, 0, 0x50, depth), \
> + MCP356X_V_CHANNEL_DIFF(6, 0, 0x60, depth), \
> + MCP356X_V_CHANNEL_DIFF(7, 0, 0x70, depth), \
> + MCP356X_V_CHANNEL_DIFF(4, 1, 0x41, depth), \
> + MCP356X_V_CHANNEL_DIFF(5, 1, 0x51, depth), \
> + MCP356X_V_CHANNEL_DIFF(6, 1, 0x61, depth), \
> + MCP356X_V_CHANNEL_DIFF(7, 1, 0x71, depth), \
> + MCP356X_V_CHANNEL_DIFF(4, 2, 0x42, depth), \
> + MCP356X_V_CHANNEL_DIFF(5, 2, 0x52, depth), \
> + MCP356X_V_CHANNEL_DIFF(6, 2, 0x62, depth), \
> + MCP356X_V_CHANNEL_DIFF(7, 2, 0x72, depth), \
> + MCP356X_V_CHANNEL_DIFF(4, 3, 0x43, depth), \
> + MCP356X_V_CHANNEL_DIFF(5, 3, 0x53, depth), \
> + MCP356X_V_CHANNEL_DIFF(6, 3, 0x63, depth), \
> + MCP356X_V_CHANNEL_DIFF(7, 3, 0x73, depth), \
> + MCP356X_V_CHANNEL_DIFF(5, 4, 0x54, depth), \
> + MCP356X_V_CHANNEL_DIFF(6, 4, 0x64, depth), \
> + MCP356X_V_CHANNEL_DIFF(7, 4, 0x74, depth), \
> + MCP356X_V_CHANNEL_DIFF(6, 5, 0x65, depth), \
> + MCP356X_V_CHANNEL_DIFF(7, 5, 0x75, depth), \
> + MCP356X_V_CHANNEL_DIFF(7, 6, 0x76, depth) \
> +}
Nice that the chip can do full 8x8 mux delivering 56 channels, but I don't
think that will be useful to many people.
Also I'd want to see buffer support added to the device, which only works for
the channels as in the table in the datasheet, so only 4 differential ones.
It'd be annoying to have 56 channels but only be able to contiuously read 4
specific ones.
...
> +
> +static int mcp356x_read_single_value(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *channel,
> + int *val)
> +{
> + struct mcp356x_state *adc = iio_priv(indio_dev);
> + int ret, tmp, ret_read = 0;
> +
> + /* Configure MUX register with the requested channel */
> + ret = mcp356x_write(adc, MCP356X_MUX, channel->address, 1);
> + if (ret) {
> + dev_err(&indio_dev->dev, "Failed to configure MUX register\n");
> + return ret;
> + }
> +
> + /* ADC Conversion starts by writing ADC_MODE[1:0] = 11 to CONFIG0[1:0] = */
> + ret = mcp356x_update(adc, MCP356X_CONFIG0, MCP356X_ADC_MODE_MASK,
> + MCP356X_ADC_CONVERSION_MODE, 1);
> + if (ret) {
> + dev_err(&indio_dev->dev,
> + "Failed to configure CONFIG0 register\n");
> + return ret;
> + }
> +
> + /*
> + * Check if the conversion is ready. If not, wait a little bit, and
> + * in case of timeout exit with an error.
> + */
> +
> + ret = read_poll_timeout(mcp356x_read, ret_read,
> + ret_read || !(tmp & MCP356X_DATA_READY_MASK),
> + 1000, MCP356X_DATA_READY_TIMEOUT_MS * 1000, true,
> + adc, MCP356X_IRQ, &tmp, 1);
I have an implementation that (optionally) uses the interrupt signal from the
chip, nicer than polling.
> +
> + /* failed to read status register */
> + if (ret_read)
> + return ret;
> +
> + if (ret)
> + return -ETIMEDOUT;
> +
> + if (tmp & MCP356X_DATA_READY_MASK)
> + /* failing to finish conversion */
> + return -EBUSY;
> +
> + ret = mcp356x_read(adc, MCP356X_ADCDATA, &tmp, 4);
> + if (ret)
> + return ret;
> +
> + *val = tmp;
> +
> + return ret;
> +}
> +
> +static int mcp356x_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *channel,
> + const int **vals, int *type,
> + int *length, long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + *type = IIO_VAL_INT;
> + *vals = mcp356x_oversampling_avail;
> + *length = ARRAY_SIZE(mcp356x_oversampling_avail);
> + return IIO_AVAIL_LIST;
> + case IIO_CHAN_INFO_HARDWAREGAIN:
Hmm, last time I tried to submit a driver with HARDWAREGAIN I got told to
change it to selectable SCALE instead. See the driver I submitted yesterday
for details.
> + *type = IIO_VAL_FRACTIONAL;
> + *length = ARRAY_SIZE(mcp356x_hwgain_frac);
> + *vals = mcp356x_hwgain_frac;
> + return IIO_AVAIL_LIST;
> + case IIO_CHAN_INFO_CALIBBIAS:
> + *vals = mcp356x_calib_bias;
> + *type = IIO_VAL_INT;
> + return IIO_AVAIL_RANGE;
> + case IIO_CHAN_INFO_CALIBSCALE:
> + *vals = mcp356x_calib_scale;
> + *type = IIO_VAL_INT;
> + return IIO_AVAIL_RANGE;
> + default:
> + return -EINVAL;
> + }
> +}
...
> +
> +static int mcp356x_config(struct mcp356x_state *adc)
> +{
> + int ret = 0;
> + unsigned int tmp;
> +
> + dev_dbg(&adc->spi->dev, "%s: Start config...\n", __func__);
> +
> + /*
> + * The address is set on a per-device basis by fuses in the factory,
> + * configured on request. If not requested, the fuses are set for 0x1.
> + * The device address is part of the device markings to avoid
> + * potential confusion. This address is coded on two bits, so four possible
> + * addresses are available when multiple devices are present on the same
> + * SPI bus with only one Chip Select line for all devices.
> + */
> + device_property_read_u32(&adc->spi->dev, "microchip,hw-device-address", &tmp);
>
tmp may be used uninitialized here. Initialize to "1" to get a proper default
value. That also makes the microchip,hw-device-address optional instead of
mandatory (see my dt-bindings comment).
> + if (tmp > 3) {
> + dev_err_probe(&adc->spi->dev, tmp,
> + "invalid device address. Must be in range 0-3.\n");
> + return -EINVAL;
> + }
> +
> + adc->dev_addr = 0xff & tmp;
> +
> + dev_dbg(&adc->spi->dev, "use device address %i\n", adc->dev_addr);
> +
> + ret = mcp356x_read(adc, MCP356X_RESERVED_E, &tmp, 2);
> +
> + if (ret == 0) {
> + switch (tmp & MCP356X_HW_ID_MASK) {
> + case MCP3461_HW_ID:
> + dev_dbg(&adc->spi->dev, "Found MCP3461 chip\n");
This will never get output unless the driver is compiled in debug mode. I'd
have expected this ID to be used to initialize things?
> + break;
> + case MCP3462_HW_ID:
> + dev_dbg(&adc->spi->dev, "Found MCP3462 chip\n");
> + break;
> + case MCP3464_HW_ID:
> + dev_dbg(&adc->spi->dev, "Found MCP3464 chip\n");
> + break;
> + case MCP3561_HW_ID:
> + dev_dbg(&adc->spi->dev, "Found MCP3561 chip\n");
> + break;
> + case MCP3562_HW_ID:
> + dev_dbg(&adc->spi->dev, "Found MCP3562 chip\n");
> + break;
> + case MCP3564_HW_ID:
> + dev_dbg(&adc->spi->dev, "Found MCP3564 chip\n");
> + break;
> + default:
> + dev_err_probe(&adc->spi->dev, tmp,
> + "Unknown chip found\n");
> + return -EINVAL;
> + }
> + } else {
> + return ret;
> + }
> +
> + /* Command sequence that ensures a recovery with
> + * the desired settings in any cases of loss-of-power scenario.
> + */
> +
> + /* Write LOCK register to 0xA5 (Write Access Password)
> + * Write access is allowed on the full register map.
> + */
> + ret = mcp356x_write(adc, MCP356X_LOCK, 0x000000A5, 1);
> + if (ret)
> + return ret;
> +
> + /* Write IRQ register to 0x03 */
> + /* IRQ --> IRQ Mode = Hi-Z IRQ Output --> (0b00000011).
> + * IRQ = 0x00000003
> + */
> + ret = mcp356x_write(adc, MCP356X_IRQ, 0x00000003, 1);
> + if (ret)
> + return ret;
> +
> + /* Device Full Reset Fast Command */
> + ret = mcp356x_fast_cmd(adc, MCP356X_FULL_RESET_CMD);
> +
> + /* wait 1ms for the chip to restart after a full reset */
> + mdelay(1);
> +
> + /* Reconfigure the ADC chip */
> +
> + /* GAINCAL --> Disabled.
> + * Default value is GAINCAL = 0x00800000; which provides a gain of 1x
> + */
> + ret = mcp356x_write(adc, MCP356X_GAINCAL, 0x00800000, 3);
> + if (ret)
> + return ret;
> +
> + adc->calib_scale = 0x00800000;
> +
> + /* OFFSETCAL --> 0 Counts of Offset Cancellation
> + * (Measured offset is negative).
> + * OFFSETCAL = 0x0
> + */
> + ret = mcp356x_write(adc, MCP356X_OFFSETCAL, 0x00000000, 3);
> + if (ret)
> + return ret;
> +
> + /* TIMER --> Disabled.
> + * TIMER = 0x00000000
> + */
> + ret = mcp356x_write(adc, MCP356X_TIMER, 0x00000000, 3);
> + if (ret)
> + return ret;
> +
> + /* SCAN --> Disabled.
> + * SCAN = 0x00000000
> + */
> + ret = mcp356x_write(adc, MCP356X_SCAN, 0x00000000, 3);
> + if (ret)
> + return ret;
> +
> + /* MUX --> VIN+ = CH0, VIN- = CH1 --> (0b00000001).
> + * MUX = 0x00000001
> + */
> + ret = mcp356x_write(adc, MCP356X_MUX, 0x00000001, 1);
> + if (ret)
> + return ret;
> +
> + /* IRQ --> IRQ Mode = Hi-Z IRQ Output --> (0b00000011).
> + * IRQ = 0x00000003
> + */
> + ret = mcp356x_write(adc, MCP356X_IRQ, 0x00000003, 1);
> + if (ret)
> + return ret;
> +
> + /* CONFIG3
> + * Conv. Mod = One-Shot/Standby,
> + * FORMAT = 32-bit (right justified data): SGN extension + ADC data,
> + * CRC_FORMAT = 16b, CRC-COM = Disabled,
> + * OFFSETCAL = Enabled, GAINCAL = Enabled --> (10100011).
> + * CONFIG3 = 0x000000A3
> + *
> + */
> + ret = mcp356x_write(adc, MCP356X_CONFIG3, 0x000000A3, 1);
> + if (ret)
> + return ret;
> +
> + /* CONFIG2 --> BOOST = 1x, GAIN = 1x, AZ_MUX = 1 --> (0b10001101).
> + * CONFIG2 = 0x0000008D
> + */
> + ret = mcp356x_write(adc, MCP356X_CONFIG2, 0x0000008D, 1);
> + if (ret)
> + return ret;
> +
> + adc->hwgain = 0x01;
> + adc->auto_zeroing_mux = true;
> + adc->auto_zeroing_ref = false;
> + adc->current_boost_mode = MCP356X_BOOST_CURRENT_x1_00;
> +
> + /* CONFIG1 --> AMCLK = MCLK, OSR = 98304 --> (0b00111100).
> + * CONFIG1 = 0x0000003C
> + */
> + ret = mcp356x_write(adc, MCP356X_CONFIG1, 0x0000003C, 1);
> + if (ret)
> + return ret;
> +
> + adc->oversampling = 0x0F;
> +
> + if (!adc->vref) {
> + /* CONFIG0 --> VREF_SEL = Internal Voltage Reference 2.4v
> + * CLK_SEL = INTOSC w/o CLKOUT, CS_SEL = No Bias,
> + * ADC_MODE = Standby Mode --> (0b11100010).
> + * CONFIG0 = 0x000000E2
> + */
> + ret = mcp356x_write(adc, MCP356X_CONFIG0, 0x000000E2, 1);
> +
> + dev_dbg(&adc->spi->dev, "%s: Using internal Vref\n",
> + __func__);
> + adc->vref_mv = MCP356XR_INT_VREF_MV;
> +
> + } else {
> + /* CONFIG0 --> CLK_SEL = INTOSC w/o CLKOUT, CS_SEL = No Bias,
> + * ADC_MODE = Standby Mode --> (0b01100010).
> + * CONFIG0 = 0x000000E2
> + */
> + ret = mcp356x_write(adc, MCP356X_CONFIG0, 0x00000062, 1);
> + }
> + adc->current_bias_mode = MCP356X_CS_SEL_0_0_uA;
> +
> + return ret;
> +}
> +
> +static int mcp356x_probe(struct spi_device *spi)
> +{
> + int ret, device_index;
> + struct iio_dev *indio_dev;
> + struct mcp356x_state *adc;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
> + if (!indio_dev) {
> + dev_err_probe(&indio_dev->dev, PTR_ERR(indio_dev),
> + "Can't allocate iio device\n");
> + return -ENOMEM;
> + }
> +
> + adc = iio_priv(indio_dev);
> + adc->spi = spi;
> +
> + dev_dbg(&adc->spi->dev, "%s: probe(spi = 0x%p)\n", __func__, spi);
> +
> + adc->vref = devm_regulator_get_optional(&adc->spi->dev, "vref");
> + if (IS_ERR(adc->vref)) {
> + if (PTR_ERR(adc->vref) == -ENODEV) {
> + adc->vref = NULL;
> + dev_dbg(&adc->spi->dev, "%s: Using internal Vref\n",
> + __func__);
> + } else {
> + dev_err_probe(&adc->spi->dev, PTR_ERR(adc->vref),
> + "failed to get regulator\n");
> + return PTR_ERR(adc->vref);
You can "return dev_err_probe(...);" directly and save a line of code
> + }
> + } else {
> + ret = regulator_enable(adc->vref);
> + if (ret)
> + return ret;
> +
Use devm_add_action_or_reset to register a disable function. That gets rid of
the gotos and the remove() callback as well. See my version of the driver for
implementation example.
> + dev_dbg(&adc->spi->dev, "%s: Using External Vref\n",
> + __func__);
> +
> + ret = regulator_get_voltage(adc->vref);
> + if (ret < 0) {
> + dev_err_probe(&adc->spi->dev, ret,
> + "Failed to read vref regulator\n");
> + goto error_disable_reg;
> + }
> +
> + adc->vref_mv = ret / 1000;
> + }
> +
> + spi_set_drvdata(spi, indio_dev);
> + device_index = spi_get_device_id(spi)->driver_data;
> + adc->chip_info = &mcp356x_chip_infos_tbl[device_index];
> +
> + adc->mcp356x_info.read_raw = mcp356x_read_raw;
> + adc->mcp356x_info.write_raw = mcp356x_write_raw;
> + adc->mcp356x_info.read_avail = mcp356x_read_avail;
> +
> + ret = mcp356x_prep_custom_attributes(adc, indio_dev);
> + if (ret) {
> + dev_err_probe(&adc->spi->dev, ret,
> + "Can't configure custom attributes for MCP356X device\n");
> + goto error_disable_reg;
> + }
> +
> + indio_dev->name = spi_get_device_id(spi)->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &adc->mcp356x_info;
> +
> + indio_dev->channels = adc->chip_info->channels;
> + indio_dev->num_channels = adc->chip_info->num_channels;
> + indio_dev->masklength = adc->chip_info->num_channels - 1;
> +
> + /* initialize the chip access mutex */
> + mutex_init(&adc->lock);
> +
> + /* Do any chip specific initialization, e.g:
> + * read/write some registers
> + * enable/disable certain channels
> + * change the sampling rate to the requested value
> + */
> + ret = mcp356x_config(adc);
> + if (ret) {
> + dev_err_probe(&adc->spi->dev, ret,
> + "Can't configure MCP356X device\n");
> + goto error_disable_reg;
> + }
> +
> + dev_dbg(&adc->spi->dev, "%s: Vref (mV): %d\n", __func__, adc->vref_mv);
> +
> + ret = devm_iio_device_register(&spi->dev, indio_dev);
> + if (ret) {
> + dev_err_probe(&adc->spi->dev, ret,
> + "Can't register IIO device\n");
> + goto error_disable_reg;
> + }
> +
> + return 0;
> +
> +error_disable_reg:
> + if (adc->vref)
> + regulator_disable(adc->vref);
> +
> + return ret;
> +}
> +
> +static void mcp356x_remove(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
> + struct mcp356x_state *adc = iio_priv(indio_dev);
> +
> + if (adc->vref)
> + regulator_disable(adc->vref);
> +}
> +
> +static const struct of_device_id mcp356x_dt_ids[] = {
> + { .compatible = "microchip,mcp3461" },
> + { .compatible = "microchip,mcp3462" },
> + { .compatible = "microchip,mcp3464" },
> + { .compatible = "microchip,mcp3461r" },
> + { .compatible = "microchip,mcp3462r" },
> + { .compatible = "microchip,mcp3464r" },
> + { .compatible = "microchip,mcp3561" },
> + { .compatible = "microchip,mcp3562" },
> + { .compatible = "microchip,mcp3564" },
> + { .compatible = "microchip,mcp3561r" },
> + { .compatible = "microchip,mcp3562r" },
> + { .compatible = "microchip,mcp3564r" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, mcp356x_dt_ids);
> +
> +static const struct spi_device_id mcp356x_id[] = {
> + { "mcp3461", mcp3461 },
> + { "mcp3462", mcp3462 },
> + { "mcp3464", mcp3464 },
> + { "mcp3461r", mcp3461r },
> + { "mcp3462r", mcp3462r },
> + { "mcp3464r", mcp3464r },
> + { "mcp3561", mcp3561 },
> + { "mcp3562", mcp3562 },
> + { "mcp3564", mcp3564 },
> + { "mcp3561r", mcp3561r },
> + { "mcp3562r", mcp3562r },
> + { "mcp3564r", mcp3564r },
> + { }
> +};
> +MODULE_DEVICE_TABLE(spi, mcp356x_id);
> +
> +static struct spi_driver mcp356x_driver = {
> + .driver = {
> + .name = "mcp3564",
> + .of_match_table = mcp356x_dt_ids,
> + },
> + .probe = mcp356x_probe,
> + .remove = mcp356x_remove,
> + .id_table = mcp356x_id,
> +};
> +
> +module_spi_driver(mcp356x_driver);
> +
> +MODULE_AUTHOR("Marius Cristea <marius.cristea@...rochip.com>");
> +MODULE_DESCRIPTION("Microchip MCP346x/MCP346xR and MCP356x/MCP346xR ADCs");
> +MODULE_LICENSE("GPL v2");
--
Mike Looijmans
System Expert
TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands
T: +31 (0) 499 33 69 69
E: mike.looijmans@...ic.nl
W: www.topic.nl
Powered by blists - more mailing lists