ftdi_sio driver improvements
Subsystems (3)
v2 cover ftdi_sio driver improvements
Hi guys,
Pali has worked out some improvements for the ftdi_sio USB serial driver and I have reviewed and tested them.
Changes since v1:
- addressed issues pointed out by Greg: better commit messages, fixed checkpatch warning, dropped usage of (u32)-1
Pali Rohár (7):
USB: serial: ftdi_sio: Fix divisor overflow
USB: serial: ftdi_sio: Add missing baud rate validation
USB: serial: ftdi_sio: Extract SIO divisor code to function
USB: serial: ftdi_sio: Do not reset baud rate to 9600 Baud on error
USB: serial: ftdi_sio: Fix baud rate rounding for ASYNC_SPD_CUST
USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for
ASYNC_SPD_CUST
USB: serial: ftdi_sio: Fill c_*speed fields with real baud rate
drivers/usb/serial/ftdi_sio.c | 187 ++++++++++++++++++++++++++++------ 1 file changed, 156 insertions(+), 31 deletions(-)
Signature · 1 line
2.35.1
PATCH v2 1/7 USB: serial: ftdi_sio: Fix divisor overflow
From: Pali Rohár <[email protected]>
The baud rate generating divisor is a 17-bit wide (14 bits integer part and 3 bits fractional part).
Example:
base clock = 48 MHz
requested baud rate = 180 Baud
divisor = 48,000,000 / (16 * 180) = 0b100000100011010.101
In this case the MSB gets discarded because of the overflow, and the actually used divisor will be 0b100011010.101 = 282.625, resulting in baud rate 10615 Baud, instead of the requested 180 Baud.
The best possible thing to do in this case is to generate lowest possible baud rate (in the example it would be 183 Baud), by using maximum possible divisor.
In case of divisor overflow, use maximum possible divisor.
--- drivers/usb/serial/ftdi_sio.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/usb/serial/ftdi_sio.c +6 -0
@@ -1157,6 +1157,8 @@ static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base)int divisor3 =DIV_ROUND_CLOSEST (base,2 * baud);if ((divisor3 &0x7 ) ==7 ) divisor3++;/* round x.7/8 up to x+1 */ +if (divisor3 >GENMASK (16 ,0 )) + divisor3 =GENMASK (16 ,0 ); divisor = divisor3 >>3 ; divisor3 &=0x7 ;if (divisor3 ==1 )
@@ -1181,6 +1183,8 @@ static u32 ftdi_232bm_baud_base_to_divisor(int baud, int base)u32 divisor;/* divisor shifted 3 bits to the left */ int divisor3 =DIV_ROUND_CLOSEST (base,2 * baud); +if (divisor3 >GENMASK (16 ,0 )) + divisor3 =GENMASK (16 ,0 ); divisor = divisor3 >>3 ; divisor |= (u32 )divfrac[divisor3 &0x7 ] <<14 ;/* Deal with special cases for highest baud rates. */
@@ -1204,6 +1208,8 @@ static u32 ftdi_2232h_baud_base_to_divisor(int baud, int base)/* hi-speed baud rate is 10-bit sampling instead of 16-bit */ divisor3 =DIV_ROUND_CLOSEST (8 * base,10 * baud); +if (divisor3 >GENMASK (16 ,0 )) + divisor3 =GENMASK (16 ,0 ); divisor = divisor3 >>3 ; divisor |= (u32 )divfrac[divisor3 &0x7 ] <<14 ;
Signature · 1 line
2.35.1
Johan Hovold
maintainerRe: [PATCH v2 1/7] USB: serial: ftdi_sio: Fix divisor overflow
Quoted 10 lines · Marek Behún
From: Pali Rohár <[email protected]>
The baud rate generating divisor is a 17-bit wide (14 bits integer part and 3 bits fractional part).
Example:
base clock = 48 MHz
requested baud rate = 180 Baud
divisor = 48,000,000 / (16 * 180) = 0b100000100011010.101
Quoted 7 lines
In this case the MSB gets discarded because of the overflow, and the actually used divisor will be 0b100011010.101 = 282.625, resulting in baud rate 10615 Baud, instead of the requested 180 Baud.
The best possible thing to do in this case is to generate lowest possible baud rate (in the example it would be 183 Baud), by using maximum possible divisor.
Actually, the best way to handle this is to add a sanity check for the lowest supported check as you do in the next patch. That one makes this change superfluous.
Quoted 1 line
In case of divisor overflow, use maximum possible divisor.
Johan
PATCH v2 2/7 USB: serial: ftdi_sio: Add missing baud rate validation
In reply to Marek BehúnFrom: Pali Rohár <[email protected]>
Add lower checks for requested baud rate for FT8U232AM, FT232BM, FT2232C, FT232RL, FTX, FT2232H, FT4232H and FT232H. For all of these the minimum baud rate they can generate is 183 Baud.
--- drivers/usb/serial/ftdi_sio.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/usb/serial/ftdi_sio.c +3 -3
@@ -1330,7 +1330,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, }break ;case FT8U232AM :/* 8U232AM chip */ -if (baud <=3000000 ) { +if (baud >=183 && baud <=3000000 ) { div_value =ftdi_232am_baud_to_divisor (baud); }else {dev_dbg (dev,"%s - Baud rate too high!\n" , __func__);
@@ -1343,7 +1343,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,case FT2232C :/* FT2232C chip */ case FT232RL :/* FT232RL chip */ case FTX :/* FT-X series */ -if (baud <=3000000 ) { +if (baud >=183 && baud <=3000000 ) {u16 product_id =le16_to_cpu ( port->serial->dev->descriptor.idProduct); if (((product_id == FTDI_NDI_HUC_PID) ||
@@ -1367,7 +1367,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,case FT232H :/* FT232H chip */ if ((baud <=12000000 ) && (baud >=1200 )) { div_value =ftdi_2232h_baud_to_divisor (baud); - }else if (baud <1200 ) { + }else if (baud >=183 && baud <1200 ) { div_value =ftdi_232bm_baud_to_divisor (baud); }else {dev_dbg (dev,"%s - Baud rate too high!\n" , __func__);
Signature · 1 line
2.35.1
Re: [PATCH v2 2/7] USB: serial: ftdi_sio: Add missing baud rate validation
Quoted 18 lines · Marek Behún
From: Pali Rohár <[email protected]>
Add lower checks for requested baud rate for FT8U232AM, FT232BM, FT2232C, FT232RL, FTX, FT2232H, FT4232H and FT232H. For all of these the minimum baud rate they can generate is 183 Baud.
Signed-off-by Pali Rohár <[email protected]>Tested-by Marek Behún <[email protected]>Signed-off-by Marek Behún <[email protected]>--- drivers/usb/serial/ftdi_sio.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)drivers/usb/serial/ftdi_sio.c +0
-0
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1336
Quoted 8 lines
drivers/usb/serial/ftdi_sio.c +1
-1@@ -1330,7 +1330,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, }break ;case FT8U232AM :/* 8U232AM chip */ -if (baud <=3000000 ) { +if (baud >=183 && baud <=3000000 ) { div_value =ftdi_232am_baud_to_divisor (baud); }else {dev_dbg (dev,"%s - Baud rate too high!\n" , __func__);
Due to the above change, this debug message is now incorrect.
Quoted 10 lines
drivers/usb/serial/ftdi_sio.c +1
-1@@ -1343,7 +1343,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,case FT2232C :/* FT2232C chip */ case FT232RL :/* FT232RL chip */ case FTX :/* FT-X series */ -if (baud <=3000000 ) { +if (baud >=183 && baud <=3000000 ) {u16 product_id =le16_to_cpu ( port->serial->dev->descriptor.idProduct); if (((product_id == FTDI_NDI_HUC_PID) ||
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1373
Quoted 8 lines
drivers/usb/serial/ftdi_sio.c +1
-1@@ -1367,7 +1367,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,case FT232H :/* FT232H chip */ if ((baud <=12000000 ) && (baud >=1200 )) { div_value =ftdi_2232h_baud_to_divisor (baud); - }else if (baud <1200 ) { + }else if (baud >=183 && baud <1200 ) { div_value =ftdi_232bm_baud_to_divisor (baud); }else {dev_dbg (dev,"%s - Baud rate too high!\n" , __func__);
As above
Re: [PATCH v2 2/7] USB: serial: ftdi_sio: Add missing baud rate validation
Quoted 14 lines · On Tue, 12 Jul 2022 18:27:28 +0100 Rob Pearce <[email protected]> wrote:
Quoted 13 lines · Marek Behún
From: Pali Rohár <[email protected]>
Add lower checks for requested baud rate for FT8U232AM, FT232BM, FT2232C, FT232RL, FTX, FT2232H, FT4232H and FT232H. For all of these the minimum baud rate they can generate is 183 Baud.
Signed-off-by Pali Rohár <[email protected]>Tested-by Marek Behún <[email protected]>Signed-off-by Marek Behún <[email protected]>--- drivers/usb/serial/ftdi_sio.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Quoted 14 lines
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1336
Quoted 13 lines
drivers/usb/serial/ftdi_sio.c +1
-1@@ -1330,7 +1330,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, }break ;case FT8U232AM :/* 8U232AM chip */ -if (baud <=3000000 ) { +if (baud >=183 && baud <=3000000 ) { div_value =ftdi_232am_baud_to_divisor (baud); }else {dev_dbg (dev,"%s - Baud rate too high!\n" , __func__);Due to the above change, this debug message is now incorrect.
Thx for noticing that. I will update. Also I will probably do some more refactors, I am not yet sure.
Marek
Quoted 21 lines
Quoted 10 lines
drivers/usb/serial/ftdi_sio.c +1
-1@@ -1343,7 +1343,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,case FT2232C :/* FT2232C chip */ case FT232RL :/* FT232RL chip */ case FTX :/* FT-X series */ -if (baud <=3000000 ) { +if (baud >=183 && baud <=3000000 ) {u16 product_id =le16_to_cpu ( port->serial->dev->descriptor.idProduct); if (((product_id == FTDI_NDI_HUC_PID) ||drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1373
Quoted 8 lines
drivers/usb/serial/ftdi_sio.c +1
-1@@ -1367,7 +1367,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,case FT232H :/* FT232H chip */ if ((baud <=12000000 ) && (baud >=1200 )) { div_value =ftdi_2232h_baud_to_divisor (baud); - }else if (baud <1200 ) { + }else if (baud >=183 && baud <1200 ) { div_value =ftdi_232bm_baud_to_divisor (baud); }else {dev_dbg (dev,"%s - Baud rate too high!\n" , __func__);As above
PATCH v2 3/7 USB: serial: ftdi_sio: Extract SIO divisor code to function
In reply to Marek BehúnFrom: Pali Rohár <[email protected]>
In preparation for following changes, extract divisor code for SIO chip into new function ftdi_sio_baud_to_divisor(), as is done for other chips.
No functional change.
--- drivers/usb/serial/ftdi_sio.c | 36 ++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-)
drivers/usb/serial/ftdi_sio.c +21 -15
@@ -1150,6 +1150,23 @@ static struct usb_serial_driver * const serial_drivers[] = {* *************************************************************************** */ +static int ftdi_sio_baud_to_divisor (int baud) +{ +switch (baud) { +case 300 :return ftdi_sio_b300; +case 600 :return ftdi_sio_b600; +case 1200 :return ftdi_sio_b1200; +case 2400 :return ftdi_sio_b2400; +case 4800 :return ftdi_sio_b4800; +case 9600 :return ftdi_sio_b9600; +case 19200 :return ftdi_sio_b19200; +case 38400 :return ftdi_sio_b38400; +case 57600 :return ftdi_sio_b57600; +case 115200 :return ftdi_sio_b115200; +default :return -1 ; + } +} +static unsigned short int ftdi_232am_baud_base_to_divisor (int baud,int base) {unsigned short int divisor;
@@ -1286,7 +1303,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, {struct ftdi_private *priv =usb_get_serial_port_data (port);struct device *dev = &port->dev; -u32 div_value =0 ; +int div_value =0 ;int div_okay =1 ;int baud;
@@ -1309,23 +1326,12 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, baud =9600 ;switch (priv->chip_type) {case SIO :/* SIO chip */ -switch (baud) { -case 300 : div_value = ftdi_sio_b300;break ; -case 600 : div_value = ftdi_sio_b600;break ; -case 1200 : div_value = ftdi_sio_b1200;break ; -case 2400 : div_value = ftdi_sio_b2400;break ; -case 4800 : div_value = ftdi_sio_b4800;break ; -case 9600 : div_value = ftdi_sio_b9600;break ; -case 19200 : div_value = ftdi_sio_b19200;break ; -case 38400 : div_value = ftdi_sio_b38400;break ; -case 57600 : div_value = ftdi_sio_b57600;break ; -case 115200 : div_value = ftdi_sio_b115200;break ; - }/* baud */ -if (div_value ==0 ) { + div_value =ftdi_sio_baud_to_divisor (baud); +if (div_value ==-1 ) {dev_dbg (dev,"%s - Baudrate (%d) requested is not supported\n" , __func__, baud); - div_value = ftdi_sio_b9600; baud =9600 ; + div_value =ftdi_sio_baud_to_divisor (baud); div_okay =0 ; }break ;
Signature · 1 line
2.35.1
Johan Hovold
maintainerRe: [PATCH v2 3/7] USB: serial: ftdi_sio: Extract SIO divisor code to function
Quoted 27 lines · Marek Behún
From: Pali Rohár <[email protected]>
In preparation for following changes, extract divisor code for SIO chip into new function
ftdi_sio_baud_to_divisor(), as is done for other chips.No functional change.
Signed-off-by Pali Rohár <[email protected]>Tested-by Marek Behún <[email protected]>Signed-off-by Marek Behún <[email protected]>--- drivers/usb/serial/ftdi_sio.c | 36 ++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-)drivers/usb/serial/ftdi_sio.c +4
-0@@ -1150,6 +1150,23 @@ static struct usb_serial_driver * const serial_drivers[] = {* *************************************************************************** */ +static int ftdi_sio_baud_to_divisor (int baud) +{ +switch (baud) { +case 300 :return ftdi_sio_b300;
drivers/usb/serial/ftdi_sio.c static struct usb_serial_driver * const serial_drivers[] = { · line 1166
Quoted 10 lines
drivers/usb/serial/ftdi_sio.c +10
-0+case 600 :return ftdi_sio_b600; +case 1200 :return ftdi_sio_b1200; +case 2400 :return ftdi_sio_b2400; +case 4800 :return ftdi_sio_b4800; +case 9600 :return ftdi_sio_b9600; +case 19200 :return ftdi_sio_b19200; +case 38400 :return ftdi_sio_b38400; +case 57600 :return ftdi_sio_b57600; +case 115200 :return ftdi_sio_b115200; +default :return -1 ;
Please add a line break after the colons.
Adding another enum for the invalid value might be preferable.
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1306
Quoted 12 lines
drivers/usb/serial/ftdi_sio.c +4
-1+ } +} +static unsigned short int ftdi_232am_baud_base_to_divisor (int baud,int base) {unsigned short int divisor;@@ -1286,7 +1303,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, {struct ftdi_private *priv =usb_get_serial_port_data (port);struct device *dev = &port->dev; -u32 div_value =0 ; +int div_value =0 ;
And don't change the type here.
Quoted 20 lines
drivers/usb/serial/ftdi_sio.c +0
-13int div_okay =1 ;int baud;@@ -1309,23 +1326,12 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, baud =9600 ;switch (priv->chip_type) {case SIO :/* SIO chip */ -switch (baud) { -case 300 : div_value = ftdi_sio_b300;break ; -case 600 : div_value = ftdi_sio_b600;break ; -case 1200 : div_value = ftdi_sio_b1200;break ; -case 2400 : div_value = ftdi_sio_b2400;break ; -case 4800 : div_value = ftdi_sio_b4800;break ; -case 9600 : div_value = ftdi_sio_b9600;break ; -case 19200 : div_value = ftdi_sio_b19200;break ; -case 38400 : div_value = ftdi_sio_b38400;break ; -case 57600 : div_value = ftdi_sio_b57600;break ; -case 115200 : div_value = ftdi_sio_b115200;break ; - }/* baud */ -if (div_value ==0 ) {
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1337
Quoted 10 lines
drivers/usb/serial/ftdi_sio.c +3
-1+ div_value =ftdi_sio_baud_to_divisor (baud); +if (div_value ==-1 ) {dev_dbg (dev,"%s - Baudrate (%d) requested is not supported\n" , __func__, baud); - div_value = ftdi_sio_b9600; baud =9600 ; + div_value =ftdi_sio_baud_to_divisor (baud); div_okay =0 ; }break ;
Johan
Re: [PATCH v2 3/7] USB: serial: ftdi_sio: Extract SIO divisor code to function
Quoted 41 lines · On Sunday 24 July 2022 14:06:38 Johan Hovold wrote:
Quoted 27 lines · Marek Behún
From: Pali Rohár <[email protected]>
In preparation for following changes, extract divisor code for SIO chip into new function
ftdi_sio_baud_to_divisor(), as is done for other chips.No functional change.
Signed-off-by Pali Rohár <[email protected]>Tested-by Marek Behún <[email protected]>Signed-off-by Marek Behún <[email protected]>--- drivers/usb/serial/ftdi_sio.c | 36 ++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-)drivers/usb/serial/ftdi_sio.c +4
-0@@ -1150,6 +1150,23 @@ static struct usb_serial_driver * const serial_drivers[] = {* *************************************************************************** */ +static int ftdi_sio_baud_to_divisor (int baud) +{ +switch (baud) { +case 300 :return ftdi_sio_b300;drivers/usb/serial/ftdi_sio.c static struct usb_serial_driver * const serial_drivers[] = { · line 1166
Quoted 10 lines
drivers/usb/serial/ftdi_sio.c +10
-0+case 600 :return ftdi_sio_b600; +case 1200 :return ftdi_sio_b1200; +case 2400 :return ftdi_sio_b2400; +case 4800 :return ftdi_sio_b4800; +case 9600 :return ftdi_sio_b9600; +case 19200 :return ftdi_sio_b19200; +case 38400 :return ftdi_sio_b38400; +case 57600 :return ftdi_sio_b57600; +case 115200 :return ftdi_sio_b115200; +default :return -1 ;Please add a line break after the colons.
Quoted 1 line
Adding another enum for the invalid value might be preferable.
Enum describe HW values, so it cannot be added to hw list.
Quoted 6 lines
Quoted 6 lines
drivers/usb/serial/ftdi_sio.c +3
-0+ } +} +static unsigned short int ftdi_232am_baud_base_to_divisor (int baud,int base) {unsigned short int divisor;
Quoted 8 lines
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1306
Quoted 6 lines
drivers/usb/serial/ftdi_sio.c +1
-1@@ -1286,7 +1303,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, {struct ftdi_private *priv =usb_get_serial_port_data (port);struct device *dev = &port->dev; -u32 div_value =0 ; +int div_value =0 ;And don't change the type here.
This type change was explicitly asked during v1 review. v1 had u32.
Quoted 32 lines
Quoted 20 lines
drivers/usb/serial/ftdi_sio.c +0
-13int div_okay =1 ;int baud;@@ -1309,23 +1326,12 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, baud =9600 ;switch (priv->chip_type) {case SIO :/* SIO chip */ -switch (baud) { -case 300 : div_value = ftdi_sio_b300;break ; -case 600 : div_value = ftdi_sio_b600;break ; -case 1200 : div_value = ftdi_sio_b1200;break ; -case 2400 : div_value = ftdi_sio_b2400;break ; -case 4800 : div_value = ftdi_sio_b4800;break ; -case 9600 : div_value = ftdi_sio_b9600;break ; -case 19200 : div_value = ftdi_sio_b19200;break ; -case 38400 : div_value = ftdi_sio_b38400;break ; -case 57600 : div_value = ftdi_sio_b57600;break ; -case 115200 : div_value = ftdi_sio_b115200;break ; - }/* baud */ -if (div_value ==0 ) {drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1337
Quoted 10 lines
drivers/usb/serial/ftdi_sio.c +3
-1+ div_value =ftdi_sio_baud_to_divisor (baud); +if (div_value ==-1 ) {dev_dbg (dev,"%s - Baudrate (%d) requested is not supported\n" , __func__, baud); - div_value = ftdi_sio_b9600; baud =9600 ; + div_value =ftdi_sio_baud_to_divisor (baud); div_okay =0 ; }break ;Johan
Johan Hovold
maintainerRe: [PATCH v2 3/7] USB: serial: ftdi_sio: Extract SIO divisor code to function
Quoted 42 lines · Pali Rohár
Quoted 41 lines · On Sunday 24 July 2022 14:06:38 Johan Hovold wrote:
Quoted 27 lines · Marek Behún
From: Pali Rohár <[email protected]>
In preparation for following changes, extract divisor code for SIO chip into new function
ftdi_sio_baud_to_divisor(), as is done for other chips.No functional change.
Signed-off-by Pali Rohár <[email protected]>Tested-by Marek Behún <[email protected]>Signed-off-by Marek Behún <[email protected]>--- drivers/usb/serial/ftdi_sio.c | 36 ++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-)drivers/usb/serial/ftdi_sio.c +4
-0@@ -1150,6 +1150,23 @@ static struct usb_serial_driver * const serial_drivers[] = {* *************************************************************************** */ +static int ftdi_sio_baud_to_divisor (int baud) +{ +switch (baud) { +case 300 :return ftdi_sio_b300;drivers/usb/serial/ftdi_sio.c static struct usb_serial_driver * const serial_drivers[] = { · line 1166
Quoted 10 lines
drivers/usb/serial/ftdi_sio.c +10
-0+case 600 :return ftdi_sio_b600; +case 1200 :return ftdi_sio_b1200; +case 2400 :return ftdi_sio_b2400; +case 4800 :return ftdi_sio_b4800; +case 9600 :return ftdi_sio_b9600; +case 19200 :return ftdi_sio_b19200; +case 38400 :return ftdi_sio_b38400; +case 57600 :return ftdi_sio_b57600; +case 115200 :return ftdi_sio_b115200; +default :return -1 ;Please add a line break after the colons.
Quoted 3 lines
Quoted 1 line
Adding another enum for the invalid value might be preferable.
Enum describe HW values, so it cannot be added to hw list.
Of course you can, but perhaps you'll figure out some other way to do this.
Quoted 15 lines
Quoted 6 lines
Quoted 6 lines
drivers/usb/serial/ftdi_sio.c +3
-0+ } +} +static unsigned short int ftdi_232am_baud_base_to_divisor (int baud,int base) {unsigned short int divisor;Quoted 8 lines
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1306
Quoted 6 lines
drivers/usb/serial/ftdi_sio.c +1
-1@@ -1286,7 +1303,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, {struct ftdi_private *priv =usb_get_serial_port_data (port);struct device *dev = &port->dev; -u32 div_value =0 ; +int div_value =0 ;And don't change the type here.
Quoted 1 line
This type change was explicitly asked during v1 review. v1 had u32.
Ok, but don't. This variable is used in other parts of this function.
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
drivers/usb/serial/ftdi_sio.c +0
-5int div_okay =1 ;int baud;@@ -1309,23 +1326,12 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, baud =9600 ;switch (priv->chip_type) {case SIO :/* SIO chip */ -switch (baud) { -case 300 : div_value = ftdi_sio_b300;break ; -case 600 : div_value = ftdi_sio_b600;break ; -case 1200 : div_value = ftdi_sio_b1200;break ; -case 2400 : div_value = ftdi_sio_b2400;break ;
Quoted 8 lines
Quoted 8 lines
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1324
Quoted 8 lines
drivers/usb/serial/ftdi_sio.c +0
-8-case 4800 : div_value = ftdi_sio_b4800;break ; -case 9600 : div_value = ftdi_sio_b9600;break ; -case 19200 : div_value = ftdi_sio_b19200;break ; -case 38400 : div_value = ftdi_sio_b38400;break ; -case 57600 : div_value = ftdi_sio_b57600;break ; -case 115200 : div_value = ftdi_sio_b115200;break ; - }/* baud */ -if (div_value ==0 ) {
Looks like there's another bug here in the current driver as b300 is defined as 0.
I'll fix that up separately first.
Quoted 10 lines
Quoted 10 lines
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1337
Quoted 10 lines
drivers/usb/serial/ftdi_sio.c +3
-1+ div_value =ftdi_sio_baud_to_divisor (baud); +if (div_value ==-1 ) {dev_dbg (dev,"%s - Baudrate (%d) requested is not supported\n" , __func__, baud); - div_value = ftdi_sio_b9600; baud =9600 ; + div_value =ftdi_sio_baud_to_divisor (baud); div_okay =0 ; }break ;
Johan
Re: [PATCH v2 3/7] USB: serial: ftdi_sio: Extract SIO divisor code to function
Quoted 9 lines · On Tuesday 13 September 2022 16:49:01 Johan Hovold wrote:
Quoted 9 lines
Quoted 8 lines
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1306
Quoted 6 lines
drivers/usb/serial/ftdi_sio.c +1
-1@@ -1286,7 +1303,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, {struct ftdi_private *priv =usb_get_serial_port_data (port);struct device *dev = &port->dev; -u32 div_value =0 ; +int div_value =0 ;And don't change the type here.
Quoted 3 lines
Quoted 1 line
This type change was explicitly asked during v1 review. v1 had u32.
Ok, but don't. This variable is used in other parts of this function.
As I said, I changed it just because I was explicitly asked for it during v1 review. I'm just doing what I was asked.
Maybe... I can change it into union?
union {
s32 signed;
u32 unsigned;
} div_value;I really do not see any other option how to achieve requirement that it should be type of "int" with your another requirement that "do not do it".
Johan Hovold
maintainerRe: [PATCH v2 3/7] USB: serial: ftdi_sio: Extract SIO divisor code to function
Quoted 19 lines · Pali Rohár
Quoted 9 lines · On Tuesday 13 September 2022 16:49:01 Johan Hovold wrote:
Quoted 9 lines
Quoted 8 lines
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1306
Quoted 6 lines
drivers/usb/serial/ftdi_sio.c +1
-1@@ -1286,7 +1303,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, {struct ftdi_private *priv =usb_get_serial_port_data (port);struct device *dev = &port->dev; -u32 div_value =0 ; +int div_value =0 ;And don't change the type here.
Quoted 3 lines
Quoted 1 line
This type change was explicitly asked during v1 review. v1 had u32.
Ok, but don't. This variable is used in other parts of this function.
As I said, I changed it just because I was explicitly asked for it during v1 review. I'm just doing what I was asked.
Maybe... I can change it into union?
Quoted 7 lines
union { s32 signed; u32 unsigned; } div_value;I really do not see any other option how to achieve requirement that it should be type of "int" with your another requirement that "do not do it".
I'm pretty sure I didn't review v1.
Perhaps you're referring to this:
https://lore.kernel.org/all/[email protected]/
But that doesn't mean you should change the type of div_value.
Johan
PATCH v2 4/7 USB: serial: ftdi_sio: Do not reset baud rate to 9600 Baud on error
In reply to Marek BehúnFrom: Pali Rohár <[email protected]>
If setting new baud rate fails, all other drivers leave the device at previous baud rate, only ftdi_sio resets to 9600 Baud.
Change this behavior to that of other drivers so that /dev/ttyUSB* devices behave in the same way.
---
Greg pointed out that this make break some people's workflow, that they may depend on this behavior.
This is of course possible, although IMO improbable: it would be weird to depend on the fall back to 9600 Baud on error, instead of expecting that the baud rate didn't change at all (like in other /dev/ttyUSB* drivers).
Nonetheless if someone complains that they workflow got broken, we will need to revert this.
--- drivers/usb/serial/ftdi_sio.c | 38 ++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 12 deletions(-)
drivers/usb/serial/ftdi_sio.c +26 -12
@@ -1299,7 +1299,7 @@ static int update_mctrl(struct usb_serial_port *port, unsigned int set,static u32 get_ftdi_divisor (struct tty_struct *tty, -struct usb_serial_port *port) +struct usb_serial_port *port,speed_t old_baud) {struct ftdi_private *priv =usb_get_serial_port_data (port);struct device *dev = &port->dev;
@@ -1322,6 +1322,8 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, __func__, priv->custom_divisor, baud); } +if (!baud) + baud = old_baud;if (!baud) baud =9600 ;switch (priv->chip_type) {
@@ -1330,8 +1332,12 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,if (div_value ==-1 ) {dev_dbg (dev,"%s - Baudrate (%d) requested is not supported\n" , __func__, baud); - baud =9600 ; + baud = old_baud ? old_baud :9600 ; div_value =ftdi_sio_baud_to_divisor (baud); +if (div_value ==-1 ) { + baud =9600 ; + div_value =ftdi_sio_baud_to_divisor (baud); + } div_okay =0 ; }break ;
@@ -1340,8 +1346,8 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, div_value =ftdi_232am_baud_to_divisor (baud); }else {dev_dbg (dev,"%s - Baud rate too high!\n" , __func__); - baud =9600 ; - div_value =ftdi_232am_baud_to_divisor (9600 ); + baud = (old_baud >=183 && old_baud <=3000000 ) ? old_baud :9600 ; + div_value =ftdi_232am_baud_to_divisor (baud); div_okay =0 ; }break ;
@@ -1363,9 +1369,9 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, div_value =ftdi_232bm_baud_to_divisor (baud); }else {dev_dbg (dev,"%s - Baud rate too high!\n" , __func__); - div_value =ftdi_232bm_baud_to_divisor (9600 ); + baud = (old_baud >=183 && old_baud <=3000000 ) ? old_baud :9600 ; + div_value =ftdi_232bm_baud_to_divisor (baud); div_okay =0 ; - baud =9600 ; }break ;case FT2232H :/* FT2232H chip */
@@ -1377,9 +1383,14 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, div_value =ftdi_232bm_baud_to_divisor (baud); }else {dev_dbg (dev,"%s - Baud rate too high!\n" , __func__); - div_value =ftdi_232bm_baud_to_divisor (9600 ); +if (old_baud >=183 && old_baud <1200 ) { + baud = old_baud; + div_value =ftdi_232bm_baud_to_divisor (baud); + }else { + baud = (old_baud >=1200 && old_baud <=12000000 ) ? old_baud :9600 ; + div_value =ftdi_2232h_baud_to_divisor (baud); + } div_okay =0 ; - baud =9600 ; }break ; }/* priv->chip_type */
@@ -1394,7 +1405,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,return div_value; } -static int change_speed (struct tty_struct *tty,struct usb_serial_port *port) +static int change_speed (struct tty_struct *tty,struct usb_serial_port *port,speed_t old_baud) {struct ftdi_private *priv =usb_get_serial_port_data (port);u16 value;
@@ -1402,7 +1413,7 @@ static int change_speed(struct tty_struct *tty, struct usb_serial_port *port)u32 index_value;int rv; - index_value =get_ftdi_divisor (tty, port); + index_value =get_ftdi_divisor (tty, port, old_baud); value = (u16 )index_value; index = (u16)(index_value >>16 ); if (priv->chip_type == FT2232C || priv->chip_type == FT2232H ||
@@ -1525,7 +1536,7 @@ static int set_serial_info(struct tty_struct *tty, struct serial_struct *ss)if (priv->flags &ASYNC_SPD_MASK )dev_warn_ratelimited (&port->dev,"use of SPD flags is deprecated\n" ); -change_speed (tty, port); +change_speed (tty, port,0 ); }mutex_unlock (&priv->cfg_lock);return 0 ;
@@ -2774,9 +2785,12 @@ static void ftdi_set_termios(struct tty_struct *tty,/* Drop RTS and DTR */ clear_mctrl (port,TIOCM_DTR |TIOCM_RTS ); }else { +speed_t old_baud = + old_termios ?tty_termios_baud_rate (old_termios) :0 ; +/* set the baudrate determined before */ mutex_lock (&priv->cfg_lock); -if (change_speed (tty, port)) +if (change_speed (tty, port, old_baud))dev_err (ddev,"%s urb failed to set baudrate\n" , __func__);mutex_unlock (&priv->cfg_lock);/* Ensure RTS and DTR are raised when baudrate changed from 0 */
Signature · 1 line
2.35.1
Johan Hovold
maintainerRe: [PATCH v2 4/7] USB: serial: ftdi_sio: Do not reset baud rate to 9600 Baud on error
Quoted 7 lines · Marek Behún
From: Pali Rohár <[email protected]>
If setting new baud rate fails, all other drivers leave the device at previous baud rate, only
ftdi_sioresets to 9600 Baud.Change this behavior to that of other drivers so that
/dev/ttyUSB*devices behave in the same way.
These statements are not true. Several USB serial driver set 9600 baud on errors for historical reasons. Yet others clamp. It's inconsistent at best.
Quoted 22 lines
Signed-off-by Pali Rohár <[email protected]>Tested-by Marek Behún <[email protected]>Signed-off-by Marek Behún <[email protected]>---
Greg pointed out that this make break some people's workflow, that they may depend on this behavior.This is of course possible, although IMO improbable: it would be weird to depend on the fall back to 9600 Baud on error, instead of expecting that the baud rate didn't change at all (like in other
/dev/ttyUSB*drivers).Nonetheless if someone complains that they workflow got broken, we will need to revert this.
static u32 get_ftdi_divisor(struct
tty_struct*tty,drivers/usb/serial/ftdi_sio.c +1
-1- struct usb_serial_port *port) + struct usb_serial_port *port, speed_t old_baud) {struct ftdi_private *priv =usb_get_serial_port_data (port);struct device *dev = &port->dev;
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1328
Quoted 7 lines
drivers/usb/serial/ftdi_sio.c +2
-0@@ -1322,6 +1322,8 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, __func__, priv->custom_divisor, baud); } +if (!baud) + baud = old_baud;if (!baud) baud =9600 ;
This looks like it breaks B0 handling. Either way, an unrelated change.
Quoted 16 lines
drivers/usb/serial/ftdi_sio.c +5
-1switch (priv->chip_type) {@@ -1330,8 +1332,12 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,if (div_value ==-1 ) {dev_dbg (dev,"%s - Baudrate (%d) requested is not supported\n" , __func__, baud); - baud =9600 ; + baud = old_baud ? old_baud :9600 ; div_value =ftdi_sio_baud_to_divisor (baud); +if (div_value ==-1 ) { + baud =9600 ; + div_value =ftdi_sio_baud_to_divisor (baud); + } div_okay =0 ; }break ;
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1349
Quoted 6 lines
drivers/usb/serial/ftdi_sio.c +1
-2@@ -1340,8 +1346,8 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, div_value =ftdi_232am_baud_to_divisor (baud); }else {dev_dbg (dev,"%s - Baud rate too high!\n" , __func__); - baud =9600 ; - div_value =ftdi_232am_baud_to_divisor (9600 ); + baud = (old_baud >=183 && old_baud <=3000000 ) ? old_baud :9600 ;
And please avoid using the ternary operator which tend to just hurt readability.
Looks like this needs to be refactored in some way.
drivers/usb/serial/ftdi_sio.c static int set_serial_info(struct tty_struct *tty, struct serial_struct *ss) · line 1539
Quoted 6 lines
drivers/usb/serial/ftdi_sio.c +1
-1@@ -1525,7 +1536,7 @@ static int set_serial_info(struct tty_struct *tty, struct serial_struct *ss)if (priv->flags &ASYNC_SPD_MASK )dev_warn_ratelimited (&port->dev,"use of SPD flags is deprecated\n" ); -change_speed (tty, port); +change_speed (tty, port,0 );
Zero has a special meaning (B0).
drivers/usb/serial/ftdi_sio.c static void ftdi_set_termios(struct tty_struct *tty, · line 2789
Quoted 9 lines
drivers/usb/serial/ftdi_sio.c +2
-0}mutex_unlock (&priv->cfg_lock);return 0 ;@@ -2774,9 +2785,12 @@ static void ftdi_set_termios(struct tty_struct *tty,/* Drop RTS and DTR */ clear_mctrl (port,TIOCM_DTR |TIOCM_RTS ); }else { +speed_t old_baud = + old_termios ?tty_termios_baud_rate (old_termios) :0 ;
Just use 9600 if you don't have an old termios. The termios rate should be valid in that case.
Johan
Re: [PATCH v2 4/7] USB: serial: ftdi_sio: Do not reset baud rate to 9600 Baud on error
Quoted 38 lines · On Sunday 24 July 2022 14:20:58 Johan Hovold wrote:
Quoted 7 lines · Marek Behún
From: Pali Rohár <[email protected]>
If setting new baud rate fails, all other drivers leave the device at previous baud rate, only
ftdi_sioresets to 9600 Baud.Change this behavior to that of other drivers so that
/dev/ttyUSB*devices behave in the same way.These statements are not true. Several USB serial driver set 9600 baud on errors for historical reasons. Yet others clamp. It's inconsistent at best.
Quoted 25 lines
Signed-off-by Pali Rohár <[email protected]>Tested-by Marek Behún <[email protected]>Signed-off-by Marek Behún <[email protected]>---
Greg pointed out that this make break some people's workflow, that they may depend on this behavior.This is of course possible, although IMO improbable: it would be weird to depend on the fall back to 9600 Baud on error, instead of expecting that the baud rate didn't change at all (like in other
/dev/ttyUSB*drivers).Nonetheless if someone complains that they workflow got broken, we will need to revert this.
static u32 get_ftdi_divisor(struct
tty_struct*tty,drivers/usb/serial/ftdi_sio.c +1
-1- struct usb_serial_port *port) + struct usb_serial_port *port, speed_t old_baud) {struct ftdi_private *priv =usb_get_serial_port_data (port);struct device *dev = &port->dev;@@ -1322,6 +1322,8 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, __func__, priv->custom_divisor, baud); }
Quoted 6 lines
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1328
Quoted 4 lines
drivers/usb/serial/ftdi_sio.c +2
-0+if (!baud) + baud = old_baud;if (!baud) baud =9600 ;This looks like it breaks B0 handling. Either way, an unrelated change.
Unreleased to this change.
Quoted 37 lines
Quoted 16 lines
drivers/usb/serial/ftdi_sio.c +5
-1switch (priv->chip_type) {@@ -1330,8 +1332,12 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,if (div_value ==-1 ) {dev_dbg (dev,"%s - Baudrate (%d) requested is not supported\n" , __func__, baud); - baud =9600 ; + baud = old_baud ? old_baud :9600 ; div_value =ftdi_sio_baud_to_divisor (baud); +if (div_value ==-1 ) { + baud =9600 ; + div_value =ftdi_sio_baud_to_divisor (baud); + } div_okay =0 ; }break ;drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1349
Quoted 6 lines
drivers/usb/serial/ftdi_sio.c +1
-2@@ -1340,8 +1346,8 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, div_value =ftdi_232am_baud_to_divisor (baud); }else {dev_dbg (dev,"%s - Baud rate too high!\n" , __func__); - baud =9600 ; - div_value =ftdi_232am_baud_to_divisor (9600 ); + baud = (old_baud >=183 && old_baud <=3000000 ) ? old_baud :9600 ;And please avoid using the ternary operator which tend to just hurt readability.
Looks like this needs to be refactored in some way.
drivers/usb/serial/ftdi_sio.c static int set_serial_info(struct tty_struct *tty, struct serial_struct *ss) · line 1539
Quoted 6 lines
drivers/usb/serial/ftdi_sio.c +1
-1@@ -1525,7 +1536,7 @@ static int set_serial_info(struct tty_struct *tty, struct serial_struct *ss)if (priv->flags &ASYNC_SPD_MASK )dev_warn_ratelimited (&port->dev,"use of SPD flags is deprecated\n" ); -change_speed (tty, port); +change_speed (tty, port,0 );Zero has a special meaning (B0).
Quoted 12 lines
drivers/usb/serial/ftdi_sio.c static void ftdi_set_termios(struct tty_struct *tty, · line 2789
Quoted 9 lines
drivers/usb/serial/ftdi_sio.c +2
-0}mutex_unlock (&priv->cfg_lock);return 0 ;@@ -2774,9 +2785,12 @@ static void ftdi_set_termios(struct tty_struct *tty,/* Drop RTS and DTR */ clear_mctrl (port,TIOCM_DTR |TIOCM_RTS ); }else { +speed_t old_baud = + old_termios ?tty_termios_baud_rate (old_termios) :0 ;Just use 9600 if you don't have an old termios. The termios rate should be valid in that case.
Ok
Johan Hovold
maintainerRe: [PATCH v2 4/7] USB: serial: ftdi_sio: Do not reset baud rate to 9600 Baud on error
Quoted 12 lines · Pali Rohár
Quoted 9 lines · On Sunday 24 July 2022 14:20:58 Johan Hovold wrote:
drivers/usb/serial/ftdi_sio.c static void ftdi_set_termios(struct tty_struct *tty, · line 2789
Quoted 6 lines
drivers/usb/serial/ftdi_sio.c +2
-0@@ -2774,9 +2785,12 @@ static void ftdi_set_termios(struct tty_struct *tty,/* Drop RTS and DTR */ clear_mctrl (port,TIOCM_DTR |TIOCM_RTS ); }else { +speed_t old_baud = + old_termios ?tty_termios_baud_rate (old_termios) :0 ;Just use 9600 if you don't have an old termios. The termios rate should be valid in that case.
Quoted 1 line
Ok
Please trim your replies.
Johan
PATCH v2 5/7 USB: serial: ftdi_sio: Fix baud rate rounding for ASYNC_SPD_CUST
In reply to Marek BehúnFrom: Pali Rohár <[email protected]>
To compute more accurate baud rate when user uses ASYNC_SPD_CUST API, use DIV_ROUND_CLOSEST() instead of just rounding down.
Rationale:
Application uses old API, so it computes divisor D for baud rate B.
The driver then tries to compute back the requested baud rate B, but rounds down in the division.
Using rounding to closest value instead should increate accuracy here.
--- drivers/usb/serial/ftdi_sio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/serial/ftdi_sio.c +1 -1
@@ -1317,7 +1317,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,if (baud ==38400 && ((priv->flags &ASYNC_SPD_MASK ) ==ASYNC_SPD_CUST ) && (priv->custom_divisor)) { - baud = priv->baud_base / priv->custom_divisor; + baud =DIV_ROUND_CLOSEST (priv->baud_base, priv->custom_divisor);dev_dbg (dev,"%s - custom divisor %d sets baud rate to %d\n" , __func__, priv->custom_divisor, baud); }
Signature · 1 line
2.35.1
Johan Hovold
maintainerRe: [PATCH v2 5/7] USB: serial: ftdi_sio: Fix baud rate rounding for ASYNC_SPD_CUST
Quoted 25 lines · Marek Behún
From: Pali Rohár <[email protected]>
To compute more accurate baud rate when user uses
ASYNC_SPD_CUSTAPI, useDIV_ROUND_CLOSEST()instead of just rounding down.Rationale:
Application uses old API, so it computes divisor D for baud rate B.The driver then tries to compute back the requested baud rate B, but rounds down in the division.
Using rounding to closest value instead should increate accuracy here.
Signed-off-by Pali Rohár <[email protected]>Tested-by Marek Behún <[email protected]>Signed-off-by Marek Behún <[email protected]>--- drivers/usb/serial/ftdi_sio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/usb/serial/ftdi_sio.c +0
-0
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1320
Quoted 5 lines
drivers/usb/serial/ftdi_sio.c +1
-1@@ -1317,7 +1317,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,if (baud ==38400 && ((priv->flags &ASYNC_SPD_MASK ) ==ASYNC_SPD_CUST ) && (priv->custom_divisor)) { - baud = priv->baud_base / priv->custom_divisor; + baud =DIV_ROUND_CLOSEST (priv->baud_base, priv->custom_divisor);
Sure, why not, but this should only be used for the following debug statement. (See next patch).
drivers/usb/serial/ftdi_sio.c static u32 get_ftdi_divisor(struct tty_struct *tty, · line 1323
Quoted 3 lines
drivers/usb/serial/ftdi_sio.c +0
-0dev_dbg (dev,"%s - custom divisor %d sets baud rate to %d\n" , __func__, priv->custom_divisor, baud); }
Johan
PATCH v2 6/7 USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for ASYNC_SPD_CUST
In reply to Marek BehúnFrom: Pali Rohár <[email protected]>
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.
--- drivers/usb/serial/ftdi_sio.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
drivers/usb/serial/ftdi_sio.c +16 -0
@@ -1303,6 +1303,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, {struct ftdi_private *priv =usb_get_serial_port_data (port);struct device *dev = &port->dev; +int fix_custom_divisor =0 ;int div_value =0 ;int div_okay =1 ;int baud;
@@ -1317,6 +1318,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,if (baud ==38400 && ((priv->flags &ASYNC_SPD_MASK ) ==ASYNC_SPD_CUST ) && (priv->custom_divisor)) { + fix_custom_divisor =1 ; baud =DIV_ROUND_CLOSEST (priv->baud_base, priv->custom_divisor);dev_dbg (dev,"%s - custom divisor %d sets baud rate to %d\n" , __func__, priv->custom_divisor, baud);
@@ -1401,7 +1403,19 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, ftdi_chip_name[priv->chip_type]); } +/* 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. */
Signature · 1 line
2.35.1
Johan Hovold
maintainerRe: [PATCH v2 6/7] USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for ASYNC_SPD_CUST
Quoted 5 lines · Marek Behún
From: Pali Rohár <[email protected]>
When
ASYNC_SPD_CUSTis used, updatecustom_divisorand 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. The rate should be left unchanged at 38400 in that case.
Quoted 48 lines
Signed-off-by Pali Rohár <[email protected]>Tested-by Marek Behún <[email protected]>Signed-off-by Marek Behún <[email protected]>--- drivers/usb/serial/ftdi_sio.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)drivers/usb/serial/ftdi_sio.c +14
-0@@ -1303,6 +1303,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, {struct ftdi_private *priv =usb_get_serial_port_data (port);struct device *dev = &port->dev; +int fix_custom_divisor =0 ;int div_value =0 ;int div_okay =1 ;int baud;@@ -1317,6 +1318,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,if (baud ==38400 && ((priv->flags &ASYNC_SPD_MASK ) ==ASYNC_SPD_CUST ) && (priv->custom_divisor)) { + fix_custom_divisor =1 ; baud =DIV_ROUND_CLOSEST (priv->baud_base, priv->custom_divisor);dev_dbg (dev,"%s - custom divisor %d sets baud rate to %d\n" , __func__, priv->custom_divisor, baud);@@ -1401,7 +1403,19 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, ftdi_chip_name[priv->chip_type]); } +/* 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; }
drivers/usb/serial/ftdi_sio.c static void ftdi_set_termios(struct tty_struct *tty, · line 2695
Quoted 8 lines
drivers/usb/serial/ftdi_sio.c +2
-0@@ -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
Re: [PATCH v2 6/7] USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for ASYNC_SPD_CUST
Quoted 11 lines · On Sunday 24 July 2022 14:28:43 Johan Hovold wrote:
Quoted 5 lines · Marek Behún
From: Pali Rohár <[email protected]>
When
ASYNC_SPD_CUSTis used, updatecustom_divisorand 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.
Quoted 1 line
The rate should be left unchanged at 38400 in that case.
With this change, rate in c_cflag is unchanged and stays at B38400.
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.
Quoted 58 lines
Quoted 48 lines
Signed-off-by Pali Rohár <[email protected]>Tested-by Marek Behún <[email protected]>Signed-off-by Marek Behún <[email protected]>--- drivers/usb/serial/ftdi_sio.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)drivers/usb/serial/ftdi_sio.c +14
-0@@ -1303,6 +1303,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, {struct ftdi_private *priv =usb_get_serial_port_data (port);struct device *dev = &port->dev; +int fix_custom_divisor =0 ;int div_value =0 ;int div_okay =1 ;int baud;@@ -1317,6 +1318,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty,if (baud ==38400 && ((priv->flags &ASYNC_SPD_MASK ) ==ASYNC_SPD_CUST ) && (priv->custom_divisor)) { + fix_custom_divisor =1 ; baud =DIV_ROUND_CLOSEST (priv->baud_base, priv->custom_divisor);dev_dbg (dev,"%s - custom divisor %d sets baud rate to %d\n" , __func__, priv->custom_divisor, baud);@@ -1401,7 +1403,19 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, ftdi_chip_name[priv->chip_type]); } +/* 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; }drivers/usb/serial/ftdi_sio.c static void ftdi_set_termios(struct tty_struct *tty, · line 2695
Quoted 8 lines
drivers/usb/serial/ftdi_sio.c +2
-0@@ -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
Johan Hovold
maintainerRe: [PATCH v2 6/7] USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for ASYNC_SPD_CUST
Quoted 8 lines · Pali Rohár
Quoted 7 lines · On Sunday 24 July 2022 14:28:43 Johan Hovold wrote:
Quoted 5 lines · Marek Behún
From: Pali Rohár <[email protected]>
When
ASYNC_SPD_CUSTis used, updatecustom_divisorand 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.
Quoted 6 lines
Quoted 4 lines
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.
Quoted 3 lines
Quoted 1 line
The rate should be left unchanged at 38400 in that case.
With this change, rate in
c_cflagis unchanged and stays at B38400.
Right.
Quoted 5 lines
What is updated is the real baudrate in
c_ispeedandc_ospeedextensions.It is really wrong? I thought that
c_cflagshould stay unchanged at B38400 whenASYNC_SPD_CUSTis used.
Yeah, cflags stay unchanged, but you shouldn't touch those fields when using the deprecated ASYNC_SPD_CUST hack.
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.
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
drivers/usb/serial/ftdi_sio.c +14
-0+/* 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); }
Quoted 1 line
Quoted 1 line
drivers/usb/serial/ftdi_sio.c static void ftdi_set_termios(struct tty_struct *tty, · line 2695
Quoted 1 line
drivers/usb/serial/ftdi_sio.c +0
-0/* Force RTS-CTS if this device requires it. */
Johan
Re: [PATCH v2 6/7] USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for ASYNC_SPD_CUST
Quoted 24 lines · On Sunday 24 July 2022 14:54:58 Johan Hovold wrote:
Quoted 8 lines · Pali Rohár
Quoted 7 lines · On Sunday 24 July 2022 14:28:43 Johan Hovold wrote:
Quoted 5 lines · Marek Behún
From: Pali Rohár <[email protected]>
When
ASYNC_SPD_CUSTis used, updatecustom_divisorand 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.Quoted 6 lines
Quoted 4 lines
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.
Quoted 3 lines
Quoted 1 line
The rate should be left unchanged at 38400 in that case.
With this change, rate in
c_cflagis unchanged and stays at B38400.Right.
Quoted 8 lines
Quoted 5 lines
What is updated is the real baudrate in
c_ispeedandc_ospeedextensions.It is really wrong? I thought that
c_cflagshould stay unchanged at B38400 whenASYNC_SPD_CUSTis used.Yeah, cflags stay unchanged, but you shouldn't touch those fields when using the deprecated
ASYNC_SPD_CUSThack.
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?
Quoted 3 lines
Note that this currently only works because the ftdi driver uses
tty_get_baud_rate()instead ofc_ospeeddirectly which is the recommended (new) way.
Yes, tty_get_baud_rate() helper function is there for this purpose, right?
Quoted 27 lines
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
drivers/usb/serial/ftdi_sio.c +14
-0+/* 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); }Quoted 1 line
Quoted 1 line
drivers/usb/serial/ftdi_sio.c static void ftdi_set_termios(struct tty_struct *tty, · line 2695
Quoted 1 line
drivers/usb/serial/ftdi_sio.c +0
-0/* Force RTS-CTS if this device requires it. */ Johan
Johan Hovold
maintainerRe: [PATCH v2 6/7] USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for ASYNC_SPD_CUST
Quoted 9 lines · Pali Rohár
Quoted 8 lines · On Sunday 24 July 2022 14:54:58 Johan Hovold wrote:
Quoted 5 lines · Pali Rohár
What is updated is the real baudrate in
c_ispeedandc_ospeedextensions.It is really wrong? I thought that
c_cflagshould stay unchanged at B38400 whenASYNC_SPD_CUSTis used.
Quoted 6 lines
Quoted 2 lines
Yeah, cflags stay unchanged, but you shouldn't touch those fields when using the deprecated
ASYNC_SPD_CUSThack.Hm... Why? I thought that new extended fields (
c_ispeedandc_ospeed) should contain current real speed. What is the reason that c_*speed fields should have 38400 whenASYNC_SPD_CUSThack is set?
Because we shouldn't go adding new features built around the deprecated ASYNC_SPD_CUST hack.
User picks 38400, sets that flag and magic happens with some drivers for a while still while we look the other way.
This is not something that we should need to care about when using the new interfaces.
Quoted 6 lines
Quoted 3 lines
Note that this currently only works because the ftdi driver uses
tty_get_baud_rate()instead ofc_ospeeddirectly which is the recommended (new) way.Yes,
tty_get_baud_rate()helper function is there for this purpose, right?
No.
Johan
Re: [PATCH v2 6/7] USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for ASYNC_SPD_CUST
Quoted 13 lines · On Sunday 24 July 2022 15:08:09 Johan Hovold wrote:
Quoted 12 lines · Pali Rohár
Quoted 10 lines · On Sunday 24 July 2022 14:54:58 Johan Hovold wrote:
Quoted 5 lines · Pali Rohár
What is updated is the real baudrate in
c_ispeedandc_ospeedextensions.It is really wrong? I thought that
c_cflagshould stay unchanged at B38400 whenASYNC_SPD_CUSTis used.Yeah, cflags stay unchanged, but you shouldn't touch those fields when using the deprecated
ASYNC_SPD_CUSThack.
Quoted 6 lines
Quoted 3 lines
Hm... Why? I thought that new extended fields (
c_ispeedandc_ospeed) should contain current real speed. What is the reason that c_*speed fields should have 38400 whenASYNC_SPD_CUSThack is set?Because we shouldn't go adding new features built around the deprecated
ASYNC_SPD_CUSThack.
But this is not a new feature in the old deprecated hack. It for the new interface.
Quoted 5 lines
User picks 38400, sets that flag and magic happens with some drivers for a while still while we look the other way.
This is not something that we should need to care about when using the new interfaces.
Exactly and with this patch it work like to described. User of new interface does not have to care about old deprecated stuff and new interface would always reports correct value.
Quoted 10 lines
Quoted 6 lines
Quoted 3 lines
Note that this currently only works because the ftdi driver uses
tty_get_baud_rate()instead ofc_ospeeddirectly which is the recommended (new) way.Yes,
tty_get_baud_rate()helper function is there for this purpose, right?No.
Johan
So for what otherwise?
Johan Hovold
maintainerRe: [PATCH v2 6/7] USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for ASYNC_SPD_CUST
Quoted 18 lines · Pali Rohár
Quoted 17 lines · On Sunday 24 July 2022 15:08:09 Johan Hovold wrote:
Quoted 15 lines · Pali Rohár
Quoted 10 lines · On Sunday 24 July 2022 14:54:58 Johan Hovold wrote:
Quoted 5 lines · Pali Rohár
What is updated is the real baudrate in
c_ispeedandc_ospeedextensions.It is really wrong? I thought that
c_cflagshould stay unchanged at B38400 whenASYNC_SPD_CUSTis used.Yeah, cflags stay unchanged, but you shouldn't touch those fields when using the deprecated
ASYNC_SPD_CUSThack.Hm... Why? I thought that new extended fields (
c_ispeedandc_ospeed) should contain current real speed. What is the reason that c_*speed fields should have 38400 whenASYNC_SPD_CUSThack is set?
Quoted 5 lines
Quoted 2 lines
Because we shouldn't go adding new features built around the deprecated
ASYNC_SPD_CUSThack.But this is not a new feature in the old deprecated hack. It for the new interface.
I think I understand what you're getting at, but no. Let's not add more features built around ASYNC_SPD_CUST.
Johan
Re: [PATCH v2 6/7] USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for ASYNC_SPD_CUST
Quoted 22 lines · On Tuesday 13 September 2022 16:59:22 Johan Hovold wrote:
Quoted 21 lines · Pali Rohár
Quoted 19 lines · On Sunday 24 July 2022 15:08:09 Johan Hovold wrote:
Quoted 9 lines · Pali Rohár
Quoted 8 lines · On Sunday 24 July 2022 14:54:58 Johan Hovold wrote:
Quoted 5 lines · Pali Rohár
What is updated is the real baudrate in
c_ispeedandc_ospeedextensions.It is really wrong? I thought that
c_cflagshould stay unchanged at B38400 whenASYNC_SPD_CUSTis used.Quoted 6 lines
Quoted 2 lines
Yeah, cflags stay unchanged, but you shouldn't touch those fields when using the deprecated
ASYNC_SPD_CUSThack.Hm... Why? I thought that new extended fields (
c_ispeedandc_ospeed) should contain current real speed. What is the reason that c_*speed fields should have 38400 whenASYNC_SPD_CUSThack is set?Because we shouldn't go adding new features built around the deprecated
ASYNC_SPD_CUSThack.
Quoted 7 lines
Quoted 2 lines
But this is not a new feature in the old deprecated hack. It for the new interface.
I think I understand what you're getting at, but no. Let's not add more features built around
ASYNC_SPD_CUST.Johan
Seems that you did not understand the point. So I will try to explain it again. This is not a new feature for _old_ ASYNC_SPD_CUST. This is the fix for the _new_ TCGETS2 API, to ensure that driver will always returns corrects values in c_*speed fields. If driver is not going to fix this _new_ TCGETS2 API then there is _NO_ point to use this new API in userspace and it is better to stick with the old ASYNC_SPD_CUST. And this is the current userspace state. So based on your input, it is the time to deprecate TCGETS2?
Johan Hovold
maintainerRe: [PATCH v2 6/7] USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for ASYNC_SPD_CUST
Quoted 29 lines · Pali Rohár
Quoted 19 lines · On Tuesday 13 September 2022 16:59:22 Johan Hovold wrote:
Quoted 18 lines · Pali Rohár
Quoted 17 lines · On Sunday 24 July 2022 15:08:09 Johan Hovold wrote:
Quoted 15 lines · Pali Rohár
Quoted 10 lines · On Sunday 24 July 2022 14:54:58 Johan Hovold wrote:
Quoted 5 lines · Pali Rohár
What is updated is the real baudrate in
c_ispeedandc_ospeedextensions.It is really wrong? I thought that
c_cflagshould stay unchanged at B38400 whenASYNC_SPD_CUSTis used.Yeah, cflags stay unchanged, but you shouldn't touch those fields when using the deprecated
ASYNC_SPD_CUSThack.Hm... Why? I thought that new extended fields (
c_ispeedandc_ospeed) should contain current real speed. What is the reason that c_*speed fields should have 38400 whenASYNC_SPD_CUSThack is set?Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Because we shouldn't go adding new features built around the deprecated
ASYNC_SPD_CUSThack.But this is not a new feature in the old deprecated hack. It for the new interface.
I think I understand what you're getting at, but no. Let's not add more features built around
ASYNC_SPD_CUST.
Quoted 8 lines
Seems that you did not understand the point. So I will try to explain it again. This is not a new feature for _old_
ASYNC_SPD_CUST. This is the fix for the _new_ TCGETS2 API, to ensure that driver will always returns corrects values in c_*speed fields. If driver is not going to fix this _new_ TCGETS2 API then there is _NO_ point to use this new API in userspace and it is better to stick with the oldASYNC_SPD_CUST. And this is the current userspace state. So based on your input, it is the time to deprecate TCGETS2?
Stop being silly. As I've said repeatedly, we don't care about ASYNC_SPD_CUST. Just return 38400 regardless of whatever magic happens behind the scenes with the TIOCSSERIAL ioctl.
Johan
Re: [PATCH v2 6/7] USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for ASYNC_SPD_CUST
Quoted 39 lines · On Wednesday 14 September 2022 10:58:48 Johan Hovold wrote:
Quoted 29 lines · Pali Rohár
Quoted 19 lines · On Tuesday 13 September 2022 16:59:22 Johan Hovold wrote:
Quoted 18 lines · Pali Rohár
Quoted 17 lines · On Sunday 24 July 2022 15:08:09 Johan Hovold wrote:
Quoted 15 lines · Pali Rohár
Quoted 10 lines · On Sunday 24 July 2022 14:54:58 Johan Hovold wrote:
Quoted 5 lines · Pali Rohár
What is updated is the real baudrate in
c_ispeedandc_ospeedextensions.It is really wrong? I thought that
c_cflagshould stay unchanged at B38400 whenASYNC_SPD_CUSTis used.Yeah, cflags stay unchanged, but you shouldn't touch those fields when using the deprecated
ASYNC_SPD_CUSThack.Hm... Why? I thought that new extended fields (
c_ispeedandc_ospeed) should contain current real speed. What is the reason that c_*speed fields should have 38400 whenASYNC_SPD_CUSThack is set?Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Because we shouldn't go adding new features built around the deprecated
ASYNC_SPD_CUSThack.But this is not a new feature in the old deprecated hack. It for the new interface.
I think I understand what you're getting at, but no. Let's not add more features built around
ASYNC_SPD_CUST.Quoted 8 lines
Seems that you did not understand the point. So I will try to explain it again. This is not a new feature for _old_
ASYNC_SPD_CUST. This is the fix for the _new_ TCGETS2 API, to ensure that driver will always returns corrects values in c_*speed fields. If driver is not going to fix this _new_ TCGETS2 API then there is _NO_ point to use this new API in userspace and it is better to stick with the oldASYNC_SPD_CUST. And this is the current userspace state. So based on your input, it is the time to deprecate TCGETS2?
Quoted 5 lines
Stop being silly. As I've said repeatedly, we don't care about
ASYNC_SPD_CUST. Just return 38400 regardless of whatever magic happens behind the scenes with the TIOCSSERIAL ioctl.Johan
I'm not silly here. Look, those APIs are for userspace. And if userspace application cannot use this new TCGETS2 API (for more reasons) then they stick with the old one TIOCSSERIAL. And your inputs just say that it is not a good idea to switch TCGETS2 as this API stay broken in some drivers. Silly is the one who do not see (or do not want to see it; because of own API perfectionism) the reasons why new "proposed API" is still not (widely) used and applications stick with TCGETS + TIOCSSERIAL.
That is why I'm asking, it is time to starting deprecating TCGETS2 and create for example TCGETS3? Only just few application use TCGETS2, so deprecation of TCGETS2 can be done _now_ without pain as this API is not widely used.
Johan Hovold
maintainerRe: [PATCH v2 6/7] USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for ASYNC_SPD_CUST
Quoted 24 lines · Pali Rohár
Quoted 11 lines · On Wednesday 14 September 2022 10:58:48 Johan Hovold wrote:
Quoted 8 lines · Pali Rohár
Seems that you did not understand the point. So I will try to explain it again. This is not a new feature for _old_
ASYNC_SPD_CUST. This is the fix for the _new_ TCGETS2 API, to ensure that driver will always returns corrects values in c_*speed fields. If driver is not going to fix this _new_ TCGETS2 API then there is _NO_ point to use this new API in userspace and it is better to stick with the oldASYNC_SPD_CUST. And this is the current userspace state. So based on your input, it is the time to deprecate TCGETS2?Quoted 3 lines
Stop being silly. As I've said repeatedly, we don't care about
ASYNC_SPD_CUST. Just return 38400 regardless of whatever magic happens behind the scenes with the TIOCSSERIAL ioctl.I'm not silly here. Look, those APIs are for userspace. And if userspace application cannot use this new TCGETS2 API (for more reasons) then they stick with the old one TIOCSSERIAL. And your inputs just say that it is not a good idea to switch TCGETS2 as this API stay broken in some drivers. Silly is the one who do not see (or do not want to see it; because of own API perfectionism) the reasons why new "proposed API" is still not (widely) used and applications stick with TCGETS + TIOCSSERIAL.
Quoted 4 lines
That is why I'm asking, it is time to starting deprecating TCGETS2 and create for example TCGETS3? Only just few application use TCGETS2, so deprecation of TCGETS2 can be done _now_ without pain as this API is not widely used.
You're trying to keep the ASYNC_SPD hack alive by forcing drivers to take it into consideration for TCGETS2. Just stop using the former and switch to using BOTHER. And if something is missing in user space for that, then fix that.
Johan
Re: [PATCH v2 6/7] USB: serial: ftdi_sio: Fix custom_divisor and c_*speed for ASYNC_SPD_CUST
Quoted 30 lines · On Wednesday 14 September 2022 11:18:03 Johan Hovold wrote:
Quoted 24 lines · Pali Rohár
Quoted 11 lines · On Wednesday 14 September 2022 10:58:48 Johan Hovold wrote:
Quoted 8 lines · Pali Rohár
Seems that you did not understand the point. So I will try to explain it again. This is not a new feature for _old_
ASYNC_SPD_CUST. This is the fix for the _new_ TCGETS2 API, to ensure that driver will always returns corrects values in c_*speed fields. If driver is not going to fix this _new_ TCGETS2 API then there is _NO_ point to use this new API in userspace and it is better to stick with the oldASYNC_SPD_CUST. And this is the current userspace state. So based on your input, it is the time to deprecate TCGETS2?Quoted 3 lines
Stop being silly. As I've said repeatedly, we don't care about
ASYNC_SPD_CUST. Just return 38400 regardless of whatever magic happens behind the scenes with the TIOCSSERIAL ioctl.I'm not silly here. Look, those APIs are for userspace. And if userspace application cannot use this new TCGETS2 API (for more reasons) then they stick with the old one TIOCSSERIAL. And your inputs just say that it is not a good idea to switch TCGETS2 as this API stay broken in some drivers. Silly is the one who do not see (or do not want to see it; because of own API perfectionism) the reasons why new "proposed API" is still not (widely) used and applications stick with TCGETS + TIOCSSERIAL.
Quoted 4 lines
That is why I'm asking, it is time to starting deprecating TCGETS2 and create for example TCGETS3? Only just few application use TCGETS2, so deprecation of TCGETS2 can be done _now_ without pain as this API is not widely used.
Quoted 6 lines
You're trying to keep the
ASYNC_SPDhack alive by forcing drivers to take it into consideration for TCGETS2. Just stop using the former and switch to using BOTHER. And if something is missing in user space for that, then fix that.Johan
And what is the point of switching to BOTHER/TCGETS2 if some drivers do not return _correct_ values?
PATCH v2 7/7 USB: serial: ftdi_sio: Fill c_*speed fields with real baud rate
In reply to Marek BehúnFrom: Pali Rohár <[email protected]>
Calculate baud rate value in c_*speed fields to the real values which were set on hardware. For this operation, add a new set of methods *_divisor_to_baud() for each chip and use them for calculating the real baud rate values.
Each *_divisor_to_baud() method is constructed as an inverse function of its corresponding *_baud_to_divisor() method.
--- drivers/usb/serial/ftdi_sio.c | 83 +++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+)
drivers/usb/serial/ftdi_sio.c +83 -0
@@ -1167,6 +1167,23 @@ static int ftdi_sio_baud_to_divisor(int baud) } } +static int ftdi_sdio_divisor_to_baud (u32 divisor) +{ +switch (divisor) { +case ftdi_sio_b300:return 300 ; +case ftdi_sio_b600:return 600 ; +case ftdi_sio_b1200:return 1200 ; +case ftdi_sio_b2400:return 2400 ; +case ftdi_sio_b4800:return 4800 ; +case ftdi_sio_b9600:return 9600 ; +case ftdi_sio_b19200:return 19200 ; +case ftdi_sio_b38400:return 38400 ; +case ftdi_sio_b57600:return 57600 ; +case ftdi_sio_b115200:return 115200 ; +default :return 9600 ; + } +} +static unsigned short int ftdi_232am_baud_base_to_divisor (int baud,int base) {unsigned short int divisor;
@@ -1189,15 +1206,33 @@ static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base)return divisor; } +static int ftdi_232am_divisor_base_to_baud (unsigned short int divisor,int base) +{ +static const unsigned char divfrac_inv[4 ] = {0 ,4 ,2 ,1 }; +unsigned int divisor3; + +if (divisor ==0 ) + divisor =1 ; + divisor3 = (GENMASK (13 ,0 ) & divisor) <<3 ; + divisor3 |= divfrac_inv[(divisor >>14 ) &0x3 ]; +return DIV_ROUND_CLOSEST (base,2 * divisor3); +} +static unsigned short int ftdi_232am_baud_to_divisor (int baud) {return ftdi_232am_baud_base_to_divisor (baud,48000000 ); } +static int ftdi_232am_divisor_to_baud (unsigned short int divisor) +{ +return ftdi_232am_divisor_base_to_baud (divisor,48000000 ); +} +static u32 ftdi_232bm_baud_base_to_divisor (int baud,int base) {static const unsigned char divfrac[8 ] = {0 ,3 ,2 ,4 ,1 ,5 ,6 ,7 };u32 divisor; +/* divisor shifted 3 bits to the left */ int divisor3 =DIV_ROUND_CLOSEST (base,2 * baud);if (divisor3 >GENMASK (16 ,0 ))
@@ -1212,11 +1247,31 @@ static u32 ftdi_232bm_baud_base_to_divisor(int baud, int base)return divisor; } +static int ftdi_232bm_divisor_base_to_baud (u32 divisor,int base) +{ +static const unsigned char divfrac_inv[8 ] = {0 ,4 ,2 ,1 ,3 ,5 ,6 ,7 }; +u32 divisor3; + +/* Deal with special cases for highest baud rates. */ +if (divisor ==0 ) + divisor =1 ;/* 1.0 */ +else if (divisor ==1 ) + divisor =0x4001 ;/* 1.5 */ + divisor3 = (GENMASK (13 ,0 ) & divisor) <<3 ; + divisor3 |= divfrac_inv[(divisor >>14 ) &0x7 ]; +return DIV_ROUND_CLOSEST (base,2 * divisor3); +} +static u32 ftdi_232bm_baud_to_divisor (int baud) {return ftdi_232bm_baud_base_to_divisor (baud,48000000 ); } +static int ftdi_232bm_divisor_to_baud (u32 divisor) +{ +return ftdi_232bm_divisor_base_to_baud (divisor,48000000 ); +} +static u32 ftdi_2232h_baud_base_to_divisor (int baud,int base) {static const unsigned char divfrac[8 ] = {0 ,3 ,2 ,4 ,1 ,5 ,6 ,7 };
@@ -1244,11 +1299,32 @@ static u32 ftdi_2232h_baud_base_to_divisor(int baud, int base)return divisor; } +static int ftdi_2232h_divisor_base_to_baud (u32 divisor,int base) +{ +static const unsigned char divfrac_inv[8 ] = {0 ,4 ,2 ,1 ,3 ,5 ,6 ,7 }; +u32 divisor3; + + divisor &=GENMASK (16 ,0 ); +/* Deal with special cases for highest baud rates. */ +if (divisor ==0 ) + divisor =1 ;/* 1.0 */ +else if (divisor ==1 ) + divisor =0x4001 ;/* 1.5 */ + divisor3 = (GENMASK (13 ,0 ) & divisor) <<3 ; + divisor3 |= divfrac_inv[(divisor >>14 ) &0x7 ]; +return DIV_ROUND_CLOSEST (8 * base,10 * divisor3); +} +static u32 ftdi_2232h_baud_to_divisor (int baud) {return ftdi_2232h_baud_base_to_divisor (baud,120000000 ); } +static int ftdi_2232h_divisor_to_baud (u32 divisor) +{ +return ftdi_2232h_divisor_base_to_baud (divisor,120000000 ); +} +#define set_mctrl (port, set) update_mctrl((port), (set), 0)#define clear_mctrl (port, clear) update_mctrl((port), 0, (clear))
@@ -1342,6 +1418,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, } div_okay =0 ; } + baud =ftdi_sdio_divisor_to_baud (div_value);break ;case FT8U232AM :/* 8U232AM chip */ if (baud >=183 && baud <=3000000 ) {
@@ -1352,6 +1429,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, div_value =ftdi_232am_baud_to_divisor (baud); div_okay =0 ; } + baud =ftdi_232am_divisor_to_baud (div_value);break ;case FT232BM :/* FT232BM chip */ case FT2232C :/* FT2232C chip */
@@ -1375,22 +1453,27 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, div_value =ftdi_232bm_baud_to_divisor (baud); div_okay =0 ; } + baud =ftdi_232bm_divisor_to_baud (div_value);break ;case FT2232H :/* FT2232H chip */ case FT4232H :/* FT4232H chip */ case FT232H :/* FT232H chip */ if ((baud <=12000000 ) && (baud >=1200 )) { div_value =ftdi_2232h_baud_to_divisor (baud); + baud =ftdi_2232h_divisor_to_baud (div_value); }else if (baud >=183 && baud <1200 ) { div_value =ftdi_232bm_baud_to_divisor (baud); + baud =ftdi_232bm_divisor_to_baud (div_value); }else {dev_dbg (dev,"%s - Baud rate too high!\n" , __func__);if (old_baud >=183 && old_baud <1200 ) { baud = old_baud; div_value =ftdi_232bm_baud_to_divisor (baud); + baud =ftdi_232bm_divisor_to_baud (div_value); }else { baud = (old_baud >=1200 && old_baud <=12000000 ) ? old_baud :9600 ; div_value =ftdi_2232h_baud_to_divisor (baud); + baud =ftdi_2232h_divisor_to_baud (div_value); } div_okay =0 ; }
Signature · 1 line
2.35.1
Johan Hovold
maintainerRe: [PATCH v2 7/7] USB: serial: ftdi_sio: Fill c_*speed fields with real baud rate
Quoted 31 lines · Marek Behún
From: Pali Rohár <[email protected]>
Calculate baud rate value in c_*speed fields to the real values which were set on hardware. For this operation, add a new set of methods
*_divisor_to_baud()for each chip and use them for calculating the real baud rate values.Each
*_divisor_to_baud()method is constructed as an inverse function of its corresponding*_baud_to_divisor()method.Signed-off-by Pali Rohár <[email protected]>Tested-by Marek Behún <[email protected]>Signed-off-by Marek Behún <[email protected]>--- drivers/usb/serial/ftdi_sio.c | 83 +++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+)drivers/usb/serial/ftdi_sio.c +6
-0@@ -1167,6 +1167,23 @@ static int ftdi_sio_baud_to_divisor(int baud) } } +static int ftdi_sdio_divisor_to_baud (u32 divisor) +{ +switch (divisor) { +case ftdi_sio_b300:return 300 ; +case ftdi_sio_b600:return 600 ; +case ftdi_sio_b1200:return 1200 ;
drivers/usb/serial/ftdi_sio.c static int ftdi_sio_baud_to_divisor(int baud) · line 1185
Quoted 10 lines
drivers/usb/serial/ftdi_sio.c +10
-0+case ftdi_sio_b2400:return 2400 ; +case ftdi_sio_b4800:return 4800 ; +case ftdi_sio_b9600:return 9600 ; +case ftdi_sio_b19200:return 19200 ; +case ftdi_sio_b38400:return 38400 ; +case ftdi_sio_b57600:return 57600 ; +case ftdi_sio_b115200:return 115200 ; +default :return 9600 ; + } +}
This one should not be needed as sio only supports this discrete set of values in the first place.
drivers/usb/serial/ftdi_sio.c static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base) · line 1209
Quoted 8 lines
drivers/usb/serial/ftdi_sio.c +1
-0static unsigned short int ftdi_232am_baud_base_to_divisor (int baud,int base) {unsigned short int divisor;@@ -1189,15 +1206,33 @@ static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base)return divisor; } +static int ftdi_232am_divisor_base_to_baud (unsigned short int divisor,int base)
I believe "base" was used as a function name suffix in the inverse function (due to the additional base argument).
drivers/usb/serial/ftdi_sio.c static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base) · line 1219
Quoted 10 lines
drivers/usb/serial/ftdi_sio.c +10
-0+{ +static const unsigned char divfrac_inv[4 ] = {0 ,4 ,2 ,1 }; +unsigned int divisor3; + +if (divisor ==0 ) + divisor =1 ; + divisor3 = (GENMASK (13 ,0 ) & divisor) <<3 ; + divisor3 |= divfrac_inv[(divisor >>14 ) &0x3 ]; +return DIV_ROUND_CLOSEST (base,2 * divisor3); +}
I don't have the motivation to try to review these inverses right now.
Let's get the rest of the series in order first.
Johan
Re: [PATCH v2 7/7] USB: serial: ftdi_sio: Fill c_*speed fields with real baud rate
Quoted 53 lines · On Sunday 24 July 2022 14:41:19 Johan Hovold wrote:
Quoted 31 lines · Marek Behún
From: Pali Rohár <[email protected]>
Calculate baud rate value in c_*speed fields to the real values which were set on hardware. For this operation, add a new set of methods
*_divisor_to_baud()for each chip and use them for calculating the real baud rate values.Each
*_divisor_to_baud()method is constructed as an inverse function of its corresponding*_baud_to_divisor()method.Signed-off-by Pali Rohár <[email protected]>Tested-by Marek Behún <[email protected]>Signed-off-by Marek Behún <[email protected]>--- drivers/usb/serial/ftdi_sio.c | 83 +++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+)drivers/usb/serial/ftdi_sio.c +6
-0@@ -1167,6 +1167,23 @@ static int ftdi_sio_baud_to_divisor(int baud) } } +static int ftdi_sdio_divisor_to_baud (u32 divisor) +{ +switch (divisor) { +case ftdi_sio_b300:return 300 ; +case ftdi_sio_b600:return 600 ; +case ftdi_sio_b1200:return 1200 ;drivers/usb/serial/ftdi_sio.c static int ftdi_sio_baud_to_divisor(int baud) · line 1185
Quoted 10 lines
drivers/usb/serial/ftdi_sio.c +10
-0+case ftdi_sio_b2400:return 2400 ; +case ftdi_sio_b4800:return 4800 ; +case ftdi_sio_b9600:return 9600 ; +case ftdi_sio_b19200:return 19200 ; +case ftdi_sio_b38400:return 38400 ; +case ftdi_sio_b57600:return 57600 ; +case ftdi_sio_b115200:return 115200 ; +default :return 9600 ; + } +}This one should not be needed as sio only supports this discrete set of values in the first place.
Quoted 7 lines
drivers/usb/serial/ftdi_sio.c +0
-0static unsigned short int ftdi_232am_baud_base_to_divisor (int baud,int base) {unsigned short int divisor;@@ -1189,15 +1206,33 @@ static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base)return divisor; }
Quoted 4 lines
drivers/usb/serial/ftdi_sio.c static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base) · line 1209
Quoted 1 line
drivers/usb/serial/ftdi_sio.c +1
-0+static int ftdi_232am_divisor_base_to_baud (unsigned short int divisor,int base)I believe "base" was used as a function name suffix in the inverse function (due to the additional base argument).
Yes, in the same way as it used in name of ftdi_232am_baud_base_to_divisor.
Quoted 16 lines
drivers/usb/serial/ftdi_sio.c static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base) · line 1219
Quoted 10 lines
drivers/usb/serial/ftdi_sio.c +10
-0+{ +static const unsigned char divfrac_inv[4 ] = {0 ,4 ,2 ,1 }; +unsigned int divisor3; + +if (divisor ==0 ) + divisor =1 ; + divisor3 = (GENMASK (13 ,0 ) & divisor) <<3 ; + divisor3 |= divfrac_inv[(divisor >>14 ) &0x3 ]; +return DIV_ROUND_CLOSEST (base,2 * divisor3); +}I don't have the motivation to try to review these inverses right now.
Let's get the rest of the series in order first.
Johan
Johan Hovold
maintainerRe: [PATCH v2 7/7] USB: serial: ftdi_sio: Fill c_*speed fields with real baud rate
Quoted 26 lines · Pali Rohár
Quoted 25 lines · On Sunday 24 July 2022 14:41:19 Johan Hovold wrote:
Quoted 24 lines · Marek Behún
From: Pali Rohár <[email protected]>
Calculate baud rate value in c_*speed fields to the real values which were set on hardware. For this operation, add a new set of methods
*_divisor_to_baud()for each chip and use them for calculating the real baud rate values.Each
*_divisor_to_baud()method is constructed as an inverse function of its corresponding*_baud_to_divisor()method.Signed-off-by Pali Rohár <[email protected]>Tested-by Marek Behún <[email protected]>Signed-off-by Marek Behún <[email protected]>--- drivers/usb/serial/ftdi_sio.c | 83 +++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+)static unsigned short int
ftdi_232am_baud_base_to_divisor(int baud, int base){drivers/usb/serial/ftdi_sio.c +0
-0unsigned short int divisor;@@ -1189,15 +1206,33 @@ static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base)return divisor; }
Quoted 6 lines
Quoted 4 lines
drivers/usb/serial/ftdi_sio.c static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base) · line 1209
Quoted 1 line
drivers/usb/serial/ftdi_sio.c +1
-0+static int ftdi_232am_divisor_base_to_baud (unsigned short int divisor,int base)I believe "base" was used as a function name suffix in the inverse function (due to the additional base argument).
Yes, in the same way as it used in name of
ftdi_232am_baud_base_to_divisor.
No, you used "base" as an infix here.
Johan
Re: [PATCH v2 7/7] USB: serial: ftdi_sio: Fill c_*speed fields with real baud rate
Quoted 32 lines · On Tuesday 13 September 2022 17:02:39 Johan Hovold wrote:
Quoted 31 lines · Pali Rohár
Quoted 29 lines · On Sunday 24 July 2022 14:41:19 Johan Hovold wrote:
Quoted 21 lines · Marek Behún
From: Pali Rohár <[email protected]>
Calculate baud rate value in c_*speed fields to the real values which were set on hardware. For this operation, add a new set of methods
*_divisor_to_baud()for each chip and use them for calculating the real baud rate values.Each
*_divisor_to_baud()method is constructed as an inverse function of its corresponding*_baud_to_divisor()method.Signed-off-by Pali Rohár <[email protected]>Tested-by Marek Behún <[email protected]>Signed-off-by Marek Behún <[email protected]>--- drivers/usb/serial/ftdi_sio.c | 83 +++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+)static unsigned short int
ftdi_232am_baud_base_to_divisor(int baud, int base){drivers/usb/serial/ftdi_sio.c +0
-0unsigned short int divisor;drivers/usb/serial/ftdi_sio.c static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base) · line 1209
Quoted 4 lines
drivers/usb/serial/ftdi_sio.c +1
-0@@ -1189,15 +1206,33 @@ static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base)return divisor; } +static int ftdi_232am_divisor_base_to_baud (unsigned short int divisor,int base)I believe "base" was used as a function name suffix in the inverse function (due to the additional base argument).
Quoted 5 lines
Quoted 1 line
Yes, in the same way as it used in name of
ftdi_232am_baud_base_to_divisor.No, you used "base" as an infix here.
Johan
Yes, of course, it is "in the middle" because I said I used it in the same way as in ftdi_232am_baud_base_to_divisor.
ftdi_232am_baud_base_to_divisor converts input "baud" and input "base" to return "divisor". Hence baud_base_to_divisor.
And so ftdi_232am_divisor_base_to_baud converts input "divisor" and input "base" to return value "baud". Hence divisor_base_to_baud.
So what is the problem? I still have not caught.
Johan Hovold
maintainerRe: [PATCH v2 7/7] USB: serial: ftdi_sio: Fill c_*speed fields with real baud rate
Quoted 23 lines · Pali Rohár
Quoted 10 lines · On Tuesday 13 September 2022 17:02:39 Johan Hovold wrote:
Quoted 9 lines · Pali Rohár
Quoted 7 lines · On Sunday 24 July 2022 14:41:19 Johan Hovold wrote:
Quoted 7 lines
static unsigned short int
ftdi_232am_baud_base_to_divisor(int baud, int base){drivers/usb/serial/ftdi_sio.c +0
-0unsigned short int divisor;@@ -1189,15 +1206,33 @@ static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base)return divisor; }Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
drivers/usb/serial/ftdi_sio.c static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base) · line 1209
Quoted 1 line
drivers/usb/serial/ftdi_sio.c +1
-0+static int ftdi_232am_divisor_base_to_baud (unsigned short int divisor,int base)I believe "base" was used as a function name suffix in the inverse function (due to the additional base argument).
Yes, in the same way as it used in name of
ftdi_232am_baud_base_to_divisor.No, you used "base" as an infix here.
Yes, of course, it is "in the middle" because I said I used it in the same way as in
ftdi_232am_baud_base_to_divisor.
Quoted 7 lines
ftdi_232am_baud_base_to_divisorconverts input "baud" and input "base" to return "divisor". Hencebaud_base_to_divisor.And so
ftdi_232am_divisor_base_to_baudconverts input "divisor" and input "base" to return value "baud". Hencedivisor_base_to_baud.So what is the problem? I still have not caught.
Ok, I should have looked closer at the code before replying.
I believe I expected "base" to be used as a suffix in the current code but that doesn't appear to be the case. So you're just following the current style.
Johan