linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: kishore kadiyala <kishorek.kadiyala@gmail.com>
To: Adrian Hunter <adrian.hunter@nokia.com>
Cc: kishore kadiyala <kishore.kadiyala@ti.com>,
	"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
	"tony@atomide.com" <tony@atomide.com>,
	"madhu.cr@ti.com" <madhu.cr@ti.com>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>
Subject: Re: [PATCH v5 2/2] OMAP4 HSMMC: Adding card detect support for MMC1 Controller
Date: Mon, 28 Jun 2010 21:40:56 +0530	[thread overview]
Message-ID: <AANLkTimVB7N0Oa9bDbYfSfcivy6afhvHW5Bdt-8dWVxD@mail.gmail.com> (raw)
In-Reply-To: <4C1FB2A0.6000503@nokia.com>

Adrian ,

Sorry for the late response

<snip>
>
> As per my email 5/5/10, I would suggest the only change to omap_hsmmc is:

Agreed  and followed the changes mostly but made some more changes on top of it.

>
<snip>
> And that the late init function is used to do the rest e.g.
> find a home for these 3 functions:

I agree just having the 3 functions makes it work.

>
> static int omap4_twl6030_hsmmc_late_init(struct device *dev)
> {
>        int ret = 0;
>        struct platform_device *pdev = container_of(dev,
>                                        struct platform_device, dev);
>        struct omap_mmc_platform_data *pdata = dev->platform_data;
>
>        /* MMC1 Card detect Configuration */
>        if (pdev->id == 0) {
>                ret = omap4_hsmmc1_card_detect_config();
>                if (ret < 0)
>                        pr_err("Unable to configure Card detect for MMC1\n");
>                pdata->slots[0].card_detect = twl6030_mmc_card_detect;
>                pdata->slots[0].card_detect_irq = TWL6030_IRQ_BASE +
>                                                  MMCDETECT_INTR_OFFSET;
>        }
>        return ret;

<snip>

Few Comments below:

1) In the above function, initializing "card_detect"  in the driver as
done in omap_hsmmc_gpio_init might be more readable and this has been
done in nongpio_init instead.
Even having initialization of  "card_detect_irq" inside nongpio_init is fine.

2)Also calling omap_hsmmc_gpio_init in case of a card detect line
which is not GPIO
doesn't make sense though it assigns -EINVAL to switch_pin in case of
invalid GPIO
which is intended for a non-removable card .

3) And also having some thing like GPIO and NON_GPIO flag to
distinguish might make sense.

Regards,
Kishore

  reply	other threads:[~2010-06-28 16:10 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-17 15:27 [PATCH v5 2/2] OMAP4 HSMMC: Adding card detect support for MMC1 Controller kishore kadiyala
2010-06-17 20:15 ` Andrew Morton
2010-07-01 12:33   ` Tony Lindgren
2010-06-17 20:27 ` Andrew Morton
2010-06-21  7:07   ` kishore kadiyala
2010-06-21 18:42 ` Adrian Hunter
2010-06-28 16:10   ` kishore kadiyala [this message]
2010-06-29  7:30     ` Adrian Hunter
2010-06-29  7:37       ` kishore kadiyala
2010-06-29  8:18         ` Adrian Hunter
2010-06-29  8:38           ` kishore kadiyala

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=AANLkTimVB7N0Oa9bDbYfSfcivy6afhvHW5Bdt-8dWVxD@mail.gmail.com \
    --to=kishorek.kadiyala@gmail.com \
    --cc=adrian.hunter@nokia.com \
    --cc=akpm@linux-foundation.org \
    --cc=kishore.kadiyala@ti.com \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=madhu.cr@ti.com \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).