* [PATCH 1/2] cxacru: Use appropriate logging for errors
@ 2007-09-23 15:32 Simon Arlott
2007-09-23 15:34 ` [PATCH 2/3] cxacru: Reduce initialisation delay Simon Arlott
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Simon Arlott @ 2007-09-23 15:32 UTC (permalink / raw)
To: Linux Kernel Mailing List; +Cc: Greg Kroah-Hartman
When an error occurs, existing logging uses dbg() so the cause of a
problem is hard to determine. Error conditions shouldn't only be
properly reported with debugging enabled.
A side effect of this change is that when an uninitialised device
is started, a log message similar to the following is sent:
cxacru 5-2:1.0: receive of cm 0x90 failed (-104)
This is normal - the device did not respond so firmware will be
loaded.
Signed-Off-By: Simon Arlott <simon@fire.lp0.eu>
---
This could be added to 2.6.23 since it only makes error logging
more verbose.
drivers/usb/atm/cxacru.c | 29 ++++++++++++++++++-----------
1 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c
index a73e714..8d8a107 100644
--- a/drivers/usb/atm/cxacru.c
+++ b/drivers/usb/atm/cxacru.c
@@ -482,7 +482,8 @@ static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm,
int rbuflen = ((rsize - 1) / stride + 1) * CMD_PACKET_SIZE;
if (wbuflen > PAGE_SIZE || rbuflen > PAGE_SIZE) {
- dbg("too big transfer requested");
+ usb_err(instance->usbatm, "requested transfer size too large (%d, %d)\n",
+ wbuflen, rbuflen);
ret = -ENOMEM;
goto fail;
}
@@ -493,7 +494,8 @@ static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm,
init_completion(&instance->rcv_done);
ret = usb_submit_urb(instance->rcv_urb, GFP_KERNEL);
if (ret < 0) {
- dbg("submitting read urb for cm %#x failed", cm);
+ usb_err(instance->usbatm, "submit of read urb for cm %#x failed (%d)\n",
+ cm, ret);
ret = ret;
goto fail;
}
@@ -510,27 +512,28 @@ static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm,
init_completion(&instance->snd_done);
ret = usb_submit_urb(instance->snd_urb, GFP_KERNEL);
if (ret < 0) {
- dbg("submitting write urb for cm %#x failed", cm);
+ usb_err(instance->usbatm, "submit of write urb for cm %#x failed (%d)\n",
+ cm, ret);
ret = ret;
goto fail;
}
ret = cxacru_start_wait_urb(instance->snd_urb, &instance->snd_done, NULL);
if (ret < 0) {
- dbg("sending cm %#x failed", cm);
+ usb_err(instance->usbatm, "send of cm %#x failed (%d)\n", cm, ret);
ret = ret;
goto fail;
}
ret = cxacru_start_wait_urb(instance->rcv_urb, &instance->rcv_done, &actlen);
if (ret < 0) {
- dbg("receiving cm %#x failed", cm);
+ usb_err(instance->usbatm, "receive of cm %#x failed (%d)\n", cm, ret);
ret = ret;
goto fail;
}
if (actlen % CMD_PACKET_SIZE || !actlen) {
- dbg("response is not a positive multiple of %d: %#x",
- CMD_PACKET_SIZE, actlen);
+ usb_err(instance->usbatm, "invalid response length to cm %#x: %d\n",
+ cm, actlen);
ret = -EIO;
goto fail;
}
@@ -538,12 +541,14 @@ static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm,
/* check the return status and copy the data to the output buffer, if needed */
for (offb = offd = 0; offd < rsize && offb < actlen; offb += CMD_PACKET_SIZE) {
if (rbuf[offb] != cm) {
- dbg("wrong cm %#x in response", rbuf[offb]);
+ usb_err(instance->usbatm, "wrong cm %#x in response to cm %#x\n",
+ rbuf[offb], cm);
ret = -EIO;
goto fail;
}
if (rbuf[offb + 1] != CM_STATUS_SUCCESS) {
- dbg("response failed: %#x", rbuf[offb + 1]);
+ usb_err(instance->usbatm, "response to cm %#x failed: %#x\n",
+ cm, rbuf[offb + 1]);
ret = -EIO;
goto fail;
}
@@ -582,14 +587,16 @@ static int cxacru_cm_get_array(struct cxacru_data *instance, enum cxacru_cm_requ
for (offb = 0; offb < len; ) {
int l = le32_to_cpu(buf[offb++]);
if (l > stride || l > (len - offb) / 2) {
- dbg("wrong data length %#x in response", l);
+ usb_err(instance->usbatm, "invalid data length from cm %#x: %d\n",
+ cm, l);
ret = -EIO;
goto cleanup;
}
while (l--) {
offd = le32_to_cpu(buf[offb++]);
if (offd >= size) {
- dbg("wrong index %#x in response", offd);
+ usb_err(instance->usbatm, "wrong index #%x in response to cm #%x\n",
+ offd, cm);
ret = -EIO;
goto cleanup;
}
--
1.5.0.1
--
Simon Arlott
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/3] cxacru: Reduce initialisation delay 2007-09-23 15:32 [PATCH 1/2] cxacru: Use appropriate logging for errors Simon Arlott @ 2007-09-23 15:34 ` Simon Arlott 2007-09-23 16:23 ` Duncan Sands 2007-09-23 15:36 ` [PATCH 3/3] cxacru: Cleanup code by removing "ret = ret;" assignments Simon Arlott 2007-09-23 16:17 ` [PATCH 1/2] " Duncan Sands 2 siblings, 1 reply; 12+ messages in thread From: Simon Arlott @ 2007-09-23 15:34 UTC (permalink / raw) To: Linux Kernel Mailing List; +Cc: Greg Kroah-Hartman Since card status updates appear to only occur every second, a delay of 1000ms on startup may not be sufficient - change to 1500ms. The long delay of 4000ms is likely to be related to the time required for the ADSL line to come up - the driver should not need to do this. Overall delay when loading firmware will change from 5000ms to 1500ms. Signed-Off-By: Simon Arlott <simon@fire.lp0.eu> --- drivers/usb/atm/cxacru.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c index 8d8a107..35308a8 100644 --- a/drivers/usb/atm/cxacru.c +++ b/drivers/usb/atm/cxacru.c @@ -931,9 +931,10 @@ static void cxacru_upload_firmware(struct cxacru_data *instance, usb_err(usbatm, "Passing control to firmware failed: %d\n", ret); return; } + usb_info(usbatm, "started firmware\n"); /* Delay to allow firmware to start up. */ - msleep_interruptible(1000); + msleep(1500); usb_clear_halt(usb_dev, usb_sndbulkpipe(usb_dev, CXACRU_EP_CMD)); usb_clear_halt(usb_dev, usb_rcvbulkpipe(usb_dev, CXACRU_EP_CMD)); @@ -947,7 +948,7 @@ static void cxacru_upload_firmware(struct cxacru_data *instance, } /* Load config data (le32), doing one packet at a time */ - if (cf) + if (cf) { for (off = 0; off < cf->size / 4; ) { u32 buf[CMD_PACKET_SIZE / 4 - 1]; int i, len = min_t(int, cf->size / 4 - off, CMD_PACKET_SIZE / 4 / 2 - 1); @@ -963,8 +964,8 @@ static void cxacru_upload_firmware(struct cxacru_data *instance, return; } } - - msleep_interruptible(4000); + usb_info(usbatm, "loaded config data\n"); + } } static int cxacru_find_firmware(struct cxacru_data *instance, -- 1.5.0.1 -- Simon Arlott ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] cxacru: Reduce initialisation delay 2007-09-23 15:34 ` [PATCH 2/3] cxacru: Reduce initialisation delay Simon Arlott @ 2007-09-23 16:23 ` Duncan Sands 2007-09-23 18:33 ` Simon Arlott 0 siblings, 1 reply; 12+ messages in thread From: Duncan Sands @ 2007-09-23 16:23 UTC (permalink / raw) To: Simon Arlott; +Cc: Linux Kernel Mailing List, Greg Kroah-Hartman Hi Simon, > + usb_info(usbatm, "started firmware\n"); ... > + usb_info(usbatm, "loaded config data\n"); maybe these should be debug messages. When are they useful? Ciao, Duncan. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] cxacru: Reduce initialisation delay 2007-09-23 16:23 ` Duncan Sands @ 2007-09-23 18:33 ` Simon Arlott 0 siblings, 0 replies; 12+ messages in thread From: Simon Arlott @ 2007-09-23 18:33 UTC (permalink / raw) To: Duncan Sands; +Cc: Linux Kernel Mailing List, Greg Kroah-Hartman On 23/09/07 17:23, Duncan Sands wrote: > Hi Simon, > >> + usb_info(usbatm, "started firmware\n"); > ... >> + usb_info(usbatm, "loaded config data\n"); > > maybe these should be debug messages. When are they useful? They are probably only useful as debug messages - although it may be desirable to know when the configuration has been set. Also... it doesn't make sense to load the configuration only in heavy_init - if the configuration is changed then there's no way in the module to resend it without powering the device down and up. Some sysfs parameters to change configuration could be useful... except there's no information as to what these settings are. -- Simon Arlott ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] cxacru: Cleanup code by removing "ret = ret;" assignments 2007-09-23 15:32 [PATCH 1/2] cxacru: Use appropriate logging for errors Simon Arlott 2007-09-23 15:34 ` [PATCH 2/3] cxacru: Reduce initialisation delay Simon Arlott @ 2007-09-23 15:36 ` Simon Arlott 2007-09-23 16:20 ` Duncan Sands 2007-09-23 16:17 ` [PATCH 1/2] " Duncan Sands 2 siblings, 1 reply; 12+ messages in thread From: Simon Arlott @ 2007-09-23 15:36 UTC (permalink / raw) To: Linux Kernel Mailing List; +Cc: Greg Kroah-Hartman Cleanup code by removing "ret = ret;" assignments. Signed-Off-By: Simon Arlott <simon@fire.lp0.eu> --- drivers/usb/atm/cxacru.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c index 35308a8..bb3169c 100644 --- a/drivers/usb/atm/cxacru.c +++ b/drivers/usb/atm/cxacru.c @@ -496,7 +496,6 @@ static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm, if (ret < 0) { usb_err(instance->usbatm, "submit of read urb for cm %#x failed (%d)\n", cm, ret); - ret = ret; goto fail; } @@ -514,21 +513,18 @@ static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm, if (ret < 0) { usb_err(instance->usbatm, "submit of write urb for cm %#x failed (%d)\n", cm, ret); - ret = ret; goto fail; } ret = cxacru_start_wait_urb(instance->snd_urb, &instance->snd_done, NULL); if (ret < 0) { usb_err(instance->usbatm, "send of cm %#x failed (%d)\n", cm, ret); - ret = ret; goto fail; } ret = cxacru_start_wait_urb(instance->rcv_urb, &instance->rcv_done, &actlen); if (ret < 0) { usb_err(instance->usbatm, "receive of cm %#x failed (%d)\n", cm, ret); - ret = ret; goto fail; } if (actlen % CMD_PACKET_SIZE || !actlen) { -- 1.5.0.1 -- Simon Arlott ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] cxacru: Cleanup code by removing "ret = ret;" assignments 2007-09-23 15:36 ` [PATCH 3/3] cxacru: Cleanup code by removing "ret = ret;" assignments Simon Arlott @ 2007-09-23 16:20 ` Duncan Sands 2007-09-23 18:36 ` Simon Arlott 0 siblings, 1 reply; 12+ messages in thread From: Duncan Sands @ 2007-09-23 16:20 UTC (permalink / raw) To: Simon Arlott; +Cc: Linux Kernel Mailing List, Greg Kroah-Hartman On Sunday 23 September 2007 17:36:08 Simon Arlott wrote: > Cleanup code by removing "ret = ret;" assignments. > > Signed-Off-By: Simon Arlott <simon@fire.lp0.eu> Acked-by: Duncan Sands <baldrick@free.fr> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] cxacru: Cleanup code by removing "ret = ret;" assignments 2007-09-23 16:20 ` Duncan Sands @ 2007-09-23 18:36 ` Simon Arlott 2007-09-24 18:32 ` Greg KH 0 siblings, 1 reply; 12+ messages in thread From: Simon Arlott @ 2007-09-23 18:36 UTC (permalink / raw) To: Duncan Sands; +Cc: Linux Kernel Mailing List, Greg Kroah-Hartman On 23/09/07 17:20, Duncan Sands wrote: > On Sunday 23 September 2007 17:36:08 Simon Arlott wrote: >> Cleanup code by removing "ret = ret;" assignments. >> >> Signed-Off-By: Simon Arlott <simon@fire.lp0.eu> > > Acked-by: Duncan Sands <baldrick@free.fr> Nacked-by: Simon Arlott <simon@fire.lp0.eu> I'm only going to create a merge conflict with myself with this :| It'll be included with 1/3 shortly. -- Simon Arlott ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] cxacru: Cleanup code by removing "ret = ret;" assignments 2007-09-23 18:36 ` Simon Arlott @ 2007-09-24 18:32 ` Greg KH 2007-09-25 19:20 ` [PATCH] cxacru: Use appropriate logging for errors Simon Arlott 0 siblings, 1 reply; 12+ messages in thread From: Greg KH @ 2007-09-24 18:32 UTC (permalink / raw) To: Simon Arlott; +Cc: Duncan Sands, Linux Kernel Mailing List On Sun, Sep 23, 2007 at 07:36:05PM +0100, Simon Arlott wrote: > On 23/09/07 17:20, Duncan Sands wrote: > > On Sunday 23 September 2007 17:36:08 Simon Arlott wrote: > >> Cleanup code by removing "ret = ret;" assignments. > >> > >> Signed-Off-By: Simon Arlott <simon@fire.lp0.eu> > > > > Acked-by: Duncan Sands <baldrick@free.fr> > > Nacked-by: Simon Arlott <simon@fire.lp0.eu> > > I'm only going to create a merge conflict with myself with this :| > It'll be included with 1/3 shortly. Ok, I'm totally confused :) Can you resend the patches you want to have included here, that are acked by both you and Duncan? thanks, greg k-h ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] cxacru: Use appropriate logging for errors 2007-09-24 18:32 ` Greg KH @ 2007-09-25 19:20 ` Simon Arlott 0 siblings, 0 replies; 12+ messages in thread From: Simon Arlott @ 2007-09-25 19:20 UTC (permalink / raw) To: Greg KH; +Cc: Duncan Sands, Linux Kernel Mailing List When an error occurs, existing logging uses dbg() so the cause of a problem is hard to determine. Error conditions shouldn't only be properly reported with debugging enabled. A side effect of this change is that when an uninitialised device is started, a log message similar to the following is sent: cxacru 5-2:1.0: receive of cm 0x90 failed (-104) This is normal - the device did not respond so firmware will be loaded. Signed-off-by: Simon Arlott <simon@fire.lp0.eu> Acked-by: Duncan Sands <baldrick@free.fr> --- On 24/09/07 19:32, Greg KH wrote: > On Sun, Sep 23, 2007 at 07:36:05PM +0100, Simon Arlott wrote: >> I'm only going to create a merge conflict with myself with this :| >> It'll be included with 1/3 shortly. > > Ok, I'm totally confused :) > > Can you resend the patches you want to have included here, that are > acked by both you and Duncan? > > thanks, > > greg k-h drivers/usb/atm/cxacru.c | 43 ++++++++++++++++++++++++++++--------------- 1 files changed, 28 insertions(+), 15 deletions(-) diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c index 1e5ee88..50249a5 100644 --- a/drivers/usb/atm/cxacru.c +++ b/drivers/usb/atm/cxacru.c @@ -482,7 +482,9 @@ static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm, int rbuflen = ((rsize - 1) / stride + 1) * CMD_PACKET_SIZE; if (wbuflen > PAGE_SIZE || rbuflen > PAGE_SIZE) { - dbg("too big transfer requested"); + if (printk_ratelimit()) + usb_err(instance->usbatm, "requested transfer size too large (%d, %d)\n", + wbuflen, rbuflen); ret = -ENOMEM; goto fail; } @@ -493,8 +495,9 @@ static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm, init_completion(&instance->rcv_done); ret = usb_submit_urb(instance->rcv_urb, GFP_KERNEL); if (ret < 0) { - dbg("submitting read urb for cm %#x failed", cm); - ret = ret; + if (printk_ratelimit()) + usb_err(instance->usbatm, "submit of read urb for cm %#x failed (%d)\n", + cm, ret); goto fail; } @@ -510,27 +513,29 @@ static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm, init_completion(&instance->snd_done); ret = usb_submit_urb(instance->snd_urb, GFP_KERNEL); if (ret < 0) { - dbg("submitting write urb for cm %#x failed", cm); - ret = ret; + if (printk_ratelimit()) + usb_err(instance->usbatm, "submit of write urb for cm %#x failed (%d)\n", + cm, ret); goto fail; } ret = cxacru_start_wait_urb(instance->snd_urb, &instance->snd_done, NULL); if (ret < 0) { - dbg("sending cm %#x failed", cm); - ret = ret; + if (printk_ratelimit()) + usb_err(instance->usbatm, "send of cm %#x failed (%d)\n", cm, ret); goto fail; } ret = cxacru_start_wait_urb(instance->rcv_urb, &instance->rcv_done, &actlen); if (ret < 0) { - dbg("receiving cm %#x failed", cm); - ret = ret; + if (printk_ratelimit()) + usb_err(instance->usbatm, "receive of cm %#x failed (%d)\n", cm, ret); goto fail; } if (actlen % CMD_PACKET_SIZE || !actlen) { - dbg("response is not a positive multiple of %d: %#x", - CMD_PACKET_SIZE, actlen); + if (printk_ratelimit()) + usb_err(instance->usbatm, "invalid response length to cm %#x: %d\n", + cm, actlen); ret = -EIO; goto fail; } @@ -538,12 +543,16 @@ static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm, /* check the return status and copy the data to the output buffer, if needed */ for (offb = offd = 0; offd < rsize && offb < actlen; offb += CMD_PACKET_SIZE) { if (rbuf[offb] != cm) { - dbg("wrong cm %#x in response", rbuf[offb]); + if (printk_ratelimit()) + usb_err(instance->usbatm, "wrong cm %#x in response to cm %#x\n", + rbuf[offb], cm); ret = -EIO; goto fail; } if (rbuf[offb + 1] != CM_STATUS_SUCCESS) { - dbg("response failed: %#x", rbuf[offb + 1]); + if (printk_ratelimit()) + usb_err(instance->usbatm, "response to cm %#x failed: %#x\n", + cm, rbuf[offb + 1]); ret = -EIO; goto fail; } @@ -582,14 +591,18 @@ static int cxacru_cm_get_array(struct cxacru_data *instance, enum cxacru_cm_requ for (offb = 0; offb < len; ) { int l = le32_to_cpu(buf[offb++]); if (l > stride || l > (len - offb) / 2) { - dbg("wrong data length %#x in response", l); + if (printk_ratelimit()) + usb_err(instance->usbatm, "invalid data length from cm %#x: %d\n", + cm, l); ret = -EIO; goto cleanup; } while (l--) { offd = le32_to_cpu(buf[offb++]); if (offd >= size) { - dbg("wrong index %#x in response", offd); + if (printk_ratelimit()) + usb_err(instance->usbatm, "wrong index #%x in response to cm #%x\n", + offd, cm); ret = -EIO; goto cleanup; } -- 1.5.0.1 -- Simon Arlott ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] cxacru: Use appropriate logging for errors 2007-09-23 15:32 [PATCH 1/2] cxacru: Use appropriate logging for errors Simon Arlott 2007-09-23 15:34 ` [PATCH 2/3] cxacru: Reduce initialisation delay Simon Arlott 2007-09-23 15:36 ` [PATCH 3/3] cxacru: Cleanup code by removing "ret = ret;" assignments Simon Arlott @ 2007-09-23 16:17 ` Duncan Sands 2007-09-23 18:44 ` [PATCH 1/2 (v2)] " Simon Arlott 2 siblings, 1 reply; 12+ messages in thread From: Duncan Sands @ 2007-09-23 16:17 UTC (permalink / raw) To: Simon Arlott; +Cc: Linux Kernel Mailing List, Greg Kroah-Hartman Hi Simon, don't these error messages (except the first) risk spamming the log if something goes wrong (like the modem being unplugged)? How about rate-limiting them, like usbatm does? Ciao, Duncan. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2 (v2)] cxacru: Use appropriate logging for errors 2007-09-23 16:17 ` [PATCH 1/2] " Duncan Sands @ 2007-09-23 18:44 ` Simon Arlott 2007-09-23 19:26 ` Duncan Sands 0 siblings, 1 reply; 12+ messages in thread From: Simon Arlott @ 2007-09-23 18:44 UTC (permalink / raw) To: Duncan Sands; +Cc: Linux Kernel Mailing List, Greg Kroah-Hartman When an error occurs, existing logging uses dbg() so the cause of a problem is hard to determine. Error conditions shouldn't only be properly reported with debugging enabled. A side effect of this change is that when an uninitialised device is started, a log message similar to the following is sent: cxacru 5-2:1.0: receive of cm 0x90 failed (-104) This is normal - the device did not respond so firmware will be loaded. Signed-Off-By: Simon Arlott <simon@fire.lp0.eu> --- On 23/09/07 17:17, Duncan Sands wrote: > Hi Simon, don't these error messages (except the first) risk spamming > the log if something goes wrong (like the modem being unplugged)? How > about rate-limiting them, like usbatm does? > > Ciao, > > Duncan. Ok. drivers/usb/atm/cxacru.c | 43 ++++++++++++++++++++++++++++--------------- 1 files changed, 28 insertions(+), 15 deletions(-) diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c index 1e5ee88..50249a5 100644 --- a/drivers/usb/atm/cxacru.c +++ b/drivers/usb/atm/cxacru.c @@ -482,7 +482,9 @@ static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm, int rbuflen = ((rsize - 1) / stride + 1) * CMD_PACKET_SIZE; if (wbuflen > PAGE_SIZE || rbuflen > PAGE_SIZE) { - dbg("too big transfer requested"); + if (printk_ratelimit()) + usb_err(instance->usbatm, "requested transfer size too large (%d, %d)\n", + wbuflen, rbuflen); ret = -ENOMEM; goto fail; } @@ -493,8 +495,9 @@ static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm, init_completion(&instance->rcv_done); ret = usb_submit_urb(instance->rcv_urb, GFP_KERNEL); if (ret < 0) { - dbg("submitting read urb for cm %#x failed", cm); - ret = ret; + if (printk_ratelimit()) + usb_err(instance->usbatm, "submit of read urb for cm %#x failed (%d)\n", + cm, ret); goto fail; } @@ -510,27 +513,29 @@ static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm, init_completion(&instance->snd_done); ret = usb_submit_urb(instance->snd_urb, GFP_KERNEL); if (ret < 0) { - dbg("submitting write urb for cm %#x failed", cm); - ret = ret; + if (printk_ratelimit()) + usb_err(instance->usbatm, "submit of write urb for cm %#x failed (%d)\n", + cm, ret); goto fail; } ret = cxacru_start_wait_urb(instance->snd_urb, &instance->snd_done, NULL); if (ret < 0) { - dbg("sending cm %#x failed", cm); - ret = ret; + if (printk_ratelimit()) + usb_err(instance->usbatm, "send of cm %#x failed (%d)\n", cm, ret); goto fail; } ret = cxacru_start_wait_urb(instance->rcv_urb, &instance->rcv_done, &actlen); if (ret < 0) { - dbg("receiving cm %#x failed", cm); - ret = ret; + if (printk_ratelimit()) + usb_err(instance->usbatm, "receive of cm %#x failed (%d)\n", cm, ret); goto fail; } if (actlen % CMD_PACKET_SIZE || !actlen) { - dbg("response is not a positive multiple of %d: %#x", - CMD_PACKET_SIZE, actlen); + if (printk_ratelimit()) + usb_err(instance->usbatm, "invalid response length to cm %#x: %d\n", + cm, actlen); ret = -EIO; goto fail; } @@ -538,12 +543,16 @@ static int cxacru_cm(struct cxacru_data *instance, enum cxacru_cm_request cm, /* check the return status and copy the data to the output buffer, if needed */ for (offb = offd = 0; offd < rsize && offb < actlen; offb += CMD_PACKET_SIZE) { if (rbuf[offb] != cm) { - dbg("wrong cm %#x in response", rbuf[offb]); + if (printk_ratelimit()) + usb_err(instance->usbatm, "wrong cm %#x in response to cm %#x\n", + rbuf[offb], cm); ret = -EIO; goto fail; } if (rbuf[offb + 1] != CM_STATUS_SUCCESS) { - dbg("response failed: %#x", rbuf[offb + 1]); + if (printk_ratelimit()) + usb_err(instance->usbatm, "response to cm %#x failed: %#x\n", + cm, rbuf[offb + 1]); ret = -EIO; goto fail; } @@ -582,14 +591,18 @@ static int cxacru_cm_get_array(struct cxacru_data *instance, enum cxacru_cm_requ for (offb = 0; offb < len; ) { int l = le32_to_cpu(buf[offb++]); if (l > stride || l > (len - offb) / 2) { - dbg("wrong data length %#x in response", l); + if (printk_ratelimit()) + usb_err(instance->usbatm, "invalid data length from cm %#x: %d\n", + cm, l); ret = -EIO; goto cleanup; } while (l--) { offd = le32_to_cpu(buf[offb++]); if (offd >= size) { - dbg("wrong index %#x in response", offd); + if (printk_ratelimit()) + usb_err(instance->usbatm, "wrong index #%x in response to cm #%x\n", + offd, cm); ret = -EIO; goto cleanup; } -- 1.5.0.1 -- Simon Arlott ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2 (v2)] cxacru: Use appropriate logging for errors 2007-09-23 18:44 ` [PATCH 1/2 (v2)] " Simon Arlott @ 2007-09-23 19:26 ` Duncan Sands 0 siblings, 0 replies; 12+ messages in thread From: Duncan Sands @ 2007-09-23 19:26 UTC (permalink / raw) To: Simon Arlott; +Cc: Linux Kernel Mailing List, Greg Kroah-Hartman > - dbg("too big transfer requested"); > + if (printk_ratelimit()) > + usb_err(instance->usbatm, "requested transfer size too large (%d, %d)\n", > + wbuflen, rbuflen); etc Acked-by: Duncan Sands <baldrick@free.fr> ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2007-09-25 19:20 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-09-23 15:32 [PATCH 1/2] cxacru: Use appropriate logging for errors Simon Arlott 2007-09-23 15:34 ` [PATCH 2/3] cxacru: Reduce initialisation delay Simon Arlott 2007-09-23 16:23 ` Duncan Sands 2007-09-23 18:33 ` Simon Arlott 2007-09-23 15:36 ` [PATCH 3/3] cxacru: Cleanup code by removing "ret = ret;" assignments Simon Arlott 2007-09-23 16:20 ` Duncan Sands 2007-09-23 18:36 ` Simon Arlott 2007-09-24 18:32 ` Greg KH 2007-09-25 19:20 ` [PATCH] cxacru: Use appropriate logging for errors Simon Arlott 2007-09-23 16:17 ` [PATCH 1/2] " Duncan Sands 2007-09-23 18:44 ` [PATCH 1/2 (v2)] " Simon Arlott 2007-09-23 19:26 ` Duncan Sands
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox