diff mbox series

extcon: palmas: hide error messages if gpio returns -EPROBE_DEFER

Message ID f65ad0ef2866e7d5b6743e13579c1efe8c572b4f.1581929741.git.hns@goldelico.com (mailing list archive)
State New, archived
Headers show
Series extcon: palmas: hide error messages if gpio returns -EPROBE_DEFER | expand

Commit Message

H. Nikolaus Schaller Feb. 17, 2020, 8:55 a.m. UTC
If the gpios are probed after this driver (e.g. if they
come from an i2c expander) there is no need to print an
error message.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/extcon/extcon-palmas.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Chanwoo Choi Feb. 17, 2020, 10:15 a.m. UTC | #1
Hi,

On 2/17/20 5:55 PM, H. Nikolaus Schaller wrote:
> If the gpios are probed after this driver (e.g. if they
> come from an i2c expander) there is no need to print an
> error message.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  drivers/extcon/extcon-palmas.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
> index edc5016f46f1..9c6254c0531c 100644
> --- a/drivers/extcon/extcon-palmas.c
> +++ b/drivers/extcon/extcon-palmas.c
> @@ -206,14 +206,16 @@ static int palmas_usb_probe(struct platform_device *pdev)
>  	palmas_usb->id_gpiod = devm_gpiod_get_optional(&pdev->dev, "id",
>  							GPIOD_IN);
>  	if (IS_ERR(palmas_usb->id_gpiod)) {
> -		dev_err(&pdev->dev, "failed to get id gpio\n");
> +		if (PTR_ERR(palmas_usb->id_gpiod) != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "failed to get id gpio\n");
>  		return PTR_ERR(palmas_usb->id_gpiod);
>  	}
>  
>  	palmas_usb->vbus_gpiod = devm_gpiod_get_optional(&pdev->dev, "vbus",
>  							GPIOD_IN);
>  	if (IS_ERR(palmas_usb->vbus_gpiod)) {
> -		dev_err(&pdev->dev, "failed to get vbus gpio\n");
> +		if (PTR_ERR(palmas_usb->vbus_gpiod) != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "failed to get vbus gpio\n");
>  		return PTR_ERR(palmas_usb->vbus_gpiod);
>  	}
>  
> 

Usually, gpio driver like pinctrl is very early probed
because almost device drivers should use gpio.
So, I have not any experience about this situation.
Do you meet this situation on any h/w board?
H. Nikolaus Schaller Feb. 17, 2020, 11:07 a.m. UTC | #2
Hi Chanwoo Choi,

> Am 17.02.2020 um 11:15 schrieb Chanwoo Choi <cw00.choi@samsung.com>:
> 
> Hi,
> 
> On 2/17/20 5:55 PM, H. Nikolaus Schaller wrote:
>> If the gpios are probed after this driver (e.g. if they
>> come from an i2c expander) there is no need to print an
>> error message.
>> 
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> drivers/extcon/extcon-palmas.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
>> index edc5016f46f1..9c6254c0531c 100644
>> --- a/drivers/extcon/extcon-palmas.c
>> +++ b/drivers/extcon/extcon-palmas.c
>> @@ -206,14 +206,16 @@ static int palmas_usb_probe(struct platform_device *pdev)
>> 	palmas_usb->id_gpiod = devm_gpiod_get_optional(&pdev->dev, "id",
>> 							GPIOD_IN);
>> 	if (IS_ERR(palmas_usb->id_gpiod)) {
>> -		dev_err(&pdev->dev, "failed to get id gpio\n");
>> +		if (PTR_ERR(palmas_usb->id_gpiod) != -EPROBE_DEFER)
>> +			dev_err(&pdev->dev, "failed to get id gpio\n");
>> 		return PTR_ERR(palmas_usb->id_gpiod);
>> 	}
>> 
>> 	palmas_usb->vbus_gpiod = devm_gpiod_get_optional(&pdev->dev, "vbus",
>> 							GPIOD_IN);
>> 	if (IS_ERR(palmas_usb->vbus_gpiod)) {
>> -		dev_err(&pdev->dev, "failed to get vbus gpio\n");
>> +		if (PTR_ERR(palmas_usb->vbus_gpiod) != -EPROBE_DEFER)
>> +			dev_err(&pdev->dev, "failed to get vbus gpio\n");
>> 		return PTR_ERR(palmas_usb->vbus_gpiod);
>> 	}
>> 
>> 
> 
> Usually, gpio driver like pinctrl is very early probed
> because almost device drivers should use gpio.
> So, I have not any experience about this situation.
> Do you meet this situation on any h/w board?

