Difference between revisions of "U-boot patches"

From ArmadeusWiki
Jump to: navigation, search
(→‎V3)
(→‎feedback from upstream)
 
(33 intermediate revisions by the same user not shown)
Line 214: Line 214:
 
*# Why do you need an extra definitions for the NFC controller : <font color="green">OK</font>
 
*# Why do you need an extra definitions for the NFC controller : <font color="green">OK</font>
 
* http://lists.denx.de/pipermail/u-boot/2012-December/141889.html
 
* http://lists.denx.de/pipermail/u-boot/2012-December/141889.html
−
#* I tried your patches, but build fails:
+
#* I tried your patches, but build fails : <font color="green">OK</font>
−
#* This should be declared static, I think.
+
#* This should be declared static, I think : <font color="green">OK</font>
−
#* Doesn't the following code work ?
+
#* Doesn't the following code work ? : <font color="orange">no, need and answer</font>
−
#* Do you need this ?
+
#* Do you need this ? : <font color="green">OK</font>
−
#* you search for the environment
+
#* you search for the environment : <font color="orange">autoload is used</font>
−
#* Not clear at all
+
#* Not clear at all : <font color="orange">need to ask jorasse</font>
−
#* This seems dangerous
+
#* This seems dangerous : <font color="green">OK</font>
−
#* I suggest you move this function in a separate patch
+
#* I suggest you move this function in a separate patch : <font color="green">OK</font>
−
#* Style, wrong multiline comment.
+
#* Style, wrong multiline comment : <font color="green">OK</font>
−
#* Is it old code ?
+
#* Is it old code ? : <font color="orange">need to ask jorasse</font>
−
#* I understand why
+
#* I understand why : <font color="orange">too big in memory, need to answer</font>
−
#* I am unsure if I have understood
+
#* I am unsure if I have understood : <font color="orange">initialize all gpio, need to answer</font>
−
#* ..but ACFG_APF27_CUSTOM is not set at all
+
#* ..but ACFG_APF27_CUSTOM is not set at all : <font color="orange">remove it ?, jorasse ?</font>
−
#* If SPL can link mxc_nand.c
+
#* If SPL can link mxc_nand.c : <font color="orange">too big in memory, need to answer</font>
−
#* This is also done by generic SPL code
+
#* This is also done by generic SPL code : <font color="orange">too big in memory, need to answer</font>
−
#* You also duplicate stuff that is in the generic
+
#* You also duplicate stuff that is in the generic : <font color="orange">too big in memory, need to answer</font>
−
#* I have not understood why using CONFIG_SPL_FRAMEWORK does not work
+
#* I have not understood why using CONFIG_SPL_FRAMEWORK does not work : <font color="orange">too big in memory, need to answer</font>
 +
 
 +
==V4==
 +
 
 +
SENT:
 +
* http://lists.denx.de/pipermail/u-boot/2013-July/159634.html
 +
* http://lists.denx.de/pipermail/u-boot/2013-July/159636.html
 +
* http://lists.denx.de/pipermail/u-boot/2013-July/159635.html
 +
* http://lists.denx.de/pipermail/u-boot/2013-July/159637.html
 +
 
 +
FEEDBACK:
 +
* http://lists.denx.de/pipermail/u-boot/2013-July/159662.html
 +
*# acked by ... <font color="green">OK</font>
 +
 
 +
* http://lists.denx.de/pipermail/u-boot/2013-July/159666.html
 +
*# use  SPDX-License-Identifier <font color="green">OK</font>
 +
*# Maybe you can use here a ifndef <font color="green">OK</font>
 +
*# There is already a function doing this <font color="green">OK</font>
 +
*# We have general gpio functions in u-boot to set/get gpios <font color="yellow">TODO</font>
 +
*# Can you use the gpio accessors (gpio_set_value() in this case) ? <font color="yellow">TODO</font>
 +
*# To my understanding: <font color="orange">need to answer</font>
 +
*# You add FPGA in a later patch, <font color="green">OK</font>
 +
*# The code here depends on the environment <font color="green">OK</font>
 +
*# Also this could be part of a script, <font color="green">OK</font>
 +
*# It looks like you had a missing function by linking <font color="green">OK</font>
 +
*# I think there are some code style issues due to multiline comments. <font color="yellow">TODO</font>
 +
*# But is the number of banks not detected at runtime ? <font color="yellow">TODO</font>
 +
*# Using SPL, the storage driver (in your case, NAND) <font color="green">OK</font>
 +
