Re: [PATCH v8 1/2] dt-bindings: iio: adc: add AD4130

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Thu, Sep 8, 2022 at 9:03 AM Cosmin Tanislav <demonsingur@xxxxxxxxx> wrote:

> >> +  adi,ext-clk-freq-hz:
> >> +    description: Specify the frequency of the external clock.
> >> +    enum: [76800, 153600]
> >> +    default: 76800
> >
> > This looks like cheating, i.e just outputting a clock on that pin
> > and ignoring to model the consumer.
>
> You got this wrong.
>
> The chip has 4 operating modes regarding clocking.
>
> Internal 76.8kHz clock (clkout can be used as an interrupt pin).
> Internal 76.8kHz clock, available externally on the clkout pin (clkout
> becomes an output).
> External 76.8kHz clock (clkout is an input).
> External 153.6kHz clock, internally divided by two (clkout is an input).
>
> This property is used to choose between what frequency to set the
> external clock up with. Indeed, if the external clock is not present,
> then exposing the 76.8kHz clock using the clock framework would be fine.
>
> Maybe you have a better suggestion about what to do with this?
> How do I tell the chip what frequency the external clock is, but also
> tell the clock what frequency to use? It's a bit of a conundrum for me.

I would imagine not specify in the device tree what frequency to use
at all.

Instead use software for that.

The clock provide for the clkout should provide

static const struct clk_ops extclk_ops = {
        .recalc_rate = extclk_recalc_rate,
        .round_rate = extclk_round_rate,
        .set_rate = extclk_set_rate,
};

This way the consumer driver can issue clk_round_rate(),
clk_set_rate() etc to make the clk driver determine which parent
to use for the consumer. Likewise .enable or .prepare should
then mux out the desired clock.

> >> +  adi,bipolar:
> >> +    description: Specify if the device should be used in bipolar mode.
> >> +    type: boolean
> >
> > Can you explain what this means? I don't understand what it would
> > mean for an analog device / AFE to be in bipolar mode.
> >
>
> Range becomes [-VRef, VRef], as opposed to [0, VRef], resolution is
> halved.

OK that makes sense, put that in the binding description.

Yours,
Linus Walleij