Re: [PATCH v3 8/8] media: qcom: camss: Decouple VFE from CSID

[Bryan O'Donoghue]

On 13/05/2024 17:26, Gjorgji Rosikopulos (Consultant) wrote:
> > > +static void __iomem
> > > +*vfe_parent_dev_ops_get_base_address(struct camss *camss, int id)
> > > +{
> > > +    if (id < camss->res->vfe_num) {
> > > +        struct vfe_device *vfe = &camss->vfe[id];
> > > +
> > > +        return vfe->base;
> > > +    }
> > > +
> > > +    return NULL;
> > I can find code snippets above like
> >
> >      if (IS_ERR(csid->base))
> >          ...
> >
> > So, is it really a good idea to return NULL on error? Probably it might
> > be better
> > to return a reasonable error to the caller.
> As general rule i agree. But here either we have address or not,
> i dont see the reason to return an error code. Also i dont see what
> caller will do if he gets error code instead of NULL.
> I am refering in particular this case. If we have different error paths
> of failiure maybe it will more sense.

I don't see a compelling reason to change the submitted code. I'd leave 
well-enough alone for v4.

Please keep changes for V4 restricted to formatting/line indentation/SPDX.

I don't want to have to reverify all of this code unless a bug is found.

---
bod