Patchwork [U-Boot,PULL] please pull u-boot-atmel/master

login
register
mail settings
Submitter Andreas Bießmann
Date Nov. 5, 2012, 10:19 a.m.
Message ID <1352110770-7198-1-git-send-email-andreas.devel@googlemail.com>
Download mbox
Permalink /patch/197176/
State Rejected
Delegated to: Albert ARIBAUD
Headers show

Pull-request

git://git.denx.de/u-boot-atmel.git master

Comments

Andreas Bießmann - Nov. 5, 2012, 10:19 a.m.
Dear Albert Aribaud,

The following changes since commit 46d626d3926cf6b9f9c477782c5a0ee620a424cd:

  socfpga/spl: Remove malloc.h (2012-11-04 16:23:23 +0100)

are available in the git repository at:

  git://git.denx.de/u-boot-atmel.git master

for you to fetch changes up to 15915a45d438b9e65e03c3052c3dbab1374927bc:

  at91: move get_chip_id() and get_extension_chip_id() to cpu.c, which can share with all at91 board. (2012-11-05 11:15:12 +0100)

----------------------------------------------------------------
Bo Shen (1):
      video: atmel: implement lcd_setcolreg funtion

Wu, Josh (1):
      at91: move get_chip_id() and get_extension_chip_id() to cpu.c, which can share with all at91 board.

 arch/arm/cpu/arm926ejs/at91/at91sam9x5_devices.c |   12 ------------
 arch/arm/cpu/arm926ejs/at91/cpu.c                |   11 +++++++++++
 arch/arm/include/asm/arch-at91/at91_dbu.h        |    4 ++++
 arch/arm/include/asm/arch-at91/at91sam9x5.h      |    1 -
 drivers/video/atmel_hlcdfb.c                     |    6 ++++++
 5 files changed, 21 insertions(+), 13 deletions(-)
Albert ARIBAUD - Nov. 5, 2012, 1:09 p.m.
Hi Andreas,

On Mon,  5 Nov 2012 11:19:30 +0100, Andreas Bießmann
<andreas.devel@googlemail.com> wrote:

> Dear Albert Aribaud,
> 
> The following changes since commit 46d626d3926cf6b9f9c477782c5a0ee620a424cd:
> 
>   socfpga/spl: Remove malloc.h (2012-11-04 16:23:23 +0100)
> 
> are available in the git repository at:
> 
>   git://git.denx.de/u-boot-atmel.git master
> 
> for you to fetch changes up to 15915a45d438b9e65e03c3052c3dbab1374927bc:
> 
>   at91: move get_chip_id() and get_extension_chip_id() to cpu.c, which can share with all at91 board. (2012-11-05 11:15:12 +0100)
> 
> ----------------------------------------------------------------
> Bo Shen (1):
>       video: atmel: implement lcd_setcolreg funtion

What is the point of adding this function? This does not seem to be a
strong override of a weak symbol, and therefore, it is dead code unless
this patch is a dependency of one or several other patches -- in which
case, please submit the other patch(es) along this one in your PR.

> Wu, Josh (1):
>       at91: move get_chip_id() and get_extension_chip_id() to cpu.c, which can share with all at91 board.

This one, despite being a cleaner rewrite, also seems to be a
preparatory patch waiting to be used.

The whole PR is dead/dormant code. Please resubmit when dependent
patches are added (or consider rolling back your master until you get
these dependent patches).

Amicalement,
Andreas Bießmann - Nov. 6, 2012, 9:19 a.m.
Hi Albert,

On 05.11.2012 14:09, Albert ARIBAUD wrote:
> Hi Andreas,
> 
> On Mon,  5 Nov 2012 11:19:30 +0100, Andreas Bießmann
> <andreas.devel@googlemail.com> wrote:
> 
>> Dear Albert Aribaud,
>>
>> The following changes since commit 46d626d3926cf6b9f9c477782c5a0ee620a424cd:
>>
>>   socfpga/spl: Remove malloc.h (2012-11-04 16:23:23 +0100)
>>
>> are available in the git repository at:
>>
>>   git://git.denx.de/u-boot-atmel.git master
>>
>> for you to fetch changes up to 15915a45d438b9e65e03c3052c3dbab1374927bc:
>>
>>   at91: move get_chip_id() and get_extension_chip_id() to cpu.c, which can share with all at91 board. (2012-11-05 11:15:12 +0100)
>>
>> ----------------------------------------------------------------
>> Bo Shen (1):
>>       video: atmel: implement lcd_setcolreg funtion
> 
> What is the point of adding this function? This does not seem to be a
> strong override of a weak symbol, and therefore, it is dead code unless
> this patch is a dependency of one or several other patches -- in which
> case, please submit the other patch(es) along this one in your PR.

This one is required for http://patchwork.ozlabs.org/patch/196566/ and
was explicitly requested in a private mail to Bo to support for getting
the RFC working
(http://article.gmane.org/gmane.comp.boot-loaders.u-boot/145434). So you
are right here, currently it is dead code and could maybe added to a series.

On the other hand the lcd_setcolreg() is exported via include/lcd.h but
not provided by the atmel_hlcd driver which is an error.

>> Wu, Josh (1):
>>       at91: move get_chip_id() and get_extension_chip_id() to cpu.c, which can share with all at91 board.
> 
> This one, despite being a cleaner rewrite, also seems to be a
> preparatory patch waiting to be used.

Well, the get_*chip_id() are already used by some macros but only for a
single variant of at91. So you are right here when you call it
'preparatory', but it is not dead code as you may think.

> The whole PR is dead/dormant code. Please resubmit when dependent
> patches are added (or consider rolling back your master until you get
> these dependent patches).

However I rolled back the u-boot-atmel/master and will provide a series
for patch 1, patch 2 is so far NAKed until we get a user for that
reworked code.

Best regards

Andreas Bießmann