[PATCH 0/8] usb: gadget: dummy_hcd: coding style improvements and error handling change

[PATCH 0/8] usb: gadget: dummy_hcd: coding style improvements and error handling change

[Clint George]

This patch series focuses on addressing various coding style issues in
the dummy_hcd USB gadget driver. The changes include simplifying error
handling by preventing kernel-space crashes, improving readability, and
ensuring consistency with kernel coding conventions.

Clint George (8):
  usb: gadget: dummy_hcd: replace BUG() with WARN_ON_ONCE()
  usb: gadget: dummy_hcd: replace symbolic permissions (S_IRUGO) with
    octal (0444)
  usb: gadget: dummy_hcd: use 'unsigned int' instead of bare 'unsigned'
  usb: gadget: dummy_hcd: fix block comments, blank lines and function
    braces
  usb: gadget: dummy_hcd: merge multi-line quoted strings into one line
  usb: gadget: dummy_hcd: use sizeof(*ptr) instead of sizeof *ptr
  usb: gadget: dummy_hcd: remove unnecessary 'else' after return
  usb: gadget: dummy_hcd: fix miscellaneous coding style warnings

 drivers/usb/gadget/udc/dummy_hcd.c | 139 ++++++++++++++---------------
 1 file changed, 67 insertions(+), 72 deletions(-)

-- 

Testing:
-  Verified dummy_hcd module loading and behavior by checking dmesg logs
for expected output.
- Ran compiler testing with no new warnings detected.
- Ensured the module builds and inserts cleanly without issues.
- Used Static Analysis tools to confirm no new issues were introduced.

Please review the changes and let me know if any modifications
or further testing of the module is required. 

Thanks,
Clint

2.43.0

[PATCH 1/8] usb: gadget: dummy_hcd: replace BUG() with WARN_ON_ONCE()

[Clint George]

Replace BUG() with WARN_ON_ONCE() in dummy_validate_stream()
when stream_id exceeds max_streams. This allows the kernel to
continue running with a warning instead of crashing.

Signed-off-by: Clint George <clintbgeorge@gmail.com>
---

Testing:
- The function dummy_validate_stream() was tested using a test module
  that i created where i sent value of urb->stream_id greater than
  max_streams. When using BUG(), the kernel-space used to crash but
  after using WARN_ON_ONCE() the kernel-space does not crash and the
  module terminates gracefully
- Ensured that the module builds properly

 drivers/usb/gadget/udc/dummy_hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 1cefca660..41b7b6907 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1254,7 +1254,7 @@ static int dummy_validate_stream(struct dummy_hcd *dum_hcd, struct urb *urb)
 	if (urb->stream_id > max_streams) {
 		dev_err(dummy_dev(dum_hcd), "Stream id %d is out of range.\n",
 				urb->stream_id);
-		BUG();
+		WARN_ON_ONCE(1);
 		return -EINVAL;
 	}
 	return 0;
-- 
2.43.0

[PATCH 2/8] usb: gadget: dummy_hcd: replace symbolic permissions (S_IRUGO) with octal (0444)

[Clint George]

Replace existing symbolic permissions S_IRUGO (Read-only) with octal
permission 0444. This makes it much easier to read and and is consistent
with the kernel coding style.

Signed-off-by: Clint George <clintbgeorge@gmail.com>
---
 drivers/usb/gadget/udc/dummy_hcd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 41b7b6907..1369b9613 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -73,11 +73,11 @@ static struct dummy_hcd_module_parameters mod_data = {
 	.is_high_speed = true,
 	.num = 1,
 };
-module_param_named(is_super_speed, mod_data.is_super_speed, bool, S_IRUGO);
+module_param_named(is_super_speed, mod_data.is_super_speed, bool, 0444);
 MODULE_PARM_DESC(is_super_speed, "true to simulate SuperSpeed connection");
-module_param_named(is_high_speed, mod_data.is_high_speed, bool, S_IRUGO);
+module_param_named(is_high_speed, mod_data.is_high_speed, bool, 0444);
 MODULE_PARM_DESC(is_high_speed, "true to simulate HighSpeed connection");
-module_param_named(num, mod_data.num, uint, S_IRUGO);
+module_param_named(num, mod_data.num, uint, 0444);
 MODULE_PARM_DESC(num, "number of emulated controllers");
 /*-------------------------------------------------------------------------*/
 
-- 
2.43.0

[PATCH 3/8] usb: gadget: dummy_hcd: use 'unsigned int' instead of bare 'unsigned'

[Clint George]

Use 'unsigned int' instead of 'unsigned' wherever possible to maintain
consistency with the kernel coding style.

Signed-off-by: Clint George <clintbgeorge@gmail.com>
---
 drivers/usb/gadget/udc/dummy_hcd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 1369b9613..6ad366640 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -506,7 +506,7 @@ static int dummy_enable(struct usb_ep *_ep,
 	struct dummy		*dum;
 	struct dummy_hcd	*dum_hcd;
 	struct dummy_ep		*ep;
-	unsigned		max;
+	unsigned int		max;
 	int			retval;
 
 	ep = usb_ep_to_dummy_ep(_ep);
@@ -1414,7 +1414,7 @@ static int transfer(struct dummy_hcd *dum_hcd, struct urb *urb,
 top:
 	/* if there's no request queued, the device is NAKing; return */
 	list_for_each_entry(req, &ep->queue, queue) {
-		unsigned	host_len, dev_len, len;
+		unsigned int	host_len, dev_len, len;
 		int		is_short, to_host;
 		int		rescan = 0;
 
@@ -1443,7 +1443,7 @@ static int transfer(struct dummy_hcd *dum_hcd, struct urb *urb,
 			/* not enough bandwidth left? */
 			if (limit < ep->ep.maxpacket && limit < len)
 				break;
-			len = min_t(unsigned, len, limit);
+			len = min_t(unsigned int, len, limit);
 			if (len == 0)
 				break;
 
@@ -1624,8 +1624,8 @@ static int handle_control_request(struct dummy_hcd *dum_hcd, struct urb *urb,
 	struct dummy_ep		*ep2;
 	struct dummy		*dum = dum_hcd->dum;
 	int			ret_val = 1;
-	unsigned	w_index;
-	unsigned	w_value;
+	unsigned int	w_index;
+	unsigned int	w_value;
 
 	w_index = le16_to_cpu(setup->wIndex);
 	w_value = le16_to_cpu(setup->wValue);
-- 
2.43.0

[PATCH 4/8] usb: gadget: dummy_hcd: fix block comments, blank lines and function braces

[Clint George]

This patch updates dummy_hcd.c to follow the Linux kernel coding style:
- Align block comment asterisks properly.
- Add blank lines after variable declarations where needed.
- Remove unnecessary spaces before semicolons.
- Move opening braces of function definitions to the next line.

These changes improve readability, maintain consistency, and make the code
easier to maintain.

Signed-off-by: Clint George <clintbgeorge@gmail.com>
---
 drivers/usb/gadget/udc/dummy_hcd.c | 49 ++++++++++++++++--------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 6ad366640..d4b2dcbc4 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -151,11 +151,11 @@ static const struct {
 	EP_INFO("ep2out-bulk",
 		USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK, USB_EP_CAPS_DIR_OUT)),
 /*
-	EP_INFO("ep3in-iso",
-		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_IN)),
-	EP_INFO("ep4out-iso",
-		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_OUT)),
-*/
+ *	EP_INFO("ep3in-iso",
+ *		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_IN)),
+ *	EP_INFO("ep4out-iso",
+ *		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_OUT)),
+ */
 	EP_INFO("ep5in-int",
 		USB_EP_CAPS(USB_EP_CAPS_TYPE_INT, USB_EP_CAPS_DIR_IN)),
 	EP_INFO("ep6in-bulk",
@@ -163,11 +163,11 @@ static const struct {
 	EP_INFO("ep7out-bulk",
 		USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK, USB_EP_CAPS_DIR_OUT)),
 /*
-	EP_INFO("ep8in-iso",
-		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_IN)),
-	EP_INFO("ep9out-iso",
-		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_OUT)),
-*/
+ *	EP_INFO("ep8in-iso",
+ *		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_IN)),
+ *	EP_INFO("ep9out-iso",
+ *		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_OUT)),
+ */
 	EP_INFO("ep10in-int",
 		USB_EP_CAPS(USB_EP_CAPS_TYPE_INT, USB_EP_CAPS_DIR_IN)),
 	EP_INFO("ep11in-bulk",
@@ -175,11 +175,11 @@ static const struct {
 	EP_INFO("ep12out-bulk",
 		USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK, USB_EP_CAPS_DIR_OUT)),
 /*
-	EP_INFO("ep13in-iso",
-		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_IN)),
-	EP_INFO("ep14out-iso",
-		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_OUT)),
-*/
+ *	EP_INFO("ep13in-iso",
+ *		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_IN)),
+ *	EP_INFO("ep14out-iso",
+ *		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_OUT)),
+ */
 	EP_INFO("ep15in-int",
 		USB_EP_CAPS(USB_EP_CAPS_TYPE_INT, USB_EP_CAPS_DIR_IN)),
 
@@ -314,6 +314,7 @@ static inline struct dummy *ep_to_dummy(struct dummy_ep *ep)
 static inline struct dummy_hcd *gadget_to_dummy_hcd(struct usb_gadget *gadget)
 {
 	struct dummy *dum = container_of(gadget, struct dummy, gadget);
+
 	if (dum->gadget.speed == USB_SPEED_SUPER)
 		return dum->ss_hcd;
 	else
@@ -388,7 +389,7 @@ static void set_link_state_by_speed(struct dummy_hcd *dum_hcd)
 		} else {
 			/* device is connected and not suspended */
 			dum_hcd->port_status |= (USB_PORT_STAT_CONNECTION |
-						 USB_PORT_STAT_SPEED_5GBPS) ;
+						 USB_PORT_STAT_SPEED_5GBPS);
 			if ((dum_hcd->old_status &
 			     USB_PORT_STAT_CONNECTION) == 0)
 				dum_hcd->port_status |=
@@ -613,8 +614,8 @@ static int dummy_enable(struct usb_ep *_ep,
 	_ep->maxpacket = max;
 	if (usb_ss_max_streams(_ep->comp_desc)) {
 		if (!usb_endpoint_xfer_bulk(desc)) {
-			dev_err(udc_dev(dum), "Can't enable stream support on "
-					"non-bulk ep %s\n", _ep->name);
+			dev_err(udc_dev(dum), "Can't enable stream support on non-bulk ep %s\n",
+					_ep->name);
 			return -EINVAL;
 		}
 		ep->stream_en = 1;
@@ -1042,7 +1043,8 @@ static int dummy_udc_stop(struct usb_gadget *g)
 #undef is_enabled
 
 /* The gadget structure is stored inside the hcd structure and will be
- * released along with it. */
+ * released along with it.
+ */
 static void init_dummy_udc_hw(struct dummy *dum)
 {
 	int i;
@@ -1264,7 +1266,8 @@ static int dummy_urb_enqueue(
 	struct usb_hcd			*hcd,
 	struct urb			*urb,
 	gfp_t				mem_flags
-) {
+)
+{
 	struct dummy_hcd *dum_hcd;
 	struct urbp	*urbp;
 	unsigned long	flags;
@@ -1323,7 +1326,8 @@ static int dummy_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 	int		rc;
 
 	/* giveback happens automatically in timer callback,
-	 * so make sure the callback happens */
+	 * so make sure the callback happens
+	 */
 	dum_hcd = hcd_to_dummy_hcd(hcd);
 	spin_lock_irqsave(&dum_hcd->dum->lock, flags);
 
@@ -2109,7 +2113,8 @@ static int dummy_hub_control(
 	u16		wIndex,
 	char		*buf,
 	u16		wLength
-) {
+)
+{
 	struct dummy_hcd *dum_hcd;
 	int		retval = 0;
 	unsigned long	flags;
-- 
2.43.0

[PATCH 5/8] usb: gadget: dummy_hcd: merge multi-line quoted strings into one line

[Clint George]

Merge multi-line quoted strings into a single line to improve readability
and comply with kernel coding style. This reduces unnecessary line
breaks in string literals and makes the code easier to follow.

Signed-off-by: Clint George <clintbgeorge@gmail.com>
---
 drivers/usb/gadget/udc/dummy_hcd.c | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index d4b2dcbc4..1363de6fe 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -2133,8 +2133,7 @@ static int dummy_hub_control(
 		case USB_PORT_FEAT_SUSPEND:
 			if (hcd->speed == HCD_USB3) {
 				dev_dbg(dummy_dev(dum_hcd),
-					 "USB_PORT_FEAT_SUSPEND req not "
-					 "supported for USB 3.0 roothub\n");
+					 "USB_PORT_FEAT_SUSPEND req not supported for USB 3.0 roothub\n");
 				goto error;
 			}
 			if (dum_hcd->port_status & USB_PORT_STAT_SUSPEND) {
@@ -2174,8 +2173,7 @@ static int dummy_hub_control(
 				(wLength < USB_DT_SS_HUB_SIZE ||
 				 wValue != (USB_DT_SS_HUB << 8))) {
 			dev_dbg(dummy_dev(dum_hcd),
-				"Wrong hub descriptor type for "
-				"USB 3.0 roothub.\n");
+				"Wrong hub descriptor type for USB 3.0 roothub.\n");
 			goto error;
 		}
 		if (hcd->speed == HCD_USB3)
@@ -2247,8 +2245,7 @@ static int dummy_hub_control(
 		case USB_PORT_FEAT_LINK_STATE:
 			if (hcd->speed != HCD_USB3) {
 				dev_dbg(dummy_dev(dum_hcd),
-					 "USB_PORT_FEAT_LINK_STATE req not "
-					 "supported for USB 2.0 roothub\n");
+					 "USB_PORT_FEAT_LINK_STATE req not supported for USB 2.0 roothub\n");
 				goto error;
 			}
 			/*
@@ -2261,8 +2258,7 @@ static int dummy_hub_control(
 			/* TODO: add suspend/resume support! */
 			if (hcd->speed != HCD_USB3) {
 				dev_dbg(dummy_dev(dum_hcd),
-					 "USB_PORT_FEAT_U1/2_TIMEOUT req not "
-					 "supported for USB 2.0 roothub\n");
+					 "USB_PORT_FEAT_U1/2_TIMEOUT req not supported for USB 2.0 roothub\n");
 				goto error;
 			}
 			break;
@@ -2270,8 +2266,7 @@ static int dummy_hub_control(
 			/* Applicable only for USB2.0 hub */
 			if (hcd->speed == HCD_USB3) {
 				dev_dbg(dummy_dev(dum_hcd),
-					 "USB_PORT_FEAT_SUSPEND req not "
-					 "supported for USB 3.0 roothub\n");
+					 "USB_PORT_FEAT_SUSPEND req not supported for USB 3.0 roothub\n");
 				goto error;
 			}
 			if (dum_hcd->active) {
@@ -2298,8 +2293,7 @@ static int dummy_hub_control(
 			/* Applicable only for USB3.0 hub */
 			if (hcd->speed != HCD_USB3) {
 				dev_dbg(dummy_dev(dum_hcd),
-					 "USB_PORT_FEAT_BH_PORT_RESET req not "
-					 "supported for USB 2.0 roothub\n");
+					 "USB_PORT_FEAT_BH_PORT_RESET req not supported for USB 2.0 roothub\n");
 				goto error;
 			}
 			fallthrough;
@@ -2347,8 +2341,7 @@ static int dummy_hub_control(
 	case GetPortErrorCount:
 		if (hcd->speed != HCD_USB3) {
 			dev_dbg(dummy_dev(dum_hcd),
-				 "GetPortErrorCount req not "
-				 "supported for USB 2.0 roothub\n");
+				 "GetPortErrorCount req not supported for USB 2.0 roothub\n");
 			goto error;
 		}
 		/* We'll always return 0 since this is a dummy hub */
@@ -2357,8 +2350,7 @@ static int dummy_hub_control(
 	case SetHubDepth:
 		if (hcd->speed != HCD_USB3) {
 			dev_dbg(dummy_dev(dum_hcd),
-				 "SetHubDepth req not supported for "
-				 "USB 2.0 roothub\n");
+				 "SetHubDepth req not supported for USB 2.0 roothub\n");
 			goto error;
 		}
 		break;
@@ -2601,8 +2593,7 @@ static int dummy_alloc_streams(struct usb_hcd *hcd, struct usb_device *udev,
 			goto out;
 		}
 		if (max_stream < ret_streams) {
-			dev_dbg(dummy_dev(dum_hcd), "Ep 0x%x only supports %u "
-					"stream IDs.\n",
+			dev_dbg(dummy_dev(dum_hcd), "Ep 0x%x only supports %u stream IDs.\n",
 					eps[i]->desc.bEndpointAddress,
 					max_stream);
 			ret_streams = max_stream;
-- 
2.43.0

[PATCH 6/8] usb: gadget: dummy_hcd: use sizeof(*ptr) instead of sizeof *ptr

[Clint George]

Use 'sizeof(*ptr)' instead of 'sizeof *ptr' to follow kernel coding style.
This improves readability and maintains consistency across the code.

Signed-off-by: Clint George <clintbgeorge@gmail.com>
---
 drivers/usb/gadget/udc/dummy_hcd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 1363de6fe..1840dd822 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1273,7 +1273,7 @@ static int dummy_urb_enqueue(
 	unsigned long	flags;
 	int		rc;
 
-	urbp = kmalloc(sizeof *urbp, mem_flags);
+	urbp = kmalloc(sizeof(*urbp), mem_flags);
 	if (!urbp)
 		return -ENOMEM;
 	urbp->urb = urb;
@@ -2082,7 +2082,7 @@ static struct {
 static inline void
 ss_hub_descriptor(struct usb_hub_descriptor *desc)
 {
-	memset(desc, 0, sizeof *desc);
+	memset(desc, 0, sizeof(*desc));
 	desc->bDescriptorType = USB_DT_SS_HUB;
 	desc->bDescLength = 12;
 	desc->wHubCharacteristics = cpu_to_le16(
@@ -2095,7 +2095,7 @@ ss_hub_descriptor(struct usb_hub_descriptor *desc)
 
 static inline void hub_descriptor(struct usb_hub_descriptor *desc)
 {
-	memset(desc, 0, sizeof *desc);
+	memset(desc, 0, sizeof(*desc));
 	desc->bDescriptorType = USB_DT_HUB;
 	desc->bDescLength = 9;
 	desc->wHubCharacteristics = cpu_to_le16(
-- 
2.43.0

[PATCH 7/8] usb: gadget: dummy_hcd: remove unnecessary 'else' after return

[Clint George]

The 'else' after a return statement is redundant and unnecessary.
This patch removes the 'else' in dummy_set_halt_and_wedge(), making
the code clearer and compliant with kernel coding style:

- Return early for the -EAGAIN condition.
- Place the subsequent code at the same indentation level instead
  of inside an 'else' block.

Signed-off-by: Clint George <clintbgeorge@gmail.com>
---
 drivers/usb/gadget/udc/dummy_hcd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 1840dd822..1114dfe61 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -803,10 +803,10 @@ dummy_set_halt_and_wedge(struct usb_ep *_ep, int value, int wedged)
 		return -ESHUTDOWN;
 	if (!value)
 		ep->halted = ep->wedged = 0;
-	else if (ep->desc && (ep->desc->bEndpointAddress & USB_DIR_IN) &&
-			!list_empty(&ep->queue))
-		return -EAGAIN;
 	else {
+		if (ep->desc && (ep->desc->bEndpointAddress & USB_DIR_IN) &&
+			!list_empty(&ep->queue))
+			return -EAGAIN;
 		ep->halted = 1;
 		if (wedged)
 			ep->wedged = 1;
-- 
2.43.0

[PATCH 8/8] usb: gadget: dummy_hcd: fix miscellaneous coding style warnings

[Clint George]

This patch addresses several minor coding style warnings in dummy_hcd.c:

- Avoid multiple line dereference (e.g, ep0->maxpacket)
- Remove unnecessary parentheses
- Remove unnecessary line continuations (\)

Signed-off-by: Clint George <clintbgeorge@gmail.com>
---
 drivers/usb/gadget/udc/dummy_hcd.c | 33 +++++++++++++++---------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 1114dfe61..5b791ce2e 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1423,7 +1423,7 @@ static int transfer(struct dummy_hcd *dum_hcd, struct urb *urb,
 		int		rescan = 0;
 
 		if (dummy_ep_stream_en(dum_hcd, urb)) {
-			if ((urb->stream_id != req->req.stream_id))
+			if (urb->stream_id != req->req.stream_id)
 				continue;
 		}
 
@@ -2222,8 +2222,7 @@ static int dummy_hub_control(
 						      USB_PORT_STAT_HIGH_SPEED;
 						break;
 					case USB_SPEED_LOW:
-						dum_hcd->dum->gadget.ep0->
-							maxpacket = 8;
+						dum_hcd->dum->gadget.ep0->maxpacket = 8;
 						dum_hcd->port_status |=
 							USB_PORT_STAT_LOW_SPEED;
 						break;
@@ -2434,20 +2433,20 @@ static inline ssize_t show_urb(char *buf, size_t size, struct urb *urb)
 			break;
 		 } s; }),
 		ep, ep ? (usb_urb_dir_in(urb) ? "in" : "out") : "",
-		({ char *s; \
-		switch (usb_pipetype(urb->pipe)) { \
-		case PIPE_CONTROL: \
-			s = ""; \
-			break; \
-		case PIPE_BULK: \
-			s = "-bulk"; \
-			break; \
-		case PIPE_INTERRUPT: \
-			s = "-int"; \
-			break; \
-		default: \
-			s = "-iso"; \
-			break; \
+		({ char *s;
+		switch (usb_pipetype(urb->pipe)) {
+		case PIPE_CONTROL:
+			s = "";
+			break;
+		case PIPE_BULK:
+			s = "-bulk";
+			break;
+		case PIPE_INTERRUPT:
+			s = "-int";
+			break;
+		default:
+			s = "-iso";
+			break;
 		} s; }),
 		urb->actual_length, urb->transfer_buffer_length);
 }
-- 
2.43.0

Re: [PATCH 7/8] usb: gadget: dummy_hcd: remove unnecessary 'else' after return

[Greg KH]

On Wed, Nov 19, 2025 at 06:38:39PM +0530, Clint George wrote:
> The 'else' after a return statement is redundant and unnecessary.
> This patch removes the 'else' in dummy_set_halt_and_wedge(), making
> the code clearer and compliant with kernel coding style:
> 
> - Return early for the -EAGAIN condition.
> - Place the subsequent code at the same indentation level instead
>   of inside an 'else' block.

When you have to list different things in a commit changelog, that is a
HUGE hint it should be multiple patches :(

> 
> Signed-off-by: Clint George <clintbgeorge@gmail.com>
> ---
>  drivers/usb/gadget/udc/dummy_hcd.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
> index 1840dd822..1114dfe61 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -803,10 +803,10 @@ dummy_set_halt_and_wedge(struct usb_ep *_ep, int value, int wedged)
>  		return -ESHUTDOWN;
>  	if (!value)
>  		ep->halted = ep->wedged = 0;
> -	else if (ep->desc && (ep->desc->bEndpointAddress & USB_DIR_IN) &&
> -			!list_empty(&ep->queue))
> -		return -EAGAIN;
>  	else {
> +		if (ep->desc && (ep->desc->bEndpointAddress & USB_DIR_IN) &&
> +			!list_empty(&ep->queue))
> +			return -EAGAIN;

Wait, what?  Why move this around like this, the original is best and
makes more sense.  This version is much harder to read :(

thanks,

greg k-h

Re: [PATCH 1/8] usb: gadget: dummy_hcd: replace BUG() with WARN_ON_ONCE()

[Greg KH]

On Wed, Nov 19, 2025 at 06:38:33PM +0530, Clint George wrote:
> Replace BUG() with WARN_ON_ONCE() in dummy_validate_stream()
> when stream_id exceeds max_streams. This allows the kernel to
> continue running with a warning instead of crashing.

Nope, you still crashed given that BILLIONS of devices out there have
panic-on-warn enabled :(

> 
> Signed-off-by: Clint George <clintbgeorge@gmail.com>
> ---
> 
> Testing:
> - The function dummy_validate_stream() was tested using a test module
>   that i created where i sent value of urb->stream_id greater than
>   max_streams. When using BUG(), the kernel-space used to crash but
>   after using WARN_ON_ONCE() the kernel-space does not crash and the
>   module terminates gracefully
> - Ensured that the module builds properly
> 
>  drivers/usb/gadget/udc/dummy_hcd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
> index 1cefca660..41b7b6907 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -1254,7 +1254,7 @@ static int dummy_validate_stream(struct dummy_hcd *dum_hcd, struct urb *urb)
>  	if (urb->stream_id > max_streams) {
>  		dev_err(dummy_dev(dum_hcd), "Stream id %d is out of range.\n",
>  				urb->stream_id);
> -		BUG();
> +		WARN_ON_ONCE(1);

If this can actually be hit, PROPERLY HANDLE THE ISSUE!

Don't paper over bugs in the code, handle them correctly.  This change
does not do any of that as obviously the author thought that if the
code path got here, the system was so broken it needed to be halted.
Your change could cause it to continue on, in a very vulnerable state
(i.e. broken.)

So I can't take this, sorry.  Nor should you want me to take it :)

thanks,

greg k-h

Re: [PATCH 4/8] usb: gadget: dummy_hcd: fix block comments, blank lines and function braces

[Greg KH]

On Wed, Nov 19, 2025 at 06:38:36PM +0530, Clint George wrote:
> This patch updates dummy_hcd.c to follow the Linux kernel coding style:
> - Align block comment asterisks properly.
> - Add blank lines after variable declarations where needed.
> - Remove unnecessary spaces before semicolons.
> - Move opening braces of function definitions to the next line.
> 
> These changes improve readability, maintain consistency, and make the code
> easier to maintain.

And are hard to review :(

Again, please break things up into "one logical change per patch", and
that does not mean "fix all coding style issues" is a "logical change".

Also:

>  /*
> -	EP_INFO("ep3in-iso",
> -		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_IN)),
> -	EP_INFO("ep4out-iso",
> -		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_OUT)),
> -*/
> + *	EP_INFO("ep3in-iso",
> + *		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_IN)),
> + *	EP_INFO("ep4out-iso",
> + *		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_OUT)),
> + */

Why not just delete commented out code?

thanks,

greg k-h

Re: [PATCH 4/8] usb: gadget: dummy_hcd: fix block comments, blank lines and function braces

[Alan Stern]

On Wed, Nov 19, 2025 at 04:29:13PM +0100, Greg KH wrote:
> On Wed, Nov 19, 2025 at 06:38:36PM +0530, Clint George wrote:
> > This patch updates dummy_hcd.c to follow the Linux kernel coding style:
> > - Align block comment asterisks properly.
> > - Add blank lines after variable declarations where needed.
> > - Remove unnecessary spaces before semicolons.
> > - Move opening braces of function definitions to the next line.
> > 
> > These changes improve readability, maintain consistency, and make the code
> > easier to maintain.
> 
> And are hard to review :(
> 
> Again, please break things up into "one logical change per patch", and
> that does not mean "fix all coding style issues" is a "logical change".
> 
> Also:
> 
> >  /*
> > -	EP_INFO("ep3in-iso",
> > -		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_IN)),
> > -	EP_INFO("ep4out-iso",
> > -		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_OUT)),
> > -*/
> > + *	EP_INFO("ep3in-iso",
> > + *		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_IN)),
> > + *	EP_INFO("ep4out-iso",
> > + *		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_OUT)),
> > + */
> 
> Why not just delete commented out code?

Or in this case, replace the code with a comment saying that if 
dummy-hcd supported isochronous transfers then endpoints 3 and 4 would 
be dedicated to isochronous IN and OUT (respectively).

Alan Stern

Re: [PATCH 0/8] usb: gadget: dummy_hcd: coding style improvements and error handling change

[David Hunter]

On 11/19/25 08:08, Clint George wrote:
> This patch series focuses on addressing various coding style issues in
> the dummy_hcd USB gadget driver. The changes include simplifying error
> handling by preventing kernel-space crashes, improving readability, and
> ensuring consistency with kernel coding conventions.
> 
> Clint George (8):
>   usb: gadget: dummy_hcd: replace BUG() with WARN_ON_ONCE()

Hey Clint,

Regarding our discussion on Discord, I wanted to give you advice and
have it be closer to the code, so you could see what I was talking
about. You asked about Greg's feedback regarding the "Bug()". Here is
some context so that you can understand Greg's feedback a little better.

In Kernel development, there are thousands of people writing code. As a
result, developers will write something like "Bug()" or "Warn()" if a
particular path/condition is met.This is to create a signal for future
developers about situations that should not occur. A later developer
might do something that causes that faulty condition to be met. When
debugging, your goal is not to simply remove that line. Your goal is to
find out what caused the faulty condition, and fix that.

If all you do is eliminate the signal that there is an error, you are
just "papering over" instead of addressing the actual issue.


>   usb: gadget: dummy_hcd: replace symbolic permissions (S_IRUGO) with
>     octal (0444)
>   usb: gadget: dummy_hcd: use 'unsigned int' instead of bare 'unsigned'
>   usb: gadget: dummy_hcd: fix block comments, blank lines and function
>     braces

As we discussed, you can break your patches into something more focused
on one type of fix. For example, one patch could be to just remove code
and put in a more meaningful comment. Another patch could be removing
unnecessary spaces.

>   usb: gadget: dummy_hcd: merge multi-line quoted strings into one line
>   usb: gadget: dummy_hcd: use sizeof(*ptr) instead of sizeof *ptr
>   usb: gadget: dummy_hcd: remove unnecessary 'else' after return

Also, some things like your changing of the "else if" are things that
you do not need to change. Some of the knowledge of what to change and
what to ignore will come with experience. For that particular one, most
developers are used to seeing "else if".

>   usb: gadget: dummy_hcd: fix miscellaneous coding style warnings
> 
>  drivers/usb/gadget/udc/dummy_hcd.c | 139 ++++++++++++++---------------
>  1 file changed, 67 insertions(+), 72 deletions(-)
> 

Overall, my recommendation for you is to reduce the amount you are
trying to tackle at once. You can work over a longer period of time on
the file. Not everything needs to be accepted all at once.

Also, feel free to continue reaching out on discord. I just wrote here
to closer tie my feedback to particular patches.

Good attempt at submitting your first patch series, and don't get
discouraged. Getting feedback is a part of the process.

Thanks,
David Hunter

[PATCH v2 0/6] usb: gadget: dummy_hcd: coding style improvements

[Clint George]

This patch series focuses on addressing various coding style issues in
the dummy_hcd USB gadget driver. The changes includes adding relevant
comments, improving readability, and ensuring consistency with kernel
coding conventions.

Clint George (6):
  usb: gadget: dummy_hcd: replace symbolic permissions (S_IRUGO) with octal (0444)
  usb: gadget: dummy_hcd: use 'unsigned int' instead of bare 'unsigned'
  usb: gadget: dummy_hcd: document ISO endpoint allocation pattern
  usb: gadget: dummy_hcd: use sizeof(*ptr) instead of sizeof *ptr
  usb: gadget: dummy_hcd: remove unnecessary parentheses
  usb: gadget: dummy_hcd: move function braces

 drivers/usb/gadget/udc/dummy_hcd.c | 52 ++++++++++++------------------
 1 file changed, 21 insertions(+), 31 deletions(-)

---

Testing:
- Ran compiler testing with no new warnings detected.
- Ensured the module builds and inserts cleanly without issues.
- Used Static Analysis tools to confirm no new issues were introduced.

Please review the changes and let me know if any modifications
or further testing of the module is required.

As part of my LKMP mentorship i have to complete 5 patches as a criteria
for graduation and thus have focused on working on such
beginner-friendly patches so that not only do i get the required number
of patches but also get familiar with the process of kernel
developement. Thus, while this patch series doesn't address the max_stream value
exceeding problem that triggers the BUG() API, i will take some time to
dig deeper and truly understand the problem and fix it and not just
paper-over the problem.

Again, i am very grateful for your feedback greg and alan to guide a
beginner like me. 

Thanks,
Clint

-- 
2.43.0

[PATCH v2 1/6] usb: gadget: dummy_hcd: replace symbolic permissions (S_IRUGO) with octal (0444)

[Clint George]

Replace existing symbolic permissions S_IRUGO (Read-only) with octal
permission 0444. This makes it much easier to read and and is consistent
with the kernel coding style.

Signed-off-by: Clint George <clintbgeorge@gmail.com>
---
 drivers/usb/gadget/udc/dummy_hcd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 1cefca660..fadd3d0c6 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -73,11 +73,11 @@ static struct dummy_hcd_module_parameters mod_data = {
 	.is_high_speed = true,
 	.num = 1,
 };
-module_param_named(is_super_speed, mod_data.is_super_speed, bool, S_IRUGO);
+module_param_named(is_super_speed, mod_data.is_super_speed, bool, 0444);
 MODULE_PARM_DESC(is_super_speed, "true to simulate SuperSpeed connection");
-module_param_named(is_high_speed, mod_data.is_high_speed, bool, S_IRUGO);
+module_param_named(is_high_speed, mod_data.is_high_speed, bool, 0444);
 MODULE_PARM_DESC(is_high_speed, "true to simulate HighSpeed connection");
-module_param_named(num, mod_data.num, uint, S_IRUGO);
+module_param_named(num, mod_data.num, uint, 0444);
 MODULE_PARM_DESC(num, "number of emulated controllers");
 /*-------------------------------------------------------------------------*/
 
-- 
2.43.0

[PATCH v2 2/6] usb: gadget: dummy_hcd: use 'unsigned int' instead of bare 'unsigned'

[Clint George]

Use 'unsigned int' instead of 'unsigned' wherever possible to maintain
consistency with the kernel coding style.
---
 drivers/usb/gadget/udc/dummy_hcd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index fadd3d0c6..39b571899 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -506,7 +506,7 @@ static int dummy_enable(struct usb_ep *_ep,
 	struct dummy		*dum;
 	struct dummy_hcd	*dum_hcd;
 	struct dummy_ep		*ep;
-	unsigned		max;
+	unsigned int		max;
 	int			retval;
 
 	ep = usb_ep_to_dummy_ep(_ep);
@@ -1414,7 +1414,7 @@ static int transfer(struct dummy_hcd *dum_hcd, struct urb *urb,
 top:
 	/* if there's no request queued, the device is NAKing; return */
 	list_for_each_entry(req, &ep->queue, queue) {
-		unsigned	host_len, dev_len, len;
+		unsigned int	host_len, dev_len, len;
 		int		is_short, to_host;
 		int		rescan = 0;
 
@@ -1443,7 +1443,7 @@ static int transfer(struct dummy_hcd *dum_hcd, struct urb *urb,
 			/* not enough bandwidth left? */
 			if (limit < ep->ep.maxpacket && limit < len)
 				break;
-			len = min_t(unsigned, len, limit);
+			len = min_t(unsigned int, len, limit);
 			if (len == 0)
 				break;
 
@@ -1624,8 +1624,8 @@ static int handle_control_request(struct dummy_hcd *dum_hcd, struct urb *urb,
 	struct dummy_ep		*ep2;
 	struct dummy		*dum = dum_hcd->dum;
 	int			ret_val = 1;
-	unsigned	w_index;
-	unsigned	w_value;
+	unsigned int	w_index;
+	unsigned int	w_value;
 
 	w_index = le16_to_cpu(setup->wIndex);
 	w_value = le16_to_cpu(setup->wValue);
-- 
2.43.0

[PATCH v2 3/6] usb: gadget: dummy_hcd: document ISO endpoint allocation pattern

[Clint George]

Remove commented-out ISO definition and add relevant comment documenting
the ISO endpoint allocation pattern.
---
 drivers/usb/gadget/udc/dummy_hcd.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 39b571899..e4124838e 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -151,35 +151,23 @@ static const struct {
 	EP_INFO("ep2out-bulk",
 		USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK, USB_EP_CAPS_DIR_OUT)),
 /*
-	EP_INFO("ep3in-iso",
-		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_IN)),
-	EP_INFO("ep4out-iso",
-		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_OUT)),
-*/
+ *	If dummy-hcd supported isochronous transfers:
+ *	- Endpoints 3 and 4 would be ISO IN/OUT respectively
+ *	- Endpoints 8 and 9 would be ISO IN/OUT respectively
+ *	- Endpoints 13 and 14 would be ISO IN/OUT respectively
+ */
 	EP_INFO("ep5in-int",
 		USB_EP_CAPS(USB_EP_CAPS_TYPE_INT, USB_EP_CAPS_DIR_IN)),
 	EP_INFO("ep6in-bulk",
 		USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK, USB_EP_CAPS_DIR_IN)),
 	EP_INFO("ep7out-bulk",
 		USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK, USB_EP_CAPS_DIR_OUT)),
-/*
-	EP_INFO("ep8in-iso",
-		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_IN)),
-	EP_INFO("ep9out-iso",
-		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_OUT)),
-*/
 	EP_INFO("ep10in-int",
 		USB_EP_CAPS(USB_EP_CAPS_TYPE_INT, USB_EP_CAPS_DIR_IN)),
 	EP_INFO("ep11in-bulk",
 		USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK, USB_EP_CAPS_DIR_IN)),
 	EP_INFO("ep12out-bulk",
 		USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK, USB_EP_CAPS_DIR_OUT)),
-/*
-	EP_INFO("ep13in-iso",
-		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_IN)),
-	EP_INFO("ep14out-iso",
-		USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO, USB_EP_CAPS_DIR_OUT)),
-*/
 	EP_INFO("ep15in-int",
 		USB_EP_CAPS(USB_EP_CAPS_TYPE_INT, USB_EP_CAPS_DIR_IN)),
 
-- 
2.43.0

[PATCH v2 4/6] usb: gadget: dummy_hcd: use sizeof(*ptr) instead of sizeof *ptr

[Clint George]

Use 'sizeof(*ptr)' instead of 'sizeof *ptr' to follow kernel coding style.
This improves readability and maintains consistency across the code.
---
 drivers/usb/gadget/udc/dummy_hcd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index e4124838e..cbf9dbf2a 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1258,7 +1258,7 @@ static int dummy_urb_enqueue(
 	unsigned long	flags;
 	int		rc;
 
-	urbp = kmalloc(sizeof *urbp, mem_flags);
+	urbp = kmalloc(sizeof(*urbp), mem_flags);
 	if (!urbp)
 		return -ENOMEM;
 	urbp->urb = urb;
@@ -2066,7 +2066,7 @@ static struct {
 static inline void
 ss_hub_descriptor(struct usb_hub_descriptor *desc)
 {
-	memset(desc, 0, sizeof *desc);
+	memset(desc, 0, sizeof(*desc));
 	desc->bDescriptorType = USB_DT_SS_HUB;
 	desc->bDescLength = 12;
 	desc->wHubCharacteristics = cpu_to_le16(
@@ -2079,7 +2079,7 @@ ss_hub_descriptor(struct usb_hub_descriptor *desc)
 
 static inline void hub_descriptor(struct usb_hub_descriptor *desc)
 {
-	memset(desc, 0, sizeof *desc);
+	memset(desc, 0, sizeof(*desc));
 	desc->bDescriptorType = USB_DT_HUB;
 	desc->bDescLength = 9;
 	desc->wHubCharacteristics = cpu_to_le16(
-- 
2.43.0

[PATCH v2 5/6] usb: gadget: dummy_hcd: remove unnecessary parentheses

[Clint George]

Remove the extra parentheses around the if-statement conditions to make
the code more readable.
---
 drivers/usb/gadget/udc/dummy_hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index cbf9dbf2a..67b453620 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1407,7 +1407,7 @@ static int transfer(struct dummy_hcd *dum_hcd, struct urb *urb,
 		int		rescan = 0;
 
 		if (dummy_ep_stream_en(dum_hcd, urb)) {
-			if ((urb->stream_id != req->req.stream_id))
+			if (urb->stream_id != req->req.stream_id)
 				continue;
 		}
 
-- 
2.43.0

[PATCH v2 6/6] usb: gadget: dummy_hcd: move function braces

[Clint George]

This patch aims to fix the violations of the kernel coding style by
moving the function braces to the next line.
---
 drivers/usb/gadget/udc/dummy_hcd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 67b453620..031f6ea2c 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -1252,7 +1252,8 @@ static int dummy_urb_enqueue(
 	struct usb_hcd			*hcd,
 	struct urb			*urb,
 	gfp_t				mem_flags
-) {
+)
+{
 	struct dummy_hcd *dum_hcd;
 	struct urbp	*urbp;
 	unsigned long	flags;
@@ -2097,7 +2098,8 @@ static int dummy_hub_control(
 	u16		wIndex,
 	char		*buf,
 	u16		wLength
-) {
+)
+{
 	struct dummy_hcd *dum_hcd;
 	int		retval = 0;
 	unsigned long	flags;
-- 
2.43.0

Re: [PATCH v2 0/6] usb: gadget: dummy_hcd: coding style improvements

[Greg KH]

On Tue, Dec 02, 2025 at 02:07:09AM +0530, Clint George wrote:
> As part of my LKMP mentorship i have to complete 5 patches as a criteria
> for graduation and thus have focused on working on such
> beginner-friendly patches so that not only do i get the required number
> of patches but also get familiar with the process of kernel
> developement.

The LKMP internship should be done in drivers/staging/ as generally
coding style cleanups are NOT accepted in other parts of the kernel,
unless you get approval from the maintainer ahead of time.

Does the maintainer of this driver want this to be used for the intern
project?

thanks,

greg k-h

Re: [PATCH v2 2/6] usb: gadget: dummy_hcd: use 'unsigned int' instead of bare 'unsigned'

[Greg KH]

On Tue, Dec 02, 2025 at 02:07:11AM +0530, Clint George wrote:
> Use 'unsigned int' instead of 'unsigned' wherever possible to maintain
> consistency with the kernel coding style.
> ---
>  drivers/usb/gadget/udc/dummy_hcd.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Hm, this patch is kind of proof of why coding style cleanups outside of
drivers/staging/ is generally not a good idea :(

Please stick to that portion of the kernel now, so you can get the
development process understood more, before making changes elsewhere.

good luck!

greg k-h

Re: [PATCH v2 0/6] usb: gadget: dummy_hcd: coding style improvements

[Alan Stern]

On Tue, Dec 02, 2025 at 06:27:46AM +0100, Greg KH wrote:
> On Tue, Dec 02, 2025 at 02:07:09AM +0530, Clint George wrote:
> > As part of my LKMP mentorship i have to complete 5 patches as a criteria
> > for graduation and thus have focused on working on such
> > beginner-friendly patches so that not only do i get the required number
> > of patches but also get familiar with the process of kernel
> > developement.
> 
> The LKMP internship should be done in drivers/staging/ as generally
> coding style cleanups are NOT accepted in other parts of the kernel,
> unless you get approval from the maintainer ahead of time.
> 
> Does the maintainer of this driver want this to be used for the intern
> project?

In fact, Clint's changes are small and inoffensive enough, I wouldn't 
mind having them applied to dummy-hcd.

However, Greg is perfectly right that this kind of stylistic update is 
not something that should be submitted for most parts of the kernel.  It 
just bulks up the Git history with essentially meaningless cruft, making 
it all that much harder to see the changes that really matter.  That's 
part of the reason for the suggestion that interns and beginners should 
confine their efforts to drivers/staging.

Also, remember that trivial changes like this are fine for learning the 
procedure of submitting kernel patches, but the effects they have on the 
kernel itself are minimal.  A patch that actually fixes a bug or adds a 
functional enhancement would be a different story.

Alan Stern

Re: [PATCH v2 0/6] usb: gadget: dummy_hcd: coding style improvements

[Robert P. J. Day]

On Tue, 2 Dec 2025, Alan Stern wrote:

> On Tue, Dec 02, 2025 at 06:27:46AM +0100, Greg KH wrote:
> > On Tue, Dec 02, 2025 at 02:07:09AM +0530, Clint George wrote:
> > > As part of my LKMP mentorship i have to complete 5 patches as a criteria
> > > for graduation and thus have focused on working on such
> > > beginner-friendly patches so that not only do i get the required number
> > > of patches but also get familiar with the process of kernel
> > > developement.
> >
> > The LKMP internship should be done in drivers/staging/ as generally
> > coding style cleanups are NOT accepted in other parts of the kernel,
> > unless you get approval from the maintainer ahead of time.
> >
> > Does the maintainer of this driver want this to be used for the intern
> > project?
>
> In fact, Clint's changes are small and inoffensive enough, I wouldn't
> mind having them applied to dummy-hcd.
>
> However, Greg is perfectly right that this kind of stylistic update is
> not something that should be submitted for most parts of the kernel.  It
> just bulks up the Git history with essentially meaningless cruft, making
> it all that much harder to see the changes that really matter.  That's
> part of the reason for the suggestion that interns and beginners should
> confine their efforts to drivers/staging.
>
> Also, remember that trivial changes like this are fine for learning the
> procedure of submitting kernel patches, but the effects they have on the
> kernel itself are minimal.  A patch that actually fixes a bug or adds a
> functional enhancement would be a different story.

  FWIW, many years ago, I threw together a collection of kernel
"cleanup" scripts that scanned the kernel source for various possible
"improvements". Earlier this year, I posted a number of suggestions
for cleanup work to the kernel janitors mailing, and slapped together
a wiki page that described some obvious cleanups:

https://crashcourse.ca/doku/doku.php?id=linux_kernel_cleanup

  As the first example on that page, look for code that insists on
manually calculating the length of an array when the appropriate macro
has been defined in include/linux/array_size.h for years. There's
still some of that lying around that could be tidied up; for example,
scan the drivers/ directory (do not make fun of my hacky solutions):

$ grep -Er "sizeof ?\(?([^\)]+)\)? ?/ ?sizeof ?\(?.*\1.*" drivers
drivers/hid/bpf/progs/hid_bpf_helpers.h:#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
drivers/media/common/tveeprom.c:	(i < sizeof(array) / sizeof(char *) ? array[i] : "unknown")
drivers/net/ethernet/mellanox/mlx4/qp.c:	for (k = MLX4_QP_TABLE_ZONE_RSS + 1; k < sizeof(*bitmap)/sizeof((*bitmap)[0]);
drivers/net/ethernet/intel/i40e/i40e_adminq.h:	if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0]))))
drivers/net/ethernet/intel/iavf/iavf_adminq.h:	if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0]))))
drivers/gpu/drm/xe/xe_guc_hxg_helpers.h:#define hxg_sizeof(T)	(sizeof(T) / sizeof(u32) + BUILD_BUG_ON_ZERO(sizeof(T) % sizeof(u32)))
drivers/gpu/drm/nouveau/nvif/fifo.c:	a->m.count = sizeof(a->v) / sizeof(a->v.runlists);
drivers/gpu/drm/amd/display/dc/mpc/dcn30/dcn30_mpc.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
drivers/gpu/drm/amd/display/dc/mpc/dcn20/dcn20_mpc.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
drivers/gpu/drm/amd/display/dc/dpp/dcn10/dcn10_dpp_cm.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
drivers/gpu/drm/amd/display/dc/dpp/dcn10/dcn10_dpp_cm.c:	int arr_size = sizeof(dpp_input_csc_matrix)/sizeof(struct dpp_input_csc_matrix);
drivers/gpu/drm/amd/display/dc/dpp/dcn30/dcn30_dpp.c:	int arr_size = sizeof(dpp_input_csc_matrix)/sizeof(struct dpp_input_csc_matrix);
drivers/gpu/drm/amd/display/dc/dpp/dcn20/dcn20_dpp_cm.c:	int arr_size = sizeof(dpp_input_csc_matrix)/sizeof(struct dpp_input_csc_matrix);
drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c:	int arr_size = sizeof(dpp_input_csc_matrix) / sizeof(struct dpp_input_csc_matrix);
drivers/gpu/drm/amd/display/dc/dce110/dce110_opp_csc_v.c:	int arr_size = sizeof(input_csc_matrix)/sizeof(struct input_csc_matrix);
drivers/gpu/drm/amd/display/dc/dml/dsc/rc_calc_fpu.c:	table_size = sizeof(qp_table_##mode##_##bpc##bpc_##max)/sizeof(*qp_table_##mode##_##bpc##bpc_##max); \
drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c:	for (k = 0; k < sizeof(wb_arb_params->cli_watermark)/sizeof(wb_arb_params->cli_watermark[0]); k++) {
drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))

  There are other ideas on that wiki page for someone who wanted
something simple to start with that aren't just aesthetic, they
would actually improve the quality and readability of the code.

rday

Re: [PATCH v2 0/6] usb: gadget: dummy_hcd: coding style improvements

[David Hunter]

On 12/1/25 15:37, Clint George wrote:
> This patch series focuses on addressing various coding style issues in
> the dummy_hcd USB gadget driver. The changes includes adding relevant
> comments, improving readability, and ensuring consistency with kernel
> coding conventions.
> 
> Clint George (6):
>   usb: gadget: dummy_hcd: replace symbolic permissions (S_IRUGO) with octal (0444)
>   usb: gadget: dummy_hcd: use 'unsigned int' instead of bare 'unsigned'
>   usb: gadget: dummy_hcd: document ISO endpoint allocation pattern
>   usb: gadget: dummy_hcd: use sizeof(*ptr) instead of sizeof *ptr
>   usb: gadget: dummy_hcd: remove unnecessary parentheses
>   usb: gadget: dummy_hcd: move function braces
> 
>  drivers/usb/gadget/udc/dummy_hcd.c | 52 ++++++++++++------------------
>  1 file changed, 21 insertions(+), 31 deletions(-)
> 
> ---
> 
> Testing:
> - Ran compiler testing with no new warnings detected.
> - Ensured the module builds and inserts cleanly without issues.
> - Used Static Analysis tools to confirm no new issues were introduced.
> 
> Please review the changes and let me know if any modifications
> or further testing of the module is required.
> 
> As part of my LKMP mentorship i have to complete 5 patches as a criteria
> for graduation and thus have focused on working on such
> beginner-friendly patches so that not only do i get the required number
> of patches but also get familiar with the process of kernel
> developement. Thus, while this patch series doesn't address the max_stream value
> exceeding problem that triggers the BUG() API, i will take some time to
> dig deeper and truly understand the problem and fix it and not just
> paper-over the problem.
> 
> Again, i am very grateful for your feedback greg and alan to guide a
> beginner like me. 
> 
> Thanks,
> Clint
> 

Hey Clint,

In general, when submitting version 2 of a patch series, most
maintainers will prefer that you send it as a new patch series and not
as a reply. I usually recommend putting the links to the respective
previous versions in the change log.

Thanks,
David Hunter

Re: [PATCH v2 0/6] usb: gadget: dummy_hcd: coding style improvements

[David Hunter]

On 12/2/25 10:53, Alan Stern wrote:
> On Tue, Dec 02, 2025 at 06:27:46AM +0100, Greg KH wrote:
>> On Tue, Dec 02, 2025 at 02:07:09AM +0530, Clint George wrote:
>>> As part of my LKMP mentorship i have to complete 5 patches as a criteria
>>> for graduation and thus have focused on working on such
>>> beginner-friendly patches so that not only do i get the required number
>>> of patches but also get familiar with the process of kernel
>>> developement.
>>
>> The LKMP internship should be done in drivers/staging/ as generally
>> coding style cleanups are NOT accepted in other parts of the kernel,
>> unless you get approval from the maintainer ahead of time.
>>
>> Does the maintainer of this driver want this to be used for the intern
>> project?
> 
> In fact, Clint's changes are small and inoffensive enough, I wouldn't 
> mind having them applied to dummy-hcd.
> 
> However, Greg is perfectly right that this kind of stylistic update is 
> not something that should be submitted for most parts of the kernel.  It 
> just bulks up the Git history with essentially meaningless cruft, making 
> it all that much harder to see the changes that really matter.  That's 
> part of the reason for the suggestion that interns and beginners should 
> confine their efforts to drivers/staging.
> 
> Also, remember that trivial changes like this are fine for learning the 
> procedure of submitting kernel patches, but the effects they have on the 
> kernel itself are minimal.  A patch that actually fixes a bug or adds a 
> functional enhancement would be a different story.
> 
> Alan Stern


Understood.

David Hunter

Re: [PATCH v2 0/6] usb: gadget: dummy_hcd: coding style improvements

[David Hunter]

On 12/2/25 11:28, Robert P. J. Day wrote:
> On Tue, 2 Dec 2025, Alan Stern wrote:
> 
>> On Tue, Dec 02, 2025 at 06:27:46AM +0100, Greg KH wrote:
>>> On Tue, Dec 02, 2025 at 02:07:09AM +0530, Clint George wrote:
>>>> As part of my LKMP mentorship i have to complete 5 patches as a criteria
>>>> for graduation and thus have focused on working on such
>>>> beginner-friendly patches so that not only do i get the required number
>>>> of patches but also get familiar with the process of kernel
>>>> developement.
>>>
>>> The LKMP internship should be done in drivers/staging/ as generally
>>> coding style cleanups are NOT accepted in other parts of the kernel,
>>> unless you get approval from the maintainer ahead of time.
>>>>>> Does the maintainer of this driver want this to be used for the intern
>>> project?
>>
>> In fact, Clint's changes are small and inoffensive enough, I wouldn't
>> mind having them applied to dummy-hcd.
>>
>> However, Greg is perfectly right that this kind of stylistic update is
>> not something that should be submitted for most parts of the kernel.  It
>> just bulks up the Git history with essentially meaningless cruft, making
>> it all that much harder to see the changes that really matter.  That's
>> part of the reason for the suggestion that interns and beginners should
>> confine their efforts to drivers/staging.
>>
>> Also, remember that trivial changes like this are fine for learning the
>> procedure of submitting kernel patches, but the effects they have on the
>> kernel itself are minimal.  A patch that actually fixes a bug or adds a
>> functional enhancement would be a different story.
> 
>   FWIW, many years ago, I threw together a collection of kernel
> "cleanup" scripts that scanned the kernel source for various possible
> "improvements". Earlier this year, I posted a number of suggestions
> for cleanup work to the kernel janitors mailing, and slapped together
> a wiki page that described some obvious cleanups:
> 
> https://crashcourse.ca/doku/doku.php?id=linux_kernel_cleanup
> 
>   As the first example on that page, look for code that insists on
> manually calculating the length of an array when the appropriate macro
> has been defined in include/linux/array_size.h for years. There's
> still some of that lying around that could be tidied up; for example,
> scan the drivers/ directory (do not make fun of my hacky solutions):
> 
> $ grep -Er "sizeof ?\(?([^\)]+)\)? ?/ ?sizeof ?\(?.*\1.*" drivers
> drivers/hid/bpf/progs/hid_bpf_helpers.h:#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> drivers/media/common/tveeprom.c:	(i < sizeof(array) / sizeof(char *) ? array[i] : "unknown")
> drivers/net/ethernet/mellanox/mlx4/qp.c:	for (k = MLX4_QP_TABLE_ZONE_RSS + 1; k < sizeof(*bitmap)/sizeof((*bitmap)[0]);
> drivers/net/ethernet/intel/i40e/i40e_adminq.h:	if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0]))))
> drivers/net/ethernet/intel/iavf/iavf_adminq.h:	if (!((u32)aq_rc < (sizeof(aq_to_posix) / sizeof((aq_to_posix)[0]))))
> drivers/gpu/drm/xe/xe_guc_hxg_helpers.h:#define hxg_sizeof(T)	(sizeof(T) / sizeof(u32) + BUILD_BUG_ON_ZERO(sizeof(T) % sizeof(u32)))
> drivers/gpu/drm/nouveau/nvif/fifo.c:	a->m.count = sizeof(a->v) / sizeof(a->v.runlists);
> drivers/gpu/drm/amd/display/dc/mpc/dcn30/dcn30_mpc.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
> drivers/gpu/drm/amd/display/dc/mpc/dcn20/dcn20_mpc.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
> drivers/gpu/drm/amd/display/dc/dpp/dcn10/dcn10_dpp_cm.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
> drivers/gpu/drm/amd/display/dc/dpp/dcn10/dcn10_dpp_cm.c:	int arr_size = sizeof(dpp_input_csc_matrix)/sizeof(struct dpp_input_csc_matrix);
> drivers/gpu/drm/amd/display/dc/dpp/dcn30/dcn30_dpp.c:	int arr_size = sizeof(dpp_input_csc_matrix)/sizeof(struct dpp_input_csc_matrix);
> drivers/gpu/drm/amd/display/dc/dpp/dcn20/dcn20_dpp_cm.c:	int arr_size = sizeof(dpp_input_csc_matrix)/sizeof(struct dpp_input_csc_matrix);
> drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
> drivers/gpu/drm/amd/display/dc/dpp/dcn401/dcn401_dpp_cm.c:	int arr_size = sizeof(dpp_input_csc_matrix) / sizeof(struct dpp_input_csc_matrix);
> drivers/gpu/drm/amd/display/dc/dce110/dce110_opp_csc_v.c:	int arr_size = sizeof(input_csc_matrix)/sizeof(struct input_csc_matrix);
> drivers/gpu/drm/amd/display/dc/dml/dsc/rc_calc_fpu.c:	table_size = sizeof(qp_table_##mode##_##bpc##bpc_##max)/sizeof(*qp_table_##mode##_##bpc##bpc_##max); \
> drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c:	for (k = 0; k < sizeof(wb_arb_params->cli_watermark)/sizeof(wb_arb_params->cli_watermark[0]); k++) {
> drivers/gpu/drm/amd/display/dc/core/dc_hw_sequencer.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
> drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c:#define NUM_ELEMENTS(a) (sizeof(a) / sizeof((a)[0]))
> 
>   There are other ideas on that wiki page for someone who wanted
> something simple to start with that aren't just aesthetic, they
> would actually improve the quality and readability of the code.
> 
> rday


Hey Rday,

Wow, this is interesting. I did something similar when I was a mentee in
the LKMP. I am a co-mentor now for the program. Would you say that
maintainers regard the patches that would come from fixing these style
issues as fundamentally different than the ones Clint attempted to fix?

Also, are your scripts open for anyone to use, including future mentees?

Greg, if mentees fixed the ones that Rday's script cause, would you
still prefer that those be confined to mainly drivers/staging?

Thanks,
David Hunter