| Message ID | 1389967140-20704-7-git-send-email-james.hogan@imgtec.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
Em Fri, 17 Jan 2014 13:58:51 +0000 James Hogan <james.hogan@imgtec.com> escreveu: > Add device tree binding for ImgTec Consumer Infrared block, specifically > major revision 1 of the hardware. @DT maintainers: ping. > > Signed-off-by: James Hogan <james.hogan@imgtec.com> > Cc: Mauro Carvalho Chehab <m.chehab@samsung.com> > Cc: linux-media@vger.kernel.org > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Pawel Moll <pawel.moll@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk> > Cc: Kumar Gala <galak@codeaurora.org> > Cc: devicetree@vger.kernel.org > Cc: Rob Landley <rob@landley.net> > Cc: linux-doc@vger.kernel.org > Cc: Tomasz Figa <tomasz.figa@gmail.com> > --- > v2: > - Future proof compatible string from "img,ir" to "img,ir1", where the 1 > corresponds to the major revision number of the hardware (Tomasz > Figa). > - Added clock-names property and three specific clock names described in > the manual, only one of which is used by the current driver (Tomasz > Figa). > --- > .../devicetree/bindings/media/img-ir1.txt | 30 ++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/img-ir1.txt > > diff --git a/Documentation/devicetree/bindings/media/img-ir1.txt b/Documentation/devicetree/bindings/media/img-ir1.txt > new file mode 100644 > index 0000000..ace5fd9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/img-ir1.txt > @@ -0,0 +1,30 @@ > +* ImgTec Infrared (IR) decoder version 1 > + > +This binding is for Imagination Technologies' Infrared decoder block, > +specifically major revision 1. > + > +Required properties: > +- compatible: Should be "img,ir1" > +- reg: Physical base address of the controller and length of > + memory mapped region. > +- interrupts: The interrupt specifier to the cpu. > + > +Optional properties: > +- clocks: List of clock specifiers as described in standard > + clock bindings. > +- clock-names: List of clock names corresponding to the clocks > + specified in the clocks property. > + Accepted clock names are: > + "core": Core clock (defaults to 32.768KHz if omitted). > + "sys": System side (fast) clock. > + "mod": Power modulation clock. > + > +Example: > + > + ir@02006200 { > + compatible = "img,ir1"; > + reg = <0x02006200 0x100>; > + interrupts = <29 4>; > + clocks = <&clk_32khz>; > + clock-names = "core"; > + };
On Fri, Jan 17, 2014 at 7:58 AM, James Hogan <james.hogan@imgtec.com> wrote: > Add device tree binding for ImgTec Consumer Infrared block, specifically > major revision 1 of the hardware. > > Signed-off-by: James Hogan <james.hogan@imgtec.com> > Cc: Mauro Carvalho Chehab <m.chehab@samsung.com> > Cc: linux-media@vger.kernel.org > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Pawel Moll <pawel.moll@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk> > Cc: Kumar Gala <galak@codeaurora.org> > Cc: devicetree@vger.kernel.org > Cc: Rob Landley <rob@landley.net> > Cc: linux-doc@vger.kernel.org > Cc: Tomasz Figa <tomasz.figa@gmail.com> > --- > v2: > - Future proof compatible string from "img,ir" to "img,ir1", where the 1 > corresponds to the major revision number of the hardware (Tomasz > Figa). > - Added clock-names property and three specific clock names described in > the manual, only one of which is used by the current driver (Tomasz > Figa). > --- > .../devicetree/bindings/media/img-ir1.txt | 30 ++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/img-ir1.txt > > diff --git a/Documentation/devicetree/bindings/media/img-ir1.txt b/Documentation/devicetree/bindings/media/img-ir1.txt > new file mode 100644 > index 0000000..ace5fd9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/img-ir1.txt > @@ -0,0 +1,30 @@ > +* ImgTec Infrared (IR) decoder version 1 > + > +This binding is for Imagination Technologies' Infrared decoder block, > +specifically major revision 1. > + > +Required properties: > +- compatible: Should be "img,ir1" Kind of short for a name. I don't have anything much better, but how about img,ir-rev1. > +- reg: Physical base address of the controller and length of > + memory mapped region. > +- interrupts: The interrupt specifier to the cpu. > + > +Optional properties: > +- clocks: List of clock specifiers as described in standard > + clock bindings. > +- clock-names: List of clock names corresponding to the clocks > + specified in the clocks property. > + Accepted clock names are: > + "core": Core clock (defaults to 32.768KHz if omitted). > + "sys": System side (fast) clock. > + "mod": Power modulation clock. You need to define the order of clocks including how they are interpreted with different number of clocks (not relying on the name). Although, if the h/w block really has different number of clock inputs, then it is a different h/w block and should have a different compatible string. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob, On 06/02/14 14:33, Rob Herring wrote: > On Fri, Jan 17, 2014 at 7:58 AM, James Hogan <james.hogan@imgtec.com> wrote: >> +Required properties: >> +- compatible: Should be "img,ir1" > > Kind of short for a name. I don't have anything much better, but how > about img,ir-rev1. Okay, that sounds reasonable. >> +Optional properties: >> +- clocks: List of clock specifiers as described in standard >> + clock bindings. >> +- clock-names: List of clock names corresponding to the clocks >> + specified in the clocks property. >> + Accepted clock names are: >> + "core": Core clock (defaults to 32.768KHz if omitted). >> + "sys": System side (fast) clock. >> + "mod": Power modulation clock. > > You need to define the order of clocks including how they are > interpreted with different number of clocks (not relying on the name). Would it be sufficient to specify that "clock-names" is required if "clocks" is provided (i.e. unnamed clocks aren't used), or is there some other reason that clock-names shouldn't be relied upon? Thanks for reviewing, Cheers James > Although, if the h/w block really has different number of clock > inputs, then it is a different h/w block and should have a different > compatible string.
On Thu, Feb 6, 2014 at 8:41 AM, James Hogan <james.hogan@imgtec.com> wrote: > Hi Rob, > > On 06/02/14 14:33, Rob Herring wrote: >> On Fri, Jan 17, 2014 at 7:58 AM, James Hogan <james.hogan@imgtec.com> wrote: >>> +Required properties: >>> +- compatible: Should be "img,ir1" >> >> Kind of short for a name. I don't have anything much better, but how >> about img,ir-rev1. > > Okay, that sounds reasonable. > >>> +Optional properties: >>> +- clocks: List of clock specifiers as described in standard >>> + clock bindings. >>> +- clock-names: List of clock names corresponding to the clocks >>> + specified in the clocks property. >>> + Accepted clock names are: >>> + "core": Core clock (defaults to 32.768KHz if omitted). >>> + "sys": System side (fast) clock. >>> + "mod": Power modulation clock. >> >> You need to define the order of clocks including how they are >> interpreted with different number of clocks (not relying on the name). > > Would it be sufficient to specify that "clock-names" is required if > "clocks" is provided (i.e. unnamed clocks aren't used), or is there some > other reason that clock-names shouldn't be relied upon? irq-names, reg-names, clock-names, etc. are considered optional to their associated property and the order is supposed to be defined. clock-names is a bit different in that clk_get needs a name, so it effectively is required by Linux when there is more than 1 clock. Really, we should fix Linux. Regardless, my other point is still valid. A given h/w block has a fixed number of clocks. You may have them all connected to the same source in some cases, but that does not change the number of inputs. Defining what are the valid combinations needs to be done. Seems like this could be: <none> - default to 32KHz <core> - only a "baud" clock <core>, <sys>, <mod> - all clocks Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob, On Fri, Feb 07, 2014 at 02:33:27PM +0000, Rob Herring wrote: > On Thu, Feb 6, 2014 at 8:41 AM, James Hogan <james.hogan@imgtec.com> wrote: > > Hi Rob, > > > > On 06/02/14 14:33, Rob Herring wrote: > >> On Fri, Jan 17, 2014 at 7:58 AM, James Hogan <james.hogan@imgtec.com> wrote: > >>> +Required properties: > >>> +- compatible: Should be "img,ir1" > >> > >> Kind of short for a name. I don't have anything much better, but how > >> about img,ir-rev1. > > > > Okay, that sounds reasonable. > > > >>> +Optional properties: > >>> +- clocks: List of clock specifiers as described in standard > >>> + clock bindings. > >>> +- clock-names: List of clock names corresponding to the clocks > >>> + specified in the clocks property. > >>> + Accepted clock names are: > >>> + "core": Core clock (defaults to 32.768KHz if omitted). > >>> + "sys": System side (fast) clock. > >>> + "mod": Power modulation clock. > >> > >> You need to define the order of clocks including how they are > >> interpreted with different number of clocks (not relying on the name). > > > > Would it be sufficient to specify that "clock-names" is required if > > "clocks" is provided (i.e. unnamed clocks aren't used), or is there some > > other reason that clock-names shouldn't be relied upon? > > irq-names, reg-names, clock-names, etc. are considered optional to > their associated property and the order is supposed to be defined. > clock-names is a bit different in that clk_get needs a name, so it > effectively is required by Linux when there is more than 1 clock. > Really, we should fix Linux. If they're optional then you can't handle optional entries (i.e. when nothing's wired to an input), and this is counter to the style I've been recommending to people (defining clocks in terms of clock-names). I really don't see the point in any *-names property if they don't define the list and allow for optional / reordered lists. Why does the order have to be fixed rather than using the -names properties? It's already a de-facto standard. > > Regardless, my other point is still valid. A given h/w block has a > fixed number of clocks. You may have them all connected to the same > source in some cases, but that does not change the number of inputs. > Defining what are the valid combinations needs to be done. Seems like > this could be: > > <none> - default to 32KHz > <core> - only a "baud" clock > <core>, <sys>, <mod> - all clocks For more complex IP blocks you might have more inputs than you actually have clocks wired to. How do you handle an unwired input in the middle of the list, or a new revision of the IP block that got rid of the first clock input from the list but is otherwise compatible? I really don't think it makes sense to say that clock-names doesn't define the list. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 7, 2014 at 8:50 AM, Mark Rutland <mark.rutland@arm.com> wrote: > Hi Rob, > > On Fri, Feb 07, 2014 at 02:33:27PM +0000, Rob Herring wrote: >> On Thu, Feb 6, 2014 at 8:41 AM, James Hogan <james.hogan@imgtec.com> wrote: >> > Hi Rob, >> > >> > On 06/02/14 14:33, Rob Herring wrote: >> >> On Fri, Jan 17, 2014 at 7:58 AM, James Hogan <james.hogan@imgtec.com> wrote: >> >>> +Required properties: >> >>> +- compatible: Should be "img,ir1" >> >> >> >> Kind of short for a name. I don't have anything much better, but how >> >> about img,ir-rev1. >> > >> > Okay, that sounds reasonable. >> > >> >>> +Optional properties: >> >>> +- clocks: List of clock specifiers as described in standard >> >>> + clock bindings. >> >>> +- clock-names: List of clock names corresponding to the clocks >> >>> + specified in the clocks property. >> >>> + Accepted clock names are: >> >>> + "core": Core clock (defaults to 32.768KHz if omitted). >> >>> + "sys": System side (fast) clock. >> >>> + "mod": Power modulation clock. >> >> >> >> You need to define the order of clocks including how they are >> >> interpreted with different number of clocks (not relying on the name). >> > >> > Would it be sufficient to specify that "clock-names" is required if >> > "clocks" is provided (i.e. unnamed clocks aren't used), or is there some >> > other reason that clock-names shouldn't be relied upon? >> >> irq-names, reg-names, clock-names, etc. are considered optional to >> their associated property and the order is supposed to be defined. >> clock-names is a bit different in that clk_get needs a name, so it >> effectively is required by Linux when there is more than 1 clock. >> Really, we should fix Linux. > > If they're optional then you can't handle optional entries (i.e. when > nothing's wired to an input), and this is counter to the style I've been > recommending to people (defining clocks in terms of clock-names). > > I really don't see the point in any *-names property if they don't > define the list and allow for optional / reordered lists. Why does the > order have to be fixed rather than using the -names properties? It's > already a de-facto standard. Maybe for clocks, but I don't think we should treat clocks differently from other properties. We've already got enough variation in binding styles, I'd like to be consistent across interrupts, reg, clocks, etc. >> Regardless, my other point is still valid. A given h/w block has a >> fixed number of clocks. You may have them all connected to the same >> source in some cases, but that does not change the number of inputs. >> Defining what are the valid combinations needs to be done. Seems like >> this could be: >> >> <none> - default to 32KHz >> <core> - only a "baud" clock >> <core>, <sys>, <mod> - all clocks > > For more complex IP blocks you might have more inputs than you actually > have clocks wired to. > > How do you handle an unwired input in the middle of the list, or a new > revision of the IP block that got rid of the first clock input from the > list but is otherwise compatible? fixed-clock with freq of 0 for unwired (really wired to gnd) inputs? With a new compatible string if it is a new block. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/media/img-ir1.txt b/Documentation/devicetree/bindings/media/img-ir1.txt new file mode 100644 index 0000000..ace5fd9 --- /dev/null +++ b/Documentation/devicetree/bindings/media/img-ir1.txt @@ -0,0 +1,30 @@ +* ImgTec Infrared (IR) decoder version 1 + +This binding is for Imagination Technologies' Infrared decoder block, +specifically major revision 1. + +Required properties: +- compatible: Should be "img,ir1" +- reg: Physical base address of the controller and length of + memory mapped region. +- interrupts: The interrupt specifier to the cpu. + +Optional properties: +- clocks: List of clock specifiers as described in standard + clock bindings. +- clock-names: List of clock names corresponding to the clocks + specified in the clocks property. + Accepted clock names are: + "core": Core clock (defaults to 32.768KHz if omitted). + "sys": System side (fast) clock. + "mod": Power modulation clock. + +Example: + + ir@02006200 { + compatible = "img,ir1"; + reg = <0x02006200 0x100>; + interrupts = <29 4>; + clocks = <&clk_32khz>; + clock-names = "core"; + };
Add device tree binding for ImgTec Consumer Infrared block, specifically major revision 1 of the hardware. Signed-off-by: James Hogan <james.hogan@imgtec.com> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com> Cc: linux-media@vger.kernel.org Cc: Rob Herring <robh+dt@kernel.org> Cc: Pawel Moll <pawel.moll@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk> Cc: Kumar Gala <galak@codeaurora.org> Cc: devicetree@vger.kernel.org Cc: Rob Landley <rob@landley.net> Cc: linux-doc@vger.kernel.org Cc: Tomasz Figa <tomasz.figa@gmail.com> --- v2: - Future proof compatible string from "img,ir" to "img,ir1", where the 1 corresponds to the major revision number of the hardware (Tomasz Figa). - Added clock-names property and three specific clock names described in the manual, only one of which is used by the current driver (Tomasz Figa). --- .../devicetree/bindings/media/img-ir1.txt | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/img-ir1.txt