| Message ID | 20220711134312.234268-3-tmaimon77@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | iio: adc: npcm: add Arbel NPCM8XX support | expand |
On Mon, Jul 11, 2022 at 3:59 PM Tomer Maimon <tmaimon77@gmail.com> wrote: > > Adding ADC NPCM8XX support to NPCM ADC driver. > ADC NPCM8XX uses a different resolution and voltage reference. > > As part of adding NPCM8XX support: > - Add NPCM8XX specific compatible string. > - Add data to handle architecture-specific ADC parameters. Good patch, but one change can make it even better! ... > struct device *dev = &pdev->dev; > + const struct of_device_id *match; > > indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info)); > if (!indio_dev) > return -ENOMEM; > info = iio_priv(indio_dev); > > - mutex_init(&info->lock); > + match = of_match_node(npcm_adc_match, pdev->dev.of_node); > + if (!match || !match->data) { > + dev_err(dev, "Failed getting npcm_adc_data\n"); > + return -ENODEV; > + } > > + info->data = (struct npcm_adc_info *)match->data; Instead of above info->data = device_get_match_data(dev); if (!info->data) return -ENODEV;
On Mon, Jul 11, 2022 at 4:14 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Mon, Jul 11, 2022 at 3:59 PM Tomer Maimon <tmaimon77@gmail.com> wrote: ... > > struct device *dev = &pdev->dev; > > + const struct of_device_id *match; > > + match = of_match_node(npcm_adc_match, pdev->dev.of_node); > > + if (!match || !match->data) { > > + dev_err(dev, "Failed getting npcm_adc_data\n"); > > + return -ENODEV; > > + } > > > > + info->data = (struct npcm_adc_info *)match->data; > > Instead of above > > info->data = device_get_match_data(dev); > if (!info->data) > return -ENODEV; Or return dev_err_probe(dev, -EINVAL, "...\n"); if you want that message to be issued.
Hi Andy, Thanks for your comments, they will be addressed next version. On Mon, 11 Jul 2022 at 17:16, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Mon, Jul 11, 2022 at 4:14 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Mon, Jul 11, 2022 at 3:59 PM Tomer Maimon <tmaimon77@gmail.com> wrote: > > ... > > > > struct device *dev = &pdev->dev; > > > + const struct of_device_id *match; > > > > + match = of_match_node(npcm_adc_match, pdev->dev.of_node); > > > + if (!match || !match->data) { > > > + dev_err(dev, "Failed getting npcm_adc_data\n"); > > > + return -ENODEV; > > > + } > > > > > > + info->data = (struct npcm_adc_info *)match->data; > > > > Instead of above > > > > info->data = device_get_match_data(dev); > > if (!info->data) > > > > return -ENODEV; > > Or > > return dev_err_probe(dev, -EINVAL, "...\n"); > > if you want that message to be issued. > > > -- > With Best Regards, > Andy Shevchenko Best regards, Tomer
diff --git a/drivers/iio/adc/npcm_adc.c b/drivers/iio/adc/npcm_adc.c index f7bc0bb7f112..efacba256056 100644 --- a/drivers/iio/adc/npcm_adc.c +++ b/drivers/iio/adc/npcm_adc.c @@ -16,6 +16,12 @@ #include <linux/uaccess.h> #include <linux/reset.h> +struct npcm_adc_info { + u32 data_mask; + u32 internal_vref; + u32 res_bits; +}; + struct npcm_adc { bool int_status; u32 adc_sample_hz; @@ -34,6 +40,7 @@ struct npcm_adc { * has finished. */ struct mutex lock; + struct npcm_adc_info *data; }; /* ADC registers */ @@ -52,13 +59,21 @@ struct npcm_adc { #define NPCM_ADCCON_CH(x) ((x) << 24) #define NPCM_ADCCON_DIV_SHIFT 1 #define NPCM_ADCCON_DIV_MASK GENMASK(8, 1) -#define NPCM_ADC_DATA_MASK(x) ((x) & GENMASK(9, 0)) #define NPCM_ADC_ENABLE (NPCM_ADCCON_ADC_EN | NPCM_ADCCON_ADC_INT_EN) /* ADC General Definition */ -#define NPCM_RESOLUTION_BITS 10 -#define NPCM_INT_VREF_MV 2000 +static const struct npcm_adc_info npxm7xx_adc_info = { + .data_mask = GENMASK(9, 0), + .internal_vref = 2048, + .res_bits = 10, +}; + +static const struct npcm_adc_info npxm8xx_adc_info = { + .data_mask = GENMASK(11, 0), + .internal_vref = 1229, + .res_bits = 12, +}; #define NPCM_ADC_CHAN(ch) { \ .type = IIO_VOLTAGE, \ @@ -129,7 +144,8 @@ static int npcm_adc_read(struct npcm_adc *info, int *val, u8 channel) if (ret < 0) return ret; - *val = NPCM_ADC_DATA_MASK(ioread32(info->regs + NPCM_ADCDATA)); + *val = ioread32(info->regs + NPCM_ADCDATA); + *val &= info->data->data_mask; return 0; } @@ -157,9 +173,9 @@ static int npcm_adc_read_raw(struct iio_dev *indio_dev, vref_uv = regulator_get_voltage(info->vref); *val = vref_uv / 1000; } else { - *val = NPCM_INT_VREF_MV; + *val = info->data->internal_vref; } - *val2 = NPCM_RESOLUTION_BITS; + *val2 = info->data->res_bits; return IIO_VAL_FRACTIONAL_LOG2; case IIO_CHAN_INFO_SAMP_FREQ: *val = info->adc_sample_hz; @@ -176,7 +192,8 @@ static const struct iio_info npcm_adc_iio_info = { }; static const struct of_device_id npcm_adc_match[] = { - { .compatible = "nuvoton,npcm750-adc", }, + { .compatible = "nuvoton,npcm750-adc", .data = &npxm7xx_adc_info}, + { .compatible = "nuvoton,npcm845-adc", .data = &npxm8xx_adc_info}, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, npcm_adc_match); @@ -190,14 +207,20 @@ static int npcm_adc_probe(struct platform_device *pdev) struct npcm_adc *info; struct iio_dev *indio_dev; struct device *dev = &pdev->dev; + const struct of_device_id *match; indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info)); if (!indio_dev) return -ENOMEM; info = iio_priv(indio_dev); - mutex_init(&info->lock); + match = of_match_node(npcm_adc_match, pdev->dev.of_node); + if (!match || !match->data) { + dev_err(dev, "Failed getting npcm_adc_data\n"); + return -ENODEV; + } + info->data = (struct npcm_adc_info *)match->data; info->dev = &pdev->dev; info->regs = devm_platform_ioremap_resource(pdev, 0);
Adding ADC NPCM8XX support to NPCM ADC driver. ADC NPCM8XX uses a different resolution and voltage reference. As part of adding NPCM8XX support: - Add NPCM8XX specific compatible string. - Add data to handle architecture-specific ADC parameters. Signed-off-by: Tomer Maimon <tmaimon77@gmail.com> --- drivers/iio/adc/npcm_adc.c | 39 ++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-)