[U-Boot] [PATCH v2 06/26] dm: core: Allocate platform data when binding a device

[Simon Glass]

Hi Masahiro,

On 23 January 2015 at 02:20, Masahiro Yamada <yamada.m at jp.panasonic.com> wrote:
> Hi Simon,
>
>
> On Mon, 19 Jan 2015 20:12:35 -0700
> Simon Glass <sjg at 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 at 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