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 <[email protected]> >>>> >>>> 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 <[email protected]> >>>> --- >>>> 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 <[email protected]>
Can this regression fix be merged now, or should I add that gating? Jan -- Siemens AG, Technology Linux Expert Center
