2016-10-12 14:26:33

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 00/34] [media] DaVinci-Video Processing: Fine-tuning for several function implementations

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 16:20:02 +0200

Several update suggestions were taken into account
from static source code analysis.

Markus Elfring (34):
Use kmalloc_array() in vpbe_initialize()
Delete two error messages for a failed memory allocation
Adjust 16 checks for null pointers
Combine substrings for four messages
Return an error code only as a constant in vpbe_probe()
Return an error code only by a single variable in vpbe_initialize()
Delete an unnecessary variable initialisation in vpbe_initialize()
Return the success indication only as a constant in vpbe_set_mode()
Reduce the scope for a variable in vpbe_set_default_output()
Check return value of a setup_if_config() call in vpbe_set_output()
Rename a jump label in vpbe_set_output()
Delete an unnecessary variable initialisation in vpbe_set_output()
Capture: Use kmalloc_array() in vpfe_probe()
Capture: Delete three error messages for a failed memory allocation
Capture: Improve another size determination in vpfe_probe()
Capture: Delete an unnecessary variable initialisation in vpfe_probe()
Capture: Improve another size determination in vpfe_enum_input()
Capture: Combine substrings for an error message in vpfe_enum_input()
Capture: Improve another size determination in vpfe_open()
Capture: Adjust 13 checks for null pointers
Capture: Delete an unnecessary variable initialisation in 11 functions
Capture: Move two assignments in vpfe_s_input()
Capture: Delete unnecessary braces in vpfe_isr()
Capture: Delete an unnecessary return statement in vpfe_unregister_ccdc_device()
Capture: Use kcalloc() in vpif_probe()
Capture: Delete an error message for a failed memory allocation
Capture: Adjust ten checks for null pointers
Capture: Delete an unnecessary variable initialisation in vpif_querystd()
Capture: Delete an unnecessary variable initialisation in vpif_channel_isr()
Display: Use kcalloc() in vpif_probe()
Display: Delete an error message for a failed memory allocation
Display: Adjust 11 checks for null pointers
Display: Delete an unnecessary variable initialisation in vpif_channel_isr()
Display: Delete an unnecessary variable initialisation in process_progressive_mode()

drivers/media/platform/davinci/vpbe.c | 93 ++++++++++++---------------
drivers/media/platform/davinci/vpfe_capture.c | 88 ++++++++++++-------------
drivers/media/platform/davinci/vpif_capture.c | 28 ++++----
drivers/media/platform/davinci/vpif_display.c | 30 ++++-----
4 files changed, 109 insertions(+), 130 deletions(-)

--
2.10.1


2016-10-12 14:29:33

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 01/34] [media] DaVinci-VPBE: Use kmalloc_array() in vpbe_initialize()

From: Markus Elfring <[email protected]>
Date: Tue, 11 Oct 2016 09:40:41 +0200

* A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array".

This issue was detected by using the Coccinelle software.

* Replace the specification of a data type by a pointer dereference
to make the corresponding size determination a bit safer according to
the Linux coding style convention.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpbe.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
index 9a6c2cc..8c062ff 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -676,9 +676,9 @@ static int vpbe_initialize(struct device *dev, struct vpbe_device *vpbe_dev)
* store venc sd index.
*/
num_encoders = vpbe_dev->cfg->num_ext_encoders + 1;
- vpbe_dev->encoders = kmalloc(
- sizeof(struct v4l2_subdev *)*num_encoders,
- GFP_KERNEL);
+ vpbe_dev->encoders = kmalloc_array(num_encoders,
+ sizeof(*vpbe_dev->encoders),
+ GFP_KERNEL);
if (NULL == vpbe_dev->encoders) {
v4l2_err(&vpbe_dev->v4l2_dev,
"unable to allocate memory for encoders sub devices");
--
2.10.1

2016-10-12 14:30:59

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 02/34] [media] DaVinci-VPBE: Delete two error messages for a failed memory allocation

From: Markus Elfring <[email protected]>
Date: Tue, 11 Oct 2016 09:56:13 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: Possible unnecessary 'out of memory' message

Thus remove such a logging statement in two functions.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpbe.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
index 8c062ff..b479747 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -680,8 +680,6 @@ static int vpbe_initialize(struct device *dev, struct vpbe_device *vpbe_dev)
sizeof(*vpbe_dev->encoders),
GFP_KERNEL);
if (NULL == vpbe_dev->encoders) {
- v4l2_err(&vpbe_dev->v4l2_dev,
- "unable to allocate memory for encoders sub devices");
ret = -ENOMEM;
goto fail_dev_unregister;
}
@@ -841,11 +839,9 @@ static int vpbe_probe(struct platform_device *pdev)
}

vpbe_dev = kzalloc(sizeof(*vpbe_dev), GFP_KERNEL);
- if (vpbe_dev == NULL) {
- v4l2_err(pdev->dev.driver, "Unable to allocate memory"
- " for vpbe_device\n");
+ if (!vpbe_dev)
return -ENOMEM;
- }
+
vpbe_dev->cfg = cfg;
vpbe_dev->ops = vpbe_dev_ops;
vpbe_dev->pdev = &pdev->dev;
--
2.10.1

2016-10-12 14:34:32

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 04/34] [media] DaVinci-VPBE: Combine substrings for four messages

From: Markus Elfring <[email protected]>
Date: Tue, 11 Oct 2016 13:40:14 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: quoted string split across lines

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpbe.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
index 496b27f..625bddf 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -702,15 +702,15 @@ static int vpbe_initialize(struct device *dev, struct vpbe_device *vpbe_dev)
"v4l2 sub device %s registered\n",
enc_info->module_name);
else {
- v4l2_err(&vpbe_dev->v4l2_dev, "encoder %s"
- " failed to register",
+ v4l2_err(&vpbe_dev->v4l2_dev,
+ "encoder %s failed to register",
enc_info->module_name);
ret = -ENODEV;
goto fail_kfree_encoders;
}
} else
- v4l2_warn(&vpbe_dev->v4l2_dev, "non-i2c encoders"
- " currently not supported");
+ v4l2_warn(&vpbe_dev->v4l2_dev,
+ "non-i2c encoders currently not supported");
}
/* Add amplifier subdevice for dm365 */
if ((strcmp(vpbe_dev->cfg->module_name, "dm365-vpbe-display") == 0) &&
@@ -731,9 +731,9 @@ static int vpbe_initialize(struct device *dev, struct vpbe_device *vpbe_dev)
"v4l2 sub device %s registered\n",
amp_info->module_name);
} else {
- vpbe_dev->amp = NULL;
- v4l2_warn(&vpbe_dev->v4l2_dev, "non-i2c amplifiers"
- " currently not supported");
+ vpbe_dev->amp = NULL;
+ v4l2_warn(&vpbe_dev->v4l2_dev,
+ "non-i2c amplifiers currently not supported");
}
} else {
vpbe_dev->amp = NULL;
@@ -832,8 +832,8 @@ static int vpbe_probe(struct platform_device *pdev)
if (!cfg->module_name[0] ||
!cfg->osd.module_name[0] ||
!cfg->venc.module_name[0]) {
- v4l2_err(pdev->dev.driver, "vpbe display module names not"
- " defined\n");
+ v4l2_err(pdev->dev.driver,
+ "vpbe display module names not defined\n");
return ret;
}

--
2.10.1

2016-10-12 14:41:50

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 06/34] [media] DaVinci-VPBE: Return an error code only by a single variable in vpbe_initialize()

From: Markus Elfring <[email protected]>
Date: Tue, 11 Oct 2016 14:15:57 +0200

An error code was assigned to the local variable "err" in an if branch.
But this variable was not used further then.

Use the local variable "ret" instead like at other places in this function.

