From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com [209.85.128.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0C59F485CCD for ; Thu, 23 Jul 2026 17:03:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.51 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784826203; cv=none; b=tETyUKKiQtUKQLnAcNoFkp7slbNZHonEi6J+PxYBKCqgBh3OX+GSF77lYyWhYkQf/KDWKklNZzsAgSseH+ZvAbcOoc4Z9c32UtNGaEMubNCbwhDIBR0AREXRC0I0hJaqyc6+JwVg/Ahtfv5DXanRU9Fpsn7IfHSgOT5jCZvDfYA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784826203; c=relaxed/simple; bh=HX9hHiD9Q8xG+TX4N8TVpWXrSbck4clGhq83nZuslMo=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=SNsw0LhGQmA/92yny2FxdvqUKgwTFnsopbfPuxvFMkzZxWUDOMbOWOG8u8DyjLNeOkzF/GpJdTygqD+xw5mSTDWat/8iR8uI8QwYXsWn1wXAqxR1x7Fo6nxQdXafhuJO+vCep73oQbpsaK+FYF/90WK6tvxkSTPKsT6oimTMPvk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com; spf=pass smtp.mailfrom=baylibre.com; dkim=pass (2048-bit key) header.d=baylibre.com header.i=@baylibre.com header.b=HUIzVuKG; arc=none smtp.client-ip=209.85.128.51 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=baylibre.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=baylibre.com header.i=@baylibre.com header.b="HUIzVuKG" Received: by mail-wm1-f51.google.com with SMTP id 5b1f17b1804b1-4954d383e64so6057935e9.1 for ; Thu, 23 Jul 2026 10:03:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre.com; s=google; t=1784826194; x=1785430994; darn=vger.kernel.org; h=content-type:mime-version:message-id:date:user-agent:references :in-reply-to:subject:cc:to:from:from:to:cc:subject:date:message-id :reply-to:content-type; bh=3bmXdjhA3HTwHbyEK6JtJWrwrPDwDFR73RowlJS9lG4=; b=HUIzVuKGgmxTMlPHTaqSHx2dL6Mp1A14WYiQsWg0uRvCi+5s97OCp1I8U5FSGiF3xO eflxzm1zsNoQ7hZ93sSQ5m2yO00WOyO9RXn3JpRU4y2C9bj9ff28sD0VueX8LfBaE4zP p3gR4Lu8Dw4WpBoV3/zP/V/sFb6XgOP3HDLqc+qkKRfY2/0fC3l/vuCk8dLel19lHCz+ Ijx0OM6KjiUVVrstuxE7TPUtEAWQh5AT1kPZuPi4BgYvbRSh43SdI8Khl2TvC/zs2zUd ShtNlSdHV+wKBtE4OHhuwD4FUp2Sk6P9tCAIzVrfUcE5+ahTsOVCyI8CQc+Oj1fy46v0 zFpQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784826194; x=1785430994; h=content-type:mime-version:message-id:date:user-agent:references :in-reply-to:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to:content-type; bh=3bmXdjhA3HTwHbyEK6JtJWrwrPDwDFR73RowlJS9lG4=; b=SwGMvqz4DQ0M+q3tPz91FTVFIdDiu6mLlh4NEphBo4AAv4QlS0L9IwcH4AAeQCgVuy vrX1v5yC672UI+Ne0MEperHLImmfDvZYVLAlp/xUJtDQkQBH03mqBhsi4U5kqs7doWDz O3Og/+i4DcObS3xWFrw+Cxx4MZemBhagjaHg0nr2ZDDfF7JK/Q0vZy4Mx5TvlViDhn0I EA22E2BbwWi6t1r50kiL9uJMupPlBmmqY8PFx8L7/62pgvBb6kQpd4MLCmNde0nQDM2n 8NNIFpMBXRyv2ZxSktmSzsVrQq6D5nf1WXAnfG/LXZMAcltIogJs3/jCemKjhnOein/2 /0BA== X-Forwarded-Encrypted: i=1; AHgh+RrP/y5i/X3Iuj4X1ZmbD3GqAt9P7AXMMsXNuDqiYdmBl3cfWPFZm5SrPFSzibuGzz2KwgJTSW2uyfJfMd4=@vger.kernel.org X-Gm-Message-State: AOJu0YyAr7oGiHh8a0ug47UGXJLTn2hvQGn45tJ7X3E1bTCNiVuL83zj YQbZlsVkf00OvbE0dQelwkGtj3cKgiJik/pZ0ODAvEzaczJ6sNuaL17cYieQNc94H20lDghiGY0 KA/Mx5A4= X-Gm-Gg: AR+sD10foDZQSAFsNUayfwcNuNba82ntJuEeorJ7EZEe+RB8oaB9l6ACumrfv7J5304 0MlN/t804oPFglvWCb9bTs3QljPErJ1NcLL4O8fyWNdvJGGx4tIQrvFHdpdvhTNz/+B6b2g3BI8 y4G+0RfZyfEmneyeuiNGW00rSMrnJ0FiHG1BX5+y8FQEHcScCs3k2YeVNTJhh2zhG/wo7HvJ+Is gxBcQ137V9iIgqtZeOR6rHJgoLVfRv2CuKbvegI2Foah46BPiC2wfsnVUqrxrGsYX/f2gdiZ+Ch suczsrV3JLbz/hEW96HNqadW5knvPHe87Vi5iG59DseGpwnvwURBJI3oDqMd9CpcNzLpdPSpbaT RVRh639jI+AYu2ys5Tpbrhdp0hZXOhFnw2PI1EcdJpze6g1fDs7k2hkNTdu7uJkzCCNpdeX1wel R+7s7NBSjDn+k= X-Received: by 2002:a05:600c:4ed1:b0:493:f5bf:4da4 with SMTP id 5b1f17b1804b1-49573d11690mr46042655e9.28.1784826194117; Thu, 23 Jul 2026 10:03:14 -0700 (PDT) Received: from localhost ([2a01:e0a:3c5:5fb1:5f98:8c28:3e2c:1c72]) by smtp.gmail.com with UTF8SMTPSA id 5b1f17b1804b1-4957af16f72sm8890055e9.0.2026.07.23.10.03.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Jul 2026 10:03:13 -0700 (PDT) From: Jerome Brunet To: Brian Masney Cc: Michael Turquette , Stephen Boyd , Russell King , linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2] clk: fix self-consuming provider module pinning In-Reply-To: (Brian Masney's message of "Thu, 23 Jul 2026 10:40:28 -0400") References: <20260723-clk-provider-pinning-v2-1-8dad72eb79f0@baylibre.com> User-Agent: mu4e 1.12.9; emacs 30.1 Date: Thu, 23 Jul 2026 19:03:11 +0200 Message-ID: <1jbjbxd4fk.fsf@starbuckisacylon.baylibre.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain On jeu. 23 juil. 2026 at 10:40, Brian Masney wrote: > Hi Jerome, > > On Thu, Jul 23, 2026 at 03:00:51PM +0200, Jerome Brunet wrote: >> clk_hw_get_clk() lets a provider get a struct clk for one of its own >> struct clk_hw. >> >> When a struct clk is created, the module usage count of the provider >> is unconditionally increased. For a self-consuming provider, this means >> it pins itself and the module can never be unloaded. >> >> Increasing the module usage count should only be done when the consumer >> lives in a different module from the provider. Use THIS_MODULE to >> capture caller's module and increase the module usage count accordingly. >> >> It is OK for consumer-only APIs such as clk_get() or of_clk_get() to >> pass a NULL owner. As a result, any provider module will get pinned, >> same as before. >> >> Fixes: 30d6f8c15d2c ("clk: add api to get clk consumer from clk_hw") >> Signed-off-by: Jerome Brunet >> --- >> This issue has been present for a while. Virtually all users of >> clk_hw_get_clk() are affected. The majority are compiled as builtins >> according to the defconfigs. It is not problem in this case but it is >> if the configuration is changed to module. >> >> The following modules are using clk_hw_get_clk() and are compiled as >> module with some shipped defconfigs: >> * drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_pll.c >> * drivers/phy/cadence/phy-cadence-sierra.c >> * drivers/pwm/pwm-meson.c >> * sound/soc/codecs/lpass-va-macro.c >> >> Currently those module cannot be unloaded once they have been loaded. >> >> """ >> rmmod: ERROR: Module blabla-module is in use >> """ >> >> With this applied, we can get back to removing the direct usage >> of the struct clk in struct clk_hw and eventually remove this >> struct member entirely. >> --- >> Changes in v2: >> - Update comment in __clk_register() >> - Add missing documention for the new parameter of clk_hw_create_clk() >> - No functional change >> - Link to v1: https://patch.msgid.link/20260721-clk-provider-pinning-v1-1-63db2e667993@baylibre.com > > Reviewed-by: Brian Masney > > This looks good to me. I'm planning to send Stephen a pull next week for > content that I think is ready during this development cycle, however I'm > not going to include this. I suspect he's going to want a kunit test for > this and I know it's going to be complicated. My suggestion is to post > some kind of rough test scenario for this, even if it's initially not in > kunit. As I get some spare time at work, I can help you to see if we can > get a kunit test together for this. > > Brian I understand but I'm not adding an new API, I'm fixing a real problem in an existing one in the kernel now. The related API is already used in some kunit, so there is some coverage already. The problem was known (as the comment in __clk_register() was showing) and there was no kunit then ... so it is a bit strange to delay fixing the problem because there is not kunit associated with it now. That said, I'll check was I can do. acutally loading and unloading module in kunit does not really feasible. Maybe poking the owner field with __clk_hw_get_clk(). This is not really supposed to used directly (which is why I did not add the documentation around it) and it's bit fake but I do not really a way around it. -- Jerome