Hi Bryan, Thank you for your comments. On 2023/7/26 18:55, Bryan O'Donoghue wrote: > On 19/06/2023 12:28, Jack Zhu wrote: > >> +static int stfcamss_of_parse_ports(struct stfcamss *stfcamss) >> +{ >> + struct device *dev = stfcamss->dev; >> + struct device_node *node = NULL; >> + int ret, num_subdevs = 0; >> + >> + for_each_endpoint_of_node(dev->of_node, node) { >> + struct stfcamss_async_subdev *csd; >> + >> + if (!of_device_is_available(node)) >> + continue; >> + >> + csd = v4l2_async_nf_add_fwnode_remote(&stfcamss->notifier, >> + of_fwnode_handle(node), >> + struct stfcamss_async_subdev); >> + if (IS_ERR(csd)) { >> + ret = PTR_ERR(csd); >> + goto err_cleanup; >> + } >> + >> + ret = stfcamss_of_parse_endpoint_node(dev, node, csd); >> + if (ret < 0) >> + goto err_cleanup; >> + >> + num_subdevs++; >> + } >> + >> + return num_subdevs; >> + >> +err_cleanup: >> + of_node_put(node); > > Where is the _get() for this and if you are releasing it on the error path when is the _get() released on the non-error path ? > OK, I will fix it. >> + return ret; >> +} >> + >> +static int stfcamss_subdev_notifier_bound(struct v4l2_async_notifier *async, >> + struct v4l2_subdev *subdev, >> + struct v4l2_async_subdev *asd) >> +{ >> + struct media_pad *pad[STF_PADS_NUM]; >> + unsigned int i, pad_num = 0; >> + >> + for (i = 0; i < pad_num; ++i) { > > Does this loop ever run ? > I don't see how it can.. > OK, I will fix it, although there are modifications to this code in patch 5 and patch 6. > --- > bod -- Regards, Jack Zhu