* [PATCH] usb: gadget: at91_udc: Check gpio lookup results
@ 2013-07-23 18:55 Olof Johansson
2013-07-25 16:14 ` Felipe Balbi
0 siblings, 1 reply; 9+ messages in thread
From: Olof Johansson @ 2013-07-23 18:55 UTC (permalink / raw)
To: Felipe Balbi; +Cc: linux-usb, linux-kernel, Olof Johansson
This resolves the following valid build warning:
drivers/usb/gadget/at91_udc.c:1685:34: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
I switched from ? : to !! mostly to save from wrapping the lines while
I was at it.
Signed-off-by: Olof Johansson <olof@lixom.net>
---
Felipe, this would be nice to see fixed for 3.11 but I'd argue that it's
been here long enough to not really be needed for -stable.
drivers/usb/gadget/at91_udc.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
index 073b938..f3dbcd0 100644
--- a/drivers/usb/gadget/at91_udc.c
+++ b/drivers/usb/gadget/at91_udc.c
@@ -1682,12 +1682,20 @@ static void at91udc_of_init(struct at91_udc *udc,
board->vbus_pin = of_get_named_gpio_flags(np, "atmel,vbus-gpio", 0,
&flags);
- board->vbus_active_low = (flags & OF_GPIO_ACTIVE_LOW) ? 1 : 0;
+ if (board->vbus_pin < 0)
+ pr_err("%s: Failed to get atmel,vbus-gpio property\n",
+ np->full_name);
+ else
+ board->vbus_active_low = !!(flags & OF_GPIO_ACTIVE_LOW);
board->pullup_pin = of_get_named_gpio_flags(np, "atmel,pullup-gpio", 0,
&flags);
- board->pullup_active_low = (flags & OF_GPIO_ACTIVE_LOW) ? 1 : 0;
+ if (board->pullup_pin < 0)
+ pr_err("%s: Failed to get atmel,pullup-gpio property\n",
+ np->full_name);
+ else
+ board->pullup_active_low = !!(flags & OF_GPIO_ACTIVE_LOW);
}
static int at91udc_probe(struct platform_device *pdev)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
2013-07-23 18:55 [PATCH] usb: gadget: at91_udc: Check gpio lookup results Olof Johansson
@ 2013-07-25 16:14 ` Felipe Balbi
2013-07-25 16:18 ` Olof Johansson
0 siblings, 1 reply; 9+ messages in thread
From: Felipe Balbi @ 2013-07-25 16:14 UTC (permalink / raw)
To: Olof Johansson; +Cc: Felipe Balbi, linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]
Hi,
On Tue, Jul 23, 2013 at 11:55:50AM -0700, Olof Johansson wrote:
> This resolves the following valid build warning:
>
> drivers/usb/gadget/at91_udc.c:1685:34: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
>
> I switched from ? : to !! mostly to save from wrapping the lines while
> I was at it.
>
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>
> Felipe, this would be nice to see fixed for 3.11 but I'd argue that it's
> been here long enough to not really be needed for -stable.
>
> drivers/usb/gadget/at91_udc.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
> index 073b938..f3dbcd0 100644
> --- a/drivers/usb/gadget/at91_udc.c
> +++ b/drivers/usb/gadget/at91_udc.c
> @@ -1682,12 +1682,20 @@ static void at91udc_of_init(struct at91_udc *udc,
>
> board->vbus_pin = of_get_named_gpio_flags(np, "atmel,vbus-gpio", 0,
> &flags);
> - board->vbus_active_low = (flags & OF_GPIO_ACTIVE_LOW) ? 1 : 0;
> + if (board->vbus_pin < 0)
> + pr_err("%s: Failed to get atmel,vbus-gpio property\n",
> + np->full_name);
> + else
> + board->vbus_active_low = !!(flags & OF_GPIO_ACTIVE_LOW);
should you even continue if you can't get the gpio ? If this gpio is
optional, then it's not really and error, rather a debugging or
informational message.
BTW, this vbus-gpio looks, to me at least, like a fixed regulator
controlled by a GPIO, no ?
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
2013-07-25 16:14 ` Felipe Balbi
@ 2013-07-25 16:18 ` Olof Johansson
2013-07-25 17:19 ` Felipe Balbi
0 siblings, 1 reply; 9+ messages in thread
From: Olof Johansson @ 2013-07-25 16:18 UTC (permalink / raw)
To: Felipe Balbi; +Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
On Thu, Jul 25, 2013 at 9:14 AM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Tue, Jul 23, 2013 at 11:55:50AM -0700, Olof Johansson wrote:
>> This resolves the following valid build warning:
>>
>> drivers/usb/gadget/at91_udc.c:1685:34: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
>>
>> I switched from ? : to !! mostly to save from wrapping the lines while
>> I was at it.
>>
>> Signed-off-by: Olof Johansson <olof@lixom.net>
>> ---
>>
>> Felipe, this would be nice to see fixed for 3.11 but I'd argue that it's
>> been here long enough to not really be needed for -stable.
>>
>> drivers/usb/gadget/at91_udc.c | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
>> index 073b938..f3dbcd0 100644
>> --- a/drivers/usb/gadget/at91_udc.c
>> +++ b/drivers/usb/gadget/at91_udc.c
>> @@ -1682,12 +1682,20 @@ static void at91udc_of_init(struct at91_udc *udc,
>>
>> board->vbus_pin = of_get_named_gpio_flags(np, "atmel,vbus-gpio", 0,
>> &flags);
>> - board->vbus_active_low = (flags & OF_GPIO_ACTIVE_LOW) ? 1 : 0;
>> + if (board->vbus_pin < 0)
>> + pr_err("%s: Failed to get atmel,vbus-gpio property\n",
>> + np->full_name);
>> + else
>> + board->vbus_active_low = !!(flags & OF_GPIO_ACTIVE_LOW);
>
> should you even continue if you can't get the gpio ? If this gpio is
> optional, then it's not really and error, rather a debugging or
> informational message.
That's what the code does today, and I wasn't trying to second-guess
their decisions on that. Chances are firmware, in some instances, have
left power on so continuing might do no harm.
> BTW, this vbus-gpio looks, to me at least, like a fixed regulator
> controlled by a GPIO, no ?
Yes, it does. We have plenty of these kind of bindings from before
everything had to be a regulator.
-Olof
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
2013-07-25 16:18 ` Olof Johansson
@ 2013-07-25 17:19 ` Felipe Balbi
2013-07-25 20:59 ` Olof Johansson
0 siblings, 1 reply; 9+ messages in thread
From: Felipe Balbi @ 2013-07-25 17:19 UTC (permalink / raw)
To: Olof Johansson
Cc: Felipe Balbi, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 2082 bytes --]
On Thu, Jul 25, 2013 at 09:18:39AM -0700, Olof Johansson wrote:
> On Thu, Jul 25, 2013 at 9:14 AM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Tue, Jul 23, 2013 at 11:55:50AM -0700, Olof Johansson wrote:
> >> This resolves the following valid build warning:
> >>
> >> drivers/usb/gadget/at91_udc.c:1685:34: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >>
> >> I switched from ? : to !! mostly to save from wrapping the lines while
> >> I was at it.
> >>
> >> Signed-off-by: Olof Johansson <olof@lixom.net>
> >> ---
> >>
> >> Felipe, this would be nice to see fixed for 3.11 but I'd argue that it's
> >> been here long enough to not really be needed for -stable.
> >>
> >> drivers/usb/gadget/at91_udc.c | 12 ++++++++++--
> >> 1 file changed, 10 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
> >> index 073b938..f3dbcd0 100644
> >> --- a/drivers/usb/gadget/at91_udc.c
> >> +++ b/drivers/usb/gadget/at91_udc.c
> >> @@ -1682,12 +1682,20 @@ static void at91udc_of_init(struct at91_udc *udc,
> >>
> >> board->vbus_pin = of_get_named_gpio_flags(np, "atmel,vbus-gpio", 0,
> >> &flags);
> >> - board->vbus_active_low = (flags & OF_GPIO_ACTIVE_LOW) ? 1 : 0;
> >> + if (board->vbus_pin < 0)
> >> + pr_err("%s: Failed to get atmel,vbus-gpio property\n",
> >> + np->full_name);
> >> + else
> >> + board->vbus_active_low = !!(flags & OF_GPIO_ACTIVE_LOW);
> >
> > should you even continue if you can't get the gpio ? If this gpio is
> > optional, then it's not really and error, rather a debugging or
> > informational message.
>
> That's what the code does today, and I wasn't trying to second-guess
> their decisions on that. Chances are firmware, in some instances, have
> left power on so continuing might do no harm.
fair enough, then let's just decrease the error message level to debug
or info.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
2013-07-25 17:19 ` Felipe Balbi
@ 2013-07-25 20:59 ` Olof Johansson
2013-07-26 9:54 ` Felipe Balbi
0 siblings, 1 reply; 9+ messages in thread
From: Olof Johansson @ 2013-07-25 20:59 UTC (permalink / raw)
To: Felipe Balbi; +Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
On Thu, Jul 25, 2013 at 10:19 AM, Felipe Balbi <balbi@ti.com> wrote:
> On Thu, Jul 25, 2013 at 09:18:39AM -0700, Olof Johansson wrote:
>> That's what the code does today, and I wasn't trying to second-guess
>> their decisions on that. Chances are firmware, in some instances, have
>> left power on so continuing might do no harm.
>
> fair enough, then let's just decrease the error message level to debug
> or info.
Fair enough. info seems appropriate (or warn). Want me to respin, or
can you edit when you apply?
Thanks,
-Olof
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
2013-07-25 20:59 ` Olof Johansson
@ 2013-07-26 9:54 ` Felipe Balbi
2013-07-26 16:23 ` Olof Johansson
0 siblings, 1 reply; 9+ messages in thread
From: Felipe Balbi @ 2013-07-26 9:54 UTC (permalink / raw)
To: Olof Johansson
Cc: Felipe Balbi, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 644 bytes --]
On Thu, Jul 25, 2013 at 01:59:51PM -0700, Olof Johansson wrote:
> On Thu, Jul 25, 2013 at 10:19 AM, Felipe Balbi <balbi@ti.com> wrote:
> > On Thu, Jul 25, 2013 at 09:18:39AM -0700, Olof Johansson wrote:
> >> That's what the code does today, and I wasn't trying to second-guess
> >> their decisions on that. Chances are firmware, in some instances, have
> >> left power on so continuing might do no harm.
> >
> > fair enough, then let's just decrease the error message level to debug
> > or info.
>
> Fair enough. info seems appropriate (or warn). Want me to respin, or
> can you edit when you apply?
please respin
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
2013-07-26 9:54 ` Felipe Balbi
@ 2013-07-26 16:23 ` Olof Johansson
2013-07-26 20:30 ` Felipe Balbi
0 siblings, 1 reply; 9+ messages in thread
From: Olof Johansson @ 2013-07-26 16:23 UTC (permalink / raw)
To: Felipe Balbi; +Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
On Fri, Jul 26, 2013 at 2:54 AM, Felipe Balbi <balbi@ti.com> wrote:
> On Thu, Jul 25, 2013 at 01:59:51PM -0700, Olof Johansson wrote:
>> On Thu, Jul 25, 2013 at 10:19 AM, Felipe Balbi <balbi@ti.com> wrote:
>> > On Thu, Jul 25, 2013 at 09:18:39AM -0700, Olof Johansson wrote:
>> >> That's what the code does today, and I wasn't trying to second-guess
>> >> their decisions on that. Chances are firmware, in some instances, have
>> >> left power on so continuing might do no harm.
>> >
>> > fair enough, then let's just decrease the error message level to debug
>> > or info.
>>
>> Fair enough. info seems appropriate (or warn). Want me to respin, or
>> can you edit when you apply?
>
> please respin
An older patch from Arnd that accomplishes the same warning removal
has mysteriously showed up in -next in the last couple of days
(ae40d64b1f2db93d7b092e6425a2f716289fbd09), even though commit date
was July 15.
So, might as well, drop this one.
-Olof
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
2013-07-26 16:23 ` Olof Johansson
@ 2013-07-26 20:30 ` Felipe Balbi
2013-07-26 20:47 ` Olof Johansson
0 siblings, 1 reply; 9+ messages in thread
From: Felipe Balbi @ 2013-07-26 20:30 UTC (permalink / raw)
To: Olof Johansson
Cc: Felipe Balbi, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 2688 bytes --]
On Fri, Jul 26, 2013 at 09:23:35AM -0700, Olof Johansson wrote:
> On Fri, Jul 26, 2013 at 2:54 AM, Felipe Balbi <balbi@ti.com> wrote:
> > On Thu, Jul 25, 2013 at 01:59:51PM -0700, Olof Johansson wrote:
> >> On Thu, Jul 25, 2013 at 10:19 AM, Felipe Balbi <balbi@ti.com> wrote:
> >> > On Thu, Jul 25, 2013 at 09:18:39AM -0700, Olof Johansson wrote:
> >> >> That's what the code does today, and I wasn't trying to second-guess
> >> >> their decisions on that. Chances are firmware, in some instances, have
> >> >> left power on so continuing might do no harm.
> >> >
> >> > fair enough, then let's just decrease the error message level to debug
> >> > or info.
> >>
> >> Fair enough. info seems appropriate (or warn). Want me to respin, or
> >> can you edit when you apply?
> >
> > please respin
>
> An older patch from Arnd that accomplishes the same warning removal
> has mysteriously showed up in -next in the last couple of days
> (ae40d64b1f2db93d7b092e6425a2f716289fbd09), even though commit date
> was July 15.
>
> So, might as well, drop this one.
doesn't look like the same thing:
commit ae40d64b1f2db93d7b092e6425a2f716289fbd09
Author: Arnd Bergmann <arnd@arndb.de>
Date: Wed Jun 19 13:27:27 2013 +0200
usb: gadget: at91_udc: call at91udc_of_init only when needed
This avoids a build error in at91sam9261_9g10_defconfig:
drivers/usb/gadget/at91_udc.c: In function 'at91udc_probe':
drivers/usb/gadget/at91_udc.c:1685:34: warning: 'flags' may be used uninitialized in this
function [-Wmaybe-uninitialized]
board->vbus_active_low = (flags & OF_GPIO_ACTIVE_LOW) ? 1 : 0;
^
drivers/usb/gadget/at91_udc.c:1678:21: note: 'flags' was declared here
enum of_gpio_flags flags;
^
Making the call to at91udc_of_init conditinal also reduces
the object code size without sacrificing build coverage.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c
index 073b938..2cbab1c 100644
--- a/drivers/usb/gadget/at91_udc.c
+++ b/drivers/usb/gadget/at91_udc.c
@@ -1725,7 +1725,7 @@ static int at91udc_probe(struct platform_device *pdev)
/* init software state */
udc = &controller;
udc->gadget.dev.parent = dev;
- if (pdev->dev.of_node)
+ if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node)
at91udc_of_init(udc, pdev->dev.of_node);
else
memcpy(&udc->board, dev->platform_data,
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
2013-07-26 20:30 ` Felipe Balbi
@ 2013-07-26 20:47 ` Olof Johansson
0 siblings, 0 replies; 9+ messages in thread
From: Olof Johansson @ 2013-07-26 20:47 UTC (permalink / raw)
To: Felipe Balbi; +Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
On Fri, Jul 26, 2013 at 1:30 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Fri, Jul 26, 2013 at 09:23:35AM -0700, Olof Johansson wrote:
>> On Fri, Jul 26, 2013 at 2:54 AM, Felipe Balbi <balbi@ti.com> wrote:
>> > On Thu, Jul 25, 2013 at 01:59:51PM -0700, Olof Johansson wrote:
>> >> On Thu, Jul 25, 2013 at 10:19 AM, Felipe Balbi <balbi@ti.com> wrote:
>> >> > On Thu, Jul 25, 2013 at 09:18:39AM -0700, Olof Johansson wrote:
>> >> >> That's what the code does today, and I wasn't trying to second-guess
>> >> >> their decisions on that. Chances are firmware, in some instances, have
>> >> >> left power on so continuing might do no harm.
>> >> >
>> >> > fair enough, then let's just decrease the error message level to debug
>> >> > or info.
>> >>
>> >> Fair enough. info seems appropriate (or warn). Want me to respin, or
>> >> can you edit when you apply?
>> >
>> > please respin
>>
>> An older patch from Arnd that accomplishes the same warning removal
>> has mysteriously showed up in -next in the last couple of days
>> (ae40d64b1f2db93d7b092e6425a2f716289fbd09), even though commit date
>> was July 15.
>>
>> So, might as well, drop this one.
>
> doesn't look like the same thing:
No, not the same patch, but fixes the same warning as a result. Please
stick to the patch from Arnd that you've already applied.
Thanks,
-Olof
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-07-26 20:47 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-23 18:55 [PATCH] usb: gadget: at91_udc: Check gpio lookup results Olof Johansson
2013-07-25 16:14 ` Felipe Balbi
2013-07-25 16:18 ` Olof Johansson
2013-07-25 17:19 ` Felipe Balbi
2013-07-25 20:59 ` Olof Johansson
2013-07-26 9:54 ` Felipe Balbi
2013-07-26 16:23 ` Olof Johansson
2013-07-26 20:30 ` Felipe Balbi
2013-07-26 20:47 ` Olof Johansson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.