Quoting Jordan Crouse (2018-11-02 10:07:22)
> 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);
Definitely, that looks awful. I'm not suggesting using IS_ERR_OR_NULL()
here.
>
> 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.
>
It may be that the llcc code needs to get an 'optional' API that lets
you get the slice and not care if it isn't specified in DT. Then when
the API isn't compiled in it's not an error, and when it's not specified
in DT it isn't an error either, it's just a NULL pointer. You may want
to if (IS_ENABLED()) the local things done in your driver based on the
ifdef for LLCC itself too, or perhaps make it only do something if the
cache size is non-zero, but the slice grabbing part could be generically
done in driver probe and if that returned NULL you could make your local
code skip whatever it does with the pointer. So then errors are blindly
treated as probe errors and NULL pointers mean it's not there in DT or
not compile in.
