From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Tue, 11 May 2021 10:39:01 -0600 Subject: [PATCH 1/2] reset: Do not return NULL on error for devm_reset_control_get_optional() In-Reply-To: References: <20210507110202.13697-1-kishon@ti.com> <20210507110202.13697-2-kishon@ti.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Kishon, On Tue, 11 May 2021 at 00:26, Kishon Vijay Abraham I wrote: > > Hi Simon, > > On 07/05/21 10:04 pm, Simon Glass wrote: > > Hi Kishon, > > > > On Fri, 7 May 2021 at 05:02, Kishon Vijay Abraham I wrote: > >> > >> In order for client to know whether it was able to successfully get a > >> reset controller or not, do not return NULL on error for > >> devm_reset_control_get_optional() and > >> devm_reset_bulk_get_optional_by_node(). > >> > >> Signed-off-by: Kishon Vijay Abraham I > >> --- > >> drivers/reset/reset-uclass.c | 16 ++-------------- > >> drivers/reset/sandbox-reset-test.c | 2 +- > >> 2 files changed, 3 insertions(+), 15 deletions(-) > >> > >> diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c > >> index ac89eaf098..906f58ce3d 100644 > >> --- a/drivers/reset/reset-uclass.c > >> +++ b/drivers/reset/reset-uclass.c > >> @@ -299,12 +299,7 @@ struct reset_ctl *devm_reset_control_get(struct udevice *dev, const char *id) > >> struct reset_ctl *devm_reset_control_get_optional(struct udevice *dev, > >> const char *id) > >> { > >> - struct reset_ctl *r = devm_reset_control_get(dev, id); > >> - > >> - if (IS_ERR(r)) > >> - return NULL; > >> - > >> - return r; > >> + return devm_reset_control_get(dev, id); > >> } > > > > We need to get some updates to the API (function comments in the > > header) here. I'm not sure what the intent is. > > right, that has to be fixed. > > > > I thought these functions were going to return a valid (but possibly > > empty) reset_ctl always? > > I thought about it again and felt it might not be correct to return > reset_ctl always. The reset control is optional only if the consumer > node doesn't have populated reset properties. > > If we always return valid reset_ctl possibly with it's dev member > initialized or not initialized, it would not be possible to tell it's > not initialized because of the absence of reset property or due to some > other valid error. reset_valid() should check if dev is NULL or not, like with clock and GPIO. Is that enough? Regards, Simon