Re: [PATCH v2] usb: chipidea: removing of_find_property

Re: [PATCH v2] usb: chipidea: removing of_find_property

[Julia Lawall]

Please check.  The code, with the blank line on line 692, looks strange.

julia

On Tue, 17 Nov 2015, kbuild test robot wrote:

> CC: kbuild-all@01.org
> In-Reply-To: <1447760410-3426-1-git-send-email-saurabh.truth@gmail.com>
> TO: Saurabh Sengar <saurabh.truth@gmail.com>
> CC: mans@mansr.com, Peter.Chen@freescale.com, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Saurabh Sengar <saurabh.truth@gmail.com>
> CC: Saurabh Sengar <saurabh.truth@gmail.com>
>
> Hi Saurabh,
>
> [auto build test WARNING on peter.chen-usb/ci-for-usb-next]
> [also build test WARNING on v4.4-rc1 next-20151117]
>
> url:    https://github.com/0day-ci/linux/commits/Saurabh-Sengar/usb-chipidea-removing-of_find_property/20151117-194333
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb ci-for-usb-next
> :::::: branch date: 68 minutes ago
> :::::: commit date: 68 minutes ago
>
> >> drivers/usb/chipidea/core.c:693:1-27: code aligned with following code on line 695
>
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 4375ac1189e900bbde912d31ec3bb66572c0784a
> vim +693 drivers/usb/chipidea/core.c
>
> 63863b98 Heikki Krogerus   2015-09-21  687  	if (usb_get_maximum_speed(dev) == USB_SPEED_FULL)
> 4f6743d5 Michael Grzeschik 2014-02-19  688  		platdata->flags |= CI_HDRC_FORCE_FULLSPEED;
> 4f6743d5 Michael Grzeschik 2014-02-19  689
> 4375ac11 Saurabh Sengar    2015-11-17  690  	if (!of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
> 4375ac11 Saurabh Sengar    2015-11-17  691  				     &platdata->phy_clkgate_delay_us))
> 1fbf4628 Fabio Estevam     2015-09-08  692
> df96ed8d Peter Chen        2014-09-22 @693  	platdata->itc_setting = 1;
> 4375ac11 Saurabh Sengar    2015-11-17  694
> df96ed8d Peter Chen        2014-09-22 @695  	ret = of_property_read_u32(dev->of_node, "itc-setting",
> df96ed8d Peter Chen        2014-09-22  696  					&platdata->itc_setting);
> 4375ac11 Saurabh Sengar    2015-11-17  697  	if (ret && ret != -EINVAL) {
> 4375ac11 Saurabh Sengar    2015-11-17  698  		dev_err(dev, "failed to get itc-setting\n");
>
> :::::: The code at line 693 was first introduced by commit
> :::::: df96ed8dced21426c54c7f69cf7513e75280957a usb: chipidea: introduce ITC tuning interface
>
> :::::: TO: Peter Chen <peter.chen@freescale.com>
> :::::: CC: Peter Chen <peter.chen@freescale.com>
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>

Re: [PATCH v2] usb: chipidea: removing of_find_property

[Saurabh Sengar]

Hi Julia,

You have used v2 of patch, I have sent v3 of patch too. 1:30 hour before
Please use version 3 as that is the latest.
version 3 : https://lkml.org/lkml/2015/11/17/243

Sorry for trouble.

Regards,
Saurabh

On 17 November 2015 at 18:28, Julia Lawall <julia.lawall@lip6.fr> wrote:
> Please check.  The code, with the blank line on line 692, looks strange.
>
> julia
>
> On Tue, 17 Nov 2015, kbuild test robot wrote:
>
>> CC: kbuild-all@01.org
>> In-Reply-To: <1447760410-3426-1-git-send-email-saurabh.truth@gmail.com>
>> TO: Saurabh Sengar <saurabh.truth@gmail.com>
>> CC: mans@mansr.com, Peter.Chen@freescale.com, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Saurabh Sengar <saurabh.truth@gmail.com>
>> CC: Saurabh Sengar <saurabh.truth@gmail.com>
>>
>> Hi Saurabh,
>>
>> [auto build test WARNING on peter.chen-usb/ci-for-usb-next]
>> [also build test WARNING on v4.4-rc1 next-20151117]
>>
>> url:    https://github.com/0day-ci/linux/commits/Saurabh-Sengar/usb-chipidea-removing-of_find_property/20151117-194333
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb ci-for-usb-next
>> :::::: branch date: 68 minutes ago
>> :::::: commit date: 68 minutes ago
>>
>> >> drivers/usb/chipidea/core.c:693:1-27: code aligned with following code on line 695
>>
>> git remote add linux-review https://github.com/0day-ci/linux
>> git remote update linux-review
>> git checkout 4375ac1189e900bbde912d31ec3bb66572c0784a
>> vim +693 drivers/usb/chipidea/core.c
>>
>> 63863b98 Heikki Krogerus   2015-09-21  687    if (usb_get_maximum_speed(dev) == USB_SPEED_FULL)
>> 4f6743d5 Michael Grzeschik 2014-02-19  688            platdata->flags |= CI_HDRC_FORCE_FULLSPEED;
>> 4f6743d5 Michael Grzeschik 2014-02-19  689
>> 4375ac11 Saurabh Sengar    2015-11-17  690    if (!of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
>> 4375ac11 Saurabh Sengar    2015-11-17  691                                 &platdata->phy_clkgate_delay_us))
>> 1fbf4628 Fabio Estevam     2015-09-08  692
>> df96ed8d Peter Chen        2014-09-22 @693    platdata->itc_setting = 1;
>> 4375ac11 Saurabh Sengar    2015-11-17  694
>> df96ed8d Peter Chen        2014-09-22 @695    ret = of_property_read_u32(dev->of_node, "itc-setting",
>> df96ed8d Peter Chen        2014-09-22  696                                    &platdata->itc_setting);
>> 4375ac11 Saurabh Sengar    2015-11-17  697    if (ret && ret != -EINVAL) {
>> 4375ac11 Saurabh Sengar    2015-11-17  698            dev_err(dev, "failed to get itc-setting\n");
>>
>> :::::: The code at line 693 was first introduced by commit
>> :::::: df96ed8dced21426c54c7f69cf7513e75280957a usb: chipidea: introduce ITC tuning interface
>>
>> :::::: TO: Peter Chen <peter.chen@freescale.com>
>> :::::: CC: Peter Chen <peter.chen@freescale.com>
>>
>> ---
>> 0-DAY kernel test infrastructure                Open Source Technology Center
>> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>>

Re: [PATCH] usb: chipidea: removing of_find_property

[Måns Rullgård]

Saurabh Sengar <saurabh.truth@gmail.com> writes:

> call to of_find_property() before of_property_read_u32() is unnecessary.
> of_property_read_u32() anyway calls to of_find_property() only.
>
> Signed-off-by: Saurabh Sengar <saurabh.truth@gmail.com>
> ---
>  drivers/usb/chipidea/core.c | 67 ++++++++++++++++++++++-----------------------
>  1 file changed, 32 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 965d0e2..8a4c22c 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -643,6 +643,7 @@ static int ci_get_platdata(struct device *dev,
>  	struct extcon_dev *ext_vbus, *ext_id;
>  	struct ci_hdrc_cable *cable;
>  	int ret;
> +	u32 pval;
>
>  	if (!platdata->phy_mode)
>  		platdata->phy_mode = of_usb_get_phy_mode(dev->of_node);
> @@ -688,52 +689,48 @@ static int ci_get_platdata(struct device *dev,
>  	if (usb_get_maximum_speed(dev) == USB_SPEED_FULL)
>  		platdata->flags |= CI_HDRC_FORCE_FULLSPEED;
>
> -	if (of_find_property(dev->of_node, "phy-clkgate-delay-us", NULL))
> -		of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
> -				     &platdata->phy_clkgate_delay_us);
> +	if (!of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
> +				     &pval))
> +		platdata->phy_clkgate_delay_us = pval;

You don't need to use the pval temporary as of_property_read_u32 only
modifies the destination on success.

-- 
Måns Rullgård
mans@mansr.com

[PATCH v2] usb: chipidea: removing of_find_property

[Saurabh Sengar]

call to of_find_property() before of_property_read_u32() is unnecessary.
of_property_read_u32() anyway calls to of_find_property() only.

Signed-off-by: Saurabh Sengar <saurabh.truth@gmail.com>
---
v2: removed pval variable
 drivers/usb/chipidea/core.c | 61 +++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 965d0e2..916a20d 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -688,52 +688,43 @@ static int ci_get_platdata(struct device *dev,
 	if (usb_get_maximum_speed(dev) == USB_SPEED_FULL)
 		platdata->flags |= CI_HDRC_FORCE_FULLSPEED;
 
-	if (of_find_property(dev->of_node, "phy-clkgate-delay-us", NULL))
-		of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
-				     &platdata->phy_clkgate_delay_us);
+	if (!of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
+				     &platdata->phy_clkgate_delay_us))
 
 	platdata->itc_setting = 1;
