2019-07-29 02:37:15

by Richard Zhu

[permalink] [raw]
Subject: [PATCH] mailbox: imx: add support for imx v1 mu

There is a version1.0 MU on i.MX7ULP platform.
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.

Signed-off-by: Richard Zhu <[email protected]>
---
drivers/mailbox/imx-mailbox.c | 67 ++++++++++++++++++++++++++++++++-----------
1 file changed, 50 insertions(+), 17 deletions(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 25be8bb..8423a38 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,39 @@ 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[0] = 0x0,
+ .xTR[1] = 0x4,
+ .xTR[2] = 0x8,
+ .xTR[3] = 0xC,
+ .xRR[0] = 0x10,
+ .xRR[1] = 0x14,
+ .xRR[2] = 0x18,
+ .xRR[3] = 0x1C,
+ .xSR = 0x20,
+ .xCR = 0x24,
+};
+
+static const struct imx_mu_dcfg imx_mu_cfg_imx7ulp = {
+ .xTR[0] = 0x20,
+ .xTR[1] = 0x24,
+ .xTR[2] = 0x28,
+ .xTR[3] = 0x2C,
+ .xRR[0] = 0x40,
+ .xRR[1] = 0x44,
+ .xRR[2] = 0x48,
+ .xRR[3] = 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 +114,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 +137,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 +164,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 +185,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 +283,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 +291,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 +309,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 +367,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);
--
2.7.4


2019-07-29 02:47:38

by Dong Aisheng

[permalink] [raw]
Subject: Re: [PATCH] mailbox: imx: add support for imx v1 mu

On Mon, Jul 29, 2019 at 10:36 AM Richard Zhu <[email protected]> wrote:
>
> There is a version1.0 MU on i.MX7ULP platform.
> 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.
>

If only the register base offset is different, there's probably a more
simple way.
Please refer to:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/tty/serial/fsl_lpuart.c?id=24b1e5f0e83c2aced8096473d20c4cf6c1355f30

Regards
Aisheng

> Signed-off-by: Richard Zhu <[email protected]>
> ---
> drivers/mailbox/imx-mailbox.c | 67 ++++++++++++++++++++++++++++++++-----------
> 1 file changed, 50 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> index 25be8bb..8423a38 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,39 @@ 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[0] = 0x0,
> + .xTR[1] = 0x4,
> + .xTR[2] = 0x8,
> + .xTR[3] = 0xC,
> + .xRR[0] = 0x10,
> + .xRR[1] = 0x14,
> + .xRR[2] = 0x18,
> + .xRR[3] = 0x1C,
> + .xSR = 0x20,
> + .xCR = 0x24,
> +};
> +
> +static const struct imx_mu_dcfg imx_mu_cfg_imx7ulp = {
> + .xTR[0] = 0x20,
> + .xTR[1] = 0x24,
> + .xTR[2] = 0x28,
> + .xTR[3] = 0x2C,
> + .xRR[0] = 0x40,
> + .xRR[1] = 0x44,
> + .xRR[2] = 0x48,
> + .xRR[3] = 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 +114,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 +137,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 +164,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 +185,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 +283,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 +291,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 +309,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 +367,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);
> --
> 2.7.4
>

2019-07-29 03:18:23

by Richard Zhu

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH] mailbox: imx: add support for imx v1 mu

Hi Aisheng:
Thanks for your comments.

> -----Original Message-----
> From: Dong Aisheng <[email protected]>
> Sent: 2019年7月29日 10:35
> To: Richard Zhu <[email protected]>
> Cc: [email protected]; Oleksij Rempel <[email protected]>;
> Aisheng Dong <[email protected]>; open list
> <[email protected]>; moderated list:ARM/FREESCALE IMX / MXC
> ARM ARCHITECTURE <[email protected]>
> Subject: [EXT] Re: [PATCH] mailbox: imx: add support for imx v1 mu
>
> On Mon, Jul 29, 2019 at 10:36 AM Richard Zhu <[email protected]>
> wrote:
> >
> > There is a version1.0 MU on i.MX7ULP platform.
> > 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.
> >
>
> If only the register base offset is different, there's probably a more simple way.
> Please refer to:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel
> .org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fnext%2Flinux-next.git%2Fcom
> mit%2Fdrivers%2Ftty%2Fserial%2Ffsl_lpuart.c%3Fid%3D24b1e5f0e83c2aced80
> 96473d20c4cf6c1355f30&amp;data=02%7C01%7Chongxing.zhu%40nxp.com%
> 7C48b26de751464410249308d713ceae22%7C686ea1d3bc2b4c6fa92cd99c5c30
> 1635%7C0%7C1%7C636999650716588024&amp;sdata=7OEZB8HMbQqfe6KgD
> HtE7AKAtgPGJHXGeUiAFjAP28k%3D&amp;reserved=0
>
[Richard Zhu] TRx, RRx and the CR/SR have the different offset addresses.
That means three different offset addresses should be manipulated if the solution listed above is used.
It seems that it's a little complex, and maybe introduce bugs when different offset address is manipulated.
According, the current method suggested by Oleksij is much clear, and easy to extend for future extension.

Hi Olekiji:
How do you think about?

Best Regards
Richard Zhu

> Regards
> Aisheng
>
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
> > drivers/mailbox/imx-mailbox.c | 67
> > ++++++++++++++++++++++++++++++++-----------
> > 1 file changed, 50 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/mailbox/imx-mailbox.c
> > b/drivers/mailbox/imx-mailbox.c index 25be8bb..8423a38 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,39 @@ 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[0] = 0x0,
> > + .xTR[1] = 0x4,
> > + .xTR[2] = 0x8,
> > + .xTR[3] = 0xC,
> > + .xRR[0] = 0x10,
> > + .xRR[1] = 0x14,
> > + .xRR[2] = 0x18,
> > + .xRR[3] = 0x1C,
> > + .xSR = 0x20,
> > + .xCR = 0x24,
> > +};
> > +
> > +static const struct imx_mu_dcfg imx_mu_cfg_imx7ulp = {
> > + .xTR[0] = 0x20,
> > + .xTR[1] = 0x24,
> > + .xTR[2] = 0x28,
> > + .xTR[3] = 0x2C,
> > + .xRR[0] = 0x40,
> > + .xRR[1] = 0x44,
> > + .xRR[2] = 0x48,
> > + .xRR[3] = 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
> > +114,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 +137,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 +164,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 +185,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 +283,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
> > +291,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 +309,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 +367,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);
> > --
> > 2.7.4
> >

2019-07-29 03:54:29

by Dong Aisheng

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH] mailbox: imx: add support for imx v1 mu

On Mon, Jul 29, 2019 at 11:03 AM Richard Zhu <[email protected]> wrote:
>
> Hi Aisheng:
> Thanks for your comments.
>
> > -----Original Message-----
> > From: Dong Aisheng <[email protected]>
> > Sent: 2019年7月29日 10:35
> > To: Richard Zhu <[email protected]>
> > Cc: [email protected]; Oleksij Rempel <[email protected]>;
> > Aisheng Dong <[email protected]>; open list
> > <[email protected]>; moderated list:ARM/FREESCALE IMX / MXC
> > ARM ARCHITECTURE <[email protected]>
> > Subject: [EXT] Re: [PATCH] mailbox: imx: add support for imx v1 mu
> >
> > On Mon, Jul 29, 2019 at 10:36 AM Richard Zhu <[email protected]>
> > wrote:
> > >
> > > There is a version1.0 MU on i.MX7ULP platform.
> > > 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.
> > >
> >
> > If only the register base offset is different, there's probably a more simple way.
> > Please refer to:
> >
> [Richard Zhu] TRx, RRx and the CR/SR have the different offset addresses.
> That means three different offset addresses should be manipulated if the solution listed above is used.
> It seems that it's a little complex, and maybe introduce bugs when different offset address is manipulated.
> According, the current method suggested by Oleksij is much clear, and easy to extend for future extension.
>

I missed that.
Maybe the patch title should be V2 and add Suggested-by: tag to
reminder reviewer
it's a new version?

If there're multiple offset differences. I'm fine with this way.
Feel free to add my tag.
Reviewed-by: Dong Aisheng <[email protected]>

Regards
Aisheng

> Hi Olekiji:
> How do you think about?
>
> Best Regards
> Richard Zhu
>
> > Regards
> > Aisheng
> >
> > > Signed-off-by: Richard Zhu <[email protected]>
> > > ---
> > > drivers/mailbox/imx-mailbox.c | 67
> > > ++++++++++++++++++++++++++++++++-----------
> > > 1 file changed, 50 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/mailbox/imx-mailbox.c
> > > b/drivers/mailbox/imx-mailbox.c index 25be8bb..8423a38 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,39 @@ 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[0] = 0x0,
> > > + .xTR[1] = 0x4,
> > > + .xTR[2] = 0x8,
> > > + .xTR[3] = 0xC,
> > > + .xRR[0] = 0x10,
> > > + .xRR[1] = 0x14,
> > > + .xRR[2] = 0x18,
> > > + .xRR[3] = 0x1C,
> > > + .xSR = 0x20,
> > > + .xCR = 0x24,
> > > +};
> > > +
> > > +static const struct imx_mu_dcfg imx_mu_cfg_imx7ulp = {
> > > + .xTR[0] = 0x20,
> > > + .xTR[1] = 0x24,
> > > + .xTR[2] = 0x28,
> > > + .xTR[3] = 0x2C,
> > > + .xRR[0] = 0x40,
> > > + .xRR[1] = 0x44,
> > > + .xRR[2] = 0x48,
> > > + .xRR[3] = 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
> > > +114,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 +137,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 +164,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 +185,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 +283,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
> > > +291,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 +309,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 +367,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);
> > > --
> > > 2.7.4
> > >

2019-07-29 05:44:36

by Richard Zhu

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH] mailbox: imx: add support for imx v1 mu



> -----Original Message-----
> From: Dong Aisheng <[email protected]>
> Sent: 2019年7月29日 11:11
> To: Richard Zhu <[email protected]>
> Cc: [email protected]; Oleksij Rempel <[email protected]>;
> Aisheng Dong <[email protected]>; open list
> <[email protected]>; moderated list:ARM/FREESCALE IMX / MXC
> ARM ARCHITECTURE <[email protected]>
> Subject: Re: [EXT] Re: [PATCH] mailbox: imx: add support for imx v1 mu
>
> Caution: EXT Email
>
> On Mon, Jul 29, 2019 at 11:03 AM Richard Zhu <[email protected]>
> wrote:
> >
> > Hi Aisheng:
> > Thanks for your comments.
> >
> > > -----Original Message-----
> > > From: Dong Aisheng <[email protected]>
> > > Sent: 2019年7月29日 10:35
> > > To: Richard Zhu <[email protected]>
> > > Cc: [email protected]; Oleksij Rempel
> > > <[email protected]>; Aisheng Dong <[email protected]>;
> open
> > > list <[email protected]>; moderated list:ARM/FREESCALE
> > > IMX / MXC ARM ARCHITECTURE <[email protected]>
> > > Subject: [EXT] Re: [PATCH] mailbox: imx: add support for imx v1 mu
> > >
> > > On Mon, Jul 29, 2019 at 10:36 AM Richard Zhu <[email protected]>
> > > wrote:
> > > >
> > > > There is a version1.0 MU on i.MX7ULP platform.
> > > > 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.
> > > >
> > >
> > > If only the register base offset is different, there's probably a more simple
> way.
> > > Please refer to:
> > >
> > [Richard Zhu] TRx, RRx and the CR/SR have the different offset addresses.
> > That means three different offset addresses should be manipulated if the
> solution listed above is used.
> > It seems that it's a little complex, and maybe introduce bugs when different
> offset address is manipulated.
> > According, the current method suggested by Oleksij is much clear, and easy to
> extend for future extension.
> >
>
> I missed that.
> Maybe the patch title should be V2 and add Suggested-by: tag to reminder
> reviewer it's a new version?
>
> If there're multiple offset differences. I'm fine with this way.
> Feel free to add my tag.
> Reviewed-by: Dong Aisheng <[email protected]>

