Re: [PATCH] staging: gdm724x: Fix alignment in gdm_mux

[Greg KH]

On Sat, Mar 09, 2019 at 02:56:14PM -0300, Ignacio Losiggio wrote:
> This patch modifies the alignment and linebreaks in gdm_mux in order to make
> the code more concise.

"concise"?

For some of these, there is no real change needed, it's all up to the
author's "taste", so they are not needed.

> 
> Signed-off-by: Ignacio Losiggio <iglosiggio@xxxxxxxxx>
> ---
>  drivers/staging/gdm724x/gdm_mux.c | 35 ++++++++++++++-----------------
>  drivers/staging/gdm724x/gdm_mux.h |  2 +-
>  2 files changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/staging/gdm724x/gdm_mux.c b/drivers/staging/gdm724x/gdm_mux.c
> index e2a050ba6fbb..be13de12ce9f 100644
> --- a/drivers/staging/gdm724x/gdm_mux.c
> +++ b/drivers/staging/gdm724x/gdm_mux.c
> @@ -164,8 +164,7 @@ static int up_to_host(struct mux_rx *r)
>  
>  		total_len = ALIGN(MUX_HEADER_SIZE + payload_size, 4);
>  
> -		if (len - packet_size_sum <
> -			total_len) {
> +		if (len - packet_size_sum < total_len) {

This is good.

>  			pr_err("invalid payload : %d %d %04x\n",
>  			       payload_size, len, packet_type);
>  			break;
> @@ -178,11 +177,10 @@ static int up_to_host(struct mux_rx *r)
>  		}
>  
>  		ret = r->callback(mux_header->data,
> -				payload_size,
> -				index,
> -				mux_dev->tty_dev,
> -				RECV_PACKET_PROCESS_CONTINUE
> -				);
> +		                  payload_size,
> +				  index,
> +				  mux_dev->tty_dev,
> +				  RECV_PACKET_PROCESS_CONTINUE);

Shouldn't you make some of these lines longer to make less lines?

>  		if (ret == TO_HOST_BUFFER_REQUEST_FAIL) {
>  			r->offset += packet_size_sum;
>  			break;
> @@ -191,11 +189,10 @@ static int up_to_host(struct mux_rx *r)
>  		packet_size_sum += total_len;
>  		if (len - packet_size_sum <= MUX_HEADER_SIZE + 2) {
>  			ret = r->callback(NULL,
> -					0,
> -					index,
> -					mux_dev->tty_dev,
> -					RECV_PACKET_PROCESS_COMPLETE
> -					);
> +					  0,
> +					  index,
> +					  mux_dev->tty_dev,
> +					  RECV_PACKET_PROCESS_COMPLETE);

Same here.

>  			break;
>  		}
>  	}
> @@ -376,8 +373,8 @@ static int gdm_mux_send(void *priv_dev, void *data, int len, int tty_index,
>  	mux_header->packet_type = __cpu_to_le16(packet_type[tty_index]);
>  
>  	memcpy(t->buf + MUX_HEADER_SIZE, data, len);
> -	memset(t->buf + MUX_HEADER_SIZE + len, 0, total_len - MUX_HEADER_SIZE -
> -	       len);
> +	memset(t->buf + MUX_HEADER_SIZE + len, 0,
> +	       total_len - MUX_HEADER_SIZE - len);

this is good.

>  
>  	t->len = total_len;
>  	t->callback = cb;
> @@ -418,8 +415,7 @@ static int gdm_mux_send_control(void *priv_dev, int request, int value,
>  			      2,
>  			      buf,
>  			      len,
> -			      5000
> -			     );
> +			      5000);

Again, you can shorten the number of lines needed here.

>  
>  	if (ret < 0)
>  		pr_err("usb_control_msg error: %d\n", ret);
> @@ -429,9 +425,10 @@ static int gdm_mux_send_control(void *priv_dev, int request, int value,
>  
>  static void release_usb(struct mux_dev *mux_dev)
>  {
> -	struct rx_cxt		*rx = &mux_dev->rx;
> -	struct mux_rx		*r, *r_next;
> -	unsigned long		flags;
> +	struct rx_cxt *rx = &mux_dev->rx;
> +	struct mux_rx *r;
> +	struct mux_rx *r_next;
> +	unsigned long flags;

No need to change this at all.

>  
>  	cancel_delayed_work(&mux_dev->work_rx);
>  
> diff --git a/drivers/staging/gdm724x/gdm_mux.h b/drivers/staging/gdm724x/gdm_mux.h
> index 51c22e3d8aeb..5bfb95be3ed3 100644
> --- a/drivers/staging/gdm724x/gdm_mux.h
> +++ b/drivers/staging/gdm724x/gdm_mux.h
> @@ -13,7 +13,7 @@
>  #define PM_NORMAL 0
>  #define PM_SUSPEND 1
>  
> -#define USB_RT_ACM          (USB_TYPE_CLASS | USB_RECIP_INTERFACE)
> +#define USB_RT_ACM (USB_TYPE_CLASS | USB_RECIP_INTERFACE)

No tab?

thanks,

greg k-h