Re: [PATCH 02/33] iris: vidc: add core functions

[Krzysztof Kozlowski <krzk@kernel.org>]

On 28/07/2023 15:23, Vikash Garodia wrote:
> From: Dikshita Agarwal <quic_dikshita@quicinc.com>
> 
> 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.

> +	}
> +
> +	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.

> +
> +	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.

Best regards,
Krzysztof