Re: [PATCH] BlueZ line discipline baud rate setting update

[Frederic Danis <frederic.danis@linux.intel.com>]

Hello Marcel,

On 16/06/2015 16:36, Marcel Holtmann wrote:
> Hi Fred,
>
> >>> Bring the tty into a known 8 bits, 1 start bit, 1 stop bit,
> >>> hardware flow control state with a given baud rate.
> >>>
> >>> Signed-off-by: Ilya Faenson <ifaenson@broadcom.com>
> >>> ---
> >>> drivers/bluetooth/hci_ldisc.c | 9 ++++++++-
> >>> 1 file changed, 8 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/bluetooth/hci_ldisc.c 
> b/drivers/bluetooth/hci_ldisc.c
> >>> index ac87346..606cc5a 100644
> >>> --- a/drivers/bluetooth/hci_ldisc.c
> >>> +++ b/drivers/bluetooth/hci_ldisc.c
> >>> @@ -271,9 +271,16 @@ void hci_uart_set_baudrate(struct hci_uart 
> *hu, unsigned int speed)
> >>>     struct tty_struct *tty = hu->tty;
> >>>     struct ktermios ktermios;
> >>>
> >>> +    /* Bring the UART into a known state with a given baud rate */
> >>>     ktermios = tty->termios;
> >>>     ktermios.c_cflag &= ~CBAUD;
> >>> -    ktermios.c_cflag |= BOTHER;
> >>> +    ktermios.c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP | INLCR |
> >>> +                IGNCR | ICRNL | IXON);
> >>
> >> I think this one needs to align like this:
> >>
> >>             &= ~(IGNBRK | .. |
> >>                  IGNCR | ..);
> >>
> >> However I can fix that one easily inline. So no worries.
> >>
> >> Fred, can you test this patch so I can add a Tested-by line.
> >
> > OK, I will test it.
> >
> > Btw, I thought that this function should only change the UART speed.
> > For other UART parameters, I thought they should be set by btattach, 
> ACPI or DT part.
>
> do you think we want this split out into one hci_uart_reset_termios or 
> something and one that actually changes the baud rate. That seems fair 
> to me as well so then the individual vendor code can pick and choose 
> what they want to do.

Yes.
Imo, the question is to know who will be responsible to open the serial 
port and configure it:
- from user space using btattach or bluetoothd (as you wrote in a 
previous mail), may be based on uevent giving those info,
- or from kernel space, which will imply to have all info from ACPI or DT

Regards

Fred