RE: [PATCH 1/1] irqchip: imx-gpcv2: Simplify the implemenation

[Shenwei Wang]

> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@xxxxxxxxxxxxx]
> Sent: 2015年8月26日 11:00
> To: Wang Shenwei-B38339
> Cc: shawn.guo@xxxxxxxxxx; jason@xxxxxxxxxxxxxx; sudeep.holla@xxxxxxx;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Huang
> Yongcai-B20788
> Subject: Re: [PATCH 1/1] irqchip: imx-gpcv2: Simplify the implemenation
> 
> On Wed, 26 Aug 2015, Shenwei Wang wrote:
> >  u32 imx_gpcv2_get_wakeup_source(u32 **sources)  {
> > -	if (!imx_gpcv2_instance)
> > +	struct gpcv2_irqchip_data *cd;
> > +	void __iomem *reg;
> > +	int i;
> > +
> > +	cd = imx_gpcv2_instance;
> > +	if (!cd)
> >  		return 0;
> >
> > +	for (i = 0; i < IMR_NUM; i++) {
> > +		reg = cd->gpc_base + cd->cpu2wakeup + i * 4;
> > +		cd->wakeup_sources[i] = readl_relaxed(reg);
> > +	}
> > +
> >  	if (sources)
> > -		*sources = imx_gpcv2_instance->wakeup_sources;
> > +		*sources = cd->wakeup_sources;
> >
> >  	return IMR_NUM;
> 
> You do not need the intermediate storage at all.
> 
> u32 imx_gpcv2_get_wakeup_source(u32 *sources) {
> 	if (sources) {
> 		for (i = 0; i < IMR_NUM; i++) {
> 			reg = cd->gpc_base + cd->cpu2wakeup + i * 4;
> 			sources[i] = readl_relaxed(reg);
> 		}
> 	}
> 	....

Using the intermediate storage here can make the caller a little easier,
because the caller does not need to malloc the memory before the call.
Especially the caller does not even know how many memory to allocate
In the beginning.

Thanks,
Shenwei

> Hmm?