Re: [U-Boot] Booting kernel from NAND flash on AT91SAM9 custom board using fsload

2011-03-30 Thread Joakim Tjernlund
>
> Dear Nicholas Kinar,
>
> In message <4d92428e.6030...@usask.ca> you wrote:
> >
> > (1) Does replacing jffs2_1pass.c with jffs2_nand_1pass.c in the fs/jffs2
> > directory influence the robustness of the fsload code?
>
> JFFS2 is more or less deprecated these days.  FOr new projects, we
> recommend to use UBI/ UBIFS instead.

JFFS2 deprecated? It is very stable and works well for NOR based filesystems.
Do you use UBIFS for NOR as well these days?

>
> > (2) Does it take a long time to load the Linux kernel from a JFFS2
> > filesystem on NAND flash using the fsload command (i.e.
>
> JFFS2 has always been slow, especially when mounting larger file
> systems.  This is one of the resons we recommend UBIFS instead.

The u-boot impl. is a bad version of the one in linux. The u-boot
version should be rewritten/fixed to match current Linux.

 Jocke

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


Re: [U-Boot] CFI flash broken for 8-bit bus

2011-03-30 Thread Rogan Dawes
On 2011/03/26 8:32 AM, Aaron Williams wrote:
> I am still doing some testing and just added a fix today so that if a flash 
> chip that supports both 8 and 16-bits is on an 8-bit bus the interface is set 
> to 8-bits. I'll try and get patches out next week.
> 
> So far it's working fairly well on all of our boards, most of which have an 8-
> bit bus but one has a 16-bit bus. I had to change all of the defined 
> addresses 
> for AMD parts and calculate the 16 and 32-bit addresses by applying a mask.
> 
> The CFI code *should* work if there's an 8-bit part on a 16-bit bus, but it 
> probably will not support two 8-bit parts in parallel. That should be doable 
> with some work.
> 
> -Aaron

Hi Aaron,

I'm looking forward to seeing your patches. With any luck, they'll also
work on my board, and a nasty hack can go away. :-)

Regards,

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


[U-Boot] using ${var} with env import

2011-03-30 Thread Holger Brunck
Hi all,
I am using env import -t to import environment variables from a textfile.

My simple textfile is:
fdt_file=${hostname}/${hostname}.dtb

I import the file with:
=> tftp 20 scripts/my_environ.txt
=> env import -t 20 ${filesize}

Now when I print the variable I get:
=> print fdt_file
fdt_file=${hostname}/${hostname}.dtb

hostname is defined as:
=> print hostname
hostname=mgcoge

I see that the ${hostname} for my fdt_file variable is not replaced. If I do
this with setenv:
=> setenv fdt_file_2 ${hostname}/${hostname}.dtb
=> print fdt_file_2
fdt_file_2=mgcoge/mgcoge.dtb

My variable is replaced.

Is the usage of ${var} in the textfiles not possible? Or is there a way to solve
this problem?

I try to use this textfile for different boards and can therfore not specify
hostname in advance in the textfile.

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


Re: [U-Boot] Booting kernel from NAND flash on AT91SAM9 custom board using fsload

2011-03-30 Thread Wolfgang Denk
Dear Joakim Tjernlund,

In message 
 you 
wrote:
>
> JFFS2 deprecated? It is very stable and works well for NOR based filesystems.

Yes, we consider it deprecated, and we do not use it in any new
projects / products.

> Do you use UBIFS for NOR as well these days?

Yes, we do.  [And yes, we know about the problems it has in certain
configurations, but 1) these are pretty rare these days and 2) we are
working on fixes.]

> > JFFS2 has always been slow, especially when mounting larger file
> > systems.  This is one of the resons we recommend UBIFS instead.
> 
> The u-boot impl. is a bad version of the one in linux. The u-boot
> version should be rewritten/fixed to match current Linux.

Patches welcome.

Nevertheless, the Linux mount times for JFFS2 are still slower than
UBIFS times, aren't they?

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
"A fractal is by definition a set for which the Hausdorff Besicovitch
dimension strictly exceeds the topological dimension."
- Mandelbrot, _The Fractal Geometry of Nature_
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] Kirkwood: SATA supports only one device per bus

2011-03-30 Thread Albert ARIBAUD
Hi Gray,

Le 29/03/2011 21:42, Gray Remlin a écrit :
> Change CONFIG_SYS_IDE_MAXDEVICE from 2 to 1
>
> Signed-off-by: Gray Remlin
> ---
>   arch/arm/include/asm/arch-kirkwood/config.h |3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-kirkwood/config.h 
> b/arch/arm/include/asm/arch-kirkwood/config.h
> index 71ba464..f582ba4 100644
> --- a/arch/arm/include/asm/arch-kirkwood/config.h
> +++ b/arch/arm/include/asm/arch-kirkwood/config.h
> @@ -128,7 +128,8 @@
>   #define CONFIG_LBA48
>   /* CONFIG_CMD_IDE requires some #defines for ATA registers */
>   #define CONFIG_SYS_IDE_MAXBUS   2
> -#define CONFIG_SYS_IDE_MAXDEVICE 2
> +/* Only one device per bus on SATA */
> +#define CONFIG_SYS_IDE_MAXDEVICE 1
>   /* ATA registers base is at SATA controller base */
>   #define CONFIG_SYS_ATA_BASE_ADDRMV_SATA_BASE
>   #endif /* CONFIG_CMD_IDE */

Did you cross-check this patch with Rogan's recent patch?



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


Re: [U-Boot] [PATCH 2/7] tsec: arrange the code to avoid useless function declaration

2011-03-30 Thread Detlev Zundel
Hi Andy,

> From: Mingkai Hu 
>
> Signed-off-by: Mingkai Hu 
> Acked-by: Andy Fleming 
> Signed-off-by: Kumar Gala 
> ---
>  drivers/net/tsec.c |  857 
> +---
>  1 files changed, 416 insertions(+), 441 deletions(-)

Does this patch really only rearrange code and not change any
functionality?  In this case this information should be in the commit
message IMHO.

Cheers
  Detlev

-- 
It is practically impossible to teach good programming to students that have
had a  prior exposure to BASIC:  as potential  programmers they are mentally
mutilated beyond hope of regeneration.-- Edsger Dijkstra 
--
DENX Software Engineering GmbH,  MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 3/7] Remove instances of phy_read/write

2011-03-30 Thread Detlev Zundel
Hi Andy,

> There were a few files which were already using phy_read and phy_write
> for their PHY function names.  It's only a few places, and the name
> seems most appropriate for the high-level abstraction, so let's
> rename the other versions to something more specific.
>
> Also, uec_phy.c had a marvell_init function which I renamed to not
> conflict with the one in marvell.c
>
> Signed-off-by: Andy Fleming 

Looks like these are harmless changes, so

Acked-by: Detlev Zundel 

Might be worthwhile to see some Tested-by from people with the hardware
though.

Cheers
  Detlev
  
-- 
By all means let's be open-minded, but not so open-minded that our
brains drop out.
-- Richard Dawkins
--
DENX Software Engineering GmbH,  MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] CFI flash broken for 8-bit bus

2011-03-30 Thread Albert ARIBAUD
Le 30/03/2011 10:01, Rogan Dawes a écrit :
> On 2011/03/26 8:32 AM, Aaron Williams wrote:
>> I am still doing some testing and just added a fix today so that if a flash
>> chip that supports both 8 and 16-bits is on an 8-bit bus the interface is set
>> to 8-bits. I'll try and get patches out next week.
>>
>> So far it's working fairly well on all of our boards, most of which have an 
>> 8-
>> bit bus but one has a 16-bit bus. I had to change all of the defined 
>> addresses
>> for AMD parts and calculate the 16 and 32-bit addresses by applying a mask.
>>
>> The CFI code *should* work if there's an 8-bit part on a 16-bit bus, but it
>> probably will not support two 8-bit parts in parallel. That should be doable
>> with some work.
>>
>> -Aaron
>
> Hi Aaron,
>
> I'm looking forward to seeing your patches. With any luck, they'll also
> work on my board, and a nasty hack can go away. :-)

I also would like to see these patches, even in RFC form. Maybe it could 
fix the way the MX29LV400CB is detected on ED Mini V2 and will allows 
removing legacy code for this board.

> Regards,
>
> Rogan

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


Re: [U-Boot] [PATCH] Kirkwood: SATA supports only one device per bus

2011-03-30 Thread Prafulla Wadaskar


> -Original Message-
> From: Gray Remlin [mailto:gryr...@gmail.com]
> Sent: Wednesday, March 30, 2011 1:12 AM
> To: Prafulla Wadaskar
> Cc: u-boot@lists.denx.de; Gray Remlin
> Subject: [PATCH] Kirkwood: SATA supports only one device per bus
> 
> Change CONFIG_SYS_IDE_MAXDEVICE from 2 to 1

Kirkwood supports max 2
Why do you want this change?

Regards..
Prafulla .. 

> 
> Signed-off-by: Gray Remlin 
> ---
>  arch/arm/include/asm/arch-kirkwood/config.h |3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-kirkwood/config.h
> b/arch/arm/include/asm/arch-kirkwood/config.h
> index 71ba464..f582ba4 100644
> --- a/arch/arm/include/asm/arch-kirkwood/config.h
> +++ b/arch/arm/include/asm/arch-kirkwood/config.h
> @@ -128,7 +128,8 @@
>  #define CONFIG_LBA48
>  /* CONFIG_CMD_IDE requires some #defines for ATA registers */
>  #define CONFIG_SYS_IDE_MAXBUS2
> -#define CONFIG_SYS_IDE_MAXDEVICE 2
> +/* Only one device per bus on SATA */
> +#define CONFIG_SYS_IDE_MAXDEVICE 1
>  /* ATA registers base is at SATA controller base */
>  #define CONFIG_SYS_ATA_BASE_ADDR MV_SATA_BASE
>  #endif /* CONFIG_CMD_IDE */
> --
> 1.7.4

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


Re: [U-Boot] [PATCH] Kirkwood: SATA supports only one device per bus

2011-03-30 Thread Prafulla Wadaskar


> -Original Message-
> From: Prafulla Wadaskar
> Sent: Wednesday, March 30, 2011 4:48 PM
> To: 'Gray Remlin'
> Cc: u-boot@lists.denx.de; Prabhanjan Sarnaik; Ashish Karkare
> Subject: RE: [PATCH] Kirkwood: SATA supports only one device per bus
> 
> 
> 
> > -Original Message-
> > From: Gray Remlin [mailto:gryr...@gmail.com]
> > Sent: Wednesday, March 30, 2011 1:12 AM
> > To: Prafulla Wadaskar
> > Cc: u-boot@lists.denx.de; Gray Remlin
> > Subject: [PATCH] Kirkwood: SATA supports only one device per bus
> >
> > Change CONFIG_SYS_IDE_MAXDEVICE from 2 to 1
> 
> Kirkwood supports max 2
> Why do you want this change?

Hi Gray
I take back my words. I will study more and let you know my comments on this.
Sorry for noise.

Regards..
Prafulla . .

> 
> Regards..
> Prafulla ..
> 
> >
> > Signed-off-by: Gray Remlin 
> > ---
> >  arch/arm/include/asm/arch-kirkwood/config.h |3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/arch-kirkwood/config.h
> > b/arch/arm/include/asm/arch-kirkwood/config.h
> > index 71ba464..f582ba4 100644
> > --- a/arch/arm/include/asm/arch-kirkwood/config.h
> > +++ b/arch/arm/include/asm/arch-kirkwood/config.h
> > @@ -128,7 +128,8 @@
> >  #define CONFIG_LBA48
> >  /* CONFIG_CMD_IDE requires some #defines for ATA registers */
> >  #define CONFIG_SYS_IDE_MAXBUS  2
> > -#define CONFIG_SYS_IDE_MAXDEVICE   2
> > +/* Only one device per bus on SATA */
> > +#define CONFIG_SYS_IDE_MAXDEVICE   1
> >  /* ATA registers base is at SATA controller base */
> >  #define CONFIG_SYS_ATA_BASE_ADDR   MV_SATA_BASE
> >  #endif /* CONFIG_CMD_IDE */
> > --
> > 1.7.4

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


Re: [U-Boot] [PATCH] ARM: Add support for displaying second ethaddr in 'bdinfo' command

2011-03-30 Thread Albert ARIBAUD
Le 30/03/2011 00:36, Gray Remlin a écrit :
> On 03/29/2011 10:49 PM, Wolfgang Denk wrote:
>> Dear Gray Remlin,
>>
>> In message<1301433395-25203-1-git-send-email-gryr...@gmail.com>   you wrote:
>>> Signed-off-by: Gray Remlin
>>> ---
>>>common/cmd_bdinfo.c |3 +++
>>>1 files changed, 3 insertions(+), 0 deletions(-)
>> Why limit this to eth1addr?  What's the chances that ARM systems may
>> have more than 2 network interfaces?
>>
>> Best regards,
>>
>> Wolfgang Denk
>>
> Good question, I have already asked myself this, so I have stuck with
> what I do know.
>
> 1. It has had the one ethaddr limit for a (relatively) long time, which
> it seems no-one else 'required'\'submitted a patch' to change it.
> 2. I only know (with my very limited knowledge) of ARM boards with a
> maximum of two interfaces as standard.
> 3. Why limit it to six (as in other parts of the source) ?
>
> My answer:
> It is not my place to dictate policy, that is the role of the Project
> Manager.
> And no, that is not a 'cop-out', it is the only way to avoid anarchy.

