Re: [PATCH V6 02/10] ASoC: SAMSUNG: Add DT support for i2s
Hi, On Fri, Feb 15, 2013 at 5:31 PM, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Thu, Feb 14, 2013 at 09:33:00PM +0100, Sylwester Nawrocki wrote: My apologies for the late review. It's already sixth version of this patch series... But I noticed it just now when applying it for Exynos4. The GPIO part of the bindings is now incompatible with Exynos4 and the bindings will need to be changed. Leaving the compatibility code in the I2S driver, when we could have it done well right from the beginning. If I'm not mistaken a file similar to arch/arm/boot/dts/exynos4x12-pinctrl.dtsi with at least entries for the I2S devices is needed and an addition of related pinctrl nodes in exynos5250.dtsi. But I guess this is not something that could be fixed in the -rc period ? Yes, and incremental fix would be fine. I will send a patch for required pinctrl support soon. Thanks 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
Re: [PATCH V6 02/10] ASoC: SAMSUNG: Add DT support for i2s
On Thu, Feb 14, 2013 at 09:33:00PM +0100, Sylwester Nawrocki wrote: My apologies for the late review. It's already sixth version of this patch series... But I noticed it just now when applying it for Exynos4. The GPIO part of the bindings is now incompatible with Exynos4 and the bindings will need to be changed. Leaving the compatibility code in the I2S driver, when we could have it done well right from the beginning. If I'm not mistaken a file similar to arch/arm/boot/dts/exynos4x12-pinctrl.dtsi with at least entries for the I2S devices is needed and an addition of related pinctrl nodes in exynos5250.dtsi. But I guess this is not something that could be fixed in the -rc period ? Yes, and incremental fix would be fine. signature.asc Description: Digital signature
Re: [PATCH V6 02/10] ASoC: SAMSUNG: Add DT support for i2s
Hi,
On 01/18/2013 12:47 PM, Padmavathi Venna wrote:
Add support for device based discovery.
Signed-off-by: Padmavathi Venna padm...@samsung.com
---
.../devicetree/bindings/sound/samsung-i2s.txt | 63 ++
sound/soc/samsung/dma.c|3 +-
sound/soc/samsung/dma.h|1 +
sound/soc/samsung/i2s.c| 209
+++-
4 files changed, 230 insertions(+), 46 deletions(-)
create mode 100644 Documentation/devicetree/bindings/sound/samsung-i2s.txt
diff --git a/Documentation/devicetree/bindings/sound/samsung-i2s.txt
b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
new file mode 100644
index 000..3070046
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
@@ -0,0 +1,63 @@
+* Samsung I2S controller
+
...
+Required Board Specific Properties:
+
+- gpios: The gpio specifier for data out,data in, LRCLK, CDCLK and SCLK
+ interface lines. The format of the gpio specifier depends on the gpio
+ controller.
+ The syntax of samsung gpio specifier is
+ [phandle of the gpio controller node]
+ [pin number within the gpio controller]
+ [mux function]
+ [flags and pull up/down]
+ [drive strength]
I don't think there is a need to copy the gpio specifier documentation
over to this file. Moreover this gpio specifier was supposed to be temporary,
until pinctrl support is added. Now Exynos4 uses generic gpio specifiers
and pinctrl for pin muxing and Exynos5 is still not converted to that,
although I believe this wouldn't require much effort now.
+Example:
+
...
+- Board Specific Portion:
+
+i2s@0383 {
+ gpios = gpz 0 2 0 0, /* I2S_0_SCLK */
+ gpz 1 2 0 0, /* I2S_0_CDCLK */
+ gpz 2 2 0 0, /* I2S_0_LRCK */
+ gpz 3 2 0 0, /* I2S_0_SDI */
+ gpz 4 2 0 0, /* I2S_0_SDO[1] */
+ gpz 5 2 0 0, /* I2S_0_SDO[2] */
+ gpz 6 2 0 0; /* I2S_0_SDO[3] */
+};
We should switch to pinctrl instead. Most of work now seems to be
to define pinctrl nodes for each Exynos5 device, so there is proper
pinctrl support for Exynos5.
--
Thanks,
Sylwester
--
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 V6 02/10] ASoC: SAMSUNG: Add DT support for i2s
On 01/18/2013 12:47 PM, Padmavathi Venna wrote:
Add support for device based discovery.
Signed-off-by: Padmavathi Venna padm...@samsung.com
---
...
/* Lock for cross i/f checks */
@@ -997,19 +1006,76 @@ static struct i2s_dai *i2s_alloc_dai(struct
platform_device *pdev, bool sec)
return i2s;
}
+#ifdef CONFIG_OF
+static int samsung_i2s_parse_dt_gpio(struct i2s_dai *i2s)
+{
+ struct device *dev = i2s-pdev-dev;
+ int index, gpio, ret;
+
+ for (index = 0; index 7; index++) {
+ gpio = of_get_gpio(dev-of_node, index);
+ if (!gpio_is_valid(gpio)) {
+ dev_err(dev, invalid gpio[%d]: %d\n, index, gpio);
+ goto free_gpio;
+ }
+
+ ret = gpio_request(gpio, dev_name(dev));
+ if (ret) {
+ dev_err(dev, gpio [%d] request failed\n, gpio);
+ goto free_gpio;
+ }
+ i2s-gpios[index] = gpio;
+ }
+ return 0;
+
+free_gpio:
+ while (--index = 0)
+ gpio_free(i2s-gpios[index]);
+ return -EINVAL;
+}
+
+static void samsung_i2s_dt_gpio_free(struct i2s_dai *i2s)
+{
+ unsigned int index;
+ for (index = 0; index 7; index++)
+ gpio_free(i2s-gpios[index]);
+}
+#else
+static int samsung_i2s_parse_dt_gpio(struct i2s_dai *dai)
+{
+ return -EINVAL;
+}
+
+static void samsung_i2s_dt_gpio_free(struct i2s_dai *dai)
+{
+}
+
+#endif
NAK.
Why we should leave with this temporary code when there is already
pinctrl support for Exynos SoCs ?
If the pinctrl driver for Exynos5 was updated you could instead just do
devm_pinctrl_get_select_default(i2s-pdev-dev);
--
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 V6 02/10] ASoC: SAMSUNG: Add DT support for i2s
Hi,
On Thursday 14 of February 2013 11:48:59 Sylwester Nawrocki wrote:
On 01/18/2013 12:47 PM, Padmavathi Venna wrote:
Add support for device based discovery.
Signed-off-by: Padmavathi Venna padm...@samsung.com
---
...
/* Lock for cross i/f checks */
@@ -997,19 +1006,76 @@ static struct i2s_dai *i2s_alloc_dai(struct
platform_device *pdev, bool sec)
return i2s;
}
+#ifdef CONFIG_OF
+static int samsung_i2s_parse_dt_gpio(struct i2s_dai *i2s)
+{
+ struct device *dev = i2s-pdev-dev;
+ int index, gpio, ret;
+
+ for (index = 0; index 7; index++) {
+ gpio = of_get_gpio(dev-of_node, index);
+ if (!gpio_is_valid(gpio)) {
+ dev_err(dev, invalid gpio[%d]: %d\n, index, gpio);
+ goto free_gpio;
+ }
+
+ ret = gpio_request(gpio, dev_name(dev));
+ if (ret) {
+ dev_err(dev, gpio [%d] request failed\n, gpio);
+ goto free_gpio;
+ }
+ i2s-gpios[index] = gpio;
+ }
+ return 0;
+
+free_gpio:
+ while (--index = 0)
+ gpio_free(i2s-gpios[index]);
+ return -EINVAL;
+}
+
+static void samsung_i2s_dt_gpio_free(struct i2s_dai *i2s)
+{
+ unsigned int index;
+ for (index = 0; index 7; index++)
+ gpio_free(i2s-gpios[index]);
+}
+#else
+static int samsung_i2s_parse_dt_gpio(struct i2s_dai *dai)
+{
+ return -EINVAL;
+}
+
+static void samsung_i2s_dt_gpio_free(struct i2s_dai *dai)
+{
+}
+
+#endif
NAK.
+1
Why we should leave with this temporary code when there is already
pinctrl support for Exynos SoCs ?
If the pinctrl driver for Exynos5 was updated you could instead just do
devm_pinctrl_get_select_default(i2s-pdev-dev);
I said it already by the way of some other Exynos5 patches that Exynos5
has to be migrated to pin control.
The legacy GPIO interface was supposed to be kept only for compatibility
with existing code (until it gets migrated to pin control), not for adding
new code relying on it.
If you intend to add new code which needs to configure pin muxes, you
should migrate Exynos5 to pin control first. This won't be particularly
hard, since pinctrl-exynos driver already supports Exynos5 and all that is
missing are Device Tree nodes for it.
Best regards,
--
Tomasz Figa
Samsung Poland RD Center
SW Solution Development, Linux Platform
--
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 V6 02/10] ASoC: SAMSUNG: Add DT support for i2s
On Thu, Feb 14, 2013 at 11:48:59AM +0100, Sylwester Nawrocki wrote: NAK. Why we should leave with this temporary code when there is already pinctrl support for Exynos SoCs ? If the pinctrl driver for Exynos5 was updated you could instead just do devm_pinctrl_get_select_default(i2s-pdev-dev); Too late, this stuff is already merged and on its way upstream. signature.asc Description: Digital signature
[PATCH V6 02/10] ASoC: SAMSUNG: Add DT support for i2s
Add support for device based discovery.
Signed-off-by: Padmavathi Venna padm...@samsung.com
---
.../devicetree/bindings/sound/samsung-i2s.txt | 63 ++
sound/soc/samsung/dma.c|3 +-
sound/soc/samsung/dma.h|1 +
sound/soc/samsung/i2s.c| 209 +++-
4 files changed, 230 insertions(+), 46 deletions(-)
create mode 100644 Documentation/devicetree/bindings/sound/samsung-i2s.txt
diff --git a/Documentation/devicetree/bindings/sound/samsung-i2s.txt
b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
new file mode 100644
index 000..3070046
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/samsung-i2s.txt
@@ -0,0 +1,63 @@
+* Samsung I2S controller
+
+Required SoC Specific Properties:
+
+- compatible : samsung,i2s-v5
+- reg: physical base address of the controller and length of memory mapped
+ region.
+- dmas: list of DMA controller phandle and DMA request line ordered pairs.
+- dma-names: identifier string for each DMA request line in the dmas property.
+ These strings correspond 1:1 with the ordered pairs in dmas.
+
+Optional SoC Specific Properties:
+
+- samsung,supports-6ch: If the I2S Primary sound source has 5.1 Channel
+ support, this flag is enabled.
+- samsung,supports-rstclr: This flag should be set if I2S software reset bit
+ control is required. When this flag is set I2S software reset bit will be
+ enabled or disabled based on need.
+- samsung,supports-secdai:If I2S block has a secondary FIFO and internal DMA,
+ then this flag is enabled.
+- samsung,idma-addr: Internal DMA register base address of the audio
+ sub system(used in secondary sound source).
+
+Required Board Specific Properties:
+
+- gpios: The gpio specifier for data out,data in, LRCLK, CDCLK and SCLK
+ interface lines. The format of the gpio specifier depends on the gpio
+ controller.
+ The syntax of samsung gpio specifier is
+ [phandle of the gpio controller node]
+[pin number within the gpio controller]
+[mux function]
+[flags and pull up/down]
+[drive strength]
+
+Example:
+
+- SoC Specific Portion:
+
+i2s@0383 {
+ compatible = samsung,i2s-v5;
+ reg = 0x0383 0x100;
+ dmas = pdma0 10
+ pdma0 9
+ pdma0 8;
+ dma-names = tx, rx, tx-sec;
+ samsung,supports-6ch;
+ samsung,supports-rstclr;
+ samsung,supports-secdai;
+ samsung,idma-addr = 0x0300;
+};
+
+- Board Specific Portion:
+
+i2s@0383 {
+ gpios = gpz 0 2 0 0, /* I2S_0_SCLK */
+ gpz 1 2 0 0, /* I2S_0_CDCLK */
+ gpz 2 2 0 0, /* I2S_0_LRCK */
+ gpz 3 2 0 0, /* I2S_0_SDI */
+ gpz 4 2 0 0, /* I2S_0_SDO[1] */
+ gpz 5 2 0 0, /* I2S_0_SDO[2] */
+ gpz 6 2 0 0; /* I2S_0_SDO[3] */
+};
diff --git a/sound/soc/samsung/dma.c b/sound/soc/samsung/dma.c
index db87628..21b7926 100644
--- a/sound/soc/samsung/dma.c
+++ b/sound/soc/samsung/dma.c
@@ -174,7 +174,8 @@ static int dma_hw_params(struct snd_pcm_substream
*substream,
config.width = prtd-params-dma_size;
config.fifo = prtd-params-dma_addr;
prtd-params-ch = prtd-params-ops-request(
- prtd-params-channel, req);
+ prtd-params-channel, req, rtd-cpu_dai-dev,
+ prtd-params-ch_name);
prtd-params-ops-config(prtd-params-ch, config);
}
diff --git a/sound/soc/samsung/dma.h b/sound/soc/samsung/dma.h
index 73d8c7c..189a7a6 100644
--- a/sound/soc/samsung/dma.h
+++ b/sound/soc/samsung/dma.h
@@ -19,6 +19,7 @@ struct s3c_dma_params {
int dma_size; /* Size of the DMA transfer */
unsigned ch;
struct samsung_dma_ops *ops;
+ char *ch_name;
};
int asoc_dma_platform_register(struct device *dev);
diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index ed5eeae..9e9c20c 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -15,11 +15,15 @@
#include linux/clk.h
#include linux/io.h
#include linux/module.h
+#include linux/of.h
+#include linux/of_gpio.h
#include linux/pm_runtime.h
#include sound/soc.h
#include sound/pcm_params.h
+#include mach/dma.h
+
#include linux/platform_data/asoc-s3c.h
#include dma.h
@@ -34,6 +38,10 @@ enum samsung_dai_type {
TYPE_SEC,
};
+struct samsung_i2s_dai_data {
+ int dai_type;
+};
+
struct i2s_dai {
/* Platform device for this DAI */
struct platform_device *pdev;
@@ -71,6 +79,7 @@ struct i2s_dai {
u32 suspend_i2smod;
u32 suspend_i2scon;
u32 suspend_i2spsr;
+ unsigned long gpios[7]; /* i2s gpio line numbers */
};
/* Lock for cross i/f checks */
@@ -997,19 +1006,76 @@ static struct i2s_dai *i2s_alloc_dai(struct
platform_device *pdev, bool sec)
