[U-Boot] SPL boot on iMX6
Hello all,
we would like to have some comments how to architecture some patches we would like to submit.
Background:
We have got SPL boot working, circumventing the need to have separate u-boot binaries depending on iMX6 CPU type and memory configuration. This allows us to have one (say) bootable SD card for all versions of our new SoMs.
Formatting the patches presents us with some questions:
1. Padconfigs. For some reason the existing padconfiguration macros are set compile time depending on target cpu variant. Hence the need to add new macros (or smth) so the binary can configure the pads for many cpu variants. This would cause duplicate and redundant sets of pad configuration macros for the imx6. Is there any alternative to this, more than rewriting code to comply with cpu specific padconfigs?
2. Is there a minimum set of features that should be supported by new boards? (Thinking of features like fdt, fat or network boot). It seems that most imx6 based boards have some standard features enabled by default, but some of those we haven't tested on our new board.
Any comments regarding this are welcome,
//Tapani
Hi Tapani,
On 26/08/2013 09:17, Tapani Utriainen wrote:
Hello all,
we would like to have some comments how to architecture some patches we would like to submit.
Nice
Background:
We have got SPL boot working, circumventing the need to have separate u-boot binaries depending on iMX6 CPU type and memory configuration. This allows us to have one (say) bootable SD card for all versions of our new SoMs.
Agree on this - we have already discussed in the past about this point, and the way to get a single image for the different i.MX6 SOCs is to add SPL support, moving part of the initialization stuff away from the DCD table managed directly from the bootrom.
Formatting the patches presents us with some questions:
- Padconfigs. For some reason the existing padconfiguration macros are set
compile time depending on target cpu variant. Hence the need to add new macros (or smth) so the binary can configure the pads for many cpu variants. This would cause duplicate and redundant sets of pad configuration macros for the imx6. Is there any alternative to this, more than rewriting code to comply with cpu specific padconfigs?
Macros wee added exactly in the time they needed, and maybe a global look was missing.
However, can you provide much more detail about this ? Which macros, in which files ?
- Is there a minimum set of features that should be supported by new boards?
(Thinking of features like fdt, fat or network boot).
No, there is not - the board maintainer is responsible to add the features the board can. You can add support for a restricted number of feature and later add a patch to full support the board.
It seems that most imx6 based boards have some standard features enabled by default, but some of those we haven't tested on our new board.
You should dropped them, and add them again once they are tested and if you really want to have them.
Best regards, Stefano
On Mon, 26 Aug 2013 09:42:30 +0200 Stefano Babic sbabic@denx.de wrote:
Hi Tapani,
- Padconfigs. For some reason the existing padconfiguration macros are set
compile time depending on target cpu variant. Hence the need to add new macros (or smth) so the binary can configure the pads for many cpu variants. This would cause duplicate and redundant sets of pad configuration macros for the imx6. Is there any alternative to this, more than rewriting code to comply with cpu specific padconfigs?
Macros wee added exactly in the time they needed, and maybe a global look was missing.
However, can you provide much more detail about this ? Which macros, in which files ?
The macros I refer to is the MX6_PAD_ ones. The semantics of them depends on the target cpu. See arch/arm/include/asm/arch-mx6/mx6-pins.h
You should dropped them, and add them again once they are tested and if you really want to have them.
Good. Then we agree there.
//Tapani
Hi Tapani,
On 26/08/2013 13:12, Tapani wrote:
Macros wee added exactly in the time they needed, and maybe a global look was missing.
However, can you provide much more detail about this ? Which macros, in which files ?
The macros I refer to is the MX6_PAD_ ones. The semantics of them depends on the target cpu. See arch/arm/include/asm/arch-mx6/mx6-pins.h
Ok - these files are not thought to be used in the same binary, we have to change something, taking into account we should remain compatible without breaking the currently supported boards.
Let's start with some proposals. Maybe you have already introduced a CONFIG_ switch, because at the moment only one SOC per image is supported, and one of MX6Q, MX6DL must be set. We have also the same issue with -ddr files (mx6q-ddr and mx6dl-ddr). Let's say we add a CONFIG_MX6_MULTI to support all SocS at the same time.
Then we could change the file you mention adding a suffix to each pin. For example, in mx6q_pins.h we could add something like this:
#ifdef CONFIG_MX6_MULTI #define PAD_SUFFIX _6Q #else #define PAD_SUFFIX #endif
And we add the macro to each pin, such as enum { MX6_PAD_SD2_DAT1__USDHC2_DAT1##PAD_SUFFIX
In this way we could have different names only if we support multiple SOCs. We need then some accessors to get the right pin, something like mx6_pin(soc_type, pin_name), that returns the right configuration. Of course, this is a very first draft, and someone else can start with different proposals.
Generally I would avoid to convert the enums into tables, because they will increase the footprint for each board.
You should dropped them, and add them again once they are tested and if you really want to have them.
Good. Then we agree there.
Right.
Best regards, Stefano Babic
On 08/26/2013 06:33 AM, Stefano Babic wrote:
Hi Tapani,
On 26/08/2013 13:12, Tapani wrote:
Macros wee added exactly in the time they needed, and maybe a global look was missing.
However, can you provide much more detail about this ? Which macros, in which files ?
The macros I refer to is the MX6_PAD_ ones. The semantics of them depends on the target cpu. See arch/arm/include/asm/arch-mx6/mx6-pins.h
Ok - these files are not thought to be used in the same binary, we have to change something, taking into account we should remain compatible without breaking the currently supported boards.
Let's start with some proposals. Maybe you have already introduced a CONFIG_ switch, because at the moment only one SOC per image is supported, and one of MX6Q, MX6DL must be set. We have also the same issue with -ddr files (mx6q-ddr and mx6dl-ddr). Let's say we add a CONFIG_MX6_MULTI to support all SocS at the same time.
Then we could change the file you mention adding a suffix to each pin. For example, in mx6q_pins.h we could add something like this:
#ifdef CONFIG_MX6_MULTI #define PAD_SUFFIX _6Q #else #define PAD_SUFFIX #endif
And we add the macro to each pin, such as enum { MX6_PAD_SD2_DAT1__USDHC2_DAT1##PAD_SUFFIX
In this way we could have different names only if we support multiple SOCs. We need then some accessors to get the right pin, something like mx6_pin(soc_type, pin_name), that returns the right configuration. Of course, this is a very first draft, and someone else can start with different proposals.
:)
This is where we started on i.MX6, with prefixes MX6Q and MX6DL. See commit cfb8b9d.
Generally I would avoid to convert the enums into tables, because they will increase the footprint for each board.
Functionally, we still need table(s) for any image which supports either variant so the proper set of pads are configured.
See this for an example http://lists.denx.de/pipermail/u-boot/2012-October/136394.html
The construct used in that patch set was to define FOR_DL_SOLO, then include the pad file. #ifdef CONFIG_MX6Q #include "pads.h" #endif #if defined(CONFIG_MX6DL) || defined(CONFIG_MX6S) #define FOR_DL_SOLO #include "pads.h" #endif
Troy's implementation used a naming convention of mx6q_X and mx6dl_solo_X such that a board supporting both would have variables
static iomux_v3_cfg_t mx6q_usdhc3_pads = ...
followed by
static iomux_v3_cfg_t mx6dl_solo_usdhc3_pads = ...
Some other data structures were also duplicated with the same naming convention (see i2c_pads).
Regards,
Eric
Hi Eric,
On 26/08/2013 16:23, Eric Nelson wrote:
Functionally, we still need table(s) for any image which supports either variant so the proper set of pads are configured.
See this for an example http://lists.denx.de/pipermail/u-boot/2012-October/136394.html
Ok - what I meant is to avoid to convert the static definitions (enums) in the header in a sort of tables. I agree that using tables in the board code is needed and makes the code more readable using imx_iomux_v3_setup_multiple_pads().
The construct used in that patch set was to define FOR_DL_SOLO, then include the pad file. #ifdef CONFIG_MX6Q #include "pads.h" #endif #if defined(CONFIG_MX6DL) || defined(CONFIG_MX6S) #define FOR_DL_SOLO #include "pads.h" #endif
Troy's implementation used a naming convention of mx6q_X and mx6dl_solo_X such that a board supporting both would have variables
static iomux_v3_cfg_t mx6q_usdhc3_pads = ...followed by
static iomux_v3_cfg_t mx6dl_solo_usdhc3_pads = ...
ok, this is a solution. Let's wait for next Tapani's patch and when we start the discussion ;-)
Best regards, Stefano
On Mon, 26 Aug 2013 15:33:56 +0200 Stefano Babic sbabic@denx.de wrote:
Hi Tapani,
The macros I refer to is the MX6_PAD_ ones. The semantics of them depends on the target cpu. See arch/arm/include/asm/arch-mx6/mx6-pins.h
Ok - these files are not thought to be used in the same binary, we have to change something, taking into account we should remain compatible without breaking the currently supported boards.
Let's start with some proposals.
Yes, this is the productive approach.
Maybe you have already introduced a CONFIG_ switch, because at the moment only one SOC per image is supported, and one of MX6Q, MX6DL must be set. We have also the same issue with -ddr files (mx6q-ddr and mx6dl-ddr). Let's say we add a CONFIG_MX6_MULTI to support all SocS at the same time.
Then we could change the file you mention adding a suffix to each pin. For example, in mx6q_pins.h we could add something like this:
#ifdef CONFIG_MX6_MULTI #define PAD_SUFFIX _6Q #else #define PAD_SUFFIX #endif
And we add the macro to each pin, such as enum { MX6_PAD_SD2_DAT1__USDHC2_DAT1##PAD_SUFFIX
In this way we could have different names only if we support multiple SOCs. We need then some accessors to get the right pin, something like mx6_pin(soc_type, pin_name), that returns the right configuration. Of course, this is a very first draft, and someone else can start with different proposals.
Your suggestion is similar to what I would first think of, but you do the extra kludging to make it work with the current syntax. My approach would be to introduce new namings in parallel to the current ones (similar, if not the same, as the linux kernel uses) until most imx6 boards have been cleaned to use the new namings (which might take a while).
For the accessor macro, our experiences from the Wandboard kernel have been very positive with the following:
To set the above mentioned pad, the board file does:
IMX6_SETUP_PAD( SD2_DAT1__USDHC2_DAT1 );
where the macro IMX6_SETUP_PAD is defined as:
#define IMX6_SETUP_PAD(p) \ if (cpu_is_mx6q()) \ mxc_iomux_v3_setup_pad(MX6Q_PAD_##p);\ else \ mxc_iomux_v3_setup_pad(MX6DL_PAD_##p)
Of course the syntax can be different. One experience is that this de-clutters the board file (compared with having arrays and arrays of padconf definitions).
The catch is, I guess, that the generated code could be a little larger than otherwise, but defining cpu_is_mx6q() with __attribute__((pure)) (or similar) should cause gcc to optimize away most of the comparisons and calls to cpu_is_mx6q().
//Tapani
Hi Tapani,
On 27/08/2013 06:07, Tapani Utriainen wrote:
Your suggestion is similar to what I would first think of, but you do the extra kludging to make it work with the current syntax. My approach would be to introduce new namings in parallel to the current ones (similar, if not the same, as the linux kernel uses) until most imx6 boards have been cleaned to use the new namings (which might take a while).
We will see when you post patches. My concern is that very often old code remains unchanged and nobody takes care of it. If there is no compatibility approach, we should try to clean up all boards.
Can you also take a look at the Troy's patches, mentioned by Eric ?
For the accessor macro, our experiences from the Wandboard kernel have been very positive with the following:
To set the above mentioned pad, the board file does:
IMX6_SETUP_PAD( SD2_DAT1__USDHC2_DAT1 );
where the macro IMX6_SETUP_PAD is defined as:
#define IMX6_SETUP_PAD(p) \ if (cpu_is_mx6q()) \ mxc_iomux_v3_setup_pad(MX6Q_PAD_##p);\ else \ mxc_iomux_v3_setup_pad(MX6DL_PAD_##p)
Sure, I thought something like that, too.
Of course the syntax can be different. One experience is that this de-clutters the board file (compared with having arrays and arrays of padconf definitions).
It is interesting how much the footprint increases.
Best regards, Stefano Babic
Hi Tapani,
On 08/26/2013 09:07 PM, Tapani Utriainen wrote:
On Mon, 26 Aug 2013 15:33:56 +0200 Stefano Babic sbabic@denx.de wrote:
Hi Tapani,
The macros I refer to is the MX6_PAD_ ones. The semantics of them
depends on
the target cpu. See arch/arm/include/asm/arch-mx6/mx6-pins.h
Ok - these files are not thought to be used in the same binary, we have to change something, taking into account we should remain compatible without breaking the currently supported boards.
Let's start with some proposals.
Yes, this is the productive approach.
Maybe you have already introduced a CONFIG_ switch, because at the moment only one SOC per image is supported, and one of MX6Q, MX6DL must be set. We have also the same issue with -ddr files (mx6q-ddr and mx6dl-ddr). Let's say we add a CONFIG_MX6_MULTI to support all SocS at the same time.
Then we could change the file you mention adding a suffix to each pin. For example, in mx6q_pins.h we could add something like this:
#ifdef CONFIG_MX6_MULTI #define PAD_SUFFIX _6Q #else #define PAD_SUFFIX #endif
And we add the macro to each pin, such as enum { MX6_PAD_SD2_DAT1__USDHC2_DAT1##PAD_SUFFIX
In this way we could have different names only if we support multiple SOCs. We need then some accessors to get the right pin, something like mx6_pin(soc_type, pin_name), that returns the right configuration. Of course, this is a very first draft, and someone else can start with different proposals.
Your suggestion is similar to what I would first think of, but you do the extra kludging to make it work with the current syntax. My approach would be to introduce new namings in parallel to the current ones (similar, if not the same, as the linux kernel uses) until most imx6 boards have been cleaned to use the new namings (which might take a while).
No matter how we implement this, there are some pre-requisites.
I believe we're all in agreement that we should be able to express a pad value in one place which defines the use of a particular pad on a particular board.
We need some cleanup to get there though. For example, pad CSI0_DAT13 has this option in mx6q_pins.h:
MX6_PAD_CSI0_DAT13__PCIE_CTRL_MUX_17
but this value in mx6dl_pins.h MX6_PAD_CSI0_DAT13__PCIE_CTRL_DIAG_STATUS_BUS_MUX_17
I'm created sorted lists of pad names for dual-quad and solo processors for easy comparison: http://linode.boundarydevices.com/imx6quad-pad-names.txt http://linode.boundarydevices.com/imx6solo-pad-names.txt
Most of the differences are simply name changes, with an extra underscore in one versus the other.
Some of them are functional differences though (e.g. SATA isn't present on solo). No matter how the pad names get changed, these should be easy to identify in the resulting code.
For the accessor macro, our experiences from the Wandboard kernel have been very positive with the following:
To set the above mentioned pad, the board file does:
IMX6_SETUP_PAD( SD2_DAT1__USDHC2_DAT1 );where the macro IMX6_SETUP_PAD is defined as:
#define IMX6_SETUP_PAD(p) \ if (cpu_is_mx6q()) \ mxc_iomux_v3_setup_pad(MX6Q_PAD_##p);\ else \ mxc_iomux_v3_setup_pad(MX6DL_PAD_##p)
Please, no!
You'd never write code like this by hand, and putting it behind a macro doesn't make it better. It just hides the ugliness.
Regards,
Eric
Hi Tapani,
On Mon, Aug 26, 2013 at 4:17 AM, Tapani Utriainen tapani@technexion.com wrote:
Hello all,
we would like to have some comments how to architecture some patches we would like to submit.
Background:
We have got SPL boot working, circumventing the need to have separate u-boot
Could you please post your mx6 spl patches so that we can advance in this discussion?
Thanks,
Fabio Estevam
participants (5)
-
Eric Nelson -
Fabio Estevam -
Stefano Babic -
Tapani -
Tapani Utriainen