linux-kernel - Re: [PATCH v2 03/10] mfd: wcd9335: add wcd irq support

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]

Message-ID: <26123a18-ad12-b654-c3bf-4faba0aa0646@linaro.org>
Date:   Thu, 2 Aug 2018 11:07:29 +0100
From:   Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     robh+dt@...nel.org, broonie@...nel.org, mark.rutland@....com,
        lgirdwood@...il.com, tiwai@...e.com, bgoswami@...eaurora.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        vkoul@...nel.org, alsa-devel@...a-project.org
Subject: Re: [PATCH v2 03/10] mfd: wcd9335: add wcd irq support



On 02/08/18 11:01, Lee Jones wrote:
>>> Any particular reason for separating out IRQ handling?
>> No particular reason, I tried to follow what other codecs like wm8994,
>> wm8350 and 14 others do.
> I haven't look at them in a while, but maybe theirs are optional?
> 
> Either way, I don't think you need to do it.
Sure, I can merge them into core in next version.
> 
>>>>    include/dt-bindings/mfd/wcd9335.h   |  43 +++++++++
>>>>    include/linux/mfd/wcd9335/wcd9335.h |   3 +
>>>>    5 files changed, 228 insertions(+), 1 deletion(-)
>>>>    create mode 100644 drivers/mfd/wcd9335-irq.c
>>>>    create mode 100644 include/dt-bindings/mfd/wcd9335.h
>>>>
>>>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>>>> index a4697370640b..210875afe78a 100644
>>>> --- a/drivers/mfd/Makefile
>>>> +++ b/drivers/mfd/Makefile
>>>> @@ -58,7 +58,7 @@ obj-$(CONFIG_MFD_ARIZONA)	+= cs47l24-tables.o
>>>>    endif
>>>>    obj-$(CONFIG_MFD_WCD9335)	+= wcd9335.o
>>>> -wcd9335-objs			:= wcd9335-core.o
>>>> +wcd9335-objs			:= wcd9335-core.o wcd9335-irq.o
>>>>    obj-$(CONFIG_MFD_WM8400)	+= wm8400-core.o
>>>>    wm831x-objs			:= wm831x-core.o wm831x-irq.o wm831x-otp.o
>>>> diff --git a/drivers/mfd/wcd9335-core.c b/drivers/mfd/wcd9335-core.c
>>>> index 8f746901f4e9..6299dfb63aca 100644
>>>> --- a/drivers/mfd/wcd9335-core.c
>>>> +++ b/drivers/mfd/wcd9335-core.c
>>>> @@ -243,12 +243,20 @@ static int wcd9335_slim_status(struct slim_device *sdev,
>>>>    		return ret;
>>>>    	}
>>>> +	wcd9335_irq_init(wcd);
>>> Why don't you check the return value?
>>>
>>> What happens if it defers?
>>>
>> It would not defer here as the regmaps are already setup in probe and the
>> status callback is only invoked when the SLIMbus device is up.
> I guess now you have seen my comment below, you know that it can
> defer, right?

Yes, I see it can defer on gpios.

> 
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +// Copyright (c) 2018, Linaro Limited
>>>> +//
>>> Blank line?
>>>
>> Yep.
> Does "yep" mean, "I'll fix it"?
Yes, I mean I agree with you and will fix this in next version.

--srini
> 
> --