| Message ID | 1456968818-22140-2-git-send-email-guodong.xu@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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; > }; >
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 -- 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
? 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 > > >
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; };