public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Cristian Marussi <cristian.marussi@arm.com>
To: Sudeep Holla <sudeep.holla@kernel.org>
Cc: "Peng Fan (OSS)" <peng.fan@oss.nxp.com>,
	Cristian Marussi <cristian.marussi@arm.com>,
	Jacky Bai <ping.bai@nxp.com>,
	arm-scmi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Peng Fan <peng.fan@nxp.com>
Subject: Re: [PATCH] firmware: arm_scmi: clock: Relax check in scmi_clock_protocol_init
Date: Tue, 24 Mar 2026 08:18:54 +0000	[thread overview]
Message-ID: <acJI7o79lVNMjV_d@pluto> (raw)
In-Reply-To: <20260324-lean-bobcat-of-reputation-6628b5@sudeepholla>

On Tue, Mar 24, 2026 at 07:49:22AM +0000, Sudeep Holla wrote:
> On Tue, Mar 24, 2026 at 02:24:14PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> > 

Hi,

> > On i.MX95, the SCMI Clock protocol defines several reserved clock IDs that
> > are not backed by real clock devices
> > (see arch/arm64/boot/dts/freescale/imx95-clock.h).
> > 
> > For these reserved IDs, the SCMI firmware correctly returns NOT_FOUND in
> > response to the CLOCK_ATTRIBUTES command. According to the SCMI Clock
> > specification, NOT_FOUND is expected when a clock_id does not correspond to
> > a valid clock device.
> > 
> > The recent hardening added in scmi_clock_protocol_init() treats any error
> > return as fatal, causing SCMI clock probe to fail and preventing i.MX9
> > platforms from booting.
> > 
> > Relax the check so that -ENOENT is treated as a non-fatal condition.
> > 
> 
> I understand the use-case and the fix here, but still wonder if this
> should be treated as quirk or handle it in the core. I am inclined to
> latter as reserved SCMI clock/resource ID seems to be trend in its usage
> and hard to classify as quirks.
> 
> Cristain, agree or have a different view ?
> 

I was just replying...

Looking at the spec 3.6.2.5 CLOCK_ATTRIBUTES

"This command returns the attributes that are associated with a specific clock. An agent might be allowed access to only
a subset of the clocks available in the system. The platform must thus guarantee that clocks that an agent cannot access
are not visible to it."

...not sure if this sheds some light or it is ambiguos anyway...I'd say that
NOT_FOUND does NOT equate to be invisible...

...BUT at the same time I think that this practice of exposing a non-contiguos
set of resources IDs (a set with holes in it) is the a well-known spec-loophole
used by many vendors to deploy one single FW image across all of their platforms
without having to reconfigure their reosurces IDs ro expose a common set of
contiguos IDs like the spec would suggest...

Having said that, since we unfortunately left this door open in the
implementation, now this loophole has become common practice
apparently...

...I am more concerned about the impact that this COULD have on underlying
resources allocations that at the driver level was certainly conceived
to manage a contiguos set of IDs, even though it was certanly the
usecase until my harden patches...so it could be safe but to be double
checked...

Quirk or core I suppose it depends on how much we want to 'legalize' this
trick for the future (thinking also about other protocols)...

...a middle ground could be to implement it as an always-on quirk that
matches any FW (like the existing out_of_spec_triplet quirk)... as a way
to keep on allowing the existing behaviour but sort of discouraging it...

Not really strong opinions about this, BUT at this point, in general I
would keep all of this series on hold for further testing, given these
issues together the bugs fixed by Geert on iterators and the lack of
clock-MAINTs acks...

...also Geert was investigating the need of a different quirks in these
regards for the Renesas boards...

Thanks,
Cristian

  reply	other threads:[~2026-03-24  8:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-24  6:24 [PATCH] firmware: arm_scmi: clock: Relax check in scmi_clock_protocol_init Peng Fan (OSS)
2026-03-24  7:49 ` Sudeep Holla
2026-03-24  8:18   ` Cristian Marussi [this message]
2026-03-24 13:15     ` Geert Uytterhoeven
2026-03-24 14:35       ` Cristian Marussi
2026-03-24 15:32         ` Geert Uytterhoeven
2026-03-25 12:06           ` Sudeep Holla
2026-03-25 12:18             ` Cristian Marussi
2026-03-25 12:14     ` Sudeep Holla

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=acJI7o79lVNMjV_d@pluto \
    --to=cristian.marussi@arm.com \
    --cc=arm-scmi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peng.fan@nxp.com \
    --cc=peng.fan@oss.nxp.com \
    --cc=ping.bai@nxp.com \
    --cc=sudeep.holla@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox