netdev - Re: [Intel-wired-lan] [PATCH iwl-net] ice: fix destination CGU for dual complex E825

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e4dba218-be8c-48ed-a4a2-cd966a553dbb@intel.com>
Date: Mon, 29 Sep 2025 15:12:58 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>, "Nitka, Grzegorz"
	<grzegorz.nitka@...el.com>, "intel-wired-lan@...ts.osuosl.org"
	<intel-wired-lan@...ts.osuosl.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, Karol Kolacinski
	<karol.kolacinski@...el.com>, "Kubalewski, Arkadiusz"
	<arkadiusz.kubalewski@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-net] ice: fix destination CGU for
 dual complex E825



On 9/23/2025 12:21 PM, Loktionov, Aleksandr wrote:
> 
> 
>> -----Original Message-----
>> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf
>> Of Grzegorz Nitka
>> Sent: Tuesday, September 23, 2025 5:29 PM
>> To: intel-wired-lan@...ts.osuosl.org
>> Cc: netdev@...r.kernel.org; Karol Kolacinski
>> <karol.kolacinski@...el.com>; Kubalewski, Arkadiusz
>> <arkadiusz.kubalewski@...el.com>
>> Subject: [Intel-wired-lan] [PATCH iwl-net] ice: fix destination CGU
>> for dual complex E825
>>
>> On dual complex E825, only complex 0 has functional CGU (Clock
>> Generation Unit), powering all the PHYs.
>> SBQ (Side Band Queue) destination device 'cgu' in current
>> implementation
>> points to CGU on current complex and, in order to access primary CGU
>> from the secondary complex, the driver should use 'cgu_peer' as
>> a destination device in read/write CGU registers operations.
>>
>> Define new 'cgu_peer' (15) as RDA (Remote Device Access) client over
>> SB-IOSF interface and use it as device target when accessing CGU from
>> secondary complex.
>>
>> This problem has been identified when working on recovery clock
>> enablement [1]. In existing implementation for E825 devices, only PF0,
>> which is clock owner, is involved in CGU configuration, thus the
>> problem was not exposed to the user.
>>
>> [1] https://patchwork.ozlabs.org/project/intel-wired-
>> lan/patch/20250905150947.871566-1-grzegorz.nitka@...el.com/
>>
>> Fixes: e2193f9f9ec9 ("ice: enable timesync operation on 2xNAC E825
>> devices")
>> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@...el.com>
>> Signed-off-by: Karol Kolacinski <karol.kolacinski@...el.com>
>> Reviewed-by: Arkadiusz Kubalewski <Arkadiusz.kubalewski@...el.com>
>> ---
>>  drivers/net/ethernet/intel/ice/ice_common.c  | 30 ++++++++++++++++++-
>> -
>>  drivers/net/ethernet/intel/ice/ice_sbq_cmd.h |  1 +
>>  2 files changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
>> b/drivers/net/ethernet/intel/ice/ice_common.c
>> index eb6abf452b05..5ea420c76f54 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_common.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
>> @@ -6382,6 +6382,32 @@ u32 ice_get_link_speed(u16 index)
>>  	return ice_aq_to_link_speed[index];
>>  }
>>
>> +/**
>> + * ice_get_dest_cgu - get destination CGU dev for given HW
>> + * @hw: pointer to the HW struct
>> + *
>> + * Get CGU client id for CGU register read/write operations.
>> + *
>> + * Return:
>> + * * ice_sbq_dev_cgu - default value
>> + * * ice_sbq_dev_cgu_peer - when accessing CGU from 2nd complex (E825
>> only)
>> + *
>> + */
> NIT: In kernel‑doc for functions, Return: is expected to be prose (not bullet items).
> Also, prefer “second” to “2nd”, and describe what is returned rather than enumerating constants.
> 
Actually kdoc does support lists:

https://origin.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#return-values

Specifically:

> The multi-line descriptive text you provide does not recognize line breaks, so if you try to format some text nicely, as in:
> 
> * Return:
> * %0 - OK
> * %-EINVAL - invalid argument
> * %-ENOMEM - out of memory
> this will all run together and produce:
> 
> Return: 0 - OK -EINVAL - invalid argument -ENOMEM - out of memory
> So, in order to produce the desired line breaks, you need to use a ReST list, e. g.:
> 
> * Return:
> * * %0            - OK to runtime suspend the device
> * * %-EBUSY       - Device should not be runtime suspended


This format is acceptable, but I think its only more typical of the case
when returning integer error codes. I'm fine with changing this if
others disagree with the format, just thought I'd point out that it is
one of the formats documented..


Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)