On 8/1/2023 2:53 AM, Krzysztof Kozlowski wrote: > On 28/07/2023 15:23, Vikash Garodia wrote: >> From: Dikshita Agarwal <quic_dikshita@xxxxxxxxxxx> >> >> This implements the platform driver methods, file >> operations and v4l2 registration. > > >> + >> + core->debugfs_parent = msm_vidc_devm_debugfs_get(&pdev->dev); >> + if (!core->debugfs_parent) >> + d_vpr_h("Failed to create debugfs for msm_vidc\n"); >> + >> + rc = msm_vidc_devm_init_core(&pdev->dev, core); >> + if (rc) { >> + d_vpr_e("%s: init core failed with %d\n", __func__, rc); >> + goto init_core_failed; >> + } >> + >> + rc = msm_vidc_init_platform(core); >> + if (rc) { >> + d_vpr_e("%s: init platform failed with %d\n", __func__, rc); >> + rc = -EINVAL; >> + goto init_plat_failed; >> + } >> + >> + rc = msm_vidc_init_resources(core); >> + if (rc) { >> + d_vpr_e("%s: init resource failed with %d\n", __func__, rc); >> + goto init_res_failed; > > NAK. > > return dev_err_probe. Sure, will update in next version. > >> + } >> + >> + rc = msm_vidc_init_core_caps(core); >> + if (rc) { >> + d_vpr_e("%s: init core caps failed with %d\n", __func__, rc); >> + goto init_res_failed; >> + } >> + >> + rc = msm_vidc_init_instance_caps(core); >> + if (rc) { >> + d_vpr_e("%s: init inst cap failed with %d\n", __func__, rc); >> + goto init_inst_caps_fail; >> + } >> + >> + core->debugfs_root = msm_vidc_debugfs_init_core(core); >> + if (!core->debugfs_root) >> + d_vpr_h("Failed to init debugfs core\n"); >> + >> + d_vpr_h("populating sub devices\n"); >> + /* >> + * Trigger probe for each sub-device i.e. qcom,msm-vidc,context-bank. >> + * When msm_vidc_probe is called for each sub-device, parse the >> + * context-bank details. >> + */ >> + rc = of_platform_populate(pdev->dev.of_node, msm_vidc_dt_match, NULL, >> + &pdev->dev); >> + if (rc) { >> + d_vpr_e("Failed to trigger probe for sub-devices\n"); >> + goto sub_dev_failed; >> + } >> + >> + rc = v4l2_device_register(&pdev->dev, &core->v4l2_dev); >> + if (rc) { >> + d_vpr_e("Failed to register v4l2 device\n"); >> + goto v4l2_reg_failed; >> + } >> + >> + /* setup the decoder device */ >> + rc = msm_vidc_register_video_device(core, MSM_VIDC_DECODER, nr); >> + if (rc) { >> + d_vpr_e("Failed to register video decoder\n"); >> + goto dec_reg_failed; >> + } >> + >> + /* setup the encoder device */ >> + rc = msm_vidc_register_video_device(core, MSM_VIDC_ENCODER, nr + 1); >> + if (rc) { >> + d_vpr_e("Failed to register video encoder\n"); >> + goto enc_reg_failed; >> + } >> + >> + rc = venus_hfi_queue_init(core); >> + if (rc) { >> + d_vpr_e("%s: interface queues init failed\n", __func__); >> + goto queues_init_failed; >> + } >> + >> + rc = msm_vidc_core_init(core); >> + if (rc) { >> + d_vpr_e("%s: sys init failed\n", __func__); >> + goto core_init_failed; >> + } >> + >> + d_vpr_h("%s(): succssful\n", __func__); > > No need, drop. Sure, will drop. > >> + >> + return rc; >> + >> +core_init_failed: >> + venus_hfi_queue_deinit(core); >> +queues_init_failed: >> + of_platform_depopulate(&pdev->dev); >> +sub_dev_failed: >> + msm_vidc_unregister_video_device(core, MSM_VIDC_ENCODER); >> +enc_reg_failed: >> + msm_vidc_unregister_video_device(core, MSM_VIDC_DECODER); >> +dec_reg_failed: >> + v4l2_device_unregister(&core->v4l2_dev); >> +v4l2_reg_failed: >> +init_inst_caps_fail: >> +init_res_failed: >> +init_plat_failed: >> +init_core_failed: > > Ykes! No. This code needs more work. > I understand, will improve the design in next version. > Best regards, > Krzysztof >