Re: [PATCH net-next 09/10] ice: dpll: update locking comments

[Ivan Vecera <ivecera@redhat.com>]

On 11/10/26 18:53, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found more severe
> issue(s) in other patches of this series. This mail is only an FYI, in case
> you want to address the nit picks below when you respin.
> 
> Low priority / nit picks for your consideration:
> 
> - [Low] The locking note this patch adds to the struct ice_dplls kernel-
>    doc (drivers/net/ethernet/intel/ice/ice_dpll.h, lines 168-170) is…
> 
> --
> 
> ice: dpll: update locking comments
> 
> This commit updates the ice DPLL locking comments now that callbacks run
> under the DPLL core domain locks instead of dpll_lock. It also adds a note
> saying that callbacks of the external DPLL's pins run under that DPLL's
> domain, so only pf->dplls.lock serializes them with the rest of the
> driver.
> 
>> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h b/drivers/net/ethernet/intel/ice/ice_dpll.h
>> index 4ba76e2e2e0b0..39e041a30090d 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_dpll.h
>> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
>> @@ -165,6 +165,9 @@ struct ice_dpll {
>>    *     tx_refclks usage map), acquire pf->dplls.lock first, then
>>    *     ctrl_pf->dplls.lock. Skip the second acquire when pf == ctrl_pf
>>    *     to avoid recursive locking.
>> + *   - Callbacks of the SYNCE and TIME_REF pins of an external DPLL run under
>> + *     the DPLL core domain lock of that DPLL, not of the TXC and TSPLL DPLLs,
>> + *     so only @lock serializes them with the callbacks of these DPLLs.
> 
> [Severity: Low]
> Could this note be clearer that TXC and TSPLL are not in the same core
> domain either?

[...]
> The new sentence is accurate per DPLL, and the first bullet still says
> @lock serializes all DPLL state mutations. Since this patch exists to
> document the domain-lock rules, though, would it make sense to say that
> TXC, TSPLL and the DSN-domain objects are each in their own domain?
Yes. v2 will say that the core serializes callbacks per (module, clock_id),
that the TXC DPLL, the TSPLL DPLL and the other objects of a PF have a
clock_id each, and that the SYNCE and TIME_REF pins of an external DPLL
are one more such case. @lock serializes all of them.

Thanks,
Ivan