Re: [PATCH v2 6/7] USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for ASYNC_SPD_CUST

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sunday 24 July 2022 14:54:58 Johan Hovold wrote:
> On Sun, Jul 24, 2022 at 02:33:51PM +0200, Pali Rohár wrote:
> > On Sunday 24 July 2022 14:28:43 Johan Hovold wrote:
> > > On Tue, Jul 12, 2022 at 01:53:05PM +0200, Marek Behún wrote:
> > > > From: Pali Rohár <pali@xxxxxxxxxx>
> > > > 
> > > > When ASYNC_SPD_CUST is used, update custom_divisor and c_*speed fields
> > > > so that they correspond to the newly set baud rate value, so that
> > > > userspace GET ioctls will see the true baud rate that is being used.
> > > 
> > > No, this is wrong.
> > > 
> > > In fact, there's a long-standing bug in this driver which started
> > > reporting back the actual baud rate when using SPD_CUST.
> > 
> > Hello! And this commit is fixing also this bug as a side change.
> 
> Ah, indeed it is, or at least to some extent.
> 
> > > The rate should be left unchanged at 38400 in that case.
> > 
> > With this change, rate in c_cflag is unchanged and stays at B38400.
> 
> Right.
> 
> > What is updated is the real baudrate in c_ispeed and c_ospeed
> > extensions.
> > 
> > It is really wrong? I thought that c_cflag should stay unchanged at
> > B38400 when ASYNC_SPD_CUST is used.
> 
> Yeah, cflags stay unchanged, but you shouldn't touch those fields when
> using the deprecated ASYNC_SPD_CUST hack.

Hm... Why? I thought that new extended fields (c_ispeed and c_ospeed)
should contain current real speed. What is the reason that c_*speed
fields should have 38400 when ASYNC_SPD_CUST hack is set?

> Note that this currently only works because the ftdi driver uses
> tty_get_baud_rate() instead of c_ospeed directly which is the
> recommended (new) way.

Yes, tty_get_baud_rate() helper function is there for this purpose,
right?

> > > > +	/* Fix deprecated async-compatible custom_divisor hack and update tty baud rate */
> > > > +	if (fix_custom_divisor) {
> > > > +		priv->custom_divisor = DIV_ROUND_CLOSEST(priv->baud_base, baud);
> > > > +		old_baud = baud;
> > > > +		baud = 38400;
> > > > +	}
> > > > +
> > > >  	tty_encode_baud_rate(tty, baud, baud);
> > > > +
> > > > +	/* For async-compatible custom_divisor store into TCGETS2 c_*speed fields real baud rate */
> > > > +	if (fix_custom_divisor)
> > > > +		tty->termios.c_ispeed = tty->termios.c_ospeed = old_baud;
> > > > +
> > > >  	return div_value;
> > > >  }
> > > >  
> > > > @@ -2674,6 +2688,8 @@ static void ftdi_set_termios(struct tty_struct *tty,
> > > >  		dev_dbg(ddev, "%s: forcing baud rate for this device\n", __func__);
> > > >  		tty_encode_baud_rate(tty, priv->force_baud,
> > > >  					priv->force_baud);
> > > > +		termios->c_ispeed = termios->c_ospeed =
> > > > +			DIV_ROUND_CLOSEST(priv->baud_base, priv->custom_divisor);
> > > >  	}
> > > >  
> > > >  	/* Force RTS-CTS if this device requires it. */
> 
> Johan



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux