| Message ID | 09092df3d8b03df99ee475357c5f5c9cc439c61c.1409629117.git.viresh.kumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hello Viresh: I am not sure if this is right in CPU hotplug case. Hongtao: Please verify this in CPU hotplug case. Thanks, Yuantian > -----Original Message----- > From: Viresh Kumar [mailto:viresh.kumar@linaro.org] > Sent: Tuesday, September 02, 2014 11:41 AM > To: Rafael Wysocki; Tang Yuantian-B29983 > Cc: linaro-kernel@lists.linaro.org; linux-pm@vger.kernel.org; Zhang > Hongbo-B45939; Li Yang-Leo-R58472; Viresh Kumar > Subject: [PATCH 1/2] cpufreq: ppc-corenet: remove duplicate update of cpu_data > > 'cpu_data' is updated for policy->cpu first and then for all CPUs in > policy->cpus. policy->cpus is guaranteed to contain policy->cpu as well > policy->and so > the first write to 'cpu_data' for policy->cpu is redundant. Remove it. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > Hi Yuantian, > > I was looking into this driver due to issues reported by Hongtao (cc'd) and found > that we can live without some code. These aren't fixing any bugs and are just > cleanups. > > I didn't had a compiler for this and so this isn't even compiled. It would be great if > you can please review/test these patches. > > drivers/cpufreq/ppc-corenet-cpufreq.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c > b/drivers/cpufreq/ppc-corenet-cpufreq.c > index 3607070..bee5df7 100644 > --- a/drivers/cpufreq/ppc-corenet-cpufreq.c > +++ b/drivers/cpufreq/ppc-corenet-cpufreq.c > @@ -199,7 +199,6 @@ static int corenet_cpufreq_cpu_init(struct cpufreq_policy > *policy) > } > > data->table = table; > - per_cpu(cpu_data, cpu) = data; > > /* update ->cpus if we have cluster, no harm if not */ > cpumask_copy(policy->cpus, per_cpu(cpu_mask, cpu)); > -- > 2.0.3.693.g996b0fd -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2 September 2014 12:08, Yuantian Tang <Yuantian.Tang@freescale.com> wrote: > Hello Viresh: > > I am not sure if this is right in CPU hotplug case. policy->cpus must have all the online CPUs and so it should work for all online CPUs. what's your doubt? -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2 September 2014 09:11, Viresh Kumar <viresh.kumar@linaro.org> wrote: > 'cpu_data' is updated for policy->cpu first and then for all CPUs in > policy->cpus. policy->cpus is guaranteed to contain policy->cpu as well and so > the first write to 'cpu_data' for policy->cpu is redundant. Remove it. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > Hi Yuantian, > > I was looking into this driver due to issues reported by Hongtao (cc'd) and > found that we can live without some code. These aren't fixing any bugs and are > just cleanups. > > I didn't had a compiler for this and so this isn't even compiled. It would be > great if you can please review/test these patches. > > drivers/cpufreq/ppc-corenet-cpufreq.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c b/drivers/cpufreq/ppc-corenet-cpufreq.c > index 3607070..bee5df7 100644 > --- a/drivers/cpufreq/ppc-corenet-cpufreq.c > +++ b/drivers/cpufreq/ppc-corenet-cpufreq.c > @@ -199,7 +199,6 @@ static int corenet_cpufreq_cpu_init(struct cpufreq_policy *policy) > } > > data->table = table; > - per_cpu(cpu_data, cpu) = data; > > /* update ->cpus if we have cluster, no harm if not */ > cpumask_copy(policy->cpus, per_cpu(cpu_mask, cpu)); Hi Yuantian, Can we get your Ack for this patch atleast so that Rafael can apply it ? -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Viresh Kumar [mailto:viresh.kumar@linaro.org] > Sent: Wednesday, September 10, 2014 12:33 PM > To: Rafael Wysocki; Tang Yuantian-B29983 > Cc: Lists linaro-kernel; linux-pm@vger.kernel.org; Zhang Hongbo-B45939; Li > Yang-Leo-R58472; Viresh Kumar > Subject: Re: [PATCH 1/2] cpufreq: ppc-corenet: remove duplicate update of > cpu_data > > On 2 September 2014 09:11, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > 'cpu_data' is updated for policy->cpu first and then for all CPUs in > > policy->cpus. policy->cpus is guaranteed to contain policy->cpu as > > policy->well and so > > the first write to 'cpu_data' for policy->cpu is redundant. Remove it. > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > --- > > Hi Yuantian, > > > > I was looking into this driver due to issues reported by Hongtao > > (cc'd) and found that we can live without some code. These aren't > > fixing any bugs and are just cleanups. > > > > I didn't had a compiler for this and so this isn't even compiled. It > > would be great if you can please review/test these patches. > > > > drivers/cpufreq/ppc-corenet-cpufreq.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c > > b/drivers/cpufreq/ppc-corenet-cpufreq.c > > index 3607070..bee5df7 100644 > > --- a/drivers/cpufreq/ppc-corenet-cpufreq.c > > +++ b/drivers/cpufreq/ppc-corenet-cpufreq.c > > @@ -199,7 +199,6 @@ static int corenet_cpufreq_cpu_init(struct > cpufreq_policy *policy) > > } > > > > data->table = table; > > - per_cpu(cpu_data, cpu) = data; > > > > /* update ->cpus if we have cluster, no harm if not */ > > cpumask_copy(policy->cpus, per_cpu(cpu_mask, cpu)); > > Hi Yuantian, > > Can we get your Ack for this patch atleast so that Rafael can apply it ? Yeah, your patch should be ok, but is it the best solution? I was thinking about a final solution just like ARM did. Thanks, Yuantian
On 10 September 2014 11:00, Yuantian Tang <Yuantian.Tang@freescale.com> wrote: >> On 2 September 2014 09:11, Viresh Kumar <viresh.kumar@linaro.org> wrote: >> > 'cpu_data' is updated for policy->cpu first and then for all CPUs in >> > policy->cpus. policy->cpus is guaranteed to contain policy->cpu as >> > policy->well and so >> > the first write to 'cpu_data' for policy->cpu is redundant. Remove it. > Yeah, your patch should be ok, but is it the best solution? > I was thinking about a final solution just like ARM did. I was talking about an Ack for just 1/2 where we are doing some cleanup, which shouldn't break anything. Regarding the second patch, yes you should come up with something that can give a list of online+offline CPUs like ARM and even some ppc drivers. We can handle that separately and this patch can be commited by Rafael.. Either you can give a replacement patch for 2/2 or you can let me know what to do and will help there. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
T2ssIHN1cmUuDQoNClRoYW5rcywNCll1YW50aWFuDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdl LS0tLS0NCj4gRnJvbTogVmlyZXNoIEt1bWFyIFttYWlsdG86dmlyZXNoLmt1bWFyQGxpbmFyby5v cmddDQo+IFNlbnQ6IFdlZG5lc2RheSwgU2VwdGVtYmVyIDEwLCAyMDE0IDE6MzkgUE0NCj4gVG86 IFRhbmcgWXVhbnRpYW4tQjI5OTgzDQo+IENjOiBSYWZhZWwgV3lzb2NraTsgTGlzdHMgbGluYXJv LWtlcm5lbDsgbGludXgtcG1Admdlci5rZXJuZWwub3JnOyBaaGFuZw0KPiBIb25nYm8tQjQ1OTM5 OyBMaSBZYW5nLUxlby1SNTg0NzINCj4gU3ViamVjdDogUmU6IFtQQVRDSCAxLzJdIGNwdWZyZXE6 IHBwYy1jb3JlbmV0OiByZW1vdmUgZHVwbGljYXRlIHVwZGF0ZSBvZg0KPiBjcHVfZGF0YQ0KPiAN Cj4gT24gMTAgU2VwdGVtYmVyIDIwMTQgMTE6MDAsIFl1YW50aWFuIFRhbmcgPFl1YW50aWFuLlRh bmdAZnJlZXNjYWxlLmNvbT4NCj4gd3JvdGU6DQo+ID4+IE9uIDIgU2VwdGVtYmVyIDIwMTQgMDk6 MTEsIFZpcmVzaCBLdW1hciA8dmlyZXNoLmt1bWFyQGxpbmFyby5vcmc+IHdyb3RlOg0KPiA+PiA+ ICdjcHVfZGF0YScgaXMgdXBkYXRlZCBmb3IgcG9saWN5LT5jcHUgZmlyc3QgYW5kIHRoZW4gZm9y IGFsbCBDUFVzDQo+ID4+ID4gaW4NCj4gPj4gPiBwb2xpY3ktPmNwdXMuIHBvbGljeS0+Y3B1cyBp cyBndWFyYW50ZWVkIHRvIGNvbnRhaW4gcG9saWN5LT5jcHUgYXMNCj4gPj4gPiBwb2xpY3ktPndl bGwgYW5kIHNvDQo+ID4+ID4gdGhlIGZpcnN0IHdyaXRlIHRvICdjcHVfZGF0YScgZm9yIHBvbGlj eS0+Y3B1IGlzIHJlZHVuZGFudC4gUmVtb3ZlIGl0Lg0KPiANCj4gPiBZZWFoLCB5b3VyIHBhdGNo IHNob3VsZCBiZSBvaywgYnV0IGlzIGl0IHRoZSBiZXN0IHNvbHV0aW9uPw0KPiA+IEkgd2FzIHRo aW5raW5nIGFib3V0IGEgZmluYWwgc29sdXRpb24ganVzdCBsaWtlIEFSTSBkaWQuDQo+IA0KPiBJ IHdhcyB0YWxraW5nIGFib3V0IGFuIEFjayBmb3IganVzdCAxLzIgd2hlcmUgd2UgYXJlIGRvaW5n IHNvbWUgY2xlYW51cCwgd2hpY2gNCj4gc2hvdWxkbid0IGJyZWFrIGFueXRoaW5nLg0KPiANCj4g UmVnYXJkaW5nIHRoZSBzZWNvbmQgcGF0Y2gsIHllcyB5b3Ugc2hvdWxkIGNvbWUgdXAgd2l0aCBz b21ldGhpbmcgdGhhdCBjYW4NCj4gZ2l2ZSBhIGxpc3Qgb2Ygb25saW5lK29mZmxpbmUgQ1BVcyBs aWtlIEFSTSBhbmQgZXZlbiBzb21lIHBwYyBkcml2ZXJzLg0KPiBXZSBjYW4gaGFuZGxlIHRoYXQg c2VwYXJhdGVseSBhbmQgdGhpcyBwYXRjaCBjYW4gYmUgY29tbWl0ZWQgYnkgUmFmYWVsLi4NCj4g DQo+IEVpdGhlciB5b3UgY2FuIGdpdmUgYSByZXBsYWNlbWVudCBwYXRjaCBmb3IgMi8yIG9yIHlv dSBjYW4gbGV0IG1lIGtub3cgd2hhdCB0bw0KPiBkbyBhbmQgd2lsbCBoZWxwIHRoZXJlLg0K -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Viresh Kumar [mailto:viresh.kumar@linaro.org] > Sent: Tuesday, September 02, 2014 11:41 AM > To: Rafael Wysocki; Tang Yuantian-B29983 > Cc: linaro-kernel@lists.linaro.org; linux-pm@vger.kernel.org; Zhang > Hongbo-B45939; Li Yang-Leo-R58472; Viresh Kumar > Subject: [PATCH 1/2] cpufreq: ppc-corenet: remove duplicate update of cpu_data > > 'cpu_data' is updated for policy->cpu first and then for all CPUs in > policy->cpus. policy->cpus is guaranteed to contain policy->cpu as well > policy->and so > the first write to 'cpu_data' for policy->cpu is redundant. Remove it. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > Hi Yuantian, > > I was looking into this driver due to issues reported by Hongtao (cc'd) and found > that we can live without some code. These aren't fixing any bugs and are just > cleanups. > > I didn't had a compiler for this and so this isn't even compiled. It would be great if > you can please review/test these patches. > > drivers/cpufreq/ppc-corenet-cpufreq.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c > b/drivers/cpufreq/ppc-corenet-cpufreq.c > index 3607070..bee5df7 100644 > --- a/drivers/cpufreq/ppc-corenet-cpufreq.c > +++ b/drivers/cpufreq/ppc-corenet-cpufreq.c > @@ -199,7 +199,6 @@ static int corenet_cpufreq_cpu_init(struct cpufreq_policy > *policy) > } > > data->table = table; > - per_cpu(cpu_data, cpu) = data; > > /* update ->cpus if we have cluster, no harm if not */ > cpumask_copy(policy->cpus, per_cpu(cpu_mask, cpu)); > -- > 2.0.3.693.g996b0fd Acked-by: Tang Yuantian <Yuantian.Tang@freescale.com> Regards, Yuantian -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, September 02, 2014 09:11:24 AM Viresh Kumar wrote: > 'cpu_data' is updated for policy->cpu first and then for all CPUs in > policy->cpus. policy->cpus is guaranteed to contain policy->cpu as well and so > the first write to 'cpu_data' for policy->cpu is redundant. Remove it. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> I'm not sure about the status here? > --- > Hi Yuantian, > > I was looking into this driver due to issues reported by Hongtao (cc'd) and > found that we can live without some code. These aren't fixing any bugs and are > just cleanups. > > I didn't had a compiler for this and so this isn't even compiled. It would be > great if you can please review/test these patches. > > drivers/cpufreq/ppc-corenet-cpufreq.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c b/drivers/cpufreq/ppc-corenet-cpufreq.c > index 3607070..bee5df7 100644 > --- a/drivers/cpufreq/ppc-corenet-cpufreq.c > +++ b/drivers/cpufreq/ppc-corenet-cpufreq.c > @@ -199,7 +199,6 @@ static int corenet_cpufreq_cpu_init(struct cpufreq_policy *policy) > } > > data->table = table; > - per_cpu(cpu_data, cpu) = data; > > /* update ->cpus if we have cluster, no harm if not */ > cpumask_copy(policy->cpus, per_cpu(cpu_mask, cpu)); >
On 24 September 2014 16:45, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Tuesday, September 02, 2014 09:11:24 AM Viresh Kumar wrote: >> 'cpu_data' is updated for policy->cpu first and then for all CPUs in >> policy->cpus. policy->cpus is guaranteed to contain policy->cpu as well and so >> the first write to 'cpu_data' for policy->cpu is redundant. Remove it. >> >> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > I'm not sure about the status here? Apply only the first patch (1/2) with Acked-by: Tang Yuantian <Yuantian.Tang@freescale.com> -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday, September 29, 2014 02:05:15 AM Viresh Kumar wrote: > On 24 September 2014 16:45, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > On Tuesday, September 02, 2014 09:11:24 AM Viresh Kumar wrote: > >> 'cpu_data' is updated for policy->cpu first and then for all CPUs in > >> policy->cpus. policy->cpus is guaranteed to contain policy->cpu as well and so > >> the first write to 'cpu_data' for policy->cpu is redundant. Remove it. > >> > >> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > > > I'm not sure about the status here? > > Apply only the first patch (1/2) with > > Acked-by: Tang Yuantian <Yuantian.Tang@freescale.com> Please resend with the ACK.
diff --git a/drivers/cpufreq/ppc-corenet-cpufreq.c b/drivers/cpufreq/ppc-corenet-cpufreq.c index 3607070..bee5df7 100644 --- a/drivers/cpufreq/ppc-corenet-cpufreq.c +++ b/drivers/cpufreq/ppc-corenet-cpufreq.c @@ -199,7 +199,6 @@ static int corenet_cpufreq_cpu_init(struct cpufreq_policy *policy) } data->table = table; - per_cpu(cpu_data, cpu) = data; /* update ->cpus if we have cluster, no harm if not */ cpumask_copy(policy->cpus, per_cpu(cpu_mask, cpu));
'cpu_data' is updated for policy->cpu first and then for all CPUs in policy->cpus. policy->cpus is guaranteed to contain policy->cpu as well and so the first write to 'cpu_data' for policy->cpu is redundant. Remove it. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- Hi Yuantian, I was looking into this driver due to issues reported by Hongtao (cc'd) and found that we can live without some code. These aren't fixing any bugs and are just cleanups. I didn't had a compiler for this and so this isn't even compiled. It would be great if you can please review/test these patches. drivers/cpufreq/ppc-corenet-cpufreq.c | 1 - 1 file changed, 1 deletion(-)