Re: [PATCH net-next 1/2] dpll: add phase-adjust-gran pin attribute

On 10/29/25 11:20 AM, Vadim Fedorenko wrote:

On 24/10/2025 15:49, Ivan Vecera wrote:

quoted

Phase-adjust values are currently limited by a min-max range. Some
hardware requires, for certain pin types, that values be multiples of
a specific granularity, as in the zl3073x driver.

Add a `phase-adjust-gran` pin attribute and an appropriate field in
dpll_pin_properties. If set by the driver, use its value to validate
user-provided phase-adjust values.

Reviewed-by: Michal Schmidt <redacted>
Reviewed-by: Petr Oros <redacted>
Tested-by: Prathosh Satish <Prathosh.Satish@microchip.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
  Documentation/driver-api/dpll.rst     | 36 +++++++++++++++------------
  Documentation/netlink/specs/dpll.yaml |  7 ++++++
  drivers/dpll/dpll_netlink.c           | 12 ++++++++-
  include/linux/dpll.h                  |  1 +
  include/uapi/linux/dpll.h             |  1 +
  5 files changed, 40 insertions(+), 17 deletions(-)

@@ -1261,7 +1265,13 @@ dpll_pin_phase_adj_set(struct dpll_pin *pin, 

struct nlattr *phase_adj_attr,
      if (phase_adj > pin->prop.phase_range.max ||
          phase_adj < pin->prop.phase_range.min) {
          NL_SET_ERR_MSG_ATTR(extack, phase_adj_attr,
-                    "phase adjust value not supported");
+                    "phase adjust value of out range");
+        return -EINVAL;
+    }
+    if (pin->prop.phase_gran && phase_adj % pin->prop.phase_gran) {
+        NL_SET_ERR_MSG_ATTR_FMT(extack, phase_adj_attr,
+                    "phase adjust value not multiple of %u",
+                    pin->prop.phase_gran);

That is pretty strict on the uAPI input. Maybe it's better to allow any
value, but report back the one that is really applied based on driver's
understanding of hardware? I mean the driver is doing some math before
applying the math, it can potentially round any value to the closest
acceptable and report it back?

I’d prefer to use the same approach as for phase-adjust-{min,max}.
Because we could treat them the same way - the user sets a value
above/below the max/min, and the driver clamps it.

Would it be better? I don't think so.

Let’s say the granularity is 1000, and the user sets a value of 499...
then the driver rounds it and sets it to 0. The user then reads the
current value via pin-get and finds that it hasn’t changed - quite
confusing, I’d say. If the user knows the granularity in advance,
they can adjust accordingly.

IMHO, strict behavior is better than smart behavior.

Ivan