| Message ID | 1377253461-20851-5-git-send-email-rahul.sharma@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. 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
> 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? Regards, Rahul Sharma. > 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 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 -- 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 24 August 2013 20:02, Tomasz Figa <tomasz.figa@gmail.com> wrote: > 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. Hi Tomasz, No, It is not a hardware restriction. Mixer can be probed independently but there is NO usecase where mixer can be used without hdmi enabled. Mixer output is hardwired to hdmi block in SoC. > > 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. I understand your point. We can keep mixer enabled in DTSI. regards, Rahul Sharma. > > 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
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>; + }; + }; }; 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"; + }; };
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(+)