linux-kernel - Re: [PATCH v2 2/2] iio: frequency: adf4350: add clk provider

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: <20240608160838.16a2a04f@jic23-huawei>
Date: Sat, 8 Jun 2024 16:08:38 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Nuno Sá <noname.nuno@...il.com>
Cc: Antoniu Miclaus <antoniu.miclaus@...log.com>, Lars-Peter Clausen 
 <lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>, Rob
 Herring <robh@...nel.org>, Krzysztof Kozlowski  <krzk+dt@...nel.org>, Conor
 Dooley <conor+dt@...nel.org>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] iio: frequency: adf4350: add clk provider


> > +static int adf4350_clk_register(struct adf4350_state *st)
> > +{
> > +	struct spi_device *spi = st->spi;
> > +	struct clk_init_data init;
> > +	struct clk *clk;
> > +	const char *parent_name;
> > +	int ret;
> > +
> > +	if (!device_property_present(&spi->dev, "#clock-cells"))
> > +		return 0;
> > +
> > +	init.name = devm_kasprintf(&spi->dev, GFP_KERNEL, "%s-clk",
> > +				   fwnode_get_name(dev_fwnode(&spi->dev)));
> > +	device_property_read_string(&spi->dev, "clock-output-names",
> > +				    &init.name);
> > +
> > +	parent_name = of_clk_get_parent_name(spi->dev.of_node, 0);
> > +	if (!parent_name)
> > +		return -EINVAL;
> > +
> > +	init.ops = &adf4350_clk_ops;
> > +	init.parent_names = &parent_name;
> > +	init.num_parents = 1;
> > +
> > +	st->output.hw.init = &init;
> > +	clk = devm_clk_register(&spi->dev, &st->output.hw);
> > +	if (IS_ERR(clk))
> > +		return PTR_ERR(clk);
> > +
> > +	ret = of_clk_add_provider(spi->dev.of_node, of_clk_src_simple_get,
> > clk);
> > +	if (ret)
> > +		return ret;
> > +  
> 
> I totally agree this chip should be a clock provider (maybe it should even in
> drivers/clk from the beginning) but there's one thing that comes to my mind.
> Should we still expose the IIO userspace interface in case we register it as a
> clock provider?

That's a reasonable suggestion.  If it's wired up as a clock we probably don't
want to provide userspace controls via IIO.

> 
> Sure, we do have clk notifiers in the kernel but I still think it's a sensible
> question :)
> 
> I suspect we have another "outliers" in drivers/iio/frequency :)

We all love the blurry boundaries in the kernel. IIRC when these were orginally
proposed, the IIO thing was mostly about how they were being used in Software
Defined Radios where the were part of the modulator circuits.
I can't remember the exact reasoning though.

As to the right answer for these today, I don't have strong feelings on it
and almost all these parts are ADI ones.

Michael, this is one of yours, so what do you think?

On that note, given this is basically registering as a clock.I'd expect
to see some clk related +CC

Jonathan

> 
> - Nuno Sá 
> 
>