On 2/12/26 4:30 PM, Siddharth Vadapalli wrote:
On 12/02/26 8:12 PM, Marek Vasut wrote:
On 2/5/26 2:11 AM, Siddharth Vadapalli wrote:
[...]
+static enum usb_dr_mode cdns3_get_otg_mode(ofnode node)
+{
+ struct cdns3 cdns, *cdnsp;
+ void __iomem *otg_regs;
+ fdt_addr_t otg_addr;
+ int otg_reg_index;
+ int vbus;
+
+ otg_reg_index = ofnode_stringlist_search(node, "reg-names",
"otg");
+ if (otg_reg_index < 0)
+ return USB_DR_MODE_OTG;
This is wrong, the function should return error and possibly mode
via function parameter , do not conflate return value and mode please .
Regarding returning an error, the patch maintains status quo for the
case where we are unable to read the USB OTG Status register. Please
refer to the following diff in the patch:
@@ -413,6 +462,10 @@ int cdns3_bind(struct udevice *parent)
if (dr_mode == USB_DR_MODE_UNKNOWN)
dr_mode = usb_get_dr_mode(dev_ofnode(parent));
+ /* Use VBUS Valid to determine role */
+ if (dr_mode == USB_DR_MODE_OTG)
+ dr_mode = cdns3_get_otg_mode(node);
+
switch (dr_mode) {
#if defined(CONFIG_SPL_USB_HOST) || \
(!defined(CONFIG_XPL_BUILD) && defined(CONFIG_USB_HOST))
Only if the 'dr_mode' is already 'OTG', the newly added function
cdns3_get_otg_mode() is invoked. In case cdns3_get_otg_mode() cannot
determine the mode (otg status register cannot be read), we return
otg mode to maintain existing behavior, following which, the code
will use the CONFIGS to determine the role.
Returning an error will alter existing behavior rather than extending
it for enhancing functionality.
Regarding passing the mode via a function parameter, I could do so,
but I followed the implementation of other functions such as
usb_get_dr_mode()
which 'returns' the mode rather than using a function parameter to
fill in the mode.
Please let me know what you think.
I think this patch is wrong, cdns3_get_otg_mode() should return an error
Ok, I will update it to return an error, but please note that it may
alter existing behavior since the code is being introduced in the middle
of cdns3_bind() and not at the end of it.
code and also return mode via parameter. This:
static int cdns3_get_otg_mode(ofnode node, enum usb_dr_mode *mode)
{
...
if (error)
return -ESOMETHING;
...
*mode = some mode
return 0;
}
Do not conflate mode and error code, this leads to problems.
Ok. I will pass the mode as parameter to the function. Thank you for the
clarification.
Thank you
