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

[Jonathan Cameron]

> > +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á 
> 
>