[PATCH 00/20] staging: vchiq_arm: more code clean-up

[PATCH 00/20] staging: vchiq_arm: more code clean-up

[Stefan Wahren]

This series mostly addresses the code readability. Additionally it contains
a follow-up patch for a change from my last series.

Stefan Wahren (20):
  staging: vchiq_core: fix return type of vchiq_init_state
  staging: vchiq_core: drop unnecessary release_count
  staging: vchiq_core: separate postfix increment
  staging: vc04_services: remove __VCCOREVER__
  staging: vchiq_arm: balance braces for if-else statements
  staging: vchiq_core: introduce poll_services_of_group
  staging: vchiq_core: avoid indention in poll_services_of_group
  staging: vchiq_arm: Use define for doorbell irq
  staging: vchiq_arm: drop ftrace-like logging
  staging: vchiq_arm: Prefer kzalloc(sizeof(*waiter)...)
  staging: vchiq_arm: drop non-beneficial comments
  staging: vchiq_arm: add blank line after declarations
  staging: vchiq_arm: re-arrange function header
  staging: vchiq_core: reduce indention in release_service_messages
  staging: vchiq_core: fix comment in vchiq_shutdown_internal
  staging: vchiq_arm: make vchiq_shutdown_internal return void
  staging: vchiq_arm: Avoid unnecessary line breaks
  staging: vchiq_core: introduce parse_message
  staging: vchiq_core: introduce defines for close_recvd
  staging: vchiq_core: introduce defines for retry_poll

 drivers/staging/vc04_services/Makefile             |   2 +-
 .../interface/vchiq_arm/vchiq_2835_arm.c           |   4 +-
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 175 ++--
 .../interface/vchiq_arm/vchiq_connected.h          |   8 -
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 897 +++++++++++----------
 .../vc04_services/interface/vchiq_arm/vchiq_core.h |   2 +-
 .../interface/vchiq_arm/vchiq_debugfs.c            |   6 +-
 7 files changed, 513 insertions(+), 581 deletions(-)

-- 
2.7.4

[PATCH 01/20] staging: vchiq_core: fix return type of vchiq_init_state

[Stefan Wahren]

Recent commit "staging: vchiq_core: drop vchiq_status from vchiq_init_state"
missed to change the return type in the definition. Let's fix this now.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 ff85327..9b6c626 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -2157,7 +2157,7 @@ vchiq_init_slots(void *mem_base, int mem_size)
 	return slot_zero;
 }
 
