From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f53.google.com (mail-pj1-f53.google.com [209.85.216.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C10792F6931 for ; Mon, 13 Apr 2026 16:17:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776097054; cv=none; b=jvoTBMsompY6MdG0qBrWuC72vTqidaOlU7BEYSENlDeZ2shxMk17hmWK2WT/3i3vlkcQri3IvkB1/+uFD5ENThB1ECGkaWpqpv4kmjGbzI6jBVQeMLR/NxvhbIcz4hQ9cvvz5qqZlyMtG6mNWtTfMTNj7JA4RXbGKQXAYsKEkvo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776097054; c=relaxed/simple; bh=/Wy8EUFyEpeEAyYGqty2YpfolfGPZl8bC76hgMD2EBc=; h=Message-ID:Date:MIME-Version:Cc:Subject:To:References:From: In-Reply-To:Content-Type; b=nAaBka7mbNgnSj7jhpZjCDTF9Wjec0Jv+dncT3EoIyjxUr8qru+9RXrcj39fh4NAU4IVeFO5GNhlXHPWgpr++1bDFl1UmqfOd1JiMntQaZtdaXWEuRl7qlzZdWRJGGE2jk3NNrjcPXWQ/uCPjNweXyqNrUKkk+dHShLq4wChI+w= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ZtYNZgEI; arc=none smtp.client-ip=209.85.216.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ZtYNZgEI" Received: by mail-pj1-f53.google.com with SMTP id 98e67ed59e1d1-3538952a464so651756a91.2 for ; Mon, 13 Apr 2026 09:17:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1776097052; x=1776701852; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:references:to:subject:cc :user-agent:mime-version:date:message-id:from:to:cc:subject:date :message-id:reply-to; bh=KjcJ7ExbmXQll1fJ8sfhOKaomR2S+dBnysGQ5TsI/p4=; b=ZtYNZgEIfbLWj/Z6HZH4tes2qv+QZynMjYOKMEBxUCy7BeOc0i0hB84oQEV8g9R0wx AgEk/8I5Nd7l5IGtsOBg5Sqq2jtFz+rkue+zEdeDfGEz5OMpOdVPfUQgzpGKsaz9NPBT pey2ABuJHh2cMuI9hQOLn9rgNHOdtpwWWYgO5E1sLzz9M4K347dVefSvnT0qMpI25djt hANmr9xEyybVyj8lSwAHpNOlJibYawiHKmMecmE4R63v2B6z6Pe+FsIS0QMPBVvegs7r rXHpv/bvNmgKz/MaKKCF21iOD68m5Xrv/01uT5OAPY4srGB3wDBKl0Sm0hjDPiLF+cd6 O2Rg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776097052; x=1776701852; h=content-transfer-encoding:in-reply-to:from:references:to:subject:cc :user-agent:mime-version:date:message-id:x-gm-gg:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=KjcJ7ExbmXQll1fJ8sfhOKaomR2S+dBnysGQ5TsI/p4=; b=Ey+8578YxhxcQcp6jS/CEh3wyOSe4z1GLZw4s8Kov89EYOLXFzWoP18eooWeDidJ3X 5X0bkhl7JDZNhznGrXSNJS0WF7y9pAUXaYuJzi4GpnUHv8eU4ebF5uCfAtlvMCaQ9GHQ XDN4OKnYxDsLflxa/ejM9NtjlJXg2OifuTli0hiHipr+t4YgeVeCyJBPzNmX78K3emYl RQ9mpF9QXXiMbwn1nPqj1Vi0qye60+W+wwzVw5boMtkY9FjgOOjcSh2qrvvtzYYCbwXV rYjDc+bmK95YPjyAjzROAIjhAS3KCYLvZFiLpYDFsuV+ud+gJIce+AZdbqlrfhP03Ruf YYww== X-Forwarded-Encrypted: i=1; AFNElJ9RLDTRqMXVYTG2eGBtvj9ftipX0GfaOqUE50BFZ2WxKNy1WMmneyjj9Oh7+k4ev4jBzADmnZTPPpkZCsM=@vger.kernel.org X-Gm-Message-State: AOJu0Yx9KKl6c6Scn+1q1xEgwUe8sepLbWzHq3sRfPqLzGYxOfekPEXG ohQW0GYK3TheyHEDReixR7Jh/Gjxk2vY6ZPKTuuizovVwbEeORqg7mAm X-Gm-Gg: AeBDieu7JUIEMfmUCDCoe2KxO0LXFrtmBIuJTRhoyqE/+TdG8Q73rGKZPfxIvbUcK1i ihWbAHZ+d2bNkA05hlwciQRvIyBZZZv4ijlNTPX8iE2euXmvEE3JR0d0bFKArKD5bOFFoKWib5I ehINXkZLgkjgfI9nNC2zHAk3BiR99DnpmMt/C6t4AXx/xl+MTvTDfyt4Mjr+4WPcmEP01mcA9dB Ot+Y0Qopk8cAdhIbGqvNtgIbkhM2qhgHUVVEUM2UdAlkeny92VpW38Z34mDro0psJzAUCp/7eh5 ixj4eEoQYF9pDhPd7Jy/PumxR39On1oB5EgeFA005Q3MamM4cfp2T1MewN/jjuMMg9MgLqTtFku E8C3i7PYuw/HUvHx7cazyDWFIoUNbgdZHZL9ljmUnCoarEwlBKWr869nqHPOX8EHNfoBA1ZG3d4 ub7l75fRxhe7VKrkUK4F4m9sZ407bbOitcIjsZJ+7X3viThtjxEc0= X-Received: by 2002:a17:90b:35cf:b0:35e:576c:7c1e with SMTP id 98e67ed59e1d1-35e576c7d9cmr4245897a91.4.1776097051938; Mon, 13 Apr 2026 09:17:31 -0700 (PDT) Received: from [192.168.0.100] ([163.125.228.136]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-35e4131d3f2sm12560433a91.12.2026.04.13.09.17.26 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 13 Apr 2026 09:17:31 -0700 (PDT) Message-ID: <86770cef-bf30-45f6-8d33-e7b74b3bb834@gmail.com> Date: Tue, 14 Apr 2026 00:17:19 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Cc: yangyccccc@gmail.com, "jonathan.cameron@huawei.com" , "alexander.shishkin@linux.intel.com" , Sanman Pradhan , "stable@vger.kernel.org" , shenyang39@huawei.com, prime.zeng@hisilicon.com Subject: Re: [PATCH 1/2] hwtracing: hisi_ptt: Propagate DMA reset timeout in trace_start() To: "Pradhan, Sanman" , "linux-kernel@vger.kernel.org" References: <20260409010704.383882-1-sanman.pradhan@hpe.com> <20260409010704.383882-2-sanman.pradhan@hpe.com> From: Yicong Yang In-Reply-To: <20260409010704.383882-2-sanman.pradhan@hpe.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 2026/4/9 09:07, Pradhan, Sanman wrote: > From: Sanman Pradhan > > hisi_ptt_wait_dma_reset_done() discards the return value of > readl_poll_timeout_atomic(). If the DMA engine does not complete its > reset within the timeout, hisi_ptt_trace_start() proceeds to start > tracing regardless. > > Return the poll result from hisi_ptt_wait_dma_reset_done() and > propagate it from hisi_ptt_trace_start(). Deassert the reset bit > before returning on timeout, preserving the existing reset cleanup > sequence. Move ctrl->started to the successful path so a failed start > does not leave the trace marked as active. > > Fixes: ff0de066b463 ("hwtracing: hisi_ptt: Add trace function support for HiSilicon PCIe Tune and Trace device") > Cc: stable@vger.kernel.org > Signed-off-by: Sanman Pradhan > --- > drivers/hwtracing/ptt/hisi_ptt.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c > index 94c371c491357..73b93df8504c4 100644 > --- a/drivers/hwtracing/ptt/hisi_ptt.c > +++ b/drivers/hwtracing/ptt/hisi_ptt.c > @@ -171,13 +171,13 @@ static bool hisi_ptt_wait_trace_hw_idle(struct hisi_ptt *hisi_ptt) > HISI_PTT_WAIT_TRACE_TIMEOUT_US); > } > > -static void hisi_ptt_wait_dma_reset_done(struct hisi_ptt *hisi_ptt) > +static int hisi_ptt_wait_dma_reset_done(struct hisi_ptt *hisi_ptt) the other status wait functions in this driver return a boolean, it's better to keep consistence. > { > u32 val; > > - readl_poll_timeout_atomic(hisi_ptt->iobase + HISI_PTT_TRACE_WR_STS, > - val, !val, HISI_PTT_RESET_POLL_INTERVAL_US, > - HISI_PTT_RESET_TIMEOUT_US); > + return readl_poll_timeout_atomic(hisi_ptt->iobase + HISI_PTT_TRACE_WR_STS, > + val, !val, HISI_PTT_RESET_POLL_INTERVAL_US, > + HISI_PTT_RESET_TIMEOUT_US); > } > > static void hisi_ptt_trace_end(struct hisi_ptt *hisi_ptt) > @@ -194,6 +194,7 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt) > { > struct hisi_ptt_trace_ctrl *ctrl = &hisi_ptt->trace_ctrl; > u32 val; > + int ret; > int i; > > /* Check device idle before start trace */ > @@ -202,19 +203,21 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt) > return -EBUSY; > } > > - ctrl->started = true; > - > /* Reset the DMA before start tracing */ > val = readl(hisi_ptt->iobase + HISI_PTT_TRACE_CTRL); > val |= HISI_PTT_TRACE_CTRL_RST; > writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL); > > - hisi_ptt_wait_dma_reset_done(hisi_ptt); > + ret = hisi_ptt_wait_dma_reset_done(hisi_ptt); > > + /* De-assert reset regardless of whether the wait timed out */ > val = readl(hisi_ptt->iobase + HISI_PTT_TRACE_CTRL); > val &= ~HISI_PTT_TRACE_CTRL_RST; > writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL); > > + if (ret) > + return ret; > + could add some error log here for better debug. otherwise looks good to me. the timeout wasn't checked since the hardware reset will be finished in the limited time normally, which is less than the HISI_PTT_RESET_TIMEOUT_US. It'll be better to add this check in case there's something wrong with the device. Thanks. > /* Reset the index of current buffer */ > hisi_ptt->trace_ctrl.buf_index = 0; > > @@ -234,6 +237,8 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt) > if (!hisi_ptt->trace_ctrl.is_port) > val |= HISI_PTT_TRACE_CTRL_FILTER_MODE; > > + ctrl->started = true; > + > /* Start the Trace */ > val |= HISI_PTT_TRACE_CTRL_EN; > writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);