Re: [PATCH net-next] dpll: use pin owner's dpll ref for pin-level attribute setting

Ivan Vecera <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Reply to Sashiko findings:

 > dpll_pin_is_freq_supported() reads a u64:
 >     u64 freq = nla_get_u64(a), old_freq;
 > while the validator takes a u32:
 >     static bool dpll_pin_is_freq_supported(struct dpll_pin *pin, u32 
freq)
 > ...
 > Would widening the helper parameter to u64 be worth folding in here?

Good catch. This is a pre-existing issue unrelated to the owner-ref
simplification. I'd prefer to fix it in a separate follow-up patch
rather than mixing it in.

 > Should Documentation/driver-api/dpll.rst be updated in the same patch?
 > ...
 > Could both changes be mentioned in the commit message, which currently
 > describes the patch only as removing "duplicate HW writes"?

Wull address in v2. Both dpll.rst sections ("Shared pins" and "SET
commands format") are updated. The commit message will describe the
-EOPNOTSUPP validation change (from scanning all owner-matching refs to
checking the single owner ref returned by dpll_pin_own_dpll_ref_first()).

 > Is the ref_sync_set callback really DPLL-independent for zl3073x?
 > ...
 > does this leave the sync reference still armed as a selectable
 > candidate in the priority tables of every other channel?
 > ...
 > If ref_sync_set does need per-registration invocation, should it keep
 > the xa_for_each() fan-out while the other three setters move to the
 > owner ref?

This is correct finding. zl3073x_dpll_input_pin_ref_sync_set() performs
a per-channel priority write that would only reach the first channel
after this simplification.

Rather than keeping xa_for_each() in the core for this one callback,
v2 will split the fix into a preparation patch (1/2) where the driver
itself iterates all DPLL channels via list_for_each_entry() and sets
priority to NONE on each one.

Ivan
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.