On Mon, Oct 25, 2010 at 07:24:25PM +0900, Toshiharu Okada wrote:
+ if (!list_empty(&ep->queue)) {
+ dev_dbg(&dev->pdev->dev, "%s: list not empty", __func__);
+ ret = -EAGAIN;
+ } else if (!halt) { /* halt or clear halt */
+ pch_udc_ep_clear_stall(ep);
+ ret = 0;
+ } else {
+ if (ep->num == PCH_UDC_EP0)
+ ep->dev->stall = 1;
+ pch_udc_ep_set_stall(ep);
+ pch_udc_enable_ep_interrupts(ep->dev,
+ PCH_UDC_EPINT(ep->in, ep->num));
+ ret = 0;
+ }
On Wed, 03 Nov 2010 19:37:50 +0100, Andy Isaacson <adi@xxxxxxxxxxxxx> wrote:
I think this would be clearer as
+ if (!list_empty(&ep->queue)) {
+ dev_dbg(&dev->pdev->dev, "%s: list not empty", __func__);
+ ret = -EAGAIN;
+ }
+ if (halt) {
+ if (ep->num == PCH_UDC_EP0)
+ ep->dev->stall = 1;
+ pch_udc_ep_set_stall(ep);
+ pch_udc_enable_ep_interrupts(ep->dev,
+ PCH_UDC_EPINT(ep->in, ep->num));
+ ret = 0;
+ } else {
+ pch_udc_ep_clear_stall(ep);
+ ret = 0;
+ }
This changes the behavior of the construct though.
because:
1. if (!list_empty is a standalone check, there is no if/else if/else
connection between list_empty() and halt.
This depends on how you look at it. The way I see it is that there are three
mutually exclusive cases: the list is not empty, it is a halt, it is not a
halt. This way, if-else-if-else seems like a good construct to me.
2. I prefer if (foo) {} else {} instead of if (!foo) {} else {}, unless
there is a significant reason to do the negated test.
I agree on that though.
+ pr_debug("%s: %s", __func__, usbep->name);
There are probably too many pr_debug() and dev_dbg()s in this driver.
Please reconsider which ones are appropriate for mainline.
Do we really care? Just don't define DEBUG...
--
Best regards, _ _
| Humble Liege of Serenely Enlightened Majesty of o' \,=./ `o
| Computer Science, MichaÅ "mina86" Nazarewicz (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
