[PATCH net-next 0/3] dpll: zl3073x: add hwmon support

[PATCH net-next 0/3] dpll: zl3073x: add hwmon support

[Ivan Vecera]

Add hwmon support to the ZL3073x DPLL driver, exposing die temperature
and input reference frequency measurements via the standard hwmon
interface.

Patch 1 registers an hwmon device with the standard temperature sensor
channel. The die temperature is only available on chip variants with the
ZL3073X_FLAG_DIE_TEMP flag and visibility is controlled via the
is_visible callback. The temperature register value in 0.1°C units is
converted to millidegrees Celsius.

Patch 2 adds the frequency measurement infrastructure. It extracts
common measurement latch logic into a zl3073x_ref_freq_meas_latch()
helper shared with the existing FFO measurement code, and adds
zl3073x_ref_freq_meas_update() to periodically read absolute input
reference frequencies in Hz.

Patch 3 exposes the measured frequencies via custom sysfs attributes
(freqN_input and freqN_label) since hwmon has no native frequency
sensor type. A cached device ready flag ensures freq_input_show()
returns -ENODATA without I2C access when firmware is not configured.

The hwmon registration is conditionally compiled using
IS_REACHABLE(CONFIG_HWMON) with a stub fallback.

Tested on Microchip EDS2 (pcb8385) platform:

  $ sensors zl3073x-i2c-1-70
  zl3073x-i2c-1-70
  Adapter: AT91
  temp1:        +24.2°C
  REF0P:        125 MHz
  REF0N:          0 Hz
  REF1P:        125 MHz
  REF1N:        125 MHz
  REF2P:         10 MHz
  REF2N:       1000 mHz
  REF3P:          0 Hz
  REF3N:       1000 mHz
  REF4P:         25 MHz
  REF4N:        170 kHz

  $ sensors -u zl3073x-i2c-1-70
  zl3073x-i2c-1-70
  Adapter: AT91
  temp1:
    temp1_input: 24.200
  REF0P:
    freq0_input: 124999220.000
  REF0N:
    freq1_input: 0.000
  REF1P:
    freq2_input: 125001660.000
  REF1N:
    freq3_input: 125001661.000
  REF2P:
    freq4_input: 10000000.000
  REF2N:
    freq5_input: 1.000
  REF3P:
    freq6_input: 0.000
  REF3N:
    freq7_input: 1.000
  REF4P:
    freq8_input: 25000000.000
  REF4N:
    freq9_input: 169598.000

Ivan Vecera (3):
  dpll: zl3073x: add hwmon support for die temperature
  dpll: zl3073x: add input reference frequency measurement
  dpll: zl3073x: add hwmon support for input reference frequencies

 drivers/dpll/zl3073x/Makefile |   1 +
 drivers/dpll/zl3073x/core.c   |  91 ++++++++++++++++----
 drivers/dpll/zl3073x/core.h   |   2 +
 drivers/dpll/zl3073x/hwmon.c  | 151 ++++++++++++++++++++++++++++++++++
 drivers/dpll/zl3073x/hwmon.h  |  16 ++++
 drivers/dpll/zl3073x/ref.h    |  14 ++++
 6 files changed, 261 insertions(+), 14 deletions(-)
 create mode 100644 drivers/dpll/zl3073x/hwmon.c
 create mode 100644 drivers/dpll/zl3073x/hwmon.h

-- 
2.52.0

[PATCH net-next 1/3] dpll: zl3073x: add hwmon support for die temperature

[Ivan Vecera]

Register an hwmon device to expose the chip die temperature via the
standard hwmon temperature interface. The temperature sensor is only
available on chip variants that have the ZL3073X_FLAG_DIE_TEMP flag
set and its visibility is controlled via the is_visible callback.

The die temperature register provides a value in 0.1°C units that is
converted to millidegrees Celsius for the hwmon subsystem.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/Makefile |  1 +
 drivers/dpll/zl3073x/core.c   |  7 ++++
 drivers/dpll/zl3073x/hwmon.c  | 67 +++++++++++++++++++++++++++++++++++
 drivers/dpll/zl3073x/hwmon.h  | 16 +++++++++
 4 files changed, 91 insertions(+)
 create mode 100644 drivers/dpll/zl3073x/hwmon.c
 create mode 100644 drivers/dpll/zl3073x/hwmon.h

diff --git a/drivers/dpll/zl3073x/Makefile b/drivers/dpll/zl3073x/Makefile
index 906ec3fbcc202..6930bf7867151 100644
--- a/drivers/dpll/zl3073x/Makefile
+++ b/drivers/dpll/zl3073x/Makefile
@@ -3,6 +3,7 @@
 obj-$(CONFIG_ZL3073X)		+= zl3073x.o
 zl3073x-objs			:= chan.o core.o devlink.o dpll.o	\
 				   flash.o fw.o out.o prop.o ref.o synth.o
+zl3073x-$(CONFIG_HWMON)		+= hwmon.o
 
 obj-$(CONFIG_ZL3073X_I2C)	+= zl3073x_i2c.o
 zl3073x_i2c-objs		:= i2c.o
diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index 7eebfc1ad1019..dcea98c31d694 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -18,6 +18,7 @@
 #include "core.h"
 #include "devlink.h"
 #include "dpll.h"
+#include "hwmon.h"
 #include "regs.h"
 
 #define ZL_CHIP_INFO(_id, _nchannels, _flags)				\
@@ -1038,6 +1039,12 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev)
 	if (rc)
 		return rc;
 
+	/* Register hwmon interface */
+	rc = zl3073x_hwmon_init(zldev);
+	if (rc)
+		return dev_err_probe(zldev->dev, rc,
+				     "Failed to register hwmon device\n");
+
 	/* Register the devlink instance and parameters */
 	rc = zl3073x_devlink_register(zldev);
 	if (rc)
