From: Seungwon Jeon <tgih.jun@samsung.com>
To: 'Thomas Abraham' <thomas.abraham@linaro.org>
Cc: linux-mmc@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
will.newton@imgtec.com, cjb@laptop.org,
grant.likely@secretlab.ca, rob.herring@calxeda.com,
linux-samsung-soc@vger.kernel.org, kgene.kim@samsung.com,
girish.shivananjappa@linaro.org, jh80.chung@samsung.com,
patches@linaro.org
Subject: RE: [PATCH v6 6/9] mmc: dw_mmc: add device tree support
Date: Fri, 21 Sep 2012 11:27:07 +0900 [thread overview]
Message-ID: <003501cd97a0$98a7fcc0$c9f7f640$%jun@samsung.com> (raw)
In-Reply-To: <CAJuYYwQOCLPsVjGYQFTnTaqDeWnaDONGMHPEF5TB3htYc5SsJA@mail.gmail.com>
On Thursday, September 20, 2012, Thomas Abraham <thomas.abraham@linaro.org> wrote:
> On 19 September 2012 19:09, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> > On Tuesday, September 18, 2012, Thomas Abraham <thomas.abraham@linaro.org> wrote:
> >> +#ifdef CONFIG_OF
> >> +/* given a slot id, find out the device node representing that slot */
> >> +static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
> >> +{
> >> + struct device_node *np;
> >> + const __be32 *addr;
> >> + int len;
> >> +
> >> + if (!dev || !dev->of_node)
> >> + return NULL;
> >> +
> >> + for_each_child_of_node(dev->of_node, np) {
> >> + addr = of_get_property(np, "reg", &len);
> >> + if (!addr || (len < sizeof(int)))
> >> + continue;
> >> + if (be32_to_cpup(addr) == slot)
> >> + return np;
> >> + }
> >> + return NULL;
> >> +}
> >> +
> >> +/* find out bus-width for a given slot */
> >> +static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
> >> +{
> >> + struct device_node *np = dw_mci_of_find_slot_node(dev, slot);
> >> + u32 bus_wd = 1;
> >> +
> >> + if (!np)
> >> + return 1;
> >> +
> >> + if (of_property_read_u32(np, "bus-width", &bus_wd))
> >> + dev_err(dev, "bus-width property not found, assuming width"
> >> + " as 1\n");
> >> + return bus_wd;
> >> +}
> >> +#else /* CONFIG_OF */
> >> +static u32 dw_mci_of_get_bus_wd(struct device *dev, u8 slot)
> >> +{
> >> + return 1;
> >> +}
> >> +static struct device_node *dw_mci_of_find_slot_node(struct device *dev, u8 slot)
> >> +{
> >> + return NULL;
> >> +}
> >> +#endif /* CONFIG_OF */
> >> +
> >> static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
> >> {
> >> struct mmc_host *mmc;
> >> struct dw_mci_slot *slot;
> >> + u8 bus_width;
> >>
> >> mmc = mmc_alloc_host(sizeof(struct dw_mci_slot), host->dev);
> >> if (!mmc)
> >> @@ -1782,6 +1830,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
> >> slot->id = id;
> >> slot->mmc = mmc;
> >> slot->host = host;
> >> + host->slot[id] = slot;
> >>
> >> mmc->ops = &dw_mci_ops;
> >> mmc->f_min = DIV_ROUND_UP(host->bus_hz, 510);
> >> @@ -1806,8 +1855,18 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
> >> mmc->caps2 = host->pdata->caps2;
> >>
> >> if (host->pdata->get_bus_wd)
> >> - if (host->pdata->get_bus_wd(slot->id) >= 4)
> >> - mmc->caps |= MMC_CAP_4_BIT_DATA;
> >> + bus_width = host->pdata->get_bus_wd(slot->id);
> >> + else if (host->dev->of_node)
> >> + bus_width = dw_mci_of_get_bus_wd(host->dev, slot->id);
> >> + else
> >> + bus_width = 1;
> >> +
> >> + switch (bus_width) {
> >> + case 8:
> >> + mmc->caps |= MMC_CAP_8_BIT_DATA;
> >> + case 4:
> >> + mmc->caps |= MMC_CAP_4_BIT_DATA;
> >> + }
> >>
> >> if (host->pdata->quirks & DW_MCI_QUIRK_HIGHSPEED)
> >> mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
> >> @@ -1852,7 +1911,6 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
> >> else
> >> clear_bit(DW_MMC_CARD_PRESENT, &slot->flags);
> >>
> >> - host->slot[id] = slot;
> >> mmc_add_host(mmc);
> >>
> >> #if defined(CONFIG_DEBUG_FS)
> >> @@ -1944,16 +2002,74 @@ static bool mci_wait_reset(struct device *dev, struct dw_mci *host)
> >> return false;
> >> }
> >>
> > The following CONFIG_OF block can be put together in upper CONFIG_OF .
>
> The placement of device tree related code is considering the logical
> structure and flow of the code. So maybe we can just retain the below
> code here for now.
>
> >
> >> +#ifdef CONFIG_OF
> >> +static struct dw_mci_of_quirks {
> >> + char *quirk;
> >> + int id;
> >> +} of_quirks[] = {
> >> + {
> >> + .quirk = "supports-highspeed",
> >> + .id = DW_MCI_QUIRK_HIGHSPEED,
> >> + }, {
> >> + .quirk = "broken-cd",
> >> + .id = DW_MCI_QUIRK_BROKEN_CARD_DETECTION,
> >> + },
> >> +};
> >> +
> >> +static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
> >> +{
> >> + struct dw_mci_board *pdata;
> >> + struct device *dev = host->dev;
> >> + struct device_node *np = dev->of_node;
> >> + int idx;
> >> +
> >> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> >> + if (!pdata) {
> >> + dev_err(dev, "could not allocate memory for pdata\n");
> >> + return ERR_PTR(-ENOMEM);
> >> + }
> >> +
> >> + /* find out number of slots supported */
> >> + if (of_property_read_u32(dev->of_node, "num-slots",
> >> + &pdata->num_slots)) {
> >> + dev_info(dev, "num-slots property not found, "
> >> + "assuming 1 slot is available\n");
> >> + pdata->num_slots = 1;
> >> + }
> >> +
> >> + /* get quirks */
> >> + for (idx = 0; idx < ARRAY_SIZE(of_quirks); idx++)
> >> + if (of_get_property(np, of_quirks[idx].quirk, NULL))
> >> + pdata->quirks |= of_quirks[idx].id;
> >> +
> >> + if (of_property_read_u32(np, "fifo-depth", &pdata->fifo_depth))
> >> + dev_info(dev, "fifo-depth property not found, using "
> >> + "value of FIFOTH register as default\n");
> >> +
> >> + of_property_read_u32(np, "card-detect-delay", &pdata->detect_delay_ms);
> >> +
> >> + return pdata;
> >> +}
> >> +
> >> +#else /* CONFIG_OF */
> >> +static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
> >> +{
> >> + return ERR_PTR(-EINVAL);
> >> +}
> >> +#endif /* CONFIG_OF */
> >> +
> >> int dw_mci_probe(struct dw_mci *host)
> >> {
> >> int width, i, ret = 0;
> >> u32 fifo_size;
> >> int init_slots = 0;
> >>
> >> - if (!host->pdata || !host->pdata->init) {
> > "!host->pdata->init" is removed.
> > Please check it.
>
> I have checked again in mmc-next branch and "init" is still there.
I mean that there is no condition whether "host->pdata->init" is present or not, unlike origin code.
In this patch this condition is removed. We don't need it anymore?
>
> Thanks,
> Thomas.
>
>
> >
> > Thanks,
> > Seungwon Jeon
> >
> >> - dev_err(host->dev,
> >> - "Platform data must supply init function\n");
> >> - return -ENODEV;
> >> + if (!host->pdata) {
> >> + host->pdata = dw_mci_parse_dt(host);
> >> + if (IS_ERR(host->pdata)) {
> >> + dev_err(host->dev, "platform data not available\n");
> >> + return -EINVAL;
> >> + }
> >> }
> >>
> >> if (!host->pdata->select_slot && host->pdata->num_slots > 1) {
> >> --
> >> 1.6.6.rc2
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
> [...]
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2012-09-21 2:27 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-17 18:16 [PATCH v6 0/9] mmc: dw_mmc: add support for device tree based instantiation Thomas Abraham
2012-09-17 18:16 ` [PATCH v6 1/9] mmc: dw_mmc: convert copy of struct device in struct dw_mci to a reference Thomas Abraham
2012-09-17 18:16 ` [PATCH v6 2/9] mmc: dw_mmc: Use devm_* functions in dw_mmc platform driver Thomas Abraham
2012-09-17 18:16 ` [PATCH v6 3/9] mmc: dw_mmc: allow probe to succeed even if one slot is initialized Thomas Abraham
2012-09-17 18:16 ` [PATCH v6 4/9] mmc: dw_mmc: lookup for optional biu and ciu clocks Thomas Abraham
2012-09-17 18:16 ` [PATCH v6 5/9] mmc: dw_mmc: add quirk to indicate missing write protect line Thomas Abraham
2012-09-17 18:16 ` [PATCH v6 6/9] mmc: dw_mmc: add device tree support Thomas Abraham
2012-09-19 13:39 ` Seungwon Jeon
[not found] ` <001301cd966c$3f99f0a0$becdd1e0$%jun-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2012-09-20 4:36 ` Thomas Abraham
2012-09-21 2:27 ` Seungwon Jeon [this message]
2012-09-21 2:43 ` Jaehoon Chung
2012-09-17 18:16 ` [PATCH v6 7/9] mmc: dw_mmc: prepare functions in dw_mmc-pltfm for reuse Thomas Abraham
2012-09-17 18:16 ` [PATCH v6 8/9] mmc: dw_mmc: add support for implementation specific callbacks Thomas Abraham
2012-09-19 13:40 ` Seungwon Jeon
2012-09-20 4:38 ` Thomas Abraham
2012-09-24 3:03 ` Chris Ball
2012-10-04 12:41 ` Seungwon Jeon
2012-10-16 10:55 ` Thomas Abraham
2012-09-17 18:16 ` [PATCH v6 9/9] mmc: dw_mmc: add support for exynos specific implementation of dw-mshc Thomas Abraham
2012-09-19 13:42 ` Seungwon Jeon
2012-09-20 4:44 ` Thomas Abraham
2012-09-21 2:27 ` Seungwon Jeon
2012-09-18 1:52 ` [PATCH v6 0/9] mmc: dw_mmc: add support for device tree based instantiation Thomas Abraham
2012-09-19 6:27 ` Thomas Abraham
2012-09-19 7:05 ` Chris Ball
2012-09-19 7:31 ` Thomas Abraham
2012-09-19 13:39 ` Seungwon Jeon
2012-09-20 4:45 ` Thomas Abraham
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='003501cd97a0$98a7fcc0$c9f7f640$%jun@samsung.com' \
--to=tgih.jun@samsung.com \
--cc=cjb@laptop.org \
--cc=devicetree-discuss@lists.ozlabs.org \
--cc=girish.shivananjappa@linaro.org \
--cc=grant.likely@secretlab.ca \
--cc=jh80.chung@samsung.com \
--cc=kgene.kim@samsung.com \
--cc=linux-mmc@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=patches@linaro.org \
--cc=rob.herring@calxeda.com \
--cc=thomas.abraham@linaro.org \
--cc=will.newton@imgtec.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).