Re: [U-Boot] [PATCH 4/4] Environment in MMC

Sudhakar Rajashekhara wrote:
Hi Alagu,
Hi,
On Wed, May 12, 2010 at 15:08:27, Alagu Sankar wrote:
This patch is to save environment data to mmc card. It uses interfaces defined in generic MMC framework. This is enabled with CONFIG_ENV_IS_IN_MMC option. Based on the earlier patch from Terry Lv at Freescale
Should be not better you send your comments to Terry as to generate a new patch? His patch had already passed some reviews and he can integrate your comments. I miss here what you changed respect Terry, and you miss some comments in the previous reviews, for example regarding the usage of the ALIGN macro. Why do you need two patches for the same purpose ?
Why this has been moved up? If there is any genuine reason for this, then you can mention it in patch description.
That is correct. The mmc must be initialized before env_init to get the environment working.
Best regards, Stefano Babic

Stefano Babic wrote:
Sudhakar Rajashekhara wrote:
Hi Alagu,
Hi,
On Wed, May 12, 2010 at 15:08:27, Alagu Sankar wrote:
This patch is to save environment data to mmc card. It uses interfaces defined in generic MMC framework. This is enabled with CONFIG_ENV_IS_IN_MMC option. Based on the earlier patch from Terry Lv at Freescale
Should be not better you send your comments to Terry as to generate a new patch? His patch had already passed some reviews and he can integrate your comments. I miss here what you changed respect Terry, and you miss some comments in the previous reviews, for example regarding the usage of the ALIGN macro. Why do you need two patches for the same purpose ?
Why this has been moved up? If there is any genuine reason for this, then you can mention it in patch description.
That is correct. The mmc must be initialized before env_init to get the environment working.
Best regards, Stefano Babic
When I searched for the Environment support in MMC, I came across Terry's earlier patches, but they were not cleanly getting applied to the current tree. There is no value add here except fixing the patch errors and generating a new patch for the current tree. So there is no question of adding any copyright here. Terry has indicated an update to his patch already. I will be resubmitting my other patches with reference to Terry's latest patch.
- Alagu Sankar

Alagu Sankar wrote:
When I searched for the Environment support in MMC, I came across Terry's earlier patches, but they were not cleanly getting applied to the current tree.
I have tried to apply again last Terry's patch, I see only a couple of coding-style problems, no errors at all:
Applying: Save environment data to mmc. /home/stefano/Projects/imx/u-boot-imx/.git/rebase-apply/patch:21: trailing whitespace. * Thus It is required that operations like pin multiplexer /home/stefano/Projects/imx/u-boot-imx/.git/rebase-apply/patch:54: trailing whitespace. * Thus It is required that operations like pin multiplexer warning: 2 lines add whitespace errors.
There is no value add here except fixing the patch errors and generating a new patch for the current tree.
Probably not needed. I have not seen the errors you reported. Are you sure you have tested with Terry's last patch ? It seems you submit an earlier version.
So there is no question of adding any copyright here.
Yes, Wolfgang has already answered. No need to add a copyright for small patches.
Best regards, Stefano Babic

Stefano Babic wrote:
Alagu Sankar wrote:
When I searched for the Environment support in MMC, I came across Terry's earlier patches, but they were not cleanly getting applied to the current tree.
I have tried to apply again last Terry's patch, I see only a couple of coding-style problems, no errors at all:
Applying: Save environment data to mmc. /home/stefano/Projects/imx/u-boot-imx/.git/rebase-apply/patch:21: trailing whitespace.
- Thus It is required that operations like pin multiplexer
/home/stefano/Projects/imx/u-boot-imx/.git/rebase-apply/patch:54: trailing whitespace.
- Thus It is required that operations like pin multiplexer
warning: 2 lines add whitespace errors.
There is no value add here except fixing the patch errors and generating a new patch for the current tree.
Probably not needed. I have not seen the errors you reported. Are you sure you have tested with Terry's last patch ? It seems you submit an earlier version.
So there is no question of adding any copyright here.
Yes, Wolfgang has already answered. No need to add a copyright for small patches.
Best regards, Stefano Babic
I was referring to Terry's old patches, and not the v5 patch he has submitted recently. I started on this some time back. Its a mistake that I did not check the updates that happened recently, before submitting the patches.
Best Regards, Alagu Sankar

Hi,
I've just send v6 version for review which fixes the coding-style problems.
Thanks~~
Yours Terry
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: 2010年5月14日 19:41 To: Alagu Sankar Cc: Stefano Babic; Sudhakar Rajashekhara; u-boot@lists.denx.de; Lv Terry-R65388 Subject: Re: [U-Boot] [PATCH 4/4] Environment in MMC
Alagu Sankar wrote:
When I searched for the Environment support in MMC, I came across Terry's earlier patches, but they were not cleanly getting
applied to
the current tree.
I have tried to apply again last Terry's patch, I see only a couple of coding-style problems, no errors at all:
Applying: Save environment data to mmc. /home/stefano/Projects/imx/u-boot-imx/.git/rebase-apply/patch:21: trailing whitespace.
- Thus It is required that operations like pin multiplexer
/home/stefano/Projects/imx/u-boot-imx/.git/rebase-apply/patch:54: trailing whitespace.
- Thus It is required that operations like pin multiplexer
warning: 2 lines add whitespace errors.
There is no value add here except fixing the patch errors and generating a new patch for the current tree.
Probably not needed. I have not seen the errors you reported. Are you sure you have tested with Terry's last patch ? It seems you submit an earlier version.
So there is no question of adding any copyright here.
Yes, Wolfgang has already answered. No need to add a copyright for small patches.
Best regards, Stefano Babic
--
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@denx.de =====================================================================
participants (3)
-
Alagu Sankar
-
Lv Terry-R65388
-
Stefano Babic