[PATCH V2 0/3] staging: vchiq_arm: move state dump to debugfs

[PATCH V2 0/3] staging: vchiq_arm: move state dump to debugfs

[Stefan Wahren]

Hello,

since recent discussion raised the question about the future of debugfs
for vchiq [1], i want to submit this cleanup patch series as part of the
discussion and a small Halloween present ;-)

Best regards

Changes in V2:
- rebase on top of current staging-next
- address suggestion from Laurent Pinchart in patch 1
- fix checkpatch issue (too long line) in patch 2

[1] - https://lore.kernel.org/lkml/7ea529c2-3da6-47df-9b09-28d4ab36c4ef@kadam.mountain/T/

Stefan Wahren (3):
  staging: vchiq_core: Make vchiq_dump_service_state static
  staging: vchiq_core: Shorten bulk TX/RX pending dump
  staging: vchiq_arm: move state dump to debugfs

 .../interface/vchiq_arm/vchiq_arm.c           |  94 ++----
 .../interface/vchiq_arm/vchiq_arm.h           |   7 -
 .../interface/vchiq_arm/vchiq_core.c          | 274 +++++++-----------
 .../interface/vchiq_arm/vchiq_core.h          |  16 +-
 .../interface/vchiq_arm/vchiq_debugfs.c       |  10 +
 .../interface/vchiq_arm/vchiq_dev.c           |  21 --
 6 files changed, 137 insertions(+), 285 deletions(-)

--
2.34.1

[PATCH V2 0/3] staging: vchiq_arm: move state dump to debugfs

[Stefan Wahren]

Hello,

since recent discussion raised the question about the future of debugfs
for vchiq [1], i want to submit this cleanup patch series as part of the
discussion and a small Halloween present ;-)

Best regards

Changes in V2:
- rebase on top of current staging-next
- address suggestion from Laurent Pinchart in patch 1
- fix checkpatch issue (too long line) in patch 2

[1] - https://lore.kernel.org/lkml/7ea529c2-3da6-47df-9b09-28d4ab36c4ef@kadam.mountain/T/

Stefan Wahren (3):
  staging: vchiq_core: Make vchiq_dump_service_state static
  staging: vchiq_core: Shorten bulk TX/RX pending dump
  staging: vchiq_arm: move state dump to debugfs

 .../interface/vchiq_arm/vchiq_arm.c           |  94 ++----
 .../interface/vchiq_arm/vchiq_arm.h           |   7 -
 .../interface/vchiq_arm/vchiq_core.c          | 274 +++++++-----------
 .../interface/vchiq_arm/vchiq_core.h          |  16 +-
 .../interface/vchiq_arm/vchiq_debugfs.c       |  10 +
 .../interface/vchiq_arm/vchiq_dev.c           |  21 --
 6 files changed, 137 insertions(+), 285 deletions(-)

--
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2 1/3] staging: vchiq_core: Make vchiq_dump_service_state static

[Stefan Wahren]

The function vchiq_dump_service_state() is only used by vchiq_dump_state()
within vchiq_core.c. So move the definition of vchiq_dump_state() below
vchiq_dump_service_state() in order to make it static.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 .../interface/vchiq_arm/vchiq_core.c          | 169 +++++++++---------
 .../interface/vchiq_arm/vchiq_core.h          |   3 -
 2 files changed, 85 insertions(+), 87 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 39b857da2d42..94073f92651a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -3428,90 +3428,8 @@ vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state,
 	return 0;
 }

-int vchiq_dump_state(void *dump_context, struct vchiq_state *state)
-{
-	char buf[80];
-	int len;
-	int i;
-	int err;
-
-	len = scnprintf(buf, sizeof(buf), "State %d: %s", state->id,
-			conn_state_names[state->conn_state]);
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	len = scnprintf(buf, sizeof(buf), "  tx_pos=%x(@%pK), rx_pos=%x(@%pK)",
-			state->local->tx_pos,
-			state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
-			state->rx_pos,
-			state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK));
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	len = scnprintf(buf, sizeof(buf), "  Version: %d (min %d)",
-			VCHIQ_VERSION, VCHIQ_VERSION_MIN);
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	if (VCHIQ_ENABLE_STATS) {
-		len = scnprintf(buf, sizeof(buf),
-				"  Stats: ctrl_tx_count=%d, ctrl_rx_count=%d, error_count=%d",
-				state->stats.ctrl_tx_count, state->stats.ctrl_rx_count,
-				state->stats.error_count);
-		err = vchiq_dump(dump_context, buf, len + 1);
-		if (err)
-			return err;
-	}
-
-	len = scnprintf(buf, sizeof(buf),
-			"  Slots: %d available (%d data), %d recyclable, %d stalls (%d data)",
-			((state->slot_queue_available * VCHIQ_SLOT_SIZE) -
-			state->local_tx_pos) / VCHIQ_SLOT_SIZE,
-			state->data_quota - state->data_use_count,
-			state->local->slot_queue_recycle - state->slot_queue_available,
-			state->stats.slot_stalls, state->stats.data_stalls);
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	err = vchiq_dump_platform_state(dump_context);
-	if (err)
-		return err;
-
-	err = vchiq_dump_shared_state(dump_context,
-				      state,
-				      state->local,
-				      "Local");
-	if (err)
-		return err;
-	err = vchiq_dump_shared_state(dump_context,
-				      state,
-				      state->remote,
-				      "Remote");
-	if (err)
-		return err;
-
-	err = vchiq_dump_platform_instances(dump_context);
-	if (err)
-		return err;
-
-	for (i = 0; i < state->unused_service; i++) {
-		struct vchiq_service *service = find_service_by_port(state, i);
-
-		if (service) {
-			err = vchiq_dump_service_state(dump_context, service);
-			vchiq_service_put(service);
-			if (err)
-				return err;
-		}
-	}
-	return 0;
-}
-
-int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
+static int
+vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
 {
 	char buf[80];
 	int len;
@@ -3606,6 +3524,89 @@ int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
 	return err;
 }

+int vchiq_dump_state(void *dump_context, struct vchiq_state *state)
+{
+	char buf[80];
+	int len;
+	int i;
+	int err;
+
+	len = scnprintf(buf, sizeof(buf), "State %d: %s", state->id,
+			conn_state_names[state->conn_state]);
+	err = vchiq_dump(dump_context, buf, len + 1);
+	if (err)
+		return err;
+
+	len = scnprintf(buf, sizeof(buf), "  tx_pos=%x(@%pK), rx_pos=%x(@%pK)",
+			state->local->tx_pos,
+			state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
+			state->rx_pos,
+			state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK));
+	err = vchiq_dump(dump_context, buf, len + 1);
+	if (err)
+		return err;
+
+	len = scnprintf(buf, sizeof(buf), "  Version: %d (min %d)",
+			VCHIQ_VERSION, VCHIQ_VERSION_MIN);
+	err = vchiq_dump(dump_context, buf, len + 1);
+	if (err)
+		return err;
+
+	if (VCHIQ_ENABLE_STATS) {
+		len = scnprintf(buf, sizeof(buf),
+				"  Stats: ctrl_tx_count=%d, ctrl_rx_count=%d, error_count=%d",
+				state->stats.ctrl_tx_count, state->stats.ctrl_rx_count,
+				state->stats.error_count);
+		err = vchiq_dump(dump_context, buf, len + 1);
+		if (err)
+			return err;
+	}
+
+	len = scnprintf(buf, sizeof(buf),
+			"  Slots: %d available (%d data), %d recyclable, %d stalls (%d data)",
+			((state->slot_queue_available * VCHIQ_SLOT_SIZE) -
+			state->local_tx_pos) / VCHIQ_SLOT_SIZE,
+			state->data_quota - state->data_use_count,
+			state->local->slot_queue_recycle - state->slot_queue_available,
+			state->stats.slot_stalls, state->stats.data_stalls);
+	err = vchiq_dump(dump_context, buf, len + 1);
+	if (err)
+		return err;
+
+	err = vchiq_dump_platform_state(dump_context);
+	if (err)
+		return err;
+
+	err = vchiq_dump_shared_state(dump_context,
+				      state,
+				      state->local,
+				      "Local");
+	if (err)
+		return err;
+	err = vchiq_dump_shared_state(dump_context,
+				      state,
+				      state->remote,
+				      "Remote");
+	if (err)
+		return err;
+
+	err = vchiq_dump_platform_instances(dump_context);
+	if (err)
+		return err;
+
+	for (i = 0; i < state->unused_service; i++) {
+		struct vchiq_service *service = find_service_by_port(state, i);
+
+		if (service) {
+			err = vchiq_dump_service_state(dump_context, service);
+			vchiq_service_put(service);
+			if (err)
+				return err;
+		}
+	}
+	return 0;
+}
+
 int vchiq_send_remote_use(struct vchiq_state *state)
 {
 	if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index 161358db457c..ea8d58844775 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -507,9 +507,6 @@ vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle, void *
 extern int
 vchiq_dump_state(void *dump_context, struct vchiq_state *state);

-extern int
-vchiq_dump_service_state(void *dump_context, struct vchiq_service *service);
-
 extern void
 vchiq_loud_error_header(void);

--
2.34.1

[PATCH V2 1/3] staging: vchiq_core: Make vchiq_dump_service_state static

[Stefan Wahren]

The function vchiq_dump_service_state() is only used by vchiq_dump_state()
within vchiq_core.c. So move the definition of vchiq_dump_state() below
vchiq_dump_service_state() in order to make it static.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 .../interface/vchiq_arm/vchiq_core.c          | 169 +++++++++---------
 .../interface/vchiq_arm/vchiq_core.h          |   3 -
 2 files changed, 85 insertions(+), 87 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 39b857da2d42..94073f92651a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -3428,90 +3428,8 @@ vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state,
 	return 0;
 }

-int vchiq_dump_state(void *dump_context, struct vchiq_state *state)
-{
-	char buf[80];
-	int len;
-	int i;
-	int err;
-
-	len = scnprintf(buf, sizeof(buf), "State %d: %s", state->id,
-			conn_state_names[state->conn_state]);
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	len = scnprintf(buf, sizeof(buf), "  tx_pos=%x(@%pK), rx_pos=%x(@%pK)",
-			state->local->tx_pos,
-			state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
-			state->rx_pos,
-			state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK));
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	len = scnprintf(buf, sizeof(buf), "  Version: %d (min %d)",
-			VCHIQ_VERSION, VCHIQ_VERSION_MIN);
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	if (VCHIQ_ENABLE_STATS) {
-		len = scnprintf(buf, sizeof(buf),
-				"  Stats: ctrl_tx_count=%d, ctrl_rx_count=%d, error_count=%d",
-				state->stats.ctrl_tx_count, state->stats.ctrl_rx_count,
-				state->stats.error_count);
-		err = vchiq_dump(dump_context, buf, len + 1);
-		if (err)
-			return err;
-	}
-
-	len = scnprintf(buf, sizeof(buf),
-			"  Slots: %d available (%d data), %d recyclable, %d stalls (%d data)",
-			((state->slot_queue_available * VCHIQ_SLOT_SIZE) -
-			state->local_tx_pos) / VCHIQ_SLOT_SIZE,
-			state->data_quota - state->data_use_count,
-			state->local->slot_queue_recycle - state->slot_queue_available,
-			state->stats.slot_stalls, state->stats.data_stalls);
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	err = vchiq_dump_platform_state(dump_context);
-	if (err)
-		return err;
-
-	err = vchiq_dump_shared_state(dump_context,
-				      state,
-				      state->local,
-				      "Local");
-	if (err)
-		return err;
-	err = vchiq_dump_shared_state(dump_context,
-				      state,
-				      state->remote,
-				      "Remote");
-	if (err)
-		return err;
-
-	err = vchiq_dump_platform_instances(dump_context);
-	if (err)
-		return err;
-
-	for (i = 0; i < state->unused_service; i++) {
-		struct vchiq_service *service = find_service_by_port(state, i);
-
-		if (service) {
-			err = vchiq_dump_service_state(dump_context, service);
-			vchiq_service_put(service);
-			if (err)
-				return err;
-		}
-	}
-	return 0;
-}
-
-int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
+static int
+vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
 {
 	char buf[80];
 	int len;
@@ -3606,6 +3524,89 @@ int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
 	return err;
 }

+int vchiq_dump_state(void *dump_context, struct vchiq_state *state)
+{
+	char buf[80];
+	int len;
+	int i;
+	int err;
+
+	len = scnprintf(buf, sizeof(buf), "State %d: %s", state->id,
+			conn_state_names[state->conn_state]);
+	err = vchiq_dump(dump_context, buf, len + 1);
+	if (err)
+		return err;
+
+	len = scnprintf(buf, sizeof(buf), "  tx_pos=%x(@%pK), rx_pos=%x(@%pK)",
+			state->local->tx_pos,
+			state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
+			state->rx_pos,
+			state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK));
+	err = vchiq_dump(dump_context, buf, len + 1);
+	if (err)
+		return err;
+
+	len = scnprintf(buf, sizeof(buf), "  Version: %d (min %d)",
+			VCHIQ_VERSION, VCHIQ_VERSION_MIN);
+	err = vchiq_dump(dump_context, buf, len + 1);
+	if (err)
+		return err;
+
+	if (VCHIQ_ENABLE_STATS) {
+		len = scnprintf(buf, sizeof(buf),
+				"  Stats: ctrl_tx_count=%d, ctrl_rx_count=%d, error_count=%d",
+				state->stats.ctrl_tx_count, state->stats.ctrl_rx_count,
+				state->stats.error_count);
+		err = vchiq_dump(dump_context, buf, len + 1);
+		if (err)
+			return err;
+	}
+
+	len = scnprintf(buf, sizeof(buf),
+			"  Slots: %d available (%d data), %d recyclable, %d stalls (%d data)",
+			((state->slot_queue_available * VCHIQ_SLOT_SIZE) -
+			state->local_tx_pos) / VCHIQ_SLOT_SIZE,
+			state->data_quota - state->data_use_count,
+			state->local->slot_queue_recycle - state->slot_queue_available,
+			state->stats.slot_stalls, state->stats.data_stalls);
+	err = vchiq_dump(dump_context, buf, len + 1);
+	if (err)
+		return err;
+
+	err = vchiq_dump_platform_state(dump_context);
+	if (err)
+		return err;
+
+	err = vchiq_dump_shared_state(dump_context,
+				      state,
+				      state->local,
+				      "Local");
+	if (err)
+		return err;
+	err = vchiq_dump_shared_state(dump_context,
+				      state,
+				      state->remote,
+				      "Remote");
+	if (err)
+		return err;
+
+	err = vchiq_dump_platform_instances(dump_context);
+	if (err)
+		return err;
+
+	for (i = 0; i < state->unused_service; i++) {
+		struct vchiq_service *service = find_service_by_port(state, i);
+
+		if (service) {
+			err = vchiq_dump_service_state(dump_context, service);
+			vchiq_service_put(service);
+			if (err)
+				return err;
+		}
+	}
+	return 0;
+}
+
 int vchiq_send_remote_use(struct vchiq_state *state)
 {
 	if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index 161358db457c..ea8d58844775 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -507,9 +507,6 @@ vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle, void *
 extern int
 vchiq_dump_state(void *dump_context, struct vchiq_state *state);

-extern int
-vchiq_dump_service_state(void *dump_context, struct vchiq_service *service);
-
 extern void
 vchiq_loud_error_header(void);

--
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 1/3] staging: vchiq_core: Make vchiq_dump_service_state static

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Sun, Oct 29, 2023 at 01:48:35PM +0100, Stefan Wahren wrote:
> The function vchiq_dump_service_state() is only used by vchiq_dump_state()
> within vchiq_core.c. So move the definition of vchiq_dump_state() below
> vchiq_dump_service_state() in order to make it static.
> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

I don't remember suggesting this, but that's fine, it looks like a good
change :-)

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  .../interface/vchiq_arm/vchiq_core.c          | 169 +++++++++---------
>  .../interface/vchiq_arm/vchiq_core.h          |   3 -
>  2 files changed, 85 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> index 39b857da2d42..94073f92651a 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -3428,90 +3428,8 @@ vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state,
>  	return 0;
>  }
> 
> -int vchiq_dump_state(void *dump_context, struct vchiq_state *state)
> -{
> -	char buf[80];
> -	int len;
> -	int i;
> -	int err;
> -
> -	len = scnprintf(buf, sizeof(buf), "State %d: %s", state->id,
> -			conn_state_names[state->conn_state]);
> -	err = vchiq_dump(dump_context, buf, len + 1);
> -	if (err)
> -		return err;
> -
> -	len = scnprintf(buf, sizeof(buf), "  tx_pos=%x(@%pK), rx_pos=%x(@%pK)",
> -			state->local->tx_pos,
> -			state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
> -			state->rx_pos,
> -			state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK));
> -	err = vchiq_dump(dump_context, buf, len + 1);
> -	if (err)
> -		return err;
> -
> -	len = scnprintf(buf, sizeof(buf), "  Version: %d (min %d)",
> -			VCHIQ_VERSION, VCHIQ_VERSION_MIN);
> -	err = vchiq_dump(dump_context, buf, len + 1);
> -	if (err)
> -		return err;
> -
> -	if (VCHIQ_ENABLE_STATS) {
> -		len = scnprintf(buf, sizeof(buf),
> -				"  Stats: ctrl_tx_count=%d, ctrl_rx_count=%d, error_count=%d",
> -				state->stats.ctrl_tx_count, state->stats.ctrl_rx_count,
> -				state->stats.error_count);
> -		err = vchiq_dump(dump_context, buf, len + 1);
> -		if (err)
> -			return err;
> -	}
> -
> -	len = scnprintf(buf, sizeof(buf),
> -			"  Slots: %d available (%d data), %d recyclable, %d stalls (%d data)",
> -			((state->slot_queue_available * VCHIQ_SLOT_SIZE) -
> -			state->local_tx_pos) / VCHIQ_SLOT_SIZE,
> -			state->data_quota - state->data_use_count,
> -			state->local->slot_queue_recycle - state->slot_queue_available,
> -			state->stats.slot_stalls, state->stats.data_stalls);
> -	err = vchiq_dump(dump_context, buf, len + 1);
> -	if (err)
> -		return err;
> -
> -	err = vchiq_dump_platform_state(dump_context);
> -	if (err)
> -		return err;
> -
> -	err = vchiq_dump_shared_state(dump_context,
> -				      state,
> -				      state->local,
> -				      "Local");
> -	if (err)
> -		return err;
> -	err = vchiq_dump_shared_state(dump_context,
> -				      state,
> -				      state->remote,
> -				      "Remote");
> -	if (err)
> -		return err;
> -
> -	err = vchiq_dump_platform_instances(dump_context);
> -	if (err)
> -		return err;
> -
> -	for (i = 0; i < state->unused_service; i++) {
> -		struct vchiq_service *service = find_service_by_port(state, i);
> -
> -		if (service) {
> -			err = vchiq_dump_service_state(dump_context, service);
> -			vchiq_service_put(service);
> -			if (err)
> -				return err;
> -		}
> -	}
> -	return 0;
> -}
> -
> -int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
> +static int
> +vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
>  {
>  	char buf[80];
>  	int len;
> @@ -3606,6 +3524,89 @@ int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
>  	return err;
>  }
> 
> +int vchiq_dump_state(void *dump_context, struct vchiq_state *state)
> +{
> +	char buf[80];
> +	int len;
> +	int i;
> +	int err;
> +
> +	len = scnprintf(buf, sizeof(buf), "State %d: %s", state->id,
> +			conn_state_names[state->conn_state]);
> +	err = vchiq_dump(dump_context, buf, len + 1);
> +	if (err)
> +		return err;
> +
> +	len = scnprintf(buf, sizeof(buf), "  tx_pos=%x(@%pK), rx_pos=%x(@%pK)",
> +			state->local->tx_pos,
> +			state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
> +			state->rx_pos,
> +			state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK));
> +	err = vchiq_dump(dump_context, buf, len + 1);
> +	if (err)
> +		return err;
> +
> +	len = scnprintf(buf, sizeof(buf), "  Version: %d (min %d)",
> +			VCHIQ_VERSION, VCHIQ_VERSION_MIN);
> +	err = vchiq_dump(dump_context, buf, len + 1);
> +	if (err)
> +		return err;
> +
> +	if (VCHIQ_ENABLE_STATS) {
> +		len = scnprintf(buf, sizeof(buf),
> +				"  Stats: ctrl_tx_count=%d, ctrl_rx_count=%d, error_count=%d",
> +				state->stats.ctrl_tx_count, state->stats.ctrl_rx_count,
> +				state->stats.error_count);
> +		err = vchiq_dump(dump_context, buf, len + 1);
> +		if (err)
> +			return err;
> +	}
> +
> +	len = scnprintf(buf, sizeof(buf),
> +			"  Slots: %d available (%d data), %d recyclable, %d stalls (%d data)",
> +			((state->slot_queue_available * VCHIQ_SLOT_SIZE) -
> +			state->local_tx_pos) / VCHIQ_SLOT_SIZE,
> +			state->data_quota - state->data_use_count,
> +			state->local->slot_queue_recycle - state->slot_queue_available,
> +			state->stats.slot_stalls, state->stats.data_stalls);
> +	err = vchiq_dump(dump_context, buf, len + 1);
> +	if (err)
> +		return err;
> +
> +	err = vchiq_dump_platform_state(dump_context);
> +	if (err)
> +		return err;
> +
> +	err = vchiq_dump_shared_state(dump_context,
> +				      state,
> +				      state->local,
> +				      "Local");
> +	if (err)
> +		return err;
> +	err = vchiq_dump_shared_state(dump_context,
> +				      state,
> +				      state->remote,
> +				      "Remote");
> +	if (err)
> +		return err;
> +
> +	err = vchiq_dump_platform_instances(dump_context);
> +	if (err)
> +		return err;
> +
> +	for (i = 0; i < state->unused_service; i++) {
> +		struct vchiq_service *service = find_service_by_port(state, i);
> +
> +		if (service) {
> +			err = vchiq_dump_service_state(dump_context, service);
> +			vchiq_service_put(service);
> +			if (err)
> +				return err;
> +		}
> +	}
> +	return 0;
> +}
> +
>  int vchiq_send_remote_use(struct vchiq_state *state)
>  {
>  	if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> index 161358db457c..ea8d58844775 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> @@ -507,9 +507,6 @@ vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle, void *
>  extern int
>  vchiq_dump_state(void *dump_context, struct vchiq_state *state);
> 
> -extern int
> -vchiq_dump_service_state(void *dump_context, struct vchiq_service *service);
> -
>  extern void
>  vchiq_loud_error_header(void);
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH V2 1/3] staging: vchiq_core: Make vchiq_dump_service_state static

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Sun, Oct 29, 2023 at 01:48:35PM +0100, Stefan Wahren wrote:
> The function vchiq_dump_service_state() is only used by vchiq_dump_state()
> within vchiq_core.c. So move the definition of vchiq_dump_state() below
> vchiq_dump_service_state() in order to make it static.
> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

I don't remember suggesting this, but that's fine, it looks like a good
change :-)

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  .../interface/vchiq_arm/vchiq_core.c          | 169 +++++++++---------
>  .../interface/vchiq_arm/vchiq_core.h          |   3 -
>  2 files changed, 85 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> index 39b857da2d42..94073f92651a 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -3428,90 +3428,8 @@ vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state,
>  	return 0;
>  }
> 
> -int vchiq_dump_state(void *dump_context, struct vchiq_state *state)
> -{
> -	char buf[80];
> -	int len;
> -	int i;
> -	int err;
> -
> -	len = scnprintf(buf, sizeof(buf), "State %d: %s", state->id,
> -			conn_state_names[state->conn_state]);
> -	err = vchiq_dump(dump_context, buf, len + 1);
> -	if (err)
> -		return err;
> -
> -	len = scnprintf(buf, sizeof(buf), "  tx_pos=%x(@%pK), rx_pos=%x(@%pK)",
> -			state->local->tx_pos,
> -			state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
> -			state->rx_pos,
> -			state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK));
> -	err = vchiq_dump(dump_context, buf, len + 1);
> -	if (err)
> -		return err;
> -
> -	len = scnprintf(buf, sizeof(buf), "  Version: %d (min %d)",
> -			VCHIQ_VERSION, VCHIQ_VERSION_MIN);
> -	err = vchiq_dump(dump_context, buf, len + 1);
> -	if (err)
> -		return err;
> -
> -	if (VCHIQ_ENABLE_STATS) {
> -		len = scnprintf(buf, sizeof(buf),
> -				"  Stats: ctrl_tx_count=%d, ctrl_rx_count=%d, error_count=%d",
> -				state->stats.ctrl_tx_count, state->stats.ctrl_rx_count,
> -				state->stats.error_count);
> -		err = vchiq_dump(dump_context, buf, len + 1);
> -		if (err)
> -			return err;
> -	}
> -
> -	len = scnprintf(buf, sizeof(buf),
> -			"  Slots: %d available (%d data), %d recyclable, %d stalls (%d data)",
> -			((state->slot_queue_available * VCHIQ_SLOT_SIZE) -
> -			state->local_tx_pos) / VCHIQ_SLOT_SIZE,
> -			state->data_quota - state->data_use_count,
> -			state->local->slot_queue_recycle - state->slot_queue_available,
> -			state->stats.slot_stalls, state->stats.data_stalls);
> -	err = vchiq_dump(dump_context, buf, len + 1);
> -	if (err)
> -		return err;
> -
> -	err = vchiq_dump_platform_state(dump_context);
> -	if (err)
> -		return err;
> -
> -	err = vchiq_dump_shared_state(dump_context,
> -				      state,
> -				      state->local,
> -				      "Local");
> -	if (err)
> -		return err;
> -	err = vchiq_dump_shared_state(dump_context,
> -				      state,
> -				      state->remote,
> -				      "Remote");
> -	if (err)
> -		return err;
> -
> -	err = vchiq_dump_platform_instances(dump_context);
> -	if (err)
> -		return err;
> -
> -	for (i = 0; i < state->unused_service; i++) {
> -		struct vchiq_service *service = find_service_by_port(state, i);
> -
> -		if (service) {
> -			err = vchiq_dump_service_state(dump_context, service);
> -			vchiq_service_put(service);
> -			if (err)
> -				return err;
> -		}
> -	}
> -	return 0;
> -}
> -
> -int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
> +static int
> +vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
>  {
>  	char buf[80];
>  	int len;
> @@ -3606,6 +3524,89 @@ int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
>  	return err;
>  }
> 
> +int vchiq_dump_state(void *dump_context, struct vchiq_state *state)
> +{
> +	char buf[80];
> +	int len;
> +	int i;
> +	int err;
> +
> +	len = scnprintf(buf, sizeof(buf), "State %d: %s", state->id,
> +			conn_state_names[state->conn_state]);
> +	err = vchiq_dump(dump_context, buf, len + 1);
> +	if (err)
> +		return err;
> +
> +	len = scnprintf(buf, sizeof(buf), "  tx_pos=%x(@%pK), rx_pos=%x(@%pK)",
> +			state->local->tx_pos,
> +			state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
> +			state->rx_pos,
> +			state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK));
> +	err = vchiq_dump(dump_context, buf, len + 1);
> +	if (err)
> +		return err;
> +
> +	len = scnprintf(buf, sizeof(buf), "  Version: %d (min %d)",
> +			VCHIQ_VERSION, VCHIQ_VERSION_MIN);
> +	err = vchiq_dump(dump_context, buf, len + 1);
> +	if (err)
> +		return err;
> +
> +	if (VCHIQ_ENABLE_STATS) {
> +		len = scnprintf(buf, sizeof(buf),
> +				"  Stats: ctrl_tx_count=%d, ctrl_rx_count=%d, error_count=%d",
> +				state->stats.ctrl_tx_count, state->stats.ctrl_rx_count,
> +				state->stats.error_count);
> +		err = vchiq_dump(dump_context, buf, len + 1);
> +		if (err)
> +			return err;
> +	}
> +
> +	len = scnprintf(buf, sizeof(buf),
> +			"  Slots: %d available (%d data), %d recyclable, %d stalls (%d data)",
> +			((state->slot_queue_available * VCHIQ_SLOT_SIZE) -
> +			state->local_tx_pos) / VCHIQ_SLOT_SIZE,
> +			state->data_quota - state->data_use_count,
> +			state->local->slot_queue_recycle - state->slot_queue_available,
> +			state->stats.slot_stalls, state->stats.data_stalls);
> +	err = vchiq_dump(dump_context, buf, len + 1);
> +	if (err)
> +		return err;
> +
> +	err = vchiq_dump_platform_state(dump_context);
> +	if (err)
> +		return err;
> +
> +	err = vchiq_dump_shared_state(dump_context,
> +				      state,
> +				      state->local,
> +				      "Local");
> +	if (err)
> +		return err;
> +	err = vchiq_dump_shared_state(dump_context,
> +				      state,
> +				      state->remote,
> +				      "Remote");
> +	if (err)
> +		return err;
> +
> +	err = vchiq_dump_platform_instances(dump_context);
> +	if (err)
> +		return err;
> +
> +	for (i = 0; i < state->unused_service; i++) {
> +		struct vchiq_service *service = find_service_by_port(state, i);
> +
> +		if (service) {
> +			err = vchiq_dump_service_state(dump_context, service);
> +			vchiq_service_put(service);
> +			if (err)
> +				return err;
> +		}
> +	}
> +	return 0;
> +}
> +
>  int vchiq_send_remote_use(struct vchiq_state *state)
>  {
>  	if (state->conn_state == VCHIQ_CONNSTATE_DISCONNECTED)
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> index 161358db457c..ea8d58844775 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
> @@ -507,9 +507,6 @@ vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle, void *
>  extern int
>  vchiq_dump_state(void *dump_context, struct vchiq_state *state);
> 
> -extern int
> -vchiq_dump_service_state(void *dump_context, struct vchiq_service *service);
> -
>  extern void
>  vchiq_loud_error_header(void);
> 

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2 2/3] staging: vchiq_core: Shorten bulk TX/RX pending dump

[Stefan Wahren]

The calculation for the bulk TX/RX pending is complex and
reaches 99 chars per line. So move the size determination
below the pending calculation and get the rid of the
ternary operator.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 .../interface/vchiq_arm/vchiq_core.c          | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 94073f92651a..36c742a2f3b2 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -3447,7 +3447,7 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
 		struct vchiq_service_quota *quota =
 			&service->state->service_quotas[service->localport];
 		int fourcc = service->base.fourcc;
-		int tx_pending, rx_pending;
+		int tx_pending, rx_pending, tx_size = 0, rx_size = 0;

 		if (service->remoteport != VCHIQ_PORT_FREE) {
 			int len2 = scnprintf(remoteport, sizeof(remoteport),
@@ -3472,18 +3472,23 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)

 		tx_pending = service->bulk_tx.local_insert -
 			service->bulk_tx.remote_insert;
+		if (tx_pending) {
+			unsigned int i = BULK_INDEX(service->bulk_tx.remove);
+
+			tx_size = service->bulk_tx.bulks[i].size;
+		}

 		rx_pending = service->bulk_rx.local_insert -
 			service->bulk_rx.remote_insert;
+		if (rx_pending) {
+			unsigned int i = BULK_INDEX(service->bulk_rx.remove);
+
+			rx_size = service->bulk_rx.bulks[i].size;
+		}

 		len = scnprintf(buf, sizeof(buf),
 				"  Bulk: tx_pending=%d (size %d), rx_pending=%d (size %d)",
-				tx_pending,
-				tx_pending ?
-				service->bulk_tx.bulks[BULK_INDEX(service->bulk_tx.remove)].size :
-				0, rx_pending, rx_pending ?
-				service->bulk_rx.bulks[BULK_INDEX(service->bulk_rx.remove)].size :
-				0);
+				tx_pending, tx_size, rx_pending, rx_size);

 		if (VCHIQ_ENABLE_STATS) {
 			err = vchiq_dump(dump_context, buf, len + 1);
--
2.34.1

[PATCH V2 2/3] staging: vchiq_core: Shorten bulk TX/RX pending dump

[Stefan Wahren]

The calculation for the bulk TX/RX pending is complex and
reaches 99 chars per line. So move the size determination
below the pending calculation and get the rid of the
ternary operator.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 .../interface/vchiq_arm/vchiq_core.c          | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 94073f92651a..36c742a2f3b2 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -3447,7 +3447,7 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
 		struct vchiq_service_quota *quota =
 			&service->state->service_quotas[service->localport];
 		int fourcc = service->base.fourcc;
-		int tx_pending, rx_pending;
+		int tx_pending, rx_pending, tx_size = 0, rx_size = 0;

 		if (service->remoteport != VCHIQ_PORT_FREE) {
 			int len2 = scnprintf(remoteport, sizeof(remoteport),
@@ -3472,18 +3472,23 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)

 		tx_pending = service->bulk_tx.local_insert -
 			service->bulk_tx.remote_insert;
+		if (tx_pending) {
+			unsigned int i = BULK_INDEX(service->bulk_tx.remove);
+
+			tx_size = service->bulk_tx.bulks[i].size;
+		}

 		rx_pending = service->bulk_rx.local_insert -
 			service->bulk_rx.remote_insert;
+		if (rx_pending) {
+			unsigned int i = BULK_INDEX(service->bulk_rx.remove);
+
+			rx_size = service->bulk_rx.bulks[i].size;
+		}

 		len = scnprintf(buf, sizeof(buf),
 				"  Bulk: tx_pending=%d (size %d), rx_pending=%d (size %d)",
-				tx_pending,
-				tx_pending ?
-				service->bulk_tx.bulks[BULK_INDEX(service->bulk_tx.remove)].size :
-				0, rx_pending, rx_pending ?
-				service->bulk_rx.bulks[BULK_INDEX(service->bulk_rx.remove)].size :
-				0);
+				tx_pending, tx_size, rx_pending, rx_size);

 		if (VCHIQ_ENABLE_STATS) {
 			err = vchiq_dump(dump_context, buf, len + 1);
--
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 2/3] staging: vchiq_core: Shorten bulk TX/RX pending dump

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Sun, Oct 29, 2023 at 01:48:36PM +0100, Stefan Wahren wrote:
> The calculation for the bulk TX/RX pending is complex and
> reaches 99 chars per line. So move the size determination
> below the pending calculation and get the rid of the
> ternary operator.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  .../interface/vchiq_arm/vchiq_core.c          | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> index 94073f92651a..36c742a2f3b2 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -3447,7 +3447,7 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
>  		struct vchiq_service_quota *quota =
>  			&service->state->service_quotas[service->localport];
>  		int fourcc = service->base.fourcc;
> -		int tx_pending, rx_pending;
> +		int tx_pending, rx_pending, tx_size = 0, rx_size = 0;
> 
>  		if (service->remoteport != VCHIQ_PORT_FREE) {
>  			int len2 = scnprintf(remoteport, sizeof(remoteport),
> @@ -3472,18 +3472,23 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
> 
>  		tx_pending = service->bulk_tx.local_insert -
>  			service->bulk_tx.remote_insert;
> +		if (tx_pending) {
> +			unsigned int i = BULK_INDEX(service->bulk_tx.remove);
> +
> +			tx_size = service->bulk_tx.bulks[i].size;
> +		}
> 
>  		rx_pending = service->bulk_rx.local_insert -
>  			service->bulk_rx.remote_insert;
> +		if (rx_pending) {
> +			unsigned int i = BULK_INDEX(service->bulk_rx.remove);
> +
> +			rx_size = service->bulk_rx.bulks[i].size;
> +		}
> 
>  		len = scnprintf(buf, sizeof(buf),
>  				"  Bulk: tx_pending=%d (size %d), rx_pending=%d (size %d)",
> -				tx_pending,
> -				tx_pending ?
> -				service->bulk_tx.bulks[BULK_INDEX(service->bulk_tx.remove)].size :
> -				0, rx_pending, rx_pending ?
> -				service->bulk_rx.bulks[BULK_INDEX(service->bulk_rx.remove)].size :
> -				0);
> +				tx_pending, tx_size, rx_pending, rx_size);
> 
>  		if (VCHIQ_ENABLE_STATS) {
>  			err = vchiq_dump(dump_context, buf, len + 1);

-- 
Regards,

Laurent Pinchart

Re: [PATCH V2 2/3] staging: vchiq_core: Shorten bulk TX/RX pending dump

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Sun, Oct 29, 2023 at 01:48:36PM +0100, Stefan Wahren wrote:
> The calculation for the bulk TX/RX pending is complex and
> reaches 99 chars per line. So move the size determination
> below the pending calculation and get the rid of the
> ternary operator.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  .../interface/vchiq_arm/vchiq_core.c          | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> index 94073f92651a..36c742a2f3b2 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -3447,7 +3447,7 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
>  		struct vchiq_service_quota *quota =
>  			&service->state->service_quotas[service->localport];
>  		int fourcc = service->base.fourcc;
> -		int tx_pending, rx_pending;
> +		int tx_pending, rx_pending, tx_size = 0, rx_size = 0;
> 
>  		if (service->remoteport != VCHIQ_PORT_FREE) {
>  			int len2 = scnprintf(remoteport, sizeof(remoteport),
> @@ -3472,18 +3472,23 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
> 
>  		tx_pending = service->bulk_tx.local_insert -
>  			service->bulk_tx.remote_insert;
> +		if (tx_pending) {
> +			unsigned int i = BULK_INDEX(service->bulk_tx.remove);
> +
> +			tx_size = service->bulk_tx.bulks[i].size;
> +		}
> 
>  		rx_pending = service->bulk_rx.local_insert -
>  			service->bulk_rx.remote_insert;
> +		if (rx_pending) {
> +			unsigned int i = BULK_INDEX(service->bulk_rx.remove);
> +
> +			rx_size = service->bulk_rx.bulks[i].size;
> +		}
> 
>  		len = scnprintf(buf, sizeof(buf),
>  				"  Bulk: tx_pending=%d (size %d), rx_pending=%d (size %d)",
> -				tx_pending,
> -				tx_pending ?
> -				service->bulk_tx.bulks[BULK_INDEX(service->bulk_tx.remove)].size :
> -				0, rx_pending, rx_pending ?
> -				service->bulk_rx.bulks[BULK_INDEX(service->bulk_rx.remove)].size :
> -				0);
> +				tx_pending, tx_size, rx_pending, rx_size);
> 
>  		if (VCHIQ_ENABLE_STATS) {
>  			err = vchiq_dump(dump_context, buf, len + 1);

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2 3/3] staging: vchiq_arm: move state dump to debugfs

[Stefan Wahren]

Besides the IOCTL interface the VCHIQ character device also provides
a state dump of the whole VCHIQ driver via read. Moving the state dump
function to debugfs has a lot advantages:

- following changes on state dump doesn't break userspace ABI
- debug doesn't depend on VCHIQ_CDEV
- dump code simplifies a lot and reduce the chance of buffer overflows

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 .../interface/vchiq_arm/vchiq_arm.c           |  94 ++------
 .../interface/vchiq_arm/vchiq_arm.h           |   7 -
 .../interface/vchiq_arm/vchiq_core.c          | 220 ++++++------------
 .../interface/vchiq_arm/vchiq_core.h          |  13 +-
 .../interface/vchiq_arm/vchiq_debugfs.c       |  10 +
 .../interface/vchiq_arm/vchiq_dev.c           |  21 --
 6 files changed, 107 insertions(+), 258 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 9fb8f657cc78..7978fb6dc4fb 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -659,13 +659,9 @@ vchiq_complete_bulk(struct vchiq_instance *instance, struct vchiq_bulk *bulk)
 			      bulk->actual);
 }

-int vchiq_dump_platform_state(void *dump_context)
+void vchiq_dump_platform_state(struct seq_file *f)
 {
-	char buf[80];
-	int len;
-
-	len = snprintf(buf, sizeof(buf), "  Platform: 2835 (VC master)");
-	return vchiq_dump(dump_context, buf, len + 1);
+	seq_puts(f, "  Platform: 2835 (VC master)\n");
 }

 #define VCHIQ_INIT_RETRIES 10
@@ -1190,56 +1186,13 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
 		bulk_userdata);
 }

-int vchiq_dump(void *dump_context, const char *str, int len)
-{
-	struct dump_context *context = (struct dump_context *)dump_context;
-	int copy_bytes;
-
-	if (context->actual >= context->space)
-		return 0;
-
-	if (context->offset > 0) {
-		int skip_bytes = min_t(int, len, context->offset);
-
-		str += skip_bytes;
-		len -= skip_bytes;
-		context->offset -= skip_bytes;
-		if (context->offset > 0)
-			return 0;
-	}
-	copy_bytes = min_t(int, len, context->space - context->actual);
-	if (copy_bytes == 0)
-		return 0;
-	if (copy_to_user(context->buf + context->actual, str,
-			 copy_bytes))
-		return -EFAULT;
-	context->actual += copy_bytes;
-	len -= copy_bytes;
-
-	/*
-	 * If the terminating NUL is included in the length, then it
-	 * marks the end of a line and should be replaced with a
-	 * carriage return.
-	 */
-	if ((len == 0) && (str[copy_bytes - 1] == '\0')) {
-		char cr = '\n';
-
-		if (copy_to_user(context->buf + context->actual - 1,
-				 &cr, 1))
-			return -EFAULT;
-	}
-	return 0;
-}
-
-int vchiq_dump_platform_instances(void *dump_context)
+void vchiq_dump_platform_instances(struct seq_file *f)
 {
 	struct vchiq_state *state = vchiq_get_state();
-	char buf[80];
-	int len;
 	int i;

 	if (!state)
-		return -ENOTCONN;
+		return;

 	/*
 	 * There is no list of instances, so instead scan all services,
@@ -1264,7 +1217,6 @@ int vchiq_dump_platform_instances(void *dump_context)
 	for (i = 0; i < state->unused_service; i++) {
 		struct vchiq_service *service;
 		struct vchiq_instance *instance;
-		int err;

 		rcu_read_lock();
 		service = rcu_dereference(state->services[i]);
@@ -1280,43 +1232,35 @@ int vchiq_dump_platform_instances(void *dump_context)
 		}
 		rcu_read_unlock();

-		len = snprintf(buf, sizeof(buf),
-			       "Instance %pK: pid %d,%s completions %d/%d",
-			       instance, instance->pid,
-			       instance->connected ? " connected, " :
-			       "",
-			       instance->completion_insert -
-			       instance->completion_remove,
-			       MAX_COMPLETIONS);
-		err = vchiq_dump(dump_context, buf, len + 1);
-		if (err)
-			return err;
+		seq_printf(f, "Instance %pK: pid %d,%s completions %d/%d\n",
+			   instance, instance->pid,
+			   instance->connected ? " connected, " :
+			   "",
+			   instance->completion_insert -
+			   instance->completion_remove,
+			   MAX_COMPLETIONS);
 		instance->mark = 1;
 	}
-	return 0;
 }

-int vchiq_dump_platform_service_state(void *dump_context,
-				      struct vchiq_service *service)
+void vchiq_dump_platform_service_state(struct seq_file *f,
+				       struct vchiq_service *service)
 {
 	struct user_service *user_service =
 			(struct user_service *)service->base.userdata;
-	char buf[80];
-	int len;

-	len = scnprintf(buf, sizeof(buf), "  instance %pK", service->instance);
+	seq_printf(f, "  instance %pK", service->instance);

 	if ((service->base.callback == service_callback) && user_service->is_vchi) {
-		len += scnprintf(buf + len, sizeof(buf) - len, ", %d/%d messages",
-				 user_service->msg_insert - user_service->msg_remove,
-				 MSG_QUEUE_SIZE);
+		seq_printf(f, ", %d/%d messages",
+			   user_service->msg_insert - user_service->msg_remove,
+			   MSG_QUEUE_SIZE);

 		if (user_service->dequeue_pending)
-			len += scnprintf(buf + len, sizeof(buf) - len,
-				" (dequeue pending)");
+			seq_puts(f, " (dequeue pending)");
 	}

-	return vchiq_dump(dump_context, buf, len + 1);
+	seq_puts(f, "\n");
 }

 struct vchiq_state *
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
index 7cdc3d70bd2c..7844ef765a00 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
@@ -69,13 +69,6 @@ struct vchiq_instance {
 	struct vchiq_debugfs_node debugfs_node;
 };

-struct dump_context {
-	char __user *buf;
-	size_t actual;
-	size_t space;
-	loff_t offset;
-};
-
 extern spinlock_t msg_queue_spinlock;
 extern struct vchiq_state g_state;

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 36c742a2f3b2..6ca83f79622e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -3372,8 +3372,8 @@ vchiq_set_service_option(struct vchiq_instance *instance, unsigned int handle,
 	return ret;
 }

-static int
-vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state,
+static void
+vchiq_dump_shared_state(struct seq_file *f, struct vchiq_state *state,
 			struct vchiq_shared_state *shared, const char *label)
 {
 	static const char *const debug_names[] = {
@@ -3390,57 +3390,37 @@ vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state,
 		"COMPLETION_QUEUE_FULL_COUNT"
 	};
 	int i;
-	char buf[80];
-	int len;
-	int err;
-
-	len = scnprintf(buf, sizeof(buf), "  %s: slots %d-%d tx_pos=%x recycle=%x",
-			label, shared->slot_first, shared->slot_last,
-			shared->tx_pos, shared->slot_queue_recycle);
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	len = scnprintf(buf, sizeof(buf), "    Slots claimed:");
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
+
+	seq_printf(f, "  %s: slots %d-%d tx_pos=%x recycle=%x\n",
+		   label, shared->slot_first, shared->slot_last,
+		   shared->tx_pos, shared->slot_queue_recycle);
+
+	seq_puts(f, "    Slots claimed:\n");

 	for (i = shared->slot_first; i <= shared->slot_last; i++) {
 		struct vchiq_slot_info slot_info =
 						*SLOT_INFO_FROM_INDEX(state, i);
 		if (slot_info.use_count != slot_info.release_count) {
-			len = scnprintf(buf, sizeof(buf), "      %d: %d/%d", i, slot_info.use_count,
-					slot_info.release_count);
-			err = vchiq_dump(dump_context, buf, len + 1);
-			if (err)
-				return err;
+			seq_printf(f, "      %d: %d/%d\n", i, slot_info.use_count,
+				   slot_info.release_count);
 		}
 	}

 	for (i = 1; i < shared->debug[DEBUG_ENTRIES]; i++) {
-		len = scnprintf(buf, sizeof(buf), "    DEBUG: %s = %d(%x)",
-				debug_names[i], shared->debug[i], shared->debug[i]);
-		err = vchiq_dump(dump_context, buf, len + 1);
-		if (err)
-			return err;
+		seq_printf(f, "    DEBUG: %s = %d(%x)\n",
+			   debug_names[i], shared->debug[i], shared->debug[i]);
 	}
-	return 0;
 }

-static int
-vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
+static void
+vchiq_dump_service_state(struct seq_file *f, struct vchiq_service *service)
 {
-	char buf[80];
-	int len;
-	int err;
 	unsigned int ref_count;

 	/*Don't include the lock just taken*/
 	ref_count = kref_read(&service->ref_count) - 1;
-	len = scnprintf(buf, sizeof(buf), "Service %u: %s (ref %u)",
-			service->localport, srvstate_names[service->srvstate],
-			ref_count);
+	seq_printf(f, "Service %u: %s (ref %u)", service->localport,
+		   srvstate_names[service->srvstate], ref_count);

 	if (service->srvstate != VCHIQ_SRVSTATE_FREE) {
 		char remoteport[30];
@@ -3460,15 +3440,10 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
 			strscpy(remoteport, "n/a", sizeof(remoteport));
 		}

-		len += scnprintf(buf + len, sizeof(buf) - len,
-				 " '%p4cc' remote %s (msg use %d/%d, slot use %d/%d)",
-				 &fourcc, remoteport,
-				 quota->message_use_count, quota->message_quota,
-				 quota->slot_use_count, quota->slot_quota);
-
-		err = vchiq_dump(dump_context, buf, len + 1);
-		if (err)
-			return err;
+		seq_printf(f, " '%p4cc' remote %s (msg use %d/%d, slot use %d/%d)\n",
+			   &fourcc, remoteport,
+			   quota->message_use_count, quota->message_quota,
+			   quota->slot_use_count, quota->slot_quota);

 		tx_pending = service->bulk_tx.local_insert -
 			service->bulk_tx.remote_insert;
@@ -3486,130 +3461,79 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
 			rx_size = service->bulk_rx.bulks[i].size;
 		}

-		len = scnprintf(buf, sizeof(buf),
-				"  Bulk: tx_pending=%d (size %d), rx_pending=%d (size %d)",
-				tx_pending, tx_size, rx_pending, rx_size);
+		seq_printf(f, "  Bulk: tx_pending=%d (size %d), rx_pending=%d (size %d)\n",
+			   tx_pending, tx_size, rx_pending, rx_size);

 		if (VCHIQ_ENABLE_STATS) {
-			err = vchiq_dump(dump_context, buf, len + 1);
-			if (err)
-				return err;
+			seq_printf(f, "  Ctrl: tx_count=%d, tx_bytes=%llu, rx_count=%d, rx_bytes=%llu\n",
+				   service->stats.ctrl_tx_count,
+				   service->stats.ctrl_tx_bytes,
+				   service->stats.ctrl_rx_count,
+				   service->stats.ctrl_rx_bytes);

-			len = scnprintf(buf, sizeof(buf),
-					"  Ctrl: tx_count=%d, tx_bytes=%llu, rx_count=%d, rx_bytes=%llu",
-					service->stats.ctrl_tx_count, service->stats.ctrl_tx_bytes,
-					service->stats.ctrl_rx_count, service->stats.ctrl_rx_bytes);
-			err = vchiq_dump(dump_context, buf, len + 1);
-			if (err)
-				return err;
+			seq_printf(f, "  Bulk: tx_count=%d, tx_bytes=%llu, rx_count=%d, rx_bytes=%llu\n",
+				   service->stats.bulk_tx_count,
+				   service->stats.bulk_tx_bytes,
+				   service->stats.bulk_rx_count,
+				   service->stats.bulk_rx_bytes);

-			len = scnprintf(buf, sizeof(buf),
-					"  Bulk: tx_count=%d, tx_bytes=%llu, rx_count=%d, rx_bytes=%llu",
-					service->stats.bulk_tx_count, service->stats.bulk_tx_bytes,
-					service->stats.bulk_rx_count, service->stats.bulk_rx_bytes);
-			err = vchiq_dump(dump_context, buf, len + 1);
-			if (err)
-				return err;
-
-			len = scnprintf(buf, sizeof(buf),
-					"  %d quota stalls, %d slot stalls, %d bulk stalls, %d aborted, %d errors",
-					service->stats.quota_stalls, service->stats.slot_stalls,
-					service->stats.bulk_stalls,
-					service->stats.bulk_aborted_count,
-					service->stats.error_count);
+			seq_printf(f, "  %d quota stalls, %d slot stalls, %d bulk stalls, %d aborted, %d errors\n",
+				   service->stats.quota_stalls,
+				   service->stats.slot_stalls,
+				   service->stats.bulk_stalls,
+				   service->stats.bulk_aborted_count,
+				   service->stats.error_count);
 		}
 	}

-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	if (service->srvstate != VCHIQ_SRVSTATE_FREE)
-		err = vchiq_dump_platform_service_state(dump_context, service);
-	return err;
+	vchiq_dump_platform_service_state(f, service);
 }

-int vchiq_dump_state(void *dump_context, struct vchiq_state *state)
+void vchiq_dump_state(struct seq_file *f, struct vchiq_state *state)
 {
-	char buf[80];
-	int len;
 	int i;
-	int err;
-
-	len = scnprintf(buf, sizeof(buf), "State %d: %s", state->id,
-			conn_state_names[state->conn_state]);
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	len = scnprintf(buf, sizeof(buf), "  tx_pos=%x(@%pK), rx_pos=%x(@%pK)",
-			state->local->tx_pos,
-			state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
-			state->rx_pos,
-			state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK));
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	len = scnprintf(buf, sizeof(buf), "  Version: %d (min %d)",
-			VCHIQ_VERSION, VCHIQ_VERSION_MIN);
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
+
+	seq_printf(f, "State %d: %s\n", state->id,
+		   conn_state_names[state->conn_state]);
+
+	seq_printf(f, "  tx_pos=%x(@%pK), rx_pos=%x(@%pK)\n",
+		   state->local->tx_pos,
+		   state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
+		   state->rx_pos,
+		   state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK));
+
+	seq_printf(f, "  Version: %d (min %d)\n", VCHIQ_VERSION,
+		   VCHIQ_VERSION_MIN);

 	if (VCHIQ_ENABLE_STATS) {
-		len = scnprintf(buf, sizeof(buf),
-				"  Stats: ctrl_tx_count=%d, ctrl_rx_count=%d, error_count=%d",
-				state->stats.ctrl_tx_count, state->stats.ctrl_rx_count,
-				state->stats.error_count);
-		err = vchiq_dump(dump_context, buf, len + 1);
-		if (err)
-			return err;
-	}
-
-	len = scnprintf(buf, sizeof(buf),
-			"  Slots: %d available (%d data), %d recyclable, %d stalls (%d data)",
-			((state->slot_queue_available * VCHIQ_SLOT_SIZE) -
-			state->local_tx_pos) / VCHIQ_SLOT_SIZE,
-			state->data_quota - state->data_use_count,
-			state->local->slot_queue_recycle - state->slot_queue_available,
-			state->stats.slot_stalls, state->stats.data_stalls);
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	err = vchiq_dump_platform_state(dump_context);
-	if (err)
-		return err;
-
-	err = vchiq_dump_shared_state(dump_context,
-				      state,
-				      state->local,
-				      "Local");
-	if (err)
-		return err;
-	err = vchiq_dump_shared_state(dump_context,
-				      state,
-				      state->remote,
-				      "Remote");
-	if (err)
-		return err;
-
-	err = vchiq_dump_platform_instances(dump_context);
-	if (err)
-		return err;
+		seq_printf(f, "  Stats: ctrl_tx_count=%d, ctrl_rx_count=%d, error_count=%d\n",
+			   state->stats.ctrl_tx_count, state->stats.ctrl_rx_count,
+			   state->stats.error_count);
+	}
+
+	seq_printf(f, "  Slots: %d available (%d data), %d recyclable, %d stalls (%d data)\n",
+		   ((state->slot_queue_available * VCHIQ_SLOT_SIZE) -
+		   state->local_tx_pos) / VCHIQ_SLOT_SIZE,
+		   state->data_quota - state->data_use_count,
+		   state->local->slot_queue_recycle - state->slot_queue_available,
+		   state->stats.slot_stalls, state->stats.data_stalls);
+
+	vchiq_dump_platform_state(f);
+
+	vchiq_dump_shared_state(f, state, state->local, "Local");
+
+	vchiq_dump_shared_state(f, state, state->remote, "Remote");
+
+	vchiq_dump_platform_instances(f);

 	for (i = 0; i < state->unused_service; i++) {
 		struct vchiq_service *service = find_service_by_port(state, i);

 		if (service) {
-			err = vchiq_dump_service_state(dump_context, service);
+			vchiq_dump_service_state(f, service);
 			vchiq_service_put(service);
-			if (err)
-				return err;
 		}
 	}
-	return 0;
 }

 int vchiq_send_remote_use(struct vchiq_state *state)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index ea8d58844775..564b5c707267 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -6,6 +6,7 @@

 #include <linux/mutex.h>
 #include <linux/completion.h>
+#include <linux/debugfs.h>
 #include <linux/dev_printk.h>
 #include <linux/kthread.h>
 #include <linux/kref.h>
@@ -504,8 +505,8 @@ vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle, void *
 		    void __user *uoffset, int size, void *userdata, enum vchiq_bulk_mode mode,
 		    enum vchiq_bulk_dir dir);

-extern int
-vchiq_dump_state(void *dump_context, struct vchiq_state *state);
+extern void
+vchiq_dump_state(struct seq_file *f, struct vchiq_state *state);

 extern void
 vchiq_loud_error_header(void);
@@ -561,13 +562,11 @@ void vchiq_complete_bulk(struct vchiq_instance *instance, struct vchiq_bulk *bul

 void remote_event_signal(struct remote_event *event);

-int vchiq_dump(void *dump_context, const char *str, int len);
-
-int vchiq_dump_platform_state(void *dump_context);
+void vchiq_dump_platform_state(struct seq_file *f);

-int vchiq_dump_platform_instances(void *dump_context);
+void vchiq_dump_platform_instances(struct seq_file *f);

-int vchiq_dump_platform_service_state(void *dump_context, struct vchiq_service *service);
+void vchiq_dump_platform_service_state(struct seq_file *f, struct vchiq_service *service);

 int vchiq_use_service_internal(struct vchiq_service *service);

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
index 58db78a9c8d4..d833e4e2973a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
@@ -40,6 +40,13 @@ static int debugfs_trace_show(struct seq_file *f, void *offset)
 	return 0;
 }

+static int vchiq_dump_show(struct seq_file *f, void *offset)
+{
+	vchiq_dump_state(f, &g_state);
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(vchiq_dump);
+
 static int debugfs_trace_open(struct inode *inode, struct file *file)
 {
 	return single_open(file, debugfs_trace_show, inode->i_private);
@@ -115,6 +122,9 @@ void vchiq_debugfs_init(void)
 {
 	vchiq_dbg_dir = debugfs_create_dir("vchiq", NULL);
 	vchiq_dbg_clients = debugfs_create_dir("clients", vchiq_dbg_dir);
+
+	debugfs_create_file("state", S_IFREG | 0444, vchiq_dbg_dir, NULL,
+			    &vchiq_dump_fops);
 }

 /* remove all the debugfs entries */
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
index 0bc93f48c14c..307a2d76cbb7 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
@@ -1306,26 +1306,6 @@ static int vchiq_release(struct inode *inode, struct file *file)
 	return ret;
 }

-static ssize_t
-vchiq_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
-{
-	struct dump_context context;
-	int err;
-
-	context.buf = buf;
-	context.actual = 0;
-	context.space = count;
-	context.offset = *ppos;
-
-	err = vchiq_dump_state(&context, &g_state);
-	if (err)
-		return err;
-
-	*ppos += context.actual;
-
-	return context.actual;
-}
-
 static const struct file_operations
 vchiq_fops = {
 	.owner = THIS_MODULE,
@@ -1335,7 +1315,6 @@ vchiq_fops = {
 #endif
 	.open = vchiq_open,
 	.release = vchiq_release,
-	.read = vchiq_read
 };

 static struct miscdevice vchiq_miscdev = {
--
2.34.1

[PATCH V2 3/3] staging: vchiq_arm: move state dump to debugfs

[Stefan Wahren]

Besides the IOCTL interface the VCHIQ character device also provides
a state dump of the whole VCHIQ driver via read. Moving the state dump
function to debugfs has a lot advantages:

- following changes on state dump doesn't break userspace ABI
- debug doesn't depend on VCHIQ_CDEV
- dump code simplifies a lot and reduce the chance of buffer overflows

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 .../interface/vchiq_arm/vchiq_arm.c           |  94 ++------
 .../interface/vchiq_arm/vchiq_arm.h           |   7 -
 .../interface/vchiq_arm/vchiq_core.c          | 220 ++++++------------
 .../interface/vchiq_arm/vchiq_core.h          |  13 +-
 .../interface/vchiq_arm/vchiq_debugfs.c       |  10 +
 .../interface/vchiq_arm/vchiq_dev.c           |  21 --
 6 files changed, 107 insertions(+), 258 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 9fb8f657cc78..7978fb6dc4fb 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -659,13 +659,9 @@ vchiq_complete_bulk(struct vchiq_instance *instance, struct vchiq_bulk *bulk)
 			      bulk->actual);
 }

-int vchiq_dump_platform_state(void *dump_context)
+void vchiq_dump_platform_state(struct seq_file *f)
 {
-	char buf[80];
-	int len;
-
-	len = snprintf(buf, sizeof(buf), "  Platform: 2835 (VC master)");
-	return vchiq_dump(dump_context, buf, len + 1);
+	seq_puts(f, "  Platform: 2835 (VC master)\n");
 }

 #define VCHIQ_INIT_RETRIES 10
@@ -1190,56 +1186,13 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
 		bulk_userdata);
 }

