netdev - Re: [PATCH 7/8] mwifiex: don't print an error if an optional DT property is missing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 1 Jun 2016 09:51:30 -0400
From:	Javier Martinez Canillas <javier@....samsung.com>
To:	Julian Calaby <julian.calaby@...il.com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Xinming Hu <huxm@...vell.com>,
	Amitkumar Karwar <akarwar@...vell.com>,
	Kalle Valo <kvalo@...eaurora.org>,
	netdev <netdev@...r.kernel.org>,
	linux-wireless <linux-wireless@...r.kernel.org>,
	Nishant Sarmukadam <nishants@...vell.com>
Subject: Re: [PATCH 7/8] mwifiex: don't print an error if an optional DT
 property is missing

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@....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@....samsung.com>
> 
> Other than that, this looks sensible to me.
> 
> Reviewed-by: Julian Calaby <julian.calaby@...il.com>
> 

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