All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 linux-next] cpufreq: pxa: replace typedef pxa_freqs_t by structure
@ 2015-04-29 19:32 Fabian Frederick
  2015-04-30  2:47 ` Viresh Kumar
  2015-04-30  4:46 ` Joe Perches
  0 siblings, 2 replies; 5+ messages in thread
From: Fabian Frederick @ 2015-04-29 19:32 UTC (permalink / raw)
  To: linux-kernel; +Cc: Fabian Frederick, Rafael J. Wysocki, Viresh Kumar, linux-pm

typedef is not really useful here. Replace it by structure
to improve readability.typedef should only be used in some cases.
(See Documentation/CodingStyle Chapter 5 for details).

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
V2: verbose changelog.

 drivers/cpufreq/pxa2xx-cpufreq.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/cpufreq/pxa2xx-cpufreq.c b/drivers/cpufreq/pxa2xx-cpufreq.c
index e24269a..fcf6e34 100644
--- a/drivers/cpufreq/pxa2xx-cpufreq.c
+++ b/drivers/cpufreq/pxa2xx-cpufreq.c
@@ -56,7 +56,7 @@ module_param(pxa27x_maxfreq, uint, 0);
 MODULE_PARM_DESC(pxa27x_maxfreq, "Set the pxa27x maxfreq in MHz"
 		 "(typically 624=>pxa270, 416=>pxa271, 520=>pxa272)");
 
-typedef struct {
+struct pxa_freqs {
 	unsigned int khz;
 	unsigned int membus;
 	unsigned int cccr;
@@ -64,7 +64,7 @@ typedef struct {
 	unsigned int cclkcfg;
 	int vmin;
 	int vmax;
-} pxa_freqs_t;
+};
 
 /* Define the refresh period in mSec for the SDRAM and the number of rows */
 #define SDRAM_TREF	64	/* standard 64ms SDRAM */
@@ -86,7 +86,7 @@ static unsigned int sdram_rows;
 /* Use the run mode frequencies for the CPUFREQ_POLICY_PERFORMANCE policy */
 #define CCLKCFG			CCLKCFG_TURBO | CCLKCFG_FCS
 
-static pxa_freqs_t pxa255_run_freqs[] =
+static struct pxa_freqs pxa255_run_freqs[] =
 {
 	/* CPU   MEMBUS  CCCR  DIV2 CCLKCFG	           run  turbo PXbus SDRAM */
 	{ 99500,  99500, 0x121, 1,  CCLKCFG, -1, -1},	/*  99,   99,   50,   50  */
@@ -98,7 +98,7 @@ static pxa_freqs_t pxa255_run_freqs[] =
 };
 
 /* Use the turbo mode frequencies for the CPUFREQ_POLICY_POWERSAVE policy */
-static pxa_freqs_t pxa255_turbo_freqs[] =
+static struct pxa_freqs pxa255_turbo_freqs[] =
 {
 	/* CPU   MEMBUS  CCCR  DIV2 CCLKCFG	   run  turbo PXbus SDRAM */
 	{ 99500, 99500,  0x121, 1,  CCLKCFG, -1, -1},	/*  99,   99,   50,   50  */
@@ -153,7 +153,7 @@ MODULE_PARM_DESC(pxa255_turbo_table, "Selects the frequency table (0 = run table
    ((HT) ? CCLKCFG_HALFTURBO : 0) | \
    ((T)  ? CCLKCFG_TURBO : 0))
 
-static pxa_freqs_t pxa27x_freqs[] = {
+static struct pxa_freqs pxa27x_freqs[] = {
 	{104000, 104000, PXA27x_CCCR(1,	 8, 2), 0, CCLKCFG2(1, 0, 1),  900000, 1705000 },
 	{156000, 104000, PXA27x_CCCR(1,	 8, 3), 0, CCLKCFG2(1, 0, 1), 1000000, 1705000 },
 	{208000, 208000, PXA27x_CCCR(0, 16, 2), 1, CCLKCFG2(0, 0, 1), 1180000, 1705000 },
@@ -171,7 +171,7 @@ extern unsigned get_clk_frequency_khz(int info);
 
 #ifdef CONFIG_REGULATOR
 
-static int pxa_cpufreq_change_voltage(pxa_freqs_t *pxa_freq)
+static int pxa_cpufreq_change_voltage(struct pxa_freqs *pxa_freq)
 {
 	int ret = 0;
 	int vmin, vmax;
@@ -202,7 +202,7 @@ static void __init pxa_cpufreq_init_voltages(void)
 	}
 }
 #else
-static int pxa_cpufreq_change_voltage(pxa_freqs_t *pxa_freq)
+static int pxa_cpufreq_change_voltage(struct pxa_freqs *pxa_freq)
 {
 	return 0;
 }
@@ -211,7 +211,7 @@ static void __init pxa_cpufreq_init_voltages(void) { }
 #endif
 
 static void find_freq_tables(struct cpufreq_frequency_table **freq_table,
-			     pxa_freqs_t **pxa_freqs)
+			     struct pxa_freqs **pxa_freqs)
 {
 	if (cpu_is_pxa25x()) {
 		if (!pxa255_turbo_table) {
@@ -270,7 +270,7 @@ static unsigned int pxa_cpufreq_get(unsigned int cpu)
 static int pxa_set_target(struct cpufreq_policy *policy, unsigned int idx)
 {
 	struct cpufreq_frequency_table *pxa_freqs_table;
-	pxa_freqs_t *pxa_freq_settings;
+	struct pxa_freqs *pxa_freq_settings;
 	unsigned long flags;
 	unsigned int new_freq_cpu, new_freq_mem;
 	unsigned int unused, preset_mdrefr, postset_mdrefr, cclkcfg;
@@ -361,7 +361,7 @@ static int pxa_cpufreq_init(struct cpufreq_policy *policy)
 	int i;
 	unsigned int freq;
 	struct cpufreq_frequency_table *pxa255_freq_table;
-	pxa_freqs_t *pxa255_freqs;
+	struct pxa_freqs *pxa255_freqs;
 
 	/* try to guess pxa27x cpu */
 	if (cpu_is_pxa27x())
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH V2 linux-next] cpufreq: pxa: replace typedef pxa_freqs_t by structure
  2015-04-29 19:32 [PATCH V2 linux-next] cpufreq: pxa: replace typedef pxa_freqs_t by structure Fabian Frederick
@ 2015-04-30  2:47 ` Viresh Kumar
  2015-04-30  4:46 ` Joe Perches
  1 sibling, 0 replies; 5+ messages in thread
From: Viresh Kumar @ 2015-04-30  2:47 UTC (permalink / raw)
  To: Fabian Frederick; +Cc: Linux Kernel Mailing List, Rafael J. Wysocki, linux-pm

On 30 April 2015 at 01:02, Fabian Frederick <fabf@skynet.be> wrote:
> typedef is not really useful here. Replace it by structure
> to improve readability.typedef should only be used in some cases.

Space after full stop. (Maybe Rafael can fix that while applying)..

> (See Documentation/CodingStyle Chapter 5 for details).
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
> V2: verbose changelog.
>
>  drivers/cpufreq/pxa2xx-cpufreq.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V2 linux-next] cpufreq: pxa: replace typedef pxa_freqs_t by structure
  2015-04-29 19:32 [PATCH V2 linux-next] cpufreq: pxa: replace typedef pxa_freqs_t by structure Fabian Frederick
  2015-04-30  2:47 ` Viresh Kumar
@ 2015-04-30  4:46 ` Joe Perches
  2015-04-30 18:28   ` Fabian Frederick
  1 sibling, 1 reply; 5+ messages in thread
From: Joe Perches @ 2015-04-30  4:46 UTC (permalink / raw)
  To: Fabian Frederick; +Cc: linux-kernel, Rafael J. Wysocki, Viresh Kumar, linux-pm

On Wed, 2015-04-29 at 21:32 +0200, Fabian Frederick wrote:
> typedef is not really useful here. Replace it by structure
> to improve readability.typedef should only be used in some cases.
> (See Documentation/CodingStyle Chapter 5 for details).

trivia:

> diff --git a/drivers/cpufreq/pxa2xx-cpufreq.c b/drivers/cpufreq/pxa2xx-cpufreq.c
[]
> @@ -86,7 +86,7 @@ static unsigned int sdram_rows;
>  /* Use the run mode frequencies for the CPUFREQ_POLICY_PERFORMANCE policy */
>  #define CCLKCFG			CCLKCFG_TURBO | CCLKCFG_FCS
>  
> -static pxa_freqs_t pxa255_run_freqs[] =
> +static struct pxa_freqs pxa255_run_freqs[] =

Should these be const?

>  {
>  	/* CPU   MEMBUS  CCCR  DIV2 CCLKCFG	           run  turbo PXbus SDRAM */
>  	{ 99500,  99500, 0x121, 1,  CCLKCFG, -1, -1},	/*  99,   99,   50,   50  */
> @@ -98,7 +98,7 @@ static pxa_freqs_t pxa255_run_freqs[] =
>  };
>  
>  /* Use the turbo mode frequencies for the CPUFREQ_POLICY_POWERSAVE policy */
> -static pxa_freqs_t pxa255_turbo_freqs[] =
> +static struct pxa_freqs pxa255_turbo_freqs[] =
>  {
>  	/* CPU   MEMBUS  CCCR  DIV2 CCLKCFG	   run  turbo PXbus SDRAM */
>  	{ 99500, 99500,  0x121, 1,  CCLKCFG, -1, -1},	/*  99,   99,   50,   50  */
> @@ -153,7 +153,7 @@ MODULE_PARM_DESC(pxa255_turbo_table, "Selects the frequency table (0 = run table
>     ((HT) ? CCLKCFG_HALFTURBO : 0) | \
>     ((T)  ? CCLKCFG_TURBO : 0))
>  
> -static pxa_freqs_t pxa27x_freqs[] = {
> +static struct pxa_freqs pxa27x_freqs[] = {
>  	{104000, 104000, PXA27x_CCCR(1,	 8, 2), 0, CCLKCFG2(1, 0, 1),  900000, 1705000 },
>  	{156000, 104000, PXA27x_CCCR(1,	 8, 3), 0, CCLKCFG2(1, 0, 1), 1000000, 1705000 },
>  	{208000, 208000, PXA27x_CCCR(0, 16, 2), 1, CCLKCFG2(0, 0, 1), 1180000, 1705000 },
> @@ -171,7 +171,7 @@ extern unsigned get_clk_frequency_khz(int info);
>  
>  #ifdef CONFIG_REGULATOR
>  
> -static int pxa_cpufreq_change_voltage(pxa_freqs_t *pxa_freq)
> +static int pxa_cpufreq_change_voltage(struct pxa_freqs *pxa_freq)
>  {
>  	int ret = 0;
>  	int vmin, vmax;
> @@ -202,7 +202,7 @@ static void __init pxa_cpufreq_init_voltages(void)
>  	}
>  }
>  #else
> -static int pxa_cpufreq_change_voltage(pxa_freqs_t *pxa_freq)
> +static int pxa_cpufreq_change_voltage(struct pxa_freqs *pxa_freq)
>  {
>  	return 0;
>  }
> @@ -211,7 +211,7 @@ static void __init pxa_cpufreq_init_voltages(void) { }
>  #endif
>  
>  static void find_freq_tables(struct cpufreq_frequency_table **freq_table,
> -			     pxa_freqs_t **pxa_freqs)
> +			     struct pxa_freqs **pxa_freqs)
>  {
>  	if (cpu_is_pxa25x()) {
>  		if (!pxa255_turbo_table) {
> @@ -270,7 +270,7 @@ static unsigned int pxa_cpufreq_get(unsigned int cpu)
>  static int pxa_set_target(struct cpufreq_policy *policy, unsigned int idx)
>  {
>  	struct cpufreq_frequency_table *pxa_freqs_table;
> -	pxa_freqs_t *pxa_freq_settings;
> +	struct pxa_freqs *pxa_freq_settings;
>  	unsigned long flags;
>  	unsigned int new_freq_cpu, new_freq_mem;
>  	unsigned int unused, preset_mdrefr, postset_mdrefr, cclkcfg;
> @@ -361,7 +361,7 @@ static int pxa_cpufreq_init(struct cpufreq_policy *policy)
>  	int i;
>  	unsigned int freq;
>  	struct cpufreq_frequency_table *pxa255_freq_table;
> -	pxa_freqs_t *pxa255_freqs;
> +	struct pxa_freqs *pxa255_freqs;
>  
>  	/* try to guess pxa27x cpu */
>  	if (cpu_is_pxa27x())
> -- 
> 1.9.1
> 
> --
> 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/




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V2 linux-next] cpufreq: pxa: replace typedef pxa_freqs_t by structure
  2015-04-30  4:46 ` Joe Perches
@ 2015-04-30 18:28   ` Fabian Frederick
  2015-04-30 20:12     ` Rafael J. Wysocki
  0 siblings, 1 reply; 5+ messages in thread
From: Fabian Frederick @ 2015-04-30 18:28 UTC (permalink / raw)
  To: Joe Perches; +Cc: Rafael J. Wysocki, linux-kernel, Viresh Kumar, linux-pm



> On 30 April 2015 at 06:46 Joe Perches <joe@perches.com> wrote:
>
>
> On Wed, 2015-04-29 at 21:32 +0200, Fabian Frederick wrote:
> > typedef is not really useful here. Replace it by structure
> > to improve readability.typedef should only be used in some cases.
> > (See Documentation/CodingStyle Chapter 5 for details).
>
> trivia:
>
> > diff --git a/drivers/cpufreq/pxa2xx-cpufreq.c
> > b/drivers/cpufreq/pxa2xx-cpufreq.c
> []
> > @@ -86,7 +86,7 @@ static unsigned int sdram_rows;
> >  /* Use the run mode frequencies for the CPUFREQ_POLICY_PERFORMANCE policy
> >*/
> >  #define CCLKCFG                    CCLKCFG_TURBO | CCLKCFG_FCS
> > 
> > -static pxa_freqs_t pxa255_run_freqs[] =
> > +static struct pxa_freqs pxa255_run_freqs[] =
>
> Should these be const?

AFAICS yes but this needs some fixes:
drivers/cpufreq/pxa2xx-cpufreq.c: In function 'find_freq_tables':
drivers/cpufreq/pxa2xx-cpufreq.c:218:15: warning: assignment discards 'const'
qualifier from pointer target type
    *pxa_freqs = pxa255_run_freqs;
               ^
Maybe another patch ?

Regards,
Fabian
>
> >  {
> >     /* CPU   MEMBUS  CCCR  DIV2 CCLKCFG                run  turbo PXbus
> >SDRAM */
> >     { 99500,  99500, 0x121, 1,  CCLKCFG, -1, -1},   /*  99,   99,   50, 
> > 50  */
> > @@ -98,7 +98,7 @@ static pxa_freqs_t pxa255_run_freqs[] =
> >  };
> > 
> >  /* Use the turbo mode frequencies for the CPUFREQ_POLICY_POWERSAVE policy
> >*/
> > -static pxa_freqs_t pxa255_turbo_freqs[] =
> > +static struct pxa_freqs pxa255_turbo_freqs[] =
> >  {
> >     /* CPU   MEMBUS  CCCR  DIV2 CCLKCFG        run  turbo PXbus SDRAM */
> >     { 99500, 99500,  0x121, 1,  CCLKCFG, -1, -1},   /*  99,   99,   50, 
> > 50  */
> > @@ -153,7 +153,7 @@ MODULE_PARM_DESC(pxa255_turbo_table, "Selects the
> > frequency table (0 = run table
> >     ((HT) ? CCLKCFG_HALFTURBO : 0) | \
> >     ((T)  ? CCLKCFG_TURBO : 0))
> > 
> > -static pxa_freqs_t pxa27x_freqs[] = {
> > +static struct pxa_freqs pxa27x_freqs[] = {
> >     {104000, 104000, PXA27x_CCCR(1,  8, 2), 0, CCLKCFG2(1, 0, 1),  900000,
> >1705000 },
> >     {156000, 104000, PXA27x_CCCR(1,  8, 3), 0, CCLKCFG2(1, 0, 1), 1000000,
> >1705000 },
> >     {208000, 208000, PXA27x_CCCR(0, 16, 2), 1, CCLKCFG2(0, 0, 1), 1180000,
> >1705000 },
> > @@ -171,7 +171,7 @@ extern unsigned get_clk_frequency_khz(int info);
> > 
> >  #ifdef CONFIG_REGULATOR
> > 
> > -static int pxa_cpufreq_change_voltage(pxa_freqs_t *pxa_freq)
> > +static int pxa_cpufreq_change_voltage(struct pxa_freqs *pxa_freq)
> >  {
> >     int ret = 0;
> >     int vmin, vmax;
> > @@ -202,7 +202,7 @@ static void __init pxa_cpufreq_init_voltages(void)
> >     }
> >  }
> >  #else
> > -static int pxa_cpufreq_change_voltage(pxa_freqs_t *pxa_freq)
> > +static int pxa_cpufreq_change_voltage(struct pxa_freqs *pxa_freq)
> >  {
> >     return 0;
> >  }
> > @@ -211,7 +211,7 @@ static void __init pxa_cpufreq_init_voltages(void) { }
> >  #endif
> > 
> >  static void find_freq_tables(struct cpufreq_frequency_table **freq_table,
> > -                        pxa_freqs_t **pxa_freqs)
> > +                        struct pxa_freqs **pxa_freqs)
> >  {
> >     if (cpu_is_pxa25x()) {
> >             if (!pxa255_turbo_table) {
> > @@ -270,7 +270,7 @@ static unsigned int pxa_cpufreq_get(unsigned int cpu)
> >  static int pxa_set_target(struct cpufreq_policy *policy, unsigned int idx)
> >  {
> >     struct cpufreq_frequency_table *pxa_freqs_table;
> > -   pxa_freqs_t *pxa_freq_settings;
> > +   struct pxa_freqs *pxa_freq_settings;
> >     unsigned long flags;
> >     unsigned int new_freq_cpu, new_freq_mem;
> >     unsigned int unused, preset_mdrefr, postset_mdrefr, cclkcfg;
> > @@ -361,7 +361,7 @@ static int pxa_cpufreq_init(struct cpufreq_policy
> > *policy)
> >     int i;
> >     unsigned int freq;
> >     struct cpufreq_frequency_table *pxa255_freq_table;
> > -   pxa_freqs_t *pxa255_freqs;
> > +   struct pxa_freqs *pxa255_freqs;
> > 
> >     /* try to guess pxa27x cpu */
> >     if (cpu_is_pxa27x())
> > --
> > 1.9.1
> >
> > --
> > 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/
>
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V2 linux-next] cpufreq: pxa: replace typedef pxa_freqs_t by structure
  2015-04-30 18:28   ` Fabian Frederick
@ 2015-04-30 20:12     ` Rafael J. Wysocki
  0 siblings, 0 replies; 5+ messages in thread
From: Rafael J. Wysocki @ 2015-04-30 20:12 UTC (permalink / raw)
  To: Fabian Frederick; +Cc: Joe Perches, linux-kernel, Viresh Kumar, linux-pm

On Thursday, April 30, 2015 08:28:56 PM Fabian Frederick wrote:
> 
> > On 30 April 2015 at 06:46 Joe Perches <joe@perches.com> wrote:
> >
> >
> > On Wed, 2015-04-29 at 21:32 +0200, Fabian Frederick wrote:
> > > typedef is not really useful here. Replace it by structure
> > > to improve readability.typedef should only be used in some cases.
> > > (See Documentation/CodingStyle Chapter 5 for details).
> >
> > trivia:
> >
> > > diff --git a/drivers/cpufreq/pxa2xx-cpufreq.c
> > > b/drivers/cpufreq/pxa2xx-cpufreq.c
> > []
> > > @@ -86,7 +86,7 @@ static unsigned int sdram_rows;
> > >  /* Use the run mode frequencies for the CPUFREQ_POLICY_PERFORMANCE policy
> > >*/
> > >  #define CCLKCFG                    CCLKCFG_TURBO | CCLKCFG_FCS
> > > 
> > > -static pxa_freqs_t pxa255_run_freqs[] =
> > > +static struct pxa_freqs pxa255_run_freqs[] =
> >
> > Should these be const?
> 
> AFAICS yes but this needs some fixes:
> drivers/cpufreq/pxa2xx-cpufreq.c: In function 'find_freq_tables':
> drivers/cpufreq/pxa2xx-cpufreq.c:218:15: warning: assignment discards 'const'
> qualifier from pointer target type
>     *pxa_freqs = pxa255_run_freqs;
>                ^
> Maybe another patch ?

Yes.  One change at a time pretty please.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-04-30 19:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-29 19:32 [PATCH V2 linux-next] cpufreq: pxa: replace typedef pxa_freqs_t by structure Fabian Frederick
2015-04-30  2:47 ` Viresh Kumar
2015-04-30  4:46 ` Joe Perches
2015-04-30 18:28   ` Fabian Frederick
2015-04-30 20:12     ` Rafael J. Wysocki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.