Re: [Patch v8 2/4] memory: tegra: Add MC error logging on tegra186 onward

From: Ashish Mhetre
Date: Tue Apr 26 2022 - 03:35:59 EST

On 4/26/2022 1:09 AM, Dmitry Osipenko wrote:
External email: Use caution opening links or attachments

On 4/25/22 10:50, Ashish Mhetre wrote:
Add support for reading MC_GLOBAL_INTSTATUS register which points to the
memory controller channels on which interrupts have occurred.
Add helper function 'mc_global_intstatus_to_channel' which returns the
channel which should be used to get the information of interrupts.
Remove static from tegra30_mc_handle_irq and use it as interrupt handler
for MC interrupts on tegra186, tegra194 and tegra234 to log the errors.
Add error specific MC status and address register bits and use them on
tegra186, tegra194 and tegra234.
Add error logging for generalized carveout interrupt on tegra186, tegra194
and tegra234.
Add error logging for route sanity interrupt on tegra194 an tegra234.
Add register for higher bits of error address which is available on
tegra194 and tegra234.
Add a boolean variable 'has_addr_hi_reg' in tegra_mc_soc struture which
will be true if soc has register for higher bits of memory controller
error address. Set it true for tegra194 and tegra234.
Add helper function 'mc_channel_to_global_intstatus' which returns the
bit of MC_GLOBAL_INSTATUS corresponding to channel of which interrupts
are logged and use it to clear that interrupt channel.
Update variable type of client_id_mask from u8 to u16 and add it for
tegra186, tegra194 and tegra234.

The formatting of the message could be improved. At minimum adding
newlines will help readability. For upstream patches it's very important
to make a good commit message that is readable and concise.

The commit message should contain:

1. A rationale for the change, i.e. you should explain why it is needed.
2. A brief description of what patch does, explaining only difficult
parts in a more details.

Okay, I will update commit message considering above comments.
I'll keep it to the point and remove unnecessary implementation details.

Signed-off-by: Ashish Mhetre <amhetre@xxxxxxxxxx>
drivers/memory/tegra/mc.c | 134 ++++++++++++++++++++++++++++----
drivers/memory/tegra/mc.h | 43 +++++++++-
drivers/memory/tegra/tegra186.c | 9 +++
drivers/memory/tegra/tegra194.c | 8 ++
drivers/memory/tegra/tegra234.c | 8 ++
include/soc/tegra/mc.h | 5 +-
6 files changed, 189 insertions(+), 18 deletions(-)

Otherwise looks okay:

Reviewed-by: Dmitry Osipenko <dmitry.osipenko@xxxxxxxxxxxxx>

Thanks Dmitry.