Re: [PATCH] scsi: pm8001: build in relevant functions and code on PM8001_USE_MSIX

[Jinpu Wang]

On Wed, Mar 22, 2017 at 7:50 PM, Colin King  wrote:
> From: Colin Ian King 
>
> Currently the misx and intx variables of the interrupt enable/disable
> helper functions are built in no matter what the setting of the
> macro PM8001_USE_MSIX.  Clean this up by just building in the
> necessary helper functions and calls to these functions depending on
> the setting of PM8001_USE_MSIX.  This addresses several dead code
> paths found by static analysis with CoverityScan.
>
> Signed-off-by: Colin Ian King 
> ---
>  drivers/scsi/pm8001/pm8001_hwi.c | 63 
> +---
>  1 file changed, 33 insertions(+), 30 deletions(-)

Thanks Colin,

Mind to also change pm80xx_hw.c?

-- 
Jack Wang
Linux Kernel Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Tel:   +49 30 577 008  042
Fax:  +49 30 577 008 299
Email:jinpu.w...@profitbricks.com
URL:  https://www.profitbricks.de

Sitz der Gesellschaft: Berlin
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B
Geschäftsführer: Achim Weiss

Re: [PATCH] scsi: pm8001: build in relevant functions and code on PM8001_USE_MSIX

[Jinpu Wang]

On Thu, Mar 23, 2017 at 9:34 AM, walter harms  wrote:

>> @@ -4613,15 +4616,15 @@ static int pm8001_chip_phy_ctl_req(struct 
>> pm8001_hba_info *pm8001_ha,
>>
>>  static u32 pm8001_chip_is_our_interupt(struct pm8001_hba_info *pm8001_ha)
>>  {
>> - u32 value;
>>  #ifdef PM8001_USE_MSIX
>>   return 1;
>> -#endif
>> - value = pm8001_cr32(pm8001_ha, 0, MSGU_ODR);
>> +#else
>> + u32 value = pm8001_cr32(pm8001_ha, 0, MSGU_ODR);
>> +
>>   if (value)
>>   return 1;
>>   return 0;
>> -
>> +#endif
>>  }
>>
>
> This is a bit strange, why do this function return u32 ?
>
> re,
>  wh
>
>>  /**

Hi Walter,

Feel free to submit a patch to change it to bool.

Thanks

-- 
Jinpu Wang
Linux Kernel Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Tel:   +49 30 577 008  042
Fax:  +49 30 577 008 299
Email:jinpu.w...@profitbricks.com
URL:  https://www.profitbricks.de

Sitz der Gesellschaft: Berlin
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B
Geschäftsführer: Achim Weiss

Re: [PATCH] scsi: pm8001: build in relevant functions and code on PM8001_USE_MSIX

[walter harms]

Am 22.03.2017 19:50, schrieb Colin King:
> From: Colin Ian King 
> 
> Currently the misx and intx variables of the interrupt enable/disable
> helper functions are built in no matter what the setting of the
> macro PM8001_USE_MSIX.  Clean this up by just building in the
> necessary helper functions and calls to these functions depending on
> the setting of PM8001_USE_MSIX.  This addresses several dead code
> paths found by static analysis with CoverityScan.
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/scsi/pm8001/pm8001_hwi.c | 63 
> +---
>  1 file changed, 33 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c 
> b/drivers/scsi/pm8001/pm8001_hwi.c
> index 10546faac58c..d1be10fd1350 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.c
> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
> @@ -1204,26 +1204,7 @@ void pm8001_chip_iounmap(struct pm8001_hba_info 
> *pm8001_ha)
>   }
>  }
>  
> -/**
> - * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
> - * @pm8001_ha: our hba card information
> - */
> -static void
> -pm8001_chip_intx_interrupt_enable(struct pm8001_hba_info *pm8001_ha)
> -{
> - pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_CLEAR_ALL);
> - pm8001_cw32(pm8001_ha, 0, MSGU_ODCR, ODCR_CLEAR_ALL);
> -}
> -
> - /**
> -  * pm8001_chip_intx_interrupt_disable- disable PM8001 chip interrupt
> -  * @pm8001_ha: our hba card information
> -  */
> -static void
> -pm8001_chip_intx_interrupt_disable(struct pm8001_hba_info *pm8001_ha)
> -{
> - pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_MASK_ALL);
> -}
> +#ifdef PM8001_USE_MSIX
>  
>  /**
>   * pm8001_chip_msix_interrupt_enable - enable PM8001 chip interrupt
> @@ -1257,6 +1238,30 @@ pm8001_chip_msix_interrupt_disable(struct 
> pm8001_hba_info *pm8001_ha,
>   pm8001_cw32(pm8001_ha, 0,  msi_index, MSIX_INTERRUPT_DISABLE);
>  }
>  
> +#else
> +
> +/**
> + * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
> + * @pm8001_ha: our hba card information
> + */
> +static void
> +pm8001_chip_intx_interrupt_enable(struct pm8001_hba_info *pm8001_ha)
> +{
> + pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_CLEAR_ALL);
> + pm8001_cw32(pm8001_ha, 0, MSGU_ODCR, ODCR_CLEAR_ALL);
> +}
> +
> + /**
> +  * pm8001_chip_intx_interrupt_disable- disable PM8001 chip interrupt
> +  * @pm8001_ha: our hba card information
> +  */
> +static void
> +pm8001_chip_intx_interrupt_disable(struct pm8001_hba_info *pm8001_ha)
> +{
> + pm8001_cw32(pm8001_ha, 0, MSGU_ODMR, ODMR_MASK_ALL);
> +}
> +#endif
> +
>  /**
>   * pm8001_chip_interrupt_enable - enable PM8001 chip interrupt
>   * @pm8001_ha: our hba card information
> @@ -1266,10 +1271,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 +1285,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
>  }
>  
>  /**
> @@ -4613,15 +4616,15 @@ static int pm8001_chip_phy_ctl_req(struct 
> pm8001_hba_info *pm8001_ha,
>  
>  static u32 pm8001_chip_is_our_interupt(struct pm8001_hba_info *pm8001_ha)
>  {
> - u32 value;
>  #ifdef PM8001_USE_MSIX
>   return 1;
> -#endif
> - value = pm8001_cr32(pm8001_ha, 0, MSGU_ODR);
> +#else
> + u32 value = pm8001_cr32(pm8001_ha, 0, MSGU_ODR);
> +
>   if (value)
>   return 1;
>   return 0;
> -
> +#endif
>  }
>  

This is a bit strange, why do this function return u32 ?

re,
 wh

>  /**