mbox series

[00/14] Add support for FM radio in hcill and kill TI_ST

Message ID 20181221011752.25627-1-sre@kernel.org (mailing list archive)
Headers show
Series Add support for FM radio in hcill and kill TI_ST | expand

Message

Sebastian Reichel Dec. 21, 2018, 1:17 a.m. UTC
Hi,

This moves all remaining users of the legacy TI_ST driver to hcill (patches
1-3). Then patches 4-7 convert wl128x-radio driver to a standard platform
device driver with support for multiple instances. Patch 7 will result in
(userless) TI_ST driver no longer supporting radio at runtime. Patch 8-11 do
some cleanups in the wl128x-radio driver. Finally patch 12 removes the TI_ST
specific parts from wl128x-radio and adds the required infrastructure to use it
with the serdev hcill driver instead. The remaining patches 13 and 14 remove
the old TI_ST code.

The new code has been tested on the Motorola Droid 4. For testing the audio
should be configured to route Ext to Speaker or Headphone. Then you need to
plug headphone, since its cable is used as antenna. For testing there is a
'radio' utility packages in Debian. When you start the utility you need to
specify a frequency, since initial get_frequency returns an error:

$ radio -f 100.0

Merry Christmas!

-- Sebastian

Sebastian Reichel (14):
  ARM: dts: LogicPD Torpedo: Add WiLink UART node
  ARM: dts: IGEP: Add WiLink UART node
  ARM: OMAP2+: pdata-quirks: drop TI_ST/KIM support
  media: wl128x-radio: remove module version
  media: wl128x-radio: remove global radio_disconnected
  media: wl128x-radio: remove global radio_dev
  media: wl128x-radio: convert to platform device
  media: wl128x-radio: use device managed memory allocation
  media: wl128x-radio: load firmware from ti-connectivity/
  media: wl128x-radio: simplify fmc_prepare/fmc_release
  media: wl128x-radio: fix skb debug printing
  media: wl128x-radio: move from TI_ST to hci_ll driver
  Bluetooth: btwilink: drop superseded driver
  misc: ti-st: Drop superseded driver

 .../boot/dts/logicpd-torpedo-37xx-devkit.dts  |   8 +
 arch/arm/boot/dts/omap3-igep0020-rev-f.dts    |   8 +
 arch/arm/boot/dts/omap3-igep0030-rev-g.dts    |   8 +
 arch/arm/mach-omap2/pdata-quirks.c            |  52 -
 drivers/bluetooth/Kconfig                     |  11 -
 drivers/bluetooth/Makefile                    |   1 -
 drivers/bluetooth/btwilink.c                  | 350 -------
 drivers/bluetooth/hci_ll.c                    | 115 ++-
 drivers/media/radio/wl128x/Kconfig            |   2 +-
 drivers/media/radio/wl128x/fmdrv.h            |   5 +-
 drivers/media/radio/wl128x/fmdrv_common.c     | 211 ++--
 drivers/media/radio/wl128x/fmdrv_common.h     |   4 +-
 drivers/media/radio/wl128x/fmdrv_v4l2.c       |  55 +-
 drivers/media/radio/wl128x/fmdrv_v4l2.h       |   2 +-
 drivers/misc/Kconfig                          |   1 -
 drivers/misc/Makefile                         |   1 -
 drivers/misc/ti-st/Kconfig                    |  18 -
 drivers/misc/ti-st/Makefile                   |   6 -
 drivers/misc/ti-st/st_core.c                  | 922 ------------------
 drivers/misc/ti-st/st_kim.c                   | 868 -----------------
 drivers/misc/ti-st/st_ll.c                    | 169 ----
 include/linux/ti_wilink_st.h                  | 337 +------
 22 files changed, 213 insertions(+), 2941 deletions(-)
 delete mode 100644 drivers/bluetooth/btwilink.c
 delete mode 100644 drivers/misc/ti-st/Kconfig
 delete mode 100644 drivers/misc/ti-st/Makefile
 delete mode 100644 drivers/misc/ti-st/st_core.c
 delete mode 100644 drivers/misc/ti-st/st_kim.c
 delete mode 100644 drivers/misc/ti-st/st_ll.c

Comments

Tony Lindgren Dec. 21, 2018, 6:02 p.m. UTC | #1
* Sebastian Reichel <sre@kernel.org> [181221 01:18]:
> The new code has been tested on the Motorola Droid 4. For testing the audio
> should be configured to route Ext to Speaker or Headphone. Then you need to
> plug headphone, since its cable is used as antenna. For testing there is a
> 'radio' utility packages in Debian. When you start the utility you need to
> specify a frequency, since initial get_frequency returns an error:

Nice, good to see that ti-st kim stuff gone :) I gave this a quick
try using fmtools.git and fmscan works just fine. No luck yet with
fm though, it gives VIDIOC_G_CTRL: Not a tty error somehow so
maybe I'm missing some options, patch below for omap2plus_defconfig.

Hmm so looks like nothing to configure for the clocks or
CPCAP_BIT_ST_L_TIMESLOT bits for cap for the EXT? So the
wl12xx audio is wired directly to cpcap EXT then and not a
TDM slot on the mcbsp huh?

> Merry Christmas!

Same to you!

Tony

8< --------------------------------
From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Fri, 21 Dec 2018 07:57:09 -0800
Subject: [PATCH] ARM: omap2plus_defconfig: Add RADIO_WL128X as a loadable
 module

This allows using the FM radio in the wl12xx chips after modprobe
fm_drv using radio from xawt, or fmtools.

Note that the firmware placed into /lib/firmware/ti-connectivity
directory:

fm_rx_ch8_1283.2.bts
fmc_ch8_1283.2.bts

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/configs/omap2plus_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -126,6 +126,7 @@ CONFIG_AF_RXRPC=m
 CONFIG_RXKAD=y
 CONFIG_CFG80211=m
 CONFIG_MAC80211=m
+CONFIG_RFKILL=m
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_DMA_CMA=y
@@ -343,12 +344,14 @@ CONFIG_IR_GPIO_TX=m
 CONFIG_IR_PWM_TX=m
 CONFIG_MEDIA_SUPPORT=m
 CONFIG_MEDIA_CAMERA_SUPPORT=y
+CONFIG_MEDIA_RADIO_SUPPORT=y
 CONFIG_MEDIA_CEC_SUPPORT=y
 CONFIG_MEDIA_CONTROLLER=y
 CONFIG_VIDEO_V4L2_SUBDEV_API=y
 CONFIG_V4L_PLATFORM_DRIVERS=y
 CONFIG_VIDEO_OMAP3=m
 CONFIG_CEC_PLATFORM_DRIVERS=y
+CONFIG_RADIO_WL128X=m
 # CONFIG_MEDIA_SUBDRV_AUTOSELECT is not set
 CONFIG_VIDEO_TVP5150=m
 CONFIG_DRM=m
Sebastian Reichel Dec. 22, 2018, 2:47 a.m. UTC | #2
Hi,

On Fri, Dec 21, 2018 at 10:02:05AM -0800, Tony Lindgren wrote:
> * Sebastian Reichel <sre@kernel.org> [181221 01:18]:
> > The new code has been tested on the Motorola Droid 4. For testing the audio
> > should be configured to route Ext to Speaker or Headphone. Then you need to
> > plug headphone, since its cable is used as antenna. For testing there is a
> > 'radio' utility packages in Debian. When you start the utility you need to
> > specify a frequency, since initial get_frequency returns an error:
> 
> Nice, good to see that ti-st kim stuff gone :) I gave this a quick
> try using fmtools.git and fmscan works just fine. No luck yet with
> fm though, it gives VIDIOC_G_CTRL: Not a tty error somehow so
> maybe I'm missing some options, patch below for omap2plus_defconfig.

I only did a few quick tests with 'radio' utility. I could scan
for stations and I could listen to some station. I suppose the
wl128x-radio driver has some buggy code paths, but that are
unrelated to this patchset.

> Hmm so looks like nothing to configure for the clocks or
> CPCAP_BIT_ST_L_TIMESLOT bits for cap for the EXT? So the
> wl12xx audio is wired directly to cpcap EXT then and not a
> TDM slot on the mcbsp huh?

For FM radio it's directly wired to EXT with no DAI being required.
I think that EXT is only used by FM radio and not by bluetooth. BT
seems to use TDM.

> > Merry Christmas!
> 
> Same to you!
> 
> Tony
> 
> 8< --------------------------------
> From tony Mon Sep 17 00:00:00 2001
> From: Tony Lindgren <tony@atomide.com>
> Date: Fri, 21 Dec 2018 07:57:09 -0800
> Subject: [PATCH] ARM: omap2plus_defconfig: Add RADIO_WL128X as a loadable
>  module
> 
> This allows using the FM radio in the wl12xx chips after modprobe
> fm_drv using radio from xawt, or fmtools.

Mh. I suppose I forgot to add alias to support autoloading the
FM module.

> Note that the firmware placed into /lib/firmware/ti-connectivity
> directory:
> 
> fm_rx_ch8_1283.2.bts
> fmc_ch8_1283.2.bts

There is also a TX firmware. The Droid 4's chip should support this,
but I don't know if there is audio routing for this feature.

-- Sebastian

> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  arch/arm/configs/omap2plus_defconfig | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
> --- a/arch/arm/configs/omap2plus_defconfig
> +++ b/arch/arm/configs/omap2plus_defconfig
> @@ -126,6 +126,7 @@ CONFIG_AF_RXRPC=m
>  CONFIG_RXKAD=y
>  CONFIG_CFG80211=m
>  CONFIG_MAC80211=m
> +CONFIG_RFKILL=m
>  CONFIG_DEVTMPFS=y
>  CONFIG_DEVTMPFS_MOUNT=y
>  CONFIG_DMA_CMA=y
> @@ -343,12 +344,14 @@ CONFIG_IR_GPIO_TX=m
>  CONFIG_IR_PWM_TX=m
>  CONFIG_MEDIA_SUPPORT=m
>  CONFIG_MEDIA_CAMERA_SUPPORT=y
> +CONFIG_MEDIA_RADIO_SUPPORT=y
>  CONFIG_MEDIA_CEC_SUPPORT=y
>  CONFIG_MEDIA_CONTROLLER=y
>  CONFIG_VIDEO_V4L2_SUBDEV_API=y
>  CONFIG_V4L_PLATFORM_DRIVERS=y
>  CONFIG_VIDEO_OMAP3=m
>  CONFIG_CEC_PLATFORM_DRIVERS=y
> +CONFIG_RADIO_WL128X=m
>  # CONFIG_MEDIA_SUBDRV_AUTOSELECT is not set
>  CONFIG_VIDEO_TVP5150=m
>  CONFIG_DRM=m
> -- 
> 2.19.2
Pavel Machek Dec. 22, 2018, 8:08 p.m. UTC | #3
Merry Christmas!

> This moves all remaining users of the legacy TI_ST driver to hcill (patches
> 1-3). Then patches 4-7 convert wl128x-radio driver to a standard platform
> device driver with support for multiple instances. Patch 7 will result in
> (userless) TI_ST driver no longer supporting radio at runtime. Patch 8-11 do
> some cleanups in the wl128x-radio driver. Finally patch 12 removes the TI_ST
> specific parts from wl128x-radio and adds the required infrastructure to use it
> with the serdev hcill driver instead. The remaining patches 13 and 14 remove
> the old TI_ST code.
> 
> The new code has been tested on the Motorola Droid 4. For testing the audio
> should be configured to route Ext to Speaker or Headphone. Then you need to
> plug headphone, since its cable is used as antenna. For testing there is a
> 'radio' utility packages in Debian. When you start the utility you need to
> specify a frequency, since initial get_frequency returns an error:
> 
> $ radio -f 100.0

Ok, it seems  the driver does not work built-in, due to firmware issue:

root@devuan:/home/user# dmesg | grep wl12
[    1.018951] reg-fixed-voltage regulator-wl12xx: GPIO lookup for
consumer (null)
[    1.026550] reg-fixed-voltage regulator-wl12xx: using device tree
for GPIO lookup
[    1.034271] of_get_named_gpiod_flags: can't parse 'gpios' property
of node '/regulator-wl12xx[0]'
[    1.043487] of_get_named_gpiod_flags: parsed 'gpio' property of
node '/regulator-wl12xx[0]' - status (0)
[    4.151885] wl12xx_driver wl12xx.1.auto: Direct firmware load for
ti-connectivity/wl128x-nvs.bin failed with error -2
[   11.368286] vwl1271: disabling
root@devuan:/home/user# find /lib/firmware/ | grep wl128
/lib/firmware/ti-connectivity/wl128x-fw-5-plt.bin
/lib/firmware/ti-connectivity/wl128x-fw-5-mr.bin
/lib/firmware/ti-connectivity/wl128x-fw-5-sr.bin
/lib/firmware/ti-connectivity/wl128x-nvs.bin
root@devuan:/home/user#

Ideas welcome... ... ... am I supposed to compile wl128-nvs.bin into
the kernel using EXTRA_FIRMWARE?

									Pavel


> Merry Christmas!
> 
> -- Sebastian
> 
> Sebastian Reichel (14):
>   ARM: dts: LogicPD Torpedo: Add WiLink UART node
>   ARM: dts: IGEP: Add WiLink UART node
>   ARM: OMAP2+: pdata-quirks: drop TI_ST/KIM support
>   media: wl128x-radio: remove module version
>   media: wl128x-radio: remove global radio_disconnected
>   media: wl128x-radio: remove global radio_dev
>   media: wl128x-radio: convert to platform device
>   media: wl128x-radio: use device managed memory allocation
>   media: wl128x-radio: load firmware from ti-connectivity/
>   media: wl128x-radio: simplify fmc_prepare/fmc_release
>   media: wl128x-radio: fix skb debug printing
>   media: wl128x-radio: move from TI_ST to hci_ll driver
>   Bluetooth: btwilink: drop superseded driver
>   misc: ti-st: Drop superseded driver
> 
>  .../boot/dts/logicpd-torpedo-37xx-devkit.dts  |   8 +
>  arch/arm/boot/dts/omap3-igep0020-rev-f.dts    |   8 +
>  arch/arm/boot/dts/omap3-igep0030-rev-g.dts    |   8 +
>  arch/arm/mach-omap2/pdata-quirks.c            |  52 -
>  drivers/bluetooth/Kconfig                     |  11 -
>  drivers/bluetooth/Makefile                    |   1 -
>  drivers/bluetooth/btwilink.c                  | 350 -------
>  drivers/bluetooth/hci_ll.c                    | 115 ++-
>  drivers/media/radio/wl128x/Kconfig            |   2 +-
>  drivers/media/radio/wl128x/fmdrv.h            |   5 +-
>  drivers/media/radio/wl128x/fmdrv_common.c     | 211 ++--
>  drivers/media/radio/wl128x/fmdrv_common.h     |   4 +-
>  drivers/media/radio/wl128x/fmdrv_v4l2.c       |  55 +-
>  drivers/media/radio/wl128x/fmdrv_v4l2.h       |   2 +-
>  drivers/misc/Kconfig                          |   1 -
>  drivers/misc/Makefile                         |   1 -
>  drivers/misc/ti-st/Kconfig                    |  18 -
>  drivers/misc/ti-st/Makefile                   |   6 -
>  drivers/misc/ti-st/st_core.c                  | 922 ------------------
>  drivers/misc/ti-st/st_kim.c                   | 868 -----------------
>  drivers/misc/ti-st/st_ll.c                    | 169 ----
>  include/linux/ti_wilink_st.h                  | 337 +------
>  22 files changed, 213 insertions(+), 2941 deletions(-)
>  delete mode 100644 drivers/bluetooth/btwilink.c
>  delete mode 100644 drivers/misc/ti-st/Kconfig
>  delete mode 100644 drivers/misc/ti-st/Makefile
>  delete mode 100644 drivers/misc/ti-st/st_core.c
>  delete mode 100644 drivers/misc/ti-st/st_kim.c
>  delete mode 100644 drivers/misc/ti-st/st_ll.c
>
Adam Ford Dec. 22, 2018, 8:36 p.m. UTC | #4
On Sat, Dec 22, 2018 at 11:09 AM Tony Lindgren <tony@atomide.com> wrote:
>
> * Sebastian Reichel <sre@kernel.org> [181221 01:18]:
> > The new code has been tested on the Motorola Droid 4. For testing the audio
> > should be configured to route Ext to Speaker or Headphone. Then you need to
> > plug headphone, since its cable is used as antenna. For testing there is a
> > 'radio' utility packages in Debian. When you start the utility you need to
> > specify a frequency, since initial get_frequency returns an error:
>
> Nice, good to see that ti-st kim stuff gone :) I gave this a quick