There is no ARM board right now which defined CONFIG_HAS_ETH2 or higher, 
and I have seen no ARM code which assumes otherwise (readers feel free 
to prove me wrong, of course).

Besides, while contributors are always welcome to generalize their 
patches beyond their needs if they so accept, by no means are they 
forced to do so against their will when their contribution is consistent 
with the existing code state.

I thus consider Gray's patch is OK and, unless some review is requested 
in the near future, will pull it in u-boot-arm (*not* for inclusion in 
2011-03, of course).

If someone wants support for CONFIG_HAS_ETH2 or higher on ARM, they are 
welcome to extend Gray's work, even to merge some ARM and PPC parts of 
common/cmd_bd_info.c.

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


Re: [U-Boot] [PATCH 4/7] Create PHY Lib for U-Boot

2011-03-30 Thread Detlev Zundel
Hi Andy,

> Extends the mii_dev structure to participate in a full-blown MDIO and
> PHY driver scheme.  The mii_dev structure and miiphy calls are modified
> in such a way to allow the original mii command and miiphy
> infrastructure to work as before, but also to support a new set of APIs
> which allow (among other things) sharing of PHY driver code and 10G support
>
> The mii command will continue to support normal PHY management functions
> (Clause 22 of 802.3), but will not be changed to support 10G
> (Clause 45).
>
> The basic design is similar to PHY Lib from Linux, but simplified for
> U-Boot's network and driver infrastructure.
>
> We now have MDIO drivers and PHY drivers
>
> An MDIO driver provides:
> read
> write
> reset
>
> A PHY driver provides:
> (optionally): probe
> config - initial setup, starting of auto-negotiation
> startup - waiting for AN, and reading link state
> shutdown - any cleanup needed
>
> The ethernet drivers interact with the PHY Lib using these functions:
> phy_connect()
> phy_config()
> phy_startup()
> phy_shutdown()
>
> Each PHY driver can be configured separately, or all at once using
> phylib_all_drivers.h (added in the patch which adds the drivers)
>
> Signed-off-by: Andy Fleming 

Looks really good - a few comments:

[...]

> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> new file mode 100644
> index 000..ca35404
> --- /dev/null
> +++ b/drivers/net/phy/phy.c
> @@ -0,0 +1,705 @@
> +/*
> + * Generic PHY Management code
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.
> + *
> + * Copyright 2011 Freescale Semiconductor, Inc.
> + * author Andy Fleming
> + *
> + * Based loosely off of Linux's PHY Lib

According to http://www.denx.de/wiki/U-Boot/Patches new files need
GPLv2+ headers and if I check Linux, I see GPLv2+ in
drivers/net/phy/phy.c (and other people have contributed under this), so
please change accrodingly.

> +/**
> + * genphy_update_link - update link status in @phydev
> + * @phydev: target phy_device struct
> + *
> + * Description: Update the value in phydev->link to reflect the
> + *   current link value.  In order to do this, we need to read
> + *   the status register twice, keeping the second value.
> + */
> +int genphy_update_link(struct phy_device *phydev)
> +{
> + unsigned int mii_reg;
> +
> + /*
> +  * Wait if the link is up, and autonegotiation is in progress
> +  * (ie - we're capable and it's not done)
> +  */
> + mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMSR);
> +
> + /*
> +  * If we already saw the link up, and it hasn't gone down, then
> +  * we don't need to wait for autoneg again
> +  */
> + if (phydev->link && mii_reg & BMSR_LSTATUS)
> + return 0;
> +
> + if ((mii_reg & BMSR_ANEGCAPABLE) && !(mii_reg & BMSR_ANEGCOMPLETE)) {
> + int i = 0;
> +
> + printf("%s Waiting for PHY auto negotiation to complete",
> + phydev->dev->name);
> + while (!(mii_reg & BMSR_ANEGCOMPLETE)) {
> + /*
> +  * Timeout reached ?
> +  */
> + if (i > 5000) {
> + printf(" TIMEOUT !\n");
> + phydev->link = 0;
> + return 0;
> + }
> +
> + if (ctrlc()) {
> + puts("user interrupt!\n");
> + phydev->link = 0;
> + return -EINTR;
> + }
> +
> + if ((i++ % 500) == 0)
> + printf(".");
> +
> + udelay(1000);   /* 1 ms */
> + mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMSR);
> + }
> + printf(" done\n");
> + phydev->link = 1;
> + udelay(50); /* another 500 ms (results in faster booting) */

I don't understand this, why does sleeping half a second speed up
booting?  A comment would be in order here.

> + } else {
> + /* Read the link a second time to clear the latched state */
> + mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMSR);
> +
> + if (mii_reg & BMSR_LSTATUS)
> + phydev->link = 1;
> + else
> + phydev->link = 0;
> + }
> +
> + return 0;
> +}

[...]

> +struct phy_driver *get_phy_driver(struct phy_device *phydev,
> + phy_interface_t interface)
> +{
> + struct list_head *entry;
> + int phy_id = phydev->phy_id;
> + struct phy_driver *drv = NULL;
> +
> + list_for_each(entry, &phy_drivers) {
> + drv = list_entry(entry, struct phy_driver, list);
> + if ((drv->uid & drv->mask) == (phy_id & drv->mask))
> + return 

Re: [U-Boot] [PATCH 5/7] Add mdio command for new PHY infrastructure

2011-03-30 Thread Detlev Zundel
Hi Andy,

> The new mdio command doesn't have all of the features of the mii
> command, but it provides the necessary read/write primitives, and allows
> users to interact with 10G PHYs, and other PHYs which use Clause 45 of
> 802.3.  This means that the mdio command requires a "Device Address"
> argument, though for clause 22 PHYs, the argument can be "-".
>
> Signed-off-by: Andy Fleming 
> ---
>  common/Makefile   |3 +
>  common/cmd_mdio.c |  293 
> +
>  2 files changed, 296 insertions(+), 0 deletions(-)
>  create mode 100644 common/cmd_mdio.c

[...]

> +static int extract_range(char *input, int *plo, int *phi)
> +{
> + char * end;
> + *plo = simple_strtol(input, &end, 0);
> + if (end == input)
> + return -1;
> +
> + if (*end == '-') {

What about the case of input="12-"? Shouldn't there be an "&& *(end+1)"?

> + end++;
> + *phi = simple_strtol(end, NULL, 0);
> + } else if (*end == '\0') {
> + *phi = *plo;
> + } else
> + return -1;
> +
> + return 0;
> +}

Other than that, looks good, so once my above concern is addressed

Acked-by: Detlev Zundel 

Thanks!
  Detlev

-- 
Choosing which tool to use is a problem for most users. Therefore
when one tool came along that did everything Perl (Ugly) it took over.
-- Rob Pike
--
DENX Software Engineering GmbH,  MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [RFC 0/7] Universal PHY Infrastructure

2011-03-30 Thread Detlev Zundel
Hi Andy,

> Or PHY Lib for U-Boot.
>
> This sequence of patches adds infrastructure for universally-available PHY
> drivers (and MDIO drivers).  It piggy-backs on the existing miiphy code, for
> backwards compatibility, but it also creates a new set of APIs. This was
> necessary partly to provide cleaner interfaces for more robust driver
> support, and partly because one goal was to support 10G (802.3 Clause 45) MDIO
> buses, which has an extra argument for addressing PHY registers.
>
> The first three patches clear the way, and are in this sequence mostly
> because the tsec changes depend on them.
>
> Special thanks goes to Mingkai Hu, who did a substantial amount
> of work up front to convert the tsec PHY code into something more usable,
> which I have mostly copied for the purposes of PHY Lib.
>
> As the subject says, these are submitted here for comment.  I hope they
> will go in for the presumptive June release.
>
> Andy Fleming (5):
>   Remove instances of phy_read/write
>   Create PHY Lib for U-Boot
>   Add mdio command for new PHY infrastructure
>   phylib: Add a bunch of PHY drivers from tsec
>   tsec: Convert tsec to use PHY Lib
>
> Mingkai Hu (2):
>   tsec: use IO accessories to access the register
>   tsec: arrange the code to avoid useless function declaration

Thanks for this extensive work!  Still the whole series has checkpatch
problems, so please clean them up:

total: 29 errors, 95 warnings, 7792 lines checked

Some of them can be ignored, but some like this really need fixing:

ERROR: trailing statements should be on next line
#206: FILE: drivers/net/tsec.c:243:
+   while ((in_be32(&phyregs->miimind) & MIIMIND_BUSY) && timeout--) ;

Cheers
  Detlev

-- 
Emacs seems a more likely candidate  to contain a mail system than the
mail system to contain an Emacs, so this is the way it was done.
-- Bernard S. Greenberg
--
DENX Software Engineering GmbH,  MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 6/7] phylib: Add a bunch of PHY drivers from tsec

2011-03-30 Thread Detlev Zundel
Hi Andy,

> The tsec driver had a bunch of PHY drivers already written. This
> converts them all into PHY Lib drivers, and serves as the first
> set of PHY drivers for PHY Lib.
>
> Signed-off-by: Andy Fleming 

[...]

> diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
> new file mode 100644
> index 000..68bb5cd
> --- /dev/null
> +++ b/drivers/net/phy/atheros.c
> @@ -0,0 +1,37 @@
> +/*
> + * Atheros PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

Again, new files need GPLv2+.  I know that this is split out from a
GPLv2 file, but we have our rules in the meantime.

[...]

> diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
> new file mode 100644
> index 000..e62a81d
> --- /dev/null
> +++ b/drivers/net/phy/broadcom.c
> @@ -0,0 +1,275 @@
> +/*
> + * Broadcom PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

GPLv2+


[...]

> +static int bcm54xx_parse_status(struct phy_device *phydev)
> +{
> + unsigned int mii_reg;
> +
> + mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXSTATUS);
> +
> + switch ((mii_reg & MIIM_BCM54xx_AUXSTATUS_LINKMODE_MASK) >>
> + MIIM_BCM54xx_AUXSTATUS_LINKMODE_SHIFT) {
> + case 1:
> + phydev->duplex = DUPLEX_HALF;
> + phydev->speed = SPEED_10;
> + break;
> + case 2:
> + phydev->duplex = DUPLEX_FULL;
> + phydev->speed = SPEED_10;
> + break;
> + case 3:
> + phydev->duplex = DUPLEX_HALF;
> + phydev->speed = SPEED_100;
> + break;
> + case 5:
> + phydev->duplex = DUPLEX_FULL;
> + phydev->speed = SPEED_100;
> + break;
> + case 6:
> + phydev->duplex = DUPLEX_HALF;
> + phydev->speed = SPEED_1000;
> + break;
> + case 7:
> + phydev->duplex = DUPLEX_FULL;
> + phydev->speed = SPEED_1000;
> + break;
> + default:
> + printf("Auto-neg error, defaulting to 10BT/HD\n");
> + phydev->duplex = DUPLEX_HALF;
> + phydev->speed = SPEED_10;
> + break;

Very strange indentation, please fix.

[...]

> diff --git a/drivers/net/phy/davicom.c b/drivers/net/phy/davicom.c
> new file mode 100644
> index 000..229e1bd
> --- /dev/null
> +++ b/drivers/net/phy/davicom.c
> @@ -0,0 +1,86 @@
> +/*
> + * Davicom PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

GPLv2+

[...]

> diff --git a/drivers/net/phy/lxt.c b/drivers/net/phy/lxt.c
> new file mode 100644
> index 000..4e38bb6
> --- /dev/null
> +++ b/drivers/net/phy/lxt.c
> @@ -0,0 +1,76 @@
> +/*
> + * LXT PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

GPLv2+

[...]

> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> new file mode 100644
> index 000..6937c93
> --- /dev/null
> +++ b/drivers/net/phy/marvell.c
> @@ -0,0 +1,357 @@
> +/*
> + * Marvell PHY drivers
> + *
> + * This software may be used and distributed according to the
> + * terms of the GNU Public License, Version 2, incorporated
> + * herein by reference.

dito

[...]

> +/* Parse the 88E1011's status register for speed and duplex
> + * information
> + */
> +static uint m88e1011s_parse_status(struct phy_device *phydev)
> +{
> + unsigned int speed;
> + unsigned int mii_reg;
> +
> + mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_88E1011_PHY_STATUS);
> +
> + if ((mii_reg & MIIM_88E1011_PHYSTAT_LINK) &&
> + !(mii_reg & MIIM_88E1011_PHYSTAT_SPDDONE)) {
> + int i = 0;
> +
> + puts("Waiting for PHY realtime link");
> + while (!(mii_reg & MIIM_88E1011_PHYSTAT_SPDDONE)) {
> + /* Timeout reached ? */
> + if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
> + puts(" TIMEOUT !\n");
> + phydev->link = 0;
> + break;
> + }
> +
> + if ((i++ % 1000) == 0) {
> + putc('.');
> + }
> + udelay(1000);
> + mii_reg = phy_read(phydev, MDIO_DEVAD_NONE,
> + MIIM_88E1011_PHY_STATUS);
> + }
> + puts(" done\n");
> + udelay(50); /* another 500 ms (results in faster booting) */

Ah, again the "faster booting when waiting half a

Re: [U-Boot] [PATCH 7/7] tsec: Convert tsec to use PHY Lib

2011-03-30 Thread Detlev Zundel
Hi Andy,

> This converts tsec to use the new PHY Lib.  All of the old PHY support
> is ripped out.  The old MDIO driver is split off, and placed in
> fsl_mdio.c.  The initialization is modified to initialize the MDIO
> driver as well.  The powerpc config file is modified to configure PHYLIB
> if TSEC_ENET is configured.
>
> Signed-off-by: Mingkai Hu 
> Signed-off-by: Andy Fleming 

[...]

> diff --git a/drivers/net/fsl_mdio.c b/drivers/net/fsl_mdio.c
> new file mode 100644
> index 000..3239171
> --- /dev/null
> +++ b/drivers/net/fsl_mdio.c
> @@ -0,0 +1,117 @@
> +/*
> + * Copyright 2009-2010 Freescale Semiconductor, Inc.
> + *   Jun-jie Zhang 
> + *   Mingkai Hu 
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +void tsec_local_mdio_write(tsec_mii_t *phyregs, int port_addr,
> + int dev_addr, int regnum, int value)
> +{
> + int timeout = 100;
> +
> + out_be32(&phyregs->miimadd, (port_addr << 8) | (regnum & 0x1f));
> + out_be32(&phyregs->miimcon, value);
> + asm("sync");
> +
> + while ((in_be32(&phyregs->miimind) & MIIMIND_BUSY) && timeout--);

Semicolon needs to be on its own line.

> +}
> +
> +int tsec_local_mdio_read(tsec_mii_t *phyregs, int port_addr,
> + int dev_addr, int regnum)
> +{
> + int value;
> + int timeout = 100;
> +
> + /* Put the address of the phy, and the register
> +  * number into MIIMADD */
> + out_be32(&phyregs->miimadd, (port_addr << 8) | (regnum & 0x1f));
> +
> + /* Clear the command register, and wait */
> + out_be32(&phyregs->miimcom, 0);
> + asm("sync");
> +
> + /* Initiate a read command, and wait */
> + out_be32(&phyregs->miimcom, MIIMCOM_READ_CYCLE);
> + asm("sync");
> +
> + /* Wait for the the indication that the read is done */
> + while ((in_be32(&phyregs->miimind) & (MIIMIND_NOTVALID | MIIMIND_BUSY))
> + && timeout--);

dito.

Other than that this also looks good, so once my concerns are addressed:

Acked-by: Detlev Zundel 

Cheers
  Detlev

-- 
Markov does it in chains.
--
DENX Software Engineering GmbH,  MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: Add support for displaying second ethaddr in 'bdinfo' command

2011-03-30 Thread Detlev Zundel
Hi Gray,

> On 03/29/2011 10:49 PM, Wolfgang Denk wrote:
>> Dear Gray Remlin,
>>
>> In message<1301433395-25203-1-git-send-email-gryr...@gmail.com>  you wrote:
>>> Signed-off-by: Gray Remlin
>>> ---
>>>   common/cmd_bdinfo.c |3 +++
>>>   1 files changed, 3 insertions(+), 0 deletions(-)
>> Why limit this to eth1addr?  What's the chances that ARM systems may
>> have more than 2 network interfaces?
>>
>> Best regards,
>>
>> Wolfgang Denk
>>
> Good question, I have already asked myself this, so I have stuck with 
> what I do know.
>
> 1. It has had the one ethaddr limit for a (relatively) long time, which 
> it seems no-one else 'required'\'submitted a patch' to change it.
> 2. I only know (with my very limited knowledge) of ARM boards with a 
> maximum of two interfaces as standard.
> 3. Why limit it to six (as in other parts of the source) ?

Because todays CPUs get drowned with six saturated links? ;)

> My answer:
> It is not my place to dictate policy, that is the role of the Project 
> Manager.
> And no, that is not a 'cop-out', it is the only way to avoid anarchy.

If we extend something, it is a good idea to sync to other places in the
sources.

Cheers
  Detlev

-- 
Question: If you were redesigning UNIX, what would you do differently?
Ken Thompson: I'd spell creat with an e.
--
DENX Software Engineering GmbH,  MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] Kirkwood: SATA supports only one device per bus

2011-03-30 Thread Gray Remlin
On 03/30/2011 11:51 AM, Albert ARIBAUD wrote:
> Hi Gray,
>
> Le 29/03/2011 21:42, Gray Remlin a écrit :
>> Change CONFIG_SYS_IDE_MAXDEVICE from 2 to 1
>>
>> Signed-off-by: Gray Remlin
>> ---
>>arch/arm/include/asm/arch-kirkwood/config.h |3 ++-
>>1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-kirkwood/config.h 
>> b/arch/arm/include/asm/arch-kirkwood/config.h
>> index 71ba464..f582ba4 100644
>> --- a/arch/arm/include/asm/arch-kirkwood/config.h
>> +++ b/arch/arm/include/asm/arch-kirkwood/config.h
>> @@ -128,7 +128,8 @@
>>#define CONFIG_LBA48
>>/* CONFIG_CMD_IDE requires some #defines for ATA registers */
>>#define CONFIG_SYS_IDE_MAXBUS 2
>> -#define CONFIG_SYS_IDE_MAXDEVICE2
>> +/* Only one device per bus on SATA */
>> +#define CONFIG_SYS_IDE_MAXDEVICE1
>>/* ATA registers base is at SATA controller base */
>>#define CONFIG_SYS_ATA_BASE_ADDR  MV_SATA_BASE
>>#endif /* CONFIG_CMD_IDE */
> Did you cross-check this patch with Rogan's recent patch?
>
> 
>
> Amicalement,
No, this patch does not take Rogan's recent patch into consideration.

Rogan's patch does not address issues such as (from cmd_ide.c)

'array[CONFIG_SYS_IDE_MAXDEVICE]'

'for (i=0; ihttp://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] using ${var} with env import

2011-03-30 Thread Wolfgang Denk
Dear Holger Brunck,

In message <4d92e7b4.9010...@keymile.com> you wrote:
>
> I am using env import -t to import environment variables from a textfile.
> 
> My simple textfile is:
> fdt_file=${hostname}/${hostname}.dtb
> 
> I import the file with:
> => tftp 20 scripts/my_environ.txt
> => env import -t 20 ${filesize}
> 
> Now when I print the variable I get:
> => print fdt_file
> fdt_file=${hostname}/${hostname}.dtb
> 
> hostname is defined as:
> => print hostname
> hostname=mgcoge

This is perfectly normal. "env export" and "env import" are inverse
operations - they export and import the environment data directly,
without any conversions (except for the formatting as text lines
versus NUL-terminated strings).  Neither of these functions performs
any variable substitutions - these are done in the command
interpreter, i. e. when you run a command in the shell.

> Is the usage of ${var} in the textfiles not possible? Or is there a way to 
> solve
> this problem?

I understand that with "usage of ${var}" you mean variable subsitution
- this is indeed not supposed to happen during an "env import".

I don't consider this a problem, though.  If you want such
substituion, use the defined strings in shell commands.

Actually I consider it more clever to _keep_ the ${hostname} stuff in
your variable definitions, as then it is sufficient to change the
"hostname" variable to take affect everywhere; if you subsitute the
value hard in other variables, you would have to fix all of these (and
provide code to do so).

So actually I think this is not a problem, it just points out some
inefficient usage of the environment in your setup.

> I try to use this textfile for different boards and can therfore not specify
> hostname in advance in the textfile.

You don't have to. It does not matter when it gets defined (as long as
the variables referencing it are not needed before that).

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
Genitiv ins Wasser, weil's Dativ ist!
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: Add support for displaying second ethaddr in 'bdinfo' command

2011-03-30 Thread Gray Remlin
On 03/30/2011 12:34 PM, Albert ARIBAUD wrote:
> Le 30/03/2011 00:36, Gray Remlin a écrit :
>> On 03/29/2011 10:49 PM, Wolfgang Denk wrote:
>>> Dear Gray Remlin,
>>>
>>> In message<1301433395-25203-1-git-send-email-gryr...@gmail.com>you 
>>> wrote:
 Signed-off-by: Gray Remlin
 ---
 common/cmd_bdinfo.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
>>> Why limit this to eth1addr?  What's the chances that ARM systems may
>>> have more than 2 network interfaces?
>>>
>>> Best regards,
>>>
>>> Wolfgang Denk
>>>
>> Good question, I have already asked myself this, so I have stuck with
>> what I do know.
>>
>> 1. It has had the one ethaddr limit for a (relatively) long time, which
>> it seems no-one else 'required'\'submitted a patch' to change it.
>> 2. I only know (with my very limited knowledge) of ARM boards with a
>> maximum of two interfaces as standard.
>> 3. Why limit it to six (as in other parts of the source) ?
>>
>> My answer:
>> It is not my place to dictate policy, that is the role of the Project
>> Manager.
>> And no, that is not a 'cop-out', it is the only way to avoid anarchy.
> There is no ARM board right now which defined CONFIG_HAS_ETH2 or higher,
> and I have seen no ARM code which assumes otherwise (readers feel free
> to prove me wrong, of course).
>
> Besides, while contributors are always welcome to generalize their
> patches beyond their needs if they so accept, by no means are they
> forced to do so against their will when their contribution is consistent
> with the existing code state.
>
> I thus consider Gray's patch is OK and, unless some review is requested
> in the near future, will pull it in u-boot-arm (*not* for inclusion in
> 2011-03, of course).
>
> If someone wants support for CONFIG_HAS_ETH2 or higher on ARM, they are
> welcome to extend Gray's work, even to merge some ARM and PPC parts of
> common/cmd_bd_info.c.
>
> Amicalement,
I hold regard for 'small is beautiful' and want to see u-boot both
'complete' and 'concise', sometimes a difficult juggling act.

There are two different variants of the same ARM board that I am working
with, the variant I have has two ethernet ports, the other only one.
I am considering releasing a patch for the configuration header file 
with the
differences commented to make it obvious, until u-boot supports this, 
there is
no point in submitting the patch other than to confuse people.

There is a new recently released ARM board that also has two ethernet ports,
but as this differs in memory configuration, will not likely share the same
configuration header, but will still require support.

These three ARM boards are all Kirkwood family.
If they release another with more ports, I will most likely submit the 
patch myself
(providing I can test it first of course, any freebies Globalscale ?).






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


Re: [U-Boot] [PATCH] Kirkwood: SATA supports only one device per bus

2011-03-30 Thread Gray Remlin
On 03/30/2011 12:21 PM, Prafulla Wadaskar wrote:
>
>> -Original Message-
>> From: Prafulla Wadaskar
>> Sent: Wednesday, March 30, 2011 4:48 PM
>> To: 'Gray Remlin'
>> Cc: u-boot@lists.denx.de; Prabhanjan Sarnaik; Ashish Karkare
>> Subject: RE: [PATCH] Kirkwood: SATA supports only one device per bus
>>
>>
>>
>>> -Original Message-
>>> From: Gray Remlin [mailto:gryr...@gmail.com]
>>> Sent: Wednesday, March 30, 2011 1:12 AM
>>> To: Prafulla Wadaskar
>>> Cc: u-boot@lists.denx.de; Gray Remlin
>>> Subject: [PATCH] Kirkwood: SATA supports only one device per bus
>>>
>>> Change CONFIG_SYS_IDE_MAXDEVICE from 2 to 1
>> Kirkwood supports max 2
>> Why do you want this change?
> Hi Gray
> I take back my words. I will study more and let you know my comments on this.
> Sorry for noise.
>
> Regards..
> Prafulla . .
>
>> Regards..
>> Prafulla ..
>>
>>> Signed-off-by: Gray Remlin
>>> ---
>>>   arch/arm/include/asm/arch-kirkwood/config.h |3 ++-
>>>   1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/include/asm/arch-kirkwood/config.h
>>> b/arch/arm/include/asm/arch-kirkwood/config.h
>>> index 71ba464..f582ba4 100644
>>> --- a/arch/arm/include/asm/arch-kirkwood/config.h
>>> +++ b/arch/arm/include/asm/arch-kirkwood/config.h
>>> @@ -128,7 +128,8 @@
>>>   #define CONFIG_LBA48
>>>   /* CONFIG_CMD_IDE requires some #defines for ATA registers */
>>>   #define CONFIG_SYS_IDE_MAXBUS 2
>>> -#define CONFIG_SYS_IDE_MAXDEVICE   2
>>> +/* Only one device per bus on SATA */
>>> +#define CONFIG_SYS_IDE_MAXDEVICE   1
>>>   /* ATA registers base is at SATA controller base */
>>>   #define CONFIG_SYS_ATA_BASE_ADDR  MV_SATA_BASE
>>>   #endif /* CONFIG_CMD_IDE */
>>> --
>>> 1.7.4
Hi Prafulla,

