| Message ID | 20220302022522.1789588-5-peng.fan@oss.nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mailbox: imx: support i.MX93 | expand |
Hi, On Tue, Mar 1, 2022 at 8:23 PM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote: > > From: Peng Fan <peng.fan@nxp.com> > > i.MX93 S401 MU support two interrupts: tx empty and rx full. > > - Introduce a new flag IMX_MU_V2_IRQ for the dual interrupt case > - Update author and Copyright > Copyright update is fair. However, I am not sure adding an extra interrupt warrants co-authorship, otherwise people submit far bigger changes to drivers. And you didn't even CC the original author Oleksij Rempel. At least please seek his ACK. > diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c > index 03699843a6fd..4bc59a6cad20 100644 > --- a/drivers/mailbox/imx-mailbox.c > +++ b/drivers/mailbox/imx-mailbox.c .... > > +/* Please not change TX & RX */ > enum imx_mu_chan_type { > IMX_MU_TYPE_TX, /* Tx */ > IMX_MU_TYPE_RX, /* Rx */ > You want to hard-code the values to make it clearer IMX_MU_TYPE_TX = 0, IMX_MU_TYPE_RX = 1, > @@ -536,7 +539,8 @@ static int imx_mu_startup(struct mbox_chan *chan) > { > struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox); > struct imx_mu_con_priv *cp = chan->con_priv; > - unsigned long irq_flag = IRQF_SHARED; > + unsigned long irq_flag = 0; > + int irq; > int ret; > > pm_runtime_get_sync(priv->dev); > @@ -551,11 +555,16 @@ static int imx_mu_startup(struct mbox_chan *chan) > if (!priv->dev->pm_domain) > irq_flag |= IRQF_NO_SUSPEND; > > - ret = request_irq(priv->irq[0], imx_mu_isr, irq_flag, > - cp->irq_desc, chan); > + if (priv->dcfg->type & IMX_MU_V2_IRQ) { > + irq = priv->irq[cp->type]; > + } else { > + irq = priv->irq[0]; > Please use some verbose define instead of the magic 0. Thanks.
> Subject: Re: [PATCH V6 4/5] mailbox: imx: support dual interrupts > > Hi, > > On Tue, Mar 1, 2022 at 8:23 PM Peng Fan (OSS) <peng.fan@oss.nxp.com> > wrote: > > > > From: Peng Fan <peng.fan@nxp.com> > > > > i.MX93 S401 MU support two interrupts: tx empty and rx full. > > > > - Introduce a new flag IMX_MU_V2_IRQ for the dual interrupt case > > - Update author and Copyright > > > Copyright update is fair. > However, I am not sure adding an extra interrupt warrants co-authorship, > otherwise people submit far bigger changes to drivers. I just thought I did lots of changes to this driver and just add my authorship here. That's fine, I'll drop. > And you didn't even CC the original author Oleksij Rempel. At least please > seek his ACK. I just use scripts/get_maintainers script, will add Oleksij. > > > diff --git a/drivers/mailbox/imx-mailbox.c > > b/drivers/mailbox/imx-mailbox.c index 03699843a6fd..4bc59a6cad20 > > 100644 > > --- a/drivers/mailbox/imx-mailbox.c > > +++ b/drivers/mailbox/imx-mailbox.c > .... > > > > +/* Please not change TX & RX */ > > enum imx_mu_chan_type { > > IMX_MU_TYPE_TX, /* Tx */ > > IMX_MU_TYPE_RX, /* Rx */ > > > You want to hard-code the values to make it clearer > IMX_MU_TYPE_TX = 0, > IMX_MU_TYPE_RX = 1, > > > > @@ -536,7 +539,8 @@ static int imx_mu_startup(struct mbox_chan *chan) > > { > > struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox); > > struct imx_mu_con_priv *cp = chan->con_priv; > > - unsigned long irq_flag = IRQF_SHARED; > > + unsigned long irq_flag = 0; > > + int irq; > > int ret; > > > > pm_runtime_get_sync(priv->dev); @@ -551,11 +555,16 @@ > static > > int imx_mu_startup(struct mbox_chan *chan) > > if (!priv->dev->pm_domain) > > irq_flag |= IRQF_NO_SUSPEND; > > > > - ret = request_irq(priv->irq[0], imx_mu_isr, irq_flag, > > - cp->irq_desc, chan); > > + if (priv->dcfg->type & IMX_MU_V2_IRQ) { > > + irq = priv->irq[cp->type]; > > + } else { > > + irq = priv->irq[0]; > > > Please use some verbose define instead of the magic 0. Fix in V7. Thanks, Peng. > > Thanks.
diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c index 03699843a6fd..4bc59a6cad20 100644 --- a/drivers/mailbox/imx-mailbox.c +++ b/drivers/mailbox/imx-mailbox.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 /* * Copyright (c) 2018 Pengutronix, Oleksij Rempel <o.rempel@pengutronix.de> + * Copyright 2022 NXP, Peng Fan <peng.fan@nxp.com> */ #include <linux/clk.h> @@ -28,6 +29,7 @@ #define IMX_MU_SECO_TX_TOUT (msecs_to_jiffies(3000)) #define IMX_MU_SECO_RX_TOUT (msecs_to_jiffies(3000)) +/* Please not change TX & RX */ enum imx_mu_chan_type { IMX_MU_TYPE_TX, /* Tx */ IMX_MU_TYPE_RX, /* Rx */ @@ -92,6 +94,7 @@ enum imx_mu_type { IMX_MU_V1, IMX_MU_V2 = BIT(1), IMX_MU_V2_S4 = BIT(15), + IMX_MU_V2_IRQ = BIT(16), }; struct imx_mu_dcfg { @@ -536,7 +539,8 @@ static int imx_mu_startup(struct mbox_chan *chan) { struct imx_mu_priv *priv = to_imx_mu_priv(chan->mbox); struct imx_mu_con_priv *cp = chan->con_priv; - unsigned long irq_flag = IRQF_SHARED; + unsigned long irq_flag = 0; + int irq; int ret; pm_runtime_get_sync(priv->dev); @@ -551,11 +555,16 @@ static int imx_mu_startup(struct mbox_chan *chan) if (!priv->dev->pm_domain) irq_flag |= IRQF_NO_SUSPEND; - ret = request_irq(priv->irq[0], imx_mu_isr, irq_flag, - cp->irq_desc, chan); + if (priv->dcfg->type & IMX_MU_V2_IRQ) { + irq = priv->irq[cp->type]; + } else { + irq = priv->irq[0]; + irq_flag |= IRQF_SHARED; + } + + ret = request_irq(irq, imx_mu_isr, irq_flag, cp->irq_desc, chan); if (ret) { - dev_err(priv->dev, - "Unable to acquire IRQ %d\n", priv->irq[0]); + dev_err(priv->dev, "Unable to acquire IRQ %d\n", irq); return ret; } @@ -762,14 +771,23 @@ static int imx_mu_probe(struct platform_device *pdev) if (IS_ERR(priv->base)) return PTR_ERR(priv->base); - priv->irq[0] = platform_get_irq(pdev, 0); - if (priv->irq[0] < 0) - return priv->irq[0]; - dcfg = of_device_get_match_data(dev); if (!dcfg) return -EINVAL; priv->dcfg = dcfg; + if (priv->dcfg->type & IMX_MU_V2_IRQ) { + priv->irq[IMX_MU_TYPE_TX] = platform_get_irq_byname(pdev, "tx"); + if (priv->irq[IMX_MU_TYPE_TX] < 0) + return priv->irq[IMX_MU_TYPE_TX]; + priv->irq[IMX_MU_TYPE_RX] = platform_get_irq_byname(pdev, "rx"); + if (priv->irq[IMX_MU_TYPE_RX] < 0) + return priv->irq[IMX_MU_TYPE_RX]; + } else { + priv->irq[0] = platform_get_irq(pdev, 0); + if (priv->irq[0] < 0) + return priv->irq[0]; + + } if (priv->dcfg->type & IMX_MU_V2_S4) size = sizeof(struct imx_s4_rpc_msg_max); @@ -1001,5 +1019,6 @@ static struct platform_driver imx_mu_driver = { module_platform_driver(imx_mu_driver); MODULE_AUTHOR("Oleksij Rempel <o.rempel@pengutronix.de>"); +MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>"); MODULE_DESCRIPTION("Message Unit driver for i.MX"); MODULE_LICENSE("GPL v2");