Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Message-ID: <9c7c794e-f6d7-4092-90f1-13d44c2111f8@amd.com>
Date:   Thu, 16 Nov 2023 12:15:52 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] soundwire: amd: add an error code check in
 amd_sdw_clock_stop_exit
Content-Language: en-US
To:     Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
        Su Hui <suhui@nfschina.com>, vkoul@kernel.org,
        yung-chuan.liao@linux.intel.com, sanyog.r.kale@intel.com,
        nathan@kernel.org, ndesaulniers@google.com, trix@redhat.com
Cc:     alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
        llvm@lists.linux.dev, kernel-janitors@vger.kernel.org,
        "Dommati, Sunil-kumar" <Sunil-kumar.Dommati@amd.com>,
        "Katragadda, Mastan" <mastan.katragadda@amd.com>
References: <20231115095407.1059704-1-suhui@nfschina.com>
 <3e461bb5-fa6b-4729-8ff5-ccc1d2887066@linux.intel.com>
 <5729c613-182b-43d2-a2ed-caff046b68e7@amd.com>
 <05147111-e46b-42a5-9e99-ec289e61730f@linux.intel.com>
From:   "Mukunda,Vijendar" <vijendar.mukunda@amd.com>
In-Reply-To: <05147111-e46b-42a5-9e99-ec289e61730f@linux.intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VGhXYlJLMlZuTzZaMndIVS9wT0tUdHlLbXFuRGpjQkdvSWRpSFR3MzJtTkdp?=
 =?utf-8?B?YVFvV0diaVVjYmVISTNYWVp0Vzdmd0VkWC9ZZDI1UFJvKy96NWR4alM5dXBS?=
 =?utf-8?B?YmFSWVVmZ09Sa0FXc1I5NkV1NkFHdC9JZFU2Rm5pSS9OaW1BSTVxelN4c2s2?=
 =?utf-8?B?WlVEWXZoWEhDK0E1RVI3Q0wvWjNBNU5mejZXejd2RkJMVWJBSFowTXY1Z3Jp?=
 =?utf-8?B?SWF0R1lmUkl3UklKOW9VMzNwRFJmS2h2NjRrM0VteGRZY0NVNFZUOUlHNy9R?=
 =?utf-8?B?cXU2cTlDSXpWMzZkaGR3WkVCYnp1SnQwY29KRTZ0QThicWEwbnN4dVhUdE5M?=
 =?utf-8?B?djdRWUc4dFZKUkl1N1BzWlZsT3FiL25Lb3JhS0J3RFNqKzYwS3BVNERzdERJ?=
 =?utf-8?B?VGJEaVNuYkZ1NHRMaEtKdHJLdnkzeDVRc1AwM0FPWHp4MEt6Vy9Pa2p0TVI5?=
 =?utf-8?B?Q1BKQWtRWmNOSHBqTklHcVQyemx1Szk1MVR6ZUVMY0lzRkM1ZXE3Z2pOTjNH?=
 =?utf-8?B?WktGdnAvdnhaWnN2VjU1a0dzWTQxSnE3aWpkWmtmcFo5UVk1ZXJaWDIreERF?=
 =?utf-8?B?b3NZMjU1RllwajVXQjdGT1k1OWFvRy9jc011VlFsUXNZREFTV1hLVitkUTVz?=
 =?utf-8?B?bGRGMTUyZEczRFhrM01wempITDNONXlKaTVRSnpWTW8vbWo5c05zOVhpN25l?=
 =?utf-8?B?RzJXOGJDcy9GRXcxUHIyZWV4V2lJVzhoTFRTRDJqS0JDVzlDUkIyODNqREtE?=
 =?utf-8?B?RUFsZTM0SXEyTEJxeFRwR1F5NVBTVWd5amV5WktWUmpRSHNyUWxhcnBIdFVy?=
 =?utf-8?B?L1ZTdG5MNnluU2NYNWNkZnk1MEI1L0cxU2x4czdWSlJzUU9RU0lzVVB3cS9s?=
 =?utf-8?B?QUdCMFZvOXphbG5KVDZJVDdkYzRhVFQvYWtDYUdQK1drWVNxQVhRd3pIeDVy?=
 =?utf-8?B?aVJRdC8vR2svQkhmeFIrMmlWNTdUYm5ycHlhLzdYajVMb3I5UUx5TVpaMnM0?=
 =?utf-8?B?bGZOVFFqRCtNY3NJMTNwRE9pUUp1Y3ZQQ3A0em1keVN5VXhyZE1ianVkN3RQ?=
 =?utf-8?B?NmFiek1EUDlxRWNUcm9ONVpkNXBqL0ttT1hpditwU2lPa1VETWhTZGlJbnJG?=
 =?utf-8?B?amZETUVSVVBFOHBnTEdjRnB5L1BJUEI5ejdmN1djRmh0dFdPUWMxMGttUlBS?=
 =?utf-8?B?eHNYbTVseXJkSnhlT3dpNjFWV2o4T0d3Vk9sTjdUZXVIVTUxRTlycUJOc2ZZ?=
 =?utf-8?B?eEM2ek1pZkx6N2J3b1h1ZWNpSnArOThud1NZd29WVGZvQkJ2am1TbmxKbDRs?=
 =?utf-8?B?N1BIQ2d2Q0U1azlLRmZEVWs5UiszYXV3VEUrdVRaMm9Dd1Y5L1E1UzRDazZV?=
 =?utf-8?B?d0xaajNHYld6d0dKVVBmYnk2ajZscVZrMWlQVFNpaGNEcmtwaTY1M28rdjlW?=
 =?utf-8?B?K1E2THRzQUlBUXdtcmFBOWNiU3ovdU5Fd1NaK2plUnBPdU1jVU1vM2k4eDJD?=
 =?utf-8?B?VnA1cWxwNUk1aHlxaUgvbzhubk9kUDRGaXJxMDAvL3ZmN0ZrM3ArckRNUEZY?=
 =?utf-8?B?alZ6aEw0MWF6WktwSXM3SG5PTWlKR2lpQ3ozTnlrb1BJU3d4aFlSVGNLUEdP?=
 =?utf-8?B?ais5WGZvbTNRbEtzWERzbjhxaS92dTlFZ0s5U3hQaklid3h3ZWRhUm1pa05l?=
 =?utf-8?B?aVJZUHhDaHh0Y2FCbkxOUzZ6V1NPMnhkblh2aWtSdXBtakVVWEVHN083UERK?=
 =?utf-8?B?UXppZWZORW5ldFU2d1dHdGV6aHZwUWdHUUJpd2VTbU5UUzhveFl5NndxMXRy?=
 =?utf-8?B?UllpcHhWeE93MTlDOFNBZCtLZ0pjb0c4RCtTekd6MkxXL3g1VmhDT3B2S3dH?=
 =?utf-8?B?WEFpZ2V5LytySTdFckt1RWlBK296V0FnRjErNGl5VlVuaVZlUXBlU3htQmhq?=
 =?utf-8?B?ME4wVldyVzBYQmQ3MEZmL2w1eXhzTDNJcWU4NHZkcCtXWStPRG9LbzJDWHQ3?=
 =?utf-8?B?WTEzZVNlSVBBa2wxVG4rdHR0aVFNWVFTNDQ3K2JsdzhUano2VWxTb2tJQ2xP?=
 =?utf-8?B?SHZlbDNLYTNrV2hnUkd1YTl3L0JLS3dLbTdrZjYwaHlwOExObHNoaUZZOFRr?=
 =?utf-8?Q?ytmfRRLfYL1hlIBaorq86ZLV9?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 39eeed03-5c58-4bcf-b893-08dbe66fb2c8
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB4123.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Nov 2023 06:46:02.0500
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 0m/JEUunvN3/pUE8xlDQSt4WwoLtql7Iq7b59pJimJ59d7XMcnt83j5YS9XdHuoR+ZqidB+LVTq6NES4XnoTOQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR12MB6263
X-Spam-Status: No, score=-1.0 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE,
	URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Wed, 15 Nov 2023 22:46:19 -0800 (PST)

On 16/11/23 01:21, Pierre-Louis Bossart wrote:
>>>> diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
>>>> index 3a99f6dcdfaf..f391b541f4b7 100644
>>>> --- a/drivers/soundwire/amd_manager.c
>>>> +++ b/drivers/soundwire/amd_manager.c
>>>> @@ -1029,6 +1029,10 @@ static int amd_sdw_clock_stop_exit(struct amd_sdw_manager *amd_manager)
>>>>  		ret = readl_poll_timeout(amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL, val,
>>>>  					 (val & AMD_SDW_CLK_RESUME_DONE), ACP_DELAY_US,
>>>>  					 AMD_SDW_TIMEOUT);
>>>> +		if (ret)
>>>> +			dev_err(amd_manager->dev, "%s: timed out: %pe\n", __func__,
>>>> +				ERR_PTR(ret));
>>> Is this really the desired behavior?
>>>
>>> This patch fixes the static analysis issue by logging the error code,
>>> but does it make sense to continue resuming here and trying to exit from
>>> the clock stop mode?
>>>
>>> At this point a bus reset might be a more relevant behavior...
>> As per earlier discussion, when we sent the initial patch series,
>> It was communicated that even clock stop sequence fails,
>> return '0' in suspend/resume callbacks that why we returned
>> status as zero.
> clock stop is for suspend and clock stop exit for resume. Different
> problems.
>
>> In this scenario, it's not continuing resume when clock stop exit
>> sequence fails. Even In Intel's case, if the clock stop sequence fails,
>> just code is exiting from that sequence.
> that's right, in the Intel SoundWire drivers we never prevent the
> pm_runtime suspend from happening, and discard any errors. In the resume
> step we do a bus reset anyways.
>
> But that's different here, this is the clock stop exit which happens on
> resume and IIRC there is no bus reset. If the resume fails, what is the
> expected behavior? If you keep going then you are going to have other
> issues down the road.
We will check and come back on clock stop exit sequence failure
adding bus reset sequence.
>>>>  		if (val & AMD_SDW_CLK_RESUME_DONE) {
>>>>  			writel(0, amd_manager->mmio + ACP_SW_CLK_RESUME_CTRL);
>>>>  			ret = sdw_bus_exit_clk_stop(&amd_manager->bus);