Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers
On Wednesday 14 of August 2013 11:21:40 Padma Venkat wrote: Hi Tomasz, On Mon, Aug 12, 2013 at 10:32 PM, Tomasz Figa tomasz.f...@gmail.com wrote: On Monday 12 of August 2013 14:12:36 Mark Brown wrote: On Mon, Aug 12, 2013 at 01:41:23PM +0200, Tomasz Figa wrote: On Monday 12 of August 2013 12:34:48 Mark Brown wrote: I'd expect that to interact badly with the pinmuxing - unless the device is disabled it'll try to grab its pins on probe which is not going to be a good idea unless it is actually wired up for use in the system. Or is there some other mechanism for handling that? Ah, good point. Now I wonder whether pinctrl nodes shouldn't be considered board-specific and specified in board-level dts instead? It seems a bit cleaner to use the current mechanism in that it stops the device appearing at all and hence repeated efforts to probe, plus a simple enable is less error prone, the way these SoCs are designed you don't have to pick which pinmux is in use for most of the IPs. Where there are multiple options it does seem like a good approach though. Tastes may differ though. Right, if this SoC has only one pinmux setting for this IP, then it's fine. Yes. This IP has only default pin configuration. Padmavathi, this was the only issue I spotted, so have my: Reviewed-by: Tomasz Figa t.f...@samsung.com Thanks for your review. You're welcome. Thanks for keeping up with this series and addressing all the comments. :) Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers
Hi Tomasz, On Mon, Aug 12, 2013 at 10:32 PM, Tomasz Figa tomasz.f...@gmail.com wrote: On Monday 12 of August 2013 14:12:36 Mark Brown wrote: On Mon, Aug 12, 2013 at 01:41:23PM +0200, Tomasz Figa wrote: On Monday 12 of August 2013 12:34:48 Mark Brown wrote: I'd expect that to interact badly with the pinmuxing - unless the device is disabled it'll try to grab its pins on probe which is not going to be a good idea unless it is actually wired up for use in the system. Or is there some other mechanism for handling that? Ah, good point. Now I wonder whether pinctrl nodes shouldn't be considered board-specific and specified in board-level dts instead? It seems a bit cleaner to use the current mechanism in that it stops the device appearing at all and hence repeated efforts to probe, plus a simple enable is less error prone, the way these SoCs are designed you don't have to pick which pinmux is in use for most of the IPs. Where there are multiple options it does seem like a good approach though. Tastes may differ though. Right, if this SoC has only one pinmux setting for this IP, then it's fine. Yes. This IP has only default pin configuration. Padmavathi, this was the only issue I spotted, so have my: Reviewed-by: Tomasz Figa t.f...@samsung.com Thanks for your review. Best regards, Tomasz Best Regards, Padma -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers
From: Andrew Bresticker abres...@chromium.org
This adds device-tree bindings for the i2s controllers on Exynos 5420.
Signed-off-by: Andrew Bresticker abres...@chromium.org
Signed-off-by: Padmavathi Venna padm...@samsung.com
Reviewed-on: https://gerrit.chromium.org/gerrit/57713
---
arch/arm/boot/dts/exynos5420.dtsi | 32
1 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5420.dtsi
b/arch/arm/boot/dts/exynos5420.dtsi
index d2fdb87..8d57369 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -242,4 +242,36 @@
pinctrl-names = default;
pinctrl-0 = i2c3_bus;
};
+
+ i2s_0: i2s@0383 {
+ compatible = samsung,exynos5420-i2s;
+ dmas = adma 0
+ adma 2
+ adma 1;
+ dma-names = tx, rx, tx-sec;
+ clocks = clock_audss EXYNOS_I2S_BUS,
+ clock_audss EXYNOS_I2S_BUS,
+ clock_audss EXYNOS_SCLK_I2S;
+ clock-names = iis, i2s_opclk0, i2s_opclk1;
+ pinctrl-names = default;
+ pinctrl-0 = i2s0_bus;
+ status = disabled;
+ };
+
+ i2s_1: i2s@12D6 {
+ clocks = clock 275, clock 138;
+ clock-names = iis, i2s_opclk0;
+ pinctrl-names = default;
+ pinctrl-0 = i2s1_bus;
+ status = disabled;
+ };
+
+ i2s_2: i2s@12D7 {
+ clocks = clock 276, clock 139;
+ clock-names = iis, i2s_opclk0;
+ pinctrl-names = default;
+ pinctrl-0 = i2s2_bus;
+ status = disabled;
+ };
+
};
--
1.7.4.4
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers
Hi Padmavathi, Andrew,
On Monday 12 of August 2013 15:37:47 Padmavathi Venna wrote:
From: Andrew Bresticker abres...@chromium.org
This adds device-tree bindings for the i2s controllers on Exynos 5420.
Signed-off-by: Andrew Bresticker abres...@chromium.org
Signed-off-by: Padmavathi Venna padm...@samsung.com
Reviewed-on: https://gerrit.chromium.org/gerrit/57713
---
arch/arm/boot/dts/exynos5420.dtsi | 32
1 files changed, 32 insertions(+), 0
deletions(-)
diff --git a/arch/arm/boot/dts/exynos5420.dtsi
b/arch/arm/boot/dts/exynos5420.dtsi index d2fdb87..8d57369 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -242,4 +242,36 @@
pinctrl-names = default;
pinctrl-0 = i2c3_bus;
};
+
+ i2s_0: i2s@0383 {
+ compatible = samsung,exynos5420-i2s;
+ dmas = adma 0
+ adma 2
+ adma 1;
+ dma-names = tx, rx, tx-sec;
+ clocks = clock_audss EXYNOS_I2S_BUS,
+ clock_audss EXYNOS_I2S_BUS,
+ clock_audss EXYNOS_SCLK_I2S;
+ clock-names = iis, i2s_opclk0, i2s_opclk1;
+ pinctrl-names = default;
+ pinctrl-0 = i2s0_bus;
+ status = disabled;
If a node does not require any board-specific properties for the device to
operate properly, there is no point in disabling it, just to add a single
status property at board level.
+ };
+
+ i2s_1: i2s@12D6 {
+ clocks = clock 275, clock 138;
+ clock-names = iis, i2s_opclk0;
+ pinctrl-names = default;
+ pinctrl-0 = i2s1_bus;
+ status = disabled;
Ditto.
+ };
+
+ i2s_2: i2s@12D7 {
+ clocks = clock 276, clock 139;
+ clock-names = iis, i2s_opclk0;
+ pinctrl-names = default;
+ pinctrl-0 = i2s2_bus;
+ status = disabled;
Ditto.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers
On Mon, Aug 12, 2013 at 01:14:20PM +0200, Tomasz Figa wrote:
On Monday 12 of August 2013 15:37:47 Padmavathi Venna wrote:
+ i2s_0: i2s@0383 {
+ status = disabled;
If a node does not require any board-specific properties for the device to
operate properly, there is no point in disabling it, just to add a single
status property at board level.
I'd expect that to interact badly with the pinmuxing - unless the device
is disabled it'll try to grab its pins on probe which is not going to be
a good idea unless it is actually wired up for use in the system. Or is
there some other mechanism for handling that?
signature.asc
Description: Digital signature
Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers
On Monday 12 of August 2013 12:34:48 Mark Brown wrote:
On Mon, Aug 12, 2013 at 01:14:20PM +0200, Tomasz Figa wrote:
On Monday 12 of August 2013 15:37:47 Padmavathi Venna wrote:
+ i2s_0: i2s@0383 {
+ status = disabled;
If a node does not require any board-specific properties for the device
to operate properly, there is no point in disabling it, just to add a
single status property at board level.
I'd expect that to interact badly with the pinmuxing - unless the device
is disabled it'll try to grab its pins on probe which is not going to be
a good idea unless it is actually wired up for use in the system. Or is
there some other mechanism for handling that?
Ah, good point. Now I wonder whether pinctrl nodes shouldn't be considered
board-specific and specified in board-level dts instead?
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers
On Mon, Aug 12, 2013 at 01:41:23PM +0200, Tomasz Figa wrote: On Monday 12 of August 2013 12:34:48 Mark Brown wrote: I'd expect that to interact badly with the pinmuxing - unless the device is disabled it'll try to grab its pins on probe which is not going to be a good idea unless it is actually wired up for use in the system. Or is there some other mechanism for handling that? Ah, good point. Now I wonder whether pinctrl nodes shouldn't be considered board-specific and specified in board-level dts instead? It seems a bit cleaner to use the current mechanism in that it stops the device appearing at all and hence repeated efforts to probe, plus a simple enable is less error prone, the way these SoCs are designed you don't have to pick which pinmux is in use for most of the IPs. Where there are multiple options it does seem like a good approach though. Tastes may differ though. signature.asc Description: Digital signature
Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers
On Monday 12 of August 2013 14:12:36 Mark Brown wrote: On Mon, Aug 12, 2013 at 01:41:23PM +0200, Tomasz Figa wrote: On Monday 12 of August 2013 12:34:48 Mark Brown wrote: I'd expect that to interact badly with the pinmuxing - unless the device is disabled it'll try to grab its pins on probe which is not going to be a good idea unless it is actually wired up for use in the system. Or is there some other mechanism for handling that? Ah, good point. Now I wonder whether pinctrl nodes shouldn't be considered board-specific and specified in board-level dts instead? It seems a bit cleaner to use the current mechanism in that it stops the device appearing at all and hence repeated efforts to probe, plus a simple enable is less error prone, the way these SoCs are designed you don't have to pick which pinmux is in use for most of the IPs. Where there are multiple options it does seem like a good approach though. Tastes may differ though. Right, if this SoC has only one pinmux setting for this IP, then it's fine. Padmavathi, this was the only issue I spotted, so have my: Reviewed-by: Tomasz Figa t.f...@samsung.com Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
