10.11.2020 23:32, Mark Brown пишет: > On Tue, Nov 10, 2020 at 09:29:45PM +0100, Thierry Reding wrote: >> On Thu, Nov 05, 2020 at 02:44:08AM +0300, Dmitry Osipenko wrote: > >>> + /* >>> + * Voltage scaling is optional and trying to set voltage for a dummy >>> + * regulator will error out. >>> + */ >>> + if (!device_property_present(dc->dev, "core-supply")) >>> + return; > >> This is a potentially heavy operation, so I think we should avoid that >> here. How about you use devm_regulator_get_optional() in ->probe()? That >> returns -ENODEV if no regulator was specified, in which case you can set >> dc->core_reg = NULL and use that as the condition here. > > Or enumerate the configurable voltages after getting the regulator and > handle that appropriately which would be more robust in case there's > missing or unusual constraints. > I already changed that code to use regulator_get_optional() for v2. Regarding the enumerating supported voltage.. I think this should be done by the OPP core, but regulator core doesn't work well if regulator_get() is invoked more than one time for the same device, at least there is a loud debugfs warning about an already existing directory for a regulator. It's easy to check whether the debug directory exists before creating it, like thermal framework does it for example, but then there were some other more difficult issues.. I don't recall what they were right now. Perhaps will be easier to simply get a error from regulator_set_voltage() for now because it shouldn't ever happen in practice, unless device-tree has wrong constraints.