Re: [PATCH net-next v8 07/15] dpll: sit9531x: add support to get and set priority on input pins

[Ali Rouhi]

Replies inline.

> This isn't a bug, but should this say 11-slot?

Fixed in v9 -- the kernel-doc now says 11 slots, matching regs.h and
the block comment above sit9531x_prio_reg().

> Could this loop read each register only once?

Reworked further in v9: the getter no longer touches the bus at all.
It answers from a priority cache that is seeded from the table read at
startup and refreshed after every table write and every poll read-back, so
pin-get issues no synchronous I2C traffic.

> Can a nibble that was never programmed be mistaken for IN0P here?

The 4-bit encoding has no reserved "unused" value, so a 0 nibble is
IN0P by the hardware's own definition -- the device's automatic selection
treats it as IN0P too, so reporting it as such matches what the chip will
actually do.  The configurations we support always arrive with the priority
block fully programmed by the efuse/NVM blob; the driver takes the table as
authoritative because there is nothing else to qualify it against.

> Is priority 11 a value userspace can be handed?

No longer -- v9 reports the lowest valid priority slot for an input
absent from the table, an in-range value the setter accepts, so a
dump/modify/restore round trip works.  Whether the input is a reference of
the DPLL at all is the state attribute's job (it reports DISCONNECTED), not
the priority value's.

> Is anything reading dpin->prio? [...] a configured priority does not survive
> a disconnect and reconnect.

The reconnect-at-lowest-priority behaviour is intentional and
documented in the kernel-doc: a disconnect is an explicit statement that the
input no longer participates, and re-adding it quietly at its old slot would
reorder other pins' priorities without a request naming them.  Userspace that
wants a specific slot sets it after reconnecting, which the setter supports.

> Can this branch ever be taken? [dir != INPUT guard in prio_set]

It cannot today; it is a cheap defensive guard against a future ops
rewiring, and the extack text documents the contract.  We kept it
deliberately.

> Can a demotion request through this path end up a no-op that still reports
> success? [duplicated-tail table]

The holdover excursion was unnecessary: v9 compares the rewritten table
against the current one and skips
the commit entirely when nothing changes.  The reported priority is the first
occurrence, which is the slot the hardware selects by, so get and set agree
on the effective value.

> Should the other affected pins get a change notification here?

Yes -- v9 walks the DPLL's input pins after a
successful priority set and emits dpll_pin_change_ntf() for every pin whose
slot shifted.