On Tue, Jun 22, 2021 at 11:46:11AM +0300, Dan Carpenter wrote:

On Mon, Jun 21, 2021 at 09:49:00PM +0800, Coiby Xu wrote:

quoted

@@ -524,8 +523,8 @@ static int qlge_set_routing_reg(struct qlge_adapter *qdev, u32 index, u32 mask,
 		{
 			value = RT_IDX_DST_DFLT_Q | /* dest */
 				RT_IDX_TYPE_NICQ | /* type */
-				(RT_IDX_IP_CSUM_ERR_SLOT <<
-				RT_IDX_IDX_SHIFT); /* index */
+			(RT_IDX_IP_CSUM_ERR_SLOT
+			 << RT_IDX_IDX_SHIFT); /* index */

The original is not great but the new indenting is definitely worse.
It might look nicer with the comments moved in the front?  Why does
RT_IDX_IDX_SHIFT have two IDX strings?

I'm not sure about it. Two IDX strings seems to be a typo.

		/* value = dest | type | index; */
		value = RT_IDX_DST_DFLT_Q |
			RT_IDX_TYPE_NICQ  |
			(RT_IDX_IP_CSUM_ERR_SLOT << RT_IDX_IDX_SHIFT);

This looks better! Thanks!

quoted

 			break;
 		}
 	case RT_IDX_TU_CSUM_ERR: /* Pass up TCP/UDP CSUM error frames. */

@@ -554,7 +553,8 @@ static int qlge_set_routing_reg(struct qlge_adapter *qdev, u32 index, u32 mask,
 		{
 			value = RT_IDX_DST_DFLT_Q |	/* dest */
 			    RT_IDX_TYPE_NICQ |	/* type */
-			    (RT_IDX_MCAST_MATCH_SLOT << RT_IDX_IDX_SHIFT);/* index */
+			(RT_IDX_MCAST_MATCH_SLOT
+			 << RT_IDX_IDX_SHIFT); /* index */

Original is better.

I'll also move the comments in the front.

quoted

 			break;
 		}
 	case RT_IDX_RSS_MATCH:	/* Pass up matched RSS frames. */

@@ -648,15 +648,15 @@ static int qlge_read_flash_word(struct qlge_adapter *qdev, int offset, __le32 *d
 {
 	int status = 0;
 	/* wait for reg to come ready */
-	status = qlge_wait_reg_rdy(qdev,
-				   FLASH_ADDR, FLASH_ADDR_RDY, FLASH_ADDR_ERR);
+	status = qlge_wait_reg_rdy(qdev, FLASH_ADDR, FLASH_ADDR_RDY,
+				   FLASH_ADDR_ERR);
 	if (status)
 		goto exit;
 	/* set up for reg read */
 	qlge_write32(qdev, FLASH_ADDR, FLASH_ADDR_R | offset);
 	/* wait for reg to come ready */
-	status = qlge_wait_reg_rdy(qdev,
-				   FLASH_ADDR, FLASH_ADDR_RDY, FLASH_ADDR_ERR);
+	status = qlge_wait_reg_rdy(qdev, FLASH_ADDR, FLASH_ADDR_RDY,
+				   FLASH_ADDR_ERR);
 	if (status)
 		goto exit;
 	/* This data is stored on flash as an array of

@@ -792,8 +792,8 @@ static int qlge_write_xgmac_reg(struct qlge_adapter *qdev, u32 reg, u32 data)
 {
 	int status;
 	/* wait for reg to come ready */
-	status = qlge_wait_reg_rdy(qdev,
-				   XGMAC_ADDR, XGMAC_ADDR_RDY, XGMAC_ADDR_XME);
+	status = qlge_wait_reg_rdy(qdev, XGMAC_ADDR, XGMAC_ADDR_RDY,
+				   XGMAC_ADDR_XME);
 	if (status)
 		return status;
 	/* write the data to the data reg */

@@ -811,15 +811,15 @@ int qlge_read_xgmac_reg(struct qlge_adapter *qdev, u32 reg, u32 *data)
 {
 	int status = 0;
 	/* wait for reg to come ready */
-	status = qlge_wait_reg_rdy(qdev,
-				   XGMAC_ADDR, XGMAC_ADDR_RDY, XGMAC_ADDR_XME);
+	status = qlge_wait_reg_rdy(qdev, XGMAC_ADDR, XGMAC_ADDR_RDY,
+				   XGMAC_ADDR_XME);

Need a blank line after the declaration block.

Sure, I will fix it in next version.

quoted

 	if (status)
 		goto exit;
 	/* set up for reg read */
 	qlge_write32(qdev, XGMAC_ADDR, reg | XGMAC_ADDR_R);
 	/* wait for reg to come ready */
-	status = qlge_wait_reg_rdy(qdev,
-				   XGMAC_ADDR, XGMAC_ADDR_RDY, XGMAC_ADDR_XME);
+	status = qlge_wait_reg_rdy(qdev, XGMAC_ADDR, XGMAC_ADDR_RDY,
+				   XGMAC_ADDR_XME);
 	if (status)
 		goto exit;
 	/* get the data */

regards,
dan carpenter

-- 
Best regards,
Coiby