| Message ID | 1397749021-2322-3-git-send-email-mauricfo@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Apr 17, 2014 at 12:37:00PM -0300, Mauricio Faria de Oliveira wrote: > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> > > The .driver_data field in the cpufreq_frequency_table was supposed to > be private to the drivers. However at some later point, it was being > used to indicate if the particular frequency in the table is the > BOOST_FREQUENCY. After patches [1] and [2], the .driver_data is once > again private to the driver. Thus we can safely use > cpufreq_frequency_table.driver_data to store pstate_ids instead of > having to maintain a separate array powernv_pstate_ids[] for this > purpose. > > [1]: > Subject: cpufreq: don't print value of .driver_data from core > From : Viresh Kumar <viresh.kumar@ linaro.org> > url : http://marc.info/?l=linux-pm&m=139601421504709&w=2 > > [2]: > Subject: cpufreq: create another field .flags in cpufreq_frequency_table > From : Viresh Kumar <viresh.kumar@linaro.org> > url : http://marc.info/?l=linux-pm&m=139601416804702&w=2 These two commits were merged by Linus in 3.15-rc1, and we don't have them in trusty. Aren't we going to need these too?
On 04/18/2014 07:12 AM, Seth Forshee wrote: > On Thu, Apr 17, 2014 at 12:37:00PM -0300, Mauricio Faria de Oliveira wrote: >> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> >> >> The .driver_data field in the cpufreq_frequency_table was supposed to >> be private to the drivers. However at some later point, it was being >> used to indicate if the particular frequency in the table is the >> BOOST_FREQUENCY. After patches [1] and [2], the .driver_data is once >> again private to the driver. Thus we can safely use >> cpufreq_frequency_table.driver_data to store pstate_ids instead of >> having to maintain a separate array powernv_pstate_ids[] for this >> purpose. >> >> [1]: >> Subject: cpufreq: don't print value of .driver_data from core >> From : Viresh Kumar <viresh.kumar@ linaro.org> >> url : http://marc.info/?l=linux-pm&m=139601421504709&w=2 >> >> [2]: >> Subject: cpufreq: create another field .flags in cpufreq_frequency_table >> From : Viresh Kumar <viresh.kumar@linaro.org> >> url : http://marc.info/?l=linux-pm&m=139601416804702&w=2 > > These two commits were merged by Linus in 3.15-rc1, and we don't have > them in trusty. Aren't we going to need these too? > Though mentioned in the commit log, those 2 patches appear to be independent. They were also merged afterwards, probably in a parallel merge branch. 7f4b04614a273089ad65654f53771c033fadc65e cpufreq: create another field .flags in cpufreq_frequency_table ae87f10f35f75deb8f74dbd92d062200932c2f26 cpufreq: don't print value of .driver_data from core 81f359027a3a383cc1dc79499ce97f1074861e5b cpufreq: powernv: Select CPUFreq related Kconfig options for powernv 0692c69138355fdbf32ecf70a2cde9c1fc3d7bb2 cpufreq: powernv: Use cpufreq_frequency_table.driver_data to store pstate ids b3d627a5f2bf1a9a486f65af6f7c2ce0e09b3d12 cpufreq: powernv: cpufreq driver for powernv platform rtg
On Fri, Apr 18, 2014 at 08:14:19AM -0600, Tim Gardner wrote: > On 04/18/2014 07:12 AM, Seth Forshee wrote: > >On Thu, Apr 17, 2014 at 12:37:00PM -0300, Mauricio Faria de Oliveira wrote: > >>From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> > >> > >>The .driver_data field in the cpufreq_frequency_table was supposed to > >>be private to the drivers. However at some later point, it was being > >>used to indicate if the particular frequency in the table is the > >>BOOST_FREQUENCY. After patches [1] and [2], the .driver_data is once > >>again private to the driver. Thus we can safely use > >>cpufreq_frequency_table.driver_data to store pstate_ids instead of > >>having to maintain a separate array powernv_pstate_ids[] for this > >>purpose. > >> > >>[1]: > >> Subject: cpufreq: don't print value of .driver_data from core > >> From : Viresh Kumar <viresh.kumar@ linaro.org> > >> url : http://marc.info/?l=linux-pm&m=139601421504709&w=2 > >> > >>[2]: > >> Subject: cpufreq: create another field .flags in cpufreq_frequency_table > >> From : Viresh Kumar <viresh.kumar@linaro.org> > >> url : http://marc.info/?l=linux-pm&m=139601416804702&w=2 > > > >These two commits were merged by Linus in 3.15-rc1, and we don't have > >them in trusty. Aren't we going to need these too? > > > > Though mentioned in the commit log, those 2 patches appear to be > independent. They were also merged afterwards, probably in a > parallel merge branch. > > 7f4b04614a273089ad65654f53771c033fadc65e cpufreq: create another > field .flags in cpufreq_frequency_table > ae87f10f35f75deb8f74dbd92d062200932c2f26 cpufreq: don't print value > of .driver_data from core > 81f359027a3a383cc1dc79499ce97f1074861e5b cpufreq: powernv: Select > CPUFreq related Kconfig options for powernv > 0692c69138355fdbf32ecf70a2cde9c1fc3d7bb2 cpufreq: powernv: Use > cpufreq_frequency_table.driver_data to store pstate ids > b3d627a5f2bf1a9a486f65af6f7c2ce0e09b3d12 cpufreq: powernv: cpufreq > driver for powernv platform Hmm, and also at second glance I guess neither of those patches actually affect anything which writes to driver_data. Furthermore it also appears that the "boost" code was added after 3.14, so those patches really aren't relevant to 3.13 anyway. Nevermind then.
On Fri, Apr 18, 2014 at 09:36:45AM -0500, Seth Forshee wrote: > On Fri, Apr 18, 2014 at 08:14:19AM -0600, Tim Gardner wrote: > > On 04/18/2014 07:12 AM, Seth Forshee wrote: > > >On Thu, Apr 17, 2014 at 12:37:00PM -0300, Mauricio Faria de Oliveira wrote: > > >>From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> > > >> > > >>The .driver_data field in the cpufreq_frequency_table was supposed to > > >>be private to the drivers. However at some later point, it was being > > >>used to indicate if the particular frequency in the table is the > > >>BOOST_FREQUENCY. After patches [1] and [2], the .driver_data is once > > >>again private to the driver. Thus we can safely use > > >>cpufreq_frequency_table.driver_data to store pstate_ids instead of > > >>having to maintain a separate array powernv_pstate_ids[] for this > > >>purpose. > > >> > > >>[1]: > > >> Subject: cpufreq: don't print value of .driver_data from core > > >> From : Viresh Kumar <viresh.kumar@ linaro.org> > > >> url : http://marc.info/?l=linux-pm&m=139601421504709&w=2 > > >> > > >>[2]: > > >> Subject: cpufreq: create another field .flags in cpufreq_frequency_table > > >> From : Viresh Kumar <viresh.kumar@linaro.org> > > >> url : http://marc.info/?l=linux-pm&m=139601416804702&w=2 > > > > > >These two commits were merged by Linus in 3.15-rc1, and we don't have > > >them in trusty. Aren't we going to need these too? > > > > > > > Though mentioned in the commit log, those 2 patches appear to be > > independent. They were also merged afterwards, probably in a > > parallel merge branch. > > > > 7f4b04614a273089ad65654f53771c033fadc65e cpufreq: create another > > field .flags in cpufreq_frequency_table > > ae87f10f35f75deb8f74dbd92d062200932c2f26 cpufreq: don't print value > > of .driver_data from core > > 81f359027a3a383cc1dc79499ce97f1074861e5b cpufreq: powernv: Select > > CPUFreq related Kconfig options for powernv > > 0692c69138355fdbf32ecf70a2cde9c1fc3d7bb2 cpufreq: powernv: Use > > cpufreq_frequency_table.driver_data to store pstate ids > > b3d627a5f2bf1a9a486f65af6f7c2ce0e09b3d12 cpufreq: powernv: cpufreq > > driver for powernv platform > > Hmm, and also at second glance I guess neither of those patches actually > affect anything which writes to driver_data. Furthermore it also appears > that the "boost" code was added after 3.14, so those patches really That should be after 3.13. > aren't relevant to 3.13 anyway. Nevermind then.
On Fri, Apr 18, 2014 at 08:12:01AM -0500, Seth Forshee wrote: > On Thu, Apr 17, 2014 at 12:37:00PM -0300, Mauricio Faria de Oliveira wrote: > > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> > > > > The .driver_data field in the cpufreq_frequency_table was supposed to > > be private to the drivers. However at some later point, it was being > > used to indicate if the particular frequency in the table is the > > BOOST_FREQUENCY. After patches [1] and [2], the .driver_data is once > > again private to the driver. Thus we can safely use > > cpufreq_frequency_table.driver_data to store pstate_ids instead of > > having to maintain a separate array powernv_pstate_ids[] for this > > purpose. > > > > [1]: > > Subject: cpufreq: don't print value of .driver_data from core > > From : Viresh Kumar <viresh.kumar@ linaro.org> > > url : http://marc.info/?l=linux-pm&m=139601421504709&w=2 > > > > [2]: > > Subject: cpufreq: create another field .flags in cpufreq_frequency_table > > From : Viresh Kumar <viresh.kumar@linaro.org> > > url : http://marc.info/?l=linux-pm&m=139601416804702&w=2 > > These two commits were merged by Linus in 3.15-rc1, and we don't have > them in trusty. Aren't we going to need these too? We need these two patches as well since the my patch which makes use of .driver_data field to record the pstates is safe only when these two patches are applied. -- Thanks and Regards gautham. >
On Mon, Apr 21, 2014 at 11:29:47AM +0530, Gautham R Shenoy wrote: > On Fri, Apr 18, 2014 at 08:12:01AM -0500, Seth Forshee wrote: > > On Thu, Apr 17, 2014 at 12:37:00PM -0300, Mauricio Faria de Oliveira wrote: > > > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> > > > > > > The .driver_data field in the cpufreq_frequency_table was supposed to > > > be private to the drivers. However at some later point, it was being > > > used to indicate if the particular frequency in the table is the > > > BOOST_FREQUENCY. After patches [1] and [2], the .driver_data is once > > > again private to the driver. Thus we can safely use > > > cpufreq_frequency_table.driver_data to store pstate_ids instead of > > > having to maintain a separate array powernv_pstate_ids[] for this > > > purpose. > > > > > > [1]: > > > Subject: cpufreq: don't print value of .driver_data from core > > > From : Viresh Kumar <viresh.kumar@ linaro.org> > > > url : http://marc.info/?l=linux-pm&m=139601421504709&w=2 > > > > > > [2]: > > > Subject: cpufreq: create another field .flags in cpufreq_frequency_table > > > From : Viresh Kumar <viresh.kumar@linaro.org> > > > url : http://marc.info/?l=linux-pm&m=139601416804702&w=2 > > > > These two commits were merged by Linus in 3.15-rc1, and we don't have > > them in trusty. Aren't we going to need these too? > > We need these two patches as well since the my patch which makes use > of .driver_data field to record the pstates is safe only when these > two patches are applied. I looked again. [1] could be backported, but it doesn't really seem necessary as it only affects a printk (i.e. read) of driver_data and thus shouldn't interfere with the use in your patch. [2] doesn't make sense for trusty though, because the boost support didn't appear until 3.14 (and thus there's no use of driver_data for storing CPUFREQ_BOOST_FREQ in trusty). I don't see anything else in the core cpufreq code in trusty which touches driver data in struct cpufreq_frequency_table, so I think we should be okay. Let me know if you find something I missed. Thanks, Seth
Hi Seth, On Wed, Apr 23, 2014 at 11:06:35AM -0500, Seth Forshee wrote: > On Mon, Apr 21, 2014 at 11:29:47AM +0530, Gautham R Shenoy wrote: > > On Fri, Apr 18, 2014 at 08:12:01AM -0500, Seth Forshee wrote: > > > On Thu, Apr 17, 2014 at 12:37:00PM -0300, Mauricio Faria de Oliveira wrote: > > > > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> > > > > > > > > The .driver_data field in the cpufreq_frequency_table was supposed to > > > > be private to the drivers. However at some later point, it was being > > > > used to indicate if the particular frequency in the table is the > > > > BOOST_FREQUENCY. After patches [1] and [2], the .driver_data is once > > > > again private to the driver. Thus we can safely use > > > > cpufreq_frequency_table.driver_data to store pstate_ids instead of > > > > having to maintain a separate array powernv_pstate_ids[] for this > > > > purpose. > > > > > > > > [1]: > > > > Subject: cpufreq: don't print value of .driver_data from core > > > > From : Viresh Kumar <viresh.kumar@ linaro.org> > > > > url : http://marc.info/?l=linux-pm&m=139601421504709&w=2 > > > > > > > > [2]: > > > > Subject: cpufreq: create another field .flags in cpufreq_frequency_table > > > > From : Viresh Kumar <viresh.kumar@linaro.org> > > > > url : http://marc.info/?l=linux-pm&m=139601416804702&w=2 > > > > > > These two commits were merged by Linus in 3.15-rc1, and we don't have > > > them in trusty. Aren't we going to need these too? > > > > We need these two patches as well since the my patch which makes use > > of .driver_data field to record the pstates is safe only when these > > two patches are applied. > > I looked again. [1] could be backported, but it doesn't really seem > necessary as it only affects a printk (i.e. read) of driver_data and > thus shouldn't interfere with the use in your patch. [2] doesn't make > sense for trusty though, because the boost support didn't appear until > 3.14 (and thus there's no use of driver_data for storing > CPUFREQ_BOOST_FREQ in trusty). I missed the fact that the kernel version against which this patch is to be backported is 3.13. I was under the impression that it was 3.14. Yes, you are right. The boost feature doesn't appear until 3.14 and we don't need to worry about [2]. > > I don't see anything else in the core cpufreq code in trusty which > touches driver data in struct cpufreq_frequency_table, so I think we > should be okay. Let me know if you find something I missed. No this is fine. > > Thanks, > Seth > -- Thanks and Regards gautham.
diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c index e1e5197..9edccc6 100644 --- a/drivers/cpufreq/powernv-cpufreq.c +++ b/drivers/cpufreq/powernv-cpufreq.c @@ -33,7 +33,6 @@ #define POWERNV_MAX_PSTATES 256 static struct cpufreq_frequency_table powernv_freqs[POWERNV_MAX_PSTATES+1]; -static int powernv_pstate_ids[POWERNV_MAX_PSTATES+1]; /* * Note: The set of pstates consists of contiguous integers, the @@ -112,7 +111,7 @@ static int init_powernv_pstates(void) pr_debug("PState id %d freq %d MHz\n", id, freq); powernv_freqs[i].frequency = freq * 1000; /* kHz */ - powernv_pstate_ids[i] = id; + powernv_freqs[i].driver_data = id; } /* End of list marker entry */ powernv_freqs[i].frequency = CPUFREQ_TABLE_END; @@ -283,7 +282,7 @@ static int powernv_cpufreq_target_index(struct cpufreq_policy *policy, { struct powernv_smp_call_data freq_data; - freq_data.pstate_id = powernv_pstate_ids[new_index]; + freq_data.pstate_id = powernv_freqs[new_index].driver_data; /* * Use smp_call_function to send IPI and execute the