Hi Masahiro,
On 23 January 2015 at 02:20, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Hi Simon,
On Mon, 19 Jan 2015 20:12:35 -0700 Simon Glass sjg@chromium.org wrote:
When using allocated platform data, allocate it when we bind the device. This makes it possible to fill in this information before the device is probed.
This fits with the platform data model (when not using device tree), since platform data exists at bind-time.
Signed-off-by: Simon Glass sjg@chromium.org
Looks like you have changed your mind to allocate platdata in device_bind() rather than device_probe().
Yes. In fact I think my attempts to avoid this were a little too heroic.
Could you explain why you think this should be done?
I might have missed something, but your motivation is still unclear to me.
I thought one reason is consistency with platform data.
But drv->ofdata_to_platdata() is still called from device_probe(), i.e. it is just like zero-filled memory is allocated at the binding stage. Filling it with real device properties is delayed until the device is probed. What is the difference from the before and what does it buy us?
Its disadvantage are clear:
- We need more malloc memory for devices that may or may not be used.
- The boot sequence becomes slower.
I want good reasons to compensate these disadvantages.
I tried to document the reasoning in the patches, but let me try to expand a bit. Hopefully this can provoke further comments / improvements.
The main motivation for me was that buses want to set up the platform data for their children before they are probed. In fact some children may never be probed. For example a SPI bus wants to know the chip select for each of its children.
At present we have to hunt around in the device tree to figure out which child is the right one, so we can probe it. Worse, the children's drivers (e.g. cros_ec on a SPI bus) have to be involved in setting themselves up. The device_probe_child() function was my attempt to make this fit better, and it did work, but I was not happy with it. You can see that from my unhappy comments in cros_ec, or SPI flash probe, for example.
The new approach makes buses easier to deal with as I hope you can see. The 'bind' step is supposed to set up the entire basic framework of the drivers at start-up. Everything should be visible in the tree (the exception being buses which must be probed at run-time) but nothing should be probed. Now, we are following that approach for buses also.
Re the disadvantages:
- the per-child platform data for a bus is small, and we need not bind devices which are disabled. So, a board should avoid having a lot of enabled devices which are never used - malloc() is very fast, it is the platform data setup that takes time. I agree this slows things down. But currently it only affects bus children, and only their basic information such as chip select.
The use of ofdata_to_platdata() is stil confined to when the device is actually probed. This helps to keep things fast in the common case where we don't need the platform data earlier.
I think we can refine this further, but what I have now feels a lot cleaner to me.
BTW, you missed to fix the comment in device_probe_child():
/* Allocate private data and platdata if requested */
OK.
Regards, Simon