From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S941923AbcJYQyM (ORCPT ); Tue, 25 Oct 2016 12:54:12 -0400 Received: from vern.gendns.com ([206.190.152.46]:48331 "EHLO vern.gendns.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S941840AbcJYQyG (ORCPT ); Tue, 25 Oct 2016 12:54:06 -0400 Subject: Re: [PATCH/RFT v2 12/17] USB: ochi-da8xx: Use a regulator for vbus/overcurrent To: Axel Haslam References: <20161024164634.4330-1-ahaslam@baylibre.com> <20161024164634.4330-13-ahaslam@baylibre.com> <672c1f1a-58b7-6947-7fb2-acd38fab8597@lechnology.com> Cc: Greg KH , Johan Hovold , robh+dt@kernel.org, Sekhar Nori , Alan Stern , Kevin Hilman , Sergei Shtylyov , Mark Brown , Alexandre Bailon , linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org From: David Lechner Message-ID: <09a4391e-63b6-1622-f964-4aff0ecae87c@lechnology.com> Date: Tue, 25 Oct 2016 11:53:58 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - vern.gendns.com X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - lechnology.com X-Get-Message-Sender-Via: vern.gendns.com: authenticated_id: davidmain+lechnology.com/only user confirmed/virtual account not confirmed X-Authenticated-Sender: vern.gendns.com: davidmain@lechnology.com X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/25/2016 03:24 AM, Axel Haslam wrote: > On Tue, Oct 25, 2016 at 3:39 AM, David Lechner wrote: >> On 10/24/2016 11:46 AM, ahaslam@baylibre.com wrote: >>> >>> From: Axel Haslam >>> >>> Currently, the da8xx ohci driver uses a set of gpios and callbacks in >>> board files to handle vbus and overcurrent irqs form the power supply. >>> However, this does not play nice when moving to a DT based boot were >>> we wont have board files. >>> >>> Instead of requesting and handling the gpio, use the regulator framework >>> to take care of enabling and disabling vbus power. >>> This has the benefit >>> that we dont need to pass any more platform data to the driver: >>> >>> These will be handled by the regulator framework: >>> set_power -> regulator_enable/regulator_disable >>> get_power -> regulator_is_enabled >>> get_oci -> regulator_get_mode >>> ocic_notify -> regulator notification >>> >>> We can keep the default potpgt and use the regulator start delay instead: >>> potpgt -> regulator startup delay time >>> >>> The hawk board does not have a GPIO/OVERCURRENT gpio to control vbus, >>> (they should not have been decleared/reserved) so, just remove those >>> definitions from the hwk board file. >>> >>> Signed-off-by: Axel Haslam >>> --- >> >> >> >> How do you recover after an overcurrent event? >> >> I have configured a fixed-regulator using device-tree, but similar to the >> configuration in the board files here. However, when I shorted out the VBUS >> and caused an overcurrent event, I see nothing in the kernel log saying that >> there was an overcurrent event and after I remove the short, the regulator >> is never turned back on. >> >> > > You should have the patch to fix gpiolib, and you should declare the > over current gpio on the regulator as such: > (if the pin is enabled high you should add oc-active-high); > > vbus_fixed: fixed-regulator-vbus { > compatible = "regulator-fixed"; > gpio = <&gpio 109 0>; > oc-gpio = <&gpio 36 0>; > regulator-boot-on; > enable-active-high; > regulator-name = "vbus"; > regulator-min-microvolt = <5000000>; > regulator-max-microvolt = <5000000>; > }; > > > Question: Do you see that the over current gpio was requested > in debugfs/gpio? and, do you see the interrupt in /proc/interrupts? > > If you unplug and plug in back the usb device it should work again. > also you can unbind and bind it should also start to work: > something like: > > echo usb1 >/sys/bus/usb/drivers/usb/unbind > echo usb1 >/sys/bus/usb/drivers/usb/bind > > I have added oc-active-high and I get different results, but it is still not quite right. When I short the VBUS, I can see that my overcurrent gpio changes state. However, the driver does not turn of the VBUS. When I remove the short, I get an overcurrent error in the kernel log. I would expect this when I create the short, not when I remove it. I also tried adding GPIO_ACTIVE_LOW to the oc-gpio, but this did not change the behavior. In either case, the oc_gpio shows as high under normal conditions, so perhaps there is a problem with the gpio-davinci driver not picking up GPIO_ACTIVE_LOW from the device tree. My regulator is basically the same. My device just uses different gpios. vbus_reg: vbus-reg { compatible = "regulator-fixed"; pinctrl-names = "default"; pinctrl-0 = <&usb11_pins>; gpio = <&gpio 101 GPIO_ACTIVE_LOW>; oc-gpio = <&gpio 99 0>; enable-active-high; oc-active-high; regulator-name = "vbus"; regulator-min-microvolt = <5000000>; regulator-max-microvolt = <5000000>; } It seems to me though that I should not have oc-active-high since under normal conditions, the oc_gpio is high and during an overcurrent event, the oc_gpio is low. Double-checking the behavior without oc-active-high, I see that the vbus gpio is turned off in response to the overcurrent event, but I don't get the overcurrent message in the kernel log. Perhaps this is because as soon as there is an overcurrent event the vbus turns off and the oc_gpio returns to normal before the usb driver has a chance to poll the overcurrent state? Also, unplugging the device and plugging it back in does nothing. Unbinding and binding the driver does work, but that does not seem like a very nice way to have to recover from an overcurrent event. From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Lechner Subject: Re: [PATCH/RFT v2 12/17] USB: ochi-da8xx: Use a regulator for vbus/overcurrent Date: Tue, 25 Oct 2016 11:53:58 -0500 Message-ID: <09a4391e-63b6-1622-f964-4aff0ecae87c@lechnology.com> References: <20161024164634.4330-1-ahaslam@baylibre.com> <20161024164634.4330-13-ahaslam@baylibre.com> <672c1f1a-58b7-6947-7fb2-acd38fab8597@lechnology.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Axel Haslam Cc: Greg KH , Johan Hovold , robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Sekhar Nori , Alan Stern , Kevin Hilman , Sergei Shtylyov , Mark Brown , Alexandre Bailon , linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: devicetree@vger.kernel.org On 10/25/2016 03:24 AM, Axel Haslam wrote: > On Tue, Oct 25, 2016 at 3:39 AM, David Lechner wrote: >> On 10/24/2016 11:46 AM, ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org wrote: >>> >>> From: Axel Haslam >>> >>> Currently, the da8xx ohci driver uses a set of gpios and callbacks in >>> board files to handle vbus and overcurrent irqs form the power supply. >>> However, this does not play nice when moving to a DT based boot were >>> we wont have board files. >>> >>> Instead of requesting and handling the gpio, use the regulator framework >>> to take care of enabling and disabling vbus power. >>> This has the benefit >>> that we dont need to pass any more platform data to the driver: >>> >>> These will be handled by the regulator framework: >>> set_power -> regulator_enable/regulator_disable >>> get_power -> regulator_is_enabled >>> get_oci -> regulator_get_mode >>> ocic_notify -> regulator notification >>> >>> We can keep the default potpgt and use the regulator start delay instead: >>> potpgt -> regulator startup delay time >>> >>> The hawk board does not have a GPIO/OVERCURRENT gpio to control vbus, >>> (they should not have been decleared/reserved) so, just remove those >>> definitions from the hwk board file. >>> >>> Signed-off-by: Axel Haslam >>> --- >> >> >> >> How do you recover after an overcurrent event? >> >> I have configured a fixed-regulator using device-tree, but similar to the >> configuration in the board files here. However, when I shorted out the VBUS >> and caused an overcurrent event, I see nothing in the kernel log saying that >> there was an overcurrent event and after I remove the short, the regulator >> is never turned back on. >> >> > > You should have the patch to fix gpiolib, and you should declare the > over current gpio on the regulator as such: > (if the pin is enabled high you should add oc-active-high); > > vbus_fixed: fixed-regulator-vbus { > compatible = "regulator-fixed"; > gpio = <&gpio 109 0>; > oc-gpio = <&gpio 36 0>; > regulator-boot-on; > enable-active-high; > regulator-name = "vbus"; > regulator-min-microvolt = <5000000>; > regulator-max-microvolt = <5000000>; > }; > > > Question: Do you see that the over current gpio was requested > in debugfs/gpio? and, do you see the interrupt in /proc/interrupts? > > If you unplug and plug in back the usb device it should work again. > also you can unbind and bind it should also start to work: > something like: > > echo usb1 >/sys/bus/usb/drivers/usb/unbind > echo usb1 >/sys/bus/usb/drivers/usb/bind > > I have added oc-active-high and I get different results, but it is still not quite right. When I short the VBUS, I can see that my overcurrent gpio changes state. However, the driver does not turn of the VBUS. When I remove the short, I get an overcurrent error in the kernel log. I would expect this when I create the short, not when I remove it. I also tried adding GPIO_ACTIVE_LOW to the oc-gpio, but this did not change the behavior. In either case, the oc_gpio shows as high under normal conditions, so perhaps there is a problem with the gpio-davinci driver not picking up GPIO_ACTIVE_LOW from the device tree. My regulator is basically the same. My device just uses different gpios. vbus_reg: vbus-reg { compatible = "regulator-fixed"; pinctrl-names = "default"; pinctrl-0 = <&usb11_pins>; gpio = <&gpio 101 GPIO_ACTIVE_LOW>; oc-gpio = <&gpio 99 0>; enable-active-high; oc-active-high; regulator-name = "vbus"; regulator-min-microvolt = <5000000>; regulator-max-microvolt = <5000000>; } It seems to me though that I should not have oc-active-high since under normal conditions, the oc_gpio is high and during an overcurrent event, the oc_gpio is low. Double-checking the behavior without oc-active-high, I see that the vbus gpio is turned off in response to the overcurrent event, but I don't get the overcurrent message in the kernel log. Perhaps this is because as soon as there is an overcurrent event the vbus turns off and the oc_gpio returns to normal before the usb driver has a chance to poll the overcurrent state? Also, unplugging the device and plugging it back in does nothing. Unbinding and binding the driver does work, but that does not seem like a very nice way to have to recover from an overcurrent event. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: david@lechnology.com (David Lechner) Date: Tue, 25 Oct 2016 11:53:58 -0500 Subject: [PATCH/RFT v2 12/17] USB: ochi-da8xx: Use a regulator for vbus/overcurrent In-Reply-To: References: <20161024164634.4330-1-ahaslam@baylibre.com> <20161024164634.4330-13-ahaslam@baylibre.com> <672c1f1a-58b7-6947-7fb2-acd38fab8597@lechnology.com> Message-ID: <09a4391e-63b6-1622-f964-4aff0ecae87c@lechnology.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 10/25/2016 03:24 AM, Axel Haslam wrote: > On Tue, Oct 25, 2016 at 3:39 AM, David Lechner wrote: >> On 10/24/2016 11:46 AM, ahaslam at baylibre.com wrote: >>> >>> From: Axel Haslam >>> >>> Currently, the da8xx ohci driver uses a set of gpios and callbacks in >>> board files to handle vbus and overcurrent irqs form the power supply. >>> However, this does not play nice when moving to a DT based boot were >>> we wont have board files. >>> >>> Instead of requesting and handling the gpio, use the regulator framework >>> to take care of enabling and disabling vbus power. >>> This has the benefit >>> that we dont need to pass any more platform data to the driver: >>> >>> These will be handled by the regulator framework: >>> set_power -> regulator_enable/regulator_disable >>> get_power -> regulator_is_enabled >>> get_oci -> regulator_get_mode >>> ocic_notify -> regulator notification >>> >>> We can keep the default potpgt and use the regulator start delay instead: >>> potpgt -> regulator startup delay time >>> >>> The hawk board does not have a GPIO/OVERCURRENT gpio to control vbus, >>> (they should not have been decleared/reserved) so, just remove those >>> definitions from the hwk board file. >>> >>> Signed-off-by: Axel Haslam >>> --- >> >> >> >> How do you recover after an overcurrent event? >> >> I have configured a fixed-regulator using device-tree, but similar to the >> configuration in the board files here. However, when I shorted out the VBUS >> and caused an overcurrent event, I see nothing in the kernel log saying that >> there was an overcurrent event and after I remove the short, the regulator >> is never turned back on. >> >> > > You should have the patch to fix gpiolib, and you should declare the > over current gpio on the regulator as such: > (if the pin is enabled high you should add oc-active-high); > > vbus_fixed: fixed-regulator-vbus { > compatible = "regulator-fixed"; > gpio = <&gpio 109 0>; > oc-gpio = <&gpio 36 0>; > regulator-boot-on; > enable-active-high; > regulator-name = "vbus"; > regulator-min-microvolt = <5000000>; > regulator-max-microvolt = <5000000>; > }; > > > Question: Do you see that the over current gpio was requested > in debugfs/gpio? and, do you see the interrupt in /proc/interrupts? > > If you unplug and plug in back the usb device it should work again. > also you can unbind and bind it should also start to work: > something like: > > echo usb1 >/sys/bus/usb/drivers/usb/unbind > echo usb1 >/sys/bus/usb/drivers/usb/bind > > I have added oc-active-high and I get different results, but it is still not quite right. When I short the VBUS, I can see that my overcurrent gpio changes state. However, the driver does not turn of the VBUS. When I remove the short, I get an overcurrent error in the kernel log. I would expect this when I create the short, not when I remove it. I also tried adding GPIO_ACTIVE_LOW to the oc-gpio, but this did not change the behavior. In either case, the oc_gpio shows as high under normal conditions, so perhaps there is a problem with the gpio-davinci driver not picking up GPIO_ACTIVE_LOW from the device tree. My regulator is basically the same. My device just uses different gpios. vbus_reg: vbus-reg { compatible = "regulator-fixed"; pinctrl-names = "default"; pinctrl-0 = <&usb11_pins>; gpio = <&gpio 101 GPIO_ACTIVE_LOW>; oc-gpio = <&gpio 99 0>; enable-active-high; oc-active-high; regulator-name = "vbus"; regulator-min-microvolt = <5000000>; regulator-max-microvolt = <5000000>; } It seems to me though that I should not have oc-active-high since under normal conditions, the oc_gpio is high and during an overcurrent event, the oc_gpio is low. Double-checking the behavior without oc-active-high, I see that the vbus gpio is turned off in response to the overcurrent event, but I don't get the overcurrent message in the kernel log. Perhaps this is because as soon as there is an overcurrent event the vbus turns off and the oc_gpio returns to normal before the usb driver has a chance to poll the overcurrent state? Also, unplugging the device and plugging it back in does nothing. Unbinding and binding the driver does work, but that does not seem like a very nice way to have to recover from an overcurrent event.