Re: [PATCH V5 3/5] iio: adc: Add support for PMIC7 ADC

[Jishnu Prakash]

Hi Jonathan,

On 5/24/2020 5:29 PM, Jonathan Cameron wrote:
> On Fri, 22 May 2020 19:54:10 +0530
> Jishnu Prakash <jprakash@xxxxxxxxxxxxxx> wrote:
>
> > The ADC architecture on PMIC7 is changed as compared to PMIC5. The
> > major change from PMIC5 is that all SW communication to ADC goes through
> > PMK8350, which communicates with other PMICs through PBS when the ADC
> > on PMK8350 works in master mode. The SID register is used to identify the
> > PMICs with which the PBS needs to communicate. Add support for the same.
> >
> > Signed-off-by: Jishnu Prakash <jprakash@xxxxxxxxxxxxxx>
> > ---
> >   drivers/iio/adc/qcom-spmi-adc5.c   | 231 ++++++++++++++++++++++++++++++--
> >   drivers/iio/adc/qcom-vadc-common.c | 260 +++++++++++++++++++++++++++++++++++++
> >   drivers/iio/adc/qcom-vadc-common.h |  14 ++
> >   3 files changed, 496 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/iio/adc/qcom-spmi-adc5.c b/drivers/iio/adc/qcom-spmi-adc5.c
> > index 21fdcde..e1482fd 100644
> > --- a/drivers/iio/adc/qcom-spmi-adc5.c
> > +++ b/drivers/iio/adc/qcom-spmi-adc5.c
> > @@ -1,6 +1,6 @@
> >   // SPDX-License-Identifier: GPL-2.0
> >   /*
> > - * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> > + * Copyright (c) 2018, 2020, The Linux Foundation. All rights reserved.
> ...
> > @@ -110,6 +120,7 @@ struct adc5_channel_prop {
> >   	enum adc5_cal_method	cal_method;
> >   	enum adc5_cal_val	cal_val;
> >   	unsigned int		decimation;
> > +	unsigned int		sid;
> >   	unsigned int		prescale;
> >   	unsigned int		hw_settle_time;
> >   	unsigned int		avg_samples;
> > @@ -140,6 +151,7 @@ struct adc5_chip {
> >   	bool			poll_eoc;
> >   	struct completion	complete;
> >   	struct mutex		lock;
> > +	int			irq_eoc;
> This is introduced but doesn't seem to be used...
I'll remove it in the next post.
> >   	const struct adc5_data	*data;
> >   };
> >   
> > @@ -155,6 +167,8 @@ static const struct vadc_prescale_ratio adc5_prescale_ratios[] = {
> >   	{.num =  1, .den = 16}
> >   };
> >   
> ...
>
> >   static int adc5_prescaling_from_dt(u32 num, u32 den)
> >   {
> >   	unsigned int pre;
> > @@ -285,7 +304,7 @@ static int adc5_configure(struct adc5_chip *adc,
> >   
> >   	/* Read registers 0x42 through 0x46 */
> >   	ret = adc5_read(adc, ADC5_USR_DIG_PARAM, buf, sizeof(buf));
> > -	if (ret < 0)
> > +	if (ret)
> >   		return ret;
> As Andy mentioned, this is better in a separate patch. It's just adding
> noise to this patch which is actually doing things...
I'll split it out into a separate patch in the next post.
> >   
> >   	/* Digital param selection */
> > @@ -314,6 +333,47 @@ static int adc5_configure(struct adc5_chip *adc,
> >   	return adc5_write(adc, ADC5_USR_DIG_PARAM, buf, sizeof(buf));
> >   }
> >   
> Thanks,
>
> Jonathan