f

Author: roma-jam

drivers/usb/uhc/uhc_dwc2.c

@@ -858,13 +858,13 @@ static inline int uhc_dwc2_config_phy(const struct device *dev)
 			gusbcfg &= ~(USB_DWC2_GUSBCFG_ULPIFSLS | USB_DWC2_GUSBCFG_ULPICLK_SUSM);
 		} else {
 			LOG_WRN("Highspeed UTMI+ PHY init");
-			// Select UTMI+ PHY (internal)
+			/* Select UTMI+ PHY (internal) */
 			gusbcfg &= ~USB_DWC2_GUSBCFG_ULPI_UTMI_SEL_ULPI;
-			// Set 16-bit interface if supported
+			/* Set 16-bit interface if supported */
 			if (priv->const_cfg.phydatawidth) {
-				gusbcfg |= USB_DWC2_GUSBCFG_PHYIF_16_BIT; // 16 bit
+				gusbcfg |= USB_DWC2_GUSBCFG_PHYIF_16_BIT;
 			} else {
-				gusbcfg &= ~USB_DWC2_GUSBCFG_PHYIF_16_BIT; // 8 bit
+				gusbcfg &= ~USB_DWC2_GUSBCFG_PHYIF_16_BIT;
 			}
 		}
 		sys_write32(gusbcfg, (mem_addr_t)&dwc2->gusbcfg);
@@ -1303,12 +1303,14 @@ static void IRAM_ATTR _buffer_fill(pipe_t *pipe)
 	/* TODO: sync CACHE */
 }
 
-static inline ctrl_stage_t cal_next_pid(ctrl_stage_t pid, uint8_t pkt_count) {
-  if (pkt_count & 0x01) {
-    return pid ^ 0x02; // toggle DATA0 and DATA1
-  } else {
-    return pid;
-  }
+static inline ctrl_stage_t cal_next_pid(ctrl_stage_t pid, uint8_t pkt_count)
+{
+	if (pkt_count & 0x01) {
+		/* Toggle DATA0 and DATA1 */
+		return pid ^ 0x02;
+	} else {
+		return pid;
+	}
 }
 
 static void IRAM_ATTR _buffer_exec_proceed(pipe_t *pipe)
@@ -1361,7 +1363,7 @@ static void IRAM_ATTR _buffer_exec_proceed(pipe_t *pipe)
 				net_buf_add(xfer->buf, size);
 			}
 		}
-	} else {        /* cur_stg == 1. // Just finished data stage. Go to status stage */
+	} else {        /* cur_stg == 1. Just finished data stage. Go to status stage */
 		/* Status stage is always the opposite direction of data stage */
 		next_dir_is_in = !buffer_exec->flags.ctrl.data_stg_in;
 		next_pid = CTRL_STAGE_DATA1;	/* Status stage always has a PID of DATA1 */
@@ -1472,9 +1474,7 @@ static pipe_event_t uhc_dwc2_decode_chan(pipe_t* pipe, uhc_dwc2_channel_t *chan_
 		break;
 	}
 	default:
-	__ASSERT(false,
-			"uhc_dwc2_decode_chan: Unknown channel event: %s",
-			dwc2_chan_event_str[chan_event]);
+		__ASSERT(false, "uhc_dwc2_decode_chan: Unknown channel event");
 		break;
 	}
 
@@ -1730,14 +1730,8 @@ static inline int uhc_dwc2_port_recovery(const struct device *dev)
 	int ret;
 
 	/* TODO: Implement port checks */
-	/*
-	* Hint:
-	* struct uhc_dwc2_data_s *priv = uhc_get_private(dev);
-	* priv->dynamic.state == UHC_PORT_STATE_RECOVERY;
-	* priv->num_channels_idle == 0;
-	* priv->num_channels_queued == 0
-	* priv->dynamic.flags.val == 0
-	*/
+	/* Port should be in recovery state and no ongoing transfers */
+	/* Port flags should be 0 */
 
 	/* TODO: enter critical section */
 	ret = uhc_dwc2_quirk_irq_disable_func(dev);
