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 <[email protected]> > --- > 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 > [email protected] > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
