[PATCH 1/4] scsi: mpt2sas: try 64 bit DMA when 32 bit DMA fails

[PATCH 1/4] scsi: mpt2sas: try 64 bit DMA when 32 bit DMA fails

[Sinan Kaya]

Current code gives up when 32 bit DMA is not supported.
This patch tests 64 bit support before bailing out in
such conditions.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/scsi/mpt2sas/mpt2sas_base.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c
index c167911..c61c82a 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -1217,8 +1217,27 @@ _base_config_dma_addressing(struct MPT2SAS_ADAPTER *ioc, struct pci_dev *pdev)
 		ioc->base_add_sg_single = &_base_add_sg_single_32;
 		ioc->sge_size = sizeof(Mpi2SGESimple32_t);
 		ioc->dma_mask = 32;
-	} else
+	} else {
+		/* Try 64 bit, 32 bit failed */
+		consistent_dma_mask = DMA_BIT_MASK(64);
+
+		if (sizeof(dma_addr_t) > 4) {
+			const uint64_t required_mask =
+				dma_get_required_mask(&pdev->dev);
+			if ((required_mask > DMA_BIT_MASK(32)) &&
+				!pci_set_dma_mask(pdev, DMA_BIT_MASK(64)) &&
+				!pci_set_consistent_dma_mask(pdev,
+							consistent_dma_mask)) {
+				ioc->base_add_sg_single =
+					&_base_add_sg_single_64;
+				ioc->sge_size = sizeof(Mpi2SGESimple64_t);
+				ioc->dma_mask = 64;
+				goto out;
+			}
+		}
+
 		return -ENODEV;
+	}
 
  out:
 	si_meminfo(&s);
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCH 2/4] scsi: mpt3sas: try 64 bit DMA when 32 bit DMA fails

[Sinan Kaya]

Current code gives up when 32 bit DMA is not supported.
This patch tests 64 bit support before bailing out in
such conditions.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index d4f1dcd..6dc391c 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1535,9 +1535,29 @@ _base_config_dma_addressing(struct MPT3SAS_ADAPTER *ioc, struct pci_dev *pdev)
 		ioc->base_add_sg_single = &_base_add_sg_single_32;
 		ioc->sge_size = sizeof(Mpi2SGESimple32_t);
 		ioc->dma_mask = 32;
-	} else
+	} else {
+		/* Try 64 bit, 32 bit failed */
+		consistent_dma_mask = DMA_BIT_MASK(64);
+		if (sizeof(dma_addr_t) > 4) {
+			const uint64_t required_mask =
+				dma_get_required_mask(&pdev->dev);
+			int consistent_mask =
+				pci_set_consistent_dma_mask(pdev,
+							consistent_dma_mask);
+
+			if ((required_mask > DMA_BIT_MASK(32)) &&
+				!pci_set_dma_mask(pdev, DMA_BIT_MASK(64)) &&
+				!consistent_mask) {
+				ioc->base_add_sg_single =
+					&_base_add_sg_single_64;
+				ioc->sge_size = sizeof(Mpi2SGESimple64_t);
+				ioc->dma_mask = 64;
+				goto out;
+			}
+		}
 		return -ENODEV;
 
+	}
  out:
 	si_meminfo(&s);
 	pr_info(MPT3SAS_FMT
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCH 3/4] scsi: fix compiler warning for sg

[Sinan Kaya]

The MULDIV macro has been designed for small
numbers. It emits an overflow warning on 64 bit
systems. This patch places type casts in the
parameters to fix the compiler warning.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/scsi/sg.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 9d7b7db..eb2739d 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -88,7 +88,10 @@ static void sg_proc_cleanup(void);
  * Of course an overflow is inavoidable if the result of muldiv doesn't fit
  * in 32 bits.
  */
-#define MULDIV(X,MUL,DIV) ((((X % DIV) * MUL) / DIV) + ((X / DIV) * MUL))
+static inline u64 MULDIV(u64 X, u32 MUL, u32 DIV)
+{
+	return ((((X % DIV) * MUL) / DIV) + ((X / DIV) * MUL));
+}
 
 #define SG_DEFAULT_TIMEOUT MULDIV(SG_DEFAULT_TIMEOUT_USER, HZ, USER_HZ)
 
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

[PATCH 4/4] scsi: mptxsas: offload IRQ execution

[Sinan Kaya]

The mpt2sas and mpt3sas drivers are spinning
forever in their IRQ handlers if there is a lot
of job queued up by the PCIe card. This handler is
causing spikes for the rest of the system and
sluggish behavior.

Marking all MSI interrupts as non-shared and
moving the MSI interrupts to thread context.
This relexes the rest of the system execution.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/scsi/mpt2sas/mpt2sas_base.c | 13 +++++++++----
 drivers/scsi/mpt3sas/mpt3sas_base.c | 14 ++++++++++----
 2 files changed, 19 insertions(+), 8 deletions(-)
 mode change 100644 => 100755 drivers/scsi/mpt3sas/mpt3sas_base.c

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c
index c61c82a..ee2aead 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -1359,14 +1359,19 @@ _base_request_irq(struct MPT2SAS_ADAPTER *ioc, u8 index, u32 vector)
 	cpumask_clear(reply_q->affinity_hint);
 
 	atomic_set(&reply_q->busy, 0);
-	if (ioc->msix_enable)
+	if (ioc->msix_enable) {
 		snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d-msix%d",
 		    MPT2SAS_DRIVER_NAME, ioc->id, index);
-	else
+		r = request_threaded_irq(vector, NULL, _base_interrupt,
+					 IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+					 reply_q->name, reply_q);
+	}
+	else {
 		snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d",
 		    MPT2SAS_DRIVER_NAME, ioc->id);
-	r = request_irq(vector, _base_interrupt, IRQF_SHARED, reply_q->name,
-	    reply_q);
+		r = request_irq(vector, _base_interrupt, IRQF_SHARED,
+				reply_q->name, reply_q);
+	}
 	if (r) {
 		printk(MPT2SAS_ERR_FMT "unable to allocate interrupt %d!\n",
 		    reply_q->name, vector);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
old mode 100644
new mode 100755
index 6dc391c..c29f359
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1661,14 +1661,20 @@ _base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index, u32 vector)
 	cpumask_clear(reply_q->affinity_hint);
 
 	atomic_set(&reply_q->busy, 0);
-	if (ioc->msix_enable)
+	if (ioc->msix_enable) {
 		snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d-msix%d",
 		    MPT3SAS_DRIVER_NAME, ioc->id, index);
-	else
+
+		r = request_threaded_irq(vector, NULL, _base_interrupt,
+					 IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+					 reply_q->name, reply_q);
+	}
+	else {
 		snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d",
 		    MPT3SAS_DRIVER_NAME, ioc->id);
-	r = request_irq(vector, _base_interrupt, IRQF_SHARED, reply_q->name,
-	    reply_q);
+		r = request_irq(vector, _base_interrupt, IRQF_SHARED,
+				reply_q->name, reply_q);
+	}
 	if (r) {
 		pr_err(MPT3SAS_FMT "unable to allocate interrupt %d!\n",
 		    reply_q->name, vector);
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

Re: [PATCH 3/4] scsi: fix compiler warning for sg

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 873 bytes --]

Hi Sinan,

[auto build test ERROR on: scsi/for-next]
[also build test ERROR on: v4.3 next-20151104]

url:    https://github.com/0day-ci/linux/commits/Sinan-Kaya/scsi-mpt2sas-try-64-bit-DMA-when-32-bit-DMA-fails/20151105-125248
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: i386-defconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `sg_ioctl':
>> sg.c:(.text+0x1b680b): undefined reference to `__umoddi3'
>> sg.c:(.text+0x1b6829): undefined reference to `__udivdi3'
   sg.c:(.text+0x1b6849): undefined reference to `__udivdi3'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 23768 bytes --]

Re: [PATCH 3/4] scsi: fix compiler warning for sg

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

Hi Sinan,

[auto build test ERROR on: scsi/for-next]
[also build test ERROR on: v4.3 next-20151105]

url:    https://github.com/0day-ci/linux/commits/Sinan-Kaya/scsi-mpt2sas-try-64-bit-DMA-when-32-bit-DMA-fails/20151105-125248
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: arm-mv78xx0_defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> ERROR: "__aeabi_uldivmod" [drivers/scsi/sg.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 16351 bytes --]

Re: [PATCH 3/4] scsi: fix compiler warning for sg

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]

Hi Sinan,

[auto build test ERROR on: scsi/for-next]
[also build test ERROR on: v4.3 next-20151105]

url:    https://github.com/0day-ci/linux/commits/Sinan-Kaya/scsi-mpt2sas-try-64-bit-DMA-when-32-bit-DMA-fails/20151105-125248
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: powerpc-mpc8610_hpcd_defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `sg_ioctl':
>> drivers/scsi/sg.c:897: undefined reference to `__umoddi3'
>> drivers/scsi/sg.c:897: undefined reference to `__udivdi3'
>> drivers/scsi/sg.c:897: undefined reference to `__udivdi3'

vim +897 drivers/scsi/sg.c

^1da177e Linus Torvalds  2005-04-16  891  		return sfp->timeout_user;
^1da177e Linus Torvalds  2005-04-16  892  	case SG_SET_FORCE_LOW_DMA:
^1da177e Linus Torvalds  2005-04-16  893  		result = get_user(val, ip);
^1da177e Linus Torvalds  2005-04-16  894  		if (result)
^1da177e Linus Torvalds  2005-04-16  895  			return result;
^1da177e Linus Torvalds  2005-04-16  896  		if (val) {
^1da177e Linus Torvalds  2005-04-16 @897  			sfp->low_dma = 1;
^1da177e Linus Torvalds  2005-04-16  898  			if ((0 == sfp->low_dma) && (0 == sg_res_in_use(sfp))) {
^1da177e Linus Torvalds  2005-04-16  899  				val = (int) sfp->reserve.bufflen;
95e159d6 Hannes Reinecke 2014-06-25  900  				sg_remove_scat(sfp, &sfp->reserve);

:::::: The code at line 897 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 16088 bytes --]

Re: [PATCH 3/4] scsi: fix compiler warning for sg

[Andy Shevchenko]

On Thu, Nov 5, 2015 at 6:46 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
> The MULDIV macro has been designed for small
> numbers. It emits an overflow warning on 64 bit
> systems. This patch places type casts in the
> parameters to fix the compiler warning.
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  drivers/scsi/sg.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 9d7b7db..eb2739d 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -88,7 +88,10 @@ static void sg_proc_cleanup(void);
>   * Of course an overflow is inavoidable if the result of muldiv doesn't fit
>   * in 32 bits.
>   */
> -#define MULDIV(X,MUL,DIV) ((((X % DIV) * MUL) / DIV) + ((X / DIV) * MUL))
> +static inline u64 MULDIV(u64 X, u32 MUL, u32 DIV)
> +{
> +       return ((((X % DIV) * MUL) / DIV) + ((X / DIV) * MUL));
> +}

Like kbuild bot already told you it would be nice to think of 32-bit
architectures.

Moreover we have mult_frac() macro already for 32-bit numbers.

For 64 bit numbers you need to do do_div().

Like:

static inline u64 mult_frac64(u64 x, u32 m, u32 n)
{
u64 ret;

ret = do_div(x, n);
return ret * m;
}


>
>  #define SG_DEFAULT_TIMEOUT MULDIV(SG_DEFAULT_TIMEOUT_USER, HZ, USER_HZ)
>
> --
> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/4] scsi: fix compiler warning for sg

[Sinan Kaya]

On 11/5/2015 3:48 AM, Andy Shevchenko wrote:
> On Thu, Nov 5, 2015 at 6:46 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
>> The MULDIV macro has been designed for small
>> numbers. It emits an overflow warning on 64 bit
>> systems. This patch places type casts in the
>> parameters to fix the compiler warning.
>>
>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>> ---
>>   drivers/scsi/sg.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
>> index 9d7b7db..eb2739d 100644
>> --- a/drivers/scsi/sg.c
>> +++ b/drivers/scsi/sg.c
>> @@ -88,7 +88,10 @@ static void sg_proc_cleanup(void);
>>    * Of course an overflow is inavoidable if the result of muldiv doesn't fit
>>    * in 32 bits.
>>    */
>> -#define MULDIV(X,MUL,DIV) ((((X % DIV) * MUL) / DIV) + ((X / DIV) * MUL))
>> +static inline u64 MULDIV(u64 X, u32 MUL, u32 DIV)
>> +{
>> +       return ((((X % DIV) * MUL) / DIV) + ((X / DIV) * MUL));
>> +}
>
> Like kbuild bot already told you it would be nice to think of 32-bit
> architectures.
>
> Moreover we have mult_frac() macro already for 32-bit numbers.
>
> For 64 bit numbers you need to do do_div().
>
> Like:
>
> static inline u64 mult_frac64(u64 x, u32 m, u32 n)
> {
> u64 ret;
>
> ret = do_div(x, n);
> return ret * m;
> }
>

OK, I didn't know that we had such a macro. To make this look like the 
other macro, I can do this.

static inline u64 mult_frac64(u64 x, u32 numer, u32 denom)
{
	u64 quot;
	u64 rem  = x % denom;
	u64 rem2;

	quot = x;
	do_div(quot, denom);

	rem2 = rem * numer;
	do_div(rem2, denom);

	return (quot * numer) + rem2;
}

#define MULDIV(X,MUL,DIV)	mult_frac64(X, MUL, DIV)

>
>>
>>   #define SG_DEFAULT_TIMEOUT MULDIV(SG_DEFAULT_TIMEOUT_USER, HZ, USER_HZ)
>>
>> --
>> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>

-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

Re: [PATCH 3/4] scsi: fix compiler warning for sg

[Timur Tabi]

Sinan Kaya wrote:
 >
 >
 > #define MULDIV(X,MUL,DIV)    mult_frac64(X, MUL, DIV)

Why bother with the macro at all? Just change the code to use do_div() 
directly. It's possible that the original code was written before 
do_div() became standard, or the developer didn't know about, which is 
why we have this macro in the first place.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.

Re: [PATCH 3/4] scsi: fix compiler warning for sg

[Andy Shevchenko]

On Thu, Nov 5, 2015 at 5:10 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
>
>
> On 11/5/2015 3:48 AM, Andy Shevchenko wrote:
>>
>> On Thu, Nov 5, 2015 at 6:46 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
>>>
>>> The MULDIV macro has been designed for small
>>> numbers. It emits an overflow warning on 64 bit
>>> systems. This patch places type casts in the
>>> parameters to fix the compiler warning.
>>>
>>> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
>>> ---
>>>   drivers/scsi/sg.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
>>> index 9d7b7db..eb2739d 100644
>>> --- a/drivers/scsi/sg.c
>>> +++ b/drivers/scsi/sg.c
>>> @@ -88,7 +88,10 @@ static void sg_proc_cleanup(void);
>>>    * Of course an overflow is inavoidable if the result of muldiv doesn't
>>> fit
>>>    * in 32 bits.
>>>    */
>>> -#define MULDIV(X,MUL,DIV) ((((X % DIV) * MUL) / DIV) + ((X / DIV) *
>>> MUL))
>>> +static inline u64 MULDIV(u64 X, u32 MUL, u32 DIV)
>>> +{
>>> +       return ((((X % DIV) * MUL) / DIV) + ((X / DIV) * MUL));
>>> +}
>>
>>
>> Like kbuild bot already told you it would be nice to think of 32-bit
>> architectures.
>>
>> Moreover we have mult_frac() macro already for 32-bit numbers.
>>
>> For 64 bit numbers you need to do do_div().
>>
>> Like:
>>
>> static inline u64 mult_frac64(u64 x, u32 m, u32 n)
>> {
>> u64 ret;
>>
>> ret = do_div(x, n);
>> return ret * m;
>> }
>>
>
> OK, I didn't know that we had such a macro. To make this look like the other
> macro, I can do this.
>
> static inline u64 mult_frac64(u64 x, u32 numer, u32 denom)
> {
>         u64 quot;
>         u64 rem  = x % denom;
>         u64 rem2;
>
>         quot = x;
>         do_div(quot, denom);
>
>         rem2 = rem * numer;
>         do_div(rem2, denom);
>
>         return (quot * numer) + rem2;
> }

Might be I did a wrong smaple, but do_div() returns two values actually.
You perhaps overlooked it and thus wrote something redundant above.

>
> #define MULDIV(X,MUL,DIV)       mult_frac64(X, MUL, DIV)
>
>>
>>>
>>>   #define SG_DEFAULT_TIMEOUT MULDIV(SG_DEFAULT_TIMEOUT_USER, HZ, USER_HZ)
>>>
>>> --
>>> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
>>> Linux Foundation Collaborative Project
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>>
>
> --
> Sinan Kaya
> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/4] scsi: fix compiler warning for sg

[Sinan Kaya]

On 11/5/2015 1:07 PM, Andy Shevchenko wrote:
>> OK, I didn't know that we had such a macro. To make this look like the other
>> >macro, I can do this.
>> >
>> >static inline u64 mult_frac64(u64 x, u32 numer, u32 denom)
>> >{
>> >         u64 quot;
>> >         u64 rem  = x % denom;
>> >         u64 rem2;
>> >
>> >         quot = x;
>> >         do_div(quot, denom);
>> >
>> >         rem2 = rem * numer;
>> >         do_div(rem2, denom);
>> >
>> >         return (quot * numer) + rem2;
>> >}
> Might be I did a wrong smaple, but do_div() returns two values actually.
> You perhaps overlooked it and thus wrote something redundant above.
>

OK, I was looking at example usages in the kernel. The ones I looked 
always used the first argument as an input & output parameter. I got 
nervous about overwriting something.

void __ndelay(unsigned long long nsecs)
{
	u64 end;

	nsecs <<= 9;
	do_div(nsecs, 125);
...
}

Let's try again.

static inline u64 mult_frac64(u64 x, u32 numer, u32 denom) {
	u64 rem  = x % denom;
	u64 quot = do_div(x, denom);
	u64 mul = rem * numer;
	
	return (quot * numer) + do_div(mul, denom);
}

I'll do a s/MULDIV/mult_frac64/g to address Timur's concern.

-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

Re: [PATCH 3/4] scsi: fix compiler warning for sg

[Andy Shevchenko]

On Thu, Nov 5, 2015 at 8:32 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
> On 11/5/2015 1:07 PM, Andy Shevchenko wrote:

> Let's try again.
>
> static inline u64 mult_frac64(u64 x, u32 numer, u32 denom) {
>         u64 rem  = x % denom;
>         u64 quot = do_div(x, denom);
>         u64 mul = rem * numer;
>
>         return (quot * numer) + do_div(mul, denom);
> }

First of all why not to put this to generic header? We have math64.h
and kernel.h.
Might be a good idea (needs to check current users) to move mult_frac
to math64.h.

Then, x % y is already a problem. After all, you seems messed quot and
remainder.

What about something like

#if BITS_PER_LONG == 64

#define mult_frac64(x,n,d)  mult_frac(x,n,d)

#else

static inline u64 mult_frac64(u64 x, u32 numer, u32 denom) {
        u64 r1 = do_div(x, denom);
        u64 r2 = r1 * numer;

        do_div(r2, denom);
        return (x * numer) + r2;
}

#endif

?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/4] scsi: fix compiler warning for sg

[Andy Shevchenko]

On Thu, Nov 5, 2015 at 9:31 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Nov 5, 2015 at 8:32 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
>> On 11/5/2015 1:07 PM, Andy Shevchenko wrote:
>
>> Let's try again.
>>
>> static inline u64 mult_frac64(u64 x, u32 numer, u32 denom) {
>>         u64 rem  = x % denom;
>>         u64 quot = do_div(x, denom);
>>         u64 mul = rem * numer;
>>
>>         return (quot * numer) + do_div(mul, denom);
>> }
>
> First of all why not to put this to generic header? We have math64.h
> and kernel.h.
> Might be a good idea (needs to check current users) to move mult_frac
> to math64.h.
>
> Then, x % y is already a problem. After all, you seems messed quot and
> remainder.
>
> What about something like
>
> #if BITS_PER_LONG == 64
>
> #define mult_frac64(x,n,d)  mult_frac(x,n,d)
>
> #else
>
> static inline u64 mult_frac64(u64 x, u32 numer, u32 denom) {
>         u64 r1 = do_div(x, denom);
>         u64 r2 = r1 * numer;
>
>         do_div(r2, denom);
>         return (x * numer) + r2;
> }
>
> #endif
>
> ?

One more look to the users of MULDIV.

They all seems 32 bit for x.
It means you don't need two do_div()s at all.

Just do something like:

u64 d = x * numer;
do_div(d, denom);
return d;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/4] scsi: fix compiler warning for sg

[Sinan Kaya]

On 11/5/2015 2:56 PM, Andy Shevchenko wrote:
> One more look to the users of MULDIV.
>
> They all seems 32 bit for x.
> It means you don't need two do_div()s at all.
>
> Just do something like:
>
> u64 d = x * numer;
> do_div(d, denom);
> return d;

OK. I assume you want a change only in this file.

-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

Re: [PATCH 3/4] scsi: fix compiler warning for sg

[Sinan Kaya]

On 11/5/2015 2:56 PM, Andy Shevchenko wrote:
> On Thu, Nov 5, 2015 at 9:31 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Thu, Nov 5, 2015 at 8:32 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
>>> On 11/5/2015 1:07 PM, Andy Shevchenko wrote:
>>
>>> Let's try again.
>>>
>>> static inline u64 mult_frac64(u64 x, u32 numer, u32 denom) {
>>>          u64 rem  = x % denom;
>>>          u64 quot = do_div(x, denom);
>>>          u64 mul = rem * numer;
>>>
>>>          return (quot * numer) + do_div(mul, denom);
>>> }
>>
>> First of all why not to put this to generic header? We have math64.h
>> and kernel.h.
>> Might be a good idea (needs to check current users) to move mult_frac
>> to math64.h.
>>
>> Then, x % y is already a problem. After all, you seems messed quot and
>> remainder.
>>
>> What about something like
>>
>> #if BITS_PER_LONG == 64
>>
>> #define mult_frac64(x,n,d)  mult_frac(x,n,d)
>>
>> #else
>>
>> static inline u64 mult_frac64(u64 x, u32 numer, u32 denom) {
>>          u64 r1 = do_div(x, denom);
>>          u64 r2 = r1 * numer;
>>
>>          do_div(r2, denom);
>>          return (x * numer) + r2;
>> }

I'll use this instead. This is cleaner, scalable and functionally 
correct to the original code. I'll post a patch with this soon.

>>
>> #endif
>>
>> ?
>
> One more look to the users of MULDIV.
>
> They all seems 32 bit for x.
> It means you don't need two do_div()s at all.
>
> Just do something like:
>
> u64 d = x * numer;
> do_div(d, denom);
> return d;
>

-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project