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=-15.8 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,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable 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 C3B11C433E6 for ; Thu, 7 Jan 2021 14:30:45 +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 849672336D for ; Thu, 7 Jan 2021 14:30:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 849672336D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org 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-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=6ClNcjafiDlm9IxMTQnM8ckexUcIsntlDiZzBeyZcL4=; b=W+QHmJoas8pcNP0EBu4JyvmMl NFBCLFFhVEuao1b2R03GqvoqOxrV7f1eFJldvuXuj16Kia5BEvrTSgrCIdeGZ85C0s33JhfDZrEnF /8iNt1Xnk4hfgwi6viHz9Vqkmh8GSHp92ERC6PG03pLxPWyykO74KsRTaYlYHOki2iH7TpOZwhQuf vHQUVbjJLWE1F+LvtKkXM4l7eqPBmbTSyyw5RHIEhwM3c0cDQvgj62H4AoOGSSZIs91VjyBt9sURF KSPsPMU0naVgKhNv60C2vuoli0e/yLRDiUThX11CFPt0RlO4NfED2/9TtFZabcBzc6DAwrPDYpU5W 6qAkZR8SQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kxWHP-0001s5-Jw; Thu, 07 Jan 2021 14:28:47 +0000 Received: from mail-io1-xd2c.google.com ([2607:f8b0:4864:20::d2c]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kxWHN-0001qg-2E for linux-arm-kernel@lists.infradead.org; Thu, 07 Jan 2021 14:28:46 +0000 Received: by mail-io1-xd2c.google.com with SMTP id d9so6304665iob.6 for ; Thu, 07 Jan 2021 06:28:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=AS955oV0sNNxmymQ9S2gFLRD6YA392eHboBTXK85YcU=; b=gcL7pNIOj/1nqlQvwP3SnVetZ+no2EJIGasfSLkk1iESsiEOfjzZqq0mxjFG8L5JUh 7eJt0+w7Cqa9iZyx7AMwLlGldWIHKCwCbBVkyT+9l4X3C3aN1fCnHWqD9z3ZyJFzfk/z j2iikI4OofqL7EQalt/fpjloqsxGazsZlrjsHr3Pokdm3c/UfRJ3stsZch718YeoY7wH xR9Xhw6MHYLT7jNoHyitprskDxI8yVhHOTN2jQ6TT/p9c5cW769I0PVLTKHzTK1EXRBn V3BdK4iFgKl9rIOq6/iXAVYqXSoiFyuCK5TIz+PBSK3cfFNSimybHlQ2awupIWNEvIzN hc5A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=AS955oV0sNNxmymQ9S2gFLRD6YA392eHboBTXK85YcU=; b=mtWNetJmzN3LSBI/C4nYbzQCon8iCzPwmZIkywfqvXjvLIFT9kwDqp/G0xQLhcayQO ItjEXTR6c0yjc3D3YWRo33Yj0ecItMfKtLOzYwgYr0oMIBNxX+Sx1KKQSPqgqoMxTQjZ oYFG81xnD1084VexMjAypPffVA2UIgBljK0nfsxE7sL+PRZGqqDdNx7+viomAFfjNp6R pHYIULpkUst6p7S1euB/he8i3MOxVj+ZPqdcG+poNRu/xDbUz0bhjsCA3m9TKNnlf0Cq P6P4H3kBZyrD32YYR+IFIGLkMtIYKOkJRoCNLEugaYu5D2fApdSZEZ1qV3oMU/3/R6AZ JMSQ== X-Gm-Message-State: AOAM531Wm4pCHzpNrnDwKv6gNhkgRwUmDASW5hzNAISuULqbFI+Ji41c Nvll5y8+GRfRrdtQ2C7gYBHvog== X-Google-Smtp-Source: ABdhPJwtQ6aJYJWUYyk8YyIOUF0sRew3HbW5wqApB9UoXF7UZWCaflM3ZEqr0A//V1q/DSN+nRSVJw== X-Received: by 2002:a6b:7704:: with SMTP id n4mr1475491iom.159.1610029719886; Thu, 07 Jan 2021 06:28:39 -0800 (PST) Received: from [192.168.1.93] (pool-71-163-245-5.washdc.fios.verizon.net. [71.163.245.5]) by smtp.gmail.com with ESMTPSA id f3sm4523162ilu.74.2021.01.07.06.28.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 07 Jan 2021 06:28:39 -0800 (PST) Subject: Re: [PATCH v4 03/37] firmware: arm_scmi: introduce devres get/put protocols operations To: Cristian Marussi , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org References: <20210106201610.26538-1-cristian.marussi@arm.com> <20210106201610.26538-4-cristian.marussi@arm.com> From: Thara Gopinath Message-ID: Date: Thu, 7 Jan 2021 09:28:37 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20210106201610.26538-4-cristian.marussi@arm.com> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210107_092845_124763_170BCA20 X-CRM114-Status: GOOD ( 30.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: f.fainelli@gmail.com, vincent.guittot@linaro.org, sudeep.holla@arm.com, james.quinlan@broadcom.com, Jonathan.Cameron@Huawei.com, souvik.chakravarty@arm.com, etienne.carriere@linaro.org, lukasz.luba@arm.com Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 1/6/21 3:15 PM, Cristian Marussi wrote: > Expose to the SCMI drivers a new devres managed common protocols API based > on generic get/put methods and protocol handles. > > All drivers still keep using the old API, no functional change. > > Signed-off-by: Cristian Marussi > --- > drivers/firmware/arm_scmi/driver.c | 92 ++++++++++++++++++++++++++++++ > include/linux/scmi_protocol.h | 11 ++++ > 2 files changed, 103 insertions(+) > > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c > index 10fe9aacae1b..fbc3ba1b69f6 100644 > --- a/drivers/firmware/arm_scmi/driver.c > +++ b/drivers/firmware/arm_scmi/driver.c > @@ -15,6 +15,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -732,6 +733,95 @@ scmi_is_protocol_implemented(const struct scmi_handle *handle, u8 prot_id) > return false; > } > > +struct scmi_protocol_devres { > + struct scmi_handle *handle; > + u8 protocol_id; > +}; > + > +static void scmi_devm_release_protocol(struct device *dev, void *res) > +{ > + struct scmi_protocol_devres *dres = res; > + > + scmi_release_protocol(dres->handle, dres->protocol_id); > +} > + > +/** > + * scmi_devm_get_protocol_ops - Devres managed get protocol operations > + * @sdev: A reference to an scmi_device whose embedded struct device is to > + * be used for devres accounting. > + * @protocol_id: The protocol being requested. > + * @ph: A pointer reference used to pass back the associated protocol handle. > + * > + * Get hold of a protocol accounting for its usage, eventually triggering its > + * initialization, and returning the protocol specific operations and related > + * protocol handle which will be used as first argument in most of the > + * protocols operations methods. > + * Being a devres based managed method, protocol hold will be automatically > + * released, and possibly de-initialized on last user, once the SCMI driver > + * owning the scmi_device is unbound from it. > + * > + * Return: A reference to the requested protocol operations or error. > + * Must be checked for errors by caller. > + */ > +static const void __must_check * > +scmi_devm_get_protocol_ops(struct scmi_device *sdev, u8 protocol_id, > + struct scmi_protocol_handle **ph) > +{ > + struct scmi_protocol_instance *pi; > + struct scmi_protocol_devres *dres; > + struct scmi_handle *handle = sdev->handle; > + > + if (!ph) > + return ERR_PTR(-EINVAL); > + > + dres = devres_alloc(scmi_devm_release_protocol, > + sizeof(*dres), GFP_KERNEL); > + if (!dres) > + return ERR_PTR(-ENOMEM); > + > + pi = scmi_get_protocol_instance(handle, protocol_id); > + if (IS_ERR(pi)) { > + devres_free(dres); > + return pi; > + } > + > + dres->handle = handle; > + dres->protocol_id = protocol_id; > + devres_add(&sdev->dev, dres); > + > + *ph = &pi->ph; > + > + return pi->proto->ops; > +} > + > +static int scmi_devm_protocol_match(struct device *dev, void *res, void *data) > +{ > + struct scmi_protocol_devres *dres = res; > + > + if (WARN_ON(!dres || !data)) > + return 0; > + > + return dres->protocol_id == *((u8 *)data); > +} > + > +/** > + * scmi_devm_put_protocol_ops - Devres managed put protocol operations > + * @sdev: A reference to an scmi_device whose embedded struct device is to > + * be used for devres accounting. > + * @protocol_id: The protocol being requested. > + * > + * Explicitly release a protocol hold previously obtained calling the above > + * @scmi_devm_get_protocol_ops. > + */ > +static void scmi_devm_put_protocol_ops(struct scmi_device *sdev, u8 protocol_id) > +{ > + int ret; > + > + ret = devres_release(&sdev->dev, scmi_devm_release_protocol, > + scmi_devm_protocol_match, &protocol_id); > + WARN_ON(ret); > +} > + > /** > * scmi_handle_get() - Get the SCMI handle for a device > * > @@ -986,6 +1076,8 @@ static int scmi_probe(struct platform_device *pdev) > handle = &info->handle; > handle->dev = info->dev; > handle->version = &info->version; > + handle->devm_get_ops = scmi_devm_get_protocol_ops; > + handle->devm_put_ops = scmi_devm_put_protocol_ops; > > ret = scmi_txrx_setup(info, dev, SCMI_PROTOCOL_BASE); > if (ret) > diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h > index 757a826e3cef..2fd2fffb4024 100644 > --- a/include/linux/scmi_protocol.h > +++ b/include/linux/scmi_protocol.h > @@ -57,6 +57,8 @@ struct scmi_clock_info { > }; > > struct scmi_handle; > +struct scmi_device; > +struct scmi_protocol_handle; > > /** > * struct scmi_clk_ops - represents the various operations provided > @@ -593,6 +595,9 @@ struct scmi_notify_ops { > * @sensor_ops: pointer to set of sensor protocol operations > * @reset_ops: pointer to set of reset protocol operations > * @voltage_ops: pointer to set of voltage protocol operations > + * @devm_get_ops: devres managed method to acquire a protocol and get specific > + * operations and a dedicated protocol handler > + * @devm_put_ops: devres managed method to release a protocol > * @notify_ops: pointer to set of notifications related operations > * @perf_priv: pointer to private data structure specific to performance > * protocol(for internal use only) > @@ -618,6 +623,12 @@ struct scmi_handle { > const struct scmi_sensor_ops *sensor_ops; > const struct scmi_reset_ops *reset_ops; > const struct scmi_voltage_ops *voltage_ops; > + > + const void __must_check * > + (*devm_get_ops)(struct scmi_device *sdev, u8 proto, > + struct scmi_protocol_handle **ph); > + void (*devm_put_ops)(struct scmi_device *sdev, u8 proto); These names are misleading. The devm_get_ops does two things. One populate the scmi_protocol_handle, second return the protocol ops. Either split this into two separate functions or rename it into something like devm_get_protocol (or something better). Similar comment for devm_put_ops as there is no releasing of ops happening here per say. Also I am still not convinced that protocol_instance should be hidden from the client drivers. But if everyone else is aligned towards this approach, I am fine. > + > const struct scmi_notify_ops *notify_ops; > /* for protocol internal use */ > void *perf_priv; > -- Warm Regards Thara _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel