| Message ID | 20170315113139.17989-1-p.zabel@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15 March 2017 at 12:31, Philipp Zabel <p.zabel@pengutronix.de> wrote: > As of commit bb475230b8e5 ("reset: make optional functions really > optional"), the reset framework API calls use NULL pointers to describe > optional, non-present reset controls. > > This allows to return errors from devm_reset_control_get_optional and to > call reset_control_(de)assert unconditionally. > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> Thanks, applied for next! Kind regards Uffe > --- > drivers/mmc/host/dw_mmc.c | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index a9ac0b4573131..3d62b0a1f81cb 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -2968,10 +2968,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host) > > /* find reset controller when exist */ > pdata->rstc = devm_reset_control_get_optional(dev, "reset"); > - if (IS_ERR(pdata->rstc)) { > - if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER) > - return ERR_PTR(-EPROBE_DEFER); > - } > + if (IS_ERR(pdata->rstc)) > + return ERR_CAST(pdata->rstc); > > /* find out number of slots supported */ > of_property_read_u32(np, "num-slots", &pdata->num_slots); > @@ -3100,7 +3098,7 @@ int dw_mci_probe(struct dw_mci *host) > } > } > > - if (!IS_ERR(host->pdata->rstc)) { > + if (host->pdata->rstc) { > reset_control_assert(host->pdata->rstc); > usleep_range(10, 50); > reset_control_deassert(host->pdata->rstc); > @@ -3257,8 +3255,7 @@ int dw_mci_probe(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); > + reset_control_assert(host->pdata->rstc); > > err_clk_ciu: > clk_disable_unprepare(host->ciu_clk); > @@ -3290,8 +3287,7 @@ 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); > + reset_control_assert(host->pdata->rstc); > > clk_disable_unprepare(host->ciu_clk); > clk_disable_unprepare(host->biu_clk); > -- > 2.11.0 > -- 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
Hi Philipp, Todays next branch does not work with exynos5433-tm2 board. I guess this patch causes regression. On MMC without reset controller I get errors: [ 4.938222] dwmmc_exynos 15540000.mshc: platform data not available [ 4.943268] dwmmc_exynos: probe of 15540000.mshc failed with error -22 [ 4.950184] dwmmc_exynos 15560000.mshc: platform data not available [ 4.955962] dwmmc_exynos: probe of 15560000.mshc failed with error -22 Commenting out reset controller get and error checks 'fixes' the issue. On 15.03.2017 12:31, Philipp Zabel wrote: > As of commit bb475230b8e5 ("reset: make optional functions really > optional"), the reset framework API calls use NULL pointers to describe > optional, non-present reset controls. > > This allows to return errors from devm_reset_control_get_optional and to > call reset_control_(de)assert unconditionally. > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > --- > drivers/mmc/host/dw_mmc.c | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index a9ac0b4573131..3d62b0a1f81cb 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -2968,10 +2968,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host) > > /* find reset controller when exist */ > pdata->rstc = devm_reset_control_get_optional(dev, "reset"); > - if (IS_ERR(pdata->rstc)) { > - if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER) > - return ERR_PTR(-EPROBE_DEFER); > - } > + if (IS_ERR(pdata->rstc)) > + return ERR_CAST(pdata->rstc); With three lines above commented out it works. Regards Andrzej -- 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
On Mon, 2017-03-20 at 10:22 +0100, Andrzej Hajda wrote: > Hi Philipp, > > Todays next branch does not work with exynos5433-tm2 board. > I guess this patch causes regression. On MMC without reset controller I > get errors: > [ 4.938222] dwmmc_exynos 15540000.mshc: platform data not available > [ 4.943268] dwmmc_exynos: probe of 15540000.mshc failed with error -22 > [ 4.950184] dwmmc_exynos 15560000.mshc: platform data not available > [ 4.955962] dwmmc_exynos: probe of 15560000.mshc failed with error -22 > > Commenting out reset controller get and error checks 'fixes' the issue. > > On 15.03.2017 12:31, Philipp Zabel wrote: > > As of commit bb475230b8e5 ("reset: make optional functions really > > optional"), the reset framework API calls use NULL pointers to describe > > optional, non-present reset controls. > > > > This allows to return errors from devm_reset_control_get_optional and to > > call reset_control_(de)assert unconditionally. > > > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > > --- > > drivers/mmc/host/dw_mmc.c | 14 +++++--------- > > 1 file changed, 5 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > > index a9ac0b4573131..3d62b0a1f81cb 100644 > > --- a/drivers/mmc/host/dw_mmc.c > > +++ b/drivers/mmc/host/dw_mmc.c > > @@ -2968,10 +2968,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host) > > > > /* find reset controller when exist */ > > pdata->rstc = devm_reset_control_get_optional(dev, "reset"); > > - if (IS_ERR(pdata->rstc)) { > > - if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER) > > - return ERR_PTR(-EPROBE_DEFER); > > - } > > + if (IS_ERR(pdata->rstc)) > > + return ERR_CAST(pdata->rstc); > > > With three lines above commented out it works. So devm_reset_control_get_optional returns -EINVAL. Why? The mshc@15560000 node is compatible to "samsung,exynos7-dw-mshc-smu", so that's dw_mmc-exynos.c calling dw_mci_pltfm_register, which then calls dw_mci_probe, passing the original platform device as host->dev = &pdev->dev, and I expect __of_reset_control_get being called with a valid DT node (dev->of_node). Since id is set to "reset", of_property_match_string(node, "reset-names", id) should then be called and return -EINVAL because the "reset-names" property does not exist. Then __of_reset_control_get should return NULL because optional == true. Some of this obviously doesn't happen, where am I wrong? regards Philipp -- 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
Hi Philipp, On 20.03.2017 10:53, Philipp Zabel wrote: > On Mon, 2017-03-20 at 10:22 +0100, Andrzej Hajda wrote: >> Hi Philipp, >> >> Todays next branch does not work with exynos5433-tm2 board. >> I guess this patch causes regression. On MMC without reset controller I >> get errors: >> [ 4.938222] dwmmc_exynos 15540000.mshc: platform data not available >> [ 4.943268] dwmmc_exynos: probe of 15540000.mshc failed with error -22 >> [ 4.950184] dwmmc_exynos 15560000.mshc: platform data not available >> [ 4.955962] dwmmc_exynos: probe of 15560000.mshc failed with error -22 >> >> Commenting out reset controller get and error checks 'fixes' the issue. >> >> On 15.03.2017 12:31, Philipp Zabel wrote: >>> As of commit bb475230b8e5 ("reset: make optional functions really >>> optional"), the reset framework API calls use NULL pointers to describe >>> optional, non-present reset controls. >>> >>> This allows to return errors from devm_reset_control_get_optional and to >>> call reset_control_(de)assert unconditionally. >>> >>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> >>> --- >>> drivers/mmc/host/dw_mmc.c | 14 +++++--------- >>> 1 file changed, 5 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >>> index a9ac0b4573131..3d62b0a1f81cb 100644 >>> --- a/drivers/mmc/host/dw_mmc.c >>> +++ b/drivers/mmc/host/dw_mmc.c >>> @@ -2968,10 +2968,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host) >>> >>> /* find reset controller when exist */ >>> pdata->rstc = devm_reset_control_get_optional(dev, "reset"); >>> - if (IS_ERR(pdata->rstc)) { >>> - if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER) >>> - return ERR_PTR(-EPROBE_DEFER); >>> - } >>> + if (IS_ERR(pdata->rstc)) >>> + return ERR_CAST(pdata->rstc); >> >> With three lines above commented out it works. > So devm_reset_control_get_optional returns -EINVAL. Why? > > The mshc@15560000 node is compatible to "samsung,exynos7-dw-mshc-smu", > so that's dw_mmc-exynos.c calling dw_mci_pltfm_register, which then > calls dw_mci_probe, passing the original platform device as > host->dev = &pdev->dev, and I expect __of_reset_control_get being called > with a valid DT node (dev->of_node). > Since id is set to "reset", of_property_match_string(node, > "reset-names", id) should then be called and return -EINVAL because the > "reset-names" property does not exist. Then __of_reset_control_get > should return NULL because optional == true. > Some of this obviously doesn't happen, where am I wrong? When RESET_CONTROLLER is not enabled dummy stubs return -ENOSUPP error [1]. [1]: http://lxr.free-electrons.com/source/include/linux/reset.h#L77 Regards Andrzej > > regards > Philipp > > > > -- 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
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index a9ac0b4573131..3d62b0a1f81cb 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -2968,10 +2968,8 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host) /* find reset controller when exist */ pdata->rstc = devm_reset_control_get_optional(dev, "reset"); - if (IS_ERR(pdata->rstc)) { - if (PTR_ERR(pdata->rstc) == -EPROBE_DEFER) - return ERR_PTR(-EPROBE_DEFER); - } + if (IS_ERR(pdata->rstc)) + return ERR_CAST(pdata->rstc); /* find out number of slots supported */ of_property_read_u32(np, "num-slots", &pdata->num_slots); @@ -3100,7 +3098,7 @@ int dw_mci_probe(struct dw_mci *host) } } - if (!IS_ERR(host->pdata->rstc)) { + if (host->pdata->rstc) { reset_control_assert(host->pdata->rstc); usleep_range(10, 50); reset_control_deassert(host->pdata->rstc); @@ -3257,8 +3255,7 @@ int dw_mci_probe(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); + reset_control_assert(host->pdata->rstc); err_clk_ciu: clk_disable_unprepare(host->ciu_clk); @@ -3290,8 +3287,7 @@ 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); + reset_control_assert(host->pdata->rstc); clk_disable_unprepare(host->ciu_clk); clk_disable_unprepare(host->biu_clk);
As of commit bb475230b8e5 ("reset: make optional functions really optional"), the reset framework API calls use NULL pointers to describe optional, non-present reset controls. This allows to return errors from devm_reset_control_get_optional and to call reset_control_(de)assert unconditionally. Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> --- drivers/mmc/host/dw_mmc.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)