Re: [PATCH v2 4/8] usb: dwc2: Clean up with bitfield macros

[Marek Vasut]

On 12/30/24 4:30 AM, Junhui Liu via B4 Relay wrote:

[...]
diff --git a/drivers/usb/gadget/dwc2_udc_otg_regs.h 
b/drivers/usb/gadget/dwc2_udc_otg_regs.h
index 
198ba7a7c37d05dca084ef017534265f5fb5fd70..33359ba56f7864525fb435ab52f2b21538e4647f
 100644
--- a/drivers/usb/gadget/dwc2_udc_otg_regs.h
+++ b/drivers/usb/gadget/dwc2_udc_otg_regs.h

[...]

@@ -78,15 +77,15 @@ struct dwc2_usbotg_phy {
  #define HIGH_SPEED_CONTROL_PKT_SIZE   64
  #define HIGH_SPEED_BULK_PKT_SIZE      512
  
-#define RX_FIFO_SIZE			(1024)
-#define NPTX_FIFO_SIZE                 (1024)
-#define PTX_FIFO_SIZE                  (384)
+#define RX_FIFO_SIZE                   1024
+#define NPTX_FIFO_SIZE                 1024
+#define PTX_FIFO_SIZE                  384
  
-#define DEPCTL_TXFNUM_0		(0x0<<22)
-#define DEPCTL_TXFNUM_1                (0x1<<22)
-#define DEPCTL_TXFNUM_2                (0x2<<22)
-#define DEPCTL_TXFNUM_3                (0x3<<22)
-#define DEPCTL_TXFNUM_4                (0x4<<22)
+#define DEPCTL_TXFNUM_0                        (0x0<<22)
+#define DEPCTL_TXFNUM_1                        (0x1<<22)
+#define DEPCTL_TXFNUM_2                        (0x2<<22)
+#define DEPCTL_TXFNUM_3                        (0x3<<22)
+#define DEPCTL_TXFNUM_4                        (0x4<<22)

Nitpick -- please add spaces around << , I think checkpatch would warn 
about this.

[...]

diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c 
b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c
index 
e2a24c53b4c8d632163484d52b4338820dcb1731..76c0b03c355b47e9523a071bd76ab8d6a17cd3ac
 100644
--- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c
+++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c
@@ -19,6 +19,7 @@
  
  #include <cpu_func.h>
  #include <log.h>
+#include <linux/bitfield.h>
  #include <linux/bug.h>
  
  static u8 clear_feature_num;
@@ -174,11 +175,11 @@ static int setdma_tx(struct dwc2_ep *ep, struct 
dwc2_request *req)
        ctrl = readl(&reg->device_regs.in_endp[ep_num].diepctl);
  
  	/* Write the FIFO number to be used for this endpoint */
-       ctrl &= DIEPCTL_TX_FIFO_NUM_MASK;
-       ctrl |= DIEPCTL_TX_FIFO_NUM(ep->fifo_num);
+       ctrl &= ~DIEPCTL_TX_FIFO_NUM_MASK;
+       ctrl |= FIELD_PREP(DIEPCTL_TX_FIFO_NUM_MASK, ep->fifo_num);
  
  	/* Clear reserved (Next EP) bits */
-       ctrl = (ctrl&~(EP_MASK<<DEPCTL_NEXT_EP_BIT));
+       ctrl &= ~DEPCTL_NEXT_EP_MASK;
  
  	writel(DEPCTL_EPENA|DEPCTL_CNAK|ctrl, &reg->device_regs.in_endp[ep_num].diepctl);
Nitpick -- in some follow up separate series, this could also be 
replaced by clrsetbits_le32() , but that's really for separate patch/series.