lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 29 Jul 2019 03:03:53 +0000
From:   Richard Zhu <hongxing.zhu@....com>
To:     Dong Aisheng <dongas86@...il.com>
CC:     "jassisinghbrar@...il.com" <jassisinghbrar@...il.com>,
        Oleksij Rempel <o.rempel@...gutronix.de>,
        Aisheng Dong <aisheng.dong@....com>,
        open list <linux-kernel@...r.kernel.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [EXT] Re: [PATCH] mailbox: imx: add support for imx v1 mu

Hi Aisheng:
Thanks for your comments.

> -----Original Message-----
> From: Dong Aisheng <dongas86@...il.com>
> Sent: 2019年7月29日 10:35
> To: Richard Zhu <hongxing.zhu@....com>
> Cc: jassisinghbrar@...il.com; Oleksij Rempel <o.rempel@...gutronix.de>;
> Aisheng Dong <aisheng.dong@....com>; open list
> <linux-kernel@...r.kernel.org>; moderated list:ARM/FREESCALE IMX / MXC
> ARM ARCHITECTURE <linux-arm-kernel@...ts.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@....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@....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
> >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