From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 75BA1C98302 for ; Tue, 22 Sep 2026 08:56:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=L+8BAFkPpkiGg/g+Cl/d9APJK8MmFCn7jQfvjZ5jFUw=; b=IWmMYoRmfjRnmC TDa89AnHr9h9o25xo5kcTIftMNjKLxNs13rgY57zUXgZw4ZhmVj09aW6IWX5V57fycbayJcnNl9xq ECipK6JVSKh6EL37KevyBQ4P76UqHgXyPvPxSnAViTDoqzQIW35wgCZ+Q6Q5ENXhoG4GgxCab97pi vey/RPMMLXMU6bVlz1izgx0ZvcM+yQ9w49h1zL9MXvPzR2heVA2FdesKDQMbAPNTGYj2k3RNvKAb7 QJ6FCfi146pLpRYFff95xTjAhwL27To22U6+2BD9RyQCj0VBRjX7ffRdwxA/P0Skc6WEl8LJfpHBg fI4+4eLoo8XRTHkN/qCg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1x8wIi-00000004n2b-0eH8; Tue, 22 Sep 2026 08:56:32 +0000 Received: from mail-wr2-x23.google.com ([2a00:1450:4864:30::23]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1x8wIg-00000004n13-1AHQ for linux-rockchip@lists.infradead.org; Tue, 22 Sep 2026 08:56:31 +0000 Received: by mail-wr2-x23.google.com with SMTP id ffacd0b85a97d-482e1b55da9so25286f8f.2 for ; Tue, 22 Sep 2026 01:56:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1790067388; x=1790672188; darn=lists.infradead.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to:content-type; bh=Wq2LoFmFiObS5jB4OwtPqD7h4w1zA3ahilWAP6xrcps=; b=M/Zyxq0sy4yag5UwoEbhIU5OZ0bUKKjlujVT5BmYpr7B7+O5iPJtHInEkWYL62RXGh O3KZRoVE0cc8OWh0clhT1b/ZYGnVN7GDDUaJAmMZDONz7Jj1Mtp3NYXK8Xh86OxZlkH4 V/2Sdu9L9n9yNaVsasn2OplJJTtPJyq5q/IvD7fiSjU/X8Tx+ilYR3mtI5duH3ncW5wg d5hkiaBtJ5Flq/jdDQ1T9VmWJmRQmoBU9L7eCu5o0Cbgx6aZfcC9yh2P4KP6pRcei3Qp DXAoFP+RZAx1p7Y3VwkuYRc7KY+eGm9f1mMq+oq0jA+Jidky1mbDdW36lqRE9NG5XTAm iIsw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1790067388; x=1790672188; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to:content-type; bh=Wq2LoFmFiObS5jB4OwtPqD7h4w1zA3ahilWAP6xrcps=; b=nyDZmenED1o+wrMxZdGbP8NZthdRrVjJ+qwVSdY9utBTTZuwoiriVxAaTsQvUdWt3X wutAmLbQLF6+jFNgbj3ASckK7uxN43nEKS/RtFK5qDopFrgHNp9QSP/+ikRje6GtT0Bk of9A27fZmjg5iL0C9x7QkyYuD9HWCAn0DDUwNdkZlZ7CWlWbRQ7Hoy849Kx9LTEpnbB/ yoYIw4N6ASg1UhCXvTiff/J+NEd4sz28B3lyrs1Von47tbDotMJW1TsILAbEig3p/nqc atURt/cVBAzgkUofJ/CBn5RzkSISh4906in0+l7qJ7aRNhl0EumwQ0NAYlM/HPxsOBae 49tA== X-Forwarded-Encrypted: i=1; AKwUvBxYnEtG2KgIzEXCNQsGkUBoUyXF5U8qFxo6P6/3oHjK311aJCjCFPcWbWzsml96etw2XFaOI6o67pcP6FIZuw==@lists.infradead.org X-Gm-Message-State: AFuF++lamk924BYOLao0pCohUkwDQKA6rulR3WOOL4OiypXVAHM1OVUN 1hw3vLqJtDU21ITpPmHoQgEKWUWMfytAqE7mR/6QBjbeWo8YsNaRVxyJ X-Gm-Gg: AYBFou0sgCS6OkTr2ZH0SapfrR1J5jyt4SC/IzpxPxMpcxdIDNB8tlhQ76xxy8yzKrC BYjGjR3jNFttebs+pvd8Z8Uf2ItpXx2GC8E1tzn2954V5fIhP2ufERtHtUCAWs72zsoBAIb/WwC ItxrvXg+vcVVOX7v9M1eeKKdFTo0dCgfBqR7HPgiSar9Bdmo1zop0fZLIp79iTpVYNp8vjsCC+b 6iI+c8TwTDAVID+DIWMzAzyDOokwVA+a3flQcuVEgwRpfcr11PS/xL9twlTvuakvIGi+cLRULNg dRz/hE1dO32m6KL1gZZXun6UBzOu+WCbiAyt6OkFD75wGZs8RKfUj/+pTDCuKxXB5kD4Gkv/soO UkQ52mSMO+pyxzOlcu9qrBhXvKp0FXKdGUD2vEIuN5F74Fbotk0BF5mWyOR+LeKjErJzxNbTq3R qJcr+yRwjrUTB6EjDQkfNBjmsZc+NZZQMwrBXWk9jDKWZp0JTg4EgTbJlOYZTux2hy9UH0zKfGC BAMZPQX363ejB4YjhF5vmZakftDD+PR4dmhcbN0/vQwKwJqs1Ui7ikOSOmdVEmingBcRePvAUkL J/Q= X-Received: by 2002:a5d:64c5:0:b0:485:ac0a:e11 with SMTP id ffacd0b85a97d-4871f9c6776mr18336406f8f.0.1790067388322; Tue, 22 Sep 2026 01:56:28 -0700 (PDT) Received: from OrangePi5-Plus.BB-HOME (20014C4E1B80530056971C6280202175.dsl.pool.telekom.hu. [2001:4c4e:1b80:5300:5697:1c62:8020:2175]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-4886279298asm2881848f8f.34.2026.09.22.01.56.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 22 Sep 2026 01:56:27 -0700 (PDT) From: Igor Paunovic To: Tomeu Vizoso , Oded Gabbay , Heiko Stuebner Cc: Igor Paunovic , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Jeff Hugo , Robert Foss , Sidong Yang , Diederik de Haas , Sebastian Reichel , Jiaxing Hu , Nicolas Dufresne , Jonas Karlman , Guangshuo Li , =?UTF-8?q?H=C3=BCseyin=20BIYIK?= , dri-devel@lists.freedesktop.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 09/11] accel/rocket: add devfreq support Date: Tue, 22 Sep 2026 10:56:17 +0200 Message-ID: <20260922085617.53620-1-royalnet026@gmail.com> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260922081855.160451F00893@smtp.kernel.org> References: <20260922080114.44662-1-royalnet026@gmail.com> <20260922080114.44662-10-royalnet026@gmail.com> <20260922081855.160451F00893@smtp.kernel.org> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.9.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260922_015630_333419_AA245B53 X-CRM114-Status: GOOD ( 12.08 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@lists.infradead.org On the five findings from the Sashiko review of this patch, in its order. All of this is from reading the code; none of it was reproduced. 1. Cores left powered after unbind: not with the driver's own settings. rocket_core_fini(), which this series does not change, calls pm_runtime_dont_use_autosuspend() before pm_runtime_disable(); that runs rpm_idle() synchronously and suspends the core on the spot. It can happen if root sets power/autosuspend_delay_ms to 0 and unbinds a core with the clock raised: the put then queues an asynchronous suspend that pm_runtime_disable() can cancel. v3 will drop those references synchronously on the teardown path. 2. Ignored return value: true, but here an error does not leave the clock raised. Scaling down, the OPP core sets the clock before the supply, so a regulator error comes after clk_set_rate() has already asked for the 200 MHz boot rate, which TF-A serves from GPLL, and a rate the firmware refuses never reaches the caller. v3 will say so in a comment at both call sites. 3. Rounding the boot rate up: not with the in-tree devicetree, where assigned-clock-rates gives exactly the lowest OPP. Without it, a boot rate above 200 MHz would map to a PVTPLL OPP and the cores would be released there; that is the code-read-only case under "Not done". v3 will create the devfreq device only when the boot rate is at or below the lowest OPP (without assigned-clock-rates this firmware reports 198 MHz, GPLL/6). 4. Restore racing with rocket_devfreq_fini(): real, and this patch introduces it. device_shutdown() pins only the core it shuts down, and __device_release_driver() drops its runtime PM reference before ->remove(), so the last core to go idle can reach rocket_npu_restore_boot_rate() from its autosuspend timer while rocket_devfreq_fini() clears ->owner and removes the OPP table and config. The window is a few microseconds and none of my runs set it up. v3 will take pm_runtime_get_noresume() and pm_runtime_barrier() on every core before ->owner is cleared, and drop them afterwards. 5. Concurrent unbinds: real. Each sysfs unbind takes only its own device lock, so two unbinds can both remove the same devfreq device. The unlocked probe and remove predate this series (Sashiko's finding on the standalone slot-search patch, which I agreed with [1] and the cover letter lists as open), but this patch makes the result worse. The OPP table is not freed twice. v3 will add a driver-wide mutex around rocket_probe(), rocket_remove() and rocket_shutdown(). I will wait for review of v2 before sending v3. [1] https://lore.kernel.org/r/20260904135938.8757-1-royalnet026@gmail.com Igor _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip