On 24.07.2018 01:31, Vladimir Zapolskiy wrote: > Hi Oleksij, > > On 07/22/2018 09:39 AM, Oleksij Rempel wrote: >> The Mailbox controller is able to send messages (up to 4 32 bit words) >> between the endpoints. >> >> This driver was tested using the mailbox-test driver sending messages >> between the Cortex-A7 and the Cortex-M4. >> >> Reviewed-by: Dong Aisheng <aisheng.dong@xxxxxxx> >> Signed-off-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> > > [snip] > >> +static int imx_mu_startup(struct mbox_chan *chan) >> +{ >> + struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox); >> + struct imx_mu_con_priv *cp = chan->con_priv; >> + int ret; >> + >> + cp->irq_desc = devm_kasprintf(priv->dev, GFP_KERNEL, "imx_mu_chan[%i]", >> + cp->idx); >> + if (!cp->irq_desc) >> + return -ENOMEM; >> + > > Again I would suggest to move this allocation to the loop in .probe function. Why? In my case I use 2 channels. Why should I allocate 4 descriptors for using only 2 of them? The SCU driver will use only one channel... > [snip] > >> + >> + for (i = 0; i < IMX_MU_CHANS; i++) { >> + struct imx_mu_con_priv *cp = &priv->con_priv[i]; >> + >> + cp->idx = i; >> + cp->irq = irq; > > ^^^^ right over here. > >> + priv->mbox_chans[i].con_priv = cp; >> + } >> + > > please feel free to add my technical side review tag to the next version: > > Reviewed-by: Vladimir Zapolskiy <vz@xxxxxxxxx> > > FWIW I find that review comments from Lucas are pretty valid, I would > recommend to incorporate them. Ok
Attachment:
signature.asc
Description: OpenPGP digital signature