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

Groups > linux.kernel > #1610409 > unrolled thread

Re: [PATCH V6 4/7] mfd: da9061: MFD core support

Started byLee Jones <lee.jones@linaro.org>
First post2017-03-28 10:30 +0200
Last post2017-03-29 11:20 +0200
Articles 7 — 3 participants

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

  Re: [PATCH V6 4/7] mfd: da9061: MFD core support Lee Jones <lee.jones@linaro.org> - 2017-03-28 10:30 +0200
    Re: [PATCH V6 4/7] mfd: da9061: MFD core support Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-28 10:40 +0200
      RE: [PATCH V6 4/7] mfd: da9061: MFD core support Steve Twiss <stwiss.opensource@diasemi.com> - 2017-03-28 12:50 +0200
        Re: [PATCH V6 4/7] mfd: da9061: MFD core support Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-28 13:00 +0200
          RE: [PATCH V6 4/7] mfd: da9061: MFD core support Steve Twiss <stwiss.opensource@diasemi.com> - 2017-03-28 13:40 +0200
          Re: [PATCH V6 4/7] mfd: da9061: MFD core support Lee Jones <lee.jones@linaro.org> - 2017-03-29 10:40 +0200
            Re: [PATCH V6 4/7] mfd: da9061: MFD core support Geert Uytterhoeven <geert@linux-m68k.org> - 2017-03-29 11:20 +0200

#1610409 — Re: [PATCH V6 4/7] mfd: da9061: MFD core support

FromLee Jones <lee.jones@linaro.org>
Date2017-03-28 10:30 +0200
SubjectRe: [PATCH V6 4/7] mfd: da9061: MFD core support
Message-ID<tpUFP-4R5-9@gated-at.bofh.it>
On Mon, 27 Mar 2017, kbuild test robot wrote:

> Hi Steve,
> 
> [auto build test WARNING on ljones-mfd/for-mfd-next]
> [also build test WARNING on v4.11-rc4 next-20170327]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Steve-Twiss/Documentation-devicetree-watchdog-da9062-61-watchdog-timer-binding/20170327-215359
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
> config: x86_64-randconfig-x009-201713 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All warnings (new ones prefixed by >>):
> 
>    drivers//mfd/da9062-core.c: In function 'da9062_i2c_probe':
> >> drivers//mfd/da9062-core.c:845:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>       chip->chip_type = (int)match->data;
>                         ^

Please use longs or enums.

> vim +845 drivers//mfd/da9062-core.c
> 
>    829		unsigned int irq_base;
>    830		const struct mfd_cell *cell;
>    831		const struct regmap_irq_chip *irq_chip;
>    832		const struct regmap_config *config;
>    833		int cell_num;
>    834		int ret;
>    835	
>    836		chip = devm_kzalloc(&i2c->dev, sizeof(*chip), GFP_KERNEL);
>    837		if (!chip)
>    838			return -ENOMEM;
>    839	
>    840		if (i2c->dev.of_node) {
>    841			match = of_match_node(da9062_dt_ids, i2c->dev.of_node);
>    842			if (!match)
>    843				return -EINVAL;
>    844	
>  > 845			chip->chip_type = (int)match->data;
>    846		} else {
>    847			chip->chip_type = id->driver_data;
>    848		}
>    849	
>    850		i2c_set_clientdata(i2c, chip);
>    851		chip->dev = &i2c->dev;
>    852	
>    853		if (!i2c->irq) {
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation



-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[toc] | [next] | [standalone]

#1610412

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2017-03-28 10:40 +0200
Message-ID<tpUPw-4W7-9@gated-at.bofh.it>
In reply to#1610409
Hi Lee,

On Tue, Mar 28, 2017 at 10:21 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> [auto build test WARNING on ljones-mfd/for-mfd-next]
>> [also build test WARNING on v4.11-rc4 next-20170327]
>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>>
>> url:    https://github.com/0day-ci/linux/commits/Steve-Twiss/Documentation-devicetree-watchdog-da9062-61-watchdog-timer-binding/20170327-215359
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
>> config: x86_64-randconfig-x009-201713 (attached as .config)
>> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>> reproduce:
>>         # save the attached .config to linux build tree
>>         make ARCH=x86_64
>>
>> All warnings (new ones prefixed by >>):
>>
>>    drivers//mfd/da9062-core.c: In function 'da9062_i2c_probe':
>> >> drivers//mfd/da9062-core.c:845:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>>       chip->chip_type = (int)match->data;
>>                        ^
>
> Please use longs or enums.

Enums would still give a warning on 64-bit.
The simple fix is change the cast from (int) to (uintptr_t).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

#1610553

FromSteve Twiss <stwiss.opensource@diasemi.com>
Date2017-03-28 12:50 +0200
Message-ID<tpWRk-6j4-3@gated-at.bofh.it>
In reply to#1610412
On 28 March 2017 09:37, Geert Uytterhoeven wrote:
> Subject: Re: [PATCH V6 4/7] mfd: da9061: MFD core support
> 
> Hi Lee,
> 
> On Tue, Mar 28, 2017 at 10:21 AM, Lee Jones <lee.jones@linaro.org> wrote:
> >> [auto build test WARNING on ljones-mfd/for-mfd-next]
> >> [also build test WARNING on v4.11-rc4 next-20170327]
> >> base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
> >> config: x86_64-randconfig-x009-201713 (attached as .config)
> >> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> >> reproduce:
> >>         # save the attached .config to linux build tree
> >>         make ARCH=x86_64
> >>
> >> All warnings (new ones prefixed by >>):
> >>
> >>    drivers//mfd/da9062-core.c: In function 'da9062_i2c_probe':
> >> >> drivers//mfd/da9062-core.c:845:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> >>       chip->chip_type = (int)match->data;
> >>                        ^
> >
> > Please use longs or enums.
> 
> Enums would still give a warning on 64-bit.
> The simple fix is change the cast from (int) to (uintptr_t).

Hi Lee and Geert,

How about this? Fix by redefining the enum chip_type to be an int.
Then, just use substitution:
#define COMPAT_TYPE_DA9061  1
#define COMPAT_TYPE_DA9062  2

That would be simple.
Are there any reasons this would not be acceptable?

Regards,
Steve

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

#1610565

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2017-03-28 13:00 +0200
Message-ID<tpX0Z-6nN-7@gated-at.bofh.it>
In reply to#1610553
Hi Steve,

On Tue, Mar 28, 2017 at 12:42 PM, Steve Twiss
<stwiss.opensource@diasemi.com> wrote:
> On 28 March 2017 09:37, Geert Uytterhoeven wrote:
>> Subject: Re: [PATCH V6 4/7] mfd: da9061: MFD core support
>> On Tue, Mar 28, 2017 at 10:21 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> [auto build test WARNING on ljones-mfd/for-mfd-next]
>> >> [also build test WARNING on v4.11-rc4 next-20170327]
>> >> base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
>> >> config: x86_64-randconfig-x009-201713 (attached as .config)
>> >> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>> >> reproduce:
>> >>         # save the attached .config to linux build tree
>> >>         make ARCH=x86_64
>> >>
>> >> All warnings (new ones prefixed by >>):
>> >>
>> >>    drivers//mfd/da9062-core.c: In function 'da9062_i2c_probe':
>> >> >> drivers//mfd/da9062-core.c:845:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>> >>       chip->chip_type = (int)match->data;
>> >>                        ^
>> >
>> > Please use longs or enums.
>>
>> Enums would still give a warning on 64-bit.
>> The simple fix is change the cast from (int) to (uintptr_t).
>
> Hi Lee and Geert,
>
> How about this? Fix by redefining the enum chip_type to be an int.
> Then, just use substitution:
> #define COMPAT_TYPE_DA9061  1
> #define COMPAT_TYPE_DA9062  2
>
> That would be simple.
> Are there any reasons this would not be acceptable?

I don't see how that can help.
The warning is caused by casting the "void *" (which is either 32-bit or
64-bit) in of_device_if.data to an integer or enum (which is always 32-bit).

The right fix is to cast it to uintptr_t intead of int, like other drivers do.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

#1610600

FromSteve Twiss <stwiss.opensource@diasemi.com>
Date2017-03-28 13:40 +0200
Message-ID<tpXDI-6Qq-9@gated-at.bofh.it>
In reply to#1610565
On 28 March 2017 11:51, Geert Uytterhoeven wrote:
> Hi Steve,
> 
> On Tue, Mar 28, 2017 at 12:42 PM, Steve Twiss wrote:
> > On 28 March 2017 09:37, Geert Uytterhoeven wrote:
> >> Subject: Re: [PATCH V6 4/7] mfd: da9061: MFD core support
> >> On Tue, Mar 28, 2017 at 10:21 AM, Lee Jones <lee.jones@linaro.org> wrote:
> >> >> [auto build test WARNING on ljones-mfd/for-mfd-next]
> >> >> [also build test WARNING on v4.11-rc4 next-20170327]
> >> >> base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
> >> >> config: x86_64-randconfig-x009-201713 (attached as .config)
> >> >> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> >> >> reproduce:
> >> >>         # save the attached .config to linux build tree
> >> >>         make ARCH=x86_64
> >> >>
> >> >> All warnings (new ones prefixed by >>):
> >> >>
> >> >>    drivers//mfd/da9062-core.c: In function 'da9062_i2c_probe':
> >> >> >> drivers//mfd/da9062-core.c:845:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> >> >>       chip->chip_type = (int)match->data;
> >> >>                        ^
> >> >
> >> > Please use longs or enums.
> >>
> >> Enums would still give a warning on 64-bit.
> >> The simple fix is change the cast from (int) to (uintptr_t).
> >
> > Hi Lee and Geert,
> >
> > How about this? Fix by redefining the enum chip_type to be an int.
> > Then, just use substitution:
> > #define COMPAT_TYPE_DA9061  1
> > #define COMPAT_TYPE_DA9062  2
> >
> > That would be simple.
> > Are there any reasons this would not be acceptable?
> 
> I don't see how that can help.
> The warning is caused by casting the "void *" (which is either 32-bit or
> 64-bit) in of_device_if.data to an integer or enum (which is always 32-bit).
> 
> The right fix is to cast it to uintptr_t intead of int, like other drivers do.

Thanks Geert,
Um. Of course. Thanks, and my apologies. It would have helped if I had compile tested the right file  :(
I'll follow the convention and cast (uintptr_t).
Thanks,
Steve

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

#1611688

FromLee Jones <lee.jones@linaro.org>
Date2017-03-29 10:40 +0200
Message-ID<tqhj3-4fY-5@gated-at.bofh.it>
In reply to#1610565
On Tue, 28 Mar 2017, Geert Uytterhoeven wrote:

> Hi Steve,
> 
> On Tue, Mar 28, 2017 at 12:42 PM, Steve Twiss
> <stwiss.opensource@diasemi.com> wrote:
> > On 28 March 2017 09:37, Geert Uytterhoeven wrote:
> >> Subject: Re: [PATCH V6 4/7] mfd: da9061: MFD core support
> >> On Tue, Mar 28, 2017 at 10:21 AM, Lee Jones <lee.jones@linaro.org> wrote:
> >> >> [auto build test WARNING on ljones-mfd/for-mfd-next]
> >> >> [also build test WARNING on v4.11-rc4 next-20170327]
> >> >> base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
> >> >> config: x86_64-randconfig-x009-201713 (attached as .config)
> >> >> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> >> >> reproduce:
> >> >>         # save the attached .config to linux build tree
> >> >>         make ARCH=x86_64
> >> >>
> >> >> All warnings (new ones prefixed by >>):
> >> >>
> >> >>    drivers//mfd/da9062-core.c: In function 'da9062_i2c_probe':
> >> >> >> drivers//mfd/da9062-core.c:845:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> >> >>       chip->chip_type = (int)match->data;
> >> >>                        ^
> >> >
> >> > Please use longs or enums.
> >>
> >> Enums would still give a warning on 64-bit.
> >> The simple fix is change the cast from (int) to (uintptr_t).
> >
> > Hi Lee and Geert,
> >
> > How about this? Fix by redefining the enum chip_type to be an int.
> > Then, just use substitution:
> > #define COMPAT_TYPE_DA9061  1
> > #define COMPAT_TYPE_DA9062  2
> >
> > That would be simple.
> > Are there any reasons this would not be acceptable?
> 
> I don't see how that can help.
> The warning is caused by casting the "void *" (which is either 32-bit or
> 64-bit) in of_device_if.data to an integer or enum (which is always 32-bit).
> 
> The right fix is to cast it to uintptr_t intead of int, like other drivers do.

That's a hack though, isn't it?  chip->chip_type is not of type
uintptr_t, so all you're doing here is making the compiler happy.

I see people using (int)(uintptr_t), which I guess keeps the
compiler happy in the first instance and actually culminates in a
correct cast to the right type.

So how about something like (enum da9062_compatible_types)(long)?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

#1611717

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2017-03-29 11:20 +0200
Message-ID<tqhVM-4OF-19@gated-at.bofh.it>
In reply to#1611688
Hi Lee,

On Wed, Mar 29, 2017 at 10:30 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 28 Mar 2017, Geert Uytterhoeven wrote:
>> On Tue, Mar 28, 2017 at 12:42 PM, Steve Twiss
>> <stwiss.opensource@diasemi.com> wrote:
>> > On 28 March 2017 09:37, Geert Uytterhoeven wrote:
>> >> Subject: Re: [PATCH V6 4/7] mfd: da9061: MFD core support
>> >> On Tue, Mar 28, 2017 at 10:21 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> >> [auto build test WARNING on ljones-mfd/for-mfd-next]
>> >> >> [also build test WARNING on v4.11-rc4 next-20170327]
>> >> >> base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
>> >> >> config: x86_64-randconfig-x009-201713 (attached as .config)
>> >> >> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>> >> >> reproduce:
>> >> >>         # save the attached .config to linux build tree
>> >> >>         make ARCH=x86_64
>> >> >>
>> >> >> All warnings (new ones prefixed by >>):
>> >> >>
>> >> >>    drivers//mfd/da9062-core.c: In function 'da9062_i2c_probe':
>> >> >> >> drivers//mfd/da9062-core.c:845:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>> >> >>       chip->chip_type = (int)match->data;
>> >> >>                        ^
>> >> >
>> >> > Please use longs or enums.
>> >>
>> >> Enums would still give a warning on 64-bit.
>> >> The simple fix is change the cast from (int) to (uintptr_t).
>> >
>> > Hi Lee and Geert,
>> >
>> > How about this? Fix by redefining the enum chip_type to be an int.
>> > Then, just use substitution:
>> > #define COMPAT_TYPE_DA9061  1
>> > #define COMPAT_TYPE_DA9062  2
>> >
>> > That would be simple.
>> > Are there any reasons this would not be acceptable?
>>
>> I don't see how that can help.
>> The warning is caused by casting the "void *" (which is either 32-bit or
>> 64-bit) in of_device_if.data to an integer or enum (which is always 32-bit).
>>
>> The right fix is to cast it to uintptr_t intead of int, like other drivers do.
>
> That's a hack though, isn't it?  chip->chip_type is not of type
> uintptr_t, so all you're doing here is making the compiler happy.

It's a hack, in the sense that using casts is usually a hack :-)
(I would love for C to have casts you can easily grep for, like C++ has)

In this case, it's not really a hack, as the void * is intended to store
arbitrary data, both pointers and integers.

> I see people using (int)(uintptr_t), which I guess keeps the
> compiler happy in the first instance and actually culminates in a
> correct cast to the right type.

The cast to int isn't really needed.

> So how about something like (enum da9062_compatible_types)(long)?

"uintptr_t" is the proper C standard type for conversion between pointers and
integers.  The cast to the enum isn't really needed, so I'd remove it (the
less number of casts, the better!).

Long is the hack that works on ILP32 and LP64 platforms only ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web