linux-kernel - Re: [PATCH 13/13] media: qcom: camss: Add support for VFE hardware version Titan 780

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2de0b7a8-b879-49e9-9656-ec86f29ce559@quicinc.com>
Date: Wed, 14 Aug 2024 21:10:29 +0800
From: Depeng Shao <quic_depengs@...cinc.com>
To: Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>, <rfoss@...nel.org>,
        <todor.too@...il.com>, <bryan.odonoghue@...aro.org>,
        <mchehab@...nel.org>, <robh@...nel.org>, <krzk+dt@...nel.org>,
        <conor+dt@...nel.org>
CC: <linux-arm-msm@...r.kernel.org>, <linux-media@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <kernel@...cinc.com>, Yongsheng Li <quic_yon@...cinc.com>
Subject: Re: [PATCH 13/13] media: qcom: camss: Add support for VFE hardware
 version Titan 780

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@kernel.org/

https://lore.kernel.org/all/d0f8b72d-4355-43cd-a5f9-c44aab8147e5@kernel.org/

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