[RFC PATCH 0/3] bluetooth/gnss: GNSS support for TiWi chips

[RFC PATCH 0/3] bluetooth/gnss: GNSS support for TiWi chips

[Andreas Kemnade]

Some of these chips have GNSS support. In some vendor kernels
a driver on top of misc/ti-st can be found providing a /dev/tigps
device which speaks the secretive Air Independent Interface (AI2) protocol.
Implement something comparable as a GNSS interface.

With some userspace tools a proof-of-concept can be shown. A position
can be successfully read out.  Basic properties of the protocol are
understood.

This was tested on the Epson Moverio BT-200.

This is sent out as an early RFC to ensure I am going onto the right
track:

So the main questions I see:
- is the approach right to abandon drivers/misc/ti-st?

- Output at /dev/gnssX:
  AI2 vs. NMEA
  The chip can be configured into sending AI2-encapsulated NMEA,
  or proving data in a binary format.
  Some research has to be done yet for the details.
  A pile of logs is waiting for further analysis...

  Arguments for/against NMEA:
  + Userspace is prepared to handle it
  + Power management can be easily done by the kernel
  - Less functionality can be used.

  Arguments for/against AI2:
  + Full functionality can be accessed from userspace (incl. A-GPS,
    maybe raw satellite data)
  - Userspace has to behave to have proper power management
  - No freely (not even as in beer) tool available to fully use AI2,
    so there will be only a real advantage after long "French Cafe"
    sessions.

More detailed tings:
  - Some live cycle management is left out. Since it depends
    on the decisions above, I have not put much thought into it.
  - Should some pieces go into drivers/gnss?
  - detection for GNSS availability: For now the node name is
    used. But the device should be there if the chip supports it
    and things are wired up properly.

Andreas Kemnade (3):
  gnss: Add AI2 protocol used by some TI combo chips.
  bluetooth: ti-st: add GNSS support for TI Wilink chips
  drivers: misc: ti-st: begin to deorbit

 drivers/bluetooth/hci_ll.c | 154 ++++++++++++++++++++++++++++++++++++-
 drivers/gnss/core.c        |   1 +
 drivers/misc/ti-st/Kconfig |   2 +-
 include/linux/gnss.h       |   1 +
 4 files changed, 156 insertions(+), 2 deletions(-)

-- 
2.39.2

[RFC PATCH 1/3] gnss: Add AI2 protocol used by some TI combo chips.

[Andreas Kemnade]

Texas Instruments uses something called Air Independent Interface (AI2) for
their WLAN/BT/GPS combo chips.
No public documentation is available, by looking into captured data, it
becomes clear that you can either raw data or encapsulated NMEA after some
initialization commands.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 drivers/gnss/core.c  | 1 +
 include/linux/gnss.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/gnss/core.c b/drivers/gnss/core.c
index 48f2ee0f78c4d..cac9f45aec4b2 100644
--- a/drivers/gnss/core.c
+++ b/drivers/gnss/core.c
@@ -335,6 +335,7 @@ static const char * const gnss_type_names[GNSS_TYPE_COUNT] = {
 	[GNSS_TYPE_SIRF]	= "SiRF",
 	[GNSS_TYPE_UBX]		= "UBX",
 	[GNSS_TYPE_MTK]		= "MTK",
+	[GNSS_TYPE_AI2]		= "AI2",
 };
 
 static const char *gnss_type_name(const struct gnss_device *gdev)
diff --git a/include/linux/gnss.h b/include/linux/gnss.h
index 36968a0f33e8d..16b565dab83ea 100644
--- a/include/linux/gnss.h
+++ b/include/linux/gnss.h
@@ -23,6 +23,7 @@ enum gnss_type {
 	GNSS_TYPE_SIRF,
 	GNSS_TYPE_UBX,
 	GNSS_TYPE_MTK,
+	GNSS_TYPE_AI2,
 
 	GNSS_TYPE_COUNT
 };
-- 
2.39.2

[RFC PATCH 2/3] bluetooth: ti-st: add GNSS support for TI Wilink chips

[Andreas Kemnade]

Some of these chips have GNSS support. GNSS support
is available through channel 9 whilst FM is through channel 8.
Add support for it.
A driver providing a /dev/tigps device is found in some vendor-kernels. The
GNSS device provided by this patch seems to be compatible.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 drivers/bluetooth/hci_ll.c | 154 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 153 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index 4a0b5c3160c2b..718dab0f529a7 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -39,6 +39,7 @@
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
+#include <linux/gnss.h>
 #include <linux/gpio/consumer.h>
 #include <linux/nvmem-consumer.h>
 
@@ -68,6 +69,10 @@ struct ll_device {
 	struct gpio_desc *enable_gpio;
 	struct clk *ext_clk;
 	bdaddr_t bdaddr;
+
+	struct mutex gdev_mutex;
+	bool gdev_open;
+	struct gnss_device *gdev;
 };
 
 struct ll_struct {
@@ -78,6 +83,20 @@ struct ll_struct {
 	struct sk_buff_head tx_wait_q;	/* HCILL wait queue	*/
 };
 
+/* Channel-9 details for GPS */
+#define GPS_CH9_PKT_HDR_SIZE		4
+#define GPS_CH9_PKT_NUMBER		0x9
+#define GPS_CH9_OP_WRITE		0x1
+#define GPS_CH9_OP_READ			0x2
+#define GPS_CH9_OP_COMPLETED_EVT	0x3
+
+struct gnssdrv_event_hdr {
+	uint8_t opcode;
+	uint16_t plen;
+} __packed;
+
+
+static int gnss_recv_frame(struct hci_dev *hdev, struct sk_buff *skb);
 /*
  * Builds and sends an HCILL command packet.
  * These are very simple packets with only 1 cmd byte
@@ -411,6 +430,13 @@ static int ll_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
 	.lsize = 0, \
 	.maxlen = 0
 
+#define LL_RECV_GNSS \
+	.type = GPS_CH9_PKT_NUMBER, \
+	.hlen = 3, \
+	.loff = 1, \
+	.lsize = 2 \
+
+
 static const struct h4_recv_pkt ll_recv_pkts[] = {
 	{ H4_RECV_ACL,       .recv = hci_recv_frame },
 	{ H4_RECV_SCO,       .recv = hci_recv_frame },
@@ -419,6 +445,7 @@ static const struct h4_recv_pkt ll_recv_pkts[] = {
 	{ LL_RECV_SLEEP_ACK, .recv = ll_recv_frame  },
 	{ LL_RECV_WAKE_IND,  .recv = ll_recv_frame  },
 	{ LL_RECV_WAKE_ACK,  .recv = ll_recv_frame  },
+	{ LL_RECV_GNSS,      .recv = gnss_recv_frame },
 };
 
 /* Recv data */
@@ -682,12 +709,124 @@ static int ll_setup(struct hci_uart *hu)
 
 static const struct hci_uart_proto llp;
 
+static int gnss_lldev_open(struct gnss_device *gdev)
+{
+	struct ll_device *lldev = gnss_get_drvdata(gdev);
+
+	mutex_lock(&lldev->gdev_mutex);
+	lldev->gdev_open = true;
+	mutex_unlock(&lldev->gdev_mutex);
+
+	return 0;
+}
+
+static void gnss_lldev_close(struct gnss_device *gdev)
+{
+	struct ll_device *lldev = gnss_get_drvdata(gdev);
+
+	mutex_lock(&lldev->gdev_mutex);
+	lldev->gdev_open = false;
+	mutex_unlock(&lldev->gdev_mutex);
+}
+
+static int gnss_lldev_write_raw(struct gnss_device *gdev,
+		const unsigned char *buf, size_t count)
+{
+	struct ll_device *lldev = gnss_get_drvdata(gdev);
+	int err = 0;
+	struct sk_buff *skb = NULL;
+	struct gnssdrv_event_hdr gnssdrv_hdr = { GPS_CH9_OP_WRITE, 0x0000 };
+
+	/* allocate packet */
+	skb = bt_skb_alloc(sizeof(gnssdrv_hdr) + count, GFP_ATOMIC);
+	if (!skb) {
+		BT_ERR("cannot allocate memory for HCILL packet");
+		err = -ENOMEM;
+		goto out;
+	}
+
+	/* GPS channel */
+
+	gnssdrv_hdr.plen = count;
+	hci_skb_pkt_type(skb) = GPS_CH9_PKT_NUMBER;
+	skb_put_data(skb, &gnssdrv_hdr, sizeof(gnssdrv_hdr));
+	skb_put_data(skb, buf, count);
+
+	lldev->hu.hdev->send(lldev->hu.hdev, skb);
+
+	/* TODO: wait on completion? */
+	return count;
+out:
+	return err;
+}
+
+static const struct gnss_operations gnss_lldev_ops = {
+	.open		= gnss_lldev_open,
+	.close		= gnss_lldev_close,
+	.write_raw	= gnss_lldev_write_raw,
+};
+
+static int gnss_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_uart *hu = hci_get_drvdata(hdev);
+	struct ll_device *lldev = container_of(hu, struct ll_device, hu);
+
+	if (!IS_ENABLED(CONFIG_GNSS))
+		return 0;
+
+	if (!lldev->gdev)
+		return 0;
+
+	if (hci_skb_pkt_type(skb) == GPS_CH9_PKT_NUMBER) {
+		struct gnssdrv_event_hdr *gnss_hdr =
+			(struct gnssdrv_event_hdr *)skb->data;
+		void *data = skb_pull(skb, sizeof(*gnss_hdr));
+		/*
+		 * REVISIT: maybe do something with the completed
+		 * event
+		 */
+		if (gnss_hdr->opcode ==	GPS_CH9_OP_READ) {
+			mutex_lock(&lldev->gdev_mutex);
+			if (lldev->gdev_open)
+				gnss_insert_raw(lldev->gdev, data, skb->len);
+			mutex_unlock(&lldev->gdev_mutex);
+		}
+	}
+	kfree_skb(skb);
+
+	return 0;
+}
+
+static int ll_gnss_register(struct ll_device *lldev)
+{
+	struct gnss_device *gdev;
+	int ret;
+
+	gdev = gnss_allocate_device(&lldev->serdev->dev);
+	if (!gdev)
+		return -ENOMEM;
+
+	gdev->ops = &gnss_lldev_ops;
+	gdev->type = GNSS_TYPE_AI2;
+	gnss_set_drvdata(gdev, lldev);
+	mutex_init(&lldev->gdev_mutex);
+
+	ret = gnss_register_device(gdev);
+	if (ret == 0) {
+		lldev->gdev = gdev;
+		return 0;
+	}
+	gnss_put_device(gdev);
+	return ret;
+}
+
 static int hci_ti_probe(struct serdev_device *serdev)
 {
 	struct hci_uart *hu;
 	struct ll_device *lldev;
 	struct nvmem_cell *bdaddr_cell;
 	u32 max_speed = 3000000;
+	int ret;
 
 	lldev = devm_kzalloc(&serdev->dev, sizeof(struct ll_device), GFP_KERNEL);
 	if (!lldev)
@@ -756,14 +895,27 @@ static int hci_ti_probe(struct serdev_device *serdev)
 		kfree(bdaddr);
 	}
 
-	return hci_uart_register_device(hu, &llp);
+	ret = hci_uart_register_device(hu, &llp);
+	if (ret)
+		return ret;
+
+	if (IS_ENABLED(CONFIG_GNSS) &&
+	    strstr(of_node_full_name(serdev->dev.of_node), "gnss"))
+		ll_gnss_register(lldev);
+
+	return 0;
 }
 
+
 static void hci_ti_remove(struct serdev_device *serdev)
 {
 	struct ll_device *lldev = serdev_device_get_drvdata(serdev);
 
 	hci_uart_unregister_device(&lldev->hu);
+	if (IS_ENABLED(CONFIG_GNSS) && lldev->gdev) {
+		gnss_deregister_device(lldev->gdev);
+		gnss_put_device(lldev->gdev);
+	}
 }
 
 static const struct of_device_id hci_ti_of_match[] = {
-- 
2.39.2

[RFC PATCH 3/3] drivers: misc: ti-st: begin to deorbit

[Andreas Kemnade]

The TI-ST driver seems not to be used anymore. For bluetooth needs
there is hci_ll.c which has device tree support and can work without
this one. Also firmware download support is there, so it is also not needed
here. GPS can also reuse parts of the framework in hci_ll

Contrary from this driver, device tree support has been removed.

So start deorbiting it by marking it as broken.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 drivers/misc/ti-st/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/ti-st/Kconfig b/drivers/misc/ti-st/Kconfig
index 1503a6496f632..6bf9cc845745c 100644
--- a/drivers/misc/ti-st/Kconfig
+++ b/drivers/misc/ti-st/Kconfig
@@ -7,7 +7,7 @@ menu "Texas Instruments shared transport line discipline"
 config TI_ST
 	tristate "Shared transport core driver"
 	depends on NET && TTY
-	depends on GPIOLIB || COMPILE_TEST
+	depends on GPIOLIB || COMPILE_TEST || BROKEN
 	select FW_LOADER
 	help
 	  This enables the shared transport core driver for TI
-- 
2.39.2

RE: bluetooth/gnss: GNSS support for TiWi chips

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2580 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=804327

---Test result---

Test Summary:
CheckPatch                    PASS      1.97 seconds
GitLint                       FAIL      1.16 seconds
SubjectPrefix                 FAIL      0.59 seconds
BuildKernel                   PASS      27.72 seconds
CheckAllWarning               PASS      30.52 seconds
CheckSparse                   PASS      36.15 seconds
CheckSmatch                   PASS      100.53 seconds
BuildKernel32                 PASS      26.99 seconds
TestRunnerSetup               PASS      419.70 seconds
TestRunner_l2cap-tester       PASS      23.19 seconds
TestRunner_iso-tester         PASS      41.07 seconds
TestRunner_bnep-tester        PASS      7.19 seconds
TestRunner_mgmt-tester        FAIL      168.76 seconds
TestRunner_rfcomm-tester      PASS      11.13 seconds
TestRunner_sco-tester         PASS      18.11 seconds
TestRunner_ioctl-tester       PASS      12.20 seconds
TestRunner_mesh-tester        PASS      9.08 seconds
TestRunner_smp-tester         PASS      10.00 seconds
TestRunner_userchan-tester    PASS      7.50 seconds
IncrementalBuild              PASS      34.80 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[RFC,1/3] gnss: Add AI2 protocol used by some TI combo chips.

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
1: T3 Title has trailing punctuation (.): "[RFC,1/3] gnss: Add AI2 protocol used by some TI combo chips."
##############################
Test: SubjectPrefix - FAIL
Desc: Check subject contains "Bluetooth" prefix
Output:
"Bluetooth: " prefix is not specified in the subject
"Bluetooth: " prefix is not specified in the subject
"Bluetooth: " prefix is not specified in the subject
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 497, Passed: 496 (99.8%), Failed: 1, Not Run: 0

Failed Test Cases
Pairing Acceptor - SMP over BR/EDR 1                 Timed out    2.260 seconds


---
Regards,
Linux Bluetooth

Re: [RFC PATCH 3/3] drivers: misc: ti-st: begin to deorbit

[Greg KH]

On Sun, Nov 26, 2023 at 08:18:40PM +0100, Andreas Kemnade wrote:
> The TI-ST driver seems not to be used anymore. For bluetooth needs
> there is hci_ll.c which has device tree support and can work without
> this one. Also firmware download support is there, so it is also not needed
> here. GPS can also reuse parts of the framework in hci_ll
> 
> Contrary from this driver, device tree support has been removed.
> 
> So start deorbiting it by marking it as broken.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
>  drivers/misc/ti-st/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/ti-st/Kconfig b/drivers/misc/ti-st/Kconfig
> index 1503a6496f632..6bf9cc845745c 100644
> --- a/drivers/misc/ti-st/Kconfig
> +++ b/drivers/misc/ti-st/Kconfig
> @@ -7,7 +7,7 @@ menu "Texas Instruments shared transport line discipline"
>  config TI_ST
>  	tristate "Shared transport core driver"
>  	depends on NET && TTY
> -	depends on GPIOLIB || COMPILE_TEST
> +	depends on GPIOLIB || COMPILE_TEST || BROKEN

Why not just delete it?  Why have it stick around any longer?

thanks,

greg k-h

Re: [RFC PATCH 2/3] bluetooth: ti-st: add GNSS support for TI Wilink chips

[kernel test robot]

Hi Andreas,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:

[auto build test ERROR on bluetooth/master]
[also build test ERROR on bluetooth-next/master char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.7-rc3 next-20231127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andreas-Kemnade/gnss-Add-AI2-protocol-used-by-some-TI-combo-chips/20231127-032152
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git master
patch link:    https://lore.kernel.org/r/20231126191840.110564-3-andreas%40kemnade.info
patch subject: [RFC PATCH 2/3] bluetooth: ti-st: add GNSS support for TI Wilink chips
config: x86_64-randconfig-012-20231127 (https://download.01.org/0day-ci/archive/20231127/202311271628.GORay76H-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231127/202311271628.GORay76H-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311271628.GORay76H-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: vmlinux.o: in function `gnss_recv_frame':
>> drivers/bluetooth/hci_ll.c:791: undefined reference to `gnss_insert_raw'
   ld: vmlinux.o: in function `hci_ti_remove':
>> drivers/bluetooth/hci_ll.c:916: undefined reference to `gnss_deregister_device'
>> ld: drivers/bluetooth/hci_ll.c:917: undefined reference to `gnss_put_device'
   ld: vmlinux.o: in function `ll_gnss_register':
>> drivers/bluetooth/hci_ll.c:805: undefined reference to `gnss_allocate_device'
>> ld: drivers/bluetooth/hci_ll.c:814: undefined reference to `gnss_register_device'
   ld: drivers/bluetooth/hci_ll.c:819: undefined reference to `gnss_put_device'


vim +791 drivers/bluetooth/hci_ll.c

   768	
   769	static int gnss_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
   770	{
   771		struct hci_uart *hu = hci_get_drvdata(hdev);
   772		struct ll_device *lldev = container_of(hu, struct ll_device, hu);
   773	
   774		if (!IS_ENABLED(CONFIG_GNSS))
   775			return 0;
   776	
   777		if (!lldev->gdev)
   778			return 0;
   779	
   780		if (hci_skb_pkt_type(skb) == GPS_CH9_PKT_NUMBER) {
   781			struct gnssdrv_event_hdr *gnss_hdr =
   782				(struct gnssdrv_event_hdr *)skb->data;
   783			void *data = skb_pull(skb, sizeof(*gnss_hdr));
   784			/*
   785			 * REVISIT: maybe do something with the completed
   786			 * event
   787			 */
   788			if (gnss_hdr->opcode ==	GPS_CH9_OP_READ) {
   789				mutex_lock(&lldev->gdev_mutex);
   790				if (lldev->gdev_open)
 > 791					gnss_insert_raw(lldev->gdev, data, skb->len);
   792				mutex_unlock(&lldev->gdev_mutex);
   793			}
   794		}
   795		kfree_skb(skb);
   796	
   797		return 0;
   798	}
   799	
   800	static int ll_gnss_register(struct ll_device *lldev)
   801	{
   802		struct gnss_device *gdev;
   803		int ret;
   804	
 > 805		gdev = gnss_allocate_device(&lldev->serdev->dev);
   806		if (!gdev)
   807			return -ENOMEM;
   808	
   809		gdev->ops = &gnss_lldev_ops;
   810		gdev->type = GNSS_TYPE_AI2;
   811		gnss_set_drvdata(gdev, lldev);
   812		mutex_init(&lldev->gdev_mutex);
   813	
 > 814		ret = gnss_register_device(gdev);
   815		if (ret == 0) {
   816			lldev->gdev = gdev;
   817			return 0;
   818		}
   819		gnss_put_device(gdev);
   820		return ret;
   821	}
   822	
   823	static int hci_ti_probe(struct serdev_device *serdev)
   824	{
   825		struct hci_uart *hu;
   826		struct ll_device *lldev;
   827		struct nvmem_cell *bdaddr_cell;
   828		u32 max_speed = 3000000;
   829		int ret;
   830	
   831		lldev = devm_kzalloc(&serdev->dev, sizeof(struct ll_device), GFP_KERNEL);
   832		if (!lldev)
   833			return -ENOMEM;
   834		hu = &lldev->hu;
   835	
   836		serdev_device_set_drvdata(serdev, lldev);
   837		lldev->serdev = hu->serdev = serdev;
   838	
   839		lldev->enable_gpio = devm_gpiod_get_optional(&serdev->dev,
   840							     "enable",
   841							     GPIOD_OUT_LOW);
   842		if (IS_ERR(lldev->enable_gpio))
   843			return PTR_ERR(lldev->enable_gpio);
   844	
   845		lldev->ext_clk = devm_clk_get(&serdev->dev, "ext_clock");
   846		if (IS_ERR(lldev->ext_clk) && PTR_ERR(lldev->ext_clk) != -ENOENT)
   847			return PTR_ERR(lldev->ext_clk);
   848	
   849		of_property_read_u32(serdev->dev.of_node, "max-speed", &max_speed);
   850		hci_uart_set_speeds(hu, 115200, max_speed);
   851	
   852		/* optional BD address from nvram */
   853		bdaddr_cell = nvmem_cell_get(&serdev->dev, "bd-address");
   854		if (IS_ERR(bdaddr_cell)) {
   855			int err = PTR_ERR(bdaddr_cell);
   856	
   857			if (err == -EPROBE_DEFER)
   858				return err;
   859	
   860			/* ENOENT means there is no matching nvmem cell and ENOSYS
   861			 * means that nvmem is not enabled in the kernel configuration.
   862			 */
   863			if (err != -ENOENT && err != -ENOSYS) {
   864				/* If there was some other error, give userspace a
   865				 * chance to fix the problem instead of failing to load
   866				 * the driver. Using BDADDR_NONE as a flag that is
   867				 * tested later in the setup function.
   868				 */
   869				dev_warn(&serdev->dev,
   870					 "Failed to get \"bd-address\" nvmem cell (%d)\n",
   871					 err);
   872				bacpy(&lldev->bdaddr, BDADDR_NONE);
   873			}
   874		} else {
   875			bdaddr_t *bdaddr;
   876			size_t len;
   877	
   878			bdaddr = nvmem_cell_read(bdaddr_cell, &len);
   879			nvmem_cell_put(bdaddr_cell);
   880			if (IS_ERR(bdaddr)) {
   881				dev_err(&serdev->dev, "Failed to read nvmem bd-address\n");
   882				return PTR_ERR(bdaddr);
   883			}
   884			if (len != sizeof(bdaddr_t)) {
   885				dev_err(&serdev->dev, "Invalid nvmem bd-address length\n");
   886				kfree(bdaddr);
   887				return -EINVAL;
   888			}
   889	
   890			/* As per the device tree bindings, the value from nvmem is
   891			 * expected to be MSB first, but in the kernel it is expected
   892			 * that bdaddr_t is LSB first.
   893			 */
   894			baswap(&lldev->bdaddr, bdaddr);
   895			kfree(bdaddr);
   896		}
   897	
   898		ret = hci_uart_register_device(hu, &llp);
   899		if (ret)
   900			return ret;
   901	
   902		if (IS_ENABLED(CONFIG_GNSS) &&
   903		    strstr(of_node_full_name(serdev->dev.of_node), "gnss"))
   904			ll_gnss_register(lldev);
   905	
   906		return 0;
   907	}
   908	
   909	
   910	static void hci_ti_remove(struct serdev_device *serdev)
   911	{
   912		struct ll_device *lldev = serdev_device_get_drvdata(serdev);
   913	
   914		hci_uart_unregister_device(&lldev->hu);
   915		if (IS_ENABLED(CONFIG_GNSS) && lldev->gdev) {
 > 916			gnss_deregister_device(lldev->gdev);
 > 917			gnss_put_device(lldev->gdev);
   918		}
   919	}
   920	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [RFC PATCH 3/3] drivers: misc: ti-st: begin to deorbit

[Tony Lindgren]

* Greg KH <gregkh@linuxfoundation.org> [231127 08:25]:
> On Sun, Nov 26, 2023 at 08:18:40PM +0100, Andreas Kemnade wrote:
> > The TI-ST driver seems not to be used anymore. For bluetooth needs
> > there is hci_ll.c which has device tree support and can work without
> > this one. Also firmware download support is there, so it is also not needed
> > here. GPS can also reuse parts of the framework in hci_ll
> > 
> > Contrary from this driver, device tree support has been removed.
> > 
> > So start deorbiting it by marking it as broken.
> > 
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> >  drivers/misc/ti-st/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/misc/ti-st/Kconfig b/drivers/misc/ti-st/Kconfig
> > index 1503a6496f632..6bf9cc845745c 100644
> > --- a/drivers/misc/ti-st/Kconfig
> > +++ b/drivers/misc/ti-st/Kconfig
> > @@ -7,7 +7,7 @@ menu "Texas Instruments shared transport line discipline"
> >  config TI_ST
> >  	tristate "Shared transport core driver"
> >  	depends on NET && TTY
> > -	depends on GPIOLIB || COMPILE_TEST
> > +	depends on GPIOLIB || COMPILE_TEST || BROKEN
> 
> Why not just delete it?  Why have it stick around any longer?

Sounds good to me too.

Regards,

Tony

Re: [RFC PATCH 0/3] bluetooth/gnss: GNSS support for TiWi chips

[Tony Lindgren]

* Andreas Kemnade <andreas@kemnade.info> [231126 19:18]:
> So the main questions I see:
> - is the approach right to abandon drivers/misc/ti-st?

Yes.

> - Output at /dev/gnssX:
>   AI2 vs. NMEA
>   The chip can be configured into sending AI2-encapsulated NMEA,
>   or proving data in a binary format.
>   Some research has to be done yet for the details.
>   A pile of logs is waiting for further analysis...
> 
>   Arguments for/against NMEA:
>   + Userspace is prepared to handle it
>   + Power management can be easily done by the kernel
>   - Less functionality can be used.

I'd go with NMEA format as the default setting :)

>   Arguments for/against AI2:
>   + Full functionality can be accessed from userspace (incl. A-GPS,
>     maybe raw satellite data)
>   - Userspace has to behave to have proper power management
>   - No freely (not even as in beer) tool available to fully use AI2,
>     so there will be only a real advantage after long "French Cafe"
>     sessions.

Seems AI2 could be optionally enabled as needed with some writes
to /dev/gnss0 to change the mode?

Regards,

Tony

Re: [RFC PATCH 0/3] bluetooth/gnss: GNSS support for TiWi chips

[Adam Ford]

On Sun, Nov 26, 2023 at 1:47 PM Andreas Kemnade <andreas@kemnade.info> wrote:
>
> Some of these chips have GNSS support. In some vendor kernels
> a driver on top of misc/ti-st can be found providing a /dev/tigps
> device which speaks the secretive Air Independent Interface (AI2) protocol.
> Implement something comparable as a GNSS interface.
>
> With some userspace tools a proof-of-concept can be shown. A position
> can be successfully read out.  Basic properties of the protocol are
> understood.
>
> This was tested on the Epson Moverio BT-200.

Can you tell me which WiLink chip this uses?

I'd like to try it on the WL1283, but I want to understand which
WiLink chips you're targeting.

adam
>
> This is sent out as an early RFC to ensure I am going onto the right
> track:
>
> So the main questions I see:
> - is the approach right to abandon drivers/misc/ti-st?
>
> - Output at /dev/gnssX:
>   AI2 vs. NMEA
>   The chip can be configured into sending AI2-encapsulated NMEA,
>   or proving data in a binary format.
>   Some research has to be done yet for the details.
>   A pile of logs is waiting for further analysis...
>
>   Arguments for/against NMEA:
>   + Userspace is prepared to handle it
>   + Power management can be easily done by the kernel
>   - Less functionality can be used.
>
>   Arguments for/against AI2:
>   + Full functionality can be accessed from userspace (incl. A-GPS,
>     maybe raw satellite data)
>   - Userspace has to behave to have proper power management
>   - No freely (not even as in beer) tool available to fully use AI2,
>     so there will be only a real advantage after long "French Cafe"
>     sessions.
>
> More detailed tings:
>   - Some live cycle management is left out. Since it depends
>     on the decisions above, I have not put much thought into it.
>   - Should some pieces go into drivers/gnss?
>   - detection for GNSS availability: For now the node name is
>     used. But the device should be there if the chip supports it
>     and things are wired up properly.
>
> Andreas Kemnade (3):
>   gnss: Add AI2 protocol used by some TI combo chips.
>   bluetooth: ti-st: add GNSS support for TI Wilink chips
>   drivers: misc: ti-st: begin to deorbit
>
>  drivers/bluetooth/hci_ll.c | 154 ++++++++++++++++++++++++++++++++++++-
>  drivers/gnss/core.c        |   1 +
>  drivers/misc/ti-st/Kconfig |   2 +-
>  include/linux/gnss.h       |   1 +
>  4 files changed, 156 insertions(+), 2 deletions(-)
>
> --
> 2.39.2
>
>

Re: [RFC PATCH 0/3] bluetooth/gnss: GNSS support for TiWi chips

[Andreas Kemnade]

On Mon, 27 Nov 2023 08:03:24 -0600
Adam Ford <aford173@gmail.com> wrote:

> On Sun, Nov 26, 2023 at 1:47 PM Andreas Kemnade <andreas@kemnade.info> wrote:
> >
> > Some of these chips have GNSS support. In some vendor kernels
> > a driver on top of misc/ti-st can be found providing a /dev/tigps
> > device which speaks the secretive Air Independent Interface (AI2) protocol.
> > Implement something comparable as a GNSS interface.
> >
> > With some userspace tools a proof-of-concept can be shown. A position
> > can be successfully read out.  Basic properties of the protocol are
> > understood.
> >
> > This was tested on the Epson Moverio BT-200.  
> 
> Can you tell me which WiLink chip this uses?
> 
> I'd like to try it on the WL1283, but I want to understand which
> WiLink chips you're targeting.
> 
I think, it is a WL1283 here, too.

If you want to play around with it now:
- set the devicetree node name to bluetooth-gnss
- for testing you can use the read-gps program at https://github.com/akemnade/bt200tools

Regards,
Andreas

Re: [RFC PATCH 0/3] bluetooth/gnss: GNSS support for TiWi chips

[Andreas Kemnade]

Hi,

On Mon, 27 Nov 2023 15:54:24 +0200
Tony Lindgren <tony@atomide.com> wrote:

[...]
> > - Output at /dev/gnssX:
> >   AI2 vs. NMEA
> >   The chip can be configured into sending AI2-encapsulated NMEA,
> >   or proving data in a binary format.
> >   Some research has to be done yet for the details.
> >   A pile of logs is waiting for further analysis...
> > 
> >   Arguments for/against NMEA:
> >   + Userspace is prepared to handle it
> >   + Power management can be easily done by the kernel
> >   - Less functionality can be used.  
> 
> I'd go with NMEA format as the default setting :)
> 
yes, that would also be my preference.

> >   Arguments for/against AI2:
> >   + Full functionality can be accessed from userspace (incl. A-GPS,
> >     maybe raw satellite data)
> >   - Userspace has to behave to have proper power management
> >   - No freely (not even as in beer) tool available to fully use AI2,
> >     so there will be only a real advantage after long "French Cafe"
> >     sessions.  
> 
> Seems AI2 could be optionally enabled as needed with some writes
> to /dev/gnss0 to change the mode?

Hmm, we have
/sys/class/gnss/gnss0/type to get the mode, maybe we add some file
to change the mode? Or having it hidden behing a module parameter
and implement something better accessible if any need arrives?

If we want NMEA output, probably some init commands will be sent at
open(), but userspace doing something with AI2 probably does not want
the device touched, so it should probably be already be set before open().

Regards,
Andreas

Re: [RFC PATCH 0/3] bluetooth/gnss: GNSS support for TiWi chips

[Adam Ford]

On Mon, Nov 27, 2023 at 2:51 PM Andreas Kemnade <andreas@kemnade.info> wrote:
>
> Hi,
>
> On Mon, 27 Nov 2023 15:54:24 +0200
> Tony Lindgren <tony@atomide.com> wrote:
>
> [...]
> > > - Output at /dev/gnssX:
> > >   AI2 vs. NMEA
> > >   The chip can be configured into sending AI2-encapsulated NMEA,
> > >   or proving data in a binary format.
> > >   Some research has to be done yet for the details.
> > >   A pile of logs is waiting for further analysis...
> > >
> > >   Arguments for/against NMEA:
> > >   + Userspace is prepared to handle it
> > >   + Power management can be easily done by the kernel
> > >   - Less functionality can be used.
> >
> > I'd go with NMEA format as the default setting :)
> >
> yes, that would also be my preference.
>
> > >   Arguments for/against AI2:
> > >   + Full functionality can be accessed from userspace (incl. A-GPS,
> > >     maybe raw satellite data)
> > >   - Userspace has to behave to have proper power management
> > >   - No freely (not even as in beer) tool available to fully use AI2,
> > >     so there will be only a real advantage after long "French Cafe"
> > >     sessions.
> >
> > Seems AI2 could be optionally enabled as needed with some writes
> > to /dev/gnss0 to change the mode?
>
> Hmm, we have
> /sys/class/gnss/gnss0/type to get the mode, maybe we add some file
> to change the mode? Or having it hidden behing a module parameter
> and implement something better accessible if any need arrives?
>
> If we want NMEA output, probably some init commands will be sent at
> open(), but userspace doing something with AI2 probably does not want
> the device touched, so it should probably be already be set before open().

Is there another revision coming or should I try to test this one?  I
am not very familiar with the GNSS part of the module, but it sounds
like there was some consensus as to which direction we should go.  I
should have a little time this weekend.

thanks

adam
>
> Regards,
> Andreas
>

Re: [RFC PATCH 0/3] bluetooth/gnss: GNSS support for TiWi chips

[Johan Hovold]

On Mon, Nov 27, 2023 at 09:51:08PM +0100, Andreas Kemnade wrote:
> On Mon, 27 Nov 2023 15:54:24 +0200
> Tony Lindgren <tony@atomide.com> wrote:

> > > - Output at /dev/gnssX:
> > >   AI2 vs. NMEA
> > >   The chip can be configured into sending AI2-encapsulated NMEA,
> > >   or proving data in a binary format.
> > >   Some research has to be done yet for the details.
> > >   A pile of logs is waiting for further analysis...

Can you say something more about what the protocol looks like? Is there
some common framing that can/should be stripped by the driver in both
modes?

> > > 
> > >   Arguments for/against NMEA:
> > >   + Userspace is prepared to handle it
> > >   + Power management can be easily done by the kernel
> > >   - Less functionality can be used.  
> > 
> > I'd go with NMEA format as the default setting :)
> > 
> yes, that would also be my preference.
> 
> > >   Arguments for/against AI2:
> > >   + Full functionality can be accessed from userspace (incl. A-GPS,
> > >     maybe raw satellite data)
> > >   - Userspace has to behave to have proper power management
> > >   - No freely (not even as in beer) tool available to fully use AI2,
> > >     so there will be only a real advantage after long "French Cafe"
> > >     sessions.  
> > 
> > Seems AI2 could be optionally enabled as needed with some writes
> > to /dev/gnss0 to change the mode?
> 
> Hmm, we have
> /sys/class/gnss/gnss0/type to get the mode, maybe we add some file
> to change the mode? Or having it hidden behing a module parameter
> and implement something better accessible if any need arrives?

The 'type' attribute is intended to reveal the GNSS receiver type
(class) as a hint to user space to avoid having to detect it at runtime
using heuristics.

It does not reflect which mode is currently active for receivers that
provide both a vendor specific protocol and NMEA (e.g. u-blox
receivers).

User space can currently switch modes at will by writing to /dev/gnss0
as Tony mentioned.

It may or may not make sense to make sure a particular mode is set
during probe, for example, if there's no real use for the proprietary
protocol and everyone would just switch away from it immediately.

Johan

Re: [RFC PATCH 0/3] bluetooth/gnss: GNSS support for TiWi chips

[Andreas Kemnade]

Hi,

On Fri, 8 Dec 2023 08:39:21 -0600
Adam Ford <aford173@gmail.com> wrote:

> On Mon, Nov 27, 2023 at 2:51 PM Andreas Kemnade <andreas@kemnade.info> wrote:
> >
> > Hi,
> >
> > On Mon, 27 Nov 2023 15:54:24 +0200
> > Tony Lindgren <tony@atomide.com> wrote:
> >
> > [...]  
> > > > - Output at /dev/gnssX:
> > > >   AI2 vs. NMEA
> > > >   The chip can be configured into sending AI2-encapsulated NMEA,
> > > >   or proving data in a binary format.
> > > >   Some research has to be done yet for the details.
> > > >   A pile of logs is waiting for further analysis...
> > > >
> > > >   Arguments for/against NMEA:
> > > >   + Userspace is prepared to handle it
> > > >   + Power management can be easily done by the kernel
> > > >   - Less functionality can be used.  
> > >
> > > I'd go with NMEA format as the default setting :)
> > >  
> > yes, that would also be my preference.
> >  
> > > >   Arguments for/against AI2:
> > > >   + Full functionality can be accessed from userspace (incl. A-GPS,
> > > >     maybe raw satellite data)
> > > >   - Userspace has to behave to have proper power management
> > > >   - No freely (not even as in beer) tool available to fully use AI2,
> > > >     so there will be only a real advantage after long "French Cafe"
> > > >     sessions.  
> > >
> > > Seems AI2 could be optionally enabled as needed with some writes
> > > to /dev/gnss0 to change the mode?  
> >
> > Hmm, we have
> > /sys/class/gnss/gnss0/type to get the mode, maybe we add some file
> > to change the mode? Or having it hidden behing a module parameter
> > and implement something better accessible if any need arrives?
> >
> > If we want NMEA output, probably some init commands will be sent at
> > open(), but userspace doing something with AI2 probably does not want
> > the device touched, so it should probably be already be set before open().  
> 
> Is there another revision coming or should I try to test this one?  I
> am not very familiar with the GNSS part of the module, but it sounds
> like there was some consensus as to which direction we should go.  I
> should have a little time this weekend.
> 
Progress is only in my test program. No more checksum errors, I have
made some progress in protocol reverse engineering. So make sure
you do a git pull for https://github.com/akemnade/bt200tools

Regards,
Andreas

Re: [RFC PATCH 0/3] bluetooth/gnss: GNSS support for TiWi chips

[Andreas Kemnade]

On Fri, 8 Dec 2023 17:25:27 +0100
Johan Hovold <johan@kernel.org> wrote:

> On Mon, Nov 27, 2023 at 09:51:08PM +0100, Andreas Kemnade wrote:
> > On Mon, 27 Nov 2023 15:54:24 +0200
> > Tony Lindgren <tony@atomide.com> wrote:  
> 
> > > > - Output at /dev/gnssX:
> > > >   AI2 vs. NMEA
> > > >   The chip can be configured into sending AI2-encapsulated NMEA,
> > > >   or proving data in a binary format.
> > > >   Some research has to be done yet for the details.
> > > >   A pile of logs is waiting for further analysis...  
> 
> Can you say something more about what the protocol looks like? Is there
> some common framing that can/should be stripped by the driver in both
> modes?
>
The frames (which can be fragmented into multiple gnss_recv_frame() calls)
consist of:
- some kind of start marker
- one or more tlv structures (start marker is escaped here)
- checksum
- end marker

In case of NMEA reporting we have with this patchset at /dev/gnssX or at /dev/tigps
found in the bt200 vendor kernel e.g.

1000 d3 1800 82050000 244750474c4c2c2c2c2c2c2c562c4e2a36340d0a 9f05 1003

which is:
1000 = start marker, d3 = NMEA report
1800 = length (LE)
82050000 = ms from turning on GPS (LE).
244750474c4c2c2c2c2c2c2c562c4e2a36340d0a = NMEA sentence
9f05 = checksum
1003 = end marker.

That is one report among others depending on what is enabled. So it
is not like the situtation with Sirf where you send some magic to turn
it into binary mode and some other magic turning it into NMEA mode
and have no other stuff on the line.

> > > > 
> > > >   Arguments for/against NMEA:
> > > >   + Userspace is prepared to handle it
> > > >   + Power management can be easily done by the kernel
> > > >   - Less functionality can be used.    
> > > 
> > > I'd go with NMEA format as the default setting :)
> > >   
> > yes, that would also be my preference.
> >   
> > > >   Arguments for/against AI2:
> > > >   + Full functionality can be accessed from userspace (incl. A-GPS,
> > > >     maybe raw satellite data)
> > > >   - Userspace has to behave to have proper power management
> > > >   - No freely (not even as in beer) tool available to fully use AI2,
> > > >     so there will be only a real advantage after long "French Cafe"
> > > >     sessions.    
> > > 
> > > Seems AI2 could be optionally enabled as needed with some writes
> > > to /dev/gnss0 to change the mode?  
> > 
> > Hmm, we have
> > /sys/class/gnss/gnss0/type to get the mode, maybe we add some file
> > to change the mode? Or having it hidden behing a module parameter
> > and implement something better accessible if any need arrives?  
> 
> The 'type' attribute is intended to reveal the GNSS receiver type
> (class) as a hint to user space to avoid having to detect it at runtime
> using heuristics.
> 
> It does not reflect which mode is currently active for receivers that
> provide both a vendor specific protocol and NMEA (e.g. u-blox
> receivers).
> 
> User space can currently switch modes at will by writing to /dev/gnss0
> as Tony mentioned.
> 
Well, switching mode would in this case also mean configuring something
in the driver to do something different as it would mean sending commands
to enable NMEA reports and strip away the AI2 encapsulation in the driver.
You usually do not configure drivers through write() but through some
out-of-band means like ioctl(), sysfs, kernel compile options,
module parameters.

> It may or may not make sense to make sure a particular mode is set
> during probe, for example, if there's no real use for the proprietary
> protocol and everyone would just switch away from it immediately.
> 
I would probably not do anything at probe time but turning on GPS and enable
NMEA reports at open() time.

With a lot of effort you can probably do interesting things with the
proprietary mode. But given the fact that apparently nobody has done 
publicly so in the past years, I would not expect that anything arises
suddenly.

So in practice only NMEA would IMHO be useful and having raw AI2
might be something behind a module option or compile option to avoid
introducing some new API before a real need is seen.
Also as not everybody is using gpsd anymore, implementing any support
for AI2 in userspace would not be at a single place.

Regards,
Andreas

Re: [RFC PATCH 3/3] drivers: misc: ti-st: begin to deorbit

[Andreas Kemnade]

Hi Greg,

On Mon, 27 Nov 2023 08:25:46 +0000
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Sun, Nov 26, 2023 at 08:18:40PM +0100, Andreas Kemnade wrote:
> > The TI-ST driver seems not to be used anymore. For bluetooth needs
> > there is hci_ll.c which has device tree support and can work without
> > this one. Also firmware download support is there, so it is also not needed
> > here. GPS can also reuse parts of the framework in hci_ll
> > 
> > Contrary from this driver, device tree support has been removed.
> > 
> > So start deorbiting it by marking it as broken.
> > 
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> >  drivers/misc/ti-st/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/misc/ti-st/Kconfig b/drivers/misc/ti-st/Kconfig
> > index 1503a6496f632..6bf9cc845745c 100644
> > --- a/drivers/misc/ti-st/Kconfig
> > +++ b/drivers/misc/ti-st/Kconfig
> > @@ -7,7 +7,7 @@ menu "Texas Instruments shared transport line discipline"
> >  config TI_ST
> >  	tristate "Shared transport core driver"
> >  	depends on NET && TTY
> > -	depends on GPIOLIB || COMPILE_TEST
> > +	depends on GPIOLIB || COMPILE_TEST || BROKEN  
> 
> Why not just delete it?  Why have it stick around any longer?
> 
Well, I just thought that marking something as broken and then deleting
it if no one complains would be the standard procedure. So we can
delete it now since there are obviously no users (no board files, no
device tree support)?

The logical connection between the other patches of this series is given
only by the fact that patches 1+2 are for me the proof that we do not
need that ti-st driver. 
I think since there are no in-tree users, having that proof in lkml is
enough, so we can probably remove the driver now?

I will send a separate remove patch including going with a brush through
include/linux/ti_wilink_st.h

Regards,
Andreas