> -----Original Message-----
> From: Ivan Vecera <[email protected]>
> Sent: Tuesday, March 24, 2026 3:44 PM
> To: Nitka, Grzegorz <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; Oros, Petr
> <[email protected]>; [email protected];
> [email protected]; Kitszel, Przemyslaw
> <[email protected]>; Nguyen, Anthony L
> <[email protected]>; [email protected];
> [email protected]; Kubalewski, Arkadiusz <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Loktionov, Aleksandr
> <[email protected]>
> 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 <[email protected]>
> > Reviewed-by: Aleksandr Loktionov <[email protected]>
> > Signed-off-by: Grzegorz Nitka <[email protected]>
> > ---
> > 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
