* Re: [GIT PULL] ASoC: Samsung: Updates for v3.8 [not found] <0MDZ007SI86SXBD0@mailout4.samsung.com> @ 2012-11-24 17:26 ` Mark Brown 0 siblings, 0 replies; 11+ messages in thread From: Mark Brown @ 2012-11-24 17:26 UTC (permalink / raw) To: PADMAVATHI VENNA; +Cc: Sangbeom Kim, alsa-devel@alsa-project.org [-- Attachment #1.1: Type: text/plain, Size: 487 bytes --] On Sat, Nov 24, 2012 at 05:20:52AM +0000, PADMAVATHI VENNA wrote: > <HTML><HEAD><TITLE>Samsung Enterprise Portal mySingle</TITLE> > <META content="text/html; charset=windows-1252" http-equiv=Content-Type> > <STYLE id=mysingle_style type=text/css>P { > MARGIN-TOP: 5px; FONT-FAMILY: Arial, arial; MARGIN-BOTTOM: 5px; FONT-SIZE: 9pt > } Please fix your mailer, the above is exactly how your mail comes out in my mail client (modulo me deleting the rest of it) - there's no text version. [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <001601cdcd28$ce5204c0$6af60e40$@com>]
* Re: [GIT PULL] ASoC: Samsung: Updates for v3.8 [not found] <001601cdcd28$ce5204c0$6af60e40$@com> @ 2012-11-28 5:51 ` Padma Venkat 0 siblings, 0 replies; 11+ messages in thread From: Padma Venkat @ 2012-11-28 5:51 UTC (permalink / raw) To: Mark Brown Cc: alsa-devel, linux-arm-kernel, linux-samsung-soc, Sangbeom Kim, Kukjin Kim, devicetree-discuss, padma venkat Hi Mark, On Wed, Nov 28, 2012 at 10:55 AM, Sangbeom Kim <sbkim73@samsung.com> wrote: >> > There's some problems with this binding. The main one is the gpios >> > property the format of which isn't specified at all. > >> All of above gpio property is i2s. >> That is, >> + gpios = <&gpz 0 2 0 0>, -> SCLK >> + <&gpz 1 2 0 0>, -> CDCLK >> + <&gpz 2 2 0 0>, -> LRCK >> + <&gpz 3 2 0 0>, -> SDI >> + <&gpz 4 2 0 0>, -> SDO[0] >> + <&gpz 5 2 0 0>, -> SDO[1] >> + <&gpz 6 2 0 0>; -> SDO[2] > >> Do you want like a below one? >> +sclk-gpios = <&gpz 0 2 0 0>, >> +cdclk-gpios = <&gpz 1 2 0 0>, ... > > That would be nice but what I was really looking for was some indiciation in the binding document as > to what all this stuff means - it should really say what gpz is (though I can figure that out) and > it definitely needs to say what the four numbers are. > >> > The requirement for an alias is also very odd, where does that come from? > >> I don't know that Which one is odd. Please let me know. > > Having them at all is odd - it's not something other DT bindings have needed and there was nothing > saying what it was for. What is the exact requirement here? Should I remove the alias ids or I2S1 and I2S2 nodes as they are in disabled state? This is done same way as SPI and I2C. Please explain me. > >> > Some of the code also looks very peculiar, like the fact that it's >> > generating a clock name i2s_opclk%d rather than hard coding the >> > clock, the physical clock would normally be resolved based on the >> > struct device. > >> This is to handle all of Samsung SOCs i2c clock mux. >> Please look at below clk_lookup table > >> In case of 6410, clk_lookup >> + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk0", &clk_i2s0), >> + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk1", &clk_audio_bus0.clk), >> + CLKDEV_INIT("samsung-i2s.1", "i2s_opclk0", &clk_i2s1), >> + CLKDEV_INIT("samsung-i2s.1", "i2s_opclk1", &clk_audio_bus1.clk), >> +#ifdef CONFIG_CPU_S3C6410 >> + CLKDEV_INIT("samsung-i2s.2", "i2s_opclk0", &clk_i2s2), >> + CLKDEV_INIT("samsung-i2s.2", "i2s_opclk1", &clk_audio_bus2.clk), > >> In case of exynos5, clk_lookup >> + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk0", &exynos5_clk_sclk_i2s.clk), >> + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk1", >> +&exynos5_clk_i2s_bus.clk), > >> We try to handle clock source of i2s by only i2s_opclk0 and i2s_opclk1. >> Each SOCs have different clock source. >> Is this wrong approach? > > That makes sense but why is the driver not just requesting by name rather than having code to > generate the names, and why is this mixed in with the patch adding DT support? > Thanks Padma ^ permalink raw reply [flat|nested] 11+ messages in thread
* [GIT PULL] ASoC: Samsung: Updates for v3.8 @ 2012-11-28 5:51 ` Padma Venkat 0 siblings, 0 replies; 11+ messages in thread From: Padma Venkat @ 2012-11-28 5:51 UTC (permalink / raw) To: linux-arm-kernel Hi Mark, On Wed, Nov 28, 2012 at 10:55 AM, Sangbeom Kim <sbkim73@samsung.com> wrote: >> > There's some problems with this binding. The main one is the gpios >> > property the format of which isn't specified at all. > >> All of above gpio property is i2s. >> That is, >> + gpios = <&gpz 0 2 0 0>, -> SCLK >> + <&gpz 1 2 0 0>, -> CDCLK >> + <&gpz 2 2 0 0>, -> LRCK >> + <&gpz 3 2 0 0>, -> SDI >> + <&gpz 4 2 0 0>, -> SDO[0] >> + <&gpz 5 2 0 0>, -> SDO[1] >> + <&gpz 6 2 0 0>; -> SDO[2] > >> Do you want like a below one? >> +sclk-gpios = <&gpz 0 2 0 0>, >> +cdclk-gpios = <&gpz 1 2 0 0>, ... > > That would be nice but what I was really looking for was some indiciation in the binding document as > to what all this stuff means - it should really say what gpz is (though I can figure that out) and > it definitely needs to say what the four numbers are. > >> > The requirement for an alias is also very odd, where does that come from? > >> I don't know that Which one is odd. Please let me know. > > Having them at all is odd - it's not something other DT bindings have needed and there was nothing > saying what it was for. What is the exact requirement here? Should I remove the alias ids or I2S1 and I2S2 nodes as they are in disabled state? This is done same way as SPI and I2C. Please explain me. > >> > Some of the code also looks very peculiar, like the fact that it's >> > generating a clock name i2s_opclk%d rather than hard coding the >> > clock, the physical clock would normally be resolved based on the >> > struct device. > >> This is to handle all of Samsung SOCs i2c clock mux. >> Please look at below clk_lookup table > >> In case of 6410, clk_lookup >> + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk0", &clk_i2s0), >> + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk1", &clk_audio_bus0.clk), >> + CLKDEV_INIT("samsung-i2s.1", "i2s_opclk0", &clk_i2s1), >> + CLKDEV_INIT("samsung-i2s.1", "i2s_opclk1", &clk_audio_bus1.clk), >> +#ifdef CONFIG_CPU_S3C6410 >> + CLKDEV_INIT("samsung-i2s.2", "i2s_opclk0", &clk_i2s2), >> + CLKDEV_INIT("samsung-i2s.2", "i2s_opclk1", &clk_audio_bus2.clk), > >> In case of exynos5, clk_lookup >> + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk0", &exynos5_clk_sclk_i2s.clk), >> + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk1", >> +&exynos5_clk_i2s_bus.clk), > >> We try to handle clock source of i2s by only i2s_opclk0 and i2s_opclk1. >> Each SOCs have different clock source. >> Is this wrong approach? > > That makes sense but why is the driver not just requesting by name rather than having code to > generate the names, and why is this mixed in with the patch adding DT support? > Thanks Padma ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [GIT PULL] ASoC: Samsung: Updates for v3.8 2012-11-28 5:51 ` Padma Venkat @ 2012-11-28 9:08 ` Mark Brown -1 siblings, 0 replies; 11+ messages in thread From: Mark Brown @ 2012-11-28 9:08 UTC (permalink / raw) To: Padma Venkat Cc: alsa-devel, linux-arm-kernel, linux-samsung-soc, Sangbeom Kim, Kukjin Kim, devicetree-discuss [-- Attachment #1: Type: text/plain, Size: 858 bytes --] On Wed, Nov 28, 2012 at 11:21:31AM +0530, Padma Venkat wrote: > On Wed, Nov 28, 2012 at 10:55 AM, Sangbeom Kim <sbkim73@samsung.com> wrote: > >> > The requirement for an alias is also very odd, where does that come from? > >> I don't know that Which one is odd. Please let me know. > > Having them at all is odd - it's not something other DT bindings have needed and there was nothing > > saying what it was for. > What is the exact requirement here? Should I remove the alias ids or > I2S1 and I2S2 nodes as they are in disabled state? This is done same > way as SPI and I2C. Please explain me. If you need those aliases you need to explain why. They're listed as mandatory so presumably they must be needed for something? It might be that this is a good idea we didn't think of before so the answer is just to document that rather than remove them. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* [GIT PULL] ASoC: Samsung: Updates for v3.8 @ 2012-11-28 9:08 ` Mark Brown 0 siblings, 0 replies; 11+ messages in thread From: Mark Brown @ 2012-11-28 9:08 UTC (permalink / raw) To: linux-arm-kernel On Wed, Nov 28, 2012 at 11:21:31AM +0530, Padma Venkat wrote: > On Wed, Nov 28, 2012 at 10:55 AM, Sangbeom Kim <sbkim73@samsung.com> wrote: > >> > The requirement for an alias is also very odd, where does that come from? > >> I don't know that Which one is odd. Please let me know. > > Having them at all is odd - it's not something other DT bindings have needed and there was nothing > > saying what it was for. > What is the exact requirement here? Should I remove the alias ids or > I2S1 and I2S2 nodes as they are in disabled state? This is done same > way as SPI and I2C. Please explain me. If you need those aliases you need to explain why. They're listed as mandatory so presumably they must be needed for something? It might be that this is a good idea we didn't think of before so the answer is just to document that rather than remove them. -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121128/ee60652f/attachment-0001.sig> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [GIT PULL] ASoC: Samsung: Updates for v3.8
@ 2012-11-23 9:27 Sangbeom Kim
2012-11-23 10:51 ` Mark Brown
2012-11-23 14:59 ` Mark Brown
0 siblings, 2 replies; 11+ messages in thread
From: Sangbeom Kim @ 2012-11-23 9:27 UTC (permalink / raw)
To: 'Mark Brown', alsa-devel
Cc: sachin.kamat, 'Sangsu Park', 'Padmavathi Venna'
Hi Mark,
It's Samsung audio updates which are mainly device tree supports.
We had fully tested on SMDK5250.
Thanks,
Sangbeom.
---------
The following changes since commit 208229ecb95b58a627d2f01fc4e2a1652c27f4d9:
ASoC: bells: Up to 512fs (2012-11-16 17:42:40 -0800)
(sound.git topic/samsung)
are available in the git repository at:
git://github.com/cornus/linux-samsung.git for-broonie
----------------------------------------------------------------
Padmavathi Venna (2):
ASoC: SAMSUNG: Add DT support for i2s
ASoC: Samsung: Register the audio dma platform device
Sachin Kamat (1):
ASoC: Samsung-dma: Fix potential NULL pointer dereference
Sangsu Park (1):
ASoC: Samsung: Update Kconfig for I2S,SPDIF and PCM audio
.../devicetree/bindings/sound/samsung-i2s.txt | 69 ++++++
sound/soc/samsung/Kconfig | 6 +-
sound/soc/samsung/dma.c | 44 ++++-
sound/soc/samsung/dma.h | 1 +
sound/soc/samsung/i2s.c | 234 ++++++++++++++++----
5 files changed, 298 insertions(+), 56 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [GIT PULL] ASoC: Samsung: Updates for v3.8 2012-11-23 9:27 Sangbeom Kim @ 2012-11-23 10:51 ` Mark Brown 2012-11-24 2:13 ` Sangbeom Kim 2012-11-23 14:59 ` Mark Brown 1 sibling, 1 reply; 11+ messages in thread From: Mark Brown @ 2012-11-23 10:51 UTC (permalink / raw) To: Sangbeom Kim Cc: sachin.kamat, alsa-devel, 'Sangsu Park', 'Padmavathi Venna' [-- Attachment #1.1: Type: text/plain, Size: 355 bytes --] On Fri, Nov 23, 2012 at 06:27:19PM +0900, Sangbeom Kim wrote: > Hi Mark, > > It's Samsung audio updates which are mainly device tree supports. > We had fully tested on SMDK5250. As we discussed offline please do post the patches, I do want to review code before I pull it. As far as I can tell none of these patches have ever been sent to me. [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [GIT PULL] ASoC: Samsung: Updates for v3.8 2012-11-23 10:51 ` Mark Brown @ 2012-11-24 2:13 ` Sangbeom Kim 0 siblings, 0 replies; 11+ messages in thread From: Sangbeom Kim @ 2012-11-24 2:13 UTC (permalink / raw) To: 'Mark Brown', 'Padmavathi Venna'; +Cc: alsa-devel Hi On Tue, Nov 23, 2012 at 7:52 PM, Mark Brown wrote: > As we discussed offline please do post the patches, I do want to review > code before I pull it. As far as I can tell none of these patches have > ever been sent to me. She had already posted 3times in alsa mainling list. http://mailman.alsa-project.org/pipermail/alsa-devel/2012-November/057347.html http://mailman.alsa-project.org/pipermail/alsa-devel/2012-September/055871.html http://mailman.alsa-project.org/pipermail/alsa-devel/2012-August/053903.html But she missed including you. Padma, Please include Mark Brown in the next patches. Thanks, Sangbeom. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [GIT PULL] ASoC: Samsung: Updates for v3.8 2012-11-23 9:27 Sangbeom Kim 2012-11-23 10:51 ` Mark Brown @ 2012-11-23 14:59 ` Mark Brown 2012-11-24 2:13 ` Sangbeom Kim 1 sibling, 1 reply; 11+ messages in thread From: Mark Brown @ 2012-11-23 14:59 UTC (permalink / raw) To: Sangbeom Kim Cc: sachin.kamat, alsa-devel, 'Sangsu Park', 'Padmavathi Venna' [-- Attachment #1.1: Type: text/plain, Size: 1021 bytes --] On Fri, Nov 23, 2012 at 06:27:19PM +0900, Sangbeom Kim wrote: > Padmavathi Venna (2): > ASoC: SAMSUNG: Add DT support for i2s There's some problems with this binding. The main one is the gpios property the format of which isn't specified at all. The requirement for an alias is also very odd, where does that come from? Some of the code also looks very peculiar, like the fact that it's generating a clock name i2s_opclk%d rather than hard coding the clock, the physical clock would normally be resolved based on the struct device. > ASoC: Samsung: Register the audio dma platform device This isn't the normal approach here, the normal approach is that the I2S device instantiates the DMA device it needs - see the Tegra or i.MX drivers for examples. I'm also concerned that this is going to collide with the existing static registrations that non-DT boards do. > Sachin Kamat (1): > ASoC: Samsung-dma: Fix potential NULL pointer dereference This should be sent separately as a bug fix patch. [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [GIT PULL] ASoC: Samsung: Updates for v3.8 2012-11-23 14:59 ` Mark Brown @ 2012-11-24 2:13 ` Sangbeom Kim 2012-11-24 17:25 ` Mark Brown 0 siblings, 1 reply; 11+ messages in thread From: Sangbeom Kim @ 2012-11-24 2:13 UTC (permalink / raw) To: 'Mark Brown' Cc: sachin.kamat, alsa-devel, 'Sangsu Park', 'Padmavathi Venna' Hi, Thanks for review. > There's some problems with this binding. The main one is the gpios > property the format of which isn't specified at all. All of above gpio property is i2s. That is, + gpios = <&gpz 0 2 0 0>, -> SCLK + <&gpz 1 2 0 0>, -> CDCLK + <&gpz 2 2 0 0>, -> LRCK + <&gpz 3 2 0 0>, -> SDI + <&gpz 4 2 0 0>, -> SDO[0] + <&gpz 5 2 0 0>, -> SDO[1] + <&gpz 6 2 0 0>; -> SDO[2] Do you want like a below one? +sclk-gpios = <&gpz 0 2 0 0>, +cdclk-gpios = <&gpz 1 2 0 0>, ... > The requirement for an alias is also very odd, where does that come from? I don't know that Which one is odd. Please let me know. > Some of the code also looks very peculiar, like the fact that it's > generating a clock name i2s_opclk%d rather than hard coding the clock, > the physical clock would normally be resolved based on the struct > device. This is to handle all of Samsung SOCs i2c clock mux. Please look at below clk_lookup table In case of 6410, clk_lookup + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk0", &clk_i2s0), + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk1", &clk_audio_bus0.clk), + CLKDEV_INIT("samsung-i2s.1", "i2s_opclk0", &clk_i2s1), + CLKDEV_INIT("samsung-i2s.1", "i2s_opclk1", &clk_audio_bus1.clk), +#ifdef CONFIG_CPU_S3C6410 + CLKDEV_INIT("samsung-i2s.2", "i2s_opclk0", &clk_i2s2), + CLKDEV_INIT("samsung-i2s.2", "i2s_opclk1", &clk_audio_bus2.clk), In case of exynos5, clk_lookup + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk0", &exynos5_clk_sclk_i2s.clk), + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk1", &exynos5_clk_i2s_bus.clk), We try to handle clock source of i2s by only i2s_opclk0 and i2s_opclk1. Each SOCs have different clock source. Is this wrong approach? Thanks, Sangbeom. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [GIT PULL] ASoC: Samsung: Updates for v3.8 2012-11-24 2:13 ` Sangbeom Kim @ 2012-11-24 17:25 ` Mark Brown 0 siblings, 0 replies; 11+ messages in thread From: Mark Brown @ 2012-11-24 17:25 UTC (permalink / raw) To: Sangbeom Kim Cc: sachin.kamat, alsa-devel, 'Sangsu Park', 'Padmavathi Venna' [-- Attachment #1.1: Type: text/plain, Size: 2420 bytes --] On Sat, Nov 24, 2012 at 11:13:28AM +0900, Sangbeom Kim wrote: > > There's some problems with this binding. The main one is the gpios > > property the format of which isn't specified at all. > All of above gpio property is i2s. > That is, > + gpios = <&gpz 0 2 0 0>, -> SCLK > + <&gpz 1 2 0 0>, -> CDCLK > + <&gpz 2 2 0 0>, -> LRCK > + <&gpz 3 2 0 0>, -> SDI > + <&gpz 4 2 0 0>, -> SDO[0] > + <&gpz 5 2 0 0>, -> SDO[1] > + <&gpz 6 2 0 0>; -> SDO[2] > Do you want like a below one? > +sclk-gpios = <&gpz 0 2 0 0>, > +cdclk-gpios = <&gpz 1 2 0 0>, ... That would be nice but what I was really looking for was some indiciation in the binding document as to what all this stuff means - it should really say what gpz is (though I can figure that out) and it definitely needs to say what the four numbers are. > > The requirement for an alias is also very odd, where does that come from? > I don't know that Which one is odd. Please let me know. Having them at all is odd - it's not something other DT bindings have needed and there was nothing saying what it was for. > > Some of the code also looks very peculiar, like the fact that it's > > generating a clock name i2s_opclk%d rather than hard coding the clock, > > the physical clock would normally be resolved based on the struct > > device. > This is to handle all of Samsung SOCs i2c clock mux. > Please look at below clk_lookup table > In case of 6410, clk_lookup > + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk0", &clk_i2s0), > + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk1", &clk_audio_bus0.clk), > + CLKDEV_INIT("samsung-i2s.1", "i2s_opclk0", &clk_i2s1), > + CLKDEV_INIT("samsung-i2s.1", "i2s_opclk1", &clk_audio_bus1.clk), > +#ifdef CONFIG_CPU_S3C6410 > + CLKDEV_INIT("samsung-i2s.2", "i2s_opclk0", &clk_i2s2), > + CLKDEV_INIT("samsung-i2s.2", "i2s_opclk1", &clk_audio_bus2.clk), > In case of exynos5, clk_lookup > + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk0", &exynos5_clk_sclk_i2s.clk), > + CLKDEV_INIT("samsung-i2s.0", "i2s_opclk1", &exynos5_clk_i2s_bus.clk), > We try to handle clock source of i2s by only i2s_opclk0 and i2s_opclk1. > Each SOCs have different clock source. > Is this wrong approach? That makes sense but why is the driver not just requesting by name rather than having code to generate the names, and why is this mixed in with the patch adding DT support? [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-11-28 9:08 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <0MDZ007SI86SXBD0@mailout4.samsung.com>
2012-11-24 17:26 ` [GIT PULL] ASoC: Samsung: Updates for v3.8 Mark Brown
[not found] <001601cdcd28$ce5204c0$6af60e40$@com>
2012-11-28 5:51 ` Padma Venkat
2012-11-28 5:51 ` Padma Venkat
2012-11-28 9:08 ` Mark Brown
2012-11-28 9:08 ` Mark Brown
2012-11-23 9:27 Sangbeom Kim
2012-11-23 10:51 ` Mark Brown
2012-11-24 2:13 ` Sangbeom Kim
2012-11-23 14:59 ` Mark Brown
2012-11-24 2:13 ` Sangbeom Kim
2012-11-24 17:25 ` Mark Brown
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.