Re: [PATCH net-next v8 14/15] dpll: sit9531x: model the inter-PLL sync net as a pair of pins

[Ali Rouhi]

Replies inline.

> Can the bit be stranded when the rollback itself fails? [and: the rollback
> writes dis_val to entries the aborted enable never touched]

v9 logs a rollback failure explicitly (original
error and rollback error both), so a stranded enable is visible rather than
silent, and intsync_src is additionally reconciled with the hardware at probe
(see below), so a stranded bit cannot survive a rebind unnoticed.  On the
second point: the disable values are the documented quiescent state of the
INTSYNC block, not a restore of arbitrary prior content -- the enable path
owns all eight registers, so driving them to the quiescent state after a
failed enable is the intended containment, and it matches what a fresh
disable request would program.

> What state is the driver in if one of the later ones fails? [partial
> disable keeps cached owner]

v9 clears the cached owner as soon
as the authoritative global-enable clear has committed, so a partial disable
leaves the cache saying "no owner" (matching the bit that controls the net)
and a retry converges instead of short-circuiting.

> Is intsync_src ever reconciled with the hardware?

Yes -- fixed in v9: probe now detects the INTSYNC state from the
global enable bit and the per-PLL EXT configuration and seeds intsync_src
from it, so a rebind (or a blob that boots with INTSYNC active) starts from
the hardware truth instead of -1.

> Should the policy errno be excluded here as the two peer setters in this
> file do?

Yes -- fixed in v9: the filter excludes -EINVAL and -EOPNOTSUPP like
the peer setters, so a policy rejection keeps its specific extack text.

> Should the hardware's current selection be reported through
> DPLL_A_PIN_OPERSTATE rather than DPLL_A_PIN_STATE here?

The getter follows the pin-state contract stated once above the ops
tables and applied uniformly to all selection-role pins in this driver:
CONNECTED reports the reference the DPLL is actually locked to, which is the
same convention ice uses for its input pins.  We agree the operstate split is
the cleaner model going forward and are happy to add
.operstate_on_dpll_get in a follow-up series -- for this series we kept one
consistent convention across all input pins rather than switching the model
for one pin pair.

> reusing the generic priority getter for the new pin also inherits its
> out-of-contract return value

Resolved by the v9 priority-getter rework (see the patch 07 reply):
the sentinel is gone; an untabled source reports the lowest valid slot.

> is that model actually introduced here? [pins already registered earlier;
> bisect window with OOB indexing]

Both points are addressed in v9: the INTSYNC pair is
registered in this patch, together with the dedicated ops tables, so the
"model" and its pins arrive as one unit and no earlier commit can dispatch
them to the generic tables; and the changelog is reworded accordingly.