From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jisheng Zhang Subject: Re: [PATCH v2] mmc: sdhci-xenon: Fix clock resource by adding an optional bus clock Date: Sat, 30 Sep 2017 14:17:53 +0800 Message-ID: <20170930141753.7c4e34ea@xhacker.debian> References: <20170929130435.6805-1-gregory.clement@free-electrons.com> <20170930104109.60853202@xhacker.debian> <20170930080710.119c0258@windsurf> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20170930080710.119c0258@windsurf> Sender: stable-owner@vger.kernel.org To: Thomas Petazzoni Cc: Gregory CLEMENT , Ulf Hansson , Adrian Hunter , linux-mmc@vger.kernel.org, Rob Herring , devicetree@vger.kernel.org, Jason Cooper , Andrew Lunn , Sebastian Hesselbarth , linux-arm-kernel@lists.infradead.org, Antoine Tenart , =?UTF-8?B?TWlxdcOo?= =?UTF-8?B?bA==?= Raynal , Nadav Haklai , Shadi Ammouri , Yehuda Yitschak , Omri Itach , Hanna Hawa , Igal Liberman , Marcin Wojtas List-Id: linux-mmc@vger.kernel.org Hi Thomas, On Sat, 30 Sep 2017 08:07:10 +0200 Thomas Petazzoni wrote: > Hello, > > On Sat, 30 Sep 2017 10:41:09 +0800, Jisheng Zhang wrote: > > > > + priv->axi_clk = devm_clk_get(&pdev->dev, "axi"); > > > + if (IS_ERR(priv->axi_clk)) { > > > + err = PTR_ERR(priv->axi_clk); > > > + if (err == -EPROBE_DEFER) > > > + goto err_clk; > > > + priv->axi_clk = NULL; > > > > IMHO, this assignment could be removed. What do you think? > > No, because if the clock is not present in the Device Tree, > priv->axi_clk will contain an error code encoded as a pointer, so > priv->axi_clk will not be NULL. Hence the rest of the code that will FWICT, the clk_disable_unprepare() could also check whether the clk is err, am I wrong? > operate on this clock will believe there is a valid clock pointed by > priv->axi_clk. So we really want to reset it to NULL when there was an > error getting the clock. > > Best regards, > > Thomas From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jisheng.Zhang@synaptics.com (Jisheng Zhang) Date: Sat, 30 Sep 2017 14:17:53 +0800 Subject: [PATCH v2] mmc: sdhci-xenon: Fix clock resource by adding an optional bus clock In-Reply-To: <20170930080710.119c0258@windsurf> References: <20170929130435.6805-1-gregory.clement@free-electrons.com> <20170930104109.60853202@xhacker.debian> <20170930080710.119c0258@windsurf> Message-ID: <20170930141753.7c4e34ea@xhacker.debian> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Thomas, On Sat, 30 Sep 2017 08:07:10 +0200 Thomas Petazzoni wrote: > Hello, > > On Sat, 30 Sep 2017 10:41:09 +0800, Jisheng Zhang wrote: > > > > + priv->axi_clk = devm_clk_get(&pdev->dev, "axi"); > > > + if (IS_ERR(priv->axi_clk)) { > > > + err = PTR_ERR(priv->axi_clk); > > > + if (err == -EPROBE_DEFER) > > > + goto err_clk; > > > + priv->axi_clk = NULL; > > > > IMHO, this assignment could be removed. What do you think? > > No, because if the clock is not present in the Device Tree, > priv->axi_clk will contain an error code encoded as a pointer, so > priv->axi_clk will not be NULL. Hence the rest of the code that will FWICT, the clk_disable_unprepare() could also check whether the clk is err, am I wrong? > operate on this clock will believe there is a valid clock pointed by > priv->axi_clk. So we really want to reset it to NULL when there was an > error getting the clock. > > Best regards, > > Thomas From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sn1nam01on0069.outbound.protection.outlook.com ([104.47.32.69]:57679 "EHLO NAM01-SN1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750984AbdI3GSg (ORCPT ); Sat, 30 Sep 2017 02:18:36 -0400 Date: Sat, 30 Sep 2017 14:17:53 +0800 From: Jisheng Zhang To: Thomas Petazzoni Cc: Gregory CLEMENT , Ulf Hansson , Adrian Hunter , , Rob Herring , , Jason Cooper , "Andrew Lunn" , Sebastian Hesselbarth , , Antoine Tenart , =?UTF-8?B?TWlxdcOo?= =?UTF-8?B?bA==?= Raynal , Nadav Haklai , Shadi Ammouri , Yehuda Yitschak , Omri Itach , Hanna Hawa , Igal Liberman , Marcin Wojtas , Stable Subject: Re: [PATCH v2] mmc: sdhci-xenon: Fix clock resource by adding an optional bus clock Message-ID: <20170930141753.7c4e34ea@xhacker.debian> In-Reply-To: <20170930080710.119c0258@windsurf> References: <20170929130435.6805-1-gregory.clement@free-electrons.com> <20170930104109.60853202@xhacker.debian> <20170930080710.119c0258@windsurf> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: Hi Thomas, On Sat, 30 Sep 2017 08:07:10 +0200 Thomas Petazzoni wrote: > Hello, > > On Sat, 30 Sep 2017 10:41:09 +0800, Jisheng Zhang wrote: > > > > + priv->axi_clk = devm_clk_get(&pdev->dev, "axi"); > > > + if (IS_ERR(priv->axi_clk)) { > > > + err = PTR_ERR(priv->axi_clk); > > > + if (err == -EPROBE_DEFER) > > > + goto err_clk; > > > + priv->axi_clk = NULL; > > > > IMHO, this assignment could be removed. What do you think? > > No, because if the clock is not present in the Device Tree, > priv->axi_clk will contain an error code encoded as a pointer, so > priv->axi_clk will not be NULL. Hence the rest of the code that will FWICT, the clk_disable_unprepare() could also check whether the clk is err, am I wrong? > operate on this clock will believe there is a valid clock pointed by > priv->axi_clk. So we really want to reset it to NULL when there was an > error getting the clock. > > Best regards, > > Thomas