*# According to README: each define whose name starts with CONFIG_ <font color="green">OK</font>
 +
*# Everything seems to me only for debug purpose <font color="green">OK</font>
 +
*# Well, this code makes what the driver for NAND is supposed to do <font color="orange">need to answer</font>
 +
*# Ok, this must be done in assembly - normally is part of lowelevel_init <font color="green">OK</font>
 +
 
 +
* http://lists.denx.de/pipermail/u-boot/2013-July/159639.html
 +
*# use  SPDX-License-Identifier <font color="green">OK</font>
 +
 
 +
* http://lists.denx.de/pipermail/u-boot/2013-July/159724.html
 +
*# u-boot-nand.bin is from the legacy nand_spl subsystem <font color="yellow">TODO</font>
 +
 
 +
==V5==
 +
 
 +
SENT:
 +
* http://lists.denx.de/pipermail/u-boot/2013-August/161546.html
 +
* http://lists.denx.de/pipermail/u-boot/2013-August/161547.html
 +
* http://lists.denx.de/pipermail/u-boot/2013-August/161548.html
 +
 
 +
FEEDBACK:
 +
* http://lists.denx.de/pipermail/u-boot/2013-August/161546.html
 +
*# Acked-by: Stefano Babic <sbabic at denx.de> : <font color="green">OK</font>
 +
 
 +
* http://lists.denx.de/pipermail/u-boot/2013-August/161547.html
 +
*# Last time I asked if it is possible to use get_ram_size() twice to get : <font color="green">OK</font>
 +
*# Having twice the branch with if (get_num_ram_bank() > 1) looks : <font color="green">OK</font>
 +
 
 +
* http://lists.denx.de/pipermail/u-boot/2013-August/161548.html
 +
*# Acked-by: Stefano babic <sbabic at denx.de> : <font color="green">OK</font>
 +
 
 +
==V6==
 +
 
 +
SENT:
 +
* http://lists.denx.de/pipermail/u-boot/2013-September/162115.html
 +
* http://lists.denx.de/pipermail/u-boot/2013-September/162116.html
 +
* http://lists.denx.de/pipermail/u-boot/2013-September/162117.html

Latest revision as of 09:39, 8 September 2013

Page under construction... Construction.png Informations on this page are not guaranteed !!

General information

patches -1xx concern the board apf9328

patches -3xx concern the board apf27

patches -4xx concern the board apf51

List of patches

U-Boot 2010.03 patches for the APF9328

u-boot-2010.03-100-imx.patch: this set of patches is specific to the CM9328MX1/L/S CPUs - This impacts all platforms based in theseCPUs in U-Boot.

  • timer.c: Fixed a timer issue on the legacy version of U-Boot - so this patch should not be usefull right now. FIXME.
  • speed.c: Improve the PLL frequency to the full resolution of the PLL registers.
  • imx-regs.h: Add I2C module registers definition to be used by the I2c driver.

u-boot-2010.03-110-apf9328.patch: APF9328 main source files.

u-boot-2010.03-111-apf9328-DM9000.patch: fixes some bugs and issues in the DM9000 ethernet driver - also support to update the MAC address.

u-boot-2010.03-112-apf9328-makefile.patch: add apf9328 entry in the U-Boot makefile.

u-boot-2010.03-120-apf9328-tools.patch: add tool to generate the U-Boot.brec to upload U-Boot over a serial port - to be removed as armadeus BSP provides a python script to recover U-Boot.

u-boot-2010.03-140-ds1374.patch: support platforms with multi I2C buses.

u-boot-2010.03-150-apf9328-fixes_gcc_4_4_EABI_linking_issues.patch: Fixes EABI compilation issue with GCC 4.3 and later versions - Already fixed upstream, so it is not anymore useful.

U-Boot 2010.03 patches for the APF27

u-boot-2010.03-300-imx27.patch

add imx27 drivers:

  • imxfuse
  • I2C
  • some new functions get the core frequency of controllers

STATUS: in progress

  • add some #if defined(...) in generic.c
  • move cmd_imxfuse.c to common/
  • remove printf with hardcoded address in generic.c
NOTE: there is already a driver mxc_i2c.c with several cpu already supported, but not imx27.
The i2c function should be added here (with a new patch)

IDEAS: split this patch in 3 patchs
- one to add i2c feature on mxc_i2c.c
- one to add cmd for imxfuse
- one to add some features in generic.c

u-boot-2010.03-301-nand.patch

add nand features:

  • enhance nand bad block management for SPL support
  • support nand biterr command
NOTE: should be splitted in 2 patches

u-boot-2010.03-310-apf27.patch

add apf27 features:

  • board support
  • fpga support
NOTE: should be splitted in 2 patches

u-boot-2010.03-311-imx-nand-lock-unlock.patch: add iMX27 nand lock/unlock support

u-boot-2010.03-312-imx-nand-write-any-size.patch: Is this patch useful?

u-boot-2010.03-320-spartan.patch: Fix a bug in spartan driver.

u-boot-2010.03-321-add_spartan6.patch: add spartan6 driver

u-boot-2010.03-322-fpga_second_load_operator.patch: support spartan6 - spartan6 for apf27

u-boot-2010.03-330-fix-nand-debug.patch: fix compilation error in nand_btt.c

u-boot-2010.03-340-fix-fecmxc-debug.patch: fix fec bug to retrieve MAC address

u-boot-2010.03-350-fix-jffs2-warns.patch: remove extra 'print' messages on console.

U-Boot 2010.03 patches for the APF51

u-boot-2010.03-400-imx51.patch

u-boot-2010.03-401-apf51.patch

u-boot-2010.03-403-apf51-wdog.patch

u-boot-2010.03-404-apf51-fec.patch

u-boot-2010.03-407-imx51-nand.patch

u-boot-2010.03-409-apf51-nand-spl.patch

u-boot-2010.03-410-imx-iim.patch

u-boot-2010.03-411-Fix-high-voltage-nand-sd.patch

u-boot-2010.03-420-freescale-mxc_i2c-add_support_for_MX53_processor.patch

u-boot-2010.03-421-denx-mxc_i2c-add_support_for_the_iMX35_processor.patch

u-boot-2010.03-422-denx-mxc_i2c-get_rid_of__REG_access.patch

u-boot-2010.03-423-denx-mxc_i2c-address_failure_with_mx35_processor.patch

u-boot-2010.03-424-armadeus-mxc_i2c-add_support_for_the_iMX51_processor.patch

New organization of patches

Here is a proposition for a new organization of patches.

Motivation

The new organization should simplify the maintenance of patches and follow as much as possible the a common strategy already used for Buildroot and the Linux kernel. Also the patch should respect the requirements of the project in order to push the patches upstream.

Changes

  • Move U-Boot patches in the armadeus directory patches/u-boot
  • Have a link of the directory mentioned here above in buildroot/boot/u-boot - instead of the legacy path buildroot/target/u-boot
  • Use a subdirectory for each version of u-boot instead of file naming convention - This will simplify the update of these patches regarding GIT for the next versions of u-boot (not yet but I hope soon :( )
    • buildroot/boot/u-boot/u-boot-1.3.4/
    • buildroot/boot/u-boot/u-boot-2010.03/
    • ...
    • to support a new U-Boot release, just copy the latest U-Boot patch dir and name it with name of the U-Boot release. Therefore, the name of patch does not to contain any reference to U-Boot version (may be in a near future as Buildroot requires patches be named with a prefix u-boot-<version>- )
  • Each patch has to respect rules defined by U-Boot to expect to push back each patch upstream: http://www.denx.de/wiki/U-Boot/Patches
  • Software changes have to respect the U-Boot coding rules (same as linux kernel): http://www.denx.de/wiki/U-Boot/CodingStyle

..

feedback from upstream

V1

SENT:

FEEDBACK:

  • http://lists.denx.de/pipermail/u-boot/2012-June/127149.html
    1. You missed an entry to the MAINTAINERS file : OK
    2. This ifdef seems to be useless at this location : OK
    3. Can't this be converted into a C file instead? : OK
    4. To many new lines here : file removed
  • http://lists.denx.de/pipermail/u-boot/2012-June/127145.html
    1. nand_spl is deprecated -- please use the new spl/ infrastructure : OK
    2. Why do you need to redefine this stuff? : dont know how to avoid this
  • http://lists.denx.de/pipermail/u-boot/2012-July/127891.html
    1. Really this patch must be merged with patch 2/5: "Add support for the : do it later
    2. See my comment about the SPL patch : OK
    3. We use a macro for CONFIG_SYS_GBL_DATA_SIZE. Is it not suitable for your : OK
    4. Do not let dead code : OK
    5. Drop also this line - check in the whole file for these occurrencies : OK
    6. drop also this dead code : OK
    7. Do you have several hardware version of the same board or which is the : need to answer
    8. Really there is another way to get the peripheral clocks for the i.MX : pending
    9. Why do these config belong to the config file ? You introduce several : OK
    10. Maybe you should move also FPGA related values to the fpga file - and as : moved to board
  • http://lists.denx.de/pipermail/u-boot/2012-July/127892.html
    1. nand_spl is obsolete - new boards should add spl support with the newer : OK
  • http://lists.denx.de/pipermail/u-boot/2012-July/127893.html
    1. Maybe you can drop this and compiling this file only if CONFIG_FPGA is : OK
    2. No, we have in u-boot a GPIO API to access the GPIOs. Check : OK
    3. Do not set your special version - use DEBUG instead : OK
    4. Wrong multiline comment - it should be like this : OK
    5. initialize : OK
  • http://lists.denx.de/pipermail/u-boot/2012-July/127894.html
    1. clean / distclean are not needed, drop these rules : OK
    2. For my understanding: who does store the version number in the fuses ? : need to answer
    3. It seems to me there are some hidden important information in the fuses : OK
    4. Why is it needed ? : need to answer
    5. This is quite dead code.. : OK
    6. It is not clear to me why you have your special way to set up the MAC : OK
    7. This is SOC-related, and not board related. It belongs to the SOC code : OK
    8. config.mk is obsolete and must not be added for new boards. TEXT_BASE : OK
    9. The board are grouped for SOC, and then sorted alphabetically. The entry : OK

V2

SENT:

FEEDBACK:

  • http://lists.denx.de/pipermail/u-boot/2012-November/140882.html
    1. Please merge 2/4 and 3/4 : OK
    2. Please sort MAINTAINERS by maintainer name as stated in the file heading : OK
    3. aside: I am somewhat surprised that something : request an answer to jorasse
    4. create a function that initializes one port passed as an argument with an array of consts passed as a second argument : OK
    5. Is this used?  : need an answer
    6. What's the point of defining lowlevel_init? : need an answer
  • http://lists.denx.de/pipermail/u-boot/2012-November/140883.html
    1. This is needed by 2/4 to get truly useable support. Please merge both patches : OK
    2. Please avoid mixed case identifiers : OK
    3. Please avoid comments that paraphrase ASM in pseudo-C : OK

V3

SENT:

FEEDBACK:

    • I tried your patches, but build fails : OK
    • This should be declared static, I think : OK
    • Doesn't the following code work ? : no, need and answer
    • Do you need this ? : OK
    • you search for the environment : autoload is used
    • Not clear at all : need to ask jorasse
    • This seems dangerous : OK
    • I suggest you move this function in a separate patch : OK
    • Style, wrong multiline comment : OK
    • Is it old code ? : need to ask jorasse
    • I understand why : too big in memory, need to answer
    • I am unsure if I have understood : initialize all gpio, need to answer
    • ..but ACFG_APF27_CUSTOM is not set at all : remove it ?, jorasse ?
    • If SPL can link mxc_nand.c : too big in memory, need to answer
    • This is also done by generic SPL code : too big in memory, need to answer
    • You also duplicate stuff that is in the generic : too big in memory, need to answer
    • I have not understood why using CONFIG_SPL_FRAMEWORK does not work : too big in memory, need to answer

V4

SENT:

FEEDBACK:

  • http://lists.denx.de/pipermail/u-boot/2013-July/159666.html
    1. use SPDX-License-Identifier OK
    2. Maybe you can use here a ifndef OK
    3. There is already a function doing this OK
    4. We have general gpio functions in u-boot to set/get gpios TODO
    5. Can you use the gpio accessors (gpio_set_value() in this case) ? TODO
    6. To my understanding: need to answer
    7. You add FPGA in a later patch, OK
    8. The code here depends on the environment OK
    9. Also this could be part of a script, OK
    10. It looks like you had a missing function by linking OK
    11. I think there are some code style issues due to multiline comments. TODO
    12. But is the number of banks not detected at runtime ? TODO
    13. Using SPL, the storage driver (in your case, NAND) OK
    14. According to README: each define whose name starts with CONFIG_ OK
    15. Everything seems to me only for debug purpose OK
    16. Well, this code makes what the driver for NAND is supposed to do need to answer
    17. Ok, this must be done in assembly - normally is part of lowelevel_init OK

V5

SENT:

FEEDBACK:

V6

SENT:

Retrieved from "http://www.armadeus.org/wiki/index.php?title=U-boot_patches&oldid=12073"