linux-kernel - Re: [PATCH v2 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <2d5c2e5c-f7a1-ede5-ba29-eaccb2b886d3@nvidia.com>
Date:   Tue, 21 Dec 2021 21:33:00 +0530
From:   Sameer Pujar <spujar@...dia.com>
To:     Dmitry Osipenko <digetx@...il.com>, tiwai@...e.com,
        broonie@...nel.org, lgirdwood@...il.com, robh+dt@...nel.org,
        thierry.reding@...il.com, perex@...ex.cz
Cc:     jonathanh@...dia.com, mkumard@...dia.com,
        alsa-devel@...a-project.org, devicetree@...r.kernel.org,
        linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org
Subject: Re: [PATCH v2 1/3] ALSA: hda/tegra: Fix Tegra194 HDA reset failure



On 12/21/2021 8:50 PM, Dmitry Osipenko wrote:
> 21.12.2021 09:18, Sameer Pujar пишет:
>>
>> On 12/21/2021 6:51 AM, Dmitry Osipenko wrote:
>>> All stable kernels affected by this problem that don't support the bulk
>>> reset API are EOL now. Please use bulk reset API like I suggested in the
>>> comment to v1, it will allow us to have a cleaner and nicer code.
>> Agree that it would be compact and cleaner, but any specific reset
>> failure in the group won't be obvious in the logs. In this case it
>> failed silently. If compactness is preferred, then may be I can keep an
>> error print at group level so that we see some failure context whenever
>> it happens.
> The group shouldn't fail ever unless device-tree is wrong. Why do you
> think we should care about the case which realistically won't ever
> happen? This is a bit unpractical approach.

Though it is very rare that something like this would happen, but can't 
be ruled out completely.

> If we really care about those error messages, then will be much more
> reasonable to add them to the reset core, like clk core does it [1],
> IMO. This will be a trivial change. Will you be happy with this variant?

It would be nicer to know why exactly it failed. Yes, it makes sense to 
have this in the core. I will send v3 with bulk APIs for HDA driver. 
Thank you.

>
> [1]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv5.16-rc6%2Fsource%2Fdrivers%2Fclk%2Fclk-bulk.c%23L100&amp;data=04%7C01%7Cspujar%40nvidia.com%7C53e278c9a4804612f74b08d9c49564a0%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637756968218491760%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=QEe8SlSdAN1N8nOu3XqtAdbXP1JbtMBPlswqnBIhq5w%3D&amp;reserved=0
>
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 61e688882643..85ce0d6eeb34 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -962,6 +962,11 @@ int __reset_control_bulk_get(struct device *dev,
> int num_rstcs,
>                                                      shared, optional, acquired);
>                  if (IS_ERR(rstcs[i].rstc)) {
>                          ret = PTR_ERR(rstcs[i].rstc);
> +
> +                       if (ret != -EPROBE_DEFER)
> +                               dev_err(dev, "Failed to get reset '%s': %d\n",
> +                                       rstcs[i].id, ret);
> +
>                          goto err;
>                  }
>          }