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 jacksonchui.qwerty@gmail.com
--- 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; } }
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 jacksonchui.qwerty@gmail.com
Reviewed-by: Dan Carpenter dan.carpenter@linaro.org
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 jacksonchui.qwerty@gmail.com
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 elder@linaro.org
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; }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");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);@@ -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);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);@@ -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");@@ -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);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));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);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;
-
Alex Elder -
Dan Carpenter -
Jackson Chui