On Fri, Jul 29, 2022 at 03:07:47PM +0100, Mark Brown wrote: > On Fri, Jul 29, 2022 at 03:35:33PM +0200, Johan Hovold wrote: > > > I guess we just need to drop all those regulator-allow-set-load > > properties for now even if using DT for power-management configuration > > this way does seem to run against the whole DT-as-hardware-description > > idea (e.g. we may want to add them back when/if active- and idle loads > > are specified by the corresponding Linux drivers). > > Well, there's also a question of if the hardware can usefully make use > of the facility - is there any non-suspend state where the regulator > needs to be on but is drawing so little current that it's worth trying > to select a lower power mode? Good point. > > But that doesn't address the problem that was trying to highlight here, > > and that you had noticed years ago, namely that using set_load only > > works reliably if *all* consumers use it. > > > Shouldn't an enabled regulator from a consumer that didn't specify a > > load somehow result in HPM always being selected (e.g. count as INT_MAX > > load as Doug suggested some years ago)? > > Possibly, but note that as well as the consumers with software drivers > you also have to consider any passive consumers on the board which may > not have any representation in DT so the actual numbers may well be off > even if every consumer is trying to keep things up to date. You also > come back to the "let's just shove a random number in here" problem. Right, but some of that could be captured in DT with 'regulator-system-load'. > For ultimate saftey we probably want a command line option to gate the > feature which people can set to say they've audited their full > software/hardware integration stack. That sounds like it could be useful. > > At some point in the discussion I thought Mark suggested removing > > set_load from drivers that don't actually manage active and idle loads. > > That would also work, at least until the day one of the drivers adds > > support for idle loads. > > Yes, if the driver isn't actively managing loads it's probably not doing > anything useful. Ok, thanks for confirming. Perhaps we should drop the set_loads() added to the PHY driver by this series then. > The difficulties with this sort of system integration question is an > unfortunate consequence of DT, having to describe what's safe for an > unknown software stack is fundamentally hard. I do question how much > effort it's worth putting into enabling this, especially in cases where > the regulator is shared - how much power is actually saved in the grand > scheme of things given that this is only taking effect when the system > is out of suspend and we tend to be talking about some percentage of the > power being drawn on something which is presumably already consuming > very little power for this to be at all relevant? I tend to agree. Thanks again for your input! Johan
Attachment:
signature.asc
Description: PGP signature
