usbserial: pl2303 tx xon/xoff flow control

usbserial: pl2303 tx xon/xoff flow control

[Florian Zumbiehl]

Hi,

> > There are historical reasons for a lot of things, but that's not
> > necessarily a reason to continue taking shortcuts.
> 
> But on second thought, I think your approach here makes sense. If
> usbserial ever gains generic IXON support, we'd just fall back to the
> line-discipline implementation whenever the control characters do no
> match.

Which wouldn't preclude indicating lack of support while the support is
lacking?! I mean, unless there is a good reason to pretend ...

(And if anything, the usb serial framework could use that indication to
recognize the lack of support in a given driver for a given configuration,
while with the current code there is no way to determine when a
generic/software implementation would have to be enabled?!)

> bools), and mention why you implemented things the way you did in the
> commit message.

Not sure what to mention there?! I mean, for the IXON/!IXANY/^S/^Q case, I
implemented things the way I did because that makes the hardware behave the
way that a serial interface should behave when IXON/!IXANY/^S/^Q is
configured, obviously. For all other cases, I have no clue why the
behaviour is the way it is, as I am just preserving existing behaviour that
was decided by others before me and the reasons for which are unknown to
me.

Regards, Florian
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

usbserial: pl2303 tx xon/xoff flow control

[Johan Hovold]

On Sun, May 20, 2018 at 02:22:27AM +0200, Florian Zumbiehl wrote:

> Before I investigated how to implement this patch I just saw that s/w flow
> control "didn't work", but my assumption was that that was due to buffering
> latencies, not because the kernel just ignored the request. But then,
> chances are a software implementation indeed wouldn't work very well anyway
> for exactly that reason?!

Indeed, the deep queues might prevent a software implementation from
being very useful.

> > The line discipline implementation kicks in whenever IXON is set, and
> > can be used as a fallback for devices where automatic hardware and
> > software flow control cannot be enabled concurrently in hardware for
> > example.
> 
> Well, yeah, my guess would be that to actually make it work (well), one
> would need more than that? Like, implement rate control in software to keep
> the hardware buffers empty in order to achieve short reaction times? Which
> one would probably want to disable though when it's not needed in order to
> take advantage of the buffers?

Maybe, but chances are none of this is worth the added complexity. If
you need XON/XOFF you should get a device which supports it in hardware.

> > While non-hardware assisted usb serial XON/XOFF is currently broken in
> > that transmission would not be halted, the line discipline would still
> > swallow any escape characters.
> 
> Actually, that is required even for the pl2303, as it does not swallow the
> control characters itself.

Yeah, I noticed that too.

Thanks,
Johan
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

usbserial: pl2303 tx xon/xoff flow control

[Johan Hovold]

On Fri, May 18, 2018 at 06:06:53AM +0200, Florian Zumbiehl wrote:
> Hi,
> 
> > > There are historical reasons for a lot of things, but that's not
> > > necessarily a reason to continue taking shortcuts.
> > 
> > But on second thought, I think your approach here makes sense. If
> > usbserial ever gains generic IXON support, we'd just fall back to the
> > line-discipline implementation whenever the control characters do no
> > match.
> 
> Which wouldn't preclude indicating lack of support while the support is
> lacking?! I mean, unless there is a good reason to pretend ...

Fixing this would be a bit involved. Consider the case where automatic
RTS/CTS cannot be used with XOR/XOFF (pl2303 or ftdi). Here we cannot
simply clear IXON when CRTSCTS is set, just because usbserial does not
support the line discipline IXON implementation.

So I'd rather leave things as is for now.

> (And if anything, the usb serial framework could use that indication to
> recognize the lack of support in a given driver for a given configuration,
> while with the current code there is no way to determine when a
> generic/software implementation would have to be enabled?!)

The line discipline implementation kicks in whenever IXON is set, and
can be used as a fallback for devices where automatic hardware and
software flow control cannot be enabled concurrently in hardware for
example.

While non-hardware assisted usb serial XON/XOFF is currently broken in
that transmission would not be halted, the line discipline would still
swallow any escape characters.

