Re: [U-Boot] [PATCH v2] imx: ddr: Move mx6q_4x_mt41j128.cfg to mx6sabresd board dir

2014-09-13 Thread Fabio Estevam
On Fri, Sep 12, 2014 at 4:53 AM, Stefano Babic sba...@denx.de wrote:

 I try to sumarize the result of the discussion. This file was
 factorized, but it was proofed that calibration data are very board
 specific, and factorizing this file is not correct. I agree with Fabio
 and Eric that it is very unlike to have the same calibration data for
 different boards and the reuse of the same data is not correct.

 However, with this statement the patch should also unbind the congatec
 board from the sabresd, not only change the reference. A congatec
 specific board must be also created in congatec/cgtqmx6eval, and it is
 only a case that it has the same values of the sabresd.

Yes, this is a good approach.

Nitin,

Please send a v3 based on Stefano's suggestion.

Regards,

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


Re: [U-Boot] [PATCH v2] imx: ddr: Move mx6q_4x_mt41j128.cfg to mx6sabresd board dir

2014-09-12 Thread Stefano Babic
Hi everybody,

On 02/09/2014 01:51, Fabio Estevam wrote:
 On Mon, Sep 1, 2014 at 8:34 PM, Eric Nelson
 eric.nel...@boundarydevices.com wrote:
 
 I believe that the Wand board is using the configuration files
 from the nitrogen6x tree.
 
 Yes, I should have said As it stands today only mx6qsabresd and
 congatec share the same mx6q_4x_mt41j128.cfg script.
 

I try to sumarize the result of the discussion. This file was
factorized, but it was proofed that calibration data are very board
specific, and factorizing this file is not correct. I agree with Fabio
and Eric that it is very unlike to have the same calibration data for
different boards and the reuse of the same data is not correct.

However, with this statement the patch should also unbind the congatec
board from the sabresd, not only change the reference. A congatec
specific board must be also created in congatec/cgtqmx6eval, and it is
only a case that it has the same values of the sabresd.

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


Re: [U-Boot] [PATCH v2] imx: ddr: Move mx6q_4x_mt41j128.cfg to mx6sabresd board dir

2014-09-01 Thread Fabio Estevam
On Mon, Sep 1, 2014 at 11:20 AM, Nitin Garg nitin.g...@freescale.com wrote:
 Move board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg to
 board/freescale/mx6sabresd/mx6q_4x_mt41j128.cfg as this is
 was designed for the mx6sabresd board. This also updates the
 cgtqmx6qeval which makes use of this configuration.

 Signed-off-by: Nitin Garg nitin.g...@freescale.com

Acked-by: Fabio Estevam fabio.este...@freescale.com

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


Re: [U-Boot] [PATCH v2] imx: ddr: Move mx6q_4x_mt41j128.cfg to mx6sabresd board dir

2014-09-01 Thread Otavio Salvador
On Mon, Sep 1, 2014 at 11:24 AM, Fabio Estevam feste...@gmail.com wrote:
 On Mon, Sep 1, 2014 at 11:20 AM, Nitin Garg nitin.g...@freescale.com wrote:
 Move board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg to
 board/freescale/mx6sabresd/mx6q_4x_mt41j128.cfg as this is
 was designed for the mx6sabresd board. This also updates the
 cgtqmx6qeval which makes use of this configuration.

 Signed-off-by: Nitin Garg nitin.g...@freescale.com

 Acked-by: Fabio Estevam fabio.este...@freescale.com

Acked-by: Otavio Salvador ota...@ossystems.com.br

-- 
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 v2] imx: ddr: Move mx6q_4x_mt41j128.cfg to mx6sabresd board dir

2014-09-01 Thread Wolfgang Denk
Dear Nitin Garg,

In message 1409581243-12695-1-git-send-email-nitin.g...@freescale.com you 
wrote:
 Move board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg to
 board/freescale/mx6sabresd/mx6q_4x_mt41j128.cfg as this is
 was designed for the mx6sabresd board. This also updates the
 cgtqmx6qeval which makes use of this configuration.

Sorry, but I don't get it.  First you say, that mx6q_4x_mt41j128.cfg
is a mx6sabresd specific file, and move it to the board specific
directory.

But in the next sentence you state that this very file is also used by
another board (cgtqmx6qeval) - so apparently it is NOT a board
specific file.  Actually this makes even more sense to me, as I would
expect that the file is more specific to the DDR type than to the
board.


So why exactly do you think it would be better to move this into a
board specific place?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
I am more bored than you could ever possibly be.  Go back to work.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] imx: ddr: Move mx6q_4x_mt41j128.cfg to mx6sabresd board dir

2014-09-01 Thread Otavio Salvador
Wolfgang,

(Dropping Leo e-mail as it bounces, adding Alex who seem to be working
on BSP support at Congatec now)

On Mon, Sep 1, 2014 at 2:59 PM, Wolfgang Denk w...@denx.de wrote:
 In message 1409581243-12695-1-git-send-email-nitin.g...@freescale.com you 
 wrote:
 Move board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg to
 board/freescale/mx6sabresd/mx6q_4x_mt41j128.cfg as this is
 was designed for the mx6sabresd board. This also updates the
 cgtqmx6qeval which makes use of this configuration.

 Sorry, but I don't get it.  First you say, that mx6q_4x_mt41j128.cfg
 is a mx6sabresd specific file, and move it to the board specific
 directory.

 But in the next sentence you state that this very file is also used by
 another board (cgtqmx6qeval) - so apparently it is NOT a board
 specific file.  Actually this makes even more sense to me, as I would
 expect that the file is more specific to the DDR type than to the
 board.

 So why exactly do you think it would be better to move this into a
 board specific place?

All the calibration has been done by Freescale and for the SABRE SD
board. This is not guarantee it works in other boards, neither
expected. Congatec opted to include this file but it is their choice
and moving to the board file makes it more evident.

Some vendors choose to reuse other vendors provided memory
configuration (this has been done in some board using Nitrogen6X
values for example). If this is good or bad ... this is a hard
question to answer.

-- 
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 v2] imx: ddr: Move mx6q_4x_mt41j128.cfg to mx6sabresd board dir

2014-09-01 Thread Wolfgang Denk
Dear Otavio,

In message CAP9ODKonaZ9v76WAdd4k7or_kUWF=attlf+-drqgqyvgkbt...@mail.gmail.com 
you wrote:
 
  But in the next sentence you state that this very file is also used by
  another board (cgtqmx6qeval) - so apparently it is NOT a board
  specific file.  Actually this makes even more sense to me, as I would
  expect that the file is more specific to the DDR type than to the
  board.
 
  So why exactly do you think it would be better to move this into a
  board specific place?
 
 All the calibration has been done by Freescale and for the SABRE SD
 board. This is not guarantee it works in other boards, neither
 expected. Congatec opted to include this file but it is their choice
 and moving to the board file makes it more evident.

This may be all true, but nevertheless this is NOT board specific
code.  And you can already see it being reused by other boards, so the
most natural way to handle it is to factor it out into a common
directory.  And actually this is what we had before.

If we are going to change code, we should have a good reason for such
a change, like fixing bugs, adding features, or cleaning up.  What
exactly is that good reason here?  Moving code used by more than one
board from a common place to a board-specific one make things worse,
not better.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Until you walk a mile in another man's moccasins, you  can't  imagine
the smell.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] imx: ddr: Move mx6q_4x_mt41j128.cfg to mx6sabresd board dir

2014-09-01 Thread Michael Trimarchi
Hi

Il 01/set/2014 21:02 Wolfgang Denk w...@denx.de ha scritto:

 Dear Otavio,

 In message CAP9ODKonaZ9v76WAdd4k7or_kUWF=
attlf+-drqgqyvgkbt...@mail.gmail.com you wrote:
 
   But in the next sentence you state that this very file is also used by
   another board (cgtqmx6qeval) - so apparently it is NOT a board
   specific file.  Actually this makes even more sense to me, as I would
   expect that the file is more specific to the DDR type than to the
   board.
  
   So why exactly do you think it would be better to move this into a
   board specific place?
 
  All the calibration has been done by Freescale and for the SABRE SD
  board. This is not guarantee it works in other boards, neither
  expected. Congatec opted to include this file but it is their choice
  and moving to the board file makes it more evident.

 This may be all true, but nevertheless this is NOT board specific
 code.  And you can already see it being reused by other boards, so the
 most natural way to handle it is to factor it out into a common
 directory.  And actually this is what we had before.

 If we are going to change code, we should have a good reason for such
 a change, like fixing bugs, adding features, or cleaning up.  What
 exactly is that good reason here?  Moving code used by more than one
 board from a common place to a board-specific one make things worse,
 not better.


Agree with Walfgang

Michael

 Best regards,

 Wolfgang Denk

 --
 DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
 HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
 Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
 Until you walk a mile in another man's moccasins, you  can't  imagine
 the smell.
 ___
 U-Boot mailing list
 U-Boot@lists.denx.de
 http://lists.denx.de/mailman/listinfo/u-boot
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] imx: ddr: Move mx6q_4x_mt41j128.cfg to mx6sabresd board dir

2014-09-01 Thread Otavio Salvador
Wolfgang,

On Mon, Sep 1, 2014 at 4:02 PM, Wolfgang Denk w...@denx.de wrote:
 In message 
 CAP9ODKonaZ9v76WAdd4k7or_kUWF=attlf+-drqgqyvgkbt...@mail.gmail.com you 
 wrote:

  But in the next sentence you state that this very file is also used by
  another board (cgtqmx6qeval) - so apparently it is NOT a board
  specific file.  Actually this makes even more sense to me, as I would
  expect that the file is more specific to the DDR type than to the
  board.
 
  So why exactly do you think it would be better to move this into a
  board specific place?

 All the calibration has been done by Freescale and for the SABRE SD
 board. This is not guarantee it works in other boards, neither
 expected. Congatec opted to include this file but it is their choice
 and moving to the board file makes it more evident.

 This may be all true, but nevertheless this is NOT board specific
 code.  And you can already see it being reused by other boards, so the
 most natural way to handle it is to factor it out into a common
 directory.  And actually this is what we had before.

 If we are going to change code, we should have a good reason for such
 a change, like fixing bugs, adding features, or cleaning up.  What
 exactly is that good reason here?  Moving code used by more than one
 board from a common place to a board-specific one make things worse,
 not better.

I disagree (but this is my personal view on this). The reason why I
disagree is because the DDR calibration is very design dependant so
if/when Freescale optimize their DDR data setting they may break any
other board using it however they shouldn't be blamed by it as this is
their DDR settings. Any board including this file (which can be and is
done) takes the responsibility and risks.

-- 
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 v2] imx: ddr: Move mx6q_4x_mt41j128.cfg to mx6sabresd board dir

2014-09-01 Thread Wolfgang Denk
Dear Otavio,

In message cap9odkrsc4o-p7+cmkkbeda8wwwxvpnpxoyoctp6wcvoj5g...@mail.gmail.com 
you wrote:
 
 I disagree (but this is my personal view on this). The reason why I
 disagree is because the DDR calibration is very design dependant so
 if/when Freescale optimize their DDR data setting they may break any
 other board using it however they shouldn't be blamed by it as this is
 their DDR settings. Any board including this file (which can be and is
 done) takes the responsibility and risks.

Obviously, any changes to common code used by several boards needs to
be tested on the affected boards.


Also, it might be instructive for you to read the commit message for
af7ec0b mx6q: Factor out common DDR3 init code.  Apparently Fabio
considers this common DDR3 initialization code, so what exactly are
your arguments to the contrary?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
To be is to program.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] imx: ddr: Move mx6q_4x_mt41j128.cfg to mx6sabresd board dir

2014-09-01 Thread Wolfgang Denk
Dear Nitin Garg,

In message 1409581243-12695-1-git-send-email-nitin.g...@freescale.com you 
wrote:
 Move board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg to
 board/freescale/mx6sabresd/mx6q_4x_mt41j128.cfg as this is
 was designed for the mx6sabresd board. This also updates the
 cgtqmx6qeval which makes use of this configuration.

I've made my mind up.  I hereby NAK this patch, as it would basically
revert Fabio Estevam's commit af7ec0b which moved the originally
board-specific code to a common place:

commit af7ec0b0582f658873713c311497626c571f3b31
Author: Fabio Estevam fabio.este...@freescale.com
Date:   Thu Sep 13 03:18:19 2012 +

mx6q: Factor out common DDR3 init code

Factor out common DDR3 initialization code, allowing easier
maintainance of such scripts.

Signed-off-by: Fabio Estevam fabio.este...@freescale.com

Fabio's intention was a good one, as is proven by the re-use of this
code by other boards.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Once they go up, who cares where  they  come  down?  That's  not  my
department.   - Werner von Braun
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2] imx: ddr: Move mx6q_4x_mt41j128.cfg to mx6sabresd board dir

2014-09-01 Thread Fabio Estevam
Hi Wolfgang,

On Mon, Sep 1, 2014 at 4:24 PM, Wolfgang Denk w...@denx.de wrote:
 Dear Nitin Garg,

 In message 1409581243-12695-1-git-send-email-nitin.g...@freescale.com you 
 wrote:
 Move board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg to
 board/freescale/mx6sabresd/mx6q_4x_mt41j128.cfg as this is
 was designed for the mx6sabresd board. This also updates the
 cgtqmx6qeval which makes use of this configuration.

 I've made my mind up.  I hereby NAK this patch, as it would basically
 revert Fabio Estevam's commit af7ec0b which moved the originally
 board-specific code to a common place:

 commit af7ec0b0582f658873713c311497626c571f3b31
 Author: Fabio Estevam fabio.este...@freescale.com
 Date:   Thu Sep 13 03:18:19 2012 +

 mx6q: Factor out common DDR3 init code

 Factor out common DDR3 initialization code, allowing easier
 maintainance of such scripts.

 Signed-off-by: Fabio Estevam fabio.este...@freescale.com

 Fabio's intention was a good one, as is proven by the re-use of this
 code by other boards.

