Re: [PATCH v3 04/29] media: iris: initialize power resources

[Krzysztof Kozlowski]

On 06/09/2024 21:47, Vikash Garodia wrote:
> Hi,
> 
> On 9/6/2024 5:34 PM, Krzysztof Kozlowski wrote:
>> On 06/09/2024 13:21, Vikash Garodia wrote:
>>>>>>
>>>>>>> +	}
>>>>>>> +
>>>>>>>  	ret = v4l2_device_register(dev, &core->v4l2_dev);
>>>>>>>  	if (ret)
>>>>>>>  		return ret;
>>>>>>> @@ -88,8 +101,14 @@ static int iris_probe(struct platform_device *pdev)
>>>>>>>  }
>>>>>>>  
>>>>>>>  static const struct of_device_id iris_dt_match[] = {
>>>>>>> -	{ .compatible = "qcom,sm8550-iris", },
>>>>>>> -	{ .compatible = "qcom,sm8250-venus", },
>>>>>>> +	{
>>>>>>> +		.compatible = "qcom,sm8550-iris",
>>>>>>> +		.data = &sm8550_data,
>>>>>>> +	},
>>>>>>> +	{
>>>>>>> +		.compatible = "qcom,sm8250-venus",
>>>>>>> +		.data = &sm8250_data,
>>>>>>
>>>>>> You just added this. No, please do not add code which is immediatly
>>>>>> incorrect.
>>>>> It's not incorrect, in earlier patch we only added the compatible strings
>>>>> and with this patch introducing the platform data and APIs to get it.
>>>>
>>>> It is incorrect to immediately remove it. You keep arguing on basic
>>>> stuff. Sorry, but that is not how it works. If you add code and
>>>> IMMEDIATELY remove it, then it means the code was not needed. Or was not
>>>> correct. Choose one.
>>> I think it is not removing it. It is adding platform data to compatibles
>>> introduced in previous patch. Maybe it appears as if it is removing it.
>>
>> I know how the diff works.
> Perhaps, i have misunderstood. Are you suggesting to add compat data and
> compatible string together in single patch rather than splitting it in 2 patches
> ? If so, that would essentially end up squashing patch #3 and #4. Let me know if
> that would address your comment and we will plan to do that.

You are supposed to organize your patches so they have logical order. I
already explained why this order is wrong. What's more, previous patch
of two equal compatibles does not have much value. Devices cannot work
and code is confusing.

Best regards,
Krzysztof