Hi Umang,
Thank you for the patch.
On Thu, Mar 28, 2024 at 11:41:27PM +0530, Umang Jain wrote:
> The vchiq_connected.[ch] just implements two function:
>
> - vchiq_add_connected_callback()
> - vchiq_call_connected_callbacks()
>
> for the deferred vchiq callbacks. Those can easily live in
> vchiq_arm.[ch], hence move them.
I would add
This allows making the vchiq_call_connected_callbacks() function static.
> The move doesn't copy over MAX_CALLBACKS because it is the
> same as VCHIQ_DRV_MAX_CALLBACKS. Hence, it now being used in
> vchiq_add_connected_callback().
You can reflow to 72 columns.
>
> No functional changes intended in this patch.
>
In case you didn't know, there's a Suggested-by tag you can use to
indicate who the idea for a patch came from.
> Signed-off-by: Umang Jain <umang.jain@xxxxxxxxxxxxxxxx>
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> drivers/staging/vc04_services/Makefile | 1 -
> .../interface/vchiq_arm/vchiq_arm.c | 51 +++++++++++++++-
> .../interface/vchiq_arm/vchiq_arm.h | 5 ++
> .../interface/vchiq_arm/vchiq_connected.c | 60 -------------------
> .../interface/vchiq_arm/vchiq_connected.h | 14 -----
> 5 files changed, 55 insertions(+), 76 deletions(-)
> delete mode 100644 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
> delete mode 100644 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.h
>
> diff --git a/drivers/staging/vc04_services/Makefile b/drivers/staging/vc04_services/Makefile
> index e8b897a7b9a6..dad3789522b8 100644
> --- a/drivers/staging/vc04_services/Makefile
> +++ b/drivers/staging/vc04_services/Makefile
> @@ -6,7 +6,6 @@ vchiq-objs := \
> interface/vchiq_arm/vchiq_arm.o \
> interface/vchiq_arm/vchiq_bus.o \
> interface/vchiq_arm/vchiq_debugfs.o \
> - interface/vchiq_arm/vchiq_connected.o \
>
> ifdef CONFIG_VCHIQ_CDEV
> vchiq-objs += interface/vchiq_arm/vchiq_dev.o
> 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 4d14ea1becaa..2005d392d0b7 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -36,7 +36,6 @@
> #include "vchiq_arm.h"
> #include "vchiq_bus.h"
> #include "vchiq_debugfs.h"
> -#include "vchiq_connected.h"
> #include "vchiq_pagelist.h"
>
> #define DEVICE_NAME "vchiq"
> @@ -189,6 +188,56 @@ is_adjacent_block(u32 *addrs, u32 addr, unsigned int k)
> return tmp == (addr & PAGE_MASK);
> }
>
> +/*
> + * This function is called by the vchiq stack once it has been connected to
> + * the videocore and clients can start to use the stack.
> + */
> +static void vchiq_call_connected_callbacks(struct vchiq_drv_mgmt *drv_mgmt)
> +{
> + int i;
> +
> + if (mutex_lock_killable(&drv_mgmt->connected_mutex))
> + return;
> +
> + for (i = 0; i < drv_mgmt->num_deferred_callbacks; i++)
> + drv_mgmt->deferred_callback[i]();
> +
> + drv_mgmt->num_deferred_callbacks = 0;
> + drv_mgmt->connected = true;
> + mutex_unlock(&drv_mgmt->connected_mutex);
> +}
> +
> +/*
> + * This function is used to defer initialization until the vchiq stack is
> + * initialized. If the stack is already initialized, then the callback will
> + * be made immediately, otherwise it will be deferred until
> + * vchiq_call_connected_callbacks is called.
> + */
> +void vchiq_add_connected_callback(struct vchiq_device *device, void (*callback)(void))
> +{
> + struct vchiq_drv_mgmt *drv_mgmt = device->drv_mgmt;
> +
> + if (mutex_lock_killable(&drv_mgmt->connected_mutex))
> + return;
> +
> + if (drv_mgmt->connected) {
> + /* We're already connected. Call the callback immediately. */
> + callback();
> + } else {
> + if (drv_mgmt->num_deferred_callbacks >= VCHIQ_DRV_MAX_CALLBACKS) {
> + dev_err(&device->dev,
> + "core: deferred callbacks(%d) exceeded the maximum limit(%d)\n",
> + drv_mgmt->num_deferred_callbacks, VCHIQ_DRV_MAX_CALLBACKS);
> + } else {
> + drv_mgmt->deferred_callback[drv_mgmt->num_deferred_callbacks] =
> + callback;
> + drv_mgmt->num_deferred_callbacks++;
> + }
> + }
> + mutex_unlock(&drv_mgmt->connected_mutex);
> +}
> +EXPORT_SYMBOL(vchiq_add_connected_callback);
> +
> /* There is a potential problem with partial cache lines (pages?)
> * at the ends of the block when reading. If the CPU accessed anything in
> * the same line (page?) then it may have pulled old data into the cache,
> 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 a774485935a2..4a5b5ae9625a 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.h
> @@ -23,6 +23,7 @@
> #define VCHIQ_DRV_MAX_CALLBACKS 10
>
> struct rpi_firmware;
> +struct vchiq_device;
>
> enum USE_TYPE_E {
> USE_TYPE_SERVICE,
> @@ -147,6 +148,10 @@ vchiq_instance_get_trace(struct vchiq_instance *instance);
> extern void
> vchiq_instance_set_trace(struct vchiq_instance *instance, int trace);
>
> +extern void
> +vchiq_add_connected_callback(struct vchiq_device *device,
> + void (*callback)(void));
> +
> #if IS_ENABLED(CONFIG_VCHIQ_CDEV)
>
> extern void
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
> deleted file mode 100644
> index 049b3f2d1bd4..000000000000
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
> +++ /dev/null
> @@ -1,60 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
> -/* Copyright (c) 2010-2012 Broadcom. All rights reserved. */
> -
> -#include "vchiq_arm.h"
> -#include "vchiq_connected.h"
> -#include "vchiq_core.h"
> -#include <linux/module.h>
> -#include <linux/mutex.h>
> -
> -#define MAX_CALLBACKS 10
> -
> -/*
> - * This function is used to defer initialization until the vchiq stack is
> - * initialized. If the stack is already initialized, then the callback will
> - * be made immediately, otherwise it will be deferred until
> - * vchiq_call_connected_callbacks is called.
> - */
> -void vchiq_add_connected_callback(struct vchiq_device *device, void (*callback)(void))
> -{
> - struct vchiq_drv_mgmt *drv_mgmt = device->drv_mgmt;
> -
> - if (mutex_lock_killable(&drv_mgmt->connected_mutex))
> - return;
> -
> - if (drv_mgmt->connected) {
> - /* We're already connected. Call the callback immediately. */
> - callback();
> - } else {
> - if (drv_mgmt->num_deferred_callbacks >= MAX_CALLBACKS) {
> - dev_err(&device->dev,
> - "core: There already %d callback registered - please increase MAX_CALLBACKS\n",
> - drv_mgmt->num_deferred_callbacks);
> - } else {
> - drv_mgmt->deferred_callback[drv_mgmt->num_deferred_callbacks] =
> - callback;
> - drv_mgmt->num_deferred_callbacks++;
> - }
> - }
> - mutex_unlock(&drv_mgmt->connected_mutex);
> -}
> -EXPORT_SYMBOL(vchiq_add_connected_callback);
> -
> -/*
> - * This function is called by the vchiq stack once it has been connected to
> - * the videocore and clients can start to use the stack.
> - */
> -void vchiq_call_connected_callbacks(struct vchiq_drv_mgmt *drv_mgmt)
> -{
> - int i;
> -
> - if (mutex_lock_killable(&drv_mgmt->connected_mutex))
> - return;
> -
> - for (i = 0; i < drv_mgmt->num_deferred_callbacks; i++)
> - drv_mgmt->deferred_callback[i]();
> -
> - drv_mgmt->num_deferred_callbacks = 0;
> - drv_mgmt->connected = true;
> - mutex_unlock(&drv_mgmt->connected_mutex);
> -}
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.h
> deleted file mode 100644
> index f57a878652b1..000000000000
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.h
> +++ /dev/null
> @@ -1,14 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */
> -/* Copyright (c) 2010-2012 Broadcom. All rights reserved. */
> -
> -#include "vchiq_bus.h"
> -
> -#ifndef VCHIQ_CONNECTED_H
> -#define VCHIQ_CONNECTED_H
> -
> -struct vchiq_drv_mgmt;
> -
> -void vchiq_add_connected_callback(struct vchiq_device *device, void (*callback)(void));
> -void vchiq_call_connected_callbacks(struct vchiq_drv_mgmt *mgmt);
> -
> -#endif /* VCHIQ_CONNECTED_H */
--
Regards,
Laurent Pinchart