-int vchiq_dump(void *dump_context, const char *str, int len)
-{
-	struct dump_context *context = (struct dump_context *)dump_context;
-	int copy_bytes;
-
-	if (context->actual >= context->space)
-		return 0;
-
-	if (context->offset > 0) {
-		int skip_bytes = min_t(int, len, context->offset);
-
-		str += skip_bytes;
-		len -= skip_bytes;
-		context->offset -= skip_bytes;
-		if (context->offset > 0)
-			return 0;
-	}
-	copy_bytes = min_t(int, len, context->space - context->actual);
-	if (copy_bytes == 0)
-		return 0;
-	if (copy_to_user(context->buf + context->actual, str,
-			 copy_bytes))
-		return -EFAULT;
-	context->actual += copy_bytes;
-	len -= copy_bytes;
-
-	/*
-	 * If the terminating NUL is included in the length, then it
-	 * marks the end of a line and should be replaced with a
-	 * carriage return.
-	 */
-	if ((len == 0) && (str[copy_bytes - 1] == '\0')) {
-		char cr = '\n';
-
-		if (copy_to_user(context->buf + context->actual - 1,
-				 &cr, 1))
-			return -EFAULT;
-	}
-	return 0;
-}
-
-int vchiq_dump_platform_instances(void *dump_context)
+void vchiq_dump_platform_instances(struct seq_file *f)
 {
 	struct vchiq_state *state = vchiq_get_state();
-	char buf[80];
-	int len;
 	int i;

 	if (!state)
-		return -ENOTCONN;
+		return;

 	/*
 	 * There is no list of instances, so instead scan all services,
@@ -1264,7 +1217,6 @@ int vchiq_dump_platform_instances(void *dump_context)
 	for (i = 0; i < state->unused_service; i++) {
 		struct vchiq_service *service;
 		struct vchiq_instance *instance;
-		int err;

 		rcu_read_lock();
 		service = rcu_dereference(state->services[i]);
@@ -1280,43 +1232,35 @@ int vchiq_dump_platform_instances(void *dump_context)
 		}
 		rcu_read_unlock();

-		len = snprintf(buf, sizeof(buf),
-			       "Instance %pK: pid %d,%s completions %d/%d",
-			       instance, instance->pid,
-			       instance->connected ? " connected, " :
-			       "",
-			       instance->completion_insert -
-			       instance->completion_remove,
-			       MAX_COMPLETIONS);
-		err = vchiq_dump(dump_context, buf, len + 1);
-		if (err)
-			return err;
+		seq_printf(f, "Instance %pK: pid %d,%s completions %d/%d\n",
+			   instance, instance->pid,
+			   instance->connected ? " connected, " :
+			   "",
+			   instance->completion_insert -
+			   instance->completion_remove,
+			   MAX_COMPLETIONS);
 		instance->mark = 1;
 	}
-	return 0;
 }

-int vchiq_dump_platform_service_state(void *dump_context,
-				      struct vchiq_service *service)
+void vchiq_dump_platform_service_state(struct seq_file *f,
+				       struct vchiq_service *service)
 {
 	struct user_service *user_service =
 			(struct user_service *)service->base.userdata;
-	char buf[80];
-	int len;

-	len = scnprintf(buf, sizeof(buf), "  instance %pK", service->instance);
+	seq_printf(f, "  instance %pK", service->instance);

 	if ((service->base.callback == service_callback) && user_service->is_vchi) {
-		len += scnprintf(buf + len, sizeof(buf) - len, ", %d/%d messages",
-				 user_service->msg_insert - user_service->msg_remove,
-				 MSG_QUEUE_SIZE);
+		seq_printf(f, ", %d/%d messages",
+			   user_service->msg_insert - user_service->msg_remove,
+			   MSG_QUEUE_SIZE);

 		if (user_service->dequeue_pending)
-			len += scnprintf(buf + len, sizeof(buf) - len,
-				" (dequeue pending)");
+			seq_puts(f, " (dequeue pending)");
 	}

-	return vchiq_dump(dump_context, buf, len + 1);
+	seq_puts(f, "\n");
 }

 struct vchiq_state *
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
index 7cdc3d70bd2c..7844ef765a00 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
@@ -69,13 +69,6 @@ struct vchiq_instance {
 	struct vchiq_debugfs_node debugfs_node;
 };

-struct dump_context {
-	char __user *buf;
-	size_t actual;
-	size_t space;
-	loff_t offset;
-};
-
 extern spinlock_t msg_queue_spinlock;
 extern struct vchiq_state g_state;

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 36c742a2f3b2..6ca83f79622e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -3372,8 +3372,8 @@ vchiq_set_service_option(struct vchiq_instance *instance, unsigned int handle,
 	return ret;
 }

-static int
-vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state,
+static void
+vchiq_dump_shared_state(struct seq_file *f, struct vchiq_state *state,
 			struct vchiq_shared_state *shared, const char *label)
 {
 	static const char *const debug_names[] = {
@@ -3390,57 +3390,37 @@ vchiq_dump_shared_state(void *dump_context, struct vchiq_state *state,
 		"COMPLETION_QUEUE_FULL_COUNT"
 	};
 	int i;
-	char buf[80];
-	int len;
-	int err;
-
-	len = scnprintf(buf, sizeof(buf), "  %s: slots %d-%d tx_pos=%x recycle=%x",
-			label, shared->slot_first, shared->slot_last,
-			shared->tx_pos, shared->slot_queue_recycle);
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	len = scnprintf(buf, sizeof(buf), "    Slots claimed:");
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
+
+	seq_printf(f, "  %s: slots %d-%d tx_pos=%x recycle=%x\n",
+		   label, shared->slot_first, shared->slot_last,
+		   shared->tx_pos, shared->slot_queue_recycle);
+
+	seq_puts(f, "    Slots claimed:\n");

 	for (i = shared->slot_first; i <= shared->slot_last; i++) {
 		struct vchiq_slot_info slot_info =
 						*SLOT_INFO_FROM_INDEX(state, i);
 		if (slot_info.use_count != slot_info.release_count) {
-			len = scnprintf(buf, sizeof(buf), "      %d: %d/%d", i, slot_info.use_count,
-					slot_info.release_count);
-			err = vchiq_dump(dump_context, buf, len + 1);
-			if (err)
-				return err;
+			seq_printf(f, "      %d: %d/%d\n", i, slot_info.use_count,
+				   slot_info.release_count);
 		}
 	}

 	for (i = 1; i < shared->debug[DEBUG_ENTRIES]; i++) {
-		len = scnprintf(buf, sizeof(buf), "    DEBUG: %s = %d(%x)",
-				debug_names[i], shared->debug[i], shared->debug[i]);
-		err = vchiq_dump(dump_context, buf, len + 1);
-		if (err)
-			return err;
+		seq_printf(f, "    DEBUG: %s = %d(%x)\n",
+			   debug_names[i], shared->debug[i], shared->debug[i]);
 	}
-	return 0;
 }

-static int
-vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
+static void
+vchiq_dump_service_state(struct seq_file *f, struct vchiq_service *service)
 {
-	char buf[80];
-	int len;
-	int err;
 	unsigned int ref_count;

 	/*Don't include the lock just taken*/
 	ref_count = kref_read(&service->ref_count) - 1;
-	len = scnprintf(buf, sizeof(buf), "Service %u: %s (ref %u)",
-			service->localport, srvstate_names[service->srvstate],
-			ref_count);
+	seq_printf(f, "Service %u: %s (ref %u)", service->localport,
+		   srvstate_names[service->srvstate], ref_count);

 	if (service->srvstate != VCHIQ_SRVSTATE_FREE) {
 		char remoteport[30];
@@ -3460,15 +3440,10 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
 			strscpy(remoteport, "n/a", sizeof(remoteport));
 		}

-		len += scnprintf(buf + len, sizeof(buf) - len,
-				 " '%p4cc' remote %s (msg use %d/%d, slot use %d/%d)",
-				 &fourcc, remoteport,
-				 quota->message_use_count, quota->message_quota,
-				 quota->slot_use_count, quota->slot_quota);
-
-		err = vchiq_dump(dump_context, buf, len + 1);
-		if (err)
-			return err;
+		seq_printf(f, " '%p4cc' remote %s (msg use %d/%d, slot use %d/%d)\n",
+			   &fourcc, remoteport,
+			   quota->message_use_count, quota->message_quota,
+			   quota->slot_use_count, quota->slot_quota);

 		tx_pending = service->bulk_tx.local_insert -
 			service->bulk_tx.remote_insert;
@@ -3486,130 +3461,79 @@ vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
 			rx_size = service->bulk_rx.bulks[i].size;
 		}

-		len = scnprintf(buf, sizeof(buf),
-				"  Bulk: tx_pending=%d (size %d), rx_pending=%d (size %d)",
-				tx_pending, tx_size, rx_pending, rx_size);
+		seq_printf(f, "  Bulk: tx_pending=%d (size %d), rx_pending=%d (size %d)\n",
+			   tx_pending, tx_size, rx_pending, rx_size);

 		if (VCHIQ_ENABLE_STATS) {
-			err = vchiq_dump(dump_context, buf, len + 1);
-			if (err)
-				return err;
+			seq_printf(f, "  Ctrl: tx_count=%d, tx_bytes=%llu, rx_count=%d, rx_bytes=%llu\n",
+				   service->stats.ctrl_tx_count,
+				   service->stats.ctrl_tx_bytes,
+				   service->stats.ctrl_rx_count,
+				   service->stats.ctrl_rx_bytes);

-			len = scnprintf(buf, sizeof(buf),
-					"  Ctrl: tx_count=%d, tx_bytes=%llu, rx_count=%d, rx_bytes=%llu",
-					service->stats.ctrl_tx_count, service->stats.ctrl_tx_bytes,
-					service->stats.ctrl_rx_count, service->stats.ctrl_rx_bytes);
-			err = vchiq_dump(dump_context, buf, len + 1);
-			if (err)
-				return err;
+			seq_printf(f, "  Bulk: tx_count=%d, tx_bytes=%llu, rx_count=%d, rx_bytes=%llu\n",
+				   service->stats.bulk_tx_count,
+				   service->stats.bulk_tx_bytes,
+				   service->stats.bulk_rx_count,
+				   service->stats.bulk_rx_bytes);

-			len = scnprintf(buf, sizeof(buf),
-					"  Bulk: tx_count=%d, tx_bytes=%llu, rx_count=%d, rx_bytes=%llu",
-					service->stats.bulk_tx_count, service->stats.bulk_tx_bytes,
-					service->stats.bulk_rx_count, service->stats.bulk_rx_bytes);
-			err = vchiq_dump(dump_context, buf, len + 1);
-			if (err)
-				return err;
-
-			len = scnprintf(buf, sizeof(buf),
-					"  %d quota stalls, %d slot stalls, %d bulk stalls, %d aborted, %d errors",
-					service->stats.quota_stalls, service->stats.slot_stalls,
-					service->stats.bulk_stalls,
-					service->stats.bulk_aborted_count,
-					service->stats.error_count);
+			seq_printf(f, "  %d quota stalls, %d slot stalls, %d bulk stalls, %d aborted, %d errors\n",
+				   service->stats.quota_stalls,
+				   service->stats.slot_stalls,
+				   service->stats.bulk_stalls,
+				   service->stats.bulk_aborted_count,
+				   service->stats.error_count);
 		}
 	}

-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	if (service->srvstate != VCHIQ_SRVSTATE_FREE)
-		err = vchiq_dump_platform_service_state(dump_context, service);
-	return err;
+	vchiq_dump_platform_service_state(f, service);
 }

-int vchiq_dump_state(void *dump_context, struct vchiq_state *state)
+void vchiq_dump_state(struct seq_file *f, struct vchiq_state *state)
 {
-	char buf[80];
-	int len;
 	int i;
-	int err;
-
-	len = scnprintf(buf, sizeof(buf), "State %d: %s", state->id,
-			conn_state_names[state->conn_state]);
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	len = scnprintf(buf, sizeof(buf), "  tx_pos=%x(@%pK), rx_pos=%x(@%pK)",
-			state->local->tx_pos,
-			state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
-			state->rx_pos,
-			state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK));
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	len = scnprintf(buf, sizeof(buf), "  Version: %d (min %d)",
-			VCHIQ_VERSION, VCHIQ_VERSION_MIN);
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
+
+	seq_printf(f, "State %d: %s\n", state->id,
+		   conn_state_names[state->conn_state]);
+
+	seq_printf(f, "  tx_pos=%x(@%pK), rx_pos=%x(@%pK)\n",
+		   state->local->tx_pos,
+		   state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
+		   state->rx_pos,
+		   state->rx_data + (state->rx_pos & VCHIQ_SLOT_MASK));
+
+	seq_printf(f, "  Version: %d (min %d)\n", VCHIQ_VERSION,
+		   VCHIQ_VERSION_MIN);

 	if (VCHIQ_ENABLE_STATS) {
-		len = scnprintf(buf, sizeof(buf),
-				"  Stats: ctrl_tx_count=%d, ctrl_rx_count=%d, error_count=%d",
-				state->stats.ctrl_tx_count, state->stats.ctrl_rx_count,
-				state->stats.error_count);
-		err = vchiq_dump(dump_context, buf, len + 1);
-		if (err)
-			return err;
-	}
-
-	len = scnprintf(buf, sizeof(buf),
-			"  Slots: %d available (%d data), %d recyclable, %d stalls (%d data)",
-			((state->slot_queue_available * VCHIQ_SLOT_SIZE) -
-			state->local_tx_pos) / VCHIQ_SLOT_SIZE,
-			state->data_quota - state->data_use_count,
-			state->local->slot_queue_recycle - state->slot_queue_available,
-			state->stats.slot_stalls, state->stats.data_stalls);
-	err = vchiq_dump(dump_context, buf, len + 1);
-	if (err)
-		return err;
-
-	err = vchiq_dump_platform_state(dump_context);
-	if (err)
-		return err;
-
-	err = vchiq_dump_shared_state(dump_context,
-				      state,
-				      state->local,
-				      "Local");
-	if (err)
-		return err;
-	err = vchiq_dump_shared_state(dump_context,
-				      state,
-				      state->remote,
-				      "Remote");
-	if (err)
-		return err;
-
-	err = vchiq_dump_platform_instances(dump_context);
-	if (err)
-		return err;
+		seq_printf(f, "  Stats: ctrl_tx_count=%d, ctrl_rx_count=%d, error_count=%d\n",
+			   state->stats.ctrl_tx_count, state->stats.ctrl_rx_count,
+			   state->stats.error_count);
+	}
+
+	seq_printf(f, "  Slots: %d available (%d data), %d recyclable, %d stalls (%d data)\n",
+		   ((state->slot_queue_available * VCHIQ_SLOT_SIZE) -
+		   state->local_tx_pos) / VCHIQ_SLOT_SIZE,
+		   state->data_quota - state->data_use_count,
+		   state->local->slot_queue_recycle - state->slot_queue_available,
+		   state->stats.slot_stalls, state->stats.data_stalls);
+
+	vchiq_dump_platform_state(f);
+
+	vchiq_dump_shared_state(f, state, state->local, "Local");
+
+	vchiq_dump_shared_state(f, state, state->remote, "Remote");
+
+	vchiq_dump_platform_instances(f);

 	for (i = 0; i < state->unused_service; i++) {
 		struct vchiq_service *service = find_service_by_port(state, i);

 		if (service) {
-			err = vchiq_dump_service_state(dump_context, service);
+			vchiq_dump_service_state(f, service);
 			vchiq_service_put(service);
-			if (err)
-				return err;
 		}
 	}
