Re: [PATCH V2 linux-next] cpufreq: pxa: replace typedef pxa_freqs_t by structure

[Fabian Frederick]

> On 30 April 2015 at 06:46 Joe Perches <joe@xxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
> > 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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/