[RFC 1/2] pwrseq: Add subsystem to handle complex power sequences
ulf.hansson at linaro.org
Tue Jul 1 09:33:29 PDT 2014
On 19 June 2014 16:23, Hans de Goede <hdegoede at redhat.com> wrote:
> On 06/19/2014 04:03 PM, Hans de Goede wrote:
>> Also I'm not sold on how you're making this a devm only thing, and
>> are using devres_alloc to not only allocate memory for resource tracking,
>> but also the actual backing struct, that is not how devres_alloc is
>> intended to be used AFAIK.
>> A problem with having this one always devm managed is that it makes it
>> hard to use in library functions without side effects.
>> e.g. if you look at how your using this in mmc_of_parse in the next
>> patch, this gives mmc_of_parse the side-effect of having allocated
>> and bound the power-seq method, even if mmc_of_parse later
>> fails on e.g. gpio binding. If then for some reason the mmc host
>> probe method decides to not propagate the mmc_of_parse method
>> error (leading to freeing of all devm managed resources), then this is
>> Where as with a non devm version, it would be clear that on error
>> mmc_of_parse would need to explicitly release the pwrseq again.
>> I realize that pwrseq implementations likely will want to use
>> devm functions too, and I'm a great fan of devm. But this is something
>> to keep in mind. At a minimum the description of of_mmc_parse needs
>> to get updated with info about it having potential side-effects even
>> when it fails, and that failure should always be treated as a fatal
>> error and cause the host probe method to fail.
> Thinking more about this, it may be a good idea to give the pwrseq
> its own struct device, turning it into a virtual device, this way the
> pwrseq-method can use devm managed resources bound to this device,
> we can set the of_node of this device to the actual powerseq
> childnode and it gets its own sysfs dir in which we could do useful things
> such as have an attribute to query the current power state.
> This would also mean introducing a non devm version of devm_pwrseq_get
> + an explicit release, which would be useful to avoid the side-effects
> mentioned above when used in library functions such as mmc_of_parse.
Hans, thanks for your comments. I will try to include all your ideas in a v2.
More information about the linux-arm-kernel