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

[Abbarapu, Venkatesh]

Hi,

> -----Original Message-----
> From: Marek Vasut <marex at denx.de>
> Sent: Monday, October 7, 2024 5:39 PM
> 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 v5 1/7] usb: onboard-hub: Add reset-gpio support
> 
> On 10/7/24 6:45 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>
> > ---
> >   common/usb_onboard_hub.c | 49
> +++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/common/usb_onboard_hub.c b/common/usb_onboard_hub.c index
> > 68a04ac041..4317e81ed8 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);
> > +
> > +	/* property is optional, don't return error! */
> > +	if (!hub->reset_gpio)
> > +		return 0;
> > +
> > +	ret = dm_gpio_set_value(hub->reset_gpio, 1);
> > +	if (ret)
> > +		return ret;
> > +
> > +	udelay(data->reset_us);
> > +
> > +	ret = dm_gpio_set_value(hub->reset_gpio, 0);
> > +	if (ret)
> > +		return ret;
> > +
> > +	udelay(data->power_on_delay_us);
> > +
> > +	return 0;
> > +}
> > +
> >   static int usb_onboard_hub_probe(struct udevice *dev)
> >   {
> >   	struct onboard_hub *hub = dev_get_priv(dev); @@ -30,7 +67,11 @@
> > static int usb_onboard_hub_probe(struct udevice *dev)
> >   	if (ret)
> >   		dev_err(dev, "can't enable vdd-supply: %d\n", ret);
> >
> > -	return ret;
> > +	ret = usb_onboard_hub_reset(dev);
> 
> return usb_onboard_hub_reset(dev);
<Venkatesh> By adding the "return usb_onboard_hub_reset(dev);" we miss the code which  does i2c initialization sequence which is part of [PATCH v4 4/7] usb: onboard-hub: Add i2c  initialization for usb5744 hub of the series.


Thanks
Venkatesh