Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1646319 > unrolled thread

[RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth

Back to article view | Back to linux.kernel

Contents

  [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-05-21 12:50 +0200
    Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module  and w2cbw003 wifi/bluetooth Rob Herring <robh+dt@kernel.org> - 2017-05-23 04:30 +0200
      Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-05-23 07:50 +0200
        Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module  and w2cbw003 wifi/bluetooth Rob Herring <robh+dt@kernel.org> - 2017-05-23 14:30 +0200
          Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-05-23 14:50 +0200
            Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module  and w2cbw003 wifi/bluetooth Rob Herring <robh+dt@kernel.org> - 2017-05-23 15:20 +0200
              Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth "H. Nikolaus Schaller" <hns@goldelico.com> - 2017-05-23 15:50 +0200

#1646319 — [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth

Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.

The approach is to write a "man in the middle" driver which is on one side a serdev client
which directly controls the UART where the device is connected to and on the other side
presents a new tty port so that user-space software can talk to the chips as if they would
directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.

For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
etc.).

Here is the result of our first hack which is working as a demo on GTA04 devices (and the
w2cbw driver can also be used to control a GTA04 variant with WL1837).

Since it is just a demo hack, the code is not yet cleaned up, nor does it completely pass
check-patch, nor follows 100% the coding styles. And certainly has some bugs.

The most significant issue is that calling tty_port_register_device() inside of the
serdev probe() function makes the serdev probe() function to be entered a second
time. This does not lead to big problems since we currently have minor = 0
and this makes the second call assume the device is not available.

But we have no idea why this happens and how it can be prevented.

Another observation is that the man-in-the-middle approach means copying
and double buffering the data. This seems to add some delay, especially if
we run the w2cbw003 bluetooth interface with its max. speed of 3 Mbit/s.

So this is still work in progress and more a demo than quite some work away
ffrom upstreaming. Therefore, we are asking for comments if the general
direction is ok and a polished driver would be acceptable. And if there are
solutions for the duplicate probe() and double-buffering delays.


H. Nikolaus Schaller (3):
  DTS: gta04: add serdev nodes for w2sg00x4, w2cbw etc.
  misc: Add w2sg0004 (gps receiver) power control driver
  misc: Add w2cbw003 (wifi/bluetooth) power control driver

 .../devicetree/bindings/misc/wi2wi,w2sg0004.txt    |  20 +
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 arch/arm/boot/dts/omap3-gta04.dtsi                 |  18 +
 drivers/misc/Kconfig                               |  23 +
 drivers/misc/Makefile                              |   2 +
 drivers/misc/w2cbw003-bluetooth.c                  | 390 +++++++++++++
 drivers/misc/w2sg0004.c                            | 646 +++++++++++++++++++++
 include/linux/w2sg0004.h                           |  27 +
 8 files changed, 1127 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/wi2wi,w2sg0004.txt
 create mode 100644 drivers/misc/w2cbw003-bluetooth.c
 create mode 100644 drivers/misc/w2sg0004.c
 create mode 100644 include/linux/w2sg0004.h

-- 
2.12.2

[toc] | [next] | [standalone]

#1647590 — Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth

On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>
> The approach is to write a "man in the middle" driver which is on one side a serdev client
> which directly controls the UART where the device is connected to and on the other side
> presents a new tty port so that user-space software can talk to the chips as if they would
> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>
> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
> etc.).

I understand from the prior discussion why you want to pass the data
thru for gps, but why do you need to do that for BT?

> Here is the result of our first hack which is working as a demo on GTA04 devices (and the
> w2cbw driver can also be used to control a GTA04 variant with WL1837).
>
> Since it is just a demo hack, the code is not yet cleaned up, nor does it completely pass
> check-patch, nor follows 100% the coding styles. And certainly has some bugs.
>
> The most significant issue is that calling tty_port_register_device() inside of the
> serdev probe() function makes the serdev probe() function to be entered a second
> time. This does not lead to big problems since we currently have minor = 0
> and this makes the second call assume the device is not available.
>
> But we have no idea why this happens and how it can be prevented.

Johan's fixes may help there, but it is intended to be temporary to
have a separate API for registering tty ports with or without serdev.

Rob

[toc] | [prev] | [next] | [standalone]

#1647652

Hi Rob,

> Am 23.05.2017 um 04:26 schrieb Rob Herring <robh+dt@kernel.org>:
> 
> On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
>> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>> 
>> The approach is to write a "man in the middle" driver which is on one side a serdev client
>> which directly controls the UART where the device is connected to and on the other side
>> presents a new tty port so that user-space software can talk to the chips as if they would
>> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
>> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>> 
>> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
>> etc.).
> 
> I understand from the prior discussion why you want to pass the data
> thru for gps, but why do you need to do that for BT?

Because we otherwise can't turn on power when /dev/ttyBT0 is opened and turn off when it
is closed. I.e. it should not be powered unless someone does a hciattach /dev/ttyBT0. And it
should be turned off by a killall hciattach.

Basically we would like to have a power control automatic like it exists for many other devices.

Since the BT chip is described as a serdev by DT, we see no other means than to pass data
through the serdev driver.

We had looked into the line discipline approach but it makes a lot of problems. The first one
is that registering a new system-wide ldesc number is required. Next we do not see how to make
a serdev driver (as it seems to be required by the DT) to register a different ldesc.

> 
>> Here is the result of our first hack which is working as a demo on GTA04 devices (and the
>> w2cbw driver can also be used to control a GTA04 variant with WL1837).
>> 
>> Since it is just a demo hack, the code is not yet cleaned up, nor does it completely pass
>> check-patch, nor follows 100% the coding styles. And certainly has some bugs.
>> 
>> The most significant issue is that calling tty_port_register_device() inside of the
>> serdev probe() function makes the serdev probe() function to be entered a second
>> time. This does not lead to big problems since we currently have minor = 0
>> and this makes the second call assume the device is not available.
>> 
>> But we have no idea why this happens and how it can be prevented.
> 
> Johan's fixes may help there, but it is intended to be temporary to
> have a separate API for registering tty ports with or without serdev.

Ah, would that mean something like a tty_port_register_device_without_serdev()?

Do you have a reference to his fixes?

BR and thanks,
Nikolaus

[toc] | [prev] | [next] | [standalone]

#1647984 — Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth

On Tue, May 23, 2017 at 12:43 AM, H. Nikolaus Schaller
<hns@goldelico.com> wrote:
> Hi Rob,
>
>> Am 23.05.2017 um 04:26 schrieb Rob Herring <robh+dt@kernel.org>:
>>
>> On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
>>> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>>>
>>> The approach is to write a "man in the middle" driver which is on one side a serdev client
>>> which directly controls the UART where the device is connected to and on the other side
>>> presents a new tty port so that user-space software can talk to the chips as if they would
>>> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
>>> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>>>
>>> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
>>> etc.).
>>
>> I understand from the prior discussion why you want to pass the data
>> thru for gps, but why do you need to do that for BT?
>
> Because we otherwise can't turn on power when /dev/ttyBT0 is opened and turn off when it
> is closed. I.e. it should not be powered unless someone does a hciattach /dev/ttyBT0. And it
> should be turned off by a killall hciattach.

Still, you can do power control within BT HCI drivers. You wouldn't be
limited to just open/close, but can handle suspend/resume as well.

> Basically we would like to have a power control automatic like it exists for many other devices.
>
> Since the BT chip is described as a serdev by DT, we see no other means than to pass data
> through the serdev driver.

We could have a blacklist if we need to have serdev not create a
device and create a tty device instead.

> We had looked into the line discipline approach but it makes a lot of problems. The first one
> is that registering a new system-wide ldesc number is required. Next we do not see how to make
> a serdev driver (as it seems to be required by the DT) to register a different ldesc.
>
>>
>>> Here is the result of our first hack which is working as a demo on GTA04 devices (and the
>>> w2cbw driver can also be used to control a GTA04 variant with WL1837).
>>>
>>> Since it is just a demo hack, the code is not yet cleaned up, nor does it completely pass
>>> check-patch, nor follows 100% the coding styles. And certainly has some bugs.
>>>
>>> The most significant issue is that calling tty_port_register_device() inside of the
>>> serdev probe() function makes the serdev probe() function to be entered a second
>>> time. This does not lead to big problems since we currently have minor = 0
>>> and this makes the second call assume the device is not available.
>>>
>>> But we have no idea why this happens and how it can be prevented.
>>
>> Johan's fixes may help there, but it is intended to be temporary to
>> have a separate API for registering tty ports with or without serdev.
>
> Ah, would that mean something like a tty_port_register_device_without_serdev()?

Yes, but other way around. The old function doesn't register with
serdev and there's a new function that will.

> Do you have a reference to his fixes?

They are in Greg's tty-linus branch if not Linus' tree now.

Rob

[toc] | [prev] | [next] | [standalone]

#1647989

Hi Rob,

> Am 23.05.2017 um 14:28 schrieb Rob Herring <robh+dt@kernel.org>:
> 
> On Tue, May 23, 2017 at 12:43 AM, H. Nikolaus Schaller
> <hns@goldelico.com> wrote:
>> Hi Rob,
>> 
>>> Am 23.05.2017 um 04:26 schrieb Rob Herring <robh+dt@kernel.org>:
>>> 
>>> On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>>> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
>>>> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>>>> 
>>>> The approach is to write a "man in the middle" driver which is on one side a serdev client
>>>> which directly controls the UART where the device is connected to and on the other side
>>>> presents a new tty port so that user-space software can talk to the chips as if they would
>>>> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
>>>> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>>>> 
>>>> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
>>>> etc.).
>>> 
>>> I understand from the prior discussion why you want to pass the data
>>> thru for gps, but why do you need to do that for BT?
>> 
>> Because we otherwise can't turn on power when /dev/ttyBT0 is opened and turn off when it
>> is closed. I.e. it should not be powered unless someone does a hciattach /dev/ttyBT0. And it
>> should be turned off by a killall hciattach.
> 
> Still, you can do power control within BT HCI drivers.

We do not use any driver for bluetooth. We just start hciattach on demand.
And afaik there is no plugin mechanism for adding power control to hciattach.

Or do you have a link to what you think about?

> You wouldn't be
> limited to just open/close, but can handle suspend/resume as well.

Well, it does not look as if we need more than open/close since suspend/resume
is already handled by the regulator driver. We just need to keep it powered off
if there is no user-space client.

> 
>> Basically we would like to have a power control automatic like it exists for many other devices.
>> 
>> Since the BT chip is described as a serdev by DT, we see no other means than to pass data
>> through the serdev driver.
> 
> We could have a blacklist if we need to have serdev not create a
> device and create a tty device instead.

Interesting idea.

> 
>> We had looked into the line discipline approach but it makes a lot of problems. The first one
>> is that registering a new system-wide ldesc number is required. Next we do not see how to make
>> a serdev driver (as it seems to be required by the DT) to register a different ldesc.
>> 
>>> 
>>>> Here is the result of our first hack which is working as a demo on GTA04 devices (and the
>>>> w2cbw driver can also be used to control a GTA04 variant with WL1837).
>>>> 
>>>> Since it is just a demo hack, the code is not yet cleaned up, nor does it completely pass
>>>> check-patch, nor follows 100% the coding styles. And certainly has some bugs.
>>>> 
>>>> The most significant issue is that calling tty_port_register_device() inside of the
>>>> serdev probe() function makes the serdev probe() function to be entered a second
>>>> time. This does not lead to big problems since we currently have minor = 0
>>>> and this makes the second call assume the device is not available.
>>>> 
>>>> But we have no idea why this happens and how it can be prevented.
>>> 
>>> Johan's fixes may help there, but it is intended to be temporary to
>>> have a separate API for registering tty ports with or without serdev.
>> 
>> Ah, would that mean something like a tty_port_register_device_without_serdev()?
> 
> Yes, but other way around. The old function doesn't register with
> serdev and there's a new function that will.

Ah, ok.

> 
>> Do you have a reference to his fixes?
> 
> They are in Greg's tty-linus branch if not Linus' tree now.

Have found them. Not yet in Linus' tree but likely in one of the next rc

Looks good. This would mean that this problem will simply go away soon.

BR and thanks,
Nikolaus

[toc] | [prev] | [next] | [standalone]

#1648030 — Re: [RFC 0/3] misc: new serdev based drivers for w2sg00x4 GPS module and w2cbw003 wifi/bluetooth

+Marcel

On Tue, May 23, 2017 at 7:48 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
> Hi Rob,
>
>> Am 23.05.2017 um 14:28 schrieb Rob Herring <robh+dt@kernel.org>:
>>
>> On Tue, May 23, 2017 at 12:43 AM, H. Nikolaus Schaller
>> <hns@goldelico.com> wrote:
>>> Hi Rob,
>>>
>>>> Am 23.05.2017 um 04:26 schrieb Rob Herring <robh+dt@kernel.org>:
>>>>
>>>> On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>>>> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
>>>>> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>>>>>
>>>>> The approach is to write a "man in the middle" driver which is on one side a serdev client
>>>>> which directly controls the UART where the device is connected to and on the other side
>>>>> presents a new tty port so that user-space software can talk to the chips as if they would
>>>>> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
>>>>> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>>>>>
>>>>> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
>>>>> etc.).
>>>>
>>>> I understand from the prior discussion why you want to pass the data
>>>> thru for gps, but why do you need to do that for BT?
>>>
>>> Because we otherwise can't turn on power when /dev/ttyBT0 is opened and turn off when it
>>> is closed. I.e. it should not be powered unless someone does a hciattach /dev/ttyBT0. And it
>>> should be turned off by a killall hciattach.
>>
>> Still, you can do power control within BT HCI drivers.
>
> We do not use any driver for bluetooth. We just start hciattach on demand.
> And afaik there is no plugin mechanism for adding power control to hciattach.

