Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
On Thu, Jul 25, 2013 at 01:59:51PM -0700, Olof Johansson wrote: On Thu, Jul 25, 2013 at 10:19 AM, Felipe Balbi ba...@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 signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
On Fri, Jul 26, 2013 at 09:23:35AM -0700, Olof Johansson wrote: On Fri, Jul 26, 2013 at 2:54 AM, Felipe Balbi ba...@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 ba...@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 a...@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 a...@arndb.de Cc: Felipe Balbi ba...@ti.com Cc: Nicolas Ferre nicolas.fe...@atmel.com Signed-off-by: Felipe Balbi ba...@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 signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
On Fri, Jul 26, 2013 at 1:30 PM, Felipe Balbi ba...@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 ba...@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 ba...@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 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
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 o...@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 signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
On Thu, Jul 25, 2013 at 9:14 AM, Felipe Balbi ba...@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 o...@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 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
On Thu, Jul 25, 2013 at 09:18:39AM -0700, Olof Johansson wrote: On Thu, Jul 25, 2013 at 9:14 AM, Felipe Balbi ba...@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 o...@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 signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: at91_udc: Check gpio lookup results
On Thu, Jul 25, 2013 at 10:19 AM, Felipe Balbi ba...@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 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: at91_udc: Check gpio lookup results
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 o...@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 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