Fixes: 66715cdc3224a4e241c1a92856b9a4af3b70e06d ("[media] davinci vpbe:
VPBE display driver")
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpbe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
index 4c4cd81..afa8ff7 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -665,7 +665,7 @@ static int vpbe_initialize(struct device *dev, struct vpbe_device *vpbe_dev)
if (err) {
v4l2_err(&vpbe_dev->v4l2_dev,
"unable to initialize the OSD device");
- err = -ENOMEM;
+ ret = -ENOMEM;
goto fail_dev_unregister;
}
}
--
2.10.1

2016-10-12 14:42:08

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 05/34] [media] DaVinci-VPBE: Return an error code only as a constant in vpbe_probe()

From: Markus Elfring <[email protected]>
Date: Tue, 11 Oct 2016 13:43:25 +0200

* Return an error code without storing it in an intermediate variable.

* Delete the local variable "ret" which became unnecessary with
this refactoring.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpbe.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
index 625bddf..4c4cd81 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -821,7 +821,6 @@ static int vpbe_probe(struct platform_device *pdev)
{
struct vpbe_device *vpbe_dev;
struct vpbe_config *cfg;
- int ret = -EINVAL;

if (!pdev->dev.platform_data) {
v4l2_err(pdev->dev.driver, "No platform data\n");
@@ -834,7 +833,7 @@ static int vpbe_probe(struct platform_device *pdev)
!cfg->venc.module_name[0]) {
v4l2_err(pdev->dev.driver,
"vpbe display module names not defined\n");
- return ret;
+ return -EINVAL;
}

vpbe_dev = kzalloc(sizeof(*vpbe_dev), GFP_KERNEL);
--
2.10.1

2016-10-12 14:42:48

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 07/34] [media] DaVinci-VPBE: Delete an unnecessary variable initialisation in vpbe_initialize()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 09:45:39 +0200

The local variable "ret" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpbe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
index afa8ff7..9fdd8c0 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -597,7 +597,7 @@ static int vpbe_initialize(struct device *dev, struct vpbe_device *vpbe_dev)
struct osd_state *osd_device;
struct i2c_adapter *i2c_adap;
int num_encoders;
- int ret = 0;
+ int ret;
int err;
int i;

--
2.10.1

2016-10-12 14:43:53

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 08/34] [media] DaVinci-VPBE: Return the success indication only as a constant in vpbe_set_mode()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 09:51:29 +0200

* Return a success code without storing it in an intermediate variable.

* Delete the local variable "ret" which became unnecessary with
this refactoring.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpbe.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
index 9fdd8c0..d6a0221 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -509,7 +509,6 @@ static int vpbe_set_mode(struct vpbe_device *vpbe_dev,
struct v4l2_dv_timings dv_timings;
struct osd_state *osd_device;
int out_index = vpbe_dev->current_out_index;
- int ret = 0;
int i;

if (!mode_info || !mode_info->name)
@@ -549,8 +548,7 @@ static int vpbe_set_mode(struct vpbe_device *vpbe_dev,
vpbe_dev->current_timings.upper_margin);

mutex_unlock(&vpbe_dev->lock);
-
- return ret;
+ return 0;
}

static int vpbe_set_default_mode(struct vpbe_device *vpbe_dev)
--
2.10.1

2016-10-12 14:45:15

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 09/34] [media] DaVinci-VPBE: Reduce the scope for a variable in vpbe_set_default_output()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 09:54:26 +0200

* Move the definition for the variable "ret" into an if branch
so that an extra initialisation can be avoided at the beginning
by this refactoring.

* Return a success code as a constant at the end.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpbe.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
index d6a0221..19611a2 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -297,19 +297,19 @@ static int vpbe_set_output(struct vpbe_device *vpbe_dev, int index)
static int vpbe_set_default_output(struct vpbe_device *vpbe_dev)
{
struct vpbe_config *cfg = vpbe_dev->cfg;
- int ret = 0;
int i;

for (i = 0; i < cfg->num_outputs; i++) {
if (!strcmp(def_output,
cfg->outputs[i].output.name)) {
- ret = vpbe_set_output(vpbe_dev, i);
+ int ret = vpbe_set_output(vpbe_dev, i);
+
if (!ret)
vpbe_dev->current_out_index = i;
return ret;
}
}
- return ret;
+ return 0;
}

/**
--
2.10.1

2016-10-12 14:47:54

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 10/34] [media] DaVinci-VPBE: Check return value of a setup_if_config() call in vpbe_set_output()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 09:56:56 +0200

* A function was called over the pointer "setup_if_config" in the data
structure "venc_platform_data". But the return value was not used so far.
Thus assign it to the local variable "ret" which will be checked with
the next statement.

Fixes: 9a7f95ad1c946efdd7a7a72df27db738260a0fd8 ("[media] davinci vpbe: add dm365 VPBE display driver changes")

* Pass a value to this function call without storing it in an intermediate
variable before.

* Delete the local variable "if_params" which became unnecessary with
this refactoring.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpbe.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
index 19611a2..6e7b0df 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -227,7 +227,6 @@ static int vpbe_set_output(struct vpbe_device *vpbe_dev, int index)
vpbe_current_encoder_info(vpbe_dev);
struct vpbe_config *cfg = vpbe_dev->cfg;
struct venc_platform_data *venc_device = vpbe_dev->venc_device;
- u32 if_params;
int enc_out_index;
int sd_index;
int ret = 0;
@@ -257,8 +256,8 @@ static int vpbe_set_output(struct vpbe_device *vpbe_dev, int index)
goto out;
}

- if_params = cfg->outputs[index].if_params;
- venc_device->setup_if_config(if_params);
+ ret = venc_device->setup_if_config(cfg
+ ->outputs[index].if_params);
if (ret)
goto out;
}
--
2.10.1

2016-10-12 14:50:30

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 12/34] [media] DaVinci-VPBE: Delete an unnecessary variable initialisation in vpbe_set_output()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 10:16:23 +0200

The local variable "ret" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpbe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
index e68a792..b2c5d8f 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -229,7 +229,7 @@ static int vpbe_set_output(struct vpbe_device *vpbe_dev, int index)
struct venc_platform_data *venc_device = vpbe_dev->venc_device;
int enc_out_index;
int sd_index;
- int ret = 0;
+ int ret;

if (index >= cfg->num_outputs)
return -EINVAL;
--
2.10.1

2016-10-12 14:51:28

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 13/34] [media] DaVinci-VPFE-Capture: Use kmalloc_array() in vpfe_probe()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 10:20:02 +0200

* A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array".

This issue was detected by using the Coccinelle software.

* Replace the specification of a data type by a pointer dereference
to make the corresponding size determination a bit safer according to
the Linux coding style convention.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpfe_capture.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
index 6efb2f1..5c1b8cf 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -1940,8 +1940,9 @@ static int vpfe_probe(struct platform_device *pdev)
video_set_drvdata(&vpfe_dev->video_dev, vpfe_dev);
i2c_adap = i2c_get_adapter(vpfe_cfg->i2c_adapter_id);
num_subdevs = vpfe_cfg->num_subdevs;
- vpfe_dev->sd = kmalloc(sizeof(struct v4l2_subdev *) * num_subdevs,
- GFP_KERNEL);
+ vpfe_dev->sd = kmalloc_array(num_subdevs,
+ sizeof(*vpfe_dev->sd),
+ GFP_KERNEL);
if (NULL == vpfe_dev->sd) {
v4l2_err(&vpfe_dev->v4l2_dev,
"unable to allocate memory for subdevice pointers\n");
--
2.10.1

2016-10-12 14:54:37

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 15/34] [media] DaVinci-VPFE-Capture: Improve another size determination in vpfe_probe()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 10:24:57 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpfe_capture.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
index 23142f0..4db3212 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -1850,7 +1850,7 @@ static int vpfe_probe(struct platform_device *pdev)
}

/* Allocate memory for ccdc configuration */
- ccdc_cfg = kmalloc(sizeof(struct ccdc_config), GFP_KERNEL);
+ ccdc_cfg = kmalloc(sizeof(*ccdc_cfg), GFP_KERNEL);
if (!ccdc_cfg)
goto probe_free_dev_mem;

--
2.10.1

2016-10-12 14:54:40

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 16/34] [media] DaVinci-VPFE-Capture: Delete an unnecessary variable initialisation in vpfe_probe()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 10:30:28 +0200

* Return an error code as a constant after a failed call of
the function "vpfe_initialize".

* The local variable "ret" will be set then to an appropriate value
a bit later. Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpfe_capture.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
index 4db3212..8314c39 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -1819,7 +1819,7 @@ static int vpfe_probe(struct platform_device *pdev)
struct vpfe_device *vpfe_dev;
struct i2c_adapter *i2c_adap;
struct video_device *vfd;
- int ret = -ENOMEM, i, j;
+ int ret, i, j;
int num_subdevs = 0;

/* Get the pointer to the device object */
@@ -1828,7 +1828,7 @@ static int vpfe_probe(struct platform_device *pdev)
if (!vpfe_dev) {
v4l2_err(pdev->dev.driver,
"Failed to allocate memory for vpfe_dev\n");
- return ret;
+ return -ENOMEM;
}

vpfe_dev->pdev = &pdev->dev;
--
2.10.1

2016-10-12 14:55:34

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 11/34] [media] DaVinci-VPBE: Rename a jump label in vpbe_set_output()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 10:10:19 +0200

Adjust jump labels according to the Linux coding style convention.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpbe.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
index 6e7b0df..e68a792 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -253,20 +253,20 @@ static int vpbe_set_output(struct vpbe_device *vpbe_dev, int index)
sd_index = vpbe_find_encoder_sd_index(cfg, index);
if (sd_index < 0) {
ret = -EINVAL;
- goto out;
+ goto unlock;
}

ret = venc_device->setup_if_config(cfg
->outputs[index].if_params);
if (ret)
- goto out;
+ goto unlock;
}

/* Set output at the encoder */
ret = v4l2_subdev_call(vpbe_dev->encoders[sd_index], video,
s_routing, 0, enc_out_index, 0);
if (ret)
- goto out;
+ goto unlock;

/*
* It is assumed that venc or extenal encoder will set a default
@@ -288,7 +288,7 @@ static int vpbe_set_output(struct vpbe_device *vpbe_dev, int index)
vpbe_dev->current_sd_index = sd_index;
vpbe_dev->current_out_index = index;
}
-out:
+unlock:
mutex_unlock(&vpbe_dev->lock);
return ret;
}
--
2.10.1

2016-10-12 14:55:57

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 17/34] [media] DaVinci-VPFE-Capture: Improve another size determination in vpfe_enum_input()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 10:33:42 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpfe_capture.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
index 8314c39..87ee35d 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -1091,7 +1091,7 @@ static int vpfe_enum_input(struct file *file, void *priv,
return -EINVAL;
}
sdinfo = &vpfe_dev->cfg->sub_devs[subdev];
- memcpy(inp, &sdinfo->inputs[index], sizeof(struct v4l2_input));
+ memcpy(inp, &sdinfo->inputs[index], sizeof(*inp));
return 0;
}

--
2.10.1

2016-10-12 14:56:59

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 18/34] [media] DaVinci-VPFE-Capture: Combine substrings for an error message in vpfe_enum_input()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 10:40:10 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: quoted string split across lines

Thus fix an affected source code place.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpfe_capture.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
index 87ee35d..ee7b3e3 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -1086,8 +1086,8 @@ static int vpfe_enum_input(struct file *file, void *priv,
&subdev,
&index,
inp->index) < 0) {
- v4l2_err(&vpfe_dev->v4l2_dev, "input information not found"
- " for the subdev\n");
+ v4l2_err(&vpfe_dev->v4l2_dev,
+ "input information not found for the subdev\n");
return -EINVAL;
}
sdinfo = &vpfe_dev->cfg->sub_devs[subdev];
--
2.10.1

2016-10-12 15:00:53

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 22/34] [media] DaVinci-VPFE-Capture: Move two assignments in vpfe_s_input()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 11:22:23 +0200

Move assignments for two local variables into an else branch so that
their setting will only be performed after corresponding data processing
succeeded by this function.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpfe_capture.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
index ba71310..f0467fe 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -1111,7 +1111,7 @@ static int vpfe_s_input(struct file *file, void *priv, unsigned int index)
struct vpfe_subdev_info *sdinfo;
int subdev_index, inp_index;
struct vpfe_route *route;
- u32 input = 0, output = 0;
+ u32 input, output;
int ret;

v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_s_input\n");
@@ -1144,6 +1144,9 @@ static int vpfe_s_input(struct file *file, void *priv, unsigned int index)
if (route && sdinfo->can_route) {
input = route->input;
output = route->output;
+ } else {
+ input = 0;
+ output = 0;
}

if (sd)
--
2.10.1

2016-10-12 15:01:06

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 21/34] [media] DaVinci-VPFE-Capture: Delete an unnecessary variable initialisation in 11 functions

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 10:50:54 +0200

The local variable "ret" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpfe_capture.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
index 9da353b..ba71310 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -384,7 +384,7 @@ static int vpfe_config_image_format(struct vpfe_device *vpfe_dev,
};
struct v4l2_mbus_framefmt *mbus_fmt = &fmt.format;
struct v4l2_pix_format *pix = &vpfe_dev->fmt.fmt.pix;
- int i, ret = 0;
+ int i, ret;

for (i = 0; i < ARRAY_SIZE(vpfe_standards); i++) {
if (vpfe_standards[i].std_id & std_id) {
@@ -453,7 +453,7 @@ static int vpfe_config_image_format(struct vpfe_device *vpfe_dev,

static int vpfe_initialize_device(struct vpfe_device *vpfe_dev)
{
- int ret = 0;
+ int ret;

/* set first input of current subdevice as the current input */
vpfe_dev->current_input = 0;
@@ -979,7 +979,7 @@ static int vpfe_s_fmt_vid_cap(struct file *file, void *priv,
{
struct vpfe_device *vpfe_dev = video_drvdata(file);
const struct vpfe_pixel_format *pix_fmts;
- int ret = 0;
+ int ret;

v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_s_fmt_vid_cap\n");

@@ -1112,7 +1112,7 @@ static int vpfe_s_input(struct file *file, void *priv, unsigned int index)
int subdev_index, inp_index;
struct vpfe_route *route;
u32 input = 0, output = 0;
- int ret = -EINVAL;
+ int ret;

v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_s_input\n");

@@ -1178,7 +1178,7 @@ static int vpfe_querystd(struct file *file, void *priv, v4l2_std_id *std_id)
{
struct vpfe_device *vpfe_dev = video_drvdata(file);
struct vpfe_subdev_info *sdinfo;
- int ret = 0;
+ int ret;

v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_querystd\n");

@@ -1197,7 +1197,7 @@ static int vpfe_s_std(struct file *file, void *priv, v4l2_std_id std_id)
{
struct vpfe_device *vpfe_dev = video_drvdata(file);
struct vpfe_subdev_info *sdinfo;
- int ret = 0;
+ int ret;

v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_s_std\n");

@@ -1346,7 +1346,7 @@ static int vpfe_reqbufs(struct file *file, void *priv,
{
struct vpfe_device *vpfe_dev = video_drvdata(file);
struct vpfe_fh *fh = file->private_data;
- int ret = 0;
+ int ret;

v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_reqbufs\n");

@@ -1478,7 +1478,7 @@ static int vpfe_streamon(struct file *file, void *priv,
struct vpfe_fh *fh = file->private_data;
struct vpfe_subdev_info *sdinfo;
unsigned long addr;
- int ret = 0;
+ int ret;

v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_streamon\n");

@@ -1561,7 +1561,7 @@ static int vpfe_streamoff(struct file *file, void *priv,
struct vpfe_device *vpfe_dev = video_drvdata(file);
struct vpfe_fh *fh = file->private_data;
struct vpfe_subdev_info *sdinfo;
- int ret = 0;
+ int ret;

v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_streamoff\n");

@@ -1647,7 +1647,7 @@ static int vpfe_s_selection(struct file *file, void *priv,
{
struct vpfe_device *vpfe_dev = video_drvdata(file);
struct v4l2_rect rect = sel->r;
- int ret = 0;
+ int ret;

v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_s_selection\n");

@@ -1705,7 +1705,7 @@ static long vpfe_param_handler(struct file *file, void *priv,
bool valid_prio, unsigned int cmd, void *param)
{
struct vpfe_device *vpfe_dev = video_drvdata(file);
- int ret = 0;
+ int ret;

v4l2_dbg(2, debug, &vpfe_dev->v4l2_dev, "vpfe_param_handler\n");

--
2.10.1

2016-10-12 15:01:37

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 14/34] [media] DaVinci-VPFE-Capture: Delete three error messages for a failed memory allocation

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 10:22:47 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: Possible unnecessary 'out of memory' message

Thus remove such a logging statement in two functions.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpfe_capture.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
index 5c1b8cf..23142f0 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -512,11 +512,9 @@ static int vpfe_open(struct file *file)

/* Allocate memory for the file handle object */
fh = kmalloc(sizeof(struct vpfe_fh), GFP_KERNEL);
- if (NULL == fh) {
- v4l2_err(&vpfe_dev->v4l2_dev,
- "unable to allocate memory for file handle object\n");
+ if (!fh)
return -ENOMEM;
- }
+
/* store pointer to fh in private_data member of file */
file->private_data = fh;
fh->vpfe_dev = vpfe_dev;
@@ -1853,11 +1851,8 @@ static int vpfe_probe(struct platform_device *pdev)

/* Allocate memory for ccdc configuration */
ccdc_cfg = kmalloc(sizeof(struct ccdc_config), GFP_KERNEL);
- if (NULL == ccdc_cfg) {
- v4l2_err(pdev->dev.driver,
- "Memory allocation failed for ccdc_cfg\n");
+ if (!ccdc_cfg)
goto probe_free_dev_mem;
- }

mutex_lock(&ccdc_lock);

@@ -1944,8 +1939,6 @@ static int vpfe_probe(struct platform_device *pdev)
sizeof(*vpfe_dev->sd),
GFP_KERNEL);
if (NULL == vpfe_dev->sd) {
- v4l2_err(&vpfe_dev->v4l2_dev,
- "unable to allocate memory for subdevice pointers\n");
ret = -ENOMEM;
goto probe_out_video_unregister;
}
--
2.10.1

2016-10-12 15:01:51

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 23/34] [media] DaVinci-VPFE-Capture: Delete unnecessary braces in vpfe_isr()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 14:54:21 +0200

Do not use curly brackets at one source code place
where a single statement should be sufficient.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpfe_capture.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
index f0467fe..e264c90 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -615,9 +615,8 @@ static irqreturn_t vpfe_isr(int irq, void *dev_id)
* interleavely or separately in memory, reconfigure
* the CCDC memory address
*/
- if (field == V4L2_FIELD_SEQ_TB) {
+ if (field == V4L2_FIELD_SEQ_TB)
vpfe_schedule_bottom_field(vpfe_dev);
- }
goto clear_intr;
}
/*
--
2.10.1

2016-10-12 14:59:13

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 19/34] [media] DaVinci-VPFE-Capture: Improve another size determination in vpfe_open()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 10:44:05 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpfe_capture.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
index ee7b3e3..e370400 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -511,7 +511,7 @@ static int vpfe_open(struct file *file)
}

/* Allocate memory for the file handle object */
- fh = kmalloc(sizeof(struct vpfe_fh), GFP_KERNEL);
+ fh = kmalloc(sizeof(*fh), GFP_KERNEL);
if (!fh)
return -ENOMEM;

--
2.10.1

2016-10-12 15:02:52

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 24/34] [media] DaVinci-VPFE-Capture: Delete an unnecessary return statement in vpfe_unregister_ccdc_device()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 15:10:54 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: void function return statements are not generally useful

Thus remove such a statement here.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpfe_capture.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
index e264c90..a79e1d5 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -281,7 +281,6 @@ void vpfe_unregister_ccdc_device(struct ccdc_hw_device *dev)
mutex_lock(&ccdc_lock);
ccdc_dev = NULL;
mutex_unlock(&ccdc_lock);
- return;
}
EXPORT_SYMBOL(vpfe_unregister_ccdc_device);

--
2.10.1

2016-10-12 14:59:12

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 20/34] [media] DaVinci-VPFE-Capture: Adjust 13 checks for null pointers

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 10:46:28 +0200

Convert comparisons with the preprocessor symbol "NULL" to condition checks
without it.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpfe_capture.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
index e370400..9da353b 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -229,7 +229,7 @@ int vpfe_register_ccdc_device(struct ccdc_hw_device *dev)
BUG_ON(!dev->hw_ops.getfid);

mutex_lock(&ccdc_lock);
- if (NULL == ccdc_cfg) {
+ if (!ccdc_cfg) {
/*
* TODO. Will this ever happen? if so, we need to fix it.
* Proabably we need to add the request to a linked list and
@@ -265,7 +265,7 @@ EXPORT_SYMBOL(vpfe_register_ccdc_device);
*/
void vpfe_unregister_ccdc_device(struct ccdc_hw_device *dev)
{
- if (NULL == dev) {
+ if (!dev) {
printk(KERN_ERR "invalid ccdc device ptr\n");
return;
}
@@ -469,7 +469,7 @@ static int vpfe_initialize_device(struct vpfe_device *vpfe_dev)

/* now open the ccdc device to initialize it */
mutex_lock(&ccdc_lock);
- if (NULL == ccdc_dev) {
+ if (!ccdc_dev) {
v4l2_err(&vpfe_dev->v4l2_dev, "ccdc device not registered\n");
ret = -ENODEV;
goto unlock;
@@ -582,7 +582,7 @@ static irqreturn_t vpfe_isr(int irq, void *dev_id)
goto clear_intr;

/* only for 6446 this will be applicable */
- if (NULL != ccdc_dev->hw_ops.reset)
+ if (ccdc_dev->hw_ops.reset)
ccdc_dev->hw_ops.reset();

if (field == V4L2_FIELD_NONE) {
@@ -822,7 +822,7 @@ static const struct vpfe_pixel_format *
int temp, found;

vpfe_pix_fmt = vpfe_lookup_pix_format(pixfmt->pixelformat);
- if (NULL == vpfe_pix_fmt) {
+ if (!vpfe_pix_fmt) {
/*
* use current pixel format in the vpfe device. We
* will find this pix format in the table
@@ -965,7 +965,7 @@ static int vpfe_enum_fmt_vid_cap(struct file *file, void *priv,

/* Fill in the information about format */
pix_fmt = vpfe_lookup_pix_format(pix);
- if (NULL != pix_fmt) {
+ if (pix_fmt) {
temp_index = fmt->index;
*fmt = pix_fmt->fmtdesc;
fmt->index = temp_index;
@@ -991,8 +991,7 @@ static int vpfe_s_fmt_vid_cap(struct file *file, void *priv,

/* Check for valid frame format */
pix_fmts = vpfe_check_format(vpfe_dev, &fmt->fmt.pix);
-
- if (NULL == pix_fmts)
+ if (!pix_fmts)
return -EINVAL;

/* store the pixel format in the device object */
@@ -1018,7 +1017,7 @@ static int vpfe_try_fmt_vid_cap(struct file *file, void *priv,
v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_try_fmt_vid_cap\n");

pix_fmts = vpfe_check_format(vpfe_dev, &f->fmt.pix);
- if (NULL == pix_fmts)
+ if (!pix_fmts)
return -EINVAL;
return 0;
}
@@ -1833,7 +1832,7 @@ static int vpfe_probe(struct platform_device *pdev)

vpfe_dev->pdev = &pdev->dev;

- if (NULL == pdev->dev.platform_data) {
+ if (!pdev->dev.platform_data) {
v4l2_err(pdev->dev.driver, "Unable to get vpfe config\n");
ret = -ENODEV;
goto probe_free_dev_mem;
@@ -1841,9 +1840,7 @@ static int vpfe_probe(struct platform_device *pdev)

vpfe_cfg = pdev->dev.platform_data;
vpfe_dev->cfg = vpfe_cfg;
- if (NULL == vpfe_cfg->ccdc ||
- NULL == vpfe_cfg->card_name ||
- NULL == vpfe_cfg->sub_devs) {
+ if (!vpfe_cfg->ccdc || !vpfe_cfg->card_name || !vpfe_cfg->sub_devs) {
v4l2_err(pdev->dev.driver, "null ptr in vpfe_cfg\n");
ret = -ENOENT;
goto probe_free_dev_mem;
@@ -1938,7 +1935,7 @@ static int vpfe_probe(struct platform_device *pdev)
vpfe_dev->sd = kmalloc_array(num_subdevs,
sizeof(*vpfe_dev->sd),
GFP_KERNEL);
- if (NULL == vpfe_dev->sd) {
+ if (!vpfe_dev->sd) {
ret = -ENOMEM;
goto probe_out_video_unregister;
}
--
2.10.1

2016-10-12 15:05:13

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 25/34] [media] DaVinci-VPIF-Capture: Use kcalloc() in vpif_probe()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 15:15:34 +0200

* A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kcalloc".

This issue was detected by using the Coccinelle software.

* Replace the specification of a data type by a pointer dereference
to make the corresponding size determination a bit safer according to
the Linux coding style convention.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpif_capture.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0..fb9e850 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1464,8 +1464,7 @@ static __init int vpif_probe(struct platform_device *pdev)
vpif_obj.config = pdev->dev.platform_data;

subdev_count = vpif_obj.config->subdev_count;
- vpif_obj.sd = kzalloc(sizeof(struct v4l2_subdev *) * subdev_count,
- GFP_KERNEL);
+ vpif_obj.sd = kcalloc(subdev_count, sizeof(*vpif_obj.sd), GFP_KERNEL);
if (vpif_obj.sd == NULL) {
vpif_err("unable to allocate memory for subdevice pointers\n");
err = -ENOMEM;
--
2.10.1

2016-10-12 15:05:12

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 26/34] [media] DaVinci-VPIF-Capture: Delete an error message for a failed memory allocation

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 15:18:45 +0200

Omit an extra message for a memory allocation failure in this function.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpif_capture.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index fb9e850..24d1f61 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1466,7 +1466,6 @@ static __init int vpif_probe(struct platform_device *pdev)
subdev_count = vpif_obj.config->subdev_count;
vpif_obj.sd = kcalloc(subdev_count, sizeof(*vpif_obj.sd), GFP_KERNEL);
if (vpif_obj.sd == NULL) {
- vpif_err("unable to allocate memory for subdevice pointers\n");
err = -ENOMEM;
goto vpif_unregister;
}
--
2.10.1

2016-10-12 15:07:56

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 28/34] [media] DaVinci-VPIF-Capture: Delete an unnecessary variable initialisation in vpif_querystd()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 15:22:45 +0200

The local variable "ret" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpif_capture.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index d9fc591..4197ca4 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -731,7 +731,7 @@ static int vpif_querystd(struct file *file, void *priv, v4l2_std_id *std_id)
{
struct video_device *vdev = video_devdata(file);
struct channel_obj *ch = video_get_drvdata(vdev);
- int ret = 0;
+ int ret;

vpif_dbg(2, debug, "vpif_querystd\n");

--
2.10.1

2016-10-12 15:13:12

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 30/34] [media] DaVinci-VPIF-Display: Use kcalloc() in vpif_probe()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 15:30:44 +0200

* A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kcalloc".

This issue was detected by using the Coccinelle software.

* Replace the specification of a data type by a pointer dereference
to make the corresponding size determination a bit safer according to
the Linux coding style convention.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpif_display.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 75b2723..80121e8 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -1279,8 +1279,7 @@ static __init int vpif_probe(struct platform_device *pdev)
vpif_obj.config = pdev->dev.platform_data;
subdev_count = vpif_obj.config->subdev_count;
subdevdata = vpif_obj.config->subdevinfo;
- vpif_obj.sd = kzalloc(sizeof(struct v4l2_subdev *) * subdev_count,
- GFP_KERNEL);
+ vpif_obj.sd = kcalloc(subdev_count, sizeof(*vpif_obj.sd), GFP_KERNEL);
if (vpif_obj.sd == NULL) {
vpif_err("unable to allocate memory for subdevice pointers\n");
err = -ENOMEM;
--
2.10.1

2016-10-12 15:14:48

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 32/34] [media] DaVinci-VPIF-Display: Adjust 11 checks for null pointers

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 15:40:32 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script "checkpatch.pl" pointed information out like the following.

Comparison to NULL could be written …

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpif_display.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 3347fa14..fff1ece 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -271,10 +271,10 @@ static void vpif_stop_streaming(struct vb2_queue *vq)
vb2_buffer_done(&common->cur_frm->vb.vb2_buf,
VB2_BUF_STATE_ERROR);
} else {
- if (common->cur_frm != NULL)
+ if (common->cur_frm)
vb2_buffer_done(&common->cur_frm->vb.vb2_buf,
VB2_BUF_STATE_ERROR);
- if (common->next_frm != NULL)
+ if (common->next_frm)
vb2_buffer_done(&common->next_frm->vb.vb2_buf,
VB2_BUF_STATE_ERROR);
}
@@ -686,7 +686,7 @@ static int vpif_s_std(struct file *file, void *priv, v4l2_std_id std_id)
struct v4l2_output output;
int ret;

- if (config->chan_config[ch->channel_id].outputs == NULL)
+ if (!config->chan_config[ch->channel_id].outputs)
return -ENODATA;

chan_cfg = &config->chan_config[ch->channel_id];
@@ -732,7 +732,7 @@ static int vpif_g_std(struct file *file, void *priv, v4l2_std_id *std)
struct vpif_display_chan_config *chan_cfg;
struct v4l2_output output;

- if (config->chan_config[ch->channel_id].outputs == NULL)
+ if (!config->chan_config[ch->channel_id].outputs)
return -ENODATA;

chan_cfg = &config->chan_config[ch->channel_id];
@@ -783,11 +783,11 @@ vpif_output_to_subdev(struct vpif_display_config *vpif_cfg,

vpif_dbg(2, debug, "vpif_output_to_subdev\n");

- if (chan_cfg->outputs == NULL)
+ if (!chan_cfg->outputs)
return -1;

subdev_name = chan_cfg->outputs[index].subdev_name;
- if (subdev_name == NULL)
+ if (!subdev_name)
return -1;

/* loop through the sub device list to get the sub device info */
@@ -833,7 +833,7 @@ static int vpif_set_output(struct vpif_display_config *vpif_cfg,
}
ch->output_idx = index;
ch->sd = sd;
- if (chan_cfg->outputs != NULL)
+ if (chan_cfg->outputs)
/* update tvnorms from the sub device output info */
ch->video_dev.tvnorms = chan_cfg->outputs[index].output.std;
return 0;
@@ -885,7 +885,7 @@ vpif_enum_dv_timings(struct file *file, void *priv,
struct v4l2_output output;
int ret;

- if (config->chan_config[ch->channel_id].outputs == NULL)
+ if (!config->chan_config[ch->channel_id].outputs)
return -ENODATA;

chan_cfg = &config->chan_config[ch->channel_id];
@@ -922,7 +922,7 @@ static int vpif_s_dv_timings(struct file *file, void *priv,
struct v4l2_output output;
int ret;

- if (config->chan_config[ch->channel_id].outputs == NULL)
+ if (!config->chan_config[ch->channel_id].outputs)
return -ENODATA;

chan_cfg = &config->chan_config[ch->channel_id];
@@ -1021,7 +1021,7 @@ static int vpif_g_dv_timings(struct file *file, void *priv,
struct video_obj *vid_ch = &ch->video;
struct v4l2_output output;

- if (config->chan_config[ch->channel_id].outputs == NULL)
+ if (!config->chan_config[ch->channel_id].outputs)
goto error;

chan_cfg = &config->chan_config[ch->channel_id];
@@ -1280,7 +1280,7 @@ static __init int vpif_probe(struct platform_device *pdev)
subdev_count = vpif_obj.config->subdev_count;
subdevdata = vpif_obj.config->subdevinfo;
vpif_obj.sd = kcalloc(subdev_count, sizeof(*vpif_obj.sd), GFP_KERNEL);
- if (vpif_obj.sd == NULL) {
+ if (!vpif_obj.sd) {
err = -ENOMEM;
goto vpif_unregister;
}
--
2.10.1

2016-10-12 15:13:11

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 31/34] [media] DaVinci-VPIF-Display: Delete an error message for a failed memory allocation

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 15:38:41 +0200

Omit an extra message for a memory allocation failure in this function.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpif_display.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 80121e8..3347fa14 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -1281,7 +1281,6 @@ static __init int vpif_probe(struct platform_device *pdev)
subdevdata = vpif_obj.config->subdevinfo;
vpif_obj.sd = kcalloc(subdev_count, sizeof(*vpif_obj.sd), GFP_KERNEL);
if (vpif_obj.sd == NULL) {
- vpif_err("unable to allocate memory for subdevice pointers\n");
err = -ENOMEM;
goto vpif_unregister;
}
--
2.10.1

2016-10-12 15:15:50

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 29/34] [media] DaVinci-VPIF-Capture: Delete an unnecessary variable initialisation in vpif_channel_isr()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 15:25:08 +0200

The local variable "channel_id" will be set to an appropriate value
a bit later. Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpif_capture.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 4197ca4..d3e2235 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -375,7 +375,7 @@ static irqreturn_t vpif_channel_isr(int irq, void *dev_id)
struct vpif_device *dev = &vpif_obj;
struct common_obj *common;
struct channel_obj *ch;
- int channel_id = 0;
+ int channel_id;
int fid = -1, i;

channel_id = *(int *)(dev_id);
--
2.10.1

2016-10-12 15:16:01

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 34/34] [media] DaVinci-VPIF-Display: Delete an unnecessary variable initialisation in process_progressive_mode()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 15:45:03 +0200

The local variable "addr" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpif_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index cd44990..a48a5111 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -301,7 +301,7 @@ static struct vb2_ops video_qops = {

static void process_progressive_mode(struct common_obj *common)
{
- unsigned long addr = 0;
+ unsigned long addr;

spin_lock(&common->irqlock);
/* Get the next buffer from buffer queue */
--
2.10.1

2016-10-12 15:16:24

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 33/34] [media] DaVinci-VPIF-Display: Delete an unnecessary variable initialisation in vpif_channel_isr()

From: Markus Elfring <[email protected]>
Date: Wed, 12 Oct 2016 15:43:12 +0200

The local variable "channel_id" will be reassigned with the following
statement at the beginning. Thus omit the explicit initialisation.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpif_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index fff1ece..cd44990 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -363,7 +363,7 @@ static irqreturn_t vpif_channel_isr(int irq, void *dev_id)
struct channel_obj *ch;
struct common_obj *common;
int fid = -1, i;
- int channel_id = 0;
+ int channel_id;

channel_id = *(int *)(dev_id);
if (!vpif_intr_status(channel_id + 2))
--
2.10.1

2016-10-23 17:26:42

by Lad, Prabhakar

[permalink] [raw]
Subject: Re: [PATCH 00/34] [media] DaVinci-Video Processing: Fine-tuning for several function implementations

Hello,

Thanks for the patches.

On Wed, Oct 12, 2016 at 3:26 PM, SF Markus Elfring
<[email protected]> wrote:
> From: Markus Elfring <[email protected]>
> Date: Wed, 12 Oct 2016 16:20:02 +0200
>
> Several update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (34):
> Use kmalloc_array() in vpbe_initialize()
> Delete two error messages for a failed memory allocation
> Adjust 16 checks for null pointers
> Combine substrings for four messages
> Return an error code only as a constant in vpbe_probe()
> Return an error code only by a single variable in vpbe_initialize()
> Delete an unnecessary variable initialisation in vpbe_initialize()
> Return the success indication only as a constant in vpbe_set_mode()
> Reduce the scope for a variable in vpbe_set_default_output()
> Check return value of a setup_if_config() call in vpbe_set_output()
> Rename a jump label in vpbe_set_output()
> Delete an unnecessary variable initialisation in vpbe_set_output()
> Capture: Use kmalloc_array() in vpfe_probe()
> Capture: Delete three error messages for a failed memory allocation
> Capture: Improve another size determination in vpfe_probe()
> Capture: Delete an unnecessary variable initialisation in vpfe_probe()
> Capture: Improve another size determination in vpfe_enum_input()
> Capture: Combine substrings for an error message in vpfe_enum_input()
> Capture: Improve another size determination in vpfe_open()
> Capture: Adjust 13 checks for null pointers
> Capture: Delete an unnecessary variable initialisation in 11 functions
> Capture: Move two assignments in vpfe_s_input()
> Capture: Delete unnecessary braces in vpfe_isr()
> Capture: Delete an unnecessary return statement in vpfe_unregister_ccdc_device()
> Capture: Use kcalloc() in vpif_probe()
> Capture: Delete an error message for a failed memory allocation
> Capture: Adjust ten checks for null pointers
> Capture: Delete an unnecessary variable initialisation in vpif_querystd()
> Capture: Delete an unnecessary variable initialisation in vpif_channel_isr()
> Display: Use kcalloc() in vpif_probe()
> Display: Delete an error message for a failed memory allocation
> Display: Adjust 11 checks for null pointers
> Display: Delete an unnecessary variable initialisation in vpif_channel_isr()
> Display: Delete an unnecessary variable initialisation in process_progressive_mode()
>
> drivers/media/platform/davinci/vpbe.c | 93 ++++++++++++---------------
> drivers/media/platform/davinci/vpfe_capture.c | 88 ++++++++++++-------------
> drivers/media/platform/davinci/vpif_capture.c | 28 ++++----
> drivers/media/platform/davinci/vpif_display.c | 30 ++++-----
> 4 files changed, 109 insertions(+), 130 deletions(-)
>

Acked-by: Lad, Prabhakar <[email protected]>

Cheers,
--Prabhakar Lad

2016-11-03 12:20:14

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH 10/34] [media] DaVinci-VPBE: Check return value of a setup_if_config() call in vpbe_set_output()

On 12/10/16 16:47, SF Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> Date: Wed, 12 Oct 2016 09:56:56 +0200
>
> * A function was called over the pointer "setup_if_config" in the data
> structure "venc_platform_data". But the return value was not used so far.
> Thus assign it to the local variable "ret" which will be checked with
> the next statement.
>
> Fixes: 9a7f95ad1c946efdd7a7a72df27db738260a0fd8 ("[media] davinci vpbe: add dm365 VPBE display driver changes")
>
> * Pass a value to this function call without storing it in an intermediate
> variable before.
>
> * Delete the local variable "if_params" which became unnecessary with
> this refactoring.
>
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> drivers/media/platform/davinci/vpbe.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
> index 19611a2..6e7b0df 100644
> --- a/drivers/media/platform/davinci/vpbe.c
> +++ b/drivers/media/platform/davinci/vpbe.c
> @@ -227,7 +227,6 @@ static int vpbe_set_output(struct vpbe_device *vpbe_dev, int index)
> vpbe_current_encoder_info(vpbe_dev);
> struct vpbe_config *cfg = vpbe_dev->cfg;
> struct venc_platform_data *venc_device = vpbe_dev->venc_device;
> - u32 if_params;
> int enc_out_index;
> int sd_index;
> int ret = 0;
> @@ -257,8 +256,8 @@ static int vpbe_set_output(struct vpbe_device *vpbe_dev, int index)
> goto out;
> }
>
> - if_params = cfg->outputs[index].if_params;
> - venc_device->setup_if_config(if_params);
> + ret = venc_device->setup_if_config(cfg
> + ->outputs[index].if_params);

Either keep this as one line or keep the if_params temp variable. This
odd linebreak
is ugly.

Regards,

Hans

> if (ret)
> goto out;
> }
>

2016-11-03 12:22:39

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH 17/34] [media] DaVinci-VPFE-Capture: Improve another size determination in vpfe_enum_input()

On 12/10/16 16:55, SF Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> Date: Wed, 12 Oct 2016 10:33:42 +0200
>
> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer.
>
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> drivers/media/platform/davinci/vpfe_capture.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
> index 8314c39..87ee35d 100644
> --- a/drivers/media/platform/davinci/vpfe_capture.c
> +++ b/drivers/media/platform/davinci/vpfe_capture.c
> @@ -1091,7 +1091,7 @@ static int vpfe_enum_input(struct file *file, void *priv,
> return -EINVAL;
> }
> sdinfo = &vpfe_dev->cfg->sub_devs[subdev];
> - memcpy(inp, &sdinfo->inputs[index], sizeof(struct v4l2_input));
> + memcpy(inp, &sdinfo->inputs[index], sizeof(*inp));

If I am not mistaken this can be written as:

*inp = sdinfo->inputs[index];

Much better.

Regards,

Hans

> return 0;
> }
>
>

2016-11-03 20:54:52

by SF Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH 10/34] [media] DaVinci-VPBE: Check return value of a setup_if_config() call in vpbe_set_output()

>> From: Markus Elfring <[email protected]>
>> Date: Wed, 12 Oct 2016 09:56:56 +0200
>>
>> * A function was called over the pointer "setup_if_config" in the data
>> structure "venc_platform_data". But the return value was not used so far.
>> Thus assign it to the local variable "ret" which will be checked with
>> the next statement.
>>
>> Fixes: 9a7f95ad1c946efdd7a7a72df27db738260a0fd8 ("[media] davinci vpbe: add dm365 VPBE display driver changes")
>>
>> * Pass a value to this function call without storing it in an intermediate
>> variable before.
>>
>> * Delete the local variable "if_params" which became unnecessary with
>> this refactoring.
>>
>> Signed-off-by: Markus Elfring <[email protected]>
>> ---
>> drivers/media/platform/davinci/vpbe.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
>> index 19611a2..6e7b0df 100644
>> --- a/drivers/media/platform/davinci/vpbe.c
>> +++ b/drivers/media/platform/davinci/vpbe.c
>> @@ -227,7 +227,6 @@ static int vpbe_set_output(struct vpbe_device *vpbe_dev, int index)
>> vpbe_current_encoder_info(vpbe_dev);
>> struct vpbe_config *cfg = vpbe_dev->cfg;
>> struct venc_platform_data *venc_device = vpbe_dev->venc_device;
>> - u32 if_params;
>> int enc_out_index;
>> int sd_index;
>> int ret = 0;
>> @@ -257,8 +256,8 @@ static int vpbe_set_output(struct vpbe_device *vpbe_dev, int index)
>> goto out;
>> }
>>
>> - if_params = cfg->outputs[index].if_params;
>> - venc_device->setup_if_config(if_params);
>> + ret = venc_device->setup_if_config(cfg
>> + ->outputs[index].if_params);
>
> Either keep this as one line

Will you tolerate a line length of 82 characters then?


> or keep the if_params temp variable.

My proposal was to get rid of it.


> This odd linebreak is ugly.

I am curious on how the desired changes can be integrated after a couple of update
suggestions were accepted from this patch series.

Regards,
Markus

2016-11-03 21:06:13

by SF Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH 17/34] [media] DaVinci-VPFE-Capture: Improve another size determination in vpfe_enum_input()

>> @@ -1091,7 +1091,7 @@ static int vpfe_enum_input(struct file *file, void *priv,
>> return -EINVAL;
>> }
>> sdinfo = &vpfe_dev->cfg->sub_devs[subdev];
>> - memcpy(inp, &sdinfo->inputs[index], sizeof(struct v4l2_input));
>> + memcpy(inp, &sdinfo->inputs[index], sizeof(*inp));
>
> If I am not mistaken this can be written as:
>
> *inp = sdinfo->inputs[index];
>
> Much better.

At which position would you like to integrate a second approach for such a change
from this patch series?

* Do you expect me to send a "V2" for the whole series?

* Will an update step be appropriate if I would rebase it on other
recently accepted suggestions?

Regards,
Markus

2016-11-04 08:06:10

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH 17/34] [media] DaVinci-VPFE-Capture: Improve another size determination in vpfe_enum_input()

On 03/11/16 22:05, SF Markus Elfring wrote:
>>> @@ -1091,7 +1091,7 @@ static int vpfe_enum_input(struct file *file, void *priv,
>>> return -EINVAL;
>>> }
>>> sdinfo = &vpfe_dev->cfg->sub_devs[subdev];
>>> - memcpy(inp, &sdinfo->inputs[index], sizeof(struct v4l2_input));
>>> + memcpy(inp, &sdinfo->inputs[index], sizeof(*inp));
>>
>> If I am not mistaken this can be written as:
>>
>> *inp = sdinfo->inputs[index];
>>
>> Much better.
>
> At which position would you like to integrate a second approach for such a change
> from this patch series?
>
> * Do you expect me to send a "V2" for the whole series?

No, just for the patches I commented upon.

>
> * Will an update step be appropriate if I would rebase it on other
> recently accepted suggestions?

You can base it on
https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=for-v4.10a

That branch has all your other patches of this series merged and is part
of a pull
request I posted yesterday.

Hans

>
> Regards,
> Markus
>

2016-11-04 08:07:04

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH 10/34] [media] DaVinci-VPBE: Check return value of a setup_if_config() call in vpbe_set_output()

On 03/11/16 21:54, SF Markus Elfring wrote:
>>> From: Markus Elfring <[email protected]>
>>> Date: Wed, 12 Oct 2016 09:56:56 +0200
>>>
>>> * A function was called over the pointer "setup_if_config" in the data
>>> structure "venc_platform_data". But the return value was not used so far.
>>> Thus assign it to the local variable "ret" which will be checked with
>>> the next statement.
>>>
>>> Fixes: 9a7f95ad1c946efdd7a7a72df27db738260a0fd8 ("[media] davinci vpbe: add dm365 VPBE display driver changes")
>>>
>>> * Pass a value to this function call without storing it in an intermediate
>>> variable before.
>>>
>>> * Delete the local variable "if_params" which became unnecessary with
>>> this refactoring.
>>>
>>> Signed-off-by: Markus Elfring <[email protected]>
>>> ---
>>> drivers/media/platform/davinci/vpbe.c | 5 ++---
>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
>>> index 19611a2..6e7b0df 100644
>>> --- a/drivers/media/platform/davinci/vpbe.c
>>> +++ b/drivers/media/platform/davinci/vpbe.c
>>> @@ -227,7 +227,6 @@ static int vpbe_set_output(struct vpbe_device *vpbe_dev, int index)
>>> vpbe_current_encoder_info(vpbe_dev);
>>> struct vpbe_config *cfg = vpbe_dev->cfg;
>>> struct venc_platform_data *venc_device = vpbe_dev->venc_device;
>>> - u32 if_params;
>>> int enc_out_index;
>>> int sd_index;
>>> int ret = 0;
>>> @@ -257,8 +256,8 @@ static int vpbe_set_output(struct vpbe_device *vpbe_dev, int index)
>>> goto out;
>>> }
>>>
>>> - if_params = cfg->outputs[index].if_params;
>>> - venc_device->setup_if_config(if_params);
>>> + ret = venc_device->setup_if_config(cfg
>>> + ->outputs[index].if_params);
>>
>> Either keep this as one line
>
> Will you tolerate a line length of 82 characters then?

Yes, if it improves readability, which it does.

>
>
>> or keep the if_params temp variable.
>
> My proposal was to get rid of it.
>
>
>> This odd linebreak is ugly.
>
> I am curious on how the desired changes can be integrated after a couple of update
> suggestions were accepted from this patch series.

See my previous reply to 17/34.

Regards,

Hans

2016-11-06 20:01:05

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH v2 10/34] [media] DaVinci-VPBE: Check return value of a setup_if_config() call in vpbe_set_output()

From: Markus Elfring <[email protected]>
Date: Sun, 6 Nov 2016 20:40:20 +0100

* A function was called over the pointer "setup_if_config" in the data
structure "venc_platform_data". But the return value was not used so far.
Thus assign it to the local variable "ret" which will be checked with
the next statement.

Fixes: 9a7f95ad1c946efdd7a7a72df27db738260a0fd8 ("[media] davinci vpbe: add dm365 VPBE display driver changes")

* Pass a value to this function call without storing it in an intermediate
variable before.

* Delete the local variable "if_params" which became unnecessary with
this refactoring.

Acked-by: Lad, Prabhakar <[email protected]>
Reviewed-by: Hans Verkuil <[email protected]>
Signed-off-by: Markus Elfring <[email protected]>
---

v2: Keep the assignment statement on one line despite of its length
of 82 characters.

drivers/media/platform/davinci/vpbe.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
index 19611a2..d04d6b7 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -227,7 +227,6 @@ static int vpbe_set_output(struct vpbe_device *vpbe_dev, int index)
vpbe_current_encoder_info(vpbe_dev);
struct vpbe_config *cfg = vpbe_dev->cfg;
struct venc_platform_data *venc_device = vpbe_dev->venc_device;
- u32 if_params;
int enc_out_index;
int sd_index;
int ret = 0;
@@ -257,5 +257,4 @@ static int vpbe_set_output(struct vpbe_device *vpbe_dev, int index)
}

- if_params = cfg->outputs[index].if_params;
- venc_device->setup_if_config(if_params);
+ ret = venc_device->setup_if_config(cfg->outputs[index].if_params);
if (ret)
--
2.10.2

2016-11-06 21:01:13

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH v2 17/34] [media] DaVinci-VPFE-Capture: Replace a memcpy() call by an assignment in vpfe_enum_input()

From: Markus Elfring <[email protected]>
Date: Sun, 6 Nov 2016 21:54:38 +0100

Use a direct assignment for an array element which can be set over the
pointer variable "inp" instead of calling the function "memcpy" here.

Suggested-by: Hans Verkuil <[email protected]>
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/media/platform/davinci/vpfe_capture.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
index 8314c39..5417f6b 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -1091,7 +1091,7 @@ static int vpfe_enum_input(struct file *file, void *priv,
return -EINVAL;
}
sdinfo = &vpfe_dev->cfg->sub_devs[subdev];
- memcpy(inp, &sdinfo->inputs[index], sizeof(struct v4l2_input));
+ *inp = sdinfo->inputs[index];
return 0;
}

--
2.10.2

2016-11-16 14:31:02

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH 06/34] [media] DaVinci-VPBE: Return an error code only by a single variable in vpbe_initialize()

Em Wed, 12 Oct 2016 16:40:22 +0200
SF Markus Elfring <[email protected]> escreveu:

> From: Markus Elfring <[email protected]>
> Date: Tue, 11 Oct 2016 14:15:57 +0200
>
> An error code was assigned to the local variable "err" in an if branch.
> But this variable was not used further then.
>
> Use the local variable "ret" instead like at other places in this function.
>
> Fixes: 66715cdc3224a4e241c1a92856b9a4af3b70e06d ("[media] davinci vpbe:
> VPBE display driver")
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> drivers/media/platform/davinci/vpbe.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
> index 4c4cd81..afa8ff7 100644
> --- a/drivers/media/platform/davinci/vpbe.c
> +++ b/drivers/media/platform/davinci/vpbe.c
> @@ -665,7 +665,7 @@ static int vpbe_initialize(struct device *dev, struct vpbe_device *vpbe_dev)
> if (err) {
> v4l2_err(&vpbe_dev->v4l2_dev,
> "unable to initialize the OSD device");
> - err = -ENOMEM;
> + ret = -ENOMEM;
> goto fail_dev_unregister;
> }
> }

Hmm... why are you keeping both "err" and "ret" variables here?
Just one var is needed. Also, why not just return the error code?

If you want to cleanup the code, please look at the entire function,
and not to just this occurrence.

I mean, IMHO, this code (and all similar occurrences), should be, instead:

ret = osd_device->ops.initialize(osd_device);
if (ret) {
v4l2_err(&vpbe_dev->v4l2_dev,
"unable to initialize the OSD device");
goto fail_dev_unregister;
}

and the "err" var can probably be removed.


Thanks,
Mauro

2016-11-16 14:35:45

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH 07/34] [media] DaVinci-VPBE: Delete an unnecessary variable initialisation in vpbe_initialize()

Em Wed, 12 Oct 2016 16:42:31 +0200
SF Markus Elfring <[email protected]> escreveu:

> From: Markus Elfring <[email protected]>
> Date: Wed, 12 Oct 2016 09:45:39 +0200
>
> The local variable "ret" will be set to an appropriate value a bit later.
> Thus omit the explicit initialisation at the beginning.
>
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> drivers/media/platform/davinci/vpbe.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
> index afa8ff7..9fdd8c0 100644
> --- a/drivers/media/platform/davinci/vpbe.c
> +++ b/drivers/media/platform/davinci/vpbe.c
> @@ -597,7 +597,7 @@ static int vpbe_initialize(struct device *dev, struct vpbe_device *vpbe_dev)
> struct osd_state *osd_device;
> struct i2c_adapter *i2c_adap;
> int num_encoders;
> - int ret = 0;
> + int ret;
> int err;

Please fold this change to the patch where you'll be addressing the
issues with "err" var, as per my previous email.




Thanks,
Mauro