You don't need hciattach. All userspace has to do for kernel BT
drivers is "hciconfig hci0 up|down".

> Or do you have a link to what you think about?

Look at the nokia BT or TI (HCI_LL) BT drivers. Those both have f/w
downloading and some GPIO controls. Given that this module is based on
Marvell chipset, I'd expect you need to add serdev support to
hci_mrvl.c.

>> You wouldn't be
>> limited to just open/close, but can handle suspend/resume as well.
>
> Well, it does not look as if we need more than open/close since suspend/resume
> is already handled by the regulator driver. We just need to keep it powered off
> if there is no user-space client.

Okay.

Rob

[toc] | [prev] | [next] | [standalone]

#1648072

Hi Rob,

> Am 23.05.2017 um 15:10 schrieb Rob Herring <robh+dt@kernel.org>:
> 
> +Marcel

Good!

> 
> On Tue, May 23, 2017 at 7:48 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> Hi Rob,
>> 
>>> Am 23.05.2017 um 14:28 schrieb Rob Herring <robh+dt@kernel.org>:
>>> 
>>> On Tue, May 23, 2017 at 12:43 AM, H. Nikolaus Schaller
>>> <hns@goldelico.com> wrote:
>>>> Hi Rob,
>>>> 
>>>>> Am 23.05.2017 um 04:26 schrieb Rob Herring <robh+dt@kernel.org>:
>>>>> 
>>>>> On Sun, May 21, 2017 at 5:44 AM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>>>>> Since our proposed API was not acceptable and the new serdev API has arrived in 4.11 kernels,
>>>>>> we finally took the challenge to update the w2sg and w2cbw drivers to use the serdev API.
>>>>>> 
>>>>>> The approach is to write a "man in the middle" driver which is on one side a serdev client
>>>>>> which directly controls the UART where the device is connected to and on the other side
>>>>>> presents a new tty port so that user-space software can talk to the chips as if they would
>>>>>> directly talk to the UART of the SoC (e.g. ttyO1). This is similar to connecting to a remote
>>>>>> serial device e.g. through USB (ttyACM) or Bluetooth UART profiles.
>>>>>> 
>>>>>> For example gpsd or hciattach expect a /dev/tty they can control (flow control, baud rate
>>>>>> etc.).
>>>>> 
>>>>> I understand from the prior discussion why you want to pass the data
>>>>> thru for gps, but why do you need to do that for BT?
>>>> 
>>>> Because we otherwise can't turn on power when /dev/ttyBT0 is opened and turn off when it
>>>> is closed. I.e. it should not be powered unless someone does a hciattach /dev/ttyBT0. And it
>>>> should be turned off by a killall hciattach.
>>> 
>>> Still, you can do power control within BT HCI drivers.
>> 
>> We do not use any driver for bluetooth. We just start hciattach on demand.
>> And afaik there is no plugin mechanism for adding power control to hciattach.
> 
> You don't need hciattach. All userspace has to do for kernel BT
> drivers is "hciconfig hci0 up|down".

Hm. Well:

root@letux:~# hciconfig hci0 up
Can't get device info: No such device
root@letux:~#

I wonder how I can tell hciconfig about the UART port if not by running hciattach /dev/ttyBT0?

> 
>> Or do you have a link to what you think about?
> 
> Look at the nokia BT or TI (HCI_LL) BT drivers. Those both have f/w
> downloading and some GPIO controls. Given that this module is based on
> Marvell chipset, I'd expect you need to add serdev support to
> hci_mrvl.c.

The w2cb003 has a Marvell WiFi (libertas) but a CSR Bluetooth side.

It has built-in firmware and already talks serial HCI over simple UART right
after power-on. This is why our serdev driver has no firmware download.

> 
>>> You wouldn't be
>>> limited to just open/close, but can handle suspend/resume as well.
>> 
>> Well, it does not look as if we need more than open/close since suspend/resume
>> is already handled by the regulator driver. We just need to keep it powered off
>> if there is no user-space client.
> 
> Okay.
> 
> Rob

BR,
Nikolaus

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web