[PATCH 1/2] ASoC: AD1980/AD73311: make sure to set dev member

[PATCH 1/2] ASoC: AD1980/AD73311: make sure to set dev member

[Mike Frysinger]

The codec driver should initialize snd_soc_codec's dev member to the
appropriate device when setting up the device, but these codecs weren't.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 sound/soc/codecs/ad1980.c  |    1 +
 sound/soc/codecs/ad73311.c |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/sound/soc/codecs/ad1980.c b/sound/soc/codecs/ad1980.c
index d7440a9..0477a91 100644
--- a/sound/soc/codecs/ad1980.c
+++ b/sound/soc/codecs/ad1980.c
@@ -203,6 +203,7 @@ static int ad1980_soc_probe(struct platform_device *pdev)
 	codec->reg_cache_size = sizeof(u16) * ARRAY_SIZE(ad1980_reg);
 	codec->reg_cache_step = 2;
 	codec->name = "AD1980";
+	codec->dev = &pdev->dev;
 	codec->owner = THIS_MODULE;
 	codec->dai = &ad1980_dai;
 	codec->num_dai = 1;
diff --git a/sound/soc/codecs/ad73311.c b/sound/soc/codecs/ad73311.c
index e61dac5..6e18149 100644
--- a/sound/soc/codecs/ad73311.c
+++ b/sound/soc/codecs/ad73311.c
@@ -50,6 +50,7 @@ static int ad73311_soc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	mutex_init(&codec->mutex);
 	codec->name = "AD73311";
+	codec->dev = &pdev->dev;
 	codec->owner = THIS_MODULE;
 	codec->dai = &ad73311_dai;
 	codec->num_dai = 1;
-- 
1.6.5.rc2

[PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mike Frysinger]

Since these drivers rely on a SPI master and fail if the SPI functions
aren't present, make sure the Kconfig reflects this dependency.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 sound/soc/blackfin/Kconfig |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/blackfin/Kconfig b/sound/soc/blackfin/Kconfig
index 97f1a25..b91e234 100644
--- a/sound/soc/blackfin/Kconfig
+++ b/sound/soc/blackfin/Kconfig
@@ -43,7 +43,7 @@ config SND_BF5XX_TDM
 
 config SND_BF5XX_SOC_AD1836
 	tristate "SoC AD1836 Audio support for BF5xx"
-	depends on SND_BF5XX_TDM
+	depends on SND_BF5XX_TDM && SPI_MASTER
 	select SND_BF5XX_SOC_TDM
 	select SND_SOC_AD1836
 	help
@@ -51,7 +51,7 @@ config SND_BF5XX_SOC_AD1836
 
 config SND_BF5XX_SOC_AD1938
 	tristate "SoC AD1938 Audio support for Blackfin"
-	depends on SND_BF5XX_TDM
+	depends on SND_BF5XX_TDM && SPI_MASTER
 	select SND_BF5XX_SOC_TDM
 	select SND_SOC_AD1938
 	help
-- 
1.6.5.rc2

Re: [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mark Brown]

On Tue, Oct 06, 2009 at 01:51:51AM -0400, Mike Frysinger wrote:
> Since these drivers rely on a SPI master and fail if the SPI functions
> aren't present, make sure the Kconfig reflects this dependency.

> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Could you please redo this as a dependency on the relevant SPI
controller driver?  The audio driver isn't going to work without that
anyway so it seems more sensible.

Re: [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mike Frysinger]

On Tue, Oct 6, 2009 at 05:33, Mark Brown wrote:
> On Tue, Oct 06, 2009 at 01:51:51AM -0400, Mike Frysinger wrote:
>> Since these drivers rely on a SPI master and fail if the SPI functions
>> aren't present, make sure the Kconfig reflects this dependency.
>
> Could you please redo this as a dependency on the relevant SPI
> controller driver?  The audio driver isn't going to work without that
> anyway so it seems more sensible.

there is no dependency on a specific spi bus master implementation.
just like the i2c framework, such a dependency would be against the
whole purpose of having a generic framework in the first place.
fortunately, this is one piece of the Blackfin ASoC drivers done
correctly.  in other words, they should work just fine if hooked up to
a GPIO SPI bus or a SPORT SPI bus or a Blackfin on-chip SPI bus or
......
-mike
_______________________________________________
Uclinux-dist-devel mailing list
Uclinux-dist-devel@blackfin.uclinux.org
https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel

Re: [alsa-devel] [PATCH 1/2] ASoC: AD1980/AD73311: make sure to set dev member

[Mark Brown]

On Tue, Oct 06, 2009 at 01:51:50AM -0400, Mike Frysinger wrote:
> The codec driver should initialize snd_soc_codec's dev member to the
> appropriate device when setting up the device, but these codecs weren't.

> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>

No, this is the wrong approach - codec->dev should never be being set to
the socdev device, it should be being set to the device model device for
the CODEC (as should the dev members for the DAIs).  For ad73311 this
means it needs to be converted to use proper device model instantiation,
see wm8731 for an example of doing this.  ad1980 should be handled by
generic AC97 code in the core.

Re: [Uclinux-dist-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mark Brown]

On Tue, Oct 06, 2009 at 05:41:40AM -0400, Mike Frysinger wrote:

> there is no dependency on a specific spi bus master implementation.
> just like the i2c framework, such a dependency would be against the
> whole purpose of having a generic framework in the first place.
> fortunately, this is one piece of the Blackfin ASoC drivers done
> correctly.  in other words, they should work just fine if hooked up to
> a GPIO SPI bus or a SPORT SPI bus or a Blackfin on-chip SPI bus or
> ......

These are board-specific drivers to hook things up on a given system
(presumably various eval boards), they're the non-generic part of the
system that says how the generic DAI and CODEC drivers have been hooked
up.

Re: [Uclinux-dist-devel] [PATCH 1/2] ASoC: AD1980/AD73311: make sure to set dev member

[Mike Frysinger]

On Tue, Oct 6, 2009 at 05:42, Mark Brown wrote:
> On Tue, Oct 06, 2009 at 01:51:50AM -0400, Mike Frysinger wrote:
>> The codec driver should initialize snd_soc_codec's dev member to the
>> appropriate device when setting up the device, but these codecs weren't.
>
> No, this is the wrong approach - codec->dev should never be being set to
> the socdev device, it should be being set to the device model device for
> the CODEC (as should the dev members for the DAIs).  For ad73311 this
> means it needs to be converted to use proper device model instantiation,
> see wm8731 for an example of doing this.  ad1980 should be handled by
> generic AC97 code in the core.

meh, this sounds like effort.  i'll trick Cliff/Barry into doing it ;).
-mike
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [Uclinux-dist-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mike Frysinger]

On Tue, Oct 6, 2009 at 05:50, Mark Brown wrote:
> On Tue, Oct 06, 2009 at 05:41:40AM -0400, Mike Frysinger wrote:
>> there is no dependency on a specific spi bus master implementation.
>> just like the i2c framework, such a dependency would be against the
>> whole purpose of having a generic framework in the first place.
>> fortunately, this is one piece of the Blackfin ASoC drivers done
>> correctly.  in other words, they should work just fine if hooked up to
>> a GPIO SPI bus or a SPORT SPI bus or a Blackfin on-chip SPI bus or
>> ......
>
> These are board-specific drivers to hook things up on a given system
> (presumably various eval boards), they're the non-generic part of the
> system that says how the generic DAI and CODEC drivers have been hooked
> up.

except the Blackfin machine drivers are written in such a way that
they arent specific to a board.  they'll work on any system with a
SPORT and a SPI/I2C bus.
-mike
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [Uclinux-dist-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mark Brown]

On Tue, Oct 06, 2009 at 06:00:23AM -0400, Mike Frysinger wrote:
> On Tue, Oct 6, 2009 at 05:50, Mark Brown wrote:

> > These are board-specific drivers to hook things up on a given system
> > (presumably various eval boards), they're the non-generic part of the
> > system that says how the generic DAI and CODEC drivers have been hooked
> > up.

> except the Blackfin machine drivers are written in such a way that
> they arent specific to a board.  they'll work on any system with a
> SPORT and a SPI/I2C bus.

That's not 100% clear - some of the drivers say they're specific to
particular designs (either in the help text or by having board-specific
GPIO setup).  It's certainly the intention of the API that the board
hookups be fixed at run time rather than at compile time, and that
things like using multiple CPU DAIs should be possible.

The other option here is to make the CODEC drivers build without their
control buses, which keeps the people doing random configurations happy.

Re: [Uclinux-dist-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mike Frysinger]

On Tue, Oct 6, 2009 at 06:39, Mark Brown wrote:
> On Tue, Oct 06, 2009 at 06:00:23AM -0400, Mike Frysinger wrote:
>> On Tue, Oct 6, 2009 at 05:50, Mark Brown wrote:
>> > These are board-specific drivers to hook things up on a given system
>> > (presumably various eval boards), they're the non-generic part of the
>> > system that says how the generic DAI and CODEC drivers have been hooked
>> > up.
>>
>> except the Blackfin machine drivers are written in such a way that
>> they arent specific to a board.  they'll work on any system with a
>> SPORT and a SPI/I2C bus.
>
> That's not 100% clear - some of the drivers say they're specific to
> particular designs (either in the help text or by having board-specific
> GPIO setup).  It's certainly the intention of the API that the board
> hookups be fixed at run time rather than at compile time, and that
> things like using multiple CPU DAIs should be possible.
>
> The other option here is to make the CODEC drivers build without their
> control buses, which keeps the people doing random configurations happy.

yes, there are many descriptions which are way more specific than they
should be.  the issue stems from people developing with a narrow focus
rather than keeping the big picture in their head.  they test one
specific configuration and then just bang out the name/info with that.
   ive been trying to stem the flow and clean up existing code, but it
doesnt always work out.

AD1836 is an example of this -- the Kconfig text says "BF5xx", but
that's useless because that covers all Blackfin parts.  "STAMP/EZKIT"
is a lie -- there is no dependency on these base board designs.  the
machine driver only needs a SPORT (TDM) for transport and the codec
needs SPI for configuration.  every Blackfin part has at least one
SPORT.  but the core SPORT handling is the crux of the matter -- as i
mentioned in another thread, our implementation is severely limited in
terms of multiple instances and is in need of restructuring.

the GPIO handling is made "generic" by putting it into the Kconfig.
so the machine-specific aspect is now in the .config file instead of
the machine driver.  really this needs to get pushed out into the
board resources (another bad habit that we've been reigning in --
encoding flexible resources in Kconfig).

back to the original issue.  the AD1836/AD1938 have their registers
programed via SPI, but they dont care what SPI bus they're connected
to.  that is specified in the board resources.  because of the way the
Kconfig options are handled (machine drivers select codecs), the
SPI_MASTER dependency cannot be added to the codec Kconfigs in
codecs/Kconfig.  so even though the SPI dependency is only in the
codec and not the machine driver, the machine driver needs to declare
the SPI_MASTER dependency to prevent incorrect config selections and
link failures.
-mike
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [Uclinux-dist-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mark Brown]

On Tue, Oct 06, 2009 at 07:32:30AM -0400, Mike Frysinger wrote:

> back to the original issue.  the AD1836/AD1938 have their registers
> programed via SPI, but they dont care what SPI bus they're connected
> to.  that is specified in the board resources.  because of the way the
> Kconfig options are handled (machine drivers select codecs), the
> SPI_MASTER dependency cannot be added to the codec Kconfigs in
> codecs/Kconfig.  so even though the SPI dependency is only in the
> codec and not the machine driver, the machine driver needs to declare
> the SPI_MASTER dependency to prevent incorrect config selections and
> link failures.

Or, like I say, the drivers for the CODEC should be changed to allow
build with no control bus.  This is a general problem for all the ASoC
drivers and I'm still not sure which way to go.  Depending on just SPI
support fixes the random build case but doesn't help end users get the
driver working so it doesn't seem so useful.  If we're only going to be
able to fix the random build case then it seems more useful to do so by
having the CODEC driver able to build without the control bus, that way
individual machine drivers don't need to worry about it.

Re: [Uclinux-dist-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mike Frysinger]

On Tue, Oct 6, 2009 at 07:52, Mark Brown wrote:
> On Tue, Oct 06, 2009 at 07:32:30AM -0400, Mike Frysinger wrote:
>> back to the original issue.  the AD1836/AD1938 have their registers
>> programed via SPI, but they dont care what SPI bus they're connected
>> to.  that is specified in the board resources.  because of the way the
>> Kconfig options are handled (machine drivers select codecs), the
>> SPI_MASTER dependency cannot be added to the codec Kconfigs in
>> codecs/Kconfig.  so even though the SPI dependency is only in the
>> codec and not the machine driver, the machine driver needs to declare
>> the SPI_MASTER dependency to prevent incorrect config selections and
>> link failures.
>
> Or, like I say, the drivers for the CODEC should be changed to allow
> build with no control bus.  This is a general problem for all the ASoC
> drivers and I'm still not sure which way to go.  Depending on just SPI
> support fixes the random build case but doesn't help end users get the
> driver working so it doesn't seem so useful.  If we're only going to be
> able to fix the random build case then it seems more useful to do so by
> having the CODEC driver able to build without the control bus, that way
> individual machine drivers don't need to worry about it.

i dont see how abstracting away the bus such that codec drivers are
allowed to build without proper bus support is useful.  the
AD1836/AD1938 only work with the SPI bus, so if support for that is
disabled, having the driver compiled and installed is pointless.
-mike
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [Uclinux-dist-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mark Brown]

On Tue, Oct 06, 2009 at 05:09:42PM -0400, Mike Frysinger wrote:

> i dont see how abstracting away the bus such that codec drivers are
> allowed to build without proper bus support is useful.  the
> AD1836/AD1938 only work with the SPI bus, so if support for that is
> disabled, having the driver compiled and installed is pointless.

Both approaches allow drivers to be built which can't actually be used
since neither guarantees that there will be a driver for the SPI
controller.  Either way all we're doing is making sure that the kernel
will build, the user can still build things so that the audio driver
won't do anything useful.

Re: [Uclinux-dist-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mike Frysinger]

On Tue, Oct 6, 2009 at 18:07, Mark Brown wrote:
> On Tue, Oct 06, 2009 at 05:09:42PM -0400, Mike Frysinger wrote:
>> i dont see how abstracting away the bus such that codec drivers are
>> allowed to build without proper bus support is useful.  the
>> AD1836/AD1938 only work with the SPI bus, so if support for that is
>> disabled, having the driver compiled and installed is pointless.
>
> Both approaches allow drivers to be built which can't actually be used
> since neither guarantees that there will be a driver for the SPI
> controller.  Either way all we're doing is making sure that the kernel
> will build, the user can still build things so that the audio driver
> won't do anything useful.

hmm, that's certainly true.  if we look at it as "screwed either way",
then going to a generic bus indirection sounds like it'd only add
runtime overhead for no real gain ?

in the face of this proposed effort being a ways off, doesnt it make
sense to still merge the original proposed patch ?
-mike
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [Uclinux-dist-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mark Brown]

On Wed, Oct 07, 2009 at 04:32:06AM -0400, Mike Frysinger wrote:

