On 23-10-19, 12:08, Sudeep Holla wrote:
> Currently the cpufreq core aborts the validation and return error
> immediately when it encounter duplicate frequency table entries.
> This change was introduced long back since commit da0c6dc00c69
> ("cpufreq: Handle sorted frequency tables more efficiently").
>
> However, this missed the testing with modified firmware for long time.
> Inorder to make it work with default settings, we need to ensure the
> merged table for bL switcher contains no duplicates. Find the duplicates
> and skip them when merging the frequenct tables of A15 and A7 clusters.
>
> Cc: Viresh Kumar <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Signed-off-by: Sudeep Holla <[email protected]>
> ---
> drivers/cpufreq/vexpress-spc-cpufreq.c | 35 ++++++++++++++++++++------
> 1 file changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c
> b/drivers/cpufreq/vexpress-spc-cpufreq.c
> index 093ef8d3a8d4..921dbd42b3bb 100644
> --- a/drivers/cpufreq/vexpress-spc-cpufreq.c
> +++ b/drivers/cpufreq/vexpress-spc-cpufreq.c
> @@ -242,6 +242,19 @@ static inline u32 get_table_max(struct
> cpufreq_frequency_table *table)
> return max_freq;
> }
>
> +static bool search_frequency(struct cpufreq_frequency_table *table, int size,
> + unsigned int freq)
> +{
> + int count;
> +
> + for (count = 0; count < size; count++) {
> + if (table[count].frequency == freq)
> + return true;
> + }
> +
> + return false;
> +}
> +
> static int merge_cluster_tables(void)
> {
> int i, j, k = 0, count = 1;
> @@ -256,13 +269,21 @@ static int merge_cluster_tables(void)
>
> freq_table[MAX_CLUSTERS] = table;
>
> - /* Add in reverse order to get freqs in increasing order */
> - for (i = MAX_CLUSTERS - 1; i >= 0; i--) {
> - for (j = 0; freq_table[i][j].frequency != CPUFREQ_TABLE_END;
> - j++, k++) {
> - table[k].frequency =
> - VIRT_FREQ(i, freq_table[i][j].frequency);
> - }
I think we can still use this single loop, which already starts from
A7 cluster. Just that we can add an if (A15) block inside it as the
first line.
> + /* Add A7_CLUSTER first to get freqs in increasing order */
> + for (j = 0; freq_table[A7_CLUSTER][j].frequency != CPUFREQ_TABLE_END;
> + j++, k++) {
> + table[k].frequency =
> + VIRT_FREQ(A7_CLUSTER,
> freq_table[A7_CLUSTER][j].frequency);
> + }
> + count = k;
> +
> + /* And then A15_CLUSTER checking for duplicates */
> + for (j = 0; freq_table[A15_CLUSTER][j].frequency != CPUFREQ_TABLE_END;
> + j++) {
> + if (search_frequency(table, count,
> + freq_table[A15_CLUSTER][j].frequency))
> + continue; /* skip duplicates */
> + table[k++].frequency = freq_table[A15_CLUSTER][j].frequency;
> }
How many duplicate entries are there anyway in the firmware? Or do we
really need to make it that generic? I mean, only the last of A7 and
first of A15 should be overlapping, in that case why search entire
table again ?
>
> table[k].driver_data = k;
> --
> 2.17.1
--
viresh
