Quoting Umang Jain (2024-03-15 10:56:56)
> Re-evaluate logs on error code paths and fix a few error logs
> with appropriate dev_* logging helpers.
>
> For a case where a null ptr check is performed, use a WARN_ON()
> instead of logging to dev_err().
>
> No functional changes intended in this patch.
>
> Signed-off-by: Umang Jain <umang.jain@xxxxxxxxxxxxxxxx>
> ---
> .../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 1579bd4e5263..3c3e6f3e48ce 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -1317,7 +1317,7 @@ vchiq_keepalive_thread_func(void *v)
> long rc = 0, uc = 0;
>
> if (wait_for_completion_interruptible(&arm_state->ka_evt)) {
> - dev_err(state->dev, "suspend: %s: interrupted\n", __func__);
> + dev_dbg(state->dev, "suspend: %s: interrupted\n", __func__);
This looks good.
> flush_signals(current);
> continue;
> }
> @@ -1380,7 +1380,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
> service->client_id);
> entity_uc = &service->service_use_count;
> } else {
> - dev_err(state->dev, "suspend: %s: null service ptr\n", __func__);
> + WARN_ON(!service);
This sounds like something that shouldn't happen. Can it actually happen?
If it can happen - can it be caused by userspace through the VCHIQ
interfaces or is this just an internal code path?
Bumping up to a WARN_ON could probably need justification that deserves
it's own patch, but for the rest of the lines:
Reviewed-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
> ret = -EINVAL;
> goto out;
> }
> @@ -1753,7 +1753,7 @@ static int vchiq_probe(struct platform_device *pdev)
> */
> err = vchiq_register_chrdev(&pdev->dev);
> if (err) {
> - dev_warn(&pdev->dev, "arm: Failed to initialize vchiq cdev\n");
> + dev_err(&pdev->dev, "arm: Failed to initialize vchiq cdev\n");
> goto error_exit;
> }
>
> @@ -1763,7 +1763,7 @@ static int vchiq_probe(struct platform_device *pdev)
> return 0;
>
> failed_platform_init:
> - dev_warn(&pdev->dev, "arm: Could not initialize vchiq platform\n");
> + dev_err(&pdev->dev, "arm: Could not initialize vchiq platform\n");
> error_exit:
> return err;
> }
> --
> 2.43.0
>
