From: Peter Hilber <peter.hilber@opensynergy.com> To: Arnd Bergmann <arnd@arndb.de>, Sudeep Holla <Sudeep.Holla@arm.com>, "Viresh Kumar" <viresh.kumar@linaro.org> Cc: Jassi Brar <jassisinghbrar@gmail.com>, Cristian Marussi <cristian.marussi@arm.com>, <peng.fan@nxp.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, ALKML <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH V4] firmware: arm_scmi: Make scmi core independent of the transport type Date: Fri, 24 Jan 2020 13:15:24 +0100 [thread overview] Message-ID: <82e1181a-b1ff-eccc-d61d-2da0e7afec25@opensynergy.com> (raw) In-Reply-To: <a9ec58818b5e0c982810e74efe3f5f22b930ae40.1579660436.git.viresh.kumar@linaro.org> On 22.01.20 03:36, 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 on 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. Sorry for being a bit late with my feedback. Accessing struct scmi_shared_mem members from driver.c forces any transport to also use the struct scmi_shared_mem layout (or at least pretend to do so). IMHO this does not work very well for transports which are not using a fixed memory region to transmit and receive. But I think the current approach will still work out. virtio transfers each message in a separate buffer, and always uses different parts of the buffer for transmit data and receive data, which is contrary to the assumptions of the struct scmi_shared_mem. The virtio transport will probably have no use for the struct scmi_shared_mem.channel_status and .flags. The check for SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE in scmi_tx_prepare() is not required for the virtio transport. I would have preferred (to have an option) to use as data passing interface to the transport just the struct scmi_xfer. A transport using this option would not implement ops (read|write)32 and memcpy_(from|to). The transport would also not call scmi_tx_prepare(), but instead take data from struct scmi_xfer directly. The transport would use a modified scmi_rx_callback() to notify that it updated the struct scmi_xfer. A helper to derive the struct scmi_xfer * from the message header would be extracted from scmi_rx_callback(). The scmi_xfer_poll_done() would become an (optional) transport op. Other remarks: If staying with this approach, it would be more elegant to add an abstraction through which the transport can set the SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE bit in the struct scmi_shared_mem. The SCMI spec allows both interrupt-based and polling-based completion notification. The transport should be able to indicate which notification methods it supports. The virtio transport would not want to support polling. > -static void scmi_rx_callback(struct mbox_client *cl, void *m) > +void scmi_rx_callback(struct scmi_chan_info *cinfo, struct scmi_xfer *t) scmi_rx_callback() doesn't need the struct scmi_xfer * parameter any more ATM (and the transport might also not know about it). Best regards, Peter Please mind our privacy notice<https://www.opensynergy.com/datenschutzerklaerung/privacy-notice-for-business-partners-pursuant-to-article-13-of-the-general-data-protection-regulation-gdpr/> pursuant to Art. 13 GDPR. // Unsere Hinweise zum Datenschutz gem. Art. 13 DSGVO finden Sie hier.<https://www.opensynergy.com/de/datenschutzerklaerung/datenschutzhinweise-fuer-geschaeftspartner-gem-art-13-dsgvo/> [tech_days_munchen] OpenSynergy TechDay München am 11. Februar 2020, ab 12:00Uhr, im Studio Balan, Moosacherstr. 86. Anmeldung bitte hier<mailto:sabine.mutumba@opensynergy.com>
WARNING: multiple messages have this Message-ID (diff)
From: Peter Hilber <peter.hilber@opensynergy.com> To: Arnd Bergmann <arnd@arndb.de>, Sudeep Holla <Sudeep.Holla@arm.com>, "Viresh Kumar" <viresh.kumar@linaro.org> Cc: ALKML <linux-arm-kernel@lists.infradead.org>, peng.fan@nxp.com, Jassi Brar <jassisinghbrar@gmail.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Cristian Marussi <cristian.marussi@arm.com> Subject: Re: [PATCH V4] firmware: arm_scmi: Make scmi core independent of the transport type Date: Fri, 24 Jan 2020 13:15:24 +0100 [thread overview] Message-ID: <82e1181a-b1ff-eccc-d61d-2da0e7afec25@opensynergy.com> (raw) In-Reply-To: <a9ec58818b5e0c982810e74efe3f5f22b930ae40.1579660436.git.viresh.kumar@linaro.org> On 22.01.20 03:36, 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 on 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. Sorry for being a bit late with my feedback. Accessing struct scmi_shared_mem members from driver.c forces any transport to also use the struct scmi_shared_mem layout (or at least pretend to do so). IMHO this does not work very well for transports which are not using a fixed memory region to transmit and receive. But I think the current approach will still work out. virtio transfers each message in a separate buffer, and always uses different parts of the buffer for transmit data and receive data, which is contrary to the assumptions of the struct scmi_shared_mem. The virtio transport will probably have no use for the struct scmi_shared_mem.channel_status and .flags. The check for SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE in scmi_tx_prepare() is not required for the virtio transport. I would have preferred (to have an option) to use as data passing interface to the transport just the struct scmi_xfer. A transport using this option would not implement ops (read|write)32 and memcpy_(from|to). The transport would also not call scmi_tx_prepare(), but instead take data from struct scmi_xfer directly. The transport would use a modified scmi_rx_callback() to notify that it updated the struct scmi_xfer. A helper to derive the struct scmi_xfer * from the message header would be extracted from scmi_rx_callback(). The scmi_xfer_poll_done() would become an (optional) transport op. Other remarks: If staying with this approach, it would be more elegant to add an abstraction through which the transport can set the SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE bit in the struct scmi_shared_mem. The SCMI spec allows both interrupt-based and polling-based completion notification. The transport should be able to indicate which notification methods it supports. The virtio transport would not want to support polling. > -static void scmi_rx_callback(struct mbox_client *cl, void *m) > +void scmi_rx_callback(struct scmi_chan_info *cinfo, struct scmi_xfer *t) scmi_rx_callback() doesn't need the struct scmi_xfer * parameter any more ATM (and the transport might also not know about it). Best regards, Peter Please mind our privacy notice<https://www.opensynergy.com/datenschutzerklaerung/privacy-notice-for-business-partners-pursuant-to-article-13-of-the-general-data-protection-regulation-gdpr/> pursuant to Art. 13 GDPR. // Unsere Hinweise zum Datenschutz gem. Art. 13 DSGVO finden Sie hier.<https://www.opensynergy.com/de/datenschutzerklaerung/datenschutzhinweise-fuer-geschaeftspartner-gem-art-13-dsgvo/> [tech_days_munchen] OpenSynergy TechDay München am 11. Februar 2020, ab 12:00Uhr, im Studio Balan, Moosacherstr. 86. Anmeldung bitte hier<mailto:sabine.mutumba@opensynergy.com> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-01-24 12:23 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-01-21 8:27 [PATCH V3] firmware: arm_scmi: Make scmi core independent of the transport type Viresh Kumar 2020-01-21 8:27 ` Viresh Kumar 2020-01-21 15:11 ` Arnd Bergmann 2020-01-21 15:11 ` Arnd Bergmann 2020-01-21 18:38 ` Sudeep Holla 2020-01-21 18:38 ` Sudeep Holla 2020-01-22 2:36 ` [PATCH V4] " Viresh Kumar 2020-01-22 2:36 ` Viresh Kumar 2020-01-22 12:15 ` Sudeep Holla 2020-01-22 12:15 ` Sudeep Holla 2020-01-23 10:30 ` Sudeep Holla 2020-01-23 10:30 ` Sudeep Holla 2020-01-23 11:27 ` Viresh Kumar 2020-01-23 11:27 ` Viresh Kumar 2020-01-23 11:37 ` Cristian Marussi 2020-01-23 11:37 ` Cristian Marussi 2020-01-23 15:17 ` Sudeep Holla 2020-01-23 15:17 ` Sudeep Holla 2020-01-24 3:02 ` Viresh Kumar 2020-01-24 3:02 ` Viresh Kumar 2020-01-24 11:22 ` Sudeep Holla 2020-01-24 11:22 ` Sudeep Holla 2020-01-24 12:15 ` Peter Hilber [this message] 2020-01-24 12:15 ` Peter Hilber 2020-01-24 18:28 ` Jassi Brar 2020-01-24 18:28 ` Jassi Brar 2020-01-22 12:44 ` [PATCH V3] " Cristian Marussi 2020-01-22 12:44 ` Cristian Marussi 2020-01-23 2:39 ` Viresh Kumar 2020-01-23 2:39 ` Viresh Kumar 2020-01-23 11:06 ` Cristian Marussi 2020-01-23 11:06 ` Cristian Marussi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=82e1181a-b1ff-eccc-d61d-2da0e7afec25@opensynergy.com \ --to=peter.hilber@opensynergy.com \ --cc=Sudeep.Holla@arm.com \ --cc=arnd@arndb.de \ --cc=cristian.marussi@arm.com \ --cc=jassisinghbrar@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=peng.fan@nxp.com \ --cc=viresh.kumar@linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.