Re: [PATCH v2] Add support for external reference voltage through the regulator framework

[Silvan Murer <silvan.murer@gmail.com>]

Thank you for the additional informations.
I just submitted the fixed patch to the mailinglist.


On Fre, 2018-05-18 at 11:59 +0200, Lars-Peter Clausen wrote:
> On 05/18/2018 11:13 AM, Silvan Murer wrote:
> > 
> > Hi Lars,
> > Thanks for your review.
> > Should I create a new version of this patch whithout the whitespace
> > issues?
> > Generally question: What is the next step? Do you include the
> > reviewed
> > patch into the iio.git repository? And do you submit the patches to
> > the
> > mainline repository?
> > Sorry for the basic questions, currently I try to understand the
> > whole
> > flow of the patch flow :)
> 
> Hi,
> 
> Jonathan will pick the patch up and add it to the iio.git repository,
> when
> he finds the time and thinks the patch is good. And then the patches
> will
> find their way to mainline.
> 
> You could ask Jonathan to fix the whitespace errors when he picks up
> the
> patch. But it would be less work for him if you just send a version
> that has
> the whitespace fixed (Keep by Reviewed-by tag).
> 
> - Lars
> 
> > 
> > 
> > 
> > On Don, 2018-05-17 at 13:01 +0200, Lars-Peter Clausen wrote:
> > > 
> > > On 05/15/2018 10:14 PM, Silvan Murer wrote:
> > > > 
> > > > 
> > > > Add support for external reference voltage through the
> > > > regulator
> > > > framework.
> > > > 
> > > > Signed-off-by: Silvan Murer <silvan.murer@gmail.com>
> > > Looks good, thanks.
> > > 
> > > Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>
> > > 
> > > Just two tiny whitespace issues.
> > > 
> > > [...]
> > > > 
> > > > 
> > > >  enum ltc2632_supported_device_ids {
> > > > @@ -90,7 +96,7 @@ static int ltc2632_read_raw(struct iio_dev
> > > > *indio_dev,
> > > >  
> > > >  	switch (m) {
> > > >  	case IIO_CHAN_INFO_SCALE:
> > > > -		*val = chip_info->vref_mv;
> > > > +		*val =  st->vref_mv;
> > > Extra space after the '='.
> > > 
> > > > 
> > > > 
> > > >  		*val2 = chan->scan_type.realbits;
> > > >  		return IIO_VAL_FRACTIONAL_LOG2;
> > > >  	}
> > > > @@ -247,6 +253,45 @@ static int ltc2632_probe(struct spi_device
> > > > *spi)
> > > >  	chip_info = (struct ltc2632_chip_info *)
> > > >  			spi_get_device_id(spi)->driver_data;
> > > >  
> > > > +	st->vref_reg = devm_regulator_get_optional(&spi->dev,
> > > > "vref");
> > > > +	if (PTR_ERR(st->vref_reg) == -ENODEV) {
> > > > +		/* use internal reference voltage */
> > > > +		st->vref_reg = NULL;
> > > > +		st->vref_mv = chip_info->vref_mv;
> > > > +
> > > > +		ret = ltc2632_spi_write(spi,
> > > > LTC2632_CMD_INTERNAL_REFER,
> > > > +				0, 0, 0);
> > > > +		if (ret) {
> > > > +			dev_err(&spi->dev,
> > > > +				"Set internal reference
> > > > command
> > > > failed, %d\n",
> > > > +				ret);
> > > > +			return ret;
> > > > +		}
> > > > +	} else if (IS_ERR(st->vref_reg)) {
> > > > +		dev_err(&spi->dev,
> > > > +				"Error getting voltage
> > > > reference
> > > > regulator\n");
> > > > +			return PTR_ERR(st->vref_reg);
> > > Extra tab before the 'return'.
> > > 
> > > > 
> > > > 
> > > > +	} else {
> > > > +		/* use external reference voltage */
> > > > +		ret = regulator_enable(st->vref_reg);
> > > > +		if (ret) {
> > > > +			dev_err(&spi->dev,
> > > > +				"enable reference regulator
> > > > failed, %d\n",
> > > > +				ret);
> > > > +			return ret;
> > > > +		}
> > > > +		st->vref_mv = regulator_get_voltage(st-
> > > > >vref_reg)
> > > > / 1000;
> > > > +
> > > > +		ret = ltc2632_spi_write(spi,
> > > > LTC2632_CMD_EXTERNAL_REFER,
> > > > +				0, 0, 0);
> > > > +		if (ret) {
> > > > +			dev_err(&spi->dev,
> > > > +				"Set external reference
> > > > command
> > > > failed, %d\n",
> > > > +				ret);
> > > > +			return ret;
> > > > +		}
> > > > +	}
> > > > +
> > > [...]