-	if (of_find_property(dev->of_node, "itc-setting", NULL)) {
-		ret = of_property_read_u32(dev->of_node, "itc-setting",
-			&platdata->itc_setting);
-		if (ret) {
-			dev_err(dev,
-				"failed to get itc-setting\n");
-			return ret;
-		}
+
+	ret = of_property_read_u32(dev->of_node, "itc-setting",
+					&platdata->itc_setting);
+	if (ret && ret != -EINVAL) {
+		dev_err(dev, "failed to get itc-setting\n");
+		return ret;
 	}
 
-	if (of_find_property(dev->of_node, "ahb-burst-config", NULL)) {
-		ret = of_property_read_u32(dev->of_node, "ahb-burst-config",
-			&platdata->ahb_burst_config);
-		if (ret) {
-			dev_err(dev,
-				"failed to get ahb-burst-config\n");
-			return ret;
-		}
+	ret = of_property_read_u32(dev->of_node, "ahb-burst-config",
+				&platdata->ahb_burst_config);
+	if (!ret) {
 		platdata->flags |= CI_HDRC_OVERRIDE_AHB_BURST;
+	} else if (ret != -EINVAL) {
+		dev_err(dev, "failed to get ahb-burst-config\n");
+		return ret;
 	}
 
-	if (of_find_property(dev->of_node, "tx-burst-size-dword", NULL)) {
-		ret = of_property_read_u32(dev->of_node, "tx-burst-size-dword",
-			&platdata->tx_burst_size);
-		if (ret) {
-			dev_err(dev,
-				"failed to get tx-burst-size-dword\n");
-			return ret;
-		}
+	ret = of_property_read_u32(dev->of_node, "tx-burst-size-dword",
+				&platdata->tx_burst_size);
+	if (!ret) {
 		platdata->flags |= CI_HDRC_OVERRIDE_TX_BURST;
+	} else if (ret != -EINVAL) {
+		dev_err(dev, "failed to get tx-burst-size-dword\n");
+		return ret;
 	}
 
-	if (of_find_property(dev->of_node, "rx-burst-size-dword", NULL)) {
-		ret = of_property_read_u32(dev->of_node, "rx-burst-size-dword",
-			&platdata->rx_burst_size);
-		if (ret) {
-			dev_err(dev,
-				"failed to get rx-burst-size-dword\n");
-			return ret;
-		}
+	ret = of_property_read_u32(dev->of_node, "rx-burst-size-dword",
+				&platdata->rx_burst_size);
+	if (!ret) {
 		platdata->flags |= CI_HDRC_OVERRIDE_RX_BURST;
+	} else if (ret != -EINVAL) {
+		dev_err(dev, "failed to get rx-burst-size-dword\n");
+		return ret;
 	}
 
 	ext_id = ERR_PTR(-ENODEV);
-- 
1.9.1

Re: [PATCH v2] usb: chipidea: removing of_find_property

[Måns Rullgård]

Saurabh Sengar <saurabh.truth@gmail.com> writes:

> call to of_find_property() before of_property_read_u32() is unnecessary.
> of_property_read_u32() anyway calls to of_find_property() only.
>
> Signed-off-by: Saurabh Sengar <saurabh.truth@gmail.com>
> ---
> v2: removed pval variable
>  drivers/usb/chipidea/core.c | 61 +++++++++++++++++++--------------------------
>  1 file changed, 26 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 965d0e2..916a20d 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -688,52 +688,43 @@ static int ci_get_platdata(struct device *dev,
>  	if (usb_get_maximum_speed(dev) == USB_SPEED_FULL)
>  		platdata->flags |= CI_HDRC_FORCE_FULLSPEED;
>
> -	if (of_find_property(dev->of_node, "phy-clkgate-delay-us", NULL))
> -		of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
> -				     &platdata->phy_clkgate_delay_us);
> +	if (!of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
> +				     &platdata->phy_clkgate_delay_us))
>
>  	platdata->itc_setting = 1;

Drop that if().  Since we're ignoring of_property_read_u32() failing,
there is no need to test its return value, and code above incorrectly
makes the next statement conditional.

-- 
Måns Rullgård
mans@mansr.com