Staging: emxx_udc: Fix checkpatch.pl warnings and remove extra code

PATCH In review Haneen Mohammed 2 participants 12 messages Mar 2, 2015 – Mar 7, 2015 Staging

Share & tools

Haneen Mohammed

author

2015-03-02 18:25 UTC #0 lore raw

v10 cover Staging: emxx_udc: Fix checkpatch.pl warnings and remove extra code

This patchset fixes the following issues:
Remove argument test from function
Fix checkpatch.pl warning: printk(KERNEL_ERR not prefered. clean dev_err logging

Changes in v10:
- Refresh against updated staging tree
- Separate a patch "removal of typedef'
- Added patch related to ERR( msg
Changes in v9:
- Edit patches commit msg
- Merged two patches into one "the removal of the use of macro"
- Justify the use of pr_err
Changes in v8:
- Edit first patch subject line.
Changes in v7:
- Add missing first patch in v6: removal of typedefs and volatile. Changes in v6:
- Merged two patches into one: remove typedef and volatile.
- Justified the use of pr_err instead of dev_err. Changes in v5:
- Fix spelling mistakes
- Justify the removal of volatile
- Replaced pr_err with dev_err when possible
Changes in v4:
- Patchset broken into smaller pathces
Changes in v3:
- Create patchset from two patches
Changes in v2:
- First patch reversion

Haneen Mohammed (4):
Staging: emxx_udc: Remove argument test from function
Staging: emxx_udc: Replace custom printk macro ERR with dev_err or
pr_err
Staging: emxx_udc: Remove custom printk macro ERR
Staging: emxx_udc: Clean dev_err() logging

 drivers/staging/emxx_udc/emxx_udc.c | 83 +++++++++++++++++--------------------
 drivers/staging/emxx_udc/emxx_udc.h |  1 -
 2 files changed, 39 insertions(+), 45 deletions(-)

Signature · 1 line

1.9.1

Haneen Mohammed

author

2015-03-02 18:33 UTC #1 lore raw

PATCH v10 1/4 Staging: emxx_udc: Remove argument test from function

This patch removes the test statement for an argument to _nbu2ss_pullup function, for it can't be null due to previous derefrences.

Signed-off-by Haneen Mohammed <[email protected]>

---
v9: integrate this patch with emxx_udc patchset

 drivers/staging/emxx_udc/emxx_udc.c | 5 -----
 1 file changed, 5 deletions(-)

drivers/staging/emxx_udc/emxx_udc.c +0 -5

index a9bbf8e..4cc6296 100644
@@ -2257,11 +2257,6 @@ static int _nbu2ss_pullup(struct nbu2ss_udc *udc, int is_on)
 {
 	u32	reg_dt;
 
-	if (!udc) {
-		ERR("%s, bad param\n", __func__);
-		return -EINVAL;
-	}
-
 	if (udc->vbus_active == 0)
 		return -ESHUTDOWN;

Signature · 1 line

1.9.1

Haneen Mohammed

author

2015-03-02 18:37 UTC #2 lore raw

PATCH v10 2/4 Staging: emxx_udc: Replace custom printk macro ERR with dev_err or pr_err

In reply to Haneen Mohammed

This patch removes the use of custom printk macros ERR and replace it with dev_err, or pr_err in the following cases:
- if no appropriate struct device *dev field where found for dev_err.
- or dev could be null eg. "dev_err(udc->dev" not possible inside "if (udc == null)"
Issue addressed by checkpatch.pl.

Signed-off-by Haneen Mohammed <[email protected]>

---

v10: nothing
v9:  merged two patches into one, and justify the use of pr_err

 drivers/staging/emxx_udc/emxx_udc.c | 80 +++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 39 deletions(-)

drivers/staging/emxx_udc/emxx_udc.c +41 ~~-39~~

index 4cc6296..35dc1c4 100644
@@ -115,7 +115,7 @@ static void _nbu2ss_dump_register(struct nbu2ss_udc *udc)
 	pr_info("=== %s()\n", __func__);
 
 	if (udc == NULL) {
-		ERR("%s udc == NULL\n", __func__);
+		pr_err("%s udc == NULL\n", __func__);
 		return;
 	}

@@ -808,7 +808,7 @@ static int _nbu2ss_ep0_out_transfer(
 		return 0;		/* Short Packet Transfer End */
 
 	if (req->req.actual > req->req.length) {
-		ERR(" *** Overrun Error\n");
+		dev_err(udc->dev, " *** Overrun Error\n");
 		return -EOVERFLOW;
 	}

@@ -1026,8 +1026,8 @@ static int _nbu2ss_epn_out_transfer(
 	}
 
 	if (req->req.actual > req->req.length) {
-		ERR(" *** Overrun Error\n");
-		ERR(" *** actual = %d, length = %d\n",
+		dev_err(udc->dev, " Overrun Error\n");
+		dev_err(udc->dev, " actual = %d, length = %d\n",
 			req->req.actual, req->req.length);
 		result = -EOVERFLOW;
 	}

@@ -1638,7 +1638,7 @@ static int std_req_get_status(struct nbu2ss_udc *udc)
 		_nbu2ss_ep0_in_transfer(udc, &udc->ep[0], &udc->ep0_req);
 
 	} else {
-		ERR("*** Error GET_STATUS\n");
+		dev_err(udc->dev, " Error GET_STATUS\n");
 	}
 
 	return result;

@@ -2345,7 +2345,7 @@ static int _nbu2ss_enable_controller(struct nbu2ss_udc *udc)
 		waitcnt++;
 		udelay(1);	/* 1us wait */
 		if (waitcnt == EPC_PLL_LOCK_COUNT) {
-			ERR("*** Reset Cancel failed\n");
+			dev_err(udc->dev, "*** Reset Cancel failed\n");
 			return -EINVAL;
 		}
 	};

@@ -2607,13 +2607,13 @@ static int nbu2ss_ep_enable(
 	struct nbu2ss_udc	*udc;
 
 	if ((_ep == NULL) || (desc == NULL)) {
-		ERR(" *** %s, bad param\n", __func__);
+		pr_err(" *** %s, bad param\n", __func__);
 		return -EINVAL;
 	}
 
 	ep = container_of(_ep, struct nbu2ss_ep, ep);
 	if ((ep == NULL) || (ep->udc == NULL)) {
-		ERR(" *** %s, ep == NULL !!\n", __func__);
+		pr_err(" *** %s, ep == NULL !!\n", __func__);
 		return -EINVAL;
 	}

@@ -2621,7 +2621,7 @@ static int nbu2ss_ep_enable(
 	if ((ep_type == USB_ENDPOINT_XFER_CONTROL)
 		|| (ep_type == USB_ENDPOINT_XFER_ISOC)) {
 
-		ERR(" *** %s, bat bmAttributes\n", __func__);
+		pr_err(" *** %s, bat bmAttributes\n", __func__);
 		return -EINVAL;
 	}

@@ -2632,7 +2632,7 @@ static int nbu2ss_ep_enable(
 	if ((udc->driver == NULL)
 		|| (udc->gadget.speed == USB_SPEED_UNKNOWN)) {
 
-		ERR(" *** %s, udc !!\n", __func__);
+		dev_err(ep->udc->dev, " *** %s, udc !!\n", __func__);
 		return -ESHUTDOWN;
 	}

@@ -2667,13 +2667,13 @@ static int nbu2ss_ep_disable(struct usb_ep *_ep)
 	unsigned long		flags;
 
 	if (_ep == NULL) {
-		ERR(" *** %s, bad param\n", __func__);
+		pr_err(" *** %s, bad param\n", __func__);
 		return -EINVAL;
 	}
 
 	ep = container_of(_ep, struct nbu2ss_ep, ep);
 	if ((ep == NULL) || (ep->udc == NULL)) {
-		ERR(" *** %s, ep == NULL !!\n", __func__);
+		pr_err("udc: *** %s, ep == NULL !!\n", __func__);
 		return -EINVAL;
 	}

@@ -2737,10 +2737,10 @@ static int nbu2ss_ep_queue(
 	/* catch various bogus parameters */
 	if ((_ep == NULL) || (_req == NULL)) {
 		if (_ep == NULL)
-			ERR("*** %s --- _ep == NULL\n", __func__);
+			pr_err("udc: %s --- _ep == NULL\n", __func__);
 
 		if (_req == NULL)
-			ERR("*** %s --- _req == NULL\n", __func__);
+			pr_err("udc: %s --- _req == NULL\n", __func__);
 
 		return -EINVAL;
 	}

@@ -2751,13 +2751,13 @@ static int nbu2ss_ep_queue(
 	     || !list_empty(&req->queue))) {
 
 		if (!_req->complete)
-			ERR("*** %s --- !_req->complete\n", __func__);
+			pr_err("udc: %s --- !_req->complete\n", __func__);
 
 		if (!_req->buf)
-			ERR("*** %s --- !_req->buf\n", __func__);
+			pr_err("udc:%s --- !_req->buf\n", __func__);
 
 		if (!list_empty(&req->queue))
-			ERR("*** %s --- !list_empty(&req->queue)\n", __func__);
+			pr_err("%s --- !list_empty(&req->queue)\n", __func__);
 
 		return -EINVAL;
 	}

@@ -2773,7 +2773,8 @@ static int nbu2ss_ep_queue(
 	}
 
 	if (unlikely(!udc->driver)) {
-		ERR("%s, bogus device state %p\n", __func__, udc->driver);
+		dev_err(udc->dev, "%s, bogus device state %p\n", __func__,
+				udc->driver);
 		return -ESHUTDOWN;
 	}

@@ -2812,7 +2813,8 @@ static int nbu2ss_ep_queue(
 
 		result = _nbu2ss_start_transfer(udc, ep, req, FALSE);
 		if (result < 0) {
-			ERR(" *** %s, result = %d\n", __func__, result);
+			dev_err(udc->dev, " *** %s, result = %d\n", __func__,
+					result);
 			list_del(&req->queue);
 		} else if ((ep->epnum > 0) && (ep->direct == USB_DIR_OUT)) {
 #ifdef USE_DMA

@@ -2844,13 +2846,13 @@ static int nbu2ss_ep_dequeue(
 
 	/* catch various bogus parameters */
 	if ((_ep == NULL) || (_req == NULL)) {
-		/* ERR("%s, bad param(1)\n", __func__); */
+		/* pr_err("%s, bad param(1)\n", __func__); */
 		return -EINVAL;
 	}
 
 	ep = container_of(_ep, struct nbu2ss_ep, ep);
 	if (!ep) {
-		ERR("%s, ep == NULL !!\n", __func__);
+		pr_err("%s, ep == NULL !!\n", __func__);
 		return -EINVAL;
 	}

@@ -2890,19 +2892,19 @@ static int nbu2ss_ep_set_halt(struct usb_ep *_ep, int value)
 /*	INFO("=== %s()\n", __func__); */
 
 	if (!_ep) {
-		ERR("%s, bad param\n", __func__);
+		pr_err("%s, bad param\n", __func__);
 		return -EINVAL;
 	}
 
 	ep = container_of(_ep, struct nbu2ss_ep, ep);
 	if (!ep) {
-		ERR("%s, bad ep\n", __func__);
+		pr_err("%s, bad ep\n", __func__);
 		return -EINVAL;
 	}
 
 	udc = ep->udc;
 	if (!udc) {
-		ERR(" *** %s, bad udc\n", __func__);
+		dev_err(ep->udc->dev, " *** %s, bad udc\n", __func__);
 		return -EINVAL;
 	}

@@ -2944,19 +2946,19 @@ static int nbu2ss_ep_fifo_status(struct usb_ep *_ep)
 /*	INFO("=== %s()\n", __func__); */
 
 	if (!_ep) {
-		ERR("%s, bad param\n", __func__);
+		pr_err("%s, bad param\n", __func__);
 		return -EINVAL;
 	}
 
 	ep = container_of(_ep, struct nbu2ss_ep, ep);
 	if (!ep) {
-		ERR("%s, bad ep\n", __func__);
+		pr_err("%s, bad ep\n", __func__);
 		return -EINVAL;
 	}
 
 	udc = ep->udc;
 	if (!udc) {
-		ERR("%s, bad udc\n", __func__);
+		dev_err(ep->udc->dev, "%s, bad udc\n", __func__);
 		return -EINVAL;
 	}

@@ -2992,19 +2994,19 @@ static void  nbu2ss_ep_fifo_flush(struct usb_ep *_ep)
 /*	INFO("=== %s()\n", __func__); */
 
 	if (!_ep) {
-		ERR("%s, bad param\n", __func__);
+		pr_err("udc: %s, bad param\n", __func__);
 		return;
 	}
 
 	ep = container_of(_ep, struct nbu2ss_ep, ep);
 	if (!_ep) {
-		ERR("%s, bad ep\n", __func__);
+		pr_err("udc: %s, bad ep\n", __func__);
 		return;
 	}
 
 	udc = ep->udc;
 	if (!udc) {
-		ERR("%s, bad udc\n", __func__);
+		dev_err(ep->udc->dev, "%s, bad udc\n", __func__);
 		return;
 	}

@@ -3048,13 +3050,13 @@ static int nbu2ss_gad_get_frame(struct usb_gadget *pgadget)
 /*	INFO("=== %s()\n", __func__); */
 
 	if (pgadget == NULL) {
-		ERR("%s, bad param\n", __func__);
+		pr_err("udc: %s, bad param\n", __func__);
 		return -EINVAL;
 	}
 
 	udc = container_of(pgadget, struct nbu2ss_udc, gadget);
 	if (udc == NULL) {
-		ERR("%s, udc == NULL\n", __func__);
+		dev_err(&pgadget->dev, "%s, udc == NULL\n", __func__);
 		return -EINVAL;
 	}

@@ -3078,13 +3080,13 @@ static int nbu2ss_gad_wakeup(struct usb_gadget *pgadget)
 /*	INFO("=== %s()\n", __func__); */
 
 	if (pgadget == NULL) {
-		ERR("%s, bad param\n", __func__);
+		pr_err("%s, bad param\n", __func__);
 		return -EINVAL;
 	}
 
 	udc = container_of(pgadget, struct nbu2ss_udc, gadget);
 	if (udc == NULL) {
-		ERR("%s, udc == NULL\n", __func__);
+		dev_err(&pgadget->dev, "%s, udc == NULL\n", __func__);
 		return -EINVAL;
 	}

@@ -3118,7 +3120,7 @@ static int nbu2ss_gad_set_selfpowered(struct usb_gadget *pgadget,
 /*	INFO("=== %s()\n", __func__); */
 
 	if (pgadget == NULL) {
-		ERR("%s, bad param\n", __func__);
+		pr_err("%s, bad param\n", __func__);
 		return -EINVAL;
 	}

@@ -3147,7 +3149,7 @@ static int nbu2ss_gad_vbus_draw(struct usb_gadget *pgadget, unsigned mA)
 /*	INFO("=== %s()\n", __func__); */
 
 	if (pgadget == NULL) {
-		ERR("%s, bad param\n", __func__);
+		pr_err("%s, bad param\n", __func__);
 		return -EINVAL;
 	}

@@ -3169,7 +3171,7 @@ static int nbu2ss_gad_pullup(struct usb_gadget *pgadget, int is_on)
 /*	INFO("=== %s()\n", __func__); */
 
 	if (pgadget == NULL) {
-		ERR("%s, bad param\n", __func__);
+		pr_err("%s, bad param\n", __func__);
 		return -EINVAL;
 	}

@@ -3340,7 +3342,7 @@ static int nbu2ss_drv_probe(struct platform_device *pdev)
 
 	/* USB Function Controller Interrupt */
 	if (status != 0) {
-		ERR("request_irq(USB_UDC_IRQ_1) failed\n");
+		dev_err(udc->dev, "request_irq(USB_UDC_IRQ_1) failed\n");
 		goto cleanup1;
 	}

@@ -3360,7 +3362,7 @@ static int nbu2ss_drv_probe(struct platform_device *pdev)
 				udc);
 
 	if (status != 0) {
-		ERR("request_irq(INT_VBUS) failed\n");
+		dev_err(udc->dev, "request_irq(INT_VBUS) failed\n");
 		goto cleanup1;
 	}

Signature · 1 line

1.9.1

Haneen Mohammed

author

2015-03-02 18:44 UTC #3 lore raw

PATCH v10 3/4 Staging: emxx_udc: Remove custom printk macro ERR

In reply to Haneen Mohammed

This patch removes custom printk macro ERR.
All the calls to this macro were replaced by de_err and pr_err.

Signed-off-by Haneen Mohammed <[email protected]>

---
v9 and 10: nothing

 drivers/staging/emxx_udc/emxx_udc.h | 1 -
 1 file changed, 1 deletion(-)

drivers/staging/emxx_udc/emxx_udc.h +0 -1

index 57727c6..c19168f 100644
@@ -642,6 +642,5 @@ union usb_reg_access {
 };
 
 /*-------------------------------------------------------------------------*/
-#define ERR(stuff...)		printk(KERN_ERR "udc: " stuff)
 
 #endif  /* _LINUX_EMXX_H */

Signature · 1 line

1.9.1

Haneen Mohammed

author

2015-03-02 18:45 UTC #4 lore raw

PATCH v10 4/4 Staging: emxx_udc: Clean dev_err() logging

In reply to Haneen Mohammed

This patch removes __func__ from dev_err.
dev_err includes the function name in the log printout, so there is no need to include it manually.

Signed-off-by Haneen Mohammed <[email protected]>

---
v10: nothing.

 drivers/staging/emxx_udc/emxx_udc.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

drivers/staging/emxx_udc/emxx_udc.c +8 ~~-10~~

index 35dc1c4..cfe55dc 100644
@@ -2632,7 +2632,7 @@ static int nbu2ss_ep_enable(
 	if ((udc->driver == NULL)
 		|| (udc->gadget.speed == USB_SPEED_UNKNOWN)) {
 
-		dev_err(ep->udc->dev, " *** %s, udc !!\n", __func__);
+		dev_err(ep->udc->dev, "  udc !!\n");
 		return -ESHUTDOWN;
 	}

@@ -2773,8 +2773,7 @@ static int nbu2ss_ep_queue(
 	}
 
 	if (unlikely(!udc->driver)) {
-		dev_err(udc->dev, "%s, bogus device state %p\n", __func__,
-				udc->driver);
+		dev_err(udc->dev, "bogus device state %p\n", udc->driver);
 		return -ESHUTDOWN;
 	}

@@ -2813,8 +2812,7 @@ static int nbu2ss_ep_queue(
 
 		result = _nbu2ss_start_transfer(udc, ep, req, FALSE);
 		if (result < 0) {
-			dev_err(udc->dev, " *** %s, result = %d\n", __func__,
-					result);
+			dev_err(udc->dev, " ** result = %d\n", result);
 			list_del(&req->queue);
 		} else if ((ep->epnum > 0) && (ep->direct == USB_DIR_OUT)) {
 #ifdef USE_DMA

@@ -2904,7 +2902,7 @@ static int nbu2ss_ep_set_halt(struct usb_ep *_ep, int value)
 
 	udc = ep->udc;
 	if (!udc) {
-		dev_err(ep->udc->dev, " *** %s, bad udc\n", __func__);
+		dev_err(ep->udc->dev, " *** bad udc\n");
 		return -EINVAL;
 	}

@@ -2958,7 +2956,7 @@ static int nbu2ss_ep_fifo_status(struct usb_ep *_ep)
 
 	udc = ep->udc;
 	if (!udc) {
-		dev_err(ep->udc->dev, "%s, bad udc\n", __func__);
+		dev_err(ep->udc->dev, "bad udc\n");
 		return -EINVAL;
 	}

@@ -3006,7 +3004,7 @@ static void  nbu2ss_ep_fifo_flush(struct usb_ep *_ep)
 
 	udc = ep->udc;
 	if (!udc) {
-		dev_err(ep->udc->dev, "%s, bad udc\n", __func__);
+		dev_err(ep->udc->dev, "bad udc\n");
 		return;
 	}

@@ -3056,7 +3054,7 @@ static int nbu2ss_gad_get_frame(struct usb_gadget *pgadget)
 
 	udc = container_of(pgadget, struct nbu2ss_udc, gadget);
 	if (udc == NULL) {
-		dev_err(&pgadget->dev, "%s, udc == NULL\n", __func__);
+		dev_err(&pgadget->dev, "udc == NULL\n");
 		return -EINVAL;
 	}

@@ -3086,7 +3084,7 @@ static int nbu2ss_gad_wakeup(struct usb_gadget *pgadget)
 
 	udc = container_of(pgadget, struct nbu2ss_udc, gadget);
 	if (udc == NULL) {
-		dev_err(&pgadget->dev, "%s, udc == NULL\n", __func__);
+		dev_err(&pgadget->dev, "udc == NULL\n");
 		return -EINVAL;
 	}

Signature · 1 line

1.9.1

Greg KH

maintainer

2015-03-02 18:54 UTC #5 lore raw

Re: [Outreachy kernel] [PATCH v10 4/4] Staging: emxx_udc: Clean dev_err() logging

Quoted 3 lines · Haneen Mohammed

This patch removes __func__ from dev_err.
dev_err includes the function name in the log printout, so there is no need to include it manually.

No, dev_err() does not include the function name, it includes much more than that (device, driver, specific instance of device, etc.).

Quoted 5 lines

Signed-off-by Haneen Mohammed <[email protected]>
---
v10: nothing.

Then why send a v10?

confused,

greg k-h

Haneen Mohammed

author

2015-03-02 19:02 UTC #6 lore raw

Re: [Outreachy kernel] [PATCH v10 4/4] Staging: emxx_udc: Clean dev_err() logging

Quoted 8 lines · Greg KH

Quoted 3 lines · Haneen Mohammed
This patch removes __func__ from dev_err.
dev_err includes the function name in the log printout, so there is no need to include it manually.
No, dev_err() does not include the function name, it includes much more than that (device, driver, specific instance of device, etc.).

Noted. I learned about this from the logs.
should I edit the commit msg and resed the patch?

Quoted 11 lines

Quoted 5 lines
Signed-off-by Haneen Mohammed <[email protected]>
---
v10: nothing.
Then why send a v10?
confused,
greg k-h

Becuase It was included in the patchset.

Thank you,
Haneen

Greg KH

maintainer

2015-03-03 18:46 UTC #7 lore raw

Re: [Outreachy kernel] [PATCH v10 4/4] Staging: emxx_udc: Clean dev_err() logging

Quoted 11 lines · Haneen Mohammed

Quoted 8 lines · Greg KH
Quoted 3 lines · Haneen Mohammed
This patch removes __func__ from dev_err.
dev_err includes the function name in the log printout, so there is no need to include it manually.
No, dev_err() does not include the function name, it includes much more than that (device, driver, specific instance of device, etc.).
Noted. I learned about this from the logs.
should I edit the commit msg and resed the patch?

Yes please.

Haneen Mohammed

author

2015-03-04 09:49 UTC #8 lore raw

PATCH 1/1 Staging: emxx_udc: Clean dev_err() logging

In reply to Haneen Mohammed

This patch removes __func__ from dev_err. dev_err includes information about:
(devcice, driver, specific instance of device, etc) in the log printout, so there is no need for __func__.

Signed-off-by Haneen Mohammed <[email protected]>

---
note: this patch separated from "Staging: emxx_udc: Fix checkpatch.pl warnings" patchset.

 drivers/staging/emxx_udc/emxx_udc.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

drivers/staging/emxx_udc/emxx_udc.c +8 ~~-10~~

index 35dc1c4..cfe55dc 100644
@@ -2632,7 +2632,7 @@ static int nbu2ss_ep_enable(
 	if ((udc->driver == NULL)
 		|| (udc->gadget.speed == USB_SPEED_UNKNOWN)) {
 
-		dev_err(ep->udc->dev, " *** %s, udc !!\n", __func__);
+		dev_err(ep->udc->dev, "  udc !!\n");
 		return -ESHUTDOWN;
 	}

@@ -2773,8 +2773,7 @@ static int nbu2ss_ep_queue(
 	}
 
 	if (unlikely(!udc->driver)) {
-		dev_err(udc->dev, "%s, bogus device state %p\n", __func__,
-				udc->driver);
+		dev_err(udc->dev, "bogus device state %p\n", udc->driver);
 		return -ESHUTDOWN;
 	}

@@ -2813,8 +2812,7 @@ static int nbu2ss_ep_queue(
 
 		result = _nbu2ss_start_transfer(udc, ep, req, FALSE);
 		if (result < 0) {
-			dev_err(udc->dev, " *** %s, result = %d\n", __func__,
-					result);
+			dev_err(udc->dev, " ** result = %d\n", result);
 			list_del(&req->queue);
 		} else if ((ep->epnum > 0) && (ep->direct == USB_DIR_OUT)) {
 #ifdef USE_DMA

@@ -2904,7 +2902,7 @@ static int nbu2ss_ep_set_halt(struct usb_ep *_ep, int value)
 
 	udc = ep->udc;
 	if (!udc) {
-		dev_err(ep->udc->dev, " *** %s, bad udc\n", __func__);
+		dev_err(ep->udc->dev, " *** bad udc\n");
 		return -EINVAL;
 	}

@@ -2958,7 +2956,7 @@ static int nbu2ss_ep_fifo_status(struct usb_ep *_ep)
 
 	udc = ep->udc;
 	if (!udc) {
-		dev_err(ep->udc->dev, "%s, bad udc\n", __func__);
+		dev_err(ep->udc->dev, "bad udc\n");
 		return -EINVAL;
 	}

@@ -3006,7 +3004,7 @@ static void  nbu2ss_ep_fifo_flush(struct usb_ep *_ep)
 
 	udc = ep->udc;
 	if (!udc) {
-		dev_err(ep->udc->dev, "%s, bad udc\n", __func__);
+		dev_err(ep->udc->dev, "bad udc\n");
 		return;
 	}

@@ -3056,7 +3054,7 @@ static int nbu2ss_gad_get_frame(struct usb_gadget *pgadget)
 
 	udc = container_of(pgadget, struct nbu2ss_udc, gadget);
 	if (udc == NULL) {
-		dev_err(&pgadget->dev, "%s, udc == NULL\n", __func__);
+		dev_err(&pgadget->dev, "udc == NULL\n");
 		return -EINVAL;
 	}

@@ -3086,7 +3084,7 @@ static int nbu2ss_gad_wakeup(struct usb_gadget *pgadget)
 
 	udc = container_of(pgadget, struct nbu2ss_udc, gadget);
 	if (udc == NULL) {
-		dev_err(&pgadget->dev, "%s, udc == NULL\n", __func__);
+		dev_err(&pgadget->dev, "udc == NULL\n");
 		return -EINVAL;
 	}

Signature · 1 line

1.9.1

Greg KH

maintainer

2015-03-07 10:55 UTC #9 lore raw

Re: [Outreachy kernel] [PATCH] Staging: emxx_udc: Clean dev_err() logging

Quoted 15 lines · Haneen Mohammed

This patch removes __func__ from dev_err. dev_err includes information about:
(devcice, driver, specific instance of device, etc) in the log printout, so there is no need for __func__.
Signed-off-by Haneen Mohammed <[email protected]>
---
note: this patch separated from "Staging: emxx_udc: Fix checkpatch.pl warnings" patchset.
 drivers/staging/emxx_udc/emxx_udc.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)
drivers/staging/emxx_udc/emxx_udc.c +0 -0
index 35dc1c4..cfe55dc 100644

drivers/staging/emxx_udc/emxx_udc.c static int nbu2ss_ep_enable( · line 2635

Quoted 5 lines

drivers/staging/emxx_udc/emxx_udc.c +1 -1

@@ -2632,7 +2632,7 @@ static int nbu2ss_ep_enable(
 	if ((udc->driver == NULL)
 		|| (udc->gadget.speed == USB_SPEED_UNKNOWN)) {
 
-		dev_err(ep->udc->dev, " *** %s, udc !!\n", __func__);
+		dev_err(ep->udc->dev, "  udc !!\n");

Why keep the leading spaces?

And dev_err() does not have __func__ in it, so maybe this should just be left alone, as these messages are not unique enough to determine the exact place the error happened.

sorry for missing that last time.

greg k-h

Haneen Mohammed

author

2015-03-07 17:08 UTC #10 lore raw

Re: [Outreachy kernel] [PATCH] Staging: emxx_udc: Clean dev_err() logging

Quoted 24 lines · Greg KH

Quoted 15 lines · Haneen Mohammed
This patch removes __func__ from dev_err. dev_err includes information about:
(devcice, driver, specific instance of device, etc) in the log printout, so there is no need for __func__.
Signed-off-by Haneen Mohammed <[email protected]>
---
note: this patch separated from "Staging: emxx_udc: Fix checkpatch.pl warnings" patchset.
 drivers/staging/emxx_udc/emxx_udc.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)
drivers/staging/emxx_udc/emxx_udc.c +0 -0
index 35dc1c4..cfe55dc 100644
drivers/staging/emxx_udc/emxx_udc.c static int nbu2ss_ep_enable( · line 2635
Quoted 5 lines
drivers/staging/emxx_udc/emxx_udc.c +1 -1
@@ -2632,7 +2632,7 @@ static int nbu2ss_ep_enable(
 	if ((udc->driver == NULL)
 		|| (udc->gadget.speed == USB_SPEED_UNKNOWN)) {
 
-		dev_err(ep->udc->dev, " *** %s, udc !!\n", __func__);
+		dev_err(ep->udc->dev, "  udc !!\n");
Why keep the leading spaces?

Quoted 7 lines

And dev_err() does not have __func__ in it, so maybe this should just be left alone, as these messages are not unique enough to determine the exact place the error happened.
sorry for missing that last time.
greg k-h

what about the other patches? You have told me to refresh my tree and resubmit the other patchset, should I do that? or just leave it as it is?

Thank you,
Haneen

Greg KH

maintainer

2015-03-07 17:11 UTC #11 lore raw

Re: [Outreachy kernel] [PATCH] Staging: emxx_udc: Clean dev_err() logging

Quoted 29 lines · Haneen Mohammed

Quoted 28 lines · Greg KH
Quoted 15 lines · Haneen Mohammed
This patch removes __func__ from dev_err. dev_err includes information about:
(devcice, driver, specific instance of device, etc) in the log printout, so there is no need for __func__.
Signed-off-by Haneen Mohammed <[email protected]>
---
note: this patch separated from "Staging: emxx_udc: Fix checkpatch.pl warnings" patchset.
 drivers/staging/emxx_udc/emxx_udc.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)
drivers/staging/emxx_udc/emxx_udc.c +0 -0
index 35dc1c4..cfe55dc 100644
drivers/staging/emxx_udc/emxx_udc.c static int nbu2ss_ep_enable( · line 2635
Quoted 5 lines
drivers/staging/emxx_udc/emxx_udc.c +1 -1
@@ -2632,7 +2632,7 @@ static int nbu2ss_ep_enable(
 	if ((udc->driver == NULL)
 		|| (udc->gadget.speed == USB_SPEED_UNKNOWN)) {
 
-		dev_err(ep->udc->dev, " *** %s, udc !!\n", __func__);
+		dev_err(ep->udc->dev, "  udc !!\n");
Why keep the leading spaces?
And dev_err() does not have __func__ in it, so maybe this should just be left alone, as these messages are not unique enough to determine the exact place the error happened.

Quoted 6 lines

Quoted 3 lines
sorry for missing that last time.
greg k-h
what about the other patches? You have told me to refresh my tree and resubmit the other patchset, should I do that? or just leave it as it is?

I'd say leave it as-is and start working on more "real" patches.

thanks,

greg k-h