Re: [U-Boot] Setting Ethernet Speed in NFS boot

[Wolfgang Denk]

Dear Sandeep C R,

In message  
you wrote:
>
> I have a powerpc system with u-boot as the boot loader. Is it possible for
> me to set the speed of the ethernet link while doing a nfs boot? ie. When
> the nfs boot is happening i want to  set the ethernet speed to say 10mbps
> or 100mbps according to my preference.

No, you don't want to do that.  You want to use autonegotiation
instead.

Please read the related FAQs (and for example the wikipedia article
about autonegotiation and why you run into a lot of problems when not
trying to force specific modes, especially when doing so on one side
of the link only).

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
 The software required `Windows 95 or better', so I installed Linux.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] blackfin: Fix using gd->baudrate before setting its value

[Marek Vasut]

Dear Axel Lin,

> Current code uses gd->baudrate before setting its value.
> Besides, I got below build warning which is introduced by
> commit ddb5c5be "blackfin: add baudrate to bdinfo".
> 
> board.c:235:3: warning: passing argument 1 of 'simple_strtoul' makes
> pointer from integer without a cast [enabled by default]
> include/vsprintf.h:27:7: note: expected 'const char *' but argument is of
> type 'unsigned int'
> 
> This patch moves the code using gd->baudrate to be after init_baudrate()
> call, this ensures we get the baudrate setting before using it.
> 
> Signed-off-by: Axel Lin 
> ---
> I forgot to CC u-boot mail list. here is a resend.
> 
> Hi,
> I don't have this hardware to test.
> I'd appreciate if someone can test it.
> 
> Thanks,
> Axel
>  arch/blackfin/lib/board.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/blackfin/lib/board.c b/arch/blackfin/lib/board.c
> index f1d5547..9e2e9de 100644
> --- a/arch/blackfin/lib/board.c
> +++ b/arch/blackfin/lib/board.c
> @@ -231,8 +231,6 @@ static int global_board_data_init(void)
>   bd->bi_sclk = get_sclk();
>   bd->bi_memstart = CONFIG_SYS_SDRAM_BASE;
>   bd->bi_memsize = CONFIG_SYS_MAX_RAM_SIZE;
> - bd->bi_baudrate = (gd->baudrate > 0)
> - ? simple_strtoul(gd->baudrate, NULL, 10) : CONFIG_BAUDRATE;


I'd rather say the fix here is to use (gd->baudrate > 0) ? gd->baudrate ? 
CONFIG_BAUDRATE ;

Otherwise you're changing the logic of the code and for that, you'd need Mikes' 
ack.

>   return 0;
>  }
> @@ -299,6 +297,7 @@ void board_init_f(ulong bootflag)
>   env_init();
>   serial_early_puts("Baudrate init\n");
>   init_baudrate();
> + gd->bd->bi_baudrate = gd->baudrate;
>   serial_early_puts("Serial init\n");
>   serial_init();
>   serial_initialize();

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

Re: [U-Boot] [PATCH] blackfin: Fix using gd->baudrate before setting its value

[Axel Lin]

2013/6/29 Marek Vasut :
> Dear Axel Lin,
>
>> Current code uses gd->baudrate before setting its value.
>> Besides, I got below build warning which is introduced by
>> commit ddb5c5be "blackfin: add baudrate to bdinfo".
>>
>> board.c:235:3: warning: passing argument 1 of 'simple_strtoul' makes
>> pointer from integer without a cast [enabled by default]
>> include/vsprintf.h:27:7: note: expected 'const char *' but argument is of
>> type 'unsigned int'
>>
>> This patch moves the code using gd->baudrate to be after init_baudrate()
>> call, this ensures we get the baudrate setting before using it.
>>
>> Signed-off-by: Axel Lin 
>> ---
>> I forgot to CC u-boot mail list. here is a resend.
>>
>> Hi,
>> I don't have this hardware to test.
>> I'd appreciate if someone can test it.
>>
>> Thanks,
>> Axel
>>  arch/blackfin/lib/board.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/blackfin/lib/board.c b/arch/blackfin/lib/board.c
>> index f1d5547..9e2e9de 100644
>> --- a/arch/blackfin/lib/board.c
>> +++ b/arch/blackfin/lib/board.c
>> @@ -231,8 +231,6 @@ static int global_board_data_init(void)
>>   bd->bi_sclk = get_sclk();
>>   bd->bi_memstart = CONFIG_SYS_SDRAM_BASE;
>>   bd->bi_memsize = CONFIG_SYS_MAX_RAM_SIZE;
>> - bd->bi_baudrate = (gd->baudrate > 0)
>> - ? simple_strtoul(gd->baudrate, NULL, 10) : CONFIG_BAUDRATE;
>
>
> I'd rather say the fix here is to use (gd->baudrate > 0) ? gd->baudrate ?
> CONFIG_BAUDRATE ;
>
> Otherwise you're changing the logic of the code and for that, you'd need 
> Mikes'
> ack.
Hi Marek,

It's because gd->baudrate is set in init_baudrate().
And if it is not found in an environment variable, the default is
CONFIG_BAUDRATE.
It's unlikely gd->baudrate is 0.

So if we have below code:
init_baudrate();
gd->bd->bi_baudrate = (gd->baudrate > 0) ? gd->baudrate : CONFIG_BAUDRATE;

In an unlikely case where baudrate environment variable is set to 0,
then we have inconsistency setting:

gd->baudrate = 0;
gd->bd->bi_baudrate = CONFIG_BAUDRATE;


Comments?

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

[U-Boot] [PATCH] net: Use ARRAY_SIZE at appropriate places

[Axel Lin]

Use ARRAY_SIZE instead of having similar implementation in each drivers.

Signed-off-by: Axel Lin 
Cc: Albert Aribaud 
Cc: Ben Warren 
Cc: Jean-Christophe PLAGNIOL-VILLARD 
Cc: Joe Hershberger 
Cc: Marek Vasut 
Cc: Mike Frysinger 
Cc: Nobuhiro Iwamatsu 
Cc: TsiChungLiew 
Cc: Wolfgang Denk 
Cc: York Sun 
---
Hi,
tools/checkpatch.pl shows
total: 2 errors, 12 warnings, 0 checks, 89 lines checked

The errors are:
ERROR: code indent should use tabs where possible

The warnings are something like below:
WARNING: line over 80 characters
WARNING: please, no spaces at the start of a line
WARNING: Avoid CamelCase: 

I don't fixup the checkpatch.pl issues because I feel having the patch
as is seems cleaner.

Regards,
Axel

 drivers/net/ax88180.c | 2 +-
 drivers/net/fsl_mcdmafec.c| 2 +-
 drivers/net/lan91c96.c| 2 +-
 drivers/net/mcffec.c  | 2 +-
 drivers/net/mcfmii.c  | 2 +-
 drivers/net/ne2000.c  | 2 +-
 drivers/net/npe/IxEthDBFeatures.c | 4 ++--
 drivers/net/npe/IxOsalIoMem.c | 3 +--
 drivers/net/npe/include/IxEthDBPortDefs.h | 2 +-
 drivers/net/npe/include/IxOsalTypes.h | 2 +-
 10 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ax88180.c b/drivers/net/ax88180.c
index f501768..7f0cfe5 100644
--- a/drivers/net/ax88180.c
+++ b/drivers/net/ax88180.c
@@ -157,7 +157,7 @@ static void ax88180_mac_reset (struct eth_device *dev)
OUTW (dev, MISC_RESET_MAC, MISC);
tmpval = INW (dev, MISC);
 
-   for (i = 0; i < (sizeof (program_seq) / sizeof (program_seq[0])); i++)
+   for (i = 0; i < ARRAY_SIZE(program_seq); i++)
OUTW (dev, program_seq[i].value, program_seq[i].offset);
 }
 
diff --git a/drivers/net/fsl_mcdmafec.c b/drivers/net/fsl_mcdmafec.c
index 63842cd..0e18764 100644
--- a/drivers/net/fsl_mcdmafec.c
+++ b/drivers/net/fsl_mcdmafec.c
@@ -520,7 +520,7 @@ int mcdmafec_initialize(bd_t * bis)
u32 tmp = CONFIG_SYS_INTSRAM + 0x2000;
 #endif
 
-   for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) {
+   for (i = 0; i < ARRAY_SIZE(fec_info); i++) {
 
dev =
(struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE,
diff --git a/drivers/net/lan91c96.c b/drivers/net/lan91c96.c
index 11d350e..47c15c4 100644
--- a/drivers/net/lan91c96.c
+++ b/drivers/net/lan91c96.c
@@ -779,7 +779,7 @@ static int lan91c96_detect_chip(struct eth_device *dev)
SMC_SELECT_BANK(dev, 3);
chip_id = (SMC_inw(dev, 0xA) & LAN91C96_REV_CHIPID) >> 4;
SMC_SELECT_BANK(dev, 0);
-   for (r = 0; r < sizeof(supported_chips) / sizeof(struct id_type); r++)
+   for (r = 0; r < ARRAY_SIZE(supported_chips); r++)
if (chip_id == supported_chips[r].id)
return r;
return 0;
diff --git a/drivers/net/mcffec.c b/drivers/net/mcffec.c
index ed7459c..7ae6320 100644
--- a/drivers/net/mcffec.c
+++ b/drivers/net/mcffec.c
@@ -559,7 +559,7 @@ int mcffec_initialize(bd_t * bis)
u32 tmp = CONFIG_SYS_INIT_RAM_ADDR + 0x1000;
 #endif
 
-   for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) {
+   for (i = 0; i < ARRAY_SIZE(fec_info); i++) {
 
dev =
(struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE,
diff --git a/drivers/net/mcfmii.c b/drivers/net/mcfmii.c
index 5e64dbd..63bc8f8 100644
--- a/drivers/net/mcfmii.c
+++ b/drivers/net/mcfmii.c
@@ -186,7 +186,7 @@ int mii_discover_phy(struct eth_device *dev)
printf("PHY @ 0x%x pass %d\n", phyno, pass);
 #endif
 
-   for (i = 0; (i < (sizeof(phyinfo) / sizeof(phy_info_t)))
+   for (i = 0; (i < ARRAY_SIZE(phyinfo))
&& (phyinfo[i].phyid != 0); i++) {
if (phyinfo[i].phyid == phytype) {
 #ifdef ET_DEBUG
diff --git a/drivers/net/ne2000.c b/drivers/net/ne2000.c
index 3939158..e6cd3e9 100644
--- a/drivers/net/ne2000.c
+++ b/drivers/net/ne2000.c
@@ -228,7 +228,7 @@ int get_prom(u8* mac_addr, u8* base_addr)
 
mdelay (10);
 
-   for (i = 0; i < sizeof (program_seq) / sizeof (program_seq[0]); i++)
+   for (i = 0; i < ARRAY_SIZE(program_seq); i++)
n2k_outb (program_seq[i].value, program_seq[i].offset);
 
PRINTK ("PROM:");
diff --git a/drivers/net/npe/IxEthDBFeatures.c 
b/drivers/net/npe/IxEthDBFeatures.c
index c5b680a..ecabec5 100644
--- a/drivers/net/npe/IxEthDBFeatures.c
+++ b/drivers/net/npe/IxEthDBFeatures.c
@@ -144,7 +144,7 @@ void ixEthDBFeatureCapabilityScan(void)
 
 /* find the traffic class definition index compatible with the 
current NPE A functionality ID */
 for (trafficClassDefinitionIndex = 0 ;
-trafficClassDefinitionIndex < sizeof 
(ixEthDBTrafficClassDefinitions) / sizeof (ixEthDBTrafficClassDefinitions[0]);
+t

Re: [U-Boot] howto write uboot to nand

[Jason Cooper]

On Fri, Jun 28, 2013 at 03:15:03PM -0500, kqt4a...@gmail.com wrote:
> I thought I should ask before I screw this up. I have a new uboot
> that I load with kwboot. It works fine and I want to write it to
> nand. I see many references to write it after loading with tftp but
> not kwboot. Can I do this? How? This is to a Guruplug. Using tftp
> 
> tftp 0x640 u-boot.kwb
> nand erase 0x 0x010
> nand write 0x640 0x000 0x8

Well, I typically use openocd and the jtag board to do the same thing.
I'll use openocd to load and execute the new version of u-boot that I
built, and then from that new u-boot, I'll transfer the .kwb image I
want to burn to flash.

So, basically, you can just use tftp like you outlined above, but from
the new image you loaded and executed via kwboot.

Ultimately, you're responsible if it breaks.  No response here is going
to mitigate that ;-)

hth,

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

Re: [U-Boot] [PATCH] net: Use ARRAY_SIZE at appropriate places

[Jagan Teki]

Hi Axel,

On Sat, Jun 29, 2013 at 6:46 PM, Axel Lin  wrote:
> Use ARRAY_SIZE instead of having similar implementation in each drivers.
>
> Signed-off-by: Axel Lin 
> Cc: Albert Aribaud 
> Cc: Ben Warren 
> Cc: Jean-Christophe PLAGNIOL-VILLARD 
> Cc: Joe Hershberger 
> Cc: Marek Vasut 
> Cc: Mike Frysinger 
> Cc: Nobuhiro Iwamatsu 
> Cc: TsiChungLiew 
> Cc: Wolfgang Denk 
> Cc: York Sun 
> ---
> Hi,
> tools/checkpatch.pl shows
> total: 2 errors, 12 warnings, 0 checks, 89 lines checked
>
> The errors are:
> ERROR: code indent should use tabs where possible
>
> The warnings are something like below:
> WARNING: line over 80 characters
> WARNING: please, no spaces at the start of a line
> WARNING: Avoid CamelCase: 
>
> I don't fixup the checkpatch.pl issues because I feel having the patch
> as is seems cleaner.

Please fix these WARNINGS/ERRORS, as these comes issues comes up with
your changed code-

--
Thanks,
Jagan.

>
> Regards,
> Axel
>
>  drivers/net/ax88180.c | 2 +-
>  drivers/net/fsl_mcdmafec.c| 2 +-
>  drivers/net/lan91c96.c| 2 +-
>  drivers/net/mcffec.c  | 2 +-
>  drivers/net/mcfmii.c  | 2 +-
>  drivers/net/ne2000.c  | 2 +-
>  drivers/net/npe/IxEthDBFeatures.c | 4 ++--
>  drivers/net/npe/IxOsalIoMem.c | 3 +--
>  drivers/net/npe/include/IxEthDBPortDefs.h | 2 +-
>  drivers/net/npe/include/IxOsalTypes.h | 2 +-
>  10 files changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ax88180.c b/drivers/net/ax88180.c
> index f501768..7f0cfe5 100644
> --- a/drivers/net/ax88180.c
> +++ b/drivers/net/ax88180.c
> @@ -157,7 +157,7 @@ static void ax88180_mac_reset (struct eth_device *dev)
> OUTW (dev, MISC_RESET_MAC, MISC);
> tmpval = INW (dev, MISC);
>
> -   for (i = 0; i < (sizeof (program_seq) / sizeof (program_seq[0])); i++)
> +   for (i = 0; i < ARRAY_SIZE(program_seq); i++)
> OUTW (dev, program_seq[i].value, program_seq[i].offset);
>  }
>
> diff --git a/drivers/net/fsl_mcdmafec.c b/drivers/net/fsl_mcdmafec.c
> index 63842cd..0e18764 100644
> --- a/drivers/net/fsl_mcdmafec.c
> +++ b/drivers/net/fsl_mcdmafec.c
> @@ -520,7 +520,7 @@ int mcdmafec_initialize(bd_t * bis)
> u32 tmp = CONFIG_SYS_INTSRAM + 0x2000;
>  #endif
>
> -   for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) {
> +   for (i = 0; i < ARRAY_SIZE(fec_info); i++) {
>
> dev =
> (struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE,
> diff --git a/drivers/net/lan91c96.c b/drivers/net/lan91c96.c
> index 11d350e..47c15c4 100644
> --- a/drivers/net/lan91c96.c
> +++ b/drivers/net/lan91c96.c
> @@ -779,7 +779,7 @@ static int lan91c96_detect_chip(struct eth_device *dev)
> SMC_SELECT_BANK(dev, 3);
> chip_id = (SMC_inw(dev, 0xA) & LAN91C96_REV_CHIPID) >> 4;
> SMC_SELECT_BANK(dev, 0);
> -   for (r = 0; r < sizeof(supported_chips) / sizeof(struct id_type); r++)
> +   for (r = 0; r < ARRAY_SIZE(supported_chips); r++)
> if (chip_id == supported_chips[r].id)
> return r;
> return 0;
> diff --git a/drivers/net/mcffec.c b/drivers/net/mcffec.c
> index ed7459c..7ae6320 100644
> --- a/drivers/net/mcffec.c
> +++ b/drivers/net/mcffec.c
> @@ -559,7 +559,7 @@ int mcffec_initialize(bd_t * bis)
> u32 tmp = CONFIG_SYS_INIT_RAM_ADDR + 0x1000;
>  #endif
>
> -   for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) {
> +   for (i = 0; i < ARRAY_SIZE(fec_info); i++) {
>
> dev =
> (struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE,
> diff --git a/drivers/net/mcfmii.c b/drivers/net/mcfmii.c
> index 5e64dbd..63bc8f8 100644
> --- a/drivers/net/mcfmii.c
> +++ b/drivers/net/mcfmii.c
> @@ -186,7 +186,7 @@ int mii_discover_phy(struct eth_device *dev)
> printf("PHY @ 0x%x pass %d\n", phyno, pass);
>  #endif
>
> -   for (i = 0; (i < (sizeof(phyinfo) / 
> sizeof(phy_info_t)))
> +   for (i = 0; (i < ARRAY_SIZE(phyinfo))
> && (phyinfo[i].phyid != 0); i++) {
> if (phyinfo[i].phyid == phytype) {
>  #ifdef ET_DEBUG
> diff --git a/drivers/net/ne2000.c b/drivers/net/ne2000.c
> index 3939158..e6cd3e9 100644
> --- a/drivers/net/ne2000.c
> +++ b/drivers/net/ne2000.c
> @@ -228,7 +228,7 @@ int get_prom(u8* mac_addr, u8* base_addr)
>
> mdelay (10);
>
> -   for (i = 0; i < sizeof (program_seq) / sizeof (program_seq[0]); i++)
> +   for (i = 0; i < ARRAY_SIZE(program_seq); i++)
> n2k_outb (program_seq[i].value, program_seq[i].offset);
>
> PRINTK ("PROM:");
> diff --git a/drivers/net/npe/IxEthDBFeatures.c 
> b/drivers/net/npe/IxEthDBFeatures.c
> index c5b680a..ecabec5 100644
> --- a/drivers/net/npe/IxEthDBFeatures.c
> +++ b/drivers/net/npe/IxEthDBFeat

Re: [U-Boot] [PATCH] net: Use ARRAY_SIZE at appropriate places

[Marek Vasut]

Dear Axel Lin,

> Use ARRAY_SIZE instead of having similar implementation in each drivers.
> 
> Signed-off-by: Axel Lin 
> Cc: Albert Aribaud 
> Cc: Ben Warren 
> Cc: Jean-Christophe PLAGNIOL-VILLARD 
> Cc: Joe Hershberger 
> Cc: Marek Vasut 
> Cc: Mike Frysinger 
> Cc: Nobuhiro Iwamatsu 
> Cc: TsiChungLiew 
> Cc: Wolfgang Denk 
> Cc: York Sun 

You can trim the CC a bit next time ;-)

> ---
> Hi,
> tools/checkpatch.pl shows
> total: 2 errors, 12 warnings, 0 checks, 89 lines checked
> 
> The errors are:
> ERROR: code indent should use tabs where possible
> 
> The warnings are something like below:
> WARNING: line over 80 characters
> WARNING: please, no spaces at the start of a line
> WARNING: Avoid CamelCase: 
> 
> I don't fixup the checkpatch.pl issues because I feel having the patch
> as is seems cleaner.

No problem on my end, that's understandable.

> Regards,
> Axel
> 
>  drivers/net/ax88180.c | 2 +-
>  drivers/net/fsl_mcdmafec.c| 2 +-
>  drivers/net/lan91c96.c| 2 +-
>  drivers/net/mcffec.c  | 2 +-
>  drivers/net/mcfmii.c  | 2 +-
>  drivers/net/ne2000.c  | 2 +-
>  drivers/net/npe/IxEthDBFeatures.c | 4 ++--
>  drivers/net/npe/IxOsalIoMem.c | 3 +--
>  drivers/net/npe/include/IxEthDBPortDefs.h | 2 +-
>  drivers/net/npe/include/IxOsalTypes.h | 2 +-
>  10 files changed, 11 insertions(+), 12 deletions(-)

[...]

>   * @def IX_ETH_DB_UNKNOWN_PORT
> diff --git a/drivers/net/npe/include/IxOsalTypes.h
> b/drivers/net/npe/include/IxOsalTypes.h index 06e71de..615c655 100644
> --- a/drivers/net/npe/include/IxOsalTypes.h
> +++ b/drivers/net/npe/include/IxOsalTypes.h
> @@ -93,7 +93,7 @@ typedef volatile INT32 VINT32;
> 
> 
>  #ifndef NUMELEMS
> -#define NUMELEMS(x) (sizeof(x) / sizeof((x)[0]))
> +#define NUMELEMS(x) ARRAY_SIZE(x)
>  #endif

Just kill this macro altogether please.

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

Re: [U-Boot] [PATCH v5 2/8] usb: ehci-marvell: add support for second USB controller

[Marek Vasut]

Dear Sascha Silbe,

> Dear Wolfgang Denk,
> 
> Wolfgang Denk  writes:
> >> -#define rdl(off)  readl(MVUSB0_BASE + (off))
> >> -#define wrl(off, val) writel((val), MVUSB0_BASE + (off))
> >> +#define rdl(base, off)readl((base) + (off))
> >> +#define wrl(base, off, val)   writel((val), (base) + (off))
> > 
> > Instead of extending this, can we eventually clean this up and use C
> > structs instead?  U-Boot does not allow device accesses through a
> > based plus offset notation.
> 
> Thanks for the review. I've given the clean-up a stab today, as a
> separate patch that the CuBox support series can build on.
> 
> >>u32 size, base, attrib;
> >> 
> >> +#ifdef MVUSB1_BASE
> >> +  u32 usb_base = (index == 0) ? MVUSB0_BASE : MVUSB1_BASE;
> >> +#else
> >> +  u32 usb_base = MVUSB0_BASE;
> >> +#endif
> > 
> > Can we please also avoid this #ifdef's ?  Eventually you can use
> > something like "base_0 + index * sizeof(struct usb_something)" ?
> 
> The two USB host controllers on Dove are separate entities at
> different base offsets (0x5 vs. 0x51000). We could fill up the
> register struct to have a size of 0x1000, but then the next SoC to be
> supported could come up with a different offset.

Check drivers/usb/host/ehci-mxs.c for handling of such a case ;-)

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

Re: [U-Boot] [PATCH] usb: ehci-marvell: use structs for registers

[Marek Vasut]

Dear Sascha Silbe,

> Use structs instead of computing offsets for accessing individual
> registers. The base address of the USB controller is still an offset
> defined in SoC headers as it can differ between SoCs.
> 
> Signed-off-by: Sascha Silbe 
> ---
> Tested on CuBox Pro only.
> 
>  drivers/usb/host/ehci-marvell.c | 48
> + 1 file changed, 34
> insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-marvell.c
> b/drivers/usb/host/ehci-marvell.c index 2b73e4a..e4d6881 100644
> --- a/drivers/usb/host/ehci-marvell.c
> +++ b/drivers/usb/host/ehci-marvell.c
> @@ -36,17 +36,35 @@
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> -#define rdl(off) readl(MVUSB0_BASE + (off))
> -#define wrl(off, val)writel((val), MVUSB0_BASE + (off))
> -
> -#define USB_WINDOW_CTRL(i)   (0x320 + ((i) << 4))
> -#define USB_WINDOW_BASE(i)   (0x324 + ((i) << 4))
>  #define USB_TARGET_DRAM  0x0
> 
> +/* USB 2.0 Bridge Address Decoding registers */
> +struct mvusb_bad_window_regs {
> + u32 control;
> + u32 base;
> + u32 reserved[2];
> +};
> +
> +struct mvusb_bridge_regs {
> + u32 bridge_control;
> + u32 reserved1[3];
> + u32 int_cause; /* Bridge Interrupt Cause Register */
> + u32 int_mask; /* Bridge Interrupt Mask Register */
> + u32 reserved2;
> + u32 error_addr; /* Bridge Error Address Register */
> + struct mvusb_bad_window_regs window[4];
> +};
> +
> +struct mvusb_regs {
> + u32 unused1[0x40];
> + u32 ehci_regs[0x80];
> + struct mvusb_bridge_regs bridge;
> +};
> +
>  /*
>   * USB 2.0 Bridge Address Decoding registers setup
>   */
> -static void usb_brg_adrdec_setup(void)
> +static void usb_brg_adrdec_setup(struct mvusb_regs *usb_base)
>  {
>   int i;
>   u32 size, base, attrib;
> @@ -75,14 +93,15 @@ static void usb_brg_adrdec_setup(void)
> 
>   size = gd->bd->bi_dram[i].size;
>   base = gd->bd->bi_dram[i].start;
> - if ((size) && (attrib))
> - wrl(USB_WINDOW_CTRL(i),
> - MVCPU_WIN_CTRL_DATA(size, USB_TARGET_DRAM,
> - attrib, MVCPU_WIN_ENABLE));
> + if (size && attrib)
> + writel(MVCPU_WIN_CTRL_DATA(size, USB_TARGET_DRAM,
> +attrib, MVCPU_WIN_ENABLE),
> +usb_base->bridge.window[i].control);
>   else
> - wrl(USB_WINDOW_CTRL(i), MVCPU_WIN_DISABLE);
> + writel(MVCPU_WIN_DISABLE,
> +usb_base->bridge.window[i].control);
> 
> - wrl(USB_WINDOW_BASE(i), base);
> + writel(base, usb_base->bridge.window[i].base);
>   }
>  }

OK

> @@ -92,9 +111,10 @@ static void usb_brg_adrdec_setup(void)
>   */
>  int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor
> **hcor) {
> - usb_brg_adrdec_setup();
> + struct mvusb_regs *usb_base = (struct mvusb_regs *)MVUSB0_BASE;
> 
> - *hccr = (struct ehci_hccr *)(MVUSB0_BASE + 0x100);
> + usb_brg_adrdec_setup(usb_base);
> + *hccr = (struct ehci_hccr *)(&usb_base->ehci_regs);
>   *hcor = (struct ehci_hcor *)((uint32_t) *hccr
>   + HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));

I'd keep this as-is, since the 0x100 is fairy standard offset in the USB case.

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

Re: [U-Boot] [PATCH] net: Use ARRAY_SIZE at appropriate places

[Axel Lin]

2013/6/30 Marek Vasut :
> Dear Axel Lin,
>
>> Use ARRAY_SIZE instead of having similar implementation in each drivers.
>>
>> Signed-off-by: Axel Lin 
>> Cc: Albert Aribaud 
>> Cc: Ben Warren 
>> Cc: Jean-Christophe PLAGNIOL-VILLARD 
>> Cc: Joe Hershberger 
>> Cc: Marek Vasut 
>> Cc: Mike Frysinger 
>> Cc: Nobuhiro Iwamatsu 
>> Cc: TsiChungLiew 
>> Cc: Wolfgang Denk 
>> Cc: York Sun 
>
> You can trim the CC a bit next time ;-)
Hi Marek,

I try to cc related people. the maintainers, the driver authers, and developer
who is active in recent commits.

Is there a smarter way to find who should be CCed?
Some tool like get_maintainer.pl used in linux kernel may help.

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

[U-Boot] [PATCH v2] net: Use ARRAY_SIZE at appropriate places

[Axel Lin]

Use ARRAY_SIZE instead of having similar implementation in each drivers.

Signed-off-by: Axel Lin 
Cc: Albert Aribaud 
Cc: Ben Warren 
Cc: Jean-Christophe PLAGNIOL-VILLARD 
Cc: Joe Hershberger 
Cc: Marek Vasut 
Cc: Mike Frysinger 
Cc: Nobuhiro Iwamatsu 
Cc: TsiChungLiew 
Cc: Wolfgang Denk 
Cc: York Sun 

Signed-off-by: Axel Lin 
---
Hi Jagan Teki,
This is v2 per your request.

v2:
Fix checkpatch issues, now checkpatch only shows below warnings:
a few WARNING: Avoid CamelCase
and a WARNING: line over 80 characters
#134: FILE: drivers/net/npe/IxEthDBFeatures.c:150:
+   if 
(ixEthDBTrafficClassDefinitions[trafficClassDefinitionIndex][IX_ETH_DB_NPE_A_FUNCTIONALITY_ID_INDEX]
 == npeAImageId.functionalityId) {

Obviously, I prefer v1 for below reasons:
1. The diff in v2 becomes bigger and makes it harder for review.
2. The whole file in drivers/net/npe/IxEthDBFeatures.c uses 4 spaces as indent,
   only the lines touched by this patch uses tab as indent (to make checkpatch 
happy).
   This makes the code looks odd after apply v2, I don't think this is an 
improvement.

Well, it's up to maintainer to pick up the version he prefer.

Regards,
Axel

 drivers/net/ax88180.c |  2 +-
 drivers/net/fsl_mcdmafec.c|  2 +-
 drivers/net/lan91c96.c|  2 +-
 drivers/net/mcffec.c  |  2 +-
 drivers/net/mcfmii.c  |  2 +-
 drivers/net/ne2000.c  |  2 +-
 drivers/net/npe/IxEthDBFeatures.c | 28 ++--
 drivers/net/npe/IxOsalIoMem.c |  3 +--
 drivers/net/npe/include/IxEthDBPortDefs.h |  2 +-
 drivers/net/npe/include/IxOsalTypes.h |  2 +-
 10 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ax88180.c b/drivers/net/ax88180.c
index f501768..7f0cfe5 100644
--- a/drivers/net/ax88180.c
+++ b/drivers/net/ax88180.c
@@ -157,7 +157,7 @@ static void ax88180_mac_reset (struct eth_device *dev)
OUTW (dev, MISC_RESET_MAC, MISC);
tmpval = INW (dev, MISC);
 
-   for (i = 0; i < (sizeof (program_seq) / sizeof (program_seq[0])); i++)
+   for (i = 0; i < ARRAY_SIZE(program_seq); i++)
OUTW (dev, program_seq[i].value, program_seq[i].offset);
 }
 
diff --git a/drivers/net/fsl_mcdmafec.c b/drivers/net/fsl_mcdmafec.c
index 63842cd..0e18764 100644
--- a/drivers/net/fsl_mcdmafec.c
+++ b/drivers/net/fsl_mcdmafec.c
@@ -520,7 +520,7 @@ int mcdmafec_initialize(bd_t * bis)
u32 tmp = CONFIG_SYS_INTSRAM + 0x2000;
 #endif
 
-   for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) {
+   for (i = 0; i < ARRAY_SIZE(fec_info); i++) {
 
dev =
(struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE,
diff --git a/drivers/net/lan91c96.c b/drivers/net/lan91c96.c
index 11d350e..47c15c4 100644
--- a/drivers/net/lan91c96.c
+++ b/drivers/net/lan91c96.c
@@ -779,7 +779,7 @@ static int lan91c96_detect_chip(struct eth_device *dev)
SMC_SELECT_BANK(dev, 3);
chip_id = (SMC_inw(dev, 0xA) & LAN91C96_REV_CHIPID) >> 4;
SMC_SELECT_BANK(dev, 0);
-   for (r = 0; r < sizeof(supported_chips) / sizeof(struct id_type); r++)
+   for (r = 0; r < ARRAY_SIZE(supported_chips); r++)
if (chip_id == supported_chips[r].id)
return r;
return 0;
diff --git a/drivers/net/mcffec.c b/drivers/net/mcffec.c
index ed7459c..7ae6320 100644
--- a/drivers/net/mcffec.c
+++ b/drivers/net/mcffec.c
@@ -559,7 +559,7 @@ int mcffec_initialize(bd_t * bis)
u32 tmp = CONFIG_SYS_INIT_RAM_ADDR + 0x1000;
 #endif
 
-   for (i = 0; i < sizeof(fec_info) / sizeof(fec_info[0]); i++) {
+   for (i = 0; i < ARRAY_SIZE(fec_info); i++) {
 
dev =
(struct eth_device *)memalign(CONFIG_SYS_CACHELINE_SIZE,
diff --git a/drivers/net/mcfmii.c b/drivers/net/mcfmii.c
index 5e64dbd..63bc8f8 100644
--- a/drivers/net/mcfmii.c
+++ b/drivers/net/mcfmii.c
@@ -186,7 +186,7 @@ int mii_discover_phy(struct eth_device *dev)
printf("PHY @ 0x%x pass %d\n", phyno, pass);
 #endif
 
-   for (i = 0; (i < (sizeof(phyinfo) / sizeof(phy_info_t)))
+   for (i = 0; (i < ARRAY_SIZE(phyinfo))
&& (phyinfo[i].phyid != 0); i++) {
if (phyinfo[i].phyid == phytype) {
 #ifdef ET_DEBUG
diff --git a/drivers/net/ne2000.c b/drivers/net/ne2000.c
index 3939158..e6cd3e9 100644
--- a/drivers/net/ne2000.c
+++ b/drivers/net/ne2000.c
@@ -228,7 +228,7 @@ int get_prom(u8* mac_addr, u8* base_addr)
 
mdelay (10);
 
-   for (i = 0; i < sizeof (program_seq) / sizeof (program_seq[0]); i++)
+   for (i = 0; i < ARRAY_SIZE(program_seq); i++)
n2k_outb (program_seq[i].value, program_seq[i].offset);
 
PRINTK ("PROM:");
diff --git a/drivers/net/npe/IxEthDBFeatures.c 
b/drivers/net/npe/IxEthDBFeatures.c
index c5b680a.

Re: [U-Boot] [PATCH] net: Use ARRAY_SIZE at appropriate places

[Axel Lin]

>> diff --git a/drivers/net/npe/include/IxOsalTypes.h
>> b/drivers/net/npe/include/IxOsalTypes.h index 06e71de..615c655 100644
>> --- a/drivers/net/npe/include/IxOsalTypes.h
>> +++ b/drivers/net/npe/include/IxOsalTypes.h
>> @@ -93,7 +93,7 @@ typedef volatile INT32 VINT32;
>>
>>
>>  #ifndef NUMELEMS
>> -#define NUMELEMS(x) (sizeof(x) / sizeof((x)[0]))
>> +#define NUMELEMS(x) ARRAY_SIZE(x)
>>  #endif
>
> Just kill this macro altogether please.
Ah, yes. I missed your comment here when I reply the mail.
NUMELEMS is not used at all so it can be removed.
Will fix it in v3.

I just sent v2 per Jagan Teki's request.
I think I'll wait for Jagan Teki's feedback before sending v3.

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

Re: [U-Boot] [PATCH] gpio: spear_gpio: Fix gpio_set_value() implementation

[Axel Lin]

2013/6/21 Michael Trimarchi :
> On 06/21/2013 06:40 AM, Vipin Kumar wrote:
>> On 6/20/2013 7:26 PM, Axel Lin wrote:
>>> 2013/6/20 Marek Vasut

 Dear Axel Lin,

> In current gpio_set_value() implementation, it always sets the gpio 
> control
> bit no matter the value argument is 0 or 1. Thus the GPIOs never set to
> low. This patch fixes this bug.
>
> Signed-off-by: Axel Lin
> ---
>   drivers/gpio/spear_gpio.c | 5 -
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/spear_gpio.c b/drivers/gpio/spear_gpio.c
> index d3c728e..8878608 100644
> --- a/drivers/gpio/spear_gpio.c
> +++ b/drivers/gpio/spear_gpio.c
> @@ -52,7 +52,10 @@ int gpio_set_value(unsigned gpio, int value)
>   {
>struct gpio_regs *regs = (struct gpio_regs *)CONFIG_GPIO_BASE;
>
> - writel(1<<  gpio,®s->gpiodata[DATA_REG_ADDR(gpio)]);
> + if (value)
> + writel(1<<  gpio,®s->gpiodata[DATA_REG_ADDR(gpio)]);
> + else
> + writel(0,®s->gpiodata[DATA_REG_ADDR(gpio)]);

 How can this possibly work? Writing 0 to the whole bank will unset all the
 GPIOs, no ?
>>>
>>>
>>> Because each GPIO is controlled by a register.
>>> And only one bit will be set when set gpio to high.
>>>
>>> So it's safe to write 0 for clearing the bit.
>>>
>>> Note, the gpio_get_value() implementation also assumes there is only one bit
>>> will be set. ( If this is not true, both gpio_get_value() and 
>>> gpio_set_value()
>>> need fix.)
>>>
>>> Vipin, can you review this patch and confirm this behavior?
>>>
>>
>> Yes this is right. and the code is fine
>>
>
> The problem is not in set one bit but in reset one bit. Can you check
> the else path?

Hi,
I'm not the best person to answer this question because I don't have the
hardware and datasheet.
In the case only one bit is meaningful and the reset bits are 0,
it looks ok for me to write 0 for clearing the bit.
( note, each gpio pin is controlled by different register.)

This patch is acked and reviewed by Stefan Roese and Vipin Kumar.
I'm wondering if this patch is acceptable?
Or maybe a test-by can help to make this patch acceptable?

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