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]

 



Hi Jassi,

> -----Original Message-----
> From: Jassi Brar [mailto:jassisinghbrar@xxxxxxxxx]
> Sent: Tuesday, July 31, 2018 8:42 PM
> To: A.s. Dong <aisheng.dong@xxxxxxx>

[...]

> >> Do you need me to send the full scu client driver patch for the reference?
> >> If yes, please feel free to let me know.
> >>
> > Yes, that'll make it clearer.
> >
> Thanks for sharing the patch offline.
> 
> I don't see why it can't be adapted to work with the simple 2-cell
> implementation. Instead of one virtual, ask for 4 physical channels and send
> data in u32 chunks sequentially over them.
> In fact, that should make it even better by removing the tight-loop (zero
> delay!!) polling in send_data() -- MU does support interrupts, why not use
> them to avoid polling?  If you find it hard, I can make a patch on top of your
> client patchset, once Oleksij is done with the MU driver.
> 

As you insist, I will try to implement multi channels mode, then we can do a
clear comparison to see which one is better for SCU mode rather than assumptions.

First of all,  using interrupt for each chan will likely to terribly slow down the SCU
Message handling speed. SCU Messages usually are handled by SCU firmware very
quickly in a few microseconds. (< 10us). That's why we use polling mode for SCU
message handling.

If we break down a SCU message into a few separate MU channel interrupts driven,
There will be a lot of unnecessary and meaningless interrupt handling.
And the whole flow may look like:

Sending Word 0 ->
Chan 0 interrupt -> (meaningless)
Sending Word 1 ->
Chan1 interrupt -> (meaningless)
Sending Word 2 ->
Chan2 interrupt -> (meaningless)
Sending Word 3 ->
Chan3 interrupt-> (meaningless)

Sending Word 4 on Chan 0 again ->
Wait for Chan 0 interrupt (meaningless) ->
...

Then waiting for Rx:

Chan 0 Rx interrupt ->
Read Word 0   ->
(Then we know response size here)

Chan 1 Rx interrupt ->
Read Word 1 ->
Chan 2 Rx interrupt ->
Read Word 2 ->
Chan 3 Rx interrupt ->
Read Word 3 ->

(If msg size > 4 words)
Chan 0 Rx interrupt ->
Read Word 4

...
And we must use a complicated way to map the received data of each channel
into one SCU msg buffer and need handle the sequence properly for size > 4
words case

Is this what you intended?

Per my understanding, there're several issues for this follow:
1) Many unnecessary and meaningless interrupts hanllding of Tx and RX
2) Performance downgrading
    A 4 word SCU MSG may need 8 interrupts to handle which terribly slow
    down the speed. (Interrupt latency will be terrible if many)
3) Complex RX flow in order to align with framework design

Any comments about these issues?

Generally, from what I see, the mailbox framework really is not designed for SCU type
devices that handling msg sending or receiving via multiple channels.  Including the
ring buffer supported for handling several msgs on the same channel and rich
completion mechanism, none of them are suitable for SCU MU in multiple channel mode.

What we really actually needed is a quite simple way to handle MSG Tx and Tx
synchronously.

int sc_call_rpc(sc_ipc_t ipc, void *msg, bool no_resp)
{       
        ...
        ret = mbox_send_message(sc_ipc->chan, msg);
        if (!no_resp) 
                ret = mbox_receive_message(sc_ipc->chan, msg);
        ...
        return ret;
}

For me, I don't think the above way driven by interrupt is suitable for SCU.
But I will still keep investigating if any better way to handle it in multi chan
mode as you wish.

How would you suggest for such situation in order to support SCU properly?

> I have some more suggestions for the client driver. But since I don't have to
> live with them (you do), I can't demand you make those changes. If you like,
> please feel free to cc me on the next revision of the client driver.

I certainly appreciate any clear and valuable improvement suggestions.
Just feel free to let me know if any better idea.

Regards
Dong Aisheng
��.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