On 30.09.24 06:54, Jan Kiszka wrote:
> On 09.08.24 17:19, Caleb Connolly wrote:
>>
>>
>> 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.kis...@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.kis...@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.conno...@linaro.org>
>
> Can this regression fix be merged now, or should I add that gating?
>
This now also missed the v2024.10 release - can we please merge it now?
Jan
--
Siemens AG, Technology
Linux Expert Center
