Reported by checkpatch:
CHECK: Macro argument 'gcam' may be better as '(gcam)' to avoid
precedence issues
Inline standard calls to 'dev_*' kernel logging functions, in favor
of 'gcam_*' macros, to clear up gcam-related logging.
Signed-off-by: Jackson Chui <[email protected]>
---
Changes in v2:
- Inline 'dev_*' logging functions, over wrappers and macros
to just use the standard call.
- Remove 'gcam_*' macros, since they are no longer used.
---
drivers/staging/greybus/camera.c | 58 +++++++++++++++-----------------
1 file changed, 27 insertions(+), 31 deletions(-)
diff --git a/drivers/staging/greybus/camera.c b/drivers/staging/greybus/camera.c
index a8173aa3a995..b8b2bdfa59e5 100644
--- a/drivers/staging/greybus/camera.c
+++ b/drivers/staging/greybus/camera.c
@@ -180,10 +180,6 @@ static const struct gb_camera_fmt_info *gb_camera_get_format_info(u16 gb_fmt)
#define GB_CAMERA_MAX_SETTINGS_SIZE 8192
-#define gcam_dbg(gcam, format...) dev_dbg(&gcam->bundle->dev, format)
-#define gcam_info(gcam, format...) dev_info(&gcam->bundle->dev, format)
-#define gcam_err(gcam, format...) dev_err(&gcam->bundle->dev, format)
-
static int gb_camera_operation_sync_flags(struct gb_connection *connection,
int type, unsigned int flags,
void *request, size_t request_size,
@@ -232,8 +228,8 @@ static int gb_camera_get_max_pkt_size(struct gb_camera *gcam,
fmt_info = gb_camera_get_format_info(cfg->format);
if (!fmt_info) {
- gcam_err(gcam, "unsupported greybus image format: %d\n",
- cfg->format);
+ dev_err(&gcam->bundle->dev, "unsupported greybus image format: %d\n",
+ cfg->format);
return -EIO;
}
@@ -241,18 +237,18 @@ static int gb_camera_get_max_pkt_size(struct gb_camera *gcam,
pkt_size = le32_to_cpu(cfg->max_pkt_size);
if (pkt_size == 0) {
- gcam_err(gcam,
- "Stream %u: invalid zero maximum packet size\n",
- i);
+ dev_err(&gcam->bundle->dev,
+ "Stream %u: invalid zero maximum packet size\n",
+ i);
return -EIO;
}
} else {
pkt_size = le16_to_cpu(cfg->width) * fmt_info->bpp / 8;
if (pkt_size != le32_to_cpu(cfg->max_pkt_size)) {
- gcam_err(gcam,
- "Stream %u: maximum packet size mismatch (%u/%u)\n",
- i, pkt_size, cfg->max_pkt_size);
+ dev_err(&gcam->bundle->dev,
+ "Stream %u: maximum packet size mismatch (%u/%u)\n",
+ i, pkt_size, cfg->max_pkt_size);
return -EIO;
}
}
@@ -275,13 +271,13 @@ static const int gb_camera_configure_streams_validate_response(struct gb_camera
/* Validate the returned response structure */
if (resp->padding[0] || resp->padding[1]) {
- gcam_err(gcam, "response padding != 0\n");
+ dev_err(&gcam->bundle->dev, "response padding != 0\n");
return -EIO;
}
if (resp->num_streams > nstreams) {
- gcam_err(gcam, "got #streams %u > request %u\n",
- resp->num_streams, nstreams);
+ dev_err(&gcam->bundle->dev, "got #streams %u > request %u\n",
+ resp->num_streams, nstreams);
return -EIO;
}
@@ -289,7 +285,7 @@ static const int gb_camera_configure_streams_validate_response(struct gb_camera
struct gb_camera_stream_config_response *cfg = &resp->config[i];
if (cfg->padding) {
- gcam_err(gcam, "stream #%u padding != 0\n", i);
+ dev_err(&gcam->bundle->dev, "stream #%u padding != 0\n", i);
return -EIO;
}
}
@@ -340,16 +336,16 @@ static int gb_camera_set_power_mode(struct gb_camera *gcam, bool hs)
ret = gb_camera_set_intf_power_mode(gcam, intf->interface_id, hs);
if (ret < 0) {
- gcam_err(gcam, "failed to set module interface to %s (%d)\n",
- hs ? "HS" : "PWM", ret);
+ dev_err(&gcam->bundle->dev, "failed to set module interface to %s (%d)\n",
+ hs ? "HS" : "PWM", ret);
return ret;
}
ret = gb_camera_set_intf_power_mode(gcam, svc->ap_intf_id, hs);
if (ret < 0) {
gb_camera_set_intf_power_mode(gcam, intf->interface_id, !hs);
- gcam_err(gcam, "failed to set AP interface to %s (%d)\n",
- hs ? "HS" : "PWM", ret);
+ dev_err(&gcam->bundle->dev, "failed to set AP interface to %s (%d)\n",
+ hs ? "HS" : "PWM", ret);
return ret;
}
@@ -435,7 +431,7 @@ static int gb_camera_setup_data_connection(struct gb_camera *gcam,
sizeof(csi_cfg),
GB_APB_REQUEST_CSI_TX_CONTROL, false);
if (ret < 0) {
- gcam_err(gcam, "failed to start the CSI transmitter\n");
+ dev_err(&gcam->bundle->dev, "failed to start the CSI transmitter\n");
goto error_power;
}
@@ -470,7 +466,7 @@ static void gb_camera_teardown_data_connection(struct gb_camera *gcam)
GB_APB_REQUEST_CSI_TX_CONTROL, false);
if (ret < 0)
- gcam_err(gcam, "failed to stop the CSI transmitter\n");
+ dev_err(&gcam->bundle->dev, "failed to stop the CSI transmitter\n");
/* Set the UniPro link to low speed mode. */
gb_camera_set_power_mode(gcam, false);
@@ -507,7 +503,7 @@ static int gb_camera_capabilities(struct gb_camera *gcam,
NULL, 0,
(void *)capabilities, size);
if (ret)
- gcam_err(gcam, "failed to retrieve capabilities: %d\n", ret);
+ dev_err(&gcam->bundle->dev, "failed to retrieve capabilities: %d\n", ret);
done:
mutex_unlock(&gcam->mutex);
@@ -723,22 +719,22 @@ static int gb_camera_request_handler(struct gb_operation *op)
struct gb_message *request;
if (op->type != GB_CAMERA_TYPE_METADATA) {
- gcam_err(gcam, "Unsupported unsolicited event: %u\n", op->type);
+ dev_err(&gcam->bundle->dev, "Unsupported unsolicited event: %u\n", op->type);
return -EINVAL;
}
request = op->request;
if (request->payload_size < sizeof(*payload)) {
- gcam_err(gcam, "Wrong event size received (%zu < %zu)\n",
- request->payload_size, sizeof(*payload));
+ dev_err(&gcam->bundle->dev, "Wrong event size received (%zu < %zu)\n",
+ request->payload_size, sizeof(*payload));
return -EINVAL;
}
payload = request->payload;
- gcam_dbg(gcam, "received metadata for request %u, frame %u, stream %u\n",
- payload->request_id, payload->frame_number, payload->stream);
+ dev_dbg(&gcam->bundle->dev, "received metadata for request %u, frame %u, stream %u\n",
+ payload->request_id, payload->frame_number, payload->stream);
return 0;
}
@@ -1347,15 +1343,15 @@ static int gb_camera_resume(struct device *dev)
ret = gb_connection_enable(gcam->connection);
if (ret) {
- gcam_err(gcam, "failed to enable connection: %d\n", ret);
+ dev_err(&gcam->bundle->dev, "failed to enable connection: %d\n", ret);
return ret;
}
if (gcam->data_connection) {
ret = gb_connection_enable(gcam->data_connection);
if (ret) {
- gcam_err(gcam,
- "failed to enable data connection: %d\n", ret);
+ dev_err(&gcam->bundle->dev,
+ "failed to enable data connection: %d\n", ret);
return ret;
}
}
--
2.34.1
On Mon, Apr 08, 2024 at 03:44:40PM -0700, Jackson Chui wrote:
> Reported by checkpatch:
>
> CHECK: Macro argument 'gcam' may be better as '(gcam)' to avoid
> precedence issues
>
> Inline standard calls to 'dev_*' kernel logging functions, in favor
> of 'gcam_*' macros, to clear up gcam-related logging.
>
> Signed-off-by: Jackson Chui <[email protected]>
Reviewed-by: Dan Carpenter <[email protected]>
regards,
dan carpenter
On 4/8/24 5:44 PM, Jackson Chui wrote:
> Reported by checkpatch:
>
> CHECK: Macro argument 'gcam' may be better as '(gcam)' to avoid
> precedence issues
>
> Inline standard calls to 'dev_*' kernel logging functions, in favor
> of 'gcam_*' macros, to clear up gcam-related logging.
>
> Signed-off-by: Jackson Chui <[email protected]>
This looks good, thanks for doing this.
Nit: for the future, on a few of the dev_err() calls, the new first
argument makes the line even wider than before. Lines wider than
80 columns are acceptable--especially when they contain formatted
output--but you could have put the format string on a new line in
a few of the cases. I'm old-skool and prefer making things fit
in 80 columns unless it gets too ugly.
Reviewed-by: Alex Elder <[email protected]>
>
> ---
> Changes in v2:
> - Inline 'dev_*' logging functions, over wrappers and macros
> to just use the standard call.
> - Remove 'gcam_*' macros, since they are no longer used.
> ---
> drivers/staging/greybus/camera.c | 58 +++++++++++++++-----------------
> 1 file changed, 27 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/staging/greybus/camera.c b/drivers/staging/greybus/camera.c
> index a8173aa3a995..b8b2bdfa59e5 100644
> --- a/drivers/staging/greybus/camera.c
> +++ b/drivers/staging/greybus/camera.c
> @@ -180,10 +180,6 @@ static const struct gb_camera_fmt_info *gb_camera_get_format_info(u16 gb_fmt)
>
> #define GB_CAMERA_MAX_SETTINGS_SIZE 8192
>
> -#define gcam_dbg(gcam, format...) dev_dbg(&gcam->bundle->dev, format)
> -#define gcam_info(gcam, format...) dev_info(&gcam->bundle->dev, format)
> -#define gcam_err(gcam, format...) dev_err(&gcam->bundle->dev, format)
> -
> static int gb_camera_operation_sync_flags(struct gb_connection *connection,
> int type, unsigned int flags,
> void *request, size_t request_size,
> @@ -232,8 +228,8 @@ static int gb_camera_get_max_pkt_size(struct gb_camera *gcam,
>
> fmt_info = gb_camera_get_format_info(cfg->format);
> if (!fmt_info) {
> - gcam_err(gcam, "unsupported greybus image format: %d\n",
> - cfg->format);
> + dev_err(&gcam->bundle->dev, "unsupported greybus image format: %d\n",
> + cfg->format);
> return -EIO;
> }
>
> @@ -241,18 +237,18 @@ static int gb_camera_get_max_pkt_size(struct gb_camera *gcam,
> pkt_size = le32_to_cpu(cfg->max_pkt_size);
>
> if (pkt_size == 0) {
> - gcam_err(gcam,
> - "Stream %u: invalid zero maximum packet size\n",
> - i);
> + dev_err(&gcam->bundle->dev,
> + "Stream %u: invalid zero maximum packet size\n",
> + i);
> return -EIO;
> }
> } else {
> pkt_size = le16_to_cpu(cfg->width) * fmt_info->bpp / 8;
>
> if (pkt_size != le32_to_cpu(cfg->max_pkt_size)) {
> - gcam_err(gcam,
> - "Stream %u: maximum packet size mismatch (%u/%u)\n",
> - i, pkt_size, cfg->max_pkt_size);
> + dev_err(&gcam->bundle->dev,
> + "Stream %u: maximum packet size mismatch (%u/%u)\n",
> + i, pkt_size, cfg->max_pkt_size);
> return -EIO;
> }
> }
> @@ -275,13 +271,13 @@ static const int gb_camera_configure_streams_validate_response(struct gb_camera
>
> /* Validate the returned response structure */
> if (resp->padding[0] || resp->padding[1]) {
> - gcam_err(gcam, "response padding != 0\n");
> + dev_err(&gcam->bundle->dev, "response padding != 0\n");
> return -EIO;
> }
>
> if (resp->num_streams > nstreams) {
> - gcam_err(gcam, "got #streams %u > request %u\n",
> - resp->num_streams, nstreams);
> + dev_err(&gcam->bundle->dev, "got #streams %u > request %u\n",
> + resp->num_streams, nstreams);
> return -EIO;
> }
>
> @@ -289,7 +285,7 @@ static const int gb_camera_configure_streams_validate_response(struct gb_camera
> struct gb_camera_stream_config_response *cfg = &resp->config[i];
>
> if (cfg->padding) {
> - gcam_err(gcam, "stream #%u padding != 0\n", i);
> + dev_err(&gcam->bundle->dev, "stream #%u padding != 0\n", i);
> return -EIO;
> }
> }
> @@ -340,16 +336,16 @@ static int gb_camera_set_power_mode(struct gb_camera *gcam, bool hs)
>
> ret = gb_camera_set_intf_power_mode(gcam, intf->interface_id, hs);
> if (ret < 0) {
> - gcam_err(gcam, "failed to set module interface to %s (%d)\n",
> - hs ? "HS" : "PWM", ret);
> + dev_err(&gcam->bundle->dev, "failed to set module interface to %s (%d)\n",
> + hs ? "HS" : "PWM", ret);
> return ret;
> }
>
> ret = gb_camera_set_intf_power_mode(gcam, svc->ap_intf_id, hs);
> if (ret < 0) {
> gb_camera_set_intf_power_mode(gcam, intf->interface_id, !hs);
> - gcam_err(gcam, "failed to set AP interface to %s (%d)\n",
> - hs ? "HS" : "PWM", ret);
> + dev_err(&gcam->bundle->dev, "failed to set AP interface to %s (%d)\n",
> + hs ? "HS" : "PWM", ret);
> return ret;
> }
>
> @@ -435,7 +431,7 @@ static int gb_camera_setup_data_connection(struct gb_camera *gcam,
> sizeof(csi_cfg),
> GB_APB_REQUEST_CSI_TX_CONTROL, false);
> if (ret < 0) {
> - gcam_err(gcam, "failed to start the CSI transmitter\n");
> + dev_err(&gcam->bundle->dev, "failed to start the CSI transmitter\n");
> goto error_power;
> }
>
> @@ -470,7 +466,7 @@ static void gb_camera_teardown_data_connection(struct gb_camera *gcam)
> GB_APB_REQUEST_CSI_TX_CONTROL, false);
>
> if (ret < 0)
> - gcam_err(gcam, "failed to stop the CSI transmitter\n");
> + dev_err(&gcam->bundle->dev, "failed to stop the CSI transmitter\n");
>
> /* Set the UniPro link to low speed mode. */
> gb_camera_set_power_mode(gcam, false);
> @@ -507,7 +503,7 @@ static int gb_camera_capabilities(struct gb_camera *gcam,
> NULL, 0,
> (void *)capabilities, size);
> if (ret)
> - gcam_err(gcam, "failed to retrieve capabilities: %d\n", ret);
> + dev_err(&gcam->bundle->dev, "failed to retrieve capabilities: %d\n", ret);
>
> done:
> mutex_unlock(&gcam->mutex);
> @@ -723,22 +719,22 @@ static int gb_camera_request_handler(struct gb_operation *op)
> struct gb_message *request;
>
> if (op->type != GB_CAMERA_TYPE_METADATA) {
> - gcam_err(gcam, "Unsupported unsolicited event: %u\n", op->type);
> + dev_err(&gcam->bundle->dev, "Unsupported unsolicited event: %u\n", op->type);
> return -EINVAL;
> }
>
> request = op->request;
>
> if (request->payload_size < sizeof(*payload)) {
> - gcam_err(gcam, "Wrong event size received (%zu < %zu)\n",
> - request->payload_size, sizeof(*payload));
> + dev_err(&gcam->bundle->dev, "Wrong event size received (%zu < %zu)\n",
> + request->payload_size, sizeof(*payload));
> return -EINVAL;
> }
>
> payload = request->payload;
>
> - gcam_dbg(gcam, "received metadata for request %u, frame %u, stream %u\n",
> - payload->request_id, payload->frame_number, payload->stream);
> + dev_dbg(&gcam->bundle->dev, "received metadata for request %u, frame %u, stream %u\n",
> + payload->request_id, payload->frame_number, payload->stream);
>
> return 0;
> }
> @@ -1347,15 +1343,15 @@ static int gb_camera_resume(struct device *dev)
>
> ret = gb_connection_enable(gcam->connection);
> if (ret) {
> - gcam_err(gcam, "failed to enable connection: %d\n", ret);
> + dev_err(&gcam->bundle->dev, "failed to enable connection: %d\n", ret);
> return ret;
> }
>
> if (gcam->data_connection) {
> ret = gb_connection_enable(gcam->data_connection);
> if (ret) {
> - gcam_err(gcam,
> - "failed to enable data connection: %d\n", ret);
> + dev_err(&gcam->bundle->dev,
> + "failed to enable data connection: %d\n", ret);
> return ret;
> }
> }