Message ID | 1564129776-19574-1-git-send-email-hongxing.zhu@nxp.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [RFC] mailbox: imx: Add support for i.MX v1 messaging unit | expand |
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://elixir.bootlin.com/linux/v5.3-rc1/source/drivers/remoteproc/imx_rproc.c#L336 dcfg = of_device_get_match_data(dev); or https://elixir.bootlin.com/linux/v5.3-rc1/source/drivers/net/ethernet/atheros/ag71xx.c#L1651 and please, do not try to detect version by reading wrong register. We have devicetree compatible for this use case. > 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 > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
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 |
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)
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. Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> --- drivers/mailbox/imx-mailbox.c | 45 +++++++++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 10 deletions(-)