From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f181.google.com (mail-pf1-f181.google.com [209.85.210.181]) (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 581BB1A9FB3 for ; Tue, 14 Oct 2025 08:11:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.181 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1760429500; cv=none; b=Kleb31p/kulJSHgoZGPN00COdqHKsW+XfS3ne8TM1lCtHsRxVpuqTbbal/B09OHF2MdkUqwKKFMKBZFFCtHBYaye4l10GSSYC4FeU7jcp155D8SyUL7gaJ2tRm4ccU7/Jzwr2ij8H1Gnzs6k/KdfQlysjjUbR19iS9Lb1hAzkYI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1760429500; c=relaxed/simple; bh=jRf7YqINEV3bz3VzIw9FGruew0rkW3g2hYiZe9BkV/s=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=YQY1Flu8hyy7UOdI7JPmutxufQswyzHF13rIKGYOv7or5H2HG16ybjcE6IK4q6HYO3aWsP5i2NHlWLGPryMfzxxBTCeeR+/PXSm5ACxA1ofrZmYiNCnV6eB7JUismu7mJX+7R0YnAMjWJETPyhL8TnFQWe1991Jrksq3SRI3yus= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ventanamicro.com; spf=pass smtp.mailfrom=ventanamicro.com; dkim=pass (2048-bit key) header.d=ventanamicro.com header.i=@ventanamicro.com header.b=db9Xkwgg; arc=none smtp.client-ip=209.85.210.181 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ventanamicro.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ventanamicro.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=ventanamicro.com header.i=@ventanamicro.com header.b="db9Xkwgg" Received: by mail-pf1-f181.google.com with SMTP id d2e1a72fcca58-781001e3846so4948143b3a.2 for ; Tue, 14 Oct 2025 01:11:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ventanamicro.com; s=google; t=1760429497; x=1761034297; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=Fjoc325jjBewUa+PMCJv73wF7zKdUoHfwyGW6ASR27s=; b=db9XkwggaIFq52fj5VBy8nLGfDS+HI+EDAYvf607o/yPHKB89UxJIerJkqOEJNabu+ n4ohwJlbREOEAs/4iH12HMCqS8f8BJhnjFwUtQyqYqXRbI6Pv8Jx69G2J6nIpADE+hBc dMZSme5f+jEQTwQbW6LYGqIKguWDBex1xd/lFK9UhZQ/4gpPSsArsRyWROiklwk+S6GH oRn2qszcW4nYpcDer9jpX5Qhka/SZSLrIgmsu7vs94TR/C/3j7isSrQRWnkHxQnfMaw9 I7SfoWtJjVbG+N8YP0yBVYa4KlKLQBCDoRdqDL+QLQ0ApgVEmbTs53rp50LbqQzohHJ2 tS5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1760429497; x=1761034297; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Fjoc325jjBewUa+PMCJv73wF7zKdUoHfwyGW6ASR27s=; b=dldYZpp1+8NPgiXz+QD8FMUXZtdpP4opKS3jNbsnfeAX6Pk83bCc8OS7464TYVTCOV DCPJ+m5i6jCrbCRQmeJnpUPjK3ZDI7oKg6aiz5KBddwnWZYwRazWqyTdQc2T89pgQK6O I9wnvdr3pakC+LbAHzk8q3/yCU1MxnrpVHv6MD4Eq7PrN9940i8aRcGbelegQ+ZfB9PI ovmbC6GluSt3bLaYTSWd6gZuV5Cp5hWa1t4csWOGjszaxT49byXTX5ZoNY9mAXr/Cb39 3DfYl0BhAK/hzN5sJnzIIrEjwn5P07l9f9YqzXoepqboNfy3XiUNykCv3TfyOKhAT6lP BzdQ== X-Forwarded-Encrypted: i=1; AJvYcCUfoNfgEd6Ith506NHvIYgHgv+kTsdpNMjh3YfD1yF82x7VWvPdm3LKA6ET+KGLmODm4TaYBV6+VOvY@vger.kernel.org X-Gm-Message-State: AOJu0Yx1g/wMIebQF42FotvgaNwA/J0pqBVTIGHqDqYP2rfPkc3jVTeQ iyf6U4LmUoj7IBFkUYpYbtmYEZdMrcahMlzOUwjt+dqnvegngZssoYgkTGB0j1oe9Q4amEPTR3k Cr/eMojldvZGgmZhxOcQcnkIk9bFL7n1nAfPjKIF26w== X-Gm-Gg: ASbGncuZtQQ3Dw/ptDw5EvaDFa1Z4m6NWQz1fgqvUQ0XzFZCjh1MhQSl5Iqw/pnzWjJ nHNDkCWwXRlnIUaYYeG18PvqaSsTm4C/w/GnUbH740diJh2Le0uG70Kis9MDO7l2osyPrtHwJfd /fl7VkfOPFOzk4z0Q8OWhKa/OCQsUDo02vEMCqL6zafKKW5LzsTfbBMqJck6scd23dTVbbBXmtD 3E8ZS0tFBrzaCvKNQhtRTnMv1A3biZjwD+vCkSmB5tlR6EnmZwsaajDkQw1DyKP1vNh3xTIf2Du GIjGljFHzcikeBfTbA== X-Google-Smtp-Source: AGHT+IGtEx/vxjLQYcfILlv2zX9NsM7oAL80JOcH+vHwVkTXyNROQzVhxfWjHb2rsJrrKGXRKWkc/te+HJHHsQEbDto= X-Received: by 2002:a17:903:37c3:b0:269:a4ed:13c9 with SMTP id d9443c01a7336-290273ee214mr299435315ad.30.1760429497506; Tue, 14 Oct 2025 01:11:37 -0700 (PDT) Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20251002060732.100213-1-apatel@ventanamicro.com> <20251002060732.100213-5-apatel@ventanamicro.com> <20edc3a9-2efe-4431-b198-f00b3940777c@gmail.com> <69205060-6a47-4140-8026-6e5a50ad1512@gmail.com> In-Reply-To: <69205060-6a47-4140-8026-6e5a50ad1512@gmail.com> From: Mayuresh Chitale Date: Tue, 14 Oct 2025 13:40:59 +0530 X-Gm-Features: AS18NWDB2JU-ih8bTEc-UVtcOhI_T3P9jU2EM9UEyVjJe9OcrAhVJmGT69VbA0s Message-ID: Subject: Re: [PATCH 04/11] rvtrace: Add functions to start/stop tracing on a component path To: Bo Gan Cc: Anup Patel , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Paul Walmsley , Palmer Dabbelt , Greg KH , Alexander Shishkin , Ian Rogers , Alexandre Ghiti , Peter Zijlstra , Ingo Molnar , Namhyung Kim , Mark Rutland , Jiri Olsa , Adrian Hunter , Liang Kan , Mayuresh Chitale , Anup Patel , Atish Patra , Andrew Jones , Sunil V L , linux-riscv@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Bo, On Mon, Oct 13, 2025 at 10:23=E2=80=AFAM Bo Gan wrote: > > On 10/12/25 20:43, Anup Patel wrote: > > On Wed, Oct 8, 2025 at 2:45=E2=80=AFPM Bo Gan wrot= e: > >> > >> On 10/1/25 23:07, Anup Patel wrote: > >>> From: Mayuresh Chitale > >>> > >>> The perf driver framework needs to be able to start / stop all compon= ents > >>> in a trace component path during its operation. Add rvtrace_path_star= t() > >>> and rvtrace_path_stop() functions for this purpose. > >>> > >>> Co-developed-by: Anup Patel > >>> Signed-off-by: Anup Patel > >>> Signed-off-by: Mayuresh Chitale > >>> --- > >>> drivers/hwtracing/rvtrace/rvtrace-core.c | 44 ++++++++++++++++++++= ++++ > >>> include/linux/rvtrace.h | 6 ++++ > >>> 2 files changed, 50 insertions(+) > >>> > >>> diff --git a/drivers/hwtracing/rvtrace/rvtrace-core.c b/drivers/hwtra= cing/rvtrace/rvtrace-core.c > >>> index 7013d50ca569..109be40d4b24 100644 > >>> --- a/drivers/hwtracing/rvtrace/rvtrace-core.c > >>> +++ b/drivers/hwtracing/rvtrace/rvtrace-core.c > >>> @@ -614,6 +614,50 @@ static void rvtrace_release_path_nodes(struct rv= trace_path *path) > >>> } > >>> } > >>> > >>> +int rvtrace_path_start(struct rvtrace_path *path) > >>> +{ > >>> + const struct rvtrace_driver *rtdrv; > >>> + struct rvtrace_component *comp; > >>> + struct rvtrace_path_node *node; > >>> + int ret; > >>> + > >>> + list_for_each_entry(node, &path->comp_list, head) { > >>> + comp =3D node->comp; > >>> + rtdrv =3D to_rvtrace_driver(comp->dev.driver); > >>> + if (!rtdrv->start) > >>> + continue; > >>> + > >>> + ret =3D rtdrv->start(comp); > >>> + if (ret) > >>> + return ret; > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> +EXPORT_SYMBOL_GPL(rvtrace_path_start); > >>> + > >>> +int rvtrace_path_stop(struct rvtrace_path *path) > >>> +{ > >>> + const struct rvtrace_driver *rtdrv; > >>> + struct rvtrace_component *comp; > >>> + struct rvtrace_path_node *node; > >>> + int ret; > >>> + > >>> + list_for_each_entry(node, &path->comp_list, head) { > >>> + comp =3D node->comp; > >>> + rtdrv =3D to_rvtrace_driver(comp->dev.driver); > >>> + if (!rtdrv->stop) > >>> + continue; > >>> + > >>> + ret =3D rtdrv->stop(comp); > >>> + if (ret) > >>> + return ret; > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> +EXPORT_SYMBOL_GPL(rvtrace_path_stop); > >>> + > >>> struct rvtrace_path *rvtrace_create_path(struct rvtrace_component = *source, > >>> struct rvtrace_component *sin= k, > >>> enum rvtrace_component_mode m= ode) > >>> diff --git a/include/linux/rvtrace.h b/include/linux/rvtrace.h > >>> index f2174f463a69..e7bd335d388f 100644 > >>> --- a/include/linux/rvtrace.h > >>> +++ b/include/linux/rvtrace.h > >>> @@ -273,10 +273,14 @@ struct rvtrace_path *rvtrace_create_path(struct= rvtrace_component *source, > >>> struct rvtrace_component *sin= k, > >>> enum rvtrace_component_mode m= ode); > >>> void rvtrace_destroy_path(struct rvtrace_path *path); > >>> +int rvtrace_path_start(struct rvtrace_path *path); > >>> +int rvtrace_path_stop(struct rvtrace_path *path); > >>> > >>> /** > >>> * struct rvtrace_driver - Representation of a RISC-V trace driver > >>> * id_table: Table to match components handled by the driver > >>> + * start: Callback to start tracing > >>> + * stop: Callback to stop tracing > >>> * probe: Driver probe() function > >>> * remove: Driver remove() function > >>> * get_trace_id: Get/allocate a trace ID > >>> @@ -285,6 +289,8 @@ void rvtrace_destroy_path(struct rvtrace_path *pa= th); > >>> */ > >>> struct rvtrace_driver { > >>> const struct rvtrace_component_id *id_table; > >>> + int (*start)(struct rvtrace_component *comp= ); > >>> + int (*stop)(struct rvtrace_component *comp)= ; > >>> int (*probe)(struct rvtrace_component *com= p); > >>> void (*remove)(struct rvtrace_component *co= mp); > >>> int (*get_trace_id)(struct rvtrace_compone= nt *comp, > >> > >> I'd suggest add another function (*quiesce) or something like that. Tr= ace > >> components have a tr??Empty bit that indicates trace has been all flus= hed > >> out. Also along the path when you do rvtrace_path_stop, you need to en= sure > >> the source has stopped and quiescent before beginning to stop the sink= . > >> Otherwise you'll get partial or corrupted trace. In essence, follow Co= ntrol > >> Interface Spec 11.3 Enabling and Disabling. FYI: my userspace driver: > >> https://github.com/ganboing/riscv-trace-umd/blob/master/rvtrace/funnel= .py#L223 > > > > It's better to add functions on a need basis rather than adding > > it now without any potential user. > > > > Regards, > > Anu > > Hi Anup, my previous comment also applies to your current use case where = you > have encoder->RAM sink directly connected together. Having a longer path, > e.g., funnels in between makes it worse. The driver needs to poll the emp= ty > bit tr??Empty (bit 3) of the control register to check if trace has been > completely flushed. Otherwise, you get a partial trace, possibly with las= t > few messages missing or truncated. So, yes, there's really a need to do s= o. I think this can also be implemented in the component's 'stop' callback. > > Bo