linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Philipp Zabel <p.zabel@pengutronix.de>
To: Ramiro Oliveira <Ramiro.Oliveira@synopsys.com>
Cc: linux-kernel@vger.kernel.org, laurent.pinchart@ideasonboard.com,
	CARLOS.PALMINHA@synopsys.com
Subject: Re: [PATCH v2 2/2] reset: make optional functions really optional
Date: Thu, 12 Jan 2017 11:54:08 +0100	[thread overview]
Message-ID: <1484218448.2376.11.camel@pengutronix.de> (raw)
In-Reply-To: <88f890ef-b16e-a558-3c2f-cfeba107b578@synopsys.com>

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

      reply	other threads:[~2017-01-12 10:55 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-27 12:37 [PATCH v2 0/2] reset: Make optional functions really optional Ramiro Oliveira
2016-12-27 12:37 ` [PATCH v2 1/2] reset: Change shared flag from int to bool Ramiro Oliveira
2016-12-27 12:37 ` [PATCH v2 2/2] reset: make optional functions really optional Ramiro Oliveira
2017-01-09 10:45   ` Philipp Zabel
2017-01-09 17:19     ` Ramiro Oliveira
2017-01-12 10:54       ` Philipp Zabel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1484218448.2376.11.camel@pengutronix.de \
    --to=p.zabel@pengutronix.de \
    --cc=CARLOS.PALMINHA@synopsys.com \
    --cc=Ramiro.Oliveira@synopsys.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).