-enum vchiq_status
+int
 vchiq_init_state(struct vchiq_state *state, struct vchiq_slot_zero *slot_zero)
 {
 	struct vchiq_shared_state *local;
-- 
2.7.4

Re: [PATCH 01/20] staging: vchiq_core: fix return type of vchiq_init_state

[Fabio Aiuto]

Hi Stefan,

On Sat, May 15, 2021 at 09:10:40PM +0200, Stefan Wahren wrote:
> Recent commit "staging: vchiq_core: drop vchiq_status from vchiq_init_state"
> missed to change the return type in the definition. Let's fix this now.

if this patch fixes something that a previous commit broke,
it's better adding Fixes: tag

> 
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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 ff85327..9b6c626 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -2157,7 +2157,7 @@ vchiq_init_slots(void *mem_base, int mem_size)
>  	return slot_zero;
>  }
>  
> -enum vchiq_status
> +int
>  vchiq_init_state(struct vchiq_state *state, struct vchiq_slot_zero *slot_zero)
>  {
>  	struct vchiq_shared_state *local;
> -- 
> 2.7.4
> 
> 

thank you,

fabio

Re: [PATCH 01/20] staging: vchiq_core: fix return type of vchiq_init_state

[Stefan Wahren]

Hi Fabio,

Am 16.05.21 um 09:27 schrieb Fabio Aiuto:
> Hi Stefan,
>
> On Sat, May 15, 2021 at 09:10:40PM +0200, Stefan Wahren wrote:
>> Recent commit "staging: vchiq_core: drop vchiq_status from vchiq_init_state"
>> missed to change the return type in the definition. Let's fix this now.
> if this patch fixes something that a previous commit broke,
> it's better adding Fixes: tag

the mentioned commit is still in next, so a Fixes tag doesn't make sense
to me. Or do i miss something?

Stefan

>
>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>> ---
>>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> 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 ff85327..9b6c626 100644
>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
>> @@ -2157,7 +2157,7 @@ vchiq_init_slots(void *mem_base, int mem_size)
>>  	return slot_zero;
>>  }
>>  
>> -enum vchiq_status
>> +int
>>  vchiq_init_state(struct vchiq_state *state, struct vchiq_slot_zero *slot_zero)
>>  {
>>  	struct vchiq_shared_state *local;
>> -- 
>> 2.7.4
>>
>>
> thank you,
>
> fabio

Re: [PATCH 01/20] staging: vchiq_core: fix return type of vchiq_init_state

[Fabio Aiuto]

On Sun, May 16, 2021 at 11:38:10AM +0200, Stefan Wahren wrote:
> Hi Fabio,
> 
> Am 16.05.21 um 09:27 schrieb Fabio Aiuto:
> > Hi Stefan,
> >
> > On Sat, May 15, 2021 at 09:10:40PM +0200, Stefan Wahren wrote:
> >> Recent commit "staging: vchiq_core: drop vchiq_status from vchiq_init_state"
> >> missed to change the return type in the definition. Let's fix this now.
> > if this patch fixes something that a previous commit broke,
> > it's better adding Fixes: tag
> 
> the mentioned commit is still in next, so a Fixes tag doesn't make sense
> to me. Or do i miss something?

ok, recently I've been told to add a Fixes: tag to a patch fixing a
previous not mainlined commit (staging-next).

https://lore.kernel.org/linux-staging/YF3qSsH%2F3vRy7BRy@kroah.com/

but I understand what you mean, let's see :)

> 
> Stefan
> 
> >
> >> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> >> ---
> >>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> 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 ff85327..9b6c626 100644
> >> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> >> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> >> @@ -2157,7 +2157,7 @@ vchiq_init_slots(void *mem_base, int mem_size)
> >>  	return slot_zero;
> >>  }
> >>  
> >> -enum vchiq_status
> >> +int
> >>  vchiq_init_state(struct vchiq_state *state, struct vchiq_slot_zero *slot_zero)
> >>  {
> >>  	struct vchiq_shared_state *local;
> >> -- 
> >> 2.7.4
> >>
> >>
> > thank you,
> >
> > fabio

thank you,

fabio

Re: [PATCH 01/20] staging: vchiq_core: fix return type of vchiq_init_state

[Dan Carpenter]

On Sun, May 16, 2021 at 11:38:10AM +0200, Stefan Wahren wrote:
> Hi Fabio,
> 
> Am 16.05.21 um 09:27 schrieb Fabio Aiuto:
> > Hi Stefan,
> >
> > On Sat, May 15, 2021 at 09:10:40PM +0200, Stefan Wahren wrote:
> >> Recent commit "staging: vchiq_core: drop vchiq_status from vchiq_init_state"
> >> missed to change the return type in the definition. Let's fix this now.
> > if this patch fixes something that a previous commit broke,
> > it's better adding Fixes: tag
> 
> the mentioned commit is still in next, so a Fixes tag doesn't make sense
> to me. Or do i miss something?
> 

This patch doesn't affect runtime so a Fixes tag is not strictly
required although probably most of the times we would add it.  But it
doesn't matter whether or not the commit is in -next or not.  The git
hash from the fixes tag will remain valid unless Greg rebases (which he
seldom does).  And if a maintainer rebases their tree, they are
responsible for updating the Fixes tag.

Fixes tags are useful for backporting because you can see that the bug
was introduced in the latest release so the patch doesn't need to be
backported.

But it's not all about backporting they're also useful for review so we
can see why a patch was introduced.  Or sometimes I like to look at the
fixes tags and see if "cleanup" patches are introducing bugs into
staging etc.

regards,
dan carpenter

[PATCH 02/20] staging: vchiq_core: drop unnecessary release_count

[Stefan Wahren]

There is no benefit of the variable release_count, so drop it.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 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 9b6c626..85fd0a6 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -1175,7 +1175,6 @@ static void
 release_slot(struct vchiq_state *state, struct vchiq_slot_info *slot_info,
 	     struct vchiq_header *header, struct vchiq_service *service)
 {
-	int release_count;
 
 	mutex_lock(&state->recycle_mutex);
 
@@ -1192,10 +1191,9 @@ release_slot(struct vchiq_state *state, struct vchiq_slot_info *slot_info,
 		header->msgid = msgid & ~VCHIQ_MSGID_CLAIMED;
 	}
 
-	release_count = slot_info->release_count;
-	slot_info->release_count = ++release_count;
+	slot_info->release_count++;
 
-	if (release_count == slot_info->use_count) {
+	if (slot_info->release_count == slot_info->use_count) {
 		int slot_queue_recycle;
 		/* Add to the freed queue */
 
-- 
2.7.4

[PATCH 03/20] staging: vchiq_core: separate postfix increment

[Stefan Wahren]

Postfix increment within a complexer statement doesn't improve readability.
So separate them.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c       | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 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 85fd0a6..a22d8b7 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -243,7 +243,8 @@ __next_service_by_instance(struct vchiq_state *state,
 	while (idx < state->unused_service) {
 		struct vchiq_service *srv;
 
-		srv = rcu_dereference(state->services[idx++]);
+		srv = rcu_dereference(state->services[idx]);
+		idx++;
 		if (srv && srv->srvstate != VCHIQ_SRVSTATE_FREE &&
 		    srv->instance == instance) {
 			service = srv;
@@ -649,11 +650,12 @@ process_free_queue(struct vchiq_state *state, BITSET_T *service_found,
 
 	while (slot_queue_available != local->slot_queue_recycle) {
 		unsigned int pos;
-		int slot_index = local->slot_queue[slot_queue_available++ &
+		int slot_index = local->slot_queue[slot_queue_available &
 			VCHIQ_SLOT_QUEUE_MASK];
 		char *data = (char *)SLOT_DATA_FROM_INDEX(state, slot_index);
 		int data_found = 0;
 
+		slot_queue_available++;
 		/*
 		 * Beware of the address dependency - data is calculated
 		 * using an index written by the other side.
@@ -1175,7 +1177,6 @@ static void
 release_slot(struct vchiq_state *state, struct vchiq_slot_info *slot_info,
 	     struct vchiq_header *header, struct vchiq_service *service)
 {
-
 	mutex_lock(&state->recycle_mutex);
 
 	if (header) {
@@ -2215,7 +2216,8 @@ vchiq_init_state(struct vchiq_state *state, struct vchiq_slot_zero *slot_zero)
 	}
 
 	for (i = local->slot_first; i <= local->slot_last; i++) {
-		local->slot_queue[state->slot_queue_available++] = i;
+		local->slot_queue[state->slot_queue_available] = i;
+		state->slot_queue_available++;
 		complete(&state->slot_available_event);
 	}
 
@@ -2319,7 +2321,8 @@ void vchiq_msg_queue_push(unsigned int handle, struct vchiq_header *header)
 			flush_signals(current);
 	}
 
-	pos = service->msg_queue_write++ & (VCHIQ_MAX_SLOTS - 1);
+	pos = service->msg_queue_write & (VCHIQ_MAX_SLOTS - 1);
+	service->msg_queue_write++;
 	service->msg_queue[pos] = header;
 
 	complete(&service->msg_queue_push);
@@ -2340,7 +2343,8 @@ struct vchiq_header *vchiq_msg_hold(unsigned int handle)
 			flush_signals(current);
 	}
 
-	pos = service->msg_queue_read++ & (VCHIQ_MAX_SLOTS - 1);
+	pos = service->msg_queue_read & (VCHIQ_MAX_SLOTS - 1);
+	service->msg_queue_read++;
 	header = service->msg_queue[pos];
 
 	complete(&service->msg_queue_pop);
-- 
2.7.4

Re: [PATCH 03/20] staging: vchiq_core: separate postfix increment

[Fabio Aiuto]

Hi Stefan,

On Sat, May 15, 2021 at 09:10:42PM +0200, Stefan Wahren wrote:
> Postfix increment within a complexer statement doesn't improve readability.
> So separate them.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
>  .../vc04_services/interface/vchiq_arm/vchiq_core.c       | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 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 85fd0a6..a22d8b7 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -243,7 +243,8 @@ __next_service_by_instance(struct vchiq_state *state,
>  	while (idx < state->unused_service) {
>  		struct vchiq_service *srv;
>  
> -		srv = rcu_dereference(state->services[idx++]);
> +		srv = rcu_dereference(state->services[idx]);
> +		idx++;
>  		if (srv && srv->srvstate != VCHIQ_SRVSTATE_FREE &&
>  		    srv->instance == instance) {
>  			service = srv;
> @@ -649,11 +650,12 @@ process_free_queue(struct vchiq_state *state, BITSET_T *service_found,
>  
>  	while (slot_queue_available != local->slot_queue_recycle) {
>  		unsigned int pos;
> -		int slot_index = local->slot_queue[slot_queue_available++ &
> +		int slot_index = local->slot_queue[slot_queue_available &
>  			VCHIQ_SLOT_QUEUE_MASK];
>  		char *data = (char *)SLOT_DATA_FROM_INDEX(state, slot_index);
>  		int data_found = 0;
>  
> +		slot_queue_available++;
>  		/*
>  		 * Beware of the address dependency - data is calculated
>  		 * using an index written by the other side.
> @@ -1175,7 +1177,6 @@ static void
>  release_slot(struct vchiq_state *state, struct vchiq_slot_info *slot_info,
>  	     struct vchiq_header *header, struct vchiq_service *service)
>  {
> -
>  	mutex_lock(&state->recycle_mutex);

this change is a separate logical change so should go in a separate patch...

>  
>  	if (header) {
> @@ -2215,7 +2216,8 @@ vchiq_init_state(struct vchiq_state *state, struct vchiq_slot_zero *slot_zero)
>  	}
>  
>  	for (i = local->slot_first; i <= local->slot_last; i++) {
> -		local->slot_queue[state->slot_queue_available++] = i;
> +		local->slot_queue[state->slot_queue_available] = i;
> +		state->slot_queue_available++;
>  		complete(&state->slot_available_event);
>  	}
>  
> @@ -2319,7 +2321,8 @@ void vchiq_msg_queue_push(unsigned int handle, struct vchiq_header *header)
>  			flush_signals(current);
>  	}
>  
> -	pos = service->msg_queue_write++ & (VCHIQ_MAX_SLOTS - 1);
> +	pos = service->msg_queue_write & (VCHIQ_MAX_SLOTS - 1);
> +	service->msg_queue_write++;
>  	service->msg_queue[pos] = header;
>  
>  	complete(&service->msg_queue_push);
> @@ -2340,7 +2343,8 @@ struct vchiq_header *vchiq_msg_hold(unsigned int handle)
>  			flush_signals(current);
>  	}
>  
> -	pos = service->msg_queue_read++ & (VCHIQ_MAX_SLOTS - 1);
> +	pos = service->msg_queue_read & (VCHIQ_MAX_SLOTS - 1);
> +	service->msg_queue_read++;
>  	header = service->msg_queue[pos];
>  
>  	complete(&service->msg_queue_pop);
> -- 
> 2.7.4
> 
> 

thank you,

fabio

[PATCH 04/20] staging: vc04_services: remove __VCCOREVER__

[Stefan Wahren]

This define isn't used anymore. Let's remove it.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/staging/vc04_services/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/Makefile b/drivers/staging/vc04_services/Makefile
index 7546d70..e21e73a 100644
--- a/drivers/staging/vc04_services/Makefile
+++ b/drivers/staging/vc04_services/Makefile
@@ -12,5 +12,5 @@ obj-$(CONFIG_SND_BCM2835)		+= bcm2835-audio/
 obj-$(CONFIG_VIDEO_BCM2835)		+= bcm2835-camera/
 obj-$(CONFIG_BCM2835_VCHIQ_MMAL)	+= vchiq-mmal/
 
-ccflags-y += -I $(srctree)/$(src)/include  -D__VCCOREVER__=0x04000000
+ccflags-y += -I $(srctree)/$(src)/include
 
-- 
2.7.4

[PATCH 05/20] staging: vchiq_arm: balance braces for if-else statements

[Stefan Wahren]

This fixes the following checkpatch notices in vchiq_arm:
CHECK: braces {} should be used on all arms of this statement

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 12 ++++---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 41 +++++++++++++---------
 2 files changed, 32 insertions(+), 21 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 1b13568..7951324 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -302,8 +302,9 @@ static enum vchiq_status vchiq_add_service(
 	if (service) {
 		*phandle = service->handle;
 		status = VCHIQ_SUCCESS;
-	} else
+	} else {
 		status = VCHIQ_ERROR;
+	}
 
 	vchiq_log_trace(vchiq_core_log_level,
 		"%s(%p): returning %d", __func__, instance, status);
@@ -942,8 +943,9 @@ static int vchiq_ioc_dequeue_message(struct vchiq_instance *instance,
 					header->data, header->size) == 0)) {
 			ret = header->size;
 			vchiq_release_message(service->handle, header);
-		} else
+		} else {
 			ret = -EFAULT;
+		}
 	} else {
 		vchiq_log_error(vchiq_arm_log_level,
 			"header %pK: bufsize %x < size %x",
@@ -1401,8 +1403,9 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 						service->base.fourcc),
 					service->client_id);
 			}
-		} else
+		} else {
 			ret = -EINVAL;
+		}
 	} break;
 
 	case VCHIQ_IOC_QUEUE_MESSAGE: {
@@ -1539,8 +1542,9 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 			struct user_service *user_service =
 				(struct user_service *)service->base.userdata;
 			close_delivered(user_service);
-		} else
+		} else {
 			ret = -EINVAL;
+		}
 	} break;
 
 	default:
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 a22d8b7..2a83c2a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -689,13 +689,13 @@ process_free_queue(struct vchiq_state *state, BITSET_T *service_found,
 						count - 1;
 				spin_unlock(&quota_spinlock);
 
-				if (count == quota->message_quota)
+				if (count == quota->message_quota) {
 					/*
 					 * Signal the service that it
 					 * has dropped below its quota
 					 */
 					complete(&quota->quota_event);
-				else if (count == 0) {
+				} else if (count == 0) {
 					vchiq_log_error(vchiq_core_log_level,
 						"service %d message_use_count=%d (header %pK, msgid %x, header->msgid %x, header->size %x)",
 						port,
@@ -1691,10 +1691,11 @@ parse_rx_slots(struct vchiq_state *state)
 				vchiq_set_service_state(service,
 					VCHIQ_SRVSTATE_OPEN);
 				complete(&service->remove_event);
-			} else
+			} else {
 				vchiq_log_error(vchiq_core_log_level,
 					"OPENACK received in state %s",
 					srvstate_names[service->srvstate]);
+			}
 			break;
 		case VCHIQ_MSG_CLOSE:
 			WARN_ON(size != 0); /* There should be no data */
@@ -2449,11 +2450,11 @@ vchiq_add_service_internal(struct vchiq_state *state,
 			struct vchiq_service *srv;
 
 			srv = rcu_dereference(state->services[i]);
-			if (!srv)
+			if (!srv) {
 				pservice = &state->services[i];
-			else if ((srv->public_fourcc == params->fourcc) &&
-				 ((srv->instance != instance) ||
-				  (srv->base.callback != params->callback))) {
+			} else if ((srv->public_fourcc == params->fourcc) &&
+				   ((srv->instance != instance) ||
+				   (srv->base.callback != params->callback))) {
 				/*
 				 * There is another server using this
 				 * fourcc which doesn't match.
@@ -2654,10 +2655,12 @@ close_service_complete(struct vchiq_service *service, int failstate)
 				service->client_id = 0;
 				service->remoteport = VCHIQ_PORT_FREE;
 				newstate = VCHIQ_SRVSTATE_LISTENING;
-			} else
+			} else {
 				newstate = VCHIQ_SRVSTATE_CLOSEWAIT;
-		} else
+			}
+		} else {
 			newstate = VCHIQ_SRVSTATE_CLOSED;
+		}
 		vchiq_set_service_state(service, newstate);
 		break;
 	case VCHIQ_SRVSTATE_LISTENING:
@@ -2687,16 +2690,17 @@ close_service_complete(struct vchiq_service *service, int failstate)
 		service->client_id = 0;
 		service->remoteport = VCHIQ_PORT_FREE;
 
-		if (service->srvstate == VCHIQ_SRVSTATE_CLOSED)
+		if (service->srvstate == VCHIQ_SRVSTATE_CLOSED) {
 			vchiq_free_service_internal(service);
-		else if (service->srvstate != VCHIQ_SRVSTATE_CLOSEWAIT) {
+		} else if (service->srvstate != VCHIQ_SRVSTATE_CLOSEWAIT) {
 			if (is_server)
 				service->closing = 0;
 
 			complete(&service->remove_event);
 		}
-	} else
+	} else {
 		vchiq_set_service_state(service, failstate);
+	}
 
 	return status;
 }
@@ -2718,11 +2722,11 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
 	case VCHIQ_SRVSTATE_HIDDEN:
 	case VCHIQ_SRVSTATE_LISTENING:
 	case VCHIQ_SRVSTATE_CLOSEWAIT:
-		if (close_recvd)
+		if (close_recvd) {
 			vchiq_log_error(vchiq_core_log_level,
 				"%s(1) called in state %s",
 				__func__, srvstate_names[service->srvstate]);
-		else if (is_server) {
+		} else if (is_server) {
 			if (service->srvstate == VCHIQ_SRVSTATE_LISTENING) {
 				status = VCHIQ_ERROR;
 			} else {
@@ -2734,8 +2738,9 @@ vchiq_close_service_internal(struct vchiq_service *service, int close_recvd)
 						VCHIQ_SRVSTATE_LISTENING);
 			}
 			complete(&service->remove_event);
-		} else
+		} else {
 			vchiq_free_service_internal(service);
+		}
 		break;
 	case VCHIQ_SRVSTATE_OPENING:
 		if (close_recvd) {
@@ -3325,8 +3330,9 @@ vchiq_release_message(unsigned int handle,
 
 			release_slot(state, slot_info, header, service);
 		}
-	} else if (slot_index == remote->slot_sync)
+	} else if (slot_index == remote->slot_sync) {
 		release_message_sync(state, header);
+	}
 
 	unlock_service(service);
 }
@@ -3620,8 +3626,9 @@ int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service)
 				scnprintf(remoteport + len2,
 					sizeof(remoteport) - len2,
 					" (client %x)", service->client_id);
-		} else
+		} else {
 			strcpy(remoteport, "n/a");
+		}
 
 		len += scnprintf(buf + len, sizeof(buf) - len,
 			" '%c%c%c%c' remote %s (msg use %d/%d, slot use %d/%d)",
-- 
2.7.4

[PATCH 06/20] staging: vchiq_core: introduce poll_services_of_group

[Stefan Wahren]

The function poll_services() has too many indention levels. So keep only
the group iteration loop and move the rest into a new function
poll_services_of_group().

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 125 +++++++++++----------
 1 file changed, 65 insertions(+), 60 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 2a83c2a..3e847d8 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -1318,74 +1318,79 @@ notify_bulks(struct vchiq_service *service, struct vchiq_bulk_queue *queue,
 	return status;
 }
 
-/* Called by the slot handler thread */
 static void
-poll_services(struct vchiq_state *state)
+poll_services_of_group(struct vchiq_state *state, int group)
 {
-	int group, i;
-
-	for (group = 0; group < BITSET_SIZE(state->unused_service); group++) {
-		u32 flags;
-
-		flags = atomic_xchg(&state->poll_services[group], 0);
-		for (i = 0; flags; i++) {
-			if (flags & BIT(i)) {
-				struct vchiq_service *service =
-					find_service_by_port(state,
-						(group<<5) + i);
-				u32 service_flags;
-
-				flags &= ~BIT(i);
-				if (!service)
-					continue;
-				service_flags =
-					atomic_xchg(&service->poll_flags, 0);
-				if (service_flags &
-					BIT(VCHIQ_POLL_REMOVE)) {
-					vchiq_log_info(vchiq_core_log_level,
-						"%d: ps - remove %d<->%d",
-						state->id, service->localport,
-						service->remoteport);
+	u32 flags = atomic_xchg(&state->poll_services[group], 0);
+	int i;
 
-					/*
-					 * Make it look like a client, because
-					 * it must be removed and not left in
-					 * the LISTENING state.
-					 */
-					service->public_fourcc =
-						VCHIQ_FOURCC_INVALID;
-
-					if (vchiq_close_service_internal(
-						service, 0/*!close_recvd*/) !=
-						VCHIQ_SUCCESS)
-						request_poll(state, service,
-							VCHIQ_POLL_REMOVE);
-				} else if (service_flags &
-					BIT(VCHIQ_POLL_TERMINATE)) {
-					vchiq_log_info(vchiq_core_log_level,
-						"%d: ps - terminate %d<->%d",
-						state->id, service->localport,
-						service->remoteport);
-					if (vchiq_close_service_internal(
-						service, 0/*!close_recvd*/) !=
-						VCHIQ_SUCCESS)
-						request_poll(state, service,
-							VCHIQ_POLL_TERMINATE);
-				}
-				if (service_flags & BIT(VCHIQ_POLL_TXNOTIFY))
-					notify_bulks(service,
-						&service->bulk_tx,
-						1/*retry_poll*/);
-				if (service_flags & BIT(VCHIQ_POLL_RXNOTIFY))
-					notify_bulks(service,
-						&service->bulk_rx,
-						1/*retry_poll*/);
-				unlock_service(service);
+	for (i = 0; flags; i++) {
+		if (flags & BIT(i)) {
+			struct vchiq_service *service =
+				find_service_by_port(state,
+					(group<<5) + i);
+			u32 service_flags;
+
+			flags &= ~BIT(i);
+			if (!service)
+				continue;
+			service_flags =
+				atomic_xchg(&service->poll_flags, 0);
+			if (service_flags &
+				BIT(VCHIQ_POLL_REMOVE)) {
+				vchiq_log_info(vchiq_core_log_level,
+					"%d: ps - remove %d<->%d",
+					state->id, service->localport,
+					service->remoteport);
+
+				/*
+				 * Make it look like a client, because
+				 * it must be removed and not left in
+				 * the LISTENING state.
+				 */
+				service->public_fourcc =
+					VCHIQ_FOURCC_INVALID;
+
+				if (vchiq_close_service_internal(
+					service, 0/*!close_recvd*/) !=
+					VCHIQ_SUCCESS)
+					request_poll(state, service,
+						VCHIQ_POLL_REMOVE);
+			} else if (service_flags &
+				BIT(VCHIQ_POLL_TERMINATE)) {
+				vchiq_log_info(vchiq_core_log_level,
+					"%d: ps - terminate %d<->%d",
+					state->id, service->localport,
+					service->remoteport);
+				if (vchiq_close_service_internal(
+					service, 0/*!close_recvd*/) !=
+					VCHIQ_SUCCESS)
+					request_poll(state, service,
+						VCHIQ_POLL_TERMINATE);
 			}
+			if (service_flags & BIT(VCHIQ_POLL_TXNOTIFY))
+				notify_bulks(service,
+					&service->bulk_tx,
+					1/*retry_poll*/);
+			if (service_flags & BIT(VCHIQ_POLL_RXNOTIFY))
+				notify_bulks(service,
+					&service->bulk_rx,
+					1/*retry_poll*/);
+			unlock_service(service);
 		}
 	}
 }
 
+/* Called by the slot handler thread */
+static void
+poll_services(struct vchiq_state *state)
+{
+	int group;
+
+	for (group = 0; group < BITSET_SIZE(state->unused_service); group++)
+		poll_services_of_group(state, group);
+}
+
 /* Called with the bulk_mutex held */
 static void
 abort_outstanding_bulks(struct vchiq_service *service,
-- 
2.7.4

[PATCH 07/20] staging: vchiq_core: avoid indention in poll_services_of_group

[Stefan Wahren]

By converting the first and the third if statement into continue early
the function poll_services_of_group() can avoid 2 indention levels.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 97 ++++++++++------------
 1 file changed, 46 insertions(+), 51 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 3e847d8..b3e81ac 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -1325,59 +1325,54 @@ poll_services_of_group(struct vchiq_state *state, int group)
 	int i;
 
 	for (i = 0; flags; i++) {
-		if (flags & BIT(i)) {
-			struct vchiq_service *service =
-				find_service_by_port(state,
-					(group<<5) + i);
-			u32 service_flags;
-
-			flags &= ~BIT(i);
-			if (!service)
-				continue;
-			service_flags =
-				atomic_xchg(&service->poll_flags, 0);
-			if (service_flags &
-				BIT(VCHIQ_POLL_REMOVE)) {
-				vchiq_log_info(vchiq_core_log_level,
-					"%d: ps - remove %d<->%d",
-					state->id, service->localport,
-					service->remoteport);
+		struct vchiq_service *service;
+		u32 service_flags;
 
-				/*
-				 * Make it look like a client, because
-				 * it must be removed and not left in
-				 * the LISTENING state.
-				 */
-				service->public_fourcc =
-					VCHIQ_FOURCC_INVALID;
-
-				if (vchiq_close_service_internal(
-					service, 0/*!close_recvd*/) !=
-					VCHIQ_SUCCESS)
-					request_poll(state, service,
-						VCHIQ_POLL_REMOVE);
-			} else if (service_flags &
-				BIT(VCHIQ_POLL_TERMINATE)) {
-				vchiq_log_info(vchiq_core_log_level,
-					"%d: ps - terminate %d<->%d",
-					state->id, service->localport,
-					service->remoteport);
-				if (vchiq_close_service_internal(
-					service, 0/*!close_recvd*/) !=
-					VCHIQ_SUCCESS)
-					request_poll(state, service,
-						VCHIQ_POLL_TERMINATE);
-			}
-			if (service_flags & BIT(VCHIQ_POLL_TXNOTIFY))
-				notify_bulks(service,
-					&service->bulk_tx,
-					1/*retry_poll*/);
-			if (service_flags & BIT(VCHIQ_POLL_RXNOTIFY))
-				notify_bulks(service,
-					&service->bulk_rx,
-					1/*retry_poll*/);
-			unlock_service(service);
+		if ((flags & BIT(i)) == 0)
+			continue;
+
+		service = find_service_by_port(state, (group << 5) + i);
+		flags &= ~BIT(i);
+
+		if (!service)
+			continue;
+
+		service_flags = atomic_xchg(&service->poll_flags, 0);
+		if ((service_flags & BIT(VCHIQ_POLL_REMOVE)) == 0)
+			continue;
+
+		vchiq_log_info(vchiq_core_log_level, "%d: ps - remove %d<->%d",
+			       state->id, service->localport,
+			       service->remoteport);
+
+		/*
+		 * Make it look like a client, because
+		 * it must be removed and not left in
+		 * the LISTENING state.
+		 */
+		service->public_fourcc = VCHIQ_FOURCC_INVALID;
+
+		if (vchiq_close_service_internal(service, 0/*!close_recvd*/) !=
+						 VCHIQ_SUCCESS) {
+			request_poll(state, service, VCHIQ_POLL_REMOVE);
+		} else if (service_flags & BIT(VCHIQ_POLL_TERMINATE)) {
+			vchiq_log_info(vchiq_core_log_level,
+				"%d: ps - terminate %d<->%d",
+				state->id, service->localport,
+				service->remoteport);
+			if (vchiq_close_service_internal(
+				service, 0/*!close_recvd*/) !=
+				VCHIQ_SUCCESS)
+				request_poll(state, service,
+					     VCHIQ_POLL_TERMINATE);
 		}
+		if (service_flags & BIT(VCHIQ_POLL_TXNOTIFY))
+			notify_bulks(service, &service->bulk_tx,
+				     1/*retry_poll*/);
+		if (service_flags & BIT(VCHIQ_POLL_RXNOTIFY))
+			notify_bulks(service, &service->bulk_rx,
+				     1/*retry_poll*/);
+		unlock_service(service);
 	}
 }
 
-- 
2.7.4

[PATCH 08/20] staging: vchiq_arm: Use define for doorbell irq

[Stefan Wahren]

The interrupt handler uses a magic number to check that the doorbell
was rung. Better replace this number with official Broadcom define.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index c3fbb29..30d6f1a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -29,6 +29,8 @@
 #define BELL0	0x00
 #define BELL2	0x08
 
+#define ARM_DS_ACTIVE	BIT(2)
+
 struct vchiq_2835_state {
 	int inited;
 	struct vchiq_arm_state arm_state;
@@ -269,7 +271,7 @@ vchiq_doorbell_irq(int irq, void *dev_id)
 	/* Read (and clear) the doorbell */
 	status = readl(g_regs + BELL0);
 
-	if (status & 0x4) {  /* Was the doorbell rung? */
+	if (status & ARM_DS_ACTIVE) {  /* Was the doorbell rung? */
 		remote_event_pollall(state);
 		ret = IRQ_HANDLED;
 	}
-- 
2.7.4

[PATCH 09/20] staging: vchiq_arm: drop ftrace-like logging

[Stefan Wahren]

This addresses the warnings reported by checkpatch:
WARNING: Unnecessary ftrace-like logging - prefer using ftrace

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 22 ----------------------
 1 file changed, 22 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 7951324..b70db46 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -153,8 +153,6 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
 	struct vchiq_instance *instance = NULL;
 	int i, ret;
 
-	vchiq_log_trace(vchiq_core_log_level, "%s called", __func__);
-
 	/*
 	 * VideoCore may not be ready due to boot up timing.
 	 * It may never be ready if kernel and firmware are mismatched,so don't
@@ -207,9 +205,6 @@ enum vchiq_status vchiq_shutdown(struct vchiq_instance *instance)
 	enum vchiq_status status;
 	struct vchiq_state *state = instance->state;
 
-	vchiq_log_trace(vchiq_core_log_level,
-		"%s(%p) called", __func__, instance);
-
 	if (mutex_lock_killable(&state->mutex))
 		return VCHIQ_RETRY;
 
@@ -249,9 +244,6 @@ enum vchiq_status vchiq_connect(struct vchiq_instance *instance)
 	enum vchiq_status status;
 	struct vchiq_state *state = instance->state;
 
-	vchiq_log_trace(vchiq_core_log_level,
-		"%s(%p) called", __func__, instance);
-
 	if (mutex_lock_killable(&state->mutex)) {
 		vchiq_log_trace(vchiq_core_log_level,
 			"%s: call to mutex_lock failed", __func__);
@@ -283,9 +275,6 @@ static enum vchiq_status vchiq_add_service(
 	struct vchiq_service *service = NULL;
 	int srvstate;
 
-	vchiq_log_trace(vchiq_core_log_level,
-		"%s(%p) called", __func__, instance);
-
 	*phandle = VCHIQ_SERVICE_HANDLE_INVALID;
 
 	srvstate = vchiq_is_connected(instance)
@@ -321,9 +310,6 @@ enum vchiq_status vchiq_open_service(
 	struct vchiq_state   *state = instance->state;
 	struct vchiq_service *service = NULL;
 
-	vchiq_log_trace(vchiq_core_log_level,
-		"%s(%p) called", __func__, instance);
-
 	*phandle = VCHIQ_SERVICE_HANDLE_INVALID;
 
 	if (!vchiq_is_connected(instance))
@@ -2350,8 +2336,6 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
 		goto out;
 	}
 
-	vchiq_log_trace(vchiq_susp_log_level, "%s", __func__);
-
 	if (use_type == USE_TYPE_VCHIQ) {
 		sprintf(entity, "VCHIQ:   ");
 		entity_uc = &arm_state->peer_use_count;
@@ -2409,8 +2393,6 @@ vchiq_release_internal(struct vchiq_state *state, struct vchiq_service *service)
 		goto out;
 	}
 
-	vchiq_log_trace(vchiq_susp_log_level, "%s", __func__);
-
 	if (service) {
 		sprintf(entity, "%c%c%c%c:%03d",
 			VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc),
@@ -2450,7 +2432,6 @@ vchiq_on_remote_use(struct vchiq_state *state)
 {
 	struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state);
 
-	vchiq_log_trace(vchiq_susp_log_level, "%s", __func__);
 	atomic_inc(&arm_state->ka_use_count);
 	complete(&arm_state->ka_evt);
 }
@@ -2460,7 +2441,6 @@ vchiq_on_remote_release(struct vchiq_state *state)
 {
 	struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state);
 
-	vchiq_log_trace(vchiq_susp_log_level, "%s", __func__);
 	atomic_inc(&arm_state->ka_release_count);
 	complete(&arm_state->ka_evt);
 }
@@ -2647,8 +2627,6 @@ vchiq_check_service(struct vchiq_service *service)
 	if (!service || !service->state)
 		goto out;
 
-	vchiq_log_trace(vchiq_susp_log_level, "%s", __func__);
-
 	arm_state = vchiq_platform_get_arm_state(service->state);
 
 	read_lock_bh(&arm_state->susp_res_lock);
-- 
2.7.4

[PATCH 10/20] staging: vchiq_arm: Prefer kzalloc(sizeof(*waiter)...)

[Stefan Wahren]

It's shorter and easier to maintain. This has been found with checkpatch.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 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 b70db46..92825b7 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -459,7 +459,7 @@ vchiq_blocking_bulk_transfer(unsigned int handle, void *data,
 			}
 		}
 	} else {
-		waiter = kzalloc(sizeof(struct bulk_waiter_node), GFP_KERNEL);
+		waiter = kzalloc(sizeof(*waiter), GFP_KERNEL);
 		if (!waiter) {
 			vchiq_log_error(vchiq_core_log_level,
 				"%s - out of memory", __func__);
@@ -962,8 +962,7 @@ static int vchiq_irq_queue_bulk_tx_rx(struct vchiq_instance *instance,
 		return -EINVAL;
 
 	if (args->mode == VCHIQ_BULK_MODE_BLOCKING) {
-		waiter = kzalloc(sizeof(struct bulk_waiter_node),
-			GFP_KERNEL);
+		waiter = kzalloc(sizeof(*waiter), GFP_KERNEL);
 		if (!waiter) {
 			ret = -ENOMEM;
 			goto out;
-- 
2.7.4

[PATCH 11/20] staging: vchiq_arm: drop non-beneficial comments

[Stefan Wahren]

Those comments doesn't provide any benefit, so drop them.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 55 ----------------------
 .../interface/vchiq_arm/vchiq_connected.h          |  8 ----
 .../interface/vchiq_arm/vchiq_debugfs.c            |  5 --
 3 files changed, 68 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 92825b7..f2f948c 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -493,11 +493,6 @@ vchiq_blocking_bulk_transfer(unsigned int handle, void *data,
 
 	return status;
 }
-/****************************************************************************
- *
- *   add_completion
- *
- ***************************************************************************/
 
 static enum vchiq_status
 add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
@@ -564,12 +559,6 @@ add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
 	return VCHIQ_SUCCESS;
 }
 
-/****************************************************************************
- *
- *   service_callback
- *
- ***************************************************************************/
-
 static enum vchiq_status
 service_callback(enum vchiq_reason reason, struct vchiq_header *header,
 		 unsigned int handle, void *bulk_userdata)
@@ -681,22 +670,12 @@ service_callback(enum vchiq_reason reason, struct vchiq_header *header,
 		bulk_userdata);
 }
 
-/****************************************************************************
- *
- *   user_service_free
- *
- ***************************************************************************/
 static void
 user_service_free(void *userdata)
 {
 	kfree(userdata);
 }
 
-/****************************************************************************
- *
- *   close_delivered
- *
- ***************************************************************************/
 static void close_delivered(struct user_service *user_service)
 {
 	vchiq_log_info(vchiq_arm_log_level,
@@ -759,11 +738,6 @@ static ssize_t vchiq_ioc_copy_element_data(void *context, void *dest,
 	return maxsize;
 }
 
-/**************************************************************************
- *
- *   vchiq_ioc_queue_message
- *
- **************************************************************************/
 static int
 vchiq_ioc_queue_message(unsigned int handle,
 			struct vchiq_element *elements,
@@ -1242,11 +1216,6 @@ static int vchiq_ioc_await_completion(struct vchiq_instance *instance,
 	return ret;
 }
 
-/****************************************************************************
- *
- *   vchiq_ioctl
- *
- ***************************************************************************/
 static long
 vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
@@ -2003,12 +1972,6 @@ static int vchiq_release(struct inode *inode, struct file *file)
 	return ret;
 }
 
-/****************************************************************************
- *
- *   vchiq_dump
- *
- ***************************************************************************/
-
 int vchiq_dump(void *dump_context, const char *str, int len)
 {
 	struct dump_context *context = (struct dump_context *)dump_context;
@@ -2050,12 +2013,6 @@ int vchiq_dump(void *dump_context, const char *str, int len)
 	return 0;
 }
 
-/****************************************************************************
- *
- *   vchiq_dump_platform_instance_state
- *
- ***************************************************************************/
-
 int vchiq_dump_platform_instances(void *dump_context)
 {
 	struct vchiq_state *state = vchiq_get_state();
@@ -2118,12 +2075,6 @@ int vchiq_dump_platform_instances(void *dump_context)
 	return 0;
 }
 
-/****************************************************************************
- *
- *   vchiq_dump_platform_service_state
- *
- ***************************************************************************/
-
 int vchiq_dump_platform_service_state(void *dump_context,
 				      struct vchiq_service *service)
 {
@@ -2149,12 +2100,6 @@ int vchiq_dump_platform_service_state(void *dump_context,
 	return vchiq_dump(dump_context, buf, len + 1);
 }
 
-/****************************************************************************
- *
- *   vchiq_read
- *
- ***************************************************************************/
-
 static ssize_t
 vchiq_read(struct file *file, char __user *buf,
 	size_t count, loff_t *ppos)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.h
index ec5d2b7..95c1867 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.h
@@ -4,16 +4,8 @@
 #ifndef VCHIQ_CONNECTED_H
 #define VCHIQ_CONNECTED_H
 
-/* ---- Include Files ----------------------------------------------------- */
-
-/* ---- Constants and Types ---------------------------------------------- */
-
 typedef void (*VCHIQ_CONNECTED_CALLBACK_T)(void);
 
-/* ---- Variable Externs ------------------------------------------------- */
-
-/* ---- Function Prototypes ---------------------------------------------- */
-
 void vchiq_add_connected_callback(VCHIQ_CONNECTED_CALLBACK_T callback);
 void vchiq_call_connected_callbacks(void);
 
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 a39757b..da550b6 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
@@ -11,11 +11,6 @@
 
 #ifdef CONFIG_DEBUG_FS
 
-/****************************************************************************
- *
- *   log category entries
- *
- ***************************************************************************/
 #define DEBUGFS_WRITE_BUF_SIZE 256
 
 #define VCHIQ_LOG_ERROR_STR   "error"
-- 
2.7.4

[PATCH 12/20] staging: vchiq_arm: add blank line after declarations

[Stefan Wahren]

Improve the readability by add a blank line after declarations. This
was found with checkpatch.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c     | 3 +++
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c | 1 +
 2 files changed, 4 insertions(+)

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 f2f948c..c08a950 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1019,6 +1019,7 @@ static inline int vchiq_get_user_ptr(void __user **buf, void __user *ubuf, int i
 	if (in_compat_syscall()) {
 		compat_uptr_t ptr32;
 		compat_uptr_t __user *uptr = ubuf;
+
 		ret = get_user(ptr32, uptr + index);
 		if (ret)
 			return ret;
@@ -1026,6 +1027,7 @@ static inline int vchiq_get_user_ptr(void __user **buf, void __user *ubuf, int i
 		*buf = compat_ptr(ptr32);
 	} else {
 		uintptr_t ptr, __user *uptr = ubuf;
+
 		ret = get_user(ptr, uptr + index);
 
 		if (ret)
@@ -1798,6 +1800,7 @@ static long
 vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	void __user *argp = compat_ptr(arg);
+
 	switch (cmd) {
 	case VCHIQ_IOC_CREATE_SERVICE32:
 		return vchiq_compat_ioctl_create_service(file, cmd, argp);
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 da550b6..8f3d9cb 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
@@ -35,6 +35,7 @@ static struct vchiq_debugfs_log_entry vchiq_debugfs_log_entries[] = {
 	{ "susp", &vchiq_susp_log_level },
 	{ "arm",  &vchiq_arm_log_level },
 };
+
 static int n_log_entries = ARRAY_SIZE(vchiq_debugfs_log_entries);
 
 static int debugfs_log_show(struct seq_file *f, void *offset)
-- 
2.7.4

[PATCH 13/20] staging: vchiq_arm: re-arrange function header

[Stefan Wahren]

This makes the function headers look more consistent.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 41 ++++++++++------------
 1 file changed, 18 insertions(+), 23 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 c08a950..65456c86 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -265,10 +265,10 @@ enum vchiq_status vchiq_connect(struct vchiq_instance *instance)
 }
 EXPORT_SYMBOL(vchiq_connect);
 
-static enum vchiq_status vchiq_add_service(
-	struct vchiq_instance             *instance,
-	const struct vchiq_service_params_kernel *params,
-	unsigned int       *phandle)
+static enum vchiq_status
+vchiq_add_service(struct vchiq_instance *instance,
+		  const struct vchiq_service_params_kernel *params,
+		  unsigned int *phandle)
 {
 	enum vchiq_status status;
 	struct vchiq_state *state = instance->state;
@@ -301,10 +301,10 @@ static enum vchiq_status vchiq_add_service(
 	return status;
 }
 
-enum vchiq_status vchiq_open_service(
-	struct vchiq_instance             *instance,
-	const struct vchiq_service_params_kernel *params,
-	unsigned int       *phandle)
+enum vchiq_status
+vchiq_open_service(struct vchiq_instance *instance,
+		   const struct vchiq_service_params_kernel *params,
+		   unsigned int *phandle)
 {
 	enum vchiq_status   status = VCHIQ_ERROR;
 	struct vchiq_state   *state = instance->state;
@@ -339,8 +339,8 @@ enum vchiq_status vchiq_open_service(
 EXPORT_SYMBOL(vchiq_open_service);
 
 enum vchiq_status
-vchiq_bulk_transmit(unsigned int handle, const void *data,
-	unsigned int size, void *userdata, enum vchiq_bulk_mode mode)
+vchiq_bulk_transmit(unsigned int handle, const void *data, unsigned int size,
+		    void *userdata, enum vchiq_bulk_mode mode)
 {
 	enum vchiq_status status;
 
@@ -414,8 +414,8 @@ enum vchiq_status vchiq_bulk_receive(unsigned int handle, void *data,
 EXPORT_SYMBOL(vchiq_bulk_receive);
 
 static enum vchiq_status
-vchiq_blocking_bulk_transfer(unsigned int handle, void *data,
-	unsigned int size, enum vchiq_bulk_dir dir)
+vchiq_blocking_bulk_transfer(unsigned int handle, void *data, unsigned int size,
+			     enum vchiq_bulk_dir dir)
 {
 	struct vchiq_instance *instance;
 	struct vchiq_service *service;
@@ -739,8 +739,7 @@ static ssize_t vchiq_ioc_copy_element_data(void *context, void *dest,
 }
 
 static int
-vchiq_ioc_queue_message(unsigned int handle,
-			struct vchiq_element *elements,
+vchiq_ioc_queue_message(unsigned int handle, struct vchiq_element *elements,
 			unsigned long count)
 {
 	struct vchiq_io_copy_callback_context context;
@@ -1560,10 +1559,8 @@ struct vchiq_create_service32 {
 	_IOWR(VCHIQ_IOC_MAGIC, 2, struct vchiq_create_service32)
 
 static long
-vchiq_compat_ioctl_create_service(
-	struct file *file,
-	unsigned int cmd,
-	struct vchiq_create_service32 __user *ptrargs32)
+vchiq_compat_ioctl_create_service(struct file *file, unsigned int cmd,
+				  struct vchiq_create_service32 __user *ptrargs32)
 {
 	struct vchiq_create_service args;
 	struct vchiq_create_service32 args32;
@@ -2104,8 +2101,7 @@ int vchiq_dump_platform_service_state(void *dump_context,
 }
 
 static ssize_t
-vchiq_read(struct file *file, char __user *buf,
-	size_t count, loff_t *ppos)
+vchiq_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 {
 	struct dump_context context;
 	int err;
@@ -2156,9 +2152,8 @@ vchiq_fops = {
 
 static enum vchiq_status
 vchiq_keepalive_vchiq_callback(enum vchiq_reason reason,
-	struct vchiq_header *header,
-	unsigned int service_user,
-	void *bulk_user)
+			       struct vchiq_header *header,
+			       unsigned int service_user, void *bulk_user)
 {
 	vchiq_log_error(vchiq_susp_log_level,
 		"%s callback reason %d", __func__, reason);
-- 
2.7.4

[PATCH 14/20] staging: vchiq_core: reduce indention in release_service_messages

[Stefan Wahren]

It's possible to convert the if statement into a continue early and
save an indention level.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 71 +++++++++++-----------
 1 file changed, 36 insertions(+), 35 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 b3e81ac..2ac3545 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -2578,42 +2578,43 @@ release_service_messages(struct vchiq_service *service)
 	for (i = state->remote->slot_first; i <= slot_last; i++) {
 		struct vchiq_slot_info *slot_info =
 			SLOT_INFO_FROM_INDEX(state, i);
-		if (slot_info->release_count != slot_info->use_count) {
-			char *data =
-				(char *)SLOT_DATA_FROM_INDEX(state, i);
-			unsigned int pos, end;
+		unsigned int pos, end;
+		char *data;
 
-			end = VCHIQ_SLOT_SIZE;
-			if (data == state->rx_data)
-				/*
-				 * This buffer is still being read from - stop
-				 * at the current read position
-				 */
-				end = state->rx_pos & VCHIQ_SLOT_MASK;
-
-			pos = 0;
-
-			while (pos < end) {
-				struct vchiq_header *header =
-					(struct vchiq_header *)(data + pos);
-				int msgid = header->msgid;
-				int port = VCHIQ_MSG_DSTPORT(msgid);
-
-				if ((port == service->localport) &&
-					(msgid & VCHIQ_MSGID_CLAIMED)) {
-					vchiq_log_info(vchiq_core_log_level,
-						"  fsi - hdr %pK", header);
-					release_slot(state, slot_info, header,
-						NULL);
-				}
-				pos += calc_stride(header->size);
-				if (pos > VCHIQ_SLOT_SIZE) {
-					vchiq_log_error(vchiq_core_log_level,
-						"fsi - pos %x: header %pK, msgid %x, header->msgid %x, header->size %x",
-						pos, header, msgid,
-						header->msgid, header->size);
-					WARN(1, "invalid slot position\n");
-				}
+		if (slot_info->release_count == slot_info->use_count)
+			continue;
+
+		data = (char *)SLOT_DATA_FROM_INDEX(state, i);
+		end = VCHIQ_SLOT_SIZE;
+		if (data == state->rx_data)
+			/*
+			 * This buffer is still being read from - stop
+			 * at the current read position
+			 */
+			end = state->rx_pos & VCHIQ_SLOT_MASK;
+
+		pos = 0;
+
+		while (pos < end) {
+			struct vchiq_header *header =
+				(struct vchiq_header *)(data + pos);
+			int msgid = header->msgid;
+			int port = VCHIQ_MSG_DSTPORT(msgid);
+
+			if ((port == service->localport) &&
+				(msgid & VCHIQ_MSGID_CLAIMED)) {
+				vchiq_log_info(vchiq_core_log_level,
+					"  fsi - hdr %pK", header);
+				release_slot(state, slot_info, header,
+					NULL);
+			}
+			pos += calc_stride(header->size);
+			if (pos > VCHIQ_SLOT_SIZE) {
+				vchiq_log_error(vchiq_core_log_level,
+					"fsi - pos %x: header %pK, msgid %x, header->msgid %x, header->size %x",
+					pos, header, msgid,
+					header->msgid, header->size);
+				WARN(1, "invalid slot position\n");
 			}
 		}
 	}
-- 
2.7.4

[PATCH 15/20] staging: vchiq_core: fix comment in vchiq_shutdown_internal

[Stefan Wahren]

The comment seems to be copied from vchiq_connect_internal(). So change
it to match the actual behavior.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 2ac3545..1174715 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -2928,7 +2928,7 @@ vchiq_shutdown_internal(struct vchiq_state *state, struct vchiq_instance *instan
 	struct vchiq_service *service;
 	int i;
 
-	/* Find all services registered to this client and enable them. */
+	/* Find all services registered to this client and remove them. */
 	i = 0;
 	while ((service = next_service_by_instance(state, instance,
 		&i)) !=	NULL) {
-- 
2.7.4

[PATCH 16/20] staging: vchiq_arm: make vchiq_shutdown_internal return void

[Stefan Wahren]

The function vchiq_shutdown_internal always returns VCHIQ_SUCCESS. So change
the return type to void and simplify the logic in vchiq_shutdown.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 25 ++++++++++------------
 .../vc04_services/interface/vchiq_arm/vchiq_core.c |  4 +---
 .../vc04_services/interface/vchiq_arm/vchiq_core.h |  2 +-
 3 files changed, 13 insertions(+), 18 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 65456c86..e7b5f14 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -202,33 +202,30 @@ EXPORT_SYMBOL(vchiq_initialise);
 
 enum vchiq_status vchiq_shutdown(struct vchiq_instance *instance)
 {
-	enum vchiq_status status;
+	enum vchiq_status status = VCHIQ_SUCCESS;
 	struct vchiq_state *state = instance->state;
+	struct bulk_waiter_node *waiter, *next;
 
 	if (mutex_lock_killable(&state->mutex))
 		return VCHIQ_RETRY;
 
 	/* Remove all services */
-	status = vchiq_shutdown_internal(state, instance);
+	vchiq_shutdown_internal(state, instance);
 
 	mutex_unlock(&state->mutex);
 
 	vchiq_log_trace(vchiq_core_log_level,
 		"%s(%p): returning %d", __func__, instance, status);
 
-	if (status == VCHIQ_SUCCESS) {
-		struct bulk_waiter_node *waiter, *next;
-
-		list_for_each_entry_safe(waiter, next,
-					 &instance->bulk_waiter_list, list) {
-			list_del(&waiter->list);
-			vchiq_log_info(vchiq_arm_log_level,
-					"bulk_waiter - cleaned up %pK for pid %d",
-					waiter, waiter->pid);
-			kfree(waiter);
-		}
-		kfree(instance);
+	list_for_each_entry_safe(waiter, next,
+				 &instance->bulk_waiter_list, list) {
+		list_del(&waiter->list);
+		vchiq_log_info(vchiq_arm_log_level,
+				"bulk_waiter - cleaned up %pK for pid %d",
+				waiter, waiter->pid);
+		kfree(waiter);
 	}
+	kfree(instance);
 
 	return status;
 }
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 1174715..0ec1c11 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -2922,7 +2922,7 @@ vchiq_connect_internal(struct vchiq_state *state, struct vchiq_instance *instanc
 	return VCHIQ_SUCCESS;
 }
 
-enum vchiq_status
+void
 vchiq_shutdown_internal(struct vchiq_state *state, struct vchiq_instance *instance)
 {
 	struct vchiq_service *service;
@@ -2935,8 +2935,6 @@ vchiq_shutdown_internal(struct vchiq_state *state, struct vchiq_instance *instan
 		(void)vchiq_remove_service(service->handle);
 		unlock_service(service);
 	}
-
-	return VCHIQ_SUCCESS;
 }
 
 enum vchiq_status
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 89f898ce..5264677 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -563,7 +563,7 @@ vchiq_terminate_service_internal(struct vchiq_service *service);
 extern void
 vchiq_free_service_internal(struct vchiq_service *service);
 
-extern enum vchiq_status
+extern void
 vchiq_shutdown_internal(struct vchiq_state *state, struct vchiq_instance *instance);
 
 extern void
-- 
2.7.4

[PATCH 17/20] staging: vchiq_arm: Avoid unnecessary line breaks

[Stefan Wahren]

There are a few statements which are unnecessary broken into multiple lines.
Let's join them into a single line to improve readability.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c      | 12 ++++--------
 .../vc04_services/interface/vchiq_arm/vchiq_core.c     | 18 ++++++------------
 2 files changed, 10 insertions(+), 20 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 e7b5f14..344f35a 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -139,8 +139,7 @@ static const char *const ioctl_names[] = {
 	"CLOSE_DELIVERED"
 };
 
-vchiq_static_assert(ARRAY_SIZE(ioctl_names) ==
-		    (VCHIQ_IOC_MAX + 1));
+vchiq_static_assert(ARRAY_SIZE(ioctl_names) == (VCHIQ_IOC_MAX + 1));
 
 static enum vchiq_status
 vchiq_blocking_bulk_transfer(unsigned int handle, void *data,
@@ -871,8 +870,7 @@ static int vchiq_ioc_dequeue_message(struct vchiq_instance *instance,
 				break;
 			}
 			spin_lock(&msg_queue_spinlock);
-		} while (user_service->msg_remove ==
-			user_service->msg_insert);
+		} while (user_service->msg_remove == user_service->msg_insert);
 
 		if (ret)
 			goto out;
@@ -1083,8 +1081,7 @@ static int vchiq_ioc_await_completion(struct vchiq_instance *instance,
 	mutex_lock(&instance->completion_mutex);
 
 	DEBUG_TRACE(AWAIT_COMPLETION_LINE);
-	while ((instance->completion_remove ==
-		instance->completion_insert)
+	while ((instance->completion_remove == instance->completion_insert)
 		&& !instance->closing) {
 		int rc;
 
@@ -1924,8 +1921,7 @@ static int vchiq_release(struct inode *inode, struct file *file)
 	}
 
 	/* Release any closed services */
-	while (instance->completion_remove !=
-		instance->completion_insert) {
+	while (instance->completion_remove != instance->completion_insert) {
 		struct vchiq_completion_data_kernel *completion;
 		struct vchiq_service *service;
 
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 0ec1c11..e3b93ed 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -685,8 +685,7 @@ process_free_queue(struct vchiq_state *state, BITSET_T *service_found,
 				spin_lock(&quota_spinlock);
 				count = quota->message_use_count;
 				if (count > 0)
-					quota->message_use_count =
-						count - 1;
+					quota->message_use_count = count - 1;
 				spin_unlock(&quota_spinlock);
 
 				if (count == quota->message_quota) {
@@ -757,8 +756,7 @@ process_free_queue(struct vchiq_state *state, BITSET_T *service_found,
 			spin_lock(&quota_spinlock);
 			count = state->data_use_count;
 			if (count > 0)
-				state->data_use_count =
-					count - 1;
+				state->data_use_count = count - 1;
 			spin_unlock(&quota_spinlock);
 			if (count == state->data_quota)
 				complete(&state->data_quota_event);
@@ -899,8 +897,7 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service,
 
 		while ((quota->message_use_count == quota->message_quota) ||
 			((tx_end_index != quota->previous_tx_index) &&
-			(quota->slot_use_count ==
-				quota->slot_quota))) {
+			(quota->slot_use_count == quota->slot_quota))) {
 			spin_unlock(&quota_spinlock);
 			vchiq_log_trace(vchiq_core_log_level,
 				"%d: qm:%d %s,%zx - quota stall (msg %d, slot %d)",
@@ -1293,8 +1290,7 @@ notify_bulks(struct vchiq_service *service, struct vchiq_bulk_queue *queue,
 					complete(&waiter->event);
 				}
 				spin_unlock(&bulk_waiter_spinlock);
-			} else if (bulk->mode ==
-				VCHIQ_BULK_MODE_CALLBACK) {
+			} else if (bulk->mode == VCHIQ_BULK_MODE_CALLBACK) {
 				enum vchiq_reason reason =
 						get_bulk_reason(bulk);
 				status = make_service_callback(service,
@@ -2076,8 +2072,7 @@ sync_func(void *v)
 				state->id, header, size, remoteport, localport);
 
 			if ((service->remoteport == remoteport) &&
-				(service->srvstate ==
-				VCHIQ_SRVSTATE_OPENSYNC)) {
+			    (service->srvstate == VCHIQ_SRVSTATE_OPENSYNC)) {
 				if (make_service_callback(service,
 					VCHIQ_MESSAGE_AVAILABLE, header,
 					NULL) == VCHIQ_RETRY)
@@ -2211,8 +2206,7 @@ vchiq_init_state(struct vchiq_state *state, struct vchiq_slot_zero *slot_zero)
 	state->slot_queue_available = 0;
 
 	for (i = 0; i < VCHIQ_MAX_SERVICES; i++) {
-		struct vchiq_service_quota *quota =
-			&state->service_quotas[i];
+		struct vchiq_service_quota *quota = &state->service_quotas[i];
 		init_completion(&quota->quota_event);
 	}
 
-- 
2.7.4

[PATCH 18/20] staging: vchiq_core: introduce parse_message

[Stefan Wahren]

The function parse_rx_slots is very longer. So move at least the message
parsing into a separate function to improve readability. In good case
the function returns the message payload length which is necessary to
move to the next message.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c | 605 +++++++++++----------
 1 file changed, 313 insertions(+), 292 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 e3b93ed..985e1c6 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -1545,335 +1545,360 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header)
 	return 0;
 }
 
-/* Called by the slot handler thread */
-static void
-parse_rx_slots(struct vchiq_state *state)
+/**
+ * parse_message() - parses a single message from the rx slot
+ * @state:  vchiq state struct
+ * @header: message header
+ *
+ * Context: Process context
+ *
+ * Return:
+ * * >= 0     - size of the parsed message payload (without header)
+ * * -EINVAL  - fatal error occurred, bail out is required
+ */
+static int
+parse_message(struct vchiq_state *state, struct vchiq_header *header)
 {
-	struct vchiq_shared_state *remote = state->remote;
 	struct vchiq_service *service = NULL;
-	int tx_pos;
+	unsigned int localport, remoteport;
+	int msgid, size, type, ret = -EINVAL;
 
 	DEBUG_INITIALISE(state->local)
 
-	tx_pos = remote->tx_pos;
-
-	while (state->rx_pos != tx_pos) {
-		struct vchiq_header *header;
-		int msgid, size;
-		int type;
-		unsigned int localport, remoteport;
-
-		DEBUG_TRACE(PARSE_LINE);
-		if (!state->rx_data) {
-			int rx_index;
-
-			WARN_ON(!((state->rx_pos & VCHIQ_SLOT_MASK) == 0));
-			rx_index = remote->slot_queue[
-				SLOT_QUEUE_INDEX_FROM_POS_MASKED(state->rx_pos)];
-			state->rx_data = (char *)SLOT_DATA_FROM_INDEX(state,
-				rx_index);
-			state->rx_info = SLOT_INFO_FROM_INDEX(state, rx_index);
+	DEBUG_VALUE(PARSE_HEADER, (int)(long)header);
+	msgid = header->msgid;
+	DEBUG_VALUE(PARSE_MSGID, msgid);
+	size = header->size;
+	type = VCHIQ_MSG_TYPE(msgid);
+	localport = VCHIQ_MSG_DSTPORT(msgid);
+	remoteport = VCHIQ_MSG_SRCPORT(msgid);
 
+	if (type != VCHIQ_MSG_DATA)
+		VCHIQ_STATS_INC(state, ctrl_rx_count);
+
+	switch (type) {
+	case VCHIQ_MSG_OPENACK:
+	case VCHIQ_MSG_CLOSE:
+	case VCHIQ_MSG_DATA:
+	case VCHIQ_MSG_BULK_RX:
+	case VCHIQ_MSG_BULK_TX:
+	case VCHIQ_MSG_BULK_RX_DONE:
+	case VCHIQ_MSG_BULK_TX_DONE:
+		service = find_service_by_port(state, localport);
+		if ((!service ||
+		     ((service->remoteport != remoteport) &&
+		      (service->remoteport != VCHIQ_PORT_FREE))) &&
+		    (localport == 0) &&
+		    (type == VCHIQ_MSG_CLOSE)) {
 			/*
-			 * Initialise use_count to one, and increment
-			 * release_count at the end of the slot to avoid
-			 * releasing the slot prematurely.
+			 * This could be a CLOSE from a client which
+			 * hadn't yet received the OPENACK - look for
+			 * the connected service
 			 */
-			state->rx_info->use_count = 1;
-			state->rx_info->release_count = 0;
+			if (service)
+				unlock_service(service);
+			service = get_connected_service(state,
+				remoteport);
+			if (service)
+				vchiq_log_warning(vchiq_core_log_level,
+					"%d: prs %s@%pK (%d->%d) - found connected service %d",
+					state->id, msg_type_str(type),
+					header, remoteport, localport,
+					service->localport);
 		}
 
-		header = (struct vchiq_header *)(state->rx_data +
-			(state->rx_pos & VCHIQ_SLOT_MASK));
-		DEBUG_VALUE(PARSE_HEADER, (int)(long)header);
-		msgid = header->msgid;
-		DEBUG_VALUE(PARSE_MSGID, msgid);
-		size = header->size;
-		type = VCHIQ_MSG_TYPE(msgid);
-		localport = VCHIQ_MSG_DSTPORT(msgid);
-		remoteport = VCHIQ_MSG_SRCPORT(msgid);
-
-		if (type != VCHIQ_MSG_DATA)
-			VCHIQ_STATS_INC(state, ctrl_rx_count);
+		if (!service) {
+			vchiq_log_error(vchiq_core_log_level,
+				"%d: prs %s@%pK (%d->%d) - invalid/closed service %d",
+				state->id, msg_type_str(type),
+				header, remoteport, localport,
+				localport);
+			goto skip_message;
+		}
+		break;
+	default:
+		break;
+	}
 
-		switch (type) {
-		case VCHIQ_MSG_OPENACK:
-		case VCHIQ_MSG_CLOSE:
-		case VCHIQ_MSG_DATA:
-		case VCHIQ_MSG_BULK_RX:
-		case VCHIQ_MSG_BULK_TX:
-		case VCHIQ_MSG_BULK_RX_DONE:
-		case VCHIQ_MSG_BULK_TX_DONE:
-			service = find_service_by_port(state, localport);
-			if ((!service ||
-			     ((service->remoteport != remoteport) &&
-			      (service->remoteport != VCHIQ_PORT_FREE))) &&
-			    (localport == 0) &&
-			    (type == VCHIQ_MSG_CLOSE)) {
-				/*
-				 * This could be a CLOSE from a client which
-				 * hadn't yet received the OPENACK - look for
-				 * the connected service
-				 */
-				if (service)
-					unlock_service(service);
-				service = get_connected_service(state,
-					remoteport);
-				if (service)
-					vchiq_log_warning(vchiq_core_log_level,
-						"%d: prs %s@%pK (%d->%d) - found connected service %d",
-						state->id, msg_type_str(type),
-						header, remoteport, localport,
-						service->localport);
-			}
+	if (SRVTRACE_ENABLED(service, VCHIQ_LOG_INFO)) {
+		int svc_fourcc;
 
-			if (!service) {
-				vchiq_log_error(vchiq_core_log_level,
-					"%d: prs %s@%pK (%d->%d) - invalid/closed service %d",
-					state->id, msg_type_str(type),
-					header, remoteport, localport,
-					localport);
-				goto skip_message;
-			}
-			break;
-		default:
-			break;
-		}
+		svc_fourcc = service
+			? service->base.fourcc
+			: VCHIQ_MAKE_FOURCC('?', '?', '?', '?');
+		vchiq_log_info(SRVTRACE_LEVEL(service),
+			"Rcvd Msg %s(%u) from %c%c%c%c s:%d d:%d len:%d",
+			msg_type_str(type), type,
+			VCHIQ_FOURCC_AS_4CHARS(svc_fourcc),
+			remoteport, localport, size);
+		if (size > 0)
+			vchiq_log_dump_mem("Rcvd", 0, header->data,
+				min(16, size));
+	}
 
-		if (SRVTRACE_ENABLED(service, VCHIQ_LOG_INFO)) {
-			int svc_fourcc;
+	if (((unsigned long)header & VCHIQ_SLOT_MASK) +
+	    calc_stride(size) > VCHIQ_SLOT_SIZE) {
+		vchiq_log_error(vchiq_core_log_level,
+			"header %pK (msgid %x) - size %x too big for slot",
+			header, (unsigned int)msgid,
+			(unsigned int)size);
+		WARN(1, "oversized for slot\n");
+	}
 
-			svc_fourcc = service
-				? service->base.fourcc
-				: VCHIQ_MAKE_FOURCC('?', '?', '?', '?');
-			vchiq_log_info(SRVTRACE_LEVEL(service),
-				"Rcvd Msg %s(%u) from %c%c%c%c s:%d d:%d len:%d",
-				msg_type_str(type), type,
-				VCHIQ_FOURCC_AS_4CHARS(svc_fourcc),
-				remoteport, localport, size);
-			if (size > 0)
-				vchiq_log_dump_mem("Rcvd", 0, header->data,
-					min(16, size));
+	switch (type) {
+	case VCHIQ_MSG_OPEN:
+		WARN_ON(!(VCHIQ_MSG_DSTPORT(msgid) == 0));
+		if (!parse_open(state, header))
+			goto bail_not_ready;
+		break;
+	case VCHIQ_MSG_OPENACK:
+		if (size >= sizeof(struct vchiq_openack_payload)) {
+			const struct vchiq_openack_payload *payload =
+				(struct vchiq_openack_payload *)
+				header->data;
+			service->peer_version = payload->version;
 		}
-
-		if (((unsigned long)header & VCHIQ_SLOT_MASK) +
-		    calc_stride(size) > VCHIQ_SLOT_SIZE) {
+		vchiq_log_info(vchiq_core_log_level,
+			"%d: prs OPENACK@%pK,%x (%d->%d) v:%d",
+			state->id, header, size, remoteport, localport,
+			service->peer_version);
+		if (service->srvstate == VCHIQ_SRVSTATE_OPENING) {
+			service->remoteport = remoteport;
+			vchiq_set_service_state(service,
+				VCHIQ_SRVSTATE_OPEN);
+			complete(&service->remove_event);
+		} else {
 			vchiq_log_error(vchiq_core_log_level,
-				"header %pK (msgid %x) - size %x too big for slot",
-				header, (unsigned int)msgid,
-				(unsigned int)size);
-			WARN(1, "oversized for slot\n");
+				"OPENACK received in state %s",
+				srvstate_names[service->srvstate]);
 		}
+		break;
+	case VCHIQ_MSG_CLOSE:
+		WARN_ON(size != 0); /* There should be no data */
 
-		switch (type) {
-		case VCHIQ_MSG_OPEN:
-			WARN_ON(!(VCHIQ_MSG_DSTPORT(msgid) == 0));
-			if (!parse_open(state, header))
-				goto bail_not_ready;
-			break;
-		case VCHIQ_MSG_OPENACK:
-			if (size >= sizeof(struct vchiq_openack_payload)) {
-				const struct vchiq_openack_payload *payload =
-					(struct vchiq_openack_payload *)
-					header->data;
-				service->peer_version = payload->version;
-			}
-			vchiq_log_info(vchiq_core_log_level,
-				"%d: prs OPENACK@%pK,%x (%d->%d) v:%d",
-				state->id, header, size, remoteport, localport,
-				service->peer_version);
-			if (service->srvstate == VCHIQ_SRVSTATE_OPENING) {
-				service->remoteport = remoteport;
-				vchiq_set_service_state(service,
-					VCHIQ_SRVSTATE_OPEN);
-				complete(&service->remove_event);
-			} else {
-				vchiq_log_error(vchiq_core_log_level,
-					"OPENACK received in state %s",
-					srvstate_names[service->srvstate]);
-			}
-			break;
-		case VCHIQ_MSG_CLOSE:
-			WARN_ON(size != 0); /* There should be no data */
-
-			vchiq_log_info(vchiq_core_log_level,
-				"%d: prs CLOSE@%pK (%d->%d)",
-				state->id, header, remoteport, localport);
-
-			mark_service_closing_internal(service, 1);
+		vchiq_log_info(vchiq_core_log_level,
+			"%d: prs CLOSE@%pK (%d->%d)",
+			state->id, header, remoteport, localport);
 
-			if (vchiq_close_service_internal(service,
-				1/*close_recvd*/) == VCHIQ_RETRY)
-				goto bail_not_ready;
+		mark_service_closing_internal(service, 1);
 
-			vchiq_log_info(vchiq_core_log_level,
-				"Close Service %c%c%c%c s:%u d:%d",
-				VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc),
-				service->localport,
-				service->remoteport);
-			break;
-		case VCHIQ_MSG_DATA:
-			vchiq_log_info(vchiq_core_log_level,
-				"%d: prs DATA@%pK,%x (%d->%d)",
-				state->id, header, size, remoteport, localport);
+		if (vchiq_close_service_internal(service,
+			1/*close_recvd*/) == VCHIQ_RETRY)
+			goto bail_not_ready;
 
-			if ((service->remoteport == remoteport) &&
-			    (service->srvstate == VCHIQ_SRVSTATE_OPEN)) {
-				header->msgid = msgid | VCHIQ_MSGID_CLAIMED;
-				claim_slot(state->rx_info);
+		vchiq_log_info(vchiq_core_log_level,
+			"Close Service %c%c%c%c s:%u d:%d",
+			VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc),
+			service->localport,
+			service->remoteport);
+		break;
+	case VCHIQ_MSG_DATA:
+		vchiq_log_info(vchiq_core_log_level,
+			"%d: prs DATA@%pK,%x (%d->%d)",
+			state->id, header, size, remoteport, localport);
+
+		if ((service->remoteport == remoteport) &&
+		    (service->srvstate == VCHIQ_SRVSTATE_OPEN)) {
+			header->msgid = msgid | VCHIQ_MSGID_CLAIMED;
+			claim_slot(state->rx_info);
+			DEBUG_TRACE(PARSE_LINE);
+			if (make_service_callback(service,
+				VCHIQ_MESSAGE_AVAILABLE, header,
+				NULL) == VCHIQ_RETRY) {
 				DEBUG_TRACE(PARSE_LINE);
-				if (make_service_callback(service,
-					VCHIQ_MESSAGE_AVAILABLE, header,
-					NULL) == VCHIQ_RETRY) {
-					DEBUG_TRACE(PARSE_LINE);
-					goto bail_not_ready;
-				}
-				VCHIQ_SERVICE_STATS_INC(service, ctrl_rx_count);
-				VCHIQ_SERVICE_STATS_ADD(service, ctrl_rx_bytes,
-					size);
-			} else {
-				VCHIQ_STATS_INC(state, error_count);
+				goto bail_not_ready;
 			}
-			break;
-		case VCHIQ_MSG_CONNECT:
-			vchiq_log_info(vchiq_core_log_level,
-				"%d: prs CONNECT@%pK", state->id, header);
-			state->version_common =	((struct vchiq_slot_zero *)
-						 state->slot_data)->version;
-			complete(&state->connect);
-			break;
-		case VCHIQ_MSG_BULK_RX:
-		case VCHIQ_MSG_BULK_TX:
-			/*
-			 * We should never receive a bulk request from the
-			 * other side since we're not setup to perform as the
-			 * master.
-			 */
-			WARN_ON(1);
-			break;
-		case VCHIQ_MSG_BULK_RX_DONE:
-		case VCHIQ_MSG_BULK_TX_DONE:
-			if ((service->remoteport == remoteport) &&
-			    (service->srvstate != VCHIQ_SRVSTATE_FREE)) {
-				struct vchiq_bulk_queue *queue;
-				struct vchiq_bulk *bulk;
+			VCHIQ_SERVICE_STATS_INC(service, ctrl_rx_count);
+			VCHIQ_SERVICE_STATS_ADD(service, ctrl_rx_bytes,
+				size);
+		} else {
+			VCHIQ_STATS_INC(state, error_count);
+		}
+		break;
+	case VCHIQ_MSG_CONNECT:
+		vchiq_log_info(vchiq_core_log_level,
+			"%d: prs CONNECT@%pK", state->id, header);
+		state->version_common =	((struct vchiq_slot_zero *)
+					 state->slot_data)->version;
+		complete(&state->connect);
+		break;
+	case VCHIQ_MSG_BULK_RX:
+	case VCHIQ_MSG_BULK_TX:
+		/*
+		 * We should never receive a bulk request from the
+		 * other side since we're not setup to perform as the
+		 * master.
+		 */
+		WARN_ON(1);
+		break;
+	case VCHIQ_MSG_BULK_RX_DONE:
+	case VCHIQ_MSG_BULK_TX_DONE:
+		if ((service->remoteport == remoteport) &&
+		    (service->srvstate != VCHIQ_SRVSTATE_FREE)) {
+			struct vchiq_bulk_queue *queue;
+			struct vchiq_bulk *bulk;
 
-				queue = (type == VCHIQ_MSG_BULK_RX_DONE) ?
-					&service->bulk_rx : &service->bulk_tx;
+			queue = (type == VCHIQ_MSG_BULK_RX_DONE) ?
+				&service->bulk_rx : &service->bulk_tx;
 
+			DEBUG_TRACE(PARSE_LINE);
+			if (mutex_lock_killable(&service->bulk_mutex)) {
 				DEBUG_TRACE(PARSE_LINE);
-				if (mutex_lock_killable(&service->bulk_mutex)) {
-					DEBUG_TRACE(PARSE_LINE);
-					goto bail_not_ready;
-				}
-				if ((int)(queue->remote_insert -
-					queue->local_insert) >= 0) {
-					vchiq_log_error(vchiq_core_log_level,
-						"%d: prs %s@%pK (%d->%d) unexpected (ri=%d,li=%d)",
-						state->id, msg_type_str(type),
-						header, remoteport, localport,
-						queue->remote_insert,
-						queue->local_insert);
-					mutex_unlock(&service->bulk_mutex);
-					break;
-				}
-				if (queue->process != queue->remote_insert) {
-					pr_err("%s: p %x != ri %x\n",
-					       __func__,
-					       queue->process,
-					       queue->remote_insert);
-					mutex_unlock(&service->bulk_mutex);
-					goto bail_not_ready;
-				}
-
-				bulk = &queue->bulks[
-					BULK_INDEX(queue->remote_insert)];
-				bulk->actual = *(int *)header->data;
-				queue->remote_insert++;
-
-				vchiq_log_info(vchiq_core_log_level,
-					"%d: prs %s@%pK (%d->%d) %x@%pad",
+				goto bail_not_ready;
+			}
+			if ((int)(queue->remote_insert -
+				queue->local_insert) >= 0) {
+				vchiq_log_error(vchiq_core_log_level,
+					"%d: prs %s@%pK (%d->%d) unexpected (ri=%d,li=%d)",
 					state->id, msg_type_str(type),
 					header, remoteport, localport,
-					bulk->actual, &bulk->data);
-
-				vchiq_log_trace(vchiq_core_log_level,
-					"%d: prs:%d %cx li=%x ri=%x p=%x",
-					state->id, localport,
-					(type == VCHIQ_MSG_BULK_RX_DONE) ?
-						'r' : 't',
-					queue->local_insert,
-					queue->remote_insert, queue->process);
-
-				DEBUG_TRACE(PARSE_LINE);
-				WARN_ON(queue->process == queue->local_insert);
-				vchiq_complete_bulk(bulk);
-				queue->process++;
+					queue->remote_insert,
+					queue->local_insert);
 				mutex_unlock(&service->bulk_mutex);
-				DEBUG_TRACE(PARSE_LINE);
-				notify_bulks(service, queue, 1/*retry_poll*/);
-				DEBUG_TRACE(PARSE_LINE);
-			}
-			break;
-		case VCHIQ_MSG_PADDING:
-			vchiq_log_trace(vchiq_core_log_level,
-				"%d: prs PADDING@%pK,%x",
-				state->id, header, size);
-			break;
-		case VCHIQ_MSG_PAUSE:
-			/* If initiated, signal the application thread */
-			vchiq_log_trace(vchiq_core_log_level,
-				"%d: prs PAUSE@%pK,%x",
-				state->id, header, size);
-			if (state->conn_state == VCHIQ_CONNSTATE_PAUSED) {
-				vchiq_log_error(vchiq_core_log_level,
-					"%d: PAUSE received in state PAUSED",
-					state->id);
 				break;
 			}
-			if (state->conn_state != VCHIQ_CONNSTATE_PAUSE_SENT) {
-				/* Send a PAUSE in response */
-				if (queue_message(state, NULL,
-					VCHIQ_MAKE_MSG(VCHIQ_MSG_PAUSE, 0, 0),
-					NULL, NULL, 0, QMFLAGS_NO_MUTEX_UNLOCK)
-				    == VCHIQ_RETRY)
-					goto bail_not_ready;
+			if (queue->process != queue->remote_insert) {
+				pr_err("%s: p %x != ri %x\n",
+				       __func__,
+				       queue->process,
+				       queue->remote_insert);
+				mutex_unlock(&service->bulk_mutex);
+				goto bail_not_ready;
 			}
-			/* At this point slot_mutex is held */
-			vchiq_set_conn_state(state, VCHIQ_CONNSTATE_PAUSED);
-			break;
-		case VCHIQ_MSG_RESUME:
-			vchiq_log_trace(vchiq_core_log_level,
-				"%d: prs RESUME@%pK,%x",
-				state->id, header, size);
-			/* Release the slot mutex */
-			mutex_unlock(&state->slot_mutex);
-			vchiq_set_conn_state(state, VCHIQ_CONNSTATE_CONNECTED);
-			break;
 
-		case VCHIQ_MSG_REMOTE_USE:
-			vchiq_on_remote_use(state);
-			break;
-		case VCHIQ_MSG_REMOTE_RELEASE:
-			vchiq_on_remote_release(state);
-			break;
-		case VCHIQ_MSG_REMOTE_USE_ACTIVE:
-			break;
+			bulk = &queue->bulks[
+				BULK_INDEX(queue->remote_insert)];
+			bulk->actual = *(int *)header->data;
+			queue->remote_insert++;
 
-		default:
+			vchiq_log_info(vchiq_core_log_level,
+				"%d: prs %s@%pK (%d->%d) %x@%pad",
+				state->id, msg_type_str(type),
+				header, remoteport, localport,
+				bulk->actual, &bulk->data);
+
+			vchiq_log_trace(vchiq_core_log_level,
+				"%d: prs:%d %cx li=%x ri=%x p=%x",
+				state->id, localport,
+				(type == VCHIQ_MSG_BULK_RX_DONE) ?
+					'r' : 't',
+				queue->local_insert,
+				queue->remote_insert, queue->process);
+
+			DEBUG_TRACE(PARSE_LINE);
+			WARN_ON(queue->process == queue->local_insert);
+			vchiq_complete_bulk(bulk);
+			queue->process++;
+			mutex_unlock(&service->bulk_mutex);
+			DEBUG_TRACE(PARSE_LINE);
+			notify_bulks(service, queue, 1/*retry_poll*/);
+			DEBUG_TRACE(PARSE_LINE);
+		}
+		break;
+	case VCHIQ_MSG_PADDING:
+		vchiq_log_trace(vchiq_core_log_level,
+			"%d: prs PADDING@%pK,%x",
+			state->id, header, size);
+		break;
+	case VCHIQ_MSG_PAUSE:
+		/* If initiated, signal the application thread */
+		vchiq_log_trace(vchiq_core_log_level,
+			"%d: prs PAUSE@%pK,%x",
+			state->id, header, size);
+		if (state->conn_state == VCHIQ_CONNSTATE_PAUSED) {
 			vchiq_log_error(vchiq_core_log_level,
-				"%d: prs invalid msgid %x@%pK,%x",
-				state->id, msgid, header, size);
-			WARN(1, "invalid message\n");
+				"%d: PAUSE received in state PAUSED",
+				state->id);
 			break;
 		}
+		if (state->conn_state != VCHIQ_CONNSTATE_PAUSE_SENT) {
+			/* Send a PAUSE in response */
+			if (queue_message(state, NULL,
+				VCHIQ_MAKE_MSG(VCHIQ_MSG_PAUSE, 0, 0),
+				NULL, NULL, 0, QMFLAGS_NO_MUTEX_UNLOCK)
+			    == VCHIQ_RETRY)
+				goto bail_not_ready;
+		}
+		/* At this point slot_mutex is held */
+		vchiq_set_conn_state(state, VCHIQ_CONNSTATE_PAUSED);
+		break;
+	case VCHIQ_MSG_RESUME:
+		vchiq_log_trace(vchiq_core_log_level,
+			"%d: prs RESUME@%pK,%x",
+			state->id, header, size);
+		/* Release the slot mutex */
+		mutex_unlock(&state->slot_mutex);
+		vchiq_set_conn_state(state, VCHIQ_CONNSTATE_CONNECTED);
+		break;
+
+	case VCHIQ_MSG_REMOTE_USE:
+		vchiq_on_remote_use(state);
+		break;
+	case VCHIQ_MSG_REMOTE_RELEASE:
+		vchiq_on_remote_release(state);
+		break;
+	case VCHIQ_MSG_REMOTE_USE_ACTIVE:
+		break;
+
+	default:
+		vchiq_log_error(vchiq_core_log_level,
+			"%d: prs invalid msgid %x@%pK,%x",
+			state->id, msgid, header, size);
+		WARN(1, "invalid message\n");
+		break;
+	}
 
 skip_message:
-		if (service) {
-			unlock_service(service);
-			service = NULL;
+	ret = size;
+
+bail_not_ready:
+	if (service)
+		unlock_service(service);
+
+	return ret;
+}
+
+/* Called by the slot handler thread */
+static void
+parse_rx_slots(struct vchiq_state *state)
+{
+	struct vchiq_shared_state *remote = state->remote;
+	int tx_pos;
+
+	DEBUG_INITIALISE(state->local)
+
+	tx_pos = remote->tx_pos;
+
+	while (state->rx_pos != tx_pos) {
+		struct vchiq_header *header;
+		int size;
+
+		DEBUG_TRACE(PARSE_LINE);
+		if (!state->rx_data) {
+			int rx_index;
+
+			WARN_ON(!((state->rx_pos & VCHIQ_SLOT_MASK) == 0));
+			rx_index = remote->slot_queue[
+				SLOT_QUEUE_INDEX_FROM_POS_MASKED(state->rx_pos)];
+			state->rx_data = (char *)SLOT_DATA_FROM_INDEX(state,
+				rx_index);
+			state->rx_info = SLOT_INFO_FROM_INDEX(state, rx_index);
+
+			/*
+			 * Initialise use_count to one, and increment
+			 * release_count at the end of the slot to avoid
+			 * releasing the slot prematurely.
+			 */
+			state->rx_info->use_count = 1;
+			state->rx_info->release_count = 0;
 		}
 
+		header = (struct vchiq_header *)(state->rx_data +
+			(state->rx_pos & VCHIQ_SLOT_MASK));
+		size = parse_message(state, header);
+		if (size < 0)
+			return;
+
 		state->rx_pos += calc_stride(size);
 
 		DEBUG_TRACE(PARSE_LINE);
@@ -1887,10 +1912,6 @@ parse_rx_slots(struct vchiq_state *state)
 			state->rx_data = NULL;
 		}
 	}
-
-bail_not_ready:
-	if (service)
-		unlock_service(service);
 }
 
 /* Called by the slot handler thread */
-- 
2.7.4

Re: [PATCH 18/20] staging: vchiq_core: introduce parse_message

[Dan Carpenter]

On Sat, May 15, 2021 at 09:10:57PM +0200, Stefan Wahren wrote:
> +bail_not_ready:
> +	if (service)
> +		unlock_service(service);

Not related to this patch, but the unlock_service() function doesn't
having anything to do with locking so it's confusing.

> +
> +	return ret;
> +}
> +
> +/* Called by the slot handler thread */
> +static void
> +parse_rx_slots(struct vchiq_state *state)
> +{
> +	struct vchiq_shared_state *remote = state->remote;
> +	int tx_pos;
> +
> +	DEBUG_INITIALISE(state->local)
> +
> +	tx_pos = remote->tx_pos;
> +
> +	while (state->rx_pos != tx_pos) {
> +		struct vchiq_header *header;
> +		int size;
> +
> +		DEBUG_TRACE(PARSE_LINE);
> +		if (!state->rx_data) {
> +			int rx_index;
> +
> +			WARN_ON(!((state->rx_pos & VCHIQ_SLOT_MASK) == 0));

Also not related but this WARN_ON() has a confusing double negative.  In
future patches someone could change this to:

			WARN_ON(state->rx_pos & VCHIQ_SLOT_MASK);

Back in the day, I sort of hired someone to create a TODO website so
that if you're reviewing comments you could put a note:

TODO: staging: vchiq_core: tidy up naming

Then the website would make a list of all the TODOs for that driver.
But then I never went live with the website and we introduces
lore.kernel.org and it's almost searchable so hopefully some day it will
be able to do searches like this...

regards,
dan carpenter

Re: [PATCH 18/20] staging: vchiq_core: introduce parse_message

[Stefan Wahren]

Hi Dan,

Am 17.05.21 um 13:49 schrieb Dan Carpenter:
> On Sat, May 15, 2021 at 09:10:57PM +0200, Stefan Wahren wrote:
>> +bail_not_ready:
>> +	if (service)
>> +		unlock_service(service);
> Not related to this patch, but the unlock_service() function doesn't
> having anything to do with locking so it's confusing.

Any preference about the naming of the both functions (lock_service /
unlock_service)? At the end they are just wrapper around kref_get /
kref_put.

>
>> +
>> +	return ret;
>> +}
>> +
>> +/* Called by the slot handler thread */
>> +static void
>> +parse_rx_slots(struct vchiq_state *state)
>> +{
>> +	struct vchiq_shared_state *remote = state->remote;
>> +	int tx_pos;
>> +
>> +	DEBUG_INITIALISE(state->local)
>> +
>> +	tx_pos = remote->tx_pos;
>> +
>> +	while (state->rx_pos != tx_pos) {
>> +		struct vchiq_header *header;
>> +		int size;
>> +
>> +		DEBUG_TRACE(PARSE_LINE);
>> +		if (!state->rx_data) {
>> +			int rx_index;
>> +
>> +			WARN_ON(!((state->rx_pos & VCHIQ_SLOT_MASK) == 0));
> Also not related but this WARN_ON() has a confusing double negative.  In
> future patches someone could change this to:
>
> 			WARN_ON(state->rx_pos & VCHIQ_SLOT_MASK);

I could add this to V2 of this series.

Regards
Stefan

>
> Back in the day, I sort of hired someone to create a TODO website so
> that if you're reviewing comments you could put a note:
>
> TODO: staging: vchiq_core: tidy up naming
>
> Then the website would make a list of all the TODOs for that driver.
> But then I never went live with the website and we introduces
> lore.kernel.org and it's almost searchable so hopefully some day it will
> be able to do searches like this...
>
> regards,
> dan carpenter
>

Re: [PATCH 18/20] staging: vchiq_core: introduce parse_message

[Dan Carpenter]

On Mon, May 17, 2021 at 07:38:19PM +0200, Stefan Wahren wrote:
> Hi Dan,
> 
> Am 17.05.21 um 13:49 schrieb Dan Carpenter:
> > On Sat, May 15, 2021 at 09:10:57PM +0200, Stefan Wahren wrote:
> >> +bail_not_ready:
> >> +	if (service)
> >> +		unlock_service(service);
> > Not related to this patch, but the unlock_service() function doesn't
> > having anything to do with locking so it's confusing.
> 
> Any preference about the naming of the both functions (lock_service /
> unlock_service)? At the end they are just wrapper around kref_get /
> kref_put.

It should be get/put or hold/release.

> 
> >
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +/* Called by the slot handler thread */
> >> +static void
> >> +parse_rx_slots(struct vchiq_state *state)
> >> +{
> >> +	struct vchiq_shared_state *remote = state->remote;
> >> +	int tx_pos;
> >> +
> >> +	DEBUG_INITIALISE(state->local)
> >> +
> >> +	tx_pos = remote->tx_pos;
> >> +
> >> +	while (state->rx_pos != tx_pos) {
> >> +		struct vchiq_header *header;
> >> +		int size;
> >> +
> >> +		DEBUG_TRACE(PARSE_LINE);
> >> +		if (!state->rx_data) {
> >> +			int rx_index;
> >> +
> >> +			WARN_ON(!((state->rx_pos & VCHIQ_SLOT_MASK) == 0));
> > Also not related but this WARN_ON() has a confusing double negative.  In
> > future patches someone could change this to:
> >
> > 			WARN_ON(state->rx_pos & VCHIQ_SLOT_MASK);
> 
> I could add this to V2 of this series.
> 

Nah, don't worry about it.  You copied that WARN_ON() from the original
code so don't change it except in a different patch.

regards,
dan carpenter

[PATCH 19/20] staging: vchiq_core: introduce defines for close_recvd

[Stefan Wahren]

Use descriptive defines instead of a number with a comment.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c        | 15 ++++++++-------
 1 file changed, 8 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 985e1c6..c22eefc 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -38,6 +38,9 @@
 #define SRVTRACE_ENABLED(srv, lev) \
 	(((srv) && (srv)->trace) || (vchiq_core_msg_log_level >= (lev)))
 
+#define NO_CLOSE_RECVD	0
+#define CLOSE_RECVD	1
+
 struct vchiq_open_payload {
 	int fourcc;
 	int client_id;
@@ -1348,7 +1351,7 @@ poll_services_of_group(struct vchiq_state *state, int group)
 		 */
 		service->public_fourcc = VCHIQ_FOURCC_INVALID;
 
-		if (vchiq_close_service_internal(service, 0/*!close_recvd*/) !=
+		if (vchiq_close_service_internal(service, NO_CLOSE_RECVD) !=
 						 VCHIQ_SUCCESS) {
 			request_poll(state, service, VCHIQ_POLL_REMOVE);
 		} else if (service_flags & BIT(VCHIQ_POLL_TERMINATE)) {
@@ -1357,7 +1360,7 @@ poll_services_of_group(struct vchiq_state *state, int group)
 				state->id, service->localport,
 				service->remoteport);
 			if (vchiq_close_service_internal(
-				service, 0/*!close_recvd*/) !=
+				service, NO_CLOSE_RECVD) !=
 				VCHIQ_SUCCESS)
 				request_poll(state, service,
 					     VCHIQ_POLL_TERMINATE);
@@ -1683,7 +1686,7 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header)
 		mark_service_closing_internal(service, 1);
 
 		if (vchiq_close_service_internal(service,
-			1/*close_recvd*/) == VCHIQ_RETRY)
+			CLOSE_RECVD) == VCHIQ_RETRY)
 			goto bail_not_ready;
 
 		vchiq_log_info(vchiq_core_log_level,
@@ -2976,8 +2979,7 @@ vchiq_close_service(unsigned int handle)
 	mark_service_closing(service);
 
 	if (current == service->state->slot_handler_thread) {
-		status = vchiq_close_service_internal(service,
-			0/*!close_recvd*/);
+		status = vchiq_close_service_internal(service, NO_CLOSE_RECVD);
 		WARN_ON(status == VCHIQ_RETRY);
 	} else {
 	/* Mark the service for termination by the slot handler */
@@ -3041,8 +3043,7 @@ vchiq_remove_service(unsigned int handle)
 		 */
 		service->public_fourcc = VCHIQ_FOURCC_INVALID;
 
-		status = vchiq_close_service_internal(service,
-			0/*!close_recvd*/);
+		status = vchiq_close_service_internal(service, NO_CLOSE_RECVD);
 		WARN_ON(status == VCHIQ_RETRY);
 	} else {
 		/* Mark the service for removal by the slot handler */
-- 
2.7.4