On 10/4/24 4:57 PM, Abbarapu, Venkatesh wrote:
Hi,
-----Original Message-----
From: Marek Vasut <ma...@denx.de>
Sent: Friday, October 4, 2024 7:22 PM
To: Abbarapu, Venkatesh <venkatesh.abbar...@amd.com>; u-boot@lists.denx.de
Cc: Simek, Michal <michal.si...@amd.com>; fabrice.gasn...@foss.st.com; git
(AMD-Xilinx) <g...@amd.com>
Subject: Re: [PATCH v3 1/7] usb: onboard-hub: Add reset-gpio support
On 10/4/24 2:03 PM, Abbarapu, Venkatesh wrote:
Hi,
-----Original Message-----
From: Marek Vasut <ma...@denx.de>
Sent: Friday, October 4, 2024 4:52 PM
To: Abbarapu, Venkatesh <venkatesh.abbar...@amd.com>;
u-boot@lists.denx.de
Cc: Simek, Michal <michal.si...@amd.com>;
fabrice.gasn...@foss.st.com; git
(AMD-Xilinx) <g...@amd.com>
Subject: Re: [PATCH v3 1/7] usb: onboard-hub: Add reset-gpio support
On 10/4/24 5:02 AM, Abbarapu, Venkatesh wrote:
Hi Marek,
-----Original Message-----
From: Marek Vasut <ma...@denx.de>
Sent: Thursday, October 3, 2024 10:35 PM
To: Abbarapu, Venkatesh <venkatesh.abbar...@amd.com>;
u-boot@lists.denx.de
Cc: Simek, Michal <michal.si...@amd.com>;
fabrice.gasn...@foss.st.com; git
(AMD-Xilinx) <g...@amd.com>
Subject: Re: [PATCH v3 1/7] usb: onboard-hub: Add reset-gpio
support
On 10/3/24 4:54 PM, Abbarapu, Venkatesh wrote:
Hi Marek,
-----Original Message-----
From: Marek Vasut <ma...@denx.de>
Sent: Thursday, October 3, 2024 5:38 PM
To: Abbarapu, Venkatesh <venkatesh.abbar...@amd.com>;
u-boot@lists.denx.de
Cc: Simek, Michal <michal.si...@amd.com>;
fabrice.gasn...@foss.st.com; git
(AMD-Xilinx) <g...@amd.com>
Subject: Re: [PATCH v3 1/7] usb: onboard-hub: Add reset-gpio
support
On 10/3/24 6:40 AM, Abbarapu, Venkatesh wrote:
Hi Marek,
Hi,
@@ -30,7 +40,24 @@ static int usb_onboard_hub_probe(struct
udevice
*dev)
if (ret)
dev_err(dev, "can't enable vdd-supply: %d\n", ret);
- return 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) {
if (!hub->reset_gpio)
return 0;
<Venkatesh> As reset_gpio is optional property, by returning 0
the i2c sequence
wont be executed.
The code in if (hub->reset_gpio) { ... } only toggles the GPIO reset ?
<Venkatesh> Yes...if(hub->reset_gpio) only toggles the GPIO reset.
So you can return 0 if reset_gpio is NULL and exit early . What's the problem ?
<Venkatesh> if reset_gpio is NULL by returning 0 we miss the below
code which
does i2c initialization sequence.
Need to toggle the gpio and then does the i2c sequence if the
reset_gpio is
present, else we do the i2c initialization sequence.
Please show me the i2c sequence that is missed in this patch hunk:
+ if (hub->reset_gpio) {
+ 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;
The i2c sequence change is part of [PATCH v4 4/7] usb: onboard-hub: Add i2c
initialization for usb5744 hub of the series
+ if (data->init) {
+ ret = data->init(dev);
+ if (ret) {
+ dev_err(dev, "onboard i2c init failed: %d\n", ret);
+ goto err;
+ }
+ }
+
return 0;
+err:
+ dm_gpio_set_value(hub->reset_gpio, 0);
+ return ret;
}
This does not seem to be part of the if (hub->reset_gpio) { ... } code block ?
Yes...this is not part of the if (hub->reset_gpio) {...} code block.
As I explained above if reset_gpio is NULL by returning 0 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.
The sequence is "Toggle the gpio and then do the i2c initialization sequence if
the
reset_gpio is present, else we do only the i2c initialization sequence".
In that case, pull the GPIO handling into a dedicated function:
usb_onboard_hub_reset()
{
reset_gpio = devm_gpiod_get_optional(...)
if (!reset_gpio)
return 0;
// toggle reset here
...
hub->reset_gpio = reset_gpio;
return 0;
}
usb_onboard_hub_probe()
{
...
ret = usb_onboard_hub_reset()
if (ret) ...
// do i2c stuff later
return 0;
}
