Hi Stephen,
On 3 June 2014 12:33, Stephen Warren swarren@wwwdotorg.org wrote:
On 06/03/2014 10:04 AM, Simon Glass wrote:
+Stephen
I don't think there's anything actionable for me in this email, although I guess I'll chime in on a couple of points:
I agree that the current way U-Boot parses DT is completely inadequate. The only way to parse it is to take a top-down recursive approach, with each node's driver initiating the parsing of any relevant child nodes. In other words, exactly how Linux (and likely *BSD, Solaris, ...) do it.
See the other thread - that has been my intention all along and is why I avoided adding this to driver model. I've ended up with a helper function only in the implementation I'm fiddling with at present.
I really don't understand the hang up with GPIOs. Here are the possible HW situations as I see them:
A single GPIO controller HW module, represented as a single DT node.
This should be: One node in DT. One DM device. One bind call (assuming that's the equivalent of Linux's probe()).
A set of completely separate HW modules, each handling N GPIOs.
This should be: N nodes in DT. N DM devices. N bind calls. 3)
A single HW module that's represented in DT as a top-level node for the HW module and arbitrarily has N child nodes for some arbitrary bank concept within the HW module:
This should be: 1 (top-level) node in DT, N child nodes in DT, 1 or N DM devices, 1 bind call (for just the top-level node). The bind call can choose whether it creates 1 single DM device object for the top-level node, or 1 for each of the child node that it manually parses without additional bind calls. That's an implementation detail in the driver.
Note that Tegra should fall into case (1) above. I'm not familiar enough with Exynos HW (which was mentioned in the email I'm replying to but didn't bother quoting) to have an opinion re: which approach is most suitable for it.
Thanks for this summary which is useful.
device_bind() is how child devices are created, so I don't think we want to avoid using that. What's the point? How else are we going to allocate a device?
I've basically settled on option 3 for now, with the device defined as a 'GPIO bank'. We then put the banks together (each can be named) to support all GPIOs on the SoC. Exynos happens to have pinctrl definitions for each bank, so we can iterate through these calling device_bind() for each bank. But note that only the top-level pinctrl has a compatible string, so we cannot call device_probe() on the banks - they have no compatible string so don't exist as far as driver model is concerned. Anyway they aren't top-level nodes.
Tegra doesn't have much in the device tree for GPIOs - it seems to be all hard-coded in the software. So I ended up with the code you saw which just iterates over a known number of banks, creating a device for each.
I don't want to create a separate data structure for 'gpio chip' like Linux for reasons I think I mentioned (briefly it adds a level of indirection, creates an unnecessary structure, hides that structure from 'dm tree' and the like, and sets a precedent of lots of little private data structures that are opaque to the poor user looking at what is in the system). Or at least I'd like to delay that until it is strictly necessary. Let's keep it all visible to driver model, and also save having code we really don't need.
I hope that clarifies things.
Regards, Simon