Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 13 May 2023 19:23:57 +0900
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 v4 8/8] tty: n_gsm: add DLCI specific rx/tx statistics
Message-ID: <2023051332-ethics-renounce-b61d@gregkh>
References: <20230426080315.7595-1-daniel.starke@siemens.com>
 <20230426080315.7595-8-daniel.starke@siemens.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230426080315.7595-8-daniel.starke@siemens.com>
Precedence: bulk

On Wed, Apr 26, 2023 at 10:03:15AM +0200, D. Starke wrote:
> From: Daniel Starke <daniel.starke@siemens.com>
> 
> Add counters for the number of data bytes received/transmitted per DLCI in
> for preparation for an upcoming patch which will expose these values to the
> user.

As this is patch 8/8, "upcoming patch" makes no sense, sorry.  Please
either drop this and add it as part of the series that provides this
functionality, or add the functionality to the patch series as the next
patch in it.

> 
> Signed-off-by: Daniel Starke <daniel.starke@siemens.com>
> ---
>  drivers/tty/n_gsm.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> v3 -> v4:
> No changes.
> 
> Link: https://lore.kernel.org/all/20230424075251.5216-8-daniel.starke@siemens.com/
> 
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index 62bff4474b57..2e2e1dafcf40 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -186,6 +186,9 @@ struct gsm_dlci {
>  	void (*data)(struct gsm_dlci *dlci, const u8 *data, int len);
>  	void (*prev_data)(struct gsm_dlci *dlci, const u8 *data, int len);
>  	struct net_device *net; /* network interface, if created */
> +	/* Statistics (not currently exposed) */

No blank line before this?

And why isn't this structure documented in kerneldoc to make it more
obvious what is happening?

thanks,

greg k-h