Messages in this thread | | | From | Peng Fan <> | Subject | RE: [PATCH V6 0/4] mailbox/firmware: imx: support SCU channel type | Date | Wed, 18 Mar 2020 12:14:31 +0000 |
| |
> Subject: Re: [PATCH V6 0/4] mailbox/firmware: imx: support SCU channel > type > > On 2020-03-13 9:38 AM, Peng Fan wrote: > >> Subject: RE: [PATCH V6 0/4] mailbox/firmware: imx: support SCU > >> channel type > >> > >> Hi Leonard, > >> > >>> Subject: Re: [PATCH V6 0/4] mailbox/firmware: imx: support SCU > >>> channel type > >>> > >>> On 2020-03-04 7:55 AM, Peng Fan wrote: > >>>> 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 > >>>> 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" > >>>> Force imx-scu to use fast_ipc > >>>> > >>>> I not found a generic method to make SCFW message generic > >>>> enough, > >>> SCFW > >>>> 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 <leonard.crestez@nxp.com> > >>> > >> > >> Thanks for the test. > >> > >>> 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 > > >>> 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 :(
Might be long that 4 word messages, because not check TX empty and RX full in my patch.
> > ----- > > 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.
No. This will introduce more complexity. Per Oleksij's suggestion, I added the compatible string.
> > It also strange that you're adding a bool fast_ipc in imx-scu, do we really want > to support the old path?
It is to avoid break DT backward compatibility.
Thanks, Peng.
> > 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
| |