> On Wed, Jun 01, 2022 at 06:25:01AM +0000, Avri Altman wrote: > > > Smatch complains that the if (flag_res) is not required: > > > > > > drivers/ufs/core/ufshpb.c:2306 ufshpb_check_hpb_reset_query() > > > warn: duplicate check 'flag_res' (previous on line 2301) > > > > > > Re-write the "if (flag_res)" checking to be more clear. > > > > > > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > In HPB Reset, the Host set this flag as '1' to inform the device that host > reset its L2P data. > > The Device resets flag as '0' when the device inactivated all region > information. > > 0h: HPB reset completed or not started yet. > > 1h: HPB reset in progress. > > > > Would make sense to me to contain this logic within this function, > > Instead of returning just the flag value. > > > > Thanks, > > Avri > > I am not sure I understand. > > To be honest, this function is not beautiful at all. With boolean functions, the > name should tell you what the return means. Examples > are: if (!access_ok()), if (IS_ERR() etc. In this case the return is not clear from > the name. Right. > > The second thing is that I really don't like returning true for failure and > returning false for success. Returning zero and negatives is good but with > true/false it should be true == success. Exactly. > > So, yes, I wasn't super happy with this function either. But I just did a > minimal clean up to make the returns more clear. If you want to drop this > patch and write a more extensive one then I would be really happy about > that. Yeah - I will. Thanks, Avri > > regards, > dan carpenter
