Re: [PATCH] wl12xx: Set correct REF CLK and TCXO CLK values to the FW

[Luciano Coelho <coelho@ti.com>]

On Mon, 2011-03-28 at 09:34 +0200, Shahar Levi wrote:
> Set the correct values to the FW of REF CLK and TCXO CLK from
> wl12xx_platform_data to ini_general_params.
> 
> Signed-off-by: Shahar Levi <shahar_levi@ti.com>
> ---

Please rephrase the commit message.  Say why we need to do this.  The
reason is that we don't want to have a mismatch between the information
that is set in the platform data and the NVS, so we override what comes
from the NVS and replace it with what comes from the platform data.


> diff --git a/drivers/net/wireless/wl12xx/cmd.c b/drivers/net/wireless/wl12xx/cmd.c
> index 2468044..beb9f88 100644
> --- a/drivers/net/wireless/wl12xx/cmd.c
> +++ b/drivers/net/wireless/wl12xx/cmd.c
> @@ -129,6 +129,9 @@ int wl1271_cmd_general_parms(struct wl1271 *wl)
>  	if (gp->tx_bip_fem_auto_detect)
>  		answer = true;
>  
> +	/* Set the ref CLK value */
> +	gen_parms->general_params.ref_clock = wl->ref_clock;
> +

The comment is too obvious.  I think it's better if you say "Override
the REF CLK from the NVS with the one from platform data".


> @@ -168,6 +171,13 @@ int wl128x_cmd_general_parms(struct wl1271 *wl)
>  	if (gp->tx_bip_fem_auto_detect)
>  		answer = true;
>  
> +	/*
> +	 * Set the ref CLK & TCXO values.
> +	 * The HW will know which of them is the valid one due to boot setting.
> +	 */
> +	gen_parms->general_params.ref_clock = wl->ref_clock;
> +	gen_parms->general_params.tcxo_ref_clock = wl->tcxo_clock;
> +

Same here.  Please rewrite the comment.  And "The HW will know which
one..." is irrelevant here, please remove.

-- 
Cheers,
Luca.