Tony,

As much as I'd like to see the ti-st kim stuff go, I am not able to
load the Bluetooth on the Torpedo board (wl1283).  The hooks on a
different, wl18xx and 127x board work fine.  I am not sure if there is
anything different about the wl1283, but I don't have any other boards
other than the Logic PD Torpedo kit.  Do you have any wl1283 boards to
test?  I'd like to see this BT timeout stuff resolved before we dump
the ti-st kim stuff, otherwise, I'll forever be porting drivers.  :-(

adam

> try using fmtools.git and fmscan works just fine. No luck yet with
> fm though, it gives VIDIOC_G_CTRL: Not a tty error somehow so
> maybe I'm missing some options, patch below for omap2plus_defconfig.
>
> Hmm so looks like nothing to configure for the clocks or
> CPCAP_BIT_ST_L_TIMESLOT bits for cap for the EXT? So the
> wl12xx audio is wired directly to cpcap EXT then and not a
> TDM slot on the mcbsp huh?
>
> > Merry Christmas!
>
> Same to you!
>
> Tony
>
> 8< --------------------------------
> From tony Mon Sep 17 00:00:00 2001
> From: Tony Lindgren <tony@atomide.com>
> Date: Fri, 21 Dec 2018 07:57:09 -0800
> Subject: [PATCH] ARM: omap2plus_defconfig: Add RADIO_WL128X as a loadable
>  module
>
> This allows using the FM radio in the wl12xx chips after modprobe
> fm_drv using radio from xawt, or fmtools.
>
> Note that the firmware placed into /lib/firmware/ti-connectivity
> directory:
>
> fm_rx_ch8_1283.2.bts
> fmc_ch8_1283.2.bts
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  arch/arm/configs/omap2plus_defconfig | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
> --- a/arch/arm/configs/omap2plus_defconfig
> +++ b/arch/arm/configs/omap2plus_defconfig
> @@ -126,6 +126,7 @@ CONFIG_AF_RXRPC=m
>  CONFIG_RXKAD=y
>  CONFIG_CFG80211=m
>  CONFIG_MAC80211=m
> +CONFIG_RFKILL=m
>  CONFIG_DEVTMPFS=y
>  CONFIG_DEVTMPFS_MOUNT=y
>  CONFIG_DMA_CMA=y
> @@ -343,12 +344,14 @@ CONFIG_IR_GPIO_TX=m
>  CONFIG_IR_PWM_TX=m
>  CONFIG_MEDIA_SUPPORT=m
>  CONFIG_MEDIA_CAMERA_SUPPORT=y
> +CONFIG_MEDIA_RADIO_SUPPORT=y
>  CONFIG_MEDIA_CEC_SUPPORT=y
>  CONFIG_MEDIA_CONTROLLER=y
>  CONFIG_VIDEO_V4L2_SUBDEV_API=y
>  CONFIG_V4L_PLATFORM_DRIVERS=y
>  CONFIG_VIDEO_OMAP3=m
>  CONFIG_CEC_PLATFORM_DRIVERS=y
> +CONFIG_RADIO_WL128X=m
>  # CONFIG_MEDIA_SUBDRV_AUTOSELECT is not set
>  CONFIG_VIDEO_TVP5150=m
>  CONFIG_DRM=m
> --
> 2.19.2
Pavel Machek Dec. 22, 2018, 10:40 p.m. UTC | #5
Hi!

> > This moves all remaining users of the legacy TI_ST driver to hcill (patches
> > 1-3). Then patches 4-7 convert wl128x-radio driver to a standard platform
> > device driver with support for multiple instances. Patch 7 will result in
> > (userless) TI_ST driver no longer supporting radio at runtime. Patch 8-11 do
> > some cleanups in the wl128x-radio driver. Finally patch 12 removes the TI_ST
> > specific parts from wl128x-radio and adds the required infrastructure to use it
> > with the serdev hcill driver instead. The remaining patches 13 and 14 remove
> > the old TI_ST code.
> > 
> > The new code has been tested on the Motorola Droid 4. For testing the audio
> > should be configured to route Ext to Speaker or Headphone. Then you need to
> > plug headphone, since its cable is used as antenna. For testing there is a
> > 'radio' utility packages in Debian. When you start the utility you need to
> > specify a frequency, since initial get_frequency returns an error:
> > 
> > $ radio -f 100.0
> 
> Ok, it seems  the driver does not work built-in, due to firmware issue:
> 
> root@devuan:/home/user# dmesg | grep wl12
> [    1.018951] reg-fixed-voltage regulator-wl12xx: GPIO lookup for
> consumer (null)
> [    1.026550] reg-fixed-voltage regulator-wl12xx: using device tree
> for GPIO lookup
> [    1.034271] of_get_named_gpiod_flags: can't parse 'gpios' property
> of node '/regulator-wl12xx[0]'
> [    1.043487] of_get_named_gpiod_flags: parsed 'gpio' property of
> node '/regulator-wl12xx[0]' - status (0)
> [    4.151885] wl12xx_driver wl12xx.1.auto: Direct firmware load for
> ti-connectivity/wl128x-nvs.bin failed with error -2
> [   11.368286] vwl1271: disabling
> root@devuan:/home/user# find /lib/firmware/ | grep wl128
> /lib/firmware/ti-connectivity/wl128x-fw-5-plt.bin
> /lib/firmware/ti-connectivity/wl128x-fw-5-mr.bin
> /lib/firmware/ti-connectivity/wl128x-fw-5-sr.bin
> /lib/firmware/ti-connectivity/wl128x-nvs.bin
> root@devuan:/home/user#
> 
> Ideas welcome... ... ... am I supposed to compile wl128-nvs.bin into
> the kernel using EXTRA_FIRMWARE?

EXTRA_FIRMWARE gets me further... some of it was not in debian.

"Speaker right" needs to be set to "Ext" in alsamixer, and then... it
works! :-)

Quality does not seem to be great, but that may be mixer settings or
something.

Thanks!
								Pavel

Tested-by: Pavel Machek <pavel@ucw.cz>
Tony Lindgren Dec. 23, 2018, 4:15 p.m. UTC | #6
* Sebastian Reichel <sre@kernel.org> [181222 02:48]:
> On Fri, Dec 21, 2018 at 10:02:05AM -0800, Tony Lindgren wrote:
> > Hmm so looks like nothing to configure for the clocks or
> > CPCAP_BIT_ST_L_TIMESLOT bits for cap for the EXT? So the
> > wl12xx audio is wired directly to cpcap EXT then and not a
> > TDM slot on the mcbsp huh?
> 
> For FM radio it's directly wired to EXT with no DAI being required.
> I think that EXT is only used by FM radio and not by bluetooth. BT
> seems to use TDM.

OK then it sounds like we just need a diff -u of the cpcap regs
from Android with cpcaprw --all before and after using a bluetooth
headset during a voice call to configure it for voice calls for the
TDM. I think snd-soc-motmd just needs the voice output specified
in the mixer in addition to the cpcap configuration.

I don't think have any bluetooth audio gear though, maybe somebody
using a headset can post the diff.

Regards,

Tony
Tony Lindgren Dec. 23, 2018, 4:22 p.m. UTC | #7
* Adam Ford <aford173@gmail.com> [181222 20:36]:
> As much as I'd like to see the ti-st kim stuff go, I am not able to
> load the Bluetooth on the Torpedo board (wl1283).  The hooks on a
> different, wl18xx and 127x board work fine.  I am not sure if there is
> anything different about the wl1283, but I don't have any other boards
> other than the Logic PD Torpedo kit.  Do you have any wl1283 boards to
> test?  I'd like to see this BT timeout stuff resolved before we dump
> the ti-st kim stuff, otherwise, I'll forever be porting drivers.  :-(

Sebastian and I both tested this with droid 4, which has wl1283. So
it's probably some missing GPIO/regulator/pinconf stuff that the
pdata-quirks.c does for you.

Maybe try adding back also the pdata-quirks.c code and see if that
helps?

Regards,

Tony
Sebastian Reichel Jan. 10, 2019, 5:42 p.m. UTC | #8
Hi,

On Sat, Dec 22, 2018 at 09:08:28PM +0100, Pavel Machek wrote:
> Merry Christmas!
> 
> > This moves all remaining users of the legacy TI_ST driver to hcill (patches
> > 1-3). Then patches 4-7 convert wl128x-radio driver to a standard platform
> > device driver with support for multiple instances. Patch 7 will result in
> > (userless) TI_ST driver no longer supporting radio at runtime. Patch 8-11 do
> > some cleanups in the wl128x-radio driver. Finally patch 12 removes the TI_ST
> > specific parts from wl128x-radio and adds the required infrastructure to use it
> > with the serdev hcill driver instead. The remaining patches 13 and 14 remove
> > the old TI_ST code.
> > 
> > The new code has been tested on the Motorola Droid 4. For testing the audio
> > should be configured to route Ext to Speaker or Headphone. Then you need to
> > plug headphone, since its cable is used as antenna. For testing there is a
> > 'radio' utility packages in Debian. When you start the utility you need to
> > specify a frequency, since initial get_frequency returns an error:
> > 
> > $ radio -f 100.0
> 
> Ok, it seems  the driver does not work built-in, due to firmware issue:
> 
> root@devuan:/home/user# dmesg | grep wl12
> [    1.018951] reg-fixed-voltage regulator-wl12xx: GPIO lookup for
> consumer (null)
> [    1.026550] reg-fixed-voltage regulator-wl12xx: using device tree
> for GPIO lookup
> [    1.034271] of_get_named_gpiod_flags: can't parse 'gpios' property
> of node '/regulator-wl12xx[0]'
> [    1.043487] of_get_named_gpiod_flags: parsed 'gpio' property of
> node '/regulator-wl12xx[0]' - status (0)
> [    4.151885] wl12xx_driver wl12xx.1.auto: Direct firmware load for
> ti-connectivity/wl128x-nvs.bin failed with error -2
> [   11.368286] vwl1271: disabling
> root@devuan:/home/user# find /lib/firmware/ | grep wl128
> /lib/firmware/ti-connectivity/wl128x-fw-5-plt.bin
> /lib/firmware/ti-connectivity/wl128x-fw-5-mr.bin
> /lib/firmware/ti-connectivity/wl128x-fw-5-sr.bin
> /lib/firmware/ti-connectivity/wl128x-nvs.bin
> root@devuan:/home/user#
>
> Ideas welcome... ... ... am I supposed to compile wl128-nvs.bin into
> the kernel using EXTRA_FIRMWARE?

This is due to the driver loading before the rootfs is available.
You can workaround this without touching your kernel configuration by
rebinding the driver via sysfs: https://lwn.net/Articles/143397/

-- Sebastian
Hans Verkuil March 14, 2019, 8:20 a.m. UTC | #9
Hi Sebastian,

On 12/21/18 2:17 AM, Sebastian Reichel wrote:
> Hi,
> 
> This moves all remaining users of the legacy TI_ST driver to hcill (patches
> 1-3). Then patches 4-7 convert wl128x-radio driver to a standard platform
> device driver with support for multiple instances. Patch 7 will result in
> (userless) TI_ST driver no longer supporting radio at runtime. Patch 8-11 do
> some cleanups in the wl128x-radio driver. Finally patch 12 removes the TI_ST
> specific parts from wl128x-radio and adds the required infrastructure to use it
> with the serdev hcill driver instead. The remaining patches 13 and 14 remove
> the old TI_ST code.
> 
> The new code has been tested on the Motorola Droid 4. For testing the audio
> should be configured to route Ext to Speaker or Headphone. Then you need to
> plug headphone, since its cable is used as antenna. For testing there is a
> 'radio' utility packages in Debian. When you start the utility you need to
> specify a frequency, since initial get_frequency returns an error:

What is the status of this series?

Based on some of the replies (from Adam Ford in particular) it appears that
this isn't ready to be merged, so is a v2 planned?

Regards,

	Hans

> 
> $ radio -f 100.0
> 
> Merry Christmas!
> 
> -- Sebastian
> 
> Sebastian Reichel (14):
>   ARM: dts: LogicPD Torpedo: Add WiLink UART node
>   ARM: dts: IGEP: Add WiLink UART node
>   ARM: OMAP2+: pdata-quirks: drop TI_ST/KIM support
>   media: wl128x-radio: remove module version
>   media: wl128x-radio: remove global radio_disconnected
>   media: wl128x-radio: remove global radio_dev
>   media: wl128x-radio: convert to platform device
>   media: wl128x-radio: use device managed memory allocation
>   media: wl128x-radio: load firmware from ti-connectivity/
>   media: wl128x-radio: simplify fmc_prepare/fmc_release
>   media: wl128x-radio: fix skb debug printing
>   media: wl128x-radio: move from TI_ST to hci_ll driver
>   Bluetooth: btwilink: drop superseded driver
>   misc: ti-st: Drop superseded driver
> 
>  .../boot/dts/logicpd-torpedo-37xx-devkit.dts  |   8 +
>  arch/arm/boot/dts/omap3-igep0020-rev-f.dts    |   8 +
>  arch/arm/boot/dts/omap3-igep0030-rev-g.dts    |   8 +
>  arch/arm/mach-omap2/pdata-quirks.c            |  52 -
>  drivers/bluetooth/Kconfig                     |  11 -
>  drivers/bluetooth/Makefile                    |   1 -
>  drivers/bluetooth/btwilink.c                  | 350 -------
>  drivers/bluetooth/hci_ll.c                    | 115 ++-
>  drivers/media/radio/wl128x/Kconfig            |   2 +-
>  drivers/media/radio/wl128x/fmdrv.h            |   5 +-
>  drivers/media/radio/wl128x/fmdrv_common.c     | 211 ++--
>  drivers/media/radio/wl128x/fmdrv_common.h     |   4 +-
>  drivers/media/radio/wl128x/fmdrv_v4l2.c       |  55 +-
>  drivers/media/radio/wl128x/fmdrv_v4l2.h       |   2 +-
>  drivers/misc/Kconfig                          |   1 -
>  drivers/misc/Makefile                         |   1 -
>  drivers/misc/ti-st/Kconfig                    |  18 -
>  drivers/misc/ti-st/Makefile                   |   6 -
>  drivers/misc/ti-st/st_core.c                  | 922 ------------------
>  drivers/misc/ti-st/st_kim.c                   | 868 -----------------
>  drivers/misc/ti-st/st_ll.c                    | 169 ----
>  include/linux/ti_wilink_st.h                  | 337 +------
>  22 files changed, 213 insertions(+), 2941 deletions(-)
>  delete mode 100644 drivers/bluetooth/btwilink.c
>  delete mode 100644 drivers/misc/ti-st/Kconfig
>  delete mode 100644 drivers/misc/ti-st/Makefile
>  delete mode 100644 drivers/misc/ti-st/st_core.c
>  delete mode 100644 drivers/misc/ti-st/st_kim.c
>  delete mode 100644 drivers/misc/ti-st/st_ll.c
>
Adam Ford March 14, 2019, 12:18 p.m. UTC | #10
On Thu, Mar 14, 2019 at 3:21 AM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> Hi Sebastian,
>
> On 12/21/18 2:17 AM, Sebastian Reichel wrote:
> > Hi,
> >
> > This moves all remaining users of the legacy TI_ST driver to hcill (patches
> > 1-3). Then patches 4-7 convert wl128x-radio driver to a standard platform
> > device driver with support for multiple instances. Patch 7 will result in
> > (userless) TI_ST driver no longer supporting radio at runtime. Patch 8-11 do
> > some cleanups in the wl128x-radio driver. Finally patch 12 removes the TI_ST
> > specific parts from wl128x-radio and adds the required infrastructure to use it
> > with the serdev hcill driver instead. The remaining patches 13 and 14 remove
> > the old TI_ST code.
> >
> > The new code has been tested on the Motorola Droid 4. For testing the audio
> > should be configured to route Ext to Speaker or Headphone. Then you need to
> > plug headphone, since its cable is used as antenna. For testing there is a
> > 'radio' utility packages in Debian. When you start the utility you need to
> > specify a frequency, since initial get_frequency returns an error:
>
> What is the status of this series?
>
> Based on some of the replies (from Adam Ford in particular) it appears that
> this isn't ready to be merged, so is a v2 planned?

If you can leave the Logic PD Torpedo board alone and don't remove the
legacy st driver for now, go ahead and migrate the others.  I know
what you proposed 'should' work on my board, but I don't know why it
doesn't.  In fact other boards I maintain use your method, but it just
doesn't work on the Torpedo and I don't know why.  (it's not for lack
of trying)

adam
>
> Regards,
>
>         Hans
>
> >
> > $ radio -f 100.0
> >
> > Merry Christmas!
> >
> > -- Sebastian
> >
> > Sebastian Reichel (14):
> >   ARM: dts: LogicPD Torpedo: Add WiLink UART node
> >   ARM: dts: IGEP: Add WiLink UART node
> >   ARM: OMAP2+: pdata-quirks: drop TI_ST/KIM support
> >   media: wl128x-radio: remove module version
> >   media: wl128x-radio: remove global radio_disconnected
> >   media: wl128x-radio: remove global radio_dev
> >   media: wl128x-radio: convert to platform device
> >   media: wl128x-radio: use device managed memory allocation
> >   media: wl128x-radio: load firmware from ti-connectivity/
> >   media: wl128x-radio: simplify fmc_prepare/fmc_release
> >   media: wl128x-radio: fix skb debug printing
> >   media: wl128x-radio: move from TI_ST to hci_ll driver
> >   Bluetooth: btwilink: drop superseded driver
> >   misc: ti-st: Drop superseded driver
> >
> >  .../boot/dts/logicpd-torpedo-37xx-devkit.dts  |   8 +
> >  arch/arm/boot/dts/omap3-igep0020-rev-f.dts    |   8 +
> >  arch/arm/boot/dts/omap3-igep0030-rev-g.dts    |   8 +
> >  arch/arm/mach-omap2/pdata-quirks.c            |  52 -
> >  drivers/bluetooth/Kconfig                     |  11 -
> >  drivers/bluetooth/Makefile                    |   1 -
> >  drivers/bluetooth/btwilink.c                  | 350 -------
> >  drivers/bluetooth/hci_ll.c                    | 115 ++-
> >  drivers/media/radio/wl128x/Kconfig            |   2 +-
> >  drivers/media/radio/wl128x/fmdrv.h            |   5 +-
> >  drivers/media/radio/wl128x/fmdrv_common.c     | 211 ++--
> >  drivers/media/radio/wl128x/fmdrv_common.h     |   4 +-
> >  drivers/media/radio/wl128x/fmdrv_v4l2.c       |  55 +-
> >  drivers/media/radio/wl128x/fmdrv_v4l2.h       |   2 +-
> >  drivers/misc/Kconfig                          |   1 -
> >  drivers/misc/Makefile                         |   1 -
> >  drivers/misc/ti-st/Kconfig                    |  18 -
> >  drivers/misc/ti-st/Makefile                   |   6 -
> >  drivers/misc/ti-st/st_core.c                  | 922 ------------------
> >  drivers/misc/ti-st/st_kim.c                   | 868 -----------------
> >  drivers/misc/ti-st/st_ll.c                    | 169 ----
> >  include/linux/ti_wilink_st.h                  | 337 +------
> >  22 files changed, 213 insertions(+), 2941 deletions(-)
> >  delete mode 100644 drivers/bluetooth/btwilink.c
> >  delete mode 100644 drivers/misc/ti-st/Kconfig
> >  delete mode 100644 drivers/misc/ti-st/Makefile
> >  delete mode 100644 drivers/misc/ti-st/st_core.c
> >  delete mode 100644 drivers/misc/ti-st/st_kim.c
> >  delete mode 100644 drivers/misc/ti-st/st_ll.c
> >
>
Sebastian Reichel March 19, 2019, 1:31 p.m. UTC | #11
Hi Hans,

On Thu, Mar 14, 2019 at 09:20:10AM +0100, Hans Verkuil wrote:
> On 12/21/18 2:17 AM, Sebastian Reichel wrote:
> > This moves all remaining users of the legacy TI_ST driver to hcill (patches
> > 1-3). Then patches 4-7 convert wl128x-radio driver to a standard platform
> > device driver with support for multiple instances. Patch 7 will result in
> > (userless) TI_ST driver no longer supporting radio at runtime. Patch 8-11 do
> > some cleanups in the wl128x-radio driver. Finally patch 12 removes the TI_ST
> > specific parts from wl128x-radio and adds the required infrastructure to use it
> > with the serdev hcill driver instead. The remaining patches 13 and 14 remove
> > the old TI_ST code.
> > 
> > The new code has been tested on the Motorola Droid 4. For testing the audio
> > should be configured to route Ext to Speaker or Headphone. Then you need to
> > plug headphone, since its cable is used as antenna. For testing there is a
> > 'radio' utility packages in Debian. When you start the utility you need to
> > specify a frequency, since initial get_frequency returns an error:
> 
> What is the status of this series?
>
> Based on some of the replies (from Adam Ford in particular) it appears that
> this isn't ready to be merged, so is a v2 planned?

Yes, a v2 is planned, but I'm super busy at the moment. I don't
expect to send something for this merge window. Neither LogicPD
nor IGEP use FM radio, so I can just remove FM support from the
TI_ST framework. Converting those platforms to hci_ll can be done
in a different patchset.

If that was the only issue there would be a v2 already. But Marcel
Holtmann suggested to pass the custom packet data through the BT
subsystem, which is non-trivial (at least for me) :)

-- Sebastian
Adam Ford May 7, 2019, 5:26 p.m. UTC | #12
On Tue, Mar 19, 2019 at 8:33 AM Sebastian Reichel <sre@kernel.org> wrote:
>
> Hi Hans,
>
> On Thu, Mar 14, 2019 at 09:20:10AM +0100, Hans Verkuil wrote:
> > On 12/21/18 2:17 AM, Sebastian Reichel wrote:
> > > This moves all remaining users of the legacy TI_ST driver to hcill (patches
> > > 1-3). Then patches 4-7 convert wl128x-radio driver to a standard platform
> > > device driver with support for multiple instances. Patch 7 will result in
> > > (userless) TI_ST driver no longer supporting radio at runtime. Patch 8-11 do
> > > some cleanups in the wl128x-radio driver. Finally patch 12 removes the TI_ST
> > > specific parts from wl128x-radio and adds the required infrastructure to use it
> > > with the serdev hcill driver instead. The remaining patches 13 and 14 remove
> > > the old TI_ST code.
> > >
> > > The new code has been tested on the Motorola Droid 4. For testing the audio
> > > should be configured to route Ext to Speaker or Headphone. Then you need to
> > > plug headphone, since its cable is used as antenna. For testing there is a
> > > 'radio' utility packages in Debian. When you start the utility you need to
> > > specify a frequency, since initial get_frequency returns an error:
> >
> > What is the status of this series?
> >
> > Based on some of the replies (from Adam Ford in particular) it appears that
> > this isn't ready to be merged, so is a v2 planned?
>
> Yes, a v2 is planned, but I'm super busy at the moment. I don't
> expect to send something for this merge window. Neither LogicPD
> nor IGEP use FM radio, so I can just remove FM support from the
> TI_ST framework. Converting those platforms to hci_ll can be done
> in a different patchset.
>
> If that was the only issue there would be a v2 already. But Marcel
> Holtmann suggested to pass the custom packet data through the BT
> subsystem, which is non-trivial (at least for me) :)

I am running some tests today on the wl1283-st on the Logic PD Torpedo
board.  Tony had suggested a few options, so I'm going to try those.
Looking at those today.  If/when you have a V2, please CC me on it. If
it's been posted, can you send me a link?  I would really like to see
the st-kim driver go away so I'd like to resolve the issues with the
torpedo board.

thanks

adam
>
> -- Sebastian
Adam Ford May 7, 2019, 6:34 p.m. UTC | #13
On Tue, May 7, 2019 at 12:26 PM Adam Ford <aford173@gmail.com> wrote:
>
> On Tue, Mar 19, 2019 at 8:33 AM Sebastian Reichel <sre@kernel.org> wrote:
> >
> > Hi Hans,
> >
> > On Thu, Mar 14, 2019 at 09:20:10AM +0100, Hans Verkuil wrote:
> > > On 12/21/18 2:17 AM, Sebastian Reichel wrote:
> > > > This moves all remaining users of the legacy TI_ST driver to hcill (patches
> > > > 1-3). Then patches 4-7 convert wl128x-radio driver to a standard platform
> > > > device driver with support for multiple instances. Patch 7 will result in
> > > > (userless) TI_ST driver no longer supporting radio at runtime. Patch 8-11 do
> > > > some cleanups in the wl128x-radio driver. Finally patch 12 removes the TI_ST
> > > > specific parts from wl128x-radio and adds the required infrastructure to use it
> > > > with the serdev hcill driver instead. The remaining patches 13 and 14 remove
> > > > the old TI_ST code.
> > > >
> > > > The new code has been tested on the Motorola Droid 4. For testing the audio
> > > > should be configured to route Ext to Speaker or Headphone. Then you need to
> > > > plug headphone, since its cable is used as antenna. For testing there is a
> > > > 'radio' utility packages in Debian. When you start the utility you need to
> > > > specify a frequency, since initial get_frequency returns an error:
> > >
> > > What is the status of this series?
> > >
> > > Based on some of the replies (from Adam Ford in particular) it appears that
> > > this isn't ready to be merged, so is a v2 planned?
> >
> > Yes, a v2 is planned, but I'm super busy at the moment. I don't
> > expect to send something for this merge window. Neither LogicPD
> > nor IGEP use FM radio, so I can just remove FM support from the
> > TI_ST framework. Converting those platforms to hci_ll can be done
> > in a different patchset.
> >
> > If that was the only issue there would be a v2 already. But Marcel
> > Holtmann suggested to pass the custom packet data through the BT
> > subsystem, which is non-trivial (at least for me) :)
>
> I am running some tests today on the wl1283-st on the Logic PD Torpedo
> board.  Tony had suggested a few options, so I'm going to try those.
> Looking at those today.  If/when you have a V2, please CC me on it. If
> it's been posted, can you send me a link?  I would really like to see
> the st-kim driver go away so I'd like to resolve the issues with the
> torpedo board.

I have run a bunch of tests on the 5.1 kernel.  I am able to get the
firmware to load now and the hci0 goes up.  I was able to establish a
BLE connection to a TI Sensor Tag and read and write data to it with
good success on the wl1283.

Unfortunately, when I tried to do some more extensive testing over
classic Bluetooth, I got an error that repeats itself at seemingly
random intervals:
      Bluetooth: hci0: Frame reassembly failed (-84)

I can still scan and pair, but these Frame reassembly failed errors
appear to come and go.

Do we need to do anything to enable hardware handshaking?  In the
older st-kim driver, the pdata structure listed flow_cntrl=1.  The
i.MX boards use "uart-has-rtscts" in their device trees, but I don't
see anything like that for the omap3-uart driver.  I am not all that
familiar with the Bluetooth subsystem, so I am not sure what would
cause the Frame reassembly error.

adam
>
> thanks
>
> adam
> >
> > -- Sebastian
Marcel Holtmann May 8, 2019, 8:58 p.m. UTC | #14
Hi Adam,

>>>>> This moves all remaining users of the legacy TI_ST driver to hcill (patches
>>>>> 1-3). Then patches 4-7 convert wl128x-radio driver to a standard platform
>>>>> device driver with support for multiple instances. Patch 7 will result in
>>>>> (userless) TI_ST driver no longer supporting radio at runtime. Patch 8-11 do
>>>>> some cleanups in the wl128x-radio driver. Finally patch 12 removes the TI_ST
>>>>> specific parts from wl128x-radio and adds the required infrastructure to use it
>>>>> with the serdev hcill driver instead. The remaining patches 13 and 14 remove
>>>>> the old TI_ST code.
>>>>> 
>>>>> The new code has been tested on the Motorola Droid 4. For testing the audio
>>>>> should be configured to route Ext to Speaker or Headphone. Then you need to
>>>>> plug headphone, since its cable is used as antenna. For testing there is a
>>>>> 'radio' utility packages in Debian. When you start the utility you need to
>>>>> specify a frequency, since initial get_frequency returns an error:
>>>> 
>>>> What is the status of this series?
>>>> 
>>>> Based on some of the replies (from Adam Ford in particular) it appears that
>>>> this isn't ready to be merged, so is a v2 planned?
>>> 
>>> Yes, a v2 is planned, but I'm super busy at the moment. I don't
>>> expect to send something for this merge window. Neither LogicPD
>>> nor IGEP use FM radio, so I can just remove FM support from the
>>> TI_ST framework. Converting those platforms to hci_ll can be done
>>> in a different patchset.
>>> 
>>> If that was the only issue there would be a v2 already. But Marcel
>>> Holtmann suggested to pass the custom packet data through the BT
>>> subsystem, which is non-trivial (at least for me) :)
>> 
>> I am running some tests today on the wl1283-st on the Logic PD Torpedo
>> board.  Tony had suggested a few options, so I'm going to try those.
>> Looking at those today.  If/when you have a V2, please CC me on it. If
>> it's been posted, can you send me a link?  I would really like to see
>> the st-kim driver go away so I'd like to resolve the issues with the
>> torpedo board.
> 
> I have run a bunch of tests on the 5.1 kernel.  I am able to get the
> firmware to load now and the hci0 goes up.  I was able to establish a
> BLE connection to a TI Sensor Tag and read and write data to it with
> good success on the wl1283.
> 
> Unfortunately, when I tried to do some more extensive testing over
> classic Bluetooth, I got an error that repeats itself at seemingly
> random intervals:
>      Bluetooth: hci0: Frame reassembly failed (-84)
> 
> I can still scan and pair, but these Frame reassembly failed errors
> appear to come and go.

there are only 3 places in h4_recv_buf that return EILSEQ. Just add an extra printk to these to figure out which one it is. Maybe it is just extra packet types that we need to handle. If it is not the packet type one, print what packet we have that is causing this.

Regards

