Re: [PATCH] bus: ti-sysc: Remove if-block in sysc_check_children()

[Nishka Dasgupta]

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@xxxxxxxxx>
> > > > ---
> > > >  Â 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;