This one was a bit of a judgement call.
You are absolutely correct in what you say that Kirkwood supports two, but
to the best of my knowledge, no one has yet implemented a board using
both busses (I am not that familiar with OpenRD, I may have to eat my 
words).

For the sheevaplug, guruplug & dreamplug, it is definitely the case that 
only
one device can be attached.







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


Re: [U-Boot] [PATCH V5 4/6] I2C: add i2c support for Pantheon platform

2011-03-30 Thread Lei Wen
Hi Prafulla,

On Tue, Mar 29, 2011 at 9:07 PM, Prafulla Wadaskar  wrote:
>
>
>> -Original Message-
>> From: Lei Wen [mailto:lei...@marvell.com]
>> Sent: Monday, March 28, 2011 12:24 PM
>> To: Heiko Schocher; Prafulla Wadaskar; Wolfgang Denk; u-
>> b...@lists.denx.de; Marek Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu
>> Tang; adrian.w...@gmail.com
>> Subject: [PATCH V5 4/6] I2C: add i2c support for Pantheon platform
>>
>> Add i2c support to dkb board with pantheon soc.
>>
>> Signed-off-by: Lei Wen 
>> ---
>> Changelog:
>> V2:
>> NO CHANGE
>>
>> V3:
>> clean code sytle issue
>> Add i2c clock enable code include in I2C configure define block
>>
>> V4:
>> make i2c definition included in the ifdef
>>
>> V5:
>> NO CHANGE
>>
>>  arch/arm/cpu/arm926ejs/pantheon/cpu.c    |   12 
>>  arch/arm/include/asm/arch-pantheon/cpu.h |    4 +++-
>>  arch/arm/include/asm/arch-pantheon/mfp.h |    6 --
>>  board/Marvell/dkb/dkb.c                  |    4 
>>  include/configs/dkb.h                    |   13 +
>>  5 files changed, 36 insertions(+), 3 deletions(-)
>>
> ...snip...
>
>> diff --git a/include/configs/dkb.h b/include/configs/dkb.h
>> index 638af5e..599c8b8 100644
>> --- a/include/configs/dkb.h
>> +++ b/include/configs/dkb.h
>> @@ -56,6 +56,19 @@
>>  #include "mv-common.h"
>>
>>  #undef CONFIG_ARCH_MISC_INIT
>> +
>> +/*
>> + * I2C definition
>> + */
>> +#define CONFIG_CMD_I2C
>
> This command definition should be moved up (below #include 
> 

I'm ok to put this define to the config_cmd_default.h, but this mean
many other platform need
which didn't not need the i2c but include the ,
need to undef the i2c now.
Does that worth the change?

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


Re: [U-Boot] [PATCH V4 1/6] io: add and* and or* operation api to set and clear bit

2011-03-30 Thread Lei Wen
Hi Scott,

On Wed, Mar 30, 2011 at 12:03 AM, Scott Wood  wrote:
> On Tue, 29 Mar 2011 10:47:04 +0800
> Lei Wen  wrote:
>
>> Hi Scott,
>>
>> On Tue, Mar 29, 2011 at 12:05 AM, Scott Wood  wrote:
>> > What does this do that setbits*/clrbits* don't?
>>
>> Those and*/or* include the dmb() operation included in the read*/write*, 
>> which
>> is not included in the __raw_read*/__raw_write* that setbits*/clrbits* refer 
>> to.
>> I think it's better to keep another instance to set the bit, since
>> there is read* and __raw_read*
>> exist and have difference.
>
> But why are setbits/clrbits using raw accesses?  That's not how they're
> defined on powerpc, which is where these accessors originated.  I suspect
> they were defined that way out of laziness from before ARM made a
> distinction between raw and non-raw accessors, and it is now a bug in
> setbits/clrbits (and out_be*, in_le*, etc) which should be fixed rather
> than introducing an alternative.
>
> And then if you want a raw version of these functions, introduce
> raw_setbits_le32, raw_in_le16, etc.
>


Yep, that make sense to me. But since this patch is tend to be more discussion.
I'm going to remove this change out of this patch set, and post seperately.
Meanwhile, just use the writel(readl* style in this patch set for current stage.

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


Re: [U-Boot] [PATCH V5 3/6] mv_i2c: use structure to replace the direclty define

2011-03-30 Thread Lei Wen
Hi Prafulla,

On Tue, Mar 29, 2011 at 9:27 PM, Prafulla Wadaskar  wrote:
>
>
>> -Original Message-
>> From: Lei Wen [mailto:lei...@marvell.com]
>> Sent: Monday, March 28, 2011 12:24 PM
>> To: Heiko Schocher; Prafulla Wadaskar; Wolfgang Denk; u-
>> b...@lists.denx.de; Marek Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu
>> Tang; adrian.w...@gmail.com
>> Subject: [PATCH V5 3/6] mv_i2c: use structure to replace the direclty
>> define
>>
>> Add i2c_clk_enable in the cpu specific code, since previous platform it,
>> while new platform don't need. In the pantheon and armada100 platform,
>> this function is defined as NULL one.
>>
>> Signed-off-by: Lei Wen 
>> ---
>> Changelog:
>> V2:
>> NO CHANGE
>>
>> V3:
>> clean code sytle issue
>>
>> V4:
>> V5:
>> NO CHANGE
>>
>>  arch/arm/cpu/pxa/cpu.c                   |   11 +++
>>  arch/arm/include/asm/arch-pxa/pxa-regs.h |   56 -
>>  board/innokom/innokom.c                  |    9 +--
>>  drivers/i2c/mv_i2c.c                     |  131 ++-
>> ---
>>  drivers/i2c/mv_i2c.h                     |   83 +++
>>  include/configs/innokom.h                |    1 +
>>  include/configs/xm250.h                  |    1 +
>>  7 files changed, 159 insertions(+), 133 deletions(-)
>>  create mode 100644 drivers/i2c/mv_i2c.h
>>
> ...snip...
>> diff --git a/drivers/i2c/mv_i2c.c b/drivers/i2c/mv_i2c.c
>> index 09756a4..734148b 100644
>> --- a/drivers/i2c/mv_i2c.c
>> +++ b/drivers/i2c/mv_i2c.c
>> @@ -8,6 +8,9 @@
>>   * (C) Copyright 2003 Pengutronix e.K.
>>   * Robert Schwebel 
>>   *
>> + * (C) Copyright 2011 Marvell Inc.
>> + * Lei Wen 
>> + *
>>   * See file CREDITS for list of people who contributed to this
>>   * project.
>>   *
>> @@ -34,24 +37,8 @@
>>  #include 
>>
>>  #ifdef CONFIG_HARD_I2C
>> -
>> -/*
>> - *   - CONFIG_SYS_I2C_SPEED
>> - *   - I2C_PXA_SLAVE_ADDR
>> - */
>> -
>> -#include 
>> -#include 
>>  #include 
>> -
>> -#if (CONFIG_SYS_I2C_SPEED == 40)
>> -#define I2C_ICR_INIT (ICR_FM | ICR_BEIE | ICR_IRFIE | ICR_ITEIE |
>> ICR_GCD \
>> -                     | ICR_SCLE)
>> -#else
>> -#define I2C_ICR_INIT (ICR_BEIE | ICR_IRFIE | ICR_ITEIE | ICR_GCD |
>> ICR_SCLE)
>> -#endif
>> -
>> -#define I2C_ISR_INIT         0x7FF
>> +#include "mv_i2c.h"
>>
>>  #ifdef DEBUG_I2C
>>  #define PRINTD(x) printf x
>> @@ -59,20 +46,6 @@
>>  #define PRINTD(x)
>>  #endif
>>
>> -/* Shall the current transfer have a start/stop condition? */
>> -#define I2C_COND_NORMAL              0
>> -#define I2C_COND_START               1
>> -#define I2C_COND_STOP                2
>> -
>> -/* Shall the current transfer be ack/nacked or being waited for it? */
>> -#define I2C_ACKNAK_WAITACK   1
>> -#define I2C_ACKNAK_SENDACK   2
>> -#define I2C_ACKNAK_SENDNAK   4
>> -
>> -/* Specify who shall transfer the data (master or slave) */
>> -#define I2C_READ             0
>> -#define I2C_WRITE            1
>> -
>>  /* All transfers are described by this data structure */
>>  struct i2c_msg {
>>       u8 condition;
>> @@ -81,27 +54,37 @@ struct i2c_msg {
>>       u8 data;
>>  };
>>
>> +struct pxa_i2c {
>> +     u32 ibmr;
>> +     u32 pad0;
>> +     u32 idbr;
>> +     u32 pad1;
>> +     u32 icr;
>> +     u32 pad2;
>> +     u32 isr;
>> +     u32 pad3;
>> +     u32 isar;
>> +};
>
> (Optional to implement)
> It is better if you can push register structure definition to the SoC 
> specific header file, so that if there are new SoC that has different 
> register structures that can be addressed cleanly.

For current there is no different register arrage for this structure,
so I think it is
ok to just keep it current state. For the adding register doc
description, I have no
objection.
>
>> +
>> +static struct pxa_i2c *base = (struct pxa_i2c *)CONFIG_PXA_I2C_REG;
>> +
>>  /*
>>   * i2c_pxa_reset: - reset the host controller
>>   *
>>   */
>>  static void i2c_reset(void)
>>  {
>> -     writel(readl(ICR) & ~ICR_IUE, ICR);     /* disable unit */
>> -     writel(readl(ICR) | ICR_UR, ICR);       /* reset the unit */
>> +     andl(~ICR_IUE, &base->icr);     /* disable unit */
>> +     orl(ICR_UR, &base->icr);        /* reset the unit */
>
> Apart from discussion going on for patch - [PATCH V5.1 1/6] io: add and* and 
> or* operation api to set and clear bit
>
> The original code looks more readable to me.
>
>>       udelay(100);
>> -     writel(readl(ICR) & ~ICR_IUE, ICR);     /* disable unit */
>> -#ifdef CONFIG_CPU_MONAHANS
>> -     /* | CKENB_1_PWM1 | CKENB_0_PWM0); */
>> -     writel(readl(CKENB) | (CKENB_4_I2C), CKENB);
>> -#else /* CONFIG_CPU_MONAHANS */
>> -     /* set the global I2C clock on */
>> -     writel(readl(CKEN) | CKEN14_I2C, CKEN);
>> -#endif
>> -     writel(I2C_PXA_SLAVE_ADDR, ISAR);       /* set our slave address */
>> -     writel(I2C_ICR_INIT, ICR);              /* set control reg values */
>> -     writel(I2C_ISR_INIT, ISR);              /* set clear interrupt bits */
>> -     writel(readl(ICR) | ICR_IUE, ICR);      /* enable unit */
>> +     andl(~ICR_I

[U-Boot] MIPS: tb0229 build error caused by reference

2011-03-30 Thread Shinya Kuribayashi
Hi Komiya-san,

Since the commit below, tb0229 target has been broken for months because
 and  can not be easily included as used to by MIPS
ports any more.

> commit b36df561154bdd0a41bb77e09c5575ca2cf48013
> Author: Stefan Roese 
> Date:   Thu Sep 9 19:18:00 2010 +0200
> 
> ppc4xx: Move ppc4xx headers to powerpc include directory
> 
> This patch moves some ppc4xx related headers from the common include
> directory (include/) to the powerpc specific one
> (arch/powerpc/include/asm/). This way to common include directory is not
> so cluttered with files.
> 
> Signed-off-by: Stefan Roese 

$ ./MAKEALL tb0229
[...]
flash.c:27: fatal error: asm/ppc4xx.h: No such file or directory
make[1]: *** [flash.o] Error 1

I can not come up with a reasonble workaround at this moment.  Could you
take a look please?  And let us know what we do next, thanks.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] using ${var} with env import

2011-03-30 Thread Holger Brunck
Hello,

Wolfgang Denk wrote:
> Dear Holger Brunck,
> 
> In message <4d92e7b4.9010...@keymile.com> you wrote:
>> I am using env import -t to import environment variables from a textfile.
>>
>> My simple textfile is:
>> fdt_file=${hostname}/${hostname}.dtb
>>
>> I import the file with:
>> => tftp 20 scripts/my_environ.txt
>> => env import -t 20 ${filesize}
>>
>> Now when I print the variable I get:
>> => print fdt_file
>> fdt_file=${hostname}/${hostname}.dtb
>>
>> hostname is defined as:
>> => print hostname
>> hostname=mgcoge
> 
> This is perfectly normal. "env export" and "env import" are inverse
> operations - they export and import the environment data directly,
> without any conversions (except for the formatting as text lines
> versus NUL-terminated strings).  Neither of these functions performs
> any variable substitutions - these are done in the command
> interpreter, i. e. when you run a command in the shell.
> 
>> Is the usage of ${var} in the textfiles not possible? Or is there a way to 
>> solve
>> this problem?
> 
> I understand that with "usage of ${var}" you mean variable subsitution
> - this is indeed not supposed to happen during an "env import".
> 
> I don't consider this a problem, though.  If you want such
> substituion, use the defined strings in shell commands.
> 
> Actually I consider it more clever to _keep_ the ${hostname} stuff in
> your variable definitions, as then it is sufficient to change the
> "hostname" variable to take affect everywhere; if you subsitute the
> value hard in other variables, you would have to fix all of these (and
> provide code to do so).
> 
> So actually I think this is not a problem, it just points out some
> inefficient usage of the environment in your setup.
> 

yes you are right and the problem I had was at a different place.

Sorry for the noise.

Best regards
Holger Brunck


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


[U-Boot] [PATCH] BeagleBoard: add xM rev C to ID table

2011-03-30 Thread Jason Kridner
A simple addition to the revision IDs.

This patch depends upon http://patchwork.ozlabs.org/patch/85303/.

Signed-off-by: Jason Kridner 
---
 board/ti/beagle/beagle.c |   12 
 board/ti/beagle/beagle.h |1 +
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
index 905b151..0596443 100644
--- a/board/ti/beagle/beagle.c
+++ b/board/ti/beagle/beagle.c
@@ -168,6 +168,7 @@ void display_init(void)
break;
case REVISION_XM_A:
case REVISION_XM_B:
+   case REVISION_XM_C:
default:
omap3_dss_panel_config(&dvid_cfg_xm);
break;
@@ -227,6 +228,16 @@ int misc_init_r(void)
TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
TWL4030_PM_RECEIVER_DEV_GRP_P1);
break;
+   case REVISION_XM_C:
+   printf("Beagle xM Rev C\n");
+   setenv("beaglerev", "xMC");
+   MUX_BEAGLE_XM();
+   /* Set VAUX2 to 1.8V for EHCI PHY */
+   twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VAUX2_DEDICATED,
+   TWL4030_PM_RECEIVER_VAUX2_VSEL_18,
+   TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
+   TWL4030_PM_RECEIVER_DEV_GRP_P1);
+   break;
default:
printf("Beagle unknown 0x%02x\n", get_board_revision());
MUX_BEAGLE_XM();
@@ -372,6 +383,7 @@ int do_userbutton (cmd_tbl_t *cmdtp, int flag, int argc, 
char *argv[])
break;
case REVISION_XM_A:
case REVISION_XM_B:
+   case REVISION_XM_C:
default:
gpio = 4;
break;
diff --git a/board/ti/beagle/beagle.h b/board/ti/beagle/beagle.h
index bdf2a6f..f31ce31 100644
--- a/board/ti/beagle/beagle.h
+++ b/board/ti/beagle/beagle.h
@@ -41,6 +41,7 @@ const omap3_sysinfo sysinfo = {
 #define REVISION_C40x5
 #define REVISION_XM_A  0x0
 #define REVISION_XM_B  0x1
+#define REVISION_XM_C  0x2
 
 /*
  * IEN  - Input Enable
-- 
1.5.6.4

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


Re: [U-Boot] [PATCH v3 0/5] MIPS: Refactoring and cleanup of CPU and SoC code

2011-03-30 Thread Shinya Kuribayashi
On 03/29/2011 01:33 AM, daniel.schwierz...@googlemail.com wrote:
> This patch series refactors the Mips CPU directory and put all current
> code into an own mips32 subdirectory. Furthermore the SoC specific code
> of IncaIP and Au1x00 is moved to separate SoC subdirectories.
> The support for Purple is dropped because not actively maintained
> anymore.
> 
> Changes since RFC/v1:
> - drop Purple supported as agreed with Wolfgang Denk
> - use MIPS rather than Mips in patch subject
> - fixed Cc lines in all patches
> 
> Changes since v2:
> - reverted patch for endianess flag setup
> - always use -march=mips32r2 as default CPU optimization for
>   all MIPS32 CPU cores
> 
> Daniel Schwierzeck (5):
>   MIPS: Purple: Remove Purple support
>   MIPS: Move content of arch/mips/cpu to arch/mips/cpu/mips32
>   MIPS: Optimize the setup of CPU optimization flags
>   MIPS: IncaIP: Move all IncaIP specific code to separate subdirectory
>   MIPS: Au1x00: Move all Au1x00 specific code to separate subdirectory

I've reviewed and build tested, all patches look good.

Applied and queued to u-boot-mips tree along with several s/Mips/MIPS/
keyword cleanups in the git commitlogs.  Will make a pull-request once
v2011.03 gets released.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [U-Boot, 2/7] IDE: Don't assume there are always two devices per bus

2011-03-30 Thread Gray Remlin
Excuse the noob question.
Is it intentional to replace '(dev >> 1) with '(dev >> 0)' ?
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [U-Boot, 2/7] IDE: Don't assume there are always two devices per bus

2011-03-30 Thread Gray Remlin
On 03/30/2011 07:27 PM, Gray Remlin wrote:
> Excuse the noob question.
> Is it intentional to replace '(dev >> 1) with '(dev >> 0)' ?
Ignore me, I get it. Divide by two, or not, as required.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH V5 3/6] mv_i2c: use structure to replace the direclty define

2011-03-30 Thread Prafulla Wadaskar


> -Original Message-
> From: Lei Wen [mailto:adrian.w...@gmail.com]
> Sent: Wednesday, March 30, 2011 7:42 PM
> To: Prafulla Wadaskar
> Cc: Lei Wen; Heiko Schocher; Wolfgang Denk; u-boot@lists.denx.de; Marek
> Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu Tang
> Subject: Re: [PATCH V5 3/6] mv_i2c: use structure to replace the
> direclty define
> 
> Hi Prafulla,
> 
> On Tue, Mar 29, 2011 at 9:27 PM, Prafulla Wadaskar
>  wrote:
> >
> >
> >> -Original Message-
> >> From: Lei Wen [mailto:lei...@marvell.com]
> >> Sent: Monday, March 28, 2011 12:24 PM
> >> To: Heiko Schocher; Prafulla Wadaskar; Wolfgang Denk; u-
> >> b...@lists.denx.de; Marek Vasut; Ashish Karkare; Prabhanjan Sarnaik;
> Yu
> >> Tang; adrian.w...@gmail.com
> >> Subject: [PATCH V5 3/6] mv_i2c: use structure to replace the direclty
> >> define
> >>
> >> Add i2c_clk_enable in the cpu specific code, since previous platform
> it,
> >> while new platform don't need. In the pantheon and armada100
> platform,
> >> this function is defined as NULL one.
> >>
> >> Signed-off-by: Lei Wen 
> >> ---
> >> Changelog:
> >> V2:
> >> NO CHANGE
> >>
> >> V3:
> >> clean code sytle issue
> >>
> >> V4:
> >> V5:
> >> NO CHANGE
> >>
> >>  arch/arm/cpu/pxa/cpu.c                   |   11 +++
> >>  arch/arm/include/asm/arch-pxa/pxa-regs.h |   56 -
> >>  board/innokom/innokom.c                  |    9 +--
> >>  drivers/i2c/mv_i2c.c                     |  131 ++--
> ---
> >> ---
> >>  drivers/i2c/mv_i2c.h                     |   83 +++
> >>  include/configs/innokom.h                |    1 +
> >>  include/configs/xm250.h                  |    1 +
> >>  7 files changed, 159 insertions(+), 133 deletions(-)
> >>  create mode 100644 drivers/i2c/mv_i2c.h
> >>
> > ...snip...
> >> diff --git a/drivers/i2c/mv_i2c.c b/drivers/i2c/mv_i2c.c
> >> index 09756a4..734148b 100644
> >> --- a/drivers/i2c/mv_i2c.c
> >> +++ b/drivers/i2c/mv_i2c.c
> >> @@ -8,6 +8,9 @@
> >>   * (C) Copyright 2003 Pengutronix e.K.
> >>   * Robert Schwebel 
> >>   *
> >> + * (C) Copyright 2011 Marvell Inc.
> >> + * Lei Wen 
> >> + *
> >>   * See file CREDITS for list of people who contributed to this
> >>   * project.
> >>   *
> >> @@ -34,24 +37,8 @@
> >>  #include 
> >>
> >>  #ifdef CONFIG_HARD_I2C
> >> -
> >> -/*
> >> - *   - CONFIG_SYS_I2C_SPEED
> >> - *   - I2C_PXA_SLAVE_ADDR
> >> - */
> >> -
> >> -#include 
> >> -#include 
> >>  #include 
> >> -
> >> -#if (CONFIG_SYS_I2C_SPEED == 40)
> >> -#define I2C_ICR_INIT (ICR_FM | ICR_BEIE | ICR_IRFIE | ICR_ITEIE |
> >> ICR_GCD \
> >> -                     | ICR_SCLE)
> >> -#else
> >> -#define I2C_ICR_INIT (ICR_BEIE | ICR_IRFIE | ICR_ITEIE | ICR_GCD |
> >> ICR_SCLE)
> >> -#endif
> >> -
> >> -#define I2C_ISR_INIT         0x7FF
> >> +#include "mv_i2c.h"
> >>
> >>  #ifdef DEBUG_I2C
> >>  #define PRINTD(x) printf x
> >> @@ -59,20 +46,6 @@
> >>  #define PRINTD(x)
> >>  #endif
> >>
> >> -/* Shall the current transfer have a start/stop condition? */
> >> -#define I2C_COND_NORMAL              0
> >> -#define I2C_COND_START               1
> >> -#define I2C_COND_STOP                2
> >> -
> >> -/* Shall the current transfer be ack/nacked or being waited for it?
> */
> >> -#define I2C_ACKNAK_WAITACK   1
> >> -#define I2C_ACKNAK_SENDACK   2
> >> -#define I2C_ACKNAK_SENDNAK   4
> >> -
> >> -/* Specify who shall transfer the data (master or slave) */
> >> -#define I2C_READ             0
> >> -#define I2C_WRITE            1
> >> -
> >>  /* All transfers are described by this data structure */
> >>  struct i2c_msg {
> >>       u8 condition;
> >> @@ -81,27 +54,37 @@ struct i2c_msg {
> >>       u8 data;
> >>  };
> >>
> >> +struct pxa_i2c {
> >> +     u32 ibmr;
> >> +     u32 pad0;
> >> +     u32 idbr;
> >> +     u32 pad1;
> >> +     u32 icr;
> >> +     u32 pad2;
> >> +     u32 isr;
> >> +     u32 pad3;
> >> +     u32 isar;
> >> +};
> >
> > (Optional to implement)
> > It is better if you can push register structure definition to the SoC
> specific header file, so that if there are new SoC that has different
> register structures that can be addressed cleanly.
> 
> For current there is no different register arrage for this structure,
> so I think it is
> ok to just keep it current state. For the adding register doc
> description, I have no
> objection.

That was just a suggestion, it up to you.

> >
> >> +
> >> +static struct pxa_i2c *base = (struct pxa_i2c *)CONFIG_PXA_I2C_REG;
> >> +
> >>  /*
> >>   * i2c_pxa_reset: - reset the host controller
> >>   *
> >>   */
> >>  static void i2c_reset(void)
> >>  {
> >> -     writel(readl(ICR) & ~ICR_IUE, ICR);     /* disable unit */
> >> -     writel(readl(ICR) | ICR_UR, ICR);       /* reset the unit */
> >> +     andl(~ICR_IUE, &base->icr);     /* disable unit */
> >> +     orl(ICR_UR, &base->icr);        /* reset the unit */
> >
> > Apart from discussion going on for patch - [PATCH V5.1 1/6] io: add
> and* and or* operation api to set and clear bit
> >
> > The original code loo

Re: [U-Boot] [PATCH V5 4/6] I2C: add i2c support for Pantheon platform

2011-03-30 Thread Prafulla Wadaskar


> -Original Message-
> From: Lei Wen [mailto:adrian.w...@gmail.com]
> Sent: Wednesday, March 30, 2011 7:36 PM
> To: Prafulla Wadaskar
> Cc: Lei Wen; Heiko Schocher; Wolfgang Denk; u-boot@lists.denx.de; Marek
> Vasut; Ashish Karkare; Prabhanjan Sarnaik; Yu Tang
> Subject: Re: [PATCH V5 4/6] I2C: add i2c support for Pantheon platform
> 
> Hi Prafulla,
> 
> On Tue, Mar 29, 2011 at 9:07 PM, Prafulla Wadaskar
>  wrote:
> >
> >
> >> -Original Message-
> >> From: Lei Wen [mailto:lei...@marvell.com]
> >> Sent: Monday, March 28, 2011 12:24 PM
> >> To: Heiko Schocher; Prafulla Wadaskar; Wolfgang Denk; u-
> >> b...@lists.denx.de; Marek Vasut; Ashish Karkare; Prabhanjan Sarnaik;
> Yu
> >> Tang; adrian.w...@gmail.com
> >> Subject: [PATCH V5 4/6] I2C: add i2c support for Pantheon platform
> >>
> >> Add i2c support to dkb board with pantheon soc.
> >>
> >> Signed-off-by: Lei Wen 
> >> ---
> >> Changelog:
> >> V2:
> >> NO CHANGE
> >>
> >> V3:
> >> clean code sytle issue
> >> Add i2c clock enable code include in I2C configure define block
> >>
> >> V4:
> >> make i2c definition included in the ifdef
> >>
> >> V5:
> >> NO CHANGE
> >>
> >>  arch/arm/cpu/arm926ejs/pantheon/cpu.c    |   12 
> >>  arch/arm/include/asm/arch-pantheon/cpu.h |    4 +++-
> >>  arch/arm/include/asm/arch-pantheon/mfp.h |    6 --
> >>  board/Marvell/dkb/dkb.c                  |    4 
> >>  include/configs/dkb.h                    |   13 +
> >>  5 files changed, 36 insertions(+), 3 deletions(-)
> >>
> > ...snip...
> >
> >> diff --git a/include/configs/dkb.h b/include/configs/dkb.h
> >> index 638af5e..599c8b8 100644
> >> --- a/include/configs/dkb.h
> >> +++ b/include/configs/dkb.h
> >> @@ -56,6 +56,19 @@
> >>  #include "mv-common.h"
> >>
> >>  #undef CONFIG_ARCH_MISC_INIT
> >> +
> >> +/*
> >> + * I2C definition
> >> + */
> >> +#define CONFIG_CMD_I2C
> >
> > This command definition should be moved up (below #include
> 
> 
> I'm ok to put this define to the config_cmd_default.h, but this mean
> many other platform need
> which didn't not need the i2c but include the ,
> need to undef the i2c now.
> Does that worth the change?

I don't mean here to put it in to the config_cmd_default.h
I means put it below #include  line where other commands 
are defined/undefed.

Regards..
Prafulla . .

> 
> Best regards,
> Lei
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] Kirkwood: SATA supports only one device per bus

2011-03-30 Thread Prafulla Wadaskar


> -Original Message-
> From: Gray Remlin [mailto:gryr...@gmail.com]
> Sent: Wednesday, March 30, 2011 7:21 PM
> To: Prafulla Wadaskar
> Cc: u-boot@lists.denx.de
> Subject: Re: [PATCH] Kirkwood: SATA supports only one device per bus
> 
> On 03/30/2011 12:21 PM, Prafulla Wadaskar wrote:
> >
> >> -Original Message-
> >> From: Prafulla Wadaskar
> >> Sent: Wednesday, March 30, 2011 4:48 PM
> >> To: 'Gray Remlin'
> >> Cc: u-boot@lists.denx.de; Prabhanjan Sarnaik; Ashish Karkare
> >> Subject: RE: [PATCH] Kirkwood: SATA supports only one device per bus
> >>
> >>
> >>
> >>> -Original Message-
> >>> From: Gray Remlin [mailto:gryr...@gmail.com]
> >>> Sent: Wednesday, March 30, 2011 1:12 AM
> >>> To: Prafulla Wadaskar
> >>> Cc: u-boot@lists.denx.de; Gray Remlin
> >>> Subject: [PATCH] Kirkwood: SATA supports only one device per bus
> >>>
> >>> Change CONFIG_SYS_IDE_MAXDEVICE from 2 to 1
> >> Kirkwood supports max 2
> >> Why do you want this change?
> > Hi Gray
> > I take back my words. I will study more and let you know my comments
> on this.
> > Sorry for noise.
> >
> > Regards..
> > Prafulla . .
> >
> >> Regards..
> >> Prafulla ..
> >>
> >>> Signed-off-by: Gray Remlin
> >>> ---
> >>>   arch/arm/include/asm/arch-kirkwood/config.h |3 ++-
> >>>   1 files changed, 2 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/arch/arm/include/asm/arch-kirkwood/config.h
> >>> b/arch/arm/include/asm/arch-kirkwood/config.h
> >>> index 71ba464..f582ba4 100644
> >>> --- a/arch/arm/include/asm/arch-kirkwood/config.h
> >>> +++ b/arch/arm/include/asm/arch-kirkwood/config.h
> >>> @@ -128,7 +128,8 @@
> >>>   #define CONFIG_LBA48
> >>>   /* CONFIG_CMD_IDE requires some #defines for ATA registers */
> >>>   #define CONFIG_SYS_IDE_MAXBUS   2
> >>> -#define CONFIG_SYS_IDE_MAXDEVICE 2
> >>> +/* Only one device per bus on SATA */
> >>> +#define CONFIG_SYS_IDE_MAXDEVICE 1
> >>>   /* ATA registers base is at SATA controller base */
> >>>   #define CONFIG_SYS_ATA_BASE_ADDRMV_SATA_BASE
> >>>   #endif /* CONFIG_CMD_IDE */
> >>> --
> >>> 1.7.4
> Hi Prafulla,
> 
> This one was a bit of a judgement call.
> You are absolutely correct in what you say that Kirkwood supports two,
> but
> to the best of my knowledge, no one has yet implemented a board using
> both busses (I am not that familiar with OpenRD, I may have to eat my
> words).

OpenRD-Base board has two, one e-sata for external hdd and one sata for onboard 
hdd interface, I am using them :-)

Regards..
Prafulla . .

> 
> For the sheevaplug, guruplug & dreamplug, it is definitely the case that
> only
> one device can be attached.
> 
> 
> 
> 
> 
> 

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


Re: [U-Boot] [U-Boot, 2/7] IDE: Don't assume there are always two devices per bus

2011-03-30 Thread Rogan Dawes
On 2011/03/30 8:49 PM, Gray Remlin wrote:
> On 03/30/2011 07:27 PM, Gray Remlin wrote:
>> Excuse the noob question.
>> Is it intentional to replace '(dev >> 1) with '(dev >> 0)' ?
> Ignore me, I get it. Divide by two, or not, as required.

Hi Gray,

Please also see the thread that starts at:

http://www.mail-archive.com/u-boot@lists.denx.de/msg36411.html

Albert was investigating changes to the IDE code to support multiple
busses for his LaCie Net5big (with 8 ports). Not sure if he ever got it
merged.

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


Re: [U-Boot] [U-Boot, 2/7] IDE: Don't assume there are always two devices per bus

2011-03-30 Thread Albert ARIBAUD
Le 30/03/2011 21:45, Rogan Dawes a écrit :
> On 2011/03/30 8:49 PM, Gray Remlin wrote:
>> On 03/30/2011 07:27 PM, Gray Remlin wrote:
>>> Excuse the noob question.
>>> Is it intentional to replace '(dev>>  1) with '(dev>>  0)' ?
>> Ignore me, I get it. Divide by two, or not, as required.
>
> Hi Gray,
>
> Please also see the thread that starts at:
>
> http://www.mail-archive.com/u-boot@lists.denx.de/msg36411.html
>
> Albert was investigating changes to the IDE code to support multiple
> busses for his LaCie Net5big (with 8 ports). Not sure if he ever got it
> merged.

Not yet but I will resume this once 2011.03 is out.

> Rogan

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


Re: [U-Boot] [PATCH] Kirkwood: SATA supports only one device per bus

2011-03-30 Thread Albert ARIBAUD
Le 30/03/2011 20:59, Prafulla Wadaskar a écrit :

>> to the best of my knowledge, no one has yet implemented a board using
>> both busses (I am not that familiar with OpenRD, I may have to eat my
>> words).
>
> OpenRD-Base board has two, one e-sata for external hdd and one sata for 
> onboard hdd interface, I am using them :-)

