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: <44235c65-160c-04c7-294d-16b13d25605c@nvidia.com>
Date:   Wed, 30 Mar 2022 15:46:59 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [Patch v5 2/4] memory: tegra: Add MC error logging on tegra186
 onward
Content-Language: en-US
To:     Dmitry Osipenko <dmitry.osipenko@collabora.com>,
        Dmitry Osipenko <digetx@gmail.com>,
        krzysztof.kozlowski@canonical.com, robh+dt@kernel.org,
        thierry.reding@gmail.com, jonathanh@nvidia.com,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        linux-tegra@vger.kernel.org
Cc:     vdumpa@nvidia.com, Snikam@nvidia.com
References: <20220316092525.4554-1-amhetre@nvidia.com>
 <20220316092525.4554-3-amhetre@nvidia.com>
 <9ab1a77c-82e6-39be-9b90-b394037fb574@gmail.com>
 <a62bb479-fed4-ada0-ac61-fb67a663a998@nvidia.com>
 <4ea801f4-7929-148d-4e69-d4126a9dfbf7@collabora.com>
From:   Ashish Mhetre <amhetre@nvidia.com>
In-Reply-To: <4ea801f4-7929-148d-4e69-d4126a9dfbf7@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?cVQyYUd0RmwyVG15dEVPTU9DQWwyRk5JZWVaSFhDYkUzYUVyS1pLMEhEMm92?=
 =?utf-8?B?TDdqYXVRSU5hdm1DalZKay9VOVFMZjI0SlJhOFNMM1Aza2xhR2NZRi9nQ0t0?=
 =?utf-8?B?N3BoNWJlVnVqd29FMkNHZEwyTjFESU1OQi9qczA5SDdBSEE4QSsyYWQxeGlD?=
 =?utf-8?B?Z3VtWStYWG15VEkyZXhjdHRMUkNicVExZVE3NVEyK2pGZlgxNWQ4WHZ2eWQ2?=
 =?utf-8?B?RlpUUy9pVHFxQkl1MEVOL1JxY3IxZy9CZTUzcjJrck82VmpTRk5DMkZnRDZ3?=
 =?utf-8?B?MlhNZFNEL1YyRGI4c3YwdFFWS05SV1JkZFVUc3JzRklRM1ZkSEtQV2tpa09F?=
 =?utf-8?B?R3lQQStZMmlKY2hSMjBSLytaZUxHVENjbjB3b29udHIwbG5GR3p4MWFXenI3?=
 =?utf-8?B?K3NrVXAwV3ZSTVVCRldHdW5HVTYvdTR6eFNtMjUzRG9hNjU2aFdxZ3dRTldH?=
 =?utf-8?B?U1JKVGFYd1hTQWpCbWxIWjRpemMyMFNOb1pFVGJHeVErVCtDQ0xYcHRzdGFD?=
 =?utf-8?B?ais3RVJKK3dXeWJPVXh2RGJHSTZjbjVLblRSOGVOdzRpcERKVllpQUNuc2xw?=
 =?utf-8?B?a2tPUHo2ZVpvakhzcVJ2Q0J1Y3BCN1pTKzNTMFcwaUFqR3FiV3l3cXZhWGQy?=
 =?utf-8?B?N250Z25JVVp4bVYrK2ZJdWpXTjRBaGx4L3NTNXNNVEJMRFJ2dUp4VlJzSWF2?=
 =?utf-8?B?YzJ3TWZXVnVMdW1CRWYrRkMrZVQwNnBGOWpIK2VHWVZyR21ZZTQvaGdvS0tV?=
 =?utf-8?B?MG4rUWNDajZMaWREODdpYXJLM0twb3BHWk9nMG4xRitnckpKc1VHZFh6K3lx?=
 =?utf-8?B?OVdFbzROVVVOc2FGZGtOY0JGYVVyQjJLbE9jNEkrNkUxN1U3QkdjSm5pVjRp?=
 =?utf-8?B?VUtRMG04dklaWmFyUU1yUU03UmxZZzlYTEFhQm5rSXZha3Z2MXkxdU5WN1FX?=
 =?utf-8?B?QnhPWG1YRHRYR0tSUUl4bDFWWUJaNkxiRHJKVis3MjdBbzZYSUlMNkRlZzVR?=
 =?utf-8?B?TUw5eDdldzVzaDNWbkU0SE15MnA4K3dOTUc4dVl5d2RQOGxXS1JMdDJpc1hz?=
 =?utf-8?B?VTZadFZJeGdvWHVHdE50TWR2bjB4Zkppbm5KcE1zMHF2Tm9Lb3FnQnZra0NP?=
 =?utf-8?B?UjN5S0toNWh6WUZiZFlHdklFczAzb3N0Yk4ycG5ZUXdTTGhMd0ZQOXdTeE1i?=
 =?utf-8?B?a001SzI5NCtJMThodUR3TmwwMy9yaFpuQ3U3bGtVSWFBMzI3cnZOWlBTTldE?=
 =?utf-8?B?SU16RlpoZnV6M0pBQ2NzdmZTSldtWGplSWljZlFxdzMwNWtJeXBvamI5VWla?=
 =?utf-8?B?QXY2Uy9xcVRHWXJaV1QrMGNlTnFGeUQ0OVR1Vml4Wk1VcWpPRlNIaWFEVysv?=
 =?utf-8?B?YWdwWTM2dVpzdUR3d3F0VG9LZXRrdTVnMXNXekFxTWJUVEpDUVZOcVhZTUx5?=
 =?utf-8?B?QjBkQTRRckNXbFJ2Vmx3b3FRcnRDQlZMOStoN2FxYlcwbVBkSjFpb1BSNlo5?=
 =?utf-8?B?cmpTbEtKcjBxV0xOZG1ZbnJ1Wndod216K3ZaMzVzaFpzTFZsRncrdW1scmJX?=
 =?utf-8?B?VkNkRUhrdmN6RkFzbCs0SjBhMnMrRjQ1TzAxZnR5SFhkbThnTVNFL0xweW4y?=
 =?utf-8?B?bjh6LzFIUGp6eXJsbjViSUkzVy9nQ3JIYjhybHhxNVllZEhsSFZydFFUbC9t?=
 =?utf-8?B?VU8wdmNVTFFoc3ZvVFgzR0JtSFRxMFVaYms4dDdnTnplZjlVamJNVHdNVmp4?=
 =?utf-8?B?QVpKWnYzMmp0Q2N3eENRQVRVam5DQVE4Z0VlL3pQNXBYUmZIYzN5R3pLT1dI?=
 =?utf-8?B?NDBxRndyL2VGQUFBc2lpNVdvMXl6anYzb2xBZE5xam1UUkxKaStVYlRyNVNX?=
 =?utf-8?B?RFJZYW04Y0tWSjZXb1lBVDlKTlJBQlRHMWJadWR1N0U2K0wzcFlQQTFUTXZK?=
 =?utf-8?B?UzZyMWxlQ2pHak5KakR2THBqamdlZFJkcis1ejVsZlRhV3lKbnZzYnJON0RZ?=
 =?utf-8?B?RDNPeGgxNmJQVzNyV25Mdm1NWnFHL2JvQXlaSDZUS2lCNmxZZFRRU2xIQzFE?=
 =?utf-8?B?bW5QNnVqWDhsOEpYZTBlSkFmYS9TbkRUcldpUDF5NkhXWEZzMkFybTNpREVp?=
 =?utf-8?B?T3FOUERSTkJWYzJKbHRuSTZIQXJTQTBxcEZ6QWsvTmlnaDNEemFYejdqWVpO?=
 =?utf-8?B?Y2czdndwaWxobDBLTi80ZlBLMzU4alVkM2ZMaThYSGtFR2gya2lYWm52NWhL?=
 =?utf-8?B?Q0ZqVjlTUXZTRlpMUkU3bFl4OEFYWkxCNnNiRlluYkJwSzcvZEFuTTE5bGVM?=
 =?utf-8?B?SlNKemdLcUhyMGNVcUpRbnNrbjNJT3JLR1gyUkdFdW9tQ0EzRGQwdz09?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3356f342-0657-464f-7470-08da12367552
X-MS-Exchange-CrossTenant-AuthSource: SA0PR12MB4349.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Mar 2022 10:17:13.6592
 (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: 9lnFj/zIukqlFwAMo9DYK+xWiIEYzHgTeKAZp7WgHkqzUTYUmu+hxOF1K+QV95gQ/grCLT0y3Hs1rI5HKZKFPA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR12MB3126
X-Spam-Status: No, score=-1.1 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,T_SCC_BODY_TEXT_LINE autolearn=no 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 3/30/2022 5:31 AM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 3/22/22 20:34, Ashish Mhetre wrote:
>>>> +     switch (status & mc->soc->int_channel_mask) {
>>>> +     case BIT(0):
>>>> +             *mc_channel = 0;
>>>> +             break;
>>>> +
>>>> +     case BIT(1):
>>>> +             *mc_channel = 1;
>>>> +             break;
>>>> +
>>>> +     case BIT(2):
>>>> +             *mc_channel = 2;
>>>> +             break;
>>>> +
>>>> +     case BIT(3):
>>>> +             *mc_channel = 3;
>>>> +             break;
>>>> +
>>>> +     case BIT(24):
>>>> +             *mc_channel = MC_BROADCAST_CHANNEL;
>>>> +             break;
>>>> +
>>>> +     default:
>>>> +             pr_err("Unknown interrupt source\n");
>>>
>>> dev_err_ratelimited("unknown interrupt channel 0x%08x\n", status) and
>>> should be moved to the common interrupt handler.
>>>
>> So return just error from default case and handle error in common
>> interrupt handler with this print, right? I'll update this in next
>> version.
> 
> Yes, just move out the common print.
> 
> Although, you could parameterize the shift per SoC and then have a
> common helper that does "status >> intmask_chan_shift", couldn't you?

Do you mean shift to get the channel, like
"channel = status >> intmask_chan_shift"?
So to get rid of this callback completely and adding a variable in
tegra_mc_soc for intmask_chan_shift, right? Or compute shift in this
callback and use it in common handler?
If we are to remove this callback then how to handle unknown interrupt
channel error?
Also we want to handle interrupts on one channel at a time and then
clear it from status register. There can be interrupts on multiple
channel. So multiple bits from status will be set. Hence it will be
hard to parameterize shift such that it gives appropriate channel.
So I think current approach is fine. Please correct me if I am wrong
somewhere.