Re: [RFC PATCH 2/2] platform/surface: aggregator: Log critical errors during SAM probing

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Thu, May 02, 2024 at 12:02:47PM +0800, Weifeng Liu wrote:
> Emits messages upon errors during probing of SAM.  Hopefully this could
> provide useful context to user for the purpose of diagnosis when
> something miserable happen.

...

> -	if (gpiod_count(&serdev->dev, NULL) < 0)
> +	if (gpiod_count(&serdev->dev, NULL) < 0) {
> +		dev_err(&serdev->dev, "no GPIO found\n");
>  		return -ENODEV;

Why not

	return dev_err_probe(...);

?

Also while at it, unshadow the error code

	status = gpiod_count(&serdev->dev, NULL);
	if (status < 0)
		return dev_err_probe(..., status, ...);

> +	}

...

>  	astatus = ssam_serdev_setup_via_acpi(ssh->handle, serdev);
>  	if (ACPI_FAILURE(astatus)) {
>  		status = -ENXIO;
> +		dev_err(&serdev->dev, "failed to setup serdev\n");

In the similar way

		status = dev_err(&serdev->dev, -ENXIO, "failed to setup serdev\n");

>  		goto err_devinit;
>  	}

...

>  	status = ssam_log_firmware_version(ctrl);
> -	if (status)
> +	if (status) {
> +		dev_err(&serdev->dev, "failed to get firmware version\n");
>  		goto err_initrq;

Use dev_err_probe() everywhere for the sake of uniform output format.

> +	}

...

>  	status = ssam_ctrl_notif_d0_entry(ctrl);
> -	if (status)
> +	if (status) {
> +		dev_err(&serdev->dev, "failed to notify EC of entry of D0\n");
>  		goto err_initrq;

Ditto.

> +	}
>  
>  	status = ssam_ctrl_notif_display_on(ctrl);
> -	if (status)
> +	if (status) {
> +		dev_err(&serdev->dev, "failed to notify EC of display on\n");
>  		goto err_initrq;

Ditto.

> +	}

-- 
With Best Regards,
Andy Shevchenko