Hi Vladimir,
On 8/24/2024 9:06 PM, Vladimir Zapolskiy wrote:
+
+/*
+ * vfe_enable_v2 - Enable streaming on VFE line
+ * @line: VFE line
+ *
+ * Return 0 on success or a negative error code otherwise
+ */
+int vfe_enable_v2(struct vfe_line *line)
+{
+ struct vfe_device *vfe = to_vfe(line);
+ int ret;
+
+ mutex_lock(&vfe->stream_lock);
+
+ if (vfe->res->hw_ops->enable_irq)
+ vfe->res->hw_ops->enable_irq(vfe);
Right so generally speaking I don't believe we should have any null
function pointers.
We just mandate that to be comitted, an impelmentation must provide a
dummy but, in this case when do we ever want a dummy function anyway
surely enable_irq() is a fundamental operation that is core to the logic.
Why? What could be a justification here?
The image capturing media pipeline for all recent Qualcomm SoCs, including
this one in the series for SM8550, can be set up and enabled without
touching VFE interrupts at all.
It might be extremely confusing to see in the code that some not ever
requested interrupts are enabled/disabled, and then to discover that just
some stubs around VFE interrupts are added. And it's the case especially
in this new vfe_enable_v2() function, which I believe is intended for
CAMSS support on new platforms.
What's worse, since these VFE interrupts are not needed on the modern
platforms, it will require to add a proposed dummy "return 0" function
into any CAMSS support for new platforms forever. I believe it'd be better
to clearly say that it's a legacy to have an obligatory support of VFE
interrupts.
Sure, I will add a proposed dummy "return 0" function for these interfaces.
Thanks,
Depeng
