* [PATCH] bus: ti-sysc: Remove if-block in sysc_check_children()
@ 2019-08-08 7:40 Nishka Dasgupta
2019-08-08 13:55 ` Roger Quadros
0 siblings, 1 reply; 6+ messages in thread
From: Nishka Dasgupta @ 2019-08-08 7:40 UTC (permalink / raw)
To: tony, rogerq, linux-kernel; +Cc: Nishka Dasgupta
In function sysc_check_children, there is an if-statement checking
whether the value returned by function sysc_check_one_child is non-zero.
However, sysc_check_one_child always returns 0, and hence this check is
not needed. Hence remove this if-block.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
drivers/bus/ti-sysc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
index e6deabd8305d..bc8082ae7cb5 100644
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -637,8 +637,6 @@ static int sysc_check_children(struct sysc *ddata)
for_each_child_of_node(ddata->dev->of_node, child) {
error = sysc_check_one_child(ddata, child);
- if (error)
- return error;
}
return 0;
--
2.19.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] bus: ti-sysc: Remove if-block in sysc_check_children()
2019-08-08 7:40 [PATCH] bus: ti-sysc: Remove if-block in sysc_check_children() Nishka Dasgupta
@ 2019-08-08 13:55 ` Roger Quadros
2019-08-13 4:35 ` Nishka Dasgupta
0 siblings, 1 reply; 6+ messages in thread
From: Roger Quadros @ 2019-08-08 13:55 UTC (permalink / raw)
To: Nishka Dasgupta, tony, linux-kernel, Kristo, Tero
Nishka,
On 08/08/2019 10:40, Nishka Dasgupta wrote:
> In function sysc_check_children, there is an if-statement checking
> whether the value returned by function sysc_check_one_child is non-zero.
> However, sysc_check_one_child always returns 0, and hence this check is
> not needed. Hence remove this if-block.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
> drivers/bus/ti-sysc.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> index e6deabd8305d..bc8082ae7cb5 100644
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -637,8 +637,6 @@ static int sysc_check_children(struct sysc *ddata)
>
> for_each_child_of_node(ddata->dev->of_node, child) {
> error = sysc_check_one_child(ddata, child);
> - if (error)
> - return error;
We cannot assume that sysc_check_one_child() will never return error in the future.
If it can never return an error then why does it have an int return type?
> }
>
> return 0;
>
cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bus: ti-sysc: Remove if-block in sysc_check_children()
2019-08-08 13:55 ` Roger Quadros
@ 2019-08-13 4:35 ` Nishka Dasgupta
2019-08-13 6:25 ` Roger Quadros
0 siblings, 1 reply; 6+ messages in thread
From: Nishka Dasgupta @ 2019-08-13 4:35 UTC (permalink / raw)
To: Roger Quadros, tony, linux-kernel, Kristo, Tero
On 08/08/19 7:25 PM, Roger Quadros wrote:
> Nishka,
>
> On 08/08/2019 10:40, Nishka Dasgupta wrote:
>> In function sysc_check_children, there is an if-statement checking
>> whether the value returned by function sysc_check_one_child is non-zero.
>> However, sysc_check_one_child always returns 0, and hence this check is
>> not needed. Hence remove this if-block.
>>
>> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
>> ---
>> drivers/bus/ti-sysc.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
>> index e6deabd8305d..bc8082ae7cb5 100644
>> --- a/drivers/bus/ti-sysc.c
>> +++ b/drivers/bus/ti-sysc.c
>> @@ -637,8 +637,6 @@ static int sysc_check_children(struct sysc *ddata)
>>
>> for_each_child_of_node(ddata->dev->of_node, child) {
>> error = sysc_check_one_child(ddata, child);
>> - if (error)
>> - return error;
>
> We cannot assume that sysc_check_one_child() will never return error in the future.
> If it can never return an error then why does it have an int return type?
I'm not sure why it has an int return type, unfortunately. This is the
function in its entirety:
static int sysc_check_one_child(struct sysc *ddata,
struct device_node *np)
{
const char *name;
name = of_get_property(np, "ti,hwmods", NULL);
if (name)
dev_warn(ddata->dev, "really a child ti,hwmods property?");
sysc_check_quirk_stdout(ddata, np);
sysc_parse_dts_quirks(ddata, np, true);
return 0;
}
I'm not sure how to understand this function. Do dev_warn() or
sysc_check_quirk_stdout() or sysc_parse_dts_quirks() cause a non-zero
return from sysc_check_one_child()? Should I drop my patch?
Thanking you,
Nishka
>
>> }
>>
>> return 0;
>>
>
> cheers,
> -roger
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bus: ti-sysc: Remove if-block in sysc_check_children()
2019-08-13 4:35 ` Nishka Dasgupta
@ 2019-08-13 6:25 ` Roger Quadros
2019-08-13 6:28 ` Tony Lindgren
2019-08-13 7:12 ` Nishka Dasgupta
0 siblings, 2 replies; 6+ messages in thread
From: Roger Quadros @ 2019-08-13 6:25 UTC (permalink / raw)
To: Nishka Dasgupta, tony, linux-kernel, Kristo, Tero
On 13/08/2019 07:35, Nishka Dasgupta wrote:
> On 08/08/19 7:25 PM, Roger Quadros wrote:
>> Nishka,
>>
>> On 08/08/2019 10:40, Nishka Dasgupta wrote:
>>> In function sysc_check_children, there is an if-statement checking
>>> whether the value returned by function sysc_check_one_child is non-zero.
>>> However, sysc_check_one_child always returns 0, and hence this check is
>>> not needed. Hence remove this if-block.
>>>
>>> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
>>> ---
>>> drivers/bus/ti-sysc.c | 2 --
>>> 1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
>>> index e6deabd8305d..bc8082ae7cb5 100644
>>> --- a/drivers/bus/ti-sysc.c
>>> +++ b/drivers/bus/ti-sysc.c
>>> @@ -637,8 +637,6 @@ static int sysc_check_children(struct sysc *ddata)
>>> for_each_child_of_node(ddata->dev->of_node, child) {
>>> error = sysc_check_one_child(ddata, child);
>>> - if (error)
>>> - return error;
>>
>> We cannot assume that sysc_check_one_child() will never return error in the future.
>> If it can never return an error then why does it have an int return type?
>
> I'm not sure why it has an int return type, unfortunately. This is the function in its entirety:
>
> static int sysc_check_one_child(struct sysc *ddata,
> struct device_node *np)
> {
> const char *name;
>
> name = of_get_property(np, "ti,hwmods", NULL);
> if (name)
> dev_warn(ddata->dev, "really a child ti,hwmods property?");
>
> sysc_check_quirk_stdout(ddata, np);
> sysc_parse_dts_quirks(ddata, np, true);
>
> return 0;
> }
>
> I'm not sure how to understand this function. Do dev_warn() or sysc_check_quirk_stdout() or sysc_parse_dts_quirks() cause a non-zero return from sysc_check_one_child()? Should I drop my patch?
None of those functions return anything.
Maybe you can fix sysc_check_one_child() to return void?
I think you can retain your patch but get rid of error variable.
>
> Thanking you,
> Nishka
>>
>>> }
>>> return 0;
>>>
>>
--
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bus: ti-sysc: Remove if-block in sysc_check_children()
2019-08-13 6:25 ` Roger Quadros
@ 2019-08-13 6:28 ` Tony Lindgren
2019-08-13 7:12 ` Nishka Dasgupta
1 sibling, 0 replies; 6+ messages in thread
From: Tony Lindgren @ 2019-08-13 6:28 UTC (permalink / raw)
To: Roger Quadros; +Cc: Nishka Dasgupta, linux-kernel, Kristo, Tero
* Roger Quadros <rogerq@ti.com> [190813 06:26]:
> None of those functions return anything.
> Maybe you can fix sysc_check_one_child() to return void?
> I think you can retain your patch but get rid of error variable.
Makes sense to me.
Regards,
Tony
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bus: ti-sysc: Remove if-block in sysc_check_children()
2019-08-13 6:25 ` Roger Quadros
2019-08-13 6:28 ` Tony Lindgren
@ 2019-08-13 7:12 ` Nishka Dasgupta
1 sibling, 0 replies; 6+ messages in thread
From: Nishka Dasgupta @ 2019-08-13 7:12 UTC (permalink / raw)
To: Roger Quadros, tony, linux-kernel, Kristo, Tero
On 13/08/19 11:55 AM, Roger Quadros wrote:
>
>
> On 13/08/2019 07:35, Nishka Dasgupta wrote:
>> On 08/08/19 7:25 PM, Roger Quadros wrote:
>>> Nishka,
>>>
>>> On 08/08/2019 10:40, Nishka Dasgupta wrote:
>>>> In function sysc_check_children, there is an if-statement checking
>>>> whether the value returned by function sysc_check_one_child is non-zero.
>>>> However, sysc_check_one_child always returns 0, and hence this check is
>>>> not needed. Hence remove this if-block.
>>>>
>>>> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
>>>> ---
>>>> drivers/bus/ti-sysc.c | 2 --
>>>> 1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
>>>> index e6deabd8305d..bc8082ae7cb5 100644
>>>> --- a/drivers/bus/ti-sysc.c
>>>> +++ b/drivers/bus/ti-sysc.c
>>>> @@ -637,8 +637,6 @@ static int sysc_check_children(struct sysc *ddata)
>>>> for_each_child_of_node(ddata->dev->of_node, child) {
>>>> error = sysc_check_one_child(ddata, child);
>>>> - if (error)
>>>> - return error;
>>>
>>> We cannot assume that sysc_check_one_child() will never return error in the future.
>>> If it can never return an error then why does it have an int return type?
>>
>> I'm not sure why it has an int return type, unfortunately. This is the function in its entirety:
>>
>> static int sysc_check_one_child(struct sysc *ddata,
>> struct device_node *np)
>> {
>> const char *name;
>>
>> name = of_get_property(np, "ti,hwmods", NULL);
>> if (name)
>> dev_warn(ddata->dev, "really a child ti,hwmods property?");
>>
>> sysc_check_quirk_stdout(ddata, np);
>> sysc_parse_dts_quirks(ddata, np, true);
>>
>> return 0;
>> }
>>
>> I'm not sure how to understand this function. Do dev_warn() or sysc_check_quirk_stdout() or sysc_parse_dts_quirks() cause a non-zero return from sysc_check_one_child()? Should I drop my patch?
>
> None of those functions return anything.
> Maybe you can fix sysc_check_one_child() to return void?
> I think you can retain your patch but get rid of error variable.
Okay, I'll fixup and send v2. What about sysc_check_children()? If error
variable is removed then sysc_check_children also only returns 0.
Thanking you,
Nishka
>
>>
>> Thanking you,
>> Nishka
>>>
>>>> }
>>>> return 0;
>>>>
>>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-13 7:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-08 7:40 [PATCH] bus: ti-sysc: Remove if-block in sysc_check_children() Nishka Dasgupta
2019-08-08 13:55 ` Roger Quadros
2019-08-13 4:35 ` Nishka Dasgupta
2019-08-13 6:25 ` Roger Quadros
2019-08-13 6:28 ` Tony Lindgren
2019-08-13 7:12 ` Nishka Dasgupta
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).