From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from wnew2-smtp.messagingengine.com (wnew2-smtp.messagingengine.com [64.147.123.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5495A2FAE for ; Fri, 1 Oct 2021 01:45:37 +0000 (UTC) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailnew.west.internal (Postfix) with ESMTP id E17062B00262; Thu, 30 Sep 2021 21:45:35 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Thu, 30 Sep 2021 21:45:36 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sholland.org; h= subject:to:cc:references:from:message-id:date:mime-version :in-reply-to:content-type:content-transfer-encoding; s=fm3; bh=v Odoat0J6oK40pz/pIFwU3UMSzNN+QQ5EnZGqlGFHak=; b=gQmCA3rjtz3PfoW/C hQUSW3jnbYvySDgW/Ltfhsf2zmDcf7sbSkGHDLOnHgd8CNRKe7Q5/MOV48arJvfZ bnmnOolkxCVlSoWvGgi7hldvleOjJWxqbBb9FlQYbWp+dLPQcAwuIuSgyyaEQ7HU Omzbl65IxjvsT/lm0jt3KK3tGcTK2ZDVMIouCq1Q1pa4LVRnOYfgB2AcHGmY7F06 4QcvlcrYFi2mDmBlMSKtEmAB7BN+p6DhRBdwl3KhZiA0bWDwWYcu8rMpIUIklptf Lzk8YD0Kwhl5oEXBEbOFq/o781In3cw6DXV+tc4FD1Pjx4pTgn10Qh6SayOrcHq7 qcK8A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=vOdoat0J6oK40pz/pIFwU3UMSzNN+QQ5EnZGqlGFH ak=; b=HEdTYRJg1zvGFpz6zaSCEe6OCgmzV/sA1VVSbHtFk+YJkPNcGVDSnzv/D yNU5UvgKCIVH7XMpnRReUFK09bSv2HD6eT153H/Ch8p5lO5wyWIHGNFIe3kUaW3A DenQF9S5xZvpkYLPKCsfIdLWIsLr4h34b6A9DtBGgk1tFfJseU70k4kAMS3TdZGc i3gzGfuAQl1IjvQdGY2br5s7c5INeyhQHLa70yChBjpbnq3Dc6oLZxg66YmobVFb Bwxe31mozgSTUBiWZVDNm++iVzBS5FoT+YGtr6jIYlNaT4OgqZpctWrohfWw+ekv q6PrwYQ71fjxUCzqiCfg2PqMMJs9w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrudekhedggeelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepuffvfhfhkffffgggjggtgfesthejredttdefjeenucfhrhhomhepufgrmhhu vghlucfjohhllhgrnhguuceoshgrmhhuvghlsehshhholhhlrghnugdrohhrgheqnecugg ftrfgrthhtvghrnhepgfevffetleehffejueekvdekvdeitdehveegfeekheeuieeiueet uefgtedtgeegnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepshgrmhhuvghlsehshhholhhlrghnugdrohhrgh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 30 Sep 2021 21:45:33 -0400 (EDT) Subject: Re: [PATCH 02/10] PM / devfreq: Do not require devices to have OPPs To: Chanwoo Choi , MyungJoo Ham , Kyungmin Park Cc: Michael Turquette , Stephen Boyd , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, Maxime Ripard , Chen-Yu Tsai , Jernej Skrabec , Rob Herring References: <20210929044254.38301-1-samuel@sholland.org> <20210929044254.38301-3-samuel@sholland.org> <114afa7e-6218-6b1f-f87e-84690f10029c@samsung.com> <7d404f89-6567-61e6-7964-d2ca578ed652@samsung.com> From: Samuel Holland Message-ID: <7107eea6-f2ca-fb7c-5975-569066ba21a7@sholland.org> Date: Thu, 30 Sep 2021 20:45:33 -0500 User-Agent: Mozilla/5.0 (X11; Linux ppc64; rv:78.0) Gecko/20100101 Thunderbird/78.10.2 Precedence: bulk X-Mailing-List: linux-sunxi@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 In-Reply-To: <7d404f89-6567-61e6-7964-d2ca578ed652@samsung.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 9/30/21 8:59 PM, Chanwoo Choi wrote: > On 9/30/21 8:37 PM, Samuel Holland wrote: >> On 9/29/21 11:19 PM, Chanwoo Choi wrote: >>> Hi Samuel, >>> >>> >>> On 9/29/21 1:42 PM, Samuel Holland wrote: >>>> Since commit ea572f816032 ("PM / devfreq: Change return type of >>>> devfreq_set_freq_table()"), all devfreq devices are required to have a >>>> valid freq_table. If freq_table is not provided by the driver, it will >>>> be filled in by set_freq_table() from the OPPs; if that fails, >>>> devfreq_add_device() will return an error. >>>> >>>> However, since commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when >>>> adding the devfreq device"), devfreq devices are _also_ required to have >>>> an OPP table, even if they provide freq_table. devfreq_add_device() >>>> requires dev_pm_opp_find_freq_ceil() and dev_pm_opp_find_freq_floor() to >>>> return successfully, specifically to initialize scaling_min/max_freq. >>>> >>>> Not all drivers need an OPP table. For example, a driver where all >>>> frequencies are determined dynamically could work by filling out only >>>> freq_table. But with the current code it must call dev_pm_opp_add() on >>>> every freq_table entry to probe successfully. >>> >>> As you commented, if device has no opp table, it should call dev_pm_opp_add(). >>> The devfreq have to use OPP for controlling the frequency/regulator. >>> >>> Actually, I want that all devfreq driver uses the OPP as default way. >> >> The current code/documentation implies that an OPP table is intended to >> be optional. For example: >> >> * struct devfreq - Device devfreq structure >> ... >> * @opp_table: Reference to OPP table of dev.parent, if one exists. >> >> So this should be updated if an OPP table is no longer optional. > > Right. Need to update it. > >> >>> Are there any reason why don't use the OPP table? >> >> dev_pm_opp_add() takes a voltage, and assumes the existence of some >> voltage regulator, but there is none involved here. The only way to have >> an OPP table without regulators is to use a static table in the >> devicetree. But that also doesn't make much sense, because the OPPs >> aren't actually customizable; they are integer dividers from a fixed >> base clock. > > You can use OPP for only clock control without regulator. OPP already > provides them. OPP already provides the helpful function which > implement the functions to handle the clock/regulator or power doamin. > It is useful framework to control clock/regulator. > > If the standard framework in Linux kernel, it is best to use > this framework in order to remove the duplicate codes on multiple > device drivers. It is one of advantage of Linux kernel. > > Also, if OPP doesn't support the some requirement of you, > you can contribute and update the OPP. > > And adding a fixed OPP table to each board would be a lot of >> work to replace a trivial loop in the driver. So it seems to be the >> wrong abstraction. > > I don't understand. As I commented for patch 10, you can add > the OPP entry of the clock without the fixed OPP table in devicetree. > >> >> Using an OPP table adds extra complexity (memory allocations, error >> cases), just to duplicate the list of frequencies that already has to >> exist in freq_table. And the driver works fine without any of that. > > 'freq_table' of devfreq was developed before of adding OPP interface to Linux kernel as I knew. Actually, I prefer to use the OPP interface > instead of initializing the freq_table directly by device driver. > I just keep the 'freq_table' for preventing the build/working issue > for older device driver. I think OPP is enough to control frequency/voltage > and it provides the various helper funcitons for user of OPP. Thanks for the explanation. I will convert the driver to use dev_pm_opp_add(), and I will drop patches 2 and 4. I think patch 3 is still worth considering. Regards, Samuel 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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4FE11C433F5 for ; Fri, 1 Oct 2021 01:47:24 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 162ED61361 for ; Fri, 1 Oct 2021 01:47:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 162ED61361 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=sholland.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org 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:In-Reply-To:MIME-Version:Date: Message-ID:From:References:Cc:To:Subject:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=21cMUMPoXMLTRpSxD3/Ju61Nr9k+qFbfMmKcdn6i77s=; b=GANbmWUga3qNxDjJDZ+z3+Ez35 dd/0UAFg7PlTbeuG6h5FkrBw45YW0EWRKjyswjsnvUqW8aSz9o0aXW+G7e4WgJtY/21tScRL7Wdgx Z4tahJKaRnWoaDR1TGk1WVi/5M/VKqI1E0I/oFnSUGVtF8NvVdNpwsszVVPuypSkgQDgSP7bZQv2a N8+V4VT0kh3bS7n8S7mCzEkF0ygY/ajbn+jBR6vh3bvBjYITgt8EQcxqn9wETb+i/RfrGr35CU4R6 ZxEhhto6O1REe0Zka1L2w4OyocqrnV12iqjvmOje3M4A2bLi0QYShjZ7Hl0YosHnw1gZXWnee2Fji IqkV2r8Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mW7cO-00GPaF-LP; Fri, 01 Oct 2021 01:45:44 +0000 Received: from wnew2-smtp.messagingengine.com ([64.147.123.27]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mW7cK-00GPZn-KS for linux-arm-kernel@lists.infradead.org; Fri, 01 Oct 2021 01:45:42 +0000 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailnew.west.internal (Postfix) with ESMTP id E17062B00262; Thu, 30 Sep 2021 21:45:35 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Thu, 30 Sep 2021 21:45:36 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sholland.org; h= subject:to:cc:references:from:message-id:date:mime-version :in-reply-to:content-type:content-transfer-encoding; s=fm3; bh=v Odoat0J6oK40pz/pIFwU3UMSzNN+QQ5EnZGqlGFHak=; b=gQmCA3rjtz3PfoW/C hQUSW3jnbYvySDgW/Ltfhsf2zmDcf7sbSkGHDLOnHgd8CNRKe7Q5/MOV48arJvfZ bnmnOolkxCVlSoWvGgi7hldvleOjJWxqbBb9FlQYbWp+dLPQcAwuIuSgyyaEQ7HU Omzbl65IxjvsT/lm0jt3KK3tGcTK2ZDVMIouCq1Q1pa4LVRnOYfgB2AcHGmY7F06 4QcvlcrYFi2mDmBlMSKtEmAB7BN+p6DhRBdwl3KhZiA0bWDwWYcu8rMpIUIklptf Lzk8YD0Kwhl5oEXBEbOFq/o781In3cw6DXV+tc4FD1Pjx4pTgn10Qh6SayOrcHq7 qcK8A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=vOdoat0J6oK40pz/pIFwU3UMSzNN+QQ5EnZGqlGFH ak=; b=HEdTYRJg1zvGFpz6zaSCEe6OCgmzV/sA1VVSbHtFk+YJkPNcGVDSnzv/D yNU5UvgKCIVH7XMpnRReUFK09bSv2HD6eT153H/Ch8p5lO5wyWIHGNFIe3kUaW3A DenQF9S5xZvpkYLPKCsfIdLWIsLr4h34b6A9DtBGgk1tFfJseU70k4kAMS3TdZGc i3gzGfuAQl1IjvQdGY2br5s7c5INeyhQHLa70yChBjpbnq3Dc6oLZxg66YmobVFb Bwxe31mozgSTUBiWZVDNm++iVzBS5FoT+YGtr6jIYlNaT4OgqZpctWrohfWw+ekv q6PrwYQ71fjxUCzqiCfg2PqMMJs9w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrudekhedggeelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepuffvfhfhkffffgggjggtgfesthejredttdefjeenucfhrhhomhepufgrmhhu vghlucfjohhllhgrnhguuceoshgrmhhuvghlsehshhholhhlrghnugdrohhrgheqnecugg ftrfgrthhtvghrnhepgfevffetleehffejueekvdekvdeitdehveegfeekheeuieeiueet uefgtedtgeegnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepshgrmhhuvghlsehshhholhhlrghnugdrohhrgh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 30 Sep 2021 21:45:33 -0400 (EDT) Subject: Re: [PATCH 02/10] PM / devfreq: Do not require devices to have OPPs To: Chanwoo Choi , MyungJoo Ham , Kyungmin Park Cc: Michael Turquette , Stephen Boyd , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, Maxime Ripard , Chen-Yu Tsai , Jernej Skrabec , Rob Herring References: <20210929044254.38301-1-samuel@sholland.org> <20210929044254.38301-3-samuel@sholland.org> <114afa7e-6218-6b1f-f87e-84690f10029c@samsung.com> <7d404f89-6567-61e6-7964-d2ca578ed652@samsung.com> From: Samuel Holland Message-ID: <7107eea6-f2ca-fb7c-5975-569066ba21a7@sholland.org> Date: Thu, 30 Sep 2021 20:45:33 -0500 User-Agent: Mozilla/5.0 (X11; Linux ppc64; rv:78.0) Gecko/20100101 Thunderbird/78.10.2 MIME-Version: 1.0 In-Reply-To: <7d404f89-6567-61e6-7964-d2ca578ed652@samsung.com> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210930_184540_805839_4252E8BE X-CRM114-Status: GOOD ( 34.76 ) 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 On 9/30/21 8:59 PM, Chanwoo Choi wrote: > On 9/30/21 8:37 PM, Samuel Holland wrote: >> On 9/29/21 11:19 PM, Chanwoo Choi wrote: >>> Hi Samuel, >>> >>> >>> On 9/29/21 1:42 PM, Samuel Holland wrote: >>>> Since commit ea572f816032 ("PM / devfreq: Change return type of >>>> devfreq_set_freq_table()"), all devfreq devices are required to have a >>>> valid freq_table. If freq_table is not provided by the driver, it will >>>> be filled in by set_freq_table() from the OPPs; if that fails, >>>> devfreq_add_device() will return an error. >>>> >>>> However, since commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when >>>> adding the devfreq device"), devfreq devices are _also_ required to have >>>> an OPP table, even if they provide freq_table. devfreq_add_device() >>>> requires dev_pm_opp_find_freq_ceil() and dev_pm_opp_find_freq_floor() to >>>> return successfully, specifically to initialize scaling_min/max_freq. >>>> >>>> Not all drivers need an OPP table. For example, a driver where all >>>> frequencies are determined dynamically could work by filling out only >>>> freq_table. But with the current code it must call dev_pm_opp_add() on >>>> every freq_table entry to probe successfully. >>> >>> As you commented, if device has no opp table, it should call dev_pm_opp_add(). >>> The devfreq have to use OPP for controlling the frequency/regulator. >>> >>> Actually, I want that all devfreq driver uses the OPP as default way. >> >> The current code/documentation implies that an OPP table is intended to >> be optional. For example: >> >> * struct devfreq - Device devfreq structure >> ... >> * @opp_table: Reference to OPP table of dev.parent, if one exists. >> >> So this should be updated if an OPP table is no longer optional. > > Right. Need to update it. > >> >>> Are there any reason why don't use the OPP table? >> >> dev_pm_opp_add() takes a voltage, and assumes the existence of some >> voltage regulator, but there is none involved here. The only way to have >> an OPP table without regulators is to use a static table in the >> devicetree. But that also doesn't make much sense, because the OPPs >> aren't actually customizable; they are integer dividers from a fixed >> base clock. > > You can use OPP for only clock control without regulator. OPP already > provides them. OPP already provides the helpful function which > implement the functions to handle the clock/regulator or power doamin. > It is useful framework to control clock/regulator. > > If the standard framework in Linux kernel, it is best to use > this framework in order to remove the duplicate codes on multiple > device drivers. It is one of advantage of Linux kernel. > > Also, if OPP doesn't support the some requirement of you, > you can contribute and update the OPP. > > And adding a fixed OPP table to each board would be a lot of >> work to replace a trivial loop in the driver. So it seems to be the >> wrong abstraction. > > I don't understand. As I commented for patch 10, you can add > the OPP entry of the clock without the fixed OPP table in devicetree. > >> >> Using an OPP table adds extra complexity (memory allocations, error >> cases), just to duplicate the list of frequencies that already has to >> exist in freq_table. And the driver works fine without any of that. > > 'freq_table' of devfreq was developed before of adding OPP interface to Linux kernel as I knew. Actually, I prefer to use the OPP interface > instead of initializing the freq_table directly by device driver. > I just keep the 'freq_table' for preventing the build/working issue > for older device driver. I think OPP is enough to control frequency/voltage > and it provides the various helper funcitons for user of OPP. Thanks for the explanation. I will convert the driver to use dev_pm_opp_add(), and I will drop patches 2 and 4. I think patch 3 is still worth considering. Regards, Samuel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel