usbserial: pl2303 tx xon/xoff flow control
Quoted 7 lines · Florian Zumbiehl
Hi,
Quoted 2 lines
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).
Quoted 9 lines
Quoted 3 lines
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.
Quoted 2 lines
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.
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
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).
Quoted 7 lines
Quoted 2 lines
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*.
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
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?!
Quoted 7 lines
Quoted 2 lines
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html