On Fri, Nov 02, 2018 at 09:50:31AM -0700, Stephen Boyd wrote:
> Quoting Jordan Crouse (2018-10-25 09:38:11)
> > The real llcc_slide_getd() function returns ERR_PTR() encoded errors
> > so the stub function should too.
> >
> > Signed-off-by: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx>
> > ---
> > include/linux/soc/qcom/llcc-qcom.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/soc/qcom/llcc-qcom.h b/include/linux/soc/qcom/llcc-qcom.h
> > index eb71a50b8afc..e9806d548834 100644
> > --- a/include/linux/soc/qcom/llcc-qcom.h
> > +++ b/include/linux/soc/qcom/llcc-qcom.h
> > @@ -171,7 +171,7 @@ int qcom_llcc_remove(struct platform_device *pdev);
> > #else
> > static inline struct llcc_slice_desc *llcc_slice_getd(u32 uid)
> > {
> > - return NULL;
> > + return ERR_PTR(-ENODEV);
>
> Do you want it to be an error if your driver uses this API and doesn't
> get the pointer it was requesting? Typically if a framework isn't
> compiled in, and it isn't essential to the operation of the device, it
> makes sense to NOP out the API by returning NULL instead of an error.
> Then drivers only check for error pointers and treat the NULL pointer as
> a cookie meaning "do nothing".
In my case, llcc is optional so if we get an error we just shrug and move
on. We also have some local stuff to do for llcc so we would have to use an
IF_ERR_OR_NULL instead of an IF_ERR() but I suppose that isn't terrible.
Mostly was looking for consistency. I hate having code that does
if (IS_ERR_OR_NULL(ptr))
pr_err("I got an error %d\n", IS_ERR(ptr) ? PTR_ERR(ptr) : -ESOMETHING);
And since the regular code uses -ENODEV when the slice doesn't exist (which is
the same as the whole thing not existing from the perspective of the driver)
I figured that we could use -ENODEV as the universal sign of no soup for you.
But I'm not angry about it - I can happily use IS_ERR_OR_NULL() if we need to.
Jordan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
