Subject: Re: [Patch V1 3/4] memory: tegra: add mc-err support for T186
To:     Ashish Mhetre <amhetre@nvidia.com>, thierry.reding@gmail.com,
        jonathanh@nvidia.com, linux-tegra@vger.kernel.org,
        krzysztof.kozlowski@canonical.com, linux-kernel@vger.kernel.org
Cc:     Snikam@nvidia.com, vdumpa@nvidia.com
References: <1641926750-27544-1-git-send-email-amhetre@nvidia.com>
 <1641926750-27544-4-git-send-email-amhetre@nvidia.com>
From:   Dmitry Osipenko <digetx@gmail.com>
Message-ID: <f9ca5794-55ac-803f-8606-f0b48e826eb7@gmail.com>
Date:   Wed, 12 Jan 2022 14:01:21 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
MIME-Version: 1.0
In-Reply-To: <1641926750-27544-4-git-send-email-amhetre@nvidia.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

11.01.2022 21:45, Ashish Mhetre пишет:
> Add all mc-errors supported by T186.
> Implement mc interrupt handling routine for T186.
> 
> Signed-off-by: Ashish Mhetre <amhetre@nvidia.com>
> ---
>  drivers/memory/tegra/mc.h       |  17 +++++++
>  drivers/memory/tegra/tegra186.c | 100 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 117 insertions(+)
> 
> diff --git a/drivers/memory/tegra/mc.h b/drivers/memory/tegra/mc.h
> index 2d4f495..7817492 100644
> --- a/drivers/memory/tegra/mc.h
> +++ b/drivers/memory/tegra/mc.h
> @@ -44,6 +44,15 @@
>  #define MC_TIMING_CONTROL_DBG				0xf8
>  #define MC_TIMING_CONTROL				0xfc
>  

this empty line is unnecessary

> +#define MC_ERR_VPR_STATUS				0x654
> +#define MC_ERR_VPR_ADR					0x658
> +#define MC_ERR_SEC_STATUS				0x67c
> +#define MC_ERR_SEC_ADR					0x680
> +#define MC_ERR_MTS_STATUS				0x9b0
> +#define MC_ERR_MTS_ADR					0x9b4
> +#define MC_ERR_GENERALIZED_CARVEOUT_STATUS		0xc00
> +#define MC_ERR_GENERALIZED_CARVEOUT_ADR			0xc04
> +
>  #define MC_INT_DECERR_ROUTE_SANITY			BIT(20)
>  #define MC_INT_WCAM_ERR					BIT(19)
>  #define MC_INT_SCRUB_ECC_WR_ACK				BIT(18)
> @@ -159,6 +168,14 @@ extern const struct tegra_mc_ops tegra186_mc_ops;
>  extern const char * const tegra_mc_status_names[32];
>  extern const char * const tegra_mc_error_names[8];
>  
> +struct tegra_mc_error {
> +	u32 int_bit;
> +	const char *msg;
> +	u32 status_reg;
> +	u32 addr_reg;
> +	u32 addr_reg_hi;
> +};
> +
>  /*
>   * These IDs are for internal use of Tegra ICC drivers. The ID numbers are
>   * chosen such that they don't conflict with the device-tree ICC node IDs.
> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
> index 6766cc4..4f3ae71 100644
> --- a/drivers/memory/tegra/tegra186.c
> +++ b/drivers/memory/tegra/tegra186.c
> @@ -146,8 +146,107 @@ static void tegra186_mc_clear_interrupt(struct tegra_mc *mc)
>  	mc_writel(mc, MC_INTSTATUS_CLEAR, MC_INTSTATUS);
>  }
>  
> +static const struct tegra_mc_error int_mc_errors[] = {
> +	{
> +		.int_bit = MC_INT_DECERR_EMEM,
> +		.msg = "EMEM address decode error",
> +		.status_reg = MC_ERR_STATUS,
> +		.addr_reg = MC_ERR_ADR,
> +	},
> +	{
> +		.int_bit = MC_INT_SECURITY_VIOLATION,
> +		.msg = "non secure access to secure region",
> +		.status_reg = MC_ERR_STATUS,
> +		.addr_reg = MC_ERR_ADR,
> +	},
> +	{
> +		.int_bit = MC_INT_DECERR_VPR,
> +		.msg = "MC request violates VPR requirements",
> +		.status_reg = MC_ERR_VPR_STATUS,
> +		.addr_reg = MC_ERR_VPR_ADR,
> +	},

I see that these VPR registers present on all SoCs starting with T124.
It doesn't look like you need the separate IRQ handlers at all, instead
please extend the common T30 handler. For example, you may add a
switch-case statements to handle those T124+ specific bits differently.

static irqreturn_t tegra30_mc_handle_irq(int irq, void *data)
{
...
	switch (bit) {
	case MC_INT_DECERR_VPR:
		status_reg = MC_ERR_VPR_STATUS;
		addr_reg   = MC_ERR_VPR_ADR;
		break;
	...
	default:
		status_reg = MC_ERR_STATUS;
		addr_reg   = MC_ERR_ADR;
	}

	value = mc_readl(mc, status_reg);
	...
	
	value = mc_readl(mc, addr_reg);