2024-03-20 14:12:19

by Depeng Shao

Subject: [PATCH v2 1/8] media: qcom: camss: Add CAMSS_8550 enum

From: Yongsheng Li <[email protected]>

Adds a CAMSS SoC identifier for the sm8550.

Signed-off-by: Yongsheng Li <[email protected]>
---
drivers/media/platform/qcom/camss/camss.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
index ac15fe23a702..2f63206a8463 100644
--- a/drivers/media/platform/qcom/camss/camss.h
+++ b/drivers/media/platform/qcom/camss/camss.h
@@ -78,6 +78,7 @@ enum camss_version {
CAMSS_845,
CAMSS_8250,
CAMSS_8280XP,
+ CAMSS_8550,
};

enum icc_count {
--
2.17.1

2024-03-20 14:50:40

by Bryan O'Donoghue

Subject: Re: [PATCH v2 1/8] media: qcom: camss: Add CAMSS_8550 enum

On 20/03/2024 14:11, Depeng Shao wrote:
> From: Yongsheng Li <[email protected]>
>
> Adds a CAMSS SoC identifier for the sm8550.
>
> Signed-off-by: Yongsheng Li <[email protected]>
> ---
> drivers/media/platform/qcom/camss/camss.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index ac15fe23a702..2f63206a8463 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -78,6 +78,7 @@ enum camss_version {
> CAMSS_845,
> CAMSS_8250,
> CAMSS_8280XP,
> + CAMSS_8550,
> };
>
> enum icc_count {

Acked-by: Bryan O'Donoghue <[email protected]>

2024-03-20 15:52:19

by Krzysztof Kozlowski

Subject: Re: [PATCH v2 1/8] media: qcom: camss: Add CAMSS_8550 enum

On 20/03/2024 15:11, Depeng Shao wrote:
> From: Yongsheng Li <[email protected]>
>
> Adds a CAMSS SoC identifier for the sm8550.

Why?

>
> Signed-off-by: Yongsheng Li <[email protected]>
> ---
> drivers/media/platform/qcom/camss/camss.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index ac15fe23a702..2f63206a8463 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -78,6 +78,7 @@ enum camss_version {
> CAMSS_845,
> CAMSS_8250,
> CAMSS_8280XP,
> + CAMSS_8550,
> };

What is the point of this patch alone? What does it change? Why adding
enum? In the next patch, are you going to add one line to some
kerneldoc? Another patch, one function?

Best regards,
Krzysztof

2024-03-20 15:53:42

by Bryan O'Donoghue

Subject: Re: [PATCH v2 1/8] media: qcom: camss: Add CAMSS_8550 enum

On 20/03/2024 15:51, Krzysztof Kozlowski wrote:
> On 20/03/2024 15:11, Depeng Shao wrote:
>> From: Yongsheng Li <[email protected]>
>>
>> Adds a CAMSS SoC identifier for the sm8550.
>
> Why?
>
>>
>> Signed-off-by: Yongsheng Li <[email protected]>
>> ---
>> drivers/media/platform/qcom/camss/camss.h | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
>> index ac15fe23a702..2f63206a8463 100644
>> --- a/drivers/media/platform/qcom/camss/camss.h
>> +++ b/drivers/media/platform/qcom/camss/camss.h
>> @@ -78,6 +78,7 @@ enum camss_version {
>> CAMSS_845,
>> CAMSS_8250,
>> CAMSS_8280XP,
>> + CAMSS_8550,
>> };
>
> What is the point of this patch alone? What does it change? Why adding
> enum? In the next patch, are you going to add one line to some
> kerneldoc? Another patch, one function?
>
> Best regards,
> Krzysztof
>

Yeah true enough, you could also add this enum where you use it..

---
bod

2024-03-20 15:59:55

by Krzysztof Kozlowski

Subject: Re: [PATCH v2 1/8] media: qcom: camss: Add CAMSS_8550 enum

On 20/03/2024 16:53, Bryan O'Donoghue wrote:
> On 20/03/2024 15:51, Krzysztof Kozlowski wrote:
>> On 20/03/2024 15:11, Depeng Shao wrote:
>>> From: Yongsheng Li <[email protected]>
>>>
>>> Adds a CAMSS SoC identifier for the sm8550.
>>
>> Why?
>>
>>>
>>> Signed-off-by: Yongsheng Li <[email protected]>
>>> ---
>>> drivers/media/platform/qcom/camss/camss.h | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
>>> index ac15fe23a702..2f63206a8463 100644
>>> --- a/drivers/media/platform/qcom/camss/camss.h
>>> +++ b/drivers/media/platform/qcom/camss/camss.h
>>> @@ -78,6 +78,7 @@ enum camss_version {
>>> CAMSS_845,
>>> CAMSS_8250,
>>> CAMSS_8280XP,
>>> + CAMSS_8550,
>>> };
>>
>> What is the point of this patch alone? What does it change? Why adding
>> enum? In the next patch, are you going to add one line to some
>> kerneldoc? Another patch, one function?
>>
>> Best regards,
>> Krzysztof
>>
>
> Yeah true enough, you could also add this enum where you use it..

Just to clarify my comment: this does not mean that entire patchset
should be squashed into one patch, because that would be unmanageable.
But your work should be organized in logical chunks, where some added
code is being used. There is at least one more such change without
immediate users in this patchset...

Best regards,
Krzysztof