All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cristian Marussi <cristian.marussi@arm.com>
To: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Cc: Cristian Marussi <cristian.marussi@arm.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, arm-scmi@vger.kernel.org,
	linux-doc@vger.kernel.org, sudeep.holla@kernel.org,
	james.quinlan@broadcom.com, f.fainelli@gmail.com,
	vincent.guittot@linaro.org, etienne.carriere@st.com,
	peng.fan@oss.nxp.com, michal.simek@amd.com, d-gole@ti.com,
	elif.topuz@arm.com, lukasz.luba@arm.com, philip.radford@arm.com,
	david@kernel.org, souvik.chakravarty@arm.com, leitao@kernel.org,
	kas@kernel.org, puranjay@kernel.org, usama.arif@linux.dev,
	kernel-team@meta.com
Subject: Re: [PATCH v15 00/35] Introduce SCMI Telemetry support
Date: Wed, 11 Nov 2026 10:15:16 +0000	[thread overview]
Message-ID: <avRANKPwp-r-QSR4@pluto> (raw)
In-Reply-To: <20261110163809.000070d6@oss.qualcomm.com>

On Tue, Nov 10, 2026 at 04:38:09PM -0800, Jonathan Cameron wrote:
> > The upcoming SCMI v4.0 specification [0] introduces a new SCMI protocol
> > dedicated to System Telemetry.
> > 
> 
> Hi Cristian,

Hi Jonathan,

thanks for your time...I sincerely hope you're travelling abroad on some
remote timezone given the time these reviews came in :D

> 
> So, the main issue here is pretty much summarized by:
> 
>  28 files changed, 12801 insertions(+), 99 deletions(-)
> (obviously there are tests etc, but it's still > 10k lines)
>

Usually I try to implement the complete set of features of a new protocol
but indeed this new Telemetry thing is huge by itself so it became quickly
overwhelming to review...imagine to code and test it.

> There are relatively few reviewers who would even consider taking that on.
> I foolishly thought it wouldn't be too bad (and I review a lot of code).
> By about 3/4 of the way though, probably much earlier, I know the quality
> of review went down considerably. Honestly it was probably a waste of
> time continuing but I'm stubborn like that.
> 
> So, unless you have a path to bribing or sweet talking various folk to
> deal with reviewing this with each having to set aside multiple days
> to do it, likely repeating for a couple of versions yet, I think you've
> got to figure out out how to make the first patch set more manageable.
> Probably don't split it up as much as the mpam or CCA folk have had to,
> but you have a lot of colleagues who are experiencing the same problem
> at the moment. They may be able to advise.
> 
> So it would be nice if what was in that first patch set was 'functional'
> but I don't think it needs to function on every systems or support more
> than an absolutely minimal feature set.  Just fail to probe when you see
> something that isn't supported by that minimal set if that means
> the thing won't be functional.
> 
> Once that's in place, over a couple of additional patch sets you can
> get to everything you have here.
> 
> So what if you choose one data path (shmti perhaps), no notifications,
> events, maybe even simplify to one of the readback paths.
> Maybe even drop event groups initially. 

Based on our real world initial customer of this I can certainly drop a
few features that are not supported anyway by the real FW and move it to
a later patchset, stuff like groups, notifications and single sample
reads, but all of the rest is hardly removable while keeping the system
functional and usable.

> Possibly even drop timestamps.  Definitely drop things like boot-on
> support and debugfs (unless you are using that as a cheeky 'functional'
> test ahead of the main userspace interface.

Not sure what you mean by drop timestamps, definitelly cannot drop
boot-on (it is how systems are configured indeed) neither debugfs used
for bring-up and FW debug.

> 
> How much would that slim things down?  It doesn't even have to be
> useful, just testable.  Realistically I think the hope of getting
> enough review doubles if you can halve the size of what you are asking
> people to look at.

I am not sure how many lines I can shed with the above features drops but
I'll give it a go anyway at slimming down the series after I have gone
through this round of Sashiko pains.

> 
> Anyhow, gut feeling only but the code felt like it was in a much
> better state than some of the versions I've looked at in the past,
> so I think the game is now how to get it over the line!
> 

From the testing/functional/performance standpoints the series has
definitely improved especially, but still some road to go.

Thanks,
Cristian

  parent reply	other threads:[~2026-11-11 10:15 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-09  9:11 [PATCH v15 00/35] Introduce SCMI Telemetry support Cristian Marussi
2026-11-09  9:11 ` [PATCH v15 01/35] firmware: arm_scmi: Harden iterators state parsing Cristian Marussi
2026-11-09 21:57   ` Jonathan Cameron
2026-11-10  8:19     ` Cristian Marussi
2026-11-09  9:11 ` [PATCH v15 02/35] firmware: arm_scmi: Add new SCMIv4.0 error codes definitions Cristian Marussi
2026-11-09  9:11 ` [PATCH v15 03/35] firmware: arm_scmi: Introduce a dedicated SCMI Errors header Cristian Marussi
2026-11-09 22:01   ` Jonathan Cameron
2026-11-10  8:23     ` Cristian Marussi
2026-11-09  9:11 ` [PATCH v15 04/35] firmware: arm_scmi: Allow registration of unknown-size events/reports Cristian Marussi
2026-11-09 22:07   ` Jonathan Cameron
2026-11-10  8:27     ` Cristian Marussi
2026-11-09  9:11 ` [PATCH v15 05/35] firmware: arm_scmi: Refactor __scmi_event_handler_get_ops helper Cristian Marussi
2026-11-09  9:11 ` [PATCH v15 06/35] firmware: arm_scmi: Introduce protocol instance notifiers Cristian Marussi
2026-11-09 23:32   ` Jonathan Cameron
2026-11-09  9:11 ` [PATCH v15 07/35] dt-bindings: firmware: arm,scmi: Add support for telemetry protocol Cristian Marussi
2026-11-09  9:11 ` [PATCH v15 08/35] include: trace: Add Telemetry trace events Cristian Marussi
2026-11-09  9:11 ` [PATCH v15 09/35] firmware: arm_scmi: Add Telemetry basic attributes enumeration Cristian Marussi
2026-11-09 23:57   ` Jonathan Cameron
2026-11-09  9:11 ` [PATCH v15 10/35] firmware: arm_scmi: Add Telemetry resources enumeration Cristian Marussi
2026-11-10  1:36   ` Jonathan Cameron
2026-11-10 15:33     ` Cristian Marussi
2026-11-12 18:01   ` Jonathan Cameron
2026-11-12 19:56     ` Cristian Marussi
2026-11-09  9:11 ` [PATCH v15 11/35] firmware: arm_scmi: Add Telemetry groups initialization Cristian Marussi
2026-11-10  1:53   ` Jonathan Cameron
2026-11-09  9:11 ` [PATCH v15 12/35] firmware: arm_scmi: Add Telemetry update intervals enumeration Cristian Marussi
2026-11-10  2:07   ` Jonathan Cameron
2026-11-09  9:11 ` [PATCH v15 13/35] firmware: arm_scmi: Add Telemetry SHMTI areas enumeration Cristian Marussi
2026-11-10  2:26   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 14/35] firmware: arm_scmi: Add Telemetry TDCF areas parsing support Cristian Marussi
2026-11-10 18:15   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 15/35] firmware: arm_scmi: Add Telemetry TDCF parsing for BLK_TS lines Cristian Marussi
2026-11-10 18:33   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 16/35] firmware: arm_scmi: Add Telemetry TDCF parsing for UUID lines Cristian Marussi
2026-11-10 18:49   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 17/35] firmware: arm_scmi: Add Telemetry configuration operations Cristian Marussi
2026-11-10 20:07   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 18/35] firmware: arm_scmi: Add Telemetry DataEvent read capabilities Cristian Marussi
2026-11-10 21:59   ` Jonathan Cameron
2026-11-12  4:30   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 19/35] firmware: arm_scmi: Add Telemetry reset logic Cristian Marussi
2026-11-10 22:07   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 20/35] firmware: arm_scmi: Add Telemetry notification support Cristian Marussi
2026-11-10 22:16   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 21/35] firmware: arm_scmi: Add Telemetry boot-on support Cristian Marussi
2026-11-10 22:25   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 22/35] firmware: arm-scmi: Add Telemetry generic event support Cristian Marussi
2026-11-10 22:33   ` Jonathan Cameron
2026-11-12 19:58     ` Cristian Marussi
2026-11-09  9:12 ` [PATCH v15 23/35] firmware: arm_scmi: Add Telemetry generation counter event Cristian Marussi
2026-11-10 22:46   ` Jonathan Cameron
2026-11-12 20:17     ` Cristian Marussi
2026-11-12 21:44       ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 24/35] firmware: arm_scmi: Add common per-protocol debugfs support Cristian Marussi
2026-11-10 22:50   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 25/35] firmware: arm_scmi: Add Telemetry debugfs SHMTI dump support Cristian Marussi
2026-11-10 22:54   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 26/35] firmware: arm_scmi: Add Telemetry debugfs ABI documentation Cristian Marussi
2026-11-10 22:55   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 27/35] firmware: arm_scmi: Expose per-instance identifier Cristian Marussi
2026-11-10 22:57   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 28/35] firmware: arm_scmi: Add un-managed methods to get/put protocols operations Cristian Marussi
2026-11-10 23:01   ` Jonathan Cameron
2026-11-10 23:18   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 29/35] uapi: Add ARM SCMI Telemetry definitions Cristian Marussi
2026-11-10 23:13   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 30/35] firmware: arm_scmi: Add System Telemetry driver Cristian Marussi
2026-11-10 23:46   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 31/35] firmware: arm_scmi: Enable autoloading for " Cristian Marussi
2026-11-09  9:12 ` [PATCH v15 32/35] docs: ioctl-number: Add SCMI Ioctls Cristian Marussi
2026-11-09  9:12 ` [PATCH v15 33/35] Documentation: Add SCMI System Telemetry documentation Cristian Marussi
2026-11-10 23:54   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 34/35] [RFC] kselftest/arm64: Add SCMI Telemetry UAPI compliance testcases Cristian Marussi
2026-11-11  0:13   ` Jonathan Cameron
2026-11-09  9:12 ` [PATCH v15 35/35] [RFC] tools/testing/scmi: Add SCMI Telemetry testing tool Cristian Marussi
2026-11-11  0:16   ` Jonathan Cameron
2026-11-11  0:38 ` [PATCH v15 00/35] Introduce SCMI Telemetry support Jonathan Cameron
2026-11-11  9:51   ` Sudeep Holla
2026-11-11 10:15   ` Cristian Marussi [this message]
2026-11-11 18:39     ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=avRANKPwp-r-QSR4@pluto \
    --to=cristian.marussi@arm.com \
    --cc=arm-scmi@vger.kernel.org \
    --cc=d-gole@ti.com \
    --cc=david@kernel.org \
    --cc=elif.topuz@arm.com \
    --cc=etienne.carriere@st.com \
    --cc=f.fainelli@gmail.com \
    --cc=james.quinlan@broadcom.com \
    --cc=jonathan.cameron@oss.qualcomm.com \
    --cc=kas@kernel.org \
    --cc=kernel-team@meta.com \
    --cc=leitao@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukasz.luba@arm.com \
    --cc=michal.simek@amd.com \
    --cc=peng.fan@oss.nxp.com \
    --cc=philip.radford@arm.com \
    --cc=puranjay@kernel.org \
    --cc=souvik.chakravarty@arm.com \
    --cc=sudeep.holla@kernel.org \
    --cc=usama.arif@linux.dev \
    --cc=vincent.guittot@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.