On Fri, Dec 13, 2013 at 10:29:43AM +0000, Li.Xiubo@freescale.com wrote:
Hi Shawn,
- ret = regulator_bulk_get(codec->dev, ARRAY_SIZE(sgtl5000->supplies),
sgtl5000->supplies);
- if (!ret)
external_vddd = 1;
- else {
- /* External VDDD only works before revision 0x11 */
- if (sgtl5000->revision < 0x11) {
vddd = regulator_get_optional(codec->dev, "VDDD");
if (IS_ERR(vddd)) {
/* See if it's just not registered yet */
if (PTR_ERR(vddd) == -EPROBE_DEFER)
return -EPROBE_DEFER;
} else {
external_vddd = 1;
regulator_put(vddd);
}
Shouldn't we handle other errors ?
We do not really care other errors. In case that an external VDDD is present on hardware design but regulator_get_optional("VDDD") still fails for some other reason, we should turn to use the internal LDO anyway rather than simply failing out.
We can see that from the regulator_get_optional()-->_regulator_get(), there are many other errors maybe returned. Not considering other error cases, If the dt is used here, only ERR_PTR(-ENODEV) will be returned if the external VDDD is absent, and only this case the ERR_PTR(_ENODEV) error will be returned, Or if dt is not used and there has no external VDDD was found, the ERR_PTR(-EPROBE_DEFER) will be returned.
So, if the external VDDD is not used, only ERR_PTR(-ENODEV) and ERR_PTR(-EPROBE_DEFER) maybe returned ignoring other errors.
How about the following ?
- if (IS_ERR(vddd)) {
/* See if it's just not registered yet */
if (PTR_ERR(vddd) != -ENODEV)
return PTR_ERR(vddd);
Just for example, if PTR_ERR(vddd) is -EBUSY here, instead of failing out, why do not we switch to internal LDO for continuing the probe?
Shawn
- } else {
external_vddd = 1;
regulator_put(vddd);
- }