On Tuesday, July 24, 2012, Arnd Bergmann wrote: > On Tuesday 24 July 2012, Rafael J. Wysocki wrote: > > On Tuesday, July 24, 2012, Arnd Bergmann wrote: > > > On Saturday 21 July 2012, Rafael J. Wysocki wrote: > > > > > > > Sorry for taking so long to reply. I am really not that familiar with the > > > power domain requirements, but I do have two comments on your approach: > > > > > > * I think when we want to add a generic concept to the device tree such > > > as power domains, we should always make it specified in a generic way. > > > > Do we really want that? I'm a bit skeptical, because apparently nobody > > cares, as the (zero) response to this patchset evidently indicates and > > since nobody cares, it's probably better not to add such "generic" things > > just yet. > > Well, the trouble with bindings is that they are much harder to change > later, at least in incompatible ways.
Hmm, so I think you wanted to say that it might be burdensome to retain the code handling the old binding once we had started to use a new generic one. I can agree with that, but that's quite similar to user space interfaces. Once we've exposed a user space interface of some kind and someone starts to use it, we'll have to maintain it going forward for the user in question. However, there is a way to deprecate old user space interfaces and it has happened. In this particular case the burden would be on Renesas, but I don't think it would affect anybody else. > > > You have used the "renesas,pmdomain" attribute, which is specific to > > > one vendor and requires platform specific code to read it. > > > Is there any reason not to put the code into the generic > > > drivers/base/power/domain.c file (or near it) and make a binding that > > > works for everyone? > > > > Yes, there is. A couple of them in fact. > > > > First off, power domains will always need platform-specific code to support > > them, no matter what. The problem is that they tend to require special > > handling, even within the same SoC, let alone different SoCs, and that > > handling > > has to be implemented in an SoC-specific way, because it has to know various > > things about the platform (like what to write to what register(s) at what > > time > > and so on). Of course, that platform-specific code needs to know which > > domain the given description corresponds to and there doesn't seem to be any > > useful way to specify that through DTs. > > We have the same problem for a lot of other subsystems: clock, regulator, > irq, gpio, pinctrl, dma, ... > > In each of these cases, we want a driver to be able to associate some > property with a driver (or platform code) from another subsystem. > We try to handle those using generic subsystem code that interprets > regular property names. For power domains those properties would be SoC-specific. That is, there may be a different set of properties for each SoC in principle and it's going to get quite messy relatively quickly. > > Second, the generic code needed to support such a binding would have to be > > quite complex and I don't see any advantage from adding it. > > The generic binding would only need to specify what the property > looks like and how the possible values can be determined. We don't > need to make that code generic right away but could wait until we > have a couple of implementations before we pull out the common bits. > > The important part to me is writing the binding in a way that allows > us to do this. Admittedly, I have a little experience with writing DT bindings. The regulator bindings look like we could do something similar for power domains, but for one I don't want platform device objects to be created for them in any case (that would make as much sense as creating a platform device for a bus type). > > > > > * Mark suggested two options (string and phandle), and (you guessed it), > > > > To me, the Mark's suggestion was to follow the example of TI hwmods, which > > I did. In fact, the power domains on Renesas platforms are an analogous > > concept, so in my opinion handling them in analogy with hwmods makes a lot > > of > > sense. > > I had never seen the hwmod binding before and if I had I would have > given them the same comments. I definitely don't think we should be > using it as a good example for common code. Well, good to know. :-) > > > IMHO using phandle makes it much easier to do this in a generic way, > > > without having to document every possible string that this can be > > > set to in the binding document. Obviously the phandle requires having > > > a node in the device tree for each domain, but I think having that > > > node is actually an advantage because it lets you describe the > > > hierarchy of the domains there. > > > > I don't quite see how it is better. I could (and should in fact) represent > > that hierarchy as an array of pairs of strings without adding any special > > parsing code to the kernel (except for a simple routine walking that table > > and calling pm_genpd_add_subdomain_names() for every pair). > > > > The only disadvantage of the current approach I can see is that whoever > > creates a dts for a board based on the given SoC has to know the names of > > the > > power domains to use in there. I don't think this is an overly burdensome > > requirement. > > Well, it does mean that we need to maintain a separate binding document > for each soc that has its own set of possible values. I'm not sure we'll be able to avoid that anyway. > > The other way around, the platform code supposed to handle the power domains > > would need a way to match DT nodes against the domains, which also would > > require some string-based identification and that would have to be > > documented > > as well. > > The part that matches the pm-domain device nodes can and should be > specific to the platform implementing the pm-domain. We can easily > do the same thing we did for regulators, where each regulator can > be either identified through its "reg" property in cases where that > makes sense or through a "regulator-compatible" property in cases > where we need a string representation. Where can I find the code that parses the regulator bindings? Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
