[PATCH net-next 0/2] dpll: Add support for phase adjustment granularity

[PATCH net-next 0/2] dpll: Add support for phase adjustment granularity

[Ivan Vecera]

Phase-adjust values are currently limited only 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.

Patch 1: Adds 'phase-adjust-gran' pin attribute and an appropriate
         handling
Patch 2: Adds a support for this attribute into zl3073x driver

Ivan Vecera (2):
  dpll: add phase-adjust-gran pin attribute
  dpll: zl3073x: Specify phase adjustment granularity for pins

 Documentation/driver-api/dpll.rst     | 36 +++++++++--------
 Documentation/netlink/specs/dpll.yaml |  7 ++++
 drivers/dpll/dpll_netlink.c           | 12 +++++-
 drivers/dpll/zl3073x/dpll.c           | 58 +++++++--------------------
 drivers/dpll/zl3073x/prop.c           | 11 +++++
 include/linux/dpll.h                  |  1 +
 include/uapi/linux/dpll.h             |  1 +
 7 files changed, 65 insertions(+), 61 deletions(-)

-- 
2.51.0

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

[Ivan Vecera]

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 <mschmidt@redhat.com>
Reviewed-by: Petr Oros <poros@redhat.com>
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(-)

diff --git a/Documentation/driver-api/dpll.rst b/Documentation/driver-api/dpll.rst
index be1fc643b645e..83118c728ed90 100644
--- a/Documentation/driver-api/dpll.rst
+++ b/Documentation/driver-api/dpll.rst
@@ -198,26 +198,28 @@ be requested with the same attribute with ``DPLL_CMD_DEVICE_SET`` command.
   ================================== ======================================
 
 Device may also provide ability to adjust a signal phase on a pin.
-If pin phase adjustment is supported, minimal and maximal values that pin
-handle shall be provide to the user on ``DPLL_CMD_PIN_GET`` respond
-with ``DPLL_A_PIN_PHASE_ADJUST_MIN`` and ``DPLL_A_PIN_PHASE_ADJUST_MAX``
+If pin phase adjustment is supported, minimal and maximal values and
+granularity that pin handle shall be provided to the user on
+``DPLL_CMD_PIN_GET`` respond with ``DPLL_A_PIN_PHASE_ADJUST_MIN``,
+``DPLL_A_PIN_PHASE_ADJUST_MAX`` and ``DPLL_A_PIN_PHASE_ADJUST_GRAN``
 attributes. Configured phase adjust value is provided with
 ``DPLL_A_PIN_PHASE_ADJUST`` attribute of a pin, and value change can be
 requested with the same attribute with ``DPLL_CMD_PIN_SET`` command.
 
-  =============================== ======================================
-  ``DPLL_A_PIN_ID``               configured pin id
-  ``DPLL_A_PIN_PHASE_ADJUST_MIN`` attr minimum value of phase adjustment
-  ``DPLL_A_PIN_PHASE_ADJUST_MAX`` attr maximum value of phase adjustment
-  ``DPLL_A_PIN_PHASE_ADJUST``     attr configured value of phase
-                                  adjustment on parent dpll device
-  ``DPLL_A_PIN_PARENT_DEVICE``    nested attribute for requesting
-                                  configuration on given parent dpll
-                                  device
-    ``DPLL_A_PIN_PARENT_ID``      parent dpll device id
-    ``DPLL_A_PIN_PHASE_OFFSET``   attr measured phase difference
-                                  between a pin and parent dpll device
-  =============================== ======================================
+  ================================ ==========================================
+  ``DPLL_A_PIN_ID``                configured pin id
+  ``DPLL_A_PIN_PHASE_ADJUST_GRAN`` attr granularity of phase adjustment value
+  ``DPLL_A_PIN_PHASE_ADJUST_MIN``  attr minimum value of phase adjustment
+  ``DPLL_A_PIN_PHASE_ADJUST_MAX``  attr maximum value of phase adjustment
+  ``DPLL_A_PIN_PHASE_ADJUST``      attr configured value of phase
+                                   adjustment on parent dpll device
+  ``DPLL_A_PIN_PARENT_DEVICE``     nested attribute for requesting
+                                   configuration on given parent dpll
+                                   device
+    ``DPLL_A_PIN_PARENT_ID``       parent dpll device id
+    ``DPLL_A_PIN_PHASE_OFFSET``    attr measured phase difference
+                                   between a pin and parent dpll device
+  ================================ ==========================================
 
 All phase related values are provided in pico seconds, which represents
 time difference between signals phase. The negative value means that
