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@xxxxxxxxxx>
> > >
> > > "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