gb_tty_set_termios() derives UART line configuration from a subset of termios->c_cflag bits, namely CSIZE, CSTOPB, PARENB, PARODD, CMSPAR, CRTSCTS, CLOCAL and CBAUD. Other c_cflag bits are not interpreted by the driver and are not represented in the Greybus UART protocol messages.
The existing FIXME suggests clearing unsupported bits from termios. However, the driver already limits its behavior to the supported subset when constructing line coding, and unused bits are effectively ignored. No invalid or unsupported values are propagated to the hardware.
Replace the FIXME with a comment documenting which c_cflag bits are consumed by the driver and clarifying that other bits are ignored.
No functional change intended.
Signed-off-by: Debjeet Banerjee debjeetbanerjee48@gmail.com --- drivers/staging/greybus/uart.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c index 7d060b4cd33d..49d685a6ad8c 100644 --- a/drivers/staging/greybus/uart.c +++ b/drivers/staging/greybus/uart.c @@ -494,8 +494,20 @@ static void gb_tty_set_termios(struct tty_struct *tty, (termios->c_cflag & CMSPAR ? 2 : 0) : 0;
newline.data_bits = tty_get_char_size(termios->c_cflag); - - /* FIXME: needs to clear unsupported bits in the termios */ + /* + * The Greybus UART driver only interprets a subset of termios + * c_cflag bits when configuring line settings: + * + * - CSIZE via tty_get_char_size() for data bits + * - CSTOPB for stop-bit format + * - PARENB, PARODD, CMSPAR for parity encoding + * - CRTSCTS for hardware flow control + * - CLOCAL for modem control handling + * - CBAUD via C_BAUD() for baud rate and B0 semantics + * + * Other c_cflag bits are ignored as they are not represented in + * the Greybus UART protocol. + */ gb_tty->clocal = ((termios->c_cflag & CLOCAL) != 0);
if (C_BAUD(tty) == B0) {
On Sun, Apr 19, 2026 at 08:56:32AM +0530, Debjeet Banerjee wrote:
gb_tty_set_termios() derives UART line configuration from a subset of termios->c_cflag bits, namely CSIZE, CSTOPB, PARENB, PARODD, CMSPAR, CRTSCTS, CLOCAL and CBAUD. Other c_cflag bits are not interpreted by the driver and are not represented in the Greybus UART protocol messages.
The existing FIXME suggests clearing unsupported bits from termios. However, the driver already limits its behavior to the supported subset when constructing line coding, and unused bits are effectively ignored. No invalid or unsupported values are propagated to the hardware.
Replace the FIXME with a comment documenting which c_cflag bits are consumed by the driver and clarifying that other bits are ignored.
Ignoring is fine, but shouldn't the bits be cleared to let userspace know about this? I think that's why we wrote that FIXME all so long ago.
thanks,
greg k-h
On Sun, 26 Apr 2026 21:16:05 +0200, Greg Kroah-Hartman wrote:
Ignoring is fine, but shouldn't the bits be cleared to let userspace know about this? I think that's why we wrote that FIXME all so long ago.
Thanks for the feedback.
That makes sense. I had considered clearing unsupported bits earlier, but held off to avoid modifying termios and left them ignored instead. Your point about userspace visibility makes sense.
I'll send a v2 that clears unsupported c_cflag bits accordingly.
Regards, Debjeet
gb_tty_set_termios() derives UART line configuration from a subset of termios->c_cflag bits (CSIZE, CSTOPB, PARENB, PARODD, CMSPAR, CRTSCTS, CLOCAL and CBAUD). Other bits are not interpreted by the driver and are not represented in the Greybus UART protocol.
Mask unsupported c_cflag bits so that userspace-visible termios reflects the supported bits implemented by the driver.
This addresses the existing FIXME.
Signed-off-by: Debjeet Banerjee debjeetbanerjee48@gmail.com --- v2: - Clear unsupported c_cflag bits as suggested - Update comment to mention the change --- drivers/staging/greybus/uart.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c index 7d060b4cd33d..9c10e6baa7e0 100644 --- a/drivers/staging/greybus/uart.c +++ b/drivers/staging/greybus/uart.c @@ -495,7 +495,24 @@ static void gb_tty_set_termios(struct tty_struct *tty,
newline.data_bits = tty_get_char_size(termios->c_cflag);
- /* FIXME: needs to clear unsupported bits in the termios */ + /* + * The Greybus UART driver only interprets a subset of termios + * c_cflag bits when configuring line settings: + * + * - CSIZE via tty_get_char_size() for data bits + * - CSTOPB for stop-bit format + * - PARENB, PARODD, CMSPAR for parity encoding + * - CRTSCTS for hardware flow control + * - CLOCAL for modem control handling + * - CBAUD via C_BAUD() for baud rate and B0 semantics + * + * Mask unsupported c_cflag bits. + */ + termios->c_cflag &= (CSIZE | CSTOPB | + PARENB | PARODD | CMSPAR | + CLOCAL | CRTSCTS | + CBAUD); + gb_tty->clocal = ((termios->c_cflag & CLOCAL) != 0);
if (C_BAUD(tty) == B0) {
On Mon, Apr 27, 2026 at 10:24:46AM +0530, Debjeet Banerjee wrote:
gb_tty_set_termios() derives UART line configuration from a subset of termios->c_cflag bits (CSIZE, CSTOPB, PARENB, PARODD, CMSPAR, CRTSCTS, CLOCAL and CBAUD). Other bits are not interpreted by the driver and are not represented in the Greybus UART protocol.
Mask unsupported c_cflag bits so that userspace-visible termios reflects the supported bits implemented by the driver.
This addresses the existing FIXME.
Signed-off-by: Debjeet Banerjee debjeetbanerjee48@gmail.com
v2:
- Clear unsupported c_cflag bits as suggested
- Update comment to mention the change
Has this been tested? If not, I'd prefer to leave this as-is until it can be.
thanks,
greg k-h