-	return 0;
 }

 int vchiq_send_remote_use(struct vchiq_state *state)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
index ea8d58844775..564b5c707267 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -6,6 +6,7 @@

 #include <linux/mutex.h>
 #include <linux/completion.h>
+#include <linux/debugfs.h>
 #include <linux/dev_printk.h>
 #include <linux/kthread.h>
 #include <linux/kref.h>
@@ -504,8 +505,8 @@ vchiq_bulk_transfer(struct vchiq_instance *instance, unsigned int handle, void *
 		    void __user *uoffset, int size, void *userdata, enum vchiq_bulk_mode mode,
 		    enum vchiq_bulk_dir dir);

-extern int
-vchiq_dump_state(void *dump_context, struct vchiq_state *state);
+extern void
+vchiq_dump_state(struct seq_file *f, struct vchiq_state *state);

 extern void
 vchiq_loud_error_header(void);
@@ -561,13 +562,11 @@ void vchiq_complete_bulk(struct vchiq_instance *instance, struct vchiq_bulk *bul

 void remote_event_signal(struct remote_event *event);

-int vchiq_dump(void *dump_context, const char *str, int len);
-
-int vchiq_dump_platform_state(void *dump_context);
+void vchiq_dump_platform_state(struct seq_file *f);

-int vchiq_dump_platform_instances(void *dump_context);
+void vchiq_dump_platform_instances(struct seq_file *f);

-int vchiq_dump_platform_service_state(void *dump_context, struct vchiq_service *service);
+void vchiq_dump_platform_service_state(struct seq_file *f, struct vchiq_service *service);

 int vchiq_use_service_internal(struct vchiq_service *service);

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
index 58db78a9c8d4..d833e4e2973a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
@@ -40,6 +40,13 @@ static int debugfs_trace_show(struct seq_file *f, void *offset)
 	return 0;
 }

+static int vchiq_dump_show(struct seq_file *f, void *offset)
+{
+	vchiq_dump_state(f, &g_state);
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(vchiq_dump);
+
 static int debugfs_trace_open(struct inode *inode, struct file *file)
 {
 	return single_open(file, debugfs_trace_show, inode->i_private);
@@ -115,6 +122,9 @@ void vchiq_debugfs_init(void)
 {
 	vchiq_dbg_dir = debugfs_create_dir("vchiq", NULL);
 	vchiq_dbg_clients = debugfs_create_dir("clients", vchiq_dbg_dir);
+
+	debugfs_create_file("state", S_IFREG | 0444, vchiq_dbg_dir, NULL,
+			    &vchiq_dump_fops);
 }

 /* remove all the debugfs entries */
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
index 0bc93f48c14c..307a2d76cbb7 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
@@ -1306,26 +1306,6 @@ static int vchiq_release(struct inode *inode, struct file *file)
 	return ret;
 }

-static ssize_t
-vchiq_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
-{
-	struct dump_context context;
-	int err;
-
-	context.buf = buf;
-	context.actual = 0;
-	context.space = count;
-	context.offset = *ppos;
-
-	err = vchiq_dump_state(&context, &g_state);
-	if (err)
-		return err;
-
-	*ppos += context.actual;
-
-	return context.actual;
-}
-
 static const struct file_operations
 vchiq_fops = {
 	.owner = THIS_MODULE,
@@ -1335,7 +1315,6 @@ vchiq_fops = {
 #endif
 	.open = vchiq_open,
 	.release = vchiq_release,
-	.read = vchiq_read
 };

 static struct miscdevice vchiq_miscdev = {
--
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 3/3] staging: vchiq_arm: move state dump to debugfs

[Ricardo B. Marliere]

Tested on both aarch64 and armv7l on a RPi 3B using the vchiq_test tool.

/sys/kernel/debug/vchiq/state
/sys/kernel/debug/vchiq/clients/**/trace
/sys/kernel/debug/vchiq/clients/**/use_count
contents are fine.

Tested-by: Ricardo B. Marliere <ricardo@marliere.net>

Re: [PATCH V2 3/3] staging: vchiq_arm: move state dump to debugfs

[Ricardo B. Marliere]

Tested on both aarch64 and armv7l on a RPi 3B using the vchiq_test tool.

/sys/kernel/debug/vchiq/state
/sys/kernel/debug/vchiq/clients/**/trace
/sys/kernel/debug/vchiq/clients/**/use_count
contents are fine.

Tested-by: Ricardo B. Marliere <ricardo@marliere.net>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 0/3] staging: vchiq_arm: move state dump to debugfs

[Stefan Wahren]

Hi Greg,

Am 29.10.23 um 13:48 schrieb Stefan Wahren:
> Hello,
>
> since recent discussion raised the question about the future of debugfs
> for vchiq [1], i want to submit this cleanup patch series as part of the
> discussion and a small Halloween present ;-)
>
> Best regards
>
> Changes in V2:
> - rebase on top of current staging-next
> - address suggestion from Laurent Pinchart in patch 1
> - fix checkpatch issue (too long line) in patch 2
>
> [1] - https://lore.kernel.org/lkml/7ea529c2-3da6-47df-9b09-28d4ab36c4ef@kadam.mountain/T/
>
> Stefan Wahren (3):
>    staging: vchiq_core: Make vchiq_dump_service_state static
>    staging: vchiq_core: Shorten bulk TX/RX pending dump
>    staging: vchiq_arm: move state dump to debugfs

should i resend incl. the received Reviewed-by tags?

>
>   .../interface/vchiq_arm/vchiq_arm.c           |  94 ++----
>   .../interface/vchiq_arm/vchiq_arm.h           |   7 -
>   .../interface/vchiq_arm/vchiq_core.c          | 274 +++++++-----------
>   .../interface/vchiq_arm/vchiq_core.h          |  16 +-
>   .../interface/vchiq_arm/vchiq_debugfs.c       |  10 +
>   .../interface/vchiq_arm/vchiq_dev.c           |  21 --
>   6 files changed, 137 insertions(+), 285 deletions(-)
>
> --
> 2.34.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 0/3] staging: vchiq_arm: move state dump to debugfs

[Stefan Wahren]

Hi Greg,

Am 29.10.23 um 13:48 schrieb Stefan Wahren:
> Hello,
>
> since recent discussion raised the question about the future of debugfs
> for vchiq [1], i want to submit this cleanup patch series as part of the
> discussion and a small Halloween present ;-)
>
> Best regards
>
> Changes in V2:
> - rebase on top of current staging-next
> - address suggestion from Laurent Pinchart in patch 1
> - fix checkpatch issue (too long line) in patch 2
>
> [1] - https://lore.kernel.org/lkml/7ea529c2-3da6-47df-9b09-28d4ab36c4ef@kadam.mountain/T/
>
> Stefan Wahren (3):
>    staging: vchiq_core: Make vchiq_dump_service_state static
>    staging: vchiq_core: Shorten bulk TX/RX pending dump
>    staging: vchiq_arm: move state dump to debugfs

should i resend incl. the received Reviewed-by tags?

>
>   .../interface/vchiq_arm/vchiq_arm.c           |  94 ++----
>   .../interface/vchiq_arm/vchiq_arm.h           |   7 -
>   .../interface/vchiq_arm/vchiq_core.c          | 274 +++++++-----------
>   .../interface/vchiq_arm/vchiq_core.h          |  16 +-
>   .../interface/vchiq_arm/vchiq_debugfs.c       |  10 +
>   .../interface/vchiq_arm/vchiq_dev.c           |  21 --
>   6 files changed, 137 insertions(+), 285 deletions(-)
>
> --
> 2.34.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 0/3] staging: vchiq_arm: move state dump to debugfs

[Greg Kroah-Hartman]

On Wed, Nov 22, 2023 at 01:17:07PM +0100, Stefan Wahren wrote:
> Hi Greg,
> 
> Am 29.10.23 um 13:48 schrieb Stefan Wahren:
> > Hello,
> > 
> > since recent discussion raised the question about the future of debugfs
> > for vchiq [1], i want to submit this cleanup patch series as part of the
> > discussion and a small Halloween present ;-)
> > 
> > Best regards
> > 
> > Changes in V2:
> > - rebase on top of current staging-next
> > - address suggestion from Laurent Pinchart in patch 1
> > - fix checkpatch issue (too long line) in patch 2
> > 
> > [1] - https://lore.kernel.org/lkml/7ea529c2-3da6-47df-9b09-28d4ab36c4ef@kadam.mountain/T/
> > 
> > Stefan Wahren (3):
> >    staging: vchiq_core: Make vchiq_dump_service_state static
> >    staging: vchiq_core: Shorten bulk TX/RX pending dump
> >    staging: vchiq_arm: move state dump to debugfs
> 
> should i resend incl. the received Reviewed-by tags?

No, just wait for me to catch up with staging patches.  Should be a week
or so...

tjhanks,

greg k-h

Re: [PATCH V2 0/3] staging: vchiq_arm: move state dump to debugfs

[Greg Kroah-Hartman]

On Wed, Nov 22, 2023 at 01:17:07PM +0100, Stefan Wahren wrote:
> Hi Greg,
> 
> Am 29.10.23 um 13:48 schrieb Stefan Wahren:
> > Hello,
> > 
> > since recent discussion raised the question about the future of debugfs
> > for vchiq [1], i want to submit this cleanup patch series as part of the
> > discussion and a small Halloween present ;-)
> > 
> > Best regards
> > 
> > Changes in V2:
> > - rebase on top of current staging-next
> > - address suggestion from Laurent Pinchart in patch 1
> > - fix checkpatch issue (too long line) in patch 2
> > 
> > [1] - https://lore.kernel.org/lkml/7ea529c2-3da6-47df-9b09-28d4ab36c4ef@kadam.mountain/T/
> > 
> > Stefan Wahren (3):
> >    staging: vchiq_core: Make vchiq_dump_service_state static
> >    staging: vchiq_core: Shorten bulk TX/RX pending dump
> >    staging: vchiq_arm: move state dump to debugfs
> 
> should i resend incl. the received Reviewed-by tags?

No, just wait for me to catch up with staging patches.  Should be a week
or so...

tjhanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel