* linux-next: build warning after merge of the mfd tree @ 2017-11-30 0:35 Stephen Rothwell 2017-11-30 3:05 ` [PATCH] leds: pm8058: Make ledtype pointer sized type Bjorn Andersson ` (2 more replies) 0 siblings, 3 replies; 23+ messages in thread From: Stephen Rothwell @ 2017-11-30 0:35 UTC (permalink / raw) To: Lee Jones Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Linus Walleij, Jacek Anaszewski, Bjorn Andersson Hi Lee, After merging the mfd tree, today's linux-next build (x86_64 allmodconfig) produced this warning: drivers/leds/leds-pm8058.c: In function 'pm8058_led_probe': drivers/leds/leds-pm8058.c:109:17: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] led->ledtype = (u32)of_device_get_match_data(&pdev->dev); ^ Exposed by commit deed10c2d603 ("mfd: pm8xxx: Make elegible for COMPILE_TEST") CONFIG_LEDS_PM8058 depends on CONFIG_MFD_PM8XXX which is now turned on by allmodconfig. The actual problematic commit is: 7f866986e705 ("leds: add PM8058 LEDs driver") from v4.9-rc1. -- Cheers, Stephen Rothwell ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-11-30 0:35 linux-next: build warning after merge of the mfd tree Stephen Rothwell @ 2017-11-30 3:05 ` Bjorn Andersson 2017-11-30 8:29 ` Lee Jones ` (2 more replies) 2017-11-30 8:28 ` linux-next: build warning after merge of the mfd tree Lee Jones 2017-12-01 5:16 ` [PATCH v2] leds: pm8058: Silence pointer to integer size warning Bjorn Andersson 2 siblings, 3 replies; 23+ messages in thread From: Bjorn Andersson @ 2017-11-30 3:05 UTC (permalink / raw) To: Richard Purdie, Jacek Anaszewski, Pavel Machek Cc: linux-leds, linux-kernel, Linus Walleij, Lee Jones, Stephen Rothwell, Linux-Next Mailing List The pointer returned by of_device_get_match_data() doesn't have the same size as u32 on 64-bit architectures, causing issues when compile testing the driver on such platform. Make ledtype unsigned long instead, to solve this problem. Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") Cc: Linus Walleij <linus.walleij@linaro.org> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- drivers/leds/leds-pm8058.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c index a52674327857..cc2afe81720d 100644 --- a/drivers/leds/leds-pm8058.c +++ b/drivers/leds/leds-pm8058.c @@ -29,7 +29,7 @@ struct pm8058_led { struct regmap *map; u32 reg; - u32 ledtype; + unsigned long ledtype; struct led_classdev cdev; }; @@ -106,7 +106,7 @@ static int pm8058_led_probe(struct platform_device *pdev) if (!led) return -ENOMEM; - led->ledtype = (u32)of_device_get_match_data(&pdev->dev); + led->ledtype = (unsigned long)of_device_get_match_data(&pdev->dev); map = dev_get_regmap(pdev->dev.parent, NULL); if (!map) { -- 2.15.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-11-30 3:05 ` [PATCH] leds: pm8058: Make ledtype pointer sized type Bjorn Andersson @ 2017-11-30 8:29 ` Lee Jones 2017-11-30 8:31 ` Lee Jones 2017-11-30 9:18 ` Linus Walleij 2017-11-30 9:40 ` Pavel Machek 2 siblings, 1 reply; 23+ messages in thread From: Lee Jones @ 2017-11-30 8:29 UTC (permalink / raw) To: Bjorn Andersson Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, linux-leds, linux-kernel, Linus Walleij, Stephen Rothwell, Linux-Next Mailing List On Wed, 29 Nov 2017, Bjorn Andersson wrote: > The pointer returned by of_device_get_match_data() doesn't have the same > size as u32 on 64-bit architectures, causing issues when compile testing > the driver on such platform. Make ledtype unsigned long instead, to > solve this problem. > > Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") > Cc: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > drivers/leds/leds-pm8058.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Hi Bjorn, (Nice to see you) :) I'm going to apply this *before* Linus' fix. Applied, thanks. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-11-30 8:29 ` Lee Jones @ 2017-11-30 8:31 ` Lee Jones 2017-11-30 22:02 ` Jacek Anaszewski 0 siblings, 1 reply; 23+ messages in thread From: Lee Jones @ 2017-11-30 8:31 UTC (permalink / raw) To: Bjorn Andersson Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, linux-leds, linux-kernel, Linus Walleij, Stephen Rothwell, Linux-Next Mailing List On Thu, 30 Nov 2017, Lee Jones wrote: > On Wed, 29 Nov 2017, Bjorn Andersson wrote: > > > The pointer returned by of_device_get_match_data() doesn't have the same > > size as u32 on 64-bit architectures, causing issues when compile testing > > the driver on such platform. Make ledtype unsigned long instead, to > > solve this problem. > > > > Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") > > Cc: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > --- > > drivers/leds/leds-pm8058.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > Hi Bjorn, > > (Nice to see you) :) > > I'm going to apply this *before* Linus' fix. > > Applied, thanks. After I rx an Ack from Richard, Jacek or Pavel of course. :) Will send a pull-request. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-11-30 8:31 ` Lee Jones @ 2017-11-30 22:02 ` Jacek Anaszewski 2017-12-01 7:56 ` Lee Jones 0 siblings, 1 reply; 23+ messages in thread From: Jacek Anaszewski @ 2017-11-30 22:02 UTC (permalink / raw) To: Lee Jones, Bjorn Andersson Cc: Richard Purdie, Pavel Machek, linux-leds, linux-kernel, Linus Walleij, Stephen Rothwell, Linux-Next Mailing List On 11/30/2017 09:31 AM, Lee Jones wrote: > On Thu, 30 Nov 2017, Lee Jones wrote: > >> On Wed, 29 Nov 2017, Bjorn Andersson wrote: >> >>> The pointer returned by of_device_get_match_data() doesn't have the same >>> size as u32 on 64-bit architectures, causing issues when compile testing >>> the driver on such platform. Make ledtype unsigned long instead, to >>> solve this problem. >>> >>> Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") >>> Cc: Linus Walleij <linus.walleij@linaro.org> >>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >>> --- >>> drivers/leds/leds-pm8058.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> Hi Bjorn, >> >> (Nice to see you) :) >> >> I'm going to apply this *before* Linus' fix. >> >> Applied, thanks. > > After I rx an Ack from Richard, Jacek or Pavel of course. :) > > Will send a pull-request. Huh? This is for LED subsystem AFAICS. -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-11-30 22:02 ` Jacek Anaszewski @ 2017-12-01 7:56 ` Lee Jones 2017-12-01 20:35 ` Jacek Anaszewski 0 siblings, 1 reply; 23+ messages in thread From: Lee Jones @ 2017-12-01 7:56 UTC (permalink / raw) To: Jacek Anaszewski Cc: Bjorn Andersson, Richard Purdie, Pavel Machek, linux-leds, linux-kernel, Linus Walleij, Stephen Rothwell, Linux-Next Mailing List On Thu, 30 Nov 2017, Jacek Anaszewski wrote: > On 11/30/2017 09:31 AM, Lee Jones wrote: > > On Thu, 30 Nov 2017, Lee Jones wrote: > > > >> On Wed, 29 Nov 2017, Bjorn Andersson wrote: > >> > >>> The pointer returned by of_device_get_match_data() doesn't have the same > >>> size as u32 on 64-bit architectures, causing issues when compile testing > >>> the driver on such platform. Make ledtype unsigned long instead, to > >>> solve this problem. > >>> > >>> Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") > >>> Cc: Linus Walleij <linus.walleij@linaro.org> > >>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > >>> --- > >>> drivers/leds/leds-pm8058.c | 4 ++-- > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> Hi Bjorn, > >> > >> (Nice to see you) :) > >> > >> I'm going to apply this *before* Linus' fix. > >> > >> Applied, thanks. > > > > After I rx an Ack from Richard, Jacek or Pavel of course. :) > > > > Will send a pull-request. > > Huh? This is for LED subsystem AFAICS. Right, hence why I said I'd sent out a pull-request. The problem, however, arose due to a change in its parent driver's Kconfig entry, which has been applied to the MFD tree. We need to ensure this patch is applied *before* the other 'fix' to quash the warning before it starts, so to speak. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-12-01 7:56 ` Lee Jones @ 2017-12-01 20:35 ` Jacek Anaszewski 0 siblings, 0 replies; 23+ messages in thread From: Jacek Anaszewski @ 2017-12-01 20:35 UTC (permalink / raw) To: Lee Jones Cc: Bjorn Andersson, Richard Purdie, Pavel Machek, linux-leds, linux-kernel, Linus Walleij, Stephen Rothwell, Linux-Next Mailing List On 12/01/2017 08:56 AM, Lee Jones wrote: > On Thu, 30 Nov 2017, Jacek Anaszewski wrote: > >> On 11/30/2017 09:31 AM, Lee Jones wrote: >>> On Thu, 30 Nov 2017, Lee Jones wrote: >>> >>>> On Wed, 29 Nov 2017, Bjorn Andersson wrote: >>>> >>>>> The pointer returned by of_device_get_match_data() doesn't have the same >>>>> size as u32 on 64-bit architectures, causing issues when compile testing >>>>> the driver on such platform. Make ledtype unsigned long instead, to >>>>> solve this problem. >>>>> >>>>> Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") >>>>> Cc: Linus Walleij <linus.walleij@linaro.org> >>>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >>>>> --- >>>>> drivers/leds/leds-pm8058.c | 4 ++-- >>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> Hi Bjorn, >>>> >>>> (Nice to see you) :) >>>> >>>> I'm going to apply this *before* Linus' fix. >>>> >>>> Applied, thanks. >>> >>> After I rx an Ack from Richard, Jacek or Pavel of course. :) >>> >>> Will send a pull-request. >> >> Huh? This is for LED subsystem AFAICS. > > Right, hence why I said I'd sent out a pull-request. > > The problem, however, arose due to a change in its parent driver's > Kconfig entry, which has been applied to the MFD tree. We need to > ensure this patch is applied *before* the other 'fix' to quash the > warning before it starts, so to speak. Ah, I hadn't seen the MFD patch and understood that you was talking about pull request for 4.15-rc2, which looked a bit strange out of context. For v2: Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com> -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-11-30 3:05 ` [PATCH] leds: pm8058: Make ledtype pointer sized type Bjorn Andersson 2017-11-30 8:29 ` Lee Jones @ 2017-11-30 9:18 ` Linus Walleij 2017-11-30 9:41 ` Lee Jones 2017-11-30 9:40 ` Pavel Machek 2 siblings, 1 reply; 23+ messages in thread From: Linus Walleij @ 2017-11-30 9:18 UTC (permalink / raw) To: Bjorn Andersson Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, linux-leds, linux-kernel, Lee Jones, Stephen Rothwell, Linux-Next Mailing List On Thu, Nov 30, 2017 at 4:05 AM, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > The pointer returned by of_device_get_match_data() doesn't have the same > size as u32 on 64-bit architectures, causing issues when compile testing > the driver on such platform. Make ledtype unsigned long instead, to > solve this problem. > > Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") > Cc: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> Thanks for covering my ass. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-11-30 9:18 ` Linus Walleij @ 2017-11-30 9:41 ` Lee Jones 0 siblings, 0 replies; 23+ messages in thread From: Lee Jones @ 2017-11-30 9:41 UTC (permalink / raw) To: Linus Walleij Cc: Bjorn Andersson, Richard Purdie, Jacek Anaszewski, Pavel Machek, linux-leds, linux-kernel, Stephen Rothwell, Linux-Next Mailing List On Thu, 30 Nov 2017, Linus Walleij wrote: > On Thu, Nov 30, 2017 at 4:05 AM, Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > > The pointer returned by of_device_get_match_data() doesn't have the same > > size as u32 on 64-bit architectures, causing issues when compile testing > > the driver on such platform. Make ledtype unsigned long instead, to > > solve this problem. > > > > Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") > > Cc: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > Thanks for covering my ass. :D > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> I still need a LED Ack if I'm going to take this. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-11-30 3:05 ` [PATCH] leds: pm8058: Make ledtype pointer sized type Bjorn Andersson 2017-11-30 8:29 ` Lee Jones 2017-11-30 9:18 ` Linus Walleij @ 2017-11-30 9:40 ` Pavel Machek 2017-11-30 21:58 ` Jacek Anaszewski 2017-11-30 22:18 ` Bjorn Andersson 2 siblings, 2 replies; 23+ messages in thread From: Pavel Machek @ 2017-11-30 9:40 UTC (permalink / raw) To: Bjorn Andersson Cc: Richard Purdie, Jacek Anaszewski, linux-leds, linux-kernel, Linus Walleij, Lee Jones, Stephen Rothwell, Linux-Next Mailing List [-- Attachment #1: Type: text/plain, Size: 1175 bytes --] On Wed 2017-11-29 19:05:43, Bjorn Andersson wrote: > The pointer returned by of_device_get_match_data() doesn't have the same > size as u32 on 64-bit architectures, causing issues when compile testing > the driver on such platform. Make ledtype unsigned long instead, to > solve this problem. > > Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") > Cc: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> Ummm... no? extern const void *of_device_get_match_data(const struct device *dev); > diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c > index a52674327857..cc2afe81720d 100644 > --- a/drivers/leds/leds-pm8058.c > +++ b/drivers/leds/leds-pm8058.c > @@ -29,7 +29,7 @@ > struct pm8058_led { > struct regmap *map; > u32 reg; > - u32 ledtype; > + unsigned long ledtype; Make it void *. u32 is buggy. unsigned long is merely ugly code. void * is not nice, but certainly better than unsigned long. Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-11-30 9:40 ` Pavel Machek @ 2017-11-30 21:58 ` Jacek Anaszewski 2017-11-30 22:34 ` Pavel Machek 2017-11-30 22:18 ` Bjorn Andersson 1 sibling, 1 reply; 23+ messages in thread From: Jacek Anaszewski @ 2017-11-30 21:58 UTC (permalink / raw) To: Pavel Machek, Bjorn Andersson Cc: Richard Purdie, linux-leds, linux-kernel, Linus Walleij, Lee Jones, Stephen Rothwell, Linux-Next Mailing List On 11/30/2017 10:40 AM, Pavel Machek wrote: > On Wed 2017-11-29 19:05:43, Bjorn Andersson wrote: >> The pointer returned by of_device_get_match_data() doesn't have the same >> size as u32 on 64-bit architectures, causing issues when compile testing >> the driver on such platform. Make ledtype unsigned long instead, to >> solve this problem. >> >> Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") >> Cc: Linus Walleij <linus.walleij@linaro.org> >> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > Ummm... no? > > extern const void *of_device_get_match_data(const struct device *dev); > > >> diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c >> index a52674327857..cc2afe81720d 100644 >> --- a/drivers/leds/leds-pm8058.c >> +++ b/drivers/leds/leds-pm8058.c >> @@ -29,7 +29,7 @@ >> struct pm8058_led { >> struct regmap *map; >> u32 reg; >> - u32 ledtype; >> + unsigned long ledtype; > > Make it void *. u32 is buggy. unsigned long is merely ugly code. void > * is not nice, but certainly better than unsigned long. unsigned long is correct, see below: static const struct of_device_id pm8058_leds_id_table[] = { { .compatible = "qcom,pm8058-led", .data = (void *)PM8058_LED_TYPE_COMMON }, { .compatible = "qcom,pm8058-keypad-led", .data = (void *)PM8058_LED_TYPE_KEYPAD }, { .compatible = "qcom,pm8058-flash-led", .data = (void *)PM8058_LED_TYPE_FLASH }, { }, }; of_device_get_match_data will return PM8058_LED_TYPE_* which clearly is a led type identifier. Thus unsigned long looks reasonable. -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-11-30 21:58 ` Jacek Anaszewski @ 2017-11-30 22:34 ` Pavel Machek 2017-11-30 23:14 ` Stephen Rothwell 0 siblings, 1 reply; 23+ messages in thread From: Pavel Machek @ 2017-11-30 22:34 UTC (permalink / raw) To: Jacek Anaszewski Cc: Bjorn Andersson, Richard Purdie, linux-leds, linux-kernel, Linus Walleij, Lee Jones, Stephen Rothwell, Linux-Next Mailing List [-- Attachment #1: Type: text/plain, Size: 2388 bytes --] On Thu 2017-11-30 22:58:55, Jacek Anaszewski wrote: > On 11/30/2017 10:40 AM, Pavel Machek wrote: > > On Wed 2017-11-29 19:05:43, Bjorn Andersson wrote: > >> The pointer returned by of_device_get_match_data() doesn't have the same > >> size as u32 on 64-bit architectures, causing issues when compile testing > >> the driver on such platform. Make ledtype unsigned long instead, to > >> solve this problem. > >> > >> Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") > >> Cc: Linus Walleij <linus.walleij@linaro.org> > >> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > > > Ummm... no? > > > > extern const void *of_device_get_match_data(const struct device *dev); > > > > > >> diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c > >> index a52674327857..cc2afe81720d 100644 > >> --- a/drivers/leds/leds-pm8058.c > >> +++ b/drivers/leds/leds-pm8058.c > >> @@ -29,7 +29,7 @@ > >> struct pm8058_led { > >> struct regmap *map; > >> u32 reg; > >> - u32 ledtype; > >> + unsigned long ledtype; > > > > Make it void *. u32 is buggy. unsigned long is merely ugly code. void > > * is not nice, but certainly better than unsigned long. > > unsigned long is correct, see below: > > static const struct of_device_id pm8058_leds_id_table[] = { > { > .compatible = "qcom,pm8058-led", > .data = (void *)PM8058_LED_TYPE_COMMON > }, > { > .compatible = "qcom,pm8058-keypad-led", > .data = (void *)PM8058_LED_TYPE_KEYPAD > }, > { > .compatible = "qcom,pm8058-flash-led", > .data = (void *)PM8058_LED_TYPE_FLASH > }, > { }, > }; > > of_device_get_match_data will return PM8058_LED_TYPE_* > which clearly is a led type identifier. > > Thus unsigned long looks reasonable. Hmm. Ok. So u32 would actually make even more sense there (because PM8058_LED_TYPE_* does not really need to be 64-bit), but it would cause a warning. I don't like this. This fix actually makes code waste memory and is uglier. ...but we get a warning fix. So I don't like the patch, but it is an improvement... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-11-30 22:34 ` Pavel Machek @ 2017-11-30 23:14 ` Stephen Rothwell 2017-12-01 0:28 ` Pavel Machek 0 siblings, 1 reply; 23+ messages in thread From: Stephen Rothwell @ 2017-11-30 23:14 UTC (permalink / raw) To: Pavel Machek Cc: Jacek Anaszewski, Bjorn Andersson, Richard Purdie, linux-leds, linux-kernel, Linus Walleij, Lee Jones, Linux-Next Mailing List Hi Pavel, On Thu, 30 Nov 2017 23:34:07 +0100 Pavel Machek <pavel@ucw.cz> wrote: > > On Thu 2017-11-30 22:58:55, Jacek Anaszewski wrote: > > On 11/30/2017 10:40 AM, Pavel Machek wrote: > > > On Wed 2017-11-29 19:05:43, Bjorn Andersson wrote: > > >> The pointer returned by of_device_get_match_data() doesn't have the same > > >> size as u32 on 64-bit architectures, causing issues when compile testing > > >> the driver on such platform. Make ledtype unsigned long instead, to > > >> solve this problem. > > >> > > >> Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") > > >> Cc: Linus Walleij <linus.walleij@linaro.org> > > >> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > > > > > Ummm... no? > > > > > > extern const void *of_device_get_match_data(const struct device *dev); > > > > > > > > >> diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c > > >> index a52674327857..cc2afe81720d 100644 > > >> --- a/drivers/leds/leds-pm8058.c > > >> +++ b/drivers/leds/leds-pm8058.c > > >> @@ -29,7 +29,7 @@ > > >> struct pm8058_led { > > >> struct regmap *map; > > >> u32 reg; > > >> - u32 ledtype; > > >> + unsigned long ledtype; > > > > > > Make it void *. u32 is buggy. unsigned long is merely ugly code. void > > > * is not nice, but certainly better than unsigned long. > > > > unsigned long is correct, see below: > > > > static const struct of_device_id pm8058_leds_id_table[] = { > > { > > .compatible = "qcom,pm8058-led", > > .data = (void *)PM8058_LED_TYPE_COMMON > > }, > > { > > .compatible = "qcom,pm8058-keypad-led", > > .data = (void *)PM8058_LED_TYPE_KEYPAD > > }, > > { > > .compatible = "qcom,pm8058-flash-led", > > .data = (void *)PM8058_LED_TYPE_FLASH > > }, > > { }, > > }; > > > > of_device_get_match_data will return PM8058_LED_TYPE_* > > which clearly is a led type identifier. > > > > Thus unsigned long looks reasonable. > > Hmm. Ok. So u32 would actually make even more sense there (because > PM8058_LED_TYPE_* does not really need to be 64-bit), but it would > cause a warning. > > I don't like this. This fix actually makes code waste memory and is > uglier. > > ...but we get a warning fix. So I don't like the patch, but it is an > improvement... I *think* you can get away with (u32)(unsigned long)ptr ... -- Cheers, Stephen Rothwell ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-11-30 23:14 ` Stephen Rothwell @ 2017-12-01 0:28 ` Pavel Machek 2017-12-01 5:10 ` Bjorn Andersson 0 siblings, 1 reply; 23+ messages in thread From: Pavel Machek @ 2017-12-01 0:28 UTC (permalink / raw) To: Stephen Rothwell Cc: Jacek Anaszewski, Bjorn Andersson, Richard Purdie, linux-leds, linux-kernel, Linus Walleij, Lee Jones, Linux-Next Mailing List [-- Attachment #1: Type: text/plain, Size: 828 bytes --] Hi! > > > of_device_get_match_data will return PM8058_LED_TYPE_* > > > which clearly is a led type identifier. > > > > > > Thus unsigned long looks reasonable. > > > > Hmm. Ok. So u32 would actually make even more sense there (because > > PM8058_LED_TYPE_* does not really need to be 64-bit), but it would > > cause a warning. > > > > I don't like this. This fix actually makes code waste memory and is > > uglier. > > > > ...but we get a warning fix. So I don't like the patch, but it is an > > improvement... > > I *think* you can get away with (u32)(unsigned long)ptr ... Yes, that should work. It would actually be my preferred solution. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-12-01 0:28 ` Pavel Machek @ 2017-12-01 5:10 ` Bjorn Andersson 0 siblings, 0 replies; 23+ messages in thread From: Bjorn Andersson @ 2017-12-01 5:10 UTC (permalink / raw) To: Pavel Machek Cc: Stephen Rothwell, Jacek Anaszewski, Richard Purdie, linux-leds, linux-kernel, Linus Walleij, Lee Jones, Linux-Next Mailing List On Thu 30 Nov 16:28 PST 2017, Pavel Machek wrote: > Hi! > > > > > of_device_get_match_data will return PM8058_LED_TYPE_* > > > > which clearly is a led type identifier. > > > > > > > > Thus unsigned long looks reasonable. > > > > > > Hmm. Ok. So u32 would actually make even more sense there (because > > > PM8058_LED_TYPE_* does not really need to be 64-bit), but it would > > > cause a warning. > > > > > > I don't like this. This fix actually makes code waste memory and is > > > uglier. > > > > > > ...but we get a warning fix. So I don't like the patch, but it is an > > > improvement... > > > > I *think* you can get away with (u32)(unsigned long)ptr ... > > Yes, that should work. It would actually be my preferred solution. > I'm fine with this suggestion, and it silence the warning. I will update and resend the patch. Regards, Bjorn ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] leds: pm8058: Make ledtype pointer sized type 2017-11-30 9:40 ` Pavel Machek 2017-11-30 21:58 ` Jacek Anaszewski @ 2017-11-30 22:18 ` Bjorn Andersson 1 sibling, 0 replies; 23+ messages in thread From: Bjorn Andersson @ 2017-11-30 22:18 UTC (permalink / raw) To: Pavel Machek Cc: Richard Purdie, Jacek Anaszewski, linux-leds, linux-kernel, Linus Walleij, Lee Jones, Stephen Rothwell, Linux-Next Mailing List On Thu 30 Nov 01:40 PST 2017, Pavel Machek wrote: > On Wed 2017-11-29 19:05:43, Bjorn Andersson wrote: > > The pointer returned by of_device_get_match_data() doesn't have the same > > size as u32 on 64-bit architectures, causing issues when compile testing > > the driver on such platform. Make ledtype unsigned long instead, to > > solve this problem. > > > > Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") > > Cc: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > Ummm... no? > > extern const void *of_device_get_match_data(const struct device *dev); > Right, this returns a pointer, which is of architecture dependent size. > > > diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c > > index a52674327857..cc2afe81720d 100644 > > --- a/drivers/leds/leds-pm8058.c > > +++ b/drivers/leds/leds-pm8058.c > > @@ -29,7 +29,7 @@ > > struct pm8058_led { > > struct regmap *map; > > u32 reg; > > - u32 ledtype; > > + unsigned long ledtype; > > Make it void *. u32 is buggy. unsigned long is merely ugly code. void > * is not nice, but certainly better than unsigned long. > unsigned long is the integer type in the kernel that has the same size as a pointer, similar to the C-standard's intptr_t. So this is the idiomatic way to pass an integer data type via a pointer variable in the kernel. Regards, Bjorn ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: linux-next: build warning after merge of the mfd tree 2017-11-30 0:35 linux-next: build warning after merge of the mfd tree Stephen Rothwell 2017-11-30 3:05 ` [PATCH] leds: pm8058: Make ledtype pointer sized type Bjorn Andersson @ 2017-11-30 8:28 ` Lee Jones 2017-12-01 5:16 ` [PATCH v2] leds: pm8058: Silence pointer to integer size warning Bjorn Andersson 2 siblings, 0 replies; 23+ messages in thread From: Lee Jones @ 2017-11-30 8:28 UTC (permalink / raw) To: Stephen Rothwell Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Linus Walleij, Jacek Anaszewski, Bjorn Andersson On Thu, 30 Nov 2017, Stephen Rothwell wrote: > Hi Lee, > > After merging the mfd tree, today's linux-next build (x86_64 allmodconfig) > produced this warning: > > drivers/leds/leds-pm8058.c: In function 'pm8058_led_probe': > drivers/leds/leds-pm8058.c:109:17: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] > led->ledtype = (u32)of_device_get_match_data(&pdev->dev); Linuuuuusssssss ... Please fix. I'm happy to squash. > Exposed by commit > > deed10c2d603 ("mfd: pm8xxx: Make elegible for COMPILE_TEST") > > CONFIG_LEDS_PM8058 depends on CONFIG_MFD_PM8XXX which is now turned on > by allmodconfig. > > The actual problematic commit is: > > 7f866986e705 ("leds: add PM8058 LEDs driver") > > from v4.9-rc1. > -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2] leds: pm8058: Silence pointer to integer size warning 2017-11-30 0:35 linux-next: build warning after merge of the mfd tree Stephen Rothwell 2017-11-30 3:05 ` [PATCH] leds: pm8058: Make ledtype pointer sized type Bjorn Andersson 2017-11-30 8:28 ` linux-next: build warning after merge of the mfd tree Lee Jones @ 2017-12-01 5:16 ` Bjorn Andersson 2017-12-01 7:57 ` Lee Jones ` (2 more replies) 2 siblings, 3 replies; 23+ messages in thread From: Bjorn Andersson @ 2017-12-01 5:16 UTC (permalink / raw) To: Richard Purdie, Jacek Anaszewski, Pavel Machek Cc: linux-leds, linux-kernel, Linus Walleij, Lee Jones, Stephen Rothwell, Linux-Next Mailing List The pointer returned by of_device_get_match_data() doesn't have the same size as u32 on 64-bit architectures, causing a compile warning when compile-testing the driver on such platform. Cast the return value of of_device_get_match_data() to unsigned long and then to u32 to silence this warning. Cc: Linus Walleij <linus.walleij@linaro.org> Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- drivers/leds/leds-pm8058.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c index a52674327857..8988ba3b2d65 100644 --- a/drivers/leds/leds-pm8058.c +++ b/drivers/leds/leds-pm8058.c @@ -106,7 +106,7 @@ static int pm8058_led_probe(struct platform_device *pdev) if (!led) return -ENOMEM; - led->ledtype = (u32)of_device_get_match_data(&pdev->dev); + led->ledtype = (u32)(unsigned long)of_device_get_match_data(&pdev->dev); map = dev_get_regmap(pdev->dev.parent, NULL); if (!map) { -- 2.15.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2] leds: pm8058: Silence pointer to integer size warning 2017-12-01 5:16 ` [PATCH v2] leds: pm8058: Silence pointer to integer size warning Bjorn Andersson @ 2017-12-01 7:57 ` Lee Jones 2017-12-01 8:36 ` Pavel Machek 2017-12-01 8:56 ` Lee Jones 2017-12-01 9:01 ` [GIT PULL] Immutable branch between MFD and LED due for the v4.16 merge window Lee Jones 2 siblings, 1 reply; 23+ messages in thread From: Lee Jones @ 2017-12-01 7:57 UTC (permalink / raw) To: Bjorn Andersson Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, linux-leds, linux-kernel, Linus Walleij, Stephen Rothwell, Linux-Next Mailing List On Thu, 30 Nov 2017, Bjorn Andersson wrote: > The pointer returned by of_device_get_match_data() doesn't have the same > size as u32 on 64-bit architectures, causing a compile warning when > compile-testing the driver on such platform. > > Cast the return value of of_device_get_match_data() to unsigned long and > then to u32 to silence this warning. > > Cc: Linus Walleij <linus.walleij@linaro.org> > Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > drivers/leds/leds-pm8058.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c > index a52674327857..8988ba3b2d65 100644 > --- a/drivers/leds/leds-pm8058.c > +++ b/drivers/leds/leds-pm8058.c > @@ -106,7 +106,7 @@ static int pm8058_led_probe(struct platform_device *pdev) > if (!led) > return -ENOMEM; > > - led->ledtype = (u32)of_device_get_match_data(&pdev->dev); > + led->ledtype = (u32)(unsigned long)of_device_get_match_data(&pdev->dev); Wouldn't (u32)(void *) be even more correct? ... if the compiler will let you get away with it. > map = dev_get_regmap(pdev->dev.parent, NULL); > if (!map) { -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] leds: pm8058: Silence pointer to integer size warning 2017-12-01 7:57 ` Lee Jones @ 2017-12-01 8:36 ` Pavel Machek 2017-12-01 8:56 ` Lee Jones 0 siblings, 1 reply; 23+ messages in thread From: Pavel Machek @ 2017-12-01 8:36 UTC (permalink / raw) To: Lee Jones Cc: Bjorn Andersson, Richard Purdie, Jacek Anaszewski, linux-leds, linux-kernel, Linus Walleij, Stephen Rothwell, Linux-Next Mailing List [-- Attachment #1: Type: text/plain, Size: 1495 bytes --] On Fri 2017-12-01 07:57:15, Lee Jones wrote: > On Thu, 30 Nov 2017, Bjorn Andersson wrote: > > > The pointer returned by of_device_get_match_data() doesn't have the same > > size as u32 on 64-bit architectures, causing a compile warning when > > compile-testing the driver on such platform. > > > > Cast the return value of of_device_get_match_data() to unsigned long and > > then to u32 to silence this warning. > > > > Cc: Linus Walleij <linus.walleij@linaro.org> > > Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> Thanks! Acked-by: Pavel Machek <pavel@ucw.cz> > > diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c > > index a52674327857..8988ba3b2d65 100644 > > --- a/drivers/leds/leds-pm8058.c > > +++ b/drivers/leds/leds-pm8058.c > > @@ -106,7 +106,7 @@ static int pm8058_led_probe(struct platform_device *pdev) > > if (!led) > > return -ENOMEM; > > > > - led->ledtype = (u32)of_device_get_match_data(&pdev->dev); > > + led->ledtype = (u32)(unsigned long)of_device_get_match_data(&pdev->dev); > > Wouldn't (u32)(void *) be even more correct? > > ... if the compiler will let you get away with it. Afaict that would still produce the warning. of_device_get_match_data already returns void *. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] leds: pm8058: Silence pointer to integer size warning 2017-12-01 8:36 ` Pavel Machek @ 2017-12-01 8:56 ` Lee Jones 0 siblings, 0 replies; 23+ messages in thread From: Lee Jones @ 2017-12-01 8:56 UTC (permalink / raw) To: Pavel Machek Cc: Bjorn Andersson, Richard Purdie, Jacek Anaszewski, linux-leds, linux-kernel, Linus Walleij, Stephen Rothwell, Linux-Next Mailing List On Fri, 01 Dec 2017, Pavel Machek wrote: > On Fri 2017-12-01 07:57:15, Lee Jones wrote: > > On Thu, 30 Nov 2017, Bjorn Andersson wrote: > > > - led->ledtype = (u32)of_device_get_match_data(&pdev->dev); > > > + led->ledtype = (u32)(unsigned long)of_device_get_match_data(&pdev->dev); > > > > Wouldn't (u32)(void *) be even more correct? > > > > ... if the compiler will let you get away with it. > > Afaict that would still produce the warning. of_device_get_match_data > already returns void *. Just tested. It actually still suffers from the original error. warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] leds: pm8058: Silence pointer to integer size warning 2017-12-01 5:16 ` [PATCH v2] leds: pm8058: Silence pointer to integer size warning Bjorn Andersson 2017-12-01 7:57 ` Lee Jones @ 2017-12-01 8:56 ` Lee Jones 2017-12-01 9:01 ` [GIT PULL] Immutable branch between MFD and LED due for the v4.16 merge window Lee Jones 2 siblings, 0 replies; 23+ messages in thread From: Lee Jones @ 2017-12-01 8:56 UTC (permalink / raw) To: Bjorn Andersson Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, linux-leds, linux-kernel, Linus Walleij, Stephen Rothwell, Linux-Next Mailing List On Thu, 30 Nov 2017, Bjorn Andersson wrote: > The pointer returned by of_device_get_match_data() doesn't have the same > size as u32 on 64-bit architectures, causing a compile warning when > compile-testing the driver on such platform. > > Cast the return value of of_device_get_match_data() to unsigned long and > then to u32 to silence this warning. > > Cc: Linus Walleij <linus.walleij@linaro.org> > Fixes: 7f866986e705 ("leds: add PM8058 LEDs driver") > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > drivers/leds/leds-pm8058.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied with Pavel's Ack thanks. Will send out a pull-request shortly. > diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c > index a52674327857..8988ba3b2d65 100644 > --- a/drivers/leds/leds-pm8058.c > +++ b/drivers/leds/leds-pm8058.c > @@ -106,7 +106,7 @@ static int pm8058_led_probe(struct platform_device *pdev) > if (!led) > return -ENOMEM; > > - led->ledtype = (u32)of_device_get_match_data(&pdev->dev); > + led->ledtype = (u32)(unsigned long)of_device_get_match_data(&pdev->dev); > > map = dev_get_regmap(pdev->dev.parent, NULL); > if (!map) { -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 23+ messages in thread
* [GIT PULL] Immutable branch between MFD and LED due for the v4.16 merge window 2017-12-01 5:16 ` [PATCH v2] leds: pm8058: Silence pointer to integer size warning Bjorn Andersson 2017-12-01 7:57 ` Lee Jones 2017-12-01 8:56 ` Lee Jones @ 2017-12-01 9:01 ` Lee Jones 2 siblings, 0 replies; 23+ messages in thread From: Lee Jones @ 2017-12-01 9:01 UTC (permalink / raw) To: Bjorn Andersson Cc: Richard Purdie, Jacek Anaszewski, Pavel Machek, linux-leds, linux-kernel, Linus Walleij, Stephen Rothwell, Linux-Next Mailing List Enjoy! The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323: Linux 4.15-rc1 (2017-11-26 16:01:47 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-leds-v4.16 for you to fetch changes up to 8f52df50d9366f770a894d14ef724e5e04574e98: leds: pm8058: Silence pointer to integer size warning (2017-12-01 08:57:42 +0000) ---------------------------------------------------------------- Immutable branch between MFD and LED due for the v4.16 merge window ---------------------------------------------------------------- Bjorn Andersson (1): leds: pm8058: Silence pointer to integer size warning drivers/leds/leds-pm8058.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2017-12-01 20:35 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-11-30 0:35 linux-next: build warning after merge of the mfd tree Stephen Rothwell 2017-11-30 3:05 ` [PATCH] leds: pm8058: Make ledtype pointer sized type Bjorn Andersson 2017-11-30 8:29 ` Lee Jones 2017-11-30 8:31 ` Lee Jones 2017-11-30 22:02 ` Jacek Anaszewski 2017-12-01 7:56 ` Lee Jones 2017-12-01 20:35 ` Jacek Anaszewski 2017-11-30 9:18 ` Linus Walleij 2017-11-30 9:41 ` Lee Jones 2017-11-30 9:40 ` Pavel Machek 2017-11-30 21:58 ` Jacek Anaszewski 2017-11-30 22:34 ` Pavel Machek 2017-11-30 23:14 ` Stephen Rothwell 2017-12-01 0:28 ` Pavel Machek 2017-12-01 5:10 ` Bjorn Andersson 2017-11-30 22:18 ` Bjorn Andersson 2017-11-30 8:28 ` linux-next: build warning after merge of the mfd tree Lee Jones 2017-12-01 5:16 ` [PATCH v2] leds: pm8058: Silence pointer to integer size warning Bjorn Andersson 2017-12-01 7:57 ` Lee Jones 2017-12-01 8:36 ` Pavel Machek 2017-12-01 8:56 ` Lee Jones 2017-12-01 8:56 ` Lee Jones 2017-12-01 9:01 ` [GIT PULL] Immutable branch between MFD and LED due for the v4.16 merge window Lee Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).