Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1348672 > unrolled thread

[PATCH 2/2] mmc: dw_mmc: add resets support to dw_mci_parse_dt()

Started byGuodong Xu <guodong.xu@linaro.org>
First post2016-03-03 02:40 +0100
Last post2016-03-04 04:50 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/2] mmc: dw_mmc: add resets support to dw_mci_parse_dt() Guodong Xu <guodong.xu@linaro.org> - 2016-03-03 02:40 +0100
    Re: [PATCH 2/2] mmc: dw_mmc: add resets support to dw_mci_parse_dt() Shawn Lin <shawn.lin@rock-chips.com> - 2016-03-03 03:10 +0100
      Re: [PATCH 2/2] mmc: dw_mmc: add resets support to dw_mci_parse_dt() zhangfei <zhangfei.gao@linaro.org> - 2016-03-04 04:30 +0100
        Re: [PATCH 2/2] mmc: dw_mmc: add resets support to dw_mci_parse_dt() Shawn Lin <shawn.lin@rock-chips.com> - 2016-03-04 04:50 +0100

#1348672 — [PATCH 2/2] mmc: dw_mmc: add resets support to dw_mci_parse_dt()

FromGuodong Xu <guodong.xu@linaro.org>
Date2016-03-03 02:40 +0100
Subject[PATCH 2/2] mmc: dw_mmc: add resets support to dw_mci_parse_dt()
Message-ID<r8qpc-89F-23@gated-at.bofh.it>
With this, user can add a 'resets' property into dw_mmc dts
node, and when driver probe and parse_dt, it will call
reset APIs to reset dw_mmc host controller.

Please also refer to Documentation/devicetree/bindings/reset/reset.txt

Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
Signed-off-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
 drivers/mmc/host/dw_mmc.c  | 7 +++++++
 include/linux/mmc/dw_mmc.h | 6 ++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 242f9a0..d3a7376 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2878,6 +2878,13 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
 	if (!pdata)
 		return ERR_PTR(-ENOMEM);
 
+	/* find reset controller when exist */
+	pdata->rstc = devm_reset_control_get_optional(dev, NULL);
+	if (IS_ERR(pdata->rstc))
+		pdata->rstc = NULL;
+	else
+		reset_control_deassert(pdata->rstc);
+
 	/* find out number of slots supported */
 	of_property_read_u32(np, "num-slots", &pdata->num_slots);
 
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 7b41c6d..b95cd84 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -14,9 +14,10 @@
 #ifndef LINUX_MMC_DW_MMC_H
 #define LINUX_MMC_DW_MMC_H
 
-#include <linux/scatterlist.h>
-#include <linux/mmc/core.h>
 #include <linux/dmaengine.h>
+#include <linux/mmc/core.h>
+#include <linux/reset.h>
+#include <linux/scatterlist.h>
 
 #define MAX_MCI_SLOTS	2
 
@@ -260,6 +261,7 @@ struct dw_mci_board {
 	/* delay in mS before detecting cards after interrupt */
 	u32 detect_delay_ms;
 
+	struct reset_control *rstc;
 	struct dw_mci_dma_ops *dma_ops;
 	struct dma_pdata *data;
 };
-- 
1.9.1

[toc] | [next] | [standalone]

#1348692

FromShawn Lin <shawn.lin@rock-chips.com>
Date2016-03-03 03:10 +0100
Message-ID<r8qSe-7S-11@gated-at.bofh.it>
In reply to#1348672
Hi Guodong,

On 2016/3/3 9:33, Guodong Xu wrote:
> With this, user can add a 'resets' property into dw_mmc dts
> node, and when driver probe and parse_dt, it will call
> reset APIs to reset dw_mmc host controller.
>
> Please also refer to Documentation/devicetree/bindings/reset/reset.txt

I have no hard objection for this patch, but I'd rather not add it
unless we actually need it. Could you elaborate more about any futher
actions you will take if it's applied(i.e: deal with some unrecoverable
broken case)?  If we just reset the controller while probing, actually I
can't find any problems without it based on tons of my reboot test.


>
> Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
> Signed-off-by: Xinwei Kong <kong.kongxinwei@hisilicon.com>
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> ---
>   drivers/mmc/host/dw_mmc.c  | 7 +++++++
>   include/linux/mmc/dw_mmc.h | 6 ++++--
>   2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 242f9a0..d3a7376 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2878,6 +2878,13 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host)
>   	if (!pdata)
>   		return ERR_PTR(-ENOMEM);
>
> +	/* find reset controller when exist */
> +	pdata->rstc = devm_reset_control_get_optional(dev, NULL);
> +	if (IS_ERR(pdata->rstc))
> +		pdata->rstc = NULL;
> +	else
> +		reset_control_deassert(pdata->rstc);
> +

if the PTR_ERR(pdata->rstc) is -EPROBE_DEFER, should we defer probing
the driver?

