Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <14750f67-2f5e-3139-b1c4-a80bb4b892b8@nvidia.com>
Date:   Tue, 26 Apr 2022 13:05:05 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Subject: Re: [Patch v8 2/4] memory: tegra: Add MC error logging on tegra186
 onward
Content-Language: en-US
To:     Dmitry Osipenko <dmitry.osipenko@collabora.com>,
        krzysztof.kozlowski@linaro.org, thierry.reding@gmail.com,
        jonathanh@nvidia.com, digetx@gmail.com, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, linux-kernel@vger.kernel.org,
        linux-tegra@vger.kernel.org, devicetree@vger.kernel.org
Cc:     vdumpa@nvidia.com, Snikam@nvidia.com
References: <20220425075036.30098-1-amhetre@nvidia.com>
 <20220425075036.30098-3-amhetre@nvidia.com>
 <777b30fd-59ba-56ec-4f66-4d82118ca921@collabora.com>
From:   Ashish Mhetre <amhetre@nvidia.com>
In-Reply-To: <777b30fd-59ba-56ec-4f66-4d82118ca921@collabora.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eS9pbWV0MmdkRlFwejBCNDFpbUVWZ1pNV0gwZDhiajdEMEpLWXI3bEtGNE9K?=
 =?utf-8?B?bS8veUJWVEhwbkwrbzJKa1RYZmdLckhoYnhGZ3Awa3RRVWwvUGgweWZIdHI3?=
 =?utf-8?B?amRNV0Q5VS9OaDVLZmhsMCtvREladmNXaVo4WUJ1LzRwTjl4ckcybVg5MTNX?=
 =?utf-8?B?a1B4T2hrOVRja3dlZ2UrOWV6ZXpMak1RSFBTMG1uaVRhTWlBbS9aNzBEK3Zy?=
 =?utf-8?B?eDB5THhxTGtQVmFQV3ZmaVBBMVloMWh4T2FwVGsxSk5EZEhHdndYei9Ga2dm?=
 =?utf-8?B?a1YvZzExQUswMkd1MU10SWhSSFRLQ0VMV3J3VjdXMERHRWZVUWJSV3NIZDFI?=
 =?utf-8?B?bzNqQXZMRUNjTlpEdENNRGJITVI4Q3lQcmZCMW41OFVUQkpWcXVKeUV0ZCth?=
 =?utf-8?B?ZHAxRkk4Q1dSZTd2a0NScGhBRXdBelo4R0VSRlRWM1FBK1ZJTXIzb1J6UGVl?=
 =?utf-8?B?OE1mYUFla0hnSm8xUHNqZVdMbEwyRzkyd2lEU3JuYlBCZGdEKzJqTTgxK1g1?=
 =?utf-8?B?R2F1WHY4VEhnUjg0R2Riei9nS05KdE9PaEdteU5lQXBuWkhDd1pJREFaN1Nw?=
 =?utf-8?B?ZVJybWFiSzVRMjhOYWdyUHBvOGpVRGpYZVh1ZTJqWEEwTkQ2QUozSUtiYTR3?=
 =?utf-8?B?TWpKZjJzWkU5UkdxdzUvS1YveUx1TUVGajNEWUxFVGFublBUOEFVQnk4QVox?=
 =?utf-8?B?Nm1lMHdCUm5iMVFHRE5pa0Voc0ZZVWVwaTFJNWd0akJhdkVtelQyM0hld3lw?=
 =?utf-8?B?cmowcTMzVU00UzlKaHk4NS96TFpTOXdWYUxDaDluY2hYWWp2eldoVFhzS1lG?=
 =?utf-8?B?ZDl1STNsZFIySG1Pc1ZIZ0RyV0NELzY0eXNqQjVFVUgrSUFCWEtxY3ZJS3Nt?=
 =?utf-8?B?UHNVc2Y4TVlkZFo1SXpTYmZsalRlWHZhMVdnUjVrTXEzaGdDUlIzOWlyYm1R?=
 =?utf-8?B?Q3FFT3Rvc2lTeG13ZEg2Yi9XUTQ1QmwyV2pZTGFXLzBKRUVXUzh4OFEzQzVX?=
 =?utf-8?B?T0N4ZzRqWlk5bmVOVjB5TWJRNXFwQVYzZmNDb2cxZ1hqN0FxY21OSU5UVDFL?=
 =?utf-8?B?UXBNMW9JNjlJallXMStjcjR0ZVlndmF3d3grZkNNN2Q5M2d1NWg3S2h6WGNt?=
 =?utf-8?B?VU5JU2laaENHNUFDNitHUEQrVXlVdnplZWt6S3dIakpxRGJoT0QvQklyTGJx?=
 =?utf-8?B?YXhEbnhRcjAraFVxS252ZkZSTFdHVnBYUkFlZVBsNDc1dHlDeW00TG14YlpP?=
 =?utf-8?B?MmhRT2Q3Q0ZKYkEzYWN0QmRPMTBtc3JJd043V05WanpkMzc4bENqc01IRS80?=
 =?utf-8?B?UHJvNDFDK20xZHhQL29WdUdHRkthblNLODM4K1ZRMGxURjdneHlMZTVINS8x?=
 =?utf-8?B?Z285MzUwcnorUWY2Y2t2WDBsekNQUTk5QjBrbDhHcnFiYnRRWXBZQzNrOWdZ?=
 =?utf-8?B?eTVPdkkxRVZjeUwxR243WHVkNHQySUpMODBYaEd2bXdxajg2NmNZR2V0L21L?=
 =?utf-8?B?b0ZNWDNhWVVuRDNNNXR3RzJSZm1PdFI3TFNuWm5WL0NlUFhlTjFBWkd3SU9X?=
 =?utf-8?B?d2Rpc1RIZ0dQdDk1YTlPUG12dkRaNjRpQU9FUzBLN29NTVhQOEdLbjhWdzBG?=
 =?utf-8?B?Vjg2WUQvOEZldndUMXBYVXRUbTFKWG0wYkpDbVh3VlRGcWVwNWx6TkdTaXpr?=
 =?utf-8?B?bmp0bVlyaXUzejNjRW1QRnpvbEhpeDhTeFBTUWhoREc4eVYxbmVSZ09GZm00?=
 =?utf-8?B?M1ppb3BzT3JHOGhGTFBjOUZ1NGxTUTFobnFuUVJWNVBiL1ZkNGdsWnFkMmZ3?=
 =?utf-8?B?Z3lNSUZrYjU4b05YMERZU1dvWXl6SGU5djN3TFREL2RNRFNFMTV6eGVaWUVy?=
 =?utf-8?B?Vng2S3ZyTXlRbHphTFQ5SGJqN1psa3dOZVM0T3R1aWhWc2xzQm02K1RTdmJ4?=
 =?utf-8?B?ckVnaWJzVEFoTmRUWEZ6S3JrYlNxY0tFM2YxUkRHeEIrem9GV01LWm9hbyt2?=
 =?utf-8?B?b3BxRmk3dTk2d1MvUnNMVkg4UHNQdWVLUTdyMHd2VmN4YXJZaCtESXpDT0Ew?=
 =?utf-8?B?M3dTc3pJa1hWeVYrSHF3OHNibW4wRk1ZMjJ6MW1GNDRNRCtkY1hrWkpndjhE?=
 =?utf-8?B?aFRvYzhKcElTTWRRT0pHVlRIRGV2Ry9VV0Q1L0R5Zk9SNldqb1Z3RHByVWVq?=
 =?utf-8?B?Tk55UFYwZlU2UXhqME5ScWZpTk9NaGJGRzJIdjhJdVdiSGxFNmk5bVNGUE8z?=
 =?utf-8?B?YlU3b04vTmkyZG5teDFkTnZGamZ4RU1ZMC9za0I5dkZuL0RmMFlhUTdFSkJx?=
 =?utf-8?B?M1hSbEh1cmt5SkZUOERsSzRhaXluVEpXcmNFVTBJTkU1T0JXM2JoQT09?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4dcb1865-19e5-4a6f-4b2c-08da275750a1
X-MS-Exchange-CrossTenant-AuthSource: SA0PR12MB4349.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Apr 2022 07:35:19.8026
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 2gZu77hZFrTgCWfrZSNRL36vdmKa/0+HJYbTKfPw6+jOF590YKPH5Eyx0P/Ob2f09TLoDHwBUhOgJL0A5tw9fQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR12MB5663
X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


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@nvidia.com>
>> ---
>>   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@collabora.com>

Thanks Dmitry.