Hi Luiz, On 19/08/2024 16:49, Luiz Augusto von Dentz wrote:
Hi Frédéric, On Wed, Aug 14, 2024 at 9:07 AM Frédéric Danis <frederic.danis@xxxxxxxxxxxxx> wrote:Starting a BCM UART controller not defined as a platform device or a serdev with "btattach -B /dev/ttyS1 -P bcm -S 3000000" works fine but the serial port remains at the init_speed, i.e. 115200. The oper_speed is only set if a device is declared in ACPI, device tree or as a platform device. When no registered device has been found this commit will use the current tty speed set by btattach as the oper_speed. Signed-off-by: Frédéric Danis <frederic.danis@xxxxxxxxxxxxx> --- This has been tested with 5.4 kernel only. Afaict there's no change in this driver which should be impacted by this commit in latest kernel. drivers/bluetooth/hci_bcm.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c index 89d4c2224546..57cacf63ae12 100644 --- a/drivers/bluetooth/hci_bcm.c +++ b/drivers/bluetooth/hci_bcm.c @@ -508,6 +508,11 @@ static int bcm_open(struct hci_uart *hu) if (err) goto err_unset_hu; + } else { + /* This is not a serdev or platform device, it should have been started by + * btattach, in this case use the tty speed set by btattach as oper_speed + */ + hu->oper_speed = hu->tty->termios.c_ospeed; }While Im fine with the general idea of derive the speed from the tty what Im not sure where it should be done since doing this on the driver may duplicate the logic if we later found there is a better place to do it. Looks at hci_uart.c it seems like we could actually register a set_termios: * @set_termios: [TTY] ``void ()(struct tty_struct *tty, const struct ktermios *old)`` * * This function notifies the line discpline that a change has been made * to the termios structure. * * Optional. So I assume when the user changes the speed the above callback gets called and we could then reflect the changes to the oper_speed, or perhaps the real problem was that hci_uart_set_baudrate was not called?
The hci_uart_set_baudrate is called correctly as when I forced the oper_speed the serial port speed changes accordingly and I saw the hci_uart_set_baudrate debug trace.
Unfortunately, the serial port speed is fixed by btattach before setting the line discipline, so that the set_termios is not called in this case. But above all, set_termios is called each time the driver change the speed, i.e. when it sets the initial speed.
I will send a new patch to copy the serial port speed to the oper_speed in hci_uart_tty_open() of drivers/bluetooth/hci_ldisc.c.
-- Frédéric Danis Senior Software Engineer Collabora Ltd. Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, United Kingdom Registered in England & Wales, no. 5513718
