Loading kernel from SPI NOR
Hi, I have a custom board based on the i.MX53. We are using barebox 2012.05.0 and booting the Freescale 2.6.35.3 kernel. Our board has SPI NOR (m25p32) and I saw there is a driver in a more recent barebox. I pulled 2013.01.0 and added our board files but I can't seem to get the kernel to load. I used the uimage tool and the -i option reported back that it can read the kernel and the -v option reported ok. When I try and boot I see arch_number and then nothing. I'm continuing to dig in to see what I can find. Any pointers to where I can look is much appreciated. Thanks, Jeff ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 1/1] macb: fix gem_recv circular buffer handling
On Wed, Mar 13, 2013 at 04:39:40PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: > as we use a full buffer no need to check the SOF > and reset the rx_tail > > fix at the same time the gem detection so we can have the rx_buffer > allocated correctly according to the IP > > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD > --- > drivers/net/macb.c | 32 > 1 file changed, 16 insertions(+), 16 deletions(-) > > diff --git a/drivers/net/macb.c b/drivers/net/macb.c > index 0cfad05..2d4e373 100644 > --- a/drivers/net/macb.c > +++ b/drivers/net/macb.c > @@ -172,7 +172,6 @@ static void reclaim_rx_buffers(struct macb_device *macb, > static int gem_recv(struct eth_device *edev) > { > struct macb_device *macb = edev->priv; > - unsigned int rx_tail = macb->rx_tail; > void *buffer; > int length; > u32 status; > @@ -181,20 +180,20 @@ static int gem_recv(struct eth_device *edev) > > for (;;) { > barrier(); > - if (!(macb->rx_ring[rx_tail].addr & MACB_BIT(RX_USED))) > + if (!(macb->rx_ring[macb->rx_tail].addr & MACB_BIT(RX_USED))) > return -1; > > barrier(); > - status = macb->rx_ring[rx_tail].ctrl; > + status = macb->rx_ring[macb->rx_tail].ctrl; > length = MACB_BFEXT(RX_FRMLEN, status); > - if (status & MACB_BIT(RX_SOF)) { > - buffer = macb->rx_buffer + macb->rx_buffer_size * > macb->rx_tail; > - net_receive(buffer, length); > - macb->rx_ring[rx_tail].ctrl &= ~MACB_BIT(RX_USED); > - barrier(); > - } > - rx_tail++; > + buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail; > + net_receive(buffer, length); > + macb->rx_ring[macb->rx_tail].addr &= ~MACB_BIT(RX_USED); > + barrier(); > + > macb->rx_tail++; > + if (macb->rx_tail >= macb->rx_ring_size) > + macb->rx_tail = 0; > } > > return 0; > @@ -619,11 +618,6 @@ static int macb_probe(struct device_d *dev) > > macb->phy_flags = pdata->phy_flags; > > - macb_init_rx_buffer_size(macb, PKTSIZE); > - macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * > macb->rx_ring_size); > - macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); > - macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); > - > macb->regs = dev_request_mem_region(dev, 0); > > /* > @@ -638,11 +632,17 @@ static int macb_probe(struct device_d *dev) > > clk_enable(macb->pclk); > > + macb->is_gem = read_is_gem(macb); > + > if (macb_is_gem(macb)) > edev->recv = gem_recv; > else > edev->recv = macb_recv; > - macb->is_gem = read_is_gem(macb); > + > + macb_init_rx_buffer_size(macb, PKTSIZE); > + macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * > macb->rx_ring_size); > + macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); > + macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); > > macb_reset_hw(macb); > ncfgr = macb_mdc_clk_div(macb); I know that the last part is not a very sophisticated patch, but you could at least mention that I found the error and/or made part of this patch. Don't just pretend that you came up with this all alone. Regards, Steffen -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 3/7] globalvar: allow to register multiple device
This will allow to reduce the time spend to search for globalvar (boot time) the access to the globalvar is retro-compatible we can now create device via the global command barebox@Somfy Animeo IP:/ # devinfo devices: ` global ` net ` bootm ` dhcp ... barebox@Somfy Animeo IP:/ resources: driver: none bus: global bus: none Parameters: hostname = barebox@Somfy Animeo IP:/ # devinfo net resources: driver: none bus: global Parameters: nameserver = domainname = barebox@Somfy Animeo IP:/ # devinfo dhcp resources: driver: none bus: global Parameters: rootpath = tftp_server_name = bootfile = oftree_file = vendor_id = barebox-animeo-ip client_id = user_class = client_uuid = barebox@Somfy Animeo IP:/ # devinfo bootm resources: driver: none bus: global Parameters: image = oftree = initrd = Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD --- commands/global.c | 41 common/globalvar.c | 133 +-- include/globalvar.h | 26 ++ 3 files changed, 178 insertions(+), 22 deletions(-) diff --git a/commands/global.c b/commands/global.c index 427a231..539fa21 100644 --- a/commands/global.c +++ b/commands/global.c @@ -23,14 +23,23 @@ #include #include -static int globalvar_set(char* name, char* value) +static int globalvar_set(char *devname, char* name, char* value) { int ret; - ret = globalvar_add_simple(name); + if (devname) { + struct device_d *dev; + + dev = get_device_by_name(devname); + if (!dev) + dev = global_add_device(devname); + ret = global_add_simple(dev, name); + } else { + ret = globalvar_add_simple(name); + } if (value) { - char *tmp = asprintf("global.%s", name); + char *tmp = asprintf("%s.%s", devname ? devname : "global", name); ret = setenv(tmp, value); free(tmp); } @@ -43,12 +52,16 @@ static int do_global(int argc, char *argv[]) int opt; int do_set_match = 0; char *value; + char *devname = NULL; - while ((opt = getopt(argc, argv, "r")) > 0) { + while ((opt = getopt(argc, argv, "rd:")) > 0) { switch (opt) { case 'r': do_set_match = 1; break; + case 'd': + devname = optarg; + break; } } @@ -68,17 +81,29 @@ static int do_global(int argc, char *argv[]) if (!value) value = ""; - globalvar_set_match(argv[0], value); + if (devname) { + struct device_d *dev; + + dev = get_device_by_name(devname); + + if (!dev) + return -EINVAL; + + global_set_match(dev, argv[0], value); + } else { + globalvar_set_match(argv[0], value); + } return 0; } - return globalvar_set(argv[0], value); + return globalvar_set(devname, argv[0], value); } BAREBOX_CMD_HELP_START(global) -BAREBOX_CMD_HELP_USAGE("global [-r] [=[=, optionally set to \n") -BAREBOX_CMD_HELP_SHORT("-r to set a value to of all globalvars beginning with 'match'") +BAREBOX_CMD_HELP_SHORT("-r to set a value to of all globalvars beginning with 'match'\n") +BAREBOX_CMD_HELP_SHORT("-d use a specific global device if do not exist create (if -r not set), it if not set use 'global'") BAREBOX_CMD_HELP_END BAREBOX_CMD_START(global) diff --git a/common/globalvar.c b/common/globalvar.c index f275a38..5bfab70 100644 --- a/common/globalvar.c +++ b/common/globalvar.c @@ -14,19 +14,19 @@ int globalvar_add(const char *name, } /* - * globalvar_get_match + * global_get_match * - * get a concatenated string of all globalvars beginning with 'match'. - * This adds whitespaces between the different globalvars + * get a concatenated string of all global vars beginning with 'match'. + * This adds whitespaces between the different global vars */ -char *globalvar_get_match(const char *match, const char *seperator) +char *global_get_match(struct device_d *dev, const char *match, const char *seperator) { char *val = NULL; struct param_d *param; - list_for_each_entry(param, global_device.parameters, list) { + list_for_each_entry(param, &dev->parameters, list) { if (!strncmp(match, param->name, strlen(match))) { - const char *p = dev_get_param(global_device, param->name); + const char *p = dev_get_param(dev, param->name); if
[PATCH 1/7] params: allow to access first sub-device and params via env
this will allow to keep retro-compatiblility and adding globalvar multiple device support barebox@Somfy Animeo IP:/ # echo $platform.soc. at91sam9260 barebox@Somfy Animeo IP:/ # echo $platform.soc.name Unknown barebox@Somfy Animeo IP:/ # echo $global.dhcp.vendor_id barebox-animeo-ip update complete too Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD --- common/complete.c | 45 - lib/parameter.c | 41 - 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/common/complete.c b/common/complete.c index 9206ef0..17ee3c4 100644 --- a/common/complete.c +++ b/common/complete.c @@ -169,7 +169,7 @@ int device_complete(struct string_list *sl, char *instr) } EXPORT_SYMBOL(device_complete); -static int device_param_complete(char *begin, struct device_d *dev, +static int device_param_complete(char *begin, char *dev_fullname, struct device_d *dev, struct string_list *sl, char *instr) { struct param_d *param; @@ -185,7 +185,7 @@ static int device_param_complete(char *begin, struct device_d *dev, continue; string_list_add_asprintf(sl, "%s%s.%s%c", - begin ? begin : "", dev_name(dev), param->name, + begin ? begin : "", dev_fullname, param->name, begin ? ' ' : '='); } @@ -253,12 +253,47 @@ static int env_param_complete(struct string_list *sl, char *instr, int eval) } for_each_device(dev) { - if (!strncmp(instr, dev_name(dev), len)) { + struct device_d *child; + char *devname = asprintf("%s", dev_name(dev)); + + if (!strncmp(instr, devname, len)) { if (eval) - device_param_complete("$", dev, sl, instr_param); + device_param_complete("$", devname, dev, sl, instr_param); else - device_param_complete(NULL, dev, sl, instr_param); + device_param_complete(NULL, devname, dev, sl, instr_param); + } + + if (dev->bus) { + free(devname); + continue; + } + + device_for_each_child(dev, child) { + char *dev_fullname; + char *child_instr_param; + int child_len; + + dev_fullname = asprintf("%s.%s", devname, dev_name(child)); + child_instr_param = strchr(instr_param, '.'); + + if (child_instr_param) { + child_len = (child_instr_param - instr); + child_instr_param++; + } else { + child_len = strlen(instr); + child_instr_param = ""; + } + + if (!strncmp(instr, dev_fullname, child_len)) { + if (eval) + device_param_complete("$", dev_fullname, child, sl, child_instr_param); + else + device_param_complete(NULL, dev_fullname, child, sl, child_instr_param); + } + + free(dev_fullname); } + free(devname); } return 0; diff --git a/lib/parameter.c b/lib/parameter.c index c00b824..4b7b26a 100644 --- a/lib/parameter.c +++ b/lib/parameter.c @@ -28,7 +28,7 @@ #include #include -struct param_d *get_param_by_name(struct device_d *dev, const char *name) +static struct param_d *__get_param_by_name(struct device_d *dev, const char *name) { struct param_d *p; @@ -40,6 +40,45 @@ struct param_d *get_param_by_name(struct device_d *dev, const char *name) return NULL; } +static struct param_d *get_child_param_by_name(struct device_d *dev, const char *name) +{ + struct device_d *child; + char *devstr; + char *par; + + if (dev->bus) + return NULL; + + if (!strchr(name, '.')) + return NULL; + + devstr = strdup(name); + par = strchr(devstr, '.'); + + *par = 0; + par++; + + device_for_each_child(dev, child) { + if (!strcmp(devstr, dev_name(child))) + return __get_param_by_name(child, par); + } + + free(devstr); + + return NULL; +} + +struct param_d *get_param_by_name(struct device_d *dev, const char *name) +{ + struct param_d *param; + + param = get_child_param_by_name(dev, name); + if (param) + return param; + + return __get_param_by_name(dev, name); +} + /** * dev_get_param - get the value of a parameter
[PATCH 2/7] globalvar: add it's own bus
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD --- common/globalvar.c | 31 --- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/common/globalvar.c b/common/globalvar.c index a8aaa72..f275a38 100644 --- a/common/globalvar.c +++ b/common/globalvar.c @@ -3,17 +3,14 @@ #include #include -static struct device_d global_device = { - .name = "global", - .id = DEVICE_ID_SINGLE, -}; +static struct device_d *global_device; int globalvar_add(const char *name, int (*set)(struct device_d *dev, struct param_d *p, const char *val), const char *(*get)(struct device_d *, struct param_d *p), unsigned long flags) { - return dev_add_param(&global_device, name, set, get, flags); + return dev_add_param(global_device, name, set, get, flags); } /* @@ -27,9 +24,9 @@ char *globalvar_get_match(const char *match, const char *seperator) char *val = NULL; struct param_d *param; - list_for_each_entry(param, &global_device.parameters, list) { + list_for_each_entry(param, global_device.parameters, list) { if (!strncmp(match, param->name, strlen(match))) { - const char *p = dev_get_param(&global_device, param->name); + const char *p = dev_get_param(global_device, param->name); if (val) { char *new = asprintf("%s%s%s", val, seperator, p); free(val); @@ -50,9 +47,9 @@ void globalvar_set_match(const char *match, const char *val) { struct param_d *param; - list_for_each_entry(param, &global_device.parameters, list) { + list_for_each_entry(param, global_device.parameters, list) { if (!strncmp(match, param->name, strlen(match))) - dev_set_param(&global_device, param->name, val); + dev_set_param(global_device, param->name, val); } } @@ -66,10 +63,22 @@ int globalvar_add_simple(const char *name) return globalvar_add(name, NULL, NULL, 0); } +static int global_match(struct device_d *dev, struct driver_d *drv) +{ + return 1; +} + +static struct bus_type global_bus = { + .name = "global", + .match = global_match, + .probe = dummy_probe, +}; + static int globalvar_init(void) { - register_device(&global_device); + bus_register(&global_bus); + global_device = &global_bus->dev; return 0; } -postconsole_initcall(globalvar_init); +pure_initcall(globalvar_init); -- 1.7.10.4 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 7/7] bootargs: switch globalvar to it's own device
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD --- common/bootargs.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/common/bootargs.c b/common/bootargs.c index 6624f72..e9248ab 100644 --- a/common/bootargs.c +++ b/common/bootargs.c @@ -22,9 +22,11 @@ #include #include #include +#include static char *linux_bootargs; static int linux_bootargs_overwritten; +static struct device_d *global_linux_dev; /* * This returns the Linux bootargs @@ -45,11 +47,11 @@ const char *linux_bootargs_get(void) free(linux_bootargs); - bootargs = globalvar_get_match("linux.bootargs.", " "); + bootargs = global_get_match(global_linux_dev, "bootargs.", " "); if (!strlen(bootargs)) return getenv("bootargs"); - mtdparts = globalvar_get_match("linux.mtdparts.", ";"); + mtdparts = global_get_match(global_linux_dev, "mtdparts.", ";"); if (strlen(mtdparts)) { linux_bootargs = asprintf("%s mtdparts=%s", bootargs, mtdparts); @@ -76,5 +78,13 @@ int linux_bootargs_overwrite(const char *bootargs) return 0; } +static int bootargs_init(void) +{ + global_linux_dev = global_add_device("linux"); + + return 0; +} +late_initcall(bootargs_init); + BAREBOX_MAGICVAR_NAMED(global_linux_bootargs_, global.linux.bootargs.*, "Linux bootargs variables"); BAREBOX_MAGICVAR_NAMED(global_linux_mtdparts_, global.linux.mtdparts.*, "Linux mtdparts variables"); -- 1.7.10.4 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 4/7] net: switch to global device
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD --- net/net.c | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/net/net.c b/net/net.c index 639bc2d..4888d60 100644 --- a/net/net.c +++ b/net/net.c @@ -35,6 +35,7 @@ #include #include #include +#include static IPaddr_tnet_netmask;/* Our subnet mask (0=unknown) */ static IPaddr_tnet_gateway;/* Our gateways IP address */ @@ -664,21 +665,19 @@ out: return ret; } -static struct device_d net_device = { - .name = "net", - .id = DEVICE_ID_SINGLE, -}; - static int net_init(void) { + struct device_d *dev; int i; for (i = 0; i < PKTBUFSRX; i++) NetRxPackets[i] = net_alloc_packet(); - register_device(&net_device); - dev_add_param(&net_device, "nameserver", NULL, NULL, 0); - dev_add_param(&net_device, "domainname", NULL, NULL, 0); + dev = global_add_device("net"); + if (dev) { + global_add_simple(dev, "nameserver"); + global_add_simple(dev, "domainname"); + } return 0; } -- 1.7.10.4 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 5/7] dhcp: switch globalvar to it's own device
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD --- net/dhcp.c | 40 1 file changed, 8 insertions(+), 32 deletions(-) diff --git a/net/dhcp.c b/net/dhcp.c index 8233ec3..7fde0d8 100644 --- a/net/dhcp.c +++ b/net/dhcp.c @@ -81,31 +81,16 @@ static uint32_t dhcp_leasetime; static IPaddr_t net_dhcp_server_ip; static uint64_t dhcp_start; static char dhcp_tftpname[256]; +static struct device_d *dhcp_dev; static const char* dhcp_get_barebox_global(const char * var) { - char * var_global = asprintf("global.dhcp.%s", var); - const char *val; - - if (!var_global) - return NULL; - - val = getenv(var_global); - free(var_global); - return val; + return dev_get_param(dhcp_dev, var); } static int dhcp_set_barebox_global(const char * var, char *val) { - char * var_global = asprintf("global.dhcp.%s", var); - int ret; - - if (!var_global) - return -ENOMEM; - - ret = setenv(var_global, val); - free(var_global); - return ret; + return dev_set_param(dhcp_dev, var, val); } struct dhcp_opt { @@ -663,30 +648,21 @@ static void dhcp_reset_env(void) } } -static void dhcp_global_add(const char *var) -{ - char * var_global = asprintf("dhcp.%s", var); - - if (!var_global) - return; - - globalvar_add_simple(var_global); - free(var_global); -} - static int dhcp_global_init(void) { struct dhcp_opt *opt; struct dhcp_param *param; int i; + dhcp_dev = global_add_device("dhcp"); + for (i = 0; i < ARRAY_SIZE(dhcp_options); i++) { opt = &dhcp_options[i]; if (!opt->barebox_dhcp_global) continue; - dhcp_global_add(opt->barebox_dhcp_global); + global_add_simple(dhcp_dev, opt->barebox_dhcp_global); } for (i = 0; i < ARRAY_SIZE(dhcp_params); i++) { @@ -695,7 +671,7 @@ static int dhcp_global_init(void) if (!param->barebox_dhcp_global) continue; - dhcp_global_add(param->barebox_dhcp_global); + global_add_simple(dhcp_dev, param->barebox_dhcp_global); } return 0; @@ -719,7 +695,7 @@ static int do_dhcp(int argc, char *argv[]) dhcp_reset_env(); - dhcp_getenv_int("global.dhcp.retries", &retries); + dhcp_getenv_int("dhcp.retries", &retries); while((opt = getopt(argc, argv, "H:v:c:u:U:r:")) > 0) { switch(opt) { -- 1.7.10.4 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 6/7] bootm: switch globalvar to it's own device
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD --- commands/bootm.c | 20 +--- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/commands/bootm.c b/commands/bootm.c index 4d3f022..b70cd05 100644 --- a/commands/bootm.c +++ b/commands/bootm.c @@ -48,6 +48,12 @@ #include static LIST_HEAD(handler_list); +static struct device_d *bootm_dev; + +static const char* bootm_get_global(const char * var) +{ + return dev_get_param(bootm_dev, var); +} /* * Additional oftree size for the fixed tree @@ -265,10 +271,10 @@ static int do_bootm(int argc, char *argv[]) data.verify = 0; data.verbose = 0; - oftree = getenv("global.bootm.oftree"); - os_file = getenv("global.bootm.image"); + oftree = bootm_get_global("oftree"); + os_file = bootm_get_global("image"); if (IS_ENABLED(CONFIG_CMD_BOOTM_INITRD)) - initrd_file = getenv("global.bootm.initrd"); + initrd_file = bootm_get_global("initrd"); while ((opt = getopt(argc, argv, BOOTM_OPTS)) > 0) { switch(opt) { @@ -443,11 +449,11 @@ err_out: static int bootm_init(void) { - - globalvar_add_simple("bootm.image"); - globalvar_add_simple("bootm.oftree"); + bootm_dev = global_add_device("bootm"); + global_add_simple(bootm_dev, "image"); + global_add_simple(bootm_dev, "oftree"); if (IS_ENABLED(CONFIG_CMD_BOOTM_INITRD)) - globalvar_add_simple("bootm.initrd"); + global_add_simple(bootm_dev, "initrd"); return 0; } -- 1.7.10.4 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 0/7 v2] globalvar: add multiple device support
HI, v2: fix bootargs this will allow to reduce the scope of device and params to search during boot time we keep the retro compatibility The following changes since commit 092bfd5eb55d1b2d7ed098aa9723a2fa63b86192: fix another brown paper bag bug introduced with compile time loglevel (2013-03-06 23:53:04 +0100) are available in the git repository at: git://git.jcrosoft.org/barebox.git delivery/globalvar for you to fetch changes up to 2a749c4b65f823da50537798a2feeaf4a01d2212: bootargs: switch globalvar to it's own device (2013-03-13 14:35:29 +0800) Jean-Christophe PLAGNIOL-VILLARD (7): params: allow to access first sub-device and params via env globalvar: add it's own bus globalvar: allow to register multiple device net: switch to global device dhcp: switch globalvar to it's own device bootm: switch globalvar to it's own device bootargs: switch globalvar to it's own device commands/bootm.c| 20 +--- commands/global.c | 41 + common/bootargs.c | 14 -- common/complete.c | 45 - common/globalvar.c | 150 -- include/globalvar.h | 26 ++ lib/parameter.c | 41 - net/dhcp.c | 40 net/net.c | 15 +++ 9 files changed, 311 insertions(+), 81 deletions(-) Best Regards, J. ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 1/2] net: macb: remove gem_recv and reorder probe
On 14:08 Wed 13 Mar , Steffen Trumtrar wrote: > On Wed, Mar 13, 2013 at 01:17:03PM +0100, Jean-Christophe PLAGNIOL-VILLARD > wrote: > > On 11:15 Wed 13 Mar , Sascha Hauer wrote: > > > On Wed, Mar 13, 2013 at 10:03:36AM +0100, Jean-Christophe > > > PLAGNIOL-VILLARD wrote: > > > > On 09:48 Wed 13 Mar , Steffen Trumtrar wrote: > > > > > The function gem_recv implements a buffer "ring" that stops at > > > > > filling level 10. > > > > > That means, when the driver is used as gem, it stops receiving after > > > > > exactly > > > > > 10 packets. Instead of implementing macb_recv twice, use it also for > > > > > the gem > > > > > part. If there needs to be an extra recv function for the gigabit > > > > > case, it can > > > > > be added than. > > > > > Also, first set the type of device (macb or gem) and then use > > > > > functions that > > > > > use this info. > > > > > > > > > > Tested on a Zynq7000. > > > > NACK > > > > > > > > on gem we can receive the packet in one buffer the gem_recv implement > > > > this > > > > the macb can only receive splited buffer and then you have to > > > > reconstruct the > > > > packet > > > > > > The gem received function was never used... > > > > > > > > static int macb_recv(struct eth_device *edev) > > > > > { > > > > > struct macb_device *macb = edev->priv; > > > > > @@ -619,11 +588,6 @@ static int macb_probe(struct device_d *dev) > > > > > > > > > > macb->phy_flags = pdata->phy_flags; > > > > > > > > > > - macb_init_rx_buffer_size(macb, PKTSIZE); > > > > > - macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * > > > > > macb->rx_ring_size); > > > > > - macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); > > > > > - macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); > > > > > - > > > > > macb->regs = dev_request_mem_region(dev, 0); > > > > > > > > > > /* > > > > > @@ -638,12 +602,15 @@ static int macb_probe(struct device_d *dev) > > > > > > > > > > clk_enable(macb->pclk); > > > > > > > > > > - if (macb_is_gem(macb)) > > > > > > ... because macb_is_gem() is used here ... > > > > > > > > - edev->recv = gem_recv; > > > > > - else > > > > > - edev->recv = macb_recv; > > > > > macb->is_gem = read_is_gem(macb); > > > > > > ... but the variable is initialized here. Up to this point macb_is_gem() > > > will return 0. > > so this is a bug when the driver was merge to the mailine but here I does > > use > > the gem_recv. > > > > What about the gem_recv function itself? It only increases rx_tail, but never > resets it. Therefore the buffer is full after 10 packets. Do you already have > a > patch for that? yes I check I've the diff with my local tree I send it on the top of your patch Best Regards, J. > > Regards, > Steffen > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 1/1] macb: fix gem_recv circular buffer handling
as we use a full buffer no need to check the SOF and reset the rx_tail fix at the same time the gem detection so we can have the rx_buffer allocated correctly according to the IP Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD --- drivers/net/macb.c | 32 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 0cfad05..2d4e373 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -172,7 +172,6 @@ static void reclaim_rx_buffers(struct macb_device *macb, static int gem_recv(struct eth_device *edev) { struct macb_device *macb = edev->priv; - unsigned int rx_tail = macb->rx_tail; void *buffer; int length; u32 status; @@ -181,20 +180,20 @@ static int gem_recv(struct eth_device *edev) for (;;) { barrier(); - if (!(macb->rx_ring[rx_tail].addr & MACB_BIT(RX_USED))) + if (!(macb->rx_ring[macb->rx_tail].addr & MACB_BIT(RX_USED))) return -1; barrier(); - status = macb->rx_ring[rx_tail].ctrl; + status = macb->rx_ring[macb->rx_tail].ctrl; length = MACB_BFEXT(RX_FRMLEN, status); - if (status & MACB_BIT(RX_SOF)) { - buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail; - net_receive(buffer, length); - macb->rx_ring[rx_tail].ctrl &= ~MACB_BIT(RX_USED); - barrier(); - } - rx_tail++; + buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail; + net_receive(buffer, length); + macb->rx_ring[macb->rx_tail].addr &= ~MACB_BIT(RX_USED); + barrier(); + macb->rx_tail++; + if (macb->rx_tail >= macb->rx_ring_size) + macb->rx_tail = 0; } return 0; @@ -619,11 +618,6 @@ static int macb_probe(struct device_d *dev) macb->phy_flags = pdata->phy_flags; - macb_init_rx_buffer_size(macb, PKTSIZE); - macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * macb->rx_ring_size); - macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); - macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); - macb->regs = dev_request_mem_region(dev, 0); /* @@ -638,11 +632,17 @@ static int macb_probe(struct device_d *dev) clk_enable(macb->pclk); + macb->is_gem = read_is_gem(macb); + if (macb_is_gem(macb)) edev->recv = gem_recv; else edev->recv = macb_recv; - macb->is_gem = read_is_gem(macb); + + macb_init_rx_buffer_size(macb, PKTSIZE); + macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * macb->rx_ring_size); + macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); + macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); macb_reset_hw(macb); ncfgr = macb_mdc_clk_div(macb); -- 1.7.10.4 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH] net: macb: initialize is_gem before usage
On 11:58 Wed 13 Mar , Steffen Trumtrar wrote: > The variable macb->is_gem is evaluated before it is initialized. > That leads to a wrong rx_buffer setup in the gem case. Also, the > function gem_recv will never be used. > > Set the variable first and then use it. > > Signed-off-by: Steffen Trumtrar > --- > drivers/net/macb.c | 13 +++-- > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/macb.c b/drivers/net/macb.c > index 0cfad05..30e8476 100644 > --- a/drivers/net/macb.c > +++ b/drivers/net/macb.c > @@ -619,11 +619,6 @@ static int macb_probe(struct device_d *dev) > > macb->phy_flags = pdata->phy_flags; > > - macb_init_rx_buffer_size(macb, PKTSIZE); > - macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * > macb->rx_ring_size); > - macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); > - macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); > - > macb->regs = dev_request_mem_region(dev, 0); > > /* > @@ -638,11 +633,17 @@ static int macb_probe(struct device_d *dev) > > clk_enable(macb->pclk); > > + macb->is_gem = read_is_gem(macb); > + > + macb_init_rx_buffer_size(macb, PKTSIZE); > + macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * > macb->rx_ring_size); > + macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); > + macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); > + I check you need to update the gem_recv at the same time otherwise this brake the sama5 support Best Regards, J. > if (macb_is_gem(macb)) > edev->recv = gem_recv; > else > edev->recv = macb_recv; > - macb->is_gem = read_is_gem(macb); > > macb_reset_hw(macb); > ncfgr = macb_mdc_clk_div(macb); > -- > 1.8.2.rc2 > > > ___ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 5/5] dfu: usb reset state workaround
When an USB_REQ_DFU_DETACH request is received, the device state switch to DFU_STATE_appDETACH, and then wait for an usb reset to switch to DFU_STATE_dfuIDLE (through dfu_disable() being called). I noticed that using dfu-util v0.7 on my AT91SAM9260 board, the programming failed because of the device not entering the DFU_STATE_dfuIDLE after an usb reset (staying in the DFU_STATE_appDETACH state). According to the current implementation, once the USB_REQ_DFU_DETACH is sent and right after the usb reset, if the DFU client set more than one configuration (by iterating over them for example), the device state will reset to the DFU_STATE_appDETACH state on the 2nd iteration because of the following line in set_config (composite.c, line 362) : ... if(cdev->config) reset_config(); // will call dfu_disable() again! ... The attached patch is a *try* to fix the issue by leaving the device in the DFU_STATE_dfuIDLE state after an usb reset if already in that state. dfu-util will complain about the device already in the DFU_STATE_dfuIDLE on the next start, but everything is still working and thus usable. This may need a REVISIT, but since the dfu_disable() gets called on both configuration change and usb reset, we can't fix this easily for now.. Signed-off-by: Cerrato Renaud --- drivers/usb/gadget/dfu.c |6 ++ 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c index e051879..e4af567 100644 --- a/drivers/usb/gadget/dfu.c +++ b/drivers/usb/gadget/dfu.c @@ -514,14 +514,12 @@ static void dfu_disable(struct usb_function *f) struct f_dfu*dfu = func_to_dfu(f); switch (dfu->dfu_state) { +case DFU_STATE_dfuIDLE: case DFU_STATE_appDETACH: dfu->dfu_state = DFU_STATE_dfuIDLE; break; -case DFU_STATE_dfuMANIFEST_WAIT_RST: -dfu->dfu_state = DFU_STATE_appIDLE; -break; default: -dfu->dfu_state = DFU_STATE_appDETACH; +dfu->dfu_state = DFU_STATE_appIDLE; break; } -- 1.7.2.5 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 4/5] gadget: possible null pointer dereference fix
This patch fix a possible null pointer dereference exception because of a missing null check on cdev->config Signed-off-by: Cerrato Renaud --- drivers/usb/gadget/composite.c | 23 +-- 1 files changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index 9af115e..1f6c5b2 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -777,18 +777,21 @@ unknown: * recipients (endpoint, other, WUSB, ...) to the current * configuration code. */ -f = cdev->config->interface[intf]; -if (f && f->setup) -value = f->setup(f, ctrl); -else -f = NULL; +if(cdev->config) { -if (value < 0 && !f) { -struct usb_configuration*c; +f = cdev->config->interface[intf]; +if (f && f->setup) +value = f->setup(f, ctrl); +else +f = NULL; -c = cdev->config; -if (c && c->setup) -value = c->setup(c, ctrl); +if (value < 0 && !f) { +struct usb_configuration*c; + +c = cdev->config; +if (c && c->setup) +value = c->setup(c, ctrl); +} } goto done; -- 1.7.2.5 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 3/5] at91udc: fix recursive poll()
I noticed some weird behavior using DFU gadget on AT91SAM9260 which worked 1 of 10 times. The bug was due to the usb_gadget_poll() being called while the previous call was still in progress : some USB calls may takes a lot of time (ex : DFU erase-then-write), resulting in the poller to be triggered again while still in the poll() function. This patch adds a poll lock to the AT91 UDC driver. Signed-off-by: Cerrato Renaud --- drivers/usb/gadget/at91_udc.c | 14 -- drivers/usb/gadget/at91_udc.h |1 + 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c index 0654038..8ae9119 100644 --- a/drivers/usb/gadget/at91_udc.c +++ b/drivers/usb/gadget/at91_udc.c @@ -1340,22 +1340,24 @@ int usb_gadget_poll(void) struct at91_udc*udc = &controller; u32 value; -if (!udc->udp_baseaddr) +if (!udc->udp_baseaddr || udc->poll_lock) return 0; +udc->poll_lock = 1; + value = gpio_get_value(udc->board.vbus_pin); value ^= udc->board.vbus_active_low; - -if (!value) { -at91_update_vbus(udc, value); -return 0; -} at91_update_vbus(udc, value); +if (!value) +goto exit; + value = at91_udp_read(udc, AT91_UDP_ISR) & (~(AT91_UDP_SOFINT)); if (value) at91_udc_irq(udc); +exit: +udc->poll_lock = 0; return value; } diff --git a/drivers/usb/gadget/at91_udc.h b/drivers/usb/gadget/at91_udc.h index e592cc5..51145a9 100644 --- a/drivers/usb/gadget/at91_udc.h +++ b/drivers/usb/gadget/at91_udc.h @@ -127,6 +127,7 @@ struct at91_udc { unsignedwait_for_config_ack:1; unsignedselfpowered:1; unsignedactive_suspend:1; +unsignedpoll_lock:1; u8addr; u32gpio_vbus_val; struct at91_udc_databoard; -- 1.7.2.5 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 2/5] at91udc: startup fix
I noticed that depending on boot timings, the USB gadget weren't detected at all by hosts. After few hours of debugging and using the at91clk output, I found that the UDP clock wasn't enabled from time to time. This patch fix the startup of UDC by adding a check into the at91_update_vbus(). Signed-off-by: Cerrato Renaud --- drivers/usb/gadget/at91_udc.c | 10 -- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c index 3899db2..0654038 100644 --- a/drivers/usb/gadget/at91_udc.c +++ b/drivers/usb/gadget/at91_udc.c @@ -1327,6 +1327,12 @@ static void at91_update_vbus(struct at91_udc *udc, u32 value) dev_set_param(udc->dev, "vbus", "0"); udc->gpio_vbus_val = value; +udc->vbus = value != 0; + +if(udc->driver) +pullup(udc, udc->vbus); +else +pullup(udc, 0); } int usb_gadget_poll(void) @@ -1508,7 +1514,7 @@ static int __init at91udc_probe(struct device_d *dev) * Get the initial state of VBUS - we cannot expect * a pending interrupt. */ -udc->vbus = gpio_get_value(udc->board.vbus_pin); +udc->vbus = gpio_get_value(udc->board.vbus_pin) ^ udc->board.vbus_active_low; DBG(udc, "VBUS detection: host:%s \n", udc->vbus ? "present":"absent"); } else { @@ -1517,7 +1523,7 @@ static int __init at91udc_probe(struct device_d *dev) } dev_add_param(dev, "vbus", NULL, NULL, 0); -dev_set_param(dev, "vbus", "0"); +dev_set_param(dev, "vbus", udc->vbus ? "1" : "0"); poller_register(&poller); -- 1.7.2.5 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 1/5] at91: debug unit typo fix
This patch fix the debug unit output by correcting a typo into an #ifdef statement. Signed-off-by: Cerrato Renaud --- arch/arm/mach-at91/include/mach/debug_ll.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-at91/include/mach/debug_ll.h b/arch/arm/mach-at91/include/mach/debug_ll.h index 1a85ae4..42728a4 100644 --- a/arch/arm/mach-at91/include/mach/debug_ll.h +++ b/arch/arm/mach-at91/include/mach/debug_ll.h @@ -11,7 +11,7 @@ #include #include -#ifdef COFNIG_HAVE_AT91_DBGU0 +#ifdef CONFIG_HAVE_AT91_DBGU0 #define UART_BASEAT91_BASE_DBGU0 #else #define UART_BASEAT91_BASE_DBGU1 -- 1.7.2.5 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 1/2] net: macb: remove gem_recv and reorder probe
On Wed, Mar 13, 2013 at 01:17:03PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 11:15 Wed 13 Mar , Sascha Hauer wrote: > > On Wed, Mar 13, 2013 at 10:03:36AM +0100, Jean-Christophe PLAGNIOL-VILLARD > > wrote: > > > On 09:48 Wed 13 Mar , Steffen Trumtrar wrote: > > > > The function gem_recv implements a buffer "ring" that stops at filling > > > > level 10. > > > > That means, when the driver is used as gem, it stops receiving after > > > > exactly > > > > 10 packets. Instead of implementing macb_recv twice, use it also for > > > > the gem > > > > part. If there needs to be an extra recv function for the gigabit case, > > > > it can > > > > be added than. > > > > Also, first set the type of device (macb or gem) and then use functions > > > > that > > > > use this info. > > > > > > > > Tested on a Zynq7000. > > > NACK > > > > > > on gem we can receive the packet in one buffer the gem_recv implement this > > > the macb can only receive splited buffer and then you have to reconstruct > > > the > > > packet > > > > The gem received function was never used... > > > > > > static int macb_recv(struct eth_device *edev) > > > > { > > > > struct macb_device *macb = edev->priv; > > > > @@ -619,11 +588,6 @@ static int macb_probe(struct device_d *dev) > > > > > > > > macb->phy_flags = pdata->phy_flags; > > > > > > > > - macb_init_rx_buffer_size(macb, PKTSIZE); > > > > - macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * > > > > macb->rx_ring_size); > > > > - macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); > > > > - macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); > > > > - > > > > macb->regs = dev_request_mem_region(dev, 0); > > > > > > > > /* > > > > @@ -638,12 +602,15 @@ static int macb_probe(struct device_d *dev) > > > > > > > > clk_enable(macb->pclk); > > > > > > > > - if (macb_is_gem(macb)) > > > > ... because macb_is_gem() is used here ... > > > > > > - edev->recv = gem_recv; > > > > - else > > > > - edev->recv = macb_recv; > > > > macb->is_gem = read_is_gem(macb); > > > > ... but the variable is initialized here. Up to this point macb_is_gem() > > will return 0. > so this is a bug when the driver was merge to the mailine but here I does use > the gem_recv. > What about the gem_recv function itself? It only increases rx_tail, but never resets it. Therefore the buffer is full after 10 packets. Do you already have a patch for that? Regards, Steffen -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 6/7] DFU : disable state fix
Hi, please start a new thread with patch [0/x] and the patch series as reply use git-send-email for this Best Resgards, J. On 11:34 Wed 13 Mar , Cerrato Renaud wrote: > Inlined patch below. > > > Signed-off-by: Cerrato Renaud > --- > drivers/usb/gadget/dfu.c |6 ++ > 1 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c > index e051879..e4af567 100644 > --- a/drivers/usb/gadget/dfu.c > +++ b/drivers/usb/gadget/dfu.c > @@ -514,14 +514,12 @@ static void dfu_disable(struct usb_function *f) > struct f_dfu*dfu = func_to_dfu(f); > > switch (dfu->dfu_state) { > +case DFU_STATE_dfuIDLE: > case DFU_STATE_appDETACH: > dfu->dfu_state = DFU_STATE_dfuIDLE; > break; > -case DFU_STATE_dfuMANIFEST_WAIT_RST: > -dfu->dfu_state = DFU_STATE_appIDLE; > -break; > default: > -dfu->dfu_state = DFU_STATE_appDETACH; > +dfu->dfu_state = DFU_STATE_appIDLE; > break; > } > > -- > 1.7.2.5 > > > On 12/03/2013 17:38, Renaud C. wrote: > > When an USB_REQ_DFU_DETACH request is received, the device state switch to > > DFU_STATE_appDETACH, and then wait for an usb reset to switch to > > DFU_STATE_dfuIDLE (through dfu_disable() being called). > > > > I noticed that using dfu-util v0.7 on my AT91SAM9260 board, the programming > > failed because of the device not entering the DFU_STATE_dfuIDLE after an > > usb reset (staying in the DFU_STATE_appDETACH state). > > > > According to the current implementation, once the USB_REQ_DFU_DETACH is > > sent and right after the usb reset, if the DFU client set more than one > > configuration (by iterating over them for example), the device state will > > reset to the DFU_STATE_appDETACH state on the 2nd iteration because of the > > following line in set_config (composite.c, line 362) : > > > > ... > > if(cdev->config) > > reset_config(); // will call dfu_disable() again! > > ... > > > > The attached patch is a *try* to fix the issue by leaving the device in the > > DFU_STATE_dfuIDLE state after an usb reset if already in that state. > > dfu-util will complain about the device already in the DFU_STATE_dfuIDLE on > > the next start, but everything is still working. > > > > This may need a REVISIT, but since the dfu_disable() gets called on both > > configuration change and usb reset, we can't do this easily for now.. > > > > > > > > > > > > > > > > > > > > > > ___ > > barebox mailing list > > barebox@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/barebox > > > > ___ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 1/2] net: macb: remove gem_recv and reorder probe
On 11:15 Wed 13 Mar , Sascha Hauer wrote: > On Wed, Mar 13, 2013 at 10:03:36AM +0100, Jean-Christophe PLAGNIOL-VILLARD > wrote: > > On 09:48 Wed 13 Mar , Steffen Trumtrar wrote: > > > The function gem_recv implements a buffer "ring" that stops at filling > > > level 10. > > > That means, when the driver is used as gem, it stops receiving after > > > exactly > > > 10 packets. Instead of implementing macb_recv twice, use it also for the > > > gem > > > part. If there needs to be an extra recv function for the gigabit case, > > > it can > > > be added than. > > > Also, first set the type of device (macb or gem) and then use functions > > > that > > > use this info. > > > > > > Tested on a Zynq7000. > > NACK > > > > on gem we can receive the packet in one buffer the gem_recv implement this > > the macb can only receive splited buffer and then you have to reconstruct > > the > > packet > > The gem received function was never used... > > > > static int macb_recv(struct eth_device *edev) > > > { > > > struct macb_device *macb = edev->priv; > > > @@ -619,11 +588,6 @@ static int macb_probe(struct device_d *dev) > > > > > > macb->phy_flags = pdata->phy_flags; > > > > > > - macb_init_rx_buffer_size(macb, PKTSIZE); > > > - macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * > > > macb->rx_ring_size); > > > - macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); > > > - macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); > > > - > > > macb->regs = dev_request_mem_region(dev, 0); > > > > > > /* > > > @@ -638,12 +602,15 @@ static int macb_probe(struct device_d *dev) > > > > > > clk_enable(macb->pclk); > > > > > > - if (macb_is_gem(macb)) > > ... because macb_is_gem() is used here ... > > > > - edev->recv = gem_recv; > > > - else > > > - edev->recv = macb_recv; > > > macb->is_gem = read_is_gem(macb); > > ... but the variable is initialized here. Up to this point macb_is_gem() > will return 0. so this is a bug when the driver was merge to the mailine but here I does use the gem_recv. Best Regrards, J. > > Sascha > > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH] net: macb: initialize is_gem before usage
The variable macb->is_gem is evaluated before it is initialized. That leads to a wrong rx_buffer setup in the gem case. Also, the function gem_recv will never be used. Set the variable first and then use it. Signed-off-by: Steffen Trumtrar --- drivers/net/macb.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 0cfad05..30e8476 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -619,11 +619,6 @@ static int macb_probe(struct device_d *dev) macb->phy_flags = pdata->phy_flags; - macb_init_rx_buffer_size(macb, PKTSIZE); - macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * macb->rx_ring_size); - macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); - macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); - macb->regs = dev_request_mem_region(dev, 0); /* @@ -638,11 +633,17 @@ static int macb_probe(struct device_d *dev) clk_enable(macb->pclk); + macb->is_gem = read_is_gem(macb); + + macb_init_rx_buffer_size(macb, PKTSIZE); + macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * macb->rx_ring_size); + macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); + macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); + if (macb_is_gem(macb)) edev->recv = gem_recv; else edev->recv = macb_recv; - macb->is_gem = read_is_gem(macb); macb_reset_hw(macb); ncfgr = macb_mdc_clk_div(macb); -- 1.8.2.rc2 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 6/7] DFU : disable state fix
Inlined patch below. Signed-off-by: Cerrato Renaud --- drivers/usb/gadget/dfu.c |6 ++ 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c index e051879..e4af567 100644 --- a/drivers/usb/gadget/dfu.c +++ b/drivers/usb/gadget/dfu.c @@ -514,14 +514,12 @@ static void dfu_disable(struct usb_function *f) struct f_dfu*dfu = func_to_dfu(f); switch (dfu->dfu_state) { +case DFU_STATE_dfuIDLE: case DFU_STATE_appDETACH: dfu->dfu_state = DFU_STATE_dfuIDLE; break; -case DFU_STATE_dfuMANIFEST_WAIT_RST: -dfu->dfu_state = DFU_STATE_appIDLE; -break; default: -dfu->dfu_state = DFU_STATE_appDETACH; +dfu->dfu_state = DFU_STATE_appIDLE; break; } -- 1.7.2.5 On 12/03/2013 17:38, Renaud C. wrote: > When an USB_REQ_DFU_DETACH request is received, the device state switch to > DFU_STATE_appDETACH, and then wait for an usb reset to switch to > DFU_STATE_dfuIDLE (through dfu_disable() being called). > > I noticed that using dfu-util v0.7 on my AT91SAM9260 board, the programming > failed because of the device not entering the DFU_STATE_dfuIDLE after an usb > reset (staying in the DFU_STATE_appDETACH state). > > According to the current implementation, once the USB_REQ_DFU_DETACH is sent > and right after the usb reset, if the DFU client set more than one > configuration (by iterating over them for example), the device state will > reset to the DFU_STATE_appDETACH state on the 2nd iteration because of the > following line in set_config (composite.c, line 362) : > > ... > if(cdev->config) > reset_config(); // will call dfu_disable() again! > ... > > The attached patch is a *try* to fix the issue by leaving the device in the > DFU_STATE_dfuIDLE state after an usb reset if already in that state. dfu-util > will complain about the device already in the DFU_STATE_dfuIDLE on the next > start, but everything is still working. > > This may need a REVISIT, but since the dfu_disable() gets called on both > configuration change and usb reset, we can't do this easily for now.. > > > > > > > > > > > ___ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 5/7] gadget: possible null pointer dereference fix
Inlined patch below. Signed-off-by: Cerrato Renaud --- drivers/usb/gadget/composite.c | 23 +-- 1 files changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index 9af115e..1f6c5b2 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -777,18 +777,21 @@ unknown: * recipients (endpoint, other, WUSB, ...) to the current * configuration code. */ -f = cdev->config->interface[intf]; -if (f && f->setup) -value = f->setup(f, ctrl); -else -f = NULL; +if(cdev->config) { -if (value < 0 && !f) { -struct usb_configuration*c; +f = cdev->config->interface[intf]; +if (f && f->setup) +value = f->setup(f, ctrl); +else +f = NULL; -c = cdev->config; -if (c && c->setup) -value = c->setup(c, ctrl); +if (value < 0 && !f) { +struct usb_configuration*c; + +c = cdev->config; +if (c && c->setup) +value = c->setup(c, ctrl); +} } goto done; -- 1.7.2.5 On 12/03/2013 17:05, Renaud C. wrote: > This patch fix a possible null pointer dereference exception because of a > missing null check on cdev->config > > > > > ___ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 4/7] AT91 UDC, fix recursive poll()
Inlined patch below. Signed-off-by: Cerrato Renaud --- drivers/usb/gadget/at91_udc.c | 14 -- drivers/usb/gadget/at91_udc.h |1 + 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c index 0654038..8ae9119 100644 --- a/drivers/usb/gadget/at91_udc.c +++ b/drivers/usb/gadget/at91_udc.c @@ -1340,22 +1340,24 @@ int usb_gadget_poll(void) struct at91_udc*udc = &controller; u32 value; -if (!udc->udp_baseaddr) +if (!udc->udp_baseaddr || udc->poll_lock) return 0; +udc->poll_lock = 1; + value = gpio_get_value(udc->board.vbus_pin); value ^= udc->board.vbus_active_low; - -if (!value) { -at91_update_vbus(udc, value); -return 0; -} at91_update_vbus(udc, value); +if (!value) +goto exit; + value = at91_udp_read(udc, AT91_UDP_ISR) & (~(AT91_UDP_SOFINT)); if (value) at91_udc_irq(udc); +exit: +udc->poll_lock = 0; return value; } diff --git a/drivers/usb/gadget/at91_udc.h b/drivers/usb/gadget/at91_udc.h index e592cc5..51145a9 100644 --- a/drivers/usb/gadget/at91_udc.h +++ b/drivers/usb/gadget/at91_udc.h @@ -127,6 +127,7 @@ struct at91_udc { unsignedwait_for_config_ack:1; unsignedselfpowered:1; unsignedactive_suspend:1; +unsignedpoll_lock:1; u8addr; u32gpio_vbus_val; struct at91_udc_databoard; -- 1.7.2.5 On 12/03/2013 17:03, Renaud C. wrote: > I noticed some weird behavior using DFU gadget on AT91SAM9260 which worked 1 > of 10 times. The bug was due to the usb_gadget_poll() being called while the > previous call was still in progress : some USB calls may takes a lot of time > (ex : DFU erase-then-write), resulting in the poller to be triggered again > while still in the poll() function. > > This patch adds a poll lock to the AT91 UDC driver. > > > ___ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 3/7] AT91 UDC startup fix
Inlined patch below. Signed-off-by: Cerrato Renaud --- drivers/usb/gadget/at91_udc.c | 10 -- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c index 3899db2..0654038 100644 --- a/drivers/usb/gadget/at91_udc.c +++ b/drivers/usb/gadget/at91_udc.c @@ -1327,6 +1327,12 @@ static void at91_update_vbus(struct at91_udc *udc, u32 value) dev_set_param(udc->dev, "vbus", "0"); udc->gpio_vbus_val = value; +udc->vbus = value != 0; + +if(udc->driver) +pullup(udc, udc->vbus); +else +pullup(udc, 0); } int usb_gadget_poll(void) @@ -1508,7 +1514,7 @@ static int __init at91udc_probe(struct device_d *dev) * Get the initial state of VBUS - we cannot expect * a pending interrupt. */ -udc->vbus = gpio_get_value(udc->board.vbus_pin); +udc->vbus = gpio_get_value(udc->board.vbus_pin) ^ udc->board.vbus_active_low; DBG(udc, "VBUS detection: host:%s \n", udc->vbus ? "present":"absent"); } else { @@ -1517,7 +1523,7 @@ static int __init at91udc_probe(struct device_d *dev) } dev_add_param(dev, "vbus", NULL, NULL, 0); -dev_set_param(dev, "vbus", "0"); +dev_set_param(dev, "vbus", udc->vbus ? "1" : "0"); poller_register(&poller); -- 1.7.2.5 On 12/03/2013 16:54, Renaud C. wrote: > I noticed that depending on boot timings, the USB gadget weren't detected at > all by hosts. After few hours of debugging and using the at91clk output, I > found that the UDP clock wasn't enabled from time to time. > > This patch fix the startup of UDC by adding a check into the > at91_update_vbus(). > > > > > ___ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 1/2] net: macb: remove gem_recv and reorder probe
On Wed, Mar 13, 2013 at 10:03:36AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 09:48 Wed 13 Mar , Steffen Trumtrar wrote: > > The function gem_recv implements a buffer "ring" that stops at filling > > level 10. > > That means, when the driver is used as gem, it stops receiving after exactly > > 10 packets. Instead of implementing macb_recv twice, use it also for the gem > > part. If there needs to be an extra recv function for the gigabit case, it > > can > > be added than. > > Also, first set the type of device (macb or gem) and then use functions that > > use this info. > > > > Tested on a Zynq7000. > NACK > > on gem we can receive the packet in one buffer the gem_recv implement this > the macb can only receive splited buffer and then you have to reconstruct the > packet The gem received function was never used... > > static int macb_recv(struct eth_device *edev) > > { > > struct macb_device *macb = edev->priv; > > @@ -619,11 +588,6 @@ static int macb_probe(struct device_d *dev) > > > > macb->phy_flags = pdata->phy_flags; > > > > - macb_init_rx_buffer_size(macb, PKTSIZE); > > - macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * > > macb->rx_ring_size); > > - macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); > > - macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); > > - > > macb->regs = dev_request_mem_region(dev, 0); > > > > /* > > @@ -638,12 +602,15 @@ static int macb_probe(struct device_d *dev) > > > > clk_enable(macb->pclk); > > > > - if (macb_is_gem(macb)) ... because macb_is_gem() is used here ... > > - edev->recv = gem_recv; > > - else > > - edev->recv = macb_recv; > > macb->is_gem = read_is_gem(macb); ... but the variable is initialized here. Up to this point macb_is_gem() will return 0. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 1/7] AT91 debug unit typo fix
Sorry. I should have read the posting guidelines beforehand. Should I start another subject for every patch I sent using [PATCH RESEND], or using a reply is acceptable? Signed-off-by: Cerrato Renaud --- arch/arm/mach-at91/include/mach/debug_ll.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-at91/include/mach/debug_ll.h b/arch/arm/mach-at91/include/mach/debug_ll.h index 1a85ae4..42728a4 100644 --- a/arch/arm/mach-at91/include/mach/debug_ll.h +++ b/arch/arm/mach-at91/include/mach/debug_ll.h @@ -11,7 +11,7 @@ #include #include -#ifdef COFNIG_HAVE_AT91_DBGU0 +#ifdef CONFIG_HAVE_AT91_DBGU0 #define UART_BASEAT91_BASE_DBGU0 #else #define UART_BASEAT91_BASE_DBGU1 -- 1.7.2.5 On 12/03/2013 17:52, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 16:39 Tue 12 Mar , Renaud C. wrote: >>This patch fix the debug unit output by correcting a typo into an #ifdef >>statement. > please resend the whole patch with inline patch > > no review otherwise > > Best Regards, > J. > > >> ___ >> barebox mailing list >> barebox@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/barebox ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 1/2] net: macb: remove gem_recv and reorder probe
On Mar 13, 2013, at 5:32 PM, Jean-Christophe PLAGNIOL-VILLARD wrote: > > On Mar 13, 2013, at 5:19 PM, Steffen Trumtrar > wrote: > >> On Wed, Mar 13, 2013 at 10:03:36AM +0100, Jean-Christophe PLAGNIOL-VILLARD >> wrote: >>> On 09:48 Wed 13 Mar , Steffen Trumtrar wrote: The function gem_recv implements a buffer "ring" that stops at filling level 10. That means, when the driver is used as gem, it stops receiving after exactly 10 packets. Instead of implementing macb_recv twice, use it also for the gem part. If there needs to be an extra recv function for the gigabit case, it can be added than. Also, first set the type of device (macb or gem) and then use functions that use this info. Tested on a Zynq7000. >>> NACK >>> >>> on gem we can receive the packet in one buffer the gem_recv implement this >>> the macb can only receive splited buffer and then you have to reconstruct >>> the >>> packet >>> >> >> Okay. That is nice and all. But try receiving more than just 10 packets with >> the current implementation. > > yes the drivers bufferize only 10 packets in the DMA which should be enough > for barebox > if we want we can increase the dma buffer size by increasing RX_NB_PACKET and this is valid for mach_recv and gem_recv btw Best Regards, J. > > Best Regards, > J. > >> >> Regards, >> str >> >>> Best Regards, >>> J. Signed-off-by: Steffen Trumtrar Cc: Nicolas Ferre Cc: Jean-Christophe PLAGNIOL-VILLARD --- drivers/net/macb.c | 47 +++ 1 file changed, 7 insertions(+), 40 deletions(-) diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 8602437..a12eea7 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -169,37 +169,6 @@ static void reclaim_rx_buffers(struct macb_device *macb, macb->rx_tail = new_tail; } -static int gem_recv(struct eth_device *edev) -{ - struct macb_device *macb = edev->priv; - unsigned int rx_tail = macb->rx_tail; - void *buffer; - int length; - u32 status; - - dev_dbg(macb->dev, "%s\n", __func__); - - for (;;) { - barrier(); - if (!(macb->rx_ring[rx_tail].addr & MACB_BIT(RX_USED))) - return -1; - - barrier(); - status = macb->rx_ring[rx_tail].ctrl; - length = MACB_BFEXT(RX_FRMLEN, status); - if (status & MACB_BIT(RX_SOF)) { - buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail; - net_receive(buffer, length); - macb->rx_ring[rx_tail].ctrl &= ~MACB_BIT(RX_USED); - barrier(); - } - rx_tail++; - macb->rx_tail++; - } - - return 0; -} - static int macb_recv(struct eth_device *edev) { struct macb_device *macb = edev->priv; @@ -619,11 +588,6 @@ static int macb_probe(struct device_d *dev) macb->phy_flags = pdata->phy_flags; - macb_init_rx_buffer_size(macb, PKTSIZE); - macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * macb->rx_ring_size); - macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); - macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); - macb->regs = dev_request_mem_region(dev, 0); /* @@ -638,12 +602,15 @@ static int macb_probe(struct device_d *dev) clk_enable(macb->pclk); - if (macb_is_gem(macb)) - edev->recv = gem_recv; - else - edev->recv = macb_recv; macb->is_gem = read_is_gem(macb); + macb_init_rx_buffer_size(macb, PKTSIZE); + macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * macb->rx_ring_size); + macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); + macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); + + edev->recv = macb_recv; + macb_reset_hw(macb); ncfgr = macb_mdc_clk_div(macb); ncfgr |= MACB_BIT(PAE); /* PAuse Enable */ -- 1.8.2.rc2 >>> >> >> -- >> Pengutronix e.K. | | >> Industrial Linux Solutions | http://www.pengutronix.de/ | >> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| >> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | > ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 1/2] net: macb: remove gem_recv and reorder probe
On Mar 13, 2013, at 5:19 PM, Steffen Trumtrar wrote: > On Wed, Mar 13, 2013 at 10:03:36AM +0100, Jean-Christophe PLAGNIOL-VILLARD > wrote: >> On 09:48 Wed 13 Mar , Steffen Trumtrar wrote: >>> The function gem_recv implements a buffer "ring" that stops at filling >>> level 10. >>> That means, when the driver is used as gem, it stops receiving after exactly >>> 10 packets. Instead of implementing macb_recv twice, use it also for the gem >>> part. If there needs to be an extra recv function for the gigabit case, it >>> can >>> be added than. >>> Also, first set the type of device (macb or gem) and then use functions that >>> use this info. >>> >>> Tested on a Zynq7000. >> NACK >> >> on gem we can receive the packet in one buffer the gem_recv implement this >> the macb can only receive splited buffer and then you have to reconstruct the >> packet >> > > Okay. That is nice and all. But try receiving more than just 10 packets with > the current implementation. yes the drivers bufferize only 10 packets in the DMA which should be enough for barebox if we want we can increase the dma buffer size by increasing RX_NB_PACKET Best Regards, J. > > Regards, > str > >> Best Regards, >> J. >>> >>> Signed-off-by: Steffen Trumtrar >>> Cc: Nicolas Ferre >>> Cc: Jean-Christophe PLAGNIOL-VILLARD >>> --- >>> drivers/net/macb.c | 47 +++ >>> 1 file changed, 7 insertions(+), 40 deletions(-) >>> >>> diff --git a/drivers/net/macb.c b/drivers/net/macb.c >>> index 8602437..a12eea7 100644 >>> --- a/drivers/net/macb.c >>> +++ b/drivers/net/macb.c >>> @@ -169,37 +169,6 @@ static void reclaim_rx_buffers(struct macb_device >>> *macb, >>> macb->rx_tail = new_tail; >>> } >>> >>> -static int gem_recv(struct eth_device *edev) >>> -{ >>> - struct macb_device *macb = edev->priv; >>> - unsigned int rx_tail = macb->rx_tail; >>> - void *buffer; >>> - int length; >>> - u32 status; >>> - >>> - dev_dbg(macb->dev, "%s\n", __func__); >>> - >>> - for (;;) { >>> - barrier(); >>> - if (!(macb->rx_ring[rx_tail].addr & MACB_BIT(RX_USED))) >>> - return -1; >>> - >>> - barrier(); >>> - status = macb->rx_ring[rx_tail].ctrl; >>> - length = MACB_BFEXT(RX_FRMLEN, status); >>> - if (status & MACB_BIT(RX_SOF)) { >>> - buffer = macb->rx_buffer + macb->rx_buffer_size * >>> macb->rx_tail; >>> - net_receive(buffer, length); >>> - macb->rx_ring[rx_tail].ctrl &= ~MACB_BIT(RX_USED); >>> - barrier(); >>> - } >>> - rx_tail++; >>> - macb->rx_tail++; >>> - } >>> - >>> - return 0; >>> -} >>> - >>> static int macb_recv(struct eth_device *edev) >>> { >>> struct macb_device *macb = edev->priv; >>> @@ -619,11 +588,6 @@ static int macb_probe(struct device_d *dev) >>> >>> macb->phy_flags = pdata->phy_flags; >>> >>> - macb_init_rx_buffer_size(macb, PKTSIZE); >>> - macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * >>> macb->rx_ring_size); >>> - macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); >>> - macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); >>> - >>> macb->regs = dev_request_mem_region(dev, 0); >>> >>> /* >>> @@ -638,12 +602,15 @@ static int macb_probe(struct device_d *dev) >>> >>> clk_enable(macb->pclk); >>> >>> - if (macb_is_gem(macb)) >>> - edev->recv = gem_recv; >>> - else >>> - edev->recv = macb_recv; >>> macb->is_gem = read_is_gem(macb); >>> >>> + macb_init_rx_buffer_size(macb, PKTSIZE); >>> + macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * >>> macb->rx_ring_size); >>> + macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); >>> + macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); >>> + >>> + edev->recv = macb_recv; >>> + >>> macb_reset_hw(macb); >>> ncfgr = macb_mdc_clk_div(macb); >>> ncfgr |= MACB_BIT(PAE); /* PAuse Enable */ >>> -- >>> 1.8.2.rc2 >>> >> > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 1/2] net: macb: remove gem_recv and reorder probe
On Wed, Mar 13, 2013 at 10:03:36AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 09:48 Wed 13 Mar , Steffen Trumtrar wrote: > > The function gem_recv implements a buffer "ring" that stops at filling > > level 10. > > That means, when the driver is used as gem, it stops receiving after exactly > > 10 packets. Instead of implementing macb_recv twice, use it also for the gem > > part. If there needs to be an extra recv function for the gigabit case, it > > can > > be added than. > > Also, first set the type of device (macb or gem) and then use functions that > > use this info. > > > > Tested on a Zynq7000. > NACK > > on gem we can receive the packet in one buffer the gem_recv implement this > the macb can only receive splited buffer and then you have to reconstruct the > packet > Okay. That is nice and all. But try receiving more than just 10 packets with the current implementation. Regards, str > Best Regards, > J. > > > > Signed-off-by: Steffen Trumtrar > > Cc: Nicolas Ferre > > Cc: Jean-Christophe PLAGNIOL-VILLARD > > --- > > drivers/net/macb.c | 47 +++ > > 1 file changed, 7 insertions(+), 40 deletions(-) > > > > diff --git a/drivers/net/macb.c b/drivers/net/macb.c > > index 8602437..a12eea7 100644 > > --- a/drivers/net/macb.c > > +++ b/drivers/net/macb.c > > @@ -169,37 +169,6 @@ static void reclaim_rx_buffers(struct macb_device > > *macb, > > macb->rx_tail = new_tail; > > } > > > > -static int gem_recv(struct eth_device *edev) > > -{ > > - struct macb_device *macb = edev->priv; > > - unsigned int rx_tail = macb->rx_tail; > > - void *buffer; > > - int length; > > - u32 status; > > - > > - dev_dbg(macb->dev, "%s\n", __func__); > > - > > - for (;;) { > > - barrier(); > > - if (!(macb->rx_ring[rx_tail].addr & MACB_BIT(RX_USED))) > > - return -1; > > - > > - barrier(); > > - status = macb->rx_ring[rx_tail].ctrl; > > - length = MACB_BFEXT(RX_FRMLEN, status); > > - if (status & MACB_BIT(RX_SOF)) { > > - buffer = macb->rx_buffer + macb->rx_buffer_size * > > macb->rx_tail; > > - net_receive(buffer, length); > > - macb->rx_ring[rx_tail].ctrl &= ~MACB_BIT(RX_USED); > > - barrier(); > > - } > > - rx_tail++; > > - macb->rx_tail++; > > - } > > - > > - return 0; > > -} > > - > > static int macb_recv(struct eth_device *edev) > > { > > struct macb_device *macb = edev->priv; > > @@ -619,11 +588,6 @@ static int macb_probe(struct device_d *dev) > > > > macb->phy_flags = pdata->phy_flags; > > > > - macb_init_rx_buffer_size(macb, PKTSIZE); > > - macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * > > macb->rx_ring_size); > > - macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); > > - macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); > > - > > macb->regs = dev_request_mem_region(dev, 0); > > > > /* > > @@ -638,12 +602,15 @@ static int macb_probe(struct device_d *dev) > > > > clk_enable(macb->pclk); > > > > - if (macb_is_gem(macb)) > > - edev->recv = gem_recv; > > - else > > - edev->recv = macb_recv; > > macb->is_gem = read_is_gem(macb); > > > > + macb_init_rx_buffer_size(macb, PKTSIZE); > > + macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * > > macb->rx_ring_size); > > + macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); > > + macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); > > + > > + edev->recv = macb_recv; > > + > > macb_reset_hw(macb); > > ncfgr = macb_mdc_clk_div(macb); > > ncfgr |= MACB_BIT(PAE); /* PAuse Enable */ > > -- > > 1.8.2.rc2 > > > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 2/2] net: macb: turn off endian_swp_pkt_en
Hi! On Wed, Mar 13, 2013 at 10:04:45AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 09:48 Wed 13 Mar , Steffen Trumtrar wrote: > > The core has a bit for swapping packet data endianism. > > Reset default from Cadence is off. Xilinx however, that uses this core on > > the > > Zynq SoCs, opted for on. Turn it off for all devices. > > is this xilinx specifc? > > on at91 and other we do not need it > Well, as stated in the commit log, Cadence default is off. I guess, at91 does not change this. So, where is the problem then forcing it to the sane default from Cadence? Regards, str > Best Regards, > J. > > > > Signed-off-by: Steffen Trumtrar > > Cc: Nicolas Ferre > > Cc: Jean-Christophe PLAGNIOL-VILLARD > > --- > > drivers/net/macb.c | 1 + > > drivers/net/macb.h | 2 ++ > > 2 files changed, 3 insertions(+) > > > > diff --git a/drivers/net/macb.c b/drivers/net/macb.c > > index a12eea7..005234e 100644 > > --- a/drivers/net/macb.c > > +++ b/drivers/net/macb.c > > @@ -277,6 +277,7 @@ static void macb_configure_dma(struct macb_device *bp) > > dmacfg |= GEM_BF(FBLDO, 16); > > dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L); > > dmacfg |= GEM_BIT(DDRP); > > + dmacfg &= ~GEM_BIT(ENDIA); > > gem_writel(bp, DMACFG, dmacfg); > > } > > } > > diff --git a/drivers/net/macb.h b/drivers/net/macb.h > > index cadd561..1be9ff9 100644 > > --- a/drivers/net/macb.h > > +++ b/drivers/net/macb.h > > @@ -168,6 +168,8 @@ > > /* Bitfields in DMACFG. */ > > #define GEM_FBLDO_OFFSET 0 > > #define GEM_FBLDO_SIZE 5 > > +#define GEM_ENDIA_OFFSET 7 > > +#define GEM_ENDIA_SIZE 1 > > #define GEM_RXBMS_OFFSET 8 > > #define GEM_RXBMS_SIZE 2 > > #define GEM_TXPBMS_OFFSET 10 > > -- > > 1.8.2.rc2 > > > > ___ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 2/2] net: macb: turn off endian_swp_pkt_en
On 09:48 Wed 13 Mar , Steffen Trumtrar wrote: > The core has a bit for swapping packet data endianism. > Reset default from Cadence is off. Xilinx however, that uses this core on the > Zynq SoCs, opted for on. Turn it off for all devices. is this xilinx specifc? on at91 and other we do not need it Best Regards, J. > > Signed-off-by: Steffen Trumtrar > Cc: Nicolas Ferre > Cc: Jean-Christophe PLAGNIOL-VILLARD > --- > drivers/net/macb.c | 1 + > drivers/net/macb.h | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/drivers/net/macb.c b/drivers/net/macb.c > index a12eea7..005234e 100644 > --- a/drivers/net/macb.c > +++ b/drivers/net/macb.c > @@ -277,6 +277,7 @@ static void macb_configure_dma(struct macb_device *bp) > dmacfg |= GEM_BF(FBLDO, 16); > dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L); > dmacfg |= GEM_BIT(DDRP); > + dmacfg &= ~GEM_BIT(ENDIA); > gem_writel(bp, DMACFG, dmacfg); > } > } > diff --git a/drivers/net/macb.h b/drivers/net/macb.h > index cadd561..1be9ff9 100644 > --- a/drivers/net/macb.h > +++ b/drivers/net/macb.h > @@ -168,6 +168,8 @@ > /* Bitfields in DMACFG. */ > #define GEM_FBLDO_OFFSET 0 > #define GEM_FBLDO_SIZE 5 > +#define GEM_ENDIA_OFFSET 7 > +#define GEM_ENDIA_SIZE 1 > #define GEM_RXBMS_OFFSET 8 > #define GEM_RXBMS_SIZE 2 > #define GEM_TXPBMS_OFFSET10 > -- > 1.8.2.rc2 > ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 1/2] net: macb: remove gem_recv and reorder probe
On 09:48 Wed 13 Mar , Steffen Trumtrar wrote: > The function gem_recv implements a buffer "ring" that stops at filling level > 10. > That means, when the driver is used as gem, it stops receiving after exactly > 10 packets. Instead of implementing macb_recv twice, use it also for the gem > part. If there needs to be an extra recv function for the gigabit case, it can > be added than. > Also, first set the type of device (macb or gem) and then use functions that > use this info. > > Tested on a Zynq7000. NACK on gem we can receive the packet in one buffer the gem_recv implement this the macb can only receive splited buffer and then you have to reconstruct the packet Best Regards, J. > > Signed-off-by: Steffen Trumtrar > Cc: Nicolas Ferre > Cc: Jean-Christophe PLAGNIOL-VILLARD > --- > drivers/net/macb.c | 47 +++ > 1 file changed, 7 insertions(+), 40 deletions(-) > > diff --git a/drivers/net/macb.c b/drivers/net/macb.c > index 8602437..a12eea7 100644 > --- a/drivers/net/macb.c > +++ b/drivers/net/macb.c > @@ -169,37 +169,6 @@ static void reclaim_rx_buffers(struct macb_device *macb, > macb->rx_tail = new_tail; > } > > -static int gem_recv(struct eth_device *edev) > -{ > - struct macb_device *macb = edev->priv; > - unsigned int rx_tail = macb->rx_tail; > - void *buffer; > - int length; > - u32 status; > - > - dev_dbg(macb->dev, "%s\n", __func__); > - > - for (;;) { > - barrier(); > - if (!(macb->rx_ring[rx_tail].addr & MACB_BIT(RX_USED))) > - return -1; > - > - barrier(); > - status = macb->rx_ring[rx_tail].ctrl; > - length = MACB_BFEXT(RX_FRMLEN, status); > - if (status & MACB_BIT(RX_SOF)) { > - buffer = macb->rx_buffer + macb->rx_buffer_size * > macb->rx_tail; > - net_receive(buffer, length); > - macb->rx_ring[rx_tail].ctrl &= ~MACB_BIT(RX_USED); > - barrier(); > - } > - rx_tail++; > - macb->rx_tail++; > - } > - > - return 0; > -} > - > static int macb_recv(struct eth_device *edev) > { > struct macb_device *macb = edev->priv; > @@ -619,11 +588,6 @@ static int macb_probe(struct device_d *dev) > > macb->phy_flags = pdata->phy_flags; > > - macb_init_rx_buffer_size(macb, PKTSIZE); > - macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * > macb->rx_ring_size); > - macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); > - macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); > - > macb->regs = dev_request_mem_region(dev, 0); > > /* > @@ -638,12 +602,15 @@ static int macb_probe(struct device_d *dev) > > clk_enable(macb->pclk); > > - if (macb_is_gem(macb)) > - edev->recv = gem_recv; > - else > - edev->recv = macb_recv; > macb->is_gem = read_is_gem(macb); > > + macb_init_rx_buffer_size(macb, PKTSIZE); > + macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * > macb->rx_ring_size); > + macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); > + macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); > + > + edev->recv = macb_recv; > + > macb_reset_hw(macb); > ncfgr = macb_mdc_clk_div(macb); > ncfgr |= MACB_BIT(PAE); /* PAuse Enable */ > -- > 1.8.2.rc2 > ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH] [ARM] imx1: add #define to configure frequency for the clk32
From: gwenhael i.MX1 allows to use a 32kHz or a 32.768kHz quartz for the 32kHz clock source (MC9328MXLRM p.177). This patch adds the ability for the developer to change the default value for the 32kHz clock source. Signed-off-by: Gwenhael Goavec-Merou --- arch/arm/mach-imx/clk-imx1.c |6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/arm/mach-imx/clk-imx1.c b/arch/arm/mach-imx/clk-imx1.c index 0d04a92..b779ab2 100644 --- a/arch/arm/mach-imx/clk-imx1.c +++ b/arch/arm/mach-imx/clk-imx1.c @@ -26,6 +26,10 @@ #include "clk.h" +#ifndef MX1_CLK32_FREQ +#define MX1_CLK32_FREQ 32000 +#endif + #define CCM_CSCR 0x0 #define CCM_MPCTL0 0x4 #define CCM_SPCTL0 0xc @@ -91,7 +95,7 @@ static int imx1_ccm_probe(struct device_d *dev) regs = dev_request_mem_region(dev, 0); - mx1_clocks_init(regs, 32000); + mx1_clocks_init(regs, MX1_CLK32_FREQ); return 0; } -- 1.7.10.4 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 1/2] net: macb: remove gem_recv and reorder probe
The function gem_recv implements a buffer "ring" that stops at filling level 10. That means, when the driver is used as gem, it stops receiving after exactly 10 packets. Instead of implementing macb_recv twice, use it also for the gem part. If there needs to be an extra recv function for the gigabit case, it can be added than. Also, first set the type of device (macb or gem) and then use functions that use this info. Tested on a Zynq7000. Signed-off-by: Steffen Trumtrar Cc: Nicolas Ferre Cc: Jean-Christophe PLAGNIOL-VILLARD --- drivers/net/macb.c | 47 +++ 1 file changed, 7 insertions(+), 40 deletions(-) diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 8602437..a12eea7 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -169,37 +169,6 @@ static void reclaim_rx_buffers(struct macb_device *macb, macb->rx_tail = new_tail; } -static int gem_recv(struct eth_device *edev) -{ - struct macb_device *macb = edev->priv; - unsigned int rx_tail = macb->rx_tail; - void *buffer; - int length; - u32 status; - - dev_dbg(macb->dev, "%s\n", __func__); - - for (;;) { - barrier(); - if (!(macb->rx_ring[rx_tail].addr & MACB_BIT(RX_USED))) - return -1; - - barrier(); - status = macb->rx_ring[rx_tail].ctrl; - length = MACB_BFEXT(RX_FRMLEN, status); - if (status & MACB_BIT(RX_SOF)) { - buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail; - net_receive(buffer, length); - macb->rx_ring[rx_tail].ctrl &= ~MACB_BIT(RX_USED); - barrier(); - } - rx_tail++; - macb->rx_tail++; - } - - return 0; -} - static int macb_recv(struct eth_device *edev) { struct macb_device *macb = edev->priv; @@ -619,11 +588,6 @@ static int macb_probe(struct device_d *dev) macb->phy_flags = pdata->phy_flags; - macb_init_rx_buffer_size(macb, PKTSIZE); - macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * macb->rx_ring_size); - macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); - macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); - macb->regs = dev_request_mem_region(dev, 0); /* @@ -638,12 +602,15 @@ static int macb_probe(struct device_d *dev) clk_enable(macb->pclk); - if (macb_is_gem(macb)) - edev->recv = gem_recv; - else - edev->recv = macb_recv; macb->is_gem = read_is_gem(macb); + macb_init_rx_buffer_size(macb, PKTSIZE); + macb->rx_buffer = dma_alloc_coherent(macb->rx_buffer_size * macb->rx_ring_size); + macb->rx_ring = dma_alloc_coherent(RX_RING_BYTES(macb)); + macb->tx_ring = dma_alloc_coherent(TX_RING_BYTES); + + edev->recv = macb_recv; + macb_reset_hw(macb); ncfgr = macb_mdc_clk_div(macb); ncfgr |= MACB_BIT(PAE); /* PAuse Enable */ -- 1.8.2.rc2 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 2/2] net: macb: turn off endian_swp_pkt_en
The core has a bit for swapping packet data endianism. Reset default from Cadence is off. Xilinx however, that uses this core on the Zynq SoCs, opted for on. Turn it off for all devices. Signed-off-by: Steffen Trumtrar Cc: Nicolas Ferre Cc: Jean-Christophe PLAGNIOL-VILLARD --- drivers/net/macb.c | 1 + drivers/net/macb.h | 2 ++ 2 files changed, 3 insertions(+) diff --git a/drivers/net/macb.c b/drivers/net/macb.c index a12eea7..005234e 100644 --- a/drivers/net/macb.c +++ b/drivers/net/macb.c @@ -277,6 +277,7 @@ static void macb_configure_dma(struct macb_device *bp) dmacfg |= GEM_BF(FBLDO, 16); dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L); dmacfg |= GEM_BIT(DDRP); + dmacfg &= ~GEM_BIT(ENDIA); gem_writel(bp, DMACFG, dmacfg); } } diff --git a/drivers/net/macb.h b/drivers/net/macb.h index cadd561..1be9ff9 100644 --- a/drivers/net/macb.h +++ b/drivers/net/macb.h @@ -168,6 +168,8 @@ /* Bitfields in DMACFG. */ #define GEM_FBLDO_OFFSET 0 #define GEM_FBLDO_SIZE 5 +#define GEM_ENDIA_OFFSET 7 +#define GEM_ENDIA_SIZE 1 #define GEM_RXBMS_OFFSET 8 #define GEM_RXBMS_SIZE 2 #define GEM_TXPBMS_OFFSET 10 -- 1.8.2.rc2 ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox