Hi Bryan,
On 8/15/2024 10:21 PM, Depeng Shao wrote:
Hi Bryan,
On 8/15/2024 8:25 AM, Bryan O'Donoghue wrote:
On 12/08/2024 15:41, Depeng Shao wrote:
+void camss_reg_update(struct camss *camss, int hw_id, int port_id,
bool is_clear)
+{
+ struct csid_device *csid;
+
+ if (hw_id < camss->res->csid_num) {
+ csid = &(camss->csid[hw_id]);
+
+ csid->res->hw_ops->reg_update(csid, port_id, is_clear);
+ }
+}
The naming here doesn't make the action clear
hw_ops->rup_update(csid, port, clear);
The register name in SWI is IFE_0_TOP_REG_UPDATE_CMD in SM8250 platform,
and it is CSID0_RUP_AUP_CMD in SM8550, so it isn't only RUP, and AUP is
also updated, so maybe the original name reg_update is better. This is
what VFE 480 driver is using.
"is_clear" is not required since the type is a bool the "is" is
implied in the the logical state so just "clear" will do.
But re: my previous comment on having the ISR do the clear as is done
in the VFE 480, I don't think this is_clear parameter is warranted.
We want the calling function to request the rup_update() for the
rup_update() function to wait on completion and the ISR() to do the
clear once the RUP interrupt has been raised.
At least I think that's how it should work - could you please
experiment with your code for the flow - as it appears to match the
VFE 480 logic.
Thanks for catching this, I forget to add the rup irq, so this logic is
also missed. I have tried it just now, the logic works good, will add it
in next version patch.
I go through the code again, and find we don't do the wait for
completion in VFE 480 driver, this is just used in VFE gen1 driver and
just during disabling port.
Here, what I tried is clearing rup_aup when receiving the rup irq.
Thanks,
Depeng
