Re: [PATCH] staging: qlge: Fix line wrapping

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Fri, Apr 22, 2022 at 11:51:04AM +0300, Lungash wrote:
> This patch fixes line wrapping following kernel coding style.
> 
> Task on TODO list
> 
> * fix weird line wrapping (all over, ex. the ql_set_routing_reg() calls in
>   qlge_set_multicast_list()).
> 
> Signed-off-by: Lungash <denzlungash@gmail.com>

We need a "full" name here, whatever you sign legal documents with.

> ---
>  drivers/staging/qlge/qlge_main.c | 235 ++++++++++++++-----------------
>  1 file changed, 107 insertions(+), 128 deletions(-)
> 
> diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
> index 113a3efd12e9..309db00e0b22 100644
> --- a/drivers/staging/qlge/qlge_main.c
> +++ b/drivers/staging/qlge/qlge_main.c
> @@ -499,77 +499,57 @@ static int qlge_set_routing_reg(struct qlge_adapter *qdev, u32 index, u32 mask,
>  
>  	switch (mask) {
>  	case RT_IDX_CAM_HIT:
> -		{
> -			value = RT_IDX_DST_CAM_Q |	/* dest */
> -			    RT_IDX_TYPE_NICQ |	/* type */
> -			    (RT_IDX_CAM_HIT_SLOT << RT_IDX_IDX_SHIFT);/* index */
> -			break;
> -		}
> +		value = RT_IDX_DST_CAM_Q |	/* dest */
> +			RT_IDX_TYPE_NICQ |	/* type */
> +			(RT_IDX_CAM_HIT_SLOT << RT_IDX_IDX_SHIFT);/* index */
> +		break;

The original was fine, but yes, the {} can be removed, but that does not
have to do with the TODO item here.  Please only do one type of fixup at
a time.

>  
> -static int qlge_validate_flash(struct qlge_adapter *qdev, u32 size, const char *str)
> +static int qlge_validate_flash(struct qlge_adapter *qdev, u32 size,
> +			       const char *str)

You just made this look worse, why?

> -static int qlge_read_flash_word(struct qlge_adapter *qdev, int offset, __le32 *data)
> +static int qlge_read_flash_word(struct qlge_adapter *qdev, int offset,
> +				__le32 *data)

Same here, why change the original?

> @@ -2952,8 +2936,8 @@ static int qlge_start_rx_ring(struct qlge_adapter *qdev, struct rx_ring *rx_ring
>  		(rx_ring->cq_id * RX_RING_SHADOW_SPACE);
>  	u64 shadow_reg_dma = qdev->rx_ring_shadow_reg_dma +
>  		(rx_ring->cq_id * RX_RING_SHADOW_SPACE);
> -	void __iomem *doorbell_area =
> -		qdev->doorbell_area + (DB_PAGE_SIZE * (128 + rx_ring->cq_id));
> +	void __iomem *doorbell_area = qdev->doorbell_area +
> +		(DB_PAGE_SIZE * (128 + rx_ring->cq_id));

This does not look better, why not put it all on one line?


thanks,

greg k-h