[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aWTQ3ud9SHrhM6lZ@orome>
Date: Mon, 12 Jan 2026 11:52:00 +0100
From: Thierry Reding <thierry.reding@...il.com>
To: Ketan Patil <ketanp@...dia.com>
Cc: krzk@...nel.org, jonathanh@...dia.com, linux-kernel@...r.kernel.org,
linux-tegra@...r.kernel.org
Subject: Re: [PATCH v5 3/4] memory: tegra: Add support for multiple irqs
On Fri, Dec 19, 2025 at 11:43:53AM +0000, Ketan Patil wrote:
> Add support to handle multiple MC interrupts lines as Tegra264 supports
> multiple MC interrupt lines. So make the necessary changes to the Tegra
> MC driver to support devices with more than one interrupt line.
> Add field to specify the number of interrupts and iterate over the
> number of interrupts to register handler for each interrupt.
>
> Signed-off-by: Ketan Patil <ketanp@...dia.com>
> ---
> drivers/memory/tegra/mc.c | 35 +++++++++++++++++++++------------
> drivers/memory/tegra/mc.h | 1 +
> drivers/memory/tegra/tegra186.c | 3 ++-
> drivers/memory/tegra/tegra20.c | 7 ++++++-
> include/soc/tegra/mc.h | 4 +++-
> 5 files changed, 34 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
> index 6c1578b25a61..82a81ae3c50d 100644
> --- a/drivers/memory/tegra/mc.c
> +++ b/drivers/memory/tegra/mc.c
> @@ -551,10 +551,15 @@ int tegra30_mc_probe(struct tegra_mc *mc)
>
> const struct tegra_mc_ops tegra30_mc_ops = {
> .probe = tegra30_mc_probe,
> - .handle_irq = tegra30_mc_handle_irq,
> + .handle_irq = tegra30_mc_irq_handlers,
> + .num_interrupts = 1,
> };
> #endif
>
> +const irq_handler_t tegra30_mc_irq_handlers[] = {
> + tegra30_mc_handle_irq
> +};
> +
Technically not necessary since we have the forward-declaration in the
header, but I think it'd make sense to move the tegra30_mc_irq_handlers
definition above tegra30_mc_ops so we keep the natural ordering of
things.
> static int mc_global_intstatus_to_channel(const struct tegra_mc *mc, u32 status,
> unsigned int *mc_channel)
> {
> @@ -953,25 +958,29 @@ static int tegra_mc_probe(struct platform_device *pdev)
> tegra_mc_num_channel_enabled(mc);
>
> if (mc->soc->ops && mc->soc->ops->handle_irq) {
> - mc->irq = platform_get_irq(pdev, 0);
> - if (mc->irq < 0)
> - return mc->irq;
> -
> WARN(!mc->soc->client_id_mask, "missing client ID mask for this SoC\n");
>
> + for (int i = 0; i < mc->soc->ops->num_interrupts; i++) {
It's more idiomatic to declare these variables outside of the loop. Also
make it unsigned to match the type of num_interrupts.
> + int irq;
> +
> + irq = platform_get_irq(pdev, i);
> + if (irq < 0)
> + return irq;
> +
> + err = devm_request_irq(&pdev->dev, irq, mc->soc->ops->handle_irq[i], 0,
> + dev_name(&pdev->dev), mc);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to request IRQ#%u: %d\n", irq,
> + err);
> + return err;
> + }
> + }
> +
> if (mc->soc->num_channels)
> mc_ch_writel(mc, MC_BROADCAST_CHANNEL, mc->soc->intmask,
> MC_INTMASK);
> else
> mc_writel(mc, mc->soc->intmask, MC_INTMASK);
> -
> - err = devm_request_irq(&pdev->dev, mc->irq, mc->soc->ops->handle_irq, 0,
> - dev_name(&pdev->dev), mc);
> - if (err < 0) {
> - dev_err(&pdev->dev, "failed to request IRQ#%u: %d\n", mc->irq,
> - err);
> - return err;
> - }
> }
>
> if (mc->soc->reset_ops) {
> diff --git a/drivers/memory/tegra/mc.h b/drivers/memory/tegra/mc.h
> index 482f836f7816..06ae3dd37a47 100644
> --- a/drivers/memory/tegra/mc.h
> +++ b/drivers/memory/tegra/mc.h
> @@ -194,6 +194,7 @@ extern const struct tegra_mc_ops tegra186_mc_ops;
> #endif
>
> irqreturn_t tegra30_mc_handle_irq(int irq, void *data);
> +extern const irq_handler_t tegra30_mc_irq_handlers[];
> extern const char * const tegra_mc_status_names[32];
> extern const char * const tegra_mc_error_names[8];
>
> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
> index a30158d92412..a3727fc383ac 100644
> --- a/drivers/memory/tegra/tegra186.c
> +++ b/drivers/memory/tegra/tegra186.c
> @@ -174,7 +174,8 @@ const struct tegra_mc_ops tegra186_mc_ops = {
> .remove = tegra186_mc_remove,
> .resume = tegra186_mc_resume,
> .probe_device = tegra186_mc_probe_device,
> - .handle_irq = tegra30_mc_handle_irq,
> + .handle_irq = tegra30_mc_irq_handlers,
> + .num_interrupts = 1,
> };
>
> #if defined(CONFIG_ARCH_TEGRA_186_SOC)
> diff --git a/drivers/memory/tegra/tegra20.c b/drivers/memory/tegra/tegra20.c
> index 46e97bb10163..75eeb49054cc 100644
> --- a/drivers/memory/tegra/tegra20.c
> +++ b/drivers/memory/tegra/tegra20.c
> @@ -761,9 +761,14 @@ static irqreturn_t tegra20_mc_handle_irq(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static const irq_handler_t tegra20_mc_irq_handlers[] = {
> + tegra20_mc_handle_irq
> +};
> +
> static const struct tegra_mc_ops tegra20_mc_ops = {
> .probe = tegra20_mc_probe,
> - .handle_irq = tegra20_mc_handle_irq,
> + .handle_irq = tegra20_mc_irq_handlers,
> + .num_interrupts = 1,
Probably not worth it since this will likely never change, but you could
use ARRAY_SIZE here.
> };
>
> const struct tegra_mc_soc tegra20_mc_soc = {
> diff --git a/include/soc/tegra/mc.h b/include/soc/tegra/mc.h
> index d11dfefbe551..4a2cadbc0084 100644
> --- a/include/soc/tegra/mc.h
> +++ b/include/soc/tegra/mc.h
> @@ -14,6 +14,7 @@
> #include <linux/reset-controller.h>
> #include <linux/types.h>
> #include <linux/tegra-icc.h>
> +#include <linux/interrupt.h>
These should be sorted alphabetically. I realize that tegra-icc.h is not
in the right order, so we might want to fix that eventually (though it's
Tegra-specific and therefore could be kept separate, too).
> struct clk;
> struct device;
> @@ -164,8 +165,9 @@ struct tegra_mc_ops {
> int (*probe)(struct tegra_mc *mc);
> void (*remove)(struct tegra_mc *mc);
> int (*resume)(struct tegra_mc *mc);
> - irqreturn_t (*handle_irq)(int irq, void *data);
> + const irq_handler_t *handle_irq;
> int (*probe_device)(struct tegra_mc *mc, struct device *dev);
> + unsigned int num_interrupts;
It's a good idea to keep the array and the variable that counts elements
in it close together.
Thierry
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists