Hi, On 06/19/2014 04:03 PM, Hans de Goede wrote: > Hi, > <snip> > 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 > undesirable. > > 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. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
