Message-ID: <4ba2b243-5622-bb27-6fc3-cd9457430e54@gmail.com>
Date:   Thu, 23 May 2019 10:30:50 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Peng Fan <peng.fan@....com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "jassisinghbrar@...il.com" <jassisinghbrar@...il.com>,
        "sudeep.holla@....com" <sudeep.holla@....com>
Cc:     "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "andre.przywara@....com" <andre.przywara@....com>,
        "van.freenix@...il.com" <van.freenix@...il.com>,
        "festevam@...il.com" <festevam@...il.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

Hi,

On 5/22/19 10:50 PM, Peng Fan wrote:
> This is a modified version from Andre Przywara's patch series
> https://lore.kernel.org/patchwork/cover/812997/.
> [1] is a draft implementation of i.MX8MM SCMI ATF implementation that
> use smc as mailbox, power/clk is included, but only part of clk has been
> implemented to work with hardware, power domain only supports get name
> for now.
> 
> The traditional Linux mailbox mechanism uses some kind of dedicated hardware
> IP to signal a condition to some other processing unit, typically a dedicated
> management processor.
> This mailbox feature is used for instance by the SCMI protocol to signal a
> request for some action to be taken by the management processor.
> However some SoCs does not have a dedicated management core to provide
> those services. In order to service TEE and to avoid linux shutdown
> power and clock that used by TEE, need let firmware to handle power
> and clock, the firmware here is ARM Trusted Firmware that could also
> run SCMI service.
> 
> The existing SCMI implementation uses a rather flexible shared memory
> region to communicate commands and their parameters, it still requires a
> mailbox to actually trigger the action.

We have had something similar done internally with a couple of minor
differences:

- a SGI is used to send SCMI notifications/delayed replies to support
asynchronism (patches are in the works to actually add that to the Linux
SCMI framework). There is no good support for SGI in the kernel right
now so we hacked up something from the existing SMP code and adding the
ability to register our own IPI handlers (SHAME!). Using a PPI should
work and should allow for using request_irq() AFAICT.

- the mailbox identifier is indicated as part of the SMC call such that
we can have multiple SCMI mailboxes serving both standard protocols and
non-standard (in the 0x80 and above) range, also they may have different
throughput (in hindsight, these could simply be different channels)

Your patch series looks both good and useful to me, I would just put a
provision in the binding to support an optional interrupt such that
asynchronism gets reasonably easy to plug in when it is available (and
desirable).

> 
> This patch series provides a Linux mailbox compatible service which uses
> smc calls to invoke firmware code, for instance taking care of SCMI requests.
> The actual requests are still communicated using the standard SCMI way of
> shared memory regions, but a dedicated mailbox hardware IP can be replaced via
> this new driver.
> 
> This simple driver uses the architected SMC calling convention to trigger
> firmware services, also allows for using "HVC" calls to call into hypervisors
> or firmware layers running in the EL2 exception level.
> 
> Patch 1 contains the device tree binding documentation, patch 2 introduces
> the actual mailbox driver.
> 
> Please note that this driver just provides a generic mailbox mechanism,
> though this is synchronous and one-way only (triggered by the OS only,
> without providing an asynchronous way of triggering request from the
> firmware).
> And while providing SCMI services was the reason for this exercise, this
> driver is in no way bound to this use case, but can be used generically
> where the OS wants to signal a mailbox condition to firmware or a
> hypervisor.
> Also the driver is in no way meant to replace any existing firmware
> interface, but actually to complement existing interfaces.
> 
> [1] https://github.com/MrVan/arm-trusted-firmware/tree/scmi
> 
> Peng Fan (2):
>   DT: mailbox: add binding doc for the ARM SMC mailbox
>   mailbox: introduce ARM SMC based mailbox
> 
>  .../devicetree/bindings/mailbox/arm-smc.txt        |  96 +++++++++++++
>  drivers/mailbox/Kconfig                            |   7 +
>  drivers/mailbox/Makefile                           |   2 +
>  drivers/mailbox/arm-smc-mailbox.c                  | 154 +++++++++++++++++++++
>  include/linux/mailbox/arm-smc-mailbox.h            |  10 ++
>  5 files changed, 269 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt
>  create mode 100644 drivers/mailbox/arm-smc-mailbox.c
>  create mode 100644 include/linux/mailbox/arm-smc-mailbox.h
> 


-- 
Florian

Powered by blists - more mailing lists