[PATCH v6 09/10] iio: adc: ad7124: Add error reporting during probe

Uwe Kleine-König
2024-12-06T17:29:29Z

A driver that silently fails to probe is annoying and hard to debug. So
add messages in the error paths of the probe function.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
drivers/iio/adc/ad7124.c | 76 +++++++++++++++++++++-------------------
1 file changed, 40 insertions(+), 36 deletions(-)

diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index b17c3dbeaeba..9405cb579324 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -360,20 +360,21 @@ static int ad7124_find_free_config_slot(struct ad7124_state *st)
return free_cfg_slot;
}

+/* Only called during probe, so dev_err_probe() can be used */
static int ad7124_init_config_vref(struct ad7124_state *st, struct ad7124_channel_config *cfg)
{
+ struct device *dev = &st->sd.spi->dev;
unsigned int refsel = cfg->refsel;

switch (refsel) {
case AD7124_REFIN1:
case AD7124_REFIN2:
case AD7124_AVDD_REF:
- if (IS_ERR(st->vref[refsel])) {
- dev_err(&st->sd.spi->dev,
- "Error, trying to use external voltage reference without a %s regulator.\n",
- ad7124_ref_names[refsel]);
- return PTR_ERR(st->vref[refsel]);
- }
+ if (IS_ERR(st->vref[refsel]))
+ return dev_err_probe(dev, PTR_ERR(st->vref[refsel]),
+ "Error, trying to use external voltage reference without a %s regulator.\n",
+ ad7124_ref_names[refsel]);
+
cfg->vref_mv = regulator_get_voltage(st->vref[refsel]);
/* Conversion from uV to mV */
cfg->vref_mv /= 1000;
@@ -384,8 +385,7 @@ static int ad7124_init_config_vref(struct ad7124_state *st, struct ad7124_channe
st->adc_control |= AD7124_ADC_CTRL_REF_EN(1);
return 0;
default:
- dev_err(&st->sd.spi->dev, "Invalid reference %d\n", refsel);
- return -EINVAL;
+ return dev_err_probe(dev, -EINVAL, "Invalid reference %d\n", refsel);
}
}

@@ -752,8 +752,10 @@ static const struct iio_info ad7124_info = {
.attrs = &ad7124_attrs_group,
};

+/* Only called during probe, so dev_err_probe() can be used */
static int ad7124_soft_reset(struct ad7124_state *st)
{
+ struct device *dev = &st->sd.spi->dev;
unsigned int readval, timeout;
int ret;

@@ -766,7 +768,7 @@ static int ad7124_soft_reset(struct ad7124_state *st)
do {
ret = ad_sd_read_reg(&st->sd, AD7124_STATUS, 1, &readval);
if (ret < 0)
- return ret;
+ return dev_err_probe(dev, ret, "Error reading status register\n");

if (!(readval & AD7124_STATUS_POR_FLAG_MSK))
return 0;
@@ -775,35 +777,30 @@ static int ad7124_soft_reset(struct ad7124_state *st)
usleep_range(100, 2000);
} while (--timeout);

- dev_err(&st->sd.spi->dev, "Soft reset failed\n");
-
- return -EIO;
+ return dev_err_probe(dev, -EIO, "Soft reset failed\n");
}

static int ad7124_check_chip_id(struct ad7124_state *st)
{
+ struct device *dev = &st->sd.spi->dev;
unsigned int readval, chip_id, silicon_rev;
int ret;

ret = ad_sd_read_reg(&st->sd, AD7124_ID, 1, &readval);
if (ret < 0)
- return ret;
+ return dev_err_probe(dev, ret, "Failure to read ID register\n");

chip_id = AD7124_DEVICE_ID_GET(readval);
silicon_rev = AD7124_SILICON_REV_GET(readval);

- if (chip_id != st->chip_info->chip_id) {
- dev_err(&st->sd.spi->dev,
- "Chip ID mismatch: expected %u, got %u\n",
- st->chip_info->chip_id, chip_id);
- return -ENODEV;
- }
+ if (chip_id != st->chip_info->chip_id)
+ return dev_err_probe(dev, -ENODEV,
+ "Chip ID mismatch: expected %u, got %u\n",
+ st->chip_info->chip_id, chip_id);

- if (silicon_rev == 0) {
- dev_err(&st->sd.spi->dev,
- "Silicon revision empty. Chip may not be present\n");
- return -ENODEV;
- }
+ if (silicon_rev == 0)
+ return dev_err_probe(dev, -ENODEV,
+ "Silicon revision empty. Chip may not be present\n");

return 0;
}
@@ -862,16 +859,18 @@ static int ad7124_parse_channel_config(struct iio_dev *indio_dev,
device_for_each_child_node_scoped(dev, child) {
ret = fwnode_property_read_u32(child, "reg", &channel);
if (ret)
- return ret;
+ return dev_err_probe(dev, ret,
+ "Failed to parse reg property of %pfwP\n", child);

if (channel >= indio_dev->num_channels)
return dev_err_probe(dev, -EINVAL,
- "Channel index >= number of channels\n");
+ "Channel index >= number of channels in %pfwP\n", child);

ret = fwnode_property_read_u32_array(child, "diff-channels",
ain, 2);
if (ret)
- return ret;
+ return dev_err_probe(dev, ret,
+ "Failed to parse diff-channels property of %pfwP\n", child);

if (!ad7124_valid_input_select(ain[0], st->chip_info) ||
!ad7124_valid_input_select(ain[1], st->chip_info))
@@ -908,12 +907,13 @@ static int ad7124_parse_channel_config(struct iio_dev *indio_dev,

static int ad7124_setup(struct ad7124_state *st)
{
+ struct device *dev = &st->sd.spi->dev;
unsigned int fclk, power_mode;
int i, ret;

fclk = clk_get_rate(st->mclk);
if (!fclk)
- return -EINVAL;
+ return dev_err_probe(dev, -EINVAL, "Failed to get mclk rate\n");

/* The power mode changes the master clock frequency */
power_mode = ad7124_find_closest_match(ad7124_master_clk_freq_hz,
@@ -922,7 +922,7 @@ static int ad7124_setup(struct ad7124_state *st)
if (fclk != ad7124_master_clk_freq_hz[power_mode]) {
ret = clk_set_rate(st->mclk, fclk);
if (ret)
- return ret;
+ return dev_err_probe(dev, ret, "Failed to set mclk rate\n");
}

/* Set the power mode */
@@ -953,7 +953,7 @@ static int ad7124_setup(struct ad7124_state *st)

ret = ad_sd_write_reg(&st->sd, AD7124_ADC_CONTROL, 2, st->adc_control);
if (ret < 0)
- return ret;
+ return dev_err_probe(dev, ret, "Failed to setup CONTROL register\n");

return ret;
}
@@ -966,13 +966,14 @@ static void ad7124_reg_disable(void *r)
static int ad7124_probe(struct spi_device *spi)
{
const struct ad7124_chip_info *info;
+ struct device *dev = &spi->dev;
struct ad7124_state *st;
struct iio_dev *indio_dev;
int i, ret;

info = spi_get_device_match_data(spi);
if (!info)
- return -ENODEV;
+ return dev_err_probe(dev, -ENODEV, "Failed to get match data\n");

indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
if (!indio_dev)
@@ -1007,17 +1008,17 @@ static int ad7124_probe(struct spi_device *spi)

ret = regulator_enable(st->vref[i]);
if (ret)
- return ret;
+ return dev_err_probe(dev, ret, "Failed to enable regulator #%d\n", i);

ret = devm_add_action_or_reset(&spi->dev, ad7124_reg_disable,
st->vref[i]);
if (ret)
- return ret;
+ return dev_err_probe(dev, ret, "Failed to register disable handler for regulator #%d\n", i);
}

st->mclk = devm_clk_get_enabled(&spi->dev, "mclk");
if (IS_ERR(st->mclk))
- return PTR_ERR(st->mclk);
+ return dev_err_probe(dev, PTR_ERR(st->mclk), "Failed to get mclk\n");

ret = ad7124_soft_reset(st);
if (ret < 0)
@@ -1033,10 +1034,13 @@ static int ad7124_probe(struct spi_device *spi)

ret = devm_ad_sd_setup_buffer_and_trigger(&spi->dev, indio_dev);
if (ret < 0)
- return ret;
+ return dev_err_probe(dev, ret, "Failed to setup triggers\n");

- return devm_iio_device_register(&spi->dev, indio_dev);
+ ret = devm_iio_device_register(&spi->dev, indio_dev);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Failed to register iio device\n");

+ return 0;
}

static const struct of_device_id ad7124_of_match[] = {
--
2.45.2