Hi Vladimir,
On 8/14/2024 7:13 PM, Vladimir Zapolskiy wrote:
Hi Depeng,
please find a few review comments, all asked changes are non-functional.
+void camss_reg_update(struct camss *camss, int hw_id, int port_id,
bool is_clear)
Please let it be just a declarative 'clear' instead of questioning
'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);
+ }
+}
+
Please add the new exported function camss_reg_update() in a separate
preceding commit.
void camss_buf_done(struct camss *camss, int hw_id, int port_id)
{
struct vfe_device *vfe;
Thanks for your comments, I will address them in new series.
But I have some concern about above comment, you want to add a separate
commit for camss_reg_update, maybe camss_buf_done also need to do this,
but I guess I will get new comments from Krzysztof if I make a separate
change, Krzysztof posted few comments in v3 series, he asked, "must
organize your patches in logical junks" and the code must have a user.
Please check below comments.
https://lore.kernel.org/all/e1b298df-05da-4881-a628-149a8a625544@xxxxxxxxxx/
https://lore.kernel.org/all/d0f8b72d-4355-43cd-a5f9-c44aab8147e5@xxxxxxxxxx/
Or I don't add reg update and buf done functionality in
camss-csid-gen3.c and camss-vfe-780.c firstly, then add them in a later
commit.
Could you please comment on whether this is acceptable? Please also help
to common on if one commit to add them or need two separate commits, one
is for reg update and the other one is for buf done.
Thanks,
Depeng
