[alsa-devel] [PATCH v4 7/9] davinci: MMC/SD support for Omapl138-Hawkboar

[Victor Rodriguez]

On Fri, Oct 15, 2010 at 11:25 AM, Sergei Shtylyov <sshtylyov at mvista.com> wrote:
> Hello.
>
> On 10/15/10 07:08, vm.rod25 at gmail.com wrote:
>
>> From: Victor Rodriguez<victor.rodriguez at sasken.com>
>
>> This patch adds MMC/SD support for the Hawkboard-L138 system
>
>> Signed-off-by: Victor Rodriguez<victor.rodriguez at sasken.com>
>
> [...]
>
>> diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c
>> b/arch/arm/mach-davinci/board-omapl138-hawk.c
>> index 6f76220..87dea28 100644
>> --- a/arch/arm/mach-davinci/board-omapl138-hawk.c
>> +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
>
> [...]
>>
>> @@ -180,6 +208,28 @@ static __init void omapl138_hawk_init(void)
>>                pr_warning("%s: mcasp mux setup failed: %d\n", __func__,
>> ret);
>>        da8xx_register_mcasp(0,&omapl138_hawk_snd_data);
>
>   I'd put all the following into a sperate function.

Thanks for the feedback Sergei I will fix that in a void function
similar to omapl138_hawk_usb_init on the 9/9 patch

>> +       ret = davinci_cfg_reg_list(hawk_mmcsd0_pins);
>> +       if (ret)
>> +               pr_warning("%s: MMC/SD0 mux setup failed: %d\n",
>> +                       __func__, ret);
>> +
>> +       ret = gpio_request(DA850_HAWK_MMCSD_CD_PIN, "MMC CD\n");
>> +       if (ret)
>> +               pr_warning("%s: can not open GPIO %d\n",
>> +                       __func__, DA850_HAWK_MMCSD_CD_PIN);
>> +       gpio_direction_input(DA850_HAWK_MMCSD_CD_PIN);
>
>   You cannot call that if gpio_request{} failed.



I think that is the idea. I am really confused here I have found in
Documentation/gpio.txt that

[...]

Using GPIOs
-----------
The first thing a system should do with a GPIO is allocate it, using
the gpio_request() call; see later.

One of the next things to do with a GPIO, often in board setup code when
setting up a platform_device using the GPIO, is mark its direction:

	/* set as input or output, returning 0 or negative errno */
	int gpio_direction_input(unsigned gpio);
	int gpio_direction_output(unsigned gpio, int value);

( ========== This is what I tried to implement ==================== )

The return value is zero for success, else a negative errno.  It should
be checked, since the get/set calls don't have error returns and since
misconfiguration is possible.  You should normally issue these calls from
a task context.  However, for spinlock-safe GPIOs it's OK to use them
before tasking is enabled, as part of early board setup.

[...]

GPIO access that may sleep


  ** IN ADDITION ** calls to setup and configure such GPIOs must be made
from contexts which may sleep, since they may need to access the GPIO
controller chip too:  (These setup calls are usually made from board
setup or driver probe/teardown code, so this is an easy constraint.)

	gpio_direction_input()
	gpio_direction_output()
	gpio_request()

( ========== Here is my confusion it is a different order
==================== )

## 	gpio_request_one()
##	gpio_request_array()
## 	gpio_free_array()

	gpio_free()
	gpio_set_debounce()


I think that you mean that I should not call gpio_direction_input if
gpio_request failed am I right ?

Regards

Victor Rodriguez




>> +       ret = gpio_request(DA850_HAWK_MMCSD_WP_PIN, "MMC WP\n");
>> +       if (ret)
>> +               pr_warning("%s: can not open GPIO %d\n",
>> +                       __func__, DA850_HAWK_MMCSD_WP_PIN);
>> +       gpio_direction_input(DA850_HAWK_MMCSD_WP_PIN);
>
>   You cannot call that if gpio_request{} failed.
>
>> +       ret = da8xx_register_mmcsd0(&da850_mmc_config);
>> +       if (ret)
>> +               pr_warning("%s: MMC/SD0 registration failed: %d\n",
>> +                       __func__, ret);
>> +
>>        ret = da8xx_register_watchdog();
>>        if (ret)
>>                pr_warning("omapl138_hawk_init: "
>
> WBR, Sergei
>