1. Patchew
  2. linux
  3. [PATCH v2 net-next 0/8] dpll/ice: Add TXC DPLL type and full TX reference clock control for E825
  4. View patch

[PATCH v2 net-next 4/8] dpll: zl3073x: allow SyncE_Ref pin state change

Grzegorz Nitka posted 8 patches 1 week, 6 days ago
There is a newer version of this series
[PATCH v2 net-next 4/8] dpll: zl3073x: allow SyncE_Ref pin state change
Posted by Grzegorz Nitka 1 week, 6 days ago 
The SyncE_Ref pin may operate as either an active or inactive reference
depending on board design and system configuration. Some platforms need
to disable the SyncE reference dynamically (e.g., when selecting a
different recovered clock input). The hardware supports toggling this
pin, therefore advertise the STATE_CAN_CHANGE capability.

Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
---
 drivers/dpll/zl3073x/prop.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/dpll/zl3073x/prop.c b/drivers/dpll/zl3073x/prop.c
index ac9d41d0f978..acd7061a741a 100644
--- a/drivers/dpll/zl3073x/prop.c
+++ b/drivers/dpll/zl3073x/prop.c
@@ -215,6 +215,15 @@ struct zl3073x_pin_props *zl3073x_pin_props_get(struct zl3073x_dev *zldev,
 
 		props->dpll_props.type = DPLL_PIN_TYPE_GNSS;
 
+	       /*
+		* The SyncE_Ref pin supports enabling/disabling dynamically.
+		* Some platforms may choose to expose this through firmware
+		* configuration later. For now, advertise this capability
+		* universally since the hardware allows state toggling.
+		*/
+		props->dpll_props.capabilities |=
+			DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;
+
 		/* The output pin phase adjustment granularity equals half of
 		 * the synth frequency count.
 		 */
-- 
2.39.3
Re: [PATCH v2 net-next 4/8] dpll: zl3073x: allow SyncE_Ref pin state change
Posted by Ivan Vecera 1 week, 3 days ago 
On 3/21/26 11:26 PM, Grzegorz Nitka wrote:
> The SyncE_Ref pin may operate as either an active or inactive reference
> depending on board design and system configuration. Some platforms need
> to disable the SyncE reference dynamically (e.g., when selecting a
> different recovered clock input). The hardware supports toggling this
> pin, therefore advertise the STATE_CAN_CHANGE capability.
> 
> Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> ---
>   drivers/dpll/zl3073x/prop.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/dpll/zl3073x/prop.c b/drivers/dpll/zl3073x/prop.c
> index ac9d41d0f978..acd7061a741a 100644
> --- a/drivers/dpll/zl3073x/prop.c
> +++ b/drivers/dpll/zl3073x/prop.c
> @@ -215,6 +215,15 @@ struct zl3073x_pin_props *zl3073x_pin_props_get(struct zl3073x_dev *zldev,
>   
>   		props->dpll_props.type = DPLL_PIN_TYPE_GNSS;
>   
> +	       /*
> +		* The SyncE_Ref pin supports enabling/disabling dynamically.
> +		* Some platforms may choose to expose this through firmware
> +		* configuration later. For now, advertise this capability
> +		* universally since the hardware allows state toggling.
> +		*/
> +		props->dpll_props.capabilities |=
> +			DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;
> +
>   		/* The output pin phase adjustment granularity equals half of
>   		 * the synth frequency count.
>   		 */

I'm wondering about the purpose of this flag, or rather the need to set
it manually from the driver. Surely, the existence of the
state_on_dpll_set() or state_on_pin_set() callback clearly indicates
this capability.

Wouldn't it be simpler for the core to set it directly based on this?

Ivan
RE: [PATCH v2 net-next 4/8] dpll: zl3073x: allow SyncE_Ref pin state change
Posted by Nitka, Grzegorz 1 week, 2 days ago 
> -----Original Message-----
> From: Ivan Vecera <ivecera@redhat.com>
> Sent: Tuesday, March 24, 2026 3:44 PM
> To: Nitka, Grzegorz <grzegorz.nitka@intel.com>; netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; intel-wired-lan@lists.osuosl.org; Oros, Petr
> <poros@redhat.com>; richardcochran@gmail.com;
> andrew+netdev@lunn.ch; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Prathosh.Satish@microchip.com;
> jiri@resnulli.us; Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>;
> vadim.fedorenko@linux.dev; donald.hunter@gmail.com;
> horms@kernel.org; pabeni@redhat.com; kuba@kernel.org;
> davem@davemloft.net; edumazet@google.com; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>
> Subject: Re: [PATCH v2 net-next 4/8] dpll: zl3073x: allow SyncE_Ref pin state
> change
> 
> On 3/21/26 11:26 PM, Grzegorz Nitka wrote:
> > The SyncE_Ref pin may operate as either an active or inactive reference
> > depending on board design and system configuration. Some platforms
> need
> > to disable the SyncE reference dynamically (e.g., when selecting a
> > different recovered clock input). The hardware supports toggling this
> > pin, therefore advertise the STATE_CAN_CHANGE capability.
> >
> > Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> > ---
> >   drivers/dpll/zl3073x/prop.c | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/dpll/zl3073x/prop.c b/drivers/dpll/zl3073x/prop.c
> > index ac9d41d0f978..acd7061a741a 100644
> > --- a/drivers/dpll/zl3073x/prop.c
> > +++ b/drivers/dpll/zl3073x/prop.c
> > @@ -215,6 +215,15 @@ struct zl3073x_pin_props
> *zl3073x_pin_props_get(struct zl3073x_dev *zldev,
> >
> >   		props->dpll_props.type = DPLL_PIN_TYPE_GNSS;
> >
> > +	       /*
> > +		* The SyncE_Ref pin supports enabling/disabling dynamically.
> > +		* Some platforms may choose to expose this through
> firmware
> > +		* configuration later. For now, advertise this capability
> > +		* universally since the hardware allows state toggling.
> > +		*/
> > +		props->dpll_props.capabilities |=
> > +			DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;
> > +
> >   		/* The output pin phase adjustment granularity equals half of
> >   		 * the synth frequency count.
> >   		 */
> 
> I'm wondering about the purpose of this flag, or rather the need to set
> it manually from the driver. Surely, the existence of the
> state_on_dpll_set() or state_on_pin_set() callback clearly indicates
> this capability.
> 
> Wouldn't it be simpler for the core to set it directly based on this?
> 
> Ivan

Hi Ivan, thanks for your review!
Yeah, I don't like the way I implemented it, but it was the simplest way.
It was more about to introduce what's needed to achieve this patchset goal.

According to your suggestion, I gave it a try with such a simple change 
on pin registration (of course we need that also for pin_on_pin):
@@ -896,6 +896,9 @@ dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
        }

        ret = __dpll_pin_register(dpll, pin, ops, priv, NULL);
+
+       if (!ret && ops->state_on_dpll_set)
+               pin->prop.capabilities |= DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;

and it of course works.

My concern is that it's a kind of implicit property/capability enablement.
Will wait on more feedback on this.

Grzegorz

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.