[PATCH v2] scsi: sli4: Remove the buggy code

[PATCH v2] scsi: sli4: Remove the buggy code

[coolrrsh]

From: Rajeshwar R Shinde <coolrrsh@gmail.com>

In the function sli_xmit_bls_rsp64_wqe, the 'if' and 'else' conditions
evaluates the same expression and gives same output. Also the variable
bls->local_n_port_id_dword is not used anywhere. Therefore removing the 
redundant code.

This fixes coccinelle warning such as:
drivers/scsi/elx/libefc_sli/sli4.c:2320:2-4: WARNING: possible
condition with no effect (if == else)

Signed-off-by: Rajeshwar R Shinde <coolrrsh@gmail.com>
---
v1->v2
Modified patch and verified with checkpatch.pl.

---
 drivers/scsi/elx/libefc_sli/sli4.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/scsi/elx/libefc_sli/sli4.c b/drivers/scsi/elx/libefc_sli/sli4.c
index 8f96049f62dd..af661b769464 100644
--- a/drivers/scsi/elx/libefc_sli/sli4.c
+++ b/drivers/scsi/elx/libefc_sli/sli4.c
@@ -2317,13 +2317,6 @@ sli_xmit_bls_rsp64_wqe(struct sli4 *sli, void *buf,
 		SLI4_GENERIC_CONTEXT_VPI << SLI4_BLS_RSP_WQE_CT_SHFT;
 		bls->context_tag = cpu_to_le16(params->vpi);
 
-		if (params->s_id != U32_MAX)
-			bls->local_n_port_id_dword |=
-				cpu_to_le32(params->s_id & 0x00ffffff);
-		else
-			bls->local_n_port_id_dword |=
-				cpu_to_le32(params->s_id & 0x00ffffff);
-
 		dw_ridflags = (dw_ridflags & ~SLI4_BLS_RSP_RID) |
 			       (params->d_id & SLI4_BLS_RSP_RID);
 
-- 
2.25.1

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v2] scsi: sli4: Remove the buggy code

[Dmitry Bogdanov]

On Thu, Aug 17, 2023 at 04:07:51PM +0530, coolrrsh@gmail.com wrote:
> 
> From: Rajeshwar R Shinde <coolrrsh@gmail.com>
> 
> In the function sli_xmit_bls_rsp64_wqe, the 'if' and 'else' conditions
> evaluates the same expression and gives same output. Also the variable
> bls->local_n_port_id_dword is not used anywhere. Therefore removing the
> redundant code.
> 
> This fixes coccinelle warning such as:
> drivers/scsi/elx/libefc_sli/sli4.c:2320:2-4: WARNING: possible
> condition with no effect (if == else)
> 
> Signed-off-by: Rajeshwar R Shinde <coolrrsh@gmail.com>
> ---
> v1->v2
> Modified patch and verified with checkpatch.pl.
> 
> ---
>  drivers/scsi/elx/libefc_sli/sli4.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/scsi/elx/libefc_sli/sli4.c b/drivers/scsi/elx/libefc_sli/sli4.c
> index 8f96049f62dd..af661b769464 100644
> --- a/drivers/scsi/elx/libefc_sli/sli4.c
> +++ b/drivers/scsi/elx/libefc_sli/sli4.c
> @@ -2317,13 +2317,6 @@ sli_xmit_bls_rsp64_wqe(struct sli4 *sli, void *buf,
>                 SLI4_GENERIC_CONTEXT_VPI << SLI4_BLS_RSP_WQE_CT_SHFT;
>                 bls->context_tag = cpu_to_le16(params->vpi);
> 
> -               if (params->s_id != U32_MAX)
> -                       bls->local_n_port_id_dword |=
> -                               cpu_to_le32(params->s_id & 0x00ffffff);
> -               else
> -                       bls->local_n_port_id_dword |=
> -                               cpu_to_le32(params->s_id & 0x00ffffff);
> -

omg, it is not an unused variable. Whole bls is a HW descriptor, all of
its variables are used by HW. You should keep v1 version of the patch.
According to the comment at the beginning of the funciton s_id here shall
be != U32_MAX. That is an explanation for your v1 patch.

>                 dw_ridflags = (dw_ridflags & ~SLI4_BLS_RSP_RID) |
>                                (params->d_id & SLI4_BLS_RSP_RID);
> 
> --
> 2.25.1
> 
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v2] scsi: sli4: Remove the buggy code

[Ram Kishore Vegesna via Linux-kernel-mentees]

[-- Attachment #1.1.1: Type: text/plain, Size: 3070 bytes --]

On Thu, Aug 17, 2023 at 4:22 PM Dmitry Bogdanov <d.bogdanov@yadro.com>
wrote:

> On Thu, Aug 17, 2023 at 04:07:51PM +0530, coolrrsh@gmail.com wrote:
> >
> > From: Rajeshwar R Shinde <coolrrsh@gmail.com>
> >
> > In the function sli_xmit_bls_rsp64_wqe, the 'if' and 'else' conditions
> > evaluates the same expression and gives same output. Also the variable
> > bls->local_n_port_id_dword is not used anywhere. Therefore removing the
> > redundant code.
> >
> > This fixes coccinelle warning such as:
> > drivers/scsi/elx/libefc_sli/sli4.c:2320:2-4: WARNING: possible
> > condition with no effect (if == else)
> >
> > Signed-off-by: Rajeshwar R Shinde <coolrrsh@gmail.com>
> > ---
> > v1->v2
> > Modified patch and verified with checkpatch.pl.
> >
> > ---
> >  drivers/scsi/elx/libefc_sli/sli4.c | 7 -------
> >  1 file changed, 7 deletions(-)
> >
> > diff --git a/drivers/scsi/elx/libefc_sli/sli4.c
> b/drivers/scsi/elx/libefc_sli/sli4.c
> > index 8f96049f62dd..af661b769464 100644
> > --- a/drivers/scsi/elx/libefc_sli/sli4.c
> > +++ b/drivers/scsi/elx/libefc_sli/sli4.c
> > @@ -2317,13 +2317,6 @@ sli_xmit_bls_rsp64_wqe(struct sli4 *sli, void
> *buf,
> >                 SLI4_GENERIC_CONTEXT_VPI << SLI4_BLS_RSP_WQE_CT_SHFT;
> >                 bls->context_tag = cpu_to_le16(params->vpi);
> >
> > -               if (params->s_id != U32_MAX)
> > -                       bls->local_n_port_id_dword |=
> > -                               cpu_to_le32(params->s_id & 0x00ffffff);
> > -               else
> > -                       bls->local_n_port_id_dword |=
> > -                               cpu_to_le32(params->s_id & 0x00ffffff);
> > -
>
> omg, it is not an unused variable. Whole bls is a HW descriptor, all of
> its variables are used by HW. You should keep v1 version of the patch.
> According to the comment at the beginning of the funciton s_id here shall
> be != U32_MAX. That is an explanation for your v1 patch.
>
> >                 dw_ridflags = (dw_ridflags & ~SLI4_BLS_RSP_RID) |
> >                                (params->d_id & SLI4_BLS_RSP_RID);
> >
> > --
> > 2.25.1
> >

local_n_port_id_dword is not an unused variable. As Dimtry pointed out
all the above variables are used by the HW.

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

[-- Attachment #1.1.2: Type: text/html, Size: 4108 bytes --]

[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4214 bytes --]

[-- Attachment #2: Type: text/plain, Size: 201 bytes --]

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v2] scsi: sli4: Remove the buggy code

[Greg KH]

On Thu, Aug 17, 2023 at 06:03:32PM +0530, Ram Kishore Vegesna via Linux-kernel-mentees wrote:
> -- 
> This electronic communication and the information and any files transmitted 
> with it, or attached to it, are confidential and are intended solely for 
> the use of the individual or entity to whom it is addressed and may contain 
> information that is confidential, legally privileged, protected by privacy 
> laws, or otherwise restricted from disclosure to anyone else. If you are 
> not the intended recipient or the person responsible for delivering the 
> e-mail to the intended recipient, you are hereby notified that any use, 
> copying, distributing, dissemination, forwarding, printing, or copying of 
> this e-mail is strictly prohibited. If you received this e-mail in error, 
> please return the e-mail to the sender, delete it from your computer, and 
> destroy any printed copy of it.

Now deleted.

(hint, this footer is not compatible with kernel development, please fix
your email system...)
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH v2] scsi: sli4: Remove the buggy code

[Randy Dunlap]

On 8/17/23 03:52, Dmitry Bogdanov wrote:
> On Thu, Aug 17, 2023 at 04:07:51PM +0530, coolrrsh@gmail.com wrote:
>>
>> From: Rajeshwar R Shinde <coolrrsh@gmail.com>
>>
>> In the function sli_xmit_bls_rsp64_wqe, the 'if' and 'else' conditions
>> evaluates the same expression and gives same output. Also the variable
>> bls->local_n_port_id_dword is not used anywhere. Therefore removing the
>> redundant code.
>>
>> This fixes coccinelle warning such as:
>> drivers/scsi/elx/libefc_sli/sli4.c:2320:2-4: WARNING: possible
>> condition with no effect (if == else)
>>
>> Signed-off-by: Rajeshwar R Shinde <coolrrsh@gmail.com>
>> ---
>> v1->v2
>> Modified patch and verified with checkpatch.pl.
>>
>> ---
>>  drivers/scsi/elx/libefc_sli/sli4.c | 7 -------
>>  1 file changed, 7 deletions(-)
>>
>> diff --git a/drivers/scsi/elx/libefc_sli/sli4.c b/drivers/scsi/elx/libefc_sli/sli4.c
>> index 8f96049f62dd..af661b769464 100644
>> --- a/drivers/scsi/elx/libefc_sli/sli4.c
>> +++ b/drivers/scsi/elx/libefc_sli/sli4.c
>> @@ -2317,13 +2317,6 @@ sli_xmit_bls_rsp64_wqe(struct sli4 *sli, void *buf,
>>                 SLI4_GENERIC_CONTEXT_VPI << SLI4_BLS_RSP_WQE_CT_SHFT;
>>                 bls->context_tag = cpu_to_le16(params->vpi);
>>
>> -               if (params->s_id != U32_MAX)
>> -                       bls->local_n_port_id_dword |=
>> -                               cpu_to_le32(params->s_id & 0x00ffffff);
>> -               else
>> -                       bls->local_n_port_id_dword |=
>> -                               cpu_to_le32(params->s_id & 0x00ffffff);
>> -
> 
> omg, it is not an unused variable. Whole bls is a HW descriptor, all of
> its variables are used by HW. You should keep v1 version of the patch.
> According to the comment at the beginning of the funciton s_id here shall
> be != U32_MAX. That is an explanation for your v1 patch.
> 

Thanks for your comments.

>>                 dw_ridflags = (dw_ridflags & ~SLI4_BLS_RSP_RID) |
>>                                (params->d_id & SLI4_BLS_RSP_RID);
>>
>> --
>> 2.25.1
>>

-- 
~Randy
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees