* [PATCH V2 01/10] staging: vchiq_arm: Unify return code variable
2024-06-10 21:02 [PATCH V2 00/10] staging: vc04_services: Random cleanups Stefan Wahren
@ 2024-06-10 21:02 ` Stefan Wahren
2024-06-11 21:14 ` Laurent Pinchart
2024-06-10 21:02 ` [PATCH V2 02/10] staging: vchiq_arm: Drop obsolete comment Stefan Wahren
` (8 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Stefan Wahren @ 2024-06-10 21:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Florian Fainelli
Cc: Umang Jain, Laurent Pinchart, linux-staging, linux-arm-kernel,
Stefan Wahren
The file vchiq_arm uses a wild mixture of variable names for
return codes. Unify them by using the common name "ret".
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
.../interface/vchiq_arm/vchiq_arm.c | 141 +++++++++---------
1 file changed, 69 insertions(+), 72 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 69daeba974f2..f3815101e1c4 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -758,8 +758,8 @@ void free_bulk_waiter(struct vchiq_instance *instance)
int vchiq_shutdown(struct vchiq_instance *instance)
{
- int status = 0;
struct vchiq_state *state = instance->state;
+ int ret = 0;
if (mutex_lock_killable(&state->mutex))
return -EAGAIN;
@@ -769,12 +769,12 @@ int vchiq_shutdown(struct vchiq_instance *instance)
mutex_unlock(&state->mutex);
- dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
+ dev_dbg(state->dev, "core: (%p): returning %d\n", instance, ret);
free_bulk_waiter(instance);
kfree(instance);
- return status;
+ return ret;
}
EXPORT_SYMBOL(vchiq_shutdown);
@@ -785,26 +785,26 @@ static int vchiq_is_connected(struct vchiq_instance *instance)
int vchiq_connect(struct vchiq_instance *instance)
{
- int status;
struct vchiq_state *state = instance->state;
+ int ret;
if (mutex_lock_killable(&state->mutex)) {
dev_dbg(state->dev,
"core: call to mutex_lock failed\n");
- status = -EAGAIN;
+ ret = -EAGAIN;
goto failed;
}
- status = vchiq_connect_internal(state, instance);
+ ret = vchiq_connect_internal(state, instance);
- if (!status)
+ if (!ret)
instance->connected = 1;
mutex_unlock(&state->mutex);
failed:
- dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
+ dev_dbg(state->dev, "core: (%p): returning %d\n", instance, ret);
- return status;
+ return ret;
}
EXPORT_SYMBOL(vchiq_connect);
@@ -813,10 +813,9 @@ vchiq_add_service(struct vchiq_instance *instance,
const struct vchiq_service_params_kernel *params,
unsigned int *phandle)
{
- int status;
struct vchiq_state *state = instance->state;
struct vchiq_service *service = NULL;
- int srvstate;
+ int srvstate, ret;
*phandle = VCHIQ_SERVICE_HANDLE_INVALID;
@@ -828,14 +827,14 @@ vchiq_add_service(struct vchiq_instance *instance,
if (service) {
*phandle = service->handle;
- status = 0;
+ ret = 0;
} else {
- status = -EINVAL;
+ ret = -EINVAL;
}
- dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
+ dev_dbg(state->dev, "core: (%p): returning %d\n", instance, ret);
- return status;
+ return ret;
}
int
@@ -843,9 +842,9 @@ vchiq_open_service(struct vchiq_instance *instance,
const struct vchiq_service_params_kernel *params,
unsigned int *phandle)
{
- int status = -EINVAL;
struct vchiq_state *state = instance->state;
struct vchiq_service *service = NULL;
+ int ret = -EINVAL;
*phandle = VCHIQ_SERVICE_HANDLE_INVALID;
@@ -856,17 +855,17 @@ vchiq_open_service(struct vchiq_instance *instance,
if (service) {
*phandle = service->handle;
- status = vchiq_open_service_internal(service, current->pid);
- if (status) {
+ ret = vchiq_open_service_internal(service, current->pid);
+ if (ret) {
vchiq_remove_service(instance, service->handle);
*phandle = VCHIQ_SERVICE_HANDLE_INVALID;
}
}
failed:
- dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
+ dev_dbg(state->dev, "core: (%p): returning %d\n", instance, ret);
- return status;
+ return ret;
}
EXPORT_SYMBOL(vchiq_open_service);
@@ -874,20 +873,20 @@ int
vchiq_bulk_transmit(struct vchiq_instance *instance, unsigned int handle, const void *data,
unsigned int size, void *userdata, enum vchiq_bulk_mode mode)
{
- int status;
+ int ret;
while (1) {
switch (mode) {
case VCHIQ_BULK_MODE_NOCALLBACK:
case VCHIQ_BULK_MODE_CALLBACK:
- status = vchiq_bulk_transfer(instance, handle,
- (void *)data, NULL,
- size, userdata, mode,
- VCHIQ_BULK_TRANSMIT);
+ ret = vchiq_bulk_transfer(instance, handle,
+ (void *)data, NULL,
+ size, userdata, mode,
+ VCHIQ_BULK_TRANSMIT);
break;
case VCHIQ_BULK_MODE_BLOCKING:
- status = vchiq_blocking_bulk_transfer(instance, handle, (void *)data, size,
- VCHIQ_BULK_TRANSMIT);
+ ret = vchiq_blocking_bulk_transfer(instance, handle, (void *)data, size,
+ VCHIQ_BULK_TRANSMIT);
break;
default:
return -EINVAL;
@@ -898,13 +897,13 @@ vchiq_bulk_transmit(struct vchiq_instance *instance, unsigned int handle, const
* to implement a retry mechanism since this function is
* supposed to block until queued
*/
- if (status != -EAGAIN)
+ if (ret != -EAGAIN)
break;
msleep(1);
}
- return status;
+ return ret;
}
EXPORT_SYMBOL(vchiq_bulk_transmit);
@@ -912,19 +911,19 @@ int vchiq_bulk_receive(struct vchiq_instance *instance, unsigned int handle,
void *data, unsigned int size, void *userdata,
enum vchiq_bulk_mode mode)
{
- int status;
+ int ret;
while (1) {
switch (mode) {
case VCHIQ_BULK_MODE_NOCALLBACK:
case VCHIQ_BULK_MODE_CALLBACK:
- status = vchiq_bulk_transfer(instance, handle, data, NULL,
- size, userdata,
- mode, VCHIQ_BULK_RECEIVE);
+ ret = vchiq_bulk_transfer(instance, handle, data, NULL,
+ size, userdata,
+ mode, VCHIQ_BULK_RECEIVE);
break;
case VCHIQ_BULK_MODE_BLOCKING:
- status = vchiq_blocking_bulk_transfer(instance, handle, (void *)data, size,
- VCHIQ_BULK_RECEIVE);
+ ret = vchiq_blocking_bulk_transfer(instance, handle, (void *)data, size,
+ VCHIQ_BULK_RECEIVE);
break;
default:
return -EINVAL;
@@ -935,13 +934,13 @@ int vchiq_bulk_receive(struct vchiq_instance *instance, unsigned int handle,
* to implement a retry mechanism since this function is
* supposed to block until queued
*/
- if (status != -EAGAIN)
+ if (ret != -EAGAIN)
break;
msleep(1);
}
- return status;
+ return ret;
}
EXPORT_SYMBOL(vchiq_bulk_receive);
@@ -950,8 +949,8 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
unsigned int size, enum vchiq_bulk_dir dir)
{
struct vchiq_service *service;
- int status;
struct bulk_waiter_node *waiter = NULL, *iter;
+ int ret;
service = find_service_by_handle(instance, handle);
if (!service)
@@ -991,10 +990,10 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
return -ENOMEM;
}
- status = vchiq_bulk_transfer(instance, handle, data, NULL, size,
- &waiter->bulk_waiter,
- VCHIQ_BULK_MODE_BLOCKING, dir);
- if ((status != -EAGAIN) || fatal_signal_pending(current) || !waiter->bulk_waiter.bulk) {
+ ret = vchiq_bulk_transfer(instance, handle, data, NULL, size,
+ &waiter->bulk_waiter,
+ VCHIQ_BULK_MODE_BLOCKING, dir);
+ if ((ret != -EAGAIN) || fatal_signal_pending(current) || !waiter->bulk_waiter.bulk) {
struct vchiq_bulk *bulk = waiter->bulk_waiter.bulk;
if (bulk) {
@@ -1013,7 +1012,7 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
waiter, current->pid);
}
- return status;
+ return ret;
}
static int
@@ -1137,17 +1136,17 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
*/
if ((user_service->message_available_pos -
instance->completion_remove) < 0) {
- int status;
+ int ret;
dev_dbg(instance->state->dev,
"arm: Inserting extra MESSAGE_AVAILABLE\n");
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
- status = add_completion(instance, reason, NULL, user_service,
- bulk_userdata);
- if (status) {
+ ret = add_completion(instance, reason, NULL, user_service,
+ bulk_userdata);
+ if (ret) {
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
vchiq_service_put(service);
- return status;
+ return ret;
}
}
@@ -1294,8 +1293,6 @@ vchiq_keepalive_thread_func(void *v)
{
struct vchiq_state *state = (struct vchiq_state *)v;
struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state);
-
- int status;
struct vchiq_instance *instance;
unsigned int ka_handle;
int ret;
@@ -1313,16 +1310,16 @@ vchiq_keepalive_thread_func(void *v)
goto exit;
}
- status = vchiq_connect(instance);
- if (status) {
- dev_err(state->dev, "suspend: %s: vchiq_connect failed %d\n", __func__, status);
+ ret = vchiq_connect(instance);
+ if (ret) {
+ dev_err(state->dev, "suspend: %s: vchiq_connect failed %d\n", __func__, ret);
goto shutdown;
}
- status = vchiq_add_service(instance, ¶ms, &ka_handle);
- if (status) {
+ ret = vchiq_add_service(instance, ¶ms, &ka_handle);
+ if (ret) {
dev_err(state->dev, "suspend: %s: vchiq_open_service failed %d\n",
- __func__, status);
+ __func__, ret);
goto shutdown;
}
@@ -1348,17 +1345,17 @@ vchiq_keepalive_thread_func(void *v)
*/
while (uc--) {
atomic_inc(&arm_state->ka_use_ack_count);
- status = vchiq_use_service(instance, ka_handle);
- if (status) {
+ ret = vchiq_use_service(instance, ka_handle);
+ if (ret) {
dev_err(state->dev, "suspend: %s: vchiq_use_service error %d\n",
- __func__, status);
+ __func__, ret);
}
}
while (rc--) {
- status = vchiq_release_service(instance, ka_handle);
- if (status) {
+ ret = vchiq_release_service(instance, ka_handle);
+ if (ret) {
dev_err(state->dev, "suspend: %s: vchiq_release_service error %d\n",
- __func__, status);
+ __func__, ret);
}
}
}
@@ -1408,13 +1405,13 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
write_unlock_bh(&arm_state->susp_res_lock);
if (!ret) {
- int status = 0;
+ int ret = 0;
long ack_cnt = atomic_xchg(&arm_state->ka_use_ack_count, 0);
- while (ack_cnt && !status) {
+ while (ack_cnt && !ret) {
/* Send the use notify to videocore */
- status = vchiq_send_remote_use_active(state);
- if (!status)
+ ret = vchiq_send_remote_use_active(state);
+ if (!ret)
ack_cnt--;
else
atomic_add(ack_cnt, &arm_state->ka_use_ack_count);
@@ -1730,7 +1727,7 @@ static int vchiq_probe(struct platform_device *pdev)
struct device_node *fw_node;
const struct vchiq_platform_info *info;
struct vchiq_drv_mgmt *mgmt;
- int err;
+ int ret;
info = of_device_get_match_data(&pdev->dev);
if (!info)
@@ -1755,8 +1752,8 @@ static int vchiq_probe(struct platform_device *pdev)
mgmt->info = info;
platform_set_drvdata(pdev, mgmt);
- err = vchiq_platform_init(pdev, &mgmt->state);
- if (err)
+ ret = vchiq_platform_init(pdev, &mgmt->state);
+ if (ret)
goto failed_platform_init;
vchiq_debugfs_init(&mgmt->state);
@@ -1768,8 +1765,8 @@ static int vchiq_probe(struct platform_device *pdev)
* Simply exit on error since the function handles cleanup in
* cases of failure.
*/
- err = vchiq_register_chrdev(&pdev->dev);
- if (err) {
+ ret = vchiq_register_chrdev(&pdev->dev);
+ if (ret) {
dev_err(&pdev->dev, "arm: Failed to initialize vchiq cdev\n");
goto error_exit;
}
@@ -1782,7 +1779,7 @@ static int vchiq_probe(struct platform_device *pdev)
failed_platform_init:
dev_err(&pdev->dev, "arm: Could not initialize vchiq platform\n");
error_exit:
- return err;
+ return ret;
}
static void vchiq_remove(struct platform_device *pdev)
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V2 01/10] staging: vchiq_arm: Unify return code variable
2024-06-10 21:02 ` [PATCH V2 01/10] staging: vchiq_arm: Unify return code variable Stefan Wahren
@ 2024-06-11 21:14 ` Laurent Pinchart
0 siblings, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2024-06-11 21:14 UTC (permalink / raw)
To: Stefan Wahren
Cc: Greg Kroah-Hartman, Florian Fainelli, Umang Jain, linux-staging,
linux-arm-kernel
Hi Stefan,
Thank you for the patch.
On Mon, Jun 10, 2024 at 11:02:11PM +0200, Stefan Wahren wrote:
> The file vchiq_arm uses a wild mixture of variable names for
> return codes. Unify them by using the common name "ret".
>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> .../interface/vchiq_arm/vchiq_arm.c | 141 +++++++++---------
> 1 file changed, 69 insertions(+), 72 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 69daeba974f2..f3815101e1c4 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -758,8 +758,8 @@ void free_bulk_waiter(struct vchiq_instance *instance)
>
> int vchiq_shutdown(struct vchiq_instance *instance)
> {
> - int status = 0;
> struct vchiq_state *state = instance->state;
> + int ret = 0;
>
> if (mutex_lock_killable(&state->mutex))
> return -EAGAIN;
> @@ -769,12 +769,12 @@ int vchiq_shutdown(struct vchiq_instance *instance)
>
> mutex_unlock(&state->mutex);
>
> - dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
> + dev_dbg(state->dev, "core: (%p): returning %d\n", instance, ret);
>
> free_bulk_waiter(instance);
> kfree(instance);
>
> - return status;
> + return ret;
> }
> EXPORT_SYMBOL(vchiq_shutdown);
>
> @@ -785,26 +785,26 @@ static int vchiq_is_connected(struct vchiq_instance *instance)
>
> int vchiq_connect(struct vchiq_instance *instance)
> {
> - int status;
> struct vchiq_state *state = instance->state;
> + int ret;
>
> if (mutex_lock_killable(&state->mutex)) {
> dev_dbg(state->dev,
> "core: call to mutex_lock failed\n");
> - status = -EAGAIN;
> + ret = -EAGAIN;
> goto failed;
> }
> - status = vchiq_connect_internal(state, instance);
> + ret = vchiq_connect_internal(state, instance);
>
> - if (!status)
> + if (!ret)
> instance->connected = 1;
>
> mutex_unlock(&state->mutex);
>
> failed:
> - dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
> + dev_dbg(state->dev, "core: (%p): returning %d\n", instance, ret);
>
> - return status;
> + return ret;
> }
> EXPORT_SYMBOL(vchiq_connect);
>
> @@ -813,10 +813,9 @@ vchiq_add_service(struct vchiq_instance *instance,
> const struct vchiq_service_params_kernel *params,
> unsigned int *phandle)
> {
> - int status;
> struct vchiq_state *state = instance->state;
> struct vchiq_service *service = NULL;
> - int srvstate;
> + int srvstate, ret;
>
> *phandle = VCHIQ_SERVICE_HANDLE_INVALID;
>
> @@ -828,14 +827,14 @@ vchiq_add_service(struct vchiq_instance *instance,
>
> if (service) {
> *phandle = service->handle;
> - status = 0;
> + ret = 0;
> } else {
> - status = -EINVAL;
> + ret = -EINVAL;
> }
>
> - dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
> + dev_dbg(state->dev, "core: (%p): returning %d\n", instance, ret);
>
> - return status;
> + return ret;
> }
>
> int
> @@ -843,9 +842,9 @@ vchiq_open_service(struct vchiq_instance *instance,
> const struct vchiq_service_params_kernel *params,
> unsigned int *phandle)
> {
> - int status = -EINVAL;
> struct vchiq_state *state = instance->state;
> struct vchiq_service *service = NULL;
> + int ret = -EINVAL;
>
> *phandle = VCHIQ_SERVICE_HANDLE_INVALID;
>
> @@ -856,17 +855,17 @@ vchiq_open_service(struct vchiq_instance *instance,
>
> if (service) {
> *phandle = service->handle;
> - status = vchiq_open_service_internal(service, current->pid);
> - if (status) {
> + ret = vchiq_open_service_internal(service, current->pid);
> + if (ret) {
> vchiq_remove_service(instance, service->handle);
> *phandle = VCHIQ_SERVICE_HANDLE_INVALID;
> }
> }
>
> failed:
> - dev_dbg(state->dev, "core: (%p): returning %d\n", instance, status);
> + dev_dbg(state->dev, "core: (%p): returning %d\n", instance, ret);
>
> - return status;
> + return ret;
> }
> EXPORT_SYMBOL(vchiq_open_service);
>
> @@ -874,20 +873,20 @@ int
> vchiq_bulk_transmit(struct vchiq_instance *instance, unsigned int handle, const void *data,
> unsigned int size, void *userdata, enum vchiq_bulk_mode mode)
> {
> - int status;
> + int ret;
>
> while (1) {
> switch (mode) {
> case VCHIQ_BULK_MODE_NOCALLBACK:
> case VCHIQ_BULK_MODE_CALLBACK:
> - status = vchiq_bulk_transfer(instance, handle,
> - (void *)data, NULL,
> - size, userdata, mode,
> - VCHIQ_BULK_TRANSMIT);
> + ret = vchiq_bulk_transfer(instance, handle,
> + (void *)data, NULL,
> + size, userdata, mode,
> + VCHIQ_BULK_TRANSMIT);
> break;
> case VCHIQ_BULK_MODE_BLOCKING:
> - status = vchiq_blocking_bulk_transfer(instance, handle, (void *)data, size,
> - VCHIQ_BULK_TRANSMIT);
> + ret = vchiq_blocking_bulk_transfer(instance, handle, (void *)data, size,
> + VCHIQ_BULK_TRANSMIT);
> break;
> default:
> return -EINVAL;
> @@ -898,13 +897,13 @@ vchiq_bulk_transmit(struct vchiq_instance *instance, unsigned int handle, const
> * to implement a retry mechanism since this function is
> * supposed to block until queued
> */
> - if (status != -EAGAIN)
> + if (ret != -EAGAIN)
> break;
>
> msleep(1);
> }
>
> - return status;
> + return ret;
> }
> EXPORT_SYMBOL(vchiq_bulk_transmit);
>
> @@ -912,19 +911,19 @@ int vchiq_bulk_receive(struct vchiq_instance *instance, unsigned int handle,
> void *data, unsigned int size, void *userdata,
> enum vchiq_bulk_mode mode)
> {
> - int status;
> + int ret;
>
> while (1) {
> switch (mode) {
> case VCHIQ_BULK_MODE_NOCALLBACK:
> case VCHIQ_BULK_MODE_CALLBACK:
> - status = vchiq_bulk_transfer(instance, handle, data, NULL,
> - size, userdata,
> - mode, VCHIQ_BULK_RECEIVE);
> + ret = vchiq_bulk_transfer(instance, handle, data, NULL,
> + size, userdata,
> + mode, VCHIQ_BULK_RECEIVE);
> break;
> case VCHIQ_BULK_MODE_BLOCKING:
> - status = vchiq_blocking_bulk_transfer(instance, handle, (void *)data, size,
> - VCHIQ_BULK_RECEIVE);
> + ret = vchiq_blocking_bulk_transfer(instance, handle, (void *)data, size,
> + VCHIQ_BULK_RECEIVE);
> break;
> default:
> return -EINVAL;
> @@ -935,13 +934,13 @@ int vchiq_bulk_receive(struct vchiq_instance *instance, unsigned int handle,
> * to implement a retry mechanism since this function is
> * supposed to block until queued
> */
> - if (status != -EAGAIN)
> + if (ret != -EAGAIN)
> break;
>
> msleep(1);
> }
>
> - return status;
> + return ret;
> }
> EXPORT_SYMBOL(vchiq_bulk_receive);
>
> @@ -950,8 +949,8 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
> unsigned int size, enum vchiq_bulk_dir dir)
> {
> struct vchiq_service *service;
> - int status;
> struct bulk_waiter_node *waiter = NULL, *iter;
> + int ret;
>
> service = find_service_by_handle(instance, handle);
> if (!service)
> @@ -991,10 +990,10 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
> return -ENOMEM;
> }
>
> - status = vchiq_bulk_transfer(instance, handle, data, NULL, size,
> - &waiter->bulk_waiter,
> - VCHIQ_BULK_MODE_BLOCKING, dir);
> - if ((status != -EAGAIN) || fatal_signal_pending(current) || !waiter->bulk_waiter.bulk) {
> + ret = vchiq_bulk_transfer(instance, handle, data, NULL, size,
> + &waiter->bulk_waiter,
> + VCHIQ_BULK_MODE_BLOCKING, dir);
> + if ((ret != -EAGAIN) || fatal_signal_pending(current) || !waiter->bulk_waiter.bulk) {
> struct vchiq_bulk *bulk = waiter->bulk_waiter.bulk;
>
> if (bulk) {
> @@ -1013,7 +1012,7 @@ vchiq_blocking_bulk_transfer(struct vchiq_instance *instance, unsigned int handl
> waiter, current->pid);
> }
>
> - return status;
> + return ret;
> }
>
> static int
> @@ -1137,17 +1136,17 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
> */
> if ((user_service->message_available_pos -
> instance->completion_remove) < 0) {
> - int status;
> + int ret;
>
> dev_dbg(instance->state->dev,
> "arm: Inserting extra MESSAGE_AVAILABLE\n");
> DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> - status = add_completion(instance, reason, NULL, user_service,
> - bulk_userdata);
> - if (status) {
> + ret = add_completion(instance, reason, NULL, user_service,
> + bulk_userdata);
> + if (ret) {
> DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> vchiq_service_put(service);
> - return status;
> + return ret;
> }
> }
>
> @@ -1294,8 +1293,6 @@ vchiq_keepalive_thread_func(void *v)
> {
> struct vchiq_state *state = (struct vchiq_state *)v;
> struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state);
> -
> - int status;
> struct vchiq_instance *instance;
> unsigned int ka_handle;
> int ret;
> @@ -1313,16 +1310,16 @@ vchiq_keepalive_thread_func(void *v)
> goto exit;
> }
>
> - status = vchiq_connect(instance);
> - if (status) {
> - dev_err(state->dev, "suspend: %s: vchiq_connect failed %d\n", __func__, status);
> + ret = vchiq_connect(instance);
> + if (ret) {
> + dev_err(state->dev, "suspend: %s: vchiq_connect failed %d\n", __func__, ret);
> goto shutdown;
> }
>
> - status = vchiq_add_service(instance, ¶ms, &ka_handle);
> - if (status) {
> + ret = vchiq_add_service(instance, ¶ms, &ka_handle);
> + if (ret) {
> dev_err(state->dev, "suspend: %s: vchiq_open_service failed %d\n",
> - __func__, status);
> + __func__, ret);
> goto shutdown;
> }
>
> @@ -1348,17 +1345,17 @@ vchiq_keepalive_thread_func(void *v)
> */
> while (uc--) {
> atomic_inc(&arm_state->ka_use_ack_count);
> - status = vchiq_use_service(instance, ka_handle);
> - if (status) {
> + ret = vchiq_use_service(instance, ka_handle);
> + if (ret) {
> dev_err(state->dev, "suspend: %s: vchiq_use_service error %d\n",
> - __func__, status);
> + __func__, ret);
> }
> }
> while (rc--) {
> - status = vchiq_release_service(instance, ka_handle);
> - if (status) {
> + ret = vchiq_release_service(instance, ka_handle);
> + if (ret) {
> dev_err(state->dev, "suspend: %s: vchiq_release_service error %d\n",
> - __func__, status);
> + __func__, ret);
> }
> }
> }
> @@ -1408,13 +1405,13 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service,
> write_unlock_bh(&arm_state->susp_res_lock);
>
> if (!ret) {
> - int status = 0;
> + int ret = 0;
> long ack_cnt = atomic_xchg(&arm_state->ka_use_ack_count, 0);
>
> - while (ack_cnt && !status) {
> + while (ack_cnt && !ret) {
> /* Send the use notify to videocore */
> - status = vchiq_send_remote_use_active(state);
> - if (!status)
> + ret = vchiq_send_remote_use_active(state);
> + if (!ret)
> ack_cnt--;
> else
> atomic_add(ack_cnt, &arm_state->ka_use_ack_count);
> @@ -1730,7 +1727,7 @@ static int vchiq_probe(struct platform_device *pdev)
> struct device_node *fw_node;
> const struct vchiq_platform_info *info;
> struct vchiq_drv_mgmt *mgmt;
> - int err;
> + int ret;
>
> info = of_device_get_match_data(&pdev->dev);
> if (!info)
> @@ -1755,8 +1752,8 @@ static int vchiq_probe(struct platform_device *pdev)
> mgmt->info = info;
> platform_set_drvdata(pdev, mgmt);
>
> - err = vchiq_platform_init(pdev, &mgmt->state);
> - if (err)
> + ret = vchiq_platform_init(pdev, &mgmt->state);
> + if (ret)
> goto failed_platform_init;
>
> vchiq_debugfs_init(&mgmt->state);
> @@ -1768,8 +1765,8 @@ static int vchiq_probe(struct platform_device *pdev)
> * Simply exit on error since the function handles cleanup in
> * cases of failure.
> */
> - err = vchiq_register_chrdev(&pdev->dev);
> - if (err) {
> + ret = vchiq_register_chrdev(&pdev->dev);
> + if (ret) {
> dev_err(&pdev->dev, "arm: Failed to initialize vchiq cdev\n");
> goto error_exit;
> }
> @@ -1782,7 +1779,7 @@ static int vchiq_probe(struct platform_device *pdev)
> failed_platform_init:
> dev_err(&pdev->dev, "arm: Could not initialize vchiq platform\n");
> error_exit:
> - return err;
> + return ret;
> }
>
> static void vchiq_remove(struct platform_device *pdev)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V2 02/10] staging: vchiq_arm: Drop obsolete comment
2024-06-10 21:02 [PATCH V2 00/10] staging: vc04_services: Random cleanups Stefan Wahren
2024-06-10 21:02 ` [PATCH V2 01/10] staging: vchiq_arm: Unify return code variable Stefan Wahren
@ 2024-06-10 21:02 ` Stefan Wahren
2024-06-10 21:02 ` [PATCH V2 03/10] staging: vchiq_core: Drop non-functional struct members Stefan Wahren
` (7 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Stefan Wahren @ 2024-06-10 21:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Florian Fainelli
Cc: Umang Jain, Laurent Pinchart, linux-staging, linux-arm-kernel,
Stefan Wahren
The BUG_ON has been replaced with WARN_ON. So we can drop the
comment.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 1 -
1 file changed, 1 deletion(-)
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 f3815101e1c4..094ba1aea8f6 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1448,7 +1448,6 @@ vchiq_release_internal(struct vchiq_state *state, struct vchiq_service *service)
write_lock_bh(&arm_state->susp_res_lock);
if (!arm_state->videocore_use_count || !(*entity_uc)) {
- /* Don't use BUG_ON - don't allow user thread to crash kernel */
WARN_ON(!arm_state->videocore_use_count);
WARN_ON(!(*entity_uc));
ret = -EINVAL;
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH V2 03/10] staging: vchiq_core: Drop non-functional struct members
2024-06-10 21:02 [PATCH V2 00/10] staging: vc04_services: Random cleanups Stefan Wahren
2024-06-10 21:02 ` [PATCH V2 01/10] staging: vchiq_arm: Unify return code variable Stefan Wahren
2024-06-10 21:02 ` [PATCH V2 02/10] staging: vchiq_arm: Drop obsolete comment Stefan Wahren
@ 2024-06-10 21:02 ` Stefan Wahren
2024-06-10 21:02 ` [PATCH V2 04/10] staging: vchiq_arm: Drop unnecessary declarations Stefan Wahren
` (6 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Stefan Wahren @ 2024-06-10 21:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Florian Fainelli
Cc: Umang Jain, Laurent Pinchart, linux-staging, linux-arm-kernel,
Stefan Wahren
There are some struct members, which don't have a real
function. So it's safe to drop them.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 2 --
.../staging/vc04_services/interface/vchiq_arm/vchiq_core.h | 4 ----
2 files changed, 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 df3af821f218..51cfc366519b 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -2163,14 +2163,12 @@ vchiq_init_state(struct vchiq_state *state, struct vchiq_slot_zero *slot_zero, s
mutex_init(&state->slot_mutex);
mutex_init(&state->recycle_mutex);
mutex_init(&state->sync_mutex);
- mutex_init(&state->bulk_transfer_mutex);
spin_lock_init(&state->msg_queue_spinlock);
spin_lock_init(&state->bulk_waiter_spinlock);
spin_lock_init(&state->quota_spinlock);
init_completion(&state->slot_available_event);
- init_completion(&state->slot_remove_event);
init_completion(&state->data_quota_event);
state->slot_queue_available = 0;
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 8af209e34fb2..be20abcfad75 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -347,8 +347,6 @@ struct vchiq_state {
struct mutex sync_mutex;
- struct mutex bulk_transfer_mutex;
-
spinlock_t msg_queue_spinlock;
spinlock_t bulk_waiter_spinlock;
@@ -393,8 +391,6 @@ struct vchiq_state {
/* Signalled when a free slot becomes available. */
struct completion slot_available_event;
- struct completion slot_remove_event;
-
/* Signalled when a free data slot becomes available. */
struct completion data_quota_event;
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH V2 04/10] staging: vchiq_arm: Drop unnecessary declarations
2024-06-10 21:02 [PATCH V2 00/10] staging: vc04_services: Random cleanups Stefan Wahren
` (2 preceding siblings ...)
2024-06-10 21:02 ` [PATCH V2 03/10] staging: vchiq_core: Drop non-functional struct members Stefan Wahren
@ 2024-06-10 21:02 ` Stefan Wahren
2024-06-10 21:02 ` [PATCH V2 05/10] staging: vchiq_arm: Get the rid off struct vchiq_2835_state Stefan Wahren
` (5 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Stefan Wahren @ 2024-06-10 21:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Florian Fainelli
Cc: Umang Jain, Laurent Pinchart, linux-staging, linux-arm-kernel,
Stefan Wahren
These declarations are left from cleanups and not necessary
anymore. So we can drop them.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h | 3 ---
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h | 2 --
2 files changed, 5 deletions(-)
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 fd1b9d3555ce..b402aac333d9 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
@@ -109,9 +109,6 @@ vchiq_release_service(struct vchiq_instance *instance, unsigned int handle);
extern int
vchiq_check_service(struct vchiq_service *service);
-extern void
-vchiq_dump_platform_use_state(struct vchiq_state *state);
-
extern void
vchiq_dump_service_use_state(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 be20abcfad75..3c7a6838ddba 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h
@@ -539,8 +539,6 @@ int vchiq_platform_init_state(struct vchiq_state *state);
int vchiq_check_service(struct vchiq_service *service);
-void vchiq_on_remote_use_active(struct vchiq_state *state);
-
int vchiq_send_remote_use(struct vchiq_state *state);
int vchiq_send_remote_use_active(struct vchiq_state *state);
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH V2 05/10] staging: vchiq_arm: Get the rid off struct vchiq_2835_state
2024-06-10 21:02 [PATCH V2 00/10] staging: vc04_services: Random cleanups Stefan Wahren
` (3 preceding siblings ...)
2024-06-10 21:02 ` [PATCH V2 04/10] staging: vchiq_arm: Drop unnecessary declarations Stefan Wahren
@ 2024-06-10 21:02 ` Stefan Wahren
2024-06-10 21:02 ` [PATCH V2 06/10] staging: vchiq_arm: Drop vchiq_arm_init_state Stefan Wahren
` (4 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Stefan Wahren @ 2024-06-10 21:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Florian Fainelli
Cc: Umang Jain, Laurent Pinchart, linux-staging, linux-arm-kernel,
Stefan Wahren
The whole benefit of this encapsulating struct is questionable.
It just stores a flag to signalize the init state of vchiq_arm_state.
Beside the fact this flag is set too soon, the access to uninitialized
members should be avoided. So initialize vchiq_arm_state properly before
assign it directly to vchiq_state.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
.../interface/vchiq_arm/vchiq_arm.c | 25 +++++--------------
1 file changed, 6 insertions(+), 19 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 094ba1aea8f6..6ef4cf702c78 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -109,11 +109,6 @@ struct vchiq_arm_state {
int first_connect;
};
-struct vchiq_2835_state {
- int inited;
- struct vchiq_arm_state arm_state;
-};
-
struct vchiq_pagelist_info {
struct pagelist *pagelist;
size_t pagelist_buffer_size;
@@ -613,29 +608,21 @@ vchiq_arm_init_state(struct vchiq_state *state,
int
vchiq_platform_init_state(struct vchiq_state *state)
{
- struct vchiq_2835_state *platform_state;
+ struct vchiq_arm_state *platform_state;
- state->platform_state = kzalloc(sizeof(*platform_state), GFP_KERNEL);
- if (!state->platform_state)
+ platform_state = kzalloc(sizeof(*platform_state), GFP_KERNEL);
+ if (!platform_state)
return -ENOMEM;
- platform_state = (struct vchiq_2835_state *)state->platform_state;
-
- platform_state->inited = 1;
- vchiq_arm_init_state(state, &platform_state->arm_state);
+ vchiq_arm_init_state(state, platform_state);
+ state->platform_state = (struct opaque_platform_state *)platform_state;
return 0;
}
static struct vchiq_arm_state *vchiq_platform_get_arm_state(struct vchiq_state *state)
{
- struct vchiq_2835_state *platform_state;
-
- platform_state = (struct vchiq_2835_state *)state->platform_state;
-
- WARN_ON_ONCE(!platform_state->inited);
-
- return &platform_state->arm_state;
+ return (struct vchiq_arm_state *)state->platform_state;
}
void
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH V2 06/10] staging: vchiq_arm: Drop vchiq_arm_init_state
2024-06-10 21:02 [PATCH V2 00/10] staging: vc04_services: Random cleanups Stefan Wahren
` (4 preceding siblings ...)
2024-06-10 21:02 ` [PATCH V2 05/10] staging: vchiq_arm: Get the rid off struct vchiq_2835_state Stefan Wahren
@ 2024-06-10 21:02 ` Stefan Wahren
2024-06-10 21:02 ` [PATCH V2 07/10] staging: vchiq_arm: Reduce indentation of service_callback Stefan Wahren
` (3 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Stefan Wahren @ 2024-06-10 21:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Florian Fainelli
Cc: Umang Jain, Laurent Pinchart, linux-staging, linux-arm-kernel,
Stefan Wahren
After removal of struct vchiq_2835_state, the init of vchiq_arm_state
can be simplified by doing it directly within vchiq_platform_init_state.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../interface/vchiq_arm/vchiq_arm.c | 27 +++++++------------
1 file changed, 9 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 6ef4cf702c78..0ba52c9d8bc3 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -588,23 +588,6 @@ static int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state
return 0;
}
-static void
-vchiq_arm_init_state(struct vchiq_state *state,
- struct vchiq_arm_state *arm_state)
-{
- if (arm_state) {
- rwlock_init(&arm_state->susp_res_lock);
-
- init_completion(&arm_state->ka_evt);
- atomic_set(&arm_state->ka_use_count, 0);
- atomic_set(&arm_state->ka_use_ack_count, 0);
- atomic_set(&arm_state->ka_release_count, 0);
-
- arm_state->state = state;
- arm_state->first_connect = 0;
- }
-}
-
int
vchiq_platform_init_state(struct vchiq_state *state)
{
@@ -614,7 +597,15 @@ vchiq_platform_init_state(struct vchiq_state *state)
if (!platform_state)
return -ENOMEM;
- vchiq_arm_init_state(state, platform_state);
+ rwlock_init(&platform_state->susp_res_lock);
+
+ init_completion(&platform_state->ka_evt);
+ atomic_set(&platform_state->ka_use_count, 0);
+ atomic_set(&platform_state->ka_use_ack_count, 0);
+ atomic_set(&platform_state->ka_release_count, 0);
+
+ platform_state->state = state;
+
state->platform_state = (struct opaque_platform_state *)platform_state;
return 0;
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH V2 07/10] staging: vchiq_arm: Reduce indentation of service_callback
2024-06-10 21:02 [PATCH V2 00/10] staging: vc04_services: Random cleanups Stefan Wahren
` (5 preceding siblings ...)
2024-06-10 21:02 ` [PATCH V2 06/10] staging: vchiq_arm: Drop vchiq_arm_init_state Stefan Wahren
@ 2024-06-10 21:02 ` Stefan Wahren
2024-06-11 6:01 ` Dan Carpenter
2024-06-11 6:04 ` Dan Carpenter
2024-06-10 21:02 ` [PATCH V2 08/10] staging: vchiq_core: Add hex prefix to debugfs output Stefan Wahren
` (2 subsequent siblings)
9 siblings, 2 replies; 20+ messages in thread
From: Stefan Wahren @ 2024-06-10 21:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Florian Fainelli
Cc: Umang Jain, Laurent Pinchart, linux-staging, linux-arm-kernel,
Stefan Wahren
The service_callback has 5 levels of indentation, which makes it
hard to read. Reduce this by splitting the code in a new function
service_single_message() as suggested by Laurent Pinchart.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
.../interface/vchiq_arm/vchiq_arm.c | 68 +++++++++++--------
1 file changed, 39 insertions(+), 29 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 0ba52c9d8bc3..706bfc7a0b90 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1055,6 +1055,39 @@ add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
return 0;
}
+static int
+service_single_message(struct vchiq_instance *instance,
+ enum vchiq_reason reason,
+ struct vchiq_service *service, void *bulk_userdata)
+{
+ struct user_service *user_service;
+
+ user_service = (struct user_service *)service->base.userdata;
+
+ dev_dbg(service->state->dev, "arm: msg queue full\n");
+ /*
+ * If there is no MESSAGE_AVAILABLE in the completion
+ * queue, add one
+ */
+ if ((user_service->message_available_pos -
+ instance->completion_remove) < 0) {
+ dev_dbg(instance->state->dev,
+ "arm: Inserting extra MESSAGE_AVAILABLE\n");
+ return add_completion(instance, reason, NULL, user_service,
+ bulk_userdata);
+ }
+
+ if (wait_for_completion_interruptible(&user_service->remove_event)) {
+ dev_dbg(instance->state->dev, "arm: interrupted\n");
+ return -EAGAIN;
+ } else if (instance->closing) {
+ dev_dbg(instance->state->dev, "arm: closing\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
int
service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
struct vchiq_header *header, unsigned int handle, void *bulk_userdata)
@@ -1104,41 +1137,18 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
spin_lock(&service->state->msg_queue_spinlock);
while (user_service->msg_insert ==
(user_service->msg_remove + MSG_QUEUE_SIZE)) {
+ int ret;
+
spin_unlock(&service->state->msg_queue_spinlock);
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
DEBUG_COUNT(MSG_QUEUE_FULL_COUNT);
- dev_dbg(service->state->dev, "arm: msg queue full\n");
- /*
- * If there is no MESSAGE_AVAILABLE in the completion
- * queue, add one
- */
- if ((user_service->message_available_pos -
- instance->completion_remove) < 0) {
- int ret;
-
- dev_dbg(instance->state->dev,
- "arm: Inserting extra MESSAGE_AVAILABLE\n");
- DEBUG_TRACE(SERVICE_CALLBACK_LINE);
- ret = add_completion(instance, reason, NULL, user_service,
- bulk_userdata);
- if (ret) {
- DEBUG_TRACE(SERVICE_CALLBACK_LINE);
- vchiq_service_put(service);
- return ret;
- }
- }
- DEBUG_TRACE(SERVICE_CALLBACK_LINE);
- if (wait_for_completion_interruptible(&user_service->remove_event)) {
- dev_dbg(instance->state->dev, "arm: interrupted\n");
- DEBUG_TRACE(SERVICE_CALLBACK_LINE);
- vchiq_service_put(service);
- return -EAGAIN;
- } else if (instance->closing) {
- dev_dbg(instance->state->dev, "arm: closing\n");
+ ret = service_single_message(instance, reason,
+ service, bulk_userdata);
+ if (ret) {
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
vchiq_service_put(service);
- return -EINVAL;
+ return ret;
}
DEBUG_TRACE(SERVICE_CALLBACK_LINE);
spin_lock(&service->state->msg_queue_spinlock);
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V2 07/10] staging: vchiq_arm: Reduce indentation of service_callback
2024-06-10 21:02 ` [PATCH V2 07/10] staging: vchiq_arm: Reduce indentation of service_callback Stefan Wahren
@ 2024-06-11 6:01 ` Dan Carpenter
2024-06-11 9:58 ` Stefan Wahren
2024-06-11 6:04 ` Dan Carpenter
1 sibling, 1 reply; 20+ messages in thread
From: Dan Carpenter @ 2024-06-11 6:01 UTC (permalink / raw)
To: Stefan Wahren
Cc: Greg Kroah-Hartman, Florian Fainelli, Umang Jain,
Laurent Pinchart, linux-staging, linux-arm-kernel
On Mon, Jun 10, 2024 at 11:02:17PM +0200, Stefan Wahren wrote:
> The service_callback has 5 levels of indentation, which makes it
> hard to read. Reduce this by splitting the code in a new function
> service_single_message() as suggested by Laurent Pinchart.
>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> ---
> .../interface/vchiq_arm/vchiq_arm.c | 68 +++++++++++--------
> 1 file changed, 39 insertions(+), 29 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 0ba52c9d8bc3..706bfc7a0b90 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -1055,6 +1055,39 @@ add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
> return 0;
> }
>
> +static int
> +service_single_message(struct vchiq_instance *instance,
> + enum vchiq_reason reason,
> + struct vchiq_service *service, void *bulk_userdata)
> +{
> + struct user_service *user_service;
> +
> + user_service = (struct user_service *)service->base.userdata;
> +
> + dev_dbg(service->state->dev, "arm: msg queue full\n");
> + /*
> + * If there is no MESSAGE_AVAILABLE in the completion
> + * queue, add one
> + */
> + if ((user_service->message_available_pos -
> + instance->completion_remove) < 0) {
> + dev_dbg(instance->state->dev,
> + "arm: Inserting extra MESSAGE_AVAILABLE\n");
> + return add_completion(instance, reason, NULL, user_service,
> + bulk_userdata);
In the original code we added a completion and
> + }
> +
> + if (wait_for_completion_interruptible(&user_service->remove_event)) {
then waited for it here... Why did this change?
regards,
dan carpenter
> + dev_dbg(instance->state->dev, "arm: interrupted\n");
> + return -EAGAIN;
> + } else if (instance->closing) {
> + dev_dbg(instance->state->dev, "arm: closing\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> int
> service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
> struct vchiq_header *header, unsigned int handle, void *bulk_userdata)
> @@ -1104,41 +1137,18 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
> spin_lock(&service->state->msg_queue_spinlock);
> while (user_service->msg_insert ==
> (user_service->msg_remove + MSG_QUEUE_SIZE)) {
> + int ret;
> +
> spin_unlock(&service->state->msg_queue_spinlock);
> DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> DEBUG_COUNT(MSG_QUEUE_FULL_COUNT);
> - dev_dbg(service->state->dev, "arm: msg queue full\n");
> - /*
> - * If there is no MESSAGE_AVAILABLE in the completion
> - * queue, add one
> - */
> - if ((user_service->message_available_pos -
> - instance->completion_remove) < 0) {
> - int ret;
> -
> - dev_dbg(instance->state->dev,
> - "arm: Inserting extra MESSAGE_AVAILABLE\n");
> - DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> - ret = add_completion(instance, reason, NULL, user_service,
> - bulk_userdata);
> - if (ret) {
> - DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> - vchiq_service_put(service);
> - return ret;
> - }
> - }
>
> - DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> - if (wait_for_completion_interruptible(&user_service->remove_event)) {
> - dev_dbg(instance->state->dev, "arm: interrupted\n");
> - DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> - vchiq_service_put(service);
> - return -EAGAIN;
> - } else if (instance->closing) {
> - dev_dbg(instance->state->dev, "arm: closing\n");
> + ret = service_single_message(instance, reason,
> + service, bulk_userdata);
> + if (ret) {
> DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> vchiq_service_put(service);
> - return -EINVAL;
> + return ret;
> }
> DEBUG_TRACE(SERVICE_CALLBACK_LINE);
> spin_lock(&service->state->msg_queue_spinlock);
> --
> 2.34.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V2 07/10] staging: vchiq_arm: Reduce indentation of service_callback
2024-06-11 6:01 ` Dan Carpenter
@ 2024-06-11 9:58 ` Stefan Wahren
0 siblings, 0 replies; 20+ messages in thread
From: Stefan Wahren @ 2024-06-11 9:58 UTC (permalink / raw)
To: Dan Carpenter
Cc: Greg Kroah-Hartman, Florian Fainelli, Umang Jain,
Laurent Pinchart, linux-staging, linux-arm-kernel
Hi Dan,
Am 11.06.24 um 08:01 schrieb Dan Carpenter:
> On Mon, Jun 10, 2024 at 11:02:17PM +0200, Stefan Wahren wrote:
>> The service_callback has 5 levels of indentation, which makes it
>> hard to read. Reduce this by splitting the code in a new function
>> service_single_message() as suggested by Laurent Pinchart.
>>
>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
>> ---
>> .../interface/vchiq_arm/vchiq_arm.c | 68 +++++++++++--------
>> 1 file changed, 39 insertions(+), 29 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 0ba52c9d8bc3..706bfc7a0b90 100644
>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
>> @@ -1055,6 +1055,39 @@ add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
>> return 0;
>> }
>>
>> +static int
>> +service_single_message(struct vchiq_instance *instance,
>> + enum vchiq_reason reason,
>> + struct vchiq_service *service, void *bulk_userdata)
>> +{
>> + struct user_service *user_service;
>> +
>> + user_service = (struct user_service *)service->base.userdata;
>> +
>> + dev_dbg(service->state->dev, "arm: msg queue full\n");
>> + /*
>> + * If there is no MESSAGE_AVAILABLE in the completion
>> + * queue, add one
>> + */
>> + if ((user_service->message_available_pos -
>> + instance->completion_remove) < 0) {
>> + dev_dbg(instance->state->dev,
>> + "arm: Inserting extra MESSAGE_AVAILABLE\n");
>> + return add_completion(instance, reason, NULL, user_service,
>> + bulk_userdata);
> In the original code we added a completion and
>
>> + }
>> +
>> + if (wait_for_completion_interruptible(&user_service->remove_event)) {
> then waited for it here... Why did this change?
Oops, this wasn't intended. Thanks for catching this.
>
> regards,
> dan carpenter
>
>> + dev_dbg(instance->state->dev, "arm: interrupted\n");
>> + return -EAGAIN;
>> + } else if (instance->closing) {
>> + dev_dbg(instance->state->dev, "arm: closing\n");
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> int
>> service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
>> struct vchiq_header *header, unsigned int handle, void *bulk_userdata)
>> @@ -1104,41 +1137,18 @@ service_callback(struct vchiq_instance *instance, enum vchiq_reason reason,
>> spin_lock(&service->state->msg_queue_spinlock);
>> while (user_service->msg_insert ==
>> (user_service->msg_remove + MSG_QUEUE_SIZE)) {
>> + int ret;
>> +
>> spin_unlock(&service->state->msg_queue_spinlock);
>> DEBUG_TRACE(SERVICE_CALLBACK_LINE);
>> DEBUG_COUNT(MSG_QUEUE_FULL_COUNT);
>> - dev_dbg(service->state->dev, "arm: msg queue full\n");
>> - /*
>> - * If there is no MESSAGE_AVAILABLE in the completion
>> - * queue, add one
>> - */
>> - if ((user_service->message_available_pos -
>> - instance->completion_remove) < 0) {
>> - int ret;
>> -
>> - dev_dbg(instance->state->dev,
>> - "arm: Inserting extra MESSAGE_AVAILABLE\n");
>> - DEBUG_TRACE(SERVICE_CALLBACK_LINE);
>> - ret = add_completion(instance, reason, NULL, user_service,
>> - bulk_userdata);
>> - if (ret) {
>> - DEBUG_TRACE(SERVICE_CALLBACK_LINE);
>> - vchiq_service_put(service);
>> - return ret;
>> - }
>> - }
>>
>> - DEBUG_TRACE(SERVICE_CALLBACK_LINE);
>> - if (wait_for_completion_interruptible(&user_service->remove_event)) {
>> - dev_dbg(instance->state->dev, "arm: interrupted\n");
>> - DEBUG_TRACE(SERVICE_CALLBACK_LINE);
>> - vchiq_service_put(service);
>> - return -EAGAIN;
>> - } else if (instance->closing) {
>> - dev_dbg(instance->state->dev, "arm: closing\n");
>> + ret = service_single_message(instance, reason,
>> + service, bulk_userdata);
>> + if (ret) {
>> DEBUG_TRACE(SERVICE_CALLBACK_LINE);
>> vchiq_service_put(service);
>> - return -EINVAL;
>> + return ret;
>> }
>> DEBUG_TRACE(SERVICE_CALLBACK_LINE);
>> spin_lock(&service->state->msg_queue_spinlock);
>> --
>> 2.34.1
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V2 07/10] staging: vchiq_arm: Reduce indentation of service_callback
2024-06-10 21:02 ` [PATCH V2 07/10] staging: vchiq_arm: Reduce indentation of service_callback Stefan Wahren
2024-06-11 6:01 ` Dan Carpenter
@ 2024-06-11 6:04 ` Dan Carpenter
2024-06-11 21:23 ` Laurent Pinchart
1 sibling, 1 reply; 20+ messages in thread
From: Dan Carpenter @ 2024-06-11 6:04 UTC (permalink / raw)
To: Stefan Wahren, Laurent Pinchart
Cc: Greg Kroah-Hartman, Florian Fainelli, Umang Jain, linux-staging,
linux-arm-kernel
On Mon, Jun 10, 2024 at 11:02:17PM +0200, Stefan Wahren wrote:
> The service_callback has 5 levels of indentation, which makes it
> hard to read. Reduce this by splitting the code in a new function
> service_single_message() as suggested by Laurent Pinchart.
>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
To be honest, I liked the version of this patch which Laurent didn't
like. Even after pulling this code into a separate function, I'd still
support flipping the conditions around and adding the goto...
Laurent, you really didn't like the goto?
regards,
dan carpenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V2 07/10] staging: vchiq_arm: Reduce indentation of service_callback
2024-06-11 6:04 ` Dan Carpenter
@ 2024-06-11 21:23 ` Laurent Pinchart
2024-06-12 5:16 ` Stefan Wahren
2024-06-12 5:51 ` Dan Carpenter
0 siblings, 2 replies; 20+ messages in thread
From: Laurent Pinchart @ 2024-06-11 21:23 UTC (permalink / raw)
To: Dan Carpenter
Cc: Stefan Wahren, Greg Kroah-Hartman, Florian Fainelli, Umang Jain,
linux-staging, linux-arm-kernel
On Tue, Jun 11, 2024 at 09:04:28AM +0300, Dan Carpenter wrote:
> On Mon, Jun 10, 2024 at 11:02:17PM +0200, Stefan Wahren wrote:
> > The service_callback has 5 levels of indentation, which makes it
> > hard to read. Reduce this by splitting the code in a new function
> > service_single_message() as suggested by Laurent Pinchart.
> >
> > Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
>
> To be honest, I liked the version of this patch which Laurent didn't
> like. Even after pulling this code into a separate function, I'd still
> support flipping the conditions around and adding the goto...
>
> Laurent, you really didn't like the goto?
I won't nack it, but I really think gotos seriously hinder readability,
with the exception of error paths.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V2 07/10] staging: vchiq_arm: Reduce indentation of service_callback
2024-06-11 21:23 ` Laurent Pinchart
@ 2024-06-12 5:16 ` Stefan Wahren
2024-06-18 1:14 ` Laurent Pinchart
2024-06-12 5:51 ` Dan Carpenter
1 sibling, 1 reply; 20+ messages in thread
From: Stefan Wahren @ 2024-06-12 5:16 UTC (permalink / raw)
To: Laurent Pinchart, Dan Carpenter
Cc: Greg Kroah-Hartman, Florian Fainelli, Umang Jain, linux-staging,
linux-arm-kernel
Hi,
Am 11.06.24 um 23:23 schrieb Laurent Pinchart:
> On Tue, Jun 11, 2024 at 09:04:28AM +0300, Dan Carpenter wrote:
>> On Mon, Jun 10, 2024 at 11:02:17PM +0200, Stefan Wahren wrote:
>>> The service_callback has 5 levels of indentation, which makes it
>>> hard to read. Reduce this by splitting the code in a new function
>>> service_single_message() as suggested by Laurent Pinchart.
>>>
>>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
>> To be honest, I liked the version of this patch which Laurent didn't
>> like. Even after pulling this code into a separate function, I'd still
>> support flipping the conditions around and adding the goto...
>>
>> Laurent, you really didn't like the goto?
> I won't nack it, but I really think gotos seriously hinder readability,
> with the exception of error paths.
>
so at least you both are fine with the split approach here (assuming the
Dan's comment will be fixed in V3)?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V2 07/10] staging: vchiq_arm: Reduce indentation of service_callback
2024-06-12 5:16 ` Stefan Wahren
@ 2024-06-18 1:14 ` Laurent Pinchart
0 siblings, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2024-06-18 1:14 UTC (permalink / raw)
To: Stefan Wahren
Cc: Dan Carpenter, Greg Kroah-Hartman, Florian Fainelli, Umang Jain,
linux-staging, linux-arm-kernel
On Wed, Jun 12, 2024 at 07:16:16AM +0200, Stefan Wahren wrote:
> Hi,
>
> Am 11.06.24 um 23:23 schrieb Laurent Pinchart:
> > On Tue, Jun 11, 2024 at 09:04:28AM +0300, Dan Carpenter wrote:
> >> On Mon, Jun 10, 2024 at 11:02:17PM +0200, Stefan Wahren wrote:
> >>> The service_callback has 5 levels of indentation, which makes it
> >>> hard to read. Reduce this by splitting the code in a new function
> >>> service_single_message() as suggested by Laurent Pinchart.
> >>>
> >>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> >> To be honest, I liked the version of this patch which Laurent didn't
> >> like. Even after pulling this code into a separate function, I'd still
> >> support flipping the conditions around and adding the goto...
> >>
> >> Laurent, you really didn't like the goto?
> > I won't nack it, but I really think gotos seriously hinder readability,
> > with the exception of error paths.
> >
> so at least you both are fine with the split approach here (assuming the
> Dan's comment will be fixed in V3)?
Fine with me.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V2 07/10] staging: vchiq_arm: Reduce indentation of service_callback
2024-06-11 21:23 ` Laurent Pinchart
2024-06-12 5:16 ` Stefan Wahren
@ 2024-06-12 5:51 ` Dan Carpenter
1 sibling, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2024-06-12 5:51 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Stefan Wahren, Greg Kroah-Hartman, Florian Fainelli, Umang Jain,
linux-staging, linux-arm-kernel
On Wed, Jun 12, 2024 at 12:23:47AM +0300, Laurent Pinchart wrote:
> On Tue, Jun 11, 2024 at 09:04:28AM +0300, Dan Carpenter wrote:
> > On Mon, Jun 10, 2024 at 11:02:17PM +0200, Stefan Wahren wrote:
> > > The service_callback has 5 levels of indentation, which makes it
> > > hard to read. Reduce this by splitting the code in a new function
> > > service_single_message() as suggested by Laurent Pinchart.
> > >
> > > Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> >
> > To be honest, I liked the version of this patch which Laurent didn't
> > like. Even after pulling this code into a separate function, I'd still
> > support flipping the conditions around and adding the goto...
> >
> > Laurent, you really didn't like the goto?
>
> I won't nack it, but I really think gotos seriously hinder readability,
> with the exception of error paths.
Hm... I guess I only hate backwards gotos (ones that go up instead of
down). To me this one is kind of like a goto unlock. It's slightly
more complicated than just an unlock because we add a completion as
well... I don't have strong feelings about it, I was just interested.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V2 08/10] staging: vchiq_core: Add hex prefix to debugfs output
2024-06-10 21:02 [PATCH V2 00/10] staging: vc04_services: Random cleanups Stefan Wahren
` (6 preceding siblings ...)
2024-06-10 21:02 ` [PATCH V2 07/10] staging: vchiq_arm: Reduce indentation of service_callback Stefan Wahren
@ 2024-06-10 21:02 ` Stefan Wahren
2024-06-10 21:02 ` [PATCH V2 09/10] staging: vchiq_arm: Don't cast scatter-gather elements Stefan Wahren
2024-06-10 21:02 ` [PATCH V2 10/10] staging: vc04_services: Update testing instructions Stefan Wahren
9 siblings, 0 replies; 20+ messages in thread
From: Stefan Wahren @ 2024-06-10 21:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Florian Fainelli
Cc: Umang Jain, Laurent Pinchart, linux-staging, linux-arm-kernel,
Stefan Wahren
The number format of VCHIQ debugfs isn't always clear. So let's
add a prefix for all hex values, in order to make things clear.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../vc04_services/interface/vchiq_arm/vchiq_core.c | 8 ++++----
1 file changed, 4 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 51cfc366519b..4f65e4021c4d 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -3370,7 +3370,7 @@ vchiq_dump_shared_state(struct seq_file *f, struct vchiq_state *state,
};
int i;
- seq_printf(f, " %s: slots %d-%d tx_pos=%x recycle=%x\n",
+ seq_printf(f, " %s: slots %d-%d tx_pos=0x%x recycle=0x%x\n",
label, shared->slot_first, shared->slot_last,
shared->tx_pos, shared->slot_queue_recycle);
@@ -3386,7 +3386,7 @@ vchiq_dump_shared_state(struct seq_file *f, struct vchiq_state *state,
}
for (i = 1; i < shared->debug[DEBUG_ENTRIES]; i++) {
- seq_printf(f, " DEBUG: %s = %d(%x)\n",
+ seq_printf(f, " DEBUG: %s = %d(0x%x)\n",
debug_names[i], shared->debug[i], shared->debug[i]);
}
}
@@ -3414,7 +3414,7 @@ vchiq_dump_service_state(struct seq_file *f, struct vchiq_service *service)
if (service->public_fourcc != VCHIQ_FOURCC_INVALID)
scnprintf(remoteport + len2, sizeof(remoteport) - len2,
- " (client %x)", service->client_id);
+ " (client 0x%x)", service->client_id);
} else {
strscpy(remoteport, "n/a", sizeof(remoteport));
}
@@ -3475,7 +3475,7 @@ void vchiq_dump_state(struct seq_file *f, struct vchiq_state *state)
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",
+ seq_printf(f, " tx_pos=0x%x(@%pK), rx_pos=0x%x(@%pK)\n",
state->local->tx_pos,
state->tx_data + (state->local_tx_pos & VCHIQ_SLOT_MASK),
state->rx_pos,
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH V2 09/10] staging: vchiq_arm: Don't cast scatter-gather elements
2024-06-10 21:02 [PATCH V2 00/10] staging: vc04_services: Random cleanups Stefan Wahren
` (7 preceding siblings ...)
2024-06-10 21:02 ` [PATCH V2 08/10] staging: vchiq_core: Add hex prefix to debugfs output Stefan Wahren
@ 2024-06-10 21:02 ` Stefan Wahren
2024-06-11 21:25 ` Laurent Pinchart
2024-06-10 21:02 ` [PATCH V2 10/10] staging: vc04_services: Update testing instructions Stefan Wahren
9 siblings, 1 reply; 20+ messages in thread
From: Stefan Wahren @ 2024-06-10 21:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Florian Fainelli
Cc: Umang Jain, Laurent Pinchart, linux-staging, linux-arm-kernel,
Stefan Wahren
The kernel uses different types for DMA length & address,
so better use them.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
.../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 6 +++---
1 file changed, 3 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 706bfc7a0b90..02392073c4aa 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -162,7 +162,7 @@ cleanup_pagelistinfo(struct vchiq_instance *instance, struct vchiq_pagelist_info
}
static inline bool
-is_adjacent_block(u32 *addrs, u32 addr, unsigned int k)
+is_adjacent_block(u32 *addrs, dma_addr_t addr, unsigned int k)
{
u32 tmp;
@@ -377,8 +377,8 @@ create_pagelist(struct vchiq_instance *instance, char *buf, char __user *ubuf,
/* Combine adjacent blocks for performance */
k = 0;
for_each_sg(scatterlist, sg, dma_buffers, i) {
- u32 len = sg_dma_len(sg);
- u32 addr = sg_dma_address(sg);
+ unsigned int len = sg_dma_len(sg);
+ dma_addr_t addr = sg_dma_address(sg);
/* Note: addrs is the address + page_count - 1
* The firmware expects blocks after the first to be page-
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V2 09/10] staging: vchiq_arm: Don't cast scatter-gather elements
2024-06-10 21:02 ` [PATCH V2 09/10] staging: vchiq_arm: Don't cast scatter-gather elements Stefan Wahren
@ 2024-06-11 21:25 ` Laurent Pinchart
0 siblings, 0 replies; 20+ messages in thread
From: Laurent Pinchart @ 2024-06-11 21:25 UTC (permalink / raw)
To: Stefan Wahren
Cc: Greg Kroah-Hartman, Florian Fainelli, Umang Jain, linux-staging,
linux-arm-kernel
Hi Stefan,
Thank you for the patch.
On Mon, Jun 10, 2024 at 11:02:19PM +0200, Stefan Wahren wrote:
> The kernel uses different types for DMA length & address,
> so better use them.
>
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> .../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 6 +++---
> 1 file changed, 3 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 706bfc7a0b90..02392073c4aa 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -162,7 +162,7 @@ cleanup_pagelistinfo(struct vchiq_instance *instance, struct vchiq_pagelist_info
> }
>
> static inline bool
> -is_adjacent_block(u32 *addrs, u32 addr, unsigned int k)
> +is_adjacent_block(u32 *addrs, dma_addr_t addr, unsigned int k)
> {
> u32 tmp;
>
> @@ -377,8 +377,8 @@ create_pagelist(struct vchiq_instance *instance, char *buf, char __user *ubuf,
> /* Combine adjacent blocks for performance */
> k = 0;
> for_each_sg(scatterlist, sg, dma_buffers, i) {
> - u32 len = sg_dma_len(sg);
> - u32 addr = sg_dma_address(sg);
> + unsigned int len = sg_dma_len(sg);
> + dma_addr_t addr = sg_dma_address(sg);
>
> /* Note: addrs is the address + page_count - 1
> * The firmware expects blocks after the first to be page-
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V2 10/10] staging: vc04_services: Update testing instructions
2024-06-10 21:02 [PATCH V2 00/10] staging: vc04_services: Random cleanups Stefan Wahren
` (8 preceding siblings ...)
2024-06-10 21:02 ` [PATCH V2 09/10] staging: vchiq_arm: Don't cast scatter-gather elements Stefan Wahren
@ 2024-06-10 21:02 ` Stefan Wahren
9 siblings, 0 replies; 20+ messages in thread
From: Stefan Wahren @ 2024-06-10 21:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Florian Fainelli
Cc: Umang Jain, Laurent Pinchart, linux-staging, linux-arm-kernel,
Stefan Wahren
Since the initial version of the testing instructions a few
things has changed. So consider the latest arm64/defconfig changes
and the new debugfs entry.
Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
.../staging/vc04_services/interface/TESTING | 45 ++++++++++++++++++-
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/vc04_services/interface/TESTING b/drivers/staging/vc04_services/interface/TESTING
index a6d63efcbcb9..273952dc9d85 100644
--- a/drivers/staging/vc04_services/interface/TESTING
+++ b/drivers/staging/vc04_services/interface/TESTING
@@ -31,7 +31,7 @@ Here are the most common kernel configurations:
3. BCM2837 target SoC (ARM 64 bit)
- Use the defconfig as a base and then enable all VCHIQ options.
+ Use the defconfig which has most of the VCHIQ options enabled.
* Scenarios
@@ -80,3 +80,46 @@ Here are the most common kernel configurations:
vchi ping (size 0, 100 async, 100 oneway) -> infus
vchi ping (size 0, 200 async, 0 oneway) -> infus
...
+
+ * Debugfs test
+
+ Command: cat /sys/kernel/debug/vchiq/state
+
+ Example output:
+ State 0: CONNECTED
+ tx_pos=0x1e8(@43b0acda), rx_pos=0x170(@05493af8)
+ Version: 8 (min 3)
+ Stats: ctrl_tx_count=7, ctrl_rx_count=7, error_count=0
+ Slots: 30 available (29 data), 0 recyclable, 0 stalls (0 data)
+ Platform: 2835 (VC master)
+ Local: slots 34-64 tx_pos=0x1e8 recycle=0x1f
+ Slots claimed:
+ DEBUG: SLOT_HANDLER_COUNT = 20(0x14)
+ DEBUG: SLOT_HANDLER_LINE = 1937(0x791)
+ DEBUG: PARSE_LINE = 1864(0x748)
+ DEBUG: PARSE_HEADER = -249155224(0xf1263168)
+ DEBUG: PARSE_MSGID = 67362817(0x403e001)
+ DEBUG: AWAIT_COMPLETION_LINE = 0(0x0)
+ DEBUG: DEQUEUE_MESSAGE_LINE = 0(0x0)
+ DEBUG: SERVICE_CALLBACK_LINE = 0(0x0)
+ DEBUG: MSG_QUEUE_FULL_COUNT = 0(0x0)
+ DEBUG: COMPLETION_QUEUE_FULL_COUNT = 0(0x0)
+ Remote: slots 2-32 tx_pos=0x170 recycle=0x1f
+ Slots claimed:
+ 2: 10/9
+ DEBUG: SLOT_HANDLER_COUNT = 20(0x14)
+ DEBUG: SLOT_HANDLER_LINE = 1851(0x73b)
+ DEBUG: PARSE_LINE = 1827(0x723)
+ DEBUG: PARSE_HEADER = -150330912(0xf70a21e0)
+ DEBUG: PARSE_MSGID = 67113022(0x400103e)
+ DEBUG: AWAIT_COMPLETION_LINE = 0(0x0)
+ DEBUG: DEQUEUE_MESSAGE_LINE = 0(0x0)
+ DEBUG: SERVICE_CALLBACK_LINE = 0(0x0)
+ DEBUG: MSG_QUEUE_FULL_COUNT = 0(0x0)
+ DEBUG: COMPLETION_QUEUE_FULL_COUNT = 0(0x0)
+ Service 0: LISTENING (ref 1) 'PEEK little-endian (0x4b454550)' remote n/a (msg use 0/3840, slot use 0/15)
+ Bulk: tx_pending=0 (size 0), rx_pending=0 (size 0)
+ Ctrl: tx_count=0, tx_bytes=0, rx_count=0, rx_bytes=0
+ Bulk: tx_count=0, tx_bytes=0, rx_count=0, rx_bytes=0
+ 0 quota stalls, 0 slot stalls, 0 bulk stalls, 0 aborted, 0 errors
+ instance b511f60b
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 20+ messages in thread