Re: [v5,3/5] USB: ohci: da8xx: Allow a regulator to handle VBUS
From: Axel Haslam
Date: Tue Nov 22 2016 - 09:20:06 EST
On Mon, Nov 21, 2016 at 5:29 PM, David Lechner <david@xxxxxxxxxxxxxx> wrote:
> On 11/21/2016 04:22 AM, Axel Haslam wrote:
>>
>> Hi David,
>>
>> Thanks for the review,
>>
>
> You're welcome.
>
>>>>
>>>> @@ -160,15 +212,41 @@ static void ohci_da8xx_ocic_handler(struct
>>>> da8xx_ohci_root_hub *hub,
>>>> hub->set_power(port, 0);
>>>> }
>>>>
>>>> +static int ohci_da8xx_regulator_event(struct notifier_block *nb,
>>>> + unsigned long event, void *data)
>>>> +{
>>>> + struct da8xx_ohci_hcd *da8xx_ohci =
>>>> + container_of(nb, struct da8xx_ohci_hcd,
>>>> nb);
>>>> + struct device *dev = da8xx_ohci->hcd->self.controller;
>>>> +
>>>> + if (event & REGULATOR_EVENT_OVER_CURRENT) {
>>>> + dev_warn(dev, "over current event\n");
>>>
>>>
>>>
>>> Won't this result in duplicate overcurrent warnings in the kernel log? It
>>> seems like in previous version of this patch series, we would get an
>>> overcurrent error from the core usb driver.
>>
>>
>> you mean in the regulator driver? i did not make changes to core usb.
>> but, no, i did not add a print in the fixed regulator driver itself.
>> Since
>> the regulator is a separate driver, and could be implemented with or
>> without
>> a trace, i think its better to leave this print. It shows that the usb
>> driver
>> has well received the notification.
>>
>
> No, I mean in drivers/usb/core/hub.c. There is
>
> if (status & USB_PORT_STAT_OVERCURRENT)
> dev_err(&port_dev->dev, "over-current condition\n");
>
> and
>
> if (status & HUB_STATUS_OVERCURRENT)
> dev_err(hub_dev, "over-current condition\n");
>
> In ohci_da8xx_hub_control(), we are setting RH_PS_POCI and RH_PS_OCIC, so
> these messages will be printed via the core hub driver. We don't need to
> print another message from the same event.
>
>
Hi David
I did a couple of tests, and i don't get those prints do you get them?.
What i understand is that they happen when we get a hub event that is
reporting the over current. Which is what the root hub of the davinci system
does not have, and why we use gpios instead).
Regards,
Axel.
>