| Message ID | 4DBA90BD.8070804@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jaehoon, On Fri, Apr 29 2011, Jaehoon Chung wrote: > This patch fixed regulator control in dw_mmc.c > If we didn't set CONFIG_REGULATOR, always entered error condition. > But that's not error..because we didn't use regulator framework. > > So when we only used CONFIG_REGULATOR, i think that need to get regulator. [..] > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index 87e1f57..62b900f 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -1441,12 +1441,14 @@ static int __init dw_mci_init_slot(struct dw_mci *host, unsigned int id) > } > #endif /* CONFIG_MMC_DW_IDMAC */ > > +#ifdef CONFIG_REGULATOR > host->vmmc = regulator_get(mmc_dev(mmc), "vmmc"); > if (IS_ERR(host->vmmc)) { > printk(KERN_INFO "%s: no vmmc regulator found\n", mmc_hostname(mmc)); > host->vmmc = NULL; > } else > regulator_enable(host->vmmc); > +#endif /* CONFIG_REGULATOR */ > > if (dw_mci_get_cd(mmc)) > set_bit(DW_MMC_CARD_PRESENT, &slot->flags); [..] As Lars pointed out, this doesn't make sense; without CONFIG_REGULATOR host->vmmc becomes NULL, which isn't IS_ERR. - Chris.
Hi Chris..
Thanks for comments...
I think that could confuse that message when didn't set CONFIG_REGULATOR.
And i wonder how do you think about regulator control in suspend (dw_mmc.c)?
@@ -1769,9 +1771,6 @@ static int dw_mci_suspend(struct platform_device *pdev, pm_message_t mesg)
int i, ret;
struct dw_mci *host = platform_get_drvdata(pdev);
- if (host->vmmc)
- regulator_enable(host->vmmc);
-
for (i = 0; i < host->num_slots; i++) {
struct dw_mci_slot *slot = host->slot[i];
if (!slot)
@@ -1798,6 +1797,9 @@ static int dw_mci_resume(struct platform_device *pdev)
int i, ret;
struct dw_mci *host = platform_get_drvdata(pdev);
+ if (host->vmmc)
+ regulator_enable(host->vmmc);
+
if (host->dma_ops->init)
host->dma_ops->init(host);
Regards,
Jaehoon Chung
Chris Ball wrote:
> Hi Jaehoon,
>
> On Fri, Apr 29 2011, Jaehoon Chung wrote:
>> This patch fixed regulator control in dw_mmc.c
>> If we didn't set CONFIG_REGULATOR, always entered error condition.
>> But that's not error..because we didn't use regulator framework.
>>
>> So when we only used CONFIG_REGULATOR, i think that need to get regulator.
> [..]
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 87e1f57..62b900f 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1441,12 +1441,14 @@ static int __init dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>> }
>> #endif /* CONFIG_MMC_DW_IDMAC */
>>
>> +#ifdef CONFIG_REGULATOR
>> host->vmmc = regulator_get(mmc_dev(mmc), "vmmc");
>> if (IS_ERR(host->vmmc)) {
>> printk(KERN_INFO "%s: no vmmc regulator found\n", mmc_hostname(mmc));
>> host->vmmc = NULL;
>> } else
>> regulator_enable(host->vmmc);
>> +#endif /* CONFIG_REGULATOR */
>>
>> if (dw_mci_get_cd(mmc))
>> set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
> [..]
>
> As Lars pointed out, this doesn't make sense; without CONFIG_REGULATOR
> host->vmmc becomes NULL, which isn't IS_ERR.
>
> - Chris.
--
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
Hi Jaehoon, On Mon, May 02 2011, Jaehoon Chung wrote: > Hi Chris.. > > Thanks for comments... > I think that could confuse that message when didn't set CONFIG_REGULATOR. The message *does not appear* when CONFIG_REGULATOR is unset, because NULL -- which the regulator subsystem returns via the regulator_get() stub -- is not an IS_ERR(). Do you agree? > And i wonder how do you think about regulator control in suspend (dw_mmc.c)? Yes, that patch looks correct, although I can't test it myself. Please send it separately. - Chris.
Chris Ball wrote: > Hi Jaehoon, > > On Mon, May 02 2011, Jaehoon Chung wrote: >> Hi Chris.. >> >> Thanks for comments... >> I think that could confuse that message when didn't set CONFIG_REGULATOR. > > The message *does not appear* when CONFIG_REGULATOR is unset, because > NULL -- which the regulator subsystem returns via the regulator_get() > stub -- is not an IS_ERR(). Do you agree? Thanks, I agreed that. I'm interesting mailing's discussion about "[PATCH] pxamci: remove an ifdef about CONFIG_REGULATOR " (in this case, i think similar case) > >> And i wonder how do you think about regulator control in suspend (dw_mmc.c)? > > Yes, that patch looks correct, although I can't test it myself. Please > send it separately. Ok. i will send that separately patch. Regards, Jaehoon Chung -- 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
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 87e1f57..62b900f 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -1441,12 +1441,14 @@ static int __init dw_mci_init_slot(struct dw_mci *host, unsigned int id) } #endif /* CONFIG_MMC_DW_IDMAC */ +#ifdef CONFIG_REGULATOR host->vmmc = regulator_get(mmc_dev(mmc), "vmmc"); if (IS_ERR(host->vmmc)) { printk(KERN_INFO "%s: no vmmc regulator found\n", mmc_hostname(mmc)); host->vmmc = NULL; } else regulator_enable(host->vmmc); +#endif /* CONFIG_REGULATOR */ if (dw_mci_get_cd(mmc)) set_bit(DW_MMC_CARD_PRESENT, &slot->flags); @@ -1769,9 +1771,6 @@ static int dw_mci_suspend(struct platform_device *pdev, pm_message_t mesg) int i, ret; struct dw_mci *host = platform_get_drvdata(pdev); - if (host->vmmc) - regulator_enable(host->vmmc); - for (i = 0; i < host->num_slots; i++) { struct dw_mci_slot *slot = host->slot[i]; if (!slot) @@ -1798,6 +1797,9 @@ static int dw_mci_resume(struct platform_device *pdev) int i, ret; struct dw_mci *host = platform_get_drvdata(pdev); + if (host->vmmc) + regulator_enable(host->vmmc); + if (host->dma_ops->init) host->dma_ops->init(host);