Messages in this thread | | | From | Richard Zhu <> | Subject | RE: [EXT] Re: [RFC] mailbox: imx: Add support for i.MX v1 messaging unit | Date | Fri, 26 Jul 2019 09:23:42 +0000 |
| |
Hi Oleksij: Thanks for your comments.
> -----Original Message----- > From: Oleksij Rempel <o.rempel@pengutronix.de> > Sent: 2019年7月26日 17:17 > To: Richard Zhu <hongxing.zhu@nxp.com> > Cc: jassisinghbrar@gmail.com; Aisheng Dong <aisheng.dong@nxp.com>; > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org > Subject: Re: [RFC] mailbox: imx: Add support for i.MX v1 messaging unit > > > On Fri, Jul 26, 2019 at 04:29:36PM +0800, Richard Zhu wrote: > > There is a version1.0 MU on i.MX7ULP platform. > > One new version ID register is added, and the offset is 0. > > TRn registers are defined at the offset 0x20 ~ 0x2C. > > RRn registers are defined at the offset 0x40 ~ 0x4C. > > SR/CR registers are defined at 0x60/0x64. > > Extend this driver to support it. > > Heh.. i feel like NXP/Freescale have old tradition of moving around registers and > adding/removing the version register :) > > Since it is not first time, I would prefer to have register layout linked on probe > and not calculating or if-ing it on runtime. Especially, because I assume it is not > the last layout change. > > For example see: > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.boo > tlin.com%2Flinux%2Fv5.3-rc1%2Fsource%2Fdrivers%2Fremoteproc%2Fimx_rpro > c.c%23L336&data=02%7C01%7Chongxing.zhu%40nxp.com%7C275ed465c > a75424f583008d711aa0e58%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7 > C0%7C636997294396884235&sdata=tTikxc3oIpCEkmQELWTz3XB%2BN8f5 > cudEAWhmNziOHtQ%3D&reserved=0 > dcfg = of_device_get_match_data(dev); > > or > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.boo > tlin.com%2Flinux%2Fv5.3-rc1%2Fsource%2Fdrivers%2Fnet%2Fethernet%2Fathe > ros%2Fag71xx.c%23L1651&data=02%7C01%7Chongxing.zhu%40nxp.com > %7C275ed465ca75424f583008d711aa0e58%7C686ea1d3bc2b4c6fa92cd99c5c3 > 01635%7C0%7C0%7C636997294396884235&sdata=jtPuiAvgvO3gyrzCo6S > WooBtNrgyNUm8bPpyJ7%2FwHyk%3D&reserved=0 > > and please, do not try to detect version by reading wrong register. We have > devicetree compatible for this use case. > [Richard Zhu] Okay, devicetree compatible is a better method. Thanks.
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > --- > > drivers/mailbox/imx-mailbox.c | 45 > > +++++++++++++++++++++++++++++++++---------- > > 1 file changed, 35 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/mailbox/imx-mailbox.c > > b/drivers/mailbox/imx-mailbox.c index 25be8bb..eb55bbe 100644 > > --- a/drivers/mailbox/imx-mailbox.c > > +++ b/drivers/mailbox/imx-mailbox.c > > @@ -12,10 +12,14 @@ > > #include <linux/of_device.h> > > #include <linux/slab.h> > > > > +#define MU_VER_ID_V1 0x0100 > > + > > /* Transmit Register */ > > #define IMX_MU_xTRn(x) (0x00 + 4 * (x)) > > +#define IMX_MU_xTRn_V1(x) (0x20 + 4 * (x)) > > /* Receive Register */ > > #define IMX_MU_xRRn(x) (0x10 + 4 * (x)) > > +#define IMX_MU_xRRn_V1(x) (0x40 + 4 * (x)) > > /* Status Register */ > > #define IMX_MU_xSR 0x20 > > #define IMX_MU_xSR_GIPn(x) BIT(28 + (3 - (x))) > > @@ -25,6 +29,7 @@ > > > > /* Control Register */ > > #define IMX_MU_xCR 0x24 > > +#define IMX_MU_xSCR_V1_OFFSET 0x40 > > /* General Purpose Interrupt Enable */ > > #define IMX_MU_xCR_GIEn(x) BIT(28 + (3 - (x))) > > /* Receive Interrupt Enable */ > > @@ -63,6 +68,7 @@ struct imx_mu_priv { > > struct imx_mu_con_priv con_priv[IMX_MU_CHANS]; > > struct clk *clk; > > int irq; > > + int version; > > > > bool side_b; > > }; > > @@ -85,13 +91,16 @@ static u32 imx_mu_read(struct imx_mu_priv *priv, > > u32 offs) static u32 imx_mu_xcr_rmw(struct imx_mu_priv *priv, u32 > > set, u32 clr) { > > unsigned long flags; > > - u32 val; > > + u32 val, offset; > > + > > + offset = unlikely(priv->version == MU_VER_ID_V1) ? > > + IMX_MU_xSCR_V1_OFFSET : 0; > > > > spin_lock_irqsave(&priv->xcr_lock, flags); > > - val = imx_mu_read(priv, IMX_MU_xCR); > > + val = imx_mu_read(priv, IMX_MU_xCR + offset); > > val &= ~clr; > > val |= set; > > - imx_mu_write(priv, val, IMX_MU_xCR); > > + imx_mu_write(priv, val, IMX_MU_xCR + offset); > > spin_unlock_irqrestore(&priv->xcr_lock, flags); > > > > return val; > > @@ -109,10 +118,13 @@ static irqreturn_t imx_mu_isr(int irq, void *p) > > struct mbox_chan *chan = p; > > struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox); > > struct imx_mu_con_priv *cp = chan->con_priv; > > - u32 val, ctrl, dat; > > + u32 val, ctrl, dat, offset; > > + > > + offset = unlikely(priv->version == MU_VER_ID_V1) ? > > + IMX_MU_xSCR_V1_OFFSET : 0; > > > > - ctrl = imx_mu_read(priv, IMX_MU_xCR); > > - val = imx_mu_read(priv, IMX_MU_xSR); > > + ctrl = imx_mu_read(priv, IMX_MU_xCR + offset); > > + val = imx_mu_read(priv, IMX_MU_xSR + offset); > > > > switch (cp->type) { > > case IMX_MU_TYPE_TX: > > @@ -138,10 +150,14 @@ static irqreturn_t imx_mu_isr(int irq, void *p) > > imx_mu_xcr_rmw(priv, 0, IMX_MU_xCR_TIEn(cp->idx)); > > mbox_chan_txdone(chan, 0); > > } else if (val == IMX_MU_xSR_RFn(cp->idx)) { > > - dat = imx_mu_read(priv, IMX_MU_xRRn(cp->idx)); > > + if (unlikely(priv->version == MU_VER_ID_V1)) > > + dat = imx_mu_read(priv, > IMX_MU_xRRn_V1(cp->idx)); > > + else > > + dat = imx_mu_read(priv, IMX_MU_xRRn(cp->idx)); > > mbox_chan_received_data(chan, (void *)&dat); > > } else if (val == IMX_MU_xSR_GIPn(cp->idx)) { > > - imx_mu_write(priv, IMX_MU_xSR_GIPn(cp->idx), > IMX_MU_xSR); > > + imx_mu_write(priv, IMX_MU_xSR_GIPn(cp->idx), > > + IMX_MU_xSR + offset); > > mbox_chan_received_data(chan, NULL); > > } else { > > dev_warn_ratelimited(priv->dev, "Not handled > > interrupt\n"); @@ -159,7 +175,10 @@ static int imx_mu_send_data(struct > > mbox_chan *chan, void *data) > > > > switch (cp->type) { > > case IMX_MU_TYPE_TX: > > - imx_mu_write(priv, *arg, IMX_MU_xTRn(cp->idx)); > > + if (unlikely(priv->version == MU_VER_ID_V1)) > > + imx_mu_write(priv, *arg, > IMX_MU_xTRn_V1(cp->idx)); > > + else > > + imx_mu_write(priv, *arg, > IMX_MU_xTRn(cp->idx)); > > imx_mu_xcr_rmw(priv, IMX_MU_xCR_TIEn(cp->idx), 0); > > break; > > case IMX_MU_TYPE_TXDB: > > @@ -253,11 +272,17 @@ static struct mbox_chan * imx_mu_xlate(struct > > mbox_controller *mbox, > > > > static void imx_mu_init_generic(struct imx_mu_priv *priv) { > > + u32 offset; > > + > > if (priv->side_b) > > return; > > > > + priv->version = imx_mu_read(priv, 0) >> 16; > > + offset = unlikely(priv->version == MU_VER_ID_V1) ? > > + IMX_MU_xSCR_V1_OFFSET : 0; > > + > > /* Set default MU configuration */ > > - imx_mu_write(priv, 0, IMX_MU_xCR); > > + imx_mu_write(priv, 0, IMX_MU_xCR + offset); > > } > > > > static int imx_mu_probe(struct platform_device *pdev) > > -- > > 2.7.4 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=02%7C > 0 > > > 1%7Chongxing.zhu%40nxp.com%7C275ed465ca75424f583008d711aa0e58%7C > 686ea1 > > > d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636997294396884235&sdat > a=hNCv > > QJXtXRWAJT84NNrXyVfnQ1V2uxWxbVqN%2Bvy7hOg%3D&reserved=0 > > > > -- > Pengutronix e.K. | > | > Industrial Linux Solutions | > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pen > gutronix.de%2F&data=02%7C01%7Chongxing.zhu%40nxp.com%7C275ed4 > 65ca75424f583008d711aa0e58%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0 > %7C0%7C636997294396894234&sdata=swwSIxuQWGx7%2F6CwtfXHm0N > ZwJNkWO4rNo%2BlC7bHjCY%3D&reserved=0 | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: > +49-5121-206917-5555 |
| |