Hi Kishon, On Tue, 4 May 2021 at 21:25, Kishon Vijay Abraham I <kis...@ti.com> wrote: > > Hi Simon, > > On 04/05/21 10:28 pm, Simon Glass wrote: > > Hi Kishon, > > > > On Tue, 4 May 2021 at 04:42, Kishon Vijay Abraham I <kis...@ti.com> wrote: > >> > >> The reset framework provides devm_reset_control_get_optional() > >> which can return NULL (not an error case). So all the other reset_ops > >> should handle NULL gracefully. Prepare the way for a managed reset > >> API by handling NULL pointers without crashing nor failing. > >> > >> Signed-off-by: Vignesh Raghavendra <vigne...@ti.com> > >> Signed-off-by: Kishon Vijay Abraham I <kis...@ti.com> > >> --- > >> drivers/reset/reset-uclass.c | 35 ++++++++++++++++++++++++++++++----- > >> 1 file changed, 30 insertions(+), 5 deletions(-) > > > > I am still not a fan of this. There is no way to know whether the > > reset API call actually did anything. Normally we return -EINVAL or > > something like that when the value is invalid (e.g. see GPIO uclass). > > The reset modification is more in-line with how clk uclass is designed. > > There every API first checks if the clock is valid and returns 0 (will > be the case for optional clocks) > if (!clk_valid(clk)) > return 0; > > If you don't prefer this approach, I'll drop this patch and handle it > appropriately in the client driver (serdes driver). Please let me know.
Yes I see that with clk. IMO the return code is incorrect there also, since it should be -ENOENT or something like that, to indicate that there is actually no clock. The clk.h header gives no indication that it accepts a NULL clock. So here is what I think: 1. Update clk_valid() to require clk to be a valid pointer, so it is considered invalid only if clk-dev. 2. Update the reset 'managed API' to return an empty reset record instead of NULL. 3. Your problem goes away, and (I think) we are still compatible with the linux API. > > > > > The function you mention says: * Returns a struct reset_ctl or a > > dummy reset controller if it failed. > > > > But it does not appear to do that? > > Right, looks like it's incorrect from when the patch was actually > introduced. It was returning "NULL" for optional resets. > > commit 139e4a1cbe60de96d4febbc6db5882929801ca46 > Author: Jean-Jacques Hiblot <jjhib...@ti.com> > Date: Wed Sep 9 15:37:03 2020 +0530 > > drivers: reset: Add a managed API to get reset controllers from the DT OK. So what should it be? Regards, Simon