Let me provide some background on the reason I sent that patch: at
that time we had the same DDR3 init code for several boards, such as
mx6qsabresd, nitrogen, sabrelite, so I wanted to avoid duplicating the
same init for several boards.

After sometime, each board used to followed its own specific settings,
as the DDR3 init is very dependant on board layout and some
optimizations that are valid for one board does not apply to others.
Each board developer has to be really careful about properly
configuring DDR in order to achieve stability, so re-use of the DCD
settings should be done really carefully.

As it stands today only mx6qsabresd and congatec share the same script.

I think Nitin's patch goes in the right direction, as it makes clearer
for other developers that the DDR specific settings are optmized for
mx6qsabresd only. Of course people can re-use it, like congatec board
does today, but if in the future we find some more optimal settings
for this board we should apply it to mx6sabresd, but we really don't
know the consequences into other hardware. So they have been warned
:-)

Moving forward we should really get rid of this DCD syntax and move to
SPL style.

Regards,

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


Re: [U-Boot] [PATCH v2] imx: ddr: Move mx6q_4x_mt41j128.cfg to mx6sabresd board dir

2014-09-01 Thread Eric Nelson
Hi Fabio,

On 09/01/2014 03:27 PM, Fabio Estevam wrote:
 Hi Wolfgang,
 
 On Mon, Sep 1, 2014 at 4:24 PM, Wolfgang Denk w...@denx.de wrote:
 Dear Nitin Garg,

 In message 1409581243-12695-1-git-send-email-nitin.g...@freescale.com you 
 wrote:
 Move board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg to
 board/freescale/mx6sabresd/mx6q_4x_mt41j128.cfg as this is
 was designed for the mx6sabresd board. This also updates the
 cgtqmx6qeval which makes use of this configuration.

 I've made my mind up.  I hereby NAK this patch, as it would basically
 revert Fabio Estevam's commit af7ec0b which moved the originally
 board-specific code to a common place:

 commit af7ec0b0582f658873713c311497626c571f3b31
 Author: Fabio Estevam fabio.este...@freescale.com
 Date:   Thu Sep 13 03:18:19 2012 +

 mx6q: Factor out common DDR3 init code

 Factor out common DDR3 initialization code, allowing easier
 maintainance of such scripts.

 Signed-off-by: Fabio Estevam fabio.este...@freescale.com

 Fabio's intention was a good one, as is proven by the re-use of this
 code by other boards.
 
 Let me provide some background on the reason I sent that patch: at
 that time we had the same DDR3 init code for several boards, such as
 mx6qsabresd, nitrogen, sabrelite, so I wanted to avoid duplicating the
 same init for several boards.
 

Specifically, the Nitrogen and SABRE Lite designs use a different
memory layout from SABRE SD and SABRE Auto and the DDR calibration
data is quite different.

Boards may share the same memory arrangement, but it's unlikely
that the calibration process has been performed on multiple board
types and achieved the same values.

It's possible that many boards copied the layout and stack-up
from the SABRE SD design such that the board functions properly
with the SABRE SD values, but also likely that some vendors just
don't know that their calibration results will differ.

 After sometime, each board used to followed its own specific settings,
 as the DDR3 init is very dependant on board layout and some
 optimizations that are valid for one board does not apply to others.
 Each board developer has to be really careful about properly
 configuring DDR in order to achieve stability, so re-use of the DCD
 settings should be done really carefully.
 

Note that mx6q_4x_mt41j128.cfg combines multiple things in the
same config file.

Separating them (especially the calibration data) as done in
the nitrogen6x/ tree will help distinguish between the design-time
parts of the configuration and the measured calibration.

 As it stands today only mx6qsabresd and congatec share the same script.
 

I believe that the Wand board is using the configuration files
from the nitrogen6x tree.

 I think Nitin's patch goes in the right direction, as it makes clearer
 for other developers that the DDR specific settings are optmized for
 mx6qsabresd only. Of course people can re-use it, like congatec board
 does today, but if in the future we find some more optimal settings
 for this board we should apply it to mx6sabresd, but we really don't
 know the consequences into other hardware. So they have been warned
 :-)
 
 Moving forward we should really get rid of this DCD syntax and move to
 SPL style.
 

There's no way to completely get rid of the DCD. It may be possible
(even beneficial) to do run-time DDR calibration, but that's off-topic
in this thread.

Regards,


Eric

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


Re: [U-Boot] [PATCH v2] imx: ddr: Move mx6q_4x_mt41j128.cfg to mx6sabresd board dir

2014-09-01 Thread Fabio Estevam
On Mon, Sep 1, 2014 at 8:34 PM, Eric Nelson
eric.nel...@boundarydevices.com wrote:

 I believe that the Wand board is using the configuration files
 from the nitrogen6x tree.

Yes, I should have said As it stands today only mx6qsabresd and
congatec share the same mx6q_4x_mt41j128.cfg script.

Regards,

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