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

[Dmitry Osipenko]

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