[PATCH v9 1/7] usb: onboard-hub: Add reset-gpio support

[Marek Vasut]

On 11/7/24 7:05 AM, Abbarapu, Venkatesh wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Marek Vasut <marex at denx.de>
>> Sent: Wednesday, November 6, 2024 6:04 AM
>> To: Abbarapu, Venkatesh <venkatesh.abbarapu at amd.com>; u-boot at lists.denx.de
>> Cc: Simek, Michal <michal.simek at amd.com>; fabrice.gasnier at foss.st.com; git
>> (AMD-Xilinx) <git at amd.com>
>> Subject: Re: [PATCH v9 1/7] usb: onboard-hub: Add reset-gpio support
>>
>> On 10/30/24 7:43 AM, Venkatesh Yadav Abbarapu wrote:
>>> As part of the reset, sets the direction of the pin to output before
>>> toggling the pin. Delay of millisecond is added in between low and
>>> high to meet the setup and hold time requirement of the reset.
>>>
>>> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu at amd.com>
>>> Reviewed-by: Marek Vasut <marex at denx.de>
>>> ---
>>>    common/usb_onboard_hub.c | 42
>> +++++++++++++++++++++++++++++++++++++++-
>>>    1 file changed, 41 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c index
>>> 68a04ac041..4174575893 100644
>>> --- a/common/usb_onboard_hub.c
>>> +++ b/common/usb_onboard_hub.c
>>> @@ -7,14 +7,51 @@
>>>     * Mostly inspired by Linux kernel v6.1 onboard_usb_hub driver
>>>     */
>>>
>>> +#include <asm/gpio.h>
>>>    #include <dm.h>
>>>    #include <dm/device_compat.h>
>>> +#include <linux/delay.h>
>>>    #include <power/regulator.h>
>>>
>>>    struct onboard_hub {
>>>    	struct udevice *vdd;
>>> +	struct gpio_desc *reset_gpio;
>>>    };
>>>
>>> +struct onboard_hub_data {
>>> +	unsigned long reset_us;
>>> +	unsigned long power_on_delay_us;
>>> +};
>>> +
>>> +int usb_onboard_hub_reset(struct udevice *dev) {
>>> +	struct onboard_hub_data *data =
>>> +		(struct onboard_hub_data *)dev_get_driver_data(dev);
>>> +	struct onboard_hub *hub = dev_get_priv(dev);
>>> +	int ret;
>>> +
>>> +	hub->reset_gpio = devm_gpiod_get_optional(dev, "reset",
>>> +						  GPIOD_IS_OUT |
>> GPIOD_ACTIVE_LOW);
>> Is the reset GPIO always active low or should this respect the GPIO flags in DT
>> instead ?
> Yes...the reset GPIO is always active low.
How did you arrive at that conclusion ?

I can very much predict there will show up a hub which expects the reset 
to be active HIGH and this will have to be removed, so better remove it now.