Marcel
Adam Ford May 10, 2019, 1:28 p.m. UTC | #15
On Wed, May 8, 2019 at 3:58 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Adam,
>
> >>>>> This moves all remaining users of the legacy TI_ST driver to hcill (patches
> >>>>> 1-3). Then patches 4-7 convert wl128x-radio driver to a standard platform
> >>>>> device driver with support for multiple instances. Patch 7 will result in
> >>>>> (userless) TI_ST driver no longer supporting radio at runtime. Patch 8-11 do
> >>>>> some cleanups in the wl128x-radio driver. Finally patch 12 removes the TI_ST
> >>>>> specific parts from wl128x-radio and adds the required infrastructure to use it
> >>>>> with the serdev hcill driver instead. The remaining patches 13 and 14 remove
> >>>>> the old TI_ST code.
> >>>>>
> >>>>> The new code has been tested on the Motorola Droid 4. For testing the audio
> >>>>> should be configured to route Ext to Speaker or Headphone. Then you need to
> >>>>> plug headphone, since its cable is used as antenna. For testing there is a
> >>>>> 'radio' utility packages in Debian. When you start the utility you need to
> >>>>> specify a frequency, since initial get_frequency returns an error:
> >>>>
> >>>> What is the status of this series?
> >>>>
> >>>> Based on some of the replies (from Adam Ford in particular) it appears that
> >>>> this isn't ready to be merged, so is a v2 planned?
> >>>
> >>> Yes, a v2 is planned, but I'm super busy at the moment. I don't
> >>> expect to send something for this merge window. Neither LogicPD
> >>> nor IGEP use FM radio, so I can just remove FM support from the
> >>> TI_ST framework. Converting those platforms to hci_ll can be done
> >>> in a different patchset.
> >>>
> >>> If that was the only issue there would be a v2 already. But Marcel
> >>> Holtmann suggested to pass the custom packet data through the BT
> >>> subsystem, which is non-trivial (at least for me) :)
> >>
> >> I am running some tests today on the wl1283-st on the Logic PD Torpedo
> >> board.  Tony had suggested a few options, so I'm going to try those.
> >> Looking at those today.  If/when you have a V2, please CC me on it. If
> >> it's been posted, can you send me a link?  I would really like to see
> >> the st-kim driver go away so I'd like to resolve the issues with the
> >> torpedo board.
> >
> > I have run a bunch of tests on the 5.1 kernel.  I am able to get the
> > firmware to load now and the hci0 goes up.  I was able to establish a
> > BLE connection to a TI Sensor Tag and read and write data to it with
> > good success on the wl1283.
> >
> > Unfortunately, when I tried to do some more extensive testing over
> > classic Bluetooth, I got an error that repeats itself at seemingly
> > random intervals:
> >      Bluetooth: hci0: Frame reassembly failed (-84)
> >
> > I can still scan and pair, but these Frame reassembly failed errors
> > appear to come and go.
>
> there are only 3 places in h4_recv_buf that return EILSEQ. Just add an extra printk to these to figure out which one it is. Maybe it is just extra packet types that we need to handle. If it is not the packet type one, print what packet we have that is causing this.
>

I added some code around

/* Check for invalid packet type */
    if (!skb) {
     printk("Check for invalid packet type %x\n", (unsigned int)
(&pkts[i])->type);
     return ERR_PTR(-EILSEQ);
}

I don't know if I did it right or I am reading the packet type
correctly, but the frame reassembly errors are being caught here.

[  408.519165] Check for invalid packet type ff
[  408.523559] Bluetooth: hci0: Frame reassembly failed (-84)


adam

> Regards
>
> Marcel
>
Marcel Holtmann May 10, 2019, 3:38 p.m. UTC | #16
Hi Adam,

>>>>>>> This moves all remaining users of the legacy TI_ST driver to hcill (patches
>>>>>>> 1-3). Then patches 4-7 convert wl128x-radio driver to a standard platform
>>>>>>> device driver with support for multiple instances. Patch 7 will result in
>>>>>>> (userless) TI_ST driver no longer supporting radio at runtime. Patch 8-11 do
>>>>>>> some cleanups in the wl128x-radio driver. Finally patch 12 removes the TI_ST
>>>>>>> specific parts from wl128x-radio and adds the required infrastructure to use it
>>>>>>> with the serdev hcill driver instead. The remaining patches 13 and 14 remove
>>>>>>> the old TI_ST code.
>>>>>>> 
>>>>>>> The new code has been tested on the Motorola Droid 4. For testing the audio
>>>>>>> should be configured to route Ext to Speaker or Headphone. Then you need to
>>>>>>> plug headphone, since its cable is used as antenna. For testing there is a
>>>>>>> 'radio' utility packages in Debian. When you start the utility you need to
>>>>>>> specify a frequency, since initial get_frequency returns an error:
>>>>>> 
>>>>>> What is the status of this series?
>>>>>> 
>>>>>> Based on some of the replies (from Adam Ford in particular) it appears that
>>>>>> this isn't ready to be merged, so is a v2 planned?
>>>>> 
>>>>> Yes, a v2 is planned, but I'm super busy at the moment. I don't
>>>>> expect to send something for this merge window. Neither LogicPD
>>>>> nor IGEP use FM radio, so I can just remove FM support from the
>>>>> TI_ST framework. Converting those platforms to hci_ll can be done
>>>>> in a different patchset.
>>>>> 
>>>>> If that was the only issue there would be a v2 already. But Marcel
>>>>> Holtmann suggested to pass the custom packet data through the BT
>>>>> subsystem, which is non-trivial (at least for me) :)
>>>> 
>>>> I am running some tests today on the wl1283-st on the Logic PD Torpedo
>>>> board.  Tony had suggested a few options, so I'm going to try those.
>>>> Looking at those today.  If/when you have a V2, please CC me on it. If
>>>> it's been posted, can you send me a link?  I would really like to see
>>>> the st-kim driver go away so I'd like to resolve the issues with the
>>>> torpedo board.
>>> 
>>> I have run a bunch of tests on the 5.1 kernel.  I am able to get the
>>> firmware to load now and the hci0 goes up.  I was able to establish a
>>> BLE connection to a TI Sensor Tag and read and write data to it with
>>> good success on the wl1283.
>>> 
>>> Unfortunately, when I tried to do some more extensive testing over
>>> classic Bluetooth, I got an error that repeats itself at seemingly
>>> random intervals:
>>>     Bluetooth: hci0: Frame reassembly failed (-84)
>>> 
>>> I can still scan and pair, but these Frame reassembly failed errors
>>> appear to come and go.
>> 
>> there are only 3 places in h4_recv_buf that return EILSEQ. Just add an extra printk to these to figure out which one it is. Maybe it is just extra packet types that we need to handle. If it is not the packet type one, print what packet we have that is causing this.
>> 
> 
> I added some code around
> 
> /* Check for invalid packet type */
>    if (!skb) {
>     printk("Check for invalid packet type %x\n", (unsigned int)
> (&pkts[i])->type);
>     return ERR_PTR(-EILSEQ);
> }
> 
> I don't know if I did it right or I am reading the packet type
> correctly, but the frame reassembly errors are being caught here.
> 
> [  408.519165] Check for invalid packet type ff
> [  408.523559] Bluetooth: hci0: Frame reassembly failed (-84)

so now we need to figure our on how to handle HCI_VENDOR_PKT.

#define LL_RECV_VENDOR \
	.type = HCI_VENDOR_PKT, \
	.hlen = aaa, \
	.loff = bbb, \
	.lsize = ccc, \
	.maxlen = ddd

static const struct h4_recv_pkt ll_recv_pkts[] = {
	...
	{ LL_RECV_WAKE_ACK,  .recv = ll_recv_frame  },
	{ LL_RECV_VENDOR,    .recv = hci_recv_diag  },
};

Can you hexdump the data inside the skb and we can figure out what it uses for the header and size.

In hci_bcm.c there are a few examples of fixed size packets and bpa10x.c contains one where it follows an actual header definition. Also hci_nokia.c contains a few for their packets.

Regards

Marcel
Adam Ford Sept. 30, 2019, 11:42 p.m. UTC | #17
On Fri, May 10, 2019 at 10:38 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Adam,
>
> >>>>>>> This moves all remaining users of the legacy TI_ST driver to hcill (patches
> >>>>>>> 1-3). Then patches 4-7 convert wl128x-radio driver to a standard platform
> >>>>>>> device driver with support for multiple instances. Patch 7 will result in
> >>>>>>> (userless) TI_ST driver no longer supporting radio at runtime. Patch 8-11 do
> >>>>>>> some cleanups in the wl128x-radio driver. Finally patch 12 removes the TI_ST
> >>>>>>> specific parts from wl128x-radio and adds the required infrastructure to use it
> >>>>>>> with the serdev hcill driver instead. The remaining patches 13 and 14 remove
> >>>>>>> the old TI_ST code.
> >>>>>>>
> >>>>>>> The new code has been tested on the Motorola Droid 4. For testing the audio
> >>>>>>> should be configured to route Ext to Speaker or Headphone. Then you need to
> >>>>>>> plug headphone, since its cable is used as antenna. For testing there is a
> >>>>>>> 'radio' utility packages in Debian. When you start the utility you need to
> >>>>>>> specify a frequency, since initial get_frequency returns an error:
> >>>>>>
> >>>>>> What is the status of this series?
> >>>>>>
> >>>>>> Based on some of the replies (from Adam Ford in particular) it appears that
> >>>>>> this isn't ready to be merged, so is a v2 planned?
> >>>>>
> >>>>> Yes, a v2 is planned, but I'm super busy at the moment. I don't
> >>>>> expect to send something for this merge window. Neither LogicPD
> >>>>> nor IGEP use FM radio, so I can just remove FM support from the
> >>>>> TI_ST framework. Converting those platforms to hci_ll can be done
> >>>>> in a different patchset.
> >>>>>
> >>>>> If that was the only issue there would be a v2 already. But Marcel
> >>>>> Holtmann suggested to pass the custom packet data through the BT
> >>>>> subsystem, which is non-trivial (at least for me) :)
> >>>>
> >>>> I am running some tests today on the wl1283-st on the Logic PD Torpedo
> >>>> board.  Tony had suggested a few options, so I'm going to try those.
> >>>> Looking at those today.  If/when you have a V2, please CC me on it. If
> >>>> it's been posted, can you send me a link?  I would really like to see
> >>>> the st-kim driver go away so I'd like to resolve the issues with the
> >>>> torpedo board.
> >>>
> >>> I have run a bunch of tests on the 5.1 kernel.  I am able to get the
> >>> firmware to load now and the hci0 goes up.  I was able to establish a
> >>> BLE connection to a TI Sensor Tag and read and write data to it with
> >>> good success on the wl1283.
> >>>
> >>> Unfortunately, when I tried to do some more extensive testing over
> >>> classic Bluetooth, I got an error that repeats itself at seemingly
> >>> random intervals:
> >>>     Bluetooth: hci0: Frame reassembly failed (-84)
> >>>
> >>> I can still scan and pair, but these Frame reassembly failed errors
> >>> appear to come and go.
> >>
> >> there are only 3 places in h4_recv_buf that return EILSEQ. Just add an extra printk to these to figure out which one it is. Maybe it is just extra packet types that we need to handle. If it is not the packet type one, print what packet we have that is causing this.
> >>
> >
> > I added some code around
> >
> > /* Check for invalid packet type */
> >    if (!skb) {
> >     printk("Check for invalid packet type %x\n", (unsigned int)
> > (&pkts[i])->type);
> >     return ERR_PTR(-EILSEQ);
> > }
> >
> > I don't know if I did it right or I am reading the packet type
> > correctly, but the frame reassembly errors are being caught here.
> >
> > [  408.519165] Check for invalid packet type ff
> > [  408.523559] Bluetooth: hci0: Frame reassembly failed (-84)
>
> so now we need to figure our on how to handle HCI_VENDOR_PKT.
>
> #define LL_RECV_VENDOR \
>         .type = HCI_VENDOR_PKT, \
>         .hlen = aaa, \
>         .loff = bbb, \
>         .lsize = ccc, \
>         .maxlen = ddd
>
> static const struct h4_recv_pkt ll_recv_pkts[] = {
>         ...
>         { LL_RECV_WAKE_ACK,  .recv = ll_recv_frame  },
>         { LL_RECV_VENDOR,    .recv = hci_recv_diag  },
> };
>
> Can you hexdump the data inside the skb and we can figure out what it uses for the header and size.
>
> In hci_bcm.c there are a few examples of fixed size packets and bpa10x.c contains one where it follows an actual header definition. Also hci_nokia.c contains a few for their packets.

I haven't forgotten this, but I was highly distracted.  I wanted to
test a bunch of stuff on omap3630 and imx6 boards to prep them for the
upcoming 5.4 LTS kernel.  As of now I 'think' this is the last item on
my to-do list.

I'm going to try and throw some debug code into the older st/kim
driver as well as debug this.  I know some people have stated they
have wl1283-st working on a dm3730.  dump some logs?  I am curious to
see if there is anything that can be gained by sharing the info.  I'd
love to see the older st/kim drivers deprecated.

adam
>
> Regards
>
> Marcel
>
Adam Ford Oct. 2, 2019, 7:03 p.m. UTC | #18
On Tue, Mar 19, 2019 at 8:33 AM Sebastian Reichel <sre@kernel.org> wrote:
>
> Hi Hans,
>
> On Thu, Mar 14, 2019 at 09:20:10AM +0100, Hans Verkuil wrote:
> > On 12/21/18 2:17 AM, Sebastian Reichel wrote:
> > > This moves all remaining users of the legacy TI_ST driver to hcill (patches
> > > 1-3). Then patches 4-7 convert wl128x-radio driver to a standard platform
> > > device driver with support for multiple instances. Patch 7 will result in
> > > (userless) TI_ST driver no longer supporting radio at runtime. Patch 8-11 do
> > > some cleanups in the wl128x-radio driver. Finally patch 12 removes the TI_ST
> > > specific parts from wl128x-radio and adds the required infrastructure to use it
> > > with the serdev hcill driver instead. The remaining patches 13 and 14 remove
> > > the old TI_ST code.
> > >
> > > The new code has been tested on the Motorola Droid 4. For testing the audio
> > > should be configured to route Ext to Speaker or Headphone. Then you need to
> > > plug headphone, since its cable is used as antenna. For testing there is a
> > > 'radio' utility packages in Debian. When you start the utility you need to
> > > specify a frequency, since initial get_frequency returns an error:
> >
> > What is the status of this series?
> >
> > Based on some of the replies (from Adam Ford in particular) it appears that
> > this isn't ready to be merged, so is a v2 planned?
>
> Yes, a v2 is planned, but I'm super busy at the moment. I don't
> expect to send something for this merge window. Neither LogicPD
> nor IGEP use FM radio, so I can just remove FM support from the
> TI_ST framework. Converting those platforms to hci_ll can be done
> in a different patchset.
>

Sebastian,

After a bunch of testing, I think the issue I was having was the BTS
file being pulled in from linux-firmware.  I was able to successfully
load a BTS file that I have from Logic PD with working BLE and BT
working together.  I have to run some tests, but if you wouldn't mind
re-basing your code and pushing it again for review, I can most likely
add my 'tested-by'
I am not sure who to discuss my perceived bug in the BTS blob.  I have
to go find the old BTS editor and see if I can determine the cause,
but the fact that I can use the BTS file that corresponds to the FCC
certified file that Logic PD used is more important to me than using
the generic BTS file provided by TI, however it would be nice for the
reference BTS file to operate without error.

adam
> If that was the only issue there would be a v2 already. But Marcel
> Holtmann suggested to pass the custom packet data through the BT
> subsystem, which is non-trivial (at least for me) :)
>
> -- Sebastian
Sebastian Reichel Oct. 3, 2019, 1:42 p.m. UTC | #19
Hi Adam,

On Wed, Oct 02, 2019 at 02:03:52PM -0500, Adam Ford wrote:
> On Tue, Mar 19, 2019 at 8:33 AM Sebastian Reichel <sre@kernel.org> wrote:
> After a bunch of testing, I think the issue I was having was the BTS
> file being pulled in from linux-firmware.  I was able to successfully
> load a BTS file that I have from Logic PD with working BLE and BT
> working together.  I have to run some tests, but if you wouldn't mind
> re-basing your code and pushing it again for review, I can most likely
> add my 'tested-by'.
>
> I am not sure who to discuss my perceived bug in the BTS blob.  I have
> to go find the old BTS editor and see if I can determine the cause,
> but the fact that I can use the BTS file that corresponds to the FCC
> certified file that Logic PD used is more important to me than using
> the generic BTS file provided by TI, however it would be nice for the
> reference BTS file to operate without error.

nice :) I just send a rebased partial series. I need some more time
for the FM radio part (I plan to work on that within the next 3
weeks).

-- Sebastian