Re: [PATCH 8/8] ASoC: tpa6130a2: Control vdd using regulator framework

2009-10-08 Thread Mark Brown
On Thu, Oct 08, 2009 at 02:58:57PM +0300, Eduardo Valentin wrote:

 + data-regulator = regulator_get(dev, vdd);
 + if (IS_ERR(data-regulator)) {
 + dev_info(dev, Could not get regulator for vdd. 
 + Executing without regulator.\n);
 + data-regulator = NULL;
 + }

Similar comments to the previous patch apply to this driver - regulator
usage should be unconditional, error messages should not be split over
multiple lines and you should represent all the supplies separately (it
looks like there's both VDD and CPVSS required here, for example) to
avoid future surprises.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 8/8] ASoC: tpa6130a2: Control vdd using regulator framework

2009-10-08 Thread Eduardo Valentin
On Thu, Oct 08, 2009 at 03:43:33PM +0200, Mark Brown wrote:
 On Thu, Oct 08, 2009 at 02:58:57PM +0300, Eduardo Valentin wrote:
 
  +   data-regulator = regulator_get(dev, vdd);
  +   if (IS_ERR(data-regulator)) {
  +   dev_info(dev, Could not get regulator for vdd. 
  +   Executing without regulator.\n);
  +   data-regulator = NULL;
  +   }
 
 Similar comments to the previous patch apply to this driver - regulator
 usage should be unconditional, error messages should not be split over
 multiple lines and you should represent all the supplies separately (it
 looks like there's both VDD and CPVSS required here, for example) to
 avoid future surprises.


Yeah. The idea here was to keep driver running even if regulators
are not properly set in board files. Maybe those in which the regulator
is always on. That's why I wrote it with nicely message.

-- 
Eduardo Valentin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 8/8] ASoC: tpa6130a2: Control vdd using regulator framework

2009-10-08 Thread Mark Brown
On Thu, Oct 08, 2009 at 04:56:28PM +0300, Eduardo Valentin wrote:
 On Thu, Oct 08, 2009 at 03:43:33PM +0200, Mark Brown wrote:

  Similar comments to the previous patch apply to this driver - regulator
  usage should be unconditional, error messages should not be split over
  multiple lines and you should represent all the supplies separately (it
  looks like there's both VDD and CPVSS required here, for example) to
  avoid future surprises.

 Yeah. The idea here was to keep driver running even if regulators
 are not properly set in board files. Maybe those in which the regulator
 is always on. That's why I wrote it with nicely message.

The issue is that this doesn't actually end up increasing the system
robustness since in some systems the regulator control will be required
and the failures can be non-obvious, such as having a shared regulator
powered off at run time due to other system activity.  A misconfigured
regulator supply that stops probe is relatively easy to diagnose and fix
compared to potential silent interactions with other subsystems.

It's simple enough to set up a fixed voltage regulator in the board for
any supplies that don't have soft regulators.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html