On Monday, January 02, 2012, Thomas Abraham wrote: > Hi Rafael, > > On 29 December 2011 03:47, Rafael J. Wysocki <rjw@xxxxxxx> wrote: > > On Wednesday, December 28, 2011, Thomas Abraham wrote: > >> Hi Mark, Rafael, > > > > Hi, > > > >> On 27 December 2011 02:14, Rafael J. Wysocki <rjw@xxxxxxx> wrote: > >> > On Monday, December 26, 2011, Mark Brown wrote: > >> >> On Mon, Dec 26, 2011 at 08:13:19PM +0100, Rafael J. Wysocki wrote: > >> >> > On Monday, December 12, 2011, Thomas Abraham wrote: > >> >> > >> >> > > A device node pointer is added to generic pm domain structure to associate > >> >> > > the domain with a node in the device tree. > >> >> > >> >> > That sounds fine except for one thing: PM domains are not devices, so adding > >> >> > "device node" pointers to them is kind of confusing. Perhaps there should be > >> >> > something like struct dt_node, representing a more general device tree node? > >> >> > >> >> There's struct of_node which is exactly that, though practically > >> >> speaking you need a device if you're going to bind automatically to > >> >> something from the device tree in a sensible fashion and there is actual > >> >> hardware under there so a device does make some sense. > >> > >> In patch 2/2 of this series, the platform code looks for nodes in > >> device tree that represent a power domain. When a power domain node is > >> found, a generic power domain is instantiated with pm_genpd_init() > >> using the information available from the node in device tree. There is > >> no automatic binding required in this case. The power domain node does > >> represent a hardware that manages the power domain. > > > > Good. So would it be possible to use struct of_node instead of > > struct device_node in struct generic_pm_domain? > > Sorry, I used 'struct of_node' and 'struct device_node' > interchangeably in my reply. All the nodes in a device tree are > represented by 'struct device_node'. > > > > >> >> > >> >> This is in part compatibility with the existing Exynos code which uses > >> >> devices to probe the domains for non-DT systems. > >> > > >> > Well, that's not a general case, though. > >> > > >> > It doesn't feel approporiate to use a "device node" pointer for something > >> > that's not based on struct device, at least not a generic level, so I wonder > >> > if there's a different way. > >> > >> A device node pointer or of_node pointer is a simple pointer to a > >> instance of a node in device tree. All nodes in a device tree need not > >> represent a corresponding 'struct device'. A node in device tree can > >> described a hardware feature such as a power domain supported in the > >> hardware. > > > > Sure. > > > >> The addition of device tree support for generic power domains in this > >> patchset is generic for all platforms. The platform code instantiates > >> generic power domains from device tree with the of_node pointer > >> assigned to 'struct generic_pm_domain'. Then, in > >> __pm_genpd_add_device(), given a of_node pointer (to gen_pd), it is > >> possible to find a matching power domain to select. > > > > My point was that adding the struct device_node pointer to > > struct generic_pm_domain didn't look good, because that structure didn't > > represent a device in general. While I understand that it may be regarded > > as a "device object" on some platforms, there are platforms that don't > > regard PM domains as devices. For this reason (and only for this reason) > > it appears to be more appropriate to use a more generic device tree node > > type for struct generic_pm_domain. > > If a platform uses some hardware controls (register read/writes) to > enable/disable/control power domain, then it can be represented in the > device tree. But such a node need not represent a 'struct device'. > Other nodes in the device tree can then reference the power domain > node. > > If a platform does not have any hardware knobs for controlling power > domains, then probably it will not be represented in the device tree. > But I am not sure of this case. > > The 'struct device_node' is a representation of a instance of a device > tree node. It can represent nodes that are not devices. Hence, it can > be used to represent a power domain in a device tree and also included > in the 'struct generic_pm_domain'. OK, so please address my second comment regarding the $subject patch, i.e. that you might add a wrapper around the original __pm_genpd_add_device() instead of modifying __pm_genpd_add_device() itself. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |