* [RFC] clk: vc5: Add bindings for output configurations @ 2020-03-26 21:32 Adam Ford 2020-03-27 9:41 ` Geert Uytterhoeven 2020-04-05 1:28 ` Rob Herring 0 siblings, 2 replies; 7+ messages in thread From: Adam Ford @ 2020-03-26 21:32 UTC (permalink / raw) To: devicetree Cc: aford, charles.stevens, Adam Ford, Michael Turquette, Stephen Boyd, Rob Herring, linux-clk, linux-kernel 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@gmail.com> 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 + + ==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> + /* 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>; + }; }; }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC] clk: vc5: Add bindings for output configurations 2020-03-26 21:32 [RFC] clk: vc5: Add bindings for output configurations Adam Ford @ 2020-03-27 9:41 ` Geert Uytterhoeven 2020-03-27 10:05 ` Adam Ford 2020-04-05 1:28 ` Rob Herring 1 sibling, 1 reply; 7+ messages in thread From: Geert Uytterhoeven @ 2020-03-27 9:41 UTC (permalink / raw) To: Adam Ford Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, aford, charles.stevens, Michael Turquette, Stephen Boyd, Rob Herring, linux-clk, Linux Kernel Mailing List, Marek Vasut Hi Adam, CC Marek On Thu, Mar 26, 2020 at 10:33 PM Adam Ford <aford173@gmail.com> 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@gmail.com> > > 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. > + > + > ==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? > + > /* 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@linux-m68k.org 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] clk: vc5: Add bindings for output configurations 2020-03-27 9:41 ` Geert Uytterhoeven @ 2020-03-27 10:05 ` Adam Ford 2020-04-01 14:51 ` Adam Ford 0 siblings, 1 reply; 7+ messages in thread From: Adam Ford @ 2020-03-27 10:05 UTC (permalink / raw) To: Geert Uytterhoeven Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Adam Ford-BE, Charles Stevens, Michael Turquette, Stephen Boyd, Rob Herring, linux-clk, Linux Kernel Mailing List, Marek Vasut On Fri, Mar 27, 2020 at 4:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Adam, > > CC Marek > > On Thu, Mar 26, 2020 at 10:33 PM Adam Ford <aford173@gmail.com> 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@gmail.com> > > > > 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. > > > + > > + > > ==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. > > > + > > /* 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@linux-m68k.org > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] clk: vc5: Add bindings for output configurations 2020-03-27 10:05 ` Adam Ford @ 2020-04-01 14:51 ` Adam Ford 0 siblings, 0 replies; 7+ messages in thread From: Adam Ford @ 2020-04-01 14:51 UTC (permalink / raw) To: Geert Uytterhoeven Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Adam Ford-BE, Charles Stevens, Michael Turquette, Stephen Boyd, Rob Herring, linux-clk, Linux Kernel Mailing List, Marek Vasut On Fri, Mar 27, 2020 at 5:05 AM Adam Ford <aford173@gmail.com> wrote: > > On Fri, Mar 27, 2020 at 4:41 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > Hi Adam, > > > > CC Marek > > > > On Thu, Mar 26, 2020 at 10:33 PM Adam Ford <aford173@gmail.com> 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@gmail.com> > > > > > > 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@linux-m68k.org > > > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] clk: vc5: Add bindings for output configurations 2020-03-26 21:32 [RFC] clk: vc5: Add bindings for output configurations Adam Ford 2020-03-27 9:41 ` Geert Uytterhoeven @ 2020-04-05 1:28 ` Rob Herring 2020-04-05 1:38 ` Adam Ford 1 sibling, 1 reply; 7+ messages in thread From: Rob Herring @ 2020-04-05 1:28 UTC (permalink / raw) To: Adam Ford Cc: devicetree, aford, charles.stevens, Michael Turquette, Stephen Boyd, linux-clk, linux-kernel On Thu, Mar 26, 2020 at 04:32:51PM -0500, Adam Ford 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@gmail.com> > > 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 > + > + > ==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> > + > /* 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 { 'ports' is already taken as a node name. > + reg = <1>; What do the reg value signify? > + mode = <CMOS>; > + pwr_sel = <1800000>; Not documented. Don't use '-' in property names. > + slew = <P80>; > + }; > }; > }; > > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] clk: vc5: Add bindings for output configurations 2020-04-05 1:28 ` Rob Herring @ 2020-04-05 1:38 ` Adam Ford 2020-04-06 16:12 ` Rob Herring 0 siblings, 1 reply; 7+ messages in thread From: Adam Ford @ 2020-04-05 1:38 UTC (permalink / raw) To: Rob Herring Cc: devicetree, Adam Ford-BE, Charles Stevens, Michael Turquette, Stephen Boyd, linux-clk, Linux Kernel Mailing List On Sat, Apr 4, 2020 at 8:28 PM Rob Herring <robh@kernel.org> wrote: > > On Thu, Mar 26, 2020 at 04:32:51PM -0500, Adam Ford 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@gmail.com> > > > > 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 > > + > > + > > ==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> > > + > > /* 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 { > > 'ports' is already taken as a node name. Rob, The clock chip can drive multiple clocks and each output is independent of the rest. The idea is that port@1 would represent output 1, port@2 would represent output 2, etc. Is there a name you'd think we should use to represent each output? Different variations of this chip can have different number of outputs. > > > + reg = <1>; > > What do the reg value signify? I am fine if we drop we drop it. I was under the assumption that reg =<1> had to correspond to the port@1 and that it was required since other devices with port sub-nodes use the reg entry. > > > + mode = <CMOS>; > > + pwr_sel = <1800000>; > > Not documented. Don't use '-' in property names. Do you have a preference to what name or convention you want us to use? > Thanks for the review. adam > > + slew = <P80>; > > + }; > > }; > > }; > > > > -- > > 2.25.1 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] clk: vc5: Add bindings for output configurations 2020-04-05 1:38 ` Adam Ford @ 2020-04-06 16:12 ` Rob Herring 0 siblings, 0 replies; 7+ messages in thread From: Rob Herring @ 2020-04-06 16:12 UTC (permalink / raw) To: Adam Ford Cc: devicetree, Adam Ford-BE, Charles Stevens, Michael Turquette, Stephen Boyd, linux-clk, Linux Kernel Mailing List On Sat, Apr 4, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote: > > On Sat, Apr 4, 2020 at 8:28 PM Rob Herring <robh@kernel.org> wrote: > > > > On Thu, Mar 26, 2020 at 04:32:51PM -0500, Adam Ford 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@gmail.com> > > > > > > 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 > > > + > > > + > > > ==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> > > > + > > > /* 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 { > > > > 'ports' is already taken as a node name. > Rob, > > The clock chip can drive multiple clocks and each output is > independent of the rest. The idea is that port@1 would represent > output 1, port@2 would represent output 2, etc. > Is there a name you'd think we should use to represent each output? clock-output@...? > Different variations of this chip can have different number of > outputs. > > > > > > + reg = <1>; > > > > What do the reg value signify? > > I am fine if we drop we drop it. I was under the assumption that reg > =<1> had to correspond to the port@1 and that it was required since > other devices with port sub-nodes use the reg entry. I wasn't suggesting dropping it. Just what 0, 1, 2, etc. corresponds to as you explained above. Just put that into the 'reg' description. > > > + mode = <CMOS>; > > > + pwr_sel = <1800000>; > > > > Not documented. Don't use '-' in property names. > > Do you have a preference to what name or convention you want us to use? Errr, that was supposed to say '_'. Using hyphens is fine. Also, needs a vendor prefix and if that's in microvolts needs a unit suffix. Rob ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-04-06 16:12 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-26 21:32 [RFC] clk: vc5: Add bindings for output configurations Adam Ford 2020-03-27 9:41 ` Geert Uytterhoeven 2020-03-27 10:05 ` Adam Ford 2020-04-01 14:51 ` Adam Ford 2020-04-05 1:28 ` Rob Herring 2020-04-05 1:38 ` Adam Ford 2020-04-06 16:12 ` Rob Herring
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).