[PATCH] usb: dwc3-generic: fix CONFIG_DM_REGULATOR-off case

[Caleb Connolly]

On 09/08/2024 07:19, Jan Kiszka wrote:
> On 08.08.24 16:27, Caleb Connolly wrote:
>> Hi Jan,
>>
>> On 08/08/2024 10:51, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka at siemens.com>
>>>
>>> When DM_REGULATOR is disabled, all calls will return -ENOSYS. Account
>>> for that so that targets like the IOT2050 will work again.
>>>
>>> Fixes: de451d5d5b6f ("usb: dwc3-generic: support external vbus
>>> regulator")
>>> Signed-off-by: Jan Kiszka <jan.kiszka at siemens.com>
>>> ---
>>>    drivers/usb/dwc3/dwc3-generic.c | 6 +++---
>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c
>>> b/drivers/usb/dwc3/dwc3-generic.c
>>> index a9ba315463c..2ab41cbae45 100644
>>> --- a/drivers/usb/dwc3/dwc3-generic.c
>>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>>> @@ -246,12 +246,12 @@ static int dwc3_generic_host_probe(struct
>>> udevice *dev)
>>>            return rc;
>>>          rc = device_get_supply_regulator(dev, "vbus-supply",
>>> &priv->vbus_supply);
>>> -    if (rc)
>>> +    if (rc && rc != -ENOSYS)
>>>            debug("%s: No vbus regulator found: %d\n", dev->name, rc);
>>>    -    /* Only returns an error if regulator is valid and failed to
>>> enable due to a driver issue */
>>> +    /* Does not return an error if regulator is invalid - but does so
>>> when DM_REGULATOR is disabled */
>>>        rc = regulator_set_enable_if_allowed(priv->vbus_supply, true);
>>> -    if (rc)
>>> +    if (rc && rc != -ENOSYS)
>>
>> regulator_set_enable_if_allowed() will return 0 if the call to
>> regulator_set_enable() returns -ENOSYS
>>
>> Maybe it would make sense to have the stub implementation of
>> regulator_set_enable_if_allowed() return 0?
>>
> 
> Possible. Would that be the only case where a stub should not return
> ENOSYS? All do so far.
> 
>> Somewhat confusing to check for -ENOSYS here imo, since it isn't really
>> obvious when that would be the case.
> 
> But that would still leave is a with a misleading message, even if it is
> just a debug output. The absence of a regulator is not per se a bug to
> my understanding.

Ah good point.

Alternatively, maybe it would be easier to gate this code block with if 
(CONFIG_IS_ENABLED(DM_REGULATOR)?

But yeah maybe fine as is.

Reviewed-by: Caleb Connolly <caleb.connolly at linaro.org>
> 
> Jan
> 
>>>            return rc;
>>>          hccr = (struct xhci_hccr *)priv->gen_priv.base;
>>
> 

-- 
// Caleb (they/them)