Yes, I have experienced that sometimes on omap5+palmas based boards.

It seems to be this:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/omap5-board-common.dtsi?h=v5.6-rc2#n384

The extcon_usb3 can potentially match this extcon-palmas driver at
a moment where the palmas_gpio it is referring to has not yet been
successfully probed. Then the palmas_gpio would return -EPROBE_DEFER.

AFAIK there is no guarantee for a specific sequence of drivers
being probed and it is pure luck that in most cases the gpios
are already probed.

BR and thanks,
Nikolaus Schaller
Chanwoo Choi Feb. 17, 2020, 11:24 a.m. UTC | #3
Hi,

On 2/17/20 8:07 PM, H. Nikolaus Schaller wrote:
> Hi Chanwoo Choi,
> 
>> Am 17.02.2020 um 11:15 schrieb Chanwoo Choi <cw00.choi@samsung.com>:
>>
>> Hi,
>>
>> On 2/17/20 5:55 PM, H. Nikolaus Schaller wrote:
>>> If the gpios are probed after this driver (e.g. if they
>>> come from an i2c expander) there is no need to print an
>>> error message.
>>>
>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>> ---
>>> drivers/extcon/extcon-palmas.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
>>> index edc5016f46f1..9c6254c0531c 100644
>>> --- a/drivers/extcon/extcon-palmas.c
>>> +++ b/drivers/extcon/extcon-palmas.c
>>> @@ -206,14 +206,16 @@ static int palmas_usb_probe(struct platform_device *pdev)
>>> 	palmas_usb->id_gpiod = devm_gpiod_get_optional(&pdev->dev, "id",
>>> 							GPIOD_IN);
>>> 	if (IS_ERR(palmas_usb->id_gpiod)) {
>>> -		dev_err(&pdev->dev, "failed to get id gpio\n");
>>> +		if (PTR_ERR(palmas_usb->id_gpiod) != -EPROBE_DEFER)
>>> +			dev_err(&pdev->dev, "failed to get id gpio\n");
>>> 		return PTR_ERR(palmas_usb->id_gpiod);
>>> 	}


How about editing it ? as following:
because following suggestion reduces the one checking behavior
when return value is -EPROBE_DEFER.

	if (PTR_ERR(palmas_usb->id_gpiod) == -EPROBE_DEFER) {
		return -EPROBE_DEFER;
	} else if (IS_ERR(palmas_usb->id_gpiod)) {
		dev_err(&pdev->dev, "failed to get id gpio\n");
		return PTR_ERR(palmas_usb->id_gpiod);
	}

>>>
>>> 	palmas_usb->vbus_gpiod = devm_gpiod_get_optional(&pdev->dev, "vbus",
>>> 							GPIOD_IN);
>>> 	if (IS_ERR(palmas_usb->vbus_gpiod)) {
>>> -		dev_err(&pdev->dev, "failed to get vbus gpio\n");
>>> +		if (PTR_ERR(palmas_usb->vbus_gpiod) != -EPROBE_DEFER)
>>> +			dev_err(&pdev->dev, "failed to get vbus gpio\n");

ditto.

>>> 		return PTR_ERR(palmas_usb->vbus_gpiod);
>>> 	}
>>>
>>>
>>
>> Usually, gpio driver like pinctrl is very early probed
>> because almost device drivers should use gpio.
>> So, I have not any experience about this situation.
>> Do you meet this situation on any h/w board?
> 
> Yes, I have experienced that sometimes on omap5+palmas based boards.
> 
> It seems to be this:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/omap5-board-common.dtsi?h=v5.6-rc2#n384
> 
> The extcon_usb3 can potentially match this extcon-palmas driver at
> a moment where the palmas_gpio it is referring to has not yet been
> successfully probed. Then the palmas_gpio would return -EPROBE_DEFER.
> 
> AFAIK there is no guarantee for a specific sequence of drivers
> being probed and it is pure luck that in most cases the gpios
> are already probed.

Thanks for explaining the example.
H. Nikolaus Schaller Feb. 17, 2020, 12:21 p.m. UTC | #4
Hi,

