Re: [PATCH 1/2] ASoC: rt5645: fix error handling for gpio detection

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

 



Hi,

On Feb 2 2017 03:27, Pierre-Louis Bossart wrote:
Optional gpio handling should not cause an error status and prevent
probing if it's missing.  Remove error return for -ENOENT case and
move error message to dev_info

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
---
 sound/soc/codecs/rt5645.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/rt5645.c b/sound/soc/codecs/rt5645.c
index 65ac841..e149f3c 100644
--- a/sound/soc/codecs/rt5645.c
+++ b/sound/soc/codecs/rt5645.c
@@ -3660,8 +3660,14 @@ static int rt5645_i2c_probe(struct i2c_client *i2c,
 						       GPIOD_IN);

 	if (IS_ERR(rt5645->gpiod_hp_det)) {
-		dev_err(&i2c->dev, "failed to initialize gpiod\n");
-		return PTR_ERR(rt5645->gpiod_hp_det);
+		dev_info(&i2c->dev, "failed to initialize gpiod\n");
+		ret = PTR_ERR(rt5645->gpiod_hp_det);
+		/*
+		 * Continue if optional gpiod is missing, bail for all other
+		 * errors, including -EPROBE_DEFER
+		 */
+		if (ret != -ENOENT)
+			return ret;
 	}

 	for (i = 0; i < ARRAY_SIZE(rt5645->supplies); i++)

(sound/soc/codecs/rt5645.c)
rt5645_i2c_probe()
 (drivers/gpio/devres.c)
 ->devm_gpiod_get_optional()
   ->devm_gpiod_get_index_optional()

As long as seeing current implementation of 'devm_gpiod_get_index_optional()', this function never returns ENOENT. In this case, it returns NULL.
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/devres.c#n185

IS_ERR(NULL) is false, thus the additional condition is useless. When entering the branch, error code should be always returned.

For your purpose, checking NULL at first, then handles the error properly, like:

$ git diff
diff --git a/sound/soc/codecs/rt5645.c b/sound/soc/codecs/rt5645.c
index 1ac96ef..a588454 100644
--- a/sound/soc/codecs/rt5645.c
+++ b/sound/soc/codecs/rt5645.c
@@ -3656,10 +3656,15 @@ static int rt5645_i2c_probe(struct i2c_client *i2c,

rt5645->gpiod_hp_det = devm_gpiod_get_optional(&i2c->dev, "hp-detect",
                                                       GPIOD_IN);
-
-       if (IS_ERR(rt5645->gpiod_hp_det)) {
-               dev_err(&i2c->dev, "failed to initialize gpiod\n");
-               return PTR_ERR(rt5645->gpiod_hp_det);
+       if (rt5645->gpiod_hp_det && IS_ERR(rt5645->gpiod_hp_det)) {
+               dev_info(&i2c->dev, "failed to initialize gpiod\n");
+               ret = PTR_ERR(rt5645->gpiod_hp_det);
+               /*
+                * Continue if optional gpiod is missing, bail for all other
+                * errors, including -EPROBE_DEFER. -ENOENT is never returns
+                * according to implementation of devm_gpiod_get_optional().
+                */
+               return ret;
        }

        for (i = 0; i < ARRAY_SIZE(rt5645->supplies); i++)


Regards

Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux