Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1262901 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 3/4] scsi: fix compiler warning for sg Sinan Kaya <okaya@codeaurora.org> - 2015-11-05 05:50 +0100
    Re: [PATCH 3/4] scsi: fix compiler warning for sg kbuild test robot <lkp@intel.com> - 2015-11-05 06:50 +0100
    Re: [PATCH 3/4] scsi: fix compiler warning for sg kbuild test robot <lkp@intel.com> - 2015-11-05 08:00 +0100
    Re: [PATCH 3/4] scsi: fix compiler warning for sg kbuild test robot <lkp@intel.com> - 2015-11-05 08:10 +0100
    Re: [PATCH 3/4] scsi: fix compiler warning for sg Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-11-05 09:50 +0100
      Re: [PATCH 3/4] scsi: fix compiler warning for sg Sinan Kaya <okaya@codeaurora.org> - 2015-11-05 16:20 +0100
        Re: [PATCH 3/4] scsi: fix compiler warning for sg Timur Tabi <timur@codeaurora.org> - 2015-11-05 16:30 +0100
        Re: [PATCH 3/4] scsi: fix compiler warning for sg Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-11-05 19:10 +0100
          Re: [PATCH 3/4] scsi: fix compiler warning for sg Sinan Kaya <okaya@codeaurora.org> - 2015-11-05 19:40 +0100
            Re: [PATCH 3/4] scsi: fix compiler warning for sg Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-11-05 20:40 +0100
              Re: [PATCH 3/4] scsi: fix compiler warning for sg Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-11-05 21:00 +0100
                Re: [PATCH 3/4] scsi: fix compiler warning for sg Sinan Kaya <okaya@codeaurora.org> - 2015-11-05 21:20 +0100
                Re: [PATCH 3/4] scsi: fix compiler warning for sg Sinan Kaya <okaya@codeaurora.org> - 2015-11-09 02:20 +0100

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1262943

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1262963

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1262969

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1263016

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263320

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263329

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263462

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263476

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263506

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263521

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263531

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1265267

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web