[v4,1/8] tty: n_gsm: add restart flag to DLC specific ioctl config

From: Daniel Starke <daniel.starke@siemens.com>

Currently, changing the parameters of a DLCI gives no direct control to the
user whether this should trigger a channel reset or not. The decision is
solely made by the driver based on the assumption which parameter changes
are compatible or not. Therefore, the user has no means to perform an
automatic channel reset after parameter configuration for non-conflicting
changes.

Add the parameter 'flags' to 'gsm_dlci_config' to force a channel reset
after ioctl setting regardless of whether the changes made require this or
not by setting this to 'GSM_FL_RESTART'.

Note that 'GSM_FL_RESTART' is currently the only allow flag to allow
additions here.

Signed-off-by: Daniel Starke <daniel.starke@siemens.com>
---
 drivers/tty/n_gsm.c         |  4 ++++
 include/uapi/linux/gsmmux.h | 13 ++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

v3 -> v4:
Changed gsm_dlci_config field name from 'restart' to 'flags' and introduced
'GSM_FL_RESTART' to set the restart flag. The patch description was changed
accordingly. This was done as suggested during the review.
The remarked kernel doc compatible field comment is done in patch 2/8.

Link: https://lore.kernel.org/all/2023042453-dubbed-botany-2ed9@gregkh/

On Wed, Apr 26, 2023 at 10:03:08AM +0200, D. Starke wrote:
> From: Daniel Starke <daniel.starke@siemens.com>
> 
> Currently, changing the parameters of a DLCI gives no direct control to the
> user whether this should trigger a channel reset or not. The decision is
> solely made by the driver based on the assumption which parameter changes
> are compatible or not. Therefore, the user has no means to perform an
> automatic channel reset after parameter configuration for non-conflicting
> changes.
> 
> Add the parameter 'flags' to 'gsm_dlci_config' to force a channel reset
> after ioctl setting regardless of whether the changes made require this or
> not by setting this to 'GSM_FL_RESTART'.
> 
> Note that 'GSM_FL_RESTART' is currently the only allow flag to allow
> additions here.
> 
> Signed-off-by: Daniel Starke <daniel.starke@siemens.com>
> ---
>  drivers/tty/n_gsm.c         |  4 ++++
>  include/uapi/linux/gsmmux.h | 13 ++++++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> v3 -> v4:
> Changed gsm_dlci_config field name from 'restart' to 'flags' and introduced
> 'GSM_FL_RESTART' to set the restart flag. The patch description was changed
> accordingly. This was done as suggested during the review.
> The remarked kernel doc compatible field comment is done in patch 2/8.
> 
> Link: https://lore.kernel.org/all/2023042453-dubbed-botany-2ed9@gregkh/
> 
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index b411a26cc092..66edcf65a4dd 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -2532,6 +2532,8 @@ static int gsm_dlci_config(struct gsm_dlci *dlci, struct gsm_dlci_config *dc, in
>  		return -EINVAL;
>  	if (dc->k > 7)
>  		return -EINVAL;
> +	if (dc->flags & ~GSM_FL_RESTART)   /* allow future extensions */
> +		return -EINVAL;
>  
>  	/*
>  	 * See what is needed for reconfiguration
> @@ -2546,6 +2548,8 @@ static int gsm_dlci_config(struct gsm_dlci *dlci, struct gsm_dlci_config *dc, in
>  	/* Requires care */
>  	if (dc->priority != dlci->prio)
>  		need_restart = true;
> +	if (dc->flags & GSM_FL_RESTART)
> +		need_restart = true;
>  
>  	if ((open && gsm->wait_config) || need_restart)
>  		need_open = true;
> diff --git a/include/uapi/linux/gsmmux.h b/include/uapi/linux/gsmmux.h
> index eb67884e5f38..958257af05ab 100644
> --- a/include/uapi/linux/gsmmux.h
> +++ b/include/uapi/linux/gsmmux.h
> @@ -2,10 +2,20 @@
>  #ifndef _LINUX_GSMMUX_H
>  #define _LINUX_GSMMUX_H
>  
> +#include <linux/const.h>
>  #include <linux/if.h>
>  #include <linux/ioctl.h>
>  #include <linux/types.h>
>  
> +/*
> + * flags definition for n_gsm
> + *
> + * Used by:
> + * struct gsm_dlci_config.flags
> + */
> +/* Force DLCI channel reset? Always cleared on retrieval. */

I do not understand this comment, sorry.  What question are you asking?
What happens if it is set?  What happens if it is not set?  More
documentation is always good, especially for new user/kernel apis that
are not documented anywhere else.

thanks,

greg k-h

> > +/* Force DLCI channel reset? Always cleared on retrieval. */
> 
> I do not understand this comment, sorry.  What question are you asking?
> What happens if it is set?  What happens if it is not set?  More
> documentation is always good, especially for new user/kernel apis that
> are not documented anywhere else.

Ok, I will rephrase it to be more explicit about it.

Best regards,
Daniel Starke