Quoting Laurent Pinchart (2023-06-04 11:49:58)
> The (large) rcar_du_modeset_init() function can fail for many reasons,
> two of two involving probe deferral. Use dev_err_probe() in those code
> paths to record the cause of the probe deferral, in order to help
> debugging probe issues.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> ---
> Change since v1:
>
> - Fix compilation
Always helps ;-)
> ---
> drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c | 4 ++++
> drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 8 ++++++--
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> index 12a8839fe3be..5b752adb1b02 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_drv.c
> @@ -701,6 +701,10 @@ static int rcar_du_probe(struct platform_device *pdev)
> /* DRM/KMS objects */
> ret = rcar_du_modeset_init(rcdu);
> if (ret < 0) {
> + /*
> + * Don't use dev_err_probe(), as it would overwrite the probe
> + * deferral reason recorded in rcar_du_modeset_init().
> + */
> if (ret != -EPROBE_DEFER)
> dev_err(&pdev->dev,
> "failed to initialize DRM/KMS (%d)\n", ret);
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> index adfb36b0e815..78b665984e35 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> @@ -932,8 +932,10 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>
> /* Initialize the Color Management Modules. */
> ret = rcar_du_cmm_init(rcdu);
> - if (ret)
> + if (ret) {
> + dev_err_probe(rcdu->dev, ret, "failed to initialize CMM\n");
This could remain a single line return statement with:
return dev_err_probe(rcdu->dev, ret, "failed to initialize CMM\n");
Or if you're really concerned about 80 chars:
return dev_err_probe(rcdu->dev, ret,
"failed to initialize CMM\n");
which is still one line cheaper than adding braces to the if statement!
Anyway, either way is functional so:
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
> return ret;
> + }
>
> /* Create the CRTCs. */
> for (swindex = 0, hwindex = 0; swindex < rcdu->num_crtcs; ++hwindex) {
> @@ -952,8 +954,10 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>
> /* Initialize the encoders. */
> ret = rcar_du_encoders_init(rcdu);
> - if (ret < 0)
> + if (ret < 0) {
> + dev_err_probe(rcdu->dev, ret, "failed to initialize encoders\n");
(same here of course)
> return ret;
> + }
>
> if (ret == 0) {
> dev_err(rcdu->dev, "error: no encoder could be initialized\n");
> --
> Regards,
>
> Laurent Pinchart
>