In which case it makes sense to keep CONFIG_SYS_IDE_MAXDEVICE set to 2 
in the common code and redefine it to 1 in board configs that only 
implement one, does it not?

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


Re: [U-Boot] [PATCH 5/7] Add mdio command for new PHY infrastructure

2011-03-30 Thread Andy Fleming

On Mar 30, 2011, at 6:55 AM, Detlev Zundel wrote:

> Hi Andy,
> 
> 
>> +static int extract_range(char *input, int *plo, int *phi)
>> +{
>> +char * end;
>> +*plo = simple_strtol(input, &end, 0);
>> +if (end == input)
>> +return -1;
>> +
>> +if (*end == '-') {
> 
> What about the case of input="12-"? Shouldn't there be an "&& *(end+1)"?


Ok.  It should be noted that cmd_mii.c has the same issue.

I'm about to send out the v2.

Andy

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


Re: [U-Boot] [PATCH 5/7] Add mdio command for new PHY infrastructure

2011-03-30 Thread Mike Frysinger
On Tue, Mar 29, 2011 at 3:30 PM, Andy Fleming wrote:
> +       "    \n"
> +       "  :\n"

why bother ?  pick one and go with it

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


Re: [U-Boot] [PATCH v3 01/23] powerpc, mpc83xx: add missing functions to mpc83xx.h

2011-03-30 Thread Kim Phillips
On Mon, 21 Mar 2011 08:01:57 +0100
Heiko Schocher  wrote:

> Signed-off-by: Heiko Schocher 
> cc: Kim Phillips 
> cc: Holger Brunck 
> cc: Wolfgang Denk 
> cc: Detlev Zundel 
> cc: Valentin Longchamp 
> ---

Hi Heiko, sorry for the late review, but I must admit it doesn't help
the reviewer at all when later patches in a patchseries modify things
added by earlier patches in the same patchseries!

>  include/mpc83xx.h |6 ++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/include/mpc83xx.h b/include/mpc83xx.h
> index ea137c7..e1b0929 100644
> --- a/include/mpc83xx.h
> +++ b/include/mpc83xx.h
> @@ -1274,6 +1274,12 @@ struct pci_region;
>  void mpc83xx_pci_init(int num_buses, struct pci_region **reg);
>  void mpc83xx_pcislave_unlock(int bus);
>  void mpc83xx_pcie_init(int num_buses, struct pci_region **reg);
> +
> +void disable_addr_trans(void);
> +void enable_addr_trans(void);
> +#if defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)
> +void ddr_enable_ecc(unsigned int dram_size);
> +#endif

I don't believe these prototypes belong in mpc83xx.h - they're really
not 83xx-specific - e.g., 74xx and 85xx have identical names for
functions that have the same...function.

Looking around I think the best place for them would be the 'start.S'
section of include/common.h.  Feel free to protect with 83xx ifdefs;
others can add their platforms as necessary.

Thanks,

Kim


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


Re: [U-Boot] [PATCH v3 06/23] mpc832x: add support for the mpc8321 based suvd3 board

2011-03-30 Thread Kim Phillips
On Mon, 21 Mar 2011 08:02:02 +0100
Heiko Schocher  wrote:

> +++ b/arch/powerpc/cpu/mpc83xx/fdt.c
> @@ -32,7 +32,8 @@ extern void ft_qe_setup(void *blob);
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> -#if defined(CONFIG_BOOTCOUNT_LIMIT) && defined(CONFIG_MPC8360)
> +#if defined(CONFIG_BOOTCOUNT_LIMIT) && \
> + (defined(CONFIG_MPC832x) || defined(CONFIG_MPC8360))

please replace 832x and 8360 with just CONFIG_QE

> +++ b/arch/powerpc/lib/bootcount.c
> @@ -51,7 +51,7 @@
>  #define CONFIG_SYS_BOOTCOUNT_ADDR(CONFIG_SYS_IMMR + CPM_BOOTCOUNT_ADDR)
>  #endif /* defined(CONFIG_MPC8260) */
>  
> -#if defined(CONFIG_MPC8360)
> +#if defined(CONFIG_MPC832x) || defined(CONFIG_MPC8360)

same here

> diff --git a/include/configs/kmeter1.h b/include/configs/kmeter1.h
> -#define CONFIG_83XX_CLKIN6600
> -#define CONFIG_SYS_CLK_FREQ  6600
> -#define CONFIG_83XX_PCICLK   6600
> +#define CONFIG_SYS_SICRH 0x0006

I realize this was a pre-existing condition, but please make this
(SICRH_UC1EOBI | SICRH_UC2E1OBI) instead of 6.

> +#define CONFIG_SYS_SICRL 0x

nit-pick, but fyi, one can save a raw write here by not defining it at
all.

>  /* PAXE:  icache cacheable, but dcache-inhibit and guarded */
>  #define CONFIG_SYS_IBAT5L(CONFIG_SYS_PAXE_BASE | BATL_PP_10 | \
> - BATL_MEMCOHERENCE)
> + BATL_MEMCOHERENCE)