> hmm, that's certainly true.  if we look at it as "screwed either way",
> then going to a generic bus indirection sounds like it'd only add
> runtime overhead for no real gain ?

I was mostly thinking ifdefs here - we'll always need some bus-specific
stuff in the drivers to register them.  It's not pretty but it meets the
needs of people doing randconfig builds.

We already have as much bus indirection as ASoC needs, and there is
actually already some bus access code sharing there as of 2.6.32 (in
soc-cache.c) but it's optional and always will be since we need to cater
for devices that are parts of MFDs which have device specific register
acceses.

> in the face of this proposed effort being a ways off, doesnt it make
> sense to still merge the original proposed patch ?

The ifdefery isn't technically hard to do and given your use case where
you don't know which controller is in use it looks like the only way to
go for this.

Re: [Uclinux-dist-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mike Frysinger]

On Wed, Oct 7, 2009 at 06:19, Mark Brown wrote:
> On Wed, Oct 07, 2009 at 04:32:06AM -0400, Mike Frysinger wrote:
>> hmm, that's certainly true.  if we look at it as "screwed either way",
>> then going to a generic bus indirection sounds like it'd only add
>> runtime overhead for no real gain ?
>
> I was mostly thinking ifdefs here - we'll always need some bus-specific
> stuff in the drivers to register them.  It's not pretty but it meets the
> needs of people doing randconfig builds.
>
> We already have as much bus indirection as ASoC needs, and there is
> actually already some bus access code sharing there as of 2.6.32 (in
> soc-cache.c) but it's optional and always will be since we need to cater
> for devices that are parts of MFDs which have device specific register
> acceses.
>
>> in the face of this proposed effort being a ways off, doesnt it make
>> sense to still merge the original proposed patch ?
>
> The ifdefery isn't technically hard to do and given your use case where
> you don't know which controller is in use it looks like the only way to
> go for this.

i dont think the soc-cache could be used currently by the ad1836 as it
doesnt use a 7/9 split with the address/data.  it uses like 6/10 (4
addr bits, one bit for read/write, one bit is always 0, and 10 data
bits).  guessing the write bit can be considered part of the addr as
the read always comes from the cache, but that still gives us 5/10
split.  maybe a new 6/10 set of funcs should be added ...

snd_soc_7_9_write() looks like it does a little more bit work than it
needs to ?  if data is declared as a u16, then you have:
u16 data = (reg << 9) | (value & 0x01ff);
this is what the ad1836 driver does now for its data split.

in the mean time, rather than adding #ifdef to the codec driver, we
could create a local header like "bus-stubs.h" that stubs all the
relevant functions to an error value.  then all codec drivers that
dont use soc-cache can use that instead and the only change needed is
to add:
#include "bus-stubs.h"
-mike
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [Uclinux-dist-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mark Brown]

On Wed, Oct 07, 2009 at 08:27:56PM -0400, Mike Frysinger wrote:

> i dont think the soc-cache could be used currently by the ad1836 as it
> doesnt use a 7/9 split with the address/data.  it uses like 6/10 (4
> addr bits, one bit for read/write, one bit is always 0, and 10 data
> bits).  guessing the write bit can be considered part of the addr as
> the read always comes from the cache, but that still gives us 5/10
> split.  maybe a new 6/10 set of funcs should be added ...

That's the idea - add new functions for any new register formats.

> snd_soc_7_9_write() looks like it does a little more bit work than it
> needs to ?  if data is declared as a u16, then you have:
> u16 data = (reg << 9) | (value & 0x01ff);
> this is what the ad1836 driver does now for its data split.

Probably.  I'd need to check but I believe that's there to handle
endianness variations in the host, though a cpu_to_ in what you have
above ought to be able to take care of that.  The code was cut'n'pasted
from what was in the drivers already.

> in the mean time, rather than adding #ifdef to the codec driver, we
> could create a local header like "bus-stubs.h" that stubs all the
> relevant functions to an error value.  then all codec drivers that
> dont use soc-cache can use that instead and the only change needed is
> to add:
> #include "bus-stubs.h"

