diff mbox series

[2/2] mailbox: imx: support channel type tx doorbell v2

Message ID 20230917-imx95-mbox-v1-2-440245287356@nxp.com
State New
Headers
Series mailbox: imx: support new tx doorbell |

Commit Message

Peng Fan (OSS) Sept. 17, 2023, 12:43 p.m. UTC 
  From: Peng Fan <peng.fan@nxp.com>

The Message Unit(MU) General Purpose Control registers are used for
TX doorbell, but there is no hardware ACK support.

The current TX doorbell channel is using tasklet to emulate hardware
ACK support to kick the TX tick from controller driver side.

The new added TX doorbell channel V2 not using tasklet to emulate the
hardware ACK support. The behavior for the channel is just writing the
GCR register, and no else. This will be used for SCMI mailbox.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mailbox/imx-mailbox.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

Comments

Daniel Baluta Sept. 18, 2023, 10:36 a.m. UTC | #1 
On Sun, Sep 17, 2023 at 5:45 PM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
>
> From: Peng Fan <peng.fan@nxp.com>
>
> The Message Unit(MU) General Purpose Control registers are used for
> TX doorbell, but there is no hardware ACK support.
>
> The current TX doorbell channel is using tasklet to emulate hardware
> ACK support to kick the TX tick from controller driver side.
>
> The new added TX doorbell channel V2 not using tasklet to emulate the
> hardware ACK support. The behavior for the channel is just writing the
> GCR register, and no else. This will be used for SCMI mailbox.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/mailbox/imx-mailbox.c | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> index 3ef4dd8adf5d..0af739ab571c 100644
> --- a/drivers/mailbox/imx-mailbox.c
> +++ b/drivers/mailbox/imx-mailbox.c
> @@ -20,7 +20,9 @@
>  #include <linux/suspend.h>
>  #include <linux/slab.h>
>
> -#define IMX_MU_CHANS           17
> +#include "mailbox.h"
> +
> +#define IMX_MU_CHANS           24
>  /* TX0/RX0/RXDB[0-3] */
>  #define IMX_MU_SCU_CHANS       6
>  /* TX0/RX0 */
> @@ -39,6 +41,7 @@ enum imx_mu_chan_type {
>         IMX_MU_TYPE_TXDB        = 2, /* Tx doorbell */
>         IMX_MU_TYPE_RXDB        = 3, /* Rx doorbell */
>         IMX_MU_TYPE_RST         = 4, /* Reset */
> +       IMX_MU_TYPE_TXDB_V2     = 5, /* Tx doorbell with S/W ACK */
>  };
>
>  enum imx_mu_xcr {
> @@ -226,6 +229,9 @@ static int imx_mu_generic_tx(struct imx_mu_priv *priv,
>                 imx_mu_xcr_rmw(priv, IMX_MU_GCR, IMX_MU_xCR_GIRn(priv->dcfg->type, cp->idx), 0);
>                 tasklet_schedule(&cp->txdb_tasklet);
>                 break;
> +       case IMX_MU_TYPE_TXDB_V2:
> +               imx_mu_xcr_rmw(priv, IMX_MU_GCR, IMX_MU_xCR_GIRn(priv->dcfg->type, cp->idx), 0);
> +               break;
>         default:
>                 dev_warn_ratelimited(priv->dev, "Send data on wrong channel type: %d\n", cp->type);
>                 return -EINVAL;
> @@ -554,6 +560,9 @@ static int imx_mu_startup(struct mbox_chan *chan)
>         int ret;
>
>         pm_runtime_get_sync(priv->dev);
> +       if (cp->type == IMX_MU_TYPE_TXDB_V2)
> +               return 0;
> +
>         if (cp->type == IMX_MU_TYPE_TXDB) {
>                 /* Tx doorbell don't have ACK support */
>                 tasklet_init(&cp->txdb_tasklet, imx_mu_txdb_tasklet,
> @@ -595,6 +604,11 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
>         int ret;
>         u32 sr;
>
> +       if (cp->type == IMX_MU_TYPE_TXDB_V2) {
> +               pm_runtime_put_sync(priv->dev);
> +               return;
> +       }
> +
>         if (cp->type == IMX_MU_TYPE_TXDB) {
>                 tasklet_kill(&cp->txdb_tasklet);
>                 pm_runtime_put_sync(priv->dev);
> @@ -671,6 +685,7 @@ static struct mbox_chan *imx_mu_specific_xlate(struct mbox_controller *mbox,
>  static struct mbox_chan * imx_mu_xlate(struct mbox_controller *mbox,
>                                        const struct of_phandle_args *sp)
>  {
> +       struct mbox_chan *p_chan;
>         u32 type, idx, chan;
>
>         if (sp->args_count != 2) {
> @@ -680,14 +695,25 @@ static struct mbox_chan * imx_mu_xlate(struct mbox_controller *mbox,
>
>         type = sp->args[0]; /* channel type */
>         idx = sp->args[1]; /* index */
> -       chan = type * 4 + idx;
>
> +       /* RST only supports 1 channel */
> +       if ((type == IMX_MU_TYPE_RST) && idx) {
> +               dev_err(mbox->dev, "Invalid RST channel %d\n", idx);
> +               return ERR_PTR(-EINVAL);
> +       }

I don't understand how is this related to introduction of a new
doorbell channel. Can you please
add it in a separate patch with proper explanation?


> +
> +       chan = type * 4 + idx;
>         if (chan >= mbox->num_chans) {
>                 dev_err(mbox->dev, "Not supported channel number: %d. (type: %d, idx: %d)\n", chan, type, idx);
>                 return ERR_PTR(-EINVAL);
>         }
>
> -       return &mbox->chans[chan];
> +       p_chan = &mbox->chans[chan];
> +
> +       if (type == IMX_MU_TYPE_TXDB_V2)
> +               p_chan->txdone_method = TXDONE_BY_ACK;
> +
> +       return p_chan;
>  }
>
>  static struct mbox_chan *imx_mu_seco_xlate(struct mbox_controller *mbox,
>
> --
> 2.37.1
>
Peng Fan Sept. 18, 2023, 12:01 p.m. UTC | #2 
Hi Daniel,

> Subject: Re: [PATCH 2/2] mailbox: imx: support channel type tx doorbell v2
> 
> On Sun, Sep 17, 2023 at 5:45 PM Peng Fan (OSS) <peng.fan@oss.nxp.com>
> wrote:
> >
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > The Message Unit(MU) General Purpose Control registers are used for TX
> > doorbell, but there is no hardware ACK support.
> >
> > The current TX doorbell channel is using tasklet to emulate hardware
> > ACK support to kick the TX tick from controller driver side.
> >
> > The new added TX doorbell channel V2 not using tasklet to emulate the
> > hardware ACK support. The behavior for the channel is just writing the
> > GCR register, and no else. This will be used for SCMI mailbox.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/mailbox/imx-mailbox.c | 32 +++++++++++++++++++++++++++++---
> >  1 file changed, 29 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/mailbox/imx-mailbox.c
> > b/drivers/mailbox/imx-mailbox.c index 3ef4dd8adf5d..0af739ab571c
> > 100644
> > --- a/drivers/mailbox/imx-mailbox.c
> > +++ b/drivers/mailbox/imx-mailbox.c
> > @@ -20,7 +20,9 @@
> >  #include <linux/suspend.h>
> >  #include <linux/slab.h>
> >
> > -#define IMX_MU_CHANS           17
> > +#include "mailbox.h"
> > +
> > +#define IMX_MU_CHANS           24
> >  /* TX0/RX0/RXDB[0-3] */
> >  #define IMX_MU_SCU_CHANS       6
> >  /* TX0/RX0 */
> > @@ -39,6 +41,7 @@ enum imx_mu_chan_type {
> >         IMX_MU_TYPE_TXDB        = 2, /* Tx doorbell */
> >         IMX_MU_TYPE_RXDB        = 3, /* Rx doorbell */
> >         IMX_MU_TYPE_RST         = 4, /* Reset */
> > +       IMX_MU_TYPE_TXDB_V2     = 5, /* Tx doorbell with S/W ACK */
> >  };
> >
> >  enum imx_mu_xcr {
> > @@ -226,6 +229,9 @@ static int imx_mu_generic_tx(struct imx_mu_priv
> *priv,
> >                 imx_mu_xcr_rmw(priv, IMX_MU_GCR, IMX_MU_xCR_GIRn(priv-
> >dcfg->type, cp->idx), 0);
> >                 tasklet_schedule(&cp->txdb_tasklet);
> >                 break;
> > +       case IMX_MU_TYPE_TXDB_V2:
> > +               imx_mu_xcr_rmw(priv, IMX_MU_GCR, IMX_MU_xCR_GIRn(priv-
> >dcfg->type, cp->idx), 0);
> > +               break;
> >         default:
> >                 dev_warn_ratelimited(priv->dev, "Send data on wrong channel
> type: %d\n", cp->type);
> >                 return -EINVAL;
> > @@ -554,6 +560,9 @@ static int imx_mu_startup(struct mbox_chan *chan)
> >         int ret;
> >
> >         pm_runtime_get_sync(priv->dev);
> > +       if (cp->type == IMX_MU_TYPE_TXDB_V2)
> > +               return 0;
> > +
> >         if (cp->type == IMX_MU_TYPE_TXDB) {
> >                 /* Tx doorbell don't have ACK support */
> >                 tasklet_init(&cp->txdb_tasklet, imx_mu_txdb_tasklet,
> > @@ -595,6 +604,11 @@ static void imx_mu_shutdown(struct mbox_chan
> *chan)
> >         int ret;
> >         u32 sr;
> >
> > +       if (cp->type == IMX_MU_TYPE_TXDB_V2) {
> > +               pm_runtime_put_sync(priv->dev);
> > +               return;
> > +       }
> > +
> >         if (cp->type == IMX_MU_TYPE_TXDB) {
> >                 tasklet_kill(&cp->txdb_tasklet);
> >                 pm_runtime_put_sync(priv->dev); @@ -671,6 +685,7 @@
> > static struct mbox_chan *imx_mu_specific_xlate(struct mbox_controller
> > *mbox,  static struct mbox_chan * imx_mu_xlate(struct mbox_controller
> *mbox,
> >                                        const struct of_phandle_args
> > *sp)  {
> > +       struct mbox_chan *p_chan;
> >         u32 type, idx, chan;
> >
> >         if (sp->args_count != 2) {
> > @@ -680,14 +695,25 @@ static struct mbox_chan * imx_mu_xlate(struct
> > mbox_controller *mbox,
> >
> >         type = sp->args[0]; /* channel type */
> >         idx = sp->args[1]; /* index */
> > -       chan = type * 4 + idx;
> >
> > +       /* RST only supports 1 channel */
> > +       if ((type == IMX_MU_TYPE_RST) && idx) {
> > +               dev_err(mbox->dev, "Invalid RST channel %d\n", idx);
> > +               return ERR_PTR(-EINVAL);
> > +       }
> 
> I don't understand how is this related to introduction of a new doorbell
> channel. Can you please add it in a separate patch with proper explanation?

RST only supports 1 channel, but its value is 4, so here I extend to 4 channel
for RST in code, to make the calculation a bit easier to get TXDB_V2 channel
pointer. But from user side, nothing changed

IMX_MU_TYPE_RST         = 4, /* Reset */
IMX_MU_TYPE_TXDB_V2     = 5, /* Tx doorbell with S/W ACK */

It does not make much sense to separate this change, because this change
Would only make sense together with TXDB_V2 changes.

Thanks,
Peng.

> 
> 
> > +
> > +       chan = type * 4 + idx;
> >         if (chan >= mbox->num_chans) {
> >                 dev_err(mbox->dev, "Not supported channel number: %d.
> (type: %d, idx: %d)\n", chan, type, idx);
> >                 return ERR_PTR(-EINVAL);
> >         }
> >
> > -       return &mbox->chans[chan];
> > +       p_chan = &mbox->chans[chan];
> > +
> > +       if (type == IMX_MU_TYPE_TXDB_V2)
> > +               p_chan->txdone_method = TXDONE_BY_ACK;
> > +
> > +       return p_chan;
> >  }
> >
> >  static struct mbox_chan *imx_mu_seco_xlate(struct mbox_controller
> > *mbox,
> >
> > --
> > 2.37.1
> >
Daniel Baluta Sept. 18, 2023, 2:40 p.m. UTC | #3 
On Mon, Sep 18, 2023 at 3:01 PM Peng Fan <peng.fan@nxp.com> wrote:
>
> Hi Daniel,
>
> > Subject: Re: [PATCH 2/2] mailbox: imx: support channel type tx doorbell v2
> >
> > On Sun, Sep 17, 2023 at 5:45 PM Peng Fan (OSS) <peng.fan@oss.nxp.com>
> > wrote:
> > >
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > The Message Unit(MU) General Purpose Control registers are used for TX
> > > doorbell, but there is no hardware ACK support.
> > >
> > > The current TX doorbell channel is using tasklet to emulate hardware
> > > ACK support to kick the TX tick from controller driver side.
> > >
> > > The new added TX doorbell channel V2 not using tasklet to emulate the
> > > hardware ACK support. The behavior for the channel is just writing the
> > > GCR register, and no else. This will be used for SCMI mailbox.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >  drivers/mailbox/imx-mailbox.c | 32 +++++++++++++++++++++++++++++---
> > >  1 file changed, 29 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/mailbox/imx-mailbox.c
> > > b/drivers/mailbox/imx-mailbox.c index 3ef4dd8adf5d..0af739ab571c
> > > 100644
> > > --- a/drivers/mailbox/imx-mailbox.c
> > > +++ b/drivers/mailbox/imx-mailbox.c
> > > @@ -20,7 +20,9 @@
> > >  #include <linux/suspend.h>
> > >  #include <linux/slab.h>
> > >
> > > -#define IMX_MU_CHANS           17
> > > +#include "mailbox.h"
> > > +
> > > +#define IMX_MU_CHANS           24
> > >  /* TX0/RX0/RXDB[0-3] */
> > >  #define IMX_MU_SCU_CHANS       6
> > >  /* TX0/RX0 */
> > > @@ -39,6 +41,7 @@ enum imx_mu_chan_type {
> > >         IMX_MU_TYPE_TXDB        = 2, /* Tx doorbell */
> > >         IMX_MU_TYPE_RXDB        = 3, /* Rx doorbell */
> > >         IMX_MU_TYPE_RST         = 4, /* Reset */
> > > +       IMX_MU_TYPE_TXDB_V2     = 5, /* Tx doorbell with S/W ACK */
> > >  };
> > >
> > >  enum imx_mu_xcr {
> > > @@ -226,6 +229,9 @@ static int imx_mu_generic_tx(struct imx_mu_priv
> > *priv,
> > >                 imx_mu_xcr_rmw(priv, IMX_MU_GCR, IMX_MU_xCR_GIRn(priv-
> > >dcfg->type, cp->idx), 0);
> > >                 tasklet_schedule(&cp->txdb_tasklet);
> > >                 break;
> > > +       case IMX_MU_TYPE_TXDB_V2:
> > > +               imx_mu_xcr_rmw(priv, IMX_MU_GCR, IMX_MU_xCR_GIRn(priv-
> > >dcfg->type, cp->idx), 0);
> > > +               break;
> > >         default:
> > >                 dev_warn_ratelimited(priv->dev, "Send data on wrong channel
> > type: %d\n", cp->type);
> > >                 return -EINVAL;
> > > @@ -554,6 +560,9 @@ static int imx_mu_startup(struct mbox_chan *chan)
> > >         int ret;
> > >
> > >         pm_runtime_get_sync(priv->dev);
> > > +       if (cp->type == IMX_MU_TYPE_TXDB_V2)
> > > +               return 0;
> > > +
> > >         if (cp->type == IMX_MU_TYPE_TXDB) {
> > >                 /* Tx doorbell don't have ACK support */
> > >                 tasklet_init(&cp->txdb_tasklet, imx_mu_txdb_tasklet,
> > > @@ -595,6 +604,11 @@ static void imx_mu_shutdown(struct mbox_chan
> > *chan)
> > >         int ret;
> > >         u32 sr;
> > >
> > > +       if (cp->type == IMX_MU_TYPE_TXDB_V2) {
> > > +               pm_runtime_put_sync(priv->dev);
> > > +               return;
> > > +       }
> > > +
> > >         if (cp->type == IMX_MU_TYPE_TXDB) {
> > >                 tasklet_kill(&cp->txdb_tasklet);
> > >                 pm_runtime_put_sync(priv->dev); @@ -671,6 +685,7 @@
> > > static struct mbox_chan *imx_mu_specific_xlate(struct mbox_controller
> > > *mbox,  static struct mbox_chan * imx_mu_xlate(struct mbox_controller
> > *mbox,
> > >                                        const struct of_phandle_args
> > > *sp)  {
> > > +       struct mbox_chan *p_chan;
> > >         u32 type, idx, chan;
> > >
> > >         if (sp->args_count != 2) {
> > > @@ -680,14 +695,25 @@ static struct mbox_chan * imx_mu_xlate(struct
> > > mbox_controller *mbox,
> > >
> > >         type = sp->args[0]; /* channel type */
> > >         idx = sp->args[1]; /* index */
> > > -       chan = type * 4 + idx;
> > >
> > > +       /* RST only supports 1 channel */
> > > +       if ((type == IMX_MU_TYPE_RST) && idx) {
> > > +               dev_err(mbox->dev, "Invalid RST channel %d\n", idx);
> > > +               return ERR_PTR(-EINVAL);
> > > +       }
> >
> > I don't understand how is this related to introduction of a new doorbell
> > channel. Can you please add it in a separate patch with proper explanation?
>
> RST only supports 1 channel, but its value is 4, so here I extend to 4 channel
> for RST in code, to make the calculation a bit easier to get TXDB_V2 channel
> pointer. But from user side, nothing changed
>
> IMX_MU_TYPE_RST         = 4, /* Reset */
> IMX_MU_TYPE_TXDB_V2     = 5, /* Tx doorbell with S/W ACK */
>
> It does not make much sense to separate this change, because this change
> Would only make sense together with TXDB_V2 changes.


Ok, I see. Good for me, then.
diff mbox series

Patch

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 3ef4dd8adf5d..0af739ab571c 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -20,7 +20,9 @@ 
 #include <linux/suspend.h>
 #include <linux/slab.h>
 
-#define IMX_MU_CHANS		17
+#include "mailbox.h"
+
+#define IMX_MU_CHANS		24
 /* TX0/RX0/RXDB[0-3] */
 #define IMX_MU_SCU_CHANS	6
 /* TX0/RX0 */
@@ -39,6 +41,7 @@  enum imx_mu_chan_type {
 	IMX_MU_TYPE_TXDB	= 2, /* Tx doorbell */
 	IMX_MU_TYPE_RXDB	= 3, /* Rx doorbell */
 	IMX_MU_TYPE_RST		= 4, /* Reset */
+	IMX_MU_TYPE_TXDB_V2	= 5, /* Tx doorbell with S/W ACK */
 };
 
 enum imx_mu_xcr {
@@ -226,6 +229,9 @@  static int imx_mu_generic_tx(struct imx_mu_priv *priv,
 		imx_mu_xcr_rmw(priv, IMX_MU_GCR, IMX_MU_xCR_GIRn(priv->dcfg->type, cp->idx), 0);
 		tasklet_schedule(&cp->txdb_tasklet);
 		break;
+	case IMX_MU_TYPE_TXDB_V2:
+		imx_mu_xcr_rmw(priv, IMX_MU_GCR, IMX_MU_xCR_GIRn(priv->dcfg->type, cp->idx), 0);
+		break;
 	default:
 		dev_warn_ratelimited(priv->dev, "Send data on wrong channel type: %d\n", cp->type);
 		return -EINVAL;
@@ -554,6 +560,9 @@  static int imx_mu_startup(struct mbox_chan *chan)
 	int ret;
 
 	pm_runtime_get_sync(priv->dev);
+	if (cp->type == IMX_MU_TYPE_TXDB_V2)
+		return 0;
+
 	if (cp->type == IMX_MU_TYPE_TXDB) {
 		/* Tx doorbell don't have ACK support */
 		tasklet_init(&cp->txdb_tasklet, imx_mu_txdb_tasklet,
@@ -595,6 +604,11 @@  static void imx_mu_shutdown(struct mbox_chan *chan)
 	int ret;
 	u32 sr;
 
+	if (cp->type == IMX_MU_TYPE_TXDB_V2) {
+		pm_runtime_put_sync(priv->dev);
+		return;
+	}
+
 	if (cp->type == IMX_MU_TYPE_TXDB) {
 		tasklet_kill(&cp->txdb_tasklet);
 		pm_runtime_put_sync(priv->dev);
@@ -671,6 +685,7 @@  static struct mbox_chan *imx_mu_specific_xlate(struct mbox_controller *mbox,
 static struct mbox_chan * imx_mu_xlate(struct mbox_controller *mbox,
 				       const struct of_phandle_args *sp)
 {
+	struct mbox_chan *p_chan;
 	u32 type, idx, chan;
 
 	if (sp->args_count != 2) {
@@ -680,14 +695,25 @@  static struct mbox_chan * imx_mu_xlate(struct mbox_controller *mbox,
 
 	type = sp->args[0]; /* channel type */
 	idx = sp->args[1]; /* index */
-	chan = type * 4 + idx;
 
+	/* RST only supports 1 channel */
+	if ((type == IMX_MU_TYPE_RST) && idx) {
+		dev_err(mbox->dev, "Invalid RST channel %d\n", idx);
+		return ERR_PTR(-EINVAL);
+	}
+
+	chan = type * 4 + idx;
 	if (chan >= mbox->num_chans) {
 		dev_err(mbox->dev, "Not supported channel number: %d. (type: %d, idx: %d)\n", chan, type, idx);
 		return ERR_PTR(-EINVAL);
 	}
 
-	return &mbox->chans[chan];
+	p_chan = &mbox->chans[chan];
+
+	if (type == IMX_MU_TYPE_TXDB_V2)
+		p_chan->txdone_method = TXDONE_BY_ACK;
+
+	return p_chan;
 }
 
 static struct mbox_chan *imx_mu_seco_xlate(struct mbox_controller *mbox,