Hi Bryan,
On 12/11/2024 11:44 PM, Bryan O'Donoghue wrote:
On 11/12/2024 14:07, Depeng Shao wrote:
There is no CSID TPG in some modern HW, so the v4l2 ctrl in CSID driver
"some modern HW" => "on some SoCs"
shouldn't be registered. Checking the supported TPG modes to indicate
if the TPG HW is existing or not, and only register v4l2 ctrl for CSID
"TP HW is existing or not, and only register" => "TPG hardware exists or
not and oly registering"
No need to abbreviate hardware to HW.
only when TPG HW is existing.
"when the TPG hardware exists" you could also say "is present" instead
of "exists".
You have additional whitespace in your log before " only"
Thanks for the suggestion, will update in new version.
}
if (!csid->testgen.enabled &&
@@ -838,7 +840,8 @@ static void csid_try_format(struct csid_device *csid,
break;
case MSM_CSID_PAD_SRC:
- if (csid->testgen_mode->cur.val == 0) {
+ if (csid->testgen.nmodes == CSID_PAYLOAD_MODE_DISABLED ||
if (csid->ctrls ||
feels like a more natural test. Less cumbersome and also less typing.
I think that change should be feasible, could you please update your
logic from if (csid->testgen.nmodes == CSID_PAYLOAD_MODE_DISABLED) to if
(csid->ctrls)
Otherwise looks good but, I'll wait to see your next version before
giving an RB.
csid->ctrls is not a pointer, so it is always true.
Thanks,
Depeng
