[alsa-devel] Reforming S3C I2S towards supporting I2Sv4
Hello,
I have prepared some patches that debug and modify the behaviour of extant S3C I2S support towards managing I2Sv4 controllers as well.
Overall pic... For now the 'new' I2Sv4 CPU driver(s3c64xx-i2s-v4.c) is almost a copy of s3c64xx-i2s.c The driver will evolve as support for more features is added. Minor quirks in otherwise similar controllers are handled by defining a new field(feature) in 'struct s3c_i2sv2_info' that is meant to contain 1-bit flags for absence/presence of such quirks. The header with I2S register map and bit definitions has been copied to where the drivers are(sound/soc/s3c24xx/) since the header has nothing usable for platform code. Also, it will help avoid need for co-ordination between ASoC and S3C ARCH trees. For now, the header regs-s3c2412-iis.h is left intact but rendered useless by making ASoC drivers include the newly copied version of it (sound/soc/s3c24xx/regs-i2s-v2.h) Later the header could be dropped by patches to S3C PLAT tree.
The patches apply against 'for-2.6.35' of http://www.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git
Regards.
Signed-off-by: Jassi Brar jassi.brar@samsung.com
On Wed, 2010-03-10 at 16:48 +0900, Jassi Brar wrote:
Hello,
I have prepared some patches that debug and modify the behaviour of extant S3C I2S support towards managing I2Sv4 controllers as well.
Overall pic... For now the 'new' I2Sv4 CPU driver(s3c64xx-i2s-v4.c) is almost a copy of s3c64xx-i2s.c The driver will evolve as support for more features is added. Minor quirks in otherwise similar controllers are handled by defining a new field(feature) in 'struct s3c_i2sv2_info' that is meant to contain 1-bit flags for absence/presence of such quirks.
patches 1 - 9 Acked-by: Liam Girdwood lrg@slimlogic.co.uk
The header with I2S register map and bit definitions has been copied to where the drivers are(sound/soc/s3c24xx/) since the header has nothing usable for platform code. Also, it will help avoid need for co-ordination between ASoC and S3C ARCH trees. For now, the header regs-s3c2412-iis.h is left intact but rendered useless by making ASoC drivers include the newly copied version of it (sound/soc/s3c24xx/regs-i2s-v2.h) Later the header could be dropped by patches to S3C PLAT tree.
I'm not too keen on moving CPU register and bit definitions out of ARCH.
Liam
On Wed, Mar 10, 2010 at 5:24 PM, Liam Girdwood lrg@slimlogic.co.uk wrote:
On Wed, 2010-03-10 at 16:48 +0900, Jassi Brar wrote:
The header with I2S register map and bit definitions has been copied to where the drivers are(sound/soc/s3c24xx/) since the header has nothing usable for platform code. Also, it will help avoid need for co-ordination between ASoC and S3C ARCH trees. For now, the header regs-s3c2412-iis.h is left intact but rendered useless by making ASoC drivers include the newly copied version of it (sound/soc/s3c24xx/regs-i2s-v2.h) Later the header could be dropped by patches to S3C PLAT tree.
I'm not too keen on moving CPU register and bit definitions out of ARCH.
The header doesn't contain absolute register addresses, but only offsets. The base address of I2S controllers are defined in PLAT specific header. So, I think we can move the header.
On Wed, 2010-03-10 at 17:31 +0900, jassi brar wrote:
On Wed, Mar 10, 2010 at 5:24 PM, Liam Girdwood lrg@slimlogic.co.uk wrote:
On Wed, 2010-03-10 at 16:48 +0900, Jassi Brar wrote:
The header with I2S register map and bit definitions has been copied to where the drivers are(sound/soc/s3c24xx/) since the header has nothing usable for platform code. Also, it will help avoid need for co-ordination between ASoC and S3C ARCH trees. For now, the header regs-s3c2412-iis.h is left intact but rendered useless by making ASoC drivers include the newly copied version of it (sound/soc/s3c24xx/regs-i2s-v2.h) Later the header could be dropped by patches to S3C PLAT tree.
I'm not too keen on moving CPU register and bit definitions out of ARCH.
The header doesn't contain absolute register addresses, but only offsets.
Isn't this really the same thing ?
The base address of I2S controllers are defined in PLAT specific header. So, I think we can move the header.
I don't think your reason of "co-ordination between ASoC and S3C ARCH trees" can justify breaking kernel policy.
Liam
On Wed, Mar 10, 2010 at 6:04 PM, Liam Girdwood lrg@slimlogic.co.uk wrote:
On Wed, 2010-03-10 at 17:31 +0900, jassi brar wrote:
On Wed, Mar 10, 2010 at 5:24 PM, Liam Girdwood lrg@slimlogic.co.uk wrote:
On Wed, 2010-03-10 at 16:48 +0900, Jassi Brar wrote:
The header with I2S register map and bit definitions has been copied to where the drivers are(sound/soc/s3c24xx/) since the header has nothing usable for platform code. Also, it will help avoid need for co-ordination between ASoC and S3C ARCH trees. For now, the header regs-s3c2412-iis.h is left intact but rendered useless by making ASoC drivers include the newly copied version of it (sound/soc/s3c24xx/regs-i2s-v2.h) Later the header could be dropped by patches to S3C PLAT tree.
I'm not too keen on moving CPU register and bit definitions out of ARCH.
The header doesn't contain absolute register addresses, but only offsets.
Isn't this really the same thing ?
If two SoCs have same controllers but mapped at different addresses, the ARCH maintainer would find a common place for the header rather than making two headers with one define different and rest all same. If another such SoC comes up the cycle starts again.(That has apparantly happened with S3C support over the years) Now, rather than moving around the header with definitions of what is _only_ used by the controller driver, why not move it close to the driver? The base addresses can(and should) always be defined in PLAT specific location.
The base address of I2S controllers are defined in PLAT specific header. So, I think we can move the header.
I don't think your reason of "co-ordination between ASoC and S3C ARCH trees" can justify breaking kernel policy.
I believe the policy is not against moving definitions of bits that are tightly linked to the device controller esp when we seeing more and more SoCs coming with standard of-the-shelf third party controllers. Some drivers do have the controller bit definitions beside drivers. I could atleast see some serial and spi drivers.
Then, I really want S3C ASoC modifications to not wait on S3C ARCH patches appearing upstream first.
Having said that, I am willing to(not to mean I have another choice) take word of the maintainer on the matter and first send patches to s3c ARCH and wait until they appear in Mark's tree before submitting the remaining patches.
On Wed, Mar 10, 2010 at 06:22:29PM +0900, jassi brar wrote:
On Wed, Mar 10, 2010 at 6:04 PM, Liam Girdwood lrg@slimlogic.co.uk wrote:
I don't think your reason of "co-ordination between ASoC and S3C ARCH trees" can justify breaking kernel policy.
I believe the policy is not against moving definitions of bits that are tightly linked to the device controller esp when we seeing more and more SoCs coming with standard of-the-shelf third party controllers.
Plus controllers that may be used in multiple subsystems (like the generic serial ports a lot of SoCs have). Some architectures also like to keep headers for the IPs with the arch either due to preference or due to device differences which can be hidden from the driver with arch level compile stuff.
I personally don't mind either way for things like the I2S controller which don't have any obvious application outside ASoC so whatever you guys and Ben want works for me.
Then, I really want S3C ASoC modifications to not wait on S3C ARCH patches appearing upstream first.
If the headers stay in arch/arm we can still deal with that by either agreeing to merge them via ASoC (as has been done in the past) or by creating a branch which is merged into both ASoC and Samsung trees. It is obviously less effort to avoid this, but it's not insurmountable.
Folks please ignore the second copy of patches 11-20. Git send-email seems to have 'buffer' ... first <git send-email *.patch> command emailed only patches 1-10 second copy was actually sent when I tried to resend 11-20 again thinking they have been dropped. Sorry.
On Wed, 2010-03-10 at 17:35 +0900, jassi brar wrote:
Folks please ignore the second copy of patches 11-20. Git send-email seems to have 'buffer' ... first <git send-email *.patch> command emailed only patches 1-10 second copy was actually sent when I tried to resend 11-20 again thinking they have been dropped. Sorry.
Ah, I did wonder when Acking the first set...
Liam
On Wed, Mar 10, 2010 at 5:51 PM, Liam Girdwood lrg@slimlogic.co.uk wrote:
On Wed, 2010-03-10 at 17:35 +0900, jassi brar wrote:
Folks please ignore the second copy of patches 11-20. Git send-email seems to have 'buffer' ... first <git send-email *.patch> command emailed only patches 1-10 second copy was actually sent when I tried to resend 11-20 again thinking they have been dropped. Sorry.
Ah, I did wonder when Acking the first set...
My another mistake(not CCing you) saved you 30 more emails though :)
On Wed, Mar 10, 2010 at 05:35:47PM +0900, jassi brar wrote:
Folks please ignore the second copy of patches 11-20. Git send-email seems to have 'buffer' ... first <git send-email *.patch> command emailed only patches 1-10 second copy was actually sent when I tried to resend 11-20 again thinking they have been dropped. Sorry.
It'll have got stuck in a MTA queue somewhere - often MTAs are configured to rate limit submissions as a way of mitigating against spam.
On Wed, Mar 10, 2010 at 04:48:55PM +0900, Jassi Brar wrote:
I have prepared some patches that debug and modify the behaviour of extant S3C I2S support towards managing I2Sv4 controllers as well.
OK, I think that's everything. I've not applied anything after the header move yet due to the need for an ack from Ben. Any patches I didn't specifically comment on are OK. Overall it's all good, most of the issues are minor.
In future could you please use --no-chain-reply-to when sending patches to avoid massively deep threads in threaded mail readers (the git default is changing in 1.7.0 for this very reason). It's also best to send the cover letter first and tell git send-email that everything should be sent in reply to that - this way people are likely to see the cover letter before the patches.
On Wed, Mar 10, 2010 at 11:23 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Mar 10, 2010 at 04:48:55PM +0900, Jassi Brar wrote:
I have prepared some patches that debug and modify the behaviour of extant S3C I2S support towards managing I2Sv4 controllers as well.
OK, I think that's everything. I've not applied anything after the header move yet due to the need for an ack from Ben.
Hi Ben,
Could you please comment ? If need be I can resend the patches.
Thanks
On Tue, Apr 6, 2010 at 9:33 AM, jassi brar jassisinghbrar@gmail.com wrote:
On Wed, Mar 10, 2010 at 11:23 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Mar 10, 2010 at 04:48:55PM +0900, Jassi Brar wrote:
I have prepared some patches that debug and modify the behaviour of extant S3C I2S support towards managing I2Sv4 controllers as well.
OK, I think that's everything. I've not applied anything after the header move yet due to the need for an ack from Ben.
Hi Ben,
Could you please comment ? If need be I can resend the patches.
ping
On Wed, Mar 10, 2010 at 02:23:15PM +0000, Mark Brown wrote:
On Wed, Mar 10, 2010 at 04:48:55PM +0900, Jassi Brar wrote:
I have prepared some patches that debug and modify the behaviour of extant S3C I2S support towards managing I2Sv4 controllers as well.
OK, I think that's everything. I've not applied anything after the header move yet due to the need for an ack from Ben. Any patches I didn't specifically comment on are OK. Overall it's all good, most of the issues are minor.
I'd like to see a bit of header reformatting to make them readable if they're not already merged.
the header moves are ok by me, much prefer to see them git mv'd than copy and rm'd.
In future could you please use --no-chain-reply-to when sending patches to avoid massively deep threads in threaded mail readers (the git default is changing in 1.7.0 for this very reason). It's also best to send the cover letter first and tell git send-email that everything should be sent in reply to that - this way people are likely to see the cover letter before the patches.
agreed on this one.
On Tue, Apr 27, 2010 at 11:58 AM, Ben Dooks ben-linux@fluff.org wrote:
On Wed, Mar 10, 2010 at 02:23:15PM +0000, Mark Brown wrote:
On Wed, Mar 10, 2010 at 04:48:55PM +0900, Jassi Brar wrote:
I have prepared some patches that debug and modify the behaviour of extant S3C I2S support towards managing I2Sv4 controllers as well.
OK, I think that's everything. I've not applied anything after the header move yet due to the need for an ack from Ben. Any patches I didn't specifically comment on are OK. Overall it's all good, most of the issues are minor.
I'd like to see a bit of header reformatting to make them readable if they're not already merged.
Ok.
the header moves are ok by me, much prefer to see them git mv'd than copy and rm'd.
Ok, I will move the header. Though the original plan was to create a copy in sound/soc/s3c24xx and render useless the one in plat-samsung/include/plat/ which could be independently removed from your tree. That was meant to avoid cross tree changes.
Hi Jassi,
Do you have a plan to move or create the sound/soc/samsung or others for future Samsung SoCs? And can you make a document which SoCs use I2S version? e.g., s3c24xx use I2Sv3 s3c64xx use I2Sv3 or 4?? s5p64xx use I2Sv4 s5pc1xx use I2Sv5 and so on
How about to give name samsung-i2cvx for common use instead of cpu name?
Thank you, Kyungmin Park
On Tue, Apr 27, 2010 at 1:25 PM, jassi brar jassisinghbrar@gmail.com wrote:
On Tue, Apr 27, 2010 at 11:58 AM, Ben Dooks ben-linux@fluff.org wrote:
On Wed, Mar 10, 2010 at 02:23:15PM +0000, Mark Brown wrote:
On Wed, Mar 10, 2010 at 04:48:55PM +0900, Jassi Brar wrote:
I have prepared some patches that debug and modify the behaviour of extant S3C I2S support towards managing I2Sv4 controllers as well.
OK, I think that's everything. I've not applied anything after the header move yet due to the need for an ack from Ben. Any patches I didn't specifically comment on are OK. Overall it's all good, most of the issues are minor.
I'd like to see a bit of header reformatting to make them readable if they're not already merged.
Ok.
the header moves are ok by me, much prefer to see them git mv'd than copy and rm'd.
Ok, I will move the header. Though the original plan was to create a copy in sound/soc/s3c24xx and render useless the one in plat-samsung/include/plat/ which could be independently removed from your tree. That was meant to avoid cross tree changes. _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Sun, May 2, 2010 at 1:38 PM, Kyungmin Park kmpark@infradead.org wrote:
Do you have a plan to move or create the sound/soc/samsung or others for future Samsung SoCs?
I don't have any such plan atm, but I am not against moving sound/soc/s3c24xx to sound/soc/samsung
And can you make a document which SoCs use I2S version? e.g., s3c24xx use I2Sv3 s3c64xx use I2Sv3 or 4?? s5p64xx use I2Sv4 s5pc1xx use I2Sv5 and so on
How about to give name samsung-i2cvx for common use instead of cpu name?
I think it is better to call drivers by the first soc that is supported in mainline. Any new soc, that has the controller reused, should be handled by the same old driver. For example, if we have driver for s5pc100, then let s5pc110/v210 use the same s5pc100-i2s.c That is, after soc support in current drivers is sorted out.
participants (6)
-
Ben Dooks -
Jassi Brar -
jassi brar -
Kyungmin Park -
Liam Girdwood -
Mark Brown