On Thu, Aug 29, 2019 at 11:09:34AM +0100, Andrew Murray wrote: > But why do we return -ENODEV and not NULL for OPTIONAL_GET? Why would we return NULL? The caller is going to have to check the error code anyway since they need to handle -EPROBE_DEFER and NULL is never a valid thing to use with the regulator API. > Looking at some of the consumer drivers I can see that lots of them don't > correctly handle the return value of regulator_get_optional: This API is *really* commonly abused, especially in the graphics subsystem which for some reason has lots of users even though I don't think I've ever seen a sensible use of the API there. As far as I can tell the graphics subsystem mostly suffers from people cut'n'pasting from the Qualcomm BSP which is full of really bad practice. It's really frustrating since I will intermittently go through and clean things up but the uses just keep coming back into the subsystem. > Given that a common pattern is to set a consumer regulator pointer to NULL > upon -ENODEV - if regulator_get_optional did this for them, then it would > be more difficult for consumer drivers to get the error handling wrong and > would remove some consumer boiler plate code. No, they'd all still be broken for probe deferral (which is commonly caused by off-chip devices) and they'd crash as soon as they try to call any further regulator API functions. > I guess I'm looking here for something that can simplify consumer error > handling - it's easy to get wrong and it seems that many drivers may be wrong. The overwhelming majority of the users just shouldn't be using this API. If there weren't devices that absolutely *need* this API it wouldn't be there in the first place since it seems like a magent for bad code, it's so disappointing how bad the majority of the consumer code is.
Attachment:
signature.asc
Description: PGP signature
