From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Thu, 13 May 2021 11:00:12 -0600 Subject: [PATCH 2/2] reset: Let reset API's handle gracefully if reset_ctl is -ENODATA In-Reply-To: <06f50698-bba0-13b3-f69e-c724fc6f6015@ti.com> References: <20210507110202.13697-1-kishon@ti.com> <20210507110202.13697-3-kishon@ti.com> <06f50698-bba0-13b3-f69e-c724fc6f6015@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 Thu, 13 May 2021 at 00:30, 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: > >> > >> Let reset API's (like reset_assert(), reset_deassert(), ..) handle > >> gracefully if the argument reset_ctl is -ENODATA. This is for seamlessly > >> handling client drivers which get optional reset controller using > >> devm_reset_control_get_optional(). > >> > >> Signed-off-by: Kishon Vijay Abraham I > >> --- > >> drivers/reset/reset-uclass.c | 35 +++++++++++++++++++++++++++++------ > >> 1 file changed, 29 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c > >> index 906f58ce3d..077ca956f4 100644 > >> --- a/drivers/reset/reset-uclass.c > >> +++ b/drivers/reset/reset-uclass.c > >> @@ -162,7 +162,12 @@ int reset_get_by_name(struct udevice *dev, const char *name, > >> > >> int reset_request(struct reset_ctl *reset_ctl) > >> { > >> - struct reset_ops *ops = reset_dev_ops(reset_ctl->dev); > >> + struct reset_ops *ops; > >> + > >> + if (IS_ERR(reset_ctl) && PTR_ERR(reset_ctl) == -ENODATA) > >> + return 0; > > > > This should call reset_valid(), not do things here. Also I thought we > > came to the conclusion that reset_ctl would always be a valid pointer? > > > > In any case, if you change the logic of these function, you must also > > change the header, otherwise there is no docs. > > > > Finally, can you please update the tests? > > Patch 1 updates sandbox_reset_test_get_devm() in sandbox-reset-test.c. > That's the only test I saw was using optional reset APIs. Do you think > any thing else needs to be updated? My point is that if you are changing the logic of reset_request(), you should add a test for it and you need to update the docs. Do you have someone at TI that can work through this with you? I am not the maintainer of this subsystem, nor the expert here, but I feel it could use a review with someone else as well, to really nut out all the issues once and for all. Regards, Simon