[PATCH v4 0/2] support config of U-Blox Zed-F9P GNSS

[PATCH v4 0/2] support config of U-Blox Zed-F9P GNSS

[alison]

From: Alison Chaiken <alison@she-devel.com>

Add generalized support for setting arbitrary configuration of the
U-Blox Zed-F9P GNSS.  Employ the new functionality to set the baud rate
of the Zed-F9P if the devicetree specifies a non-default value.

Tested with 6.1.22, only on a U-Blox Zed-F9P GNSS.

V4 -> V3 Lookup device-specific properties by matching driver data.
V2 -> V3 Add email recipients whom I foolishly missed the first two times.
V1 -> V2 Fixes error identified by kernel test robot:
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202304300819.u0hfUj33-lkp@intel.com/

Alison Chaiken (2):
  gnss: ubx: customize serial device open to set U-Blox Zed-F9P baud
  dt-bindings: gnss: Add U-Blox Zed-F9

 .../bindings/gnss/u-blox,neo-6m.yaml          |   1 +
 drivers/gnss/ubx.c                            | 237 +++++++++++++++++-
 2 files changed, 228 insertions(+), 10 deletions(-)


base-commit: dd9e11d6477a52ede9ebe575c83285e79e823889
-- 
2.39.2

[PATCH v3 1/2] gnss: ubx: customize serial device open to set U-Blox Zed-F9P baud

[alison]

From: Alison Chaiken <achaiken@aurora.tech>

Add support for setting the baud rate of U-Blox Zed-F9P GNSS devices.
Provide functions that support writing of arbitrary configuration
messages to the device plus one that specifically configures the baud
rate.  Override the default gnss_serial_open() with a new method that
writes the configuration message to the GNSS if the devicetree declares
it to be a Zed F9P and requests a non-default baud.  Add a boolean flag
to the ubx_data private data of the GNSS driver in order to track
whether the configuration message has already been written.  Set the Zed
F9P to its default port speed if the devicetree does not specify a
value.

Signed-off-by: Alison Chaiken <achaiken@aurora.tech>
---

V4 -> V3 Modified to lookup Zed-F9P-specific data via the device_id
               rather than the compatible string.

 drivers/gnss/ubx.c | 237 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 227 insertions(+), 10 deletions(-)

diff --git a/drivers/gnss/ubx.c b/drivers/gnss/ubx.c
index c951be202ca2..b7687a928383 100644
--- a/drivers/gnss/ubx.c
+++ b/drivers/gnss/ubx.c
@@ -9,18 +9,212 @@
 #include <linux/gnss.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/kstrtox.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 #include <linux/serdev.h>
 
 #include "serial.h"
 
+/* Total configuration message length = PREAMBLE_LEN + MESSAGE_CLASS_LEN +
+ *   MESSAGE_LENGTH_LEN + payload length + CHECKSUM_LEN
+ */
+const int32_t PREAMBLE_LEN = 2;
+const int32_t MESSAGE_CLASS_LEN = 2;
+const int32_t MESSAGE_LENGTH_LEN = 2;
+const int32_t CHECKSUM_LEN = 2;
+const size_t FIRST_CONFIG_REGISTER_BYTE = 10U;
+const size_t FIRST_VALUE_BYTE = 14U;
+const size_t FIRST_CHECKSUM_BYTE = 18U;
+const size_t CFG_MSG_TOTAL_LEN = 20U;
+
+uint8_t ZED_F9P_CFG_VALSET_MSG[] = {
+	0xB5, 0x62, /* 0-1 preamble */
+	0x06, 0x8A, /* 2-3 CFG_VALSET command */
+	0x0C, 0x00, /* 4-5 payload length = 12 for one key-value pair */
+	0x00, /* 6 U-Blox API version */
+	0x01, /* 7 Write to RAM */
+	0x00, 0x00, /* 8-9 Reserved */
+	0x00, 0x00, 0x00, 0x00, /* 10-13 Placeholder for configuration register */
+	0x00, 0x00, 0x00, 0x00, /* 14-17 Placeholder for baud value */
+	0x00, 0x00 /* 18-19 Placeholder for checksum */
+};
+
+struct ubx_features {
+	u32 min_baud;
+	u32 default_baud;
+	u32 max_baud;
+	size_t baud_config_reg;
+	int (*open)(struct gnss_device *gdev);
+};
+
 struct ubx_data {
 	struct regulator *v_bckp;
 	struct regulator *vcc;
+	const struct ubx_features *features;
+	unsigned long is_configured;
 };
 
+union message_length {
+	uint16_t ml;
+	uint8_t bytes[2];
+};
+
+union int_to_bytes {
+	uint32_t int_val;
+	uint8_t bytes[4];
+};
+
+/* Payload  length is contained in bytes 0-2 after message class and ID.
+ *  While the checksum includes the Message class and ID plus message length, the
+ *  payload does not.
+ */
+static uint16_t get_payload_length(const uint8_t msg[])
+{
+	union message_length hs_msg_len;
+
+	hs_msg_len.bytes[0] = msg[PREAMBLE_LEN + MESSAGE_CLASS_LEN];
+	hs_msg_len.bytes[1] = msg[PREAMBLE_LEN + MESSAGE_CLASS_LEN + 1U];
+	return hs_msg_len.ml;
+}
+
+static int32_t get_msg_total_len(const uint8_t msg[])
+{
+	const size_t payload_len = get_payload_length(msg);
+
+	return PREAMBLE_LEN + MESSAGE_CLASS_LEN + MESSAGE_LENGTH_LEN + payload_len
+		+ CHECKSUM_LEN;
+}
+
+/* The checksum is calculated on message class, message ID, message length and
+ * payload.
+ */
+static void calc_ubx_checksum(const uint8_t msg[], uint8_t checksum[],
+			   const uint16_t total_len)
+{
+	uint8_t CK_A = 0;
+	uint8_t CK_B = 0;
+	int i;
+
+	for (i = PREAMBLE_LEN; i < (total_len - CHECKSUM_LEN); i++) {
+		CK_A += msg[i];
+		CK_B += CK_A;
+	}
+	checksum[0] = CK_A;
+	checksum[1] = CK_B;
+}
+
+static uint32_t  check_baud(speed_t speed, const struct device *dev,
+					const struct ubx_features *features)
+{
+	if ((speed < features->min_baud) || (speed > features->max_baud)) {
+		dev_warn(dev, "Baud rate specification %d out of range\n", speed);
+		speed = features->default_baud;
+	}
+	return speed;
+}
+
+static int prepare_zedf9p_config_msg(const speed_t speed,
+					const struct device *dev,
+					    const struct ubx_features *features)
+{
+	union int_to_bytes cfg_val, cfg_register;
+	int i = 0;
+	uint8_t checksum[2];
+	const size_t total_len = get_msg_total_len(ZED_F9P_CFG_VALSET_MSG);
+
+	if (total_len != CFG_MSG_TOTAL_LEN)
+		goto bad_msg;
+
+	cfg_val.int_val = check_baud(speed, dev, features);
+	cfg_register.int_val = features->baud_config_reg;
+	for (i = 0; i < 4; i++) {
+		ZED_F9P_CFG_VALSET_MSG[FIRST_VALUE_BYTE + i] = cfg_val.bytes[i];
+		ZED_F9P_CFG_VALSET_MSG[FIRST_CONFIG_REGISTER_BYTE + i] = cfg_register.bytes[i];
+	}
+	calc_ubx_checksum(ZED_F9P_CFG_VALSET_MSG, checksum, total_len);
+	ZED_F9P_CFG_VALSET_MSG[FIRST_CHECKSUM_BYTE] = checksum[0];
+	ZED_F9P_CFG_VALSET_MSG[FIRST_CHECKSUM_BYTE + 1U] = checksum[1];
+	return 0;
+
+ bad_msg:
+	dev_err(dev, "Malformed UBX-CFG-VALSET message\n");
+	return -EINVAL;
+}
+
+/* Configure the Zed F9P baud rate via the UBX-CFG-VALSET message. */
+static int set_zedf9p_baud(struct gnss_device *gdev,
+					struct serdev_device *serdev, struct gnss_serial *gserial)
+{
+	const struct ubx_data *data = gnss_serial_get_drvdata(gserial);
+	const struct ubx_features *features = data->features;
+	size_t count = 0U;
+	int ret;
+
+	if (!data->features)
+		return -EINVAL;
+	if (gserial->speed == features->default_baud)
+		return 0;
+
+	ret = prepare_zedf9p_config_msg(gserial->speed, &gdev->dev, features);
+	if (ret)
+		return ret;
+	/* Initially set the UART to the default speed to match the GNSS' power-on value. */
+	serdev_device_set_baudrate(serdev, features->default_baud);
+	/* Now set the new baud rate. */
+	count = gdev->ops->write_raw(gdev, ZED_F9P_CFG_VALSET_MSG, CFG_MSG_TOTAL_LEN);
+	if (count != CFG_MSG_TOTAL_LEN)
+		return count;
+
+	return 0;
+}
+
+static int zed_f9p_serial_open(struct gnss_device *gdev)
+{
+	struct gnss_serial *gserial = gnss_get_drvdata(gdev);
+	struct serdev_device *serdev = gserial->serdev;
+	struct ubx_data *data = gnss_serial_get_drvdata(gserial);
+	int ret;
+
+	ret = serdev_device_open(serdev);
+	if (ret)
+		return ret;
+	if (!data->features)
+		return -EINVAL;
+
+	serdev_device_set_flow_control(serdev, false);
+
+	if (!data->is_configured) {
+		/* 4800 is the default value set by gnss_serial_parse_dt() */
+		if (gserial->speed == 4800) {
+			/* Fall back instead to Zed F9P default */
+			gserial->speed = data->features->default_baud;
+		} else {
+			ret = set_zedf9p_baud(gdev, serdev, gserial);
+			if (ret)
+				return ret;
+		}
+		data->is_configured = 1;
+	}
+	serdev_device_set_baudrate(serdev, gserial->speed);
+
+	ret = pm_runtime_get_sync(&serdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&serdev->dev);
+		goto err_close;
+	}
+	return 0;
+
+err_close:
+	serdev_device_close(serdev);
+
+	return ret;
+}
+
 static int ubx_set_active(struct gnss_serial *gserial)
 {
 	struct ubx_data *data = gnss_serial_get_drvdata(gserial);
@@ -63,10 +257,30 @@ static const struct gnss_serial_ops ubx_gserial_ops = {
 	.set_power = ubx_set_power,
 };
 
+#ifdef CONFIG_OF
+static const struct ubx_features zedf9p_feats = {
+	.min_baud		=	9600,
+	.default_baud		=	38400,
+	.max_baud		=	921600,
+	.baud_config_reg	=	0x40520001,
+	.open			=	zed_f9p_serial_open,
+};
+
+static const struct of_device_id ubx_of_match[] = {
+	{ .compatible = "u-blox,neo-6m" },
+	{ .compatible = "u-blox,neo-8" },
+	{ .compatible = "u-blox,neo-m8" },
+	{ .compatible = "u-blox,zed-f9p", .data = &zedf9p_feats },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ubx_of_match);
+#endif
+
 static int ubx_probe(struct serdev_device *serdev)
 {
 	struct gnss_serial *gserial;
 	struct ubx_data *data;
+	struct gnss_operations *ubx_gnss_ops;
 	int ret;
 
 	gserial = gnss_serial_allocate(serdev, sizeof(*data));
@@ -74,12 +288,25 @@ static int ubx_probe(struct serdev_device *serdev)
 		ret = PTR_ERR(gserial);
 		return ret;
 	}
+	ubx_gnss_ops = kzalloc(sizeof(struct gnss_operations), GFP_KERNEL);
+	if (IS_ERR(ubx_gnss_ops)) {
+		ret = PTR_ERR(ubx_gnss_ops);
+		return ret;
+	}
 
 	gserial->ops = &ubx_gserial_ops;
 
 	gserial->gdev->type = GNSS_TYPE_UBX;
 
 	data = gnss_serial_get_drvdata(gserial);
+	data->is_configured = 0;
+	data->features = of_match_device(ubx_of_match, &serdev->dev)->data;
+	if (data->features && data->features->open) {
+		ubx_gnss_ops->open  = data->features->open;
+		ubx_gnss_ops->close = gserial->gdev->ops->close;
+		ubx_gnss_ops->write_raw = gserial->gdev->ops->write_raw;
+		gserial->gdev->ops = ubx_gnss_ops;
+	}
 
 	data->vcc = devm_regulator_get(&serdev->dev, "vcc");
 	if (IS_ERR(data->vcc)) {
@@ -128,16 +355,6 @@ static void ubx_remove(struct serdev_device *serdev)
 	gnss_serial_free(gserial);
 }
 
-#ifdef CONFIG_OF
-static const struct of_device_id ubx_of_match[] = {
-	{ .compatible = "u-blox,neo-6m" },
-	{ .compatible = "u-blox,neo-8" },
-	{ .compatible = "u-blox,neo-m8" },
-	{},
-};
-MODULE_DEVICE_TABLE(of, ubx_of_match);
-#endif
-
 static struct serdev_device_driver ubx_driver = {
 	.driver	= {
 		.name		= "gnss-ubx",
-- 
2.39.2

[PATCH 2/2] dt-bindings: gnss: Add U-Blox Zed-F9

[alison]

From: Alison Chaiken <achaiken@aurora.tech>

Add support for the U-Blox Zed-F9P GNSS device.

Signed-off-by: Alison Chaiken <achaiken@aurora.tech>
---
 Documentation/devicetree/bindings/gnss/u-blox,neo-6m.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/gnss/u-blox,neo-6m.yaml b/Documentation/devicetree/bindings/gnss/u-blox,neo-6m.yaml
index 4835a280b3bf..86b65d4d9266 100644
--- a/Documentation/devicetree/bindings/gnss/u-blox,neo-6m.yaml
+++ b/Documentation/devicetree/bindings/gnss/u-blox,neo-6m.yaml
@@ -21,6 +21,7 @@ properties:
       - u-blox,neo-6m
       - u-blox,neo-8
       - u-blox,neo-m8
+      - u-blox,zed-f9p
 
   reg:
     description: >
-- 
2.39.2

Re: [PATCH v3 1/2] gnss: ubx: customize serial device open to set U-Blox Zed-F9P baud

[kernel test robot]

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.3 next-20230505]
[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/alison-she-devel-com/dt-bindings-gnss-Add-U-Blox-Zed-F9/20230507-075854
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20230506225849.2752103-2-alison%40she-devel.com
patch subject: [PATCH v3 1/2] gnss: ubx: customize serial device open to set U-Blox Zed-F9P baud
config: i386-randconfig-a001 (https://download.01.org/0day-ci/archive/20230507/202305070938.8vWQFfIQ-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/637fad08b78d20ec3f1c07e6e37dc97f85eac7b9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review alison-she-devel-com/dt-bindings-gnss-Add-U-Blox-Zed-F9/20230507-075854
        git checkout 637fad08b78d20ec3f1c07e6e37dc97f85eac7b9
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 olddefconfig
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305070938.8vWQFfIQ-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/gnss/ubx.c: In function 'ubx_probe':
>> drivers/gnss/ubx.c:303:42: error: 'ubx_of_match' undeclared (first use in this function)
     303 |         data->features = of_match_device(ubx_of_match, &serdev->dev)->data;
         |                                          ^~~~~~~~~~~~
   drivers/gnss/ubx.c:303:42: note: each undeclared identifier is reported only once for each function it appears in
   At top level:
   drivers/gnss/ubx.c:176:12: warning: 'zed_f9p_serial_open' defined but not used [-Wunused-function]
     176 | static int zed_f9p_serial_open(struct gnss_device *gdev)
         |            ^~~~~~~~~~~~~~~~~~~


vim +/ubx_of_match +303 drivers/gnss/ubx.c

   278	
   279	static int ubx_probe(struct serdev_device *serdev)
   280	{
   281		struct gnss_serial *gserial;
   282		struct ubx_data *data;
   283		struct gnss_operations *ubx_gnss_ops;
   284		int ret;
   285	
   286		gserial = gnss_serial_allocate(serdev, sizeof(*data));
   287		if (IS_ERR(gserial)) {
   288			ret = PTR_ERR(gserial);
   289			return ret;
   290		}
   291		ubx_gnss_ops = kzalloc(sizeof(struct gnss_operations), GFP_KERNEL);
   292		if (IS_ERR(ubx_gnss_ops)) {
   293			ret = PTR_ERR(ubx_gnss_ops);
   294			return ret;
   295		}
   296	
   297		gserial->ops = &ubx_gserial_ops;
   298	
   299		gserial->gdev->type = GNSS_TYPE_UBX;
   300	
   301		data = gnss_serial_get_drvdata(gserial);
   302		data->is_configured = 0;
 > 303		data->features = of_match_device(ubx_of_match, &serdev->dev)->data;
   304		if (data->features && data->features->open) {
   305			ubx_gnss_ops->open  = data->features->open;
   306			ubx_gnss_ops->close = gserial->gdev->ops->close;
   307			ubx_gnss_ops->write_raw = gserial->gdev->ops->write_raw;
   308			gserial->gdev->ops = ubx_gnss_ops;
   309		}
   310	
   311		data->vcc = devm_regulator_get(&serdev->dev, "vcc");
   312		if (IS_ERR(data->vcc)) {
   313			ret = PTR_ERR(data->vcc);
   314			goto err_free_gserial;
   315		}
   316	
   317		data->v_bckp = devm_regulator_get_optional(&serdev->dev, "v-bckp");
   318		if (IS_ERR(data->v_bckp)) {
   319			ret = PTR_ERR(data->v_bckp);
   320			if (ret == -ENODEV)
   321				data->v_bckp = NULL;
   322			else
   323				goto err_free_gserial;
   324		}
   325	
   326		if (data->v_bckp) {
   327			ret = regulator_enable(data->v_bckp);
   328			if (ret)
   329				goto err_free_gserial;
   330		}
   331	
   332		ret = gnss_serial_register(gserial);
   333		if (ret)
   334			goto err_disable_v_bckp;
   335	
   336		return 0;
   337	
   338	err_disable_v_bckp:
   339		if (data->v_bckp)
   340			regulator_disable(data->v_bckp);
   341	err_free_gserial:
   342		gnss_serial_free(gserial);
   343	
   344		return ret;
   345	}
   346	

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