[PATCH net-next v11 13/14] dpll: zl3073x: Add support to get/set frequency on input pins

Ivan Vecera posted 14 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH net-next v11 13/14] dpll: zl3073x: Add support to get/set frequency on input pins
Posted by Ivan Vecera 3 months, 3 weeks ago
Add support to get/set frequency on input pins. The frequency for input
pins (references) is computed in the device according this formula:

 freq = base_freq * multiplier * (nominator / denominator)

where the base_freq comes from the list of supported base frequencies
and other parameters are arbitrary numbers. All these parameters are
16-bit unsigned integers.

Co-developed-by: Prathosh Satish <Prathosh.Satish@microchip.com>
Signed-off-by: Prathosh Satish <Prathosh.Satish@microchip.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/dpll.c | 124 ++++++++++++++++++++++++++++++++++++
 drivers/dpll/zl3073x/regs.h |   5 ++
 2 files changed, 129 insertions(+)

diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index f78a5b209fce7..a110109a738c3 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -8,6 +8,7 @@
 #include <linux/dpll.h>
 #include <linux/err.h>
 #include <linux/kthread.h>
+#include <linux/math64.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/netlink.h>
@@ -84,6 +85,127 @@ zl3073x_dpll_pin_direction_get(const struct dpll_pin *dpll_pin, void *pin_priv,
 	return 0;
 }
 
+/**
+ * zl3073x_dpll_input_ref_frequency_get - get input reference frequency
+ * @zldpll: pointer to zl3073x_dpll
+ * @ref_id: reference id
+ * @frequency: pointer to variable to store frequency
+ *
+ * Reads frequency of given input reference.
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int
+zl3073x_dpll_input_ref_frequency_get(struct zl3073x_dpll *zldpll, u8 ref_id,
+				     u32 *frequency)
+{
+	struct zl3073x_dev *zldev = zldpll->dev;
+	u16 base, mult, num, denom;
+	int rc;
+
+	guard(mutex)(&zldev->multiop_lock);
+
+	/* Read reference configuration */
+	rc = zl3073x_mb_op(zldev, ZL_REG_REF_MB_SEM, ZL_REF_MB_SEM_RD,
+			   ZL_REG_REF_MB_MASK, BIT(ref_id));
+	if (rc)
+		return rc;
+
+	/* Read registers to compute resulting frequency */
+	rc = zl3073x_read_u16(zldev, ZL_REG_REF_FREQ_BASE, &base);
+	if (rc)
+		return rc;
+	rc = zl3073x_read_u16(zldev, ZL_REG_REF_FREQ_MULT, &mult);
+	if (rc)
+		return rc;
+	rc = zl3073x_read_u16(zldev, ZL_REG_REF_RATIO_M, &num);
+	if (rc)
+		return rc;
+	rc = zl3073x_read_u16(zldev, ZL_REG_REF_RATIO_N, &denom);
+	if (rc)
+		return rc;
+
+	/* Sanity check that HW has not returned zero denominator */
+	if (!denom) {
+		dev_err(zldev->dev,
+			"Zero divisor for ref %u frequency got from device\n",
+			ref_id);
+		return -EINVAL;
+	}
+
+	/* Compute the frequency */
+	*frequency = base * mult * num / denom;
+
+	return rc;
+}
+
+static int
+zl3073x_dpll_input_pin_frequency_get(const struct dpll_pin *dpll_pin,
+				     void *pin_priv,
+				     const struct dpll_device *dpll,
+				     void *dpll_priv, u64 *frequency,
+				     struct netlink_ext_ack *extack)
+{
+	struct zl3073x_dpll *zldpll = dpll_priv;
+	struct zl3073x_dpll_pin *pin = pin_priv;
+	u32 ref_freq;
+	u8 ref;
+	int rc;
+
+	/* Read and return ref frequency */
+	ref = zl3073x_input_pin_ref_get(pin->id);
+	rc = zl3073x_dpll_input_ref_frequency_get(zldpll, ref, &ref_freq);
+	if (!rc)
+		*frequency = ref_freq;
+
+	return rc;
+}
+
+static int
+zl3073x_dpll_input_pin_frequency_set(const struct dpll_pin *dpll_pin,
+				     void *pin_priv,
+				     const struct dpll_device *dpll,
+				     void *dpll_priv, u64 frequency,
+				     struct netlink_ext_ack *extack)
+{
+	struct zl3073x_dpll *zldpll = dpll_priv;
+	struct zl3073x_dev *zldev = zldpll->dev;
+	struct zl3073x_dpll_pin *pin = pin_priv;
+	u16 base, mult;
+	u8 ref;
+	int rc;
+
+	/* Get base frequency and multiplier for the requested frequency */
+	rc = zl3073x_ref_freq_factorize(frequency, &base, &mult);
+	if (rc)
+		return rc;
+
+	guard(mutex)(&zldev->multiop_lock);
+
+	/* Load reference configuration */
+	ref = zl3073x_input_pin_ref_get(pin->id);
+	rc = zl3073x_mb_op(zldev, ZL_REG_REF_MB_SEM, ZL_REF_MB_SEM_RD,
+			   ZL_REG_REF_MB_MASK, BIT(ref));
+
+	/* Update base frequency, multiplier, numerator & denominator */
+	rc = zl3073x_write_u16(zldev, ZL_REG_REF_FREQ_BASE, base);
+	if (rc)
+		return rc;
+	rc = zl3073x_write_u16(zldev, ZL_REG_REF_FREQ_MULT, mult);
+	if (rc)
+		return rc;
+	rc = zl3073x_write_u16(zldev, ZL_REG_REF_RATIO_M, 1);
+	if (rc)
+		return rc;
+	rc = zl3073x_write_u16(zldev, ZL_REG_REF_RATIO_N, 1);
+	if (rc)
+		return rc;
+
+	/* Commit reference configuration */
+	return zl3073x_mb_op(zldev, ZL_REG_REF_MB_SEM, ZL_REF_MB_SEM_WR,
+			     ZL_REG_REF_MB_MASK, BIT(ref));
+}
+
 /**
  * zl3073x_dpll_selected_ref_get - get currently selected reference
  * @zldpll: pointer to zl3073x_dpll
@@ -592,6 +714,8 @@ zl3073x_dpll_mode_get(const struct dpll_device *dpll, void *dpll_priv,
 
 static const struct dpll_pin_ops zl3073x_dpll_input_pin_ops = {
 	.direction_get = zl3073x_dpll_pin_direction_get,
+	.frequency_get = zl3073x_dpll_input_pin_frequency_get,
+	.frequency_set = zl3073x_dpll_input_pin_frequency_set,
 	.prio_get = zl3073x_dpll_input_pin_prio_get,
 	.prio_set = zl3073x_dpll_input_pin_prio_set,
 	.state_on_dpll_get = zl3073x_dpll_input_pin_state_on_dpll_get,
diff --git a/drivers/dpll/zl3073x/regs.h b/drivers/dpll/zl3073x/regs.h
index 34e905053a1ef..09dd314663dff 100644
--- a/drivers/dpll/zl3073x/regs.h
+++ b/drivers/dpll/zl3073x/regs.h
@@ -135,6 +135,11 @@
 #define ZL_REF_MB_SEM_WR			BIT(0)
 #define ZL_REF_MB_SEM_RD			BIT(1)
 
+#define ZL_REG_REF_FREQ_BASE			ZL_REG(10, 0x05, 2)
+#define ZL_REG_REF_FREQ_MULT			ZL_REG(10, 0x07, 2)
+#define ZL_REG_REF_RATIO_M			ZL_REG(10, 0x09, 2)
+#define ZL_REG_REF_RATIO_N			ZL_REG(10, 0x0b, 2)
+
 #define ZL_REG_REF_CONFIG			ZL_REG(10, 0x0d, 1)
 #define ZL_REF_CONFIG_ENABLE			BIT(0)
 #define ZL_REF_CONFIG_DIFF_EN			BIT(2)
-- 
2.49.0
Re: [PATCH net-next v11 13/14] dpll: zl3073x: Add support to get/set frequency on input pins
Posted by Paolo Abeni 3 months, 3 weeks ago
On 6/16/25 10:14 PM, Ivan Vecera wrote:
> +/**
> + * zl3073x_dpll_input_ref_frequency_get - get input reference frequency
> + * @zldpll: pointer to zl3073x_dpll
> + * @ref_id: reference id
> + * @frequency: pointer to variable to store frequency
> + *
> + * Reads frequency of given input reference.
> + *
> + * Return: 0 on success, <0 on error
> + */
> +static int
> +zl3073x_dpll_input_ref_frequency_get(struct zl3073x_dpll *zldpll, u8 ref_id,
> +				     u32 *frequency)
> +{
> +	struct zl3073x_dev *zldev = zldpll->dev;
> +	u16 base, mult, num, denom;
> +	int rc;
> +
> +	guard(mutex)(&zldev->multiop_lock);
> +
> +	/* Read reference configuration */
> +	rc = zl3073x_mb_op(zldev, ZL_REG_REF_MB_SEM, ZL_REF_MB_SEM_RD,
> +			   ZL_REG_REF_MB_MASK, BIT(ref_id));
> +	if (rc)
> +		return rc;
> +
> +	/* Read registers to compute resulting frequency */
> +	rc = zl3073x_read_u16(zldev, ZL_REG_REF_FREQ_BASE, &base);
> +	if (rc)
> +		return rc;
> +	rc = zl3073x_read_u16(zldev, ZL_REG_REF_FREQ_MULT, &mult);
> +	if (rc)
> +		return rc;
> +	rc = zl3073x_read_u16(zldev, ZL_REG_REF_RATIO_M, &num);
> +	if (rc)
> +		return rc;
> +	rc = zl3073x_read_u16(zldev, ZL_REG_REF_RATIO_N, &denom);
> +	if (rc)
> +		return rc;
> +
> +	/* Sanity check that HW has not returned zero denominator */
> +	if (!denom) {
> +		dev_err(zldev->dev,
> +			"Zero divisor for ref %u frequency got from device\n",
> +			ref_id);
> +		return -EINVAL;
> +	}
> +
> +	/* Compute the frequency */
> +	*frequency = base * mult * num / denom;

As base, mult, num and denom are u16, the above looks like integer
overflow prone.

I think you should explicitly cast to u64, and possibly use a u64 frequency.

/P
Re: [PATCH net-next v11 13/14] dpll: zl3073x: Add support to get/set frequency on input pins
Posted by Vadim Fedorenko 3 months, 3 weeks ago
On 19/06/2025 12:15, Paolo Abeni wrote:
> On 6/16/25 10:14 PM, Ivan Vecera wrote:
>> +/**
>> + * zl3073x_dpll_input_ref_frequency_get - get input reference frequency
>> + * @zldpll: pointer to zl3073x_dpll
>> + * @ref_id: reference id
>> + * @frequency: pointer to variable to store frequency
>> + *
>> + * Reads frequency of given input reference.
>> + *
>> + * Return: 0 on success, <0 on error
>> + */
>> +static int
>> +zl3073x_dpll_input_ref_frequency_get(struct zl3073x_dpll *zldpll, u8 ref_id,
>> +				     u32 *frequency)
>> +{
>> +	struct zl3073x_dev *zldev = zldpll->dev;
>> +	u16 base, mult, num, denom;
>> +	int rc;
>> +
>> +	guard(mutex)(&zldev->multiop_lock);
>> +
>> +	/* Read reference configuration */
>> +	rc = zl3073x_mb_op(zldev, ZL_REG_REF_MB_SEM, ZL_REF_MB_SEM_RD,
>> +			   ZL_REG_REF_MB_MASK, BIT(ref_id));
>> +	if (rc)
>> +		return rc;
>> +
>> +	/* Read registers to compute resulting frequency */
>> +	rc = zl3073x_read_u16(zldev, ZL_REG_REF_FREQ_BASE, &base);
>> +	if (rc)
>> +		return rc;
>> +	rc = zl3073x_read_u16(zldev, ZL_REG_REF_FREQ_MULT, &mult);
>> +	if (rc)
>> +		return rc;
>> +	rc = zl3073x_read_u16(zldev, ZL_REG_REF_RATIO_M, &num);
>> +	if (rc)
>> +		return rc;
>> +	rc = zl3073x_read_u16(zldev, ZL_REG_REF_RATIO_N, &denom);
>> +	if (rc)
>> +		return rc;
>> +
>> +	/* Sanity check that HW has not returned zero denominator */
>> +	if (!denom) {
>> +		dev_err(zldev->dev,
>> +			"Zero divisor for ref %u frequency got from device\n",
>> +			ref_id);
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Compute the frequency */
>> +	*frequency = base * mult * num / denom;
> 
> As base, mult, num and denom are u16, the above looks like integer
> overflow prone.
> 
> I think you should explicitly cast to u64, and possibly use a u64 frequency.

I might be a good idea to use mul_u64_u32_div together with mul_u32_u32?
These macroses will take care of overflow on 32bit platforms as well.

> 
> /P
>
Re: [PATCH net-next v11 13/14] dpll: zl3073x: Add support to get/set frequency on input pins
Posted by Ivan Vecera 3 months, 1 week ago

On 19. 06. 25 2:15 odp., Vadim Fedorenko wrote:
> On 19/06/2025 12:15, Paolo Abeni wrote:
>> On 6/16/25 10:14 PM, Ivan Vecera wrote:
>>> +/**
>>> + * zl3073x_dpll_input_ref_frequency_get - get input reference frequency
>>> + * @zldpll: pointer to zl3073x_dpll
>>> + * @ref_id: reference id
>>> + * @frequency: pointer to variable to store frequency
>>> + *
>>> + * Reads frequency of given input reference.
>>> + *
>>> + * Return: 0 on success, <0 on error
>>> + */
>>> +static int
>>> +zl3073x_dpll_input_ref_frequency_get(struct zl3073x_dpll *zldpll, u8 
>>> ref_id,
>>> +                     u32 *frequency)
>>> +{
>>> +    struct zl3073x_dev *zldev = zldpll->dev;
>>> +    u16 base, mult, num, denom;
>>> +    int rc;
>>> +
>>> +    guard(mutex)(&zldev->multiop_lock);
>>> +
>>> +    /* Read reference configuration */
>>> +    rc = zl3073x_mb_op(zldev, ZL_REG_REF_MB_SEM, ZL_REF_MB_SEM_RD,
>>> +               ZL_REG_REF_MB_MASK, BIT(ref_id));
>>> +    if (rc)
>>> +        return rc;
>>> +
>>> +    /* Read registers to compute resulting frequency */
>>> +    rc = zl3073x_read_u16(zldev, ZL_REG_REF_FREQ_BASE, &base);
>>> +    if (rc)
>>> +        return rc;
>>> +    rc = zl3073x_read_u16(zldev, ZL_REG_REF_FREQ_MULT, &mult);
>>> +    if (rc)
>>> +        return rc;
>>> +    rc = zl3073x_read_u16(zldev, ZL_REG_REF_RATIO_M, &num);
>>> +    if (rc)
>>> +        return rc;
>>> +    rc = zl3073x_read_u16(zldev, ZL_REG_REF_RATIO_N, &denom);
>>> +    if (rc)
>>> +        return rc;
>>> +
>>> +    /* Sanity check that HW has not returned zero denominator */
>>> +    if (!denom) {
>>> +        dev_err(zldev->dev,
>>> +            "Zero divisor for ref %u frequency got from device\n",
>>> +            ref_id);
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    /* Compute the frequency */
>>> +    *frequency = base * mult * num / denom;
>>
>> As base, mult, num and denom are u16, the above looks like integer
>> overflow prone.
>>
>> I think you should explicitly cast to u64, and possibly use a u64 
>> frequency.
> 
> I might be a good idea to use mul_u64_u32_div together with mul_u32_u32?
> These macroses will take care of overflow on 32bit platforms as well.

Will fix

Thanks for tip.

Ivan

> 
>>
>> /P
>>
> 

Re: [PATCH net-next v11 13/14] dpll: zl3073x: Add support to get/set frequency on input pins
Posted by Paolo Abeni 3 months, 3 weeks ago
On 6/19/25 2:15 PM, Vadim Fedorenko wrote:
> On 19/06/2025 12:15, Paolo Abeni wrote:
>> On 6/16/25 10:14 PM, Ivan Vecera wrote:
>>> +/**
>>> + * zl3073x_dpll_input_ref_frequency_get - get input reference frequency
>>> + * @zldpll: pointer to zl3073x_dpll
>>> + * @ref_id: reference id
>>> + * @frequency: pointer to variable to store frequency
>>> + *
>>> + * Reads frequency of given input reference.
>>> + *
>>> + * Return: 0 on success, <0 on error
>>> + */
>>> +static int
>>> +zl3073x_dpll_input_ref_frequency_get(struct zl3073x_dpll *zldpll, u8 ref_id,
>>> +				     u32 *frequency)
>>> +{
>>> +	struct zl3073x_dev *zldev = zldpll->dev;
>>> +	u16 base, mult, num, denom;
>>> +	int rc;
>>> +
>>> +	guard(mutex)(&zldev->multiop_lock);
>>> +
>>> +	/* Read reference configuration */
>>> +	rc = zl3073x_mb_op(zldev, ZL_REG_REF_MB_SEM, ZL_REF_MB_SEM_RD,
>>> +			   ZL_REG_REF_MB_MASK, BIT(ref_id));
>>> +	if (rc)
>>> +		return rc;
>>> +
>>> +	/* Read registers to compute resulting frequency */
>>> +	rc = zl3073x_read_u16(zldev, ZL_REG_REF_FREQ_BASE, &base);
>>> +	if (rc)
>>> +		return rc;
>>> +	rc = zl3073x_read_u16(zldev, ZL_REG_REF_FREQ_MULT, &mult);
>>> +	if (rc)
>>> +		return rc;
>>> +	rc = zl3073x_read_u16(zldev, ZL_REG_REF_RATIO_M, &num);
>>> +	if (rc)
>>> +		return rc;
>>> +	rc = zl3073x_read_u16(zldev, ZL_REG_REF_RATIO_N, &denom);
>>> +	if (rc)
>>> +		return rc;
>>> +
>>> +	/* Sanity check that HW has not returned zero denominator */
>>> +	if (!denom) {
>>> +		dev_err(zldev->dev,
>>> +			"Zero divisor for ref %u frequency got from device\n",
>>> +			ref_id);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	/* Compute the frequency */
>>> +	*frequency = base * mult * num / denom;
>>
>> As base, mult, num and denom are u16, the above looks like integer
>> overflow prone.
>>
>> I think you should explicitly cast to u64, and possibly use a u64 frequency.
> 
> I might be a good idea to use mul_u64_u32_div together with mul_u32_u32?
> These macroses will take care of overflow on 32bit platforms as well.

I guess such macros will work, but u64 is available on 32bits platform
as well - and possibly simpler/more readable.

/P