lkml.org 
[lkml]   [2019]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [EXT] Re: [PATCH] mailbox: imx: add support for imx v1 mu
Date
Hi Aisheng:
Thanks for your comments.

> -----Original Message-----
> From: Dong Aisheng <dongas86@gmail.com>
> Sent: 2019年7月29日 10:35
> To: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: jassisinghbrar@gmail.com; Oleksij Rempel <o.rempel@pengutronix.de>;
> Aisheng Dong <aisheng.dong@nxp.com>; open list
> <linux-kernel@vger.kernel.org>; moderated list:ARM/FREESCALE IMX / MXC
> ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>
> Subject: [EXT] Re: [PATCH] mailbox: imx: add support for imx v1 mu
>
> On Mon, Jul 29, 2019 at 10:36 AM Richard Zhu <hongxing.zhu@nxp.com>
> 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 <hongxing.zhu@nxp.com>
> > ---
> > 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
> >
\
 
 \ /
  Last update: 2019-07-29 05:04    [W:0.358 / U:0.508 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site