| Message ID | 1564563107-23736-1-git-send-email-hongxing.zhu@nxp.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | c6c6bc6ea9fce31baaca053befc31215cfcb3dd9 |
| Headers | show |
| Series | [v3] mailbox: imx: add support for imx v1 mu | expand |
Hi Richard, Thanks for the patch. Please always add linux-imx@nxp.com mailing list for imx related patches. I missed it. Few comments inline. Please also update in a separate patch attached to this series the devictree bindings doc Documentation/devicetree/bindings/mailbox/fsl,mu.txt by adding description for mx7ulp-mu <snip> > There is a version1.0 MU on i.MX7ULP platform. space between version and 1.0 > One new version ID register is added, and it's 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. Do you have a little bit of history about MU versioning? So there was: * version 0.5 on i.MX6-es * version 1.0 on i.MX7ULP Next, is this 1.0 compatbile with i.MX8 right? Also, can you please rebase your patch on my 2 bugfixes attached? thanks, Daniel.
> -----Original Message----- > From: Daniel Baluta <daniel.baluta@gmail.com> > Sent: 2019年8月1日 22:47 > To: Richard Zhu <hongxing.zhu@nxp.com> > Cc: jassisinghbrar@gmail.com; Oleksij Rempel <o.rempel@pengutronix.de>; > Aisheng Dong <aisheng.dong@nxp.com>; Linux Kernel Mailing List > <linux-kernel@vger.kernel.org>; linux-arm-kernel > <linux-arm-kernel@lists.infradead.org>; dl-linux-imx <linux-imx@nxp.com> > Subject: [EXT] Re: [PATCH v3] mailbox: imx: add support for imx v1 mu > > Hi Richard, > > Thanks for the patch. Please always add linux-imx@nxp.com mailing list for imx > related patches. I missed it. > [Richard Zhu] Okay, roger that. > Few comments inline. > > Please also update in a separate patch attached to this series the devictree > bindings doc Documentation/devicetree/bindings/mailbox/fsl,mu.txt > by adding description for mx7ulp-mu > > <snip> [Richard Zhu] Okay, the binding doc would be added later. > > > There is a version1.0 MU on i.MX7ULP platform. > > space between version and 1.0 > [Richard Zhu] Okay. > > One new version ID register is added, and it's 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. > > Do you have a little bit of history about MU versioning? So there was: > > * version 0.5 on i.MX6-es > * version 1.0 on i.MX7ULP > > Next, is this 1.0 compatbile with i.MX8 right? > [Richard Zhu] Only i.MX7ULP has the version 1.0 MU. i.MX8 has the same version MU that i.MX6-es have. IMHO, I don't know why design team do it this way. > Also, can you please rebase your patch on my 2 bugfixes attached? [Richard Zhu] Okay, no problem. I would send the v4 version out later after rebase your 2 bugfixes patches. > > thanks, > Daniel.
One more thing. See below: On Wed, Jul 31, 2019 at 12:14 PM Richard Zhu <hongxing.zhu@nxp.com> wrote: <snip> > -/* Control Register */ > -#define IMX_MU_xCR 0x24 > /* General Purpose Interrupt Enable */ > #define IMX_MU_xCR_GIEn(x) BIT(28 + (3 - (x))) > /* Receive Interrupt Enable */ > @@ -44,6 +36,13 @@ enum imx_mu_chan_type { > IMX_MU_TYPE_RXDB, /* Rx doorbell */ > }; > > +struct imx_mu_dcfg { Can you rename this into imx_mu_regs ? > + u32 xTR[4]; /* Transmit Registers */ > + u32 xRR[4]; /* Receive Registers */ > + u32 xSR; /* Status Register */ > + u32 xCR; /* Control Register */ > +};
On Fri, Aug 02, 2019 at 06:54:27AM +0300, Daniel Baluta wrote: > One more thing. See below: > > On Wed, Jul 31, 2019 at 12:14 PM Richard Zhu <hongxing.zhu@nxp.com> wrote: > > <snip> > > > -/* Control Register */ > > -#define IMX_MU_xCR 0x24 > > /* General Purpose Interrupt Enable */ > > #define IMX_MU_xCR_GIEn(x) BIT(28 + (3 - (x))) > > /* Receive Interrupt Enable */ > > @@ -44,6 +36,13 @@ enum imx_mu_chan_type { > > IMX_MU_TYPE_RXDB, /* Rx doorbell */ > > }; > > > > +struct imx_mu_dcfg { > > Can you rename this into imx_mu_regs ? I decided not blame this part. Otherwise adding other type of quirks will lead to more refactoring work. > > + u32 xTR[4]; /* Transmit Registers */ > > + u32 xRR[4]; /* Receive Registers */ > > + u32 xSR; /* Status Register */ > > + u32 xCR; /* Control Register */ > > +}; >
diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c index 25be8bb..c81be1c 100644 --- a/drivers/mailbox/imx-mailbox.c +++ b/drivers/mailbox/imx-mailbox.c @@ -12,19 +12,11 @@ #include <linux/of_device.h> #include <linux/slab.h> -/* Transmit Register */ -#define IMX_MU_xTRn(x) (0x00 + 4 * (x)) -/* Receive Register */ -#define IMX_MU_xRRn(x) (0x10 + 4 * (x)) -/* Status Register */ -#define IMX_MU_xSR 0x20 #define IMX_MU_xSR_GIPn(x) BIT(28 + (3 - (x))) #define IMX_MU_xSR_RFn(x) BIT(24 + (3 - (x))) #define IMX_MU_xSR_TEn(x) BIT(20 + (3 - (x))) #define IMX_MU_xSR_BRDIP BIT(9) -/* Control Register */ -#define IMX_MU_xCR 0x24 /* General Purpose Interrupt Enable */ #define IMX_MU_xCR_GIEn(x) BIT(28 + (3 - (x))) /* Receive Interrupt Enable */ @@ -44,6 +36,13 @@ enum imx_mu_chan_type { IMX_MU_TYPE_RXDB, /* Rx doorbell */ }; +struct imx_mu_dcfg { + u32 xTR[4]; /* Transmit Registers */ + u32 xRR[4]; /* Receive Registers */ + u32 xSR; /* Status Register */ + u32 xCR; /* Control Register */ +}; + struct imx_mu_con_priv { unsigned int idx; char irq_desc[IMX_MU_CHAN_NAME_SIZE]; @@ -61,12 +60,27 @@ struct imx_mu_priv { struct mbox_chan mbox_chans[IMX_MU_CHANS]; struct imx_mu_con_priv con_priv[IMX_MU_CHANS]; + const struct imx_mu_dcfg *dcfg; struct clk *clk; int irq; bool side_b; }; +static const struct imx_mu_dcfg imx_mu_cfg_imx6sx = { + .xTR = {0x0, 0x4, 0x8, 0xc}, + .xRR = {0x10, 0x14, 0x18, 0x1c}, + .xSR = 0x20, + .xCR = 0x24, +}; + +static const struct imx_mu_dcfg imx_mu_cfg_imx7ulp = { + .xTR = {0x20, 0x24, 0x28, 0x2c}, + .xRR = {0x40, 0x44, 0x48, 0x4c}, + .xSR = 0x60, + .xCR = 0x64, +}; + static struct imx_mu_priv *to_imx_mu_priv(struct mbox_controller *mbox) { return container_of(mbox, struct imx_mu_priv, mbox); @@ -88,10 +102,10 @@ static u32 imx_mu_xcr_rmw(struct imx_mu_priv *priv, u32 set, u32 clr) u32 val; spin_lock_irqsave(&priv->xcr_lock, flags); - val = imx_mu_read(priv, IMX_MU_xCR); + val = imx_mu_read(priv, priv->dcfg->xCR); val &= ~clr; val |= set; - imx_mu_write(priv, val, IMX_MU_xCR); + imx_mu_write(priv, val, priv->dcfg->xCR); spin_unlock_irqrestore(&priv->xcr_lock, flags); return val; @@ -111,8 +125,8 @@ static irqreturn_t imx_mu_isr(int irq, void *p) struct imx_mu_con_priv *cp = chan->con_priv; u32 val, ctrl, dat; - ctrl = imx_mu_read(priv, IMX_MU_xCR); - val = imx_mu_read(priv, IMX_MU_xSR); + ctrl = imx_mu_read(priv, priv->dcfg->xCR); + val = imx_mu_read(priv, priv->dcfg->xSR); switch (cp->type) { case IMX_MU_TYPE_TX: @@ -138,10 +152,10 @@ 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)); + dat = imx_mu_read(priv, priv->dcfg->xRR[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), priv->dcfg->xSR); mbox_chan_received_data(chan, NULL); } else { dev_warn_ratelimited(priv->dev, "Not handled interrupt\n"); @@ -159,7 +173,7 @@ 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)); + imx_mu_write(priv, *arg, priv->dcfg->xTR[cp->idx]); imx_mu_xcr_rmw(priv, IMX_MU_xCR_TIEn(cp->idx), 0); break; case IMX_MU_TYPE_TXDB: @@ -257,7 +271,7 @@ static void imx_mu_init_generic(struct imx_mu_priv *priv) return; /* Set default MU configuration */ - imx_mu_write(priv, 0, IMX_MU_xCR); + imx_mu_write(priv, 0, priv->dcfg->xCR); } static int imx_mu_probe(struct platform_device *pdev) @@ -265,6 +279,7 @@ static int imx_mu_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct device_node *np = dev->of_node; struct imx_mu_priv *priv; + const struct imx_mu_dcfg *dcfg; unsigned int i; int ret; @@ -282,6 +297,11 @@ static int imx_mu_probe(struct platform_device *pdev) if (priv->irq < 0) return priv->irq; + dcfg = of_device_get_match_data(dev); + if (!dcfg) + return -EINVAL; + priv->dcfg = dcfg; + priv->clk = devm_clk_get(dev, NULL); if (IS_ERR(priv->clk)) { if (PTR_ERR(priv->clk) != -ENOENT) @@ -335,7 +355,8 @@ static int imx_mu_remove(struct platform_device *pdev) } static const struct of_device_id imx_mu_dt_ids[] = { - { .compatible = "fsl,imx6sx-mu" }, + { .compatible = "fsl,imx7ulp-mu", .data = &imx_mu_cfg_imx7ulp }, + { .compatible = "fsl,imx6sx-mu", .data = &imx_mu_cfg_imx6sx }, { }, }; MODULE_DEVICE_TABLE(of, imx_mu_dt_ids);