Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v4 5/5] soc/tegra: pmc: Rate-limit error message about
 failed to acquire of reset
To:     Thierry Reding <thierry.reding@gmail.com>
Cc:     Jonathan Hunter <jonathanh@nvidia.com>,
        Peter Geis <pgwipeout@gmail.com>,
        Matt Merhar <mattmerhar@protonmail.com>,
        linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20210302122502.20874-1-digetx@gmail.com>
 <20210302122502.20874-6-digetx@gmail.com> <YFyhR1XgFGzJkkLt@orome.fritz.box>
From:   Dmitry Osipenko <digetx@gmail.com>
Message-ID: <dc3eeba3-b319-8892-4fdb-9162f4d465ed@gmail.com>
Date:   Thu, 25 Mar 2021 17:53:39 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.8.1
MIME-Version: 1.0
In-Reply-To: <YFyhR1XgFGzJkkLt@orome.fritz.box>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

25.03.2021 17:42, Thierry Reding пишет:
> On Tue, Mar 02, 2021 at 03:25:02PM +0300, Dmitry Osipenko wrote:
>> PMC domain could be easily bombarded with the enable requests if there is
>> a problem in regards to acquiring reset control of a domain and kernel
>> log will be flooded with the error message in this case. Hence rate-limit
>> the message in order to prevent missing other important messages.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/soc/tegra/pmc.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index bf29ea22480a..84ab27d85d92 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -868,8 +868,8 @@ static int tegra_genpd_power_off(struct generic_pm_domain *domain)
>>  
>>  	err = reset_control_acquire(pg->reset);
>>  	if (err < 0) {
>> -		dev_err(dev, "failed to acquire resets for PM domain %s: %d\n",
>> -			pg->genpd.name, err);
>> +		dev_err_ratelimited(dev, "failed to acquire resets for PM domain %s: %d\n",
>> +				    pg->genpd.name, err);
> 
> That doesn't look right. This is a serious error condition that
> shouldn't happen at all. Ever. If this shows up even once we've got a
> serious bug somewhere and we need to fix it rather than "downplay" it
> by ratelimiting these errors.
> 
> What's the exact use-case where you see this?

This was firing up badly while I was wiring up power management and
GENPD support to the GR3D and VDE drivers and testing them because of
the bugs that I was creating.

Looking back again at this now, I agree that the commit message isn't
good and could be improved. What about this variant:

"Rate-limit error message about failing to acquire reset in order to
prevent missing other important messages. This was proven to be very
useful to have for development and debugging purposes of a power
management support for various Tegra drivers".