[alsa-devel] [PATCH] EDB93xx: Add support for CS4271 CODEC on EDB93xx boards
From: Alexander Sverdlin subaparts@yandex.ru
Add support for CS4271 SPI-connected CODEC on EDB93xx boards. Major rework based on code provided by H Hartley Sweeten hsweeten@visionengravers.com. Tested on EDB9302, others (EDB9301, EDB9302A, EDB9307A, EDB0315A) should work.
Signed-off-by: Alexander Sverdlin subaparts@yandex.ru --- arch/arm/mach-ep93xx/edb93xx.c | 117 ++++++++++++++++++++++++++++++++++++++++ 1 files changed, 117 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-ep93xx/edb93xx.c b/arch/arm/mach-ep93xx/edb93xx.c index 4b04316..f20be96 100644 --- a/arch/arm/mach-ep93xx/edb93xx.c +++ b/arch/arm/mach-ep93xx/edb93xx.c @@ -30,12 +30,22 @@ #include <linux/gpio.h> #include <linux/i2c.h> #include <linux/i2c-gpio.h> +#include <linux/spi/spi.h> +#include <linux/delay.h> +#include <mach/ep93xx_spi.h>
#include <mach/hardware.h>
#include <asm/mach-types.h> #include <asm/mach/arch.h>
+#include <sound/cs4271.h> + +#define edb93xx_has_audio() (machine_is_edb9301() || \ + machine_is_edb9302() || \ + machine_is_edb9302a() || \ + machine_is_edb9307a() || \ + machine_is_edb9315a())
static void __init edb93xx_register_flash(void) { @@ -93,6 +103,111 @@ static void __init edb93xx_register_i2c(void)
/************************************************************************* + * EDB93xx SPI peripheral handling + *************************************************************************/ +static int edb93xx_cs4271_hw_setup(struct spi_device *spi) +{ + int gpio_nreset; + int err; + + if (machine_is_edb9301() || machine_is_edb9302()) { + gpio_nreset = EP93XX_GPIO_LINE_EGPIO1; + } else if (machine_is_edb9302a() || machine_is_edb9307a()) { + ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_HONIDE); + gpio_nreset = EP93XX_GPIO_LINE_DD2; + } else if (machine_is_edb9315a()) { + gpio_nreset = EP93XX_GPIO_LINE_EGPIO14; + } else { + return -EINVAL; + } + + err = gpio_request(gpio_nreset, spi->modalias); + if (err) + return err; + err = gpio_request(EP93XX_GPIO_LINE_EGPIO6, spi->modalias); + if (err) + return err; + + gpio_direction_output(EP93XX_GPIO_LINE_EGPIO6, 1); + + /* Reset codec */ + gpio_direction_output(gpio_nreset, 0); + udelay(1); + gpio_set_value(gpio_nreset, 1); + /* Give the codec time to wake up */ + udelay(1); + + return 0; +} + +static void edb93xx_cs4271_hw_cleanup(struct spi_device *spi) +{ + int gpio_nreset; + + if (machine_is_edb9301() || machine_is_edb9302()) + gpio_nreset = EP93XX_GPIO_LINE_EGPIO1; + else if (machine_is_edb9302a() || machine_is_edb9307a()) + gpio_nreset = EP93XX_GPIO_LINE_DD2; + else if (machine_is_edb9315a()) + gpio_nreset = EP93XX_GPIO_LINE_EGPIO14; + else + return; + + gpio_set_value(gpio_nreset, 0); + gpio_free(gpio_nreset); + + gpio_set_value(EP93XX_GPIO_LINE_EGPIO6, 1); + gpio_free(EP93XX_GPIO_LINE_EGPIO6); +} + +static void edb93xx_cs4271_hw_cs_control(struct spi_device *spi, int value) +{ + gpio_set_value(EP93XX_GPIO_LINE_EGPIO6, value); +} + +static struct ep93xx_spi_chip_ops edb93xx_cs4271_hw = { + .setup = edb93xx_cs4271_hw_setup, + .cleanup = edb93xx_cs4271_hw_cleanup, + .cs_control = edb93xx_cs4271_hw_cs_control, +}; + +static struct spi_board_info edb93xx_spi_board_info[] __initdata = { + { + .modalias = "cs4271", + .controller_data = &edb93xx_cs4271_hw, + .max_speed_hz = 6000000, + .bus_num = 0, + .chip_select = 0, + .mode = SPI_MODE_3, + }, +}; + +static struct ep93xx_spi_info edb93xx_spi_info = { + .num_chipselect = ARRAY_SIZE(edb93xx_spi_board_info), +}; + +static void __init edb93xx_register_spi(void) +{ + if (edb93xx_has_audio()) { + ep93xx_register_spi(&edb93xx_spi_info, + edb93xx_spi_board_info, + ARRAY_SIZE(edb93xx_spi_board_info)); + } +} + + +/************************************************************************* + * EDB93xx I2S + *************************************************************************/ +static void __init edb93xx_register_i2s(void) +{ + if (edb93xx_has_audio()) { + ep93xx_register_i2s(); + } +} + + +/************************************************************************* * EDB93xx pwm *************************************************************************/ static void __init edb93xx_register_pwm(void) @@ -117,6 +232,8 @@ static void __init edb93xx_init_machine(void) edb93xx_register_flash(); ep93xx_register_eth(&edb93xx_eth_data, 1); edb93xx_register_i2c(); + edb93xx_register_spi(); + edb93xx_register_i2s(); edb93xx_register_pwm(); }
On Tuesday, February 01, 2011 4:41 PM, Alexander Sverdlin wrote:
From: Alexander Sverdlin subaparts@yandex.ru
Add support for CS4271 SPI-connected CODEC on EDB93xx boards. Major rework based on code provided by H Hartley Sweeten hsweeten@visionengravers.com. Tested on EDB9302, others (EDB9301, EDB9302A, EDB9307A, EDB0315A) should work.
Signed-off-by: Alexander Sverdlin subaparts@yandex.ru
arch/arm/mach-ep93xx/edb93xx.c | 117 ++++++++++++++++++++++++++++++++++++++++ 1 files changed, 117 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-ep93xx/edb93xx.c b/arch/arm/mach-ep93xx/edb93xx.c index 4b04316..f20be96 100644 --- a/arch/arm/mach-ep93xx/edb93xx.c +++ b/arch/arm/mach-ep93xx/edb93xx.c @@ -30,12 +30,22 @@ #include <linux/gpio.h> #include <linux/i2c.h> #include <linux/i2c-gpio.h> +#include <linux/spi/spi.h> +#include <linux/delay.h> +#include <mach/ep93xx_spi.h>
#include <mach/hardware.h>
#include <asm/mach-types.h> #include <asm/mach/arch.h>
+#include <sound/cs4271.h>
+#define edb93xx_has_audio() (machine_is_edb9301() || \
machine_is_edb9302() || \machine_is_edb9302a() || \machine_is_edb9307a() || \machine_is_edb9315a())static void __init edb93xx_register_flash(void) { @@ -93,6 +103,111 @@ static void __init edb93xx_register_i2c(void)
/*************************************************************************
- EDB93xx SPI peripheral handling
- *************************************************************************/
+static int edb93xx_cs4271_hw_setup(struct spi_device *spi) +{
- int gpio_nreset;
- int err;
- if (machine_is_edb9301() || machine_is_edb9302()) {
gpio_nreset = EP93XX_GPIO_LINE_EGPIO1;
Are you planning on removing gpio_nreset and gpio_disable from struct cs4271_platform_data? If not, you should just setup the gpio mapping in edb93xx_register_spi before you actually register the spi_board_info. Then this function and the following two can just do something like the following and not worry about the if ... else if ... etc.
struct cs4271_platform_data *cs4271 = spi->dev.platform_data; int err;
err = gpio_request(cs4271->gpio_disable, spi->modalias); ...
- } else if (machine_is_edb9302a() || machine_is_edb9307a()) {
ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_HONIDE);
You shouldn't need this. The ep93xx gpio core will now defaults all the ports to gpio mode. See commit fd015480c29deb52ae3bfaf41e888c450765edd8.
gpio_nreset = EP93XX_GPIO_LINE_DD2;
Please use EP93XX_GPIO_LINE_H(2) instead. None of the datasheets reference the "DD2" gpio.
- } else if (machine_is_edb9315a()) {
gpio_nreset = EP93XX_GPIO_LINE_EGPIO14;- } else {
return -EINVAL;- }
- err = gpio_request(gpio_nreset, spi->modalias);
- if (err)
return err;- err = gpio_request(EP93XX_GPIO_LINE_EGPIO6, spi->modalias);
- if (err)
return err;- gpio_direction_output(EP93XX_GPIO_LINE_EGPIO6, 1);
- /* Reset codec */
- gpio_direction_output(gpio_nreset, 0);
- udelay(1);
- gpio_set_value(gpio_nreset, 1);
- /* Give the codec time to wake up */
- udelay(1);
- return 0;
+}
+static void edb93xx_cs4271_hw_cleanup(struct spi_device *spi) +{
- int gpio_nreset;
- if (machine_is_edb9301() || machine_is_edb9302())
gpio_nreset = EP93XX_GPIO_LINE_EGPIO1;- else if (machine_is_edb9302a() || machine_is_edb9307a())
gpio_nreset = EP93XX_GPIO_LINE_DD2;- else if (machine_is_edb9315a())
gpio_nreset = EP93XX_GPIO_LINE_EGPIO14;- else
return;- gpio_set_value(gpio_nreset, 0);
- gpio_free(gpio_nreset);
- gpio_set_value(EP93XX_GPIO_LINE_EGPIO6, 1);
- gpio_free(EP93XX_GPIO_LINE_EGPIO6);
+}
+static void edb93xx_cs4271_hw_cs_control(struct spi_device *spi, int value) +{
- gpio_set_value(EP93XX_GPIO_LINE_EGPIO6, value);
+}
+static struct ep93xx_spi_chip_ops edb93xx_cs4271_hw = {
- .setup = edb93xx_cs4271_hw_setup,
- .cleanup = edb93xx_cs4271_hw_cleanup,
- .cs_control = edb93xx_cs4271_hw_cs_control,
+};
+static struct spi_board_info edb93xx_spi_board_info[] __initdata = {
- {
.modalias = "cs4271",.controller_data = &edb93xx_cs4271_hw,.max_speed_hz = 6000000,.bus_num = 0,.chip_select = 0,.mode = SPI_MODE_3,- },
+};
+static struct ep93xx_spi_info edb93xx_spi_info = {
- .num_chipselect = ARRAY_SIZE(edb93xx_spi_board_info),
+};
+static void __init edb93xx_register_spi(void) +{
- if (edb93xx_has_audio()) {
ep93xx_register_spi(&edb93xx_spi_info,edb93xx_spi_board_info,ARRAY_SIZE(edb93xx_spi_board_info));- }
+}
+/*************************************************************************
- EDB93xx I2S
- *************************************************************************/
+static void __init edb93xx_register_i2s(void) +{
- if (edb93xx_has_audio()) {
ep93xx_register_i2s();- }
+}
+/*************************************************************************
- EDB93xx pwm
*************************************************************************/ static void __init edb93xx_register_pwm(void) @@ -117,6 +232,8 @@ static void __init edb93xx_init_machine(void) edb93xx_register_flash(); ep93xx_register_eth(&edb93xx_eth_data, 1); edb93xx_register_i2c();
- edb93xx_register_spi();
- edb93xx_register_i2s(); edb93xx_register_pwm();
}
Other than using struct cs4271_platform_data to hold the gpio information, this looks better. Now other spi devices can live on bus.
Hartley
Dear Hartley, Dear Mark,
On Tue, 2011-02-01 at 18:02 -0600, H Hartley Sweeten wrote:
+static int edb93xx_cs4271_hw_setup(struct spi_device *spi) +{
- int gpio_nreset;
- int err;
- if (machine_is_edb9301() || machine_is_edb9302()) {
gpio_nreset = EP93XX_GPIO_LINE_EGPIO1;Are you planning on removing gpio_nreset and gpio_disable from struct cs4271_platform_data? If not, you should just setup the gpio mapping in edb93xx_register_spi before you actually register the spi_board_info. Then this function and the following two can just do something like the following and not worry about the if ... else if ... etc.
Initially I had GPIO code in machine driver, but during the conversations in alsa-devel list it has been moved to CODEC driver: http://mailman.alsa-project.org/pipermail/alsa-devel/2010-October/032358.htm...
I'm really confused now, where should it be? If I will init cs4271_platform_data with GPIO values, CODEC driver will manage it. Otherwise CODEC driver should be rewritten without GPIO support and I can add it to platform code.
Mark Brown, could you please comment on this, as I've moved GPIO things to CODEC part based on your review.
Best regards, Alexander Sverdlin.
On Wed, Feb 02, 2011 at 01:48:08PM +0300, Alexander Sverdlin wrote:
I'm really confused now, where should it be? If I will init cs4271_platform_data with GPIO values, CODEC driver will manage it. Otherwise CODEC driver should be rewritten without GPIO support and I can add it to platform code.
What is the problem?
Dear Mark,
On Wed, 2011-02-02 at 10:49 +0000, Mark Brown wrote:
On Wed, Feb 02, 2011 at 01:48:08PM +0300, Alexander Sverdlin wrote:
I'm really confused now, where should it be? If I will init cs4271_platform_data with GPIO values, CODEC driver will manage it. Otherwise CODEC driver should be rewritten without GPIO support and I can add it to platform code.
What is the problem?
On 07 OCT 2010 you advised to integrate GPIO handling (reset and SPI-enable) into CODEC code flow.
Now Hartley suggested to move GPIO handling to platform code. This makes sense, as SPI-enable will be managed automatically, and reset is also more related to platform (depending on schematics, inversion and so on).
As for me, I think handle it in platform code is better. So we can now remove all the GPIO code and cs4271_platform_data structure from CS4271 CODEC.
Best regards, Alexander.
On Wed, Feb 02, 2011 at 02:12:47PM +0300, Alexander Sverdlin wrote:
On 07 OCT 2010 you advised to integrate GPIO handling (reset and SPI-enable) into CODEC code flow.
Now Hartley suggested to move GPIO handling to platform code. This makes sense, as SPI-enable will be managed automatically, and reset is also more related to platform (depending on schematics, inversion and so on).
What is the advantage of this? The GPIO management has to be done somewhere and replicating it in every platform seems like a complete waste of time. Given that one of the GPIOs is a power control it'll also mean that it's not possible to manage the power dynamically at runtime.
Dear Mark, Dear Hartley,
On Wed, 2011-02-02 at 12:53 +0000, Mark Brown wrote:
Now Hartley suggested to move GPIO handling to platform code. This makes sense, as SPI-enable will be managed automatically, and reset is also more related to platform (depending on schematics, inversion and so on).
What is the advantage of this? The GPIO management has to be done somewhere and replicating it in every platform seems like a complete waste of time. Given that one of the GPIOs is a power control it'll also mean that it's not possible to manage the power dynamically at runtime.
I think we can leave reset management in CODEC (as it will be almost the same for all platforms), leave enable pin management in CODEC unused, but attach enable GPIO to spi_board_info in platform to enable other SPI devices on bus.
Hartley, in this case we cannot use existing cs4271_platform_data and have to leave long if-else statemens as is. Would it be ok?
Best regards, Alexander.
On Wednesday, February 02, 2011 6:27 AM, Alexander Sverdlin wrote:
Dear Mark, Dear Hartley,
On Wed, 2011-02-02 at 12:53 +0000, Mark Brown wrote:
Now Hartley suggested to move GPIO handling to platform code. This makes sense, as SPI-enable will be managed automatically, and reset is also more related to platform (depending on schematics, inversion and so on).
What is the advantage of this? The GPIO management has to be done somewhere and replicating it in every platform seems like a complete waste of time. Given that one of the GPIOs is a power control it'll also mean that it's not possible to manage the power dynamically at runtime.
I think we can leave reset management in CODEC (as it will be almost the same for all platforms), leave enable pin management in CODEC unused, but attach enable GPIO to spi_board_info in platform to enable other SPI devices on bus.
Hartley, in this case we cannot use existing cs4271_platform_data and have to leave long if-else statemens as is. Would it be ok?
I don't have any problem with leaving the reset management in the codec. The enable (chip-select) should be handled in the platform code. But, gpio_disable should be removed from struct cs4271_platform_data to avoid any confusion.
I have an idea on how to remove the if-else mess in the ep93xx platform init. Please make any updates you have and post the patches. I will comment on it then.
Thanks, Hartley
On Wed, Feb 02, 2011 at 10:53:15AM -0600, H Hartley Sweeten wrote:
I don't have any problem with leaving the reset management in the codec. The enable (chip-select) should be handled in the platform code. But, gpio_disable should be removed from struct cs4271_platform_data to avoid any confusion.
Is that really a chip select or is it a power control?
On Wednesday, February 02, 2011 10:00 AM, Mark Brown wrote:
On Wed, Feb 02, 2011 at 10:53:15AM -0600, H Hartley Sweeten wrote:
I don't have any problem with leaving the reset management in the codec. The enable (chip-select) should be handled in the platform code. But, gpio_disable should be removed from struct cs4271_platform_data to avoid any confusion.
Is that really a chip select or is it a power control?
_From the datasheet:
Pin Name # Pin Description -------- -- ---------------------------------------------------------- AD0/nCS 13 Address Bit 0 (I2S)/Control Port Chip Select (SPI) (Input) - AD0 is a chip address pin in I2C mode; nCS is the chip signal select for SPI format.
The other pin in question:
Pin Name # Pin Description -------- -- ---------------------------------------------------------- nRST 14 Reset (Input) - The device enters a low power mode when this pin is driven low.
Regards, Hartley=
Dear Mark,
On Wed, 2011-02-02 at 17:00 +0000, Mark Brown wrote:
On Wed, Feb 02, 2011 at 10:53:15AM -0600, H Hartley Sweeten wrote:
I don't have any problem with leaving the reset management in the codec. The enable (chip-select) should be handled in the platform code. But, gpio_disable should be removed from struct cs4271_platform_data to avoid any confusion.
Is that really a chip select or is it a power control?
It's a chip select. The way I've managed it in CODEC it's because Cirrus boards do not have any other SPI devices. Supported by current mainline, at least.
On Wednesday, February 02, 2011 10:33 AM, Alexander Sverdlin wrote:
It's a chip select. The way I've managed it in CODEC it's because Cirrus boards do not have any other SPI devices. Supported by current mainline, at least.
All the EDB93xx boards also have an spi flash device. And there are a couple drivers in mainline that can be used. Depending on the actual spi flash device: drivers/mtd/m25p80.c, drivers/mtd/sst25l.c, or /drivers/misc/eeprom/at25.c.
There are a couple patches floating around to add the spi flash support to the edb93xx platform init but nothing is merged yet.
BTW, you really should remove the or-gate's that Cirrus used with the spi chip-selects to logically-and the SFRM and GPIO signals. The SFRM signal deasserts when the spi fifo's empty which could cause a chip-select deassertion to the chip in the middle of a transfer. For the spi flash, this breaks any read's from the device. Just connect the gpio line directly to the chip-select of the device.
Regards, Hartley
On Wednesday, February 02, 2011 6:27 AM, Alexander Sverdlin wrote:
I think we can leave reset management in CODEC (as it will be almost the same for all platforms), leave enable pin management in CODEC unused, but attach enable GPIO to spi_board_info in platform to enable other SPI devices on bus.
Hartley, in this case we cannot use existing cs4271_platform_data and have to leave long if-else statemens as is. Would it be ok?
Alexander,
Below is what I propose for the platform init code. This assumes that the reset gpio will still be handled by the codec since it's related to power management.
Regards, Hartley
static struct cs4271_platform_data edb93xx_cs4271_data = { .gpio_nreset = -EINVAL, /* filled in later */ };
static int edb93xx_cs4271_hw_setup(struct spi_device *spi) { return gpio_request_one(EP93XX_GPIO_LINE_EGPIO6, GPIOF_OUT_INIT_HIGH, spi->modalias); }
static void edb93xx_cs4271_hw_cleanup(struct spi_device *spi) { gpio_free(EP93XX_GPIO_LINE_EGPIO6); }
static void edb93xx_cs4271_hw_cs_control(struct spi_device *spi, int value) { gpio_set_value(EP93XX_GPIO_LINE_EGPIO6, value); }
static struct ep93xx_spi_chip_ops edb93xx_cs4721_hw = { .setup = edb93xx_cs4271_hw_setup, .cleanup = edb93xx_cs4271_hw_cleanup, .cs_control = edb93xx_cs4271_hw_cs_control, };
static struct spi_board_info edb93xx_spi_board_info[] __initdata = { { .modalias = "cs4271", .platform_data = &edb93xx_cs4271_data, .controller_data = &edb93xx_cs4721_hw, .max_speed_hz = 6000000, .bus_num = 0, .chip_select = 0, .mode = SPI_MODE_3, }, };
static struct ep93xx_spi_info edb93xx_spi_master __initdata = { .num_chipselect = ARRAY_SIZE(edb93xx_spi_board_info), };
static void __init edb93xx_register_spi(void) { if (machine_is_edb9301() || machine_is_edb9302()) { edb93xx_cs4271_data.gpio_nreset = EP93XX_GPIO_LINE_EGPIO1; } else if (machine_is_edb9302a() || machine_is_edb9307a()) { edb93xx_cs4271_data.gpio_nreset = EP93XX_GPIO_LINE_H(2); } else if (machine_is_edb9315a()) { edb93xx_cs4271_data.gpio_nreset = EP93XX_GPIO_LINE_EGPIO14; }
ep93xx_register_spi(&edb93xx_spi_master, edb93xx_spi_board_info, ARRAY_SIZE(edb93xx_spi_board_info)); }
On Wednesday, February 02, 2011 3:48 AM, Alexander Sverdlin wrote:
Dear Hartley, Dear Mark,
On Tue, 2011-02-01 at 18:02 -0600, H Hartley Sweeten wrote:
+static int edb93xx_cs4271_hw_setup(struct spi_device *spi) +{
- int gpio_nreset;
- int err;
- if (machine_is_edb9301() || machine_is_edb9302()) {
gpio_nreset = EP93XX_GPIO_LINE_EGPIO1;Are you planning on removing gpio_nreset and gpio_disable from struct cs4271_platform_data? If not, you should just setup the gpio mapping in edb93xx_register_spi before you actually register the spi_board_info. Then this function and the following two can just do something like the following and not worry about the if ... else if ... etc.
Initially I had GPIO code in machine driver, but during the conversations in alsa-devel list it has been moved to CODEC driver: http://mailman.alsa-project.org/pipermail/alsa-devel/2010-October/032358.htm...
I'm really confused now, where should it be? If I will init cs4271_platform_data with GPIO values, CODEC driver will manage it. Otherwise CODEC driver should be rewritten without GPIO support and I can add it to platform code.
Mark Brown, could you please comment on this, as I've moved GPIO things to CODEC part based on your review.
The chip-select request/configuration needs to be done before loading the driver so that the spi subsystem can actually select the chip. Also, some spi master drivers might not even use a 'gpio' for the chip-select. They might have dedicated pins that are used.
For the reset, again a gpio might not actually be used. The systems reset signal might be directly connected to the codec.
Regardless, the cs4271 codec driver isn't doing anything with the gpios except in the probe function. And all it's does there is request and configure them, drive the chip-select gpio low (permenatly enabing it), and then toggles the reset line to reset the codec.
It appears to me that none of this should be in the codec driver itself. It's really platform specific and serves no use in the driver. The <sound/cs4271.h> header should just be removed. Let the platform deal with the non-generic configuration and setup of the gpio's.
Regards, Hartley
On Wed, Feb 02, 2011 at 02:40:53AM +0300, Alexander Sverdlin wrote:
From: Alexander Sverdlin subaparts@yandex.ru
Add support for CS4271 SPI-connected CODEC on EDB93xx boards. Major rework based on code provided by H Hartley Sweeten hsweeten@visionengravers.com. Tested on EDB9302, others (EDB9301, EDB9302A, EDB9307A, EDB0315A) should work.
One minor comment, see below.
Signed-off-by: Alexander Sverdlin subaparts@yandex.ru
arch/arm/mach-ep93xx/edb93xx.c | 117 ++++++++++++++++++++++++++++++++++++++++ 1 files changed, 117 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-ep93xx/edb93xx.c b/arch/arm/mach-ep93xx/edb93xx.c index 4b04316..f20be96 100644 --- a/arch/arm/mach-ep93xx/edb93xx.c +++ b/arch/arm/mach-ep93xx/edb93xx.c @@ -30,12 +30,22 @@ #include <linux/gpio.h> #include <linux/i2c.h> #include <linux/i2c-gpio.h> +#include <linux/spi/spi.h> +#include <linux/delay.h> +#include <mach/ep93xx_spi.h>
#include <mach/hardware.h>
#include <asm/mach-types.h> #include <asm/mach/arch.h>
+#include <sound/cs4271.h>
+#define edb93xx_has_audio() (machine_is_edb9301() || \
machine_is_edb9302() || \machine_is_edb9302a() || \machine_is_edb9307a() || \machine_is_edb9315a())static void __init edb93xx_register_flash(void) { @@ -93,6 +103,111 @@ static void __init edb93xx_register_i2c(void)
/*************************************************************************
- EDB93xx SPI peripheral handling
- *************************************************************************/
+static int edb93xx_cs4271_hw_setup(struct spi_device *spi) +{
- int gpio_nreset;
- int err;
- if (machine_is_edb9301() || machine_is_edb9302()) {
gpio_nreset = EP93XX_GPIO_LINE_EGPIO1;- } else if (machine_is_edb9302a() || machine_is_edb9307a()) {
ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_HONIDE);gpio_nreset = EP93XX_GPIO_LINE_DD2;- } else if (machine_is_edb9315a()) {
gpio_nreset = EP93XX_GPIO_LINE_EGPIO14;- } else {
return -EINVAL;- }
- err = gpio_request(gpio_nreset, spi->modalias);
- if (err)
return err;- err = gpio_request(EP93XX_GPIO_LINE_EGPIO6, spi->modalias);
- if (err)
Should you call gpio_free() for gpio_nreset here?
Regards, MW
return err;- gpio_direction_output(EP93XX_GPIO_LINE_EGPIO6, 1);
- /* Reset codec */
- gpio_direction_output(gpio_nreset, 0);
- udelay(1);
- gpio_set_value(gpio_nreset, 1);
- /* Give the codec time to wake up */
- udelay(1);
- return 0;
+}
On Wednesday, February 02, 2011 12:55 AM, Mika Westerberg wrote:
/*************************************************************************
- EDB93xx SPI peripheral handling
- *************************************************************************/
+static int edb93xx_cs4271_hw_setup(struct spi_device *spi) +{
- int gpio_nreset;
- int err;
- if (machine_is_edb9301() || machine_is_edb9302()) {
gpio_nreset = EP93XX_GPIO_LINE_EGPIO1;- } else if (machine_is_edb9302a() || machine_is_edb9307a()) {
ep93xx_devcfg_set_bits(EP93XX_SYSCON_DEVCFG_HONIDE);gpio_nreset = EP93XX_GPIO_LINE_DD2;- } else if (machine_is_edb9315a()) {
gpio_nreset = EP93XX_GPIO_LINE_EGPIO14;- } else {
return -EINVAL;- }
- err = gpio_request(gpio_nreset, spi->modalias);
- if (err)
return err;- err = gpio_request(EP93XX_GPIO_LINE_EGPIO6, spi->modalias);
- if (err)
Should you call gpio_free() for gpio_nreset here?
Yes. If the second gpio_request fails, the first gpio should be freed.
A cleaner way of handling the gpios would be to use gpio_request_array() and remove the gpio knowledge from the codec driver completely.
Hartley
On Wed, Feb 02, 2011 at 02:40:53AM +0300, Alexander Sverdlin wrote:
- err = gpio_request(gpio_nreset, spi->modalias);
- if (err)
return err;- err = gpio_request(EP93XX_GPIO_LINE_EGPIO6, spi->modalias);
- if (err)
return err;- gpio_direction_output(EP93XX_GPIO_LINE_EGPIO6, 1);
Why is this stuff in the arch/arm code? I'd expect driver to deal with all this sequencing.
participants (4)
-
Alexander Sverdlin -
H Hartley Sweeten -
Mark Brown -
Mika Westerberg