[PATCH 0/4] staging: vc04_services: avoid the use of typedef for function pointers

[PATCH 0/4] staging: vc04_services: avoid the use of typedef for function pointers

[Gaston Gonzalez]

This patch set removes some typedefs for function pointers in vc04_services.

After patches 01 to 03, there are no remaining typedef under vc04_services.
Hence, the patch 04/04 updates the TODO file removing the 'remove typedefs'
task.

Gaston Gonzalez (4):
  staging: bcm2835-audio: replace function typedefs with equivalent
    declaration
  staging: vc04_services: replace function typedef with equivalent
    declaration
  staging: vc04_services: avoid the use of typedef for function pointers
  staging: vc04_services: update TODO file

 .../vc04_services/bcm2835-audio/bcm2835.c     | 28 +++----
 .../vc04_services/bcm2835-camera/controls.c   | 76 +++++++++----------
 drivers/staging/vc04_services/interface/TODO  |  8 +-
 .../vc04_services/vchiq-mmal/mmal-vchiq.c     | 24 +++---
 .../vc04_services/vchiq-mmal/mmal-vchiq.h     | 13 ++--
 5 files changed, 66 insertions(+), 83 deletions(-)

-- 
2.34.1

[PATCH 1/4] staging: vc04_services: avoid the use of typedef for function pointers

[Gaston Gonzalez]

Replace typedefs bcm2835_audio_newpcm_func and bcm2835_audio_newctl_func
with equivalent declarations to better align with the linux kernel
coding style.

As the '_func' in the function names is somehow reduntant, it was
dropped in favour of the shorter names: 'bcm2835_audio_newpcm' and
'bcm2835_audio_newctl'

Signed-off-by: Gaston Gonzalez <gascoar@gmail.com>
---
 .../vc04_services/bcm2835-audio/bcm2835.c     | 28 ++++++++-----------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
index c250fbef2fa3..412342d5b6c9 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
@@ -52,20 +52,14 @@ static int bcm2835_devm_add_vchi_ctx(struct device *dev)
 	return 0;
 }
 
-typedef int (*bcm2835_audio_newpcm_func)(struct bcm2835_chip *chip,
-					 const char *name,
-					 enum snd_bcm2835_route route,
-					 u32 numchannels);
-
-typedef int (*bcm2835_audio_newctl_func)(struct bcm2835_chip *chip);
-
 struct bcm2835_audio_driver {
 	struct device_driver driver;
 	const char *shortname;
 	const char *longname;
 	int minchannels;
-	bcm2835_audio_newpcm_func newpcm;
-	bcm2835_audio_newctl_func newctl;
+	int (*bcm2835_audio_newpcm)(struct bcm2835_chip *chip, const char *name,
+				    enum snd_bcm2835_route route, u32 numchannels);
+	int (*bcm2835_audio_newctl)(struct bcm2835_chip *chip);
 	enum snd_bcm2835_route route;
 };
 
@@ -104,8 +98,8 @@ static struct bcm2835_audio_driver bcm2835_audio_alsa = {
 	.shortname = "bcm2835 ALSA",
 	.longname  = "bcm2835 ALSA",
 	.minchannels = 2,
-	.newpcm = bcm2835_audio_alsa_newpcm,
-	.newctl = snd_bcm2835_new_ctl,
+	.bcm2835_audio_newpcm = bcm2835_audio_alsa_newpcm,
+	.bcm2835_audio_newctl = snd_bcm2835_new_ctl,
 };
 
 static struct bcm2835_audio_driver bcm2835_audio_hdmi = {
@@ -116,8 +110,8 @@ static struct bcm2835_audio_driver bcm2835_audio_hdmi = {
 	.shortname = "bcm2835 HDMI",
 	.longname  = "bcm2835 HDMI",
 	.minchannels = 1,
-	.newpcm = bcm2835_audio_simple_newpcm,
-	.newctl = snd_bcm2835_new_hdmi_ctl,
+	.bcm2835_audio_newpcm = bcm2835_audio_simple_newpcm,
+	.bcm2835_audio_newctl = snd_bcm2835_new_hdmi_ctl,
 	.route = AUDIO_DEST_HDMI
 };
 
@@ -129,8 +123,8 @@ static struct bcm2835_audio_driver bcm2835_audio_headphones = {
 	.shortname = "bcm2835 Headphones",
 	.longname  = "bcm2835 Headphones",
 	.minchannels = 1,
-	.newpcm = bcm2835_audio_simple_newpcm,
-	.newctl = snd_bcm2835_new_headphones_ctl,
+	.bcm2835_audio_newpcm = bcm2835_audio_simple_newpcm,
+	.bcm2835_audio_newctl = snd_bcm2835_new_headphones_ctl,
 	.route = AUDIO_DEST_HEADPHONES
 };
 
@@ -189,7 +183,7 @@ static int snd_add_child_device(struct device *dev,
 	strscpy(card->shortname, audio_driver->shortname, sizeof(card->shortname));
 	strscpy(card->longname, audio_driver->longname, sizeof(card->longname));
 
-	err = audio_driver->newpcm(chip, audio_driver->shortname,
+	err = audio_driver->bcm2835_audio_newpcm(chip, audio_driver->shortname,
 		audio_driver->route,
 		numchans);
 	if (err) {
@@ -197,7 +191,7 @@ static int snd_add_child_device(struct device *dev,
 		goto error;
 	}
 
-	err = audio_driver->newctl(chip);
+	err = audio_driver->bcm2835_audio_newctl(chip);
 	if (err) {
 		dev_err(dev, "Failed to create controls, error %d\n", err);
 		goto error;
-- 
2.34.1

[PATCH 2/4] staging: vc04_services: avoid the use of typedef for function pointers

[Gaston Gonzalez]

Replace typedef bm2835_mmal_v4l2_ctrl_cb with equivalent declaration to
better align with the linux kernel coding style.

Signed-off-by: Gaston Gonzalez <gascoar@gmail.com>
---
 .../vc04_services/bcm2835-camera/controls.c   | 76 +++++++++----------
 1 file changed, 35 insertions(+), 41 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
index b096a12387f7..7782742396fc 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
@@ -65,13 +65,6 @@ enum bm2835_mmal_ctrl_type {
 	MMAL_CONTROL_TYPE_CLUSTER, /* special cluster entry */
 };
 
-struct bm2835_mmal_v4l2_ctrl;
-
-typedef	int(bm2835_mmal_v4l2_ctrl_cb)(
-				struct bm2835_mmal_dev *dev,
-				struct v4l2_ctrl *ctrl,
-				const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl);
-
 struct bm2835_mmal_v4l2_ctrl {
 	u32 id; /* v4l2 control identifier */
 	enum bm2835_mmal_ctrl_type type;
@@ -84,7 +77,8 @@ struct bm2835_mmal_v4l2_ctrl {
 	u64 step; /* step size of the control */
 	const s64 *imenu; /* integer menu array */
 	u32 mmal_id; /* mmal parameter id */
-	bm2835_mmal_v4l2_ctrl_cb *setter;
+	int (*bm2835_mmal_v4l2_ctrl_cb)(struct bm2835_mmal_dev *dev, struct v4l2_ctrl *ctrl,
+					const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl);
 };
 
 struct v4l2_to_mmal_effects_setting {
@@ -898,12 +892,12 @@ static int bm2835_mmal_s_ctrl(struct v4l2_ctrl *ctrl)
 	const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl = ctrl->priv;
 	int ret;
 
-	if (!mmal_ctrl || mmal_ctrl->id != ctrl->id || !mmal_ctrl->setter) {
+	if (!mmal_ctrl || mmal_ctrl->id != ctrl->id || !mmal_ctrl->bm2835_mmal_v4l2_ctrl_cb) {
 		pr_warn("mmal_ctrl:%p ctrl id:%d\n", mmal_ctrl, ctrl->id);
 		return -EINVAL;
 	}
 
-	ret = mmal_ctrl->setter(dev, ctrl, mmal_ctrl);
+	ret = mmal_ctrl->bm2835_mmal_v4l2_ctrl_cb(dev, ctrl, mmal_ctrl);
 	if (ret)
 		pr_warn("ctrl id:%d/MMAL param %08X- returned ret %d\n",
 			ctrl->id, mmal_ctrl->mmal_id, ret);
@@ -924,7 +918,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_SATURATION,
-		.setter = ctrl_set_rational,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_rational,
 	},
 	{
 		.id = V4L2_CID_SHARPNESS,
@@ -935,7 +929,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_SHARPNESS,
-		.setter = ctrl_set_rational,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_rational,
 	},
 	{
 		.id = V4L2_CID_CONTRAST,
@@ -946,7 +940,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_CONTRAST,
-		.setter = ctrl_set_rational,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_rational,
 	},
 	{
 		.id = V4L2_CID_BRIGHTNESS,
@@ -957,7 +951,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_BRIGHTNESS,
-		.setter = ctrl_set_rational,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_rational,
 	},
 	{
 		.id = V4L2_CID_ISO_SENSITIVITY,
@@ -968,7 +962,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = iso_qmenu,
 		.mmal_id = MMAL_PARAMETER_ISO,
-		.setter = ctrl_set_iso,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_iso,
 	},
 	{
 		.id = V4L2_CID_ISO_SENSITIVITY_AUTO,
@@ -979,7 +973,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_ISO,
-		.setter = ctrl_set_iso,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_iso,
 	},
 	{
 		.id = V4L2_CID_IMAGE_STABILIZATION,
@@ -990,7 +984,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_VIDEO_STABILISATION,
-		.setter = ctrl_set_value,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_value,
 	},
 	{
 		.id = V4L2_CID_EXPOSURE_AUTO,
@@ -1001,7 +995,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 0,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_EXPOSURE_MODE,
-		.setter = ctrl_set_exposure,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_exposure,
 	},
 	{
 		.id = V4L2_CID_EXPOSURE_ABSOLUTE,
@@ -1013,7 +1007,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_SHUTTER_SPEED,
-		.setter = ctrl_set_exposure,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_exposure,
 	},
 	{
 		.id = V4L2_CID_AUTO_EXPOSURE_BIAS,
@@ -1024,7 +1018,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 0,
 		.imenu = ev_bias_qmenu,
 		.mmal_id = MMAL_PARAMETER_EXPOSURE_COMP,
-		.setter = ctrl_set_value_ev,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_value_ev,
 	},
 	{
 		.id = V4L2_CID_EXPOSURE_AUTO_PRIORITY,
@@ -1036,7 +1030,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.imenu = NULL,
 		/* Dummy MMAL ID as it gets mapped into FPS range */
 		.mmal_id = 0,
-		.setter = ctrl_set_exposure,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_exposure,
 	},
 	{
 		.id = V4L2_CID_EXPOSURE_METERING,
@@ -1047,7 +1041,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 0,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_EXP_METERING_MODE,
-		.setter = ctrl_set_metering_mode,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_metering_mode,
 	},
 	{
 		.id = V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE,
@@ -1058,7 +1052,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 0,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_AWB_MODE,
-		.setter = ctrl_set_awb_mode,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_awb_mode,
 	},
 	{
 		.id = V4L2_CID_RED_BALANCE,
@@ -1069,7 +1063,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_CUSTOM_AWB_GAINS,
-		.setter = ctrl_set_awb_gains,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_awb_gains,
 	},
 	{
 		.id = V4L2_CID_BLUE_BALANCE,
@@ -1080,7 +1074,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_CUSTOM_AWB_GAINS,
-		.setter = ctrl_set_awb_gains,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_awb_gains,
 	},
 	{
 		.id = V4L2_CID_COLORFX,
@@ -1091,7 +1085,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 0,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_IMAGE_EFFECT,
-		.setter = ctrl_set_image_effect,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_image_effect,
 	},
 	{
 		.id = V4L2_CID_COLORFX_CBCR,
@@ -1102,7 +1096,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_COLOUR_EFFECT,
-		.setter = ctrl_set_colfx,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_colfx,
 	},
 	{
 		.id = V4L2_CID_ROTATE,
@@ -1113,7 +1107,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 90,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_ROTATION,
-		.setter = ctrl_set_rotate,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_rotate,
 	},
 	{
 		.id = V4L2_CID_HFLIP,
@@ -1124,7 +1118,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_MIRROR,
-		.setter = ctrl_set_flip,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_flip,
 	},
 	{
 		.id = V4L2_CID_VFLIP,
@@ -1135,7 +1129,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_MIRROR,
-		.setter = ctrl_set_flip,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_flip,
 	},
 	{
 		.id = V4L2_CID_MPEG_VIDEO_BITRATE_MODE,
@@ -1146,7 +1140,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 0,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_RATECONTROL,
-		.setter = ctrl_set_bitrate_mode,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_bitrate_mode,
 	},
 	{
 		.id = V4L2_CID_MPEG_VIDEO_BITRATE,
@@ -1157,7 +1151,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 25 * 1000,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_VIDEO_BIT_RATE,
-		.setter = ctrl_set_bitrate,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_bitrate,
 	},
 	{
 		.id = V4L2_CID_JPEG_COMPRESSION_QUALITY,
@@ -1168,7 +1162,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_JPEG_Q_FACTOR,
-		.setter = ctrl_set_image_encode_output,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_image_encode_output,
 	},
 	{
 		.id = V4L2_CID_POWER_LINE_FREQUENCY,
@@ -1179,7 +1173,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_FLICKER_AVOID,
-		.setter = ctrl_set_flicker_avoidance,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_flicker_avoidance,
 	},
 	{
 		.id = V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER,
@@ -1190,7 +1184,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_VIDEO_ENCODE_INLINE_HEADER,
-		.setter = ctrl_set_video_encode_param_output,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_video_encode_param_output,
 	},
 	{
 		.id = V4L2_CID_MPEG_VIDEO_H264_PROFILE,
@@ -1204,7 +1198,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_PROFILE,
-		.setter = ctrl_set_video_encode_profile_level,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_video_encode_profile_level,
 	},
 	{
 		.id = V4L2_CID_MPEG_VIDEO_H264_LEVEL,
@@ -1226,7 +1220,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_PROFILE,
-		.setter = ctrl_set_video_encode_profile_level,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_video_encode_profile_level,
 	},
 	{
 		.id = V4L2_CID_SCENE_MODE,
@@ -1238,7 +1232,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_PROFILE,
-		.setter = ctrl_set_scene_mode,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_scene_mode,
 	},
 	{
 		.id = V4L2_CID_MPEG_VIDEO_H264_I_PERIOD,
@@ -1249,7 +1243,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		.step = 1,
 		.imenu = NULL,
 		.mmal_id = MMAL_PARAMETER_INTRAPERIOD,
-		.setter = ctrl_set_video_encode_param_output,
+		.bm2835_mmal_v4l2_ctrl_cb = ctrl_set_video_encode_param_output,
 	},
 };
 
@@ -1259,8 +1253,8 @@ int bm2835_mmal_set_all_camera_controls(struct bm2835_mmal_dev *dev)
 	int ret = 0;
 
 	for (c = 0; c < V4L2_CTRL_COUNT; c++) {
-		if ((dev->ctrls[c]) && (v4l2_ctrls[c].setter)) {
-			ret = v4l2_ctrls[c].setter(dev, dev->ctrls[c],
+		if ((dev->ctrls[c]) && (v4l2_ctrls[c].bm2835_mmal_v4l2_ctrl_cb)) {
+			ret = v4l2_ctrls[c].bm2835_mmal_v4l2_ctrl_cb(dev, dev->ctrls[c],
 						   &v4l2_ctrls[c]);
 			if (ret) {
 				v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
-- 
2.34.1

Re: [PATCH 2/4] staging: vc04_services: avoid the use of typedef for function pointers

[Stefan Wahren]

Am 20.12.21 um 22:29 schrieb Gaston Gonzalez:
> Replace typedef bm2835_mmal_v4l2_ctrl_cb with equivalent declaration to
> better align with the linux kernel coding style.
>
> Signed-off-by: Gaston Gonzalez <gascoar@gmail.com>
> ---
>  .../vc04_services/bcm2835-camera/controls.c   | 76 +++++++++----------
>  1 file changed, 35 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> index b096a12387f7..7782742396fc 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> @@ -65,13 +65,6 @@ enum bm2835_mmal_ctrl_type {
>  	MMAL_CONTROL_TYPE_CLUSTER, /* special cluster entry */
>  };
>  
> -struct bm2835_mmal_v4l2_ctrl;

Oh dear, in the whole kernel it's always bcm2835 and not bm2835. I don't
see any good reason for the different naming and the resulting confusion.

@Gaston: it's not your fault, but it would be nice to have this fixed.

Re: [PATCH 2/4] staging: vc04_services: avoid the use of typedef for function pointers

[Greg KH]

On Mon, Dec 20, 2021 at 06:29:12PM -0300, Gaston Gonzalez wrote:
> Replace typedef bm2835_mmal_v4l2_ctrl_cb with equivalent declaration to
> better align with the linux kernel coding style.
> 
> Signed-off-by: Gaston Gonzalez <gascoar@gmail.com>
> ---
>  .../vc04_services/bcm2835-camera/controls.c   | 76 +++++++++----------
>  1 file changed, 35 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> index b096a12387f7..7782742396fc 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> @@ -65,13 +65,6 @@ enum bm2835_mmal_ctrl_type {
>  	MMAL_CONTROL_TYPE_CLUSTER, /* special cluster entry */
>  };
>  
> -struct bm2835_mmal_v4l2_ctrl;
> -
> -typedef	int(bm2835_mmal_v4l2_ctrl_cb)(
> -				struct bm2835_mmal_dev *dev,
> -				struct v4l2_ctrl *ctrl,
> -				const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl);

Function pointer typedefs are ok, if they are needed.

> -
>  struct bm2835_mmal_v4l2_ctrl {
>  	u32 id; /* v4l2 control identifier */
>  	enum bm2835_mmal_ctrl_type type;
> @@ -84,7 +77,8 @@ struct bm2835_mmal_v4l2_ctrl {
>  	u64 step; /* step size of the control */
>  	const s64 *imenu; /* integer menu array */
>  	u32 mmal_id; /* mmal parameter id */
> -	bm2835_mmal_v4l2_ctrl_cb *setter;
> +	int (*bm2835_mmal_v4l2_ctrl_cb)(struct bm2835_mmal_dev *dev, struct v4l2_ctrl *ctrl,
> +					const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl);

No need to rename this function pointer, why not keep it at "setter"?
That would make this patch much smaller and more obvious.

thanks,

greg k-h

Re: [PATCH 2/4] staging: vc04_services: avoid the use of typedef for function pointers

[Gaston Gonzalez]

On Tue, Dec 21, 2021 at 07:19:45AM +0100, Greg KH wrote:
> On Mon, Dec 20, 2021 at 06:29:12PM -0300, Gaston Gonzalez wrote:
> > Replace typedef bm2835_mmal_v4l2_ctrl_cb with equivalent declaration to
> > better align with the linux kernel coding style.
> > 
> > Signed-off-by: Gaston Gonzalez <gascoar@gmail.com>
> > ---
> >  .../vc04_services/bcm2835-camera/controls.c   | 76 +++++++++----------
> >  1 file changed, 35 insertions(+), 41 deletions(-)
> > 
> > diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > index b096a12387f7..7782742396fc 100644
> > --- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > +++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
> > @@ -65,13 +65,6 @@ enum bm2835_mmal_ctrl_type {
> >  	MMAL_CONTROL_TYPE_CLUSTER, /* special cluster entry */
> >  };
> >  
> > -struct bm2835_mmal_v4l2_ctrl;
> > -
> > -typedef	int(bm2835_mmal_v4l2_ctrl_cb)(
> > -				struct bm2835_mmal_dev *dev,
> > -				struct v4l2_ctrl *ctrl,
> > -				const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl);
> 
> Function pointer typedefs are ok, if they are needed.
> 
> > -
> >  struct bm2835_mmal_v4l2_ctrl {
> >  	u32 id; /* v4l2 control identifier */
> >  	enum bm2835_mmal_ctrl_type type;
> > @@ -84,7 +77,8 @@ struct bm2835_mmal_v4l2_ctrl {
> >  	u64 step; /* step size of the control */
> >  	const s64 *imenu; /* integer menu array */
> >  	u32 mmal_id; /* mmal parameter id */
> > -	bm2835_mmal_v4l2_ctrl_cb *setter;
> > +	int (*bm2835_mmal_v4l2_ctrl_cb)(struct bm2835_mmal_dev *dev, struct v4l2_ctrl *ctrl,
> > +					const struct bm2835_mmal_v4l2_ctrl *mmal_ctrl);
> 
> No need to rename this function pointer, why not keep it at "setter"?
> That would make this patch much smaller and more obvious.
> 

Ok, will do that in v2.

thanks,

Gaston


> thanks,
> 
> greg k-h
> 

[PATCH 3/4] staging: vc04_services: avoid the use of typedef for function pointers

[Gaston Gonzalez]

Replace the function pointer typedef vchiq_mmal_buffer_cb with
equivalent declaration to better align with the linux kernel coding
style.

While at it, realignments were done in some touched lines.

Signed-off-by: Gaston Gonzalez <gascoar@gmail.com>
---
 .../vc04_services/vchiq-mmal/mmal-vchiq.c     | 24 +++++++++----------
 .../vc04_services/vchiq-mmal/mmal-vchiq.h     | 13 +++++-----
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
index 76d3f0399964..54e5ce245ae7 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
@@ -269,10 +269,10 @@ static void buffer_work_cb(struct work_struct *work)
 
 	atomic_dec(&msg_context->u.bulk.port->buffers_with_vpu);
 
-	msg_context->u.bulk.port->buffer_cb(msg_context->u.bulk.instance,
-					    msg_context->u.bulk.port,
-					    msg_context->u.bulk.status,
-					    msg_context->u.bulk.buffer);
+	msg_context->u.bulk.port->vchiq_mmal_buffer_cb(msg_context->u.bulk.instance,
+						       msg_context->u.bulk.port,
+						       msg_context->u.bulk.status,
+						       msg_context->u.bulk.buffer);
 }
 
 /* workqueue scheduled callback to handle receiving buffers
@@ -1327,13 +1327,12 @@ static int port_disable(struct vchiq_mmal_instance *instance,
 			mmalbuf = list_entry(buf_head, struct mmal_buffer,
 					     list);
 			list_del(buf_head);
-			if (port->buffer_cb) {
+			if (port->vchiq_mmal_buffer_cb) {
 				mmalbuf->length = 0;
 				mmalbuf->mmal_flags = 0;
 				mmalbuf->dts = MMAL_TIME_UNKNOWN;
 				mmalbuf->pts = MMAL_TIME_UNKNOWN;
-				port->buffer_cb(instance,
-						port, 0, mmalbuf);
+				port->vchiq_mmal_buffer_cb(instance, port, 0, mmalbuf);
 			}
 		}
 
@@ -1363,7 +1362,7 @@ static int port_enable(struct vchiq_mmal_instance *instance,
 
 	port->enabled = 1;
 
-	if (port->buffer_cb) {
+	if (port->vchiq_mmal_buffer_cb) {
 		/* send buffer headers to videocore */
 		hdr_count = 1;
 		list_for_each_safe(buf_head, q, &port->buffers) {
@@ -1454,9 +1453,10 @@ EXPORT_SYMBOL_GPL(vchiq_mmal_port_parameter_get);
  * enables a port and queues buffers for satisfying callbacks if we
  * provide a callback handler
  */
-int vchiq_mmal_port_enable(struct vchiq_mmal_instance *instance,
-			   struct vchiq_mmal_port *port,
-			   vchiq_mmal_buffer_cb buffer_cb)
+int vchiq_mmal_port_enable(struct vchiq_mmal_instance *instance, struct vchiq_mmal_port *port,
+			   void (*vchiq_mmal_buffer_cb)(struct vchiq_mmal_instance  *instance,
+							struct vchiq_mmal_port *port, int status,
+							struct mmal_buffer *buffer))
 {
 	int ret;
 
@@ -1469,7 +1469,7 @@ int vchiq_mmal_port_enable(struct vchiq_mmal_instance *instance,
 		goto unlock;
 	}
 
-	port->buffer_cb = buffer_cb;
+	port->vchiq_mmal_buffer_cb = vchiq_mmal_buffer_cb;
 
 	ret = port_enable(instance, port);
 
diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h
index 1dc81ecf9268..39615ce6584a 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h
+++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h
@@ -42,11 +42,6 @@ struct vchiq_mmal_port_buffer {
 
 struct vchiq_mmal_port;
 
-typedef void (*vchiq_mmal_buffer_cb)(
-		struct vchiq_mmal_instance  *instance,
-		struct vchiq_mmal_port *port,
-		int status, struct mmal_buffer *buffer);
-
 struct vchiq_mmal_port {
 	u32 enabled:1;
 	u32 handle;
@@ -76,7 +71,9 @@ struct vchiq_mmal_port {
 	/* Count of buffers the VPU has yet to return */
 	atomic_t buffers_with_vpu;
 	/* callback on buffer completion */
-	vchiq_mmal_buffer_cb buffer_cb;
+	void (*vchiq_mmal_buffer_cb)(struct vchiq_mmal_instance  *instance,
+				     struct vchiq_mmal_port *port, int status,
+				     struct mmal_buffer *buffer);
 	/* callback context */
 	void *cb_ctx;
 };
@@ -126,7 +123,9 @@ int vchiq_mmal_component_disable(
 int vchiq_mmal_port_enable(
 		struct vchiq_mmal_instance *instance,
 		struct vchiq_mmal_port *port,
-		vchiq_mmal_buffer_cb buffer_cb);
+		void (*vchiq_mmal_buffer_cb)(struct vchiq_mmal_instance  *instance,
+					     struct vchiq_mmal_port *port, int status,
+					     struct mmal_buffer *buffer));
 
 /* disable a port
  *
-- 
2.34.1

Re: [PATCH 3/4] staging: vc04_services: avoid the use of typedef for function pointers

[Greg KH]

On Mon, Dec 20, 2021 at 06:29:13PM -0300, Gaston Gonzalez wrote:
> Replace the function pointer typedef vchiq_mmal_buffer_cb with
> equivalent declaration to better align with the linux kernel coding
> style.
> 
> While at it, realignments were done in some touched lines.
> 
> Signed-off-by: Gaston Gonzalez <gascoar@gmail.com>
> ---
>  .../vc04_services/vchiq-mmal/mmal-vchiq.c     | 24 +++++++++----------
>  .../vc04_services/vchiq-mmal/mmal-vchiq.h     | 13 +++++-----
>  2 files changed, 18 insertions(+), 19 deletions(-)

Same subject line as patch 1/4 :(

> 
> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> index 76d3f0399964..54e5ce245ae7 100644
> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> @@ -269,10 +269,10 @@ static void buffer_work_cb(struct work_struct *work)
>  
>  	atomic_dec(&msg_context->u.bulk.port->buffers_with_vpu);
>  
> -	msg_context->u.bulk.port->buffer_cb(msg_context->u.bulk.instance,
> -					    msg_context->u.bulk.port,
> -					    msg_context->u.bulk.status,
> -					    msg_context->u.bulk.buffer);
> +	msg_context->u.bulk.port->vchiq_mmal_buffer_cb(msg_context->u.bulk.instance,
> +						       msg_context->u.bulk.port,
> +						       msg_context->u.bulk.status,
> +						       msg_context->u.bulk.buffer);
>  }
>  
>  /* workqueue scheduled callback to handle receiving buffers
> @@ -1327,13 +1327,12 @@ static int port_disable(struct vchiq_mmal_instance *instance,
>  			mmalbuf = list_entry(buf_head, struct mmal_buffer,
>  					     list);
>  			list_del(buf_head);
> -			if (port->buffer_cb) {
> +			if (port->vchiq_mmal_buffer_cb) {
>  				mmalbuf->length = 0;
>  				mmalbuf->mmal_flags = 0;
>  				mmalbuf->dts = MMAL_TIME_UNKNOWN;
>  				mmalbuf->pts = MMAL_TIME_UNKNOWN;
> -				port->buffer_cb(instance,
> -						port, 0, mmalbuf);
> +				port->vchiq_mmal_buffer_cb(instance, port, 0, mmalbuf);
>  			}
>  		}
>  
> @@ -1363,7 +1362,7 @@ static int port_enable(struct vchiq_mmal_instance *instance,
>  
>  	port->enabled = 1;
>  
> -	if (port->buffer_cb) {
> +	if (port->vchiq_mmal_buffer_cb) {
>  		/* send buffer headers to videocore */
>  		hdr_count = 1;
>  		list_for_each_safe(buf_head, q, &port->buffers) {
> @@ -1454,9 +1453,10 @@ EXPORT_SYMBOL_GPL(vchiq_mmal_port_parameter_get);
>   * enables a port and queues buffers for satisfying callbacks if we
>   * provide a callback handler
>   */
> -int vchiq_mmal_port_enable(struct vchiq_mmal_instance *instance,
> -			   struct vchiq_mmal_port *port,
> -			   vchiq_mmal_buffer_cb buffer_cb)
> +int vchiq_mmal_port_enable(struct vchiq_mmal_instance *instance, struct vchiq_mmal_port *port,
> +			   void (*vchiq_mmal_buffer_cb)(struct vchiq_mmal_instance  *instance,
> +							struct vchiq_mmal_port *port, int status,
> +							struct mmal_buffer *buffer))
>  {
>  	int ret;
>  
> @@ -1469,7 +1469,7 @@ int vchiq_mmal_port_enable(struct vchiq_mmal_instance *instance,
>  		goto unlock;
>  	}
>  
> -	port->buffer_cb = buffer_cb;
> +	port->vchiq_mmal_buffer_cb = vchiq_mmal_buffer_cb;
>  
>  	ret = port_enable(instance, port);
>  
> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h
> index 1dc81ecf9268..39615ce6584a 100644
> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h
> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h
> @@ -42,11 +42,6 @@ struct vchiq_mmal_port_buffer {
>  
>  struct vchiq_mmal_port;
>  
> -typedef void (*vchiq_mmal_buffer_cb)(
> -		struct vchiq_mmal_instance  *instance,
> -		struct vchiq_mmal_port *port,
> -		int status, struct mmal_buffer *buffer);
> -
>  struct vchiq_mmal_port {
>  	u32 enabled:1;
>  	u32 handle;
> @@ -76,7 +71,9 @@ struct vchiq_mmal_port {
>  	/* Count of buffers the VPU has yet to return */
>  	atomic_t buffers_with_vpu;
>  	/* callback on buffer completion */
> -	vchiq_mmal_buffer_cb buffer_cb;
> +	void (*vchiq_mmal_buffer_cb)(struct vchiq_mmal_instance  *instance,
> +				     struct vchiq_mmal_port *port, int status,
> +				     struct mmal_buffer *buffer);

There is no need to rename the function pointer at all.

>  	/* callback context */
>  	void *cb_ctx;
>  };
> @@ -126,7 +123,9 @@ int vchiq_mmal_component_disable(
>  int vchiq_mmal_port_enable(
>  		struct vchiq_mmal_instance *instance,
>  		struct vchiq_mmal_port *port,
> -		vchiq_mmal_buffer_cb buffer_cb);
> +		void (*vchiq_mmal_buffer_cb)(struct vchiq_mmal_instance  *instance,
> +					     struct vchiq_mmal_port *port, int status,
> +					     struct mmal_buffer *buffer));
>  

Here is where using a typedef is ok.  Again, typedefs for function
pointers is normal and keeps code smaller and easier to follow.

thanks,

greg k-h

Re: [PATCH 3/4] staging: vc04_services: avoid the use of typedef for function pointers

[Gaston Gonzalez]

On Tue, Dec 21, 2021 at 07:22:05AM +0100, Greg KH wrote:
> On Mon, Dec 20, 2021 at 06:29:13PM -0300, Gaston Gonzalez wrote:
> > Replace the function pointer typedef vchiq_mmal_buffer_cb with
> > equivalent declaration to better align with the linux kernel coding
> > style.
> > 
> > While at it, realignments were done in some touched lines.
> > 
> > Signed-off-by: Gaston Gonzalez <gascoar@gmail.com>
> > ---
> >  .../vc04_services/vchiq-mmal/mmal-vchiq.c     | 24 +++++++++----------
> >  .../vc04_services/vchiq-mmal/mmal-vchiq.h     | 13 +++++-----
> >  2 files changed, 18 insertions(+), 19 deletions(-)
> 
> Same subject line as patch 1/4 :(
> 
> > 
> > diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > index 76d3f0399964..54e5ce245ae7 100644
> > --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > @@ -269,10 +269,10 @@ static void buffer_work_cb(struct work_struct *work)
> >  
> >  	atomic_dec(&msg_context->u.bulk.port->buffers_with_vpu);
> >  
> > -	msg_context->u.bulk.port->buffer_cb(msg_context->u.bulk.instance,
> > -					    msg_context->u.bulk.port,
> > -					    msg_context->u.bulk.status,
> > -					    msg_context->u.bulk.buffer);
> > +	msg_context->u.bulk.port->vchiq_mmal_buffer_cb(msg_context->u.bulk.instance,
> > +						       msg_context->u.bulk.port,
> > +						       msg_context->u.bulk.status,
> > +						       msg_context->u.bulk.buffer);
> >  }
> >  
> >  /* workqueue scheduled callback to handle receiving buffers
> > @@ -1327,13 +1327,12 @@ static int port_disable(struct vchiq_mmal_instance *instance,
> >  			mmalbuf = list_entry(buf_head, struct mmal_buffer,
> >  					     list);
> >  			list_del(buf_head);
> > -			if (port->buffer_cb) {
> > +			if (port->vchiq_mmal_buffer_cb) {
> >  				mmalbuf->length = 0;
> >  				mmalbuf->mmal_flags = 0;
> >  				mmalbuf->dts = MMAL_TIME_UNKNOWN;
> >  				mmalbuf->pts = MMAL_TIME_UNKNOWN;
> > -				port->buffer_cb(instance,
> > -						port, 0, mmalbuf);
> > +				port->vchiq_mmal_buffer_cb(instance, port, 0, mmalbuf);
> >  			}
> >  		}
> >  
> > @@ -1363,7 +1362,7 @@ static int port_enable(struct vchiq_mmal_instance *instance,
> >  
> >  	port->enabled = 1;
> >  
> > -	if (port->buffer_cb) {
> > +	if (port->vchiq_mmal_buffer_cb) {
> >  		/* send buffer headers to videocore */
> >  		hdr_count = 1;
> >  		list_for_each_safe(buf_head, q, &port->buffers) {
> > @@ -1454,9 +1453,10 @@ EXPORT_SYMBOL_GPL(vchiq_mmal_port_parameter_get);
> >   * enables a port and queues buffers for satisfying callbacks if we
> >   * provide a callback handler
> >   */
> > -int vchiq_mmal_port_enable(struct vchiq_mmal_instance *instance,
> > -			   struct vchiq_mmal_port *port,
> > -			   vchiq_mmal_buffer_cb buffer_cb)
> > +int vchiq_mmal_port_enable(struct vchiq_mmal_instance *instance, struct vchiq_mmal_port *port,
> > +			   void (*vchiq_mmal_buffer_cb)(struct vchiq_mmal_instance  *instance,
> > +							struct vchiq_mmal_port *port, int status,
> > +							struct mmal_buffer *buffer))
> >  {
> >  	int ret;
> >  
> > @@ -1469,7 +1469,7 @@ int vchiq_mmal_port_enable(struct vchiq_mmal_instance *instance,
> >  		goto unlock;
> >  	}
> >  
> > -	port->buffer_cb = buffer_cb;
> > +	port->vchiq_mmal_buffer_cb = vchiq_mmal_buffer_cb;
> >  
> >  	ret = port_enable(instance, port);
> >  
> > diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h
> > index 1dc81ecf9268..39615ce6584a 100644
> > --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h
> > +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.h
> > @@ -42,11 +42,6 @@ struct vchiq_mmal_port_buffer {
> >  
> >  struct vchiq_mmal_port;
> >  
> > -typedef void (*vchiq_mmal_buffer_cb)(
> > -		struct vchiq_mmal_instance  *instance,
> > -		struct vchiq_mmal_port *port,
> > -		int status, struct mmal_buffer *buffer);
> > -
> >  struct vchiq_mmal_port {
> >  	u32 enabled:1;
> >  	u32 handle;
> > @@ -76,7 +71,9 @@ struct vchiq_mmal_port {
> >  	/* Count of buffers the VPU has yet to return */
> >  	atomic_t buffers_with_vpu;
> >  	/* callback on buffer completion */
> > -	vchiq_mmal_buffer_cb buffer_cb;
> > +	void (*vchiq_mmal_buffer_cb)(struct vchiq_mmal_instance  *instance,
> > +				     struct vchiq_mmal_port *port, int status,
> > +				     struct mmal_buffer *buffer);
> 
> There is no need to rename the function pointer at all.
> 
> >  	/* callback context */
> >  	void *cb_ctx;
> >  };
> > @@ -126,7 +123,9 @@ int vchiq_mmal_component_disable(
> >  int vchiq_mmal_port_enable(
> >  		struct vchiq_mmal_instance *instance,
> >  		struct vchiq_mmal_port *port,
> > -		vchiq_mmal_buffer_cb buffer_cb);
> > +		void (*vchiq_mmal_buffer_cb)(struct vchiq_mmal_instance  *instance,
> > +					     struct vchiq_mmal_port *port, int status,
> > +					     struct mmal_buffer *buffer));
> >  
> 
> Here is where using a typedef is ok.  Again, typedefs for function
> pointers is normal and keeps code smaller and easier to follow.
>

Ok, I had my doubts about this one just because that lines.

Will drop it.

thanks,

Gaston


> thanks,
> 
> greg k-h

[PATCH 4/4] staging: vc04_services: update TODO file

[Gaston Gonzalez]

There are no typedef remaining under vc04_services/. Hence, remove the
task from the TODO file.

While at it, fix the items sequential numbering.

Signed-off-by: Gaston Gonzalez <gascoar@gmail.com>
---
 drivers/staging/vc04_services/interface/TODO | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/TODO b/drivers/staging/vc04_services/interface/TODO
index 39810ce017cd..241ca004735c 100644
--- a/drivers/staging/vc04_services/interface/TODO
+++ b/drivers/staging/vc04_services/interface/TODO
@@ -80,11 +80,7 @@ vchiq-core.ko and vchiq-dev.ko. This would also ease the upstreaming process.
 
 The code in vchiq_bcm2835_arm.c should fit in the generic platform file.
 
-12) Get rid of all the struct typedefs
-
-Most structs are typedefd, it's not encouraged in the kernel.
-
-13) Get rid of all non essential global structures and create a proper per
+11) 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
@@ -92,6 +88,6 @@ 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.
 
-14) Clean up Sparse warnings from __user annotations. See
+12) 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.
-- 
2.34.1

Re: [PATCH 0/4] staging: vc04_services: avoid the use of typedef for function pointers

[Greg KH]

On Mon, Dec 20, 2021 at 06:29:10PM -0300, Gaston Gonzalez wrote:
> This patch set removes some typedefs for function pointers in vc04_services.
> 
> After patches 01 to 03, there are no remaining typedef under vc04_services.
> Hence, the patch 04/04 updates the TODO file removing the 'remove typedefs'
> task.
> 
> Gaston Gonzalez (4):
>   staging: bcm2835-audio: replace function typedefs with equivalent
>     declaration
>   staging: vc04_services: replace function typedef with equivalent
>     declaration
>   staging: vc04_services: avoid the use of typedef for function pointers
>   staging: vc04_services: update TODO file

These are not the name of the patches that you sent out at all :(

Are you sure you created this properly?

Something went wrong.

greg k-h

Re: [PATCH 0/4] staging: vc04_services: avoid the use of typedef for function pointers

[Gaston Gonzalez]

On Tue, Dec 21, 2021 at 07:20:36AM +0100, Greg KH wrote:
> On Mon, Dec 20, 2021 at 06:29:10PM -0300, Gaston Gonzalez wrote:
> > This patch set removes some typedefs for function pointers in vc04_services.
> > 
> > After patches 01 to 03, there are no remaining typedef under vc04_services.
> > Hence, the patch 04/04 updates the TODO file removing the 'remove typedefs'
> > task.
> > 
> > Gaston Gonzalez (4):
> >   staging: bcm2835-audio: replace function typedefs with equivalent
> >     declaration
> >   staging: vc04_services: replace function typedef with equivalent
> >     declaration
> >   staging: vc04_services: avoid the use of typedef for function pointers
> >   staging: vc04_services: update TODO file
> 
> These are not the name of the patches that you sent out at all :(
> 
> Are you sure you created this properly?
> 
> Something went wrong.
> 
> greg k-h

Hi Greg,

I'm sorry, I messed up editing the subjects using git send-email
directly. I didn't realized that that would create a mismatch with the
summary in patch 0/4. It is clear now ...

thanks,

Gaston