linux-kernel - Re: [RFC 17/19] staging: qlge: fix weird line wrapping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 22 Jun 2021 11:46:11 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Coiby Xu <coiby.xu@...il.com>
Cc:     linux-staging@...ts.linux.dev, netdev@...r.kernel.org,
        Benjamin Poirier <benjamin.poirier@...il.com>,
        Shung-Hsi Yu <shung-hsi.yu@...e.com>,
        Manish Chopra <manishc@...vell.com>,
        "supporter:QLOGIC QLGE 10Gb ETHERNET DRIVER" 
        <GR-Linux-NIC-Dev@...vell.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        open list <linux-kernel@...r.kernel.org>,
        "open list:CLANG/LLVM BUILD SUPPORT" 
        <clang-built-linux@...glegroups.com>
Subject: Re: [RFC 17/19] staging: qlge: fix weird line wrapping

On Mon, Jun 21, 2021 at 09:49:00PM +0800, Coiby Xu wrote:
> @@ -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?

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


>  			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.

>  			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.

>  	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