Hi,
Thank you for reviewing this series.
On 9/1/22 21:56, Andy Shevchenko wrote:
> On Thu, Sep 01, 2022 at 11:46:22AM +0200, Hans de Goede wrote:
>> Register /dev/* nodes at the end of atomisp_pci_probe(), this is
>> a prerequisite for dropping the loading mutex + ready flag kludge
>> for delaying open() calls on the /dev/* nodes .
>
> ...
>
>> for (; i > 0; i--)
>> atomisp_subdev_unregister_entities(
>> &isp->asd[i - 1]);
>
> This...
I presume you mean the few lines above that actually:
@@ -1194,11 +1194,8 @@ static int atomisp_register_entities(struct atomisp_device *isp)
struct atomisp_sub_device *asd = &isp->asd[i];
ret = atomisp_subdev_register_subdev(asd, &isp->v4l2_dev);
- if (ret == 0)
- ret = atomisp_subdev_register_video_nodes(asd, &isp->v4l2_dev);
if (ret < 0) {
- dev_err(isp->dev,
- "atomisp_subdev_register_entities fail\n");
+ dev_err(isp->dev, "atomisp_subdev_register_subdev fail\n");
for (; i > 0; i--)
atomisp_subdev_unregister_entities(
&isp->asd[i - 1]);
Notice the atomisp_subdev_register_video_nodes() call is removed there,
leaving only atomisp_subdev_register_subdev() (which is also why the dev_err
msg is changed).
Actually moving that call (+ a few others) to later is the whole
purpose of this patch.
>
>> + for (i = 0; i < isp->num_of_streams; i++) {
>> + err = atomisp_subdev_register_video_nodes(&isp->asd[i], &isp->v4l2_dev);
>> + if (err)
>> + return err;
>> + }
>
> ...and this looks like a dup.
Where as this time while looping over the stream the code is calling
atomisp_subdev_register_video_nodes().
So yes we have 2 "for (i = 0; i < isp->num_of_streams; i++) {}"
loops, but:
Loop 1 does: atomisp_subdev_register_subdev()
Loop 2 does: atomisp_subdev_register_video_nodes()
So I see no duplication here?
Regards,
Hans
