Re: [U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM
On Mon, 4 Nov 2013 15:28:39 -0200 Fabio Estevam wrote: > Hi Tapani, > > On Tue, Sep 10, 2013 at 11:47 AM, Stefano Babic wrote: > >> We are worried that we might not familiar enough with u-boot development > >> to get > >> such changes accepted in reasonable time. > > > > I do not know what you mean for a reasonable time. Merge window is > > closed, that is patchsets adding new features will not be merged in the > > next release 2013.10. The next release should be 2013.01, and there are > > chances to get them merged. > > Do you plan to send a new version of the patch that adds support for > booting a single binary on the mx6q/dl/solo? > > Regards, > > Fabio Estevam Fabio (and others), mainlining SPL boot for edm-cf-imx6 is on hold until more of the required framework in u-boot is in place. //Tapani ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM
Hi Eric, On 04/11/2013 18:37, Eric Nelson wrote: > On 11/04/2013 10:28 AM, Fabio Estevam wrote: >> Hi Tapani, >> >> On Tue, Sep 10, 2013 at 11:47 AM, Stefano Babic wrote: We are worried that we might not familiar enough with u-boot development to get such changes accepted in reasonable time. >>> >>> I do not know what you mean for a reasonable time. Merge window is >>> closed, that is patchsets adding new features will not be merged in the >>> next release 2013.10. The next release should be 2013.01, and there are >>> chances to get them merged. >> >> Do you plan to send a new version of the patch that adds support for >> booting a single binary on the mx6q/dl/solo? >> > > BTW, I have updated patches to consolidate the IOMUX registers, and > they include updates to board/freescale/titanium. > > Should I re-base them on top of the patches which move titanium into > board/barco? > Yes, please do it ! I think your patches will be the base for the other ones, and I would like we can merge your efforts soon.. > It seems that this change is likely to be applied once Stefano waits > the requisite 48 hours... ;-D Regards, Stefano -- = DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM
On Mon, Nov 4, 2013 at 3:37 PM, Eric Nelson wrote: > On 11/04/2013 10:28 AM, Fabio Estevam wrote: >> >> Hi Tapani, >> >> On Tue, Sep 10, 2013 at 11:47 AM, Stefano Babic wrote: We are worried that we might not familiar enough with u-boot development to get such changes accepted in reasonable time. >>> >>> >>> I do not know what you mean for a reasonable time. Merge window is >>> closed, that is patchsets adding new features will not be merged in the >>> next release 2013.10. The next release should be 2013.01, and there are >>> chances to get them merged. >> >> >> Do you plan to send a new version of the patch that adds support for >> booting a single binary on the mx6q/dl/solo? >> > > BTW, I have updated patches to consolidate the IOMUX registers, and > they include updates to board/freescale/titanium. > > Should I re-base them on top of the patches which move titanium into > board/barco? It makes sense to base on this patch, for sure. -- Otavio Salvador O.S. Systems http://www.ossystems.com.brhttp://code.ossystems.com.br Mobile: +55 (53) 9981-7854Mobile: +1 (347) 903-9750 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM
On 11/04/2013 10:28 AM, Fabio Estevam wrote: Hi Tapani, On Tue, Sep 10, 2013 at 11:47 AM, Stefano Babic wrote: We are worried that we might not familiar enough with u-boot development to get such changes accepted in reasonable time. I do not know what you mean for a reasonable time. Merge window is closed, that is patchsets adding new features will not be merged in the next release 2013.10. The next release should be 2013.01, and there are chances to get them merged. Do you plan to send a new version of the patch that adds support for booting a single binary on the mx6q/dl/solo? BTW, I have updated patches to consolidate the IOMUX registers, and they include updates to board/freescale/titanium. Should I re-base them on top of the patches which move titanium into board/barco? It seems that this change is likely to be applied once Stefano waits the requisite 48 hours... Please advise, Eric ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM
Hi Tapani, On Tue, Sep 10, 2013 at 11:47 AM, Stefano Babic wrote: >> We are worried that we might not familiar enough with u-boot development to >> get >> such changes accepted in reasonable time. > > I do not know what you mean for a reasonable time. Merge window is > closed, that is patchsets adding new features will not be merged in the > next release 2013.10. The next release should be 2013.01, and there are > chances to get them merged. Do you plan to send a new version of the patch that adds support for booting a single binary on the mx6q/dl/solo? Regards, Fabio Estevam ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM
Hi Tapani, On 04/09/2013 13:21, Tapani wrote: > > Are the strict rules written down somewhere, so people less involved in > u-boot development can read them before submitting? > As far as I know, there is not such documentation ;-( > > Sure, we might do it for the mmdc. But long term maintainable, it is not. > >>> * It is a lot of effort to do struct accessors for huge tables. Both >>> of IOMUX and MMDC are large (offsets of 0x800+). >> >> You forget that for iomuxc the job was already done - there is structure >> and functions to setup the pinmux. >> > > You would not accept code using the current iomux structure... I do not get what you are meaning. Supported boards are using iomux structures and utility functions to set up the pinmux. > >>> Having struct >>> accessors would take quite long to enter manually (two tables of 500+ >>> entries >>> each, and different between cpu types). This would be hours, if not a day of >>> braindead work without any tangible benefit. >>> >> >> Sorry, I see benefits in terms of readability and maintenability of the >> source code and it makes easier to add new boards. This is the reason >> why there are accessors for iomuxc(), as well as for most SOC's internal >> controller. >> > > If making the addition of new boards easier is a goal, there are other parts > of the process that are currently a greater hurdle than writing the code. :-) Well, of course it is an iterative process. We are trying to make everything better ;-) > > To summarize, we are expected to: > (i) Create a more general DDR3 API for IMX6, to setup memory chips on any > board? Yes - as the setup of the DDR controller is moving from DCD to code, I am expecting to reuse that code. > (ii) Use the above API to redo our already working DDR setup for our board. Agree. > (iii) Rewrite the iomux struct(s) to more accurately reflect the iomux memory > space. Not sure what you are meaning here. If mean what we have discuss before, that is a way to declare a set of pins independently from the processor generating then the required setup / tables, yes. > There is more than plain pinmuxing there. > (iv) Rewrite any code that gets broken from changing the iomux struct(s) Right. If something is changed that breaks boads, we should adapt the broken boards. > (v) Use the new struct(s) in setting up memory for our board > > Some of the above might need to be done differently for different cpu > variants. Let's see - we will have to check the single detail. > > We are worried that we might not familiar enough with u-boot development to > get > such changes accepted in reasonable time. I do not know what you mean for a reasonable time. Merge window is closed, that is patchsets adding new features will not be merged in the next release 2013.10. The next release should be 2013.01, and there are chances to get them merged. Best regards, Stefano -- = DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM
Stefano,
On Fri, 30 Aug 2013 16:43:28 +0200 Stefano Babic wrote:
> Hi Tapani,
> >
> > On some things we probably need some clarification, see inlined responses
> > to some of your questions.
> >
>
> I hope I can help you.
>
Well, we start to get it. Correct us if we are wrong.
>
> Ma main concern iy your patchset is that the tables must be maintained
> and duplicated in the source code. However, on the board if a pin is
> dedicated to a specific function, it will be dedicated for the same
> function even with the other variants of the processor.
>
We agree fully. Again, no need to beat a dead horse. :-)
The reason our patch used duplicate tables was because of the way our original
idea was received on the u-boot mailing list. It had the definitions just once.
> Using C structure in u-boot is a strict rule - if you see, all code is
> done in this way. No new code is accepted with base + offset syntax.
>
Are the strict rules written down somewhere, so people less involved in
u-boot development can read them before submitting?
We both know it is not valid C to use structs the way you want us to.
It is a quirk of the compiler that it works at all. The coding standard
for u-boot sounds to be very picky to adhere to the C standard.
Afair, Microsoft shot themselves in the foot badly assuming structs could be
used this way in early versions of MS Office. (Wrote structs to files, but
new versions could not load since the memory layout was different).
Sure, we might do it for the mmdc. But long term maintainable, it is not.
> > * It is a lot of effort to do struct accessors for huge tables. Both
> > of IOMUX and MMDC are large (offsets of 0x800+).
>
> You forget that for iomuxc the job was already done - there is structure
> and functions to setup the pinmux.
>
You would not accept code using the current iomux structure...
> > Having struct
> > accessors would take quite long to enter manually (two tables of 500+
> > entries
> > each, and different between cpu types). This would be hours, if not a day of
> > braindead work without any tangible benefit.
> >
>
> Sorry, I see benefits in terms of readability and maintenability of the
> source code and it makes easier to add new boards. This is the reason
> why there are accessors for iomuxc(), as well as for most SOC's internal
> controller.
>
If making the addition of new boards easier is a goal, there are other parts
of the process that are currently a greater hurdle than writing the code. :-)
> > I could make those tables of { offset, value } and do the writels using
> > a for-loop, but that would just mariginally improve on the ugliness.
>
> It is not what I meant - if we see the code, we can recognize the
> sequence how the DDR3 must be programmed. We can have a function
> realizing the logic (that is, wehich registers in which sequence) must
> be written, and taking as arguments the parameters (calibration, and so
> on) that for each chip are different. In other words, I would like that
> some kind of function will be moved into common code, and not here in
> board code.
>
To summarize, we are expected to:
(i) Create a more general DDR3 API for IMX6, to setup memory chips on any board?
(ii) Use the above API to redo our already working DDR setup for our board.
(iii) Rewrite the iomux struct(s) to more accurately reflect the iomux memory
space.
There is more than plain pinmuxing there.
(iv) Rewrite any code that gets broken from changing the iomux struct(s)
(v) Use the new struct(s) in setting up memory for our board
Some of the above might need to be done differently for different cpu variants.
We are worried that we might not familiar enough with u-boot development to get
such changes accepted in reasonable time.
Did we understand this correctly?
regards,
Tapani
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM
Hi Tapani,
On 30/08/2013 07:07, Tapani Utriainen wrote:
>
> Stefano,
>
> Thank you for reviewing this patch, and for the constructive comments.
> Most of your comments are taken on board, and we will re-submit updated
> patches
> in the near future.
>
> On some things we probably need some clarification, see inlined responses
> to some of your questions.
>
I hope I can help you.
>> The patch should be split into separate patches, and each of them fixes
>> a specific issue. From our previous discussion, we agree about:
>>
>> - we need to clean up conflicts in pad definitions - see Eric's answer.
>> - we need a way to simply defines pins for the different SOC variations.
>> Eric and Troy have already proposed a schema adding tables *only* into
>> the board file, and the generation of the table is quite automatic.
>> Let's say, if I am a board maintainer and I want to add support for a
>> board having all iMX6 variations, I would like to define only once which
>> pins I need, without replicating for each SOC variant.
>> - the same should be done with DDR and clocks, if necessary.
>>
>> After these preparation patches, there should be a patch preparing i.MX6
>> for SPL - changes in i.MX6 common code should go here.
>>
>> Last, there will be a patch on top of the rest adding support to your board.
>>
>
> Understand. However, as far as I can tell Troy's suggestion is still
> creating the pad setup compile time for one cpu.
> Is there something obvious we are missing?
>
Ma main concern iy your patchset is that the tables must be maintained
and duplicated in the source code. However, on the board if a pin is
dedicated to a specific function, it will be dedicated for the same
function even with the other variants of the processor.
To beclearer, I take an example from your patch:
+static iomux_v3_cfg_t const edmdl_uart1_pads[] = {
+ MX6DL_PAD_CSI0_DAT10__UART1_TXD | MUX_PAD_CTRL(UART_PAD_CTRL),
+ MX6DL_PAD_CSI0_DAT11__UART1_RXD | MUX_PAD_CTRL(UART_PAD_CTRL),
+};
+
+static iomux_v3_cfg_t const edmq_uart1_pads[] = {
+ MX6Q_PAD_CSI0_DAT10__UART1_TXD | MUX_PAD_CTRL(UART_PAD_CTRL),
+ MX6Q_PAD_CSI0_DAT11__UART1_RXD | MUX_PAD_CTRL(UART_PAD_CTRL),
+};
CSI0_DAT10 is the pad and its function is UART, Does it make sense to
set the pin for dual as MX6_PAD_CSI0_DAT10__UART1_TXD and for dual as
MX6Q_PAD_CSI0_DAT10__GPIO_5_28 ? Surely not, but your approach makes the
tables hard to maintain. What I am complaining is not the creation of
tables, but the cut&paste work in the code that can generate errors. You
can choose of course another solution for the problem, but what I like
to see is some macro(s) that let this job to the compiler and not to the
board maintainer.
>> +
>>> +int dram_init(void)
>>> +{
>>> + uint cpurev, imxtype;
>>> + u32 sdram_size;
>>> +
>>> + cpurev = get_cpu_rev();
>>> + imxtype = (cpurev & 0xFF000) >> 12;
>>
>> I am expecting to have a global function getting the cputype, with
>> macros of the type is_cpu_mx6q() (I see you use it later) to help us the
>> usage. Not here in board code.
>>
>
> Added.
Eric sends already a patch:
http://patchwork.ozlabs.org/patch/270889/
I will apply it, so feel free to use the macros.
>
>> What about Troy's solution ?
>>
> Did not apply? And still had a mess of arrays in the board file. Or what we
> have *is* the multi-cpu variant of Troy's approach...?
> (With both CPU type macros expanded, manually)
As I said, my concern is not relating to the final result (multiple
array in the board file), but how easy is to maintain the code. I would
like to set a pad only once independently from the choosen SOC.
>> Really I do not like this solution. First, you should accessor to set
>> the iomux, without using base address + offset. And of course, access to
>> the ram controller should be done in the same way using a C structure,
>> not offsets.
>>
>> Then the problem is similar to the pads. I will propose we have a
>> general function, and the values of board specific parameters (32
>> against 64 bit size, calibration registers, and so on), and start the
>> ddr procedure. The functions here do the same on different registers.
>>
>
> We agree that the does does not look pretty. But there needs to be some
> clarification.
>
> However, using this has some benefits:
> * It is easier to convert from (and compare to) DCD tables
With the usage of the precompiler, DCD tables in configuration files are
easy to read as your code. Anyway, if a board uses SPL for setup, there
is no need to let the SOC doing that via DCD tables. And if there would
be a need to move to or from DCD, this can be done by some scripts. We
are not constrained to use imximage syntax in a C file.
> * Easier to look things up in the TRM (base + offset are easier to find
> in a long list of registers sorted by offset, than a name)
Really not - which register is written with "writel(0x001F001F,
MMDC_P1_BASE_ADDR + 0x80c)" ? Only the memory map of all
Re: [U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM
Hi Tapani,
On 08/29/2013 10:07 PM, Tapani Utriainen wrote:
Stefano,
Thank you for reviewing this patch, and for the constructive comments.
Most of your comments are taken on board, and we will re-submit updated patches
in the near future.
On some things we probably need some clarification, see inlined responses
to some of your questions.
After these preparation patches, there should be a patch preparing i.MX6
for SPL - changes in i.MX6 common code should go here.
Last, there will be a patch on top of the rest adding support to your board.
Understand. However, as far as I can tell Troy's suggestion is still
creating the pad setup compile time for one cpu.
Is there something obvious we are missing?
Troy's patch-set defines some macros, then includes "pads.h" either
once (for single architecture) or twice (for multi-arch images):
#ifdef CONFIG_MX6Q
#include "pads.h"
#endif
#if defined(CONFIG_MX6DL) || defined(CONFIG_MX6S)
#define FOR_DL_SOLO
#include "pads.h"
#endif
This allows pads.h to define the set of pads once, and build
either one or two sets of tables.
This allows you to define the pads in one place, easing maintenance
but requires the use of formalized naming of tables using
MX6NAME() macro when defining and referencing per-architecture
data.
+static iomux_v3_cfg_t const edmdl_uart1_pads[] = {
+ MX6DL_PAD_CSI0_DAT10__UART1_TXD | MUX_PAD_CTRL(UART_PAD_CTRL),
+ MX6DL_PAD_CSI0_DAT11__UART1_RXD | MUX_PAD_CTRL(UART_PAD_CTRL),
+};
+
+static iomux_v3_cfg_t const edmq_uart1_pads[] = {
+ MX6Q_PAD_CSI0_DAT10__UART1_TXD | MUX_PAD_CTRL(UART_PAD_CTRL),
+ MX6Q_PAD_CSI0_DAT11__UART1_RXD | MUX_PAD_CTRL(UART_PAD_CTRL),
+};
+
+static iomux_v3_cfg_t const edmdl_usdhc1_pads[] = {
+ MX6DL_PAD_SD1_CLK__USDHC1_CLK | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+ MX6DL_PAD_SD1_CMD__USDHC1_CMD | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+ MX6DL_PAD_SD1_DAT0__USDHC1_DAT0 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+ MX6DL_PAD_SD1_DAT1__USDHC1_DAT1 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+ MX6DL_PAD_SD1_DAT2__USDHC1_DAT2 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+ MX6DL_PAD_SD1_DAT3__USDHC1_DAT3 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+ /* Card detect */
+ MX6DL_PAD_GPIO_2__GPIO_1_2 | MUX_PAD_CTRL(NO_PAD_CTRL),
+};
+
+static iomux_v3_cfg_t const edmq_usdhc1_pads[] = {
+ MX6Q_PAD_SD1_CLK__USDHC1_CLK | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+ MX6Q_PAD_SD1_CMD__USDHC1_CMD | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+ MX6Q_PAD_SD1_DAT0__USDHC1_DAT0 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+ MX6Q_PAD_SD1_DAT1__USDHC1_DAT1 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+ MX6Q_PAD_SD1_DAT2__USDHC1_DAT2 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+ MX6Q_PAD_SD1_DAT3__USDHC1_DAT3 | MUX_PAD_CTRL(USDHC_PAD_CTRL),
+ /* Card detect */
+ MX6Q_PAD_GPIO_2__GPIO_1_2 | MUX_PAD_CTRL(NO_PAD_CTRL),
+};
I do not like this solution: this is a bare duplication of the pads, and
it is prone to errors. The definitions of the table must be in some way
automatic. I want to define a pin: if SD1_CLK is used for MMC, this does
not depend from the SOC variant.
TBH, I hate this solution as well. But guess we heard Eric's cries for mercy,
and did it the way Boundary (and several kernel board files) do it -- rather
than the way we did it in the Wandboard kernel.
What about Troy's solution ?
Did not apply? And still had a mess of arrays in the board file. Or what we
have *is* the multi-cpu variant of Troy's approach...?
(With both CPU type macros expanded, manually)
Right.
There is no way around having multiple tables. The "Troy" approach
just makes it easier to create and maintain.
+static void spl_dram_init_mx6solo_512mb(void)
+{
+ /* DDR3 initialization based on the MX6Solo Auto Reference Design (ARD)
*/
+ /* DDR IO TYPE */
+ writel(0x000c, IOMUXC_BASE_ADDR + 0x774);
+ writel(0x, IOMUXC_BASE_ADDR + 0x754);
+ /* Clock */
+ writel(0x0030, IOMUXC_BASE_ADDR + 0x4ac);
+ writel(0x0030, IOMUXC_BASE_ADDR + 0x4b0);
+ /* Address */
[ long list of writels ... ]
+ writel(0x04008032, MMDC_P0_BASE_ADDR + 0x01c);
+ writel(0x8033, MMDC_P0_BASE_ADDR + 0x01c);
+ writel(0x00048031, MMDC_P0_BASE_ADDR + 0x01c);
+ writel(0x07208030, MMDC_P0_BASE_ADDR + 0x01c);
+ writel(0x04008040, MMDC_P0_BASE_ADDR + 0x01c);
+ writel(0x5800, MMDC_P0_BASE_ADDR + 0x020);
+ writel(0x0007, MMDC_P0_BASE_ADDR + 0x818);
+ writel(0x0007, MMDC_P1_BASE_ADDR + 0x818);
+ writel(0x0002556d, MMDC_P0_BASE_ADDR + 0x004);
+ writel(0x00011006, MMDC_P1_BASE_ADDR + 0x004);
+ writel(0x, MMDC_P0_BASE_ADDR + 0x01c);
+}
Really I do not like this solution. First, you should accessor to set
the iomux, without using base address + offset. And of course, access to
the ram controller should be done in the same way using a C structure,
not offsets.
Then
Re: [U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM
Eric, On Wed, 28 Aug 2013 16:26:44 +0200 Eric Bénard wrote: > Le Wed, 28 Aug 2013 19:23:33 +0800, > Tapani a écrit : > > > > > Add support for TechNexion edm-cf-imx6 SoM > > > > The edm1-cf-imx6 SoM comes in three variants, one with imx6 solo cpu, > > one with an imx6 dual lite cpu and one with an imx6 quad cpu. > > > aren't these boards Wandboard without the MicroSD connector but with > either a eMMC or a NAND ? No. While u-boot level edm-cf-imx6 is compatible with Wandboard, there are differences (some different pin definitions, different wifi chips, different board ids, different maintainers, ... ). Also there is going to be a edm2-cf-imx6 module, which we are hoping to be able to share code with instead. However if someone wants to play with the SPL boot, a edm-cf-imx6 u-boot should work on a Wandboard (after changing machine id). //Tapani ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM
Stefano,
Thank you for reviewing this patch, and for the constructive comments.
Most of your comments are taken on board, and we will re-submit updated patches
in the near future.
On some things we probably need some clarification, see inlined responses
to some of your questions.
> > ---
> > MAINTAINERS | 4 +
> > arch/arm/include/asm/arch-mx6/spl.h | 19 +
> > board/technexion/edm_cf_imx6/Makefile | 26 +
> > board/technexion/edm_cf_imx6/README | 30 +
> > board/technexion/edm_cf_imx6/clocks.cfg | 44 ++
> > board/technexion/edm_cf_imx6/edm_cf_imx6.c | 801
> >
> > board/technexion/edm_cf_imx6/edm_cf_imx6_pins.h | 44 ++
> > board/technexion/edm_cf_imx6/imximage.cfg | 17 +
> > boards.cfg | 1 +
> > include/configs/edm_cf_imx6.h | 140 +
> > 10 files changed, 1126 insertions(+)
> > create mode 100644 arch/arm/include/asm/arch-mx6/spl.h
> > create mode 100644 board/technexion/edm_cf_imx6/Makefile
> > create mode 100644 board/technexion/edm_cf_imx6/README
> > create mode 100644 board/technexion/edm_cf_imx6/clocks.cfg
> > create mode 100644 board/technexion/edm_cf_imx6/edm_cf_imx6.c
> > create mode 100644 board/technexion/edm_cf_imx6/edm_cf_imx6_pins.h
> > create mode 100644 board/technexion/edm_cf_imx6/imximage.cfg
> > create mode 100644 include/configs/edm_cf_imx6.h
> >
>
> The patch should be split into separate patches, and each of them fixes
> a specific issue. From our previous discussion, we agree about:
>
> - we need to clean up conflicts in pad definitions - see Eric's answer.
> - we need a way to simply defines pins for the different SOC variations.
> Eric and Troy have already proposed a schema adding tables *only* into
> the board file, and the generation of the table is quite automatic.
> Let's say, if I am a board maintainer and I want to add support for a
> board having all iMX6 variations, I would like to define only once which
> pins I need, without replicating for each SOC variant.
> - the same should be done with DDR and clocks, if necessary.
>
> After these preparation patches, there should be a patch preparing i.MX6
> for SPL - changes in i.MX6 common code should go here.
>
> Last, there will be a patch on top of the rest adding support to your board.
>
Understand. However, as far as I can tell Troy's suggestion is still
creating the pad setup compile time for one cpu.
Is there something obvious we are missing?
>
> Please change all license text according to the new rule with
> SPDX-License-Identifier.
>
Will do.
>
> Maybe you should add some comments explaining that this is your
> decision, not due to the SOC. Only the first dd is mandatory by iMX
> (offset is 0x400 in flash). For u-boot, you have decided to put it into
> raw data exactly after SPL and not into a filesystem.
>
> > +
> > +Only raw mmc boot has been verified to work.
>
> The phrase is misleading, and let me think the other ways do not work. I
> assume that you tested only raw mmc, so please rephrase to explain this.
>
Not using a filesystem is by choice. We'll clarify on that
(or maybe better, enable FAT)
...
> > +
> > +static inline void setup_boot_device(void)
> > +{
> > + uint soc_sbmr = readl(SRC_BASE_ADDR + 0x4);
> > + uint bt_mem_ctl = (soc_sbmr & 0x00FF) >> 4 ;
> > + uint bt_mem_type = (soc_sbmr & 0x0008) >> 3;
> > + uint bt_mem_mmc = (soc_sbmr & 0x1000) >> 12;
> > +
> > + switch (bt_mem_ctl) {
> > + case 0x0:
> > + if (bt_mem_type)
> > + boot_dev = ONE_NAND_BOOT;
> > + else
> > + boot_dev = WEIM_NOR_BOOT;
> > + break;
> > + case 0x2:
> > + boot_dev = SATA_BOOT;
> > + break;
> > + case 0x3:
> > + if (bt_mem_type)
> > + boot_dev = I2C_BOOT;
> > + else
> > + boot_dev = SPI_NOR_BOOT;
> > + break;
> > + case 0x4:
> > + case 0x5:
> > + if (bt_mem_mmc)
> > + boot_dev = SD0_BOOT;
> > + else
> > + boot_dev = SD1_BOOT;
> > + break;
> > + case 0x6:
> > + case 0x7:
> > + boot_dev = MMC_BOOT;
> > + break;
> > + case 0x8 ... 0xf:
> > + boot_dev = NAND_BOOT;
> > + break;
> > + default:
> > + boot_dev = UNKNOWN_BOOT;
> > + break;
> > + }
> > +}
> >
>
> Let's say: boot device is not board dependent, and the required SPL
> function should be moved into general code (imx_common or
> arch/arm/cpu/armv7/mx6).
>
Will do. In the first patch attempt we deliberately tried not to touch
common imx6 code.
> +
> > +int dram_init(void)
> > +{
> > + uint cpurev, imxtype;
> > + u32 sdram_size;
> > +
> > + cpurev = get_cpu_rev();
> > + imxtype = (cpurev & 0xFF000) >> 12;
>
> I am expecting to have
Re: [U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM
Hi Tapani, On 28/08/2013 13:23, Tapani wrote: > > Add support for TechNexion edm-cf-imx6 SoM > > The edm1-cf-imx6 SoM comes in three variants, one with imx6 solo cpu, > one with an imx6 dual lite cpu and one with an imx6 quad cpu. > > This patch adds basic support for the module that utilizes SPL boot > mechanism for detecting imx6 CPU runtime and sets the system accordingly. > > Signed-off-by: Richard Hu > Signed-off-by: Tapani Utriainen > Signed-off-by: Edward Lin > --- > MAINTAINERS | 4 + > arch/arm/include/asm/arch-mx6/spl.h | 19 + > board/technexion/edm_cf_imx6/Makefile | 26 + > board/technexion/edm_cf_imx6/README | 30 + > board/technexion/edm_cf_imx6/clocks.cfg | 44 ++ > board/technexion/edm_cf_imx6/edm_cf_imx6.c | 801 > > board/technexion/edm_cf_imx6/edm_cf_imx6_pins.h | 44 ++ > board/technexion/edm_cf_imx6/imximage.cfg | 17 + > boards.cfg | 1 + > include/configs/edm_cf_imx6.h | 140 + > 10 files changed, 1126 insertions(+) > create mode 100644 arch/arm/include/asm/arch-mx6/spl.h > create mode 100644 board/technexion/edm_cf_imx6/Makefile > create mode 100644 board/technexion/edm_cf_imx6/README > create mode 100644 board/technexion/edm_cf_imx6/clocks.cfg > create mode 100644 board/technexion/edm_cf_imx6/edm_cf_imx6.c > create mode 100644 board/technexion/edm_cf_imx6/edm_cf_imx6_pins.h > create mode 100644 board/technexion/edm_cf_imx6/imximage.cfg > create mode 100644 include/configs/edm_cf_imx6.h > The patch should be split into separate patches, and each of them fixes a specific issue. From our previous discussion, we agree about: - we need to clean up conflicts in pad definitions - see Eric's answer. - we need a way to simply defines pins for the different SOC variations. Eric and Troy have already proposed a schema adding tables *only* into the board file, and the generation of the table is quite automatic. Let's say, if I am a board maintainer and I want to add support for a board having all iMX6 variations, I would like to define only once which pins I need, without replicating for each SOC variant. - the same should be done with DDR and clocks, if necessary. After these preparation patches, there should be a patch preparing i.MX6 for SPL - changes in i.MX6 common code should go here. Last, there will be a patch on top of the rest adding support to your board. > diff --git a/arch/arm/include/asm/arch-mx6/spl.h > b/arch/arm/include/asm/arch-mx6/spl.h > new file mode 100644 > index 000..dd04088 > --- /dev/null > +++ b/arch/arm/include/asm/arch-mx6/spl.h > @@ -0,0 +1,19 @@ > +/* > + * Copyright (C) 2013 TechNexion Ltd. > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation; either version 2 of > + * the License, or (at your option) any later version. > + */ Please change all license text according to the new rule with SPDX-License-Identifier. > + > +Flashing U-boot into the SD card > + > + > +- After the 'make u-boot.img' command completes, the generated 'SPL' and > +'u-boot.img' binary must be flashed into the SD card: > + > +# dd if=SPL of=/dev/$dev bs=1k seek=1 > + > +# dd if=u-boot.img of=/dev/$dev bs=64k seek=1; sync Maybe you should add some comments explaining that this is your decision, not due to the SOC. Only the first dd is mandatory by iMX (offset is 0x400 in flash). For u-boot, you have decided to put it into raw data exactly after SPL and not into a filesystem. > + > +Only raw mmc boot has been verified to work. The phrase is misleading, and let me think the other ways do not work. I assume that you tested only raw mmc, so please rephrase to explain this. > diff --git a/board/technexion/edm_cf_imx6/clocks.cfg > b/board/technexion/edm_cf_imx6/clocks.cfg > new file mode 100644 > index 000..9a182c8 > --- /dev/null > +++ b/board/technexion/edm_cf_imx6/clocks.cfg > @@ -0,0 +1,44 @@ > +/* > + * Copyright (C) 2013 TechNexion Ltd. > + * Where have you taken the file ? If this comes as it seems from nitrogen, you cannot drop their copyright. > diff --git a/board/technexion/edm_cf_imx6/edm_cf_imx6.c > b/board/technexion/edm_cf_imx6/edm_cf_imx6.c > new file mode 100644 > index 000..1a98168 > --- /dev/null > +++ b/board/technexion/edm_cf_imx6/edm_cf_imx6.c > @@ -0,0 +1,801 @@ > +/* > + * Copyright (C) 2013 TechNexion Ltd. > + * > + * Author: Richard Hu (richard...@technexion.com) > + * Tapani Utriainen (tap...@technexion.com) > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include > +#include > +#include > +#include > +#ifdef CONFIG_SPL > +#include > +#endif > +#include > +#include > +#include > +#include > +#include > +#include > +
Re: [U-Boot] [PATCH] Add support for TechNexion edm1-cf-imx6 SoM
Hi Tapani, Le Wed, 28 Aug 2013 19:23:33 +0800, Tapani a écrit : > > Add support for TechNexion edm-cf-imx6 SoM > > The edm1-cf-imx6 SoM comes in three variants, one with imx6 solo cpu, > one with an imx6 dual lite cpu and one with an imx6 quad cpu. > aren't these boards Wandboard without the MicroSD connector but with either a eMMC or a NAND ? If yes maybe you could extend wandboard support instead of adding a new board ? Best regards, Eric ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
