On Mon, 7 Oct 2013, Tony Lindgren wrote: > And assuming Paul is OK with these patches in general. Actually, I have several concerns with this series, as you and I discussed. Some of us have been talking them over with other folks for the past few months to try to figure out what to do. Most of the concerns have fairly easy technical solutions, but we shouldn't merge these patches until they're resolved. The issues are: 1. whether the clock data should go into DT 2. what the right place for DT clock data is in the DT 3. whether it makes sense to merge "experimental" DT bindings in this case 4. where clockdomain data belongs The first issue - and the one I'm least concerned about - is that, in my view, it still does not make technical sense to move this data into DT. This is chip-level hardware data that never needs to be changed by board designers or end users. Adding it to DT is going to result in a boot-time cost (due to DT parse overhead) and additional memory consumption for very little benefit, compared to an implementation that places this data into a dynamically loadable module. For some users, the boot-time bloat is a big deal: the example that's been mentioned to me recently is an automotive back-up camera that needs to cold-boot to complete functionality in a few hundred microseconds. However, according to some other upstream maintainers, Linus's goal is to move most of the device-specific static data out of the kernel tree, into DT files (in the ARM case). If that non-technical constraint is indeed the dominant consideration, then I agree that moving this data to DT is the only viable course of action. ... The second issue concerns where the SoC clock nodes should go in the DT. In these patches, the clock data has been encoded in a large "clocks" node off the top level. This is almost certainly not the right thing to do from a device hardware point of view. These clocks don't exist as standalone devices with their own address spaces decoded on the interconnect. In all of the SoC cases that I'm aware of, clock control registers are part of larger IP blocks. For example, in the OMAP case, most of the system integration clock control registers are part of the OMAP-specific PRCM block, PRM block, or CM block. Then there are other device-specific clocks, like DSS PLLs or UART dividers. The control registers for these are generally located in the subsystem or device IP block itself, and are inaccessible when the subsystem or IP block is disabled. These device-specific clocks belong to the IP block, not the SoC integration. So, for example, if two SoCs use the same IP block, then the clock registers, and their offsets from the IP block address space base, are likely to be identical. So in my view, the right things to do here are to: 1. associate SoC DT clock data with the device node that contains the clock control registers 2. specify relative offsets for clock registers from the start of the IP block address range, rather than absolute addresses for clock registers 3. place the responsibility for registering clocks into the IP block drivers themselves This naturally separates clocks into per-IP block DT files. It also provides the CCF with an easy way to ensure that the device that encloses the clock is enabled and accessible by the CPU core, before trying to access registers inside. Similarly, non-SoC off-chip clock data (such as for dedicated I2C PLLs) should also be associated with their I2C device node. Making these changes to Tero's existing patches should be relatively straightforward, based on what I've seen. ... Regarding the third issue: let's postulate for the moment that the clock binding issues that I mention in #2 above are ignored (ha!), and that the existing DT clock data is merged. These bindings are currently marked as "Unstable - ABI compatibility may be broken in the future". What happens if, when we meet to discuss clock bindings at the kernel summit, we decide that significant changes are needed? We could easily wind up with kernels that won't boot at all when used with newer DT data. Not to mention, merging such a large amount of code and data before the bindings are stable will increase the risk of massive churn as the bindings evolve towards stability. So IMHO, the way to fix this is to consider the clock data to be IP-block specific, as mentioned in #2. Then there's no need for global clock bindings for the SoC clock cases. Otherwise, it seems prudent to at least wait until the global clock bindings are no longer marked as unstable. The whole DT migration was predicated on the ideas of reducing churn in the Linux codebase, and preserving forward compatibility for DT data. We shouldn't discard these goals just to merge something a little sooner. ... The fourth issue is where the clockdomain data should go. The basic issue here is that the notion of "clockdomains" here is completely OMAP-specific. OMAP clockdomains demarcate a group of clocks or IP blocks that share the same automatic idle behavior, controlled by their enclosing IP block (e.g., PRCM, PRM, CM, etc.). Clockdomains are also used in the OMAP kernel code to link the clocks to their enclosing power domains, voltage rail control, etc. So since these are OMAP PRCM/CM/PRM-specific constructs, the right place for OMAP clockdomain data is underneath the OMAP-specific CM/PRCM DT nodes. Again this should be an easy change to Tero's existing patches. ... So hey, if you've made it this far, thanks for reading. My sense is that implementing #2 and #4 are relatively straightforward. Also, since this seems to have been a problem for some folks in the past, I want to make clear that I think both Mike and Tero have been doing good jobs on the CCF and OMAP clock patches so far. regards - Paul -- 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
