| Message ID | 1376034053-31910-6-git-send-email-chanho61.park@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Chanho, On 9 August 2013 13:10, Chanho Park <chanho61.park@samsung.com> wrote: > This patch describes each nodes of rotator and specifies a example how to bind > it. > > Signed-off-by: Chanho Park <chanho61.park@samsung.com> > Cc: Thomas Abraham <thomas.abraham@linaro.org> > Cc: Kukjin Kim <kgene.kim@samsung.com> > Cc: Inki Dae <inki.dae@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > .../devicetree/bindings/gpu/samsung-rotator.txt | 26 ++++++++++++++++++++ > 1 file changed, 26 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpu/samsung-rotator.txt > > diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.txt b/Documentation/devicetree/bindings/gpu/samsung-rotator.txt > new file mode 100644 > index 0000000..31ee7b5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpu/samsung-rotator.txt > @@ -0,0 +1,26 @@ > +* Samsung Image Rotator > + > +Required properties: > + - compatible : value should be following: This should be "should be one of the following:" > + (a) "samsung,exynos4210-rotator" for Rotator IP in Exynos4210 > + (b) "samsung,exynos4212-rotator" for Rotator IP in Exynos4212/4412 > + (c) "samsung,exynos5250-rotator" for Rotator IP in Exynos5250 > + > + - reg : Physical base address of the IP registers and length of memory > + mapped region. > + > + - interrupts : Interrupt number of Rotator > + > + - clocks : Clock number described in Documentations/devicetree/binding/clock. The path has several typos. Instead you can simply mention clocks : from common clock binding: handle to rotator clock.
Hi Sachin, > -----Original Message----- > From: Sachin Kamat [mailto:sachin.kamat@linaro.org] > Sent: Friday, August 09, 2013 6:40 PM > To: Chanho Park > Cc: inki.dae@samsung.com; kgene.kim@samsung.com; dri- > devel@lists.freedesktop.org; kyungmin.park@samsung.com; > mark.rutland@arm.com; l.stach@pengutronix.de; s.nawrocki@samsung.com; > tomasz.figa@gmail.com; linux-samsung-soc@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; devicetree@vger.kernel.org; Thomas Abraham > Subject: Re: [PATCHv2 5/5] ARM: dts: Add dt binding documentation for > exynos rotator > > Hi Chanho, > > On 9 August 2013 13:10, Chanho Park <chanho61.park@samsung.com> wrote: > > This patch describes each nodes of rotator and specifies a example how > > to bind it. > > > > Signed-off-by: Chanho Park <chanho61.park@samsung.com> > > Cc: Thomas Abraham <thomas.abraham@linaro.org> > > Cc: Kukjin Kim <kgene.kim@samsung.com> > > Cc: Inki Dae <inki.dae@samsung.com> > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > > --- > > .../devicetree/bindings/gpu/samsung-rotator.txt | 26 > ++++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/gpu/samsung-rotator.txt > > > > diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.txt > > b/Documentation/devicetree/bindings/gpu/samsung-rotator.txt > > new file mode 100644 > > index 0000000..31ee7b5 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/gpu/samsung-rotator.txt > > @@ -0,0 +1,26 @@ > > +* Samsung Image Rotator > > + > > +Required properties: > > + - compatible : value should be following: > > This should be "should be one of the following:" I left out while I changed it to select multiple choices. Thanks. > > > + (a) "samsung,exynos4210-rotator" for Rotator IP in Exynos4210 > > + (b) "samsung,exynos4212-rotator" for Rotator IP in > Exynos4212/4412 > > + (c) "samsung,exynos5250-rotator" for Rotator IP in Exynos5250 > > + > > + - reg : Physical base address of the IP registers and length of > memory > > + mapped region. > > + > > + - interrupts : Interrupt number of Rotator > > + > > + - clocks : Clock number described in > Documentations/devicetree/binding/clock. > > The path has several typos. Instead you can simply mention clocks : from > common clock binding: handle to rotator clock. Oh, it has many typos. I'll change it. Thanks for your review. Best Regards, Chanho Park -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Chanho, On Friday 09 of August 2013 16:40:53 Chanho Park wrote: > This patch describes each nodes of rotator and specifies a example how to > bind it. > > Signed-off-by: Chanho Park <chanho61.park@samsung.com> > Cc: Thomas Abraham <thomas.abraham@linaro.org> > Cc: Kukjin Kim <kgene.kim@samsung.com> > Cc: Inki Dae <inki.dae@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > .../devicetree/bindings/gpu/samsung-rotator.txt | 26 > ++++++++++++++++++++ 1 file changed, 26 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/gpu/samsung-rotator.txt > > diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.txt > b/Documentation/devicetree/bindings/gpu/samsung-rotator.txt new file > mode 100644 > index 0000000..31ee7b5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpu/samsung-rotator.txt > @@ -0,0 +1,26 @@ > +* Samsung Image Rotator > + > +Required properties: > + - compatible : value should be following: > + (a) "samsung,exynos4210-rotator" for Rotator IP in Exynos4210 > + (b) "samsung,exynos4212-rotator" for Rotator IP in Exynos4212/4412 > + (c) "samsung,exynos5250-rotator" for Rotator IP in Exynos5250 > + > + - reg : Physical base address of the IP registers and length of memory > + mapped region. > + > + - interrupts : Interrupt number of Rotator What about: interrupt specifier for rotator interrupt, according to format specific to interrupt parent. > + > + - clocks : Clock number described in > Documentations/devicetree/binding/clock. + Perhaps: clock specifier for rotator clock, according to generic clock bindings. > + - clock-names : Clock name. Names of clocks specified in "clocks" property. For exynos rotator this property should contain only one name: "rotator". > +Example: > + rotator: rotator@12800000 { I wonder if we shouldn't use a more generic name here, according to ePAPR node naming recommendation. > + compatible = "samsung,exynos4210-rotator"; > + reg = <0x12810000 0x1000>; Typo. Node has 12800000 in unit-address suffix, but this property has 12810000 instead. > + interrupts = <0 83 0>; > + clocks = <&clock 278>; > + clock-names = "rotator"; > + status = "disabled"; Status property should be omitted from this example, as it's not a part of this binding. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/09/2013 07:15 AM, Tomasz Figa wrote: > Hi Chanho, > > On Friday 09 of August 2013 16:40:53 Chanho Park wrote: >> This patch describes each nodes of rotator and specifies a example how to >> bind it. >> diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.txt >> +* Samsung Image Rotator >> + >> +Required properties: >> + - compatible : value should be following: >> + (a) "samsung,exynos4210-rotator" for Rotator IP in Exynos4210 >> + (b) "samsung,exynos4212-rotator" for Rotator IP in Exynos4212/4412 >> + (c) "samsung,exynos5250-rotator" for Rotator IP in Exynos5250 Is "rotator" the name that the HW documentation uses to refer to this block? If so, those compatible values seem fine. If not, they seem slightly too generic for me; perhaps better to use the raw HW block name? -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 09 of August 2013 10:38:30 Stephen Warren wrote: > On 08/09/2013 07:15 AM, Tomasz Figa wrote: > > Hi Chanho, > > > > On Friday 09 of August 2013 16:40:53 Chanho Park wrote: > >> This patch describes each nodes of rotator and specifies a example how > >> to bind it. > >> > >> diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.txt > >> > >> +* Samsung Image Rotator > >> + > >> +Required properties: > >> + - compatible : value should be following: > >> + (a) "samsung,exynos4210-rotator" for Rotator IP in Exynos4210 > >> + (b) "samsung,exynos4212-rotator" for Rotator IP in Exynos4212/4412 > >> + (c) "samsung,exynos5250-rotator" for Rotator IP in Exynos5250 > > Is "rotator" the name that the HW documentation uses to refer to this > block? Yes, it is. More specifically it is referred to as "Image Rotator". Best regards, Tomasz > If so, those compatible values seem fine. If not, they seem > slightly too generic for me; perhaps better to use the raw HW block name? -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Tomasz, > -----Original Message----- > From: Tomasz Figa [mailto:t.figa@samsung.com] > Sent: Friday, August 09, 2013 10:16 PM > To: Chanho Park > Cc: inki.dae@samsung.com; kgene.kim@samsung.com; dri- > devel@lists.freedesktop.org; kyungmin.park@samsung.com; > mark.rutland@arm.com; l.stach@pengutronix.de; s.nawrocki@samsung.com; > tomasz.figa@gmail.com; linux-samsung-soc@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; devicetree@vger.kernel.org; Thomas Abraham; > swarren@wwwdotorg.org; ian.campbell@citrix.com; rob.herring@calxeda.com; > pawel.moll@arm.com > Subject: Re: [PATCHv2 5/5] ARM: dts: Add dt binding documentation for > exynos rotator > > Hi Chanho, > > On Friday 09 of August 2013 16:40:53 Chanho Park wrote: > > This patch describes each nodes of rotator and specifies a example how > > to bind it. > > > > Signed-off-by: Chanho Park <chanho61.park@samsung.com> > > Cc: Thomas Abraham <thomas.abraham@linaro.org> > > Cc: Kukjin Kim <kgene.kim@samsung.com> > > Cc: Inki Dae <inki.dae@samsung.com> > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > > --- > > .../devicetree/bindings/gpu/samsung-rotator.txt | 26 > > ++++++++++++++++++++ 1 file changed, 26 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/gpu/samsung-rotator.txt > > > > diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.txt > > b/Documentation/devicetree/bindings/gpu/samsung-rotator.txt new file > > mode 100644 index 0000000..31ee7b5 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/gpu/samsung-rotator.txt > > @@ -0,0 +1,26 @@ > > +* Samsung Image Rotator > > + > > +Required properties: > > + - compatible : value should be following: > > + (a) "samsung,exynos4210-rotator" for Rotator IP in Exynos4210 > > + (b) "samsung,exynos4212-rotator" for Rotator IP in Exynos4212/4412 > > + (c) "samsung,exynos5250-rotator" for Rotator IP in Exynos5250 > > + > > + - reg : Physical base address of the IP registers and length of > memory > > + mapped region. > > + > > + - interrupts : Interrupt number of Rotator > > What about: interrupt specifier for rotator interrupt, according to > format specific to interrupt parent. > > > + > > + - clocks : Clock number described in > > Documentations/devicetree/binding/clock. + > > Perhaps: clock specifier for rotator clock, according to generic clock > bindings. Become more clearly. Thanks. > > > + - clock-names : Clock name. > > Names of clocks specified in "clocks" property. For exynos rotator this > property should contain only one name: "rotator". > > > +Example: > > + rotator: rotator@12800000 { > > I wonder if we shouldn't use a more generic name here, according to > ePAPR node naming recommendation. Hmm. I'm not sure what name is better than rotator. How about image-rotator@12810000 > > > + compatible = "samsung,exynos4210-rotator"; > > + reg = <0x12810000 0x1000>; > > Typo. Node has 12800000 in unit-address suffix, but this property has > 12810000 instead. Yes. It's typo. I'll correct it now. > > > + interrupts = <0 83 0>; > > + clocks = <&clock 278>; > > + clock-names = "rotator"; > > + status = "disabled"; > > Status property should be omitted from this example, as it's not a part > of this binding. I'll remove the property. Thanks. Best Regards, Chanho Park -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 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/gpu/samsung-rotator.txt b/Documentation/devicetree/bindings/gpu/samsung-rotator.txt new file mode 100644 index 0000000..31ee7b5 --- /dev/null +++ b/Documentation/devicetree/bindings/gpu/samsung-rotator.txt @@ -0,0 +1,26 @@ +* Samsung Image Rotator + +Required properties: + - compatible : value should be following: + (a) "samsung,exynos4210-rotator" for Rotator IP in Exynos4210 + (b) "samsung,exynos4212-rotator" for Rotator IP in Exynos4212/4412 + (c) "samsung,exynos5250-rotator" for Rotator IP in Exynos5250 + + - reg : Physical base address of the IP registers and length of memory + mapped region. + + - interrupts : Interrupt number of Rotator + + - clocks : Clock number described in Documentations/devicetree/binding/clock. + + - clock-names : Clock name. + +Example: + rotator: rotator@12800000 { + compatible = "samsung,exynos4210-rotator"; + reg = <0x12810000 0x1000>; + interrupts = <0 83 0>; + clocks = <&clock 278>; + clock-names = "rotator"; + status = "disabled"; + };