On Tue, 3 Aug 2021 11:09:47 +0800
David Wu <david.wu@xxxxxxxxxxxxxx> wrote:
> Hi Jonathan,
>
> 在 2021/8/2 下午6:42, Jonathan Cameron 写道:
> > On Mon, 2 Aug 2021 17:09:29 +0800
> > Simon Xue <xxm@xxxxxxxxxxxxxx> wrote:
> >
> >> From: David Wu <david.wu@xxxxxxxxxxxxxx>
> >>
> >> The referenced voltage is not changed after initiation, so just only
> >> get referenced voltage once.
> > Hi David,
> >
> > Isn't this an external reference voltage? If so how do you know
> > it is not changed at runtime? It might be unlikely and not happen
> > on particular platforms, but that's not he same as saying it can never
> > happen. Clearly it's racey anyway if that does happen, but we definitely
> > don't expect frequent voltage changes.
> >
>
> The current regulator is not changed and not subject to external
> changes, this can reduce the getting voltage. Assuming that there will
> be changes in the future, we then add the notify of the regulator, so
> that the voltage change can be obtained.
If this patch added the notifier that would be a nice solution, but
right now it potentially introduced a regression. You have made me a little curious...
Are you seeing a significant cost to querying that regulator voltage?
If so, I'd imagine it's a lack of caching in the regulator driver or similar.
Scale readback via sysfs shouldn't be in a fast path anyway.
You can't depend on what boards today do, because someone with a board
built tomorrow may well use an old kernel which supports the voltage
changing, and then see a regression when they upgrade to the kernel
containing this patch.
Jonathan
>
> > Jonathan
> >
> >>
> >> Signed-off-by: Simon Xue <xxm@xxxxxxxxxxxxxx>
> >> Signed-off-by: David Wu <david.wu@xxxxxxxxxxxxxx>
> >> ---
> >> drivers/iio/adc/rockchip_saradc.c | 16 +++++++++-------
> >> 1 file changed, 9 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
> >> index f3eb8d2e50dc..cd33c0b9d3eb 100644
> >> --- a/drivers/iio/adc/rockchip_saradc.c
> >> +++ b/drivers/iio/adc/rockchip_saradc.c
> >> @@ -49,6 +49,7 @@ struct rockchip_saradc {
> >> struct clk *clk;
> >> struct completion completion;
> >> struct regulator *vref;
> >> + int uv_vref;
> >> struct reset_control *reset;
> >> const struct rockchip_saradc_data *data;
> >> u16 last_val;
> >> @@ -105,13 +106,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
> >> mutex_unlock(&indio_dev->mlock);
> >> return IIO_VAL_INT;
> >> case IIO_CHAN_INFO_SCALE:
> >> - ret = regulator_get_voltage(info->vref);
> >> - if (ret < 0) {
> >> - dev_err(&indio_dev->dev, "failed to get voltage\n");
> >> - return ret;
> >> - }
> >> -
> >> - *val = ret / 1000;
> >> + *val = info->uv_vref / 1000;
> >> *val2 = chan->scan_type.realbits;
> >> return IIO_VAL_FRACTIONAL_LOG2;
> >> default:
> >> @@ -410,6 +405,13 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
> >> return ret;
> >> }
> >>
> >> + info->uv_vref = regulator_get_voltage(info->vref);
> >> + if (info->uv_vref < 0) {
> >> + dev_err(&pdev->dev, "failed to get voltage\n");
> >> + ret = info->uv_vref;
> >> + return ret;
> >> + }
> >> +
> >> ret = clk_prepare_enable(info->pclk);
> >> if (ret < 0) {
> >> dev_err(&pdev->dev, "failed to enable pclk\n");
> >
> >
> >
> >
>
>
