From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Figa Subject: Re: [PATCH v3 1/6] dts: exynos: add device tree support for exynos5 hdmi Date: Wed, 17 Oct 2012 13:49:38 +0200 Message-ID: <1928447.6kgkcZq6oY@amdc1227> References: <1350343834-23992-1-git-send-email-rahul.sharma@samsung.com> <1350343834-23992-2-git-send-email-rahul.sharma@samsung.com> <006101cdac5b$08e06ee0$1aa14ca0$%kim@samsung.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2974919636818003418==" Return-path: In-reply-to: <006101cdac5b$08e06ee0$1aa14ca0$%kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: Kukjin Kim Cc: t.stanislaws-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, jy0922.shim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, sw0312.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, joshi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, inki.dae-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, thomas.ab-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, s.shirish-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, r.sh.open-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, prashanth.g-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, 'Rahul Sharma' List-Id: devicetree@vger.kernel.org This is a multi-part message in MIME format. --===============2974919636818003418== Content-type: multipart/alternative; boundary=nextPart1416501.9iVMmlV1Qx Content-transfer-encoding: 7Bit This is a multi-part message in MIME format. --nextPart1416501.9iVMmlV1Qx Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Hi Rahul, Kgene, On Wednesday 17 of October 2012 20:32:05 Kukjin Kim wrote: > Rahul Sharma wrote: > > This patch adds support for device tree based discovery for exynos5 > > hdmi. Hdmi node is also renamed with "exynos5-hdmi". > > > > Signed-off-by: Rahul Sharma > > --- > > > > .../devicetree/bindings/drm/exynos/hdmi.txt | 22 > > > > ++++++++++++++++++++ > > > > arch/arm/boot/dts/exynos5250-smdk5250.dts | 4 +++ > > arch/arm/boot/dts/exynos5250.dtsi | 6 +++++ > > arch/arm/mach-exynos/include/mach/map.h | 1 + > > arch/arm/mach-exynos/mach-exynos5-dt.c | 2 + > > 5 files changed, 35 insertions(+), 0 deletions(-) > > create mode 100644 > > Documentation/devicetree/bindings/drm/exynos/hdmi.txt > [...] > > > + hdmi { > > + compatible = "samsung,exynos5-hdmi"; > > + reg = <0x14530000 0x100000>; > > + interrupts = <0 95 0>; > > + hpd-gpio = <&gpx3 7 0xf 1 3>; > > + }; > > \ No newline at end of file > > What's this? > > [...] > > > + hdmi@14530000 { > > + hdmi { > Isn't there a convention to use @ADDR whenever a node contains a reg property? Please see exynos4.dtsi, all the nodes with reg have @ADDR. > > + hpd-gpio = <&gpx3 7 0xf 1 3>; > > + }; > > > > }; > > > > diff --git a/arch/arm/boot/dts/exynos5250.dtsi > > b/arch/arm/boot/dts/exynos5250.dtsi > > index f69e389..ec7ea2d 100644 > > --- a/arch/arm/boot/dts/exynos5250.dtsi > > +++ b/arch/arm/boot/dts/exynos5250.dtsi > > @@ -492,4 +492,10 @@ > > > > #gpio-cells = <4>; > > > > }; > > > > }; > > > > + > > + hdmi@14530000 { > > Ditto. > Ditto. > [...] > > > +#define EXYNOS5_PA_HDMI 0x14530000 > > I think, we don't need this because see below. > > [...] > > > + OF_DEV_AUXDATA("samsung,exynos5-hdmi", EXYNOS5_PA_HDMI, > > + "exynos5-hdmi", NULL), > > + OF_DEV_AUXDATA("samsung,exynos5-hdmi", 0x14530000, "exynos5-hdmi", > NULL), > > I think, if the address as defined above is used only here, let's just > the value because nobody don't know it is the address. I don't like the idea of having such magic numbers in code, but since the whole auxdata array is going to be removed in future anyway, I guess this is OK. Best regards, -- Tomasz Figa Samsung Poland R&D Center --nextPart1416501.9iVMmlV1Qx Content-Transfer-Encoding: 7Bit Content-Type: text/html; charset="us-ascii"

Hi Rahul, Kgene,

 

On Wednesday 17 of October 2012 20:32:05 Kukjin Kim wrote:

> Rahul Sharma wrote:

> > This patch adds support for device tree based discovery for exynos5

> > hdmi. Hdmi node is also renamed with "exynos5-hdmi".

> >

> > Signed-off-by: Rahul Sharma <rahul.sharma-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

> > ---

> >

> > .../devicetree/bindings/drm/exynos/hdmi.txt | 22

> >

> > ++++++++++++++++++++

> >

> > arch/arm/boot/dts/exynos5250-smdk5250.dts | 4 +++

> > arch/arm/boot/dts/exynos5250.dtsi | 6 +++++

> > arch/arm/mach-exynos/include/mach/map.h | 1 +

> > arch/arm/mach-exynos/mach-exynos5-dt.c | 2 +

> > 5 files changed, 35 insertions(+), 0 deletions(-)

> > create mode 100644

> > Documentation/devicetree/bindings/drm/exynos/hdmi.txt

> [...]

>

> > + hdmi {

> > + compatible = "samsung,exynos5-hdmi";

> > + reg = <0x14530000 0x100000>;

> > + interrupts = <0 95 0>;

> > + hpd-gpio = <&gpx3 7 0xf 1 3>;

> > + };

> > \ No newline at end of file

>

> What's this?

>

> [...]

>

> > + hdmi@14530000 {

>

> + hdmi {

>

 

Isn't there a convention to use @ADDR whenever a node contains a reg property? Please see exynos4.dtsi, all the nodes with reg have @ADDR.

 

> > + hpd-gpio = <&gpx3 7 0xf 1 3>;

> > + };

> >

> > };

> >

> > diff --git a/arch/arm/boot/dts/exynos5250.dtsi

> > b/arch/arm/boot/dts/exynos5250.dtsi

> > index f69e389..ec7ea2d 100644

> > --- a/arch/arm/boot/dts/exynos5250.dtsi

> > +++ b/arch/arm/boot/dts/exynos5250.dtsi

> > @@ -492,4 +492,10 @@

> >

> > #gpio-cells = <4>;

> >

> > };

> >

> > };

> >

> > +

> > + hdmi@14530000 {

>

> Ditto.

>

 

Ditto.

 

> [...]

>

> > +#define EXYNOS5_PA_HDMI 0x14530000

>

> I think, we don't need this because see below.

>

> [...]

>

> > + OF_DEV_AUXDATA("samsung,exynos5-hdmi", EXYNOS5_PA_HDMI,

> > + "exynos5-hdmi", NULL),

>

> + OF_DEV_AUXDATA("samsung,exynos5-hdmi", 0x14530000, "exynos5-hdmi",

> NULL),

>

> I think, if the address as defined above is used only here, let's just

> the value because nobody don't know it is the address.

 

I don't like the idea of having such magic numbers in code, but since the whole auxdata array is going to be removed in future anyway, I guess this is OK.

 

Best regards,

--

Tomasz Figa

Samsung Poland R&D Center

 

--nextPart1416501.9iVMmlV1Qx-- --===============2974919636818003418== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ devicetree-discuss mailing list devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org https://lists.ozlabs.org/listinfo/devicetree-discuss --===============2974919636818003418==--