>   	/* find out number of slots supported */
>   	of_property_read_u32(np, "num-slots", &pdata->num_slots);
>
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 7b41c6d..b95cd84 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -14,9 +14,10 @@
>   #ifndef LINUX_MMC_DW_MMC_H
>   #define LINUX_MMC_DW_MMC_H
>
> -#include <linux/scatterlist.h>
> -#include <linux/mmc/core.h>
>   #include <linux/dmaengine.h>
> +#include <linux/mmc/core.h>
> +#include <linux/reset.h>
> +#include <linux/scatterlist.h>
>
>   #define MAX_MCI_SLOTS	2
>
> @@ -260,6 +261,7 @@ struct dw_mci_board {
>   	/* delay in mS before detecting cards after interrupt */
>   	u32 detect_delay_ms;
>
> +	struct reset_control *rstc;
>   	struct dw_mci_dma_ops *dma_ops;
>   	struct dma_pdata *data;
>   };
>


-- 
Best Regards
Shawn Lin

[toc] | [prev] | [next] | [standalone]

#1349810

Fromzhangfei <zhangfei.gao@linaro.org>
Date2016-03-04 04:30 +0100
Message-ID<r8OBc-wF-5@gated-at.bofh.it>
In reply to#1348692

On 03/03/2016 10:00 AM, Shawn Lin wrote:
> Hi Guodong,
>
> On 2016/3/3 9:33, Guodong Xu wrote:
>> With this, user can add a 'resets' property into dw_mmc dts
>> node, and when driver probe and parse_dt, it will call
>> reset APIs to reset dw_mmc host controller.
>>
>> Please also refer to Documentation/devicetree/bindings/reset/reset.txt
>
> I have no hard objection for this patch, but I'd rather not add it
> unless we actually need it. Could you elaborate more about any futher
> actions you will take if it's applied(i.e: deal with some unrecoverable
> broken case)?  If we just reset the controller while probing, actually I
> can't find any problems without it based on tons of my reboot test.

mmc register maybe abnormal state, if mmc is used in uefi, like boot 
from emmc.
So we need reset mmc register when kernel boot up, instead of assuming 
mmc is in clean state.
Next step is adding reset node in dts for drivers/mmc/host/dw_mmc-k3.c 
as reset driver is already there.

>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 242f9a0..d3a7376 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -2878,6 +2878,13 @@ static struct dw_mci_board
>> *dw_mci_parse_dt(struct dw_mci *host)
>>       if (!pdata)
>>           return ERR_PTR(-ENOMEM);
>>
>> +    /* find reset controller when exist */
>> +    pdata->rstc = devm_reset_control_get_optional(dev, NULL);
>> +    if (IS_ERR(pdata->rstc))
>> +        pdata->rstc = NULL;
>> +    else
>> +        reset_control_deassert(pdata->rstc);
>> +
>
> if the PTR_ERR(pdata->rstc) is -EPROBE_DEFER, should we defer probing
> the driver?
Yes, good catch.

Thanks

[toc] | [prev] | [next] | [standalone]

#1349823

FromShawn Lin <shawn.lin@rock-chips.com>
Date2016-03-04 04:50 +0100
Message-ID<r8OUx-F5-1@gated-at.bofh.it>
In reply to#1349810
在 2016/3/4 11:27, zhangfei 写道:
>
>
> On 03/03/2016 10:00 AM, Shawn Lin wrote:
>> Hi Guodong,
>>
>> On 2016/3/3 9:33, Guodong Xu wrote:
>>> With this, user can add a 'resets' property into dw_mmc dts
>>> node, and when driver probe and parse_dt, it will call
>>> reset APIs to reset dw_mmc host controller.
>>>
>>> Please also refer to Documentation/devicetree/bindings/reset/reset.txt
>>
>> I have no hard objection for this patch, but I'd rather not add it
>> unless we actually need it. Could you elaborate more about any futher
>> actions you will take if it's applied(i.e: deal with some unrecoverable
>> broken case)?  If we just reset the controller while probing, actually I
>> can't find any problems without it based on tons of my reboot test.
>
> mmc register maybe abnormal state, if mmc is used in uefi, like boot
> from emmc.
> So we need reset mmc register when kernel boot up, instead of assuming
> mmc is in clean state.
> Next step is adding reset node in dts for drivers/mmc/host/dw_mmc-k3.c
> as reset driver is already there.

Fair enough. Thanks for explaining.

>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index 242f9a0..d3a7376 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -2878,6 +2878,13 @@ static struct dw_mci_board
>>> *dw_mci_parse_dt(struct dw_mci *host)
>>>       if (!pdata)
>>>           return ERR_PTR(-ENOMEM);
>>>
>>> +    /* find reset controller when exist */
>>> +    pdata->rstc = devm_reset_control_get_optional(dev, NULL);
>>> +    if (IS_ERR(pdata->rstc))
>>> +        pdata->rstc = NULL;
>>> +    else
>>> +        reset_control_deassert(pdata->rstc);
>>> +
>>
>> if the PTR_ERR(pdata->rstc) is -EPROBE_DEFER, should we defer probing
>> the driver?
> Yes, good catch.
>
> Thanks
>
>
>


-- 
Best Regards
Shawn Lin

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web