I'm not sure I feel up to doing that locally in ASoC rather than in the
relevant subsystems.

Re: [Uclinux-dist-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mike Frysinger]

On Thu, Oct 8, 2009 at 06:01, Mark Brown wrote:
> On Wed, Oct 07, 2009 at 08:27:56PM -0400, Mike Frysinger wrote:
>> i dont think the soc-cache could be used currently by the ad1836 as it
>> doesnt use a 7/9 split with the address/data.  it uses like 6/10 (4
>> addr bits, one bit for read/write, one bit is always 0, and 10 data
>> bits).  guessing the write bit can be considered part of the addr as
>> the read always comes from the cache, but that still gives us 5/10
>> split.  maybe a new 6/10 set of funcs should be added ...
>
> That's the idea - add new functions for any new register formats.

i'll add a tracker item to get the ad codecs converted to soc-cache
when possible and that'll naturally address the SPI dependency

>> snd_soc_7_9_write() looks like it does a little more bit work than it
>> needs to ?  if data is declared as a u16, then you have:
>> u16 data = (reg << 9) | (value & 0x01ff);
>> this is what the ad1836 driver does now for its data split.
>
> Probably.  I'd need to check but I believe that's there to handle
> endianness variations in the host, though a cpu_to_ in what you have
> above ought to be able to take care of that.  The code was cut'n'pasted
> from what was in the drivers already.

true ... being little endian sometimes makes you forget about these
things ;).  the ad1836 codec probably doesnt work on BE systems.

>> in the mean time, rather than adding #ifdef to the codec driver, we
>> could create a local header like "bus-stubs.h" that stubs all the
>> relevant functions to an error value.  then all codec drivers that
>> dont use soc-cache can use that instead and the only change needed is
>> to add:
>> #include "bus-stubs.h"
>
> I'm not sure I feel up to doing that locally in ASoC rather than in the
> relevant subsystems.

i dont feel like convincing subsystem maintainers that this is a good
idea for all consumers of SPI/I2C, especially since i'm not convinced
myself.  we'll just go the soc-cache route and not worry about it.
-mike
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [Uclinux-dist-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Barry Song]

On Thu, Oct 8, 2009 at 6:01 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Oct 07, 2009 at 08:27:56PM -0400, Mike Frysinger wrote:
>
>> i dont think the soc-cache could be used currently by the ad1836 as it
>> doesnt use a 7/9 split with the address/data.  it uses like 6/10 (4
>> addr bits, one bit for read/write, one bit is always 0, and 10 data
>> bits).  guessing the write bit can be considered part of the addr as
>> the read always comes from the cache, but that still gives us 5/10
>> split.  maybe a new 6/10 set of funcs should be added ...
>
> That's the idea - add new functions for any new register formats.
we do those based on the idea that it is a waste all codecs need to
use almost same ways to handle register access. If we use soc-cache to
handle register access issues and run as abstract layer for all
codecs, why not rename it to soc-reg or soc-bus? It seems cache is
only a little part of the responsibility of  this module.
It's better that functions like xx_7_9_read xx_7_9_write xx_8_8_read
xx_8_8_write should not become API because we never know what will be
the format for codecs. We should have a xx_n_m_read/xx_n_m_write or
just a xx_read/xx_write, with n and m as parameters?
A codec using snd_soc_7_9_spi_write/snd_soc_8_16_read_i2c should
select SPI and I2C in fact. Otherwise, why does it use these functions
as codec->hw_read()/codec->hw_write()  but not enable SPI and I2C? It
seems that defining snd_soc_7_9_spi_write and so on as NULL when SPI
is not selected is really useless, just let us pass the compiling to
get a module which can't work at all!

>
>> snd_soc_7_9_write() looks like it does a little more bit work than it
>> needs to ?  if data is declared as a u16, then you have:
>> u16 data = (reg << 9) | (value & 0x01ff);
>> this is what the ad1836 driver does now for its data split.
>
> Probably.  I'd need to check but I believe that's there to handle
> endianness variations in the host, though a cpu_to_ in what you have
> above ought to be able to take care of that.  The code was cut'n'pasted
> from what was in the drivers already.
>
>> in the mean time, rather than adding #ifdef to the codec driver, we
>> could create a local header like "bus-stubs.h" that stubs all the
>> relevant functions to an error value.  then all codec drivers that
>> dont use soc-cache can use that instead and the only change needed is
>> to add:
>> #include "bus-stubs.h"
>
> I'm not sure I feel up to doing that locally in ASoC rather than in the
> relevant subsystems.
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH 2/2] ASoC: Blackfin AD1836/AD1938 machine drivers: require SPI master

[Mark Brown]

On Sat, Oct 10, 2009 at 11:47:10AM +0800, Barry Song wrote:
> On Thu, Oct 8, 2009 at 6:01 PM, Mark Brown

> > That's the idea - add new functions for any new register formats.

> we do those based on the idea that it is a waste all codecs need to
> use almost same ways to handle register access. If we use soc-cache to

Not just that, the goal is to make it easier to make changes to the way
register accesses are managed by avoiding the need to go through each
and every single driver and update it.  The most obvious thing is
adding support for powering off the CODEC rather than just bringing it
down to standby.

> handle register access issues and run as abstract layer for all
> codecs, why not rename it to soc-reg or soc-bus? It seems cache is
> only a little part of the responsibility of  this module.

soc-reg (or probably register) would do equally well, like I say longer
term the goal is to support doing interesting things with the cache.  I
don't know that it's worth renaming at this point.

> It's better that functions like xx_7_9_read xx_7_9_write xx_8_8_read
> xx_8_8_write should not become API because we never know what will be
> the format for codecs. We should have a xx_n_m_read/xx_n_m_write or
> just a xx_read/xx_write, with n and m as parameters?

They're not APIs, they're only visible within the file.  Drivers using
the cache only see the one function they use to initialise the cache
with everything else in there hidden from them.  They're not specified
as part of the interface for the read and write functions because they'd
be a lot of noise for callers - they don't generally change at runtime
and are something that should really be abstracted away from the generic
code anyway.

> A codec using snd_soc_7_9_spi_write/snd_soc_8_16_read_i2c should
> select SPI and I2C in fact. Otherwise, why does it use these functions

The CODECs can't do this since they are selected themselves and Kconfig
ignores all dependencies from things that are selected.  In any case, a
given board is only going to need one or the other of the control
interfaces so the CODEC driver needs to leave this up to the machine
drivers anyway.  

The register cache code is the wrong level to be making decisions about
things like this, the CODEC drivers and the register cache code are
themselves not usable without a machine driver - this is utility code
which should just do whatever it is asked by its users rather than
forcing decisons on them.

> as codec->hw_read()/codec->hw_write()  but not enable SPI and I2C? It
> seems that defining snd_soc_7_9_spi_write and so on as NULL when SPI
> is not selected is really useless, just let us pass the compiling to
> get a module which can't work at all!

It's no use from the point of view of getting a working driver, yes, but
then to get something useful you really need the machine driver to
ensure that the correct controller driver is being built - simple
support for the bus is not enough, we also need the controller.  As
discussed earlier in this thread for some of the Blackfin machines it's
not even possible to do that since we can't tell which controller driver
needs to be used since the machine driver can be used with many boards.

Another thing to bear in mind here is that there are people who do
things like build the kernel with random .configs and we want to avoid
breaking their builds due to poor luck in the configuration they
generate while still getting the build coverage that results.  We also
need to be able to support compilation with either I2C or SPI but not
both in order to avoid forcing boards to include support for a bus that
may not be used anywhere on the board at all.  That means that both
buses need to be individually optional at the CODEC level.