| Message ID | 1474886150-16207-1-git-send-email-nikita.yoush@cogentembedded.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 020eab35390b976e6b93b61805002d3e2cd195de |
| Headers | show |
On Mon, Sep 26, 2016 at 01:35:50PM +0300, Nikita Yushchenko wrote: > I doubt that copying 7 lines into error path is better than having a tiny > logic with return values, but still here it is. Consider what happens if someone wants to change the code...
26.09.2016 19:08, Mark Brown пишет: > On Mon, Sep 26, 2016 at 01:35:50PM +0300, Nikita Yushchenko wrote: > >> I doubt that copying 7 lines into error path is better than having a tiny >> logic with return values, but still here it is. > > Consider what happens if someone wants to change the code... I believe that "undo poweron" and "poweroff" sequences still will have to remain the same. This having two copies of code is more error-prone than having one. Nikita
diff --git a/sound/soc/codecs/tpa6130a2.c b/sound/soc/codecs/tpa6130a2.c index 3b6faed91d7e..3712db0881d0 100644 --- a/sound/soc/codecs/tpa6130a2.c +++ b/sound/soc/codecs/tpa6130a2.c @@ -71,7 +71,14 @@ static int tpa6130a2_power(struct tpa6130a2_data *data, bool enable) if (ret != 0) { dev_err(data->dev, "Failed to sync registers: %d\n", ret); - goto regcache_sync_failed; + regcache_cache_only(data->regmap, true); + if (data->power_gpio >= 0) + gpio_set_value(data->power_gpio, 0); + ret2 = regulator_disable(data->supply); + if (ret2 != 0) + dev_err(data->dev, + "Failed to disable supply: %d\n", ret2); + return ret; } } else { /* Powered off device does not retain registers. While device @@ -79,18 +86,17 @@ static int tpa6130a2_power(struct tpa6130a2_data *data, bool enable) * happen in cache only. */ regcache_mark_dirty(data->regmap); -regcache_sync_failed: regcache_cache_only(data->regmap, true); /* Power off */ if (data->power_gpio >= 0) gpio_set_value(data->power_gpio, 0); - ret2 = regulator_disable(data->supply); - if (ret2 != 0) { + ret = regulator_disable(data->supply); + if (ret != 0) { dev_err(data->dev, - "Failed to disable supply: %d\n", ret2); - return ret ? ret : ret2; + "Failed to disable supply: %d\n", ret); + return ret; } }
Code undo operations in power enable errror path explicitly, instead of reusing power disable path and playing with return values there. Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> --- I doubt that copying 7 lines into error path is better than having a tiny logic with return values, but still here it is. sound/soc/codecs/tpa6130a2.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)