On 28. Feb 2022, at 12:24, Dan Carpenter dan.carpenter@oracle.com wrote:
On Mon, Feb 28, 2022 at 12:08:17PM +0100, Jakob Koschel wrote:
diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c index 9040a0561466..0fd0307bc07b 100644 --- a/drivers/usb/gadget/udc/at91_udc.c +++ b/drivers/usb/gadget/udc/at91_udc.c @@ -150,13 +150,14 @@ static void proc_ep_show(struct seq_file *s, struct at91_ep *ep) if (list_empty (&ep->queue)) seq_printf(s, "\t(queue empty)\n");
- else list_for_each_entry (req, &ep->queue, queue) {
unsigned length = req->req.actual;
- else
list_for_each_entry(req, &ep->queue, queue) {unsigned int length = req->req.actual;
seq_printf(s, "\treq %p len %d/%d buf %p\n",&req->req, length,req->req.length, req->req.buf);- }
seq_printf(s, "\treq %p len %d/%d buf %p\n",&req->req, length,req->req.length, req->req.buf);}Don't make unrelated white space changes. It just makes the patch harder to review. As you're writing the patch make note of any additional changes and do them later in a separate patch.
Also a multi-line indent gets curly braces for readability even though it's not required by C. And then both sides would get curly braces.
spin_unlock_irqrestore(&udc->lock, flags); }
@@ -226,7 +227,7 @@ static int proc_udc_show(struct seq_file *s, void *unused)
if (udc->enabled && udc->vbus) { proc_ep_show(s, &udc->ep[0]);
list_for_each_entry (ep, &udc->gadget.ep_list, ep.ep_list) {
list_for_each_entry(ep, &udc->gadget.ep_list, ep.ep_list) {Another unrelated change.
if (ep->ep.desc) proc_ep_show(s, ep); }[ snip ]
Thanks for pointing out, I'll remove the changes here and note them down to send them separately.
diff --git a/drivers/usb/gadget/udc/net2272.c b/drivers/usb/gadget/udc/net2272.c index 7c38057dcb4a..bb59200f1596 100644 --- a/drivers/usb/gadget/udc/net2272.c +++ b/drivers/usb/gadget/udc/net2272.c @@ -926,7 +926,8 @@ static int net2272_dequeue(struct usb_ep *_ep, struct usb_request *_req) { struct net2272_ep *ep;
- struct net2272_request *req;
- struct net2272_request *req = NULL;
- struct net2272_request *tmp; unsigned long flags; int stopped;
@@ -939,11 +940,13 @@ net2272_dequeue(struct usb_ep *_ep, struct usb_request *_req) ep->stopped = 1;
/* make sure it's still queued on this endpoint */
- list_for_each_entry(req, &ep->queue, queue) {
if (&req->req == _req)
- list_for_each_entry(tmp, &ep->queue, queue) {
if (&tmp->req == _req) {req = tmp; break;}
- if (&req->req != _req) {
- if (!req) { ep->stopped = stopped; spin_unlock_irqrestore(&ep->dev->lock, flags); return -EINVAL;
@@ -954,7 +957,6 @@ net2272_dequeue(struct usb_ep *_ep, struct usb_request *_req) dev_dbg(ep->dev->dev, "unlink (%s) pio\n", _ep->name); net2272_done(ep, req, -ECONNRESET); }
- req = NULL;
Another unrelated change. These are all good changes but send them as separate patches.
You are referring to the req = NULL, right?
I've changed the use of 'req' in the same function and assumed that I can just remove the unnecessary statement. But if it's better to do separately I'll do that.
ep->stopped = stopped;
spin_unlock_irqrestore(&ep->dev->lock, flags);
regards, dan carpenter
thanks, Jakob Koschel