diff mbox series

scsi: pm8001: clean up structurally dead code when PM8001_USE_MSIX is defined

Message ID 20190328234328.15860-1-colin.king@canonical.com (mailing list archive)
State Accepted
Headers show
Series scsi: pm8001: clean up structurally dead code when PM8001_USE_MSIX is defined | expand

Commit Message

Colin King March 28, 2019, 11:43 p.m. UTC 
From: Colin Ian King <colin.king@canonical.com>

When macro PM8001_USE_MSIX is defined there are redundant dead code
calls to pm8001_chip_intx_interrupt_{enable|disable}. Clean this up
by compiling in the appropriate enable/disable handlers for the
defined PM8001_USE_MSIX and undefined PM8001_USE_MSIX cases.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/scsi/pm8001/pm8001_hwi.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Comments

Jinpu Wang March 29, 2019, 8:16 a.m. UTC | #1 
On Fri, Mar 29, 2019 at 12:43 AM Colin King <colin.king@canonical.com> wrote:
>
> From: Colin Ian King <colin.king@canonical.com>
>
> When macro PM8001_USE_MSIX is defined there are redundant dead code
> calls to pm8001_chip_intx_interrupt_{enable|disable}. Clean this up
> by compiling in the appropriate enable/disable handlers for the
> defined PM8001_USE_MSIX and undefined PM8001_USE_MSIX cases.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Thanks, Colin,
Acked-by: Jack Wang <jinpu.wang@cloud.ionos.com>
> ---
>  drivers/scsi/pm8001/pm8001_hwi.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
> index e4209091c1da..7d81246a432b 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.c
> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
> @@ -1204,6 +1204,7 @@ void pm8001_chip_iounmap(struct pm8001_hba_info *pm8001_ha)
>         }
>  }
>
> +#ifndef PM8001_USE_MSIX
>  /**
>   * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
>   * @pm8001_ha: our hba card information
> @@ -1225,6 +1226,8 @@ pm8001_chip_intx_interrupt_disable(struct pm8001_hba_info *pm8001_ha)
>         pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_MASK_ALL);
>  }
>
> +#else
> +
>  /**
>   * pm8001_chip_msix_interrupt_enable - enable PM8001 chip interrupt
>   * @pm8001_ha: our hba card information
> @@ -1256,6 +1259,7 @@ pm8001_chip_msix_interrupt_disable(struct pm8001_hba_info *pm8001_ha,
>         msi_index += MSIX_TABLE_BASE;
>         pm8001_cw32(pm8001_ha, 0,  msi_index, MSIX_INTERRUPT_DISABLE);
>  }
> +#endif
>
>  /**
>   * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
> @@ -1266,10 +1270,9 @@ pm8001_chip_interrupt_enable(struct pm8001_hba_info *pm8001_ha, u8 vec)
>  {
>  #ifdef PM8001_USE_MSIX
>         pm8001_chip_msix_interrupt_enable(pm8001_ha, 0);
> -       return;
> -#endif
> +#else
>         pm8001_chip_intx_interrupt_enable(pm8001_ha);
> -
> +#endif
>  }
>
>  /**
> @@ -1281,10 +1284,9 @@ pm8001_chip_interrupt_disable(struct pm8001_hba_info *pm8001_ha, u8 vec)
>  {
>  #ifdef PM8001_USE_MSIX
>         pm8001_chip_msix_interrupt_disable(pm8001_ha, 0);
> -       return;
> -#endif
> +#else
>         pm8001_chip_intx_interrupt_disable(pm8001_ha);
> -
> +#endif
>  }
>
>  /**
> --
> 2.20.1
>
Mukesh Ojha March 29, 2019, 10:29 a.m. UTC | #2 
On 3/29/2019 5:13 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> When macro PM8001_USE_MSIX is defined there are redundant dead code
> calls to pm8001_chip_intx_interrupt_{enable|disable}. Clean this up
> by compiling in the appropriate enable/disable handlers for the
> defined PM8001_USE_MSIX and undefined PM8001_USE_MSIX cases.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>


This looks good.

Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>


Not relevant to this patch but Can you do something about 
pm8001_chip_is_our_interupt() as well ?


Cheers,
-Mukesh
> ---
>   drivers/scsi/pm8001/pm8001_hwi.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
> index e4209091c1da..7d81246a432b 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.c
> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
> @@ -1204,6 +1204,7 @@ void pm8001_chip_iounmap(struct pm8001_hba_info *pm8001_ha)
>   	}
>   }
>   
> +#ifndef PM8001_USE_MSIX
>   /**
>    * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
>    * @pm8001_ha: our hba card information
> @@ -1225,6 +1226,8 @@ pm8001_chip_intx_interrupt_disable(struct pm8001_hba_info *pm8001_ha)
>   	pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_MASK_ALL);
>   }
>   
> +#else
> +
>   /**
>    * pm8001_chip_msix_interrupt_enable - enable PM8001 chip interrupt
>    * @pm8001_ha: our hba card information
> @@ -1256,6 +1259,7 @@ pm8001_chip_msix_interrupt_disable(struct pm8001_hba_info *pm8001_ha,
>   	msi_index += MSIX_TABLE_BASE;
>   	pm8001_cw32(pm8001_ha, 0,  msi_index, MSIX_INTERRUPT_DISABLE);
>   }
> +#endif
>   
>   /**
>    * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
> @@ -1266,10 +1270,9 @@ pm8001_chip_interrupt_enable(struct pm8001_hba_info *pm8001_ha, u8 vec)
>   {
>   #ifdef PM8001_USE_MSIX
>   	pm8001_chip_msix_interrupt_enable(pm8001_ha, 0);
> -	return;
> -#endif
> +#else
>   	pm8001_chip_intx_interrupt_enable(pm8001_ha);
> -
> +#endif
>   }
>   
>   /**
> @@ -1281,10 +1284,9 @@ pm8001_chip_interrupt_disable(struct pm8001_hba_info *pm8001_ha, u8 vec)
>   {
>   #ifdef PM8001_USE_MSIX
>   	pm8001_chip_msix_interrupt_disable(pm8001_ha, 0);
> -	return;
> -#endif
> +#else
>   	pm8001_chip_intx_interrupt_disable(pm8001_ha);
> -
> +#endif
>   }
>   
>   /**
Colin King March 29, 2019, 10:38 a.m. UTC | #3 
On 29/03/2019 10:29, Mukesh Ojha wrote:
> 
> On 3/29/2019 5:13 AM, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> When macro PM8001_USE_MSIX is defined there are redundant dead code
>> calls to pm8001_chip_intx_interrupt_{enable|disable}. Clean this up
>> by compiling in the appropriate enable/disable handlers for the
>> defined PM8001_USE_MSIX and undefined PM8001_USE_MSIX cases.
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> 
> 
> This looks good.
> 
> Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>
> 
> 
> Not relevant to this patch but Can you do something about
> pm8001_chip_is_our_interupt() as well ?

Ah, yes, I missed that one, will do that sometime today.

> 
> 
> Cheers,
> -Mukesh
>> ---
>>   drivers/scsi/pm8001/pm8001_hwi.c | 14 ++++++++------
>>   1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c
>> b/drivers/scsi/pm8001/pm8001_hwi.c
>> index e4209091c1da..7d81246a432b 100644
>> --- a/drivers/scsi/pm8001/pm8001_hwi.c
>> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
>> @@ -1204,6 +1204,7 @@ void pm8001_chip_iounmap(struct pm8001_hba_info
>> *pm8001_ha)
>>       }
>>   }
>>   +#ifndef PM8001_USE_MSIX
>>   /**
>>    * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
>>    * @pm8001_ha: our hba card information
>> @@ -1225,6 +1226,8 @@ pm8001_chip_intx_interrupt_disable(struct
>> pm8001_hba_info *pm8001_ha)
>>       pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_MASK_ALL);
>>   }
>>   +#else
>> +
>>   /**
>>    * pm8001_chip_msix_interrupt_enable - enable PM8001 chip interrupt
>>    * @pm8001_ha: our hba card information
>> @@ -1256,6 +1259,7 @@ pm8001_chip_msix_interrupt_disable(struct
>> pm8001_hba_info *pm8001_ha,
>>       msi_index += MSIX_TABLE_BASE;
>>       pm8001_cw32(pm8001_ha, 0,  msi_index, MSIX_INTERRUPT_DISABLE);
>>   }
>> +#endif
>>     /**
>>    * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
>> @@ -1266,10 +1270,9 @@ pm8001_chip_interrupt_enable(struct
>> pm8001_hba_info *pm8001_ha, u8 vec)
>>   {
>>   #ifdef PM8001_USE_MSIX
>>       pm8001_chip_msix_interrupt_enable(pm8001_ha, 0);
>> -    return;
>> -#endif
>> +#else
>>       pm8001_chip_intx_interrupt_enable(pm8001_ha);
>> -
>> +#endif
>>   }
>>     /**
>> @@ -1281,10 +1284,9 @@ pm8001_chip_interrupt_disable(struct
>> pm8001_hba_info *pm8001_ha, u8 vec)
>>   {
>>   #ifdef PM8001_USE_MSIX
>>       pm8001_chip_msix_interrupt_disable(pm8001_ha, 0);
>> -    return;
>> -#endif
>> +#else
>>       pm8001_chip_intx_interrupt_disable(pm8001_ha);
>> -
>> +#endif
>>   }
>>     /**
Martin K. Petersen March 29, 2019, 2:14 p.m. UTC | #4 
Colin,

> When macro PM8001_USE_MSIX is defined there are redundant dead code
> calls to pm8001_chip_intx_interrupt_{enable|disable}. Clean this up by
> compiling in the appropriate enable/disable handlers for the defined
> PM8001_USE_MSIX and undefined PM8001_USE_MSIX cases.

Applied to 5.2/scsi-queue, thanks!
diff mbox series

Patch

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index e4209091c1da..7d81246a432b 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -1204,6 +1204,7 @@  void pm8001_chip_iounmap(struct pm8001_hba_info *pm8001_ha)
 	}
 }
 
+#ifndef PM8001_USE_MSIX
 /**
  * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
  * @pm8001_ha: our hba card information
@@ -1225,6 +1226,8 @@  pm8001_chip_intx_interrupt_disable(struct pm8001_hba_info *pm8001_ha)
 	pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_MASK_ALL);
 }
 
+#else
+
 /**
  * pm8001_chip_msix_interrupt_enable - enable PM8001 chip interrupt
  * @pm8001_ha: our hba card information
@@ -1256,6 +1259,7 @@  pm8001_chip_msix_interrupt_disable(struct pm8001_hba_info *pm8001_ha,
 	msi_index += MSIX_TABLE_BASE;
 	pm8001_cw32(pm8001_ha, 0,  msi_index, MSIX_INTERRUPT_DISABLE);
 }
+#endif
 
 /**
  * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
@@ -1266,10 +1270,9 @@  pm8001_chip_interrupt_enable(struct pm8001_hba_info *pm8001_ha, u8 vec)
 {
 #ifdef PM8001_USE_MSIX
 	pm8001_chip_msix_interrupt_enable(pm8001_ha, 0);
-	return;
-#endif
+#else
 	pm8001_chip_intx_interrupt_enable(pm8001_ha);
-
+#endif
 }
 
 /**
@@ -1281,10 +1284,9 @@  pm8001_chip_interrupt_disable(struct pm8001_hba_info *pm8001_ha, u8 vec)
 {
 #ifdef PM8001_USE_MSIX
 	pm8001_chip_msix_interrupt_disable(pm8001_ha, 0);
-	return;
-#endif
+#else
 	pm8001_chip_intx_interrupt_disable(pm8001_ha);
-
+#endif
 }
 
 /**