From: grinberg@compulab•co.il (Igor Grinberg)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v2 5/7] ARM: OMAP2+: Split omap2_hsmmc_init() to properly support I2C GPIO pins
Date: Fri, 24 Feb 2012 09:01:57 +0200 [thread overview]
Message-ID: <4F4735E5.3070709@compulab.co.il> (raw)
In-Reply-To: <4F470DDA.7050603@ti.com>
On 02/24/12 06:11, Rajendra Nayak wrote:
> On Friday 24 February 2012 12:17 AM, Tony Lindgren wrote:
>> * Igor Grinberg<grinberg@compulab•co.il> [120223 05:56]:
>>>> --- a/arch/arm/mach-omap2/board-cm-t35.c
>>>> +++ b/arch/arm/mach-omap2/board-cm-t35.c
>>>> @@ -411,9 +411,9 @@ static struct omap2_hsmmc_info mmc[] = {
>>>> {
>>>> .mmc = 1,
>>>> .caps = MMC_CAP_4_BIT_DATA,
>>>> - .gpio_cd = -EINVAL,
>>>> + .gpio_cd = OMAP_MAX_GPIO_LINES + 0,
>>>> .gpio_wp = -EINVAL,
>>
>> I don't have these changes, in my second revision of the patch.
>>
>> It's best not to hardcode the values here.
>
> Ok, I just though doing it the other way is more round about
> hardcoding where in the board passes a hardcoded value to
> twl4030 gpio driver and the driver passes the *same* hardcoded
> value back to the board callback, which is then populated in
> mmc pdata. The board could as well directly hardcode it in the
> mmc pdata.
> That makes it much more readable than this round about
> hardcoding.
> But I'll go back to the old way if you think thats how we should
> handle this.
The "old" way is preferable. Thanks.
>
>>
>>>> -
>>>> + .deferred = true,
>>>> },
>>>> {
>>>> .mmc = 2,
>>>> @@ -422,6 +422,7 @@ static struct omap2_hsmmc_info mmc[] = {
>>>> .gpio_cd = -EINVAL,
>>>> .gpio_wp = -EINVAL,
>>>> .ocr_mask = 0x00100000, /* 3.3V */
>>>> + .deferred = true,
>>>
>>> Why do you defer this one?
>>> It does not use external GPIO chip, in fact it does not use CD/WP at all.
>>
>> Why do you have the following then to set gpio_cd?
>
> There is only one instance of gpio_cd being populated for the
> first element in the array mmc[], which is already deferred.
>
>>>> - mmc[0].gpio_cd = gpio + 0;
>
> Igor was asking about the second element in the array which never
> has gpio_cd populated. Its just initialized to -EINVAL and stays
> that way.
Exactly.
>
>>
>>>> },
>>>> {} /* Terminator */
>>>> };
>>>> @@ -469,9 +470,7 @@ static int cm_t35_twl_gpio_setup(struct device *dev, unsigned gpio,
>>>> pr_err("CM-T35: could not obtain gpio for WiFi reset\n");
>>>> }
>>>>
>>>> - /* gpio + 0 is "mmc0_cd" (input/IRQ) */
>>>> - mmc[0].gpio_cd = gpio + 0;
>>>> - omap2_hsmmc_init(mmc);
>>>> + omap_hsmmc_deferred_add(mmc);
>>>>
>>>> return 0;
>>>> }
>>
>> Hmm I don't have omap_hsmmc_deferred_add() in my second version
>> of the patch either.
>>
>> Rajendra, please do the patches on that, now it's impossible to
>> see what else you've changed. That's the version posted here:
>>
>> http://www.spinics.net/lists/linux-omap/msg64884.html
>
> Sorry for the mess up. I will make sure I have taken all changes/fixes
> from your reposts.
>
> regards,
> Rajendra
>>
>>>> @@ -639,6 +638,7 @@ static void __init cm_t3x_common_init(void)
>>>> omap_serial_init();
>>>> omap_sdrc_init(mt46h32m32lf6_sdrc_params,
>>>> mt46h32m32lf6_sdrc_params);
>>>> + omap_hsmmc_init(mmc);
>>>> cm_t35_init_i2c();
>>>> omap_ads7846_init(1, CM_T35_GPIO_PENDOWN, 0, NULL);
>>>> cm_t35_init_ethernet();
>>>
>>> Other then the comment above, looks fine.
>>> I will probably be able to test this on Sunday.
>>
>> OK
>>
>>>> diff --git a/arch/arm/mach-omap2/board-igep0020.c b/arch/arm/mach-omap2/board-igep0020.c
>>>> index a59ace0..11a6aa4 100644
>>>> --- a/arch/arm/mach-omap2/board-igep0020.c
>>>> +++ b/arch/arm/mach-omap2/board-igep0020.c
>>>> @@ -293,8 +293,9 @@ static struct omap2_hsmmc_info mmc[] = {
>>>> {
>>>> .mmc = 1,
>>>> .caps = MMC_CAP_4_BIT_DATA,
>>>> - .gpio_cd = -EINVAL,
>>>> + .gpio_cd = OMAP_MAX_GPIO_LINES + 0,
>>>> .gpio_wp = -EINVAL,
>>>> + .deferred = true,
>>>> },
>>>> #if defined(CONFIG_LIBERTAS_SDIO) || defined(CONFIG_LIBERTAS_SDIO_MODULE)
>>>> {
>>>> @@ -302,6 +303,7 @@ static struct omap2_hsmmc_info mmc[] = {
>>>> .caps = MMC_CAP_4_BIT_DATA,
>>>> .gpio_cd = -EINVAL,
>>>> .gpio_wp = -EINVAL,
>>>> + .deferred = true,
>>>
>>> same here, why defer it?
>>
>> Because it too sets gpio_cd in the callback.
Well, not this array element...
>>
>>> ditto
>>
>> ditto, that too sets gpio_cd..
Nope... ditto ;)
>>
>>>> },
>>>> #endif
>>>> {} /* Terminator */
>>>> @@ -360,10 +362,8 @@ static int omap3evm_twl_gpio_setup(struct device *dev,
>>>> {
>>>> int r, lcd_bl_en;
>>>>
>>>> - /* gpio + 0 is "mmc0_cd" (input/IRQ) */
>>>> omap_mux_init_gpio(63, OMAP_PIN_INPUT);
>>>> - mmc[0].gpio_cd = gpio + 0;
>>
>> ..here. Same for the others. So maybe check is some are wrong?
I failed parsing the question...
Here only the first array element gets the gpio_cd value set.
--
Regards,
Igor.
next prev parent reply other threads:[~2012-02-24 7:01 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-23 11:40 [PATCH v2 0/7] omap hsmmc init cleanup and section warning fixes for v3.4 merge window Rajendra Nayak
2012-02-23 11:40 ` [PATCH v2 1/7] ARM: OMAP: omap_device: Expose omap_device_{alloc, delete, register} Rajendra Nayak
2012-02-23 11:40 ` [PATCH v2 2/7] ARM: OMAP: omap_device: Add omap_device_unregister() Rajendra Nayak
2012-02-23 11:40 ` [PATCH v2 3/7] mmc: omap_hsmmc: Make the driver support hotpluggable devices Rajendra Nayak
2012-02-23 11:40 ` [PATCH v2 4/7] mmc: omap_hsmmc: If probe fails, give our error messages Rajendra Nayak
2012-02-23 11:40 ` [PATCH v2 5/7] ARM: OMAP2+: Split omap2_hsmmc_init() to properly support I2C GPIO pins Rajendra Nayak
2012-02-23 14:28 ` Igor Grinberg
2012-02-23 18:47 ` Tony Lindgren
2012-02-24 4:11 ` Rajendra Nayak
2012-02-24 7:01 ` Igor Grinberg [this message]
2012-02-24 22:19 ` Tony Lindgren
2012-02-24 3:24 ` Rajendra Nayak
2012-02-23 11:40 ` [PATCH v2 6/7] ARM: OMAP3: Use .teardown of twl4030-gpio to clean board requests Rajendra Nayak
2012-02-23 14:55 ` Igor Grinberg
2012-02-24 3:43 ` Rajendra Nayak
2012-02-23 11:40 ` [PATCH v2 7/7] ARM: OMAP2+: Mark omap_hsmmc_init and omap_mux related functions as __init Rajendra Nayak
2012-03-19 9:08 ` Tomi Valkeinen
2012-03-19 10:20 ` Tomi Valkeinen
2012-03-19 18:09 ` Tony Lindgren
2012-03-20 7:30 ` Tomi Valkeinen
2012-03-20 22:17 ` Tony Lindgren
2012-02-23 18:52 ` [PATCH v2 0/7] omap hsmmc init cleanup and section warning fixes for v3.4 merge window Tony Lindgren
2012-02-24 4:14 ` Rajendra Nayak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F4735E5.3070709@compulab.co.il \
--to=grinberg@compulab$(echo .)co.il \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox