Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753951AbdCTJyI (ORCPT ); Mon, 20 Mar 2017 05:54:08 -0400 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:47853 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753795AbdCTJyC (ORCPT ); Mon, 20 Mar 2017 05:54:02 -0400 Message-ID: <1490003602.2917.16.camel@pengutronix.de> Subject: Re: [PATCH v2 06/14] mmc: dw_mmc: simplify optional reset handling From: Philipp Zabel To: Andrzej Hajda Cc: linux-mmc@vger.kernel.org, Jaehoon Chung , Ulf Hansson , linux-kernel@vger.kernel.org, "Szyprowski, Marek" , Bartlomiej Zolnierkiewicz Date: Mon, 20 Mar 2017 10:53:22 +0100 In-Reply-To: References: <20170315113139.17989-1-p.zabel@pengutronix.de> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.9-1+b1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 2001:67c:670:100:3ad5:47ff:feaf:1a17 X-SA-Exim-Mail-From: p.zabel@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2483 Lines: 59 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 > > --- > > 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