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=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,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 91F6BC33CA5 for ; Fri, 10 Jan 2020 12:31:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6C7F520721 for ; Fri, 10 Jan 2020 12:31:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728115AbgAJMbH (ORCPT ); Fri, 10 Jan 2020 07:31:07 -0500 Received: from foss.arm.com ([217.140.110.172]:43950 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727924AbgAJMbH (ORCPT ); Fri, 10 Jan 2020 07:31:07 -0500 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 84B151063; Fri, 10 Jan 2020 04:31:06 -0800 (PST) Received: from bogus (e103737-lin.cambridge.arm.com [10.1.197.49]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6AA483F534; Fri, 10 Jan 2020 04:31:05 -0800 (PST) Date: Fri, 10 Jan 2020 12:31:03 +0000 From: Sudeep Holla To: Viresh Kumar Cc: Arnd Bergmann , peng.fan@nxp.com, Vincent Guittot , Jassi Brar , "linux-kernel@vger.kernel.org" , Linux ARM , Sudeep Holla Subject: Re: [PATCH] firmware: arm_scmi: Make scmi core independent of transport type Message-ID: <20200110123103.GC45077@bogus> References: <5c545c2866ba075ddb44907940a1dae1d823b8a1.1575019719.git.viresh.kumar@linaro.org> <20200109093442.4jt44eu2zlmjaq3f@vireshk-i7> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200109093442.4jt44eu2zlmjaq3f@vireshk-i7> User-Agent: Mutt/1.9.4 (2018-02-28) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 09, 2020 at 03:04:42PM +0530, Viresh Kumar wrote: > On 09-01-20, 09:18, Arnd Bergmann wrote: > > On Fri, Nov 29, 2019 at 10:32 AM Viresh Kumar wrote: > > > > > > The SCMI specification is fairly independent of the transport protocol, > > > which can be a simple mailbox (already implemented) or anything else. > > > The current Linux implementation however is very much dependent of the > > > mailbox transport layer. > > > > > > This patch makes the SCMI core code (driver.c) independent of the > > > mailbox transport layer and moves all mailbox related code to a new > > > file: mailbox.c. > > > > > > We can now implement more transport protocols to transport SCMI > > > messages. > > > > > > The transport protocols just need to provide struct scmi_transport_ops, > > > with its version of the callbacks to enable exchange of SCMI messages. > > > > > > Signed-off-by: Viresh Kumar > > > > Conceptually I think this is fine, but as others have said, it would be > > better to have another transport implementation posted along with this > > to see if the interfaces actually work out. > > @Sudeep/Vincent: Do you think we can add another transport > implementation something right away for it ? > > @Peng ? > > > > +/** > > > + * struct scmi_chan_info - Structure representing a SCMI channel information > > > + * > > > + * @payload: Transmit/Receive payload area > > > + * @dev: Reference to device in the SCMI hierarchy corresponding to this > > > + * channel > > > + * @handle: Pointer to SCMI entity handle > > > + * @transport_info: Transport layer related information > > > + */ > > > +struct scmi_chan_info { > > > + void __iomem *payload; > > > + struct device *dev; > > > + struct scmi_handle *handle; > > > + void *transport_info; > > > +}; > > > > I would assume that with another transport, the 'payload' pointer would > > not be __iomem > > Hmm, okay. I just separated things based on the current transport and > didn't add much changes on top of it as I wasn't sure how things are > going to look with next transport and so left the changes for then. > > I can now drop it though. > > > > +static int scmi_set_transport_ops(struct scmi_info *info) > > > +{ > > > + struct scmi_transport_ops *ops; > > > + struct device *dev = info->dev; > > > + > > > + /* Only mailbox method supported for now */ > > > + ops = scmi_mailbox_get_ops(dev); > > > + if (!ops) { > > > + dev_err(dev, "Transport protocol not found in %pOF\n", > > > + dev->of_node); > > > + return -EINVAL; > > > + } > > > + > > > + info->transport_ops = ops; > > > + return 0; > > > +} > > > > This looks odd: rather than guessing the transport type based on > > random DT properties, I would prefer to have it determined by > > the device compatible string, and have different drivers bind > > to one of them each, with each driver linking against a common > > base implementation, either as separate modules or in one file. > > Since there are no platforms using the scmi binding in mainline kernel > for now, it won't be difficult to add new compatible strings. I am fine adding new compatible but since the binding is present in the mainline for several releases now, we may have to have fallback to mailbox as default if any of the new compatibles added is missing. -- Regards, Sudeep 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=-5.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,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 02FE5C33CA5 for ; Fri, 10 Jan 2020 12:31:12 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id C8E6220721 for ; Fri, 10 Jan 2020 12:31:11 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="Nv6LZTME" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C8E6220721 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-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=bombadil.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=3LXzBlkeCDHnIDtRQl/nLa2F6BJbB2FV2Yrh06WvnTI=; b=Nv6LZTMEJpq/5+ ZUP6b7L6ZZXni+hEEhfyLtn6wHuyrH+Pr3I0yYh3rXdxrXSspNiJNDU5ufc4TJukz/vuYwg1FXUdW WtNjhC4PGzVw5HK7sLAPCQCvJ544NXy6se76ApGqyB4k2LIDjjyGQg58qAGPEBJPZx1mga4DftnF1 TM1zAfO5C3YXvTxbZ+KdlgnYHbg6or5sIShqmh+bp1iCbor4S46CbUJTrE7N7BTro1oy/fIaQwmcz 2e9qZvF2yULcHoHP8xs43c+CgYuVD/UUQXGdO91HEgGLSO5tq4DeqGP0R3ahjBbryU7fs5xuiAE75 MRT+R33/FNlpkv8w6joA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1iptRW-0003FD-So; Fri, 10 Jan 2020 12:31:10 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1iptRT-0003EW-C5 for linux-arm-kernel@lists.infradead.org; Fri, 10 Jan 2020 12:31: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 84B151063; Fri, 10 Jan 2020 04:31:06 -0800 (PST) Received: from bogus (e103737-lin.cambridge.arm.com [10.1.197.49]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6AA483F534; Fri, 10 Jan 2020 04:31:05 -0800 (PST) Date: Fri, 10 Jan 2020 12:31:03 +0000 From: Sudeep Holla To: Viresh Kumar Subject: Re: [PATCH] firmware: arm_scmi: Make scmi core independent of transport type Message-ID: <20200110123103.GC45077@bogus> References: <5c545c2866ba075ddb44907940a1dae1d823b8a1.1575019719.git.viresh.kumar@linaro.org> <20200109093442.4jt44eu2zlmjaq3f@vireshk-i7> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200109093442.4jt44eu2zlmjaq3f@vireshk-i7> 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-20200110_043107_504628_84D841CB X-CRM114-Status: GOOD ( 27.00 ) 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: peng.fan@nxp.com, Arnd Bergmann , Jassi Brar , "linux-kernel@vger.kernel.org" , Sudeep Holla , Linux ARM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Jan 09, 2020 at 03:04:42PM +0530, Viresh Kumar wrote: > On 09-01-20, 09:18, Arnd Bergmann wrote: > > On Fri, Nov 29, 2019 at 10:32 AM Viresh Kumar wrote: > > > > > > The SCMI specification is fairly independent of the transport protocol, > > > which can be a simple mailbox (already implemented) or anything else. > > > The current Linux implementation however is very much dependent of the > > > mailbox transport layer. > > > > > > This patch makes the SCMI core code (driver.c) independent of the > > > mailbox transport layer and moves all mailbox related code to a new > > > file: mailbox.c. > > > > > > We can now implement more transport protocols to transport SCMI > > > messages. > > > > > > The transport protocols just need to provide struct scmi_transport_ops, > > > with its version of the callbacks to enable exchange of SCMI messages. > > > > > > Signed-off-by: Viresh Kumar > > > > Conceptually I think this is fine, but as others have said, it would be > > better to have another transport implementation posted along with this > > to see if the interfaces actually work out. > > @Sudeep/Vincent: Do you think we can add another transport > implementation something right away for it ? > > @Peng ? > > > > +/** > > > + * struct scmi_chan_info - Structure representing a SCMI channel information > > > + * > > > + * @payload: Transmit/Receive payload area > > > + * @dev: Reference to device in the SCMI hierarchy corresponding to this > > > + * channel > > > + * @handle: Pointer to SCMI entity handle > > > + * @transport_info: Transport layer related information > > > + */ > > > +struct scmi_chan_info { > > > + void __iomem *payload; > > > + struct device *dev; > > > + struct scmi_handle *handle; > > > + void *transport_info; > > > +}; > > > > I would assume that with another transport, the 'payload' pointer would > > not be __iomem > > Hmm, okay. I just separated things based on the current transport and > didn't add much changes on top of it as I wasn't sure how things are > going to look with next transport and so left the changes for then. > > I can now drop it though. > > > > +static int scmi_set_transport_ops(struct scmi_info *info) > > > +{ > > > + struct scmi_transport_ops *ops; > > > + struct device *dev = info->dev; > > > + > > > + /* Only mailbox method supported for now */ > > > + ops = scmi_mailbox_get_ops(dev); > > > + if (!ops) { > > > + dev_err(dev, "Transport protocol not found in %pOF\n", > > > + dev->of_node); > > > + return -EINVAL; > > > + } > > > + > > > + info->transport_ops = ops; > > > + return 0; > > > +} > > > > This looks odd: rather than guessing the transport type based on > > random DT properties, I would prefer to have it determined by > > the device compatible string, and have different drivers bind > > to one of them each, with each driver linking against a common > > base implementation, either as separate modules or in one file. > > Since there are no platforms using the scmi binding in mainline kernel > for now, it won't be difficult to add new compatible strings. I am fine adding new compatible but since the binding is present in the mainline for several releases now, we may have to have fallback to mailbox as default if any of the new compatibles added is missing. -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel