devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tomasz Figa <tomasz.figa@gmail.com>
To: Rahul Sharma <r.sh.open@gmail.com>
Cc: Rahul Sharma <rahul.sharma@samsung.com>,
	linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
	devicetree@vger.kernel.org, Kukjin Kim <kgene.kim@samsung.com>,
	"sw0312.kim" <sw0312.kim@samsung.com>,
	Inki Dae <inki.dae@samsung.com>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	sunil joshi <joshi@samsung.com>
Subject: Re: [PATCH v4 4/7] ARM: dts: add dt nodes for exynos5420 hdmi subsystem
Date: Sat, 24 Aug 2013 16:32:59 +0200	[thread overview]
Message-ID: <3791850.fQhgslc95O@flatron> (raw)
In-Reply-To: <CAPdUM4N9cBfJFL1XMXug+QjbCdgq9b9u6u6ViH3Rc4Bh-5MOOQ@mail.gmail.com>

On Saturday 24 of August 2013 12:05:36 Rahul Sharma wrote:
> > Hi Rahul,
> > 
> > Please see my comments inline.
> > 
> > On Friday 23 of August 2013 15:54:18 Rahul Sharma wrote:
> >> Add hdmi, mixer, ddc device tree nodes for Exynos 5420 SoC.
> >> 
> >> Signed-off-by: Rahul Sharma <rahul.sharma@samsung.com>
> >> ---
> >> 
> >>  arch/arm/boot/dts/exynos5420-smdk5420.dts |   19 +++++++++++++++++++
> >>  arch/arm/boot/dts/exynos5420.dtsi         |   14 ++++++++++++++
> >>  2 files changed, 33 insertions(+)
> >> 
> >> diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts
> >> b/arch/arm/boot/dts/exynos5420-smdk5420.dts index bafba25..7dbf1fa
> >> 100644
> >> --- a/arch/arm/boot/dts/exynos5420-smdk5420.dts
> >> +++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts
> >> @@ -61,4 +61,23 @@
> >> 
> >>               };
> >>       
> >>       };
> >> 
> >> +     hdmi@14530000 {
> >> +             status = "okay";
> >> +             hpd-gpio = <&gpx3 7 0>;
> >> +     };
> >> +
> >> +     mixer@14450000 {
> >> +             status = "okay";
> >> +     };
> >> +
> >> +     i2c_2: i2c@12C80000 {
> >> +             samsung,i2c-sda-delay = <100>;
> >> +             samsung,i2c-max-bus-freq = <66000>;
> >> +             status = "okay";
> >> +
> >> +             hdmiddc@50 {
> >> +                     compatible = "samsung,exynos4210-hdmiddc";
> >> +                     reg = <0x50>;
> >> +             };
> >> +     };
> >> 
> >>  };
> > 
> > You should separate above board-specific changes from this patch and
> > squash it with patch 6/7, adjusting patch subject and description
> > appropriately.
> > 
> >> diff --git a/arch/arm/boot/dts/exynos5420.dtsi
> >> b/arch/arm/boot/dts/exynos5420.dtsi index 05dee86..7c6214d 100644
> >> --- a/arch/arm/boot/dts/exynos5420.dtsi
> >> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> >> @@ -274,4 +274,18 @@
> >> 
> >>               pinctrl-0 = <&i2c3_bus>;
> >>               status = "disabled";
> >>       
> >>       };
> >> 
> >> +
> >> +     hdmi@14530000 {
> >> +             compatible = "samsung,exynos4212-hdmi";
> >> +             reg = <0x14530000 0x70000>;
> >> +             interrupts = <0 95 0>;
> >> +             status = "disabled";
> >> +     };
> >> +
> >> +     mixer@14450000 {
> >> +             compatible = "samsung,exynos5420-mixer";
> >> +             reg = <0x14450000 0x10000>;
> >> +             interrupts = <0 94 0>;
> >> +             status = "disabled";
> > 
> > This node looks like a candidate to be enabled here on SoC level,
> > since it does not require any board specific information.
> 
> Hi Tomasz,
> 
> Both hdmi and mixer should be enabled at same time. Mixer don't
> but hdmi needs hpd gpio information which is board dependent. Thats
> why I kept them disabled in SoC file. What you say?

Hmm, is it a hardware restriction? I mean, if HDMI is not enabled, is it 
impossible to enable mixer and use it for some purposes? I don't know much 
about the Exynos HDMI subsystem, so sorry for the noise if this is 
obvious.

What I have in mind is that device tree should describe the hardware, 
regardless of use case. This is, all the hardware that can possibly 
operate should be listed as operational.

Best regards,
Tomasz


  reply	other threads:[~2013-08-24 14:33 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-23 10:24 [PATCH v4 0/7] ARM: dts: add support for exynos hdmi subsystem Rahul Sharma
2013-08-23 10:24 ` [PATCH v4 1/7] ARM: dts: add mixer clocks to mixer node for Exynos5250 Rahul Sharma
2013-08-23 10:24 ` [PATCH v4 2/7] ARM: dts: add clocks to hdmi dt node for exynos5250 Rahul Sharma
2013-08-23 10:24 ` [PATCH v4 3/7] ARM: dts: add i2c device nodes for Exynos5420 Rahul Sharma
2013-08-23 10:24 ` [PATCH v4 4/7] ARM: dts: add dt nodes for exynos5420 hdmi subsystem Rahul Sharma
2013-08-23 19:48   ` Tomasz Figa
2013-08-24  6:35     ` Rahul Sharma
2013-08-24 14:32       ` Tomasz Figa [this message]
2013-08-26  5:12         ` Rahul Sharma
2013-08-23 10:24 ` [PATCH v4 5/7] ARM: dts: add clocks " Rahul Sharma
2013-08-23 19:43   ` Tomasz Figa
2013-08-23 10:24 ` [PATCH v4 6/7] ARM: dts: add hdmi hpd gpio pinctrl node for exynos5420 Rahul Sharma
2013-08-23 10:24 ` [PATCH v4 7/7] of/documentation: update with clock information for exynos hdmi subsystem Rahul Sharma

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3791850.fQhgslc95O@flatron \
    --to=tomasz.figa@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=inki.dae@samsung.com \
    --cc=joshi@samsung.com \
    --cc=kgene.kim@samsung.com \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=r.sh.open@gmail.com \
    --cc=rahul.sharma@samsung.com \
    --cc=s.nawrocki@samsung.com \
    --cc=sw0312.kim@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).