Re: [PATCH v3 4/4] memory: tegra: Add MC error logging support for Tegra264

[Jon Hunter]

Hi Ketan,

On 04/10/2025 20:11, Ketan Patil wrote:
> In Tegra264, different components from memory subsystems like Memory
> Controller Fabric (MCF), HUB, HUB Common (HUBC), Side Band Shim (SBS)
> and MC Channels have different interrupt lines for receiving memory
> controller error interrupts.
>
> Add support for logging memory controller errors on Tegra264.
> - Add MC error handling flow for MCF, HUB, HUBC, SBS and MC Channel.
> - Each of these components have different interrupt lines for MC error.
> - Register interrupt handlers for interrupts from these different MC
> components.
> - There is no global interrupt status register in Tegra264 unlike older
> Tegra chips.
> - There are common interrupt status registers in case of MCF, HUB, HUBC
> from which figure out the slice number or hub number responsible for
> generating interrupt and then read interrupt status register to find out
> type of violation.
> - Introduce new SoC specific fields in tegra_mc_soc like interrupt mask
> values for MCF, HUB, HUBC etc., which are SOC specific.
>
> Signed-off-by: Ketan Patil <ketanp@xxxxxxxxxx>

...

> +static irqreturn_t handle_hub_irq(int irq, void *data)
> +{
> +	struct tegra_mc *mc = data;
> +	unsigned long hub_global_intstat, hub_intstat, hub_interrupted = 0;
> +	unsigned int hub_gobal_mask = 0x7F00, hub_gobal_shift = 8, hub;

I assume the above are typos and these should be 'hub_global_mask' and 
'hub_global_shift'?

> +
> +	/* Read MSS_HUB_GLOBAL_INTSTATUS_0 from MCB block */
> +	hub_global_intstat = mc_ch_readl(mc, MC_BROADCAST_CHANNEL, MSS_HUB_GLOBAL_INTSTATUS_0);
> +	if (hub_global_intstat == 0) {
> +		dev_err(mc->dev, "No interrupt in HUB/HUBC\n");

It appears that this handle_hub_irq() is used for multiple interrupts, 
should the error messages in this function print the IRQ number that 
triggered this?

BTW, it appears that all interrupts that use this handle_hub_irq() 
function pass the same tegra_mc structure and so from what I can tell 
they are all reading and accessing the same registers. Is that intentional?

Jon

--
nvpublic