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

Simon Glass sjg at chromium.org
Sat Jan 24 15:23:07 CET 2015


Hi Masahiro,

On 23 January 2015 at 22:04, Masahiro YAMADA <yamada.m at jp.panasonic.com> wrote:
> Hi Simon,
>
>
> 2015-01-24 0:50 GMT+09:00 Simon Glass <sjg at chromium.org>:
>
>>
>> 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.
>
> OK.
> I understand that this concept makes things much simpler, so
>
> Reviewed-by: Masahiro Yamada <yamada.m at jp.panasonic.com>
>

OK I will respin this series.

>
> Perhaps, we can have a flag like DM_FLAG_ALLOC_PLATDATA_LATE:
>  If this flag is set, platdata is allocated in device_probe(), not
> device_bind().
> But, I think it might make things much more complicated and
> probably make you unhappy.
> I do not have a strong option about this.
>

Well I'm keen on anything that makes it more efficient, but yes if we
can keep it simple we should do that for now. Let's see how things pan
out.

>
> Let's go with your idea!
> If something does not go well, then we can come back here.
>
>

Yes we can.

>
>
>> 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.
>
> OK!
>
>
>
> I have done with a quick review of this series.
>
> I left some comments on some patches,but they are all minor issues.
>
> If you agree to fix them, please send v3.
> I am OK with the whole concept, so I guess we can get it in during  this MW.

Will do. Thanks very much for going through this in detail.

Regards,
Simon


More information about the U-Boot mailing list