From patchwork Mon Mar 20 10:27:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philipp Zabel X-Patchwork-Id: 9633699 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 7D2816020B for ; Mon, 20 Mar 2017 10:29:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6447627FBB for ; Mon, 20 Mar 2017 10:29:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 584FD27F94; Mon, 20 Mar 2017 10:29:28 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E035727F94 for ; Mon, 20 Mar 2017 10:29:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753437AbdCTK2D (ORCPT ); Mon, 20 Mar 2017 06:28:03 -0400 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:54387 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753201AbdCTK2C (ORCPT ); Mon, 20 Mar 2017 06:28:02 -0400 Received: from lupine.hi.pengutronix.de ([2001:67c:670:100:3ad5:47ff:feaf:1a17] helo=lupine) by metis.ext.pengutronix.de with esmtp (Exim 4.84_2) (envelope-from ) id 1cpuXg-000485-CV; Mon, 20 Mar 2017 11:28:00 +0100 Message-ID: <1490005679.2917.32.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 , linux-samsung-soc Date: Mon, 20 Mar 2017 11:27:59 +0100 In-Reply-To: References: <20170315113139.17989-1-p.zabel@pengutronix.de> <1490003602.2917.16.camel@pengutronix.de> X-Mailer: Evolution 3.12.9-1+b1 Mime-Version: 1.0 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-mmc@vger.kernel.org Sender: linux-mmc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-mmc@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Andrzej, On Mon, 2017-03-20 at 11:03 +0100, Andrzej Hajda wrote: > 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 I was thrown off by this. Should maybe dw_mci_probe return the error value reported by dw_mci_parse_dt instead of always returning -EINVAL? > >> [ 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? > > > When RESET_CONTROLLER is not enabled dummy stubs return -ENOSUPP error [1]. > > [1]: http://lxr.free-electrons.com/source/include/linux/reset.h#L77 Thanks, I suppose that issue should be fixed by: ----------8<---------- ---------->8---------- 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/include/linux/reset.h b/include/linux/reset.h index 86b4ed75359e8..c905ff1c21ec6 100644 --- a/include/linux/reset.h +++ b/include/linux/reset.h @@ -74,14 +74,14 @@ static inline struct reset_control *__of_reset_control_get( const char *id, int index, bool shared, bool optional) { - return ERR_PTR(-ENOTSUPP); + return optional ? NULL : ERR_PTR(-ENOTSUPP); } static inline struct reset_control *__devm_reset_control_get( struct device *dev, const char *id, int index, bool shared, bool optional) { - return ERR_PTR(-ENOTSUPP); + return optional ? NULL : ERR_PTR(-ENOTSUPP); } #endif /* CONFIG_RESET_CONTROLLER */