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: <5729c613-182b-43d2-a2ed-caff046b68e7@amd.com>
Date:   Wed, 15 Nov 2023 22:57:46 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] soundwire: amd: add an error code check in
 amd_sdw_clock_stop_exit
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
References: <20231115095407.1059704-1-suhui@nfschina.com>
 <3e461bb5-fa6b-4729-8ff5-ccc1d2887066@linux.intel.com>
Content-Language: en-US
From:   "Mukunda,Vijendar" <vijendar.mukunda@amd.com>
In-Reply-To: <3e461bb5-fa6b-4729-8ff5-ccc1d2887066@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?ak5PNm5WYXZ5YXdGb3JZUE85ODE5b3g3MjNjeC9MbVBPVWd3dXk3dFZ5Wmhj?=
 =?utf-8?B?MkV5L0lxeUlSY3N4UTFueVoyd0RFQS91QnJzK2JZZTJ0ODVOT2lKOXJGK0NJ?=
 =?utf-8?B?aG9LRDdQcGFxYmtxeGFjZ2hNb2NPWHVtUVF3aXdqdWY0UXliMHhaQVhrMUQ2?=
 =?utf-8?B?ZmRUT2tQcE1PdThmMTZHS2xDUzlFbUt0elhWSkdCQnl0U2taSkRnRHFuK3Ax?=
 =?utf-8?B?V2lnUDJPc2R6NXJCeWxTVlhJdlp3WDFuRzlTcXc2QXlIK1Z0ajNGc0VqOXlK?=
 =?utf-8?B?OThaVlJqYTgzZFNPVEZUTUc1TG0xeFl2NHNMdHhEelYyNVBDeFh5d0tuellU?=
 =?utf-8?B?Vzhjc2lJcTVOMW13NWlRMlhMVXQwU0FXZWFWMXdEMU1VeE9raVFHbFFvWmxB?=
 =?utf-8?B?M2JhSzVjUFVWUUVzYmkzRWJiYU54UUUxZXhzZHNIR1J4dFhDOU1GM1BnSXBs?=
 =?utf-8?B?eDRpV241dEpxUGdTOXAyQTk5RVBlN1JXT0hmcHVzejgwMHNPblp0Z1lXaERu?=
 =?utf-8?B?UUNQc0xlenpheXpJdmIxWFJ5RVE2ODRzdmpDeW13TFduYlB6VzNpKytrVzFv?=
 =?utf-8?B?SEhodTR6cXZFRlhodUMyenY3QjVRYmRKdmJ3dEtkdWlYb0ZUQS9hN3FSVTRn?=
 =?utf-8?B?Q3Q3ZWhPK1dwV1c2eUlhdnF1VlNjLzRNamFnMjNhNWRYdk5wQ1FEdXNnSVo2?=
 =?utf-8?B?KzRGdXA0akM2aG5iekdYYlhjM21NMVhreUl0WXovNktYbGlnUTlYTnhLeWd3?=
 =?utf-8?B?aUQzNmhmU2VKQVJ6bHYraDRmZXhEZ1VTMXAwYUZXYjFSWW9SUHQwR1YwdnF0?=
 =?utf-8?B?eDNDWWtld21LQ25XdnZiRTNnano3YUhBMm94SUQ5VktLWWk1ckZuZFZlaUN4?=
 =?utf-8?B?M1RFdS92K0FER1kwS2xBRlpSdTVDbElPNFJZOXFXWHovZ0dzSnExY3J3bnZB?=
 =?utf-8?B?WmJkTzMrYkpwaGFtZ3Y4THRmdyt3Tm4zOWFpaDdOSDVmVlVzbldTV3ZmZmJQ?=
 =?utf-8?B?K3RCRmRBS0svbmxRaG0yemNJWEJ2b0h2bDJLWU00Q1lRNlFDKzRUUUFnQjRH?=
 =?utf-8?B?TVJSZll4aEpNMUNpaUhnS0JwbURnSEZacVN0d2lLZEFzeGlxak5Scm1wd3Fz?=
 =?utf-8?B?TklESDJMNkhNVkRWdkpPMlVPNWVLTnJMTmdVSE9BSHNqbUlnR0NZODVPSkw3?=
 =?utf-8?B?TGtENU1WeFV6NEZ1ZEUyU3g5Wm1OZEM4d1FvSlB0a3dSYkErM0lKaS82QjlV?=
 =?utf-8?B?RnVWWW11Q0tNVlpaV3p5YlZNOXVOUTArYitWd0srR0s0eHVHWjlVM0VrWWxa?=
 =?utf-8?B?YVdqU1pXamVVUXNaYU9HaTFKakJzbEtLeXM2SjRRaVdXdjNpcUJuOERkeEF5?=
 =?utf-8?B?TklraW1lVUFDQm4vL0xOQnZDWVNDS0JSS3BNWmpid09EOXF6VElqQk1Jbzhv?=
 =?utf-8?B?NzJwSnhKOTlTUUpLYVBFb0tYbVhXeTk1T2Y3RGlaZ2FjVjQxMVdmOGdiSkli?=
 =?utf-8?B?NXEwamx4b2xaZUVQR2YzRjlHUzNJT1plcXNXdjQyZlBnSWFCamw0SUFkL0Za?=
 =?utf-8?B?MWRjRlhaRG9VdmpEclV1czJ6R2RacUVkaVhkVE1QcnA4U2tFY1ZvdUlKVUZ2?=
 =?utf-8?B?Y05UcUIrbGIvSTVXalkzYkJxVEJsMWhQZit4dklOa3J5b1JYTGJ5VDQ3K0Q2?=
 =?utf-8?B?bFdkTE5hQ3hpM2dzaG1kSWNyUzkvaCtLU1lnNllMSWZ6S20xbDI2STIxUVIx?=
 =?utf-8?B?elJ3VURKSDZsOE9QRVdDZXo3Qi92c1h0RTNpY3JTak9DRjJaUWVEcVVQR2V0?=
 =?utf-8?B?MGJDbC90cnh0THJqSW1rRFk2MklscVVCa1c4a3BEazZ6WFYrWVZUZkdkaWdp?=
 =?utf-8?B?Lzg2elY2NjJ0RUNZOUtsaXJpS0VpWjNoN3g5elNQYmlrUlNXNUs4TnNNeXZB?=
 =?utf-8?B?Y256WmxDZVJ0NjBtdGx3SVJBZ0loVWdmK0FPdjE0Rko1UVBGVVdYWVVQd3JR?=
 =?utf-8?B?d1Q3MWlqSHlYY0hBbncwQmc4S1hzbnNBS3hWanhPbmpwalo5bDlSd1pPTE1o?=
 =?utf-8?B?QmFaZVpaa1NNaGQ3QUVQMFdkWVkyVVNOdC80WjJ0bGZrWU50b01VQm52dlBz?=
 =?utf-8?Q?gDpkb9FlojE2nt5YsDZaJMvO/?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bd11c071-bc58-4898-2fbe-08dbe60034b3
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB4123.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Nov 2023 17:27:57.3566
 (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: D46yoj4lJ39eDpcBrFyoDQxHooj8sZV/htvRvzpA0oCZZbjPH7u0M0U3lhWpQyV2gJz8LlTIBxE6adH3R5QLpw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB9204
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
	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 09:28:11 -0800 (PST)

On 15/11/23 20:14, Pierre-Louis Bossart wrote:
>
> On 11/15/23 03:54, Su Hui wrote:
>> Clang static analyzer complains that value stored to 'ret' is never read.
>> Add an error code check and print an error message if 'readl_poll_timeout'
>> failed.
>>
>> Signed-off-by: Su Hui <suhui@nfschina.com>
>> ---
>>  drivers/soundwire/amd_manager.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> 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.
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.
>
>>  		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);