[U-Boot] [PATCH 1/1] i.MX31: switch to CFG_HZ=1000

[Guennadi Liakhovetski]

Hi Jean-Christophe,

On Tue, 16 Sep 2008, Jean-Christophe PLAGNIOL-VILLARD wrote:

> From: Guennadi Liakhovetski <lg at denx.de>
> 
> Switch to the standard CFG_HZ=1000 value, while at it, minor white-space
> cleanup, remove CFG_CLKS_IN_HZ from config-headers. Tested on mx31ads,
> provides 2% or 0.4% precision depending on the
> CONFIG_MX31_TIMER_HIGH_PRECISION flag. Measured with stop-watch on 100s
> boot-delay.
> 
> Signed-off-by: Guennadi Liakhovetski <lg at denx.de>

sorry, I haven't had time to reply to this email earlier. But it looks 
like many of Wolfgang's comments to your previous re-sending of this patch 
to the list still apply:

I still don't understand the purpose of this post. The patch has been 
posted by the author to the list, you are the subsystem custodian. I 
think, you have mainly three possibilities now:

1. you are happy with the patch, you add your Sob, pull it into your tree 
and ask Wolfgang to pull from it.

2. you have minor corrections to the patch. You may correct them yourself, 
add your Sob, re-post to the list and to the original author _specifying_ 
in the comments section, what you have changed and asking the original 
author if he agrees with the changes, if they are non-trivial.

3. comment back on the patch and ask the author to fix the issues and 
re-submit.

What you have done is you modified the patch, sent it back to the list and 
the author without your Sob and without explaining what and why you 
modified.

>  /* General purpose timers bitfields */
> -#define GPTCR_SWR       (1<<15) /* Software reset */
> -#define GPTCR_FRR       (1<<9)  /* Freerun / restart */
> -#define GPTCR_CLKSOURCE_32 (4<<6)  /* Clock source */
> -#define GPTCR_TEN       (1)     /* Timer enable */
> +#define GPTCR_SWR		(1 << 15)	/* Software reset	*/
> +#define GPTCR_FRR		(1 << 9)	/* Freerun / restart	*/
> +#define GPTCR_CLKSOURCE_32	(4 << 6)	/* Clock source		*/
> +#define GPTCR_TEN		(1)		/* Timer enable		*/

The original patch had

+#define GPTCR_TEN		1		/* Timer enable		*/

without the parenthesis. Your change is neither necessary - this is not a 
compound value like "4 << 6", thus it doesn't require parenthesis, nor I 
believe parenthesis make the define any better, which is a matter of 
personal preference, I think, and I don't think it is good to impose your 
personal likes and dislikes on other patch authors, as long as the 
original patch doesn't violate any coding style rules.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de