linux-kernel - Re: [PATCH v5 04/10] coresight: Appropriately disable programming clocks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250730085637.GB143191@e132581.arm.com>
Date: Wed, 30 Jul 2025 09:56:37 +0100
From: Leo Yan <leo.yan@....com>
To: Suzuki K Poulose <suzuki.poulose@....com>
Cc: Mark Brown <broonie@...nel.org>, Mike Leach <mike.leach@...aro.org>,
	James Clark <james.clark@...aro.org>,
	Anshuman Khandual <anshuman.khandual@....com>,
	Yeoreum Yun <yeoreum.yun@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 04/10] coresight: Appropriately disable programming
 clocks

On Tue, Jul 29, 2025 at 01:30:28PM +0100, Suzuki Kuruppassery Poulose wrote:
> On 29/07/2025 12:31, Mark Brown wrote:
> > On Mon, Jul 28, 2025 at 05:45:04PM +0100, Mark Brown wrote:
> > > On Thu, Jul 24, 2025 at 04:22:34PM +0100, Leo Yan wrote:
> > > 
> > > Previously we would return NULL for any error (which isn't super great
> > > for deferred probe but never mind).
> > > 
> > > > +	pclk = devm_clk_get_enabled(dev, "apb_pclk");
> > > > +	if (IS_ERR(pclk))
> > > > +		pclk = devm_clk_get_enabled(dev, "apb");
> > > 
> > > ...
> > > 
> > > >   	return pclk;
> > > >   }
> > > 
> > > Now we pass errors back to the caller, making missing clocks fatal.
> > 
> > Thinking about this some more I think for compatiblity these clocks need
> > to be treated as optional - that's what the original code was
> > effectively doing, and I can imagine this isn't the only SoC which has
> > (hopefully) always on clocks and didn't wire things up in DT.
> 
> You're right. The static components (funnels, replicators) don't have
> APB programming interface and hence no clocks. That said, may be the
> "is amba device" check could be used to enforce the presence of a clock.

I was wondering how this issue slipped through when I tested it on the
Hikey960 board. The Hikey960 also has one static funnel, but it binds
pclk with the static funnel node. That's why I didn't detect the issue.

I don't think using optional clock API is right thing, as DT binding
schema claims the pclk is mandatory for dynamic components. My proposal
is to enable the clocks only when IORESOURCE_MEM is available, something
like:

  if (res) {
      ret = coresight_get_enable_clocks(dev, &drvdata->pclk,
                                        &drvdata->atclk);
      if (ret)
              return ret;

      base = devm_ioremap_resource(dev, res);
      ...
  }

The static components don't bind I/O resources, it is naturally not to
enable clocks for them. Please let me know if this is reasonable
solution.

@Mark, thanks a lot for testing and bisection.

Leo