[<prev] [next>] [day] [month] [year] [list]
Message-ID: <0f61f5ab-2358-4d1c-ab3c-78c345335bda@intel.com>
Date: Wed, 4 Sep 2024 14:37:23 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
CC: <intel-wired-lan@...ts.osuosl.org>, Larysa Zaremba
<larysa.zaremba@...el.com>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>, Tony Nguyen <anthony.l.nguyen@...el.com>
Subject: Re: [PATCH iwl-next] ice: make ice_cfg_tx_topo() almost const-correct
On 9/4/24 11:31, Przemek Kitszel wrote:
> Constify ice_cfg_tx_topo() @buf parameter.
> This cascades further down to few more functions.
>
> I set the boundary on ice_get_set_tx_topo(), which could not be easily
> constified as it is get-or-set, but it's set variant does not change
> provided data, the same holds for ice_aqc_opc_set_tx_topo() in particular.
> That's why there is (u8 *) cast to loose const prior to entering into AQ
> world of const-obliviousness.
>
> Reviewed-by: Larysa Zaremba <larysa.zaremba@...el.com>
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
I've forgot to CC netdev, sorry :F
I think there is no need to repost though.
Original URL (content is quoted entriely here) if it is more helpful
https://lore.kernel.org/intel-wired-lan/20240904093135.8795-2-przemyslaw.kitszel@intel.com/
I will make an alias for my git-sent-email invocations ;)
> ---
> drivers/net/ethernet/intel/ice/ice_ddp.h | 4 +-
> drivers/net/ethernet/intel/ice/ice_ddp.c | 48 ++++++++++++------------
> 2 files changed, 26 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.h b/drivers/net/ethernet/intel/ice/ice_ddp.h
> index 97f272317475..79551da2a4b0 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ddp.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ddp.h
> @@ -438,7 +438,7 @@ struct ice_pkg_enum {
> u32 buf_idx;
>
> u32 type;
> - struct ice_buf_hdr *buf;
> + const struct ice_buf_hdr *buf;
> u32 sect_idx;
> void *sect;
> u32 sect_type;
> @@ -467,6 +467,6 @@ ice_pkg_enum_entry(struct ice_seg *ice_seg, struct ice_pkg_enum *state,
> void *ice_pkg_enum_section(struct ice_seg *ice_seg, struct ice_pkg_enum *state,
> u32 sect_type);
>
> -int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len);
> +int ice_cfg_tx_topo(struct ice_hw *hw, const void *buf, u32 len);
>
> #endif
> diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c
> index 953262b88a58..f2ac1d265510 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ddp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
> @@ -31,7 +31,7 @@ static const struct ice_tunnel_type_scan tnls[] = {
> * Verifies various attributes of the package file, including length, format
> * version, and the requirement of at least one segment.
> */
> -static enum ice_ddp_state ice_verify_pkg(struct ice_pkg_hdr *pkg, u32 len)
> +static enum ice_ddp_state ice_verify_pkg(const struct ice_pkg_hdr *pkg, u32 len)
> {
> u32 seg_count;
> u32 i;
> @@ -57,13 +57,13 @@ static enum ice_ddp_state ice_verify_pkg(struct ice_pkg_hdr *pkg, u32 len)
> /* all segments must fit within length */
> for (i = 0; i < seg_count; i++) {
> u32 off = le32_to_cpu(pkg->seg_offset[i]);
> - struct ice_generic_seg_hdr *seg;
> + const struct ice_generic_seg_hdr *seg;
>
> /* segment header must fit */
> if (len < off + sizeof(*seg))
> return ICE_DDP_PKG_INVALID_FILE;
>
> - seg = (struct ice_generic_seg_hdr *)((u8 *)pkg + off);
> + seg = (void *)pkg + off;
>
> /* segment body must fit */
> if (len < off + le32_to_cpu(seg->seg_size))
> @@ -119,13 +119,13 @@ static enum ice_ddp_state ice_chk_pkg_version(struct ice_pkg_ver *pkg_ver)
> *
> * This helper function validates a buffer's header.
> */
> -static struct ice_buf_hdr *ice_pkg_val_buf(struct ice_buf *buf)
> +static const struct ice_buf_hdr *ice_pkg_val_buf(const struct ice_buf *buf)
> {
> - struct ice_buf_hdr *hdr;
> + const struct ice_buf_hdr *hdr;
> u16 section_count;
> u16 data_end;
>
> - hdr = (struct ice_buf_hdr *)buf->buf;
> + hdr = (const struct ice_buf_hdr *)buf->buf;
> /* verify data */
> section_count = le16_to_cpu(hdr->section_count);
> if (section_count < ICE_MIN_S_COUNT || section_count > ICE_MAX_S_COUNT)
> @@ -165,8 +165,8 @@ static struct ice_buf_table *ice_find_buf_table(struct ice_seg *ice_seg)
> * unexpected value has been detected (for example an invalid section count or
> * an invalid buffer end value).
> */
> -static struct ice_buf_hdr *ice_pkg_enum_buf(struct ice_seg *ice_seg,
> - struct ice_pkg_enum *state)
> +static const struct ice_buf_hdr *ice_pkg_enum_buf(struct ice_seg *ice_seg,
> + struct ice_pkg_enum *state)
> {
> if (ice_seg) {
> state->buf_table = ice_find_buf_table(ice_seg);
> @@ -1800,9 +1800,9 @@ int ice_update_pkg(struct ice_hw *hw, struct ice_buf *bufs, u32 count)
> * success it returns a pointer to the segment header, otherwise it will
> * return NULL.
> */
> -static struct ice_generic_seg_hdr *
> +static const struct ice_generic_seg_hdr *
> ice_find_seg_in_pkg(struct ice_hw *hw, u32 seg_type,
> - struct ice_pkg_hdr *pkg_hdr)
> + const struct ice_pkg_hdr *pkg_hdr)
> {
> u32 i;
>
> @@ -1813,11 +1813,9 @@ ice_find_seg_in_pkg(struct ice_hw *hw, u32 seg_type,
>
> /* Search all package segments for the requested segment type */
> for (i = 0; i < le32_to_cpu(pkg_hdr->seg_count); i++) {
> - struct ice_generic_seg_hdr *seg;
> + const struct ice_generic_seg_hdr *seg;
>
> - seg = (struct ice_generic_seg_hdr
> - *)((u8 *)pkg_hdr +
> - le32_to_cpu(pkg_hdr->seg_offset[i]));
> + seg = (void *)pkg_hdr + le32_to_cpu(pkg_hdr->seg_offset[i]);
>
> if (le32_to_cpu(seg->seg_type) == seg_type)
> return seg;
> @@ -2354,14 +2352,15 @@ ice_get_set_tx_topo(struct ice_hw *hw, u8 *buf, u16 buf_size,
> *
> * Return: zero when update was successful, negative values otherwise.
> */
> -int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
> +int ice_cfg_tx_topo(struct ice_hw *hw, const void *buf, u32 len)
> {
> - u8 *current_topo, *new_topo = NULL;
> - struct ice_run_time_cfg_seg *seg;
> - struct ice_buf_hdr *section;
> - struct ice_pkg_hdr *pkg_hdr;
> + const struct ice_run_time_cfg_seg *seg;
> + const struct ice_buf_hdr *section;
> + const struct ice_pkg_hdr *pkg_hdr;
> + const u8 *new_topo = NULL;
> enum ice_ddp_state state;
> u16 offset, size = 0;
> + u8 *current_topo;
> u32 reg = 0;
> int status;
> u8 flags;
> @@ -2379,7 +2378,7 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
> if (!current_topo)
> return -ENOMEM;
>
> - /* Get the current Tx topology */
> + /* Get the current Tx topology flags */
> status = ice_get_set_tx_topo(hw, current_topo, ICE_AQ_MAX_BUF_LEN, NULL,
> &flags, false);
>
> @@ -2419,16 +2418,16 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
> goto update_topo;
> }
>
> - pkg_hdr = (struct ice_pkg_hdr *)buf;
> + pkg_hdr = (const struct ice_pkg_hdr *)buf;
> state = ice_verify_pkg(pkg_hdr, len);
> if (state) {
> ice_debug(hw, ICE_DBG_INIT, "Failed to verify pkg (err: %d)\n",
> state);
> return -EIO;
> }
>
> /* Find runtime configuration segment */
> - seg = (struct ice_run_time_cfg_seg *)
> + seg = (const struct ice_run_time_cfg_seg *)
> ice_find_seg_in_pkg(hw, SEGMENT_TYPE_ICE_RUN_TIME_CFG, pkg_hdr);
> if (!seg) {
> ice_debug(hw, ICE_DBG_INIT, "5 layer topology segment is missing\n");
> @@ -2462,7 +2461,7 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
> }
>
> /* Get the new topology buffer */
> - new_topo = ((u8 *)section) + offset;
> + new_topo = (const u8 *)section + offset;
>
> update_topo:
> /* Acquire global lock to make sure that set topology issued
> @@ -2485,7 +2484,8 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len)
> }
>
> /* Set new topology */
> - status = ice_get_set_tx_topo(hw, new_topo, size, NULL, NULL, true);
> + status = ice_get_set_tx_topo(hw, (u8 *)new_topo, size, NULL, NULL,
> + true);
> if (status) {
> ice_debug(hw, ICE_DBG_INIT, "Failed setting Tx topology\n");
> return status;
>
> base-commit: d6f75d86aa786740ef7a7607685e9e1039e30aab
Powered by blists - more mailing lists