B in BATL_ should be aligned to fall directly under the C in
CONFIG_SYS_PAXE_BASE, like this:

#define CONFIG_SYS_IBAT5L   (CONFIG_SYS_PAXE_BASE | BATL_PP_10 | \
 BATL_MEMCOHERENCE)

>  #define CONFIG_SYS_IBAT5U(CONFIG_SYS_PAXE_BASE | BATU_BL_256M | \
> - BATU_VS | BATU_VP)
> + BATU_VS | BATU_VP)

same here, just as the next one is already:

>  #define CONFIG_SYS_DBAT5L(CONFIG_SYS_PAXE_BASE | BATL_PP_10 | \
>BATL_CACHEINHIBIT | BATL_GUARDEDSTORAGE)


> diff --git a/include/configs/suvd3.h b/include/configs/suvd3.h

> +#define CONFIG_SYS_SICRH 0x0006

afaict, SICRH doesn't documentally exist on 832x, so omit this line.
This comment also applies to the following patches in this series:

[PATCH v3 07/23] mpc832x: add support for mpc8321 based tuxa1 board
[PATCH v3 08/23] mpc832x: add support for mpc8321 based tuda1 board
[PATCH v3 12/23] keymile, 8321 boards: move common definitions to 
km8321-common.h

even though the 12/23 patch seems to be a refactoring of the definition
added by the others

