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,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 E78D6C433DF for ; Thu, 15 Oct 2020 08:54:30 +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 6172520BED for ; Thu, 15 Oct 2020 08:54:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="tm6II83P" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6172520BED 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=0cSG+2LLD0uKWZebB2grVpZ0WixAQ80Wv0NsOxrD8gk=; b=tm6II83PnEWpkThCXq7Vi4+p1 Hr6U3K/5W5t03NgQDqEgq04RLXLkrSwMNwtV8C2xYyT0eE1Ju16XBwX5HkknSTrN6EywZZKUiIFVb Zm0/OBtDW+acuYFH6OdO2GWbEJgx3TIukmwmpZbZJpEAwN0s4icDxAR6l7Au984HDMT0wsrbpwfKD QRa1ZuCQdFxXgtb1SaR0H+umg4+ZwRw/e6jzFF368pa+HwkuVspg0TDO69klWDMrz9QreggHs1ARd BADV04ajb7y5k9mL6I6iUwgvbRlR4oBzIQIc2mJgim/Xu8funnfWoVyY3pTl5StUDcRH+c/O3o9xI isKBOeF0A==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kSz0Y-00013C-AH; Thu, 15 Oct 2020 08:53:10 +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 1kSz0V-00012b-P1 for linux-arm-kernel@lists.infradead.org; Thu, 15 Oct 2020 08:53:08 +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 048BDD6E; Thu, 15 Oct 2020 01:53:06 -0700 (PDT) Received: from e120937-lin (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7CCA33F66B; Thu, 15 Oct 2020 01:53:04 -0700 (PDT) Date: Thu, 15 Oct 2020 09:52:58 +0100 From: Cristian Marussi To: Florian Fainelli Subject: Re: [PATCH 02/11] firmware: arm_scmi: hide protocols' private data Message-ID: <20201015085258.GB34395@e120937-lin> References: <20201014150545.44807-1-cristian.marussi@arm.com> <20201014150545.44807-3-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-20201015_045307_860349_44F7E8C9 X-CRM114-Status: GOOD ( 24.01 ) 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: vincent.guittot@linaro.org, sudeep.holla@arm.com, linux-kernel@vger.kernel.org, thara.gopinath@linaro.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-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, Oct 14, 2020 at 12:19:43PM -0700, Florian Fainelli wrote: > On 10/14/20 8:05 AM, Cristian Marussi wrote: > > Protocols private data were meant to be used exclusively by protocol code > > but they are currently exposed through the handle, so available also to > > SCMI drivers: move them away from handle into instance specific data and > > provide internal helpers to let protocols implementation set/get their own > > private data from protocol code. > > > > Signed-off-by: Cristian Marussi > > [snip] > > > > diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c > > index 94bcad9a7d19..4e8dafc36d7e 100644 > > --- a/drivers/firmware/arm_scmi/clock.c > > +++ b/drivers/firmware/arm_scmi/clock.c > > @@ -240,7 +240,8 @@ static int scmi_clock_rate_set(const struct scmi_handle *handle, u32 clk_id, > > u32 flags = 0; > > struct scmi_xfer *t; > > struct scmi_clock_set_rate *cfg; > > - struct clock_info *ci = handle->clk_priv; > > + struct clock_info *ci = > > + scmi_get_proto_priv(handle, SCMI_PROTOCOL_CLOCK); > > Having to repeat the protocol identifier is a bit error prone, does not > scale really and it will not change throughout the lifecycle of the > protocol handler. This is the least path of resistance though so it > makes sense you went that route for now. > > Instead of exposing scmi_handle towards the various operations what you > could do is just pass down an encapsulating object which is comprised of > a scmi_handle reference and the protocol's private structure already > resolved? That's really a good point. Later on in the series when modularization is introduced you'll see that I modified the protocol init callback to receive the core xfer_ops as a parameter so that any protocol even if loaded as a module can use the core xfer ops to built and generate SCMI msgs (without EXPORTing them), but one thing which is not addressed by this series still, is the fact that this way each protocol would have to specify for each of this ops a protocol_id param, thing which, beside being cumbersome as you said, lets the protocol potentially cheat, registering itself for protocol X but then potentially building and sending SCMI msgs for any other protocol: so your proposed solution could address also this case. Thanks for the hint ! Cristian > -- > Florian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel