From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 45366C433FE for ; Wed, 30 Mar 2022 10:36:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S245443AbiC3KiZ (ORCPT ); Wed, 30 Mar 2022 06:38:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44978 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S245435AbiC3KiV (ORCPT ); Wed, 30 Mar 2022 06:38:21 -0400 Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2B4602BB23; Wed, 30 Mar 2022 03:36:37 -0700 (PDT) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: dmitry.osipenko) with ESMTPSA id 1464B1F41835 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1648636590; bh=yjx1kf6POnDpobtQKgZbaVsr1GNaW5K6KF98SWEVWiM=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=STXGHTZaL+HoSt7mA/RAo1TnYAHbOJ6tBgq4FSPAg8mEBjbHhc06wB6W33kfd7urQ gFg1Cqsu+u6hdBBSR1U6qOjnPacSYejBxGIYdQZL0osJPgS7lnJ5OZDdUFXwFeOuSG qmQrV0CyrS67EOCOJM4B6YZvuGYnZchS/YNwSEjzYvJ/A7wB6eNCVrLqSr1KeZmmPP VdG+modtas8Ru9tlRQSwabg0TpXkLAvn/TRy1iBWdtTCrRq0C0MKBHpsbmzVcExX6q USaswk6D+YAWBg1kEJU/EfolPlM24QHndy0aPj8eUWd4XKm6Wa+G7x34K2we/+BAUT lo8fLk1z2Hkfg== Message-ID: Date: Wed, 30 Mar 2022 13:36:27 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; 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: Ashish Mhetre , Dmitry Osipenko , 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> <4ea801f4-7929-148d-4e69-d4126a9dfbf7@collabora.com> <44235c65-160c-04c7-294d-16b13d25605c@nvidia.com> From: Dmitry Osipenko In-Reply-To: <44235c65-160c-04c7-294d-16b13d25605c@nvidia.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 3/30/22 13:16, Ashish Mhetre wrote: > > > 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? Add variable to tegra_mc_soc. The intmask_chan_shift is a misnomer, perhaps something like status_reg_chan_shift will be a better name. > 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. > 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.