Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1214001 > unrolled thread

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

Started byThomas Gleixner <tglx@linutronix.de>
First post2015-08-26 18:10 +0200
Last post2015-08-26 21:10 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/1] irqchip: imx-gpcv2: Simplify the implemenation Thomas Gleixner <tglx@linutronix.de> - 2015-08-26 18:10 +0200
    RE: [PATCH 1/1] irqchip: imx-gpcv2: Simplify the implemenation Shenwei Wang <Shenwei.Wang@freescale.com> - 2015-08-26 18:40 +0200
      RE: [PATCH 1/1] irqchip: imx-gpcv2: Simplify the implemenation Thomas Gleixner <tglx@linutronix.de> - 2015-08-26 21:00 +0200
        RE: [PATCH 1/1] irqchip: imx-gpcv2: Simplify the implemenation Shenwei Wang <Shenwei.Wang@freescale.com> - 2015-08-26 21:10 +0200

#1214001 — Re: [PATCH 1/1] irqchip: imx-gpcv2: Simplify the implemenation

FromThomas Gleixner <tglx@linutronix.de>
Date2015-08-26 18:10 +0200
SubjectRe: [PATCH 1/1] irqchip: imx-gpcv2: Simplify the implemenation
Message-ID<q1LqX-2d7-67@gated-at.bofh.it>
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);
		}
	}
	....

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

[toc] | [next] | [standalone]

#1214027

FromShenwei Wang <Shenwei.Wang@freescale.com>
Date2015-08-26 18:40 +0200
Message-ID<q1LTX-2LN-1@gated-at.bofh.it>
In reply to#1214001
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogVGhvbWFzIEdsZWl4bmVy
IFttYWlsdG86dGdseEBsaW51dHJvbml4LmRlXQ0KPiBTZW50OiAyMDE1xOo41MIyNsjVIDExOjAw
DQo+IFRvOiBXYW5nIFNoZW53ZWktQjM4MzM5DQo+IENjOiBzaGF3bi5ndW9AbGluYXJvLm9yZzsg
amFzb25AbGFrZWRhZW1vbi5uZXQ7IHN1ZGVlcC5ob2xsYUBhcm0uY29tOw0KPiBsaW51eC1hcm0t
a2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmc7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7
IEh1YW5nDQo+IFlvbmdjYWktQjIwNzg4DQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggMS8xXSBpcnFj
aGlwOiBpbXgtZ3BjdjI6IFNpbXBsaWZ5IHRoZSBpbXBsZW1lbmF0aW9uDQo+IA0KPiBPbiBXZWQs
IDI2IEF1ZyAyMDE1LCBTaGVud2VpIFdhbmcgd3JvdGU6DQo+ID4gIHUzMiBpbXhfZ3BjdjJfZ2V0
X3dha2V1cF9zb3VyY2UodTMyICoqc291cmNlcykgIHsNCj4gPiAtCWlmICghaW14X2dwY3YyX2lu
c3RhbmNlKQ0KPiA+ICsJc3RydWN0IGdwY3YyX2lycWNoaXBfZGF0YSAqY2Q7DQo+ID4gKwl2b2lk
IF9faW9tZW0gKnJlZzsNCj4gPiArCWludCBpOw0KPiA+ICsNCj4gPiArCWNkID0gaW14X2dwY3Yy
X2luc3RhbmNlOw0KPiA+ICsJaWYgKCFjZCkNCj4gPiAgCQlyZXR1cm4gMDsNCj4gPg0KPiA+ICsJ
Zm9yIChpID0gMDsgaSA8IElNUl9OVU07IGkrKykgew0KPiA+ICsJCXJlZyA9IGNkLT5ncGNfYmFz
ZSArIGNkLT5jcHUyd2FrZXVwICsgaSAqIDQ7DQo+ID4gKwkJY2QtPndha2V1cF9zb3VyY2VzW2ld
ID0gcmVhZGxfcmVsYXhlZChyZWcpOw0KPiA+ICsJfQ0KPiA+ICsNCj4gPiAgCWlmIChzb3VyY2Vz
KQ0KPiA+IC0JCSpzb3VyY2VzID0gaW14X2dwY3YyX2luc3RhbmNlLT53YWtldXBfc291cmNlczsN
Cj4gPiArCQkqc291cmNlcyA9IGNkLT53YWtldXBfc291cmNlczsNCj4gPg0KPiA+ICAJcmV0dXJu
IElNUl9OVU07DQo+IA0KPiBZb3UgZG8gbm90IG5lZWQgdGhlIGludGVybWVkaWF0ZSBzdG9yYWdl
IGF0IGFsbC4NCj4gDQo+IHUzMiBpbXhfZ3BjdjJfZ2V0X3dha2V1cF9zb3VyY2UodTMyICpzb3Vy
Y2VzKSB7DQo+IAlpZiAoc291cmNlcykgew0KPiAJCWZvciAoaSA9IDA7IGkgPCBJTVJfTlVNOyBp
KyspIHsNCj4gCQkJcmVnID0gY2QtPmdwY19iYXNlICsgY2QtPmNwdTJ3YWtldXAgKyBpICogNDsN
Cj4gCQkJc291cmNlc1tpXSA9IHJlYWRsX3JlbGF4ZWQocmVnKTsNCj4gCQl9DQo+IAl9DQo+IAku
Li4uDQoNClVzaW5nIHRoZSBpbnRlcm1lZGlhdGUgc3RvcmFnZSBoZXJlIGNhbiBtYWtlIHRoZSBj
YWxsZXIgYSBsaXR0bGUgZWFzaWVyLA0KYmVjYXVzZSB0aGUgY2FsbGVyIGRvZXMgbm90IG5lZWQg
dG8gbWFsbG9jIHRoZSBtZW1vcnkgYmVmb3JlIHRoZSBjYWxsLg0KRXNwZWNpYWxseSB0aGUgY2Fs
bGVyIGRvZXMgbm90IGV2ZW4ga25vdyBob3cgbWFueSBtZW1vcnkgdG8gYWxsb2NhdGUNCkluIHRo
ZSBiZWdpbm5pbmcuDQoNClRoYW5rcywNClNoZW53ZWkNCg0KPiBIbW0/DQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1214134

FromThomas Gleixner <tglx@linutronix.de>
Date2015-08-26 21:00 +0200
Message-ID<q1O5s-5Pg-23@gated-at.bofh.it>
In reply to#1214027
On Wed, 26 Aug 2015, Shenwei Wang wrote:
> > From: Thomas Gleixner [mailto:tglx@linutronix.de]
> > > +	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.

Fair enough, but why do you need that case where sources can be NULL
just to return IMR_NUM?

Thanks,

	tglx

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

[toc] | [prev] | [next] | [standalone]

#1214143

FromShenwei Wang <Shenwei.Wang@freescale.com>
Date2015-08-26 21:10 +0200
Message-ID<q1Of8-6fF-13@gated-at.bofh.it>
In reply to#1214134

> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@linutronix.de]
> > > > +
> > > >  	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.
> 
> Fair enough, but why do you need that case where sources can be NULL just to
> return IMR_NUM?

The intention is to get the IMR_NUM only. But so far no user uses this feature.

Thanks,
Shenwei

> Thanks,
> 
> 	tglx

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web