@@ -384,6 +386,8 @@ according to attribute purpose.
                                        frequencies
       ``DPLL_A_PIN_ANY_FREQUENCY_MIN`` attr minimum value of frequency
       ``DPLL_A_PIN_ANY_FREQUENCY_MAX`` attr maximum value of frequency
+    ``DPLL_A_PIN_PHASE_ADJUST_GRAN``   attr granularity of phase
+                                       adjustment value
     ``DPLL_A_PIN_PHASE_ADJUST_MIN``    attr minimum value of phase
                                        adjustment
     ``DPLL_A_PIN_PHASE_ADJUST_MAX``    attr maximum value of phase
diff --git a/Documentation/netlink/specs/dpll.yaml b/Documentation/netlink/specs/dpll.yaml
index cafb4ec20447e..e9b476b5db1b4 100644
--- a/Documentation/netlink/specs/dpll.yaml
+++ b/Documentation/netlink/specs/dpll.yaml
@@ -440,6 +440,12 @@ attribute-sets:
         doc: |
           Capable pin provides list of pins that can be bound to create a
           reference-sync pin pair.
+      -
+        name: phase-adjust-gran
+        type: s32
+        doc: |
+          Granularity of phase adjustment, in picoseconds. The value of
+          phase adjustment must be a multiple of this granularity.
 
   -
     name: pin-parent-device
@@ -614,6 +620,7 @@ operations:
             - capabilities
             - parent-device
             - parent-pin
+            - phase-adjust-gran
             - phase-adjust-min
             - phase-adjust-max
             - phase-adjust
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index 74c1f0ca95f24..25d3a46e889b5 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -637,6 +637,10 @@ dpll_cmd_pin_get_one(struct sk_buff *msg, struct dpll_pin *pin,
 	ret = dpll_msg_add_pin_freq(msg, pin, ref, extack);
 	if (ret)
 		return ret;
+	if (prop->phase_gran &&
+	    nla_put_s32(msg, DPLL_A_PIN_PHASE_ADJUST_GRAN,
+			prop->phase_gran))
+		return -EMSGSIZE;
 	if (nla_put_s32(msg, DPLL_A_PIN_PHASE_ADJUST_MIN,
 			prop->phase_range.min))
 		return -EMSGSIZE;
@@ -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);
 		return -EINVAL;
 	}
 
diff --git a/include/linux/dpll.h b/include/linux/dpll.h
index 25be745bf41f1..4455d095925e9 100644
--- a/include/linux/dpll.h
+++ b/include/linux/dpll.h
@@ -163,6 +163,7 @@ struct dpll_pin_properties {
 	u32 freq_supported_num;
 	struct dpll_pin_frequency *freq_supported;
 	struct dpll_pin_phase_adjust_range phase_range;
+	s32 phase_gran;
 };
 
 #if IS_ENABLED(CONFIG_DPLL)
diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h
index ab1725a954d74..69d35570ac4f1 100644
--- a/include/uapi/linux/dpll.h
+++ b/include/uapi/linux/dpll.h
@@ -251,6 +251,7 @@ enum dpll_a_pin {
 	DPLL_A_PIN_ESYNC_FREQUENCY_SUPPORTED,
 	DPLL_A_PIN_ESYNC_PULSE,
 	DPLL_A_PIN_REFERENCE_SYNC,
+	DPLL_A_PIN_PHASE_ADJUST_GRAN,
 
 	__DPLL_A_PIN_MAX,
 	DPLL_A_PIN_MAX = (__DPLL_A_PIN_MAX - 1)
-- 
2.51.0

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

[Jakub Kicinski]

On Fri, 24 Oct 2025 16:49:26 +0200 Ivan Vecera wrote:
> +      -
> +        name: phase-adjust-gran
> +        type: s32
> +        doc: |
> +          Granularity of phase adjustment, in picoseconds. The value of
> +          phase adjustment must be a multiple of this granularity.

Do we need this to be signed?

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

[Ivan Vecera]

Hi Kuba,

On 10/29/25 2:39 AM, Jakub Kicinski wrote:
> On Fri, 24 Oct 2025 16:49:26 +0200 Ivan Vecera wrote:
>> +      -
>> +        name: phase-adjust-gran
>> +        type: s32
>> +        doc: |
>> +          Granularity of phase adjustment, in picoseconds. The value of
>> +          phase adjustment must be a multiple of this granularity.
> 
> Do we need this to be signed?
> 
To have it unsigned brings a need to use explicit type casting in the 
core and driver's code. The phase adjustment can be both positive and
negative it has to be signed. The granularity specifies that adjustment
has to be multiple of granularity value so the core checks for zero
remainder (this patch) and the driver converts the given adjustment
value using division by the granularity.

If we would have phase-adjust-gran and corresponding structure fields
defined as u32 then we have to explicitly cast the granularity to s32
because for:

<snip>
s32 phase_adjust, remainder;
u32 phase_gran = 1000;

phase_adjust = 5000;
remainder = phase_adjust % phase_gran;
/* remainder = 0 -> OK for positive adjust */

phase_adjust = -5000;
remainder = phase_adjust % phase_gran;
/* remainder = 296
  * Wrong for negative adjustment because phase_adjust is casted to u32
  * prior division -> 2^32 - 5000 = 4294962296.
  * 4294962296 % 1000 = 296
  */

  remainder = phase_adjust % (s32)phase_gran;
  /* remainder = 0
   * Now OK because phase_adjust remains to be s32
   */
</snip>

Similarly for division in the driver code if the granularity would be
u32.

So I have proposed phase adjustment granularity to be s32 to avoid these
explicit type castings and potential bugs in drivers.

Thanks,
Ivan

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

[Jiri Pirko]

Wed, Oct 29, 2025 at 08:44:52AM +0100, ivecera@redhat.com wrote:
>Hi Kuba,
>
>On 10/29/25 2:39 AM, Jakub Kicinski wrote:
>> On Fri, 24 Oct 2025 16:49:26 +0200 Ivan Vecera wrote:
>> > +      -
>> > +        name: phase-adjust-gran
>> > +        type: s32
>> > +        doc: |
>> > +          Granularity of phase adjustment, in picoseconds. The value of
>> > +          phase adjustment must be a multiple of this granularity.
>> 
>> Do we need this to be signed?
>> 
>To have it unsigned brings a need to use explicit type casting in the core
>and driver's code. The phase adjustment can be both positive and
>negative it has to be signed. The granularity specifies that adjustment
>has to be multiple of granularity value so the core checks for zero
>remainder (this patch) and the driver converts the given adjustment
>value using division by the granularity.
>
>If we would have phase-adjust-gran and corresponding structure fields
>defined as u32 then we have to explicitly cast the granularity to s32
>because for:

I prefer cast. The uapi should be clear. There is not point of having
negative granularity.


>
><snip>
>s32 phase_adjust, remainder;
>u32 phase_gran = 1000;
>
>phase_adjust = 5000;
>remainder = phase_adjust % phase_gran;
>/* remainder = 0 -> OK for positive adjust */
>
>phase_adjust = -5000;
>remainder = phase_adjust % phase_gran;
>/* remainder = 296
> * Wrong for negative adjustment because phase_adjust is casted to u32
> * prior division -> 2^32 - 5000 = 4294962296.
> * 4294962296 % 1000 = 296
> */
>
> remainder = phase_adjust % (s32)phase_gran;
> /* remainder = 0
>  * Now OK because phase_adjust remains to be s32
>  */
></snip>
>
>Similarly for division in the driver code if the granularity would be
>u32.
>
>So I have proposed phase adjustment granularity to be s32 to avoid these
>explicit type castings and potential bugs in drivers.

Cast in dpll core, no?


>
>Thanks,
>Ivan
>

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

[Ivan Vecera]

On 10/29/25 3:20 PM, Jiri Pirko wrote:
> Wed, Oct 29, 2025 at 08:44:52AM +0100, ivecera@redhat.com wrote:
>> Hi Kuba,
>>
>> On 10/29/25 2:39 AM, Jakub Kicinski wrote:
>>> On Fri, 24 Oct 2025 16:49:26 +0200 Ivan Vecera wrote:
>>>> +      -
>>>> +        name: phase-adjust-gran
>>>> +        type: s32
>>>> +        doc: |
>>>> +          Granularity of phase adjustment, in picoseconds. The value of
>>>> +          phase adjustment must be a multiple of this granularity.
>>>
>>> Do we need this to be signed?
>>>
>> To have it unsigned brings a need to use explicit type casting in the core
>> and driver's code. The phase adjustment can be both positive and
>> negative it has to be signed. The granularity specifies that adjustment
>> has to be multiple of granularity value so the core checks for zero
>> remainder (this patch) and the driver converts the given adjustment
>> value using division by the granularity.
>>
>> If we would have phase-adjust-gran and corresponding structure fields
>> defined as u32 then we have to explicitly cast the granularity to s32
>> because for:
> 
> I prefer cast. The uapi should be clear. There is not point of having
> negative granularity.
> 
> 
I will use u32 for phase-adjust-gran and dpll_pin_properties.phase_gran.

OK?
>> <snip>
>> s32 phase_adjust, remainder;
>> u32 phase_gran = 1000;
>>
>> phase_adjust = 5000;
>> remainder = phase_adjust % phase_gran;
>> /* remainder = 0 -> OK for positive adjust */
>>
>> phase_adjust = -5000;
>> remainder = phase_adjust % phase_gran;
>> /* remainder = 296
>> * Wrong for negative adjustment because phase_adjust is casted to u32
>> * prior division -> 2^32 - 5000 = 4294962296.
>> * 4294962296 % 1000 = 296
>> */
>>
>> remainder = phase_adjust % (s32)phase_gran;
>> /* remainder = 0
>>   * Now OK because phase_adjust remains to be s32
>>   */
>> </snip>
>>
>> Similarly for division in the driver code if the granularity would be
>> u32.
>>
>> So I have proposed phase adjustment granularity to be s32 to avoid these
>> explicit type castings and potential bugs in drivers.
> 
> Cast in dpll core, no?
Depends... if the driver will use s32 (sse patch 2) then no castings are 
necessary.

Thanks,
Ivan

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

[Vadim Fedorenko]

On 24/10/2025 15:49, Ivan Vecera wrote:
> 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 <mschmidt@redhat.com>
> Reviewed-by: Petr Oros <poros@redhat.com>
> 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?

>   		return -EINVAL;
>   	}
>   

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

[Ivan Vecera]

On 10/29/25 11:20 AM, Vadim Fedorenko wrote:
> On 24/10/2025 15:49, Ivan Vecera wrote:
>> 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 <mschmidt@redhat.com>
>> Reviewed-by: Petr Oros <poros@redhat.com>
>> 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

[PATCH net-next 2/2] dpll: zl3073x: Specify phase adjustment granularity for pins

[Ivan Vecera]

Output pins phase adjustment values in the device are expressed
in half synth clock cycles. Use this number of cycles as output
pins' phase adjust granularity and simplify both get/set callbacks.

Reviewed-by: Michal Schmidt <mschmidt@redhat.com>
Reviewed-by: Petr Oros <poros@redhat.com>
Tested-by: Prathosh Satish <Prathosh.Satish@microchip.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/dpll.c | 58 +++++++++----------------------------
 drivers/dpll/zl3073x/prop.c | 11 +++++++
 2 files changed, 25 insertions(+), 44 deletions(-)

diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index 93dc93eec79ed..b13eb4e342d58 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -35,6 +35,7 @@
  * @prio: pin priority <0, 14>
  * @selectable: pin is selectable in automatic mode
  * @esync_control: embedded sync is controllable
+ * @phase_gran: phase adjustment granularity
  * @pin_state: last saved pin state
  * @phase_offset: last saved pin phase offset
  * @freq_offset: last saved fractional frequency offset
@@ -49,6 +50,7 @@ struct zl3073x_dpll_pin {
 	u8			prio;
 	bool			selectable;
 	bool			esync_control;
+	s32			phase_gran;
 	enum dpll_pin_state	pin_state;
 	s64			phase_offset;
 	s64			freq_offset;
@@ -1388,25 +1390,14 @@ zl3073x_dpll_output_pin_phase_adjust_get(const struct dpll_pin *dpll_pin,
 	struct zl3073x_dpll *zldpll = dpll_priv;
 	struct zl3073x_dev *zldev = zldpll->dev;
 	struct zl3073x_dpll_pin *pin = pin_priv;
-	u32 synth_freq;
 	s32 phase_comp;
-	u8 out, synth;
+	u8 out;
 	int rc;
 
-	out = zl3073x_output_pin_out_get(pin->id);
-	synth = zl3073x_out_synth_get(zldev, out);
-	synth_freq = zl3073x_synth_freq_get(zldev, synth);
-
-	/* Check synth freq for zero */
-	if (!synth_freq) {
-		dev_err(zldev->dev, "Got zero synth frequency for output %u\n",
-			out);
-		return -EINVAL;
-	}
-
 	guard(mutex)(&zldev->multiop_lock);
 
 	/* Read output configuration */
+	out = zl3073x_output_pin_out_get(pin->id);
 	rc = zl3073x_mb_op(zldev, ZL_REG_OUTPUT_MB_SEM, ZL_OUTPUT_MB_SEM_RD,
 			   ZL_REG_OUTPUT_MB_MASK, BIT(out));
 	if (rc)
@@ -1417,11 +1408,10 @@ zl3073x_dpll_output_pin_phase_adjust_get(const struct dpll_pin *dpll_pin,
 	if (rc)
 		return rc;
 
-	/* Value in register is expressed in half synth clock cycles */
-	phase_comp *= (int)div_u64(PSEC_PER_SEC, 2 * synth_freq);
-
-	/* Reverse two's complement negation applied during 'set' */
-	*phase_adjust = -phase_comp;
+	/* Convert value to ps and reverse two's complement negation applied
+	 * during 'set'
+	 */
+	*phase_adjust = -phase_comp * pin->phase_gran;
 
 	return rc;
 }
@@ -1437,39 +1427,18 @@ zl3073x_dpll_output_pin_phase_adjust_set(const struct dpll_pin *dpll_pin,
 	struct zl3073x_dpll *zldpll = dpll_priv;
 	struct zl3073x_dev *zldev = zldpll->dev;
 	struct zl3073x_dpll_pin *pin = pin_priv;
-	int half_synth_cycle;
-	u32 synth_freq;
-	u8 out, synth;
+	u8 out;
 	int rc;
 
-	/* Get attached synth */
-	out = zl3073x_output_pin_out_get(pin->id);
-	synth = zl3073x_out_synth_get(zldev, out);
-
-	/* Get synth's frequency */
-	synth_freq = zl3073x_synth_freq_get(zldev, synth);
-
-	/* Value in register is expressed in half synth clock cycles so
-	 * the given phase adjustment a multiple of half synth clock.
-	 */
-	half_synth_cycle = (int)div_u64(PSEC_PER_SEC, 2 * synth_freq);
-
-	if ((phase_adjust % half_synth_cycle) != 0) {
-		NL_SET_ERR_MSG_FMT(extack,
-				   "Phase adjustment value has to be multiple of %d",
-				   half_synth_cycle);
-		return -EINVAL;
-	}
-	phase_adjust /= half_synth_cycle;
-
 	/* The value in the register is stored as two's complement negation
-	 * of requested value.
+	 * of requested value and expressed in half synth clock cycles.
 	 */
-	phase_adjust = -phase_adjust;
+	phase_adjust = -phase_adjust / pin->phase_gran;
 
 	guard(mutex)(&zldev->multiop_lock);
 
 	/* Read output configuration */
+	out = zl3073x_output_pin_out_get(pin->id);
 	rc = zl3073x_mb_op(zldev, ZL_REG_OUTPUT_MB_SEM, ZL_OUTPUT_MB_SEM_RD,
 			   ZL_REG_OUTPUT_MB_MASK, BIT(out));
 	if (rc)
@@ -1758,9 +1727,10 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
 	if (IS_ERR(props))
 		return PTR_ERR(props);
 
-	/* Save package label & esync capability */
+	/* Save package label, esync capability and phase adjust granularity */
 	strscpy(pin->label, props->package_label);
 	pin->esync_control = props->esync_control;
+	pin->phase_gran = props->dpll_props.phase_gran;
 
 	if (zl3073x_dpll_is_input_pin(pin)) {
 		rc = zl3073x_dpll_ref_prio_get(pin, &pin->prio);
diff --git a/drivers/dpll/zl3073x/prop.c b/drivers/dpll/zl3073x/prop.c
index 4cf7e8aefcb37..af43b19e35a18 100644
--- a/drivers/dpll/zl3073x/prop.c
+++ b/drivers/dpll/zl3073x/prop.c
@@ -208,7 +208,18 @@ struct zl3073x_pin_props *zl3073x_pin_props_get(struct zl3073x_dev *zldev,
 			DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE |
 			DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;
 	} else {
+		u8 out, synth;
+		s32 f;
+
 		props->dpll_props.type = DPLL_PIN_TYPE_GNSS;
+
+		/* The output pin phase adjustment granularity equals half of
+		 * the synth frequency count.
+		 */
+		out = zl3073x_output_pin_out_get(index);
+		synth = zl3073x_out_synth_get(zldev, out);
+		f = 2 * zl3073x_synth_freq_get(zldev, synth);
+		props->dpll_props.phase_gran = f ? div_s64(PSEC_PER_SEC, f) : 1;
 	}
 
 	props->dpll_props.phase_range.min = S32_MIN;
-- 
2.51.0

Re: [PATCH net-next 0/2] dpll: Add support for phase adjustment granularity

[Jakub Kicinski]

On Fri, 24 Oct 2025 16:49:25 +0200 Ivan Vecera wrote:
> Phase-adjust values are currently limited only 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.

DPLL maintainers, please review