[Richard Zhu] Thanks. Okay, understand.
Would resend it after change the commit.

>
> Regards
> Aisheng
>
> > Hi Olekiji:
> > How do you think about?
> >
> > Best Regards
> > Richard Zhu
> >
> > > Regards
> > > Aisheng
> > >
> > > > Signed-off-by: Richard Zhu <[email protected]>
> > > > ---
> > > > drivers/mailbox/imx-mailbox.c | 67
> > > > ++++++++++++++++++++++++++++++++-----------
> > > > 1 file changed, 50 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/drivers/mailbox/imx-mailbox.c
> > > > b/drivers/mailbox/imx-mailbox.c index 25be8bb..8423a38 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,39 @@ 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[0] = 0x0,
> > > > + .xTR[1] = 0x4,
> > > > + .xTR[2] = 0x8,
> > > > + .xTR[3] = 0xC,
> > > > + .xRR[0] = 0x10,
> > > > + .xRR[1] = 0x14,
> > > > + .xRR[2] = 0x18,
> > > > + .xRR[3] = 0x1C,
> > > > + .xSR = 0x20,
> > > > + .xCR = 0x24,
> > > > +};
> > > > +
> > > > +static const struct imx_mu_dcfg imx_mu_cfg_imx7ulp = {
> > > > + .xTR[0] = 0x20,
> > > > + .xTR[1] = 0x24,
> > > > + .xTR[2] = 0x28,
> > > > + .xTR[3] = 0x2C,
> > > > + .xRR[0] = 0x40,
> > > > + .xRR[1] = 0x44,
> > > > + .xRR[2] = 0x48,
> > > > + .xRR[3] = 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
> > > > +114,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 +137,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 +164,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 +185,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 +283,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
> > > > +291,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 +309,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
> > > > +367,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);
> > > > --
> > > > 2.7.4
> > > >

2019-07-29 08:41:12

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH] mailbox: imx: add support for imx v1 mu

please use format [PATCH v2] description. It is automatically done with
git format-patch -v2

On Mon, Jul 29, 2019 at 10:14:00AM +0800, Richard Zhu wrote:
> There is a version1.0 MU on i.MX7ULP platform.
> 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.
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---

change log can be done here or in a message automatically generate by
format patch:
git format-patch --cover-letter -v2

> drivers/mailbox/imx-mailbox.c | 67 ++++++++++++++++++++++++++++++++-----------
> 1 file changed, 50 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> index 25be8bb..8423a38 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,39 @@ 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[0] = 0x0,
> + .xTR[1] = 0x4,
> + .xTR[2] = 0x8,
> + .xTR[3] = 0xC,

I would prefer to init arrays this way:
.xTR[] = {0x0, 0x4, 0x8, 0xc},

For every thing else
Reviewed-by: Oleksij Rempel <[email protected]>

> + .xRR[0] = 0x10,
> + .xRR[1] = 0x14,
> + .xRR[2] = 0x18,
> + .xRR[3] = 0x1C,
> + .xSR = 0x20,
> + .xCR = 0x24,
> +};
> +
> +static const struct imx_mu_dcfg imx_mu_cfg_imx7ulp = {
> + .xTR[0] = 0x20,
> + .xTR[1] = 0x24,
> + .xTR[2] = 0x28,
> + .xTR[3] = 0x2C,
> + .xRR[0] = 0x40,
> + .xRR[1] = 0x44,
> + .xRR[2] = 0x48,
> + .xRR[3] = 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 +114,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 +137,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 +164,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 +185,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 +283,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 +291,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 +309,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 +367,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);
> --
> 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 |

2019-07-29 13:06:31

by Richard Zhu

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH] mailbox: imx: add support for imx v1 mu

> -----Original Message-----
> From: Oleksij Rempel <[email protected]>
> Sent: 2019??7??29?? 16:38
> To: Richard Zhu <[email protected]>
> Cc: [email protected]; Aisheng Dong <[email protected]>;
> [email protected]; [email protected]
> Subject: [EXT] Re: [PATCH] mailbox: imx: add support for imx v1 mu
>
>
> please use format [PATCH v2] description. It is automatically done with git
> format-patch -v2
>
> On Mon, Jul 29, 2019 at 10:14:00AM +0800, Richard Zhu wrote:
> > There is a version1.0 MU on i.MX7ULP platform.
> > 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.
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
>
> change log can be done here or in a message automatically generate by format
> patch:
> git format-patch --cover-letter -v2
>
[Richard Zhu] Great. Thanks for the format guidance.
Would use it to send out the v3 patch.
Thanks.

> > drivers/mailbox/imx-mailbox.c | 67
> > ++++++++++++++++++++++++++++++++-----------
> > 1 file changed, 50 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/mailbox/imx-mailbox.c
> > b/drivers/mailbox/imx-mailbox.c index 25be8bb..8423a38 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,39 @@ 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[0] = 0x0,
> > + .xTR[1] = 0x4,
> > + .xTR[2] = 0x8,
> > + .xTR[3] = 0xC,
>
> I would prefer to init arrays this way:
> .xTR[] = {0x0, 0x4, 0x8, 0xc},
>
> For every thing else
> Reviewed-by: Oleksij Rempel <[email protected]>
>
[Richard Zhu] Okay, thanks.

> > + .xRR[0] = 0x10,
> > + .xRR[1] = 0x14,
> > + .xRR[2] = 0x18,
> > + .xRR[3] = 0x1C,
> > + .xSR = 0x20,
> > + .xCR = 0x24,
> > +};
> > +
> > +static const struct imx_mu_dcfg imx_mu_cfg_imx7ulp = {
> > + .xTR[0] = 0x20,
> > + .xTR[1] = 0x24,
> > + .xTR[2] = 0x28,
> > + .xTR[3] = 0x2C,
> > + .xRR[0] = 0x40,
> > + .xRR[1] = 0x44,
> > + .xRR[2] = 0x48,
> > + .xRR[3] = 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
> > +114,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 +137,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 +164,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 +185,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 +283,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
> > +291,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 +309,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 +367,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);
> > --
> > 2.7.4
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&amp;data=02%7C
> 0
> >
> 1%7Chongxing.zhu%40nxp.com%7C21018279263a437e93c408d714001caf%7C
> 686ea1
> >
> d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636999863030173002&amp;sdat
> a=rAkj
> > 55SuJCjGN8ZC0ylLhGDHLeZlEQX9HvPxbjYqND4%3D&amp;reserved=0
> >
>
> --
> Pengutronix e.K. |
> |
> Industrial Linux Solutions |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pen
> gutronix.de%2F&amp;data=02%7C01%7Chongxing.zhu%40nxp.com%7C210182
> 79263a437e93c408d714001caf%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636999863030182999&amp;sdata=PKBRJCM1Gd8Fv1ogzuLD7MrT%2
> FROpbk4Ezzk3KSxKnF0%3D&amp;reserved=0 |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:
> +49-5121-206917-5555 |