On 5/30/24 14:21, Laurent Pinchart wrote:
On Thu, May 30, 2024 at 02:18:05PM -0600, Shuah Khan wrote:
On 5/30/24 13:45, Laurent Pinchart wrote:
On Thu, May 30, 2024 at 01:27:53PM -0600, Shuah Khan wrote:
On 4/24/24 17:57, Laurent Pinchart wrote:
The .init_state() operations of the debayer and sensor entities iterate
over the entity's pads. In practice, the iteration covers a single pad
only. Access the pad directly and remove the loops.
I am not seeing much of a reason to do this. This code is good
even when num_pads change.
Don't change the loops.
Why so ? Beside the fact that the loop wastes some CPU cycles, the
current code implies that there would be multiple source pads, which is
confusing for the reader. I think the result of this patch is both
improved efficiency and improved readability.
It is currently flexible and if and when more pads get added,
there is no need to change it. I am not concerned about the
efficiency on this test driver. Also people use the test
driver as a sample.
If pad gets added, we don't know yet if the code will work as-is.
If it doesn't that would be part of the work to add more pads? Did
you test with more than one pad t say for sure that it doesn't work?
Chances are it will need to change anyway. I don't think it's a good
idea to prepare for a situation that we can't foresee without having
good reasons to make assumptions>
I have plans to refactor the vimc driver exteensively, changing how the
different entities behave, to bring it closer to how a real inline ISP
is architectured. The .init_state() functions will likely be rewritten
completely.
You can change that at that time if need be.
I agree with the sample argument, and that's one more reason why I think
this patch does the right thing :-)
I am not sure about that.
Please leave it the way it is.
thanks,
-- Shuah
