linux-kernel - Re: [PATCH 1/1] irqchip: imx-gpcv2: Simplify the implemenation

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, 26 Aug 2015 18:10:53 +0100
From:	Sudeep Holla <sudeep.holla@....com>
To:	Shenwei Wang <Shenwei.Wang@...escale.com>
CC:	"jason@...edaemon.net" <jason@...edaemon.net>,
	Sudeep Holla <sudeep.holla@....com>,
	"shawn.guo@...aro.org" <shawn.guo@...aro.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Huang Anson <Anson.Huang@...escale.com>
Subject: Re: [PATCH 1/1] irqchip: imx-gpcv2:  Simplify the implemenation



On 26/08/15 17:44, Shenwei Wang wrote:
>
>
>> -----Original Message-----
>> From: Sudeep Holla [mailto:sudeep.holla@....com]
>> Sent: 2015年8月26日 11:13
>> To: Wang Shenwei-B38339; jason@...edaemon.net
>> Cc: shawn.guo@...aro.org; tglx@...utronix.de; Sudeep Holla;
>> linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org; Huang
>> Yongcai-B20788
>> Subject: Re: [PATCH 1/1] irqchip: imx-gpcv2: Simplify the implemenation
>>
>> typo in $subject
>
> Just noticed. Will change it.
>
>> On 26/08/15 16:49, Shenwei Wang wrote:
>>> Based on Sudeep Holla's review comments, the implementation can be
>>> simplified by using the two flags: IRQCHIP_SKIP_SET_WAKE and
>>> IRQCHIP_MASK_ON_SUSPEND. This patch enables the flags in the struct
>>> irq_chip and removes the unnecessory syscore_ops callbacks.
>>>
>>> Signed-off-by: Shenwei Wang <shenwei.wang@...escale.com>
>>> ---
>>>    drivers/irqchip/irq-imx-gpcv2.c | 83 +++++++----------------------------------
>>>    1 file changed, 13 insertions(+), 70 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-imx-gpcv2.c
>>> b/drivers/irqchip/irq-imx-gpcv2.c index 4a97afa..e25df78 100644
>>> --- a/drivers/irqchip/irq-imx-gpcv2.c
>>> +++ b/drivers/irqchip/irq-imx-gpcv2.c
>>> @@ -22,7 +22,6 @@ struct gpcv2_irqchip_data {
>>>    	struct raw_spinlock    rlock;
>>>    	void __iomem       *gpc_base;
>>>    	u32 wakeup_sources[IMR_NUM];
>>> -	u32 saved_irq_mask[IMR_NUM];
>>>    	u32 cpu2wakeup;
>>>    };
>>>
>>> @@ -30,79 +29,25 @@ static struct gpcv2_irqchip_data
>>> *imx_gpcv2_instance;
>>>
>>>    u32 imx_gpcv2_get_wakeup_source(u32 **sources)
>>
>> I assume this patch is against -next and I don't see any users of
>> imx_gpcv2_get_wakeup_source in -next.
>>
>> If possible I would avoid exposing this function by implementing suspend_ops just
>> as before(just saving raw h/w reg values and restoring then back on resume w/o
>> tagging them as wakeup mask though they might be indeed wakeup mask).
>>
>> In that way, this driver is self-contained and whatever imx code calls this function
>> will not have dependency on this driver, no ? Do you need access to
>> imx_gpcv2_get_wakeup_source too early in resume much before suspend_ops
>> resume ? I would like to see the user of that function to comment on that any
>> further.
>
> It is linux-next. The user is in the following patch which is under review.
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/361388.html
>

I got lost trying to follow through that 1000 odd line of code with lots
of register accesses. I couldn't understand much, so I gave up.

Such details are abstracted well and hidden in firmware with PSCI(good
that it's enforced in ARM64, hopefully ARM32 also sees more adoption).

So, for the parts adding those 2 flags and removing the unnecessary
code, you can add:
Reviewed-by: Sudeep Holla <sudeep.holla@....com>

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/