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: <1d9e2055-ac22-334a-f003-911034ef75f4@nvidia.com>
Date:   Fri, 1 Apr 2022 03:25:01 +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>
 <44235c65-160c-04c7-294d-16b13d25605c@nvidia.com>
 <bf851834-7812-13f1-a382-1f64078ff2a5@collabora.com>
 <06174428-edb1-6478-1b2c-ede83c4bfa87@nvidia.com>
 <bf8bee92-0d0e-453b-5ff1-1829a4406fd1@collabora.com>
From:   Ashish Mhetre <amhetre@nvidia.com>
In-Reply-To: <bf8bee92-0d0e-453b-5ff1-1829a4406fd1@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?bWdUTW5HZ2VCam1UQWdEYldXSVk3MEhUVFYxVGFEVXB0VWowOXRxTUgvSEtq?=
 =?utf-8?B?OVNGVGNBOUIzd21aN0haU1JxTFhCaFlwL012V3VSVUVVaXdtamhEQ3F3K3hU?=
 =?utf-8?B?T25uQ1FsVmJkNkMvY25jVEc1YThXWUZHTXlaVGNDUE1lVjJjdlAxdGJKcmt0?=
 =?utf-8?B?eWREcE5aWUNXTFBuRmFaOXdTQXpkNWdsZkdIRTZaK0wyYWM2cnVPWGxWamdr?=
 =?utf-8?B?RGFtejZhaXluSXhyTktTSld1K3dzWXBuaE43YkZCVEphRkh6N2FjV0hkWmV6?=
 =?utf-8?B?eDhiVjlBSFE2THExQm5SSnlHUzZGMEN2MHF6c2JOTVMwZ2ZsaGNQeE03SlhV?=
 =?utf-8?B?R2dKOGk4QmwxT2FyUXF1NzVlUkdNRmNaZjNZOXRqakRKM2RUKzF4TGFtZldh?=
 =?utf-8?B?VzUrSU13MENJODBGQVZsT2V2ZldheHJaYkZzUWp5emJSM3VjYWp1Qi9UMFpZ?=
 =?utf-8?B?VFBPdkxBMVlGOVlNYlVNTVRYNXlIdFVnbFQzNG1GOW9TMVUrSVk1VGZmeXJZ?=
 =?utf-8?B?ODBFTzg2VGxrRlgwaXBkY1JuRzlsRjBrMUVzbDFGSjFxVkk3WU83U3JJeDdh?=
 =?utf-8?B?b0FHQ1JLb3h6MVl2bkdSU2svUjk5UzFHQ0d4N3RPM3dVUTdoQloxdHVYZnF0?=
 =?utf-8?B?N093ZVUxaisyODgxbnpIdTYxWXE4bm9BQThBOUtZS3VrMkNmUVdYdTBYM21q?=
 =?utf-8?B?S3JFRXc1TWRFb2s3T0lZTWdUS05zL1VYc0E1dktSeE92TXoyRTVNUXNzMlJy?=
 =?utf-8?B?QkFScTNhUGtJeHVWM0FYRWcwc1lnelJzOXZVcyswVkZ3Q1JLaTlBaENreExl?=
 =?utf-8?B?NjIyQVlkMVM5R1owQ3I0WFVkc3Rrc0dlVVp4ME9WOEwvdlAyTnc4VUx3YzI5?=
 =?utf-8?B?SE85ckZlWVNtdzQ4V3ZSVzVGcENVOHF0TE1PMlNWUFF0RkRWajdUMndQNmFS?=
 =?utf-8?B?bmhZczZwcFBxRDZzdnowZWNqNlVuOHBaZUJXSkI4Z3VzdGpqQ3hNRjZ1M2g3?=
 =?utf-8?B?QVBvTk9tOEUwd2phTll6UUJEMHVFYmhIOWt3UUhrbkJZRFJUbS95cVlGRVhu?=
 =?utf-8?B?NC9Vcnd2SS9NTlVxSXJrSlEybnplTDlCdlZ5QzZJMkxWZXp2ZFpvUHR6NlpN?=
 =?utf-8?B?K3lYczVza1h5QTR2MTNtWTBtWGdEWkNrVERPSnkzWWtoM3RhWXVxeXVLbE5Q?=
 =?utf-8?B?RzRwL1RyQVVZcHBCNG9rZGdsRzJ6UVZsamFMaHRNaVgrd0pxSG1jL1pBMjdj?=
 =?utf-8?B?Mnc1RjFtMXNqNW01dU8wTEU4Ull3T3hmWDlWcVVJY0pwUldKc1RTQmFXZVVw?=
 =?utf-8?B?S2NKVnV1N3dHVG9vaUpGV2JIekdtc3NmQjkvTGh1VzVvZ2MxckFoVE83bTdy?=
 =?utf-8?B?a3hmVlg2RFFxQjFINWt4VjBxQ0RPOTFsQXZOZllhU1ZMMVBBVjhxM2xXZEVz?=
 =?utf-8?B?MFk3bEtaRXlZc3krbGF3ODA5aDE1c1VVK0RxUjFDaGhUTHB0M0FYV1h1RzVj?=
 =?utf-8?B?RS91TVBLTi9CY1BCaU9IdnhpaUIrZm9lUnc0dThBL0RxUUp3YjZpS0RuNFVE?=
 =?utf-8?B?a2FZdWxLaVdUUzlUNVBITUNHRWdFQm1BWGFiMHZxWmFGNzU0cEFkeFRtbXhv?=
 =?utf-8?B?MDR2cEJmVi80Nk8yS3RyMlV2WXo0djhWc0RCYXNtNVpKWk9NcVByZ3Q4Y3FU?=
 =?utf-8?B?UTJvWXN3YzBnOEdBb25BQ3RYT1lDTWZFS3pYYXJKdGhDVWs5aUN2ei9jVG1y?=
 =?utf-8?B?UHhsRXUxZVJkN1JnZ1RWZXp2ZFVkMlRheWROYVlmaFd2M3RoR1FZQkEwOFBw?=
 =?utf-8?B?SVhqRFNIanQ1OUNPVnNGR0g1dzJOSExjK1dLajA0VlFyWWJ1by9JK1Ruck5L?=
 =?utf-8?B?ZzhtUUEydlllVXVGRUpPY2NzMzdOekZrMC9DUDBadlV0YmtPdk1VZFhTT0Q3?=
 =?utf-8?B?ZnhnL0NuZC9vZk8vS2FPZTBJMFBZNjZ0R0tUZ0pCZytLd0wvQ0FxbG9oTHBT?=
 =?utf-8?B?Ly9rZ213WmVpQXdpeFBkVllrSmtLRU85QUwwSVBKZUE4OXlKc09TQkI1S2Qw?=
 =?utf-8?B?akErUk9KMUM2ZzZ0Qi9YelhFOURXNExiSnJOdVJ1cXdXQVBUSHM1VEVCWVcr?=
 =?utf-8?B?L3B0dU1ZQ0pmOFhvSUZXak81eFVKY0M5VkhNaDYycDUwVlJVM2NkOGJvc1B5?=
 =?utf-8?B?ZWFCOXNVbytqaG1tNWZTbklCUVZwckdDaWxJdVQvV1lkdGYxNmduZTBHb3hK?=
 =?utf-8?B?YzIwTWZwR3BkQWIwRTJSdXduWGllcHlPQ0JjenNiVis5WTlQdFR6TFl6c2NF?=
 =?utf-8?B?RnhzOEIxUXo0ejZ5RU9oS0VCayttd2NiQzZBK2EwRVZFajM4OEFmdz09?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3141544f-df40-48d4-e697-08da136122c2
X-MS-Exchange-CrossTenant-AuthSource: SA0PR12MB4349.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Mar 2022 21:55:14.6205
 (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: DLvxvoN+TIBzBzTzfB6VFAWng04jlch4M7umFnShTKKu8dLGlwVsAqrFnCSjHPRgVI9mHDYoa/mlot5yDw53Cw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB1913
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_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 4/1/2022 1:19 AM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 3/30/22 14:22, Ashish Mhetre wrote:
> ...
>>>> If we are to remove this callback then how to handle unknown interrupt
>>>> channel error?
>>>
>>> Create a common helper function that returns ID of the raised channel or
>>> errorno if not bits are set.
>>>
>> So something like this:
>>
>> int status_to_channel(const struct tegra_mc *mc, u32 status,
>>                unsigned int *mc_channel)
>> {
>>      if ((status & mc->soc->ch_intmask) == 0)
>>          return -EINVAL;
>>
>>      *mc_channel = __ffs((status & mc->soc->ch_intmask) >>
>>                   mc->soc->status_reg_chan_shift);
>>
>>      return 0;
>> }
>>
>> Correct?
> 
> Yes
> 
>>>> 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.
>>>
>>> You may do the following:
>>>
>>> 1. find the first channel bit set in the status reg
>>> 2. handle that channel
>>> 3. clear only the handled status bit, don't clear the other bits
>>> 4. return from interrupt
>>>
>>> If there are other bits set, then interrupt handler will fire again and
>>> next channel will be handled.
>>
>> For clearing status bit after handling, we can retrieve channel bit by
>> something like this:
>>
>> ch_bit = BIT(*mc_channel) << mc->soc->status_reg_chan_shift;
>>
>> Correct?
> 
> Yes
> 
> Perhaps using FIELD_PREP() and alike helpers could make it look nice in
> the code.

I tried using FIELD_PREP() and FIELD_GET() for our use-case but
compilation is failing because these macros require the mask to be 
compile time constant and our mask "mc->soc->ch_intmask" cannot qualify
to be compile time constant.