| Message ID | 20241003001433.11211-5-przemyslaw.kitszel@intel.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Refactor sending DDP + E830 support | expand |
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Przemek Kitszel > Sent: Thursday, October 3, 2024 5:41 AM > To: intel-wired-lan@lists.osuosl.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com> > Cc: Nowlin, Dan <dan.nowlin@intel.com>; Greenwalt, Paul <paul.greenwalt@intel.com>; netdev@vger.kernel.org; Zaki, Ahmed <ahmed.zaki@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > Subject: [Intel-wired-lan] [PATCH iwl-next v2 1/2] ice: refactor "last" segment of DDP pkg > > Add ice_ddp_send_hunk() that buffers "sent FW hunk" calls to AQ in order to mark the "last" one in more elegant way. Next commit will add even more complicated "sent FW" flow, so it's better to untangle a bit before. > > Note that metadata buffers were not skipped for NOT-@indicate_last segments, this is fixed now. > > Minor: > + use ice_is_buffer_metadata() instead of open coding it in > ice_dwnld_cfg_bufs(); > + ice_dwnld_cfg_bufs_no_lock() + dependencies were moved up a bit to have > better git-diff, as this function was rewritten (in terms of git-blame) > > CC: Paul Greenwalt <paul.greenwalt@intel.com> > CC: Dan Nowlin <dan.nowlin@intel.com> > CC: Ahmed Zaki <ahmed.zaki@intel.com> > Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > --- > git: --inter-hunk-context=6 > > v2: fixed one kdoc warning > --- > drivers/net/ethernet/intel/ice/ice_ddp.c | 280 ++++++++++++----------- > 1 file changed, 145 insertions(+), 135 deletions(-) > Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
On Thu, Oct 03, 2024 at 02:10:31AM +0200, Przemek Kitszel wrote: > Add ice_ddp_send_hunk() that buffers "sent FW hunk" calls to AQ in order > to mark the "last" one in more elegant way. Next commit will add even > more complicated "sent FW" flow, so it's better to untangle a bit before. > > Note that metadata buffers were not skipped for NOT-@indicate_last > segments, this is fixed now. > > Minor: > + use ice_is_buffer_metadata() instead of open coding it in > ice_dwnld_cfg_bufs(); > + ice_dwnld_cfg_bufs_no_lock() + dependencies were moved up a bit to have > better git-diff, as this function was rewritten (in terms of git-blame) > > CC: Paul Greenwalt <paul.greenwalt@intel.com> > CC: Dan Nowlin <dan.nowlin@intel.com> > CC: Ahmed Zaki <ahmed.zaki@intel.com> > Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> Hi Przemek, Some minor feedback from my side. > --- > git: --inter-hunk-context=6 > > v2: fixed one kdoc warning > --- > drivers/net/ethernet/intel/ice/ice_ddp.c | 280 ++++++++++++----------- > 1 file changed, 145 insertions(+), 135 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c > index 016fcab6ba34..a2bb8442f281 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ddp.c > +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c > @@ -1210,6 +1210,127 @@ ice_aq_download_pkg(struct ice_hw *hw, struct ice_buf_hdr *pkg_buf, > return status; > } > > +/** > + * ice_is_buffer_metadata - determine if package buffer is a metadata buffer > + * @buf: pointer to buffer header > + * Return: whether given @buf is a metadata one. > + */ > +static bool ice_is_buffer_metadata(struct ice_buf_hdr *buf) > +{ > + return le32_to_cpu(buf->section_entry[0].type) & ICE_METADATA_BUF; I see this is moving existing logic around. And I see that this is a no-op on LE systems. But it might be nicer to perform the byte-order conversion on the constant. > +} > + > +/** > + * struct ice_ddp_send_ctx - sending context of current DDP segment > + * @hw: pointer to the hardware struct > + * > + * Keeps current sending state (header, error) for the purpose of proper "last" > + * bit settting in ice_aq_download_pkg(). Use via calls to ice_ddp_send_hunk(). setting > + */ > +struct ice_ddp_send_ctx { > + struct ice_hw *hw; > +/* private: only for ice_ddp_send_hunk() */ > + struct ice_buf_hdr *hdr; > + int err; > +}; > + > +/** > + * ice_ddp_send_hunk - send one hunk of data to FW > + * @ctx - current segment sending context > + * @hunk - next hunk to send, size is always ICE_PKG_BUF_SIZE Tooling seems to expect the following syntax. * @ctx: ... * @hunk: ... > + * > + * Send the next hunk of data to FW, retrying if needed. > + * > + * Notice: must be called once more with a NULL @hunk to finish up; such call > + * will set up the "last" bit of an AQ request. After such call @ctx.hdr is > + * cleared, @hw is still valid. > + * > + * Return: %ICE_DDP_PKG_SUCCESS if there were no problems; a sticky @err > + * otherwise. > + */ > +static enum ice_ddp_state ice_ddp_send_hunk(struct ice_ddp_send_ctx *ctx, > + struct ice_buf_hdr *hunk) ... > +/** > + * ice_dwnld_cfg_bufs_no_lock > + * @ctx: context of the current buffers section to send > + * @bufs: pointer to an array of buffers > + * @start: buffer index of first buffer to download > + * @count: the number of buffers to download > + * > + * Downloads package configuration buffers to the firmware. Metadata buffers > + * are skipped, and the first metadata buffer found indicates that the rest > + * of the buffers are all metadata buffers. > + */ > +static enum ice_ddp_state > +ice_dwnld_cfg_bufs_no_lock(struct ice_ddp_send_ctx *ctx, struct ice_buf *bufs, > + u32 start, u32 count) > +{ > + struct ice_buf_hdr *bh; > + enum ice_ddp_state err; > + > + if (!bufs || !count) { > + ctx->err = ICE_DDP_PKG_ERR; > + return ctx->err; > + } > + > + bufs += start; > + bh = (struct ice_buf_hdr *)bufs; Again I see that, to some extent, this is moving existing logic around. But as bh is set in each loop iteration does it also need to be set here? > + > + for (int i = 0; i < count; i++, bufs++) { > + bh = (struct ice_buf_hdr *)bufs; > + /* Metadata buffers should not be sent to FW, > + * their presence means "we are done here". > + */ > + if (ice_is_buffer_metadata(bh)) > + break; > + > + err = ice_ddp_send_hunk(ctx, bh); > + if (err) > + return err; > + } > + > + return 0; > +} > + > /** > * ice_get_pkg_seg_by_idx > * @pkg_hdr: pointer to the package header to be searched ... > @@ -1454,17 +1459,16 @@ ice_dwnld_sign_and_cfg_segs(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr, > } > > count = le32_to_cpu(seg->signed_buf_count); > - state = ice_download_pkg_sig_seg(hw, seg); > + state = ice_download_pkg_sig_seg(ctx, seg); > if (state || !count) > goto exit; > > conf_idx = le32_to_cpu(seg->signed_seg_idx); > start = le32_to_cpu(seg->signed_buf_start); > > - state = ice_download_pkg_config_seg(hw, pkg_hdr, conf_idx, start, > - count); > - > + return ice_download_pkg_config_seg(ctx, pkg_hdr, conf_idx, start, count); This changes the conditions under which this function sets ctx->err, which is then changed again by the following patch. Is that intentional? > exit: > + ctx->err = state; > return state; > } ...
On 10/17/24 12:06, Simon Horman wrote: > On Thu, Oct 03, 2024 at 02:10:31AM +0200, Przemek Kitszel wrote: >> Add ice_ddp_send_hunk() that buffers "sent FW hunk" calls to AQ in order >> to mark the "last" one in more elegant way. Next commit will add even >> more complicated "sent FW" flow, so it's better to untangle a bit before. >> >> Note that metadata buffers were not skipped for NOT-@indicate_last >> segments, this is fixed now. >> >> Minor: >> + use ice_is_buffer_metadata() instead of open coding it in >> ice_dwnld_cfg_bufs(); >> + ice_dwnld_cfg_bufs_no_lock() + dependencies were moved up a bit to have >> better git-diff, as this function was rewritten (in terms of git-blame) >> >> CC: Paul Greenwalt <paul.greenwalt@intel.com> >> CC: Dan Nowlin <dan.nowlin@intel.com> >> CC: Ahmed Zaki <ahmed.zaki@intel.com> >> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> >> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > > Hi Przemek, > > Some minor feedback from my side. Thank you for reaching out! >> +static bool ice_is_buffer_metadata(struct ice_buf_hdr *buf) >> +{ >> + return le32_to_cpu(buf->section_entry[0].type) & ICE_METADATA_BUF; > > I see this is moving existing logic around. > And I see that this is a no-op on LE systems. > But it might be nicer to perform the byte-order conversion on the constant. As far as I remember, for this driver we always do have binary-arith constants (flags, masks, etc) in CPU-order, so do as I did. I could imagine keeping all such constants in HW-order, and such approach could potentially set the boundary for byte-order conversions to be better expressed/illustrated. For new drivers, I will still think more about unit-test-abilty instead, and those will be easiest with as much constants expressed in CPU-order. No strong opinion here anyway, and I think we agree that it's most important to be consistent within the driver/component. I manually sampled that for ice, but I don't have a proof. >> +/** >> + * struct ice_ddp_send_ctx - sending context of current DDP segment >> + * @hw: pointer to the hardware struct >> + * >> + * Keeps current sending state (header, error) for the purpose of proper "last" >> + * bit settting in ice_aq_download_pkg(). Use via calls to ice_ddp_send_hunk(). > > setting > >> + */ >> +struct ice_ddp_send_ctx { >> + struct ice_hw *hw; >> +/* private: only for ice_ddp_send_hunk() */ >> + struct ice_buf_hdr *hdr; >> + int err; >> +}; >> + >> +/** >> + * ice_ddp_send_hunk - send one hunk of data to FW >> + * @ctx - current segment sending context >> + * @hunk - next hunk to send, size is always ICE_PKG_BUF_SIZE > > Tooling seems to expect the following syntax. > > * @ctx: ... > * @hunk: ... oops, sorry! >> +static enum ice_ddp_state >> +ice_dwnld_cfg_bufs_no_lock(struct ice_ddp_send_ctx *ctx, struct ice_buf *bufs, >> + u32 start, u32 count) >> +{ >> + struct ice_buf_hdr *bh; >> + enum ice_ddp_state err; >> + >> + if (!bufs || !count) { >> + ctx->err = ICE_DDP_PKG_ERR; >> + return ctx->err; >> + } >> + >> + bufs += start; >> + bh = (struct ice_buf_hdr *)bufs; > > Again I see that, to some extent, this is moving existing logic around. > But as bh is set in each loop iteration does it also need to be set here? good point ice_is_buffer_metadata() check was prior to the loop, now it is inside, I will remove the redundant assignment. >> @@ -1454,17 +1459,16 @@ ice_dwnld_sign_and_cfg_segs(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr, >> } >> >> count = le32_to_cpu(seg->signed_buf_count); >> - state = ice_download_pkg_sig_seg(hw, seg); >> + state = ice_download_pkg_sig_seg(ctx, seg); >> if (state || !count) >> goto exit; >> >> conf_idx = le32_to_cpu(seg->signed_seg_idx); >> start = le32_to_cpu(seg->signed_buf_start); >> >> - state = ice_download_pkg_config_seg(hw, pkg_hdr, conf_idx, start, >> - count); >> - >> + return ice_download_pkg_config_seg(ctx, pkg_hdr, conf_idx, start, count); > > This changes the conditions under which this function sets > ctx->err, which is then changed again by the following patch. > Is that intentional? > >> exit: >> + ctx->err = state; This line is unusual as it changes ctx->err from ctx user code. ctx itself updates @err only on new error, it uses "retained error" style of API (that I'm clearly a fan of ;)) Next commit replaces the last (successful) write (via ctx) of ddp, and error return from new path would result in "ctx->err = ctx->err" update. Not clear, not intentional, not harmful. I will update code to leave less space for confusion. >> return state; >> } > > ... >
On Fri, Oct 18, 2024 at 02:06:27PM +0200, Przemek Kitszel wrote: > On 10/17/24 12:06, Simon Horman wrote: > > On Thu, Oct 03, 2024 at 02:10:31AM +0200, Przemek Kitszel wrote: > > > Add ice_ddp_send_hunk() that buffers "sent FW hunk" calls to AQ in order > > > to mark the "last" one in more elegant way. Next commit will add even > > > more complicated "sent FW" flow, so it's better to untangle a bit before. > > > > > > Note that metadata buffers were not skipped for NOT-@indicate_last > > > segments, this is fixed now. > > > > > > Minor: > > > + use ice_is_buffer_metadata() instead of open coding it in > > > ice_dwnld_cfg_bufs(); > > > + ice_dwnld_cfg_bufs_no_lock() + dependencies were moved up a bit to have > > > better git-diff, as this function was rewritten (in terms of git-blame) > > > > > > CC: Paul Greenwalt <paul.greenwalt@intel.com> > > > CC: Dan Nowlin <dan.nowlin@intel.com> > > > CC: Ahmed Zaki <ahmed.zaki@intel.com> > > > Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > > > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > > > > Hi Przemek, > > > > Some minor feedback from my side. > > Thank you for reaching out! > > > > +static bool ice_is_buffer_metadata(struct ice_buf_hdr *buf) > > > +{ > > > + return le32_to_cpu(buf->section_entry[0].type) & ICE_METADATA_BUF; > > > > I see this is moving existing logic around. > > And I see that this is a no-op on LE systems. > > But it might be nicer to perform the byte-order conversion on the constant. > > As far as I remember, for this driver we always do have binary-arith > constants (flags, masks, etc) in CPU-order, so do as I did. > > I could imagine keeping all such constants in HW-order, and such > approach could potentially set the boundary for byte-order conversions > to be better expressed/illustrated. > > For new drivers, I will still think more about unit-test-abilty instead, > and those will be easiest with as much constants expressed in CPU-order. > > No strong opinion here anyway, and I think we agree that it's most > important to be consistent within the driver/component. I manually > sampled that for ice, but I don't have a proof. Yes, we agree. And I also have no strong opinion on this. So lets leave things as you have them. ... > > > @@ -1454,17 +1459,16 @@ ice_dwnld_sign_and_cfg_segs(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr, > > > } > > > count = le32_to_cpu(seg->signed_buf_count); > > > - state = ice_download_pkg_sig_seg(hw, seg); > > > + state = ice_download_pkg_sig_seg(ctx, seg); > > > if (state || !count) > > > goto exit; > > > conf_idx = le32_to_cpu(seg->signed_seg_idx); > > > start = le32_to_cpu(seg->signed_buf_start); > > > - state = ice_download_pkg_config_seg(hw, pkg_hdr, conf_idx, start, > > > - count); > > > - > > > + return ice_download_pkg_config_seg(ctx, pkg_hdr, conf_idx, start, count); > > > > This changes the conditions under which this function sets > > ctx->err, which is then changed again by the following patch. > > Is that intentional? > > > > > exit: > > > + ctx->err = state; > > This line is unusual as it changes ctx->err from ctx user code. > ctx itself updates @err only on new error, it uses "retained error" > style of API (that I'm clearly a fan of ;)) > > Next commit replaces the last (successful) write (via ctx) of ddp, > and error return from new path would result in > "ctx->err = ctx->err" update. Not clear, not intentional, not harmful. > I will update code to leave less space for confusion. Thanks for the clarification, much appreciated. ...
diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c index 016fcab6ba34..a2bb8442f281 100644 --- a/drivers/net/ethernet/intel/ice/ice_ddp.c +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c @@ -1210,6 +1210,127 @@ ice_aq_download_pkg(struct ice_hw *hw, struct ice_buf_hdr *pkg_buf, return status; } +/** + * ice_is_buffer_metadata - determine if package buffer is a metadata buffer + * @buf: pointer to buffer header + * Return: whether given @buf is a metadata one. + */ +static bool ice_is_buffer_metadata(struct ice_buf_hdr *buf) +{ + return le32_to_cpu(buf->section_entry[0].type) & ICE_METADATA_BUF; +} + +/** + * struct ice_ddp_send_ctx - sending context of current DDP segment + * @hw: pointer to the hardware struct + * + * Keeps current sending state (header, error) for the purpose of proper "last" + * bit settting in ice_aq_download_pkg(). Use via calls to ice_ddp_send_hunk(). + */ +struct ice_ddp_send_ctx { + struct ice_hw *hw; +/* private: only for ice_ddp_send_hunk() */ + struct ice_buf_hdr *hdr; + int err; +}; + +/** + * ice_ddp_send_hunk - send one hunk of data to FW + * @ctx - current segment sending context + * @hunk - next hunk to send, size is always ICE_PKG_BUF_SIZE + * + * Send the next hunk of data to FW, retrying if needed. + * + * Notice: must be called once more with a NULL @hunk to finish up; such call + * will set up the "last" bit of an AQ request. After such call @ctx.hdr is + * cleared, @hw is still valid. + * + * Return: %ICE_DDP_PKG_SUCCESS if there were no problems; a sticky @err + * otherwise. + */ +static enum ice_ddp_state ice_ddp_send_hunk(struct ice_ddp_send_ctx *ctx, + struct ice_buf_hdr *hunk) +{ + struct ice_buf_hdr *prev_hunk = ctx->hdr; + struct ice_hw *hw = ctx->hw; + bool prev_was_last = !hunk; + enum ice_aq_err aq_err; + u32 offset, info; + int attempt, err; + + if (ctx->err) + return ctx->err; + + ctx->hdr = hunk; + if (!prev_hunk) + return ICE_DDP_PKG_SUCCESS; /* no problem so far */ + + for (attempt = 0; attempt < 5; attempt++) { + if (attempt) + msleep(20); + + err = ice_aq_download_pkg(hw, prev_hunk, ICE_PKG_BUF_SIZE, + prev_was_last, &offset, &info, NULL); + + aq_err = hw->adminq.sq_last_status; + if (aq_err != ICE_AQ_RC_ENOSEC && aq_err != ICE_AQ_RC_EBADSIG) + break; + } + + if (err) { + ice_debug(hw, ICE_DBG_PKG, "Pkg download failed: err %d off %d inf %d\n", + err, offset, info); + ctx->err = ice_map_aq_err_to_ddp_state(aq_err); + } else if (attempt) { + dev_dbg(ice_hw_to_dev(hw), + "ice_aq_download_pkg number of retries: %d\n", attempt); + } + + return ctx->err; +} + +/** + * ice_dwnld_cfg_bufs_no_lock + * @ctx: context of the current buffers section to send + * @bufs: pointer to an array of buffers + * @start: buffer index of first buffer to download + * @count: the number of buffers to download + * + * Downloads package configuration buffers to the firmware. Metadata buffers + * are skipped, and the first metadata buffer found indicates that the rest + * of the buffers are all metadata buffers. + */ +static enum ice_ddp_state +ice_dwnld_cfg_bufs_no_lock(struct ice_ddp_send_ctx *ctx, struct ice_buf *bufs, + u32 start, u32 count) +{ + struct ice_buf_hdr *bh; + enum ice_ddp_state err; + + if (!bufs || !count) { + ctx->err = ICE_DDP_PKG_ERR; + return ctx->err; + } + + bufs += start; + bh = (struct ice_buf_hdr *)bufs; + + for (int i = 0; i < count; i++, bufs++) { + bh = (struct ice_buf_hdr *)bufs; + /* Metadata buffers should not be sent to FW, + * their presence means "we are done here". + */ + if (ice_is_buffer_metadata(bh)) + break; + + err = ice_ddp_send_hunk(ctx, bh); + if (err) + return err; + } + + return 0; +} + /** * ice_get_pkg_seg_by_idx * @pkg_hdr: pointer to the package header to be searched @@ -1269,147 +1390,32 @@ ice_is_signing_seg_type_at_idx(struct ice_pkg_hdr *pkg_hdr, u32 idx, return false; } -/** - * ice_is_buffer_metadata - determine if package buffer is a metadata buffer - * @buf: pointer to buffer header - */ -static bool ice_is_buffer_metadata(struct ice_buf_hdr *buf) -{ - if (le32_to_cpu(buf->section_entry[0].type) & ICE_METADATA_BUF) - return true; - - return false; -} - -/** - * ice_is_last_download_buffer - * @buf: pointer to current buffer header - * @idx: index of the buffer in the current sequence - * @count: the buffer count in the current sequence - * - * Note: this routine should only be called if the buffer is not the last buffer - */ -static bool -ice_is_last_download_buffer(struct ice_buf_hdr *buf, u32 idx, u32 count) -{ - struct ice_buf *next_buf; - - if ((idx + 1) == count) - return true; - - /* A set metadata flag in the next buffer will signal that the current - * buffer will be the last buffer downloaded - */ - next_buf = ((struct ice_buf *)buf) + 1; - - return ice_is_buffer_metadata((struct ice_buf_hdr *)next_buf); -} - -/** - * ice_dwnld_cfg_bufs_no_lock - * @hw: pointer to the hardware structure - * @bufs: pointer to an array of buffers - * @start: buffer index of first buffer to download - * @count: the number of buffers to download - * @indicate_last: if true, then set last buffer flag on last buffer download - * - * Downloads package configuration buffers to the firmware. Metadata buffers - * are skipped, and the first metadata buffer found indicates that the rest - * of the buffers are all metadata buffers. - */ -static enum ice_ddp_state -ice_dwnld_cfg_bufs_no_lock(struct ice_hw *hw, struct ice_buf *bufs, u32 start, - u32 count, bool indicate_last) -{ - enum ice_ddp_state state = ICE_DDP_PKG_SUCCESS; - struct ice_buf_hdr *bh; - enum ice_aq_err err; - u32 offset, info, i; - - if (!bufs || !count) - return ICE_DDP_PKG_ERR; - - /* If the first buffer's first section has its metadata bit set - * then there are no buffers to be downloaded, and the operation is - * considered a success. - */ - bh = (struct ice_buf_hdr *)(bufs + start); - if (le32_to_cpu(bh->section_entry[0].type) & ICE_METADATA_BUF) - return ICE_DDP_PKG_SUCCESS; - - for (i = 0; i < count; i++) { - bool last = false; - int try_cnt = 0; - int status; - - bh = (struct ice_buf_hdr *)(bufs + start + i); - - if (indicate_last) - last = ice_is_last_download_buffer(bh, i, count); - - while (1) { - status = ice_aq_download_pkg(hw, bh, ICE_PKG_BUF_SIZE, - last, &offset, &info, - NULL); - if (hw->adminq.sq_last_status != ICE_AQ_RC_ENOSEC && - hw->adminq.sq_last_status != ICE_AQ_RC_EBADSIG) - break; - - try_cnt++; - - if (try_cnt == 5) - break; - - msleep(20); - } - - if (try_cnt) - dev_dbg(ice_hw_to_dev(hw), - "ice_aq_download_pkg number of retries: %d\n", - try_cnt); - - /* Save AQ status from download package */ - if (status) { - ice_debug(hw, ICE_DBG_PKG, "Pkg download failed: err %d off %d inf %d\n", - status, offset, info); - err = hw->adminq.sq_last_status; - state = ice_map_aq_err_to_ddp_state(err); - break; - } - - if (last) - break; - } - - return state; -} - /** * ice_download_pkg_sig_seg - download a signature segment - * @hw: pointer to the hardware structure + * @ctx: context of the current buffers section to send * @seg: pointer to signature segment */ static enum ice_ddp_state -ice_download_pkg_sig_seg(struct ice_hw *hw, struct ice_sign_seg *seg) +ice_download_pkg_sig_seg(struct ice_ddp_send_ctx *ctx, struct ice_sign_seg *seg) { - return ice_dwnld_cfg_bufs_no_lock(hw, seg->buf_tbl.buf_array, 0, - le32_to_cpu(seg->buf_tbl.buf_count), - false); + return ice_dwnld_cfg_bufs_no_lock(ctx, seg->buf_tbl.buf_array, 0, + le32_to_cpu(seg->buf_tbl.buf_count)); } /** * ice_download_pkg_config_seg - download a config segment - * @hw: pointer to the hardware structure + * @ctx: context of the current buffers section to send * @pkg_hdr: pointer to package header * @idx: segment index * @start: starting buffer * @count: buffer count * * Note: idx must reference a ICE segment */ static enum ice_ddp_state -ice_download_pkg_config_seg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr, - u32 idx, u32 start, u32 count) +ice_download_pkg_config_seg(struct ice_ddp_send_ctx *ctx, + struct ice_pkg_hdr *pkg_hdr, u32 idx, u32 start, + u32 count) { struct ice_buf_table *bufs; struct ice_seg *seg; @@ -1425,21 +1431,20 @@ ice_download_pkg_config_seg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr, if (start >= buf_count || start + count > buf_count) return ICE_DDP_PKG_ERR; - return ice_dwnld_cfg_bufs_no_lock(hw, bufs->buf_array, start, count, - true); + return ice_dwnld_cfg_bufs_no_lock(ctx, bufs->buf_array, start, count); } /** * ice_dwnld_sign_and_cfg_segs - download a signing segment and config segment - * @hw: pointer to the hardware structure + * @ctx: context of the current buffers section to send * @pkg_hdr: pointer to package header * @idx: segment index (must be a signature segment) * * Note: idx must reference a signature segment */ static enum ice_ddp_state -ice_dwnld_sign_and_cfg_segs(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr, - u32 idx) +ice_dwnld_sign_and_cfg_segs(struct ice_ddp_send_ctx *ctx, + struct ice_pkg_hdr *pkg_hdr, u32 idx) { enum ice_ddp_state state; struct ice_sign_seg *seg; @@ -1454,17 +1459,16 @@ ice_dwnld_sign_and_cfg_segs(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr, } count = le32_to_cpu(seg->signed_buf_count); - state = ice_download_pkg_sig_seg(hw, seg); + state = ice_download_pkg_sig_seg(ctx, seg); if (state || !count) goto exit; conf_idx = le32_to_cpu(seg->signed_seg_idx); start = le32_to_cpu(seg->signed_buf_start); - state = ice_download_pkg_config_seg(hw, pkg_hdr, conf_idx, start, - count); - + return ice_download_pkg_config_seg(ctx, pkg_hdr, conf_idx, start, count); exit: + ctx->err = state; return state; } @@ -1519,6 +1523,7 @@ ice_download_pkg_with_sig_seg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr) { enum ice_aq_err aq_err = hw->adminq.sq_last_status; enum ice_ddp_state state = ICE_DDP_PKG_ERR; + struct ice_ddp_send_ctx ctx = { .hw = hw }; int status; u32 i; @@ -1539,7 +1544,9 @@ ice_download_pkg_with_sig_seg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr) hw->pkg_sign_type)) continue; - state = ice_dwnld_sign_and_cfg_segs(hw, pkg_hdr, i); + ice_dwnld_sign_and_cfg_segs(&ctx, pkg_hdr, i); + /* finish up by sending last hunk with "last" flag set */ + state = ice_ddp_send_hunk(&ctx, NULL); if (state) break; } @@ -1564,6 +1571,7 @@ ice_download_pkg_with_sig_seg(struct ice_hw *hw, struct ice_pkg_hdr *pkg_hdr) static enum ice_ddp_state ice_dwnld_cfg_bufs(struct ice_hw *hw, struct ice_buf *bufs, u32 count) { + struct ice_ddp_send_ctx ctx = { .hw = hw }; enum ice_ddp_state state; struct ice_buf_hdr *bh; int status; @@ -1576,17 +1584,19 @@ ice_dwnld_cfg_bufs(struct ice_hw *hw, struct ice_buf *bufs, u32 count) * considered a success. */ bh = (struct ice_buf_hdr *)bufs; - if (le32_to_cpu(bh->section_entry[0].type) & ICE_METADATA_BUF) + if (ice_is_buffer_metadata(bh)) return ICE_DDP_PKG_SUCCESS; status = ice_acquire_global_cfg_lock(hw, ICE_RES_WRITE); if (status) { if (status == -EALREADY) return ICE_DDP_PKG_ALREADY_LOADED; return ice_map_aq_err_to_ddp_state(hw->adminq.sq_last_status); } - state = ice_dwnld_cfg_bufs_no_lock(hw, bufs, 0, count, true); + ice_dwnld_cfg_bufs_no_lock(&ctx, bufs, 0, count); + /* finish up by sending last hunk with "last" flag set */ + state = ice_ddp_send_hunk(&ctx, NULL); if (!state) state = ice_post_dwnld_pkg_actions(hw);