* [PATCH] BlueZ line discipline baud rate setting update @ 2015-06-15 16:50 Ilya Faenson 2015-06-15 16:50 ` Ilya Faenson 0 siblings, 1 reply; 13+ messages in thread From: Ilya Faenson @ 2015-06-15 16:50 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth, Ilya Faenson At Marcel's request, I'm providing a version of the hci_uart_set_baudrate function that works well for me. Ilya Faenson (1): BlueZ line discipline baud rate setting update drivers/bluetooth/hci_ldisc.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] BlueZ line discipline baud rate setting update 2015-06-15 16:50 [PATCH] BlueZ line discipline baud rate setting update Ilya Faenson @ 2015-06-15 16:50 ` Ilya Faenson 2015-06-15 17:35 ` Marcel Holtmann ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Ilya Faenson @ 2015-06-15 16:50 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth, Ilya Faenson 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); + ktermios.c_oflag &= ~OPOST; + ktermios.c_lflag &= ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN); + ktermios.c_cflag &= ~(CSIZE | PARENB | CBAUD); + ktermios.c_cflag |= CS8; + ktermios.c_cflag |= CRTSCTS; tty_termios_encode_baud_rate(&ktermios, speed, speed); /* tty_set_termios() return not checked as it is always 0 */ -- 1.9.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] BlueZ line discipline baud rate setting update 2015-06-15 16:50 ` Ilya Faenson @ 2015-06-15 17:35 ` Marcel Holtmann 2015-06-16 8:43 ` Frederic Danis 2015-06-15 21:40 ` Marcel Holtmann 2015-06-17 13:16 ` Marcel Holtmann 2 siblings, 1 reply; 13+ messages in thread From: Marcel Holtmann @ 2015-06-15 17:35 UTC (permalink / raw) To: Ilya Faenson; +Cc: linux-bluetooth Hi Ilya, > 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. Regards Marcel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] BlueZ line discipline baud rate setting update 2015-06-15 17:35 ` Marcel Holtmann @ 2015-06-16 8:43 ` Frederic Danis 2015-06-16 12:19 ` Ilya Faenson ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Frederic Danis @ 2015-06-16 8:43 UTC (permalink / raw) To: Marcel Holtmann, Ilya Faenson; +Cc: linux-bluetooth Hello Marcel and Ilya, On 15/06/2015 19:35, Marcel Holtmann wrote: > Hi Ilya, > >> 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. Regards Fred ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] BlueZ line discipline baud rate setting update 2015-06-16 8:43 ` Frederic Danis @ 2015-06-16 12:19 ` Ilya Faenson 2015-06-16 13:34 ` Frederic Danis 2015-06-16 14:36 ` Marcel Holtmann 2 siblings, 0 replies; 13+ messages in thread From: Ilya Faenson @ 2015-06-16 12:19 UTC (permalink / raw) To: Frederic Danis, Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org Hi Fred, ________________________________________ From: Frederic Danis [frederic.danis@linux.intel.com] Sent: Tuesday, June 16, 2015 4:43 AM To: Marcel Holtmann; Ilya Faenson Cc: linux-bluetooth@vger.kernel.org Subject: Re: [PATCH] BlueZ line discipline baud rate setting update Hello Marcel and Ilya, On 15/06/2015 19:35, Marcel Holtmann wrote: > Hi Ilya, > >> 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. IF: UART parameters should be fully configured in kernel space in my opinion. I don't set any UART parameters from user space in my testing. As far as the DT/ACPI are concerned, none of the current ACPI DSL BT nodes configure UART parameters yet. I've introduced the oper-speed parameter for DT and that's the only UART parameter there for now. If we decide to add more, we'll have to change the line discipline configuration accordingly. Not sure it is needed though as all the BT UART-connected devices I've ever worked with in the last 10 years or so (including test devices connected via the FTDI-based USB boards) always worked fine with 8 bits, 1 start bit, 1 stop bit, no parity, hardware flow control. Regards Fred ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] BlueZ line discipline baud rate setting update 2015-06-16 8:43 ` Frederic Danis 2015-06-16 12:19 ` Ilya Faenson @ 2015-06-16 13:34 ` Frederic Danis 2015-06-16 14:36 ` Marcel Holtmann 2 siblings, 0 replies; 13+ messages in thread From: Frederic Danis @ 2015-06-16 13:34 UTC (permalink / raw) To: Marcel Holtmann; +Cc: Ilya Faenson, linux-bluetooth Hello Marcel, On 16/06/2015 10:43, Frederic Danis wrote: > Hello Marcel and Ilya, > > On 15/06/2015 19:35, Marcel Holtmann wrote: >> Hi Ilya, >> >>> 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. Device setup works correctly. Regards Fred -- Frederic Danis Open Source Technology Center frederic.danis@intel.com Intel Corporation ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] BlueZ line discipline baud rate setting update 2015-06-16 8:43 ` Frederic Danis 2015-06-16 12:19 ` Ilya Faenson 2015-06-16 13:34 ` Frederic Danis @ 2015-06-16 14:36 ` Marcel Holtmann 2015-06-16 16:15 ` Frederic Danis 2 siblings, 1 reply; 13+ messages in thread From: Marcel Holtmann @ 2015-06-16 14:36 UTC (permalink / raw) To: Frederic Danis; +Cc: Ilya Faenson, linux-bluetooth 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. Regards Marcel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] BlueZ line discipline baud rate setting update 2015-06-16 14:36 ` Marcel Holtmann @ 2015-06-16 16:15 ` Frederic Danis 2015-06-16 16:28 ` Marcel Holtmann 0 siblings, 1 reply; 13+ messages in thread From: Frederic Danis @ 2015-06-16 16:15 UTC (permalink / raw) To: Marcel Holtmann; +Cc: Ilya Faenson, linux-bluetooth 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] BlueZ line discipline baud rate setting update 2015-06-16 16:15 ` Frederic Danis @ 2015-06-16 16:28 ` Marcel Holtmann 0 siblings, 0 replies; 13+ messages in thread From: Marcel Holtmann @ 2015-06-16 16:28 UTC (permalink / raw) To: Frederic Danis; +Cc: Ilya Faenson, linux-bluetooth 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 I think we need both. Some vendor drivers might rely on that userspace is opening the port correctly. Otherwise need to toggle things and actually take control. Regards Marcel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] BlueZ line discipline baud rate setting update 2015-06-15 16:50 ` Ilya Faenson 2015-06-15 17:35 ` Marcel Holtmann @ 2015-06-15 21:40 ` Marcel Holtmann 2015-06-15 21:45 ` Ilya Faenson 2015-06-17 13:16 ` Marcel Holtmann 2 siblings, 1 reply; 13+ messages in thread From: Marcel Holtmann @ 2015-06-15 21:40 UTC (permalink / raw) To: Ilya Faenson; +Cc: linux-bluetooth Hi Ilya, > 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); > + ktermios.c_oflag &= ~OPOST; > + ktermios.c_lflag &= ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN); > + ktermios.c_cflag &= ~(CSIZE | PARENB | CBAUD); I just realize that we are clearing CBAUD twice here. I think the one above BOTHER should also be removed. > + ktermios.c_cflag |= CS8; > + ktermios.c_cflag |= CRTSCTS; > tty_termios_encode_baud_rate(&ktermios, speed, speed); Regards Marcel ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] BlueZ line discipline baud rate setting update 2015-06-15 21:40 ` Marcel Holtmann @ 2015-06-15 21:45 ` Ilya Faenson 0 siblings, 0 replies; 13+ messages in thread From: Ilya Faenson @ 2015-06-15 21:45 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org Hi Marcel, -----Original Message----- From: Marcel Holtmann [mailto:marcel@holtmann.org]=20 Sent: Monday, June 15, 2015 5:41 PM To: Ilya Faenson Cc: linux-bluetooth@vger.kernel.org Subject: Re: [PATCH] BlueZ line discipline baud rate setting update Hi Ilya, > Bring the tty into a known 8 bits, 1 start bit, 1 stop bit, > hardware flow control state with a given baud rate. >=20 > Signed-off-by: Ilya Faenson <ifaenson@broadcom.com> > --- > drivers/bluetooth/hci_ldisc.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) >=20 > 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, unsi= gned int speed) > struct tty_struct *tty =3D hu->tty; > struct ktermios ktermios; >=20 > + /* Bring the UART into a known state with a given baud rate */ > ktermios =3D tty->termios; > ktermios.c_cflag &=3D ~CBAUD; > - ktermios.c_cflag |=3D BOTHER; > + ktermios.c_iflag &=3D ~(IGNBRK | BRKINT | PARMRK | ISTRIP | INLCR | > + IGNCR | ICRNL | IXON); > + ktermios.c_oflag &=3D ~OPOST; > + ktermios.c_lflag &=3D ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN); > + ktermios.c_cflag &=3D ~(CSIZE | PARENB | CBAUD); I just realize that we are clearing CBAUD twice here. I think the one above= BOTHER should also be removed. IF: Agreed, good catch. > + ktermios.c_cflag |=3D CS8; > + ktermios.c_cflag |=3D CRTSCTS; > tty_termios_encode_baud_rate(&ktermios, speed, speed); Regards Marcel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] BlueZ line discipline baud rate setting update 2015-06-15 16:50 ` Ilya Faenson 2015-06-15 17:35 ` Marcel Holtmann 2015-06-15 21:40 ` Marcel Holtmann @ 2015-06-17 13:16 ` Marcel Holtmann 2015-06-17 13:26 ` Ilya Faenson 2 siblings, 1 reply; 13+ messages in thread From: Marcel Holtmann @ 2015-06-17 13:16 UTC (permalink / raw) To: Ilya Faenson; +Cc: linux-bluetooth Hi Ilya, > 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); > + ktermios.c_oflag &= ~OPOST; > + ktermios.c_lflag &= ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN); > + ktermios.c_cflag &= ~(CSIZE | PARENB | CBAUD); > + ktermios.c_cflag |= CS8; > + ktermios.c_cflag |= CRTSCTS; > tty_termios_encode_baud_rate(&ktermios, speed, speed); with the questions raised by the others, I think for now we just want to change the baud rate and leave the rest to a more generic function. I am fine if the kernel side wants to enforce the settings on the UART. That seems fully reasonable. However we need to give drivers a chance to let it come from userspace as well and honor whatever they configured there. So we need to clear CBAUD and then encode the baud rate. Do we need to bother with BOTHER or is that just not needed at all. I really like to get this detail fixed as soon as possible since we are close to the merge window. Regards Marcel ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH] BlueZ line discipline baud rate setting update 2015-06-17 13:16 ` Marcel Holtmann @ 2015-06-17 13:26 ` Ilya Faenson 0 siblings, 0 replies; 13+ messages in thread From: Ilya Faenson @ 2015-06-17 13:26 UTC (permalink / raw) To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org Hi Marcel, ________________________________________ From: Marcel Holtmann [marcel@holtmann.org] Sent: Wednesday, June 17, 2015 9:16 AM To: Ilya Faenson Cc: linux-bluetooth@vger.kernel.org Subject: Re: [PATCH] BlueZ line discipline baud rate setting update Hi Ilya, > 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, unsi= gned int speed) > struct tty_struct *tty =3D hu->tty; > struct ktermios ktermios; > > + /* Bring the UART into a known state with a given baud rate */ > ktermios =3D tty->termios; > ktermios.c_cflag &=3D ~CBAUD; > - ktermios.c_cflag |=3D BOTHER; > + ktermios.c_iflag &=3D ~(IGNBRK | BRKINT | PARMRK | ISTRIP | INLCR | > + IGNCR | ICRNL | IXON); > + ktermios.c_oflag &=3D ~OPOST; > + ktermios.c_lflag &=3D ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN); > + ktermios.c_cflag &=3D ~(CSIZE | PARENB | CBAUD); > + ktermios.c_cflag |=3D CS8; > + ktermios.c_cflag |=3D CRTSCTS; > tty_termios_encode_baud_rate(&ktermios, speed, speed); with the questions raised by the others, I think for now we just want to ch= ange the baud rate and leave the rest to a more generic function. I am fine= if the kernel side wants to enforce the settings on the UART. That seems f= ully reasonable. However we need to give drivers a chance to let it come fr= om userspace as well and honor whatever they configured there. So we need to clear CBAUD and then encode the baud rate. Do we need to both= er with BOTHER or is that just not needed at all. I really like to get this= detail fixed as soon as possible since we are close to the merge window. IF: The BOTHER is not needed. Okay: if this function deals with the baud ra= te only, I will then introduce a separate function to bring the UART into a= reasonable default state for Bluetooth as part of the next Broadcom patch = set. I'll try publishing them today if my other duties do not interfere. Regards Marcel= ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-06-17 13:26 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-06-15 16:50 [PATCH] BlueZ line discipline baud rate setting update Ilya Faenson 2015-06-15 16:50 ` Ilya Faenson 2015-06-15 17:35 ` Marcel Holtmann 2015-06-16 8:43 ` Frederic Danis 2015-06-16 12:19 ` Ilya Faenson 2015-06-16 13:34 ` Frederic Danis 2015-06-16 14:36 ` Marcel Holtmann 2015-06-16 16:15 ` Frederic Danis 2015-06-16 16:28 ` Marcel Holtmann 2015-06-15 21:40 ` Marcel Holtmann 2015-06-15 21:45 ` Ilya Faenson 2015-06-17 13:16 ` Marcel Holtmann 2015-06-17 13:26 ` Ilya Faenson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.