On 2020-03-13 9:38 AM, Peng Fan wrote:

quoted

Subject: RE: [PATCH V6 0/4] mailbox/firmware: imx: support SCU channel
type

Hi Leonard,

quoted

Subject: Re: [PATCH V6 0/4] mailbox/firmware: imx: support SCU channel
type

On 2020-03-04 7:55 AM, Peng Fan wrote:

quoted

From: Peng Fan <peng.fan@nxp.com>

V6:
   Add Oleksij's R-b tag
   Patch 3/4, per

https://www.kernel.org/doc/Documentation/printk-formats.txt

quoted

   should use %zu for printk sizeof

V5:
   Move imx_mu_dcfg below imx_mu_priv
   Add init hooks to imx_mu_dcfg
   drop __packed __aligned
   Add more debug msg
   code style cleanup

V4:
   Drop IMX_MU_TYPE_[GENERIC, SCU]
   Pack MU chans init to separate function
   Add separate function for SCU chans init and xlate
   Add santity check to msg hdr.size
   Limit SCU MU chans to 6, TX0/RX0/RXDB[0-3]

V3:
   Rebase to Shawn's for-next
   Include fsl,imx8-mu-scu compatible
   Per Oleksij's comments, introduce generic tx/rx and added scu mu type
   Check fsl,imx8-mu-scu in firmware driver for fast_ipc

V2:
   Drop patch 1/3 which added fsl,scu property
   Force to use scu channel type when machine has node compatible

"fsl,imx-scu"

quoted

   Force imx-scu to use fast_ipc

   I not found a generic method to make SCFW message generic enough,

SCFW

quoted

   message is not fixed length including TX and RX. And it use TR0/RR0
   interrupt.

V1:
Sorry to bind the mailbox/firmware patch together. This is make it
to understand what changed to support using 1 TX and 1 RX channel
for SCFW message.

Per i.MX8QXP Reference mannual, there are several message using
examples. One of them is:
Passing short messages: Transmit register(s) can be used to pass
short messages from one to four words in length. For example, when a
four-word message is desired, only one of the registers needs to
have its corresponding interrupt enable bit set at the receiver side.

This patchset is to using this for SCFW message to replace four TX
and four RX method.

Tested-by: Leonard Crestez <redacted>

Thanks for the test.

quoted

My stress tests pass on imx8qxp with this patcheset, however
performance is not greatly improved. My guess is that this happens
because of too many interrupts.

Might be. Could you share your testcase?

https://github.com/cdleonard/imx-scu-test

quoted

quoted

Is there really a reason to enable TIE? Spinning on TE bits without
any interrupts should be just plain faster.

I could try to disable TIE and give a try. If performance improves lot, I could
change to non TX interrupt.

After rethinking about this, we need TX interrupt, otherwise we have to
use TX_POLL which is slower or let the client kick the TX state machine.

Compared with original method, this already reduces to use 1 TX and 1 RX
interrupt. This already good for system.

Sorry, I missed that fact that your patches don't include the required 
DTS changes. Indeed that is only one TX and one RX irq per call now.

Running my test now results in RX timeout :(

-----

On an unrelated note: are you sure it is appropriate to change the 
compat string here? Another way to implement direct SCU communication 
would be as another channel type, IMX_MU_TYPE_SCUTX.

It also strange that you're adding a bool fast_ipc in imx-scu, do we 
really want to support the old path?

If SCU protocol was implemented as a channel type then maybe we could 
sidestep mbox_request_channel_by_name, parse mboxes manually and always 
request MU_TYPE_SCUTX.

--
Regards,
Leonard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel