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. 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. I agree with the sample argument, and that's one more reason why I think this patch does the right thing :-) > Please leave it the way it is. -- Regards, Laurent Pinchart