@@ -1827,11 +1821,10 @@ static void uhc_dwc2_pipe_set_ep_char(const uhc_pipe_config_t *pipe_config,
 									uhc_dwc2_ep_char_t *ep_char)
 {
 	uhc_dwc2_xfer_type_t dw2_ll_xfer_type;
-	switch (type) {
-	case USB_TRANSFER_TYPE_CTRL:
+
+	if (type == USB_TRANSFER_TYPE_CTRL) {
 		dw2_ll_xfer_type = UHC_DWC2_XFER_TYPE_CTRL;
-		break;
-	default:
+	} else {
 		LOG_ERR("Unsupported transfer type %d", type);
 		return;
 	}
@@ -2203,41 +2196,35 @@ static inline void uhc_dwc2_handle_pipe_events(const struct device *dev)
 
 	LOG_DBG("Pipe event: %s", uhc_pipe_event_str[pipe->last_event]);
 
-	switch (pipe->last_event) {
-		case PIPE_EVENT_XFER_DONE: {
-			/* XFER transfer is done, process the transfer and release the pipe buffer */
-			struct uhc_transfer *const xfer = (struct uhc_transfer *)pipe->buffer->xfer;
+	if (pipe->last_event == PIPE_EVENT_XFER_DONE) {
+		/* XFER transfer is done, process the transfer and release the pipe buffer */
+		struct uhc_transfer *const xfer = (struct uhc_transfer *)pipe->buffer->xfer;
 
-			if (xfer->buf != NULL && xfer->buf->len) {
-				LOG_HEXDUMP_WRN(xfer->buf->data, xfer->buf->len, "data");
-			}
+		if (xfer->buf != NULL && xfer->buf->len) {
+			LOG_HEXDUMP_WRN(xfer->buf->data, xfer->buf->len, "data");
+		}
 
-			/* TODO: Refactor the address setting logic. */
-			/* To configure the channel, we need to get the dev addr from higher logic */
-			if (pipe->buffer->flags.ctrl.set_addr) {
-				pipe->buffer->flags.ctrl.set_addr = 0;
-				pipe->ep_char.dev_addr = pipe->buffer->flags.ctrl.new_addr;
-				/* Set the new device address in the channel */
-				sys_set_bits((mem_addr_t)&pipe->chan_obj->regs->hcchar,
-								(pipe->ep_char.dev_addr << USB_DWC2_HCCHAR0_DEVADDR_POS));
-				k_msleep(SET_ADDR_DELAY_MS);
-			}
+		/* TODO: Refactor the address setting logic. */
+		/* To configure the channel, we need to get the dev addr from higher logic */
+		if (pipe->buffer->flags.ctrl.set_addr) {
+			pipe->buffer->flags.ctrl.set_addr = 0;
+			pipe->ep_char.dev_addr = pipe->buffer->flags.ctrl.new_addr;
+			/* Set the new device address in the channel */
+			sys_set_bits((mem_addr_t)&pipe->chan_obj->regs->hcchar,
+							(pipe->ep_char.dev_addr << USB_DWC2_HCCHAR0_DEVADDR_POS));
+			k_msleep(SET_ADDR_DELAY_MS);
+		}
 
-			/* TODO: Refactor pipe resources release */
-			pipe->flags.has_xfer = 0;
-			priv->num_pipes_idle++;
-			priv->num_pipes_queued--;
+		/* TODO: Refactor pipe resources release */
+		pipe->flags.has_xfer = 0;
+		priv->num_pipes_idle++;
+		priv->num_pipes_queued--;
 
-			uhc_xfer_return(dev, xfer, 0);
-			break;
-		}
-		case PIPE_EVENT_ERROR:
-		case PIPE_EVENT_HALTED:
-		case PIPE_EVENT_NONE:
-		default: {
-			LOG_ERR("Unhandled pipe event %s", uhc_pipe_event_str[pipe->last_event]);
-			break;
-		}
+		uhc_xfer_return(dev, xfer, 0);
+
+	} else {
+		/* TODO: Handle the rest pipe events */
+		LOG_ERR("Unhandled pipe event %s", uhc_pipe_event_str[pipe->last_event]);
 	}
 }
 
@@ -2283,15 +2270,15 @@ static inline int uhc_dwc2_submit_ctrl_xfer(const struct device *dev,
 
 	/* TODO: Check that XFER has not already been enqueued? */
 
-	/* TODO: dma addr must be aligned 4 bytes */
-	/*
-	* Hint: if (((uintptr_t)xfer->setup_pkt % 4)) { ... }
-	*/
+	/* TODO: setup packet must be aligned 4 bytes? */
+	if (((uintptr_t)xfer->setup_pkt % 4)) {
+		LOG_WRN("Setup packet address %p is not 4-byte aligned", xfer->setup_pkt);
+	}
 
 	/* TODO: Buffer addr that will used as dma addr also should be aligned */
-	/*
-	* Hint: if(((uintptr_t)net_buf_tail(xfer->buf) % 4)) { ... }
-	*/
+	if((xfer->buf != NULL) && ((uintptr_t)net_buf_tail(xfer->buf) % 4)) {
+		LOG_WRN("XFER buffer address %08lXh is not 4-byte aligned", (uintptr_t)net_buf_tail(xfer->buf));
+	}
 
 	sys_dlist_append(&pipe->xfer_pending_list, &xfer->node);
 	pipe->num_xfer_pending++;