Re: [PATCH] i2c: scmi: Use standard device message logging functions

["Rafael J. Wysocki" <rafael.j.wysocki@xxxxxxxxx>]

On 2/27/2018 10:57 PM, Jean Delvare wrote:
> ACPI_ERROR and ACPI_DEBUG_PRINT are not intended to be used by device
> drivers. Use standard device message logging functions instead (on
> the acpi_handle while probing, and then on the i2c_adapter device.)
>
> As a nice side effect, it removes the following compiler warnings
> which were printed when ACPI debug was disabled:
>
> drivers/i2c/busses/i2c-scmi.c: In function "acpi_smbus_cmi_add_cap":
> drivers/i2c/busses/i2c-scmi.c:328:39: warning: suggest braces around empty body in an "else" statement [-Wempty-body]
> drivers/i2c/busses/i2c-scmi.c:338:12: warning: suggest braces around empty body in an "else" statement [-Wempty-body]
>
> Suggested-by: "Rafael J. Wysocki" <rafael.j.wysocki@xxxxxxxxx>
> Signed-off-by: Jean Delvare <jdelvare@xxxxxxx>
> Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx>
> ---
>   drivers/i2c/busses/i2c-scmi.c |   32 ++++++++++++++++----------------
>   1 file changed, 16 insertions(+), 16 deletions(-)
>
> --- linux-4.16-rc3.orig/drivers/i2c/busses/i2c-scmi.c	2018-02-27 22:46:28.669596683 +0100
> +++ linux-4.16-rc3/drivers/i2c/busses/i2c-scmi.c	2018-02-27 22:55:21.627411047 +0100
> @@ -182,7 +182,8 @@ acpi_smbus_cmi_access(struct i2c_adapter
>   	status = acpi_evaluate_object(smbus_cmi->handle, method, &input,
>   				      &buffer);
>   	if (ACPI_FAILURE(status)) {
> -		ACPI_ERROR((AE_INFO, "Evaluating %s: %i", method, status));
> +		dev_err(&adap->dev, "Failed to evaluate %s: %i\n", method,
> +			status);

I would use acpi_handle_err(ACPI_HANDLE(&adap->dev), ...) here and below 
as that would retain the ACPI prefix and would make it easier to find 
the problematic ACPI object in the ASL.

>   		return -EIO;
>   	}
>   
> @@ -190,19 +191,18 @@ acpi_smbus_cmi_access(struct i2c_adapter
>   	if (pkg && pkg->type == ACPI_TYPE_PACKAGE)
>   		obj = pkg->package.elements;
>   	else {
> -		ACPI_ERROR((AE_INFO, "Invalid argument type"));
> +		dev_err(&adap->dev, "Invalid argument type\n");
>   		result = -EIO;
>   		goto out;
>   	}
>   	if (obj == NULL || obj->type != ACPI_TYPE_INTEGER) {
> -		ACPI_ERROR((AE_INFO, "Invalid argument type"));
> +		dev_err(&adap->dev, "Invalid argument type\n");
>   		result = -EIO;
>   		goto out;
>   	}
>   
>   	result = obj->integer.value;
> -	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "%s return status: %i\n",
> -			  method, result));
> +	dev_dbg(&adap->dev, "%s return status: %i\n", method, result);
>   
>   	switch (result) {
>   	case ACPI_SMBUS_STATUS_OK:
> @@ -227,7 +227,7 @@ acpi_smbus_cmi_access(struct i2c_adapter
>   
>   	obj = pkg->package.elements + 1;
>   	if (obj->type != ACPI_TYPE_INTEGER) {
> -		ACPI_ERROR((AE_INFO, "Invalid argument type"));
> +		dev_err(&adap->dev, "Invalid argument type\n");
>   		result = -EIO;
>   		goto out;
>   	}
> @@ -239,7 +239,7 @@ acpi_smbus_cmi_access(struct i2c_adapter
>   	case I2C_SMBUS_BYTE_DATA:
>   	case I2C_SMBUS_WORD_DATA:
>   		if (obj->type != ACPI_TYPE_INTEGER) {
> -			ACPI_ERROR((AE_INFO, "Invalid argument type"));
> +			dev_err(&adap->dev, "Invalid argument type\n");
>   			result = -EIO;
>   			goto out;
>   		}
> @@ -250,7 +250,7 @@ acpi_smbus_cmi_access(struct i2c_adapter
>   		break;
>   	case I2C_SMBUS_BLOCK_DATA:
>   		if (obj->type != ACPI_TYPE_BUFFER) {
> -			ACPI_ERROR((AE_INFO, "Invalid argument type"));
> +			dev_err(&adap->dev, "Invalid argument type\n");
>   			result = -EIO;
>   			goto out;
>   		}
> @@ -300,6 +300,7 @@ static int acpi_smbus_cmi_add_cap(struct
>   				  const char *name)
>   {
>   	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct acpi_handle *handle = smbus_cmi->handle;
>   	union acpi_object *obj;
>   	acpi_status status;
>   
> @@ -308,8 +309,8 @@ static int acpi_smbus_cmi_add_cap(struct
>   					smbus_cmi->methods->mt_info,
>   					NULL, &buffer);
>   		if (ACPI_FAILURE(status)) {
> -			ACPI_ERROR((AE_INFO, "Evaluating %s: %i",
> -				   smbus_cmi->methods->mt_info, status));
> +			acpi_handle_err(handle, "Failed to evaluate %s: %i\n",
> +					smbus_cmi->methods->mt_info, status);
>   			return -EIO;
>   		}
>   
> @@ -317,18 +318,18 @@ static int acpi_smbus_cmi_add_cap(struct
>   		if (obj && obj->type == ACPI_TYPE_PACKAGE)
>   			obj = obj->package.elements;
>   		else {
> -			ACPI_ERROR((AE_INFO, "Invalid argument type"));
> +			acpi_handle_err(handle, "Invalid argument type\n");
>   			kfree(buffer.pointer);
>   			return -EIO;
>   		}
>   
>   		if (obj->type != ACPI_TYPE_INTEGER) {
> -			ACPI_ERROR((AE_INFO, "Invalid argument type"));
> +			acpi_handle_err(handle, "Invalid argument type\n");
>   			kfree(buffer.pointer);
>   			return -EIO;
>   		} else
> -			ACPI_DEBUG_PRINT((ACPI_DB_INFO, "SMBus CMI Version %x"
> -					  "\n", (int)obj->integer.value));
> +			acpi_handle_debug(handle, "SMBus CMI Version %x\n",
> +					  (int)obj->integer.value);
>   
>   		kfree(buffer.pointer);
>   		smbus_cmi->cap_info = 1;
> @@ -337,8 +338,7 @@ static int acpi_smbus_cmi_add_cap(struct
>   	else if (!strcmp(name, smbus_cmi->methods->mt_sbw))
>   		smbus_cmi->cap_write = 1;
>   	else
> -		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Unsupported CMI method: %s\n",
> -				 name));
> +		acpi_handle_debug(handle, "Unsupported CMI method: %s\n", name);
>   
>   	return 0;
>   }