Re: [PATCH v5 2/4] ASoC: tlv320aic32x4: Support for regulators

[Markus Pargmann <mpa@pengutronix.de>]

[-- Attachment #1.1: Type: text/plain, Size: 1758 bytes --]

On Tue, Feb 18, 2014 at 10:27:25AM +0900, Mark Brown wrote:
> On Mon, Feb 17, 2014 at 11:04:18AM +0100, Markus Pargmann wrote:
> 
> > +	if (IS_ERR(aic32x4->supply_iov)) {
> > +		dev_err(dev, "Missing supply 'iov'\n");
> > +		ret = -EINVAL;
> > +	}
> 
> This won't work with deferred probe and is ignoring the error value
> reported by the regualtor API.  It should pass back what it was given
> and still needs to handle deferred probe even if it's happy for a
> regulator not to be there.

Right, I added -EPROBE_DEFER checks for every regulator.
I replaced the usage of the return value to directly return the errors.
Otherwise it could be confusing to see multiple error messages and get
one error value returned.

> 
> > +	if (!IS_ERR(aic32x4->supply_iov)) {
> > +		ret = regulator_enable(aic32x4->supply_iov);
> > +		if (ret) {
> > +			printk("Failed to enable regulator iov\n");
> > +			return ret;
> > +		}
> > +	}
> 
> dev_err().  It would probably be easier to put one of the child
> regulators you did get into supply_iov instead so that outside of the
> probe you don't need to worry about which is actually there.
> 
> I'm also not seeing any disables prior to remove.

Replaced all printk's with dev_err. I am now disabling all regulators in
the remove function and added error handling which disables the
regulators.

I will test all changes tomorrow and resend the series.

Thanks,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]