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

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

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 :)


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;
> > +		}
> > +	}
> > +
> [...]