Re: [PATCH] cdc-acm: acm_init: Set initial BAUD to B0

[Johan Hovold <johan@kernel.org>]

On Fri, Jul 10, 2020 at 10:16:33AM +0000, Joakim Tjernlund wrote:
> On Fri, 2020-07-10 at 11:54 +0200, Johan Hovold wrote:
> > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> > 
> > 
> > On Fri, Jul 10, 2020 at 11:35:18AM +0200, Joakim Tjernlund wrote:
> > > BO will disable USB input until the device opens. This will
> > > avoid garbage chars waiting flood the TTY. This mimics a real UART
> > > device better.
> > > For initial termios to reach USB core, USB driver has to be
> > > registered before TTY driver.
> > > 
> > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> > > Cc: stable@vger.kernel.org
> > > ---
> > > 
> > >  I hope this change makes sense to you, if so I belive
> > >  ttyUSB could do the same.
> > 
> > No, this doesn't make sense. B0 is used to hang up an already open tty.
> 
> This is at module init so there is no tty yet.

Right, but init_termios is used for each tty later registered.

And B0 is supposed to be set after opening a tty if you want to trigger
a modem disconnect (deassert DTR).

> acm_probe() will later set:
>         acm->line.dwDTERate = cpu_to_le32(9600);
> 	acm->line.bDataBits = 8;
> 	acm_set_line(acm, &acm->line);
> 
> > 
> > Furthermore, this change only affects the initial terminal settings and
> > won't have any effect the next time you open the same port.
> 
> hmm, it is not ideal but acm_probe() will later set:
>         acm->line.dwDTERate = cpu_to_le32(9600);
> 	acm->line.bDataBits = 8;
> 	acm_set_line(acm, &acm->line);

That's only during probe and won't affect a second open.

> But, would it not make sense to not accept input until TTY is opened ?
> That would be more inline with a real RS232, would it not?

It's a quirk of some of these devices that some will accept input before
the tty has been opened. It should be possible to handle using flow
control if you control the other side of the application.

I don't see how this patch would work at all as B0 only deasserts DTR
when initialising the line settings in set_termios() (for example)
during open but then the tty layer will assert it again shortly after.
And your firmware ignores DTR anyway...

Ahh, I see now that the driver is passing zero as dwDTERate to the
device in set_termios() if B0 is requested, which is non-standard
(drivers typically keep the current rate or set 9600 baud). Perhaps your
firmware happens to disable the UART, but I can't seem to find anything
in the spec that says that this is what is supposed to happen.

> > Why not fix your firmware so that it doesn't transmit before DTR is
> > asserted during open()?
> 
> I would but it is not my firmware, it is a ready made USB to UART
> chip. will talk to the manufacturer though.

Hope that works out.

Johan