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

From: Bryan O'Donoghue
Date: Mon May 13 2024 - 13:48:44 EST


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