Re: [RFC] clk: vc5: Add bindings for output configurations

[Adam Ford]

On Fri, Mar 27, 2020 at 5:05 AM Adam Ford <aford173@xxxxxxxxx> wrote:
>
> On Fri, Mar 27, 2020 at 4:41 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> >
> > Hi Adam,
> >
> > CC Marek
> >
> > On Thu, Mar 26, 2020 at 10:33 PM Adam Ford <aford173@xxxxxxxxx> wrote:
> > > The Versaclock can be purchased in a non-programmed configuration.
> > > If that is the case, the driver needs to configure the chip to
> > > output the correct signal type, voltage and slew.
> > >
> > > This RFC is proposing an additional binding to allow non-programmed
> > > chips to be configured beyond their default configuration.
> > >
> > > Signed-off-by: Adam Ford <aford173@xxxxxxxxx>
> > >
> > > diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.txt b/Documentation/devicetree/bindings/clock/idt,versaclock5.txt
> > > index 05a245c9df08..4bc46ed9ba4a 100644
> > > --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.txt
> > > +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.txt
> > > @@ -30,6 +30,25 @@ Required properties:
> > >                 - 5p49v5933 and
> > >                 - 5p49v5935: (optional) property not present or "clkin".
> > >
> > > +For all output ports, an option child node can be used to specify:
> > > +
> > > +- mode: can be one of
> > > +                 - LVPECL: Low-voltage positive/psuedo emitter-coupled logic
> > > +                 - CMOS
> > > +                 - HCSL
> > > +                 - LVDS: Low voltage differential signal
> > > +
> > > +- voltage-level:  can be one of the following microvolts
> > > +                 - 1800000
> > > +                 - 2500000
> > > +                 - 3300000
> > > +-  slew: Percent of normal, can be one of
> > > +                 - P80
> > > +                 - P85
> > > +                 - P90
> > > +                 - P100
> >
> > Why the P prefixes? Can't you just use integer values?
> > After the conversion to json-schema, these values can be validated, too.

That makes sense.  We can just use numbers.

> >
> > > +
> > > +
> > >  ==Mapping between clock specifier and physical pins==
> > >
> > >  When referencing the provided clock in the DT using phandle and
> > > @@ -62,6 +81,8 @@ clock specifier, the following mapping applies:
> > >
> > >  ==Example==
> > >
> > > +#include <dt-bindings/versaclock.h>
> >
> > Does not exist?
>
> Not yet.  Before actually coding anything, I wanted to get feedback on
> how the bindings should look.  In this file would be definitions of
> terms like P80, CMOS, and the other items that are defined for mode
> and slew.

The intent was to create this file and define a sensible translation
between the arbitrary the numbers 0 to 7 and the acronyms for "output
type". Would it be better to just use strings for output type (and not
create this header file)? I think I've seen something like that in a
TI driver. I hesitate to put a bunch of string compares in a driver.
Is there another way? Could we use properties and only allow one?

>
> >
> > > +
> > >  /* 25MHz reference crystal */
> > >  ref25: ref25m {
> > >         compatible = "fixed-clock";
> > > @@ -80,6 +101,13 @@ i2c-master-node {
> > >                 /* Connect XIN input to 25MHz reference */
> > >                 clocks = <&ref25m>;
> > >                 clock-names = "xin";
> > > +
> > > +               ports@1 {
> > > +                       reg = <1>;
> > > +                       mode = <CMOS>;
> > > +                       pwr_sel = <1800000>;
> > > +                       slew = <P80>;
> > > +               };
> > >         };
> > >  };
> >
> > Gr{oetje,eeting}s,
> >
> >                         Geert
> >
> > --
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
> >
> > In personal conversations with technical people, I call myself a hacker. But
> > when I'm talking to journalists I just say "programmer" or something like that.
> >                                 -- Linus Torvalds