Date:   Thu, 2 Feb 2023 16:37:42 +0100
From:   Greg KH <gregkh@linuxfoundation.org>
To:     "D. Starke" <daniel.starke@siemens.com>
Cc:     linux-serial@vger.kernel.org, jirislaby@kernel.org,
        ilpo.jarvinen@linux.intel.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/3] tty: n_gsm: add keep alive support
Message-ID: <Y9vYxgGd6kC+ZIgR@kroah.com>
References: <20230202145934.22641-1-daniel.starke@siemens.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230202145934.22641-1-daniel.starke@siemens.com>
Precedence: bulk

On Thu, Feb 02, 2023 at 03:59:32PM +0100, D. Starke wrote:
> From: Daniel Starke <daniel.starke@siemens.com>
> 
> n_gsm is based on the 3GPP 07.010 and its newer version is the 3GPP 27.010.
> See https://portal.3gpp.org/desktopmodules/Specifications/SpecificationDetails.aspx?specificationId=1516
> The changes from 07.010 to 27.010 are non-functional. Therefore, I refer to
> the newer 27.010 here. Chapters 5.4.6.3.4 and 5.1.8.1.3 describe the test
> command which can be used to test the mux connection between both sides.
> 
> Currently, no algorithm is implemented to make use of this command. This
> requires that each multiplexed upper layer protocol supervises the
> underlying muxer connection to handle possible connection losses.
> 
> Introduce an ioctl commands and functions to optionally enable keep alive
> handling via the test command as described in chapter 5.4.6.3.4. A single
> incrementing octet is being used to distinguish between multiple retries.
> Retry count and interval are taken from the general parameters N2 and T2.
> 
> Note that support for the test command is mandatory and already present in
> the muxer implementation since the very first version.
> Also note that the previous ioctl structure gsm_config cannot be extended
> due to missing checks against zero of the field "unused".
> 
> Signed-off-by: Daniel Starke <daniel.starke@siemens.com>
> ---
>  drivers/tty/n_gsm.c         | 106 +++++++++++++++++++++++++++++++++++-
>  include/uapi/linux/gsmmux.h |  17 ++++--
>  2 files changed, 117 insertions(+), 6 deletions(-)
> 
> v1 -> v2:
> The "unused" fields of "gsm_config" and "gsm_netconfig" have been marked as
> unusable due to missing checks against zero. See review comments.

Please just make the first patch marking these as "unable to be used"
and document why this is so, as it's independant of this new feature you
are adding.

Remember, only do one logical thing per patch.

thanks,

greg k-h