Hi Stephen,
Thanks for looking into this.
On 03/05/2015 09:58 PM, Stephen Boyd wrote:
> On 02/25, Georgi Djakov wrote:
>> diff --git a/drivers/clk/qcom/gcc-msm8916.c b/drivers/clk/qcom/gcc-msm8916.c
>> new file mode 100644
>> index 000000000000..810c38004520
>> --- /dev/null
>> +++ b/drivers/clk/qcom/gcc-msm8916.c
>> +
>> +#define P_XO 0
>> +#define P_GPLL0 1
>> +#define P_GPLL0_AUX 1
>> +#define P_BIMC 2
>> +#define P_GPLL1 2
>> +#define P_GPLL1_AUX 2
>> +#define P_GPLL2 3
>> +#define P_GPLL2_AUX 3
>> +#define P_SLEEP_CLK 3
>> +#define P_DSI0_PHYPLL_BYTE 2
>> +#define P_DSI0_PHYPLL_DSI 2
>
> Ok...
>
>> +
>> +static const u8 gcc_xo_gpll0_map[] = {
>> + [P_XO] = 0,
>> + [P_GPLL0] = 1,
>> +};
>> +
>> +static const char *gcc_xo_gpll0[] = {
>> + "xo",
>> + "gpll0_vote",
>> +};
>> +
>> +static const u8 gcc_xo_gpll0_bimc_map[] = {
>> + [P_XO] = 0,
>> + [P_GPLL0] = 1,
>> + [P_BIMC] = 2,
>> +};
>> +
>> +static const char *gcc_xo_gpll0_bimc[] = {
>> + "xo",
>> + "gpll0_vote",
>> + "bimc_pll_vote",
>> +};
>> +
>> +static const u8 gcc_xo_gpll0a_gpll1_gpll2a_map[] = {
>> + [P_XO] = 0,
>> + [P_GPLL0_AUX] = 3,
>> + [P_GPLL1] = 1,
>> + [P_GPLL2_AUX] = 2,
>> +};
>> +
>> +static const char *gcc_xo_gpll0a_gpll1_gpll2a[] = {
>> + "xo",
>> + "gpll0_vote",
>> + "gpll1_vote",
>> + "gpll2_vote",
>> +};
>> +
>> +static const u8 gcc_xo_gpll0_gpll2_map[] = {
>> + [P_XO] = 0,
>> + [P_GPLL0] = 1,
>> + [P_GPLL2] = 2,
>> +};
>> +
>> +static const char *gcc_xo_gpll0_gpll2[] = {
>> + "xo",
>> + "gpll0_vote",
>> + "gpll2_vote",
>> +};
>> +
>> +static const u8 gcc_xo_gpll0a_map[] = {
>> + [P_XO] = 0,
>> + [P_GPLL0_AUX] = 2,
>> +};
>> +
>> +static const char *gcc_xo_gpll0a[] = {
>> + "xo",
>> + "gpll0_vote",
>> +};
>> +
>> +static const u8 gcc_xo_gpll0_gpll1a_sleep_map[] = {
>> + [P_XO] = 0,
>> + [P_GPLL0] = 1,
>> + [P_GPLL1_AUX] = 2,
>> + [P_SLEEP_CLK] = 6,
>> +};
>> +
>> +static const char *gcc_xo_gpll0_gpll1a_sleep[] = {
>> + "xo",
>> + "gpll0_vote",
>> + "gpll1_vote",
>> + "sleep_clk",
>> +};
>> +
>> +static const u8 gcc_xo_gpll0_gpll1a_map[] = {
>> + [P_XO] = 0,
>> + [P_GPLL0] = 1,
>> + [P_GPLL1_AUX] = 2,
>> +};
>> +
>> +static const char *gcc_xo_gpll0_gpll1a[] = {
>> + "xo",
>> + "gpll0_vote",
>> + "gpll1_vote",
>> +};
>> +
>> +static const u8 gcc_xo_dsibyte_map[] = {
>> + [P_XO] = 0,
>> + [P_DSI0_PHYPLL_BYTE] = 2,
>> +};
>
> This table has a hole because P_XO is 0 and P_DSI0_PHYPLL_BYTE is
> 2. In clk_rcg2_get_parent() we'll just iterate over the number of
> parents and index into this map from 0 to number of parents which
> unfortunately won't work. Is it time to rethink that code and
> these index tables? It might be easier to just make the P_*
> defines into an enum and then iterate over a tuple of { P_* , hw
> mux index } instead. It would certainly make this type of problem
> go away. Some other map tables here have the same problem.
>
I am not sure that i understand your suggestion. It seems that
changing clk_rcg2_get_parent() and also the above map structs is
the best way to resolve this. Other solution requiring minimal
changes is to fill the holes with a magic value, that we will skip
when iterating, but this is not elegant at all.
BR,
Georgi
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
