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 <[email protected]>
---
Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
index 8636f5a..9b4896c 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.
@@ -48,7 +52,7 @@ Optional properties:
clock-frequency. It is an error to omit both the ciu clock and the
clock-frequency.
-* clock-frequency: should be the frequency (in Hz) of the ciu clock. If this
+* clock-frequency: should be tke frequency (in Hz) of the ciu clock. If this
is specified and the ciu clock is specified then we'll try to set the ciu
clock to this at probe time.
--
1.9.1
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 <[email protected]>
Signed-off-by: Xinwei Kong <[email protected]>
Signed-off-by: Zhangfei Gao <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Xinwei Kong <[email protected]>
> Signed-off-by: Zhangfei Gao <[email protected]>
> ---
> 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
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
?? 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
On Thu, Mar 03, 2016 at 09:33:37AM +0800, 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 <[email protected]>
> ---
> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> index 8636f5a..9b4896c 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.
>
> @@ -48,7 +52,7 @@ Optional properties:
> clock-frequency. It is an error to omit both the ciu clock and the
> clock-frequency.
>
> -* clock-frequency: should be the frequency (in Hz) of the ciu clock. If this
> +* clock-frequency: should be tke frequency (in Hz) of the ciu clock. If this
You've added a typo here.
Otherwise,
Acked-by: Rob Herring <[email protected]>