mmc controller registers may in abnormal state if mmc is used in
bootloader, eg. to load kernel from eMMC. Some controllers cann't
clear their registers when clk is set. They use dedicated reset
logics to do this.
In this patch, a 'resets' property is added into dw_mmc dts
node. When driver does 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.
Chip vendor's actual reset logics is implemented in reset driver, not
in dw_mmc code.
Please also refer to Documentation/devicetree/bindings/reset/reset.txt
Guodong Xu (2):
Documentation: synopsys-dw-mshc: add binding for resets
mmc: dw_mmc: add resets support to dw_mmc
.../devicetree/bindings/mmc/synopsys-dw-mshc.txt | 4 ++++
drivers/mmc/host/dw_mmc.c | 20 +++++++++++++++++++-
include/linux/mmc/dw_mmc.h | 6 ++++--
3 files changed, 27 insertions(+), 3 deletions(-)
--
1.9.1
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]>
Acked-by: Rob Herring <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Xinwei Kong <[email protected]>
Signed-off-by: Zhangfei Gao <[email protected]>
---
drivers/mmc/host/dw_mmc.c | 20 +++++++++++++++++++-
include/linux/mmc/dw_mmc.h | 6 ++++--
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 242f9a0..d0a4535 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)) {
+ if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER)
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
/* find out number of slots supported */
of_property_read_u32(np, "num-slots", &pdata->num_slots);
@@ -2949,7 +2956,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 +3021,9 @@ int dw_mci_probe(struct dw_mci *host)
}
}
+ if (!IS_ERR(host->pdata->rstc))
+ reset_control_deassert(host->pdata->rstc);
+
setup_timer(&host->cmd11_timer,
dw_mci_cmd11_timer, (unsigned long)host);
@@ -3164,6 +3176,9 @@ err_dmaunmap:
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
+ if (!IS_ERR(host->pdata->rstc))
+ reset_control_assert(host->pdata->rstc);
+
err_clk_ciu:
if (!IS_ERR(host->ciu_clk))
clk_disable_unprepare(host->ciu_clk);
@@ -3196,6 +3211,9 @@ void dw_mci_remove(struct dw_mci *host)
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
+ if (!IS_ERR(host->pdata->rstc))
+ reset_control_assert(host->pdata->rstc);
+
if (!IS_ERR(host->ciu_clk))
clk_disable_unprepare(host->ciu_clk);
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
modified Rob's mail address.
On 03/30/2016 04:24 PM, 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.
Do you mean mmc(card side) register or dwmmc host controller's register on host side?
According to dwmmc controller TMR, there are two reset signals. One is reset_n, other is rst_n.
It seems this patch is relevant to reset_n(For host). (rst_n is hardware reset for card.)
So could you clarify better? Then it's helpful to me for understanding..
It seems that it means "mmc" is card, mmc registers is host controller register, right?
>
> 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 <[email protected]>
> Signed-off-by: Xinwei Kong <[email protected]>
> Signed-off-by: Zhangfei Gao <[email protected]>
> ---
> drivers/mmc/host/dw_mmc.c | 20 +++++++++++++++++++-
> include/linux/mmc/dw_mmc.h | 6 ++++--
> 2 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 242f9a0..d0a4535 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)) {
> + if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER)
> + return ERR_PTR(-EPROBE_DEFER);
> + }
> +
> /* find out number of slots supported */
> of_property_read_u32(np, "num-slots", &pdata->num_slots);
>
> @@ -2949,7 +2956,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 +3021,9 @@ int dw_mci_probe(struct dw_mci *host)
> }
> }
>
> + if (!IS_ERR(host->pdata->rstc))
> + reset_control_deassert(host->pdata->rstc);
> +
> setup_timer(&host->cmd11_timer,
> dw_mci_cmd11_timer, (unsigned long)host);
>
> @@ -3164,6 +3176,9 @@ err_dmaunmap:
> if (host->use_dma && host->dma_ops->exit)
> host->dma_ops->exit(host);
>
> + if (!IS_ERR(host->pdata->rstc))
> + reset_control_assert(host->pdata->rstc);
location is correct?
> +
> err_clk_ciu:
> if (!IS_ERR(host->ciu_clk))
> clk_disable_unprepare(host->ciu_clk);
> @@ -3196,6 +3211,9 @@ void dw_mci_remove(struct dw_mci *host)
> if (host->use_dma && host->dma_ops->exit)
> host->dma_ops->exit(host);
>
> + if (!IS_ERR(host->pdata->rstc))
> + reset_control_assert(host->pdata->rstc);
> +
> if (!IS_ERR(host->ciu_clk))
> clk_disable_unprepare(host->ciu_clk);
>
> 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>
Why did you touch other things?
>
> #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;
> };
>
+ Heiko
On 2016/3/30 15:24, Guodong Xu wrote:
> mmc controller registers may in abnormal state if mmc is used in
> bootloader, eg. to load kernel from eMMC. Some controllers cann't
> clear their registers when clk is set. They use dedicated reset
> logics to do this.
>
> In this patch, a 'resets' property is added into dw_mmc dts
> node. When driver does 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.
>
> Chip vendor's actual reset logics is implemented in reset driver, not
> in dw_mmc code.
>
> Please also refer to Documentation/devicetree/bindings/reset/reset.txt
>
> Guodong Xu (2):
> Documentation: synopsys-dw-mshc: add binding for resets
> mmc: dw_mmc: add resets support to dw_mmc
>
> .../devicetree/bindings/mmc/synopsys-dw-mshc.txt | 4 ++++
> drivers/mmc/host/dw_mmc.c | 20 +++++++++++++++++++-
> include/linux/mmc/dw_mmc.h | 6 ++++--
> 3 files changed, 27 insertions(+), 3 deletions(-)
>
--
Best Regards
Shawn Lin
Am Mittwoch, 30. M?rz 2016, 20:40:31 schrieb Jaehoon Chung:
> modified Rob's mail address.
>
> On 03/30/2016 04:24 PM, 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.
>
> Do you mean mmc(card side) register or dwmmc host controller's register on
> host side?
>
> According to dwmmc controller TMR, there are two reset signals. One is
> reset_n, other is rst_n. It seems this patch is relevant to reset_n(For
> host). (rst_n is hardware reset for card.)
>
> So could you clarify better? Then it's helpful to me for understanding..
I think that actually means a reset of controller IP block logic, outside
the control of the dw_mmc block itself.
On Rockchip SoCs this gets triggered from the CRU (clock and reset unit), so
I guess if I'm reading the manual correctly, should be the reset_n signal of
the ip block.
rst_n on the other hand gets triggered through a dw_mmc register setting and
is already handled by the dw_mmc driver.
Heiko
Am Mittwoch, 30. M?rz 2016, 15:24:56 schrieb Guodong Xu:
[...]
> @@ -2949,7 +2956,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)) {
how is this related to adding the reset handling?
Making the driver handle probe deferral better should be a separate patch.
> dev_err(host->dev, "platform data not available\n");
> return -EINVAL;
> }
> @@ -3012,6 +3021,9 @@ int dw_mci_probe(struct dw_mci *host)
> }
> }
>
> + if (!IS_ERR(host->pdata->rstc))
> + reset_control_deassert(host->pdata->rstc);
> +
Wouldn't reset_control_reset be better? The way it is now it would expect
the reset to be asserted somewhere else before dw_mmc probes?
> setup_timer(&host->cmd11_timer,
> dw_mci_cmd11_timer, (unsigned long)host);
>
[...]
> 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>
unrelated changed regarding the reordering of includes.
Heiko
Am Samstag, 2. April 2016, 21:39:11 schrieb Guodong Xu:
> On 2 April 2016 at 02:42, Heiko Stuebner <[email protected]> wrote:
> > Am Mittwoch, 30. M?rz 2016, 15:24:56 schrieb Guodong Xu:
> >
> > [...]
> >
> > > @@ -2949,7 +2956,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)) {
> >
> > how is this related to adding the reset handling?
>
> I added this into dw_mci_parse_dt(host), and that's the first time it may
> return -EPROBE_DEFER
>
> /* find reset controller when exist */
> pdata->rstc = devm_reset_control_get_optional(dev, NULL);
>
> So, I added processing to this error in this patch.
ah, you're right of course
> > Making the driver handle probe deferral better should be a separate
> > patch.>
> > > dev_err(host->dev, "platform data not
> >
> > available\n");
> >
> > > return -EINVAL;
> > >
> > > }
> > >
> > > @@ -3012,6 +3021,9 @@ int dw_mci_probe(struct dw_mci *host)
> > >
> > > }
> > >
> > > }
> > >
> > > + if (!IS_ERR(host->pdata->rstc))
> > > + reset_control_deassert(host->pdata->rstc);
> > > +
> >
> > Wouldn't reset_control_reset be better? The way it is now it would
> > expect
> > the reset to be asserted somewhere else before dw_mmc probes?
>
> It relates to how the SoC's reset logic is like. One bit set can clear all
> dw_mmc host controller registers. It doesn't need do assert then
> deassert.
>
> That's what I saw in hi6220 (it integrates three dw_mmc host controller),
> drivers/reset/hisilicon/hi6220_reset.c
> , which I wrote this patch for.
I just realized again that reset_control_reset is a completely separate
operation (not related to assert / deassert).
What I was originally getting at is that I don't see any assert-counterpart.
So if the reset-control is already deasserted, nothing will happen on some
designs.
For example on Rockchip SoCs the reset controller needs the signal to be
high to assert the reset and the dw_mmc part of the manual explicitly says
that the "reset_n should be asserted(active-low) for at least two clocks of
clk or cclk_in".
So I would expect something like
reset_control_assert(reset);
usleep_range(x, y);
reset_control_deassert(reset);
instead of only trying to deassert the reset.
> > > setup_timer(&host->cmd11_timer,
> > >
> > > dw_mci_cmd11_timer, (unsigned long)host);
> >
> > [...]
> >
> > > 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>
> >
> > unrelated changed regarding the reordering of includes.
>
> Making them in the order of alphabetic. If you dislike, I will not add.
It's Jaehoon's call and that change above is pretty small, but generally
introducing things unrelated to the change you actually want to make is not
that nice - that's what separate patches are for :-) .
Heiko
On 04/02/2016 03:42 AM, Heiko Stuebner wrote:
> Am Mittwoch, 30. M?rz 2016, 20:40:31 schrieb Jaehoon Chung:
>> modified Rob's mail address.
>>
>> On 03/30/2016 04:24 PM, 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.
>>
>> Do you mean mmc(card side) register or dwmmc host controller's register on
>> host side?
>>
>> According to dwmmc controller TMR, there are two reset signals. One is
>> reset_n, other is rst_n. It seems this patch is relevant to reset_n(For
>> host). (rst_n is hardware reset for card.)
>>
>> So could you clarify better? Then it's helpful to me for understanding..
>
> I think that actually means a reset of controller IP block logic, outside
> the control of the dw_mmc block itself.
>
> On Rockchip SoCs this gets triggered from the CRU (clock and reset unit), so
> I guess if I'm reading the manual correctly, should be the reset_n signal of
> the ip block.
>
> rst_n on the other hand gets triggered through a dw_mmc register setting and
> is already handled by the dw_mmc driver.
Right, this patch is for reset_n signal. I didn't have seen the SoC that reset_n is designed.
(Or i didn't realize...)
If Rockchip is used from CRU (clock and reset unit), then i think that it makes sense.
Thanks for explanation.
Best Regards,
Jaehoon Chung
>
>
> Heiko
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>