intel-wired-lan.lists.osuosl.org archive mirror
 help / color / mirror / Atom feed
From: "Kwapulinski, Piotr" <piotr.kwapulinski@intel.com>
To: Simon Horman <horms@kernel.org>
Cc: "Sokolowski, Jan" <jan.sokolowski@intel.com>,
	"Gomes, Vinicius" <vinicius.gomes@intel.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"Jagielski, Jedrzej" <jedrzej.jagielski@intel.com>,
	"Wegrzyn, Stefan" <stefan.wegrzyn@intel.com>,
	"intel-wired-lan@lists.osuosl.org"
	<intel-wired-lan@lists.osuosl.org>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next v2 2/5] ixgbe: Add support for E610 device capabilities detection
Date: Wed, 24 Apr 2024 08:49:23 +0000	[thread overview]
Message-ID: <DM6PR11MB46100E8A3535D361C9DBE462F3102@DM6PR11MB4610.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20240422183206.GE42092@kernel.org>

 >-----Original Message-----
>From: Simon Horman <horms@kernel.org> 
>Sent: Monday, April 22, 2024 8:32 PM
>To: Kwapulinski, Piotr <piotr.kwapulinski@intel.com>
>Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Gomes, Vinicius <vinicius.gomes@intel.com>; Wegrzyn, Stefan <stefan.wegrzyn@intel.com>; Jagielski, Jedrzej <jedrzej.jagielski@intel.com>; Sokolowski, Jan <jan.sokolowski@intel.com>
>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 <horms@kernel.org>
>> >Sent: Saturday, April 20, 2024 8:18 PM
>> >To: Kwapulinski, Piotr <piotr.kwapulinski@intel.com>
>> >Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Gomes, 
>> >Vinicius <vinicius.gomes@intel.com>; Wegrzyn, Stefan 
>> ><stefan.wegrzyn@intel.com>; Jagielski, Jedrzej 
>> ><jedrzej.jagielski@intel.com>; Sokolowski, Jan 
>> ><jan.sokolowski@intel.com>
>> >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 <stefan.wegrzyn@intel.com>
>> >> Signed-off-by: Stefan Wegrzyn <stefan.wegrzyn@intel.com>
>> >> Co-developed-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
>> >> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
>> >> Reviewed-by: Jan Sokolowski <jan.sokolowski@intel.com>
>> >> Signed-off-by: Piotr Kwapulinski <piotr.kwapulinski@intel.com>
>> >
>> >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.
Piotr

>
>> 
>> >
>> >> +
>> >> +	return ixgbe_aci_send_cmd(hw, &desc, NULL, 0); }
>> >
>> >...
>> Thank you for review
>> Piotr
>>

  parent reply	other threads:[~2024-04-24  8:49 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-15 10:34 [Intel-wired-lan] [PATCH iwl-next v2 0/5] ixgbe: Add support for Intel(R) E610 device Piotr Kwapulinski
2024-04-15 10:34 ` [Intel-wired-lan] [PATCH iwl-next v2 1/5] ixgbe: Add support for E610 FW Admin Command Interface Piotr Kwapulinski
2024-04-18  0:07   ` Tony Nguyen
2024-04-15 10:34 ` [Intel-wired-lan] [PATCH iwl-next v2 2/5] ixgbe: Add support for E610 device capabilities detection Piotr Kwapulinski
2024-04-20 18:18   ` Simon Horman
2024-04-22 10:41     ` Kwapulinski, Piotr
2024-04-22 18:32       ` Simon Horman
2024-04-24  8:13         ` Kwapulinski, Piotr
2024-04-24  8:49         ` Kwapulinski, Piotr [this message]
2024-04-15 10:34 ` [Intel-wired-lan] [PATCH iwl-next v2 3/5] ixgbe: Add link management support for E610 device Piotr Kwapulinski
2024-04-15 10:34 ` [Intel-wired-lan] [PATCH iwl-next v2 4/5] ixgbe: Add support for NVM handling in " Piotr Kwapulinski
2024-04-15 10:34 ` [Intel-wired-lan] [PATCH iwl-next v2 5/5] ixgbe: Enable link management " Piotr Kwapulinski
2024-04-18  0:07   ` Tony Nguyen
2024-04-16  5:17 ` [Intel-wired-lan] [PATCH iwl-next v2 0/5] ixgbe: Add support for Intel(R) " Kwapulinski, Piotr

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR11MB46100E8A3535D361C9DBE462F3102@DM6PR11MB4610.namprd11.prod.outlook.com \
    --to=piotr.kwapulinski@intel.com \
    --cc=horms@kernel.org \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jan.sokolowski@intel.com \
    --cc=jedrzej.jagielski@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=stefan.wegrzyn@intel.com \
    --cc=vinicius.gomes@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).