linux-kernel - Re: [PATCH v2] clk: qcom: Add MSM8916 Global Clock Controller support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <54FDD7C1.6090704@linaro.org>
Date:	Mon, 09 Mar 2015 19:26:25 +0200
From:	Georgi Djakov <georgi.djakov@...aro.org>
To:	Stephen Boyd <sboyd@...eaurora.org>
CC:	mturquette@...aro.org, linux-kernel@...r.kernel.org,
	linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v2] clk: qcom: Add MSM8916 Global Clock Controller support

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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/