[Intel-wired-lan] [PATCH iwl-next v1] idpf: remove unreachable code from setting mailbox

[Intel-wired-lan] [PATCH iwl-next v1] idpf: remove unreachable code from setting mailbox

[Michal Swiatkowski]

Remove code that isn't reached. There is no need to check for
adapter->req_vec_chunks, because if it isn't set idpf_set_mb_vec_id()
won't be called.

Only one path when idpf_set_mb_vec_id() is called:
idpf_intr_req()
 -> idpf_send_alloc_vectors_msg() -> adapter->req_vec_chunk is allocated
 here, otherwise an error is returned and idpf_intr_req() exits with an
 error.

The idpf_set_mb_vec_id() becomes one-linear and it is called only once.
Remove it and set mailbox vector index directly.

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
Reviewed-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
---
 drivers/net/ethernet/intel/idpf/idpf_lib.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 730a9c7a59f2..4c18c5fceb97 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -143,22 +143,6 @@ static int idpf_mb_intr_req_irq(struct idpf_adapter *adapter)
 	return 0;
 }
 
-/**
- * idpf_set_mb_vec_id - Set vector index for mailbox
- * @adapter: adapter structure to access the vector chunks
- *
- * The first vector id in the requested vector chunks from the CP is for
- * the mailbox
- */
-static void idpf_set_mb_vec_id(struct idpf_adapter *adapter)
-{
-	if (adapter->req_vec_chunks)
-		adapter->mb_vector.v_idx =
-			le16_to_cpu(adapter->caps.mailbox_vector_id);
-	else
-		adapter->mb_vector.v_idx = 0;
-}
-
 /**
  * idpf_mb_intr_init - Initialize the mailbox interrupt
  * @adapter: adapter structure to store the mailbox vector
@@ -349,7 +333,7 @@ int idpf_intr_req(struct idpf_adapter *adapter)
 		goto free_irq;
 	}
 
-	idpf_set_mb_vec_id(adapter);
+	adapter->mb_vector.v_idx = le16_to_cpu(adapter->caps.mailbox_vector_id);
 
 	vecids = kcalloc(total_vecs, sizeof(u16), GFP_KERNEL);
 	if (!vecids) {
-- 
2.42.0

[PATCH iwl-next v1] idpf: remove unreachable code from setting mailbox

[Michal Swiatkowski]

Remove code that isn't reached. There is no need to check for
adapter->req_vec_chunks, because if it isn't set idpf_set_mb_vec_id()
won't be called.

Only one path when idpf_set_mb_vec_id() is called:
idpf_intr_req()
 -> idpf_send_alloc_vectors_msg() -> adapter->req_vec_chunk is allocated
 here, otherwise an error is returned and idpf_intr_req() exits with an
 error.

The idpf_set_mb_vec_id() becomes one-linear and it is called only once.
Remove it and set mailbox vector index directly.

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
Reviewed-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
---
 drivers/net/ethernet/intel/idpf/idpf_lib.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 730a9c7a59f2..4c18c5fceb97 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -143,22 +143,6 @@ static int idpf_mb_intr_req_irq(struct idpf_adapter *adapter)
 	return 0;
 }
 
-/**
- * idpf_set_mb_vec_id - Set vector index for mailbox
- * @adapter: adapter structure to access the vector chunks
- *
- * The first vector id in the requested vector chunks from the CP is for
- * the mailbox
- */
-static void idpf_set_mb_vec_id(struct idpf_adapter *adapter)
-{
-	if (adapter->req_vec_chunks)
-		adapter->mb_vector.v_idx =
-			le16_to_cpu(adapter->caps.mailbox_vector_id);
-	else
-		adapter->mb_vector.v_idx = 0;
-}
-
 /**
  * idpf_mb_intr_init - Initialize the mailbox interrupt
  * @adapter: adapter structure to store the mailbox vector
@@ -349,7 +333,7 @@ int idpf_intr_req(struct idpf_adapter *adapter)
 		goto free_irq;
 	}
 
-	idpf_set_mb_vec_id(adapter);
+	adapter->mb_vector.v_idx = le16_to_cpu(adapter->caps.mailbox_vector_id);
 
 	vecids = kcalloc(total_vecs, sizeof(u16), GFP_KERNEL);
 	if (!vecids) {
-- 
2.42.0

Re: [Intel-wired-lan] [PATCH iwl-next v1] idpf: remove unreachable code from setting mailbox

[Simon Horman]

On Wed, Apr 09, 2025 at 08:29:45AM +0200, Michal Swiatkowski wrote:
> Remove code that isn't reached. There is no need to check for
> adapter->req_vec_chunks, because if it isn't set idpf_set_mb_vec_id()
> won't be called.
> 
> Only one path when idpf_set_mb_vec_id() is called:
> idpf_intr_req()
>  -> idpf_send_alloc_vectors_msg() -> adapter->req_vec_chunk is allocated
>  here, otherwise an error is returned and idpf_intr_req() exits with an
>  error.

I agree this is correct, but perhaps it would be clearer to say something
like this:

* idpf_set_mb_vec_id() is only called from idpf_intr_req()
* Before that idpf_intr_req() calls idpf_send_alloc_vectors_msg()
* idpf_send_alloc_vectors_msg() allocates adapter->req_vec_chunk

> 
> The idpf_set_mb_vec_id() becomes one-linear and it is called only once.

nit: one liner

> Remove it and set mailbox vector index directly.
> 
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> Reviewed-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

The above notwithstanding, this looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH iwl-next v1] idpf: remove unreachable code from setting mailbox

[Simon Horman]

On Wed, Apr 09, 2025 at 08:29:45AM +0200, Michal Swiatkowski wrote:
> Remove code that isn't reached. There is no need to check for
> adapter->req_vec_chunks, because if it isn't set idpf_set_mb_vec_id()
> won't be called.
> 
> Only one path when idpf_set_mb_vec_id() is called:
> idpf_intr_req()
>  -> idpf_send_alloc_vectors_msg() -> adapter->req_vec_chunk is allocated
>  here, otherwise an error is returned and idpf_intr_req() exits with an
>  error.

I agree this is correct, but perhaps it would be clearer to say something
like this:

* idpf_set_mb_vec_id() is only called from idpf_intr_req()
* Before that idpf_intr_req() calls idpf_send_alloc_vectors_msg()
* idpf_send_alloc_vectors_msg() allocates adapter->req_vec_chunk

> 
> The idpf_set_mb_vec_id() becomes one-linear and it is called only once.

nit: one liner

> Remove it and set mailbox vector index directly.
> 
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> Reviewed-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

The above notwithstanding, this looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [Intel-wired-lan] [PATCH iwl-next v1] idpf: remove unreachable code from setting mailbox

[Salin, Samuel]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Simon Horman
> Sent: Friday, April 11, 2025 3:45 AM
> To: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Loktionov,
> Aleksandr <aleksandr.loktionov@intel.com>; Kubiak, Michal
> <michal.kubiak@intel.com>; Linga, Pavan Kumar
> <pavan.kumar.linga@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1] idpf: remove unreachable
> code from setting mailbox
> 
> On Wed, Apr 09, 2025 at 08:29:45AM +0200, Michal Swiatkowski wrote:
> > Remove code that isn't reached. There is no need to check for
> > adapter->req_vec_chunks, because if it isn't set idpf_set_mb_vec_id()
> > won't be called.
> >
> > Only one path when idpf_set_mb_vec_id() is called:
> > idpf_intr_req()
> >  -> idpf_send_alloc_vectors_msg() -> adapter->req_vec_chunk is
> > allocated  here, otherwise an error is returned and idpf_intr_req()
> > exits with an  error.
> 
> I agree this is correct, but perhaps it would be clearer to say something like
> this:
> 
> * idpf_set_mb_vec_id() is only called from idpf_intr_req()
> * Before that idpf_intr_req() calls idpf_send_alloc_vectors_msg()
> * idpf_send_alloc_vectors_msg() allocates adapter->req_vec_chunk
> 
> >
> > The idpf_set_mb_vec_id() becomes one-linear and it is called only once.
> 
> nit: one liner
> 
> > Remove it and set mailbox vector index directly.
> >
> > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> > Reviewed-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> 
> The above notwithstanding, this looks good to me.
> 
> Reviewed-by: Simon Horman <horms@kernel.org>

Tested-by: Samuel Salin <Samuel.salin@intel.com>

RE: [Intel-wired-lan] [PATCH iwl-next v1] idpf: remove unreachable code from setting mailbox

[Salin, Samuel]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Simon Horman
> Sent: Friday, April 11, 2025 3:45 AM
> To: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Loktionov,
> Aleksandr <aleksandr.loktionov@intel.com>; Kubiak, Michal
> <michal.kubiak@intel.com>; Linga, Pavan Kumar
> <pavan.kumar.linga@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1] idpf: remove unreachable
> code from setting mailbox
> 
> On Wed, Apr 09, 2025 at 08:29:45AM +0200, Michal Swiatkowski wrote:
> > Remove code that isn't reached. There is no need to check for
> > adapter->req_vec_chunks, because if it isn't set idpf_set_mb_vec_id()
> > won't be called.
> >
> > Only one path when idpf_set_mb_vec_id() is called:
> > idpf_intr_req()
> >  -> idpf_send_alloc_vectors_msg() -> adapter->req_vec_chunk is
> > allocated  here, otherwise an error is returned and idpf_intr_req()
> > exits with an  error.
> 
> I agree this is correct, but perhaps it would be clearer to say something like
> this:
> 
> * idpf_set_mb_vec_id() is only called from idpf_intr_req()
> * Before that idpf_intr_req() calls idpf_send_alloc_vectors_msg()
> * idpf_send_alloc_vectors_msg() allocates adapter->req_vec_chunk
> 
> >
> > The idpf_set_mb_vec_id() becomes one-linear and it is called only once.
> 
> nit: one liner
> 
> > Remove it and set mailbox vector index directly.
> >
> > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
> > Reviewed-by: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> 
> The above notwithstanding, this looks good to me.
> 
> Reviewed-by: Simon Horman <horms@kernel.org>

Tested-by: Samuel Salin <Samuel.salin@intel.com>