RE: [PATCH 1/1] tty: n_gsm: fix encoding of control signal octet bit DV

RE: [PATCH 1/1] tty: n_gsm: fix encoding of control signal octet bit DV

[Starke, Daniel]

> > > All of these are patch 1/1, which is odd :(
> > > 
> > > Please renumber them properly and resend.
> > 
> > All these patches are based on the current version of tty-next and are 
> > completely independent from each other. The only common part is the 
> > file they apply to. Therefore, this is not a patch series. Would you 
> > still suggest to apply a different numbering?
> 
> Yes, please send them as a patch series, numbered correctly, as a whole
> series of patches, all listed as 1/1 does not do good things for our
> tools.

I have noticed that all patches have been sent in one thread. This was not
my intention. I see two options here now:
1. Resend all patches one by one as they are completely independent.
2. Create a patch series.

And the following two variants in case of option 2:
a. Just renumber the subjects and resend all (i.e. without a cover letter).
b. Create a patch series from commits that depend on each other. Send this
   together with a cover letter.

Please let me know what you prefer.

With best regards,
Daniel Starke

Re: [PATCH 1/1] tty: n_gsm: fix encoding of control signal octet bit DV

[Greg KH]

On Thu, Feb 17, 2022 at 02:03:37PM +0000, Starke, Daniel wrote:
> > > > All of these are patch 1/1, which is odd :(
> > > > 
> > > > Please renumber them properly and resend.
> > > 
> > > All these patches are based on the current version of tty-next and are 
> > > completely independent from each other. The only common part is the 
> > > file they apply to. Therefore, this is not a patch series. Would you 
> > > still suggest to apply a different numbering?
> > 
> > Yes, please send them as a patch series, numbered correctly, as a whole
> > series of patches, all listed as 1/1 does not do good things for our
> > tools.
> 
> I have noticed that all patches have been sent in one thread. This was not
> my intention. I see two options here now:
> 1. Resend all patches one by one as they are completely independent.
> 2. Create a patch series.
> 
> And the following two variants in case of option 2:
> a. Just renumber the subjects and resend all (i.e. without a cover letter).

This is fine, and the easiest for everyone involved, as git makes it
automatic.

thanks,

greg k-h

Re: [PATCH 1/1] tty: n_gsm: fix encoding of control signal octet bit DV

[Greg KH]

On Thu, Feb 17, 2022 at 10:45:14AM +0000, Starke, Daniel wrote:
> > All of these are patch 1/1, which is odd :(
> > 
> > Please renumber them properly and resend.
> 
> All these patches are based on the current version of tty-next and are
> completely independent from each other. The only common part is the file
> they apply to. Therefore, this is not a patch series. Would you still
> suggest to apply a different numbering?

Yes, please send them as a patch series, numbered correctly, as a whole
series of patches, all listed as 1/1 does not do good things for our
tools.

thanks,

greg k-h

RE: [PATCH 1/1] tty: n_gsm: fix encoding of control signal octet bit DV

[Starke, Daniel]

> All of these are patch 1/1, which is odd :(
> 
> Please renumber them properly and resend.

All these patches are based on the current version of tty-next and are
completely independent from each other. The only common part is the file
they apply to. Therefore, this is not a patch series. Would you still
suggest to apply a different numbering?

With best regards,
Daniel Starke

Re: [PATCH 1/1] tty: n_gsm: fix encoding of control signal octet bit DV

[Greg KH]

On Thu, Feb 17, 2022 at 12:05:49AM -0800, daniel.starke@siemens.com wrote:
> 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. Chapter 5.4.6.3.7 describes the encoding of the
> control signal octet used by the MSC (modem status command). The same
> encoding is also used in convergence layer type 2 as described in chapter
> 5.5.2. Table 7 and 24 both require the DV (data valid) bit to be set 1 for
> outgoing control signal octets sent by the DTE (data terminal equipment),
> i.e. for the initiator side.
> Currently, the DV bit is only set if CD (carrier detect) is on, regardless
> of the side.
> 
> This patch fixes this behavior by setting the DV bit on the initiator side
> unconditionally.
> 
> Fixes: e1eaea46bb40 ("tty: n_gsm line discipline")
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Starke <daniel.starke@siemens.com>
> ---
>  drivers/tty/n_gsm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index 0b1808e3a912..e199315a158e 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -439,7 +439,7 @@ static u8 gsm_encode_modem(const struct gsm_dlci *dlci)
>  		modembits |= MDM_RTR;
>  	if (dlci->modem_tx & TIOCM_RI)
>  		modembits |= MDM_IC;
> -	if (dlci->modem_tx & TIOCM_CD)
> +	if (dlci->modem_tx & TIOCM_CD || dlci->gsm->initiator)
>  		modembits |= MDM_DV;
>  	return modembits;
>  }
> -- 
> 2.25.1
> 

All of these are patch 1/1, which is odd :(

Please renumber them properly and resend.

thanks,

greg k-h

[PATCH 1/1] tty: n_gsm: fix encoding of control signal octet bit DV

[daniel.starke]

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. Chapter 5.4.6.3.7 describes the encoding of the
control signal octet used by the MSC (modem status command). The same
encoding is also used in convergence layer type 2 as described in chapter
5.5.2. Table 7 and 24 both require the DV (data valid) bit to be set 1 for
outgoing control signal octets sent by the DTE (data terminal equipment),
i.e. for the initiator side.
Currently, the DV bit is only set if CD (carrier detect) is on, regardless
of the side.

This patch fixes this behavior by setting the DV bit on the initiator side
unconditionally.

Fixes: e1eaea46bb40 ("tty: n_gsm line discipline")
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Starke <daniel.starke@siemens.com>
---
 drivers/tty/n_gsm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 0b1808e3a912..e199315a158e 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -439,7 +439,7 @@ static u8 gsm_encode_modem(const struct gsm_dlci *dlci)
 		modembits |= MDM_RTR;
 	if (dlci->modem_tx & TIOCM_RI)
 		modembits |= MDM_IC;
-	if (dlci->modem_tx & TIOCM_CD)
+	if (dlci->modem_tx & TIOCM_CD || dlci->gsm->initiator)
 		modembits |= MDM_DV;
 	return modembits;
 }
-- 
2.25.1