| Message ID | 1430335980-9765-1-git-send-email-fabf@skynet.be (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
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> -- 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 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/ -- 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 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/ > > > -- 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 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.
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())
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(-)