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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <71934373-8425-345b-7719-0903f846119f@gmail.com>
Date:   Wed, 11 Nov 2020 00:22:34 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Thierry Reding <thierry.reding@...il.com>
Cc:     Jonathan Hunter <jonathanh@...dia.com>,
        Alan Stern <stern@...land.harvard.edu>,
        Peter Chen <Peter.Chen@....com>,
        Mark Brown <broonie@...nel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Lee Jones <lee.jones@...aro.org>,
        Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Peter Geis <pgwipeout@...il.com>,
        Nicolas Chauvet <kwizart@...il.com>,
        linux-samsung-soc@...r.kernel.org, devel@...verdev.osuosl.org,
        linux-usb@...r.kernel.org, linux-pwm@...r.kernel.org,
        linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-media@...r.kernel.org, linux-tegra@...r.kernel.org
Subject: Re: [PATCH v1 07/30] soc/tegra: Add sync state API

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