[PATCH v2] bus: ti-sysc: sysc_check_one_child(): Change return type to void

[PATCH v2] bus: ti-sysc: sysc_check_one_child(): Change return type to void

[Nishka Dasgupta]

Change return type of function sysc_check_one_child() from int to void
as it always returns 0. Accordingly, at its callsite, delete the
variable that previously stored the return value.

Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
Changes in v2:
- Remove error variable entirely.
- Change return type of sysc_check_one_child().

 drivers/bus/ti-sysc.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
index e6deabd8305d..1c30fa58d70c 100644
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -615,8 +615,8 @@ static void sysc_check_quirk_stdout(struct sysc *ddata,
  * node but children have "ti,hwmods". These belong to the interconnect
  * target node and are managed by this driver.
  */
-static int sysc_check_one_child(struct sysc *ddata,
-				struct device_node *np)
+static void sysc_check_one_child(struct sysc *ddata,
+				 struct device_node *np)
 {
 	const char *name;
 
@@ -633,12 +633,9 @@ static int sysc_check_one_child(struct sysc *ddata,
 static int sysc_check_children(struct sysc *ddata)
 {
 	struct device_node *child;
-	int error;
 
 	for_each_child_of_node(ddata->dev->of_node, child) {
-		error = sysc_check_one_child(ddata, child);
-		if (error)
-			return error;
+		sysc_check_one_child(ddata, child);
 	}
 
 	return 0;
-- 
2.19.1

Re: [PATCH v2] bus: ti-sysc: sysc_check_one_child(): Change return type to void

[Roger Quadros]

On 13/08/2019 10:17, Nishka Dasgupta wrote:
> Change return type of function sysc_check_one_child() from int to void
> as it always returns 0. Accordingly, at its callsite, delete the
> variable that previously stored the return value.
> 
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
> Changes in v2:
> - Remove error variable entirely.
> - Change return type of sysc_check_one_child().
> 
>  drivers/bus/ti-sysc.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> index e6deabd8305d..1c30fa58d70c 100644
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -615,8 +615,8 @@ static void sysc_check_quirk_stdout(struct sysc *ddata,
>   * node but children have "ti,hwmods". These belong to the interconnect
>   * target node and are managed by this driver.
>   */
> -static int sysc_check_one_child(struct sysc *ddata,
> -				struct device_node *np)
> +static void sysc_check_one_child(struct sysc *ddata,
> +				 struct device_node *np)
>  {
>  	const char *name;
>  

You didn't remove the "return 0" at end of this function.
Doesn't it complain during build?

> @@ -633,12 +633,9 @@ static int sysc_check_one_child(struct sysc *ddata,
>  static int sysc_check_children(struct sysc *ddata)
>  {

This could return void as well.

>  	struct device_node *child;
> -	int error;
>  
>  	for_each_child_of_node(ddata->dev->of_node, child) {
> -		error = sysc_check_one_child(ddata, child);
> -		if (error)
> -			return error;
> +		sysc_check_one_child(ddata, child);
>  	}

You don't need the braces { }.

Please run ./scripts/checkpatch.pl --strict on your patch and fix any
issues.

>  
>  	return 0;
> 

return not required.

You will also need to fix all instances using sysc_check_children()

cheers,
-roger
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH v2] bus: ti-sysc: sysc_check_one_child(): Change return type to void

[Nishka Dasgupta]

On 13/08/19 12:58 PM, Roger Quadros wrote:
> 
> 
> On 13/08/2019 10:17, Nishka Dasgupta wrote:
>> Change return type of function sysc_check_one_child() from int to void
>> as it always returns 0. Accordingly, at its callsite, delete the
>> variable that previously stored the return value.
>>
>> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
>> ---
>> Changes in v2:
>> - Remove error variable entirely.
>> - Change return type of sysc_check_one_child().
>>
>>   drivers/bus/ti-sysc.c | 9 +++------
>>   1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
>> index e6deabd8305d..1c30fa58d70c 100644
>> --- a/drivers/bus/ti-sysc.c
>> +++ b/drivers/bus/ti-sysc.c
>> @@ -615,8 +615,8 @@ static void sysc_check_quirk_stdout(struct sysc *ddata,
>>    * node but children have "ti,hwmods". These belong to the interconnect
>>    * target node and are managed by this driver.
>>    */
>> -static int sysc_check_one_child(struct sysc *ddata,
>> -				struct device_node *np)
>> +static void sysc_check_one_child(struct sysc *ddata,
>> +				 struct device_node *np)
>>   {
>>   	const char *name;
>>   
> 
> You didn't remove the "return 0" at end of this function.
> Doesn't it complain during build?
> 
>> @@ -633,12 +633,9 @@ static int sysc_check_one_child(struct sysc *ddata,
>>   static int sysc_check_children(struct sysc *ddata)
>>   {
> 
> This could return void as well.

Okay. Sorry for the errors; I'll fix-up and resend.

However, while building it, I'm running into a compilation problem:
on line 764 (function sysc_ioremap) there is apparently an "undeclared 
variable", "SZ_1K". Is it a problem with the architecture? What arch 
should I compile it with?

Thanking you,
Nishka

> 
>>   	struct device_node *child;
>> -	int error;
>>   
>>   	for_each_child_of_node(ddata->dev->of_node, child) {
>> -		error = sysc_check_one_child(ddata, child);
>> -		if (error)
>> -			return error;
>> +		sysc_check_one_child(ddata, child);
>>   	}
> 
> You don't need the braces { }.
> 
> Please run ./scripts/checkpatch.pl --strict on your patch and fix any
> issues.
> 
>>   
>>   	return 0;
>>
> 
> return not required.
> 
> You will also need to fix all instances using sysc_check_children()
> 
> cheers,
> -roger
> 

Re: [PATCH v2] bus: ti-sysc: sysc_check_one_child(): Change return type to void

[Roger Quadros]

On 13/08/2019 10:37, Nishka Dasgupta wrote:
> On 13/08/19 12:58 PM, Roger Quadros wrote:
>>
>>
>> On 13/08/2019 10:17, Nishka Dasgupta wrote:
>>> Change return type of function sysc_check_one_child() from int to void
>>> as it always returns 0. Accordingly, at its callsite, delete the
>>> variable that previously stored the return value.
>>>
>>> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
>>> ---
>>> Changes in v2:
>>> - Remove error variable entirely.
>>> - Change return type of sysc_check_one_child().
>>>
>>>   drivers/bus/ti-sysc.c | 9 +++------
>>>   1 file changed, 3 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
>>> index e6deabd8305d..1c30fa58d70c 100644
>>> --- a/drivers/bus/ti-sysc.c
>>> +++ b/drivers/bus/ti-sysc.c
>>> @@ -615,8 +615,8 @@ static void sysc_check_quirk_stdout(struct sysc *ddata,
>>>    * node but children have "ti,hwmods". These belong to the interconnect
>>>    * target node and are managed by this driver.
>>>    */
>>> -static int sysc_check_one_child(struct sysc *ddata,
>>> -                struct device_node *np)
>>> +static void sysc_check_one_child(struct sysc *ddata,
>>> +                 struct device_node *np)
>>>   {
>>>       const char *name;
>>>   
>>
>> You didn't remove the "return 0" at end of this function.
>> Doesn't it complain during build?
>>
>>> @@ -633,12 +633,9 @@ static int sysc_check_one_child(struct sysc *ddata,
>>>   static int sysc_check_children(struct sysc *ddata)
>>>   {
>>
>> This could return void as well.
> 
> Okay. Sorry for the errors; I'll fix-up and resend.
> 
> However, while building it, I'm running into a compilation problem:
> on line 764 (function sysc_ioremap) there is apparently an "undeclared variable", "SZ_1K". Is it a problem with the architecture? What arch should I compile it with?

make ARCH=arm omap2plus_defconfig
make -j4 ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf-

> 
> Thanking you,
> Nishka
> 
>>
>>>       struct device_node *child;
>>> -    int error;
>>>         for_each_child_of_node(ddata->dev->of_node, child) {
>>> -        error = sysc_check_one_child(ddata, child);
>>> -        if (error)
>>> -            return error;
>>> +        sysc_check_one_child(ddata, child);
>>>       }
>>
>> You don't need the braces { }.
>>
>> Please run ./scripts/checkpatch.pl --strict on your patch and fix any
>> issues.
>>
>>>         return 0;
>>>
>>
>> return not required.
>>
>> You will also need to fix all instances using sysc_check_children()
>>

cheers,
-roger
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH v3 1/2] bus: ti-sysc: sysc_check_one_child(): Change return type to void

[Nishka Dasgupta]

Change return type of function sysc_check_one_child() from int to void
as it always returns 0. Remove the now-unnecessary return statement as
well. Accordingly, at its call site, delete the error variable that
previously stored the return value.

Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
Changes in v3:
- Remove return statement in sysc_check_one_child().
- Remove braces at call site.
Changes in v2:
- Remove error variable entirely.
- Change return type of sysc_check_one_child().

 drivers/bus/ti-sysc.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
index e6deabd8305d..9c6d3e121d37 100644
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -615,8 +615,8 @@ static void sysc_check_quirk_stdout(struct sysc *ddata,
  * node but children have "ti,hwmods". These belong to the interconnect
  * target node and are managed by this driver.
  */
-static int sysc_check_one_child(struct sysc *ddata,
-				struct device_node *np)
+static void sysc_check_one_child(struct sysc *ddata,
+				 struct device_node *np)
 {
 	const char *name;
 
@@ -626,20 +626,14 @@ static int sysc_check_one_child(struct sysc *ddata,
 
 	sysc_check_quirk_stdout(ddata, np);
 	sysc_parse_dts_quirks(ddata, np, true);
-
-	return 0;
 }
 
 static int sysc_check_children(struct sysc *ddata)
 {
 	struct device_node *child;
-	int error;
 
-	for_each_child_of_node(ddata->dev->of_node, child) {
-		error = sysc_check_one_child(ddata, child);
-		if (error)
-			return error;
-	}
+	for_each_child_of_node(ddata->dev->of_node, child)
+		sysc_check_one_child(ddata, child);
 
 	return 0;
 }
-- 
2.19.1

[PATCH v3 2/2] bus: ti-sysc: sysc_check_children(): Change return type to void

[Nishka Dasgupta]

Change return type of function sysc_check_children() from int to void as
it always returns 0. Remove its return statement as well.
At call site, remove the variable that was used to store the return
value, as well as the check on the return value.

Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
- This is a new patch; labelled v3 only because it is in the same series
  as the previous patch.

 drivers/bus/ti-sysc.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
index 9c6d3e121d37..a2eae8f36ef8 100644
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -628,14 +628,12 @@ static void sysc_check_one_child(struct sysc *ddata,
 	sysc_parse_dts_quirks(ddata, np, true);
 }
 
-static int sysc_check_children(struct sysc *ddata)
+static void sysc_check_children(struct sysc *ddata)
 {
 	struct device_node *child;
 
 	for_each_child_of_node(ddata->dev->of_node, child)
 		sysc_check_one_child(ddata, child);
-
-	return 0;
 }
 
 /*
@@ -788,9 +786,7 @@ static int sysc_map_and_check_registers(struct sysc *ddata)
 	if (error)
 		return error;
 
-	error = sysc_check_children(ddata);
-	if (error)
-		return error;
+	sysc_check_children(ddata);
 
 	error = sysc_parse_registers(ddata);
 	if (error)
-- 
2.19.1

Re: [PATCH v3 2/2] bus: ti-sysc: sysc_check_children(): Change return type to void

[Roger Quadros]

On 13/08/2019 10:55, Nishka Dasgupta wrote:
> Change return type of function sysc_check_children() from int to void as
> it always returns 0. Remove its return statement as well.
> At call site, remove the variable that was used to store the return
> value, as well as the check on the return value.
> 

You don't need to describe each and everything as it is obvious
from code. How about?
"Change return type of sysc_check_children() to "void"
as it never returns error"

Should both patches can be squashed into one patch?

> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
> - This is a new patch; labelled v3 only because it is in the same series
>   as the previous patch.
> 
>  drivers/bus/ti-sysc.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> index 9c6d3e121d37..a2eae8f36ef8 100644
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -628,14 +628,12 @@ static void sysc_check_one_child(struct sysc *ddata,
>  	sysc_parse_dts_quirks(ddata, np, true);
>  }
>  
> -static int sysc_check_children(struct sysc *ddata)
> +static void sysc_check_children(struct sysc *ddata)
>  {
>  	struct device_node *child;
>  
>  	for_each_child_of_node(ddata->dev->of_node, child)
>  		sysc_check_one_child(ddata, child);
> -
> -	return 0;
>  }
>  
>  /*
> @@ -788,9 +786,7 @@ static int sysc_map_and_check_registers(struct sysc *ddata)
>  	if (error)
>  		return error;
>  
> -	error = sysc_check_children(ddata);
> -	if (error)
> -		return error;
> +	sysc_check_children(ddata);
>  
>  	error = sysc_parse_registers(ddata);
>  	if (error)
> 

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH v3 2/2] bus: ti-sysc: sysc_check_children(): Change return type to void

[Tony Lindgren]

* Roger Quadros <rogerq@ti.com> [190813 11:14]:
> 
> On 13/08/2019 10:55, Nishka Dasgupta wrote:
> > Change return type of function sysc_check_children() from int to void as
> > it always returns 0. Remove its return statement as well.
> > At call site, remove the variable that was used to store the return
> > value, as well as the check on the return value.
> > 
> 
> You don't need to describe each and everything as it is obvious
> from code. How about?
> "Change return type of sysc_check_children() to "void"
> as it never returns error"
> 
> Should both patches can be squashed into one patch?

Sure why not, makes it easier to follow :)

Regards,

Tony

[PATCH v4] bus: ti-sysc: Change return types of functions

[Nishka Dasgupta]

Change return type of functions sysc_check_one_child() and
sysc_check_children() from int to void as neither ever returns an error.
Modify call sites of both functions accordingly.

Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
Changes in v4:
- Merge into a single patch the two patches for sysc_check_one_child()
  and sysc_check_children().
Changes in v3:
- Add patch for sysc_check_children().
- Remove return statement in sysc_check_one_child().
- Remove braces at call site.
Changes in v2:
- Remove error variable entirely.
- Change return type of sysc_check_one_child().

 drivers/bus/ti-sysc.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
index e6deabd8305d..a2eae8f36ef8 100644
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -615,8 +615,8 @@ static void sysc_check_quirk_stdout(struct sysc *ddata,
  * node but children have "ti,hwmods". These belong to the interconnect
  * target node and are managed by this driver.
  */
-static int sysc_check_one_child(struct sysc *ddata,
-				struct device_node *np)
+static void sysc_check_one_child(struct sysc *ddata,
+				 struct device_node *np)
 {
 	const char *name;
 
@@ -626,22 +626,14 @@ static int sysc_check_one_child(struct sysc *ddata,
 
 	sysc_check_quirk_stdout(ddata, np);
 	sysc_parse_dts_quirks(ddata, np, true);
-
-	return 0;
 }
 
-static int sysc_check_children(struct sysc *ddata)
+static void sysc_check_children(struct sysc *ddata)
 {
 	struct device_node *child;
-	int error;
-
-	for_each_child_of_node(ddata->dev->of_node, child) {
-		error = sysc_check_one_child(ddata, child);
-		if (error)
-			return error;
-	}
 
-	return 0;
+	for_each_child_of_node(ddata->dev->of_node, child)
+		sysc_check_one_child(ddata, child);
 }
 
 /*
@@ -794,9 +786,7 @@ static int sysc_map_and_check_registers(struct sysc *ddata)
 	if (error)
 		return error;
 
-	error = sysc_check_children(ddata);
-	if (error)
-		return error;
+	sysc_check_children(ddata);
 
 	error = sysc_parse_registers(ddata);
 	if (error)
-- 
2.19.1

Re: [PATCH v4] bus: ti-sysc: Change return types of functions

[Roger Quadros]

On 15/08/2019 08:46, Nishka Dasgupta wrote:
> Change return type of functions sysc_check_one_child() and
> sysc_check_children() from int to void as neither ever returns an error.
> Modify call sites of both functions accordingly.
> 
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>

Acked-by: Roger Quadros <rogerq@ti.com>

cheers,
-roger

> ---
> Changes in v4:
> - Merge into a single patch the two patches for sysc_check_one_child()
>   and sysc_check_children().
> Changes in v3:
> - Add patch for sysc_check_children().
> - Remove return statement in sysc_check_one_child().
> - Remove braces at call site.
> Changes in v2:
> - Remove error variable entirely.
> - Change return type of sysc_check_one_child().
> 
>  drivers/bus/ti-sysc.c | 22 ++++++----------------
>  1 file changed, 6 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> index e6deabd8305d..a2eae8f36ef8 100644
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -615,8 +615,8 @@ static void sysc_check_quirk_stdout(struct sysc *ddata,
>   * node but children have "ti,hwmods". These belong to the interconnect
>   * target node and are managed by this driver.
>   */
> -static int sysc_check_one_child(struct sysc *ddata,
> -				struct device_node *np)
> +static void sysc_check_one_child(struct sysc *ddata,
> +				 struct device_node *np)
>  {
>  	const char *name;
>  
> @@ -626,22 +626,14 @@ static int sysc_check_one_child(struct sysc *ddata,
>  
>  	sysc_check_quirk_stdout(ddata, np);
>  	sysc_parse_dts_quirks(ddata, np, true);
> -
> -	return 0;
>  }
>  
> -static int sysc_check_children(struct sysc *ddata)
> +static void sysc_check_children(struct sysc *ddata)
>  {
>  	struct device_node *child;
> -	int error;
> -
> -	for_each_child_of_node(ddata->dev->of_node, child) {
> -		error = sysc_check_one_child(ddata, child);
> -		if (error)
> -			return error;
> -	}
>  
> -	return 0;
> +	for_each_child_of_node(ddata->dev->of_node, child)
> +		sysc_check_one_child(ddata, child);
>  }
>  
>  /*
> @@ -794,9 +786,7 @@ static int sysc_map_and_check_registers(struct sysc *ddata)
>  	if (error)
>  		return error;
>  
> -	error = sysc_check_children(ddata);
> -	if (error)
> -		return error;
> +	sysc_check_children(ddata);
>  
>  	error = sysc_parse_registers(ddata);
>  	if (error)
> 

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH v4] bus: ti-sysc: Change return types of functions

[Tony Lindgren]

* Roger Quadros <rogerq@ti.com> [190815 13:03]:
> On 15/08/2019 08:46, Nishka Dasgupta wrote:
> > Change return type of functions sysc_check_one_child() and
> > sysc_check_children() from int to void as neither ever returns an error.
> > Modify call sites of both functions accordingly.
> > 
> > Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> 
> Acked-by: Roger Quadros <rogerq@ti.com>

Applying into omap-for-v5.4/ti-sysc thanks.

Tony