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: <IA3PR11MB898664E18FD62E9C3BA2FB26E51DA@IA3PR11MB8986.namprd11.prod.outlook.com>
Date: Tue, 23 Sep 2025 19:21:51 +0000
From: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>
To: "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



> -----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.

> +static enum ice_sbq_dev_id ice_get_dest_cgu(struct ice_hw *hw)
> +{
> +	/* On dual complex E825 only complex 0 has functional CGU
> powering all
> +	 * the PHYs.
> +	 * SBQ destination device cgu points to CGU on a current
> complex and to
> +	 * access primary CGU from the secondary complex, the driver
> should use
> +	 * cgu_peer as a destination device.
> +	 */
> +	if (hw->mac_type == ICE_MAC_GENERIC_3K_E825 && ice_is_dual(hw)
> &&
> +	    !ice_is_primary(hw))
> +		return ice_sbq_dev_cgu_peer;
> +	else
> +		return ice_sbq_dev_cgu;
> +}
Kernel style prefers dropping else when the if branch returns—makes the code shorter and idiomatic.


> +
>  /**
>   * ice_read_cgu_reg - Read a CGU register
>   * @hw: Pointer to the HW struct
> @@ -6396,8 +6422,8 @@ u32 ice_get_link_speed(u16 index)
>  int ice_read_cgu_reg(struct ice_hw *hw, u32 addr, u32 *val)
>  {
>  	struct ice_sbq_msg_input cgu_msg = {
> +		.dest_dev = ice_get_dest_cgu(hw),
>  		.opcode = ice_sbq_msg_rd,
> -		.dest_dev = ice_sbq_dev_cgu,
>  		.msg_addr_low = addr
>  	};
>  	int err;
> @@ -6428,8 +6454,8 @@ int ice_read_cgu_reg(struct ice_hw *hw, u32
> addr, u32 *val)
>  int ice_write_cgu_reg(struct ice_hw *hw, u32 addr, u32 val)
>  {
>  	struct ice_sbq_msg_input cgu_msg = {
> +		.dest_dev = ice_get_dest_cgu(hw),
>  		.opcode = ice_sbq_msg_wr,
> -		.dest_dev = ice_sbq_dev_cgu,
>  		.msg_addr_low = addr,
>  		.data = val
>  	};
> diff --git a/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
> b/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
> index 183dd5457d6a..21bb861febbf 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
> +++ b/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
> @@ -50,6 +50,7 @@ enum ice_sbq_dev_id {
>  	ice_sbq_dev_phy_0	= 0x02,
>  	ice_sbq_dev_cgu		= 0x06,
>  	ice_sbq_dev_phy_0_peer	= 0x0D,
> +	ice_sbq_dev_cgu_peer	= 0x0F,
>  };
> 
>  enum ice_sbq_msg_opcode {
> 
> base-commit: 84cb3483445f9ac0a106eb846fa100393433d469
> --
> 2.39.3

Some style warnings, otherwise fine.
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>