> > bools), and mention why you implemented things the way you did in the
> > commit message.
> 
> Not sure what to mention there?! I mean, for the IXON/!IXANY/^S/^Q case, I
> implemented things the way I did because that makes the hardware behave the
> way that a serial interface should behave when IXON/!IXANY/^S/^Q is
> configured, obviously. For all other cases, I have no clue why the
> behaviour is the way it is, as I am just preserving existing behaviour that
> was decided by others before me and the reasons for which are unknown to
> me.

Then please put that in the commit message (i.e. that you do not know
how to change the control characters or enable IXANY so in that case we
fall back to the broken generic implementation).

Johan
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

usbserial: pl2303 tx xon/xoff flow control

[Johan Hovold]

On Fri, May 18, 2018 at 06:06:09AM +0200, Florian Zumbiehl wrote:
> Hi,
> 
> > That looks like an HX device according to the current way we identify
> > these types (which may be wrong).
> 
> That at least matches the labeling on the chip, so I guess that might be
> correct?

Then it's an HX (even if the driver algorithm for detecting the type may
be wrong).

> > Depends on how you name your helper, I'd say. Another option could be to
> > use an intermediate bool. Either way, the above is hardly readable and
> > must be fixed.
> 
> I don't think a descriptive name would make up for the overhead of the
> indirection as it necessarily still hides what exactly is being tested from
> view, but doesn't add anything to readability as the basic function of the
> condition is obvious from the code itself (a list of fields being checked
> for changes from old to new).

The compiler would inline the function, and this isn't a hot path
anyway.

> But assigning the result to a variable inline seems fine to me as that
> doesn't introduce any indirection overhead.

Great, then please do that.

> > > I don't know how to enable IXANY or change the control characters, so I am
> > > working with what I do know (also, I guess the screenshot of the "ROM
> > > programming tool" in the "datasheet" suggests that none of that is
> > > supported).
> > 
> > Fair enough, that's why I asked. My device exhibits the same behaviour
> > with regards to IXANY by the way.
> 
> I am not sure I understand. Are you saying your device exhibits IXANY
> behaviour if you enable IXON with the patch applied? Mine definitely does
> not, I just re-checked, it receives other data just fine, but only resumes
> transmission when ^Q is received.

Mine exhibits the same behaviour *as yours*.

> > > Now, I agree that signaling lack of support would be better in general, but
> > > it does change what the code does for (still) unsupported cases. After all,
> > > usbserial in general does not signal that it doesn't support IXON/IXANY,
> > > but just pretends that it does. So, if there is a good reason why usbserial
> > > ignores POSIX on this point, I would think that would still apply for the
> > > remaining unsupported cases after partial support is implemented?!
> > 
> > There are historical reasons for a lot of things, but that's not
> > necessarily a reason to continue taking shortcuts.
> 
> Well, sure, I just assumed that there was a *good* reason for why it is the
> way it is, for lack of any information to the contrary, and so just
> preserved the existing behaviour for cases that I wasn't obviously
> improving.

Yep, and after taking a closer look at this, that seems sensible.

The usbserial code has behaved this way for decades without anyone
really complaining, and fixing this up properly would require quite a
bit of work.

I just don't think that should block this patch.

Johan
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

usbserial: pl2303 tx xon/xoff flow control

[Florian Zumbiehl]

Hi,

> That looks like an HX device according to the current way we identify
> these types (which may be wrong).

That at least matches the labeling on the chip, so I guess that might be
correct?

> Continuation lines (e.g. the broken if statement) should be indented
> substantially to the right, which some subsystems interpret as at least
> two tabs further (you used just one).
> 
> 	if (........................................... &&
> 			...) {
> 		return;
> 	}

Ah, that's what is meant by that (i.e., to the right relative to the body,
not (just) the surrounding code).

> Depends on how you name your helper, I'd say. Another option could be to
> use an intermediate bool. Either way, the above is hardly readable and
> must be fixed.

I don't think a descriptive name would make up for the overhead of the
indirection as it necessarily still hides what exactly is being tested from
view, but doesn't add anything to readability as the basic function of the
condition is obvious from the code itself (a list of fields being checked
for changes from old to new).

But assigning the result to a variable inline seems fine to me as that
doesn't introduce any indirection overhead.

> > I don't know how to enable IXANY or change the control characters, so I am
> > working with what I do know (also, I guess the screenshot of the "ROM
> > programming tool" in the "datasheet" suggests that none of that is
> > supported).
> 
> Fair enough, that's why I asked. My device exhibits the same behaviour
> with regards to IXANY by the way.

I am not sure I understand. Are you saying your device exhibits IXANY
behaviour if you enable IXON with the patch applied? Mine definitely does
not, I just re-checked, it receives other data just fine, but only resumes
transmission when ^Q is received.

> > Now, I agree that signaling lack of support would be better in general, but
> > it does change what the code does for (still) unsupported cases. After all,
> > usbserial in general does not signal that it doesn't support IXON/IXANY,
> > but just pretends that it does. So, if there is a good reason why usbserial
> > ignores POSIX on this point, I would think that would still apply for the
> > remaining unsupported cases after partial support is implemented?!
> 
> There are historical reasons for a lot of things, but that's not
> necessarily a reason to continue taking shortcuts.

Well, sure, I just assumed that there was a *good* reason for why it is the
way it is, for lack of any information to the contrary, and so just
preserved the existing behaviour for cases that I wasn't obviously
improving.

> > However, in any case, the part of setting particular control characters
> > only when IXON is set seems like a very bad idea, in that it potentially
> > changes settings where no change was requested, so userspace code cannot
> > really be expected to check for that (and certainly not based on POSIX,
> > which explicitly states that other fields have to stay unchanged). So, if
> > we want to go with indicating lack of support, I think the correct strategy
> > would be to always force the control characters to what the hardware
> > supports.
> 
> You're right, but since the defaults termios control characters match
> what the device uses, you can still reset them whenever user space tries
> to change them to indicate lack of support (instead of silently
> disabling sw flow control while leaving IXON set).

Well, yeah, that's what I suggested--while you had suggested to do so only
when IXON was set, which would be problematic IMO. Making them completely
unchangeable should be fine I'd think.

Regards, Florian
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

usbserial: pl2303 tx xon/xoff flow control

[Johan Hovold]

On Thu, May 17, 2018 at 10:29:14AM +0200, Johan Hovold wrote:
> On Thu, May 17, 2018 at 05:39:21AM +0200, Florian Zumbiehl wrote:

> > Now, I agree that signaling lack of support would be better in general, but
> > it does change what the code does for (still) unsupported cases. After all,
> > usbserial in general does not signal that it doesn't support IXON/IXANY,
> > but just pretends that it does. So, if there is a good reason why usbserial
> > ignores POSIX on this point, I would think that would still apply for the
> > remaining unsupported cases after partial support is implemented?!
> 
> There are historical reasons for a lot of things, but that's not
> necessarily a reason to continue taking shortcuts.

But on second thought, I think your approach here makes sense. If
usbserial ever gains generic IXON support, we'd just fall back to the
line-discipline implementation whenever the control characters do no
match.

Just clean up some of the lengthy conditionals (e.g. using intermediate
bools), and mention why you implemented things the way you did in the
commit message.

Thanks,
Johan
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

usbserial: pl2303 tx xon/xoff flow control

[Johan Hovold]

On Thu, May 17, 2018 at 05:39:21AM +0200, Florian Zumbiehl wrote:
> Hi,
> 
> > > Note that the patch has only been fully tested with kernel 4.9.28, and
> > > test-compiled against 4.16.8. Tested only with the pl2303 adapters I have
> > > available (which seem to be type 0 if I interpret the code correctly), I
> > > don't have a clue whether this works with other versions of the chip.
> > 
> > What's the usb-devices (or lsusb -v) output for your device?
> 
> | T:  Bus=06 Lev=01 Prnt=01 Port=02 Cnt=02 Dev#=  4 Spd=12  MxCh= 0
> | D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
> | P:  Vendor=067b ProdID=2303 Rev=03.00
> | S:  Manufacturer=Prolific Technology Inc.
> | S:  Product=USB-Serial Controller
> | C:  #Ifs= 1 Cfg#= 1 Atr=80 MxPwr=100mA
> | I:  If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=pl2303

That looks like an HX device according to the current way we identify
these types (which may be wrong).

I have a similar device here but with Rev 04.00 and I can confirm that
your patch works with that one too.

> > > --- linux-4.16.8/drivers/usb/serial/pl2303.c.orig	2018-05-09 09:53:14.000000000 +0200
> > > +++ linux-4.16.8/drivers/usb/serial/pl2303.c	2018-05-14 04:32:27.860780856 +0200
> > > @@ -544,8 +544,12 @@ static void pl2303_set_termios(struct tt
> > >  	int ret;
> > >  	u8 control;
> > >  
> > > -	if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
> > > -		return;
> > > +	if (old_termios &&
> > > +		!(tty_termios_hw_change(&tty->termios, old_termios) ||
> > > +		  ((tty->termios.c_iflag ^ old_termios->c_iflag) & (IXON | IXANY)) ||
> > > +		  tty->termios.c_cc[VSTOP] != old_termios->c_cc[VSTOP] ||
> > > +		  tty->termios.c_cc[VSTART] != old_termios->c_cc[VSTART]))
> > 
> > Use a helper function for this (if everything is indeed needed, see
> > below).
> > 
> > > +			return;
> > 
> > Odd indentation (which checkpatch would have caught).
> 
> Well, it did, but it didn't exactly offer a suggestion as to how to
> reformat things that would still be readable, nor does the style guide,
> AFAICS, so I kept it as it is.

You used double indentation (two tabs further) for the inner block
(return statement) instead of one.

Continuation lines (e.g. the broken if statement) should be indented
substantially to the right, which some subsystems interpret as at least
two tabs further (you used just one).

	if (........................................... &&
			...) {
		return;
	}

> (Also, I doubt that had I encountered this
> condition moved out of line into a separate function that would have made
> it easier for me to follow what's going on.)

Depends on how you name your helper, I'd say. Another option could be to
use an intermediate bool. Either way, the above is hardly readable and
must be fixed.

> > >  	buf = kzalloc(7, GFP_KERNEL);
> > >  	if (!buf) {
> > > @@ -662,6 +666,9 @@ static void pl2303_set_termios(struct tt
> > >  			pl2303_vendor_write(serial, 0x0, 0x41);
> > >  		else
> > >  			pl2303_vendor_write(serial, 0x0, 0x61);
> > > +	} else if (I_IXON(tty) && !I_IXANY(tty) &&
> > > +		   START_CHAR(tty) == 0x11 && STOP_CHAR(tty) == 0x13) {
> > 
> > If the device does not support IXANY, I think you should just clear that
> > bit to indicate lack of support.
> > 
> > And the start and stop characters cannot be modified (as far as you
> > know)? Then we should probably set these in the termios whenever IXON is
> > set as well since usbserial does not support falling back to the
> > line-discipline IXON implementation.
> 
> I don't know how to enable IXANY or change the control characters, so I am
> working with what I do know (also, I guess the screenshot of the "ROM
> programming tool" in the "datasheet" suggests that none of that is
> supported).

Fair enough, that's why I asked. My device exhibits the same behaviour
with regards to IXANY by the way.

> Now, I agree that signaling lack of support would be better in general, but
> it does change what the code does for (still) unsupported cases. After all,
> usbserial in general does not signal that it doesn't support IXON/IXANY,
> but just pretends that it does. So, if there is a good reason why usbserial
> ignores POSIX on this point, I would think that would still apply for the
> remaining unsupported cases after partial support is implemented?!

There are historical reasons for a lot of things, but that's not
necessarily a reason to continue taking shortcuts.

> However, in any case, the part of setting particular control characters
> only when IXON is set seems like a very bad idea, in that it potentially
> changes settings where no change was requested, so userspace code cannot
> really be expected to check for that (and certainly not based on POSIX,
> which explicitly states that other fields have to stay unchanged). So, if
> we want to go with indicating lack of support, I think the correct strategy
> would be to always force the control characters to what the hardware
> supports.

You're right, but since the defaults termios control characters match
what the device uses, you can still reset them whenever user space tries
to change them to indicate lack of support (instead of silently
disabling sw flow control while leaving IXON set).

> > > +			pl2303_vendor_write(serial, 0x0, 0xc0);
> > 
> > Odd indentation again.
> 
> See above.

You too. ;)

Thanks,
Johan
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

usbserial: pl2303 tx xon/xoff flow control

[Florian Zumbiehl]

Hi,

> > Note that the patch has only been fully tested with kernel 4.9.28, and
> > test-compiled against 4.16.8. Tested only with the pl2303 adapters I have
> > available (which seem to be type 0 if I interpret the code correctly), I
> > don't have a clue whether this works with other versions of the chip.
> 
> What's the usb-devices (or lsusb -v) output for your device?

| T:  Bus=06 Lev=01 Prnt=01 Port=02 Cnt=02 Dev#=  4 Spd=12  MxCh= 0
| D:  Ver= 1.10 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs=  1
| P:  Vendor=067b ProdID=2303 Rev=03.00
| S:  Manufacturer=Prolific Technology Inc.
| S:  Product=USB-Serial Controller
| C:  #Ifs= 1 Cfg#= 1 Atr=80 MxPwr=100mA
| I:  If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=pl2303

> > --- linux-4.16.8/drivers/usb/serial/pl2303.c.orig	2018-05-09 09:53:14.000000000 +0200
> > +++ linux-4.16.8/drivers/usb/serial/pl2303.c	2018-05-14 04:32:27.860780856 +0200
> > @@ -544,8 +544,12 @@ static void pl2303_set_termios(struct tt
> >  	int ret;
> >  	u8 control;
> >  
> > -	if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
> > -		return;
> > +	if (old_termios &&
> > +		!(tty_termios_hw_change(&tty->termios, old_termios) ||
> > +		  ((tty->termios.c_iflag ^ old_termios->c_iflag) & (IXON | IXANY)) ||
> > +		  tty->termios.c_cc[VSTOP] != old_termios->c_cc[VSTOP] ||
> > +		  tty->termios.c_cc[VSTART] != old_termios->c_cc[VSTART]))
> 
> Use a helper function for this (if everything is indeed needed, see
> below).
> 
> > +			return;
> 
> Odd indentation (which checkpatch would have caught).

Well, it did, but it didn't exactly offer a suggestion as to how to
reformat things that would still be readable, nor does the style guide,
AFAICS, so I kept it as it is. (Also, I doubt that had I encountered this
condition moved out of line into a separate function that would have made
it easier for me to follow what's going on.)

> >  	buf = kzalloc(7, GFP_KERNEL);
> >  	if (!buf) {
> > @@ -662,6 +666,9 @@ static void pl2303_set_termios(struct tt
> >  			pl2303_vendor_write(serial, 0x0, 0x41);
> >  		else
> >  			pl2303_vendor_write(serial, 0x0, 0x61);
> > +	} else if (I_IXON(tty) && !I_IXANY(tty) &&
> > +		   START_CHAR(tty) == 0x11 && STOP_CHAR(tty) == 0x13) {
> 
> If the device does not support IXANY, I think you should just clear that
> bit to indicate lack of support.
> 
> And the start and stop characters cannot be modified (as far as you
> know)? Then we should probably set these in the termios whenever IXON is
> set as well since usbserial does not support falling back to the
> line-discipline IXON implementation.

I don't know how to enable IXANY or change the control characters, so I am
working with what I do know (also, I guess the screenshot of the "ROM
programming tool" in the "datasheet" suggests that none of that is
supported).

Now, I agree that signaling lack of support would be better in general, but
it does change what the code does for (still) unsupported cases. After all,
usbserial in general does not signal that it doesn't support IXON/IXANY,
but just pretends that it does. So, if there is a good reason why usbserial
ignores POSIX on this point, I would think that would still apply for the
remaining unsupported cases after partial support is implemented?!

However, in any case, the part of setting particular control characters
only when IXON is set seems like a very bad idea, in that it potentially
changes settings where no change was requested, so userspace code cannot
really be expected to check for that (and certainly not based on POSIX,
which explicitly states that other fields have to stay unchanged). So, if
we want to go with indicating lack of support, I think the correct strategy
would be to always force the control characters to what the hardware
supports.

> > +			pl2303_vendor_write(serial, 0x0, 0xc0);
> 
> Odd indentation again.

See above.

Regards, Florian
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

usbserial: pl2303 tx xon/xoff flow control

[Johan Hovold]

On Mon, May 14, 2018 at 05:15:11AM +0200, Florian Zumbiehl wrote:
> Support hardware-level Xon/Xoff flow control in transmit direction with
> pl2303.
> 
> Signed-off-by: Florian Zumbiehl <florz@florz.de>
> ---
> Note that the patch has only been fully tested with kernel 4.9.28, and
> test-compiled against 4.16.8. Tested only with the pl2303 adapters I have
> available (which seem to be type 0 if I interpret the code correctly), I
> don't have a clue whether this works with other versions of the chip.

What's the usb-devices (or lsusb -v) output for your device?

> --- linux-4.16.8/drivers/usb/serial/pl2303.c.orig	2018-05-09 09:53:14.000000000 +0200
> +++ linux-4.16.8/drivers/usb/serial/pl2303.c	2018-05-14 04:32:27.860780856 +0200
> @@ -544,8 +544,12 @@ static void pl2303_set_termios(struct tt
>  	int ret;
>  	u8 control;
>  
> -	if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
> -		return;
> +	if (old_termios &&
> +		!(tty_termios_hw_change(&tty->termios, old_termios) ||
> +		  ((tty->termios.c_iflag ^ old_termios->c_iflag) & (IXON | IXANY)) ||
> +		  tty->termios.c_cc[VSTOP] != old_termios->c_cc[VSTOP] ||
> +		  tty->termios.c_cc[VSTART] != old_termios->c_cc[VSTART]))

Use a helper function for this (if everything is indeed needed, see
below).

> +			return;

Odd indentation (which checkpatch would have caught).

>  
>  	buf = kzalloc(7, GFP_KERNEL);
>  	if (!buf) {
> @@ -662,6 +666,9 @@ static void pl2303_set_termios(struct tt
>  			pl2303_vendor_write(serial, 0x0, 0x41);
>  		else
>  			pl2303_vendor_write(serial, 0x0, 0x61);
> +	} else if (I_IXON(tty) && !I_IXANY(tty) &&
> +		   START_CHAR(tty) == 0x11 && STOP_CHAR(tty) == 0x13) {

If the device does not support IXANY, I think you should just clear that
bit to indicate lack of support.

And the start and stop characters cannot be modified (as far as you
know)? Then we should probably set these in the termios whenever IXON is
set as well since usbserial does not support falling back to the
line-discipline IXON implementation.

> +			pl2303_vendor_write(serial, 0x0, 0xc0);

Odd indentation again.

>  	} else {
>  		pl2303_vendor_write(serial, 0x0, 0x0);
>  	}

Thanks,
Johan
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

usbserial: pl2303 tx xon/xoff flow control

[Florian Zumbiehl]

Support hardware-level Xon/Xoff flow control in transmit direction with
pl2303.

Signed-off-by: Florian Zumbiehl <florz@florz.de>
---
Note that the patch has only been fully tested with kernel 4.9.28, and
test-compiled against 4.16.8. Tested only with the pl2303 adapters I have
available (which seem to be type 0 if I interpret the code correctly), I
don't have a clue whether this works with other versions of the chip.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--- linux-4.16.8/drivers/usb/serial/pl2303.c.orig	2018-05-09 09:53:14.000000000 +0200
+++ linux-4.16.8/drivers/usb/serial/pl2303.c	2018-05-14 04:32:27.860780856 +0200
@@ -544,8 +544,12 @@ static void pl2303_set_termios(struct tt
 	int ret;
 	u8 control;
 
-	if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
-		return;
+	if (old_termios &&
+		!(tty_termios_hw_change(&tty->termios, old_termios) ||
+		  ((tty->termios.c_iflag ^ old_termios->c_iflag) & (IXON | IXANY)) ||
+		  tty->termios.c_cc[VSTOP] != old_termios->c_cc[VSTOP] ||
+		  tty->termios.c_cc[VSTART] != old_termios->c_cc[VSTART]))
+			return;
 
 	buf = kzalloc(7, GFP_KERNEL);
 	if (!buf) {
@@ -662,6 +666,9 @@ static void pl2303_set_termios(struct tt
 			pl2303_vendor_write(serial, 0x0, 0x41);
 		else
 			pl2303_vendor_write(serial, 0x0, 0x61);
+	} else if (I_IXON(tty) && !I_IXANY(tty) &&
+		   START_CHAR(tty) == 0x11 && STOP_CHAR(tty) == 0x13) {
+			pl2303_vendor_write(serial, 0x0, 0xc0);
 	} else {
 		pl2303_vendor_write(serial, 0x0, 0x0);
 	}