mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Javier Martinez Canillas <javier@osg.samsung.com>
To: Julian Calaby <julian.calaby@gmail.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Xinming Hu <huxm@marvell.com>,
	Amitkumar Karwar <akarwar@marvell.com>,
	Kalle Valo <kvalo@codeaurora.org>,
	netdev <netdev@vger.kernel.org>,
	linux-wireless <linux-wireless@vger.kernel.org>,
	Nishant Sarmukadam <nishants@marvell.com>
Subject: Re: [PATCH 7/8] mwifiex: don't print an error if an optional DT property is missing
Date: Wed, 1 Jun 2016 09:51:30 -0400	[thread overview]
Message-ID: <ec09757e-302a-b341-6bf0-46737abb7976@osg.samsung.com> (raw)
In-Reply-To: <CAGRGNgUZ+mG_KzhiOzquB3qd-V_Pdbazag=nrFpG87mRQGmXLg@mail.gmail.com>

Hello Julian,

Thanks a lot for your feedback and reviews.

On 06/01/2016 12:20 AM, Julian Calaby wrote:
> Hi All,
> 
> On Sat, May 28, 2016 at 12:18 AM, Javier Martinez Canillas
> <javier@osg.samsung.com> wrote:
>> The Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt DT
>> binding document say that the "interrupts" property in the child node is
>> optional. So the property being missed shouldn't be treated as an error.
> 
> Have you checked whether it is truly optional? I.e. nothing else
> breaks if this property isn't set?
>

That's what the DT binding says and the IRQ is only used as a wakeup source
during system suspend, it is not used during runtime. And that is why the
mwifiex_sdio_probe_of() function does not fail if the IRQ is missing.

Now, I just got to that conclusion by reading the binding docs, the message
in the commits that introduced this and the driver code. Xinming Hu should
comment on how critical this feature is for systems that needs to be wakeup.

In any case I think that the code should be consistent with what the binding
doc says and also the function does (i.e: dev_err only if returns an error).
 
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> Other than that, this looks sensible to me.
> 
> Reviewed-by: Julian Calaby <julian.calaby@gmail.com>
> 

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

  reply	other threads:[~2016-06-01 13:51 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-27 14:18 [PATCH 0/8] mwifiex: Fix some error handling issues in mwifiex_sdio_probe() function Javier Martinez Canillas
2016-05-27 14:18 ` [PATCH 1/8] mwifiex: only call mwifiex_sdio_probe_of() if dev has an OF node Javier Martinez Canillas
2016-06-01  4:14   ` Julian Calaby
2016-06-16 15:05   ` [1/8] " Kalle Valo
2016-05-27 14:18 ` [PATCH 2/8] mwifiex: propagate sdio_enable_func() errno code in mwifiex_sdio_probe() Javier Martinez Canillas
2016-06-01  4:15   ` Julian Calaby
2016-05-27 14:18 ` [PATCH 3/8] mwifiex: propagate mwifiex_add_card() " Javier Martinez Canillas
2016-06-01  4:17   ` Julian Calaby
2016-05-27 14:18 ` [PATCH 4/8] mwifiex: consolidate mwifiex_sdio_probe() error paths Javier Martinez Canillas
2016-06-01  4:18   ` Julian Calaby
2016-05-27 14:18 ` [PATCH 5/8] mwifiex: use dev_err() instead of pr_err() in mwifiex_sdio_probe() Javier Martinez Canillas
2016-06-01  4:16   ` Julian Calaby
2016-05-27 14:18 ` [PATCH 6/8] mwifiex: check if mwifiex_sdio_probe_of() fails and return error Javier Martinez Canillas
2016-06-01  4:19   ` Julian Calaby
2016-05-27 14:18 ` [PATCH 7/8] mwifiex: don't print an error if an optional DT property is missing Javier Martinez Canillas
2016-06-01  4:20   ` Julian Calaby
2016-06-01 13:51     ` Javier Martinez Canillas [this message]
2016-06-01 23:13       ` Julian Calaby
2016-06-09 13:51         ` Amitkumar Karwar
2016-05-27 14:18 ` [PATCH 8/8] mwifiex: use better message and error code when OF node doesn't match Javier Martinez Canillas
2016-06-01  4:23   ` Julian Calaby
2016-05-30  9:57 ` [PATCH 0/8] mwifiex: Fix some error handling issues in mwifiex_sdio_probe() function Enric Balletbo Serra
2016-06-09 13:43 ` Amitkumar Karwar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ec09757e-302a-b341-6bf0-46737abb7976@osg.samsung.com \
    --to=javier@osg.samsung.com \
    --cc=akarwar@marvell.com \
    --cc=huxm@marvell.com \
    --cc=julian.calaby@gmail.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nishants@marvell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®