The rest of the 83xx-touching patches in the series look good to me.

Kim

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


Re: [U-Boot] [PATCH 6/7] phylib: Add a bunch of PHY drivers from tsec

2011-03-30 Thread Andy Fleming

On Mar 30, 2011, at 7:26 AM, Detlev Zundel wrote:

> Hi Andy,
> 
>> The tsec driver had a bunch of PHY drivers already written. This
>> converts them all into PHY Lib drivers, and serves as the first
>> set of PHY drivers for PHY Lib.
>> 
>> Signed-off-by: Andy Fleming 
> 
> [...]
> 
>> diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
>> new file mode 100644
>> index 000..68bb5cd
>> --- /dev/null
>> +++ b/drivers/net/phy/atheros.c
>> @@ -0,0 +1,37 @@
>> +/*
>> + * Atheros PHY drivers
>> + *
>> + * This software may be used and distributed according to the
>> + * terms of the GNU Public License, Version 2, incorporated
>> + * herein by reference.
> 
> Again, new files need GPLv2+.  I know that this is split out from a
> GPLv2 file, but we have our rules in the meantime.

Fixed.
> 
> 
> 
>> +static int bcm54xx_parse_status(struct phy_device *phydev)
>> +{
>> +unsigned int mii_reg;
>> +
>> +mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_BCM54xx_AUXSTATUS);
>> +
>> +switch ((mii_reg & MIIM_BCM54xx_AUXSTATUS_LINKMODE_MASK) >>
>> +MIIM_BCM54xx_AUXSTATUS_LINKMODE_SHIFT) {
>> +case 1:
>> +phydev->duplex = DUPLEX_HALF;
>> +phydev->speed = SPEED_10;
>> +break;
>> +case 2:
>> +phydev->duplex = DUPLEX_FULL;
>> +phydev->speed = SPEED_10;
>> +break;
>> +case 3:
>> +phydev->duplex = DUPLEX_HALF;
>> +phydev->speed = SPEED_100;
>> +break;
>> +case 5:
>> +phydev->duplex = DUPLEX_FULL;
>> +phydev->speed = SPEED_100;
>> +break;
>> +case 6:
>> +phydev->duplex = DUPLEX_HALF;
>> +phydev->speed = SPEED_1000;
>> +break;
>> +case 7:
>> +phydev->duplex = DUPLEX_FULL;
>> +phydev->speed = SPEED_1000;
>> +break;
>> +default:
>> +printf("Auto-neg error, defaulting to 10BT/HD\n");
>> +phydev->duplex = DUPLEX_HALF;
>> +phydev->speed = SPEED_10;
>> +break;
> 
> Very strange indentation, please fix.

done
> 
> 
> 
>> +/* Parse the 88E1011's status register for speed and duplex
>> + * information
>> + */
>> +static uint m88e1011s_parse_status(struct phy_device *phydev)
>> +{
>> +unsigned int speed;
>> +unsigned int mii_reg;
>> +
>> +mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MIIM_88E1011_PHY_STATUS);
>> +
>> +if ((mii_reg & MIIM_88E1011_PHYSTAT_LINK) &&
>> +!(mii_reg & MIIM_88E1011_PHYSTAT_SPDDONE)) {
>> +int i = 0;
>> +
>> +puts("Waiting for PHY realtime link");
>> +while (!(mii_reg & MIIM_88E1011_PHYSTAT_SPDDONE)) {
>> +/* Timeout reached ? */
>> +if (i > PHY_AUTONEGOTIATE_TIMEOUT) {
>> +puts(" TIMEOUT !\n");
>> +phydev->link = 0;
>> +break;
>> +}
>> +
>> +if ((i++ % 1000) == 0) {
>> +putc('.');
>> +}
>> +udelay(1000);
>> +mii_reg = phy_read(phydev, MDIO_DEVAD_NONE,
>> +MIIM_88E1011_PHY_STATUS);
>> +}
>> +puts(" done\n");
>> +udelay(50); /* another 500 ms (results in faster booting) */
> 
> Ah, again the "faster booting when waiting half a second" - probably
> that's the origin ;)  Still a comment will be nice.


Hmm... I'm going to think about this one.  We had some discussions about it on 
the list before, and I'm once again pondering whether we can drop it with 
proper changes.  As usual, the 802.3 spec likes to leave some things up to PHY 
vendors.  Like the meaning of "link up".

> 
>> +m88e1011s_parse_status(phydev);
>> +
>> +return 0;
>> +}
>> +
>> +/* Marvell 88E1149S */
>> +static int m88e1149_config(struct phy_device *phydev)
>> +{
>> +/* Reset and configure the PHY */
>> +phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, BMCR_RESET);
>> +
>> +phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x1f);
>> +phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x200c);
>> +phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x5);
>> +phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x0);
>> +phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x100);
> 
> Lots of magic numbers - defines would be nice.


Sadly, I can fill in some of these, but Marvell did not provide documentation 
to us on these fixes.  Just a sequence of register writes.

I'll ponder the half-second thing some more.  I will note that this is the way 
it currently is for tsec, so it's not really a degradation in functionality, 
but I agree that other ports may not want to use Freescale's ugly hacks when 
things work just fine for them without them.

Andy
___
U-B

Re: [U-Boot] [PATCH] [MPC837x v2] Make it work again with USB.

2011-03-30 Thread Kim Phillips
On Mon, 28 Feb 2011 17:18:38 +0100
Andre Schwarz  wrote:

> sorry to bother you again, but I again stumbled over the discussed USB 
> init issue :
> >>> nack, 837x has a usb controller at IMMR+0x23000.
> >> yes - but offset 0x00-0xff is explicitly reserved regarding to the manual.
> >> Don't know whether it is a "should not" or "must not be touched".
> >>   
> >> All I can see is a CPU hang with arbiter event register reporting a 
> >> timeout on
> >> 0xe0023000.
> >>   
> >>
> >>Check to see whether there is an invalid USB clock setting in the SCCR? 
> >> All clocks are turned on except SEC and 2nd TSEC.
> >>   
> >> After all USB is running fine with this patch, i.e. there can hardly be a
> >> missing clock.
> >>
> >>
> >> Please re-think you NAK.
> > afaik, 834x and 837x don't have any special USB settings in common, so,
> > this patch, at least in it's current form, is not on.
> >
> > 0xe0023500 should be the address of the config register being accessed
> > here; please check the code isn't accessing 0xe0023000, as you mention
> > above.
> 
> ok - this was some kind of misunderstanding.
> ehci regs are based at immr + 0x23000 with the "config" pointing to 
> offset 0x500 inside ehci.
> This looks sane to me.

ok, as long as it's confirmed.

> > If that's correct, try something like the following so we can determine
> > what setting the USB controller didn't agree with:
> >
> > diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c 
> > b/arch/powerpc/cpu/mpc83xx/cpu_init.c
> > index 7a1cae7..cbc4157 100644
> > --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c
> > +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c
> > @@ -332,7 +332,7 @@ void cpu_init_f (volatile immap_t * im)
> > struct usb_ehci *ehci = (struct usb_ehci *)CONFIG_SYS_FSL_USB_ADDR;
> >
> > /* Configure interface. */
> > -   setbits_be32(&ehci->control, REFSEL_16MHZ | UTMI_PHY_EN);
> MPC837x has only 2 working bits inside the control register :
> 
> Bit29: USB_EN -> should be set to 1 before USB can be used.
> Bit31: ULPI_INT_EN -> enables an ULPI wake-up irq.
> 
> Both "REFSEL_16MHZ" and "UTMI_PHY_EN" are completely out of scope for 
> MPC837x.

that's why I'm suggesting we confirm that touching the REFSEL_16MHZ and
UTMI_PHY_EN bits aren't sending the 837x controller into oblivion - did
you test the patch?

> > +   setbits_be32(&ehci->control, 0);
> >
> > /* Wait for clock to stabilize */
> This loop never returns on MPC837x because "PHY_CLK_VALID" isn't valid.

right, we need to narrow down the reason for this.

> > do {temp = __raw_readl(&ehci->control);
> >  udelay(1000);
> >  } while (!(temp&  PHY_CLK_VALID));
> >
> I still wonder how there can be a single working MPC837x board with 
> CONFIG_USB_EHCI_FSL set.
> 
> Some pending patches on your side ?
> What kind of patch might get an ACK from your side ?

nothing that suggests 834x and 837x have any special USB settings in
common - because it's not true and therefore misleading.

Kim

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


Re: [U-Boot] [PATCH] ftsmc020: enhance for features and asm support.

2011-03-30 Thread Macpaul Lin
Hi Wolfgang,


2011/3/25 Wolfgang Denk :
> Dear macp...@andestech.com,
>
> There is no (known) problem with it, but you have to be careful about
> the context you are running in - the script as is is supposed to be
> run in "make" context, which means different $ expansion rules then
> when you try running directly under a shell.

I'm not sure if this way I use make-asm-offset is correct.

First I add the OFFSET marco of ftsmc020 into "lib/asm-offsets.c".
The file "lib/asm-offsets.c" becomes
int main(void)
{
/* Round up to make sure size gives nice stack alignment */
DEFINE(GENERATED_GBL_DATA_SIZE,
(sizeof(struct global_data) + 15) & ~15);

DEFINE(GENERATED_BD_INFO_SIZE,
(sizeof(struct bd_info) + 15) & ~15);

OFFSET(FTSMC020_BANK0_CR,  ftsmc020, bank[0].cr);
OFFSET(FTSMC020_BANK0_TPR, ftsmc020, bank[0].tpr);
OFFSET(FTSMC020_BANK1_CR,  ftsmc020, bank[1].cr);
OFFSET(FTSMC020_BANK1_TPR, ftsmc020, bank[1].tpr);
OFFSET(FTSMC020_BANK2_CR,  ftsmc020, bank[2].cr);
OFFSET(FTSMC020_BANK2_TPR, ftsmc020, bank[2].tpr);
OFFSET(FTSMC020_BANK3_CR,  ftsmc020, bank[3].cr);
OFFSET(FTSMC020_BANK3_TPR, ftsmc020, bank[3].tpr);
OFFSET(FTSMC020_PAD0, ftsmc020, pad[0]);
OFFSET(FTSMC020_PAD1, ftsmc020, pad[1]);
OFFSET(FTSMC020_PAD2, ftsmc020, pad[2]);
OFFSET(FTSMC020_PAD3, ftsmc020, pad[3]);
OFFSET(FTSMC020_PAD4, ftsmc020, pad[4]);
OFFSET(FTSMC020_PAD5, ftsmc020, pad[5]);
OFFSET(FTSMC020_PAD6, ftsmc020, pad[6]);
OFFSET(FTSMC020_PAD7, ftsmc020, pad[7]);
OFFSET(FTSMC020_SSR, ftsmc020, ssr);
}

Then I ran make process, the result of make-asm-offset goes into
"include/generated/generic-asm-offsets.h" as
#define GENERATED_GBL_DATA_SIZE (80) /* (sizeof(struct global_data) +
15) & ~15 */
#define GENERATED_BD_INFO_SIZE (64) /* (sizeof(struct bd_info) + 15) & ~15 */
#define FTSMC020_BANK0_CR (0) /* offsetof(struct ftsmc020, bank[0].cr) */
#define FTSMC020_BANK0_TPR (4) /* offsetof(struct ftsmc020, bank[0].tpr) */
#define FTSMC020_BANK1_CR (8) /* offsetof(struct ftsmc020, bank[1].cr) */
#define FTSMC020_BANK1_TPR (12) /* offsetof(struct ftsmc020, bank[1].tpr) */
#define FTSMC020_BANK2_CR (16) /* offsetof(struct ftsmc020, bank[2].cr) */
#define FTSMC020_BANK2_TPR (20) /* offsetof(struct ftsmc020, bank[2].tpr) */
#define FTSMC020_BANK3_CR (24) /* offsetof(struct ftsmc020, bank[3].cr) */
#define FTSMC020_BANK3_TPR (28) /* offsetof(struct ftsmc020, bank[3].tpr) */
#define FTSMC020_PAD0 (32) /* offsetof(struct ftsmc020, pad[0]) */
#define FTSMC020_PAD1 (36) /* offsetof(struct ftsmc020, pad[1]) */
#define FTSMC020_PAD2 (40) /* offsetof(struct ftsmc020, pad[2]) */
#define FTSMC020_PAD3 (44) /* offsetof(struct ftsmc020, pad[3]) */
#define FTSMC020_PAD4 (48) /* offsetof(struct ftsmc020, pad[4]) */
#define FTSMC020_PAD5 (52) /* offsetof(struct ftsmc020, pad[5]) */
#define FTSMC020_PAD6 (56) /* offsetof(struct ftsmc020, pad[6]) */
#define FTSMC020_PAD7 (60) /* offsetof(struct ftsmc020, pad[7]) */
#define FTSMC020_SSR (64) /* offsetof(struct ftsmc020, ssr) */

However, this looks weird. It doesn't look like the other automated
generated code.
And, how does the offset address in decimal could be generated in hex?
Could I move the generated code into ftsmc020.h?

>
>> Moreover, the structure of ftsmc020 was nested like
>> struct ftsmc020 {
>>         struct {
>>                 unsigned int    cr;     /* 0x00, 0x08, 0x10, 0x18 */
>>                 unsigned int    tpr;    /* 0x04, 0x0c, 0x14, 0x1c */
>>         } bank[4];
>>         unsigned int    pad[8]; /* 0x20 - 0x3c */
>>         unsigned int    ssr;    /* 0x40 */
>> };

Thanks.

-- 
Best regards,
Macpaul Lin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH RFC v2] Fix build problems caused by "_end" -> "__bss_end__" rename

2011-03-30 Thread Kumar Gala

On Mar 29, 2011, at 8:38 PM, Po-Yu Chuang wrote:

> Hi Wolfgang,
> 
> On Wed, Mar 30, 2011 at 3:51 AM, Wolfgang Denk  wrote:
>> Dear Po-Yu Chuang,
>> 
>> In message <1301402371-8697-1-git-send-email...@denx.de> I wrote:
>>> Commit 44c6e65 "rename _end to __bss_end__ broke building of a large
>>> number of systems (at least all PowerPC?):
>>> 
>>> libstubs.o: In function `app_startup':
>>> examples/standalone/stubs.c:197: undefined reference to `__bss_end__'
>>> 
>>> The rename should not be done for the files in the
>>> examples/standalone/ directory, as these are not using the code from
>>> start.S, but do their own BSS clearing, and either use their own
>>> linker scripts or the ones provided by the compilers.
>>> 
>>> Signed-off-by: Wolfgang Denk 
>>> ---
>>> V2:   Instead of messing with linker defines, revert the patch in
>>>   question for the files in examples/standalone/ as suggested by
>>>   Albert Aribaud. (Thanks!)
>> 
>> Only now I realize that you submitted the very same patch before.
>> Stupid me.
>> 
>> Does that mean that we have an agreement that this hould be the fix
>> then?  If yes, I would like to pull this in (Po-Yu Chuang's patch,
>> that is).
>> 
>> Do you agree?
> 
> Your commit message is better. Please just use your v2 as is. :-)

We putting this into the tree, would be nice to have builds working again ;)

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


Re: [U-Boot] [PATCH v3 01/23] powerpc, mpc83xx: add missing functions to mpc83xx.h

2011-03-30 Thread Heiko Schocher
Hello Kim,

Kim Phillips wrote:
> On Mon, 21 Mar 2011 08:01:57 +0100
> Heiko Schocher  wrote:
> 
>> Signed-off-by: Heiko Schocher 
>> cc: Kim Phillips 
>> cc: Holger Brunck 
>> cc: Wolfgang Denk 
>> cc: Detlev Zundel 
>> cc: Valentin Longchamp 
>> ---
> 
> Hi Heiko, sorry for the late review, but I must admit it doesn't help
> the reviewer at all when later patches in a patchseries modify things
> added by earlier patches in the same patchseries!

Sorry for that, I know, it is a big patchset ...

>>  include/mpc83xx.h |6 ++
>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/mpc83xx.h b/include/mpc83xx.h
>> index ea137c7..e1b0929 100644
>> --- a/include/mpc83xx.h
>> +++ b/include/mpc83xx.h
>> @@ -1274,6 +1274,12 @@ struct pci_region;
>>  void mpc83xx_pci_init(int num_buses, struct pci_region **reg);
>>  void mpc83xx_pcislave_unlock(int bus);
>>  void mpc83xx_pcie_init(int num_buses, struct pci_region **reg);
>> +
>> +void disable_addr_trans(void);
>> +void enable_addr_trans(void);
>> +#if defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)
>> +void ddr_enable_ecc(unsigned int dram_size);
>> +#endif
> 
> I don't believe these prototypes belong in mpc83xx.h - they're really
> not 83xx-specific - e.g., 74xx and 85xx have identical names for
> functions that have the same...function.
> 
> Looking around I think the best place for them would be the 'start.S'
> section of include/common.h.  Feel free to protect with 83xx ifdefs;
> others can add their platforms as necessary.

Hmm.. I thought of this too, but that will result in adding ifdefs.
(and special this file has a lot of ifdefs, so I wanted to prevent
 another ifdef mess...). But I can of course move it to
include/common.h if thats the preferred place ... ?

bye,
Heiko
-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v3 06/23] mpc832x: add support for the mpc8321 based suvd3 board

2011-03-30 Thread Heiko Schocher
Hello Kim,

Kim Phillips wrote:
> On Mon, 21 Mar 2011 08:02:02 +0100
> Heiko Schocher  wrote:
> 
>> +++ b/arch/powerpc/cpu/mpc83xx/fdt.c
>> @@ -32,7 +32,8 @@ extern void ft_qe_setup(void *blob);
>>  
>>  DECLARE_GLOBAL_DATA_PTR;
>>  
>> -#if defined(CONFIG_BOOTCOUNT_LIMIT) && defined(CONFIG_MPC8360)
>> +#if defined(CONFIG_BOOTCOUNT_LIMIT) && \
>> +(defined(CONFIG_MPC832x) || defined(CONFIG_MPC8360))
> 
> please replace 832x and 8360 with just CONFIG_QE

Ok.

>> +++ b/arch/powerpc/lib/bootcount.c
>> @@ -51,7 +51,7 @@
>>  #define CONFIG_SYS_BOOTCOUNT_ADDR   (CONFIG_SYS_IMMR + CPM_BOOTCOUNT_ADDR)
>>  #endif /* defined(CONFIG_MPC8260) */
>>  
>> -#if defined(CONFIG_MPC8360)
>> +#if defined(CONFIG_MPC832x) || defined(CONFIG_MPC8360)
> 
> same here

Ok.

>> diff --git a/include/configs/kmeter1.h b/include/configs/kmeter1.h
>> -#define CONFIG_83XX_CLKIN   6600
>> -#define CONFIG_SYS_CLK_FREQ 6600
>> -#define CONFIG_83XX_PCICLK  6600
>> +#define CONFIG_SYS_SICRH0x0006
> 
> I realize this was a pre-existing condition, but please make this
> (SICRH_UC1EOBI | SICRH_UC2E1OBI) instead of 6.

Ok.

>> +#define CONFIG_SYS_SICRL0x
> 
> nit-pick, but fyi, one can save a raw write here by not defining it at
> all.

removed.

>>  /* PAXE:  icache cacheable, but dcache-inhibit and guarded */
>>  #define CONFIG_SYS_IBAT5L   (CONFIG_SYS_PAXE_BASE | BATL_PP_10 | \
>> -BATL_MEMCOHERENCE)
>> +BATL_MEMCOHERENCE)
> 
> B in BATL_ should be aligned to fall directly under the C in
> CONFIG_SYS_PAXE_BASE, like this:
> 
> #define CONFIG_SYS_IBAT5L   (CONFIG_SYS_PAXE_BASE | BATL_PP_10 | \
>BATL_MEMCOHERENCE)

Ok.

>>  #define CONFIG_SYS_IBAT5U   (CONFIG_SYS_PAXE_BASE | BATU_BL_256M | \
>> -BATU_VS | BATU_VP)
>> +BATU_VS | BATU_VP)
> 
> same here, just as the next one is already:

Ok.

> 
>>  #define CONFIG_SYS_DBAT5L   (CONFIG_SYS_PAXE_BASE | BATL_PP_10 | \
>>   BATL_CACHEINHIBIT | BATL_GUARDEDSTORAGE)
> 
> 
>> diff --git a/include/configs/suvd3.h b/include/configs/suvd3.h
> 
>> +#define CONFIG_SYS_SICRH0x0006
> 
> afaict, SICRH doesn't documentally exist on 832x, so omit this line.
> This comment also applies to the following patches in this series:

Ok.

> [PATCH v3 07/23] mpc832x: add support for mpc8321 based tuxa1 board
> [PATCH v3 08/23] mpc832x: add support for mpc8321 based tuda1 board
> [PATCH v3 12/23] keymile, 8321 boards: move common definitions to 
> km8321-common.h
> 
> even though the 12/23 patch seems to be a refactoring of the definition
> added by the others
> 
> The rest of the 83xx-touching patches in the series look good to me.

Thanks for reviewing.

bye
Heiko
-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] ftsmc020: enhance for features and asm support.

2011-03-30 Thread Wolfgang Denk
Dear Macpaul Lin,

In message  you 
wrote:
> 
> I'm not sure if this way I use make-asm-offset is correct.
> 
> First I add the OFFSET marco of ftsmc020 into "lib/asm-offsets.c".
> The file "lib/asm-offsets.c" becomes

We should probably split architecture and/or board specific additions
like these into separate files in the respectice architecture / board
directories.  Eventually we add make targets for these, then; for now
it's probably sufficient to add some #include to lib/asm-offsets.c

Otherwise lib/asm-offsets.c will quickly become an unreadable mess.

Otherwise this looks OK with me.

> Then I ran make process, the result of make-asm-offset goes into
> "include/generated/generic-asm-offsets.h" as
...
> #define FTSMC020_BANK2_TPR (20) /* offsetof(struct ftsmc020, bank[2].tpr) *> /
> #define FTSMC020_BANK3_CR (24) /* offsetof(struct ftsmc020, bank[3].cr) */
> #define FTSMC020_BANK3_TPR (28) /* offsetof(struct ftsmc020, bank[3].tpr) *> /

Keep in mind that I dislike this manual unrolling of the nested
structs. It may work in your code, but it is ugly and doesn't scale.
Also, it does not allow any kind of looping over the entries which
might be needed here and there.  I strongly recommend to get rid of
these nested declarations.

> #define FTSMC020_PAD0 (32) /* offsetof(struct ftsmc020, pad[0]) */
> #define FTSMC020_PAD1 (36) /* offsetof(struct ftsmc020, pad[1]) */
> #define FTSMC020_PAD2 (40) /* offsetof(struct ftsmc020, pad[2]) */
> #define FTSMC020_PAD3 (44) /* offsetof(struct ftsmc020, pad[3]) */
> #define FTSMC020_PAD4 (48) /* offsetof(struct ftsmc020, pad[4]) */
> #define FTSMC020_PAD5 (52) /* offsetof(struct ftsmc020, pad[5]) */
> #define FTSMC020_PAD6 (56) /* offsetof(struct ftsmc020, pad[6]) */
> #define FTSMC020_PAD7 (60) /* offsetof(struct ftsmc020, pad[7]) */
> #define FTSMC020_SSR (64) /* offsetof(struct ftsmc020, ssr) */
>
> However, this looks weird. It doesn't look like the other automated
> generated code.

What exactly looks weird?  And what "other automated generated code"
do you mean?

> And, how does the offset address in decimal could be generated in hex?

I don't know of a way to do that - it's the cpp + assembler which
generates this code, and I am not aware of any ways to ask them for
specific formatting or number bases.

> Could I move the generated code into ftsmc020.h?

No - what for?  This is automatically generated code, that gets used
somewhere. No human eye is supposed to have to read it.

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
Each honest calling, each walk of life, has its own  elite,  its  own
aristocracy based on excellence of performance. - James Bryant Conant
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [RFC PATCH] rename __bss_end__ back to _end for standalone programs

2011-03-30 Thread Wolfgang Denk
Dear Po-Yu Chuang,

In message <1301301782-1644-1-git-send-email-ratbert.chu...@gmail.com> you 
wrote:
> From: Po-Yu Chuang 
> 
> It seems __bss_end__ is not a true convention for all toolchains,
> at least not for PPC. Using  _end for standalone programs might be
> the simplest way to fix this problem.
> 
> One of the other choices may be writing a linker script to provide
> __bss_end__ for PPC.
> 
> Signed-off-by: Po-Yu Chuang 
> ---
> Hi all,
> 
> Not sure if this is the best solution, but I think this
> could fix Heiko's problem.
> 
>  examples/standalone/mips.lds  |2 +-
>  examples/standalone/sparc.lds |2 +-
>  examples/standalone/stubs.c   |4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)

Applied, with updated changelog as discussed.  Thanks.

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
Objects in mirror are closer than they appear.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot