[PATCH net-next v12 04/14] dpll: zl3073x: Add support for devlink device info

Ivan Vecera posted 14 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH net-next v12 04/14] dpll: zl3073x: Add support for devlink device info
Posted by Ivan Vecera 3 months, 1 week ago
Use devlink_alloc() to allocate zl3073x_dev structure, register
the device as a devlink device, and add devlink callback to provide
device info.

Sample output:
 # devlink dev info
 i2c/1-0070:
   driver zl3073x-i2c
   versions:
       fixed:
         asic.id 1E94
         asic.rev 300
       running:
         fw 7006

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 Documentation/networking/devlink/index.rst   |   1 +
 Documentation/networking/devlink/zl3073x.rst |  37 +++++++
 drivers/dpll/zl3073x/Kconfig                 |   2 +
 drivers/dpll/zl3073x/core.c                  | 108 ++++++++++++++++++-
 4 files changed, 146 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/networking/devlink/zl3073x.rst

diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index 8319f43b5933d..250ae71f40236 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -98,3 +98,4 @@ parameters, info versions, and other features it supports.
    iosm
    octeontx2
    sfc
+   zl3073x
diff --git a/Documentation/networking/devlink/zl3073x.rst b/Documentation/networking/devlink/zl3073x.rst
new file mode 100644
index 0000000000000..9a6744fb2e866
--- /dev/null
+++ b/Documentation/networking/devlink/zl3073x.rst
@@ -0,0 +1,37 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=======================
+zl3073x devlink support
+=======================
+
+This document describes the devlink features implemented by the ``zl3073x``
+device driver.
+
+Info versions
+=============
+
+The ``zl3073x`` driver reports the following versions
+
+.. list-table:: devlink info versions implemented
+    :widths: 5 5 5 90
+
+    * - Name
+      - Type
+      - Example
+      - Description
+    * - ``asic.id``
+      - fixed
+      - 1E94
+      - Chip identification number
+    * - ``asic.rev``
+      - fixed
+      - 300
+      - Chip revision number
+    * - ``fw``
+      - running
+      - 7006
+      - Firmware version number
+    * - ``cfg.custom_ver``
+      - running
+      - 1.3.0.1
+      - Device configuration version customized by OEM
diff --git a/drivers/dpll/zl3073x/Kconfig b/drivers/dpll/zl3073x/Kconfig
index 217160df0f49a..8f3056b727aa0 100644
--- a/drivers/dpll/zl3073x/Kconfig
+++ b/drivers/dpll/zl3073x/Kconfig
@@ -2,7 +2,9 @@
 
 config ZL3073X
 	tristate "Microchip Azurite DPLL/PTP/SyncE devices"
+	depends on NET
 	select DPLL
+	select NET_DEVLINK
 	help
 	  This driver supports Microchip Azurite DPLL/PTP/SyncE devices.
 
diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index e26b17ccbaaf2..f43e463142435 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -7,8 +7,11 @@
 #include <linux/device.h>
 #include <linux/export.h>
 #include <linux/module.h>
+#include <linux/netlink.h>
 #include <linux/regmap.h>
+#include <linux/sprintf.h>
 #include <linux/unaligned.h>
+#include <net/devlink.h>
 
 #include "core.h"
 #include "regs.h"
@@ -373,6 +376,83 @@ int zl3073x_poll_zero_u8(struct zl3073x_dev *zldev, unsigned int reg, u8 mask)
 					ZL_POLL_SLEEP_US, ZL_POLL_TIMEOUT_US);
 }
 
+/**
+ * zl3073x_devlink_info_get - Devlink device info callback
+ * @devlink: devlink structure pointer
+ * @req: devlink request pointer to store information
+ * @extack: netlink extack pointer to report errors
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int zl3073x_devlink_info_get(struct devlink *devlink,
+				    struct devlink_info_req *req,
+				    struct netlink_ext_ack *extack)
+{
+	struct zl3073x_dev *zldev = devlink_priv(devlink);
+	u16 id, revision, fw_ver;
+	char buf[16];
+	u32 cfg_ver;
+	int rc;
+
+	rc = zl3073x_read_u16(zldev, ZL_REG_ID, &id);
+	if (rc)
+		return rc;
+
+	snprintf(buf, sizeof(buf), "%X", id);
+	rc = devlink_info_version_fixed_put(req,
+					    DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
+					    buf);
+	if (rc)
+		return rc;
+
+	rc = zl3073x_read_u16(zldev, ZL_REG_REVISION, &revision);
+	if (rc)
+		return rc;
+
+	snprintf(buf, sizeof(buf), "%X", revision);
+	rc = devlink_info_version_fixed_put(req,
+					    DEVLINK_INFO_VERSION_GENERIC_ASIC_REV,
+					    buf);
+	if (rc)
+		return rc;
+
+	rc = zl3073x_read_u16(zldev, ZL_REG_FW_VER, &fw_ver);
+	if (rc)
+		return rc;
+
+	snprintf(buf, sizeof(buf), "%u", fw_ver);
+	rc = devlink_info_version_running_put(req,
+					      DEVLINK_INFO_VERSION_GENERIC_FW,
+					      buf);
+	if (rc)
+		return rc;
+
+	rc = zl3073x_read_u32(zldev, ZL_REG_CUSTOM_CONFIG_VER, &cfg_ver);
+	if (rc)
+		return rc;
+
+	/* No custom config version */
+	if (cfg_ver == U32_MAX)
+		return 0;
+
+	snprintf(buf, sizeof(buf), "%lu.%lu.%lu.%lu",
+		 FIELD_GET(GENMASK(31, 24), cfg_ver),
+		 FIELD_GET(GENMASK(23, 16), cfg_ver),
+		 FIELD_GET(GENMASK(15, 8), cfg_ver),
+		 FIELD_GET(GENMASK(7, 0), cfg_ver));
+
+	return devlink_info_version_running_put(req, "cfg.custom_ver", buf);
+}
+
+static const struct devlink_ops zl3073x_devlink_ops = {
+	.info_get = zl3073x_devlink_info_get,
+};
+
+static void zl3073x_devlink_free(void *ptr)
+{
+	devlink_free(ptr);
+}
+
 /**
  * zl3073x_devm_alloc - allocates zl3073x device structure
  * @dev: pointer to device structure
@@ -384,11 +464,19 @@ int zl3073x_poll_zero_u8(struct zl3073x_dev *zldev, unsigned int reg, u8 mask)
 struct zl3073x_dev *zl3073x_devm_alloc(struct device *dev)
 {
 	struct zl3073x_dev *zldev;
+	struct devlink *devlink;
+	int rc;
 
-	zldev = devm_kzalloc(dev, sizeof(*zldev), GFP_KERNEL);
-	if (!zldev)
+	devlink = devlink_alloc(&zl3073x_devlink_ops, sizeof(*zldev), dev);
+	if (!devlink)
 		return ERR_PTR(-ENOMEM);
 
+	/* Add devres action to free devlink device */
+	rc = devm_add_action_or_reset(dev, zl3073x_devlink_free, devlink);
+	if (rc)
+		return ERR_PTR(rc);
+
+	zldev = devlink_priv(devlink);
 	zldev->dev = dev;
 	dev_set_drvdata(zldev->dev, zldev);
 
@@ -396,6 +484,11 @@ struct zl3073x_dev *zl3073x_devm_alloc(struct device *dev)
 }
 EXPORT_SYMBOL_NS_GPL(zl3073x_devm_alloc, "ZL3073X");
 
+static void zl3073x_devlink_unregister(void *ptr)
+{
+	devlink_unregister(ptr);
+}
+
 /**
  * zl3073x_dev_probe - initialize zl3073x device
  * @zldev: pointer to zl3073x device
@@ -409,6 +502,7 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev,
 		      const struct zl3073x_chip_info *chip_info)
 {
 	u16 id, revision, fw_ver;
+	struct devlink *devlink;
 	unsigned int i;
 	u32 cfg_ver;
 	int rc;
@@ -449,6 +543,16 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev,
 		FIELD_GET(GENMASK(15, 8), cfg_ver),
 		FIELD_GET(GENMASK(7, 0), cfg_ver));
 
+	/* Register the device as devlink device */
+	devlink = priv_to_devlink(zldev);
+	devlink_register(devlink);
+
+	/* Add devres action to unregister devlink device */
+	rc = devm_add_action_or_reset(zldev->dev, zl3073x_devlink_unregister,
+				      devlink);
+	if (rc)
+		return rc;
+
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(zl3073x_dev_probe, "ZL3073X");
-- 
2.49.0
Re: [PATCH net-next v12 04/14] dpll: zl3073x: Add support for devlink device info
Posted by Jiri Pirko 3 months, 1 week ago
Sun, Jun 29, 2025 at 09:10:39PM +0200, ivecera@redhat.com wrote:

[...]

>+	snprintf(buf, sizeof(buf), "%lu.%lu.%lu.%lu",
>+		 FIELD_GET(GENMASK(31, 24), cfg_ver),
>+		 FIELD_GET(GENMASK(23, 16), cfg_ver),
>+		 FIELD_GET(GENMASK(15, 8), cfg_ver),
>+		 FIELD_GET(GENMASK(7, 0), cfg_ver));
>+
>+	return devlink_info_version_running_put(req, "cfg.custom_ver", buf);

Nit:

It's redundant to put "ver" string into version name. Also, isn't it
rather "custom_config" or "custom_cfg"?
Re: [PATCH net-next v12 04/14] dpll: zl3073x: Add support for devlink device info
Posted by Ivan Vecera 3 months, 1 week ago
On 02. 07. 25 12:25 odp., Jiri Pirko wrote:
> Sun, Jun 29, 2025 at 09:10:39PM +0200, ivecera@redhat.com wrote:
> 
> [...]
> 
>> +	snprintf(buf, sizeof(buf), "%lu.%lu.%lu.%lu",
>> +		 FIELD_GET(GENMASK(31, 24), cfg_ver),
>> +		 FIELD_GET(GENMASK(23, 16), cfg_ver),
>> +		 FIELD_GET(GENMASK(15, 8), cfg_ver),
>> +		 FIELD_GET(GENMASK(7, 0), cfg_ver));
>> +
>> +	return devlink_info_version_running_put(req, "cfg.custom_ver", buf);
> 
> Nit:
> 
> It's redundant to put "ver" string into version name. Also, isn't it
> rather "custom_config" or "custom_cfg"?

As per datasheet, this is configuration custom version.

I.
Re: [PATCH net-next v12 04/14] dpll: zl3073x: Add support for devlink device info
Posted by Jiri Pirko 3 months, 1 week ago
Wed, Jul 02, 2025 at 01:38:03PM +0200, ivecera@redhat.com wrote:
>On 02. 07. 25 12:25 odp., Jiri Pirko wrote:
>> Sun, Jun 29, 2025 at 09:10:39PM +0200, ivecera@redhat.com wrote:
>> 
>> [...]
>> 
>> > +	snprintf(buf, sizeof(buf), "%lu.%lu.%lu.%lu",
>> > +		 FIELD_GET(GENMASK(31, 24), cfg_ver),
>> > +		 FIELD_GET(GENMASK(23, 16), cfg_ver),
>> > +		 FIELD_GET(GENMASK(15, 8), cfg_ver),
>> > +		 FIELD_GET(GENMASK(7, 0), cfg_ver));
>> > +
>> > +	return devlink_info_version_running_put(req, "cfg.custom_ver", buf);
>> 
>> Nit:
>> 
>> It's redundant to put "ver" string into version name. Also, isn't it
>> rather "custom_config" or "custom_cfg"?
>
>As per datasheet, this is configuration custom version.

This is UAPI, we define it and we should make sure it make sense.
Datasheet is sort of irrelevant.
Re: [PATCH net-next v12 04/14] dpll: zl3073x: Add support for devlink device info
Posted by Ivan Vecera 3 months, 1 week ago

On 02. 07. 25 1:41 odp., Jiri Pirko wrote:
> Wed, Jul 02, 2025 at 01:38:03PM +0200, ivecera@redhat.com wrote:
>> On 02. 07. 25 12:25 odp., Jiri Pirko wrote:
>>> Sun, Jun 29, 2025 at 09:10:39PM +0200, ivecera@redhat.com wrote:
>>>
>>> [...]
>>>
>>>> +	snprintf(buf, sizeof(buf), "%lu.%lu.%lu.%lu",
>>>> +		 FIELD_GET(GENMASK(31, 24), cfg_ver),
>>>> +		 FIELD_GET(GENMASK(23, 16), cfg_ver),
>>>> +		 FIELD_GET(GENMASK(15, 8), cfg_ver),
>>>> +		 FIELD_GET(GENMASK(7, 0), cfg_ver));
>>>> +
>>>> +	return devlink_info_version_running_put(req, "cfg.custom_ver", buf);
>>>
>>> Nit:
>>>
>>> It's redundant to put "ver" string into version name. Also, isn't it
>>> rather "custom_config" or "custom_cfg"?
>>
>> As per datasheet, this is configuration custom version.
> 
> This is UAPI, we define it and we should make sure it make sense.
> Datasheet is sort of irrelevant.

OK, 'custom_cfg' make sense.

Will update.

I.