Hi Simon,
On 11/05/21 10:09 pm, Simon Glass wrote:
Hi Kishon,
On Tue, 11 May 2021 at 00:26, Kishon Vijay Abraham I kishon@ti.com 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 kishon@ti.com 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 kishon@ti.com
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?
clock compares with ENODATA and return "0" (code snippet below). For reset we are modifying this to return ENODATA itself and use it for comparing in the reset APIs.
int clk_get_optional_nodev(ofnode node, const char *name, struct clk *clk) { int ret;
ret = clk_get_by_name_nodev(node, name, clk); if (ret == -ENODATA) return 0;
return ret; }
I can add reset_valid() and add the conditional checks corresponding to the changes made for reset (like return -ENODATA instead of 0).
Thanks Kishon