diff --git a/drivers/dpll/zl3073x/hwmon.c b/drivers/dpll/zl3073x/hwmon.c
new file mode 100644
index 0000000000000..4b44df4def820
--- /dev/null
+++ b/drivers/dpll/zl3073x/hwmon.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/device.h>
+#include <linux/hwmon.h>
+
+#include "core.h"
+#include "hwmon.h"
+#include "regs.h"
+
+static int zl3073x_hwmon_read(struct device *dev,
+			      enum hwmon_sensor_types type,
+			      u32 attr, int channel, long *val)
+{
+	struct zl3073x_dev *zldev = dev_get_drvdata(dev);
+	u16 raw;
+	int rc;
+
+	if (type != hwmon_temp || attr != hwmon_temp_input)
+		return -EOPNOTSUPP;
+
+	rc = zl3073x_read_u16(zldev, ZL_REG_DIE_TEMP_STATUS, &raw);
+	if (rc)
+		return rc;
+
+	/* Convert from 0.1°C units to millidegrees Celsius */
+	*val = (s16)raw * 100;
+
+	return 0;
+}
+
+static umode_t zl3073x_hwmon_is_visible(const void *data,
+					enum hwmon_sensor_types type,
+					u32 attr, int channel)
+{
+	const struct zl3073x_dev *zldev = data;
+
+	if (type == hwmon_temp && (zldev->info->flags & ZL3073X_FLAG_DIE_TEMP))
+		return 0444;
+
+	return 0;
+}
+
+static const struct hwmon_channel_info * const zl3073x_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
+	NULL,
+};
+
+static const struct hwmon_ops zl3073x_hwmon_ops = {
+	.is_visible = zl3073x_hwmon_is_visible,
+	.read = zl3073x_hwmon_read,
+};
+
+static const struct hwmon_chip_info zl3073x_hwmon_chip_info = {
+	.ops = &zl3073x_hwmon_ops,
+	.info = zl3073x_hwmon_info,
+};
+
+int zl3073x_hwmon_init(struct zl3073x_dev *zldev)
+{
+	struct device *hwmon;
+
+	hwmon = devm_hwmon_device_register_with_info(zldev->dev, "zl3073x",
+						     zldev,
+						     &zl3073x_hwmon_chip_info,
+						     NULL);
+	return PTR_ERR_OR_ZERO(hwmon);
+}
diff --git a/drivers/dpll/zl3073x/hwmon.h b/drivers/dpll/zl3073x/hwmon.h
new file mode 100644
index 0000000000000..6048d596985ad
--- /dev/null
+++ b/drivers/dpll/zl3073x/hwmon.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _ZL3073X_HWMON_H
+#define _ZL3073X_HWMON_H
+
+#include <linux/kconfig.h>
+
+struct zl3073x_dev;
+
+#if IS_REACHABLE(CONFIG_HWMON)
+int zl3073x_hwmon_init(struct zl3073x_dev *zldev);
+#else
+static inline int zl3073x_hwmon_init(struct zl3073x_dev *zldev) { return 0; }
+#endif
+
+#endif /* _ZL3073X_HWMON_H */
-- 
2.52.0

[PATCH net-next 2/3] dpll: zl3073x: add input reference frequency measurement

[Ivan Vecera]

Extract common measurement latch logic from zl3073x_ref_ffo_update()
into a new zl3073x_ref_freq_meas_latch() helper and add
zl3073x_ref_freq_meas_update() that uses it to latch and read absolute
input reference frequencies in Hz.

Add meas_freq field to struct zl3073x_ref and the corresponding
zl3073x_ref_meas_freq_get() accessor. The measured frequencies are
updated periodically alongside the existing FFO measurements.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/core.c | 80 +++++++++++++++++++++++++++++++------
 drivers/dpll/zl3073x/ref.h  | 14 +++++++
 2 files changed, 81 insertions(+), 13 deletions(-)

diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index dcea98c31d694..67e65f8e7e7d4 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -633,22 +633,21 @@ int zl3073x_ref_phase_offsets_update(struct zl3073x_dev *zldev, int channel)
 }
 
 /**
- * zl3073x_ref_ffo_update - update reference fractional frequency offsets
+ * zl3073x_ref_freq_meas_latch - latch reference frequency measurements
  * @zldev: pointer to zl3073x_dev structure
+ * @type: measurement type (ZL_REF_FREQ_MEAS_CTRL_*)
  *
- * The function asks device to update fractional frequency offsets latch
- * registers the latest measured values, reads and stores them into
+ * The function waits for the previous measurement to finish, selects all
+ * references and requests a new measurement of the given type.
  *
  * Return: 0 on success, <0 on error
  */
 static int
-zl3073x_ref_ffo_update(struct zl3073x_dev *zldev)
+zl3073x_ref_freq_meas_latch(struct zl3073x_dev *zldev, u8 type)
 {
-	int i, rc;
+	int rc;
 
-	/* Per datasheet we have to wait for 'ref_freq_meas_ctrl' to be zero
-	 * to ensure that the measured data are coherent.
-	 */
+	/* Wait for previous measurement to finish */
 	rc = zl3073x_poll_zero_u8(zldev, ZL_REG_REF_FREQ_MEAS_CTRL,
 				  ZL_REF_FREQ_MEAS_CTRL);
 	if (rc)
@@ -664,15 +663,63 @@ zl3073x_ref_ffo_update(struct zl3073x_dev *zldev)
 	if (rc)
 		return rc;
 
-	/* Request frequency offset measurement */
-	rc = zl3073x_write_u8(zldev, ZL_REG_REF_FREQ_MEAS_CTRL,
-			      ZL_REF_FREQ_MEAS_CTRL_REF_FREQ_OFF);
+	/* Request measurement */
+	rc = zl3073x_write_u8(zldev, ZL_REG_REF_FREQ_MEAS_CTRL, type);
 	if (rc)
 		return rc;
 
 	/* Wait for finish */
-	rc = zl3073x_poll_zero_u8(zldev, ZL_REG_REF_FREQ_MEAS_CTRL,
-				  ZL_REF_FREQ_MEAS_CTRL);
+	return zl3073x_poll_zero_u8(zldev, ZL_REG_REF_FREQ_MEAS_CTRL,
+				    ZL_REF_FREQ_MEAS_CTRL);
+}
+
+/**
+ * zl3073x_ref_freq_meas_update - update measured input reference frequencies
+ * @zldev: pointer to zl3073x_dev structure
+ *
+ * The function asks device to latch measured input reference frequencies
+ * and stores the results in the ref state.
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int
+zl3073x_ref_freq_meas_update(struct zl3073x_dev *zldev)
+{
+	int i, rc;
+
+	rc = zl3073x_ref_freq_meas_latch(zldev, ZL_REF_FREQ_MEAS_CTRL_REF_FREQ);
+	if (rc)
+		return rc;
+
+	/* Read measured frequencies in Hz (unsigned 32-bit, LSB = 1 Hz) */
+	for (i = 0; i < ZL3073X_NUM_REFS; i++) {
+		u32 value;
+
+		rc = zl3073x_read_u32(zldev, ZL_REG_REF_FREQ(i), &value);
+		if (rc)
+			return rc;
+
+		zldev->ref[i].meas_freq = value;
+	}
+
+	return 0;
+}
+
+/**
+ * zl3073x_ref_ffo_update - update reference fractional frequency offsets
+ * @zldev: pointer to zl3073x_dev structure
+ *
+ * The function asks device to update fractional frequency offsets latch
+ * registers the latest measured values, reads and stores them into
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int
+zl3073x_ref_ffo_update(struct zl3073x_dev *zldev)
+{
+	int i, rc;
+
+	rc = zl3073x_ref_freq_meas_latch(zldev, ZL_REF_FREQ_MEAS_CTRL_REF_FREQ_OFF);
 	if (rc)
 		return rc;
 
@@ -715,6 +762,13 @@ zl3073x_dev_periodic_work(struct kthread_work *work)
 		dev_warn(zldev->dev, "Failed to update phase offsets: %pe\n",
 			 ERR_PTR(rc));
 
+	/* Update measured input reference frequencies */
+	rc = zl3073x_ref_freq_meas_update(zldev);
+	if (rc)
+		dev_warn(zldev->dev,
+			 "Failed to update measured frequencies: %pe\n",
+			 ERR_PTR(rc));
+
 	/* Update references' fractional frequency offsets */
 	rc = zl3073x_ref_ffo_update(zldev);
 	if (rc)
diff --git a/drivers/dpll/zl3073x/ref.h b/drivers/dpll/zl3073x/ref.h
index 09fab97a71d7e..55e80e4f08734 100644
--- a/drivers/dpll/zl3073x/ref.h
+++ b/drivers/dpll/zl3073x/ref.h
@@ -23,6 +23,7 @@ struct zl3073x_dev;
  * @sync_ctrl: reference sync control
  * @config: reference config
  * @ffo: current fractional frequency offset
+ * @meas_freq: measured input frequency in Hz
  * @mon_status: reference monitor status
  */
 struct zl3073x_ref {
@@ -40,6 +41,7 @@ struct zl3073x_ref {
 	);
 	struct_group(stat, /* Status */
 		s64	ffo;
+		u32	meas_freq;
 		u8	mon_status;
 	);
 };
@@ -68,6 +70,18 @@ zl3073x_ref_ffo_get(const struct zl3073x_ref *ref)
 	return ref->ffo;
 }
 
+/**
+ * zl3073x_ref_meas_freq_get - get measured input frequency
+ * @ref: pointer to ref state
+ *
+ * Return: measured input frequency in Hz
+ */
+static inline u32
+zl3073x_ref_meas_freq_get(const struct zl3073x_ref *ref)
+{
+	return ref->meas_freq;
+}
+
 /**
  * zl3073x_ref_freq_get - get given input reference frequency
  * @ref: pointer to ref state
-- 
2.52.0

[PATCH net-next 3/3] dpll: zl3073x: add hwmon support for input reference frequencies

[Ivan Vecera]

Expose measured input reference frequencies via the hwmon interface
using custom sysfs attributes (freqN_input and freqN_label) since
hwmon has no native frequency sensor type. The frequency values are
read from the cached measurements updated by the periodic work thread.

Cache the device ready state in struct zl3073x_dev so that
freq_input_show() can return -ENODATA without an I2C access when
the device firmware is not configured.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/core.c  |  4 +-
 drivers/dpll/zl3073x/core.h  |  2 +
 drivers/dpll/zl3073x/hwmon.c | 86 +++++++++++++++++++++++++++++++++++-
 3 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index 67e65f8e7e7d4..5805f87167c20 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -874,7 +874,9 @@ int zl3073x_dev_start(struct zl3073x_dev *zldev, bool full)
 		return rc;
 	}
 
-	if (!FIELD_GET(ZL_INFO_READY, info)) {
+	zldev->ready = !!FIELD_GET(ZL_INFO_READY, info);
+
+	if (!zldev->ready) {
 		/* The ready bit indicates that the firmware was successfully
 		 * configured and is ready for normal operation. If it is
 		 * cleared then the configuration stored in flash is wrong
diff --git a/drivers/dpll/zl3073x/core.h b/drivers/dpll/zl3073x/core.h
index 99440620407da..a416b8a65f41b 100644
--- a/drivers/dpll/zl3073x/core.h
+++ b/drivers/dpll/zl3073x/core.h
@@ -48,6 +48,7 @@ struct zl3073x_chip_info {
  * @regmap: regmap to access device registers
  * @info: detected chip info
  * @multiop_lock: to serialize multiple register operations
+ * @ready: true if device firmware is configured and ready for normal operation
  * @ref: array of input references' invariants
  * @out: array of outs' invariants
  * @synth: array of synths' invariants
@@ -63,6 +64,7 @@ struct zl3073x_dev {
 	struct regmap			*regmap;
 	const struct zl3073x_chip_info	*info;
 	struct mutex			multiop_lock;
+	bool				ready;
 
 	/* Invariants */
 	struct zl3073x_ref	ref[ZL3073X_NUM_REFS];
diff --git a/drivers/dpll/zl3073x/hwmon.c b/drivers/dpll/zl3073x/hwmon.c
index 4b44df4def820..96879609ce100 100644
--- a/drivers/dpll/zl3073x/hwmon.c
+++ b/drivers/dpll/zl3073x/hwmon.c
@@ -2,9 +2,11 @@
 
 #include <linux/device.h>
 #include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
 
 #include "core.h"
 #include "hwmon.h"
+#include "ref.h"
 #include "regs.h"
 
 static int zl3073x_hwmon_read(struct device *dev,
@@ -55,6 +57,88 @@ static const struct hwmon_chip_info zl3073x_hwmon_chip_info = {
 	.info = zl3073x_hwmon_info,
 };
 
+static ssize_t freq_input_show(struct device *dev,
+			       struct device_attribute *devattr, char *buf)
+{
+	struct zl3073x_dev *zldev = dev_get_drvdata(dev);
+	int index = to_sensor_dev_attr(devattr)->index;
+	const struct zl3073x_ref *ref;
+
+	if (!zldev->ready)
+		return -ENODATA;
+
+	ref = zl3073x_ref_state_get(zldev, index);
+
+	return sysfs_emit(buf, "%u\n", zl3073x_ref_meas_freq_get(ref));
+}
+
+static ssize_t freq_label_show(struct device *dev,
+			       struct device_attribute *devattr, char *buf)
+{
+	static const char * const labels[] = {
+		"REF0P", "REF0N", "REF1P", "REF1N", "REF2P",
+		"REF2N", "REF3P", "REF3N", "REF4P", "REF4N",
+	};
+	int index = to_sensor_dev_attr(devattr)->index;
+
+	return sysfs_emit(buf, "%s\n", labels[index]);
+}
+
+static SENSOR_DEVICE_ATTR_RO(freq0_input, freq_input, 0);
+static SENSOR_DEVICE_ATTR_RO(freq1_input, freq_input, 1);
+static SENSOR_DEVICE_ATTR_RO(freq2_input, freq_input, 2);
+static SENSOR_DEVICE_ATTR_RO(freq3_input, freq_input, 3);
+static SENSOR_DEVICE_ATTR_RO(freq4_input, freq_input, 4);
+static SENSOR_DEVICE_ATTR_RO(freq5_input, freq_input, 5);
+static SENSOR_DEVICE_ATTR_RO(freq6_input, freq_input, 6);
+static SENSOR_DEVICE_ATTR_RO(freq7_input, freq_input, 7);
+static SENSOR_DEVICE_ATTR_RO(freq8_input, freq_input, 8);
+static SENSOR_DEVICE_ATTR_RO(freq9_input, freq_input, 9);
+
+static SENSOR_DEVICE_ATTR_RO(freq0_label, freq_label, 0);
+static SENSOR_DEVICE_ATTR_RO(freq1_label, freq_label, 1);
+static SENSOR_DEVICE_ATTR_RO(freq2_label, freq_label, 2);
+static SENSOR_DEVICE_ATTR_RO(freq3_label, freq_label, 3);
+static SENSOR_DEVICE_ATTR_RO(freq4_label, freq_label, 4);
+static SENSOR_DEVICE_ATTR_RO(freq5_label, freq_label, 5);
+static SENSOR_DEVICE_ATTR_RO(freq6_label, freq_label, 6);
+static SENSOR_DEVICE_ATTR_RO(freq7_label, freq_label, 7);
+static SENSOR_DEVICE_ATTR_RO(freq8_label, freq_label, 8);
+static SENSOR_DEVICE_ATTR_RO(freq9_label, freq_label, 9);
+
+static struct attribute *zl3073x_freq_attrs[] = {
+	&sensor_dev_attr_freq0_input.dev_attr.attr,
+	&sensor_dev_attr_freq0_label.dev_attr.attr,
+	&sensor_dev_attr_freq1_input.dev_attr.attr,
+	&sensor_dev_attr_freq1_label.dev_attr.attr,
+	&sensor_dev_attr_freq2_input.dev_attr.attr,
+	&sensor_dev_attr_freq2_label.dev_attr.attr,
+	&sensor_dev_attr_freq3_input.dev_attr.attr,
+	&sensor_dev_attr_freq3_label.dev_attr.attr,
+	&sensor_dev_attr_freq4_input.dev_attr.attr,
+	&sensor_dev_attr_freq4_label.dev_attr.attr,
+	&sensor_dev_attr_freq5_input.dev_attr.attr,
+	&sensor_dev_attr_freq5_label.dev_attr.attr,
+	&sensor_dev_attr_freq6_input.dev_attr.attr,
+	&sensor_dev_attr_freq6_label.dev_attr.attr,
+	&sensor_dev_attr_freq7_input.dev_attr.attr,
+	&sensor_dev_attr_freq7_label.dev_attr.attr,
+	&sensor_dev_attr_freq8_input.dev_attr.attr,
+	&sensor_dev_attr_freq8_label.dev_attr.attr,
+	&sensor_dev_attr_freq9_input.dev_attr.attr,
+	&sensor_dev_attr_freq9_label.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group zl3073x_freq_group = {
+	.attrs = zl3073x_freq_attrs,
+};
+
+static const struct attribute_group *zl3073x_hwmon_groups[] = {
+	&zl3073x_freq_group,
+	NULL,
+};
+
 int zl3073x_hwmon_init(struct zl3073x_dev *zldev)
 {
 	struct device *hwmon;
@@ -62,6 +146,6 @@ int zl3073x_hwmon_init(struct zl3073x_dev *zldev)
 	hwmon = devm_hwmon_device_register_with_info(zldev->dev, "zl3073x",
 						     zldev,
 						     &zl3073x_hwmon_chip_info,
-						     NULL);
+						     zl3073x_hwmon_groups);
 	return PTR_ERR_OR_ZERO(hwmon);
 }
-- 
2.52.0

Re: [PATCH net-next 3/3] dpll: zl3073x: add hwmon support for input reference frequencies

[Guenter Roeck]

On 3/20/26 03:59, Ivan Vecera wrote:
> Expose measured input reference frequencies via the hwmon interface
> using custom sysfs attributes (freqN_input and freqN_label) since
> hwmon has no native frequency sensor type. The frequency values are
> read from the cached measurements updated by the periodic work thread.
> 
> Cache the device ready state in struct zl3073x_dev so that
> freq_input_show() can return -ENODATA without an I2C access when
> the device firmware is not configured.
> 
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>

"frequency" is not a hardware monitoring attribute. I understand that it is
convenient to report it as one, and that other drivers implement it as well,
but that doesn't change that.

I understand that the code lives outside the hardware monitoring subsystem and is
thus not in control of its maintainers, so you can essentially do whatever you want,
even if it is wrong. That doesn't change the fact that it is wrong.

However, do _not_ try to add it into the official list of hardware monitoring
attributes. I would NACK that.

Guenter

> ---
>   drivers/dpll/zl3073x/core.c  |  4 +-
>   drivers/dpll/zl3073x/core.h  |  2 +
>   drivers/dpll/zl3073x/hwmon.c | 86 +++++++++++++++++++++++++++++++++++-
>   3 files changed, 90 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
> index 67e65f8e7e7d4..5805f87167c20 100644
> --- a/drivers/dpll/zl3073x/core.c
> +++ b/drivers/dpll/zl3073x/core.c
> @@ -874,7 +874,9 @@ int zl3073x_dev_start(struct zl3073x_dev *zldev, bool full)
>   		return rc;
>   	}
>   
> -	if (!FIELD_GET(ZL_INFO_READY, info)) {
> +	zldev->ready = !!FIELD_GET(ZL_INFO_READY, info);
> +
> +	if (!zldev->ready) {
>   		/* The ready bit indicates that the firmware was successfully
>   		 * configured and is ready for normal operation. If it is
>   		 * cleared then the configuration stored in flash is wrong
> diff --git a/drivers/dpll/zl3073x/core.h b/drivers/dpll/zl3073x/core.h
> index 99440620407da..a416b8a65f41b 100644
> --- a/drivers/dpll/zl3073x/core.h
> +++ b/drivers/dpll/zl3073x/core.h
> @@ -48,6 +48,7 @@ struct zl3073x_chip_info {
>    * @regmap: regmap to access device registers
>    * @info: detected chip info
>    * @multiop_lock: to serialize multiple register operations
> + * @ready: true if device firmware is configured and ready for normal operation
>    * @ref: array of input references' invariants
>    * @out: array of outs' invariants
>    * @synth: array of synths' invariants
> @@ -63,6 +64,7 @@ struct zl3073x_dev {
>   	struct regmap			*regmap;
>   	const struct zl3073x_chip_info	*info;
>   	struct mutex			multiop_lock;
> +	bool				ready;
>   
>   	/* Invariants */
>   	struct zl3073x_ref	ref[ZL3073X_NUM_REFS];
> diff --git a/drivers/dpll/zl3073x/hwmon.c b/drivers/dpll/zl3073x/hwmon.c
> index 4b44df4def820..96879609ce100 100644
> --- a/drivers/dpll/zl3073x/hwmon.c
> +++ b/drivers/dpll/zl3073x/hwmon.c
> @@ -2,9 +2,11 @@
>   
>   #include <linux/device.h>
>   #include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
>   
>   #include "core.h"
>   #include "hwmon.h"
> +#include "ref.h"
>   #include "regs.h"
>   
>   static int zl3073x_hwmon_read(struct device *dev,
> @@ -55,6 +57,88 @@ static const struct hwmon_chip_info zl3073x_hwmon_chip_info = {
>   	.info = zl3073x_hwmon_info,
>   };
>   
> +static ssize_t freq_input_show(struct device *dev,
> +			       struct device_attribute *devattr, char *buf)
> +{
> +	struct zl3073x_dev *zldev = dev_get_drvdata(dev);
> +	int index = to_sensor_dev_attr(devattr)->index;
> +	const struct zl3073x_ref *ref;
> +
> +	if (!zldev->ready)
> +		return -ENODATA;
> +
> +	ref = zl3073x_ref_state_get(zldev, index);
> +
> +	return sysfs_emit(buf, "%u\n", zl3073x_ref_meas_freq_get(ref));
> +}
> +
> +static ssize_t freq_label_show(struct device *dev,
> +			       struct device_attribute *devattr, char *buf)
> +{
> +	static const char * const labels[] = {
> +		"REF0P", "REF0N", "REF1P", "REF1N", "REF2P",
> +		"REF2N", "REF3P", "REF3N", "REF4P", "REF4N",
> +	};
> +	int index = to_sensor_dev_attr(devattr)->index;
> +
> +	return sysfs_emit(buf, "%s\n", labels[index]);
> +}
> +
> +static SENSOR_DEVICE_ATTR_RO(freq0_input, freq_input, 0);
> +static SENSOR_DEVICE_ATTR_RO(freq1_input, freq_input, 1);
> +static SENSOR_DEVICE_ATTR_RO(freq2_input, freq_input, 2);
> +static SENSOR_DEVICE_ATTR_RO(freq3_input, freq_input, 3);
> +static SENSOR_DEVICE_ATTR_RO(freq4_input, freq_input, 4);
> +static SENSOR_DEVICE_ATTR_RO(freq5_input, freq_input, 5);
> +static SENSOR_DEVICE_ATTR_RO(freq6_input, freq_input, 6);
> +static SENSOR_DEVICE_ATTR_RO(freq7_input, freq_input, 7);
> +static SENSOR_DEVICE_ATTR_RO(freq8_input, freq_input, 8);
> +static SENSOR_DEVICE_ATTR_RO(freq9_input, freq_input, 9);
> +
> +static SENSOR_DEVICE_ATTR_RO(freq0_label, freq_label, 0);
> +static SENSOR_DEVICE_ATTR_RO(freq1_label, freq_label, 1);
> +static SENSOR_DEVICE_ATTR_RO(freq2_label, freq_label, 2);
> +static SENSOR_DEVICE_ATTR_RO(freq3_label, freq_label, 3);
> +static SENSOR_DEVICE_ATTR_RO(freq4_label, freq_label, 4);
> +static SENSOR_DEVICE_ATTR_RO(freq5_label, freq_label, 5);
> +static SENSOR_DEVICE_ATTR_RO(freq6_label, freq_label, 6);
> +static SENSOR_DEVICE_ATTR_RO(freq7_label, freq_label, 7);
> +static SENSOR_DEVICE_ATTR_RO(freq8_label, freq_label, 8);
> +static SENSOR_DEVICE_ATTR_RO(freq9_label, freq_label, 9);
> +
> +static struct attribute *zl3073x_freq_attrs[] = {
> +	&sensor_dev_attr_freq0_input.dev_attr.attr,
> +	&sensor_dev_attr_freq0_label.dev_attr.attr,
> +	&sensor_dev_attr_freq1_input.dev_attr.attr,
> +	&sensor_dev_attr_freq1_label.dev_attr.attr,
> +	&sensor_dev_attr_freq2_input.dev_attr.attr,
> +	&sensor_dev_attr_freq2_label.dev_attr.attr,
> +	&sensor_dev_attr_freq3_input.dev_attr.attr,
> +	&sensor_dev_attr_freq3_label.dev_attr.attr,
> +	&sensor_dev_attr_freq4_input.dev_attr.attr,
> +	&sensor_dev_attr_freq4_label.dev_attr.attr,
> +	&sensor_dev_attr_freq5_input.dev_attr.attr,
> +	&sensor_dev_attr_freq5_label.dev_attr.attr,
> +	&sensor_dev_attr_freq6_input.dev_attr.attr,
> +	&sensor_dev_attr_freq6_label.dev_attr.attr,
> +	&sensor_dev_attr_freq7_input.dev_attr.attr,
> +	&sensor_dev_attr_freq7_label.dev_attr.attr,
> +	&sensor_dev_attr_freq8_input.dev_attr.attr,
> +	&sensor_dev_attr_freq8_label.dev_attr.attr,
> +	&sensor_dev_attr_freq9_input.dev_attr.attr,
> +	&sensor_dev_attr_freq9_label.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group zl3073x_freq_group = {
> +	.attrs = zl3073x_freq_attrs,
> +};
> +
> +static const struct attribute_group *zl3073x_hwmon_groups[] = {
> +	&zl3073x_freq_group,
> +	NULL,
> +};
> +
>   int zl3073x_hwmon_init(struct zl3073x_dev *zldev)
>   {
>   	struct device *hwmon;
> @@ -62,6 +146,6 @@ int zl3073x_hwmon_init(struct zl3073x_dev *zldev)
>   	hwmon = devm_hwmon_device_register_with_info(zldev->dev, "zl3073x",
>   						     zldev,
>   						     &zl3073x_hwmon_chip_info,
> -						     NULL);
> +						     zl3073x_hwmon_groups);
>   	return PTR_ERR_OR_ZERO(hwmon);
>   }

Re: [PATCH net-next 3/3] dpll: zl3073x: add hwmon support for input reference frequencies

[Ivan Vecera]

On 3/20/26 1:21 PM, Guenter Roeck wrote:
> On 3/20/26 03:59, Ivan Vecera wrote:
>> Expose measured input reference frequencies via the hwmon interface
>> using custom sysfs attributes (freqN_input and freqN_label) since
>> hwmon has no native frequency sensor type. The frequency values are
>> read from the cached measurements updated by the periodic work thread.
>>
>> Cache the device ready state in struct zl3073x_dev so that
>> freq_input_show() can return -ENODATA without an I2C access when
>> the device firmware is not configured.
>>
>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> 
> "frequency" is not a hardware monitoring attribute. I understand that it is
> convenient to report it as one, and that other drivers implement it as 
> well,
> but that doesn't change that.
> 
> I understand that the code lives outside the hardware monitoring 
> subsystem and is
> thus not in control of its maintainers, so you can essentially do 
> whatever you want,
> even if it is wrong. That doesn't change the fact that it is wrong.
> 
> However, do _not_ try to add it into the official list of hardware 
> monitoring
> attributes. I would NACK that.
> 
> Guenter

Hi Guenter,

Understood. I recognize that frequency falls outside the strict scope of
hardware monitoring and does not belong in the official hwmon ABI.

I'm using it here as a convenient way to expose these specific driver
metrics, but I hear you loud and clear. I will absolutely not propose
adding frequency to the official list of hwmon attributes or
documentation.

Thank you for your time and for reviewing the patch.

Regards,
Ivan

Re: [PATCH net-next 3/3] dpll: zl3073x: add hwmon support for input reference frequencies

[Jakub Kicinski]

On Fri, 20 Mar 2026 14:48:01 +0100 Ivan Vecera wrote:
> On 3/20/26 1:21 PM, Guenter Roeck wrote:
> > On 3/20/26 03:59, Ivan Vecera wrote:  
> >> Expose measured input reference frequencies via the hwmon interface
> >> using custom sysfs attributes (freqN_input and freqN_label) since
> >> hwmon has no native frequency sensor type. The frequency values are
> >> read from the cached measurements updated by the periodic work thread.
> >>
> >> Cache the device ready state in struct zl3073x_dev so that
> >> freq_input_show() can return -ENODATA without an I2C access when
> >> the device firmware is not configured.
> >>
> >> Signed-off-by: Ivan Vecera <ivecera@redhat.com>  
> > 
> > "frequency" is not a hardware monitoring attribute. I understand that it is
> > convenient to report it as one, and that other drivers implement it as 
> > well,
> > but that doesn't change that.
> > 
> > I understand that the code lives outside the hardware monitoring 
> > subsystem and is
> > thus not in control of its maintainers, so you can essentially do 
> > whatever you want,
> > even if it is wrong. That doesn't change the fact that it is wrong.
> > 
> > However, do _not_ try to add it into the official list of hardware 
> > monitoring
> > attributes. I would NACK that.
> 
> Understood. I recognize that frequency falls outside the strict scope of
> hardware monitoring and does not belong in the official hwmon ABI.
> 
> I'm using it here as a convenient way to expose these specific driver
> metrics, but I hear you loud and clear. I will absolutely not propose
> adding frequency to the official list of hwmon attributes or
> documentation.
> 
> Thank you for your time and for reviewing the patch.

Guenter, should this be a debugfs interface, then?

Also an hwmon noob question - isn't it better for the monitoring
interface to report frequency error / instability in this case
instead of absolute value? Or do you not know the expected freq?

Re: [PATCH net-next 3/3] dpll: zl3073x: add hwmon support for input reference frequencies

[Guenter Roeck]

On 3/23/26 15:48, Jakub Kicinski wrote:
> On Fri, 20 Mar 2026 14:48:01 +0100 Ivan Vecera wrote:
>> On 3/20/26 1:21 PM, Guenter Roeck wrote:
>>> On 3/20/26 03:59, Ivan Vecera wrote:
>>>> Expose measured input reference frequencies via the hwmon interface
>>>> using custom sysfs attributes (freqN_input and freqN_label) since
>>>> hwmon has no native frequency sensor type. The frequency values are
>>>> read from the cached measurements updated by the periodic work thread.
>>>>
>>>> Cache the device ready state in struct zl3073x_dev so that
>>>> freq_input_show() can return -ENODATA without an I2C access when
>>>> the device firmware is not configured.
>>>>
>>>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>>>
>>> "frequency" is not a hardware monitoring attribute. I understand that it is
>>> convenient to report it as one, and that other drivers implement it as
>>> well,
>>> but that doesn't change that.
>>>
>>> I understand that the code lives outside the hardware monitoring
>>> subsystem and is
>>> thus not in control of its maintainers, so you can essentially do
>>> whatever you want,
>>> even if it is wrong. That doesn't change the fact that it is wrong.
>>>
>>> However, do _not_ try to add it into the official list of hardware
>>> monitoring
>>> attributes. I would NACK that.
>>
>> Understood. I recognize that frequency falls outside the strict scope of
>> hardware monitoring and does not belong in the official hwmon ABI.
>>
>> I'm using it here as a convenient way to expose these specific driver
>> metrics, but I hear you loud and clear. I will absolutely not propose
>> adding frequency to the official list of hwmon attributes or
>> documentation.
>>
>> Thank you for your time and for reviewing the patch.
> 
> Guenter, should this be a debugfs interface, then?
> 

There is nothing that prevents the actual (generated ?) frequency to
be reported as sysfs attribute attached to the chip (spi) driver, if
it is indeed of interest. If it is of interest for all dpll drivers,
it could be attached to the dpll device, the chip driver could make it
available via dpll_device_ops to the dpll subsystem, and the dpll
subsystem could provide a common API function (such as the existing
temp_get) and generate a common set of sysfs attributes for all dpll
devices.

> Also an hwmon noob question - isn't it better for the monitoring
> interface to report frequency error / instability in this case
> instead of absolute value? Or do you not know the expected freq?
> 
In the hardware monitoring world one would have min/max attributes and
one or more alarm attributes. I never heard about the idea of reporting
deviations, and for typical hardware monitoring attributes it does not
really make sense. What would be a "deviation" of a temperature/
voltage/current/power/humidity sensor ? Such attributes typically have
an operational range, and they are allowed and even expected to
fluctuate within that range.

Thanks,
Guenter

Re: [PATCH net-next 3/3] dpll: zl3073x: add hwmon support for input reference frequencies

[Paolo Abeni]

On 3/24/26 6:16 AM, Guenter Roeck wrote:
> On 3/23/26 15:48, Jakub Kicinski wrote:
>> On Fri, 20 Mar 2026 14:48:01 +0100 Ivan Vecera wrote:
>>> On 3/20/26 1:21 PM, Guenter Roeck wrote:
>>>> On 3/20/26 03:59, Ivan Vecera wrote:
>>>>> Expose measured input reference frequencies via the hwmon interface
>>>>> using custom sysfs attributes (freqN_input and freqN_label) since
>>>>> hwmon has no native frequency sensor type. The frequency values are
>>>>> read from the cached measurements updated by the periodic work thread.
>>>>>
>>>>> Cache the device ready state in struct zl3073x_dev so that
>>>>> freq_input_show() can return -ENODATA without an I2C access when
>>>>> the device firmware is not configured.
>>>>>
>>>>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>>>>
>>>> "frequency" is not a hardware monitoring attribute. I understand that it is
>>>> convenient to report it as one, and that other drivers implement it as
>>>> well,
>>>> but that doesn't change that.
>>>>
>>>> I understand that the code lives outside the hardware monitoring
>>>> subsystem and is
>>>> thus not in control of its maintainers, so you can essentially do
>>>> whatever you want,
>>>> even if it is wrong. That doesn't change the fact that it is wrong.
>>>>
>>>> However, do _not_ try to add it into the official list of hardware
>>>> monitoring
>>>> attributes. I would NACK that.
>>>
>>> Understood. I recognize that frequency falls outside the strict scope of
>>> hardware monitoring and does not belong in the official hwmon ABI.
>>>
>>> I'm using it here as a convenient way to expose these specific driver
>>> metrics, but I hear you loud and clear. I will absolutely not propose
>>> adding frequency to the official list of hwmon attributes or
>>> documentation.
>>>
>>> Thank you for your time and for reviewing the patch.
>>
>> Guenter, should this be a debugfs interface, then?
>>
> 
> There is nothing that prevents the actual (generated ?) frequency to
> be reported as sysfs attribute attached to the chip (spi) driver, if
> it is indeed of interest. If it is of interest for all dpll drivers,
> it could be attached to the dpll device, the chip driver could make it
> available via dpll_device_ops to the dpll subsystem, and the dpll
> subsystem could provide a common API function (such as the existing
> temp_get) and generate a common set of sysfs attributes for all dpll
> devices.
> 
>> Also an hwmon noob question - isn't it better for the monitoring
>> interface to report frequency error / instability in this case
>> instead of absolute value? Or do you not know the expected freq?
>>
> In the hardware monitoring world one would have min/max attributes and
> one or more alarm attributes. I never heard about the idea of reporting
> deviations, and for typical hardware monitoring attributes it does not
> really make sense. What would be a "deviation" of a temperature/
> voltage/current/power/humidity sensor ? Such attributes typically have
> an operational range, and they are allowed and even expected to
> fluctuate within that range.

Ivan, my take on all the above is that using the HWmon interface here is
stretching it too much. I think it would be better to move debugfs
and/or netlink events.

/P

Re: [PATCH net-next 3/3] dpll: zl3073x: add hwmon support for input reference frequencies

[Ivan Vecera]

Hi Paolo,

On 3/24/26 11:49 AM, Paolo Abeni wrote:
> On 3/24/26 6:16 AM, Guenter Roeck wrote:
>> On 3/23/26 15:48, Jakub Kicinski wrote:
>>> On Fri, 20 Mar 2026 14:48:01 +0100 Ivan Vecera wrote:
>>>> On 3/20/26 1:21 PM, Guenter Roeck wrote:
>>>>> On 3/20/26 03:59, Ivan Vecera wrote:
>>>>>> Expose measured input reference frequencies via the hwmon interface
>>>>>> using custom sysfs attributes (freqN_input and freqN_label) since
>>>>>> hwmon has no native frequency sensor type. The frequency values are
>>>>>> read from the cached measurements updated by the periodic work thread.
>>>>>>
>>>>>> Cache the device ready state in struct zl3073x_dev so that
>>>>>> freq_input_show() can return -ENODATA without an I2C access when
>>>>>> the device firmware is not configured.
>>>>>>
>>>>>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>>>>>
>>>>> "frequency" is not a hardware monitoring attribute. I understand that it is
>>>>> convenient to report it as one, and that other drivers implement it as
>>>>> well,
>>>>> but that doesn't change that.
>>>>>
>>>>> I understand that the code lives outside the hardware monitoring
>>>>> subsystem and is
>>>>> thus not in control of its maintainers, so you can essentially do
>>>>> whatever you want,
>>>>> even if it is wrong. That doesn't change the fact that it is wrong.
>>>>>
>>>>> However, do _not_ try to add it into the official list of hardware
>>>>> monitoring
>>>>> attributes. I would NACK that.
>>>>
>>>> Understood. I recognize that frequency falls outside the strict scope of
>>>> hardware monitoring and does not belong in the official hwmon ABI.
>>>>
>>>> I'm using it here as a convenient way to expose these specific driver
>>>> metrics, but I hear you loud and clear. I will absolutely not propose
>>>> adding frequency to the official list of hwmon attributes or
>>>> documentation.
>>>>
>>>> Thank you for your time and for reviewing the patch.
>>>
>>> Guenter, should this be a debugfs interface, then?
>>>
>>
>> There is nothing that prevents the actual (generated ?) frequency to
>> be reported as sysfs attribute attached to the chip (spi) driver, if
>> it is indeed of interest. If it is of interest for all dpll drivers,
>> it could be attached to the dpll device, the chip driver could make it
>> available via dpll_device_ops to the dpll subsystem, and the dpll
>> subsystem could provide a common API function (such as the existing
>> temp_get) and generate a common set of sysfs attributes for all dpll
>> devices.
>>
>>> Also an hwmon noob question - isn't it better for the monitoring
>>> interface to report frequency error / instability in this case
>>> instead of absolute value? Or do you not know the expected freq?
>>>
>> In the hardware monitoring world one would have min/max attributes and
>> one or more alarm attributes. I never heard about the idea of reporting
>> deviations, and for typical hardware monitoring attributes it does not
>> really make sense. What would be a "deviation" of a temperature/
>> voltage/current/power/humidity sensor ? Such attributes typically have
>> an operational range, and they are allowed and even expected to
>> fluctuate within that range.
> 
> Ivan, my take on all the above is that using the HWmon interface here is
> stretching it too much. I think it would be better to move debugfs
> and/or netlink events.

I'd rather avoid debugfs... My proposal is to expose absolute measured
frequency attribute of dpll-pin and follow phase-offset-monitor
functionality:

So:
* add real-frequency attribute to dpll pin
* add real-frequency-monitor attribute dpll device
* user will be able to enable/disable monitoring by enabling/disabling
   real-frequency-monitor feature (similarly to phase-offset-monitor)

Thoughts?

Thanks,
Ivan

Re: [PATCH net-next 3/3] dpll: zl3073x: add hwmon support for input reference frequencies

[Jakub Kicinski]

On Tue, 24 Mar 2026 13:59:19 +0100 Ivan Vecera wrote:
> >> In the hardware monitoring world one would have min/max attributes and
> >> one or more alarm attributes. I never heard about the idea of reporting
> >> deviations, and for typical hardware monitoring attributes it does not
> >> really make sense. What would be a "deviation" of a temperature/
> >> voltage/current/power/humidity sensor ? Such attributes typically have
> >> an operational range, and they are allowed and even expected to
> >> fluctuate within that range.  
> > 
> > Ivan, my take on all the above is that using the HWmon interface here is
> > stretching it too much. I think it would be better to move debugfs
> > and/or netlink events.  
> 
> I'd rather avoid debugfs... My proposal is to expose absolute measured
> frequency attribute of dpll-pin and follow phase-offset-monitor
> functionality:
> 
> So:
> * add real-frequency attribute to dpll pin
> * add real-frequency-monitor attribute dpll device
> * user will be able to enable/disable monitoring by enabling/disabling
>    real-frequency-monitor feature (similarly to phase-offset-monitor)
> 
> Thoughts?

I don't have a strong opinion. IDK where to draw the line between DPLL
and "random functionality some devices may have". We have 3 DPLL
maintainers, let's see what their majority opinion is..

Re: [PATCH net-next 3/3] dpll: zl3073x: add hwmon support for input reference frequencies

[Ivan Vecera]

On 3/24/26 10:36 PM, Jakub Kicinski wrote:
> On Tue, 24 Mar 2026 13:59:19 +0100 Ivan Vecera wrote:
>>>> In the hardware monitoring world one would have min/max attributes and
>>>> one or more alarm attributes. I never heard about the idea of reporting
>>>> deviations, and for typical hardware monitoring attributes it does not
>>>> really make sense. What would be a "deviation" of a temperature/
>>>> voltage/current/power/humidity sensor ? Such attributes typically have
>>>> an operational range, and they are allowed and even expected to
>>>> fluctuate within that range.
>>>
>>> Ivan, my take on all the above is that using the HWmon interface here is
>>> stretching it too much. I think it would be better to move debugfs
>>> and/or netlink events.
>>
>> I'd rather avoid debugfs... My proposal is to expose absolute measured
>> frequency attribute of dpll-pin and follow phase-offset-monitor
>> functionality:
>>
>> So:
>> * add real-frequency attribute to dpll pin
>> * add real-frequency-monitor attribute dpll device
>> * user will be able to enable/disable monitoring by enabling/disabling
>>     real-frequency-monitor feature (similarly to phase-offset-monitor)
>>
>> Thoughts?
> 
> I don't have a strong opinion. IDK where to draw the line between DPLL
> and "random functionality some devices may have". We have 3 DPLL
> maintainers, let's see what their majority opinion is..

I think that line has already been crossed by temperature reporting,
which has been there since the beginning. Actual frequency measurement
is, in my opinion, much closer to the DPLL world than temperature 
reporting. :-)

Anyway, I have the relevant patch-set already prepared, so I can submit
it if you want to see it.

Thanks,
Ivan

Re: [PATCH net-next 3/3] dpll: zl3073x: add hwmon support for input reference frequencies

[Vadim Fedorenko]

On 25/03/2026 11:19, Ivan Vecera wrote:
> 
> 
> On 3/24/26 10:36 PM, Jakub Kicinski wrote:
>> On Tue, 24 Mar 2026 13:59:19 +0100 Ivan Vecera wrote:
>>>>> In the hardware monitoring world one would have min/max attributes and
>>>>> one or more alarm attributes. I never heard about the idea of 
>>>>> reporting
>>>>> deviations, and for typical hardware monitoring attributes it does not
>>>>> really make sense. What would be a "deviation" of a temperature/
>>>>> voltage/current/power/humidity sensor ? Such attributes typically have
>>>>> an operational range, and they are allowed and even expected to
>>>>> fluctuate within that range.
>>>>
>>>> Ivan, my take on all the above is that using the HWmon interface 
>>>> here is
>>>> stretching it too much. I think it would be better to move debugfs
>>>> and/or netlink events.
>>>
>>> I'd rather avoid debugfs... My proposal is to expose absolute measured
>>> frequency attribute of dpll-pin and follow phase-offset-monitor
>>> functionality:
>>>
>>> So:
>>> * add real-frequency attribute to dpll pin
>>> * add real-frequency-monitor attribute dpll device
>>> * user will be able to enable/disable monitoring by enabling/disabling
>>>     real-frequency-monitor feature (similarly to phase-offset-monitor)
>>>
>>> Thoughts?
>>
>> I don't have a strong opinion. IDK where to draw the line between DPLL
>> and "random functionality some devices may have". We have 3 DPLL
>> maintainers, let's see what their majority opinion is..
> 
> I think that line has already been crossed by temperature reporting,
> which has been there since the beginning. Actual frequency measurement
> is, in my opinion, much closer to the DPLL world than temperature 
> reporting. :-)

Frequency measurements are DPLL-world properties indeed. Temperature
reporting was added as it has some influence on the stability of DPLL 
devices as well as on any oscillators.

> Anyway, I have the relevant patch-set already prepared, so I can submit
> it if you want to see it.
Feel free to submit, I'll be happy to review it.

Re: [PATCH net-next 3/3] dpll: zl3073x: add hwmon support for input reference frequencies

[Ivan Vecera]

On 3/25/26 4:56 PM, Vadim Fedorenko wrote:
> On 25/03/2026 11:19, Ivan Vecera wrote:
>>
>>
>> On 3/24/26 10:36 PM, Jakub Kicinski wrote:
>>> On Tue, 24 Mar 2026 13:59:19 +0100 Ivan Vecera wrote:
>>>>>> In the hardware monitoring world one would have min/max attributes 
>>>>>> and
>>>>>> one or more alarm attributes. I never heard about the idea of 
>>>>>> reporting
>>>>>> deviations, and for typical hardware monitoring attributes it does 
>>>>>> not
>>>>>> really make sense. What would be a "deviation" of a temperature/
>>>>>> voltage/current/power/humidity sensor ? Such attributes typically 
>>>>>> have
>>>>>> an operational range, and they are allowed and even expected to
>>>>>> fluctuate within that range.
>>>>>
>>>>> Ivan, my take on all the above is that using the HWmon interface 
>>>>> here is
>>>>> stretching it too much. I think it would be better to move debugfs
>>>>> and/or netlink events.
>>>>
>>>> I'd rather avoid debugfs... My proposal is to expose absolute measured
>>>> frequency attribute of dpll-pin and follow phase-offset-monitor
>>>> functionality:
>>>>
>>>> So:
>>>> * add real-frequency attribute to dpll pin
>>>> * add real-frequency-monitor attribute dpll device
>>>> * user will be able to enable/disable monitoring by enabling/disabling
>>>>     real-frequency-monitor feature (similarly to phase-offset-monitor)
>>>>
>>>> Thoughts?
>>>
>>> I don't have a strong opinion. IDK where to draw the line between DPLL
>>> and "random functionality some devices may have". We have 3 DPLL
>>> maintainers, let's see what their majority opinion is..
>>
>> I think that line has already been crossed by temperature reporting,
>> which has been there since the beginning. Actual frequency measurement
>> is, in my opinion, much closer to the DPLL world than temperature 
>> reporting. :-)
> 
> Frequency measurements are DPLL-world properties indeed. Temperature
> reporting was added as it has some influence on the stability of DPLL 
> devices as well as on any oscillators.
> 
>> Anyway, I have the relevant patch-set already prepared, so I can submit
>> it if you want to see it.
> Feel free to submit, I'll be happy to review it.
> 
Thanks, will do..

Ivan