On Mon, 12 Aug 2024 17:00:56 +0200
Thomas Bonnefille <thomas.bonnefille@xxxxxxxxxxx> wrote:
> This adds a driver for the common Sophgo SARADC.
>
> Signed-off-by: Thomas Bonnefille <thomas.bonnefille@xxxxxxxxxxx>
Similar comment on wild cards to the one you resolved in the dt binding
applies to the driver.
So normally convention is pick a part and name after that for
all function names, parameters etc.
cv1800b seems appropriate given the binding.
Other than that, just one trivial comment from me.
Jonathan
> diff --git a/drivers/iio/adc/sophgo-cv18xx-adc.c b/drivers/iio/adc/sophgo-cv18xx-adc.c
> new file mode 100644
> index 000000000000..ab7ee0f482cc
> --- /dev/null
> +++ b/drivers/iio/adc/sophgo-cv18xx-adc.c
> @@ -0,0 +1,208 @@
> +static int cv18xx_adc_probe(struct platform_device *pdev)
> +{
> + struct cv18xx_adc *saradc;
> + struct iio_dev *indio_dev;
> + struct clk *clk;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*saradc));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + saradc = iio_priv(indio_dev);
> + indio_dev->name = "sophgo-cv18xx-adc";
This definitely doesn't want to be wildcard based
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &cv18xx_adc_info;
> + indio_dev->num_channels = ARRAY_SIZE(sophgo_channels);
> + indio_dev->channels = sophgo_channels;
> +
> + clk = devm_clk_get_enabled(&pdev->dev, NULL);
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + saradc->regs = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(saradc->regs))
> + return PTR_ERR(saradc->regs);
> +
> + saradc->irq = platform_get_irq_optional(pdev, 0);
> + if (saradc->irq >= 0) {
> + init_completion(&saradc->completion);
> + ret = devm_request_irq(&pdev->dev, saradc->irq,
> + cv18xx_adc_interrupt_handler, 0,
> + dev_name(&pdev->dev), saradc);
> + if (ret)
> + return ret;
> +
> + writel(1, saradc->regs + CV18XX_ADC_INTR_EN_REG);
> + }
> +
> + ret = devm_mutex_init(&pdev->dev, &saradc->lock);
> + if (ret)
> + return ret;
> +
> + platform_set_drvdata(pdev, indio_dev);
Is this used?
> + writel(FIELD_PREP(CV18XX_ADC_DEF_STARTUP_CYCLE_MASK, 0xF) |
> + FIELD_PREP(CV18XX_ADC_DEF_SAMPLE_WINDOW_MASK, 0xF) |
> + FIELD_PREP(CV18XX_ADC_DEF_CLOCK_DIVIDER_MASK, 0x1) |
> + FIELD_PREP(CV18XX_ADC_DEF_COMPARE_CYCLE_MASK, 0xF),
> + saradc->regs + CV18XX_ADC_CYC_SET_REG);
> +
> + return devm_iio_device_register(&pdev->dev, indio_dev);
> +}
