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 <[email protected]>
---
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);
return err;
}
--
2.29.2
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 <[email protected]>
> ---
> 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?
Thierry
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 <[email protected]>
>> ---
>> 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".