Re: [U-Boot] [PATCH 0/4] udoo: Improve stability of DDR3 setting

2013-11-07 Thread Stefano Babic
Hi Giuseppe,

On 06/11/2013 21:30, Giuseppe Pagano wrote:
 Move udoo configuration files to board/udoo/ folder.
 Align clock configuration and DDR3 calibration to Seco suggested value
 to increase system stability.
 

There are some basic issues with your patchset. First, patches are
corrupted by your editor and/or by your mailer and cannot be applied.

ERROR: DOS line endings
#222: FILE: board/udoo/ddr-setup.cfg:71:
+DATA 4, MX6_IOM_GRP_DDRMODE, 0x0002^M$

ERROR: DOS line endings
#223: FILE: board/udoo/ddr-setup.cfg:72:
+/* disable ddr pullups */^M$

Do not use editor that add carriage return at the end of the line.
Instead of generating the patches with diff, use git format-patch, and
then submit the patches to the mailing list with git send-email. This avoi

Please take a look at the rules to submit patches :

http://www.denx.de/wiki/U-Boot/Patches

Do not fix multiple issues in the same patch if not strictly needed. The
commit message is misleading: you say you are moving the configuration
files, but they are not moved (they can't because they belong to
nitrogen) and new files are generated.

I do not understand what you mean with Align clock configuration and
DDR3 calibration to Seco suggested value. Please explain: which values
are wrong, what you have fix with your patch.

There should be a good reason to copy files. Generally, we do not want
to have copies of the same files.

If you make change to a board, you should send your patches in CC to the
board maintainer, too (for udoo, Fabio: I put him in CC).

 Signed-off-by: Giuseppe Pagano giuseppe.pag...@seco.com
 Cc: sba...@denx.de
 
 ...
 
 diff -uNr a/board/udoo/1066mhz_4x256mx16.cfg
 b/board/udoo/1066mhz_4x256mx16.cfg
 --- a/board/udoo/1066mhz_4x256mx16.cfg
 +++ b/board/udoo/1066mhz_4x256mx16.cfg

General issue for generating patches. please use git format-patch as I
explained befoe.

 @@ -0,0 +1,56 @@
 +/*
 + * Copyright (C) 2013 Boundary Devices
 + *
 + * SPDX-License-Identifier:  GPL-2.0+
 + */
 +
 +
 +DATA 4, MX6_MMDC_P0_MDPDC, 0x00020036
 +DATA 4, MX6_MMDC_P0_MDOTC, 0x09444040
 +
 +DATA 4, MX6_MMDC_P0_MDCFG0, 0x54597955
 +DATA 4, MX6_MMDC_P0_MDCFG1, 0xFF328F64
 +DATA 4, MX6_MMDC_P0_MDCFG2, 0x01FF00DB
 +
 +DATA 4, MX6_MMDC_P0_MDMISC, 0x1740
 +DATA 4, MX6_MMDC_P0_MDSCR,  0x8000
 +DATA 4, MX6_MMDC_P0_MDRWD,  0x26D2
 +
 +DATA 4, MX6_MMDC_P0_MDOR,  0x00591023
 +DATA 4, MX6_MMDC_P0_MDASP, 0x0027
 +DATA 4, MX6_MMDC_P0_MDCTL, 0x831A
 +
 +DATA 4, MX6_MMDC_P0_MDSCR, 0x04088032
 +DATA 4, MX6_MMDC_P0_MDSCR, 0x8033
 +
 +DATA 4, MX6_MMDC_P0_MDSCR,   0x00048031
 +DATA 4, MX6_MMDC_P0_MDSCR,   0x09408030
 +DATA 4, MX6_MMDC_P0_MDSCR,   0x04008040
 +DATA 4, MX6_MMDC_P0_MPZQHWCTRL, 0xA1380003
 +DATA 4, MX6_MMDC_P1_MPZQHWCTRL, 0xA1380003
 +DATA 4, MX6_MMDC_P0_MDREF,   0x5800
 +DATA 4, MX6_MMDC_P0_MPODTCTRL,   0x0007
 +DATA 4, MX6_MMDC_P1_MPODTCTRL,   0x0007
 +
 +DATA 4, MX6_MMDC_P0_MPDGCTRL0, 0x43510360
 +DATA 4, MX6_MMDC_P0_MPDGCTRL1, 0x0342033F
 +DATA 4, MX6_MMDC_P1_MPDGCTRL0, 0x033F033F
 +DATA 4, MX6_MMDC_P1_MPDGCTRL1, 0x03290266
 +
 +DATA 4, MX6_MMDC_P0_MPRDDLCTL, 0x4B3E4141
 +DATA 4, MX6_MMDC_P1_MPRDDLCTL, 0x47413B4A
 +DATA 4, MX6_MMDC_P0_MPWRDLCTL, 0x42404843
 +DATA 4, MX6_MMDC_P1_MPWRDLCTL, 0x4C3F4C45
 +
 +DATA 4, MX6_MMDC_P0_MPWLDECTRL0, 0x00350035
 +DATA 4, MX6_MMDC_P0_MPWLDECTRL1, 0x001F001F
 +DATA 4, MX6_MMDC_P1_MPWLDECTRL0, 0x00010001
 +DATA 4, MX6_MMDC_P1_MPWLDECTRL1, 0x00010001
 +
 +DATA 4, MX6_MMDC_P0_MPMUR0, 0x0800
 +DATA 4, MX6_MMDC_P1_MPMUR0, 0x0800
 +
 +DATA 4, MX6_MMDC_P0_MDPDC, 0x00025576
 +DATA 4, MX6_MMDC_P0_MAPSR, 0x00011006
 +DATA 4, MX6_MMDC_P0_MDSCR, 0x
 +
 diff -uNr a/board/udoo/clocks.cfg b/board/udoo/clocks.cfg
 --- a/board/udoo/clocks.cfg
 +++ b/board/udoo/clocks.cfg
 @@ -0,0 +1,32 @@
 +/*
 + * Copyright (C) 2013 Boundary Devices
 + *
 + * SPDX-License-Identifier:  GPL-2.0+
 + *
 + * Device Configuration Data (DCD)
 + *
 + * Each entry must have the format:
 + * Addr-type   AddressValue
 + *
 + * where:
 + *  Addr-type register length (1,2 or 4 bytes)
 + *  Address   absolute address of the register
 + *  value value to be stored in the register
 + */
 +
 +/* set the default clock gate to save power */
 +DATA 4, CCM_CCGR0, 0x00C03F3F
 +DATA 4, CCM_CCGR1, 0x0030FC03
 +DATA 4, CCM_CCGR2, 0x0FFFC000
 +DATA 4, CCM_CCGR3, 0x3FF0
 +DATA 4, CCM_CCGR4, 0x00FFF300
 +DATA 4, CCM_CCGR5, 0x0FC3
 +DATA 4, CCM_CCGR6, 0x03FF
 +
 +/* enable AXI cache for VDOA/VPU/IPU */
 +DATA 4, MX6_IOMUXC_GPR4, 0xF0FF
 +
 +/* set IPU AXI-id0 Qos=0xf(bypass) AXI-id1 Qos=0x7 */
 +DATA 4, MX6_IOMUXC_GPR6, 0x007F007F
 +DATA 4, MX6_IOMUXC_GPR7, 0x007F007F
 +
 diff -uNr a/board/udoo/ddr-setup.cfg b/board/udoo/ddr-setup.cfg
 --- a/board/udoo/ddr-setup.cfg
 +++ b/board/udoo/ddr-setup.cfg
 @@ -0,0 +1,87 @@
 +/*
 + * Copyright (C) 2013 Boundary Devices
 + *
 + * SPDX-License-Identifier:  GPL-2.0+
 + *
 + * Device Configuration Data (DCD)
 + *
 + * Each 

Re: [U-Boot] [PATCH 0/4] udoo: Improve stability of DDR3 setting

2013-11-07 Thread Giuseppe Pagano
Hi Stefano,

On Thu, 2013-11-07 at 09:12 +0100, Stefano Babic wrote:
 Hi Giuseppe,
 
 On 06/11/2013 21:30, Giuseppe Pagano wrote:
  Move udoo configuration files to board/udoo/ folder.
  Align clock configuration and DDR3 calibration to Seco suggested value
  to increase system stability.
 
 There are some basic issues with your patchset. First, patches are
 corrupted by your editor and/or by your mailer and cannot be applied.
.
 Instead of generating the patches with diff, use git format-patch, and
 then submit the patches to the mailing list with git send-email. 

Sorry, I used vim and imported patch as a file in evolution, I
understood too late that I also need to change Format from normal to
preformatted. In the future I will use git send-email.

 
 Please take a look at the rules to submit patches :
 
   http://www.denx.de/wiki/U-Boot/Patches

Sure, I read it, nevertheless I made lots of errors. This was my first
submit..sorry

 Do not fix multiple issues in the same patch if not strictly needed. The
 commit message is misleading: you say you are moving the configuration
 files, but they are not moved (they can't because they belong to
 nitrogen) and new files are generated.

Maybe I was wrong in writing move configuration files.. 

I think [PATCH 0/4] can be consider an atomical change: Fabio first uDoo
support adopt nitrogenx register setting for DDR3, clock, muxing, etc

uDoo schematics is rather different from nitrogen6x, and it needs
customized setting for most of the register (as every platform). It
takes too long describe every single new setting. 
Previous configuration was very unstable and adopting those settings
uDoo board has frequently crash. Seco had validated new setting I'm
going to propose using climatic room and various tests, which stressed
system and cpu for about 7 days long cycles. 

 
 I do not understand what you mean with Align clock configuration and
 DDR3 calibration to Seco suggested value. Please explain: which values
 are wrong, what you have fix with your patch.

See above.

 If you make change to a board, you should send your patches in CC to the
 board maintainer, too (for udoo, Fabio: I put him in CC).

Fabio was abreast of this changes, but not in cc. I'll use CC in next
post.

 
  Signed-off-by: Giuseppe Pagano giuseppe.pag...@seco.com
  Cc: sba...@denx.de
  
  ...

 
 Best regards,
 Stefano Babic

Best Regards,
Giuseppe Pagano


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 0/4] udoo: Improve stability of DDR3 setting

2013-11-07 Thread Stefano Babic
Hi Giuseppe,

On 07/11/2013 11:41, Giuseppe Pagano wrote:

 Sorry, I used vim and imported patch as a file in evolution, I
 understood too late that I also need to change Format from normal to
 preformatted. In the future I will use git send-email.
 

Ok


 Please take a look at the rules to submit patches :

  http://www.denx.de/wiki/U-Boot/Patches
 
 Sure, I read it, nevertheless I made lots of errors. This was my first
 submit..sorry

No problem ;-)

 
 Do not fix multiple issues in the same patch if not strictly needed. The
 commit message is misleading: you say you are moving the configuration
 files, but they are not moved (they can't because they belong to
 nitrogen) and new files are generated.
 
 Maybe I was wrong in writing move configuration files.. 
 
 I think [PATCH 0/4] can be consider an atomical change: Fabio first uDoo
 support adopt nitrogenx register setting for DDR3, clock, muxing, etc
 
 uDoo schematics is rather different from nitrogen6x, and it needs
 customized setting for most of the register (as every platform). It
 takes too long describe every single new setting. 
 Previous configuration was very unstable and adopting those settings
 uDoo board has frequently crash.

Ok - this is an explanation that can be simply added to the commit message.

 If you make change to a board, you should send your patches in CC to the
 board maintainer, too (for udoo, Fabio: I put him in CC).
 
 Fabio was abreast of this changes, but not in cc. I'll use CC in next
 post.
 

Thanks !

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-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