linux-kernel - Re: [PATCH v2 2/2] reset: make optional functions really optional

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1484218448.2376.11.camel@pengutronix.de>
Date:   Thu, 12 Jan 2017 11:54:08 +0100
From:   Philipp Zabel <p.zabel@...gutronix.de>
To:     Ramiro Oliveira <Ramiro.Oliveira@...opsys.com>
Cc:     linux-kernel@...r.kernel.org, laurent.pinchart@...asonboard.com,
        CARLOS.PALMINHA@...opsys.com
Subject: Re: [PATCH v2 2/2] reset: make optional functions really optional

Hi Ramiro,

Am Montag, den 09.01.2017, 17:19 +0000 schrieb Ramiro Oliveira:
> Hi Philipp
> 
> On 1/9/2017 10:45 AM, Philipp Zabel wrote:
> > Hi Ramiro,
> > 
> > Am Dienstag, den 27.12.2016, 12:37 +0000 schrieb Ramiro Oliveira:
> >> The optional functions weren't really optional so this patch makes them
> >> really optional.
> > 
> > Please add a bit of detail to the description. Since this changes the
> > API, you should mention that the reset_control_get_optional variants now
> > return NULL instead of an error if there is no matching reset phandle in
> > the device tree and that the reset_control_* functions accept NULL rstc
> > pointers.
> > 
> 
> Would you be ok with something like this:
> 
> "The *_get_optional_* functions weren't really optional so this patch makes them
> really optional.
> 
> These *_get_optional_* functions will now return NULL instead of an error if no
> matching reset phandle is found in the DT, and all the reset_control_* functions
> now accept NULL rstc pointers

Yes, that looks fine to me.

[...]
> >> @@ -273,13 +295,13 @@ struct reset_control *__of_reset_control_get(struct device_node *node,
> >>  		index = of_property_match_string(node,
> >>  						 "reset-names", id);
> >>  		if (index < 0)
> >> -			return ERR_PTR(-ENOENT);
> >> +			return optional ? NULL : ERR_PTR(-ENOENT);
> > 
> > of_property_match_string can return -EINVAL, -ENODATA, or -EILSEQ.
> > I think -EILSEQ should still be returned.
> > 
> 
> I'll make the function return NULL, -ENOENT, or -EILSEQ.

Ok.

> >>  	}
> >>  
> >>  	ret = of_parse_phandle_with_args(node, "resets", "#reset-cells",
> >>  					 index, &args);
> >>  	if (ret)
> >> -		return ERR_PTR(ret);
> >> +		return optional ? NULL : ERR_PTR(ret);
> > 
> > of_parse_phandle_with_args can return -ENOENT or -EINVAL.
> > I think -EINVAL should still be returned.
> > 
> 
> Same as above. I'll make the function return NULL, -ENOENT, or -EINVAL.

thanks
Philipp