Re: [PATCH v2 6/7] iio: adc: qcom-spmi-iadc: Migrate to devm_spmi_subdevice_alloc_and_add()

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Tue, 22 Jul 2025 12:13:16 +0200
AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote:

> Some Qualcomm PMICs integrate an Current ADC device, reachable
> in a specific address range over SPMI.
> 
> Instead of using the parent SPMI device (the main PMIC) as a kind
> of syscon in this driver, register a new SPMI sub-device and
> initialize its own regmap with this sub-device's specific base
> address, retrieved from the devicetree.
> 
> This allows to stop manually adding the register base address to
> every R/W call in this driver, as this can be, and is now, handled
> by the regmap API instead.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Mostly fine. Trivial comments inline.

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

>  
>  static int iadc_reset(struct iadc_chip *iadc)
> @@ -270,7 +269,7 @@ static int iadc_poll_wait_eoc(struct iadc_chip *iadc, unsigned int interval_us)
>  
>  static int iadc_read_result(struct iadc_chip *iadc, u16 *data)
>  {
> -	return regmap_bulk_read(iadc->regmap, iadc->base + IADC_DATA, data, 2);
> +	return regmap_bulk_read(iadc->regmap, IADC_DATA, data, 2);

Perhaps not the time for it, but I'd prefer that 2 replaced with sizeof(*data)

>  }
>  
>  static int iadc_do_conversion(struct iadc_chip *iadc, int chan, u16 *data)
> @@ -483,12 +482,22 @@ static const struct iio_chan_spec iadc_channels[] = {
>  
>  static int iadc_probe(struct platform_device *pdev)
>  {
> +	struct regmap_config iadc_regmap_config = {
> +		.reg_bits = 16,
> +		.val_bits = 8,
> +		.max_register = 0x100,
> +		.fast_io = true,
> +	};
>  	struct device_node *node = pdev->dev.of_node;
>  	struct device *dev = &pdev->dev;
> +	struct spmi_subdevice *sub_sdev;
> +	struct spmi_device *sparent;
>  	struct iio_dev *indio_dev;
>  	struct iadc_chip *iadc;
>  	int ret, irq_eoc;
> -	u32 res;
> +
> +	if (!dev->parent)

How would this happen?

> +		return -ENODEV;
>