>-----Original Message----- >From: Simon Horman <[email protected]> >Sent: Monday, April 22, 2024 8:32 PM >To: Kwapulinski, Piotr <[email protected]> >Cc: [email protected]; [email protected]; Gomes, Vinicius ><[email protected]>; Wegrzyn, Stefan <[email protected]>; >Jagielski, Jedrzej <[email protected]>; Sokolowski, Jan ><[email protected]> >Subject: Re: [PATCH iwl-next v2 2/5] ixgbe: Add support for E610 device >capabilities detection > >On Mon, Apr 22, 2024 at 10:41:47AM +0000, Kwapulinski, Piotr wrote: >> >-----Original Message----- >> >From: Simon Horman <[email protected]> >> >Sent: Saturday, April 20, 2024 8:18 PM >> >To: Kwapulinski, Piotr <[email protected]> >> >Cc: [email protected]; [email protected]; Gomes, >> >Vinicius <[email protected]>; Wegrzyn, Stefan >> ><[email protected]>; Jagielski, Jedrzej >> ><[email protected]>; Sokolowski, Jan >> ><[email protected]> >> >Subject: Re: [PATCH iwl-next v2 2/5] ixgbe: Add support for E610 >> >device capabilities detection >> > >> >On Mon, Apr 15, 2024 at 12:34:32PM +0200, Piotr Kwapulinski wrote: >> >> Add low level support for E610 device capabilities detection. The >> >> capabilities are discovered via the Admin Command Interface. >> >> Discover the following capabilities: >> >> - function caps: vmdq, dcb, rss, rx/tx qs, msix, nvm, orom, reset >> >> - device caps: vsi, fdir, 1588 >> >> - phy caps >> >> >> >> Co-developed-by: Stefan Wegrzyn <[email protected]> >> >> Signed-off-by: Stefan Wegrzyn <[email protected]> >> >> Co-developed-by: Jedrzej Jagielski <[email protected]> >> >> Signed-off-by: Jedrzej Jagielski <[email protected]> >> >> Reviewed-by: Jan Sokolowski <[email protected]> >> >> Signed-off-by: Piotr Kwapulinski <[email protected]> >> > >> >Hi Pitor, >> > >> >A few minor nits from my side. >> >No need to respin just because of these. >> > >> >> --- >> >> drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c | 517 >> >> ++++++++++++++++++ drivers/net/ethernet/intel/ixgbe/ixgbe_e610.h | >> >> 11 + >> >> 2 files changed, 528 insertions(+) >> >> >> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c >> >> b/drivers/net/ethernet/intel/ixgbe/ixgbe_e610.c >> > >> >... >> > >> >> +/** >> >> + * ixgbe_get_num_per_func - determine number of resources per PF >> >> + * @hw: pointer to the HW structure >> >> + * @max: value to be evenly split between each PF >> >> + * >> >> + * Determine the number of valid functions by going through the >> >> +bitmap returned >> >> + * from parsing capabilities and use this to calculate the number >> >> +of resources >> >> + * per PF based on the max value passed in. >> >> + * >> >> + * Return: the number of resources per PF or 0, if no PH are available. >> >> + */ >> >> +static u32 ixgbe_get_num_per_func(struct ixgbe_hw *hw, u32 max) { >> >> + const u32 IXGBE_CAPS_VALID_FUNCS_M = 0xFF; >> > >> >nit: Maybe this could simply be a #define? >> Hello, >> will do >> >> > >> >> + u8 funcs = hweight8(hw->dev_caps.common_cap.valid_functions & >> >> + IXGBE_CAPS_VALID_FUNCS_M); >> > >> >nit: Please consider using reverse xmas tree order - longest line to >> >shortest - >> > for local variables in new Networking code >> Will do >> >> > >> >> + >> >> + return funcs ? (max / funcs) : 0; } >> > >> >... >> > >> >> +/** >> >> + * ixgbe_aci_disable_rxen - disable RX >> >> + * @hw: pointer to the HW struct >> >> + * >> >> + * Request a safe disable of Receive Enable using ACI command (0x000C). >> >> + * >> >> + * Return: the exit code of the operation. >> >> + */ >> >> +int ixgbe_aci_disable_rxen(struct ixgbe_hw *hw) { >> >> + struct ixgbe_aci_cmd_disable_rxen *cmd; >> >> + struct ixgbe_aci_desc desc; >> >> + >> >> + cmd = &desc.params.disable_rxen; >> >> + >> >> + ixgbe_fill_dflt_direct_cmd_desc(&desc, >> >> +ixgbe_aci_opc_disable_rxen); >> >> + >> >> + cmd->lport_num = (u8)hw->bus.func; >> > >> >nit: This cast seems unnecessary. >> > AFAICT the type of hw->bus.func is u8. >> Will do > >Thanks. FWIIW, I think I noticed a similar cast at least once more elsewhere >in the patchset Will fix. Thank you for review.
> >> >> > >> >> + >> >> + return ixgbe_aci_send_cmd(hw, &desc, NULL, 0); } >> > >> >... >> Thank you for review >> Piotr
