linux-kernel - Re: [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining the clock producer

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] [day] [month] [year] [list]

Message-ID: <aCIWk3tiTUM0TeEa@svinhufvud>
Date: Mon, 12 May 2025 18:41:07 +0300
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Mehdi Djait <mehdi.djait@...ux.intel.com>
Cc: laurent.pinchart@...asonboard.com, tomi.valkeinen@...asonboard.com,
	jacopo.mondi@...asonboard.com, hverkuil@...all.nl,
	kieran.bingham@...asonboard.com, naush@...pberrypi.com,
	mchehab@...nel.org, hdegoede@...hat.com,
	dave.stevenson@...pberrypi.com, linux-media@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v2] media: v4l2-common: Add a helper for obtaining
 the clock producer

Hi Mehdi,

On Mon, May 12, 2025 at 10:21:21AM +0200, Mehdi Djait wrote:
> Hi Sakari,
> 
> On Sat, May 10, 2025 at 12:56:02PM +0000, Sakari Ailus wrote:
> > Hi Mehdi,
> > 
> > On Mon, Mar 10, 2025 at 01:23:05PM +0100, Mehdi Djait wrote:
> > > Introduce a helper for v4l2 sensor drivers on both DT- and ACPI-based
> > > platforms to retrieve a reference to the clock producer from firmware.
> > > 
> > > This helper behaves the same as clk_get_optional() except where there is
> > > no clock producer like ACPI-based platforms.
> > > 
> > > For ACPI-based platforms the function will read the "clock-frequency"
> > > ACPI _DSD property and register a fixed frequency clock with the frequency
> > > indicated in the property.
> > > 
> > > Signed-off-by: Mehdi Djait <mehdi.djait@...ux.intel.com>
> 
> SNIP
> 
> > > +struct clk *devm_v4l2_sensor_clk_get(struct device *dev, const char *id)
> > > +{
> > > +	struct clk_hw *clk_hw;
> > > +	struct clk *clk;
> > > +	u32 rate;
> > > +	int ret;
> > > +
> > > +	clk = devm_clk_get_optional(dev, id);
> > > +	if (clk)
> > > +		return clk;
> > > +
> > > +	if (!is_acpi_node(dev_fwnode(dev)))
> > > +		return ERR_PTR(-ENOENT);
> > > +
> > > +	ret = device_property_read_u32(dev, "clock-frequency", &rate);
> > > +	if (ret)
> > > +		return ERR_PTR(ret);
> > > +
> > > +	if (!id) {
> > > +		id = devm_kasprintf(dev, GFP_KERNEL, "clk-%s", dev_name(dev));
> > > +		if (!id)
> > > +			return ERR_PTR(-ENOMEM);
> > > +	}
> > > +
> > > +	clk_hw = devm_clk_hw_register_fixed_rate(dev, id, NULL, 0, rate);
> > 
> > devm_clk_hw_register_fixed_rate() is only available when COMMON_CLK is
> > enabled. You need #ifdefs here. In practice without CCF only
> > devm_clk_get_optional() is useful I guess.
> > 
> 
> I added a call to IS_REACHABLE(CONFIG_COMMON_CLK) in the v4 of this patch:
> https://lore.kernel.org/linux-media/20250321130329.342236-1-mehdi.djait@linux.intel.com/

I wonder if this approach works. Depending on the compiler implementation,
the compiler could (or even should) still run into issues in finding an
unresolvable symbol, even if the symbol is not reachable and can be
optimised away.

> 
> > Another question is then how commonly COMMON_CLK is enabled e.g. on x86
> > systems. At least Debian kernel has it. Presumably it's common elsewhere,
> > too.
> 
> on Arch linux it is also enabled and Fedora also. I would also assume it
> is also enabled in the other linux distros.

Ack, thanks for checking.

-- 
Regards,

Sakari Ailus