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 483E2C54798 for ; Thu, 29 Feb 2024 10:09: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-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=aniIKd5uB+XPLsyStIzbYqNnA3pG37Z5OZVKmfpG9tY=; b=gPvm1xXFyIKWQB 0YJeN2nxLzOBBy8kb24+43dWOf4Tjt5lyMrlDp381w/pb1DvjkfT6W7iNLjXobxvCwCML3pIJc4Dl F4F67aIRcKkvNr2xWSHtmkovWWv/HV0zi3+ChrrPiTPBavyO/R/JxlIbVP1Fi3qEcJO3mfnMx2f8P LhIeHaIaami7z3wqd0gwZMFa+hfsxi62DEVcHLpEtgvSr9WVAnqlAQqCvWBnOrcryTGwLRfVYHMnj Z2NkBjI2t6AaMNg2qdGm5EkZgbeCNVwj1Km1nuxUEjktj2Or+k2y3OEPXN6eEGzdALw2vkQza8wCa yyCkMhzzvd+AvhQC+G4A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rfdLw-0000000D4uX-28ho; Thu, 29 Feb 2024 10:09:24 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rfdLt-0000000D4qx-0Kmr for linux-arm-kernel@lists.infradead.org; Thu, 29 Feb 2024 10:09:23 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 9978D1FB; Thu, 29 Feb 2024 02:09:53 -0800 (PST) Received: from pluto (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id F29D73F762; Thu, 29 Feb 2024 02:09:12 -0800 (PST) Date: Thu, 29 Feb 2024 10:09:10 +0000 From: Cristian Marussi To: Stephen Boyd Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, sudeep.holla@arm.com, james.quinlan@broadcom.com, f.fainelli@gmail.com, vincent.guittot@linaro.org, peng.fan@oss.nxp.com, michal.simek@amd.com, quic_sibis@quicinc.com, quic_nkela@quicinc.com, souvik.chakravarty@arm.com, Michael Turquette , linux-clk@vger.kernel.org Subject: Re: [PATCH 6/7] clk: scmi: Allocate CLK operations dynamically Message-ID: References: <20240214183006.3403207-1-cristian.marussi@arm.com> <20240214183006.3403207-7-cristian.marussi@arm.com> <500e265eb7c6a03a40e0067c8806e059.sboyd@kernel.org> <1d0baf6dbaa1c2ca6594f9a2bcade2c4.sboyd@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1d0baf6dbaa1c2ca6594f9a2bcade2c4.sboyd@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240229_020921_256529_F609EA2D X-CRM114-Status: GOOD ( 31.06 ) 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 Wed, Feb 28, 2024 at 06:20:34PM -0800, Stephen Boyd wrote: > Quoting Cristian Marussi (2024-02-22 00:28:41) > > On Wed, Feb 21, 2024 at 09:44:14PM -0800, Stephen Boyd wrote: > > > > > > It's not great to move these function pointer structs out of RO memory > > > to RW. I'm also not convinced that it's any better to construct them at > > > runtime. Isn't there a constant set of possible clk configurations? Or > > > why can't we simply add some failures to the clk_ops functions instead? > > > > Well, the real clock devices managed by the SCMI server can be a of > > SCMI is a server!? :) > ...well the platform fw act as a server in the client-server SCMI model...so...I know these days it's cooler to be "serverless" but..hey... ...at least is not a BO2k server :P > > varying nature and so the minimum set of possible clk configurations > > to cover will amount to all the possible combinations of supported ops > > regarding the specific clock properties (i.e. .set_parent / .set_rate / > > .enable / .get/set_duty_cycle / atomic_capability ... for now)...we > > simply cannot know in advance what the backend SCMI server is handling. > > > > These seemed to me too much in number (and growing) to be pre-allocated > > in all possible combinations. (and mostly wasted since you dont really > > probably use all combinations all the time) > > > > Moreover, SCMI latest spec now exposes some clock properties (or not) to > > be able avoid even sending an actual SCMI message that we know will be > > denied all the time; one option is that we return an error,, as you said, > > but what is the point (I thought) to provide at all a clk-callback that > > we know upfront will fail to be executed every time ? (and some consumer > > drivers have been reported by partners not to be happy with these errors) > > > > What I think could be optimized here instead, and I will try in the next > > respin, it is that now I am allocating one set of custom ops for each clock > > at the end, even if exactly the same ops are provided since the clock > > capabilities are the same; I could instead allocate dynamically and fill only > > one single set of ops for each distinct set of combinations, so as to avoid > > useless duplication and use only the miminum strict amount of RW memory > > needed. > > > > Yes please don't allocate a clk_op per clk. And, please add these > answers to the commit text so that we know why it's not possible to know > all combinations or fail clk_ops calls. Sure I posted this series a couple of days ago about this rework: https://lore.kernel.org/linux-arm-kernel/20240227194812.1209532-1-cristian.marussi@arm.com/ with a bit of context in the cover-letter and in the commit...but I can add more commenting of course if needed. Thanks for the review, Cristian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel