Move HCI UART vendor specific setup to kernel

This allows user space application to set the correct vendor specific setup function.

Signed-off-by Frederic Danis <[email protected]>

---
drivers/bluetooth/hci_ldisc.c | 50 +++++++++++++++++++++++++++++++++++++++++++ drivers/bluetooth/hci_uart.h | 4 ++++

2 files changed, 54 insertions(+)

drivers/bluetooth/hci_ldisc.c +0 -0

index 1363dc6..cb7fcc6 100644
@@ -467,9 +467,19 @@ static int hci_uart_register_dev(struct hci_uart *hu)
 	return 0;
 }

This allows user space application to set final speed requested for UART device. UART port is open at init speed by user space application.

Signed-off-by Frederic Danis <[email protected]>

---
drivers/bluetooth/hci_ldisc.c | 4 ++++
drivers/bluetooth/hci_uart.h | 2 ++

2 files changed, 6 insertions(+)

drivers/bluetooth/hci_ldisc.c +4 -0

index e8412f8..4b09369 100644
@@ -614,6 +614,10 @@ static int hci_uart_tty_ioctl(struct tty_struct *tty, struct file * file,
 	case HCIUARTGETFLAGS:
 		return hu->hdev_flags;
 
+	case HCIUARTSETBAUDRATE:
+		hu->speed = arg;
+		break;
+
 	case HCIUARTSETDEVTYPE:
 		err = hci_tty_ioctl_set_devtype(hu, cmd, arg);
 		if (err)

drivers/bluetooth/hci_uart.h +0 -0

index bf6f0e5..dcbedaf 100644

This allows user space application to set final speed requested for UART device. UART port is open at init speed by user space application.

Quoted 2 lines · Frederic Danis

This allows user space application to set final speed requested for UART device. UART port is open at init speed by user space application.

If userspace opened the tty and knows this is a UART, why can it not use the for-purpose tty ioctls to change the line rate?

[ +cc GregKH, JiriS ]

Quoted 6 lines · Peter Hurley

Quoted 2 lines · Frederic Danis

This allows user space application to set final speed requested for UART device. UART port is open at init speed by user space application.

If userspace opened the tty and knows this is a UART, why can it not use the for-purpose tty ioctls to change the line rate?

The purpose of this Bluetooth set of patches is to move device setup for Bluetooth UART devices into the kernel.
The idea is that userspace only needs to open the tty at initial speed and set correct line discipline, then userspace has no more to do, its up to HCI UART driver to manage the device (including firmware loading, bluetooth address and device speed setup).

However the driver may want to modify device speed (by using vendor specific HCI commands) to operate at full speed. Moreover, in case of firmware loading, some devices reset their speed to the default one, which will imply to change UART host speed to be able to continue communication.

Userspace can not know when all those speed changes occur.

I'd like to be able to help out with the firmware download problem but it's not clear to me at what point that is being executed since the patch that adds a setup() method to the protocol doesn't compile:

/home/peter/src/kernels/mainline/drivers/bluetooth/hci_ldisc.c: In function ‘hci_uart_set_proto’:
/home/peter/src/kernels/mainline/drivers/bluetooth/hci_ldisc.c:485:11: error: ‘struct hci_uart_proto’ has no member named ‘setup’

  hu->proto->setup = NULL;
           ^
/home/peter/src/kernels/mainline/drivers/bluetooth/hci_ldisc.c:488:13: error: ‘struct hci_uart_proto’ has no member named ‘setup’
    hu->proto->setup = devtypes[i].setup;
             ^

make[3]: *** [drivers/bluetooth/hci_ldisc.o] Error 1

The line discipline is always notified of line rate changes via the set_termios() method, so if you add that to the hci_ldisc, you'll be able to keep the BT device in sync with your vendor-specific commands.

Hi Peter,

Quoted 4 lines · Peter Hurley

The line discipline is always notified of line rate changes via the set_termios() method, so if you add that to the hci_ldisc, you'll be able to keep the BT device in sync with your vendor-specific commands.

If I'm not wrong, line discipline is notified after the tty termios change. So, it's too late to send the HCI change speed command. Moreover, user space is not aware of the device behavior, the driver should be the only one to manage the device and its serial link.
Here, user space is just a bootstrap which hands over to the driver.

Quoted 9 lines

Quoted 4 lines

The line discipline is always notified of line rate changes via the set_termios() method, so if you add that to the hci_ldisc, you'll be able to keep the BT device in sync with your vendor-specific commands.

If I'm not wrong, line discipline is notified after the tty termios change. So, it's too late to send the HCI change speed command. Moreover, user space is not aware of the device behavior, the driver should be the only one to manage the device and its serial link.
Here, user space is just a bootstrap which hands over to the driver.

You realize that you're telling me that userspace is unaware of this in a thread that begins with a proposed userspace ioctl change to the line discipline? An ioctl change that adds a new ioctl to set the speed in a line discipline? For which there are already lots of ioctls to set the line speed?

Setting aside the problem with the firmware load, I see no reason why this can't be done within the existing line discipline framework.

Hi Peter,

Quoted 10 lines

Quoted 9 lines

Quoted 4 lines

The line discipline is always notified of line rate changes via the set_termios() method, so if you add that to the hci_ldisc, you'll be able to keep the BT device in sync with your vendor-specific commands.

If I'm not wrong, line discipline is notified after the tty termios change. So, it's too late to send the HCI change speed command. Moreover, user space is not aware of the device behavior, the driver should be the only one to manage the device and its serial link.
Here, user space is just a bootstrap which hands over to the driver.

Quoted 5 lines

You realize that you're telling me that userspace is unaware of this in a thread that begins with a proposed userspace ioctl change to the line discipline? An ioctl change that adds a new ioctl to set the speed in a line discipline? For which there are already lots of ioctls to set the line speed?

actually the problem is a little bit different here.

Every Bluetooth UART chip has its default baudrate. Mostly that is 115k, but some actually have others.

The general idea was that userspace deals with opening the TTY, send the HCI commands to change the rate, then change the termios setting, attach the ldisc and then hand it to the kernel.

This is however not how at least two major Bluetooth controllers work when you have to do firmware loading. When they boot into the firmware, then baudrate falls back to default and you have to do that all over again.

So what this ioctl will just tell the kernel about is the desired baudrate that it wants to be set whenever possible. The whenever possible is important here since depending on the manufacturer that might be a total different times. And with firmware download procedures being fully vendor specific this will vary.

Quoted 2 lines

Setting aside the problem with the firmware load, I see no reason why this can't be done within the existing line discipline framework.

So far everything has been done in hciattach tool. And it is so ugly and does not even work properly for any complex UART protocols like BCSP or 3-Wire. There is a patch for a modified 3-Wire setup from one vendor that made my eyes bleed. So what you really want is having the kernel take control over HCI from the beginning. Otherwise this goes out of control and handling exceptions like hardware error event becomes impossible.

In summary what we have to do these days is this:

- Send Reset
- Send a few basic HCI commands
- Send baud rate change command
- Change actual baud rate on the TTY
- Download firmware
- Send Reset to boot the firmware
- Change actual baud rate back to default
- Send baud rate change command
- Change actual baud rate
- Send Reset
- Standard init sequence

Now tell me how the line discipline framework helps us here?

Keep in mind you have to tell the controller via HCI commands over the old baud rate to change its rate. And then change it on the TTY to match what the controller does on its side.

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

Quoted 4 lines

The line discipline is always notified of line rate changes via the set_termios() method, so if you add that to the hci_ldisc, you'll be able to keep the BT device in sync with your vendor-specific commands.

If I'm not wrong, line discipline is notified after the tty termios change. So, it's too late to send the HCI change speed command. Moreover, user space is not aware of the device behavior, the driver should be the only one to manage the device and its serial link.
Here, user space is just a bootstrap which hands over to the driver.

Quoted 7 lines

Quoted 5 lines

You realize that you're telling me that userspace is unaware of this in a thread that begins with a proposed userspace ioctl change to the line discipline? An ioctl change that adds a new ioctl to set the speed in a line discipline? For which there are already lots of ioctls to set the line speed?

actually the problem is a little bit different here.

Thanks for taking the time to outline the operation of hciattach. It's been a while since I was involved in the BT userspace code so I hadn't realized how much hciattach had morphed.

Quoted 27 lines

Every Bluetooth UART chip has its default baudrate. Mostly that is 115k, but some actually have others.

The general idea was that userspace deals with opening the TTY, send the HCI commands to change the rate, then change the termios setting, attach the ldisc and then hand it to the kernel.

This is however not how at least two major Bluetooth controllers work when you have to do firmware loading. When they boot into the firmware, then baudrate falls back to default and you have to do that all over again.

So what this ioctl will just tell the kernel about is the desired baudrate that it wants to be set whenever possible. The whenever possible is important here since depending on the manufacturer that might be a total different times. And with firmware download procedures being fully vendor specific this will vary.

Quoted 2 lines

Setting aside the problem with the firmware load, I see no reason why this can't be done within the existing line discipline framework.

So far everything has been done in hciattach tool. And it is so ugly and does not even work properly for any complex UART protocols like BCSP or 3-Wire. There is a patch for a modified 3-Wire setup from one vendor that made my eyes bleed. So what you really want is having the kernel take control over HCI from the beginning. Otherwise this goes out of control and handling exceptions like hardware error event becomes impossible.

In summary what we have to do these days is this:

- Send Reset
- Send a few basic HCI commands
- Send baud rate change command
- Change actual baud rate on the TTY
- Download firmware
- Send Reset to boot the firmware
- Change actual baud rate back to default
- Send baud rate change command
- Change actual baud rate
- Send Reset
- Standard init sequence

Quoted 1 line

Now tell me how the line discipline framework helps us here?

Well,
1. in hciattach I would have set the N_HCI line discipline immediately after opening the uart device.
2. in the N_HCI line discipline, I would have implemented write() so userspace command write()s would still write through to the uart device.
3. either,
a. I would have added a new line discipline method for pre-termios changes, or b. I would have caught set termios ioctls in the N_HCI line discipline ioctl.

and handled sending target line speed change commands there. Userspace would simply change the line speed.

Userspace firmware loads are straightforward.

However, it seems that the new intel approach (tied in with the protocol) is trickier to accommodate, but I haven't spent a lot of time trying to follow that setup.

I'm not sure I understand why the firmware load is tied to the protocol; can the intel part load a different firmware that implements a different protocol?

Quoted 1 line

Keep in mind you have to tell the controller via HCI commands over the old baud rate to change its rate. And then change it on the TTY to match what the controller does on its side.

I'm conscious of the limitations of using tty/serial in the current manner.

A fun project for one of these vendors would be to split the serial core into a separate tty driver and an abstraction layer that would allow other kernel subsystems to enslave the port.

Hi Peter,

Quoted 70 lines

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

Quoted 4 lines

The line discipline is always notified of line rate changes via the set_termios() method, so if you add that to the hci_ldisc, you'll be able to keep the BT device in sync with your vendor-specific commands.

If I'm not wrong, line discipline is notified after the tty termios change. So, it's too late to send the HCI change speed command. Moreover, user space is not aware of the device behavior, the driver should be the only one to manage the device and its serial link.
Here, user space is just a bootstrap which hands over to the driver.

Quoted 7 lines

Quoted 5 lines

You realize that you're telling me that userspace is unaware of this in a thread that begins with a proposed userspace ioctl change to the line discipline? An ioctl change that adds a new ioctl to set the speed in a line discipline? For which there are already lots of ioctls to set the line speed?

actually the problem is a little bit different here.

Thanks for taking the time to outline the operation of hciattach. It's been a while since I was involved in the BT userspace code so I hadn't realized how much hciattach had morphed.

Quoted 27 lines

Every Bluetooth UART chip has its default baudrate. Mostly that is 115k, but some actually have others.

The general idea was that userspace deals with opening the TTY, send the HCI commands to change the rate, then change the termios setting, attach the ldisc and then hand it to the kernel.

This is however not how at least two major Bluetooth controllers work when you have to do firmware loading. When they boot into the firmware, then baudrate falls back to default and you have to do that all over again.

So what this ioctl will just tell the kernel about is the desired baudrate that it wants to be set whenever possible. The whenever possible is important here since depending on the manufacturer that might be a total different times. And with firmware download procedures being fully vendor specific this will vary.

Quoted 2 lines

Setting aside the problem with the firmware load, I see no reason why this can't be done within the existing line discipline framework.

So far everything has been done in hciattach tool. And it is so ugly and does not even work properly for any complex UART protocols like BCSP or 3-Wire. There is a patch for a modified 3-Wire setup from one vendor that made my eyes bleed. So what you really want is having the kernel take control over HCI from the beginning. Otherwise this goes out of control and handling exceptions like hardware error event becomes impossible.

In summary what we have to do these days is this:

- Send Reset
- Send a few basic HCI commands
- Send baud rate change command
- Change actual baud rate on the TTY
- Download firmware
- Send Reset to boot the firmware
- Change actual baud rate back to default
- Send baud rate change command
- Change actual baud rate
- Send Reset
- Standard init sequence

Quoted 1 line

Now tell me how the line discipline framework helps us here?

Well,
1. in hciattach I would have set the N_HCI line discipline immediately after opening the uart device.
2. in the N_HCI line discipline, I would have implemented write() so userspace command write()s would still write through to the uart device.
3. either,
a. I would have added a new line discipline method for pre-termios changes, or b. I would have caught set termios ioctls in the N_HCI line discipline ioctl.

and handled sending target line speed change commands there. Userspace would simply change the line speed.

Userspace firmware loads are straightforward.

However, it seems that the new intel approach (tied in with the protocol) is trickier to accommodate, but I haven't spent a lot of time trying to follow that setup.

Quoted 2 lines

I'm not sure I understand why the firmware load is tied to the protocol; can the intel part load a different firmware that implements a different protocol?

the firmware loading procedure for USB and UART is actually the same. That applies to Broadcom and Intel controllers for sure and most likely also for other vendors. One goal is to share the firmware loading code between USB and UART based SKUs of these controllers. Where are not there yet, but that is where this is heading. So while we can hack around certain things in userspace, we really do not want to do that since it just means duplicated code.

Implementing the write() is a bad idea in my opinion. That is like injecting HCI commands behind your back. All sorts of messy things can happen if you allow that. And that is why this is actually not even implemented at the moment. Remember that HCI protocol comes with flow control for their commands and events. Injecting something without taking care of the command flow control means you are doing evil stuff. The firmware loading is using HCI commands and thus it should go through the high-level Bluetooth core that understands HCI commands.

Our idea is essentially that we set the N_HCI line discipline and then set the vendor protocol. That is it. From that point on it is the kernels problem to handle the bringup of the Bluetooth device. Including firmware download, address configuration and so on. Before setting N_HCI we would correctly configure the default baudrate and all other needed settings. However that is as much as we do from userspace.

For this to work, we need to be able to tell the TTY to change its baudrate. And this has to happen at least 3 times now. Higher baudrate before downloading the firmware, back to default baudrate after that, switch to higher baudrate and only then we are at normal operation.

So how does intercepting the termios ioctl will help us in this regard. The kernel needs to control the baudrate change and not userspace.

Quoted 7 lines

Quoted 1 line

Keep in mind you have to tell the controller via HCI commands over the old baud rate to change its rate. And then change it on the TTY to match what the controller does on its side.

I'm conscious of the limitations of using tty/serial in the current manner.

A fun project for one of these vendors would be to split the serial core into a separate tty driver and an abstraction layer that would allow other kernel subsystems to enslave the port.

They are proposing TTY slave devices and maybe that will help eventually, but finding agreement on that seems hard as well. And that is also mostly for handling the extra GPIO configuration for power up/down the Bluetooth chip behind the TTY and not the TTY itself.

I would love if we could just expose these TTYs as bi-directional pipes on a "serial bus" and just have a driver attach to it instead of going through a line discipline.

Quoted 73 lines

Quoted 72 lines

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

Quoted 4 lines

The line discipline is always notified of line rate changes via the set_termios() method, so if you add that to the hci_ldisc, you'll be able to keep the BT device in sync with your vendor-specific commands.

If I'm not wrong, line discipline is notified after the tty termios change. So, it's too late to send the HCI change speed command. Moreover, user space is not aware of the device behavior, the driver should be the only one to manage the device and its serial link.
Here, user space is just a bootstrap which hands over to the driver.

Quoted 7 lines

Quoted 5 lines

You realize that you're telling me that userspace is unaware of this in a thread that begins with a proposed userspace ioctl change to the line discipline? An ioctl change that adds a new ioctl to set the speed in a line discipline? For which there are already lots of ioctls to set the line speed?

actually the problem is a little bit different here.

Thanks for taking the time to outline the operation of hciattach. It's been a while since I was involved in the BT userspace code so I hadn't realized how much hciattach had morphed.

Quoted 27 lines

Every Bluetooth UART chip has its default baudrate. Mostly that is 115k, but some actually have others.

The general idea was that userspace deals with opening the TTY, send the HCI commands to change the rate, then change the termios setting, attach the ldisc and then hand it to the kernel.

This is however not how at least two major Bluetooth controllers work when you have to do firmware loading. When they boot into the firmware, then baudrate falls back to default and you have to do that all over again.

So what this ioctl will just tell the kernel about is the desired baudrate that it wants to be set whenever possible. The whenever possible is important here since depending on the manufacturer that might be a total different times. And with firmware download procedures being fully vendor specific this will vary.

Quoted 2 lines

Setting aside the problem with the firmware load, I see no reason why this can't be done within the existing line discipline framework.

So far everything has been done in hciattach tool. And it is so ugly and does not even work properly for any complex UART protocols like BCSP or 3-Wire. There is a patch for a modified 3-Wire setup from one vendor that made my eyes bleed. So what you really want is having the kernel take control over HCI from the beginning. Otherwise this goes out of control and handling exceptions like hardware error event becomes impossible.

In summary what we have to do these days is this:

- Send Reset
- Send a few basic HCI commands
- Send baud rate change command
- Change actual baud rate on the TTY
- Download firmware
- Send Reset to boot the firmware
- Change actual baud rate back to default
- Send baud rate change command
- Change actual baud rate
- Send Reset
- Standard init sequence

Quoted 1 line

Now tell me how the line discipline framework helps us here?

Well,
1. in hciattach I would have set the N_HCI line discipline immediately after opening the uart device.
2. in the N_HCI line discipline, I would have implemented write() so userspace command write()s would still write through to the uart device.
3. either,
a. I would have added a new line discipline method for pre-termios changes, or b. I would have caught set termios ioctls in the N_HCI line discipline ioctl.

and handled sending target line speed change commands there. Userspace would simply change the line speed.

Userspace firmware loads are straightforward.

However, it seems that the new intel approach (tied in with the protocol) is trickier to accommodate, but I haven't spent a lot of time trying to follow that setup.

I'm not sure I understand why the firmware load is tied to the protocol; can the intel part load a different firmware that implements a different protocol?

Quoted 3 lines

the firmware loading procedure for USB and UART is actually the same. That applies to Broadcom and Intel controllers for sure and most likely also for other vendors. One goal is to share the firmware loading code between USB and UART based SKUs of these controllers. Where are not there yet, but that is where this is heading. So while we can hack around certain things in userspace, we really do not want to do that since it just means duplicated code.

Implementing the write() is a bad idea in my opinion. That is like injecting HCI commands behind your back. All sorts of messy things can happen if you allow that. And that is why this is actually not even implemented at the moment. Remember that HCI protocol comes with flow control for their commands and events. Injecting something without taking care of the command flow control means you are doing evil stuff. The firmware loading is using HCI commands and thus it should go through the high-level Bluetooth core that understands HCI commands.

hciattach is doing lots of write(fd) right now; those raw serial writes would simply be going though N_HCI line discipline write() instead of the N_TTY line discipline write().

Quoted 4 lines

Our idea is essentially that we set the N_HCI line discipline and then set the vendor protocol. That is it. From that point on it is the kernels problem to handle the bringup of the Bluetooth device. Including firmware download, address configuration and so on. Before setting N_HCI we would correctly configure the default baudrate and all other needed settings. However that is as much as we do from userspace.

For this to work, we need to be able to tell the TTY to change its baudrate. And this has to happen at least 3 times now. Higher baudrate before downloading the firmware, back to default baudrate after that, switch to higher baudrate and only then we are at normal operation.

Quoted 1 line

So how does intercepting the termios ioctl will help us in this regard. The kernel needs to control the baudrate change and not userspace.

The tty is a userspace object with a userspace api, simple as that. That's why hciattach has to stay alive, because it has to own the tty file reference.

Anything directly to tty from kernel is a hack that is missing locks and bypassing state.

Quoted 9 lines

Quoted 7 lines

Quoted 1 line

Keep in mind you have to tell the controller via HCI commands over the old baud rate to change its rate. And then change it on the TTY to match what the controller does on its side.

I'm conscious of the limitations of using tty/serial in the current manner.

A fun project for one of these vendors would be to split the serial core into a separate tty driver and an abstraction layer that would allow other kernel subsystems to enslave the port.

They are proposing TTY slave devices and maybe that will help eventually, but finding agreement on that seems hard as well. And that is also mostly for handling the extra GPIO configuration for power up/down the Bluetooth chip behind the TTY and not the TTY itself.

That's different.

Quoted 1 line

I would love if we could just expose these TTYs as bi-directional pipes on a "serial bus" and just have a driver attach to it instead of going through a line discipline.

That's what I'm saying.

Ultimately, the stuff you want to implement on the BT side will need a native kernel interface where you own the device reference.

Hi Peter,

Quoted 75 lines

Quoted 75 lines

Quoted 64 lines

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

Quoted 4 lines

The line discipline is always notified of line rate changes via the set_termios() method, so if you add that to the hci_ldisc, you'll be able to keep the BT device in sync with your vendor-specific commands.

If I'm not wrong, line discipline is notified after the tty termios change. So, it's too late to send the HCI change speed command. Moreover, user space is not aware of the device behavior, the driver should be the only one to manage the device and its serial link.
Here, user space is just a bootstrap which hands over to the driver.

Quoted 7 lines

Quoted 5 lines

You realize that you're telling me that userspace is unaware of this in a thread that begins with a proposed userspace ioctl change to the line discipline? An ioctl change that adds a new ioctl to set the speed in a line discipline? For which there are already lots of ioctls to set the line speed?

actually the problem is a little bit different here.

Thanks for taking the time to outline the operation of hciattach. It's been a while since I was involved in the BT userspace code so I hadn't realized how much hciattach had morphed.

Quoted 27 lines

Every Bluetooth UART chip has its default baudrate. Mostly that is 115k, but some actually have others.

The general idea was that userspace deals with opening the TTY, send the HCI commands to change the rate, then change the termios setting, attach the ldisc and then hand it to the kernel.

This is however not how at least two major Bluetooth controllers work when you have to do firmware loading. When they boot into the firmware, then baudrate falls back to default and you have to do that all over again.

So what this ioctl will just tell the kernel about is the desired baudrate that it wants to be set whenever possible. The whenever possible is important here since depending on the manufacturer that might be a total different times. And with firmware download procedures being fully vendor specific this will vary.

Quoted 2 lines

Setting aside the problem with the firmware load, I see no reason why this can't be done within the existing line discipline framework.

So far everything has been done in hciattach tool. And it is so ugly and does not even work properly for any complex UART protocols like BCSP or 3-Wire. There is a patch for a modified 3-Wire setup from one vendor that made my eyes bleed. So what you really want is having the kernel take control over HCI from the beginning. Otherwise this goes out of control and handling exceptions like hardware error event becomes impossible.

In summary what we have to do these days is this:

- Send Reset
- Send a few basic HCI commands
- Send baud rate change command
- Change actual baud rate on the TTY
- Download firmware
- Send Reset to boot the firmware
- Change actual baud rate back to default
- Send baud rate change command
- Change actual baud rate
- Send Reset
- Standard init sequence

Quoted 1 line

Now tell me how the line discipline framework helps us here?

Well,
1. in hciattach I would have set the N_HCI line discipline immediately after opening the uart device.
2. in the N_HCI line discipline, I would have implemented write() so userspace command write()s would still write through to the uart device.
3. either,
a. I would have added a new line discipline method for pre-termios changes, or b. I would have caught set termios ioctls in the N_HCI line discipline ioctl.

and handled sending target line speed change commands there. Userspace would simply change the line speed.

Quoted 8 lines

Userspace firmware loads are straightforward.

However, it seems that the new intel approach (tied in with the protocol) is trickier to accommodate, but I haven't spent a lot of time trying to follow that setup.

I'm not sure I understand why the firmware load is tied to the protocol; can the intel part load a different firmware that implements a different protocol?

the firmware loading procedure for USB and UART is actually the same. That applies to Broadcom and Intel controllers for sure and most likely also for other vendors. One goal is to share the firmware loading code between USB and UART based SKUs of these controllers. Where are not there yet, but that is where this is heading. So while we can hack around certain things in userspace, we really do not want to do that since it just means duplicated code.

Quoted 4 lines

Quoted 1 line

Implementing the write() is a bad idea in my opinion. That is like injecting HCI commands behind your back. All sorts of messy things can happen if you allow that. And that is why this is actually not even implemented at the moment. Remember that HCI protocol comes with flow control for their commands and events. Injecting something without taking care of the command flow control means you are doing evil stuff. The firmware loading is using HCI commands and thus it should go through the high-level Bluetooth core that understands HCI commands.

hciattach is doing lots of write(fd) right now; those raw serial writes would simply be going though N_HCI line discipline write() instead of the N_TTY line discipline write().

and we want to get rid of these since it implements the HCI protocol layer. And it implements it pretty poorly. It is mainly write(), fingers-crossed, hope you get the right bytes back.

It works for dead simple protocols like H:5, but for BCSP, 3-Wire or the vendor specific power management, this is all not going to work out. The problem area arises when you have to recover from an error. All of this falls flat on its face.

Quoted 10 lines

Quoted 5 lines

Our idea is essentially that we set the N_HCI line discipline and then set the vendor protocol. That is it. From that point on it is the kernels problem to handle the bringup of the Bluetooth device. Including firmware download, address configuration and so on. Before setting N_HCI we would correctly configure the default baudrate and all other needed settings. However that is as much as we do from userspace.

For this to work, we need to be able to tell the TTY to change its baudrate. And this has to happen at least 3 times now. Higher baudrate before downloading the firmware, back to default baudrate after that, switch to higher baudrate and only then we are at normal operation.

So how does intercepting the termios ioctl will help us in this regard. The kernel needs to control the baudrate change and not userspace.

The tty is a userspace object with a userspace api, simple as that. That's why hciattach has to stay alive, because it has to own the tty file reference.

Quoted 2 lines

Anything directly to tty from kernel is a hack that is missing locks and bypassing state.

So what you are saying is that we want to have a callback mechanism back into hciattach. So when kernel needs a changed baudrate, it should tell hciattach, it will then change the baudrate, and tell the kernel when it did so.

I wonder if I could just repurpose the ldisc read/write/poll callback handlers to implement such a simple callback/notification protocol. That would actually make sense to me since the kernel wants to drive the TTY with the agreed upon HCI transport protocol from the point we attached N_HCI to it.

Quoted 12 lines

Quoted 9 lines

Quoted 7 lines

Quoted 1 line

Keep in mind you have to tell the controller via HCI commands over the old baud rate to change its rate. And then change it on the TTY to match what the controller does on its side.

I'm conscious of the limitations of using tty/serial in the current manner.

A fun project for one of these vendors would be to split the serial core into a separate tty driver and an abstraction layer that would allow other kernel subsystems to enslave the port.

They are proposing TTY slave devices and maybe that will help eventually, but finding agreement on that seems hard as well. And that is also mostly for handling the extra GPIO configuration for power up/down the Bluetooth chip behind the TTY and not the TTY itself.

That's different.

Quoted 6 lines

Quoted 1 line

I would love if we could just expose these TTYs as bi-directional pipes on a "serial bus" and just have a driver attach to it instead of going through a line discipline.

That's what I'm saying.

Ultimately, the stuff you want to implement on the BT side will need a native kernel interface where you own the device reference.

Even the HSU on our devices expose a TTY instead of an actual HSU bus where we could have be a HSU driver on it. If you know how we can get this changed, I am all ears.

The hci_uart driver and N_HCI line discipline will be still needed since there are actual development boards with USB/TTY converters attached to it. However all the embedded devices where it is all hardwired, proper exposed busses or platform devices would be a lot better.

Quoted 80 lines

Quoted 79 lines

Quoted 70 lines

Quoted 70 lines

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

Quoted 4 lines

The line discipline is always notified of line rate changes via the set_termios() method, so if you add that to the hci_ldisc, you'll be able to keep the BT device in sync with your vendor-specific commands.

If I'm not wrong, line discipline is notified after the tty termios change. So, it's too late to send the HCI change speed command. Moreover, user space is not aware of the device behavior, the driver should be the only one to manage the device and its serial link.
Here, user space is just a bootstrap which hands over to the driver.

Quoted 7 lines

Quoted 5 lines

You realize that you're telling me that userspace is unaware of this in a thread that begins with a proposed userspace ioctl change to the line discipline? An ioctl change that adds a new ioctl to set the speed in a line discipline? For which there are already lots of ioctls to set the line speed?

actually the problem is a little bit different here.

Thanks for taking the time to outline the operation of hciattach. It's been a while since I was involved in the BT userspace code so I hadn't realized how much hciattach had morphed.

Quoted 27 lines

Every Bluetooth UART chip has its default baudrate. Mostly that is 115k, but some actually have others.

The general idea was that userspace deals with opening the TTY, send the HCI commands to change the rate, then change the termios setting, attach the ldisc and then hand it to the kernel.

This is however not how at least two major Bluetooth controllers work when you have to do firmware loading. When they boot into the firmware, then baudrate falls back to default and you have to do that all over again.

So what this ioctl will just tell the kernel about is the desired baudrate that it wants to be set whenever possible. The whenever possible is important here since depending on the manufacturer that might be a total different times. And with firmware download procedures being fully vendor specific this will vary.

Quoted 2 lines

Setting aside the problem with the firmware load, I see no reason why this can't be done within the existing line discipline framework.

So far everything has been done in hciattach tool. And it is so ugly and does not even work properly for any complex UART protocols like BCSP or 3-Wire. There is a patch for a modified 3-Wire setup from one vendor that made my eyes bleed. So what you really want is having the kernel take control over HCI from the beginning. Otherwise this goes out of control and handling exceptions like hardware error event becomes impossible.

In summary what we have to do these days is this:

- Send Reset
- Send a few basic HCI commands
- Send baud rate change command
- Change actual baud rate on the TTY
- Download firmware
- Send Reset to boot the firmware
- Change actual baud rate back to default
- Send baud rate change command
- Change actual baud rate
- Send Reset
- Standard init sequence

Quoted 1 line

Now tell me how the line discipline framework helps us here?

Well,
1. in hciattach I would have set the N_HCI line discipline immediately after opening the uart device.
2. in the N_HCI line discipline, I would have implemented write() so userspace command write()s would still write through to the uart device.
3. either,
a. I would have added a new line discipline method for pre-termios changes, or b. I would have caught set termios ioctls in the N_HCI line discipline ioctl.

and handled sending target line speed change commands there. Userspace would simply change the line speed.

Userspace firmware loads are straightforward.

However, it seems that the new intel approach (tied in with the protocol) is trickier to accommodate, but I haven't spent a lot of time trying to follow that setup.

Quoted 6 lines

Quoted 2 lines

I'm not sure I understand why the firmware load is tied to the protocol; can the intel part load a different firmware that implements a different protocol?

the firmware loading procedure for USB and UART is actually the same. That applies to Broadcom and Intel controllers for sure and most likely also for other vendors. One goal is to share the firmware loading code between USB and UART based SKUs of these controllers. Where are not there yet, but that is where this is heading. So while we can hack around certain things in userspace, we really do not want to do that since it just means duplicated code.

Implementing the write() is a bad idea in my opinion. That is like injecting HCI commands behind your back. All sorts of messy things can happen if you allow that. And that is why this is actually not even implemented at the moment. Remember that HCI protocol comes with flow control for their commands and events. Injecting something without taking care of the command flow control means you are doing evil stuff. The firmware loading is using HCI commands and thus it should go through the high-level Bluetooth core that understands HCI commands.

hciattach is doing lots of write(fd) right now; those raw serial writes would simply be going though N_HCI line discipline write() instead of the N_TTY line discipline write().

Quoted 3 lines

and we want to get rid of these since it implements the HCI protocol layer. And it implements it pretty poorly. It is mainly write(), fingers-crossed, hope you get the right bytes back.

It works for dead simple protocols like H:5, but for BCSP, 3-Wire or the vendor specific power management, this is all not going to work out. The problem area arises when you have to recover from an error. All of this falls flat on its face.

Ok, what code should I look at to understand your in-kernel interface requirements?

Is it at least from process context?

Quoted 15 lines

Quoted 4 lines

Quoted 4 lines

Our idea is essentially that we set the N_HCI line discipline and then set the vendor protocol. That is it. From that point on it is the kernels problem to handle the bringup of the Bluetooth device. Including firmware download, address configuration and so on. Before setting N_HCI we would correctly configure the default baudrate and all other needed settings. However that is as much as we do from userspace.

For this to work, we need to be able to tell the TTY to change its baudrate. And this has to happen at least 3 times now. Higher baudrate before downloading the firmware, back to default baudrate after that, switch to higher baudrate and only then we are at normal operation.

Quoted 8 lines

Quoted 1 line

So how does intercepting the termios ioctl will help us in this regard. The kernel needs to control the baudrate change and not userspace.

The tty is a userspace object with a userspace api, simple as that. That's why hciattach has to stay alive, because it has to own the tty file reference.

Anything directly to tty from kernel is a hack that is missing locks and bypassing state.

So what you are saying is that we want to have a callback mechanism back into hciattach. So when kernel needs a changed baudrate, it should tell hciattach, it will then change the baudrate, and tell the kernel when it did so.

Quoted 1 line

I wonder if I could just repurpose the ldisc read/write/poll callback handlers to implement such a simple callback/notification protocol. That would actually make sense to me since the kernel wants to drive the TTY with the agreed upon HCI transport protocol from the point we attached N_HCI to it.

Any of the above would be so ugly.
Until I can work up something better, the lesser evil is using tty_set_termios().

Please cc me for patches using tty_set_termios() or other tty workarounds.

Quoted 12 lines

Quoted 12 lines

Quoted 9 lines

Quoted 7 lines

Quoted 1 line

Keep in mind you have to tell the controller via HCI commands over the old baud rate to change its rate. And then change it on the TTY to match what the controller does on its side.

I'm conscious of the limitations of using tty/serial in the current manner.

A fun project for one of these vendors would be to split the serial core into a separate tty driver and an abstraction layer that would allow other kernel subsystems to enslave the port.

They are proposing TTY slave devices and maybe that will help eventually, but finding agreement on that seems hard as well. And that is also mostly for handling the extra GPIO configuration for power up/down the Bluetooth chip behind the TTY and not the TTY itself.

That's different.

Quoted 8 lines

Quoted 6 lines

Quoted 1 line

I would love if we could just expose these TTYs as bi-directional pipes on a "serial bus" and just have a driver attach to it instead of going through a line discipline.

That's what I'm saying.

Ultimately, the stuff you want to implement on the BT side will need a native kernel interface where you own the device reference.

Even the HSU on our devices expose a TTY instead of an actual HSU bus where we could have be a HSU driver on it. If you know how we can get this changed, I am all ears.

There's no magic bullet here; it would be a lot of work.

Quoted 1 line

The hci_uart driver and N_HCI line discipline will be still needed since there are actual development boards with USB/TTY converters attached to it. However all the embedded devices where it is all hardwired, proper exposed busses or platform devices would be a lot better.

But never both simultaneously on the same device, right?
Because the idea would be that the UART is exclusive and wholly owned, without a userspace dev node.

Hi Peter,

Quoted 84 lines

Quoted 83 lines

Quoted 73 lines

Quoted 73 lines

Quoted 72 lines

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

Quoted 4 lines

The line discipline is always notified of line rate changes via the set_termios() method, so if you add that to the hci_ldisc, you'll be able to keep the BT device in sync with your vendor-specific commands.

If I'm not wrong, line discipline is notified after the tty termios change. So, it's too late to send the HCI change speed command. Moreover, user space is not aware of the device behavior, the driver should be the only one to manage the device and its serial link.
Here, user space is just a bootstrap which hands over to the driver.

Quoted 7 lines

Quoted 5 lines

You realize that you're telling me that userspace is unaware of this in a thread that begins with a proposed userspace ioctl change to the line discipline? An ioctl change that adds a new ioctl to set the speed in a line discipline? For which there are already lots of ioctls to set the line speed?

actually the problem is a little bit different here.

Thanks for taking the time to outline the operation of hciattach. It's been a while since I was involved in the BT userspace code so I hadn't realized how much hciattach had morphed.

Quoted 27 lines

Every Bluetooth UART chip has its default baudrate. Mostly that is 115k, but some actually have others.

The general idea was that userspace deals with opening the TTY, send the HCI commands to change the rate, then change the termios setting, attach the ldisc and then hand it to the kernel.

This is however not how at least two major Bluetooth controllers work when you have to do firmware loading. When they boot into the firmware, then baudrate falls back to default and you have to do that all over again.

So what this ioctl will just tell the kernel about is the desired baudrate that it wants to be set whenever possible. The whenever possible is important here since depending on the manufacturer that might be a total different times. And with firmware download procedures being fully vendor specific this will vary.

Quoted 2 lines

Setting aside the problem with the firmware load, I see no reason why this can't be done within the existing line discipline framework.

So far everything has been done in hciattach tool. And it is so ugly and does not even work properly for any complex UART protocols like BCSP or 3-Wire. There is a patch for a modified 3-Wire setup from one vendor that made my eyes bleed. So what you really want is having the kernel take control over HCI from the beginning. Otherwise this goes out of control and handling exceptions like hardware error event becomes impossible.

In summary what we have to do these days is this:

- Send Reset
- Send a few basic HCI commands
- Send baud rate change command
- Change actual baud rate on the TTY
- Download firmware
- Send Reset to boot the firmware
- Change actual baud rate back to default
- Send baud rate change command
- Change actual baud rate
- Send Reset
- Standard init sequence

Quoted 1 line

Now tell me how the line discipline framework helps us here?

Well,
1. in hciattach I would have set the N_HCI line discipline immediately after opening the uart device.
2. in the N_HCI line discipline, I would have implemented write() so userspace command write()s would still write through to the uart device.
3. either,
a. I would have added a new line discipline method for pre-termios changes, or b. I would have caught set termios ioctls in the N_HCI line discipline ioctl.

and handled sending target line speed change commands there. Userspace would simply change the line speed.

Userspace firmware loads are straightforward.

However, it seems that the new intel approach (tied in with the protocol) is trickier to accommodate, but I haven't spent a lot of time trying to follow that setup.

I'm not sure I understand why the firmware load is tied to the protocol; can the intel part load a different firmware that implements a different protocol?

Quoted 6 lines

Quoted 3 lines

the firmware loading procedure for USB and UART is actually the same. That applies to Broadcom and Intel controllers for sure and most likely also for other vendors. One goal is to share the firmware loading code between USB and UART based SKUs of these controllers. Where are not there yet, but that is where this is heading. So while we can hack around certain things in userspace, we really do not want to do that since it just means duplicated code.

Implementing the write() is a bad idea in my opinion. That is like injecting HCI commands behind your back. All sorts of messy things can happen if you allow that. And that is why this is actually not even implemented at the moment. Remember that HCI protocol comes with flow control for their commands and events. Injecting something without taking care of the command flow control means you are doing evil stuff. The firmware loading is using HCI commands and thus it should go through the high-level Bluetooth core that understands HCI commands.

hciattach is doing lots of write(fd) right now; those raw serial writes would simply be going though N_HCI line discipline write() instead of the N_TTY line discipline write().

and we want to get rid of these since it implements the HCI protocol layer. And it implements it pretty poorly. It is mainly write(), fingers-crossed, hope you get the right bytes back.

It works for dead simple protocols like H:5, but for BCSP, 3-Wire or the vendor specific power management, this is all not going to work out. The problem area arises when you have to recover from an error. All of this falls flat on its face.

Quoted 2 lines

Ok, what code should I look at to understand your in-kernel interface requirements?

the chips are using mainly HCI vendor commands with in some cases extra packet types for power management. The kernel side is mainly that we have a special setup routing that drivers can use to program the controller. However the driver uses HCI commands via __hci_cmd_sync(). Good example from bluetooth-next might be drivers/bluetooth/btbcm.c since there we already split out the common pieces. Keep in mind that the HCI commands are in most cases the same no matter what the transport underneath is. UART just needs an extra baud rate change.

Quoted 1 line

Is it at least from process context?

The setup callback in executed from a workqueue.

Quoted 17 lines

Quoted 16 lines

Quoted 4 lines

Quoted 4 lines

Our idea is essentially that we set the N_HCI line discipline and then set the vendor protocol. That is it. From that point on it is the kernels problem to handle the bringup of the Bluetooth device. Including firmware download, address configuration and so on. Before setting N_HCI we would correctly configure the default baudrate and all other needed settings. However that is as much as we do from userspace.

For this to work, we need to be able to tell the TTY to change its baudrate. And this has to happen at least 3 times now. Higher baudrate before downloading the firmware, back to default baudrate after that, switch to higher baudrate and only then we are at normal operation.

Quoted 8 lines

Quoted 1 line

So how does intercepting the termios ioctl will help us in this regard. The kernel needs to control the baudrate change and not userspace.

The tty is a userspace object with a userspace api, simple as that. That's why hciattach has to stay alive, because it has to own the tty file reference.

Anything directly to tty from kernel is a hack that is missing locks and bypassing state.

So what you are saying is that we want to have a callback mechanism back into hciattach. So when kernel needs a changed baudrate, it should tell hciattach, it will then change the baudrate, and tell the kernel when it did so.

I wonder if I could just repurpose the ldisc read/write/poll callback handlers to implement such a simple callback/notification protocol. That would actually make sense to me since the kernel wants to drive the TTY with the agreed upon HCI transport protocol from the point we attached N_HCI to it.

Quoted 5 lines

Any of the above would be so ugly.
Until I can work up something better, the lesser evil is using tty_set_termios().

Please cc me for patches using tty_set_termios() or other tty workarounds.

I was proposing to use tty->ops->set_termios. Is there any benefit from calling the native ops compared to using the not yet exposed wrapper?

Quoted 23 lines

Quoted 12 lines

Quoted 12 lines

Quoted 9 lines

Quoted 7 lines

Quoted 1 line

Keep in mind you have to tell the controller via HCI commands over the old baud rate to change its rate. And then change it on the TTY to match what the controller does on its side.

I'm conscious of the limitations of using tty/serial in the current manner.

A fun project for one of these vendors would be to split the serial core into a separate tty driver and an abstraction layer that would allow other kernel subsystems to enslave the port.

They are proposing TTY slave devices and maybe that will help eventually, but finding agreement on that seems hard as well. And that is also mostly for handling the extra GPIO configuration for power up/down the Bluetooth chip behind the TTY and not the TTY itself.

That's different.

Quoted 8 lines

Quoted 6 lines

Quoted 1 line

I would love if we could just expose these TTYs as bi-directional pipes on a "serial bus" and just have a driver attach to it instead of going through a line discipline.

That's what I'm saying.

Ultimately, the stuff you want to implement on the BT side will need a native kernel interface where you own the device reference.

Even the HSU on our devices expose a TTY instead of an actual HSU bus where we could have be a HSU driver on it. If you know how we can get this changed, I am all ears.

There's no magic bullet here; it would be a lot of work.

Quoted 5 lines

Quoted 1 line

The hci_uart driver and N_HCI line discipline will be still needed since there are actual development boards with USB/TTY converters attached to it. However all the embedded devices where it is all hardwired, proper exposed busses or platform devices would be a lot better.

But never both simultaneously on the same device, right?
Because the idea would be that the UART is exclusive and wholly owned, without a userspace dev node.

Yes. It is exclusive access to the device. No userspace device node needed.

So if we could just even set the line discipline from the kernel similar to attaching a driver to a device, then that would be something I would be interested as well.

Quoted 84 lines

Quoted 84 lines

Quoted 83 lines

Quoted 73 lines

Quoted 73 lines

Quoted 72 lines

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

Quoted 4 lines

The line discipline is always notified of line rate changes via the set_termios() method, so if you add that to the hci_ldisc, you'll be able to keep the BT device in sync with your vendor-specific commands.

If I'm not wrong, line discipline is notified after the tty termios change. So, it's too late to send the HCI change speed command. Moreover, user space is not aware of the device behavior, the driver should be the only one to manage the device and its serial link.
Here, user space is just a bootstrap which hands over to the driver.

Quoted 7 lines

Quoted 5 lines

You realize that you're telling me that userspace is unaware of this in a thread that begins with a proposed userspace ioctl change to the line discipline? An ioctl change that adds a new ioctl to set the speed in a line discipline? For which there are already lots of ioctls to set the line speed?

actually the problem is a little bit different here.

Thanks for taking the time to outline the operation of hciattach. It's been a while since I was involved in the BT userspace code so I hadn't realized how much hciattach had morphed.

Quoted 27 lines

Every Bluetooth UART chip has its default baudrate. Mostly that is 115k, but some actually have others.

The general idea was that userspace deals with opening the TTY, send the HCI commands to change the rate, then change the termios setting, attach the ldisc and then hand it to the kernel.

This is however not how at least two major Bluetooth controllers work when you have to do firmware loading. When they boot into the firmware, then baudrate falls back to default and you have to do that all over again.

So what this ioctl will just tell the kernel about is the desired baudrate that it wants to be set whenever possible. The whenever possible is important here since depending on the manufacturer that might be a total different times. And with firmware download procedures being fully vendor specific this will vary.

Quoted 2 lines

Setting aside the problem with the firmware load, I see no reason why this can't be done within the existing line discipline framework.

So far everything has been done in hciattach tool. And it is so ugly and does not even work properly for any complex UART protocols like BCSP or 3-Wire. There is a patch for a modified 3-Wire setup from one vendor that made my eyes bleed. So what you really want is having the kernel take control over HCI from the beginning. Otherwise this goes out of control and handling exceptions like hardware error event becomes impossible.

In summary what we have to do these days is this:

- Send Reset
- Send a few basic HCI commands
- Send baud rate change command
- Change actual baud rate on the TTY
- Download firmware
- Send Reset to boot the firmware
- Change actual baud rate back to default
- Send baud rate change command
- Change actual baud rate
- Send Reset
- Standard init sequence

Quoted 1 line

Now tell me how the line discipline framework helps us here?

Well,
1. in hciattach I would have set the N_HCI line discipline immediately after opening the uart device.
2. in the N_HCI line discipline, I would have implemented write() so userspace command write()s would still write through to the uart device.
3. either,
a. I would have added a new line discipline method for pre-termios changes, or b. I would have caught set termios ioctls in the N_HCI line discipline ioctl.

and handled sending target line speed change commands there. Userspace would simply change the line speed.

Userspace firmware loads are straightforward.

However, it seems that the new intel approach (tied in with the protocol) is trickier to accommodate, but I haven't spent a lot of time trying to follow that setup.

I'm not sure I understand why the firmware load is tied to the protocol; can the intel part load a different firmware that implements a different protocol?

Quoted 6 lines

Quoted 3 lines

the firmware loading procedure for USB and UART is actually the same. That applies to Broadcom and Intel controllers for sure and most likely also for other vendors. One goal is to share the firmware loading code between USB and UART based SKUs of these controllers. Where are not there yet, but that is where this is heading. So while we can hack around certain things in userspace, we really do not want to do that since it just means duplicated code.

Implementing the write() is a bad idea in my opinion. That is like injecting HCI commands behind your back. All sorts of messy things can happen if you allow that. And that is why this is actually not even implemented at the moment. Remember that HCI protocol comes with flow control for their commands and events. Injecting something without taking care of the command flow control means you are doing evil stuff. The firmware loading is using HCI commands and thus it should go through the high-level Bluetooth core that understands HCI commands.

hciattach is doing lots of write(fd) right now; those raw serial writes would simply be going though N_HCI line discipline write() instead of the N_TTY line discipline write().

and we want to get rid of these since it implements the HCI protocol layer. And it implements it pretty poorly. It is mainly write(), fingers-crossed, hope you get the right bytes back.

It works for dead simple protocols like H:5, but for BCSP, 3-Wire or the vendor specific power management, this is all not going to work out. The problem area arises when you have to recover from an error. All of this falls flat on its face.

Quoted 8 lines

Quoted 2 lines

Ok, what code should I look at to understand your in-kernel interface requirements?

the chips are using mainly HCI vendor commands with in some cases extra packet types for power management. The kernel side is mainly that we have a special setup routing that drivers can use to program the controller. However the driver uses HCI commands via __hci_cmd_sync(). Good example from bluetooth-next might be drivers/bluetooth/btbcm.c since there we already split out the common pieces. Keep in mind that the HCI commands are in most cases the same no matter what the transport underneath is. UART just needs an extra baud rate change.

Quoted 1 line

Is it at least from process context?

The setup callback in executed from a workqueue.

Ok.

Quoted 17 lines

Quoted 17 lines

Quoted 16 lines

Quoted 4 lines

Quoted 4 lines

Our idea is essentially that we set the N_HCI line discipline and then set the vendor protocol. That is it. From that point on it is the kernels problem to handle the bringup of the Bluetooth device. Including firmware download, address configuration and so on. Before setting N_HCI we would correctly configure the default baudrate and all other needed settings. However that is as much as we do from userspace.

For this to work, we need to be able to tell the TTY to change its baudrate. And this has to happen at least 3 times now. Higher baudrate before downloading the firmware, back to default baudrate after that, switch to higher baudrate and only then we are at normal operation.

Quoted 8 lines

Quoted 1 line

So how does intercepting the termios ioctl will help us in this regard. The kernel needs to control the baudrate change and not userspace.

The tty is a userspace object with a userspace api, simple as that. That's why hciattach has to stay alive, because it has to own the tty file reference.

Anything directly to tty from kernel is a hack that is missing locks and bypassing state.

So what you are saying is that we want to have a callback mechanism back into hciattach. So when kernel needs a changed baudrate, it should tell hciattach, it will then change the baudrate, and tell the kernel when it did so.

I wonder if I could just repurpose the ldisc read/write/poll callback handlers to implement such a simple callback/notification protocol. That would actually make sense to me since the kernel wants to drive the TTY with the agreed upon HCI transport protocol from the point we attached N_HCI to it.

Quoted 7 lines

Quoted 5 lines

Any of the above would be so ugly.
Until I can work up something better, the lesser evil is using tty_set_termios().

Please cc me for patches using tty_set_termios() or other tty workarounds.

I was proposing to use tty->ops->set_termios. Is there any benefit from calling the native ops compared to using the not yet exposed wrapper?

tty_set_termios() performs the termios swap before calling the driver; and grabs the proper lock.

As it is, the wrapper fn that calls tty_set_termios() will want to perform a tty_wait_until_sent(tty, 0) before tty_set_termios(); otherwise, hci command bytes could still be in the xmit buffer when the line settings are changed by the UART driver's set_termios(). [That doesn't happen automagically because there's times when the line settings are commanded to change without waiting.]

Quoted 23 lines

Quoted 23 lines

Quoted 12 lines

Quoted 12 lines

Quoted 9 lines

Quoted 7 lines

Quoted 1 line

Keep in mind you have to tell the controller via HCI commands over the old baud rate to change its rate. And then change it on the TTY to match what the controller does on its side.

I'm conscious of the limitations of using tty/serial in the current manner.

A fun project for one of these vendors would be to split the serial core into a separate tty driver and an abstraction layer that would allow other kernel subsystems to enslave the port.

They are proposing TTY slave devices and maybe that will help eventually, but finding agreement on that seems hard as well. And that is also mostly for handling the extra GPIO configuration for power up/down the Bluetooth chip behind the TTY and not the TTY itself.

That's different.

Quoted 8 lines

Quoted 6 lines

Quoted 1 line

I would love if we could just expose these TTYs as bi-directional pipes on a "serial bus" and just have a driver attach to it instead of going through a line discipline.

That's what I'm saying.

Ultimately, the stuff you want to implement on the BT side will need a native kernel interface where you own the device reference.

Even the HSU on our devices expose a TTY instead of an actual HSU bus where we could have be a HSU driver on it. If you know how we can get this changed, I am all ears.

There's no magic bullet here; it would be a lot of work.

Quoted 7 lines

Quoted 5 lines

Quoted 1 line

The hci_uart driver and N_HCI line discipline will be still needed since there are actual development boards with USB/TTY converters attached to it. However all the embedded devices where it is all hardwired, proper exposed busses or platform devices would be a lot better.

But never both simultaneously on the same device, right?
Because the idea would be that the UART is exclusive and wholly owned, without a userspace dev node.

Yes. It is exclusive access to the device. No userspace device node needed.

Ok.

Quoted 1 line

So if we could just even set the line discipline from the kernel similar to attaching a driver to a device, then that would be something I would be interested as well.

That's probably doable right now; most of the heavy lifting of setting the line discipline is in a single function. If this were being done from workqueue, it'd have to be an unbounded queue because of the implicit module load of the line discipline.

This is needed by Bluetooth hci_uart module to be able to change speed of Bluetooth controller and local UART.

Signed-off-by Frederic Danis <[email protected]>

---
drivers/tty/tty_ioctl.c | 3 ++-
include/linux/tty.h | 1 +

2 files changed, 3 insertions(+), 1 deletion(-)

drivers/tty/tty_ioctl.c +1 -1

index 632fc81..8e53fe4 100644
@@ -536,7 +536,7 @@ EXPORT_SYMBOL(tty_termios_hw_change);
 *	Locking: termios_rwsem
 */
 
-static int tty_set_termios(struct tty_struct *tty, struct ktermios *new_termios)
+int tty_set_termios(struct tty_struct *tty, struct ktermios *new_termios)
 {
 	struct ktermios old_termios;
 	struct tty_ldisc *ld;