> Am 17.02.2020 um 12:24 schrieb Chanwoo Choi <cw00.choi@samsung.com>:
> 
> Hi,
> 
> On 2/17/20 8:07 PM, H. Nikolaus Schaller wrote:
>> Hi Chanwoo Choi,
>> 
>>> Am 17.02.2020 um 11:15 schrieb Chanwoo Choi <cw00.choi@samsung.com>:
>>> 
>>> Hi,
>>> 
>>> On 2/17/20 5:55 PM, H. Nikolaus Schaller wrote:
>>>> If the gpios are probed after this driver (e.g. if they
>>>> come from an i2c expander) there is no need to print an
>>>> error message.
>>>> 
>>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>> ---
>>>> drivers/extcon/extcon-palmas.c | 6 ++++--
>>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
>>>> index edc5016f46f1..9c6254c0531c 100644
>>>> --- a/drivers/extcon/extcon-palmas.c
>>>> +++ b/drivers/extcon/extcon-palmas.c
>>>> @@ -206,14 +206,16 @@ static int palmas_usb_probe(struct platform_device *pdev)
>>>> 	palmas_usb->id_gpiod = devm_gpiod_get_optional(&pdev->dev, "id",
>>>> 							GPIOD_IN);
>>>> 	if (IS_ERR(palmas_usb->id_gpiod)) {
>>>> -		dev_err(&pdev->dev, "failed to get id gpio\n");
>>>> +		if (PTR_ERR(palmas_usb->id_gpiod) != -EPROBE_DEFER)
>>>> +			dev_err(&pdev->dev, "failed to get id gpio\n");
>>>> 		return PTR_ERR(palmas_usb->id_gpiod);
>>>> 	}
> 
> 
> How about editing it ? as following:
> because following suggestion reduces the one checking behavior
> when return value is -EPROBE_DEFER.
> 
> 	if (PTR_ERR(palmas_usb->id_gpiod) == -EPROBE_DEFER) {
> 		return -EPROBE_DEFER;
> 	} else if (IS_ERR(palmas_usb->id_gpiod)) {
> 		dev_err(&pdev->dev, "failed to get id gpio\n");
> 		return PTR_ERR(palmas_usb->id_gpiod);
> 	}

Yes, looks indeed to be valid (some discussion around
is here: https://lore.kernel.org/patchwork/patch/999602/ ).

So I'll send an update asap.

> 
>>>> 
>>>> 	palmas_usb->vbus_gpiod = devm_gpiod_get_optional(&pdev->dev, "vbus",
>>>> 							GPIOD_IN);
>>>> 	if (IS_ERR(palmas_usb->vbus_gpiod)) {
>>>> -		dev_err(&pdev->dev, "failed to get vbus gpio\n");
>>>> +		if (PTR_ERR(palmas_usb->vbus_gpiod) != -EPROBE_DEFER)
>>>> +			dev_err(&pdev->dev, "failed to get vbus gpio\n");
> 
> ditto.
> 
>>>> 		return PTR_ERR(palmas_usb->vbus_gpiod);
>>>> 	}
>>>> 
>>>> 
>>> 
>>> Usually, gpio driver like pinctrl is very early probed
>>> because almost device drivers should use gpio.
>>> So, I have not any experience about this situation.
>>> Do you meet this situation on any h/w board?
>> 
>> Yes, I have experienced that sometimes on omap5+palmas based boards.
>> 
>> It seems to be this:
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/omap5-board-common.dtsi?h=v5.6-rc2#n384
>> 
>> The extcon_usb3 can potentially match this extcon-palmas driver at
>> a moment where the palmas_gpio it is referring to has not yet been
>> successfully probed. Then the palmas_gpio would return -EPROBE_DEFER.
>> 
>> AFAIK there is no guarantee for a specific sequence of drivers
>> being probed and it is pure luck that in most cases the gpios
>> are already probed.
> 
> Thanks for explaining the example.

BR and thanks,
Nikolaus Schaller
diff mbox series

Patch

diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
index edc5016f46f1..9c6254c0531c 100644
--- a/drivers/extcon/extcon-palmas.c
+++ b/drivers/extcon/extcon-palmas.c
@@ -206,14 +206,16 @@  static int palmas_usb_probe(struct platform_device *pdev)
 	palmas_usb->id_gpiod = devm_gpiod_get_optional(&pdev->dev, "id",
 							GPIOD_IN);
 	if (IS_ERR(palmas_usb->id_gpiod)) {
-		dev_err(&pdev->dev, "failed to get id gpio\n");
+		if (PTR_ERR(palmas_usb->id_gpiod) != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "failed to get id gpio\n");
 		return PTR_ERR(palmas_usb->id_gpiod);
 	}
 
 	palmas_usb->vbus_gpiod = devm_gpiod_get_optional(&pdev->dev, "vbus",
 							GPIOD_IN);
 	if (IS_ERR(palmas_usb->vbus_gpiod)) {
-		dev_err(&pdev->dev, "failed to get vbus gpio\n");
+		if (PTR_ERR(palmas_usb->vbus_gpiod) != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "failed to get vbus gpio\n");
 		return PTR_ERR(palmas_usb->vbus_gpiod);
 	}