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 5FD7AC4345F for ; Fri, 12 Apr 2024 06:28:25 +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:Date:To:Cc:From:Subject:References: In-Reply-To:MIME-Version:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=K71Ixg3qcv2dJDcxXQpgRp00dNd1QNSQ9R0fXQNvFIM=; b=R+LIpoKHO+zZ06 4Hg6MHUPjxwAtsXC6MX2STQxzNd1cUMbYRH2ypcnFw1NM+UEXtm5HMssQ6PR4qVplGCmZey27EsNp 0RIbtSIaDYaH53fSx29UxKKpph0H0daRpur3CQV2XH+oOiFF+ZtbCov+NC411GnOu1b/bFciR0nY5 8LXkpvu4FKjEnPC9u3gPy5nCXVuJ5mNW0btpcDa7djcK35QvgkKz48deTLqctdyz/OyO1vJ1Mu8w8 HQaPrymIP00K/u/7YWRbMVJt6R/3Y6kFDUj7iSCsLTjGCsEbNOPo5LqwJrDkVSXN5vkiiIA7/TUJk gOyeChY5h8rFFHLqHH5w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rvAOU-0000000FZVH-2aYA; Fri, 12 Apr 2024 06:28:14 +0000 Received: from sin.source.kernel.org ([2604:1380:40e1:4800::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rvAOR-0000000FZTu-3hWN for linux-arm-kernel@lists.infradead.org; Fri, 12 Apr 2024 06:28:13 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 1C4E1CE319F; Fri, 12 Apr 2024 06:28:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 405FDC2BBFC; Fri, 12 Apr 2024 06:28:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1712903289; bh=zgUjpYLth8c3Bov77A49CZpA8m7933+tVeBvXp/8wOY=; h=In-Reply-To:References:Subject:From:Cc:To:Date:From; b=mcD9oVlH4scQ0n8FoGrPsd+Wh/j9+FrlK3kD+wBSEvJRwiB2l3BmDvsbJfmQ8YsCg bgUu5iA9baeH3R3QfZh9bYBuBVqPG9rsn2EOGxf+97lT+G0H9mnbQ9FhBpqzdW2oXh 0d3j7/KzvaMREQOA8mkfLJRlpWVgFjvV/IgeKTv86yArvOytXcFl0n1QKUoQ2Vimo4 Li9mZ/OQHPNMRB8mCmQW3Z84qs+wcf8MHRO9/YmmlkHqJFhW3ELj+diPpTsmzDFW/R ADRW15Nbxqr78VwBKSXFS1A3kUUgWZam+5Jt4yAOV5fJtg0nTh4P4w8QQyUXtqUl+n RhFDBtVGuR2Wg== Message-ID: <3d6d34c5075559f3df506d14e38a9c0c.sboyd@kernel.org> MIME-Version: 1.0 In-Reply-To: References: <20231002180854.1603452-1-ben.wolsieffer@hefring.com> <20231002180854.1603452-2-ben.wolsieffer@hefring.com> <883a61872f94c972cc410da84eaf7b97.sboyd@kernel.org> Subject: Re: [PATCH 1/2] clk: stm32: initialize syscon after clocks are registered From: Stephen Boyd Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, Rob Herring , Krzysztof Kozlowski , Conor Dooley , Maxime Coquelin , Alexandre Torgue , Michael Turquette To: Ben Wolsieffer Date: Thu, 11 Apr 2024 23:28:07 -0700 User-Agent: alot/0.10 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240411_232812_307163_53201C2D X-CRM114-Status: GOOD ( 32.80 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Quoting Ben Wolsieffer (2024-01-12 14:00:04) > On Sun, Dec 17, 2023 at 03:05:01PM -0800, Stephen Boyd wrote: > > Quoting Ben Wolsieffer (2023-10-02 11:08:53) > > > The stm32-power-config syscon (PWR peripheral) is used in this driver > > > and the STM32 RTC driver to enable write access to backup domain > > > registers. The syscon's clock has a gate controlled by this clock > > > driver, but this clock is currently not registered in the device tree. > > > This only happens to work currently because all relevant clock setup and > > > RTC initialization happens before clk_disabled_unused(). After this > > > point, all syscon register writes are ignored. > > > > Seems like we should mark those clks as CLK_IGNORE_UNUSED and add a > > comment to that fact. > > That seems like a worse solution than specifying the clock dependency in > the device tree. > > > > > > > > > If we simply add the syscon clock in the device tree, we end up with a > > > circular dependency because the clock has not been registered at the > > > point this driver requests the syscon. > > > > > > This patch avoids this circular dependency by moving the syscon lookup > > > after the clocks are registered. This does appear to create a possible > > > race condition where someone could attempt to perform an operation on a > > > backup domain clock before the syscon has been initialized. This would > > > result in the operation having no effect because backup domain writes > > > could not be enabled. I'm not sure if this is a problem or if there is > > > a way to avoid it. > > > > There's no comment in the code that says the regmap must be set there > > instead of earlier. What's to stop someone from tripping over this > > problem later? At the least, please add a comment. > > Yeah, I'll fix that. Do you have any thoughts on the race condition I > described? Should I add some kind of locking to block > enable/disable_power_domain_write_protection() until stm32f4_rcc_init() > attempts to initialize the syscon? Maybe. I don't really know and it's probably because I don't really understand the problem. Maybe you can solve it by turning on the backup domain clock manually when the driver probes via direct register writes and then only publish the syscon once the backup domain clock is registered? _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel