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 X-Spam-Level: X-Spam-Status: No, score=-20.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AB0BCC433E0 for ; Wed, 27 Jan 2021 16:27:38 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 0A75A20789 for ; Wed, 27 Jan 2021 16:27:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0A75A20789 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject: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=+puc6r1zzD9iqO+5jC8p7l97fJ7tIncXhl3b8qQJnrU=; b=US+Ua5Tm4FovIiVgpCnPFmQGG Gs+rrF/+HNl/3I2qX95B2027qxLx4/colTOfp4oa6W19Kfs5NMk8Rkd6eZxGHr0hQFvZS0YzERN+p nlxhc09/rF56sogmf6236g6WIYHbZegds9zMA+LB7m7TTXdTvZ7YMimB667VgA2LhCPTjVkICLcoD Xuv6WvDhYyQyW/34vcouhnnwszakkSQuA68fkv3zRhfaRJnfJrg4+BQSzYsZaUI7pGDI6BzXIuzmd gaFkDvQSQjL7R4tI0ObDOUmZQAbvhssRVbDL3PgLbbvQUI3tjZl3Ik8lc+C6t3YsUoLTkZHj8o6f2 hV1fN1IRQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l4ne9-0003Vc-Dw; Wed, 27 Jan 2021 16:26:21 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l4ne6-0003Up-6t for linux-arm-kernel@lists.infradead.org; Wed, 27 Jan 2021 16:26:19 +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 9D64E31B; Wed, 27 Jan 2021 08:26:14 -0800 (PST) Received: from e120937-lin (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B96AA3F66E; Wed, 27 Jan 2021 08:26:12 -0800 (PST) Date: Wed, 27 Jan 2021 16:26:05 +0000 From: Cristian Marussi To: Vincent Guittot Subject: Re: [PATCH v5 0/36] SCMI vendor protocols and modularization Message-ID: <20210127162605.GB9873@e120937-lin> References: <20210112192018.34994-1-cristian.marussi@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210127_112618_427591_940EE09E X-CRM114-Status: GOOD ( 41.71 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Florian Fainelli , Souvik Chakravarty , Sudeep Holla , linux-kernel , Thara Gopinath , LAK , james.quinlan@broadcom.com, Jonathan Cameron , Etienne Carriere , Lukasz Luba 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 Hi Vincent, On Wed, Jan 27, 2021 at 05:21:15PM +0100, Vincent Guittot wrote: > Hi Cristian, > > > On Tue, 12 Jan 2021 at 20:20, Cristian Marussi wrote: > > > > Hi all, > > > > The current SCMI implementation does not provide an interface to easily > > develop and include a custom vendor protocol implementation as prescribed > > by the SCMI standard, also because, there is not currently any custom > > protocol in the upstream to justify the development of a custom interface > > and its maintenance. > > > > Moreover the current interface exposes protocol operations to the SCMI > > driver users attaching per-protocol operations directly to the handle > > structure, which, in this way, tends to grow indefinitely for each new > > protocol addition. > > > > Beside this, protocols private data are also exposed via handle *_priv > > pointers, making such private data accessible also to the SCMI drivers > > even if neither really needed nor advisable. > > > > This series wants to address this by simplifying the SCMI protocols > > interface and reducing it, roughly, to these common generic operations: > > > > - handle->devm_get_protocol() > > - handle->devm_put_protocol() > > - handle->notify_ops() > > > > All protocols' private data pointers are removed from handle too and made > > accessible only to the protocols code through dedicated internal helpers. > > > > The concept of protocol handle is also introduced in the SCMI protocol code > > to represent a protocol instance initialized against a specific SCMI > > instance (handle), so that all the new protocol code uses such protocol > > handles wherever previously SCMI handle was used: this enable tigther > > control of what is exposed to the protocol code vs the SCMI drivers. > > > > Moreover protocol initialization is moved away from device probe and now > > happens on demand when the first user shows up (first .get_protocol), while > > de-initialization is performed once the last user of the protocol, even in > > terms of registered notifications callback, is gone, with the SCMI core > > taking care to perform all the needed underlying resource accounting. > > > > This way any new future standard or custom protocol implementation will > > expose a common unified interface which does not need to be extended > > endlessly: no need to maintain a custom interface only for vendor protos. > > SCMI drivers written on top of standard or custom protocols will use this > > same common interface to access any protocol operations. > > > > All existent upstream SCMI drivers are converted to this new interface. > > > > In order to make this migration painless and to avoid the need of a big > > un-mergeable jumbo patch touching all over the protocols and drivers (like > > it was in v2), since v3 the migration process has been heavily split with a > > bit of transient code added along the way (to preserve bisectability) and > > finally removed towards the ends of the series. > > Protocols and SCMI drivers migration to the new interface happens along > > patches 10->29. > > > > Note that even in v5 all the related SCMI drivers maintainers are still NOT > > CC'ed given I am still sort of gather consensus about the interface itself. > > > > Leveraging this new centralized and common initialization flow we took > > care also to refactor and simplify protocol-events registration and remove > > *notify_priv from the handle interface making it accessible only to the > > notification core. > > > > Patch 35 builds on top of this new interface and introduces a mechanism to > > define an SCMI protocol as a full blown module (possibly loadable) while > > leaving the core dealing with proper resource accounting. > > Standard protocols are still kept as builtins in this series, though. > > > > Finally, patch 36 introduces dynamic SCMI devices creation to avoid having > > to update the static module device table in the core each time a new driver > > is added. > > > > The whole SCMI stack can still be built alternatively as a module, with all > > the standard protocols included in scmi-module.ko in such a case. > > > > On top of this series an example SCMI Custom protocol 0x99 and related > > SCMI Custom Dummy driver has been built and it is available at [1] as a > > series of DEBUG patches on top this same series. > > > > The series is currently based on for-next/scmi [2] on top of: > > > > commit 6054d97ab512 ("MAINTAINERS: Update ARM SCMI entry") > > > > Any feedback welcome. > > I have tested your patchset on my setup which includes: > > - scmi performance protocol > - scmi power domain protocol > - scmi clock protocol > - scmi sensor protocol > - scmi notification > - registration of a custom scmi module (still under dev) > - 2 scmi channels > > And everything worked fine. So FWIW, > > Tested-by: Vincent Guittot > Great ! Thanks to have spent time on this. Cristian > > > > Thanks, > > > > Cristian > > > > --- > > v4 --> v5 > > - using standard kernel list instead of ad-hoc lists in 36/36 > > - renamed devm_get/put_ops to devm_get/put_protocol > > - dropped RFC patch on non devres get/put_ops > > > > v3 --> v4 > > - rebased on sudeep/for-next/scmi v5.11-rc1 > > - added a few comments more > > > > v2 --> v3 > > - added dynamic SCMI devices creation (getting rid of static device table) > > - heavy split of protocols and drivers migrations to the new interface > > - rebased on top of next-20201201 so migrating also: > > + SCMIv3.0 Voltage Domain protocol & SCMI Regulator > > + SCMIv3.0 Sensor Extensions > > > > v1 --> v2 > > - rebased on for-next/scmi v5.10-rc1 > > - introduced protocol handles > > - added devres managed devm_ variant for protocols operations > > - made all scmi_protocol refs const > > - introduced IDR to handle protocols instead of static array > > - refactored code around fast path > > > > [1]:https://gitlab.arm.com/linux-arm/linux-cm/-/commits/scmi_modules_ext_V5/ > > [2]:https://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git/log/?h=for-next/scmi > > > > > > Cristian Marussi (36): > > firmware: arm_scmi: review protocol registration interface > > firmware: arm_scmi: introduce protocol handle definitions > > firmware: arm_scmi: introduce devres get/put protocols operations > > firmware: arm_scmi: make notifications aware of protocols users > > firmware: arm_scmi: introduce new devres notification ops > > firmware: arm_scmi: refactor events registration > > firmware: arm_scmi: convert events registration to protocol handles > > firmware: arm_scmi: add new protocol handle core xfer ops > > firmware: arm_scmi: add helper to access revision area memory > > firmware: arm_scmi: port Base protocol to new interface > > firmware: arm_scmi: port Perf protocol to new protocols interface > > cpufreq: scmi: port driver to the new scmi_perf_proto_ops interface > > firmware: arm_scmi: remove legacy scmi_perf_ops protocol interface > > firmware: arm_scmi: port Power protocol to new protocols interface > > firmware: arm_scmi: port GenPD driver to the new scmi_power_proto_ops > > interface > > firmware: arm_scmi: remove legacy scmi_power_ops protocol interface > > firmware: arm_scmi: port Clock protocol to new protocols interface > > clk: scmi: port driver to the new scmi_clk_proto_ops interface > > firmware: arm_scmi: remove legacy scmi_clk_ops protocol interface > > firmware: arm_scmi: port Reset protocol to new protocols interface > > reset: reset-scmi: port driver to the new scmi_reset_proto_ops > > interface > > firmware: arm_scmi: remove legacy scmi_reset_ops protocol interface > > firmware: arm_scmi: port Sensor protocol to new protocols interface > > hwmon: (scmi) port driver to the new scmi_sensor_proto_ops interface > > firmware: arm_scmi: remove legacy scmi_sensor_ops protocol interface > > firmware: arm_scmi: port SystemPower protocol to new protocols > > interface > > firmware: arm_scmi: port Voltage protocol to new protocols interface > > regulator: scmi: port driver to the new scmi_voltage_proto_ops > > interface > > firmware: arm_scmi: remove legacy scmi_voltage_ops protocol interface > > firmware: arm_scmi: make references to handle const > > firmware: arm_scmi: cleanup legacy protocol init code > > firmware: arm_scmi: cleanup unused core xfer wrappers > > firmware: arm_scmi: cleanup events registration transient code > > firmware: arm_scmi: make notify_priv really private > > firmware: arm_scmi: add protocol modularization support > > firmware: arm_scmi: add dynamic scmi devices creation > > > > drivers/clk/clk-scmi.c | 27 +- > > drivers/cpufreq/scmi-cpufreq.c | 37 +- > > drivers/firmware/arm_scmi/base.c | 140 ++-- > > drivers/firmware/arm_scmi/bus.c | 100 ++- > > drivers/firmware/arm_scmi/clock.c | 129 ++-- > > drivers/firmware/arm_scmi/common.h | 117 +++- > > drivers/firmware/arm_scmi/driver.c | 759 +++++++++++++++++++-- > > drivers/firmware/arm_scmi/notify.c | 297 ++++++-- > > drivers/firmware/arm_scmi/notify.h | 38 +- > > drivers/firmware/arm_scmi/perf.c | 262 +++---- > > drivers/firmware/arm_scmi/power.c | 134 ++-- > > drivers/firmware/arm_scmi/reset.c | 146 ++-- > > drivers/firmware/arm_scmi/scmi_pm_domain.c | 26 +- > > drivers/firmware/arm_scmi/sensors.c | 230 ++++--- > > drivers/firmware/arm_scmi/system.c | 61 +- > > drivers/firmware/arm_scmi/voltage.c | 122 ++-- > > drivers/hwmon/scmi-hwmon.c | 24 +- > > drivers/regulator/scmi-regulator.c | 40 +- > > drivers/reset/reset-scmi.c | 33 +- > > include/linux/scmi_protocol.h | 183 ++--- > > 20 files changed, 1996 insertions(+), 909 deletions(-) > > > > -- > > 2.17.1 > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel