Re: [PATCH v1 07/30] soc/tegra: Add sync state API

Dmitry Osipenko
2020-11-10T21:22:44Z

10.11.2020 23:47, Thierry Reding пишет:
...
> tegra_soc_for_each_device
>
> I wonder if you copy/pasted this or if you got really lucky to mistype
> this all three times.

Copied of course :)

I added a special spell checking rule for this typo, but it does help
reliably.

...
>> + terga_soc_for_each_device(soc_dev) {
>> + do {
>> + /*
>> + * Devices like display controller have multiple
>> + * instances with the same compatible. Hence we need
>> + * to walk up the whole tree in order to account those
>> + * multiple instances.
>> + */
>> + np = of_find_compatible_node(prev_np, NULL,
>> + soc_dev->compatible);
>> + of_node_put(prev_np);
>> + prev_np = np;
>> +
>> + if (of_device_is_available(np)) {
>> + pr_debug("added %s\n", soc_dev->compatible);
>> + soc_dev->sync_count++;
>> + }
>> + } while (np);
>
> Maybe use for_each_compatible_node() for that inside loop?

Good point! I think there is actually an of_node_put() bug in current
variant, which for_each_compatible_node() would safe from.

>> + }
>> +
>> + return 0;
>> }
>> +postcore_initcall_sync(tegra_soc_devices_init);
>
> This is unfortunate. I recall having this discussion multiple times and
> one idea that has been floating around for a while was to let a driver
> bind against the top-level "bus" node. That has the advantage that it
> both anchors the code somewhere, so we don't have to play this game of
> checking for the SoC with soc_is_tegra(), and it properly orders this
> with respect to the child devices, so we wouldn't have to make this a
> postcore_initcall.
>
> Might be worth looking at that again, but for now this seems okay.

Thanks