Re: [PATCH v2 1/2] iio: adc: ti-ads7950: Allow to use on ACPI platforms

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2017-08-01 at 12:09 -0500, David Lechner wrote:
> On 08/01/2017 11:41 AM, Andy Shevchenko wrote:
> > On Tue, 2017-08-01 at 11:21 -0500, David Lechner wrote:
> > > On 08/01/2017 10:45 AM, Andy Shevchenko wrote:
> > > >     
> > > > > > > +	/* Use hard coded value for reference voltage in
> > > > > > > ACPI
> > > > > > > case */
> > > > > > > +	if (ACPI_COMPANION(&spi->dev))
> > > > > > > +		st->vref_mv =
> > > > > > > TI_ADS7950_VA_MV_ACPI_DEFAULT;
> > > > > > 
> > > > > > Instead of checking or ACPI, you could just say "if we have
> > > > > > a
> > > > > > dummy
> > > > > > regulator, then use the default value".

> > > > > Agreed. Sounds sensible to me.  Hopefully in DT people will
> > > > > provide the right regulator, but chances are this won't
> > > > > always happen.
> > > > 
> > > > There is no call like
> > > > regulator_is_dummy()
> > > > (and, looking into the code of regulator framework, can't be)
> > > > 
> > > > Can you elaborate a bit, maybe I'm missing something obvious?
> > > > 
> > > 
> > > I haven't tested this, but shouldn't regulator_get_voltage()
> > > return
> > > an
> > > error for a dummy regulator? You could use this as your test.
> > 
> > While it would work it's very fragile.
> > 
> > _regulator_get_voltage() will return error code even for normal
> > regulators if something happened there. And user gets an impression
> > that
> > everything is okay while it's not.
> > 
> > So, I wouldn't go this way.

> OK. There is also devm_regulator_get_optional(). This will return 
> ERR_PTR(-ENODEV) instead of a dummy regulator. Then you could add a 
> check for this error and set st->reg = NULL if we get -ENODEV. Make
> the 
> enable and disable conditional. And use the default voltage if we
> don't 
> have a regulator.

Luckily it is consistent for now with error code, so -ENODEV it is, but
see comment below...

reg = devm_regulator_get_optional();
if (IS_ERR()) {
  if (PTR_ERR() != -ENODEV)
    return PTR_ERR();

  /*
OK     >> dummy, or absent regulator, or ... 
Not OK >> supply can't be found, or ->ops->enable() returns -ENODEV
   */

Less fragile, but still fragile...

}

> But this is probably even messier than the original ACPI patch. :-/

It's still no-go from my point of view.

So... ?

-- 
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux