> -----Original Message----- > From: Jassi Brar [mailto:jassisinghbrar@xxxxxxxxx] > Sent: Monday, July 30, 2018 9:05 PM > To: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> > Cc: A.s. Dong <aisheng.dong@xxxxxxx>; , Sascha Hauer > <kernel@xxxxxxxxxxxxxx>; Shawn Guo <shawnguo@xxxxxxxxxx>; Fabio > Estevam <fabio.estevam@xxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; > Mark Rutland <mark.rutland@xxxxxxx>; Vladimir Zapolskiy > <vladimir_zapolskiy@xxxxxxxxxx>; ", linux-arm- > kernel"@lists.infradead.org; , linux-arm-kernel@xxxxxxxxxxxxxxxxxxx, linux- > mediatek@xxxxxxxxxxxxxxxxxxx, srv_heupstream <linux- > mediatek@xxxxxxxxxxxxxxxxxxx>; srv_heupstream <linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx>; Devicetree List <devicetree@xxxxxxxxxxxxxxx>; > dl-linux-imx <linux-imx@xxxxxxx> > Subject: Re: [PATCH v7 3/6] dt-bindings: mailbox: imx-mu: add generic MU > channel support > > On Mon, Jul 30, 2018 at 1:05 PM, Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> > wrote: > > Hi Aishen, Jassie, > > > > i'm lost in this discussion. Please, as soon as I need to add some > > changes to my patches, notify me. Preferably on top for email. > > > I am ok with however you choose to implement, 8 unidirectional or 4 > bidirectional channels or whatever. > > We just can't have protocol specific s/w modes in the controller drivers. > Do you think implement "Passing short messages" defined in reference manual in controller driver is protocol specific? That seems like our most argument before. > The best solution is to fix the SCU firmware. If that is _really_ impossible, I > provided a solution (3 cells work around). If you have a better idea please > feel free to propose and implement that. > That's out of our control. SCU firmware is not using Linux and there's no mailbox framework. It's simply follow "passing short messages" way to send multi words with 4 data Registers. > It will also help if you could share the user code of "scu-mode". If there is no > such code (and we know the driver doesn't respect the "scu-mode" property) > why do we even have that binding? Maybe drop it. You can refer to this patch for the user code of "scu-mode". [RFC PATCH v6 2/2] mailbox: imx-mailbox: add scu protocol support https://www.spinics.net/lists/arm-kernel/msg665135.html And I do admit that there're two protocol specific bits in the driver. 1. Unlike TI message header, SCU size is in the second u8. Ti message header: struct ti_msgmgr_message { size_t len; u8 *buf; }; Controller get the size from the first u32. SCU message header: struct sc_rpc_msg { uint8_t ver; uint8_t size; uint8_t svc; uint8_t func; }; Controller get the size from the second u8. And a full SCU message is like: struct imx_sc_msg_req_set_clock_rate { struct sc_rpc_msg hdr; u32 rate; u16 resource; u8 clk; } __packed; 2. There's no synchronous read function in mailbox framework, So we use mbox_client_peek_data to read data which can't return the read data. In order to save a mem copy from read data to the data buffer, the controller driver saves the tx data buffer in itself. (Anyway to improve? Maybe we should implement a synchronous mbox_client_read_data()?) And for SCU IPC client, it's something like: int sc_call_rpc(sc_ipc_t ipc, void *msg, bool no_resp) { ... mbox_send_message(sc_ipc->chan, msg); if (!no_resp) ret = mbox_client_peek_data(sc_ipc->chan); mbox_client_txdone(sc_ipc->chan, ret); ... } int sc_ipc_open(sc_ipc_t *ipc, struct device *dev) { sc_ipc->cl.dev = dev; sc_ipc->cl.tx_block = false; sc_ipc->cl.knows_txdone = true; sc_ipc->chan = mbox_request_channel(&sc_ipc->cl, 0); ... } Any better ideas? Regards Dong Aisheng > > thnx. ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f