Re: [PATCH] rsi: update in boot parameters

[Kalle Valo <kvalo@codeaurora.org>]

Prameela Rani Garnepudi <prameela.j04cs@gmail.com> writes:

> Added more clock switch fields in boot parameters configured to device
>
> Signed-off-by: Prameela Rani Garnepudi <prameela.j04cs@gmail.com>

Why? How does this help?

>  			.tapll_info_g = {
> -				.pll_reg_1 = cpu_to_le16((TA_PLL_N_VAL_20 << 8)|
> -					      (TA_PLL_M_VAL_20)),
> -				.pll_reg_2 = cpu_to_le16(TA_PLL_P_VAL_20),
> +				.pll_reg_1 = cpu_to_le16((TAPLL_N_VAL_20 << 8) |
> +							 (TAPLL_M_VAL_20)),
> +				.pll_reg_2 = cpu_to_le16(TAPLL_P_VAL_20),

Doing changes from TA_PLL_N_VAL_20 to TAPLL_N_VAL_20 makes this hard to
review. Only one logical change per patch, please. And make style
changes separately from functional changes.

>  			},
>  			.pll960_info_g = {
> -				.pll_reg_1 = cpu_to_le16((PLL960_P_VAL_20 << 8)|
> +				.pll_reg_1 = cpu_to_le16((PLL960_P_VAL_20 << 8) |

Again unnecessary style change.

> -#define TA_PLL_M_VAL_20                  8
> -#define TA_PLL_N_VAL_20                  1
> -#define TA_PLL_P_VAL_20                  4
> +#define TAPLL_M_VAL_20			8
> +#define TAPLL_N_VAL_20			1
> +#define TAPLL_P_VAL_20			4

I don't really see the point with the rename. Changing the spaces to
tabs I understand, though.

But now that you are just starting don't waste time doing code style
changes, instead focus on fixing functionality or adding new features.
The style changes can be done later when you are more familiar with
everything.

I stopped now, please resend without the style changes so that it's
easier to review.

-- 
Kalle Valo