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=-11.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SIGNED_OFF_BY,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 A2406C2D0A3 for ; Fri, 6 Nov 2020 16:08:37 +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 0218A2078B for ; Fri, 6 Nov 2020 16:08:36 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="1+bPptJS"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="EQVqsYo7" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0218A2078B 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=xtayL1sU+Zf0TSLDX39335Arfk5TCNgGPERu9A3VZZc=; b=1+bPptJShbJx5fwE1YH1yEwnN SLr2ct0xhn+sHbFHM1RG5ZShB3IZhd74kdSReGuA7zHl1VBx6buIF4shFCTt4Oe18xYT7BsC//XlV LwN7WUwwVFXMvuPWPc+DcwbrROCJLDl08kHam7+oYhodTB19Dcn8rRq5wzEpAOc59Xtq43GNg394Q kjK+XX47e+mgsYJyjOQmCLDmqtqvIB0I34oJQCFglRBKjmB2rpeoJb2LhAD7J9B2+QS5eziX1PliO WhoXpmZlcFOCelAdDnaLx0KjiXZ6Hh/YtUq/w1HP0OmOgBeWFFTDniNUSu3ewKfKdYddS6FwU6iAv 3AYtLZ2WQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kb4Gr-0000s7-JZ; Fri, 06 Nov 2020 16:07:25 +0000 Received: from mail-qk1-x744.google.com ([2607:f8b0:4864:20::744]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kb4Go-0000qJ-Tg for linux-arm-kernel@lists.infradead.org; Fri, 06 Nov 2020 16:07:23 +0000 Received: by mail-qk1-x744.google.com with SMTP id k9so1499472qki.6 for ; Fri, 06 Nov 2020 08:07:20 -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=WJnI/SYPYAielaIfnGZAZL8QkgwPBos547PDjdyfv5s=; b=EQVqsYo7w7ewip2Ujdl1KiYAuehAgsBEHK2DG38TFQzla/7Zt25aWFvNSFpJPSSHJ/ FkJ6izGYTZDd2uLFmbnD5CeT/0hLKp/c9MORdvTC/74iPTyBU9zp5APa4P2GOCTYMkbp Agk7JezDubCiQZvZE7m/Pzm45DuBIRJGw9sBi5y/rZY2q2gKX/6wDneAcPRdK2ctkpN5 Xvpsh2Rt8Leag0e41VBIAu8mXNZanHY3nB3Y/ZbY6awfh5xsiYgGCIF3SbI02P3LpDyP iz4zO8abBYYA/r4QpYenq+8SF8utVghwWY5HhAgSianBgCj33OmVmqymY985ZUacCl/3 cXVg== 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=WJnI/SYPYAielaIfnGZAZL8QkgwPBos547PDjdyfv5s=; b=MZL7hXKERHwr4R3h/mJ+b8FDItQSFERpwnWjA/tZLvPqPxZIunhWmu1tXI+08ClKYz MfHxCIRGe+iXaduy1s8gw0zTRSzoetoK7V1FC1BTovEa2qLBsmc8dLKr87Y+5Q4TA66w VKkiwmVZIez4NgBug/aElKxTPnxh2EXScRXmyidGN08fxFpfOtljCgMgChgMewbHj83r XPqQmRTBGUIPcvTOgz42huhKhWn/u89sXEsz2ABNK+5TBz2jkmNQWTEqbNsaPMLHy5Wl rAXs1PgG5U363kOhnF6K9SnaegDhr2Pg+1VnC5XbCtfJZRx6DRqtZ0fYSEevD06mSOdb 1btw== X-Gm-Message-State: AOAM532OahQ6VIxNE9s/QRBSUPEFeDor9xERwDeQMMIioR1sQLobeP1z tWyJPHqdFof+OHgEH8f18ZAPBQ== X-Google-Smtp-Source: ABdhPJzhJaXoh4g+SVHm9yyh5uCEVL9MPYlcYh97JUPsT5gIjrRv7pGw9NAqKwJ4QBQvAbrzcrBsEA== X-Received: by 2002:a37:6584:: with SMTP id z126mr2094666qkb.187.1604678839335; Fri, 06 Nov 2020 08:07:19 -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 z6sm749172qtw.36.2020.11.06.08.07.18 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 06 Nov 2020 08:07:18 -0800 (PST) Subject: Re: [PATCH v2 3/8] firmware: arm_scmi: introduce new protocol operations support To: Cristian Marussi References: <20201028202914.43662-1-cristian.marussi@arm.com> <20201028202914.43662-4-cristian.marussi@arm.com> <2b09a607-6470-ad41-fd19-6a7a248237c5@linaro.org> <20201104180808.GC24640@e120937-lin> From: Thara Gopinath Message-ID: <3c77ccfb-edc7-e36f-2a7f-8751bd8aee96@linaro.org> Date: Fri, 6 Nov 2020 11:07:17 -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: <20201104180808.GC24640@e120937-lin> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201106_110722_988383_A6675C31 X-CRM114-Status: GOOD ( 33.57 ) 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, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, 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 11/4/20 1:08 PM, Cristian Marussi wrote: > Hi, > > On Wed, Nov 04, 2020 at 11:16:31AM -0500, Thara Gopinath wrote: >> Hi Cristian, >> >> On 10/28/20 4:29 PM, Cristian Marussi wrote: >>> Expose a new generic get/put protocols API based on protocol handles; >>> provide also a devres managed version also for notifications. >> >> minor nit.. Maybe yous should reword this! Kind of confusing to understand! >> >> Also, if it was me, I will separate the notifications and get/put hooks >> into two separate patches. Not an issue though if you want to keep it >> in the same patch. > > You're right I have to reword the commit message, and I'll review the > possibility of a further split, even though many parts of this series > are tightly bound together given the kind of changes so sometimes to > avoid breaking bisectability I had to push painfully long patches (like > 6/8 :< )...but maybe I've got it wrong and this is not the case here. > >> >>> All SCMI drivers still keep using the old handle based interface. >>> >>> Signed-off-by: Cristian Marussi >>> --- >>> drivers/firmware/arm_scmi/driver.c | 126 +++++++++++++++++++++++++++++ >>> drivers/firmware/arm_scmi/notify.c | 123 ++++++++++++++++++++++++++++ >>> include/linux/scmi_protocol.h | 34 +++++++- >>> 3 files changed, 282 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c >>> index 8ca04acb6abb..4d86aafbf465 100644 >>> --- a/drivers/firmware/arm_scmi/driver.c >>> +++ b/drivers/firmware/arm_scmi/driver.c >>> @@ -15,6 +15,7 @@ >>> */ >>> #include >>> +#include >>> #include >>> #include >>> #include >>> @@ -728,6 +729,38 @@ void scmi_release_protocol(struct scmi_handle *handle, u8 protocol_id) >>> mutex_unlock(&info->protocols_mtx); >>> } >>> +/** >>> + * scmi_get_protocol_operations - Get protocol operations >>> + * @handle: A reference to the SCMI platform instance. >>> + * @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. >>> + * >>> + * Return: A reference to the requested protocol operations or error. >>> + * Must be checked for errors by caller. >>> + */ >>> +static const void __must_check * >>> +scmi_get_protocol_operations(struct scmi_handle *handle, u8 protocol_id, >>> + struct scmi_protocol_handle **ph) >>> +{ >>> + struct scmi_protocol_instance *pi; >>> + >>> + if (!ph) >>> + return ERR_PTR(-EINVAL); >>> + >>> + pi = scmi_get_protocol_instance(handle, protocol_id); >>> + if (IS_ERR(pi)) >>> + return pi; >>> + >>> + *ph = &pi->ph; >>> + >>> + return pi->proto->ops; >>> +} >>> + >>> void scmi_setup_protocol_implemented(const struct scmi_handle *handle, >>> u8 *prot_imp) >>> { >>> @@ -751,6 +784,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 >>> * >>> @@ -1004,6 +1126,10 @@ 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; >>> + handle->get_ops = scmi_get_protocol_operations; >>> + handle->put_ops = scmi_release_protocol; >> >> Why do you need a dev_res version and a non dev_res version? I checked >> patch 6 where you convert the drivers to use these hooks and all of them >> are using the dev res apis. >> > > Yes indeed, I wanted to drop the non devm_ version, but I was not sure > if for completeness I should have instead not provided...So I left it > to have it discussed on the list...I was hoping to be told to remove > those non devres :D > > Also because in fact any current or future SCMI driver (that are the only > possible users of this interface) will certainly have an scmi_dev to use > with devm_() methods. > > Probably the same is true for notify_ops, I could stick with the new > devres managed versions. I think you should drop it, unless any one else has issues not having it. -- Warm Regards Thara _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel