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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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
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
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 <[email protected]>
:::::: CC: Linus Torvalds <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Thu, Nov 5, 2015 at 6:46 AM, Sinan Kaya <[email protected]> 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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
With Best Regards,
Andy Shevchenko
On 11/5/2015 3:48 AM, Andy Shevchenko wrote:
> On Thu, Nov 5, 2015 at 6:46 AM, Sinan Kaya <[email protected]> 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 <[email protected]>
>> ---
>> 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 [email protected]
>> 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
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.
On Thu, Nov 5, 2015 at 5:10 PM, Sinan Kaya <[email protected]> wrote:
>
>
> On 11/5/2015 3:48 AM, Andy Shevchenko wrote:
>>
>> On Thu, Nov 5, 2015 at 6:46 AM, Sinan Kaya <[email protected]> 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 <[email protected]>
>>> ---
>>> 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 [email protected]
>>> 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
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
On Thu, Nov 5, 2015 at 8:32 PM, Sinan Kaya <[email protected]> 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
On Thu, Nov 5, 2015 at 9:31 PM, Andy Shevchenko
<[email protected]> wrote:
> On Thu, Nov 5, 2015 at 8:32 PM, Sinan Kaya <[email protected]> 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
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
On 11/5/2015 2:56 PM, Andy Shevchenko wrote:
> On Thu, Nov 5, 2015 at 9:31 PM, Andy Shevchenko
> <[email protected]> wrote:
>> On Thu, Nov 5, 2015 at 8:32 PM, Sinan Kaya <[email protected]> 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