Hi Umang,
just 2 nits
Am 12.04.24 um 09:57 schrieb Umang Jain:
The patch intended to drop the g_state pointer.
g_state is supposed to be a central place to track the state
via vchiq_state. This is now moved to be contained in the
struct vchiq_drv_mgmt. As a result vchiq_get_state() is also removed.
In order to have access to vchiq_drv_mgmt, vchiq_initialise()
and vchiq_mmal_init() are modified to receive a struct device pointer
as one of their function parameter
The vchiq_state pointer is now passed directly to
vchiq_dump_platform_instances() to get access to the state instead
getting it via vchiq_get_state().
For the char device, struct miscdevice is retrieved by struct file's
private data in vchiq_open and struct vchiq_drv_mgmt is retrieved
thereafter.
Removal of global variable members is now addressed hence, drop
the corresponding item from the TODO list.
Signed-off-by: Umang Jain <umang.jain@xxxxxxxxxxxxxxxx>
---
.../bcm2835-audio/bcm2835-vchiq.c | 5 ++-
.../bcm2835-camera/bcm2835-camera.c | 4 +--
.../include/linux/raspberrypi/vchiq.h | 4 ++-
drivers/staging/vc04_services/interface/TODO | 8 -----
.../interface/vchiq_arm/vchiq_arm.c | 36 +++++--------------
.../interface/vchiq_arm/vchiq_arm.h | 7 ++--
.../interface/vchiq_arm/vchiq_core.c | 2 +-
.../interface/vchiq_arm/vchiq_core.h | 2 +-
.../interface/vchiq_arm/vchiq_debugfs.c | 5 ++-
.../interface/vchiq_arm/vchiq_dev.c | 10 +++---
.../vc04_services/vchiq-mmal/mmal-vchiq.c | 6 ++--
.../vc04_services/vchiq-mmal/mmal-vchiq.h | 3 +-
12 files changed, 37 insertions(+), 55 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index d74110ca17ab..133ed15f3dbc 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -7,6 +7,8 @@
#include "bcm2835.h"
#include "vc_vchi_audioserv_defs.h"
+#include "../interface/vchiq_arm/vchiq_arm.h"
+
struct bcm2835_audio_instance {
struct device *dev;
unsigned int service_handle;
@@ -175,10 +177,11 @@ static void vc_vchi_audio_deinit(struct bcm2835_audio_instance *instance)
int bcm2835_new_vchi_ctx(struct device *dev, struct bcm2835_vchi_ctx *vchi_ctx)
{
+ struct vchiq_drv_mgmt *mgmt = dev_get_drvdata(dev->parent);
int ret;
/* Initialize and create a VCHI connection */
- ret = vchiq_initialise(&vchi_ctx->instance);
+ ret = vchiq_initialise(&mgmt->state, &vchi_ctx->instance);
if (ret) {
dev_err(dev, "failed to initialise VCHI instance (ret=%d)\n",
ret);
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index c3ba490e53cb..b3599ec6293a 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -1555,7 +1555,7 @@ static int mmal_init(struct bcm2835_mmal_dev *dev)
u32 param_size;
struct vchiq_mmal_component *camera;
- ret = vchiq_mmal_init(&dev->instance);
+ ret = vchiq_mmal_init(dev->v4l2_dev.dev, &dev->instance);
if (ret < 0) {
v4l2_err(&dev->v4l2_dev, "%s: vchiq mmal init failed %d\n",
__func__, ret);
@@ -1854,7 +1854,7 @@ static int bcm2835_mmal_probe(struct vchiq_device *device)
return ret;
}
- ret = vchiq_mmal_init(&instance);
+ ret = vchiq_mmal_init(&device->dev, &instance);
if (ret < 0)
return ret;
diff --git a/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
index 52e106f117da..6c40d8c1dde6 100644
--- a/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
+++ b/drivers/staging/vc04_services/include/linux/raspberrypi/vchiq.h
@@ -48,6 +48,7 @@ struct vchiq_element {
};
struct vchiq_instance;
+struct vchiq_state;
struct vchiq_service_base {
int fourcc;
@@ -78,7 +79,8 @@ struct vchiq_service_params_kernel {
short version_min; /* Update for incompatible changes */
};
-extern int vchiq_initialise(struct vchiq_instance **pinstance);
+extern int vchiq_initialise(struct vchiq_state *state,
+ struct vchiq_instance **pinstance);
extern int vchiq_shutdown(struct vchiq_instance *instance);
extern int vchiq_connect(struct vchiq_instance *instance);
extern int vchiq_open_service(struct vchiq_instance *instance,
diff --git a/drivers/staging/vc04_services/interface/TODO b/drivers/staging/vc04_services/interface/TODO
index 05eb5140d096..15f12b8f213e 100644
--- a/drivers/staging/vc04_services/interface/TODO
+++ b/drivers/staging/vc04_services/interface/TODO
@@ -41,14 +41,6 @@ The code follows the 80 characters limitation yet tends to go 3 or 4 levels of
indentation deep making it very unpleasant to read. This is specially relevant
in the character driver ioctl code and in the core thread functions.
-* Get rid of all non essential global structures and create a proper per
-device structure
-
-The first thing one generally sees in a probe function is a memory allocation
-for all the device specific data. This structure is then passed all over the
-driver. This is good practice since it makes the driver work regardless of the
-number of devices probed.
-
* Clean up Sparse warnings from __user annotations. See
vchiq_irq_queue_bulk_tx_rx(). Ensure that the address of "&waiter->bulk_waiter"
is never disclosed to userspace.
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 4bdd383a060c..502ddc0f6e46 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -59,8 +59,6 @@
#define KEEPALIVE_VER 1
#define KEEPALIVE_VER_MIN KEEPALIVE_VER
-struct vchiq_state g_state;
-
/*
* The devices implemented in the VCHIQ firmware are not discoverable,
* so we need to maintain a list of them in order to register them with
@@ -698,9 +696,8 @@ void vchiq_dump_platform_state(struct seq_file *f)
}
#define VCHIQ_INIT_RETRIES 10
-int vchiq_initialise(struct vchiq_instance **instance_out)
+int vchiq_initialise(struct vchiq_state *state, struct vchiq_instance **instance_out)
{
- struct vchiq_state *state;
struct vchiq_instance *instance = NULL;
int i, ret;
@@ -710,7 +707,6 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
* block forever.
*/
for (i = 0; i < VCHIQ_INIT_RETRIES; i++) {
- state = vchiq_get_state();
if (state)
break;
usleep_range(500, 600);
@@ -1026,9 +1022,10 @@ add_completion(struct vchiq_instance *instance, enum vchiq_reason reason,
void *bulk_userdata)
{
struct vchiq_completion_data_kernel *completion;
+ struct vchiq_drv_mgmt *mgmt = dev_get_drvdata(instance->state->dev);
Please make it the first line of the declarations
int insert;
- DEBUG_INITIALISE(g_state.local);
+ DEBUG_INITIALISE(mgmt->state.local);
insert = instance->completion_insert;
while ((insert - instance->completion_remove) >= MAX_COMPLETIONS) {
...
diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
index 680961a7c61b..fca920d41e4f 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
@@ -26,6 +26,7 @@
#include <media/videobuf2-vmalloc.h>
#include "../include/linux/raspberrypi/vchiq.h"
+#include "../interface/vchiq_arm/vchiq_arm.h"
#include "mmal-common.h"
#include "mmal-vchiq.h"
#include "mmal-msg.h"
@@ -1852,7 +1853,7 @@ int vchiq_mmal_finalise(struct vchiq_mmal_instance *instance)
}
EXPORT_SYMBOL_GPL(vchiq_mmal_finalise);
-int vchiq_mmal_init(struct vchiq_mmal_instance **out_instance)
+int vchiq_mmal_init(struct device *dev, struct vchiq_mmal_instance **out_instance)
{
int status;
int err = -ENODEV;
@@ -1865,6 +1866,7 @@ int vchiq_mmal_init(struct vchiq_mmal_instance **out_instance)
.callback = mmal_service_callback,
.userdata = NULL,
};
+ struct vchiq_drv_mgmt *mgmt = dev_get_drvdata(dev->parent);
here too
