Re: mwifiex: propagate error if IRQ request fails in mwifiex_sdio_of()

3 messages, 2 authors, 2016-09-08 · open the first message on its own page

Re: mwifiex: propagate error if IRQ request fails in mwifiex_sdio_of()

From: Javier Martinez Canillas <hidden>
Date: 2016-09-06 12:12:44

Hello Kalle,

On 09/03/2016 12:35 PM, Kalle Valo wrote:
Javier Martinez Canillas [off-list ref] wrote:
quoted
If request_irq() fails in mwifiex_sdio_probe_of(), only an error message
is printed but the actual error is not propagated to the caller function.

Signed-off-by: Javier Martinez Canillas <redacted>
What's the conclusion with this patch? Should I drop it or take it?

(The discussion is available from the patchwork link in the signature.)
My understanding is that Arend agrees with the patch and that the question
raised was caused by looking at an older kernel version. IOW, the patch is
OK and should be picked.

I'm adding Arend to cc, so can comment in case I misunderstood him though.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

RE: mwifiex: propagate error if IRQ request fails in mwifiex_sdio_of()

From: Amitkumar Karwar <hidden>
Date: 2016-09-08 15:55:43

Hi Javier,
From: Javier Martinez Canillas [mailto:javier@osg.samsung.com]
Sent: Tuesday, September 06, 2016 5:43 PM
To: Kalle Valo
Cc: linux-kernel@vger.kernel.org; Amitkumar Karwar;
netdev@vger.kernel.org; linux-wireless@vger.kernel.org; Nishant
Sarmukadam; Arend van Spriel
Subject: Re: mwifiex: propagate error if IRQ request fails in
mwifiex_sdio_of()

Hello Kalle,

On 09/03/2016 12:35 PM, Kalle Valo wrote:
quoted
Javier Martinez Canillas [off-list ref] wrote:
quoted
If request_irq() fails in mwifiex_sdio_probe_of(), only an error
message is printed but the actual error is not propagated to the
caller function.
quoted
quoted
Signed-off-by: Javier Martinez Canillas <redacted>
What's the conclusion with this patch? Should I drop it or take it?

(The discussion is available from the patchwork link in the
signature.)
My understanding is that Arend agrees with the patch and that the
question raised was caused by looking at an older kernel version. IOW,
the patch is OK and should be picked.

I'm adding Arend to cc, so can comment in case I misunderstood him
though.
This error doesn't affect actual wifi functionality. Only thing is wakeup on interrupt when system is in suspended state won't work.
I think, we can make below change. 

------------------
@@ -122,9 +122,11 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct sdio_mmc_card *card)
                                               IRQF_TRIGGER_LOW,
                                               "wifi_wake", cfg);
           if (ret) {
-                    dev_err(dev,
+                    dev_dbg(dev,
                             "Failed to request irq_wifi %d (%d)\n",
                             cfg->irq_wifi, ret);
+                    card->plt_wake_cfg = NULL;
+                    return 0;
             }
                disable_irq(cfg->irq_wifi);
            }
-----------------
Regards,
Amitkumar

Re: mwifiex: propagate error if IRQ request fails in mwifiex_sdio_of()

From: Javier Martinez Canillas <hidden>
Date: 2016-09-08 15:57:53

Hello Amitkumar,

On 09/08/2016 11:55 AM, Amitkumar Karwar wrote:
quoted hunk
Hi Javier,
quoted
From: Javier Martinez Canillas [mailto:javier@osg.samsung.com]
Sent: Tuesday, September 06, 2016 5:43 PM
To: Kalle Valo
Cc: linux-kernel@vger.kernel.org; Amitkumar Karwar;
netdev@vger.kernel.org; linux-wireless@vger.kernel.org; Nishant
Sarmukadam; Arend van Spriel
Subject: Re: mwifiex: propagate error if IRQ request fails in
mwifiex_sdio_of()

Hello Kalle,

On 09/03/2016 12:35 PM, Kalle Valo wrote:
quoted
Javier Martinez Canillas [off-list ref] wrote:
quoted
If request_irq() fails in mwifiex_sdio_probe_of(), only an error
message is printed but the actual error is not propagated to the
caller function.
quoted
quoted
Signed-off-by: Javier Martinez Canillas <redacted>
What's the conclusion with this patch? Should I drop it or take it?

(The discussion is available from the patchwork link in the
signature.)
My understanding is that Arend agrees with the patch and that the
question raised was caused by looking at an older kernel version. IOW,
the patch is OK and should be picked.

I'm adding Arend to cc, so can comment in case I misunderstood him
though.
This error doesn't affect actual wifi functionality. Only thing is wakeup on interrupt when system is in suspended state won't work.
I think, we can make below change. 

------------------
@@ -122,9 +122,11 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct sdio_mmc_card *card)
                                               IRQF_TRIGGER_LOW,
                                               "wifi_wake", cfg);
           if (ret) {
-                    dev_err(dev,
+                    dev_dbg(dev,
                             "Failed to request irq_wifi %d (%d)\n",
                             cfg->irq_wifi, ret);
+                    card->plt_wake_cfg = NULL;
+                    return 0;
             }
I'm OK with that change. Feel free too add my Reviewed-by if you post it.
                disable_irq(cfg->irq_wifi);
            }
-----------------

Regards,
Amitkumar
Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help