linux-kernel - Re: [PATCH RFC] timer: of: Create a platform_device before the framework is initialized

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] [day] [month] [year] [list]

Message-ID: <44062b00-8903-49e5-922e-aed68c377c70@linaro.org>
Date: Wed, 25 Jun 2025 14:25:36 +0200
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Bryan O'Donoghue <bryan.odonoghue@...aro.org>, gregkh@...uxfoundation.org
Cc: linux-kernel@...r.kernel.org, lorenzo.pieralisi@...aro.org,
 Hans de Goede <hansg@...nel.org>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 Rob Herring <robh@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
 Arnd Bergmann <arnd@...db.de>, John Stultz <jstultz@...gle.com>,
 Stephen Boyd <sboyd@...nel.org>, Saravana Kannan <saravanak@...gle.com>,
 "open list:GENERIC INCLUDE/ASM HEADER FILES" <linux-arch@...r.kernel.org>,
 "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE"
 <devicetree@...r.kernel.org>
Subject: Re: [PATCH RFC] timer: of: Create a platform_device before the
 framework is initialized


Hi Bryan,

thanks for your review

On 25/06/2025 12:20, Bryan O'Donoghue wrote:

[ ... ]

>> +
>> +    for_each_matching_node_and_match(np, __timer_pdev_of_table, 
>> &match) {
>> +        if (!of_device_is_available(np))
>> +            continue;
>> +
>> +        init_func = match->data;
>> +
>> +        pdev = platform_device_alloc(of_node_full_name(np), -1);
>> +        if (!pdev)
>> +            continue;
> 
> Shouldn't this be return -ENOMEM;
> 
>> +
>> +        ret = device_add_of_node(&pdev->dev, np);
>> +        if (ret) {
>> +            platform_device_put(pdev);
>> +            continue;
> 
> Why is this a continue ? you can get -EINVAL and -EBUSY from 
> device_add_of_node() - can/should you really continue this loop after an 
> -EINVAL ?
> 
> Understood for architected you want to keep going and get the system up 
> at the very least I'd add a noisy message about it.

Yes, that's correct. If we bail out on failure, the system will hang. 
The platform can have more timers which will work allowing the system to 
boot.

It is expected to have the driver probe function to print a message on 
error, so adding one message seems to be redundant.

If timer_probe() fails to initialize a timer (the number of initialized 
driver is zero) then an error will be emitted.


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog