| Message ID | 25c5d22f6f0cbd1355eee2e9d9103c3ee71cebdc.1739967040.git.mazziesaccount@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Support ROHM BD79124 ADC | expand |
On 19/02/2025 14:31, Matti Vaittinen wrote: > The new devm_iio_adc_device_alloc_chaninfo() -helper is intended to help > drivers avoid open-coding the for_each_node -loop for getting the > channel IDs. The helper provides standard way to detect the ADC channel > nodes (by the node name), and a standard way to convert the "reg", > "diff-channels", "single-channel" and the "common-mode-channel" to > channel identification numbers used in the struct iio_chan_spec. > Furthermore, the helper checks the ID is in range of 0 ... num-channels. > > The original driver treated all found child nodes as channel nodes. The > new helper requires channel nodes to be named channel[@N]. This should > help avoid problems with devices which may contain also other but ADC > child nodes. Quick grep from arch/* with the rzg2l_adc's compatible > string didn't reveal any in-tree .dts with channel nodes named > othervice. Also, same grep shows all the .dts seem to have channel IDs > between 0..num of channels. > > Use the new helper. > > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> > > --- > Revision history: > v2 => v3: > - New patch > > I picked the rzg2l_adc in this series because it has a straightforward > approach for populating the struct iio_chan_spec. Only other member > in the stuct besides the .channel, which can't use a 'template' -data, > is the .datasheet_name. This makes the rzg2l_adc well suited for example > user of this new helper. I hope this patch helps to evaluate whether these > helpers are worth the hassle. > > The change is compile tested only!! Testing before applying is highly > appreciated (as always!). > --- > drivers/iio/adc/rzg2l_adc.c | 41 ++++++++++++++++++------------------- > 1 file changed, 20 insertions(+), 21 deletions(-) > > diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c > index cd3a7e46ea53..3e1c74019785 100644 > --- a/drivers/iio/adc/rzg2l_adc.c > +++ b/drivers/iio/adc/rzg2l_adc.c > @@ -11,6 +11,7 @@ ... > > +static const struct iio_chan_spec rzg2l_adc_chan_template = { > + .type = IIO_VOLTAGE, I just rebased this to v6.14-rc3 and noticed the channel type can no longer come from the template. There are also some other minor changes. I'll fix this in v4 if this same approach is kept. > + .indexed = 1, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > +}; > + Yours, -- Matti
Hi Matti, kernel test robot noticed the following build errors: [auto build test ERROR on 5bc55a333a2f7316b58edc7573e8e893f7acb532] url: https://github.com/intel-lab-lkp/linux/commits/Matti-Vaittinen/dt-bindings-ROHM-BD79124-ADC-GPO/20250219-203748 base: 5bc55a333a2f7316b58edc7573e8e893f7acb532 patch link: https://lore.kernel.org/r/25c5d22f6f0cbd1355eee2e9d9103c3ee71cebdc.1739967040.git.mazziesaccount%40gmail.com patch subject: [PATCH v3 6/9] iio: adc: rzg2l_adc: Use adc-helpers config: i386-buildonly-randconfig-005-20250220 (https://download.01.org/0day-ci/archive/20250220/202502202332.jrEGhoBM-lkp@intel.com/config) compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250220/202502202332.jrEGhoBM-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202502202332.jrEGhoBM-lkp@intel.com/ All errors (new ones prefixed by >>): >> ld.lld: error: undefined symbol: devm_iio_adc_device_alloc_chaninfo >>> referenced by rzg2l_adc.c:324 (drivers/iio/adc/rzg2l_adc.c:324) >>> drivers/iio/adc/rzg2l_adc.o:(rzg2l_adc_probe) in archive vmlinux.a
diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c index cd3a7e46ea53..3e1c74019785 100644 --- a/drivers/iio/adc/rzg2l_adc.c +++ b/drivers/iio/adc/rzg2l_adc.c @@ -11,6 +11,7 @@ #include <linux/clk.h> #include <linux/completion.h> #include <linux/delay.h> +#include <linux/iio/adc-helpers.h> #include <linux/iio/iio.h> #include <linux/interrupt.h> #include <linux/io.h> @@ -299,20 +300,34 @@ static irqreturn_t rzg2l_adc_isr(int irq, void *dev_id) return IRQ_HANDLED; } +static const struct iio_chan_spec rzg2l_adc_chan_template = { + .type = IIO_VOLTAGE, + .indexed = 1, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), +}; + +static const struct iio_adc_props rzg2l_adc_chan_props = { + .required = IIO_ADC_CHAN_PROP_TYPE_REG, +}; + static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l_adc *adc) { struct iio_chan_spec *chan_array; struct rzg2l_adc_data *data; - unsigned int channel; int num_channels; - int ret; u8 i; data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); if (!data) return -ENOMEM; - num_channels = device_get_child_node_count(&pdev->dev); + num_channels = devm_iio_adc_device_alloc_chaninfo(&pdev->dev, + &rzg2l_adc_chan_template, &chan_array, + &rzg2l_adc_chan_props); + + if (num_channels < 0) + return num_channels; + if (!num_channels) { dev_err(&pdev->dev, "no channel children\n"); return -ENODEV; @@ -323,26 +338,10 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l return -EINVAL; } - chan_array = devm_kcalloc(&pdev->dev, num_channels, sizeof(*chan_array), - GFP_KERNEL); - if (!chan_array) - return -ENOMEM; - - i = 0; - device_for_each_child_node_scoped(&pdev->dev, fwnode) { - ret = fwnode_property_read_u32(fwnode, "reg", &channel); - if (ret) - return ret; - - if (channel >= RZG2L_ADC_MAX_CHANNELS) - return -EINVAL; + for (i = 0; i < num_channels; i++) { + int channel = chan_array[i].channel; - chan_array[i].type = IIO_VOLTAGE; - chan_array[i].indexed = 1; - chan_array[i].channel = channel; - chan_array[i].info_mask_separate = BIT(IIO_CHAN_INFO_RAW); chan_array[i].datasheet_name = rzg2l_adc_channel_name[channel]; - i++; } data->num_channels = num_channels;
The new devm_iio_adc_device_alloc_chaninfo() -helper is intended to help drivers avoid open-coding the for_each_node -loop for getting the channel IDs. The helper provides standard way to detect the ADC channel nodes (by the node name), and a standard way to convert the "reg", "diff-channels", "single-channel" and the "common-mode-channel" to channel identification numbers used in the struct iio_chan_spec. Furthermore, the helper checks the ID is in range of 0 ... num-channels. The original driver treated all found child nodes as channel nodes. The new helper requires channel nodes to be named channel[@N]. This should help avoid problems with devices which may contain also other but ADC child nodes. Quick grep from arch/* with the rzg2l_adc's compatible string didn't reveal any in-tree .dts with channel nodes named othervice. Also, same grep shows all the .dts seem to have channel IDs between 0..num of channels. Use the new helper. Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> --- Revision history: v2 => v3: - New patch I picked the rzg2l_adc in this series because it has a straightforward approach for populating the struct iio_chan_spec. Only other member in the stuct besides the .channel, which can't use a 'template' -data, is the .datasheet_name. This makes the rzg2l_adc well suited for example user of this new helper. I hope this patch helps to evaluate whether these helpers are worth the hassle. The change is compile tested only!! Testing before applying is highly appreciated (as always!). --- drivers/iio/adc/rzg2l_adc.c | 41 ++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 21 deletions(-)