[PATCH v10 0/4] Staging: emxx_udc: Fix checkpatch.pl warnings and remove extra code

[PATCH v10 0/4] Staging: emxx_udc: Fix checkpatch.pl warnings and remove extra code

[Haneen Mohammed]

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(-)

-- 
1.9.1

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

[Haneen Mohammed]

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 <hamohammed.sa@gmail.com>
---
v9: integrate this patch with emxx_udc patchset

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

diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
index a9bbf8e..4cc6296 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -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;
 
-- 
1.9.1

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

[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 <hamohammed.sa@gmail.com>
---
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(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
index 4cc6296..35dc1c4 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -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;
 	}
 
-- 
1.9.1

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

[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 <hamohammed.sa@gmail.com>
---
v9 and 10: nothing

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

diff --git a/drivers/staging/emxx_udc/emxx_udc.h b/drivers/staging/emxx_udc/emxx_udc.h
index 57727c6..c19168f 100644
--- a/drivers/staging/emxx_udc/emxx_udc.h
+++ b/drivers/staging/emxx_udc/emxx_udc.h
@@ -642,6 +642,5 @@ union usb_reg_access {
 };
 
 /*-------------------------------------------------------------------------*/
-#define ERR(stuff...)		printk(KERN_ERR "udc: " stuff)
 
 #endif  /* _LINUX_EMXX_H */
-- 
1.9.1

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

[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 <hamohammed.sa@gmail.com>
---
v10: nothing. 

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

diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
index 35dc1c4..cfe55dc 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -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;
 	}
 
-- 
1.9.1

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

[Greg KH]

On Mon, Mar 02, 2015 at 09:45:23PM +0300, Haneen Mohammed wrote:
> 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.).

> 
> Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com>
> ---
> v10: nothing. 
> 

Then why send a v10?

confused,

greg k-h

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

[Haneen Mohammed]

On Mon, Mar 02, 2015 at 10:54:12AM -0800, Greg KH wrote:
> On Mon, Mar 02, 2015 at 09:45:23PM +0300, Haneen Mohammed wrote:
> > 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?
> > 
> > Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com>
> > ---
> > v10: nothing. 
> > 
> 
> Then why send a v10?
> 
> confused,
> 
> greg k-h
Becuase It was included in the patchset.

Thank you,
Haneen

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

[Greg KH]

On Mon, Mar 02, 2015 at 10:02:28PM +0300, Haneen Mohammed wrote:
> On Mon, Mar 02, 2015 at 10:54:12AM -0800, Greg KH wrote:
> > On Mon, Mar 02, 2015 at 09:45:23PM +0300, Haneen Mohammed wrote:
> > > 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.

[PATCH] Staging: emxx_udc: Clean dev_err() logging

[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 <hamohammed.sa@gmail.com>
---
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(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
index 35dc1c4..cfe55dc 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -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;
 	}
 
-- 
1.9.1

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

[Greg KH]

On Wed, Mar 04, 2015 at 12:10:24PM +0300, Haneen Mohammed wrote:
> 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 <hamohammed.sa@gmail.com>
> ---
> 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(-)
> 
> diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
> index 35dc1c4..cfe55dc 100644
> --- a/drivers/staging/emxx_udc/emxx_udc.c
> +++ b/drivers/staging/emxx_udc/emxx_udc.c
> @@ -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

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

[Haneen Mohammed]

On Fri, Mar 06, 2015 at 03:29:51PM -0800, Greg KH wrote:
> On Wed, Mar 04, 2015 at 12:10:24PM +0300, Haneen Mohammed wrote:
> > 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 <hamohammed.sa@gmail.com>
> > ---
> > 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(-)
> > 
> > diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
> > index 35dc1c4..cfe55dc 100644
> > --- a/drivers/staging/emxx_udc/emxx_udc.c
> > +++ b/drivers/staging/emxx_udc/emxx_udc.c
> > @@ -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

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

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

[Greg KH]

On Sat, Mar 07, 2015 at 08:08:07PM +0300, Haneen Mohammed wrote:
> On Fri, Mar 06, 2015 at 03:29:51PM -0800, Greg KH wrote:
> > On Wed, Mar 04, 2015 at 12:10:24PM +0300, Haneen Mohammed wrote:
> > > 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 <hamohammed.sa@gmail.com>
> > > ---
> > > 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(-)
> > > 
> > > diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
> > > index 35dc1c4..cfe55dc 100644
> > > --- a/drivers/staging/emxx_udc/emxx_udc.c
> > > +++ b/drivers/staging/emxx_udc/emxx_udc.c
> > > @@ -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
> 
> 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