Re: [PATCH/RFT v2 12/17] USB: ochi-da8xx: Use a regulator for vbus/overcurrent

2016-10-25 Thread Axel Haslam 
On Tue, Oct 25, 2016 at 6:53 PM, David Lechner  wrote:
> 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, ahas...@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 = < 109 0>;
>>oc-gpio = < 36 0>;
>>regulator-boot-on;
>>enable-active-high;
>>regulator-name = "vbus";
>>regulator-min-microvolt = <500>;
>>regulator-max-microvolt = <500>;
>>};
>>
>>
>> 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 = <_pins>;
> gpio = < 101 GPIO_ACTIVE_LOW>;
> oc-gpio = < 99 0>;
> enable-active-high;
> oc-active-high;
> regulator-name = "vbus";
> regulator-min-microvolt = <500>;
> regulator-max-microvolt = <500>;
> }
>
>
> 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

in my board the over current gpio is active low too, i was just mentiontioning
to add that in case yours  was not.

> 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?

Perhaps. i dont have a board that has overcurrent, or that i can
switch vbus off.
what is exactly the log you are refering to?

im wondering, was the behavior different before the patches?
it should be the same without the

Re: [PATCH/RFT v2 12/17] USB: ochi-da8xx: Use a regulator for vbus/overcurrent

2016-10-25 Thread David Lechner 

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, ahas...@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 = < 109 0>;
   oc-gpio = < 36 0>;
   regulator-boot-on;
   enable-active-high;
   regulator-name = "vbus";
   regulator-min-microvolt = <500>;
   regulator-max-microvolt = <500>;
   };


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 = <_pins>;
gpio = < 101 GPIO_ACTIVE_LOW>;
oc-gpio = < 99 0>;
enable-active-high;
oc-active-high;
regulator-name = "vbus";
regulator-min-microvolt = <500>;
regulator-max-microvolt = <500>;
}


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 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/RFT v2 12/17] USB: ochi-da8xx: Use a regulator for vbus/overcurrent

2016-10-25 Thread Axel Haslam 
On Tue, Oct 25, 2016 at 12:43 PM, Sekhar Nori  wrote:
> On Monday 24 October 2016 10:16 PM, ahas...@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 
>> ---
>>  arch/arm/mach-davinci/board-da830-evm.c |  97 
>>  arch/arm/mach-davinci/board-omapl138-hawk.c |  96 +---
>>  arch/arm/mach-davinci/include/mach/da8xx.h  |   2 +-
>>  arch/arm/mach-davinci/usb-da8xx.c   |   3 +-
>>  drivers/usb/host/ohci-da8xx.c   | 111 
>> ++--
>>  include/linux/platform_data/usb-davinci.h   |  19 -
>>  6 files changed, 105 insertions(+), 223 deletions(-)
>
> Can you separate out the driver enhancement from the platform
> (mach-davinci) changes? They need to go through different trees.
>

Ok, i will do that,  (it might require intermediate code to have
the driver working on each patch)

> Thanks,
> Sekhar
>
>
--
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/RFT v2 12/17] USB: ochi-da8xx: Use a regulator for vbus/overcurrent

2016-10-25 Thread Sekhar Nori 
On Monday 24 October 2016 10:16 PM, ahas...@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 
> ---
>  arch/arm/mach-davinci/board-da830-evm.c |  97 
>  arch/arm/mach-davinci/board-omapl138-hawk.c |  96 +---
>  arch/arm/mach-davinci/include/mach/da8xx.h  |   2 +-
>  arch/arm/mach-davinci/usb-da8xx.c   |   3 +-
>  drivers/usb/host/ohci-da8xx.c   | 111 
> ++--
>  include/linux/platform_data/usb-davinci.h   |  19 -
>  6 files changed, 105 insertions(+), 223 deletions(-)

Can you separate out the driver enhancement from the platform
(mach-davinci) changes? They need to go through different trees.

Thanks,
Sekhar


--
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/RFT v2 12/17] USB: ochi-da8xx: Use a regulator for vbus/overcurrent

2016-10-25 Thread Axel Haslam 
On Tue, Oct 25, 2016 at 3:39 AM, David Lechner  wrote:
> On 10/24/2016 11:46 AM, ahas...@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 = < 109 0>;
   oc-gpio = < 36 0>;
   regulator-boot-on;
   enable-active-high;
   regulator-name = "vbus";
   regulator-min-microvolt = <500>;
   regulator-max-microvolt = <500>;
   };


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


>
>> @@ -163,7 +198,6 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd,
>> u16 typeReq, u16 wValue,
>>   u16 wIndex, char *buf, u16 wLength)
>>  {
>> struct device *dev  = hcd->self.controller;
>> -   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
>
>
> nit: unnecessary whitespace change
>
>> int temp;
>>
>> switch (typeReq) {
>
>
--
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/RFT v2 12/17] USB: ochi-da8xx: Use a regulator for vbus/overcurrent

2016-10-24 Thread David Lechner 

On 10/24/2016 11:46 AM, ahas...@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.





@@ -163,7 +198,6 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 
typeReq, u16 wValue,
  u16 wIndex, char *buf, u16 wLength)
 {
struct device *dev  = hcd->self.controller;
-   struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);


nit: unnecessary whitespace change


int temp;

switch (typeReq) {


--
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