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 F0C30C3F6B0 for ; Tue, 16 Aug 2022 04:56:40 +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-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:Subject:From:References:Cc:To: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=A1+4S3LvWglPyN1eBeJvajoE2AtMVncTemGf8sECQSA=; b=tUESJOLPOYsWoh IDpPoqHxKePuzyNw61sXBuFng+MPy4OwRI4ce/y7O84d2vSDRMlLdHPqo8eiqyVijRFpAvMtQVKQD OAuF08QAxZsrN7xyyuhh7HeWJLO2I5j7puRxONE1VsURPbu7s2zVvha3P7G7fjyE+No1FeMqowTfB Dw6yimxVDyDw4KR/e5TSmaaRhLQqVX9JitiGrTOJUztA+TjxdIX/467Ft05a53jbgXqEj9Oww96ks ta1K/TtN6nc5ZygTu5zvg7jaWwn3weSyjJTDFmYAuiPlHE+CBzd+7G+Cx9G6fzavs9k9dqUuIlgjK P0uv8Yd8Ou7JUa4YUdjw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oNoch-00B6kG-Q7; Tue, 16 Aug 2022 04:56:15 +0000 Received: from mail-lj1-x22c.google.com ([2a00:1450:4864:20::22c]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oNocf-00B6gl-9O; Tue, 16 Aug 2022 04:56:14 +0000 Received: by mail-lj1-x22c.google.com with SMTP id l21so9557424ljj.2; Mon, 15 Aug 2022 21:56:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:subject:from:references:cc:to :content-language:user-agent:mime-version:date:message-id:from:to:cc; bh=lMyaBQ0fr/YKfcqNoaKSdb+50RM2Rxj1DLsmzLOg6p8=; b=emj3c4qKkUjUv1uibUM0Ft8DTfEO05AQs/3Y741be7NMkNUqfjGZwGCKNZo9C6YjXp T3u8ogp+Q7xyqUvWTbVxqcfKvlVXs+sCdEW176mhvN95YWDcTH1MCuzscHr20UM4t23y efx6c0daYiXNg01uKsfggL1TIi4sG62wT7fq7xNMdYWmSyv0IojrLQ3X+x3JlfiYjCWI ZGleJlTBA7imnc87Mtsd2wKxuRqiB4CfXoOM/VC02Ai37op0ZH3k+3vNM5TFazrPwDbX I3K6wSQIq8cyMpVj2t+/iVXSTMsuGrDBGJEDknJ3KmY9XUsqIPVIorSVNOoEmEoQm85s ld9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:subject:from:references:cc:to :content-language:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc; bh=lMyaBQ0fr/YKfcqNoaKSdb+50RM2Rxj1DLsmzLOg6p8=; b=8Dzuxc/LdXBSfc3IxBeqlNcQcQba+F8BxtnZOXwbt4AE6dn9DBcmKSsKf0pKle5ax+ u1VPXKOpjEMZl8z8mGwlXl8NbnidFQzmMnarqzIEtVwnIBKAzw9xE2m0wG42pcG9w9M4 Yl7kV6C3yuWwgSrg3OByS6p9P8QR3bL2J6Ebsmdf0QVoGV7t//QmoTSd0xxbK7rmkLbc 2yZH4m7a7Kh0KwdyJFBIIgUVU1+KVklHi1cpwKQJT5MHaZl2oydiWxRGz0TFPhbqiuhx gpIxhs3PODDk8QXyhpR088n4ep9P7k/4T+ycbrEruCVaX5Ai/GjbXCX+K+NQDp+3xE2q m6Kg== X-Gm-Message-State: ACgBeo1s3fzEmzIDJOU1TyVuYvcwzqpo4qKwcsqMKUP74sBiUQHwPXCn YWcDk0WU46ipL9RzU3aqhTY= X-Google-Smtp-Source: AA6agR5Now0GlaC9p5MWRyZT/Uc6WkRd+5NRB4I7ljhPTH6IshCkE/HR/M28GX+goQncDQyVAAYQYQ== X-Received: by 2002:a05:651c:1241:b0:261:9313:9cb9 with SMTP id h1-20020a05651c124100b0026193139cb9mr1021791ljh.213.1660625769185; Mon, 15 Aug 2022 21:56:09 -0700 (PDT) Received: from ?IPV6:2001:14ba:16ee:fa00::9? (dc73szyyyyyyyyyyyyyft-3.rev.dnainternet.fi. [2001:14ba:16ee:fa00::9]) by smtp.gmail.com with ESMTPSA id w9-20020ac24429000000b0048a7b1530cesm1254427lfl.284.2022.08.15.21.56.07 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 15 Aug 2022 21:56:08 -0700 (PDT) Message-ID: <57c312b3-ca5b-6efb-6356-43b6513a0c88@gmail.com> Date: Tue, 16 Aug 2022 07:56:06 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Content-Language: en-US To: Mark Brown , Laurent Pinchart Cc: Stephen Boyd , Matti Vaittinen , dri-devel@lists.freedesktop.org, Johan Hovold , Neil Armstrong , Lars-Peter Clausen , Kevin Hilman , linux-kernel@vger.kernel.org, Daniel Vetter , linux-amlogic@lists.infradead.org, Greg Kroah-Hartman , linux-doc@vger.kernel.org, Jonathan Cameron , Andy Shevchenko , Liam Girdwood , Michael Hennerich , Miaoqian Lin , linux-arm-kernel@lists.infradead.org, Alexandru Tachici , Jerome Brunet , Andrzej Hajda , Jonathan Corbet , Guenter Roeck , Jonas Karlman , Lorenzo Bianconi , Michael Turq uette , Jernej Skrabec , Martin Blumenstingl , Jean Delvare , Alexandru Ardelean , linux-hwmon@vger.kernel.org, linux-clk@vger.kernel.org, =?UTF-8?Q?Nuno_S=c3=a1?= , Robert Foss , Aswath Govindraju , David Airlie , linux-iio@vger.kernel.org References: <166057828406.697572.228317501909350108.b4-ty@kernel.org> <20220815205857.308B1C433D6@smtp.kernel.org> From: Matti Vaittinen Subject: Re: (subset) [PATCH v2 0/7] Devm helpers for regulator get and enable In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220815_215613_401365_5C757A00 X-CRM114-Status: GOOD ( 34.70 ) X-BeenThere: linux-amlogic@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-amlogic" Errors-To: linux-amlogic-bounces+linux-amlogic=archiver.kernel.org@lists.infradead.org Hi dee Ho Mark, Laurent, Stephen, all On 8/16/22 01:55, Mark Brown wrote: > On Tue, Aug 16, 2022 at 12:17:17AM +0300, Laurent Pinchart wrote: >> On Mon, Aug 15, 2022 at 01:58:55PM -0700, Stephen Boyd wrote: > >> You will very quickly see drivers doing this (either directly or >> indirectly): > >> probe() >> { >> devm_clk_get_enabled(); >> devm_regulator_get_enable(); >> } > >> Without a devres-based get+enable API drivers can get the resources they >> need in any order, possibly moving some of those resource acquisition >> operations to different functions, and then have a clear block of code >> that enables the resources in the right order. I agree. And I think that drivers which do that should stick with it. Still, as you know the devm-unwinding is also done in well defined order. I believe that instead of fighting against the devm we should try educate people to pay attention in the order of unwinding (also when not handled by the devm. Driver writers occasionally break things also w/o devm for example by freeing resources needed by IRQ handlers prior freeing the IRQ.) If "purging" must not be done in reverse order compared to the aquisition - then one should not use devm. I know people have done errors with devm - OTOH, I've seen devm also fixing bunch of errors. >> These devres helpers give >> a false sense of security to driver authors and they will end up >> introducing problems, the same way that devm_kzalloc() makes it >> outrageously easy to crash the kernel by disconnecting a device that is >> in use. I think this is going a bit "off-topic" but I'd like to understand what is behind this statement? From device-writer's perspective - I don't know much better alternatives to free up the memory. I don't see how freeing stuff at .remove would be any better? As far as I understand - if someone is using driver's resources after the device has gone and the driver is detached, then there is not much the driver could do to free-up the stuff be it devm or not? This sounds like fundamentally different problem (to me). > TBH I think the problem you have here is with devm not with this > particular function. I must say I kind of agree with Mark. If we stop for a second to think what would the Laurent's example look like if there were no devm_regulator_get_enable() provided. I bet the poor driver author could have used devm_clk_get_enabled() - and then implemented a .remove for disabling the regulator. That would be even worse, right? > That's a different conversation, and a totally > valid one especially when you start looking at things like implementing > userspace APIs which need to cope with hardware going away while still > visible to userspace. This is interesting. It's not easy for me to spot how devm changes things here? If we consider some removable device - then I guess also the .remove() is ran only after HW has already gone? Yes, devm might make the time window when userspace can see hardware that has gone longer but does it bring any new problem there? It seems to me devm can make hitting the spot more likely - but I don't think it brings completely new issues? (Well, I may be wrong here - wouldn't be the first time :]) > It's *probably* more of a subsystem conversation > than a driver one though, or at least I think subsystems should try to > arrange to make it so. -- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~ Discuss - Estimate - Plan - Report and finally accomplish this: void do_work(int time) __attribute__ ((const)); _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic