RE: [PATCH v7 3/6] dt-bindings: mailbox: imx-mu: add generic MU channel support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux