Re: [PATCH v2 4/6] tty: n_gsm: introduce gsm_control_command() function

[Jiri Slaby]

On 23. 08. 22, 8:22, D. Starke wrote:
> From: Daniel Starke <daniel.starke@xxxxxxxxxxx>
>
> Move the content of gsm_control_transmit() to a new function
> gsm_control_command() with a more generic signature and analog to
> gsm_control_reply(). Use this within gsm_control_transmit().
>
> This is needed to simplify upcoming functional additions.
>
> Signed-off-by: Daniel Starke <daniel.starke@xxxxxxxxxxx>
> ---
>   drivers/tty/n_gsm.c | 30 +++++++++++++++++++++++-------
>   1 file changed, 23 insertions(+), 7 deletions(-)
>
> No changes since v1.
>
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index 9535e84f3063..5a20561c0a5d 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -1316,6 +1316,28 @@ static void gsm_dlci_data_kick(struct gsm_dlci *dlci)
>    */
>   
>   
> +/**
> + *	gsm_control_command	-	send a command frame to a control
> + *	@gsm: gsm channel
> + *	@cmd: the command to use
> + *	@data: data to follow encoded info
> + *	@dlen: length of data
> + *
> + *	Encode up and queue a UI/UIH frame containing our command.

These tabs after asterisks are not understood by kernel-doc. You should 
add no new ones.

> + */
> +static int gsm_control_command(struct gsm_mux *gsm, int cmd, u8 *data, int dlen)

Can gsm and data be const? I assume gsm cannot due to call to 
gsm_data_queue(). And what about cmd & dlen being uint? cmd seems to be 
u8 at least...

> +{
> +	struct gsm_msg *msg = gsm_data_alloc(gsm, 0, dlen + 2, gsm->ftype);
> +
> +	if (msg == NULL)
> +		return -ENOMEM;

\n here

> +	msg->data[0] = (cmd << 1) | CR | EA;	/* Set C/R */
> +	msg->data[1] = (dlen << 1) | EA;
> +	memcpy(msg->data + 2, data, dlen);
> +	gsm_data_queue(gsm->dlci[0], msg);

\n here

> +	return 0;
> +}
> +
>   /**
>    *	gsm_control_reply	-	send a response frame to a control
>    *	@gsm: gsm channel
> @@ -1623,13 +1645,7 @@ static void gsm_control_response(struct gsm_mux *gsm, unsigned int command,
>   
>   static void gsm_control_transmit(struct gsm_mux *gsm, struct gsm_control *ctrl)
>   {
> -	struct gsm_msg *msg = gsm_data_alloc(gsm, 0, ctrl->len + 2, gsm->ftype);
> -	if (msg == NULL)
> -		return;
> -	msg->data[0] = (ctrl->cmd << 1) | CR | EA;	/* command */
> -	msg->data[1] = (ctrl->len << 1) | EA;
> -	memcpy(msg->data + 2, ctrl->data, ctrl->len);
> -	gsm_data_queue(gsm->dlci[0], msg);
> +	gsm_control_command(gsm, ctrl->cmd, ctrl->data, ctrl->len);
>   }

thanks,
--
js
suse labs