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

Groups > linux.kernel > #1351066 > unrolled thread

[PATCH 1/2] Documentation: synopsys-dw-mshc: add binding for resets

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/2] Documentation: synopsys-dw-mshc: add binding for resets Guodong Xu <guodong.xu@linaro.org> - 2016-03-06 09:50 +0100
    [PATCH 2/2] mmc: dw_mmc: add resets support to dw_mmc Guodong Xu <guodong.xu@linaro.org> - 2016-03-06 09:50 +0100
      Re: [PATCH 2/2] mmc: dw_mmc: add resets support to dw_mmc Shawn Lin <shawn.lin@rock-chips.com> - 2016-03-06 15:20 +0100
    Re: [PATCH 1/2] Documentation: synopsys-dw-mshc: add binding for resets Jaehoon Chung <jh80.chung@samsung.com> - 2016-03-07 02:00 +0100
      Re: [PATCH 1/2] Documentation: synopsys-dw-mshc: add binding for  resets Shawn Lin <shawn.lin@rock-chips.com> - 2016-03-07 10:40 +0100
        Re: [PATCH 1/2] Documentation: synopsys-dw-mshc: add binding for resets Jaehoon Chung <jh80.chung@samsung.com> - 2016-03-07 11:00 +0100

#1351066 — [PATCH 1/2] Documentation: synopsys-dw-mshc: add binding for resets

Add resets property to synopsys-dw-mshc bindings. It is intended to
represent the hardware reset signal present internally in some host
controller IC designs.

See Documentation/devicetree/bindings/reset/reset.txt for details.

Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
index 8636f5a..4e00e85 100644
--- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
@@ -39,6 +39,10 @@ Required Properties:
 
 Optional properties:
 
+* resets: phandle + reset specifier pair, intended to represent hardware
+  reset signal present internally in some host controller IC designs.
+  See Documentation/devicetree/bindings/reset/reset.txt for details.
+
 * clocks: from common clock binding: handle to biu and ciu clocks for the
   bus interface unit clock and the card interface unit clock.
 
-- 
1.9.1

[toc] | [next] | [standalone]

#1351067 — [PATCH 2/2] mmc: dw_mmc: add resets support to dw_mmc

mmc registers may in abnormal state if mmc is used in bootloader,
eg. to support booting from eMMC. So we need reset mmc registers
when kernel boots up, instead of assuming mmc is in clean state.

With this patch, user can add a 'resets' property into dw_mmc dts
node. When driver parse_dt and probe, it calls reset API to
deassert the 'reset' of dw_mmc host controller. When probe error or
remove, it calls reset API to assert it.

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 | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 242f9a0..281ea9c 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2878,6 +2878,14 @@ 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)) {
+		if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER)
+			return ERR_PTR(-EPROBE_DEFER);
+		pdata->rstc = NULL;
+	}
+
 	/* find out number of slots supported */
 	of_property_read_u32(np, "num-slots", &pdata->num_slots);
 
@@ -2949,7 +2957,9 @@ int dw_mci_probe(struct dw_mci *host)
 
 	if (!host->pdata) {
 		host->pdata = dw_mci_parse_dt(host);
-		if (IS_ERR(host->pdata)) {
+		if (PTR_ERR(host->pdata) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		else if (IS_ERR(host->pdata)) {
 			dev_err(host->dev, "platform data not available\n");
 			return -EINVAL;
 		}
@@ -3012,6 +3022,9 @@ int dw_mci_probe(struct dw_mci *host)
 		}
 	}
 
+	if (host->pdata->rstc != NULL)
+		reset_control_deassert(host->pdata->rstc);
+
 	setup_timer(&host->cmd11_timer,
 		    dw_mci_cmd11_timer, (unsigned long)host);
 
@@ -3164,6 +3177,9 @@ err_dmaunmap:
 	if (host->use_dma && host->dma_ops->exit)
 		host->dma_ops->exit(host);
 
+	if (host->pdata->rstc != NULL)
+		reset_control_assert(host->pdata->rstc);
+
 err_clk_ciu:
 	if (!IS_ERR(host->ciu_clk))
 		clk_disable_unprepare(host->ciu_clk);
@@ -3196,11 +3212,15 @@ void dw_mci_remove(struct dw_mci *host)
 	if (host->use_dma && host->dma_ops->exit)
 		host->dma_ops->exit(host);
 
+	if (host->pdata->rstc != NULL)
+		reset_control_assert(host->pdata->rstc);
+
 	if (!IS_ERR(host->ciu_clk))
 		clk_disable_unprepare(host->ciu_clk);
 
 	if (!IS_ERR(host->biu_clk))
 		clk_disable_unprepare(host->biu_clk);
+
 }
 EXPORT_SYMBOL(dw_mci_remove);
 
-- 
1.9.1

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

#1351124 — Re: [PATCH 2/2] mmc: dw_mmc: add resets support to dw_mmc

On 2016/3/6 16:47, Guodong Xu wrote:
> mmc registers may in abnormal state if mmc is used in bootloader,
> eg. to support booting from eMMC. So we need reset mmc registers
> when kernel boots up, instead of assuming mmc is in clean state.
>
> With this patch, user can add a 'resets' property into dw_mmc dts
> node. When driver parse_dt and probe, it calls reset API to
> deassert the 'reset' of dw_mmc host controller. When probe error or
> remove, it calls reset API to assert it.
>
> 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>

Really should V2 and add the changelog.

> ---
>   drivers/mmc/host/dw_mmc.c | 22 +++++++++++++++++++++-
>   1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 242f9a0..281ea9c 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2878,6 +2878,14 @@ 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)) {
> +		if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER)
> +			return ERR_PTR(-EPROBE_DEFER);
> +		pdata->rstc = NULL;

maybe we can remove "pdata->rstc = NULL", and directly
use IS_ERR(..) for the following "if (host->pdata->rstc != NULL)"
statement


> +	}
> +
>   	/* find out number of slots supported */
>   	of_property_read_u32(np, "num-slots", &pdata->num_slots);
>
> @@ -2949,7 +2957,9 @@ int dw_mci_probe(struct dw_mci *host)
>
>   	if (!host->pdata) {
>   		host->pdata = dw_mci_parse_dt(host);
> -		if (IS_ERR(host->pdata)) {
> +		if (PTR_ERR(host->pdata) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;

please fix the coding style here.

> +		else if (IS_ERR(host->pdata)) {
>   			dev_err(host->dev, "platform data not available\n");
>   			return -EINVAL;
>   		}
> @@ -3012,6 +3022,9 @@ int dw_mci_probe(struct dw_mci *host)
>   		}
>   	}
>
> +	if (host->pdata->rstc != NULL)
> +		reset_control_deassert(host->pdata->rstc);
> +

sorry, I can't follow your intention here. Shouldn't it be something
like "assert mmc -> may need delay -> deassert mmc". As your current
code, nothing happend right?

>   	setup_timer(&host->cmd11_timer,
>   		    dw_mci_cmd11_timer, (unsigned long)host);
>
> @@ -3164,6 +3177,9 @@ err_dmaunmap:
>   	if (host->use_dma && host->dma_ops->exit)
>   		host->dma_ops->exit(host);
>
> +	if (host->pdata->rstc != NULL)
> +		reset_control_assert(host->pdata->rstc);
> +
>   err_clk_ciu:
>   	if (!IS_ERR(host->ciu_clk))
>   		clk_disable_unprepare(host->ciu_clk);
> @@ -3196,11 +3212,15 @@ void dw_mci_remove(struct dw_mci *host)
>   	if (host->use_dma && host->dma_ops->exit)
>   		host->dma_ops->exit(host);
>
> +	if (host->pdata->rstc != NULL)
> +		reset_control_assert(host->pdata->rstc);
> +
>   	if (!IS_ERR(host->ciu_clk))
>   		clk_disable_unprepare(host->ciu_clk);
>
>   	if (!IS_ERR(host->biu_clk))
>   		clk_disable_unprepare(host->biu_clk);
> +
>   }

unnecessary new line here.

>   EXPORT_SYMBOL(dw_mci_remove);
>
>


-- 
Best Regards
Shawn Lin

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

#1351233

Hi Goudong,

On 03/06/2016 05:47 PM, Guodong Xu wrote:
> Add resets property to synopsys-dw-mshc bindings. It is intended to
> represent the hardware reset signal present internally in some host
> controller IC designs.
> 
> See Documentation/devicetree/bindings/reset/reset.txt for details.
> 
> Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
>  Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> index 8636f5a..4e00e85 100644
> --- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> +++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> @@ -39,6 +39,10 @@ Required Properties:
>  
>  Optional properties:
>  
> +* resets: phandle + reset specifier pair, intended to represent hardware
> +  reset signal present internally in some host controller IC designs.
> +  See Documentation/devicetree/bindings/reset/reset.txt for details.

Is this reset property for common dwmmc IP controller?

Best Regards,
Jaehoon Chung

> +
>  * clocks: from common clock binding: handle to biu and ciu clocks for the
>    bus interface unit clock and the card interface unit clock.
>  
> 

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

#1351429 — Re: [PATCH 1/2] Documentation: synopsys-dw-mshc: add binding for resets

Hi Jaehoon,

On 2016/3/7 8:53, Jaehoon Chung wrote:
> Hi Goudong,
>
> On 03/06/2016 05:47 PM, Guodong Xu wrote:
>> Add resets property to synopsys-dw-mshc bindings. It is intended to
>> represent the hardware reset signal present internally in some host
>> controller IC designs.
>>
>> See Documentation/devicetree/bindings/reset/reset.txt for details.
>>
>> Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
>> Acked-by: Rob Herring <robh@kernel.org>
>> ---
>>   Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>> index 8636f5a..4e00e85 100644
>> --- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>> @@ -39,6 +39,10 @@ Required Properties:
>>
>>   Optional properties:
>>
>> +* resets: phandle + reset specifier pair, intended to represent hardware
>> +  reset signal present internally in some host controller IC designs.
>> +  See Documentation/devicetree/bindings/reset/reset.txt for details.
>
> Is this reset property for common dwmmc IP controller?

I think so. By discussion with my ASIC team, it's provided by synopsys.
 From dw_mmc databook version 270a, section 3.2.5, FBE Scenarios:

An FBE occurs due to an AHB error response on the AHB bus. This is a
system error, so the software driver should not perform any further
programming to the DWC_mobile_storage. The only recovery mechanism
from such scenarios is to do one of the following:
■ Issue a hard reset by asserting the reset_n signal
■ Do a program controller reset by writing to the CTRL[0] register

the reset_n signal can be used to reset all the internal logic block
with dwmmc and reset the register value to default stat.

Note: reset_n is a internal signal, which is diff from rst_n for mmc hw
reset. (refer to databook section 5.2 Signal Descriptions, table 5-1)

>
> Best Regards,
> Jaehoon Chung
>
>> +
>>   * clocks: from common clock binding: handle to biu and ciu clocks for the
>>     bus interface unit clock and the card interface unit clock.
>>
>>
>
>
>
>


-- 
Best Regards
Shawn Lin

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

#1351463

Hi Shawn,

On 03/07/2016 06:35 PM, Shawn Lin wrote:
> Hi Jaehoon,
> 
> On 2016/3/7 8:53, Jaehoon Chung wrote:
>> Hi Goudong,
>>
>> On 03/06/2016 05:47 PM, Guodong Xu wrote:
>>> Add resets property to synopsys-dw-mshc bindings. It is intended to
>>> represent the hardware reset signal present internally in some host
>>> controller IC designs.
>>>
>>> See Documentation/devicetree/bindings/reset/reset.txt for details.
>>>
>>> Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
>>> Acked-by: Rob Herring <robh@kernel.org>
>>> ---
>>>   Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>> index 8636f5a..4e00e85 100644
>>> --- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>> +++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>> @@ -39,6 +39,10 @@ Required Properties:
>>>
>>>   Optional properties:
>>>
>>> +* resets: phandle + reset specifier pair, intended to represent hardware
>>> +  reset signal present internally in some host controller IC designs.
>>> +  See Documentation/devicetree/bindings/reset/reset.txt for details.
>>
>> Is this reset property for common dwmmc IP controller?
> 
> I think so. By discussion with my ASIC team, it's provided by synopsys.
> From dw_mmc databook version 270a, section 3.2.5, FBE Scenarios:
> 
> An FBE occurs due to an AHB error response on the AHB bus. This is a
> system error, so the software driver should not perform any further
> programming to the DWC_mobile_storage. The only recovery mechanism
> from such scenarios is to do one of the following:
> ■ Issue a hard reset by asserting the reset_n signal
> ■ Do a program controller reset by writing to the CTRL[0] register
> 
> the reset_n signal can be used to reset all the internal logic block
> with dwmmc and reset the register value to default stat.
> 
> Note: reset_n is a internal signal, which is diff from rst_n for mmc hw
> reset. (refer to databook section 5.2 Signal Descriptions, table 5-1)

Thanks for this information. :)

> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>> +
>>>   * clocks: from common clock binding: handle to biu and ciu clocks for the
>>>     bus interface unit clock and the card interface unit clock.
>>>
>>>
>>
>>
>>
>>
> 
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web