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

[Sameer Pujar <spujar@nvidia.com>]

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&data=04%7C01%7Cspujar%40nvidia.com%7C53e278c9a4804612f74b08d9c49564a0%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637756968218491760%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=QEe8SlSdAN1N8nOu3XqtAdbXP1JbtMBPlswqnBIhq5w%3D&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;
>                  }
>          }