From: Shenwei Wang <Shenwei.Wang@freescale.com>
To: Sudeep Holla <sudeep.holla@arm.com>,
        "jason@lakedaemon.net" <jason@lakedaemon.net>
CC: "shawn.guo@linaro.org" <shawn.guo@linaro.org>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Huang Anson <Anson.Huang@freescale.com>
Subject: RE: [PATCH 1/1] irqchip: imx-gpcv2:  Simplify the implemenation
Thread-Topic: [PATCH 1/1] irqchip: imx-gpcv2:  Simplify the implemenation
Thread-Index: AQHQ4BbdeP3hNhpXOU+CwxPgWZ80Wp4ec9YAgAAHCeA=
Date: Wed, 26 Aug 2015 16:44:02 +0000
Message-ID: <CY1PR0301MB0843D36145639EB35E956CA383600@CY1PR0301MB0843.namprd03.prod.outlook.com>
References: <1440604166-2624-1-git-send-email-shenwei.wang@freescale.com>
 <55DDE56E.2040206@arm.com>
In-Reply-To: <55DDE56E.2040206@arm.com>
Accept-Language: zh-CN, en-US
Content-Language: en-US
Content-Type: text/plain; charset="gb2312"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Aug 2015 16:44:02.5749
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 710a03f5-10f6-4d38-9ff4-a80b81da590d
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR03MB1372
X-Microsoft-Exchange-Diagnostics: 1;BY1PR03MB1356;2:+0WnuFFSpeFFDeXoeoC8AXx8G9Piayexd24sLnTvgiR66rc4PHkOKndV3fTLsWnzUmYiBk30SM5gXaCR4uVSB5FkPoHBFhk9+NgQqoq1X01sEOKvZI9kyHxhau2oPeGrszMoZtKJ1aY4A+YefChE3fF/hr1ASGsMdJVZLEdc44E=;3:RXAB4t70iGHuJcNiVEVOwYDjGX6cvpCPQV9spFX2VMSXkNFns6M0diyswr3GxEY63rSgJM+bky9bi1jI8DM+PvWt2T10Gkf1Jhva77pq26eCnXYMSTHNrQCE+T4dlXTBK4y5QnNDQWYC32fn0tcNFw==;25:WfryRXiYhtft4jc4o333VnrXkHVpx0EmHiIXxWhbRHGAqMaoUHbjfdLuBrv1LKXB9OD5l8DvlapQ6FO7NkeSX3/U6V2Hy+CIvNJudZHdFXd2ItGnedeUYXbDxGqgNJuG1PC+DD1cqESnBC5q8fQeicDqPkC/4LyR3hvPCy+2kRGyZLSjFA50KQBvkpn69+8sCQcqpyKvMdlV5fAnVUWe4gDn6rCIiieaTkpg4ZEjxrgbnL55iSjrW1r8Fn534tuiKtaAntgM7LPdMw52rZHTXw==;23:Mrl343lLQWBMvdliHsVDWzHuMJ7m+HVNJit8j004BbdQB6UDauYpsvD/tOWfPNjXvFzuM3n/b0cr0oCqhN5lepp/eFFQ6Fk+wV9C7C/7tgrYVFaGh3p2mIL9KdJ25diqJ8+M3SBtWTEYGP27UnYBnl0WIeH2+SbZtTxc74JaCvDHJ6WKEqgyLXJRb1YwtDr+
X-OriginatorOrg: freescale.com
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id t7QGiGDU026491
Content-Length: 2641
Lines: 68


> -----Original Message-----
> From: Sudeep Holla [mailto:sudeep.holla@arm.com]
> Sent: 2015??8??26?? 11:13
> To: Wang Shenwei-B38339; jason@lakedaemon.net
> Cc: shawn.guo@linaro.org; tglx@linutronix.de; Sudeep Holla;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.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@freescale.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

The access to this function will only happen in suspend_ops so far.

Regards,
Shenwei


> Regards,
> Sudeep
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?