Re: [PATCH 1/2] staging: iio: adc: ad7192: Add clock for external clock reference

[Alexandru Ardelean <ardeleanalex@gmail.com>]

On Fri, Jan 25, 2019 at 12:41 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Jonathan Cameron (2018-12-16 02:07:41)
> > Rob, Clk experts, questions for you below.
> >
> > Jonathan
> >
> >
> > On Thu, 13 Dec 2018 17:39:22 -0800
> > Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > > Quoting Jonathan Cameron (2018-12-08 07:29:54)
> > > > On Thu, 6 Dec 2018 11:10:51 +0200
> > > > Mircea Caprioru <mircea.caprioru@analog.com> wrote:
> > > >
> > > > > This patch adds a clock to the state structure of ad7192 for getting the
> > > > > external clock frequency. This modifications is in accordance with clock
> > > > > framework dt bindings documentation.
> > > > >
> > > > > Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
> > > >
> > > > +cc Rob and the clk list for advise on how to do the binding for this one.
> > > >
> > > > It is basically 2 pins, you can put a clock in on one of them or connect
> > > > a crystal across them.  The driver has to set a register to say which is
> > > > the case.
> > > >
> > > > Current proposal is two optional clocks (fall back to internal oscillator)
> > > > but that doesn't seem to be commonly done, so I'm wondering if there
> > > > is a 'standard' way to handle this sort of thing.
> > > >
> > >
> > > I'm not sure I fully understand, but I think perhaps
> > > assigned-clock-parents would work? Or does that not work because either
> > > way some parent is assigned, either the crystal or the optional clk that
> > > isn't a crystal?
> > >
> > My concern is they aren't really separate clock inputs.   They are just different
> > ways of providing the same fundamental clock.  So I think we may want to just
> > provide a single clock and have another dt binding to say what it is.
> >
> > So lots of ways we could do it, but I'm not sure what the right one to
> > go with is!
> >
>
> Sorry for getting back to this so late. Is the datasheet public for this
> device? If so, any link to it?
>

Hey,
Link is http://analog.com/ad7192 and that should resolve to the proper
page, where the datasheet is available.
[ General info: most [if not all] datasheets from Analog Devices can
be found by concatenating "http://analog.com/ + "<part name>" ]

But more directly, the link to the PDF is:
https://www.analog.com/media/en/technical-documentation/data-sheets/AD7192.pdf
Page 10 provides some description of the pins, page 20 the mode
register for the clock, and page 32 a general description of the
clock.
If you search for MCLK1 or MCLK2 you should navigate pretty quick
through the doc.

The clock circuitry [the 2 pins] is common for all chips this driver
supports [AD7190/2/3/5].

Thanks
Alex

> If it's two pins, and sometimes one pin is connected and other times two
> pins are connected but a register needs to be set if the pins are
> connected in one configuration or the other I would say your plan for a
> DT property indicating how the pins are configured sounds good. Usually
> the hardware can figure these things out itself so I find this sort of
> odd, but if this is how it is then there's not much that we can do.
>
> It sounds like there aren't two different clk inputs to the device.
> Given that information specifying two optional clks is incorrect because
> there is only one 'slot' is the external clk source.
>