[U-Boot] [PATCH] Add mvsata support for the Sheevaplug devices
Add mvsata support to SHEEVAPLUG Signed-off-by: Gérald Kerma gera...@gmail.com --- include/configs/sheevaplug.h | 29 + 1 files changed, 29 insertions(+), 0 deletions(-) diff --git a/include/configs/sheevaplug.h b/include/configs/sheevaplug.h index c5de86e..6a80c15 100644 --- a/include/configs/sheevaplug.h +++ b/include/configs/sheevaplug.h @@ -98,6 +98,7 @@ #define CONFIG_CMD_NAND #define CONFIG_CMD_PING #define CONFIG_CMD_USB +#define CONFIG_CMD_IDE /* * NAND configuration @@ -196,6 +197,34 @@ #endif /* CONFIG_CMD_USB */ /* + * IDe Support on SATA port0 + */ +#ifdef CONFIG_CMD_IDE +#define __io +#define CONFIG_CMD_EXT2 +#define CONFIG_MVSATA_IDE +#define CONFIG_IDE_PREINIT +#define CONFIG_MVSATA_IDE_USE_PORT0 +/* Needs byte-swapping for ATA data register */ +#define CONFIG_IDE_SWAP_IO +/* Data, registers and alternate blocks are at the same offset */ +#define CONFIG_SYS_ATA_DATA_OFFSET (0x0100) +#define CONFIG_SYS_ATA_REG_OFFSET (0x0100) +#define CONFIG_SYS_ATA_ALT_OFFSET (0x0100) +/* Each 8-bit ATA register is aligned to a 4-bytes address */ +#define CONFIG_SYS_ATA_STRIDE 4 +/* Controller supports 48-bits LBA addressing */ +#define CONFIG_LBA48 +/* CONFIG_CMD_IDE requires some #defines for ATA registers */ +#define CONFIG_SYS_IDE_MAXBUS 1 +#define CONFIG_SYS_IDE_MAXDEVICE 1 +/* ATA registers base is at SATA controller base */ +#define CONFIG_SYS_ATA_BASE_ADDR KW_SATA_BASE +/* ATA bus 0 is Kirkwood port 1 on sheevaplug */ +#define CONFIG_SYS_ATA_IDE0_OFFSET KW_SATA_PORT1_OFFSET +#endif /* CONFIG_CMD_IDE */ + +/* * File system */ #define CONFIG_CMD_EXT2 -- ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add mvsata support for the Sheevaplug devices
Dear GEAK, In message 4c6635c1.2080...@gmail.com you wrote: Add mvsata support to SHEEVAPLUG ... --- a/include/configs/sheevaplug.h +++ b/include/configs/sheevaplug.h @@ -98,6 +98,7 @@ #define CONFIG_CMD_NAND #define CONFIG_CMD_PING #define CONFIG_CMD_USB +#define CONFIG_CMD_IDE Please keep list sorted. /* + * IDe Support on SATA port0 + */ Please fix typo. 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 Technology is dominated by those who manage what they do not under- stand. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v4] at91: Don't initialize watchdog if CONFIG_SKIP_WATCHDOG_INIT is defined
On Mon, Aug 9, 2010 at 7:42 AM, Alexander Stein wrote: Am Montag, 9. August 2010, 09:13:45 schrieb Mike Frysinger: On AT91 the watchdog mode register can only be written once after reset. If this register is written by u-boot e.g. a Linux driver can't reconfigure the watchdog later. If the watchdog is left untouched this is possible. Without touching the mode register the watchdog has a default setup and u-boot is still able to trigger the watchdog. [...] +- CONFIG_SKIP_WATCHDOG_INIT + + [arm AT91 only] If this variable is defined, then the + watchdog will not be programmed upon u-boot start. + On AT91 the watchdog mode register can only be written + once after reset. If this register is written by u-boot + e.g. a Linux driver can't reconfigure the watchdog later. If + the watchdog is left untouched this is possible. + Without touching the mode register the watchdog has a default + setup and u-boot is still able to trigger the watchdog. isnt the at91 logic inverted ? shouldnt the watchdog programming only be done when someone has opted in to it via some watchdog define ? This was my first version, but Wolfgang NAK'ed it as he see this as the wrong approach. See http://article.gmane.org/gmane.comp.boot-loaders.u-boot/81589 i think you interpreted his statement incorrectly ? he said it should raise an error, not that supporting CONFIG_HW_WATCHDOG is wrong. -mike ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2] Add mvsata support for the Sheevaplug devices
Add mvsata support to SHEEVAPLUG Signed-off-by: Gérald Kerma gera...@gmail.com --- Changes in v2: * typo fixed * list reordered include/configs/sheevaplug.h | 29 + 1 files changed, 29 insertions(+), 0 deletions(-) diff --git a/include/configs/sheevaplug.h b/include/configs/sheevaplug.h index c5de86e..dc803e1 100644 --- a/include/configs/sheevaplug.h +++ b/include/configs/sheevaplug.h @@ -94,6 +94,7 @@ #define CONFIG_CMD_AUTOSCRIPT #define CONFIG_CMD_DHCP #define CONFIG_CMD_ENV +#define CONFIG_CMD_IDE #define CONFIG_CMD_MII #define CONFIG_CMD_NAND #define CONFIG_CMD_PING @@ -196,6 +197,34 @@ #endif /* CONFIG_CMD_USB */ /* + * IDE Support on SATA port0 + */ +#ifdef CONFIG_CMD_IDE +#define __io +#define CONFIG_CMD_EXT2 +#define CONFIG_MVSATA_IDE +#define CONFIG_IDE_PREINIT +#define CONFIG_MVSATA_IDE_USE_PORT0 +/* Needs byte-swapping for ATA data register */ +#define CONFIG_IDE_SWAP_IO +/* Data, registers and alternate blocks are at the same offset */ +#define CONFIG_SYS_ATA_DATA_OFFSET (0x0100) +#define CONFIG_SYS_ATA_REG_OFFSET (0x0100) +#define CONFIG_SYS_ATA_ALT_OFFSET (0x0100) +/* Each 8-bit ATA register is aligned to a 4-bytes address */ +#define CONFIG_SYS_ATA_STRIDE 4 +/* Controller supports 48-bits LBA addressing */ +#define CONFIG_LBA48 +/* CONFIG_CMD_IDE requires some #defines for ATA registers */ +#define CONFIG_SYS_IDE_MAXBUS 1 +#define CONFIG_SYS_IDE_MAXDEVICE 1 +/* ATA registers base is at SATA controller base */ +#define CONFIG_SYS_ATA_BASE_ADDR KW_SATA_BASE +/* ATA bus 0 is Kirkwood port 1 on sheevaplug */ +#define CONFIG_SYS_ATA_IDE0_OFFSET KW_SATA_PORT1_OFFSET +#endif /* CONFIG_CMD_IDE */ + +/* * File system */ #define CONFIG_CMD_EXT2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] Align linebuf to avoid misaligned aliases of it
Commit 64419e47518bbba059c80b77558f93ad4804145c aliases the uint16_t usp and uint32_t uip variables in print_buffer() to uint8_t variable linebuf without aligning it to an uint32_t address, thus causing data aborts on ARM when doing md.l on 32-bit wide area (and probably 16-bit wide as well). Aligning linebuf fixes the issue. Signed-off-by: Albert Aribaud albert.arib...@free.fr --- lib/display_options.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/lib/display_options.c b/lib/display_options.c index 20319e6..c544777 100644 --- a/lib/display_options.c +++ b/lib/display_options.c @@ -101,7 +101,8 @@ void print_size(unsigned long long size, const char *s) #define DEFAULT_LINE_LENGTH_BYTES (16) int print_buffer (ulong addr, void* data, uint width, uint count, uint linelen) { - uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1]; + uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1] + __attribute__((__aligned__(sizeof(uint32_t; uint32_t *uip = (void*)linebuf; uint16_t *usp = (void*)linebuf; uint8_t *ucp = (void*)linebuf; -- 1.7.1 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] commit 64419e47 causes data aborts in md on orion5x internal registers
(adding Mike for info as he is the submitter of 64419e47) Hi all, on current origin/master, doing an md on internal registers area (0xf1xx) causes a data abort on orion5x-based edminiv2. A git bisect between origin/master ('bad') and the last edminiv2 commit ecaf3af2977e76484e0231d3113be3f4bbf2b8d6 ('good', known to work) points to commit 64419e47518bbba059c80b77558f93ad4804145c (print_buffer: optimize shrink). The data abort is in the aliasing of uip, usp and ucp to linebuf, which is not 4-bytes aligned. Aligning it fixes the issue. I've just posted a fix to the u-boot list: http://article.gmane.org/gmane.comp.boot-loaders.u-boot/82812 Rogan, please test it and see if it fixes your issue as well (can't swear that it will). Mike, please test it and see if it does not regress on your side (can't see why it should, but maybe the __aligned__ attribute is going to choke your build toolchain?) Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH V2 1/2] USB-CDC: correct wrong alignment in ether.c
The buffer for the status request must be word aligned because it is accessed with 32 bit pointer in the eth_status_complete function. Signed-off-by: Stefano Babic sba...@denx.de --- drivers/usb/gadget/ether.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 313f15f..0b4ed18 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -684,7 +684,7 @@ static struct usb_gadget_stringsstringtab = { /**/ static u8 control_req[USB_BUFSIZ]; -static u8 status_req[STATUS_BYTECOUNT]; +static u8 status_req[STATUS_BYTECOUNT] __attribute__ ((aligned(32))); -- 1.6.3.3 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH V2 2/2] USB-CDC: called handle_interrupts inside usb_eth_send
The patch removes an endless loop in the usb_eth_send if the tx_complete is not called before going in the loop. The driver interrupt routine is called allowing the driver to check if the TX is completed. Signed-off-by: Stefano Babic sba...@denx.de --- Changes from V1: - Comments by Remy Bohmer: moved usb_ep_queue inside timeout to not break at91 code drivers/usb/gadget/ether.c | 12 ++-- 1 files changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 0b4ed18..f082c03 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -1808,6 +1808,8 @@ static int usb_eth_send(struct eth_device* netdev, volatile void* packet, int le struct usb_request *req = NULL; struct eth_dev *dev = l_ethdev; + unsigned long ts; + unsigned long timeout = USB_CONNECT_TIMEOUT; dprintf(%s:...\n,__func__); req = dev-tx_req; @@ -1833,13 +1835,19 @@ static int usb_eth_send(struct eth_device* netdev, volatile void* packet, int le #endif dev-tx_qlen=1; + ts = get_timer(0); + packet_sent=0; retval = usb_ep_queue (dev-in_ep, req, GFP_ATOMIC); if (!retval) - dprintf(%s: packet queued\n,__func__); + debug(%s: packet queued\n,__func__); while(!packet_sent) { - packet_sent=0; + if (get_timer(ts) timeout) { + printf (timeout sending packets to usb ethernet\n); + return -1; + } + usb_gadget_handle_interrupts(); } return 0; -- 1.6.3.3 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Align linebuf to avoid misaligned aliases of it
On Sat, Aug 14, 2010 at 4:11 AM, Albert Aribaud wrote: Commit 64419e47518bbba059c80b77558f93ad4804145c aliases the uint16_t usp and uint32_t uip variables in print_buffer() to uint8_t variable linebuf without aligning it to an uint32_t address, thus causing data aborts on ARM when doing md.l on 32-bit wide area (and probably 16-bit wide as well). hmm, i guess i overlooked that issue. usually i'm pretty good at this stuff. sorry about that. Aligning linebuf fixes the issue. i dont believe there are issues with gcc stack aligning for native sizes ... just above that int print_buffer (ulong addr, void* data, uint width, uint count, uint linelen) { - uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1]; + uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1] + __attribute__((__aligned__(sizeof(uint32_t; __aligned(sizeof(uint32_t)) -mike ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Align linebuf to avoid misaligned aliases of it
Le 14/08/2010 10:25, Mike Frysinger a écrit : int print_buffer (ulong addr, void* data, uint width, uint count, uint linelen) { - uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1]; + uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1] + __attribute__((__aligned__(sizeof(uint32_t; __aligned(sizeof(uint32_t)) -mike I based the __aligned__ on what's in post/cpu/ppc4xx/cache.c. What is the difference between __aligned and __aligned__? Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] Make preparatory patches that initially have no effect?
Hello, my current work in the AT91 SoCs will add several new drivers. Some of them require new header files in arch-at91 or amendments to existing header files there. Same for some at91 specific header files in driver/*. In existing header files it is adding adresses for going-to-be-used devices and proper struct SoC defines for them, the new header files will only allow struct SoC access anyway. If I add those discrete changes to each driver patch (where it might actually belong), the incremental changes to some of those files would require all those driver patches to be applied in the right order to avoid conflicts. Therefore I would like to put all new header files and all changes to header files in one patch which would need to be applied before the driver patches. That patch would essentially cause no change to existing code. Anyone find this idea bad? Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] commit 64419e47 causes data aborts in md on orion5x internal registers
On 2010/08/14 10:14 AM, Albert ARIBAUD wrote: Rogan, please test it and see if it fixes your issue as well (can't swear that it will). Mike, please test it and see if it does not regress on your side (can't see why it should, but maybe the __aligned__ attribute is going to choke your build toolchain?) Amicalement, Hi Albert, Mike, Yes, it fixes my problem on the DNS323. Thanks! Rogan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2a] AT91: add SD/MMC support
Dear Schleifer, Alexander, If you have two days patience, wait for the new driver, I will be most happy to have a tester ;) TOP9000 mmci mci: setting clock 194000 Hz, block size 512 mci: setting clock 24832000 Hz, block size 512 mci: setting clock 194000 Hz, block size 512 mci: setting clock 194000 Hz, block size 512 mci: setting clock 24832000 Hz, block size 512 Device: mci Manufacturer ID: 3 OEM: 5344 Name: SD32G Tran Speed: 2500 Rd Block Len: 512 SD version 2.0 High Capacity: Yes SDHC Capacity: 31914983424 32 GB !!! Bus Width: 4-bit TOP9000 mmc read mci 2100 3b7 100 MMC read: dev # 0, block # 62324736, count 256 ... mci: setting clock 194000 Hz, block size 512 mci: setting clock 24832000 Hz, block size 512 mci: setting clock 194000 Hz, block size 512 mci: setting clock 194000 Hz, block size 512 mci: setting clock 24832000 Hz, block size 512 256 blocks read: OK TOP9000 mmc read mci 0 3b7 100 MMC read: dev # 0, block # 62324736, count 256 ... mci: setting clock 194000 Hz, block size 512 mci: setting clock 24832000 Hz, block size 512 mci: setting clock 194000 Hz, block size 512 mci: setting clock 194000 Hz, block size 512 mci: setting clock 24832000 Hz, block size 512 256 blocks read: OK fatls, however fails, probably cannot handle 32 Gigs :) (fatls works with a SDHC card with 4 Gigs) Once my other question pertaining to *.h files is answered, I can supply the driver as a patch. Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] IDE_BUS unconditionally expects 2 devices per bus
Hi folks Since include/ide.h defines IDE_BUS(dev) as (dev 1), it ignores the values of CONFIG_SYS_IDE_MAXBUS and CONFIG_SYS_MAXDEVICE, and unconditionally expects an IDE bus to have two devices. This expectation falls down with the Orion5x SATA support, which uses that SATA controller in IDE compatability mode, but can obviously only have one device per bus. I'm not 100% sure whether the problem is with the IDE code, or with the Orion5x code which identifies a drive at both positions on the IDE bus. i.e. if I define CONFIG_SYS_IDE_MAXDEVICE as (CONFIG_SYS_IDE_MAXBUS*2), I get the first drive shown twice, and the second drive shown twice. The following RFC patch works for me, but may well break other boards that do not define CONFIG_SYS_IDE_MAXDEVICE and CONFIG_SYS_IDE_MAXBUS properly: diff --git a/include/ide.h b/include/ide.h index 6a1b7ae..3f81fb1 100644 --- a/include/ide.h +++ b/include/ide.h @@ -24,7 +24,7 @@ #ifndef_IDE_H #define _IDE_H -#defineIDE_BUS(dev)(dev 1) +#defineIDE_BUS(dev)(dev (CONFIG_SYS_IDE_MAXDEVICE / CONFIG_SYS_IDE_MAXBUS - 1)) #defineATA_CURR_BASE(dev) (CONFIG_SYS_ATA_BASE_ADDR+ide_bus_offset[IDE_BUS(dev)]) Ok, I'm sure it is line wrapped, tab-damaged, etc, and the line itself is too long, but conceptually, would that be acceptable? Thanks Rogan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] IDE_BUS unconditionally expects 2 devices per bus
On 2010/08/14 12:41 PM, Rogan Dawes wrote: -#defineIDE_BUS(dev)(dev 1) +#defineIDE_BUS(dev)(dev (CONFIG_SYS_IDE_MAXDEVICE / CONFIG_SYS_IDE_MAXBUS - 1)) #defineATA_CURR_BASE(dev) (CONFIG_SYS_ATA_BASE_ADDR+ide_bus_offset[IDE_BUS(dev)]) Ok, I'm an idiot! The reason was staring me in the face! ATA_CURR_BASE(dev) relies on IDE_BUS, which is why the same disk was being enumerated twice. It seems that the above patch is indeed correct. Rogan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] IDE_BUS unconditionally expects 2 devices per bus
Le 14/08/2010 12:46, Rogan Dawes a écrit : On 2010/08/14 12:41 PM, Rogan Dawes wrote: -#defineIDE_BUS(dev)(dev 1) +#defineIDE_BUS(dev)(dev (CONFIG_SYS_IDE_MAXDEVICE / CONFIG_SYS_IDE_MAXBUS - 1)) #defineATA_CURR_BASE(dev) (CONFIG_SYS_ATA_BASE_ADDR+ide_bus_offset[IDE_BUS(dev)]) Ok, I'm an idiot! The reason was staring me in the face! ATA_CURR_BASE(dev) relies on IDE_BUS, which is why the same disk was being enumerated twice. It seems that the above patch is indeed correct. Good findings, Rogan. :) However, you should submit patches using git format-patch and git send-email, both properly configured -- make sure format-patch has the -s option and send-email has the proper e-mail address settings for sender and recipient(s). And before submitting, think of checking the patch with linux's script/checkpatch.pl. --no-tree. At least it'll let you know about the long line. Mind you, ide.h itself has several long lines, that checkpatch won't tell you about since this is outside your patch. :) I applied the change manually, With 2 busse and 2 devices on the ED Mini (which has only one disk), I don't get the duplicate drive. With 1 bus and 2 devices, I do see the disk twice. :( Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add mvsata support for the Sheevaplug devices
Hello. GEAK wrote: Add mvsata support to SHEEVAPLUG Signed-off-by: Gérald Kerma gera...@gmail.com --- include/configs/sheevaplug.h | 29 + 1 files changed, 29 insertions(+), 0 deletions(-) diff --git a/include/configs/sheevaplug.h b/include/configs/sheevaplug.h index c5de86e..6a80c15 100644 --- a/include/configs/sheevaplug.h +++ b/include/configs/sheevaplug.h [...] @@ -196,6 +197,34 @@ #endif /* CONFIG_CMD_USB */ /* + * IDe Support on SATA port0 + */ +#ifdef CONFIG_CMD_IDE +#define __io +#define CONFIG_CMD_EXT2 +#define CONFIG_MVSATA_IDE +#define CONFIG_IDE_PREINIT +#define CONFIG_MVSATA_IDE_USE_PORT0 +/* Needs byte-swapping for ATA data register */ +#define CONFIG_IDE_SWAP_IO +/* Data, registers and alternate blocks are at the same offset */ +#define CONFIG_SYS_ATA_DATA_OFFSET (0x0100) +#define CONFIG_SYS_ATA_REG_OFFSET(0x0100) +#define CONFIG_SYS_ATA_ALT_OFFSET(0x0100) The parens are useless here. WBR, Sergei ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] USB-CDC: correct wrong alignment in ether.c
Hello. Stefano Babic wrote: The buffer for the status request must be word aligned because it is accessed with 32 bit pointer in the eth_status_complete function. Signed-off-by: Stefano Babic sba...@denx.de [...] diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 313f15f..0b4ed18 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -684,7 +684,7 @@ static struct usb_gadget_strings stringtab = { /**/ static u8 control_req[USB_BUFSIZ]; -static u8 status_req[STATUS_BYTECOUNT]; +static u8 status_req[STATUS_BYTECOUNT] __attribute__ ((aligned(32))); You're aligning to 32 *bytes* -- is that what you meant by 32 bit pointer? WBR, Sergei ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 2/2] USB-CDC: called handle_interrupts inside usb_eth_send
Stefano Babic wrote: The patch removes an endless loop in the usb_eth_send if the tx_complete is not called before going in the loop. The driver interrupt routine is called allowing the driver to check if the TX is completed. Signed-off-by: Stefano Babic sba...@denx.de --- [...] diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 0b4ed18..f082c03 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -1808,6 +1808,8 @@ static int usb_eth_send(struct eth_device* netdev, volatile void* packet, int le struct usb_request *req = NULL; struct eth_dev *dev = l_ethdev; + unsigned long ts; + unsigned long timeout = USB_CONNECT_TIMEOUT; Please keep an empty line after the declaration block. @@ -1833,13 +1835,19 @@ static int usb_eth_send(struct eth_device* netdev, volatile void* packet, int le #endif dev-tx_qlen=1; + ts = get_timer(0); + packet_sent=0; Please keep the style consistent and add spaces before/after =. retval = usb_ep_queue (dev-in_ep, req, GFP_ATOMIC); if (!retval) - dprintf(%s: packet queued\n,__func__); + debug(%s: packet queued\n,__func__); while(!packet_sent) { - packet_sent=0; + if (get_timer(ts) timeout) { + printf (timeout sending packets to usb ethernet\n); Please be consistent and remove space before paren. U-Boot nwo seems to follow the Linux coding style... WBR, Sergei ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Add mvsata support for the Sheevaplug devices
Le 14/08/2010 14:19, Sergei Shtylyov a écrit : +#define CONFIG_SYS_ATA_DATA_OFFSET (0x0100) +#define CONFIG_SYS_ATA_REG_OFFSET (0x0100) +#define CONFIG_SYS_ATA_ALT_OFFSET (0x0100) The parens are useless here. As this code comes from the mvsata_ide driver introduction, Sergei's comment also applies to edminiv2, openrd_base and the upcoming dns323 configs. However, the fix is not necessarily an urgent one; and I think it could be handled only once for all orion5x boards including edminv2 and dns323 if it is done within the SoC-specific configuration header file we've already discussed. The fix could even apply to kirkwood too, if there was a good way of defining a driver-specific configuration file and then have it included from SoC-specific config files for orion5x and kirkwood. Where could such a driver-specifc config file live? Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Make preparatory patches that initially have no effect?
Dear Reinhard Meyer, In message 4c665cb9.2040...@emk-elektronik.de you wrote: If I add those discrete changes to each driver patch (where it might actually belong), the incremental changes to some of those files would require all those driver patches to be applied in the right order to avoid conflicts. Yes, and? What's the problem with that? Therefore I would like to put all new header files and all changes to header files in one patch which would need to be applied before the driver patches. That patch would essentially cause no change to existing code. Anyone find this idea bad? Yes,m that's a bad idea. Please re-read the patches wiki page. Commits shall be atomic, and complete. Splitting stuff that belongstogether is a bad idea, and your first patch that adds unused stuff will be rejected because of that reason: adding unused stuff. 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 Perl already has an IDE. It's called Unix. -- Tom Christiansen in 375bd...@cs.colorado.edu ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] USB-CDC: correct wrong alignment in ether.c
Hi, 2010/8/14 Stefano Babic sba...@denx.de: The buffer for the status request must be word aligned because it is accessed with 32 bit pointer in the eth_status_complete function. Signed-off-by: Stefano Babic sba...@denx.de --- drivers/usb/gadget/ether.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) Applied to u-boot-usb/cdc Thanks. Remy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/2] USB-CDC: correct wrong alignment in ether.c
Hi, 2010/8/14 Sergei Shtylyov sshtyl...@mvista.com: Hello. Stefano Babic wrote: The buffer for the status request must be word aligned because it is accessed with 32 bit pointer in the eth_status_complete function. Signed-off-by: Stefano Babic sba...@denx.de [...] diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 313f15f..0b4ed18 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -684,7 +684,7 @@ static struct usb_gadget_strings stringtab = { /**/ static u8 control_req[USB_BUFSIZ]; -static u8 status_req[STATUS_BYTECOUNT]; +static u8 status_req[STATUS_BYTECOUNT] __attribute__ ((aligned(32))); You're aligning to 32 *bytes* -- is that what you meant by 32 bit pointer? Whoops... Good remark... Overlooked it myself... Unapplied it... Kind regards, Remy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 2/2] USB-CDC: called handle_interrupts inside usb_eth_send
Hi, 2010/8/14 Stefano Babic sba...@denx.de: The patch removes an endless loop in the usb_eth_send if the tx_complete is not called before going in the loop. The driver interrupt routine is called allowing the driver to check if the TX is completed. Signed-off-by: Stefano Babic sba...@denx.de --- Changes from V1: - Comments by Remy Bohmer: moved usb_ep_queue inside timeout to not break at91 code Sorry, patch does not apply any more after applying the series from Vitaly yesterday. please rebase. Kind regards, Remy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Make preparatory patches that initially have no effect?
Dear Wolfgang Denk, If I add those discrete changes to each driver patch (where it might actually belong), the incremental changes to some of those files would require all those driver patches to be applied in the right order to avoid conflicts. Yes, and? What's the problem with that? None for me. Only for people that want to try out a single (driver) patch. For example at91_gpbr.h is required by two independent patches. Of course, I could base both patches such that each one introduces that file. Yes,m that's a bad idea. Please re-read the patches wiki page. Commits shall be atomic, and complete. Splitting stuff that belongstogether is a bad idea, and your first patch that adds unused stuff will be rejected because of that reason: adding unused stuff. I know that, however it could be argued that adding header files to describe an architectures' hardware is not exactly specific to a driver. Thats why I asked Best Regards, Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Align linebuf to avoid misaligned aliases of it
On Sat, Aug 14, 2010 at 4:33 AM, Albert ARIBAUD wrote: Le 14/08/2010 10:25, Mike Frysinger a écrit : int print_buffer (ulong addr, void* data, uint width, uint count, uint linelen) { - uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1]; + uint8_t linebuf[MAX_LINE_LENGTH_BYTES + 1] + __attribute__((__aligned__(sizeof(uint32_t; __aligned(sizeof(uint32_t)) I based the __aligned__ on what's in post/cpu/ppc4xx/cache.c. What is the difference between __aligned and __aligned__? i dont mean __attribute__((__aligned(sizeof(uint32_t, i mean __aligned(sizeof(uint32_t)). the linux compiler headers take care of expanding it to an attribute. -mike ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Make preparatory patches that initially have no effect?
Dear Reinhard Meyer, In message 4c66cccf.9080...@emk-elektronik.de you wrote: None for me. Only for people that want to try out a single (driver) patch. They can apply the patch series anyway (at least to the patch they are interested in). For example at91_gpbr.h is required by two independent patches. Of course, I could base both patches such that each one introduces that file. Have the first add that file, and the second assume it comes later in the sequence. Yes,m that's a bad idea. Please re-read the patches wiki page. Commits shall be atomic, and complete. Splitting stuff that belongstogether is a bad idea, and your first patch that adds unused stuff will be rejected because of that reason: adding unused stuff. I know that, however it could be argued that adding header files to describe an architectures' hardware is not exactly specific to a driver. Thats why I asked The wiki page does not talk about drivers... It's a general rule and applies to all sorts of code. Only add what is really used (this also refers, for example, to struct definitions for register blocks etc. - don't try to provide a complete description of your SoC; add only stuff that is actually used by the code). Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de I like your game but we have to change the rules. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access (was:Make preparatory patches that initially have no effect?)
Dear Wolfgang Denk, Have the first add that file, and the second assume it comes later in the sequence. You don't mean by sequence PATCH 1/n, 2/n, etc? The drivers are so independent that that would not really make sense... The wiki page does not talk about drivers... It's a general rule and applies to all sorts of code. Only add what is really used (this also refers, for example, to struct definitions for register blocks etc. - don't try to provide a complete description of your SoC; add only stuff that is actually used by the code). That's a thin line. Although I need only one register of the DBU (for example) I think its wise to define all registers in it, and not to _reserve[] the unused ones Anyway, is the method of (for example!) #define DBU_ADDR 0xsomething (in a SoC header file) dbu_t *dbu = (dbu_t *)DBU_ADDR; (in a function) OK? Or do we need to further encapsulate that in a function like dbut_t *get_dbu_addr(void) {return (dbu_t *)DBU_ADDR;} I was even thinking of something like struct soc { u32 xyz[0x80]; /* XYZ unit */ u32 dbu[0x80]; /* Debug Unit */ u32 rstc[0x80]; /* Reset Controller */ and so on. Then in a driver one could write dbu_t *dbu = (dbu_t *)soc.dbu; or something along that line Best Regards Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access (was:Make preparatory patches that initially have no effect?)
On Sat, Aug 14, 2010 at 3:30 PM, Reinhard Meyer wrote: #define DBU_ADDR 0xsomething (in a SoC header file) dbu_t *dbu = (dbu_t *)DBU_ADDR; (in a function) needs to be volatile ... -mike ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
Dear Mike Frysinger, On Sat, Aug 14, 2010 at 3:30 PM, Reinhard Meyer wrote: #define DBU_ADDR 0xsomething (in a SoC header file) dbu_t *dbu = (dbu_t *)DBU_ADDR; (in a function) needs to be volatile ... -mike Why? The elements are used as parameters to readl/writel functions: status = readl(dbu-sr); I am quite sure we are NOT supposed to directly access hardware: status = dbu-sr; Best Regards, Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access (was:Make preparatory patches that initially have no effect?)
Dear Reinhard Meyer, In message 4c66eeca.5020...@emk-elektronik.de you wrote: Have the first add that file, and the second assume it comes later in the sequence. You don't mean by sequence PATCH 1/n, 2/n, etc? The drivers are so independent that that would not really make sense... Then just write in the comment part of the second patch that the other one has to be applied first... That's a thin line. Although I need only one register of the DBU (for example) I think its wise to define all registers in it, and not to _reserve[] the unused ones Right. If you add a function, add all the registers in it. But don't bother to explain each and every bit in the registers you never refer to, nor add completely unrelated blocks. Anyway, is the method of (for example!) #define DBU_ADDR 0xsomething (in a SoC header file) dbu_t *dbu = (dbu_t *)DBU_ADDR; (in a function) OK? Yes. Or do we need to further encapsulate that in a function like No. I was even thinking of something like struct soc { u32 xyz[0x80]; /* XYZ unit */ u32 dbu[0x80]; /* Debug Unit */ u32 rstc[0x80]; /* Reset Controller */ and so on. This is what PPC used to do; I like that - but ARM people always explained to me that it makes no sense because address space on ARM SoC is only sparely populated. Then in a driver one could write dbu_t *dbu = (dbu_t *)soc.dbu; or something along that line I think this looks nice, but as mentioned before - I'm not an ARM expert. They tend to do it differently. 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 Q: How many DEC repairman does it take to fix a flat ? A: Five; four to hold the car up and one to swap tires. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
Dear Wolfgang Denk, Then just write in the comment part of the second patch that the other one has to be applied first... Thats fine with me. Anyway, is the method of (for example!) #define DBU_ADDR 0xsomething (in a SoC header file) dbu_t *dbu = (dbu_t *)DBU_ADDR; (in a function) OK? Yes. Thats the way I do it currently Or do we need to further encapsulate that in a function like No. I have seen that somewhere in u-boot. I was even thinking of something like struct soc { u32 xyz[0x80]; /* XYZ unit */ u32 dbu[0x80]; /* Debug Unit */ u32 rstc[0x80]; /* Reset Controller */ and so on. This is what PPC used to do; I like that - but ARM people always explained to me that it makes no sense because address space on ARM SoC is only sparely populated. Even if, that's no reason, on can write u32 spi0[0x1000], on AT91 the spacing of peripherals is 0x4000 bytes, in fact it repeats times in its window. The system stuff is like one peripheral with its components spaced by 0x200 bytes (hence the 0x80 above). I think this looks nice, but as mentioned before - I'm not an ARM expert. They tend to do it differently. I can be done for AT91, but I'm not Don Quichote ;) Would the toolchain gulp when one defines the whole 4 GB that way? Best Regards, Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
Would the toolchain gulp when one defines the whole 4 GB that way? In fact, a rather novel approach (just theorizing here): #define SRAM_BASE offsetof(soc.sram) #define SRAM_SIZE sizeof(soc.sram) dbu_t *dbu = (dbu_t *)offsetof(soc.dbu); without ever assigning soc the address 0... Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
On Sat, Aug 14, 2010 at 3:40 PM, Reinhard Meyer wrote: Dear Mike Frysinger, On Sat, Aug 14, 2010 at 3:30 PM, Reinhard Meyer wrote: #define DBU_ADDR 0xsomething (in a SoC header file) dbu_t *dbu = (dbu_t *)DBU_ADDR; (in a function) needs to be volatile ... Why? The elements are used as parameters to readl/writel functions: status = readl(dbu-sr); if you use accessor functions and those guarantee volatility, then you dont need the markings on the pointer I am quite sure we are NOT supposed to directly access hardware: status = dbu-sr; it depends on the hardware -mike ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
Dear Reinhard Meyer, In message 4c66f54d.2060...@emk-elektronik.de you wrote: I was even thinking of something like struct soc { u32 xyz[0x80]; /* XYZ unit */ u32 dbu[0x80]; /* Debug Unit */ u32 rstc[0x80]; /* Reset Controller */ and so on. This is what PPC used to do; I like that - but ARM people always explained to me that it makes no sense because address space on ARM SoC is only sparely populated. Even if, that's no reason, on can write u32 spi0[0x1000], on AT91 the spacing of peripherals is 0x4000 bytes, in fact it repeats times in its window. The system stuff is like one peripheral with its components spaced by 0x200 bytes (hence the 0x80 above). I think we have a misunderstaning ehre - I thought the entries like xyz were indeed u32 types - buut now I get the impression that what you have in mind is that they are actually structs describing hardware blocks. Then it should be written like that. For example, see file arch/powerpc/include/asm/8xx_immap.h: struct immap is what corresponds to your struct soc above. Would the toolchain gulp when one defines the whole 4 GB that way? Why not? 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 It [being a Vulcan] means to adopt a philosophy, a way of life which is logical and beneficial. We cannot disregard that philosophy merely for personal gain, no matter how important that gain might be. -- Spock, Journey to Babel, stardate 3842.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
Dear Reinhard Meyer, In message 4c6700e5.2070...@emk-elektronik.de you wrote: Would the toolchain gulp when one defines the whole 4 GB that way? In fact, a rather novel approach (just theorizing here): #define SRAM_BASE offsetof(soc.sram) #define SRAM_SIZE sizeof(soc.sram) dbu_t *dbu = (dbu_t *)offsetof(soc.dbu); without ever assigning soc the address 0... Urgh... that's a log of pretty heavy assumptions, and not exactly readable / understandable code either. I gues your're not targeting the next IOCCC? 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 Die Scheu vor Verantwortung ist die Krankheit unserer Zeit. -- Otto von Bismarck ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
Dear Wolfgang Denk, I think we have a misunderstaning ehre - I thought the entries like xyz were indeed u32 types - buut now I get the impression that what you have in mind is that they are actually structs describing hardware blocks. No, thats just spacemakers, otherwise one would have do declare all structs for all hardware blocks in that file (or include a bunch of files). struct immap is what corresponds to your struct soc above. Yes, with at least all currently used blocks declared as structs, the currently unused ones made up of the proper amount of u32s Since different AT91 SoC might have some blocks missing or at other addresses in another amount etc., but with the same layout of each individual block, it would make sense to keep the block structure definitions in separate files like at91_dbu.h, at91_rstc.h, etc. They would have to be included in the corresponding at91sam9260.h, at91sam9261.h, etc. where the individual soc layouts would be defined. Would the toolchain gulp when one defines the whole 4 GB that way? Why not? Since the AT91s have no base address registers at all, the memory layout is completely fixed. Even chip selects have a fixed address and fixed size of 256MB each. Therefore it _might_ make sense to completely define the 4GB in the soc struct. Then assign struct soc *soc = (struct soc *)0; Did you read Mike's comment of hardware dependent direct usage of struct members to access hardware? I thought that was generally discouraged for several reasons (cache and access sequencing)? Whats a IOCCC? Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 1/4] tools: enable img2srec for all target
Signed-off-by: Mike Frysinger vap...@gentoo.org --- tools/Makefile |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/tools/Makefile b/tools/Makefile index 749d994..ea271bf 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -41,6 +41,7 @@ include $(TOPDIR)/config.mk # Enable all the config-independent tools ifneq ($(HOST_TOOLS_ALL),) CONFIG_LCD_LOGO = y +CONFIG_CMD_LOADS = y CONFIG_CMD_NET = y CONFIG_INCA_IP = y CONFIG_NETCONSOLE = y -- 1.7.2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 4/4] tools/env: use host build flags
Convert the tools/env/Makefile to use the same host tool syntax as the other tool subdirs. Signed-off-by: Mike Frysinger vap...@gentoo.org --- tools/env/Makefile |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/env/Makefile b/tools/env/Makefile index 2df631e..f893040 100644 --- a/tools/env/Makefile +++ b/tools/env/Makefile @@ -26,16 +26,16 @@ include $(TOPDIR)/config.mk SRCS := $(obj)crc32.c fw_env.c fw_env_main.c HEADERS:= fw_env.h -CPPFLAGS := -Wall -DUSE_HOSTCC -I$(SRCTREE)/include +HOSTCFLAGS += -Wall -DUSE_HOSTCC -I$(SRCTREE)/include ifeq ($(MTD_VERSION),old) -CPPFLAGS += -DMTD_OLD +HOSTCFLAGS += -DMTD_OLD endif all: $(obj)fw_printenv $(obj)fw_printenv: $(SRCS) $(HEADERS) - $(CROSS_COMPILE)gcc $(CPPFLAGS) $(SRCS) -o $(obj)fw_printenv + $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $(SRCS) clean: rm -f $(obj)fw_printenv $(obj)crc32.c -- 1.7.2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/4] tools: update .gitignore
Signed-off-by: Mike Frysinger vap...@gentoo.org --- tools/.gitignore |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/tools/.gitignore b/tools/.gitignore index cb067a4..07f21a3 100644 --- a/tools/.gitignore +++ b/tools/.gitignore @@ -9,3 +9,8 @@ /ubsha1 /inca-swap-bytes /*.exe +/easylogo/easylogo +/env/crc32.c +/env/fw_printenv +/gdb/gdbcont +/gdb/gdbsend -- 1.7.2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot