Received-SPF: pass (google.com: domain of linux-kernel+bounces-207084-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Sat, 8 Jun 2024 16:08:38 +0100
From: Jonathan Cameron <jic23@kernel.org>
To: Nuno =?UTF-8?B?U8Oh?= <noname.nuno@gmail.com>
Cc: Antoniu Miclaus <antoniu.miclaus@analog.com>, Lars-Peter Clausen 
 <lars@metafoo.de>, Michael Hennerich <Michael.Hennerich@analog.com>, Rob
 Herring <robh@kernel.org>, Krzysztof Kozlowski  <krzk+dt@kernel.org>, Conor
 Dooley <conor+dt@kernel.org>, linux-iio@vger.kernel.org,
 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/2] iio: frequency: adf4350: add clk provider
Message-ID: <20240608160838.16a2a04f@jic23-huawei>
In-Reply-To: <751abd157213736e376ca43ef1082362a4ca1149.camel@gmail.com>
References: <20240607095806.3299-1-antoniu.miclaus@analog.com>
	<20240607095806.3299-2-antoniu.miclaus@analog.com>
	<751abd157213736e376ca43ef1082362a4ca1149.camel@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable


> > +static int adf4350_clk_register(struct adf4350_state *st)
> > +{
> > +	struct spi_device *spi =3D 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 =3D devm_kasprintf(&spi->dev, GFP_KERNEL, "%s-clk",
> > +				=C2=A0=C2=A0 fwnode_get_name(dev_fwnode(&spi->dev)));
> > +	device_property_read_string(&spi->dev, "clock-output-names",
> > +				=C2=A0=C2=A0=C2=A0 &init.name);
> > +
> > +	parent_name =3D of_clk_get_parent_name(spi->dev.of_node, 0);
> > +	if (!parent_name)
> > +		return -EINVAL;
> > +
> > +	init.ops =3D &adf4350_clk_ops;
> > +	init.parent_names =3D &parent_name;
> > +	init.num_parents =3D 1;
> > +
> > +	st->output.hw.init =3D &init;
> > +	clk =3D devm_clk_register(&spi->dev, &st->output.hw);
> > +	if (IS_ERR(clk))
> > +		return PTR_ERR(clk);
> > +
> > +	ret =3D of_clk_add_provider(spi->dev.of_node, of_clk_src_simple_get,
> > clk);
> > +	if (ret)
> > +		return ret;
> > + =20
>=20
> I totally agree this chip should be a clock provider (maybe it should eve=
n in
> drivers/clk from the beginning) but there's one thing that comes to my mi=
nd.
> 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 do=
n't
want to provide userspace controls via IIO.

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

We all love the blurry boundaries in the kernel. IIRC when these were orgin=
ally
proposed, the IIO thing was mostly about how they were being used in Softwa=
re
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

>=20
> - Nuno S=C3=A1=20
>=20
>=20