Re: [PATCH][next] staging: media: atomisp: fix memory leak of object flash
Em Wed, 2 Sep 2020 21:15:31 +0300 Andy Shevchenko escreveu: > On Wed, Sep 2, 2020 at 8:02 PM Colin King wrote: > > > > From: Colin Ian King > > > > In the case where the call to lm3554_platform_data_func returns an > > error there is a memory leak on the error return path of object > > flash. Fix this by adding an error return path that will free > > flash and rename labels fail2 to fail3 and fail1 to fail2. > > > > Wouldn't be proper fix to move to devm_kmalloc() and return > dev_err_probe() where appropriate? Actually, we prefer not using devm_*() at media subsystem. Once we started migrating alloc stuff to use it. We end needing to revert those, as it caused side effects related to lifecycle management: some object were de-allocating too late. Others with multiple interfaces (USB, pci) had even worse troubles. Thanks, Mauro ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH][next] staging: media: atomisp: fix memory leak of object flash
On Wed, Sep 02, 2020 at 05:58:52PM +0100, Colin King wrote:
> From: Colin Ian King
>
> In the case where the call to lm3554_platform_data_func returns an
> error there is a memory leak on the error return path of object
> flash. Fix this by adding an error return path that will free
> flash and rename labels fail2 to fail3 and fail1 to fail2.
Colin, I know you know this and I don't want to explain things which you
already know but this for the other people in Kernel Janitors.
The error handling in this function is still pretty messed up. Why
does it "goto fail2" if media_entity_pads_init() fails? There is no
clean up if atomisp_register_i2c_module() fails.
It's just better to re-write it using the "free the most recent
allocation" system. The key to the system is if the last allocation
was "flash" then the goto should be something like "goto free_flash;"
so that we know it does the right thing.
One of the advantages of the this system is that it basically writes the
->remove() for you. All we have to do is add one more line to free the
final allocation from the probe function. In this driver the lm3554_remove()
has a few things which aren't cleaned up in the probe error handling so
it doesn't seem right. For example, we need to delete the timer.
834 static int lm3554_probe(struct i2c_client *client)
835 {
836 int err = 0;
837 struct lm3554 *flash;
838 unsigned int i;
839 int ret;
We have both "ret" and "err". It causes bugs where ever "ret" is used
below. Let's delete "err".
840
841 flash = kzalloc(sizeof(*flash), GFP_KERNEL);
842 if (!flash)
843 return -ENOMEM;
"flash" is allocated.
844
845 flash->pdata = lm3554_platform_data_func(client);
846 if (IS_ERR(flash->pdata))
847 return PTR_ERR(flash->pdata);
if (IS_ERR(flash->pdata)) {
ret = PTR_ERR(flash->pdata);
goto free_flash;
}
The lm3554_platform_data_func() function doesn't allocate anything so
"flash" is still the most recent allocation.
848
849 v4l2_i2c_subdev_init(&flash->sd, client, &lm3554_ops);
^
I think this needs to be unwound with the v4l2_device_unregister_subdev()
function. I'm not totally sure. But that's how the existing code works
so let's keep it as-is. Meaning that "subdev" is the most recent
allocation.
850 flash->sd.internal_ops = &lm3554_internal_ops;
851 flash->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
852 flash->mode = ATOMISP_FLASH_MODE_OFF;
853 flash->timeout = LM3554_MAX_TIMEOUT / LM3554_TIMEOUT_STEPSIZE -
1;
854 ret =
855 v4l2_ctrl_handler_init(&flash->ctrl_handler,
856 ARRAY_SIZE(lm3554_controls));
857 if (ret) {
858 dev_err(&client->dev, "error initialize a
ctrl_handler.\n");
859 goto fail2;
860 }
This becomes "goto unregister_subdev;". In the original code the goto
fail2 freed the handler, which is harmless but unnecessary.
"flash->ctrl_handler" is now the most recent allocated.
861
862 for (i = 0; i < ARRAY_SIZE(lm3554_controls); i++)
863 v4l2_ctrl_new_custom(&flash->ctrl_handler,
&lm3554_controls[i],
864 NULL);
865
866 if (flash->ctrl_handler.error) {
867 dev_err(&client->dev, "ctrl_handler error.\n");
868 goto fail2;
Missing error code.
if (flash->ctrl_handler.error) {
dev_err(&client->dev, "ctrl_handler error.\n");
ret = flash->ctrl_handler.error;
goto free_handler;
}
I don't think the v4l2_ctrl_new_custom() needs to be unwound so
"flash->ctrl_handler" is still the most recent allocation.
869 }
870
871 flash->sd.ctrl_handler = &flash->ctrl_handler;
872 err = media_entity_pads_init(&flash->sd.entity, 0, NULL);
873 if (err) {
874 dev_err(&client->dev, "error initialize a media
entity.\n");
875 goto fail1;
876 }
This goto leaks handler. I suspect the reason is that the original
coder didn't want to call media_entity_cleanup() if media_entity_pads_init()
failed. The media_entity_cleanup() function doesn't do anything. We
added it as stub function in 2009 but have was never used it. The
comments say "It must be called during the cleanup phase after
unregistering the entity and before freeing it." We haven't
unregistered anything here but we are freeing something. ¯\_(ツ)_/¯
Anyway calling media_entity_cleanup() is harmless:
goto free_handler;
877
878 flash->sd.entity.function = MEDIA_ENT_F_FLASH;
879
Re: [PATCH] staging: media: atomisp: Fix error path in lm3554_probe()
On Wed, Sep 02, 2020 at 07:41:50PM +0100, Alex Dewar wrote:
> The error path for lm3554_probe() contains a number of bugs, including:
> * resource leaks
> * jumping to error labels out of sequence
> * not setting the return value appropriately
>
> Fix it up and give the labels more memorable names.
>
> This issue has existed since the code was originally contributed in
> commit a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2"),
> although the code was subsequently removed altogether and then
> reinstated with commit ad85094b293e ("Revert "media: staging: atomisp: Remove
> driver"").
>
Almost perfect! Just a couple other leaks we should fix as well.
> + ret = media_entity_pads_init(&flash->sd.entity, 0, NULL);
> + if (ret) {
> + dev_err(&client->dev, "error initializing media entity");
> + goto err_free_ctrl_handler;
> }
>
> flash->sd.entity.function = MEDIA_ENT_F_FLASH;
> @@ -881,20 +882,22 @@ static int lm3554_probe(struct i2c_client *client)
>
> timer_setup(&flash->flash_off_delay, lm3554_flash_off_delay, 0);
We need to delete this timer.
>
> - err = lm3554_gpio_init(client);
> - if (err) {
> + ret = lm3554_gpio_init(client);
> + if (ret) {
> dev_err(&client->dev, "gpio request/direction_output fail");
> - goto fail2;
> + goto err_cleanup_entity;
> }
> return atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH);
If atomisp_register_i2c_module() fails then we need to call
lm3554_gpio_uninit(client) and do other cleanup.
> -fail2:
> +
> +err_cleanup_entity:
> media_entity_cleanup(&flash->sd.entity);
> +err_free_ctrl_handler:
> v4l2_ctrl_handler_free(&flash->ctrl_handler);
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/5] address W=1 warnings at staging/media/atomisp
The linux-media policy is to have zero warnings with W=1. However, when I started using a Jenkins instance at https://builder.linuxtv.org to automate such tests, I didn't notice that a bug at the scripts were just ignoring warnings. Now that this is fixed, we need to get rid of the warnings that got re-introduced when the atomisp driver was reverted, as otherwise, every time a common header is touched, we'll see messages like those: Error/warnings: patches/0001-media-mxl5xx-remove-unused-including-linux-version.h.patch: allyesconfig: return code #0: ../drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:164:2: warning: function ‘atomisp_css2_dbg_print’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format] ../drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:170:2: warning: function ‘atomisp_css2_dbg_ftrace_print’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format] ../drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:170:2: warning: function ‘atomisp_css2_dbg_ftrace_print’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format] ../drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:176:2: warning: function ‘atomisp_css2_err_print’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format] ../drivers/staging/media/atomisp/pci/atomisp_cmd.c:2814:31: warning: variable ‘stream_config’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/atomisp_cmd.c:2893:31: warning: variable ‘stream_config’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/atomisp_v4l2.c:1432:15: warning: variable ‘a0_max_id’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css_mipi.c:410:27: warning: variable ‘mipi_intermediate_info’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css.c:1366:24: warning: variable ‘stream’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css.c:2702:22: warning: variable ‘capture_pipe’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css.c:2831:7: warning: variable ‘continuous’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css.c:3534:15: warning: variable ‘num_output_pins’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css.c:3937:55: warning: variable ‘vf_pp_binary’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css.c:3937:38: warning: variable ‘preview_binary’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css.c:3937:24: warning: variable ‘copy_binary’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css.c:3936:26: warning: variable ‘me’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css.c:5749:24: warning: variable ‘copy_binary’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css.c:6013:7: warning: variable ‘continuous’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css.c:6012:7: warning: variable ‘memory’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css.c:7329:24: warning: variable ‘copy_binary’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css.c:7459:26: warning: variable ‘num_vf_pp_stage’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css.c:10420:22: warning: variable ‘pipe_id’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css_param_shading.c:239:4: warning: variable ‘padded_width’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/sh_css_params.c:1099:36: warning: variable ‘row_padding’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/isp/kernels/dvs/dvs_1.0/ia_css_dvs.host.c:237:31: warning: variable ‘isp_data_ptr’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c:119:28: warning: variable ‘ver_num_isp’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c:119:15: warning: variable ‘hor_num_isp’ set but not used [-Wunused-but-set-variable] ../drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c:127:6: warning: variable ‘err’ set but not used [-Wunused-but-set-variable] In file included from ../drivers/staging/med
[PATCH 1/5] media: atomisp: get rid of some cleanup leftovers
The code that used to check for ISP2400/2401 version were
using the revision number in order to decide wheather
version should be used. With the new version, the code
doesn't need to check anymore for ISP version.
Fixes: ca133c395f2f ("media: atomisp: improve device detection code")
Signed-off-by: Mauro Carvalho Chehab
---
drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 8c1d5a3b8f45..304857bffc6a 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -1429,7 +1429,6 @@ atomisp_load_firmware(struct atomisp_device *isp)
*/
static bool is_valid_device(struct pci_dev *pdev, const struct pci_device_id
*id)
{
- unsigned int a0_max_id = 0;
const char *name;
const char *product;
@@ -1437,11 +1436,9 @@ static bool is_valid_device(struct pci_dev *pdev, const
struct pci_device_id *id
switch (id->device & ATOMISP_PCI_DEVICE_SOC_MASK) {
case ATOMISP_PCI_DEVICE_SOC_MRFLD:
- a0_max_id = ATOMISP_PCI_REV_MRFLD_A0_MAX;
name = "Merrifield";
break;
case ATOMISP_PCI_DEVICE_SOC_BYT:
- a0_max_id = ATOMISP_PCI_REV_BYT_A0_MAX;
name = "Baytrail";
break;
case ATOMISP_PCI_DEVICE_SOC_ANN:
--
2.26.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/5] media: atomisp: print a warning if error while setting downscaler
As warned by gcc: drivers/staging/media/atomisp//pci/isp/kernels/dvs/dvs_1.0/ia_css_dvs.host.c:237:31: warning: variable ‘isp_data_ptr’ set but not used [-Wunused-but-set-variable] ia_css_vf_configure() logic has an error var that detects troubles when setting the kernel for downscaling. The driver just ignores it, without producing any warning. Add at least a warning message on such cases. Signed-off-by: Mauro Carvalho Chehab --- .../media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c index 3b850bb2d39d..dd3670972936 100644 --- a/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c +++ b/drivers/staging/media/atomisp/pci/isp/kernels/vf/vf_1.0/ia_css_vf.host.c @@ -13,6 +13,8 @@ * more details. */ +#include "atomisp_internal.h" + #include "ia_css_vf.host.h" #include #include @@ -129,6 +131,9 @@ ia_css_vf_configure( const struct ia_css_binary_info *info = &binary->info->sp; err = configure_kernel(info, out_info, vf_info, downscale_log2, &config); + if (err) + dev_warn(atomisp_dev, "Couldn't setup downscale\n"); + configure_dma(&config, vf_info); if (vf_info) -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/5] media: atomisp: get rid of -Wsuggest-attribute=format warnings
There are some warnings reported by gcc:
drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:164:2:
warning: function ‘atomisp_css2_dbg_print’ might be a candidate for
‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:170:2:
warning: function ‘atomisp_css2_dbg_ftrace_print’ might be a candidate for
‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:170:2:
warning: function ‘atomisp_css2_dbg_ftrace_print’ might be a candidate for
‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:176:2:
warning: function ‘atomisp_css2_err_print’ might be a candidate for
‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
That are due to the usage of printf-like messages without
enabling the error checking logic.
Add the proper attributes in order to shut up such warnings.
Signed-off-by: Mauro Carvalho Chehab
---
.../media/atomisp/pci/atomisp_compat_css20.c | 20 ---
.../staging/media/atomisp/pci/ia_css_env.h| 4 ++--
2 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index e54242dc0888..5a730e17cc6e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -159,24 +159,13 @@ static void atomisp_css2_hw_load(hrt_address addr, void
*to, uint32_t n)
spin_unlock_irqrestore(&mmio_lock, flags);
}
-static int atomisp_css2_dbg_print(const char *fmt, va_list args)
-{
- vprintk(fmt, args);
- return 0;
-}
-
-static int atomisp_css2_dbg_ftrace_print(const char *fmt, va_list args)
+static int __attribute__((format (printf, 1, 0)))
+atomisp_css2_dbg_ftrace_print(const char *fmt, va_list args)
{
ftrace_vprintk(fmt, args);
return 0;
}
-static int atomisp_css2_err_print(const char *fmt, va_list args)
-{
- vprintk(fmt, args);
- return 0;
-}
-
void atomisp_load_uint32(hrt_address addr, uint32_t *data)
{
*data = atomisp_css2_hw_load_32(addr);
@@ -869,8 +858,7 @@ static inline int __set_css_print_env(struct atomisp_device
*isp, int opt)
isp->css_env.isp_css_env.print_env.debug_print =
atomisp_css2_dbg_ftrace_print;
else if (opt == 2)
- isp->css_env.isp_css_env.print_env.debug_print =
- atomisp_css2_dbg_print;
+ isp->css_env.isp_css_env.print_env.debug_print = vprintk;
else
ret = -EINVAL;
@@ -903,7 +891,7 @@ int atomisp_css_load_firmware(struct atomisp_device *isp)
__set_css_print_env(isp, dbg_func);
- isp->css_env.isp_css_env.print_env.error_print = atomisp_css2_err_print;
+ isp->css_env.isp_css_env.print_env.error_print = vprintk;
/* load isp fw into ISP memory */
err = ia_css_load_firmware(isp->dev, &isp->css_env.isp_css_env,
diff --git a/drivers/staging/media/atomisp/pci/ia_css_env.h
b/drivers/staging/media/atomisp/pci/ia_css_env.h
index 8debf334c15c..9808ff9e0492 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_env.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_env.h
@@ -75,9 +75,9 @@ struct ia_css_hw_access_env {
/* Environment with function pointers to print error and debug messages.
*/
struct ia_css_print_env {
- int (*debug_print)(const char *fmt, va_list args);
+ int (*debug_print)(const char *fmt, va_list args) __attribute__((format
(printf, 1, 0)));
/** Print a debug message. */
- int (*error_print)(const char *fmt, va_list args);
+ int (*error_print)(const char *fmt, va_list args) __attribute__((format
(printf, 1, 0)));
/** Print an error message.*/
};
--
2.26.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/5] media: atomisp: move a static constant out of a header file
Gcc reports that input_formatter_alignment is not used:
In file included from
./drivers/staging/media/atomisp//pci/hive_isp_css_include/input_formatter.h:34,
from
drivers/staging/media/atomisp//pci/runtime/debug/src/ia_css_debug.c:55:
./drivers/staging/media/atomisp//pci/hive_isp_css_common/host/input_formatter_local.h:118:27:
warning: ‘input_formatter_alignment’ defined but not used
[-Wunused-const-variable=]
However, it is, but only inside input_formatter.c.
So, move it out of the header file.
Signed-off-by: Mauro Carvalho Chehab
---
.../atomisp/pci/hive_isp_css_common/host/input_formatter.c| 4
.../pci/hive_isp_css_common/host/input_formatter_local.h | 4
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git
a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_formatter.c
b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_formatter.c
index 1b196cd265b9..5cd6136f21a2 100644
---
a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_formatter.c
+++
b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_formatter.c
@@ -27,6 +27,10 @@
#include "input_formatter_private.h"
#endif /* __INLINE_INPUT_FORMATTER__ */
+static const unsigned int input_formatter_alignment[N_INPUT_FORMATTER_ID] = {
+ ISP_VEC_ALIGN, ISP_VEC_ALIGN, HIVE_ISP_CTRL_DATA_BYTES
+};
+
const hrt_address HIVE_IF_SRST_ADDRESS[N_INPUT_FORMATTER_ID] = {
INPUT_FORMATTER0_SRST_OFFSET,
INPUT_FORMATTER1_SRST_OFFSET,
diff --git
a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_formatter_local.h
b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_formatter_local.h
index 94fff77584f7..dfb593c109af 100644
---
a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_formatter_local.h
+++
b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_formatter_local.h
@@ -115,8 +115,4 @@ struct input_formatter_bin_state_s {
u32 en_status_update;
};
-static const unsigned int input_formatter_alignment[N_INPUT_FORMATTER_ID] = {
- ISP_VEC_ALIGN, ISP_VEC_ALIGN, HIVE_ISP_CTRL_DATA_BYTES
-};
-
#endif /* __INPUT_FORMATTER_LOCAL_H_INCLUDED__ */
--
2.26.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/5] media: atomisp: get rid of unused vars
There are several vars declared and set but unused:
drivers/staging/media/atomisp//pci/sh_css_param_shading.c:239:4:
warning: variable ‘padded_width’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:1366:24: warning: variable
‘stream’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:2702:22: warning: variable
‘capture_pipe’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:2831:7: warning: variable
‘continuous’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:3534:15: warning: variable
‘num_output_pins’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:3937:55: warning: variable
‘vf_pp_binary’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:3937:38: warning: variable
‘preview_binary’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:3937:24: warning: variable
‘copy_binary’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:3936:26: warning: variable
‘me’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:5749:24: warning: variable
‘copy_binary’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:6013:7: warning: variable
‘continuous’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:6012:7: warning: variable
‘memory’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:7329:24: warning: variable
‘copy_binary’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:7459:26: warning: variable
‘num_vf_pp_stage’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css.c:10420:22: warning: variable
‘pipe_id’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css_params.c:1099:36: warning:
variable ‘row_padding’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/sh_css_mipi.c:410:27: warning:
variable ‘mipi_intermediate_info’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/atomisp_cmd.c:2814:31: warning:
variable ‘stream_config’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/atomisp_cmd.c:2893:31: warning:
variable ‘stream_config’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/isp/kernels/dvs/dvs_1.0/ia_css_dvs.host.c:237:31:
warning: variable ‘isp_data_ptr’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c:119:28:
warning: variable ‘ver_num_isp’ set but not used [-Wunused-but-set-variable]
drivers/staging/media/atomisp//pci/isp/kernels/sdis/sdis_2/ia_css_sdis2.host.c:119:15:
warning: variable ‘hor_num_isp’ set but not used [-Wunused-but-set-variable]
Signed-off-by: Mauro Carvalho Chehab
---
.../staging/media/atomisp/pci/atomisp_cmd.c | 6 ---
.../isp/kernels/dvs/dvs_1.0/ia_css_dvs.host.c | 3 --
.../kernels/sdis/sdis_2/ia_css_sdis2.host.c | 3 --
.../atomisp/pci/runtime/rmgr/src/rmgr_vbuf.c | 4 +-
drivers/staging/media/atomisp/pci/sh_css.c| 47 ++-
.../staging/media/atomisp/pci/sh_css_mipi.c | 12 -
.../media/atomisp/pci/sh_css_param_shading.c | 25 +++---
.../staging/media/atomisp/pci/sh_css_params.c | 3 +-
8 files changed, 14 insertions(+), 89 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 391c4acbd5b2..a1772bbc3042 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -2811,7 +2811,6 @@ int atomisp_get_metadata(struct atomisp_sub_device *asd,
int flag,
struct atomisp_metadata *md)
{
struct atomisp_device *isp = asd->isp;
- struct ia_css_stream_config *stream_config;
struct ia_css_stream_info *stream_info;
struct camera_mipi_info *mipi_info;
struct atomisp_metadata_buf *md_buf;
@@ -2821,8 +2820,6 @@ int atomisp_get_metadata(struct atomisp_sub_device *asd,
int flag,
if (flag != 0)
return -EINVAL;
- stream_config = &asd->stream_env[ATOMISP_INPUT_STREAM_GENERAL].
- stream_config;
stream_info = &asd->stream_env[ATOMISP_INPUT_STREAM_GENERAL].
stream_info;
@@ -2890,7 +2887,6 @@ int atomisp_get_metadata_by_type(struct
atomisp_sub_device *asd, int flag,
struct atomisp_metadata_with_type *md)
{
INVESTMENT FUNDING:
Hello dear how are you doing? I am Ms. Melvida, am 56 years old from France.I
have a proposal to share with you please kindly get back to me for details via
my email {melvidabullo...@gmail.com}.
thank you as i await your response.
Best Regards,
Melvida.
Email:melvdabullo...@gmail.com
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] media: atomisp: get rid of -Wsuggest-attribute=format warnings
On Thu, Sep 03, 2020 at 03:57:32PM +0200, Mauro Carvalho Chehab wrote:
> There are some warnings reported by gcc:
> drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:164:2:
> warning: function ‘atomisp_css2_dbg_print’ might be a candidate for
> ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
> drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:170:2:
> warning: function ‘atomisp_css2_dbg_ftrace_print’ might be a candidate for
> ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
> drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:170:2:
> warning: function ‘atomisp_css2_dbg_ftrace_print’ might be a candidate for
> ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
> drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:176:2:
> warning: function ‘atomisp_css2_err_print’ might be a candidate for
> ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
>
> That are due to the usage of printf-like messages without
> enabling the error checking logic.
>
> Add the proper attributes in order to shut up such warnings.
> +static int __attribute__((format (printf, 1, 0)))
> +atomisp_css2_dbg_ftrace_print(const char *fmt, va_list args)
> {
> ftrace_vprintk(fmt, args);
> return 0;
> }
>
Why not to drop it completely as well?
> -static int atomisp_css2_err_print(const char *fmt, va_list args)
> -{
> - vprintk(fmt, args);
> - return 0;
> -}
--
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] media: atomisp: get rid of -Wsuggest-attribute=format warnings
Em Thu, 3 Sep 2020 17:07:24 +0300
Andy Shevchenko escreveu:
> On Thu, Sep 03, 2020 at 03:57:32PM +0200, Mauro Carvalho Chehab wrote:
> > There are some warnings reported by gcc:
> > drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:164:2:
> > warning: function ‘atomisp_css2_dbg_print’ might be a candidate for
> > ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
> > drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:170:2:
> > warning: function ‘atomisp_css2_dbg_ftrace_print’ might be a candidate for
> > ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
> > drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:170:2:
> > warning: function ‘atomisp_css2_dbg_ftrace_print’ might be a candidate for
> > ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
> > drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:176:2:
> > warning: function ‘atomisp_css2_err_print’ might be a candidate for
> > ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
> >
> > That are due to the usage of printf-like messages without
> > enabling the error checking logic.
> >
> > Add the proper attributes in order to shut up such warnings.
>
> > +static int __attribute__((format (printf, 1, 0)))
> > +atomisp_css2_dbg_ftrace_print(const char *fmt, va_list args)
> > {
> > ftrace_vprintk(fmt, args);
> > return 0;
> > }
> >
>
> Why not to drop it completely as well?
Because of this:
make -s CC="gcc -fno-diagnostics-show-caret" CF=-D__CHECK_ENDIAN__
CONFIG_DEBUG_SECTION_MISMATCH=y W=1 M=drivers/staging/media/atomisp/ 2>&1|grep
-v ": In function ‘"
drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:858:52:
error: ‘ftrace_vprintk’ undeclared (first use in this function); did you mean
‘tracepoint’?
drivers/staging/media/atomisp//pci/atomisp_compat_css20.c:858:52: note:
each undeclared identifier is reported only once for each function it appears in
Basically, kernel.h defines it as a macro:
#define ftrace_vprintk(fmt, vargs)
\
do {
\
if (__builtin_constant_p(fmt)) {
\
static const char *trace_printk_fmt __used
\
__attribute__((section("__trace_printk_fmt"))) =
\
__builtin_constant_p(fmt) ? fmt : NULL;
\
\
__ftrace_vbprintk(_THIS_IP_, trace_printk_fmt, vargs);
\
} else
\
__ftrace_vprintk(_THIS_IP_, fmt, vargs);
\
} while (0)
Now, a different thing would be to get rid of using trace as a debug
mechanism. Right now, I don't have any strong opinion, but I guess
that, while this driver is still at staging, it sounds good to be
able of using traces instead of dmesg for debugging purposes, but
to be honest, I didn't test yet to use ftrace for such purpose.
Thanks,
Mauro
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: media: atomisp: Fix error path in lm3554_probe()
Good point about the timer!
> >
> > - err = lm3554_gpio_init(client);
> > - if (err) {
> > + ret = lm3554_gpio_init(client);
> > + if (ret) {
> > dev_err(&client->dev, "gpio request/direction_output fail");
> > - goto fail2;
> > + goto err_cleanup_entity;
> > }
> > return atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH);
>
> If atomisp_register_i2c_module() fails then we need to call
> lm3554_gpio_uninit(client) and do other cleanup.
I'm probably showing my ignorance here, but I can't see what cleanup we
need. Inside lm3554_gpio_init we have:
ret = gpiod_direction_output(pdata->gpio_reset, 0);
if (ret < 0)
return ret;
dev_info(&client->dev, "flash led reset successfully\n");
if (!pdata->gpio_strobe)
return -EINVAL;
ret = gpiod_direction_output(pdata->gpio_strobe, 0);
if (ret < 0)
return ret;
I'm not sure how you "undo" a call to gpiod_direction_output, but I'm
thinking you won't need to do anything because it should be ok to leave
a gpio to output 0... right?
Alex
>
> > -fail2:
> > +
> > +err_cleanup_entity:
> > media_entity_cleanup(&flash->sd.entity);
> > +err_free_ctrl_handler:
> > v4l2_ctrl_handler_free(&flash->ctrl_handler);
>
> regards,
> dan carpenter
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
FYI, I've moved it off for-next and into its own dma-ranges branch for now until we sort out the regressions. I still hope to bring it back soon. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] media: atomisp: get rid of -Wsuggest-attribute=format warnings
Hi Mauro,
I love your patch! Yet something to improve:
[auto build test ERROR on linuxtv-media/master]
[cannot apply to staging/staging-testing linus/master v5.9-rc3 next-20200903]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/address-W-1-warnings-at-staging-media-atomisp/20200903-220729
base: git://linuxtv.org/media_tree.git master
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All error/warnings (new ones prefixed by >>):
drivers/staging/media/atomisp/pci/atomisp_compat_css20.c: In function
'__set_css_print_env':
>> drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:861:50: error:
>> assignment to 'int (*)(const char *, char *)' from incompatible pointer type
>> 'int (__attribute__((regparm(0))) *)(const char *, char *)'
>> [-Werror=incompatible-pointer-types]
861 | isp->css_env.isp_css_env.print_env.debug_print = vprintk;
| ^
drivers/staging/media/atomisp/pci/atomisp_compat_css20.c: In function
'atomisp_css_load_firmware':
drivers/staging/media/atomisp/pci/atomisp_compat_css20.c:894:49: error:
assignment to 'int (*)(const char *, char *)' from incompatible pointer type
'int (__attribute__((regparm(0))) *)(const char *, char *)'
[-Werror=incompatible-pointer-types]
894 | isp->css_env.isp_css_env.print_env.error_print = vprintk;
| ^
cc1: some warnings being treated as errors
--
drivers/staging/media/atomisp/pci/sh_css.c: In function 'ia_css_init':
>> drivers/staging/media/atomisp/pci/sh_css.c:1685:16: warning: assignment
>> left-hand side might be a candidate for a format attribute
>> [-Wsuggest-attribute=format]
1685 | sh_css_printf = env->print_env.debug_print;
|^
#
https://github.com/0day-ci/linux/commit/bee6e6549cd6c55a1d62347514bb1cb11acd6aca
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Mauro-Carvalho-Chehab/address-W-1-warnings-at-staging-media-atomisp/20200903-220729
git checkout bee6e6549cd6c55a1d62347514bb1cb11acd6aca
vim +861 drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
850
851 static inline int __set_css_print_env(struct atomisp_device *isp, int
opt)
852 {
853 int ret = 0;
854
855 if (opt == 0)
856 isp->css_env.isp_css_env.print_env.debug_print = NULL;
857 else if (opt == 1)
858 isp->css_env.isp_css_env.print_env.debug_print =
859 atomisp_css2_dbg_ftrace_print;
860 else if (opt == 2)
> 861 isp->css_env.isp_css_env.print_env.debug_print =
> vprintk;
862 else
863 ret = -EINVAL;
864
865 return ret;
866 }
867
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: media: atomisp: Fix error path in lm3554_probe()
On Thu, Sep 03, 2020 at 04:48:41PM +0100, Alex Dewar wrote:
> Good point about the timer!
>
> > >
> > > - err = lm3554_gpio_init(client);
> > > - if (err) {
> > > + ret = lm3554_gpio_init(client);
> > > + if (ret) {
> > > dev_err(&client->dev, "gpio request/direction_output fail");
> > > - goto fail2;
> > > + goto err_cleanup_entity;
> > > }
> > > return atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH);
> >
> > If atomisp_register_i2c_module() fails then we need to call
> > lm3554_gpio_uninit(client) and do other cleanup.
>
> I'm probably showing my ignorance here, but I can't see what cleanup we
> need. Inside lm3554_gpio_init we have:
>
> ret = gpiod_direction_output(pdata->gpio_reset, 0);
> if (ret < 0)
> return ret;
> dev_info(&client->dev, "flash led reset successfully\n");
>
> if (!pdata->gpio_strobe)
> return -EINVAL;
>
> ret = gpiod_direction_output(pdata->gpio_strobe, 0);
> if (ret < 0)
> return ret;
>
> I'm not sure how you "undo" a call to gpiod_direction_output, but I'm
> thinking you won't need to do anything because it should be ok to leave
> a gpio to output 0... right?
You're right. I wonder if there is really any need for the
lm3554_gpio_uninit() function at all? It's basically the same as
lm3554_gpio_init() except for the order of function calls. Probably
we could just rename lm3554_gpio_init() to something like
lm3554_gpio_set_default() and use it in both the probe() and remove
functions()...
But I don't know the code and can't test it so let's leave that for
another day.
We still do need to clean up if atomisp_register_i2c_module() fails
though, and the timer as well so could you resend a v2?
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Mrs.Rosa Gomes ;
Dear Beloved Life is gradually passing away from me as a result of my present medical condition and my personal doctor confided in me yesterday that I have only but few more weeks to live. In view of this setback, I want to donate my estate for humanitarian assistance, since this has always been the plan of my late husband and besides I have no child. In an effort to compliment the good work of God almighty and the wish of my late Husband I donate the sum of $2,847,000.00 (Two Million Eight Hundred and Forty Seven Thousand United States Dollars) to you. On your acknowledgment of this mail and informing me of your nationality and current place of resident, my Bank will facilitate due processes for transfer of this legacy to you. May God bless you as you use this money judiciously for the work of charity. Sincere regards, Mrs.Rosa Gomes. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Mrs.Rosa Gomes:
Dear Beloved Life is gradually passing away from me as a result of my present medical condition and my personal doctor confided in me yesterday that I have only but few more weeks to live. In view of this setback, I want to donate my estate for humanitarian assistance, since this has always been the plan of my late husband and besides I have no child. In an effort to compliment the good work of God almighty and the wish of my late Husband I donate the sum of $2,847,000.00 (Two Million Eight Hundred and Forty Seven Thousand United States Dollars) to you. On your acknowledgment of this mail and informing me of your nationality and current place of resident, my Bank will facilitate due processes for transfer of this legacy to you. May God bless you as you use this money judiciously for the work of charity. Sincere regards, Mrs.Rosa Gomes. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: media: atomisp: Fix error path in lm3554_probe()
> You're right. I wonder if there is really any need for the > lm3554_gpio_uninit() function at all? It's basically the same as > lm3554_gpio_init() except for the order of function calls. Probably > we could just rename lm3554_gpio_init() to something like > lm3554_gpio_set_default() and use it in both the probe() and remove > functions()... I think you probably also don't want to return error values from lm3554_gpio_uninit() as it is only called on module removal, so it'd probably make more sense to just print a warning and carry on. I'll do this as a separate patch and send it to the list, though. v2 to follow... > > But I don't know the code and can't test it so let's leave that for > another day. > > We still do need to clean up if atomisp_register_i2c_module() fails > though, and the timer as well so could you resend a v2? > > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: media: atomisp: Fix error path in lm3554_probe()
The error path for lm3554_probe() contains a number of bugs, including:
* resource leaks
* jumping to error labels out of sequence
* not setting the return value appropriately
Fix it up and give the labels more memorable names.
This issue has existed since the code was originally contributed in
commit a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2"),
although the code was subsequently removed altogether and then
reinstated with commit ad85094b293e ("Revert "media: staging: atomisp: Remove
driver"").
Addresses-Coverity: 1496802 ("Resource leaks")
Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Alex Dewar
---
.../media/atomisp/i2c/atomisp-lm3554.c| 53 +++
1 file changed, 30 insertions(+), 23 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
index 7ca7378b1859..cca10a4c2db0 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
@@ -833,7 +833,6 @@ static void *lm3554_platform_data_func(struct i2c_client
*client)
static int lm3554_probe(struct i2c_client *client)
{
- int err = 0;
struct lm3554 *flash;
unsigned int i;
int ret;
@@ -843,36 +842,38 @@ static int lm3554_probe(struct i2c_client *client)
return -ENOMEM;
flash->pdata = lm3554_platform_data_func(client);
- if (IS_ERR(flash->pdata))
- return PTR_ERR(flash->pdata);
+ if (IS_ERR(flash->pdata)) {
+ ret = PTR_ERR(flash->pdata);
+ goto err_free_flash;
+ }
v4l2_i2c_subdev_init(&flash->sd, client, &lm3554_ops);
flash->sd.internal_ops = &lm3554_internal_ops;
flash->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
flash->mode = ATOMISP_FLASH_MODE_OFF;
flash->timeout = LM3554_MAX_TIMEOUT / LM3554_TIMEOUT_STEPSIZE - 1;
- ret =
- v4l2_ctrl_handler_init(&flash->ctrl_handler,
- ARRAY_SIZE(lm3554_controls));
+ ret = v4l2_ctrl_handler_init(&flash->ctrl_handler,
+ARRAY_SIZE(lm3554_controls));
if (ret) {
- dev_err(&client->dev, "error initialize a ctrl_handler.\n");
- goto fail2;
+ dev_err(&client->dev, "error initializing ctrl_handler");
+ goto err_unregister_sd;
}
for (i = 0; i < ARRAY_SIZE(lm3554_controls); i++)
v4l2_ctrl_new_custom(&flash->ctrl_handler, &lm3554_controls[i],
NULL);
- if (flash->ctrl_handler.error) {
- dev_err(&client->dev, "ctrl_handler error.\n");
- goto fail2;
+ ret = flash->ctrl_handler.error;
+ if (ret) {
+ dev_err(&client->dev, "ctrl_handler error");
+ goto err_free_ctrl_handler;
}
flash->sd.ctrl_handler = &flash->ctrl_handler;
- err = media_entity_pads_init(&flash->sd.entity, 0, NULL);
- if (err) {
- dev_err(&client->dev, "error initialize a media entity.\n");
- goto fail1;
+ ret = media_entity_pads_init(&flash->sd.entity, 0, NULL);
+ if (ret) {
+ dev_err(&client->dev, "error initializing media entity");
+ goto err_free_ctrl_handler;
}
flash->sd.entity.function = MEDIA_ENT_F_FLASH;
@@ -881,20 +882,26 @@ static int lm3554_probe(struct i2c_client *client)
timer_setup(&flash->flash_off_delay, lm3554_flash_off_delay, 0);
- err = lm3554_gpio_init(client);
- if (err) {
+ ret = lm3554_gpio_init(client);
+ if (ret) {
dev_err(&client->dev, "gpio request/direction_output fail");
- goto fail2;
+ goto err_del_timer;
}
- return atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH);
-fail2:
+
+ ret = atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH);
+ if (!ret)
+ return 0;
+
+err_del_timer:
+ del_timer_sync(&flash->flash_off_delay);
media_entity_cleanup(&flash->sd.entity);
+err_free_ctrl_handler:
v4l2_ctrl_handler_free(&flash->ctrl_handler);
-fail1:
+err_unregister_sd:
v4l2_device_unregister_subdev(&flash->sd);
+err_free_flash:
kfree(flash);
-
- return err;
+ return ret;
}
static int lm3554_remove(struct i2c_client *client)
--
2.28.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: media: atomisp: Remove unhelpful info message
We don't really need to know that the LED pin reset successfully. Signed-off-by: Alex Dewar --- drivers/staging/media/atomisp/i2c/atomisp-lm3554.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c index 621555b0cf6b..77b7f59e62d7 100644 --- a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c +++ b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c @@ -771,7 +771,6 @@ static int lm3554_gpio_init(struct i2c_client *client) ret = gpiod_direction_output(pdata->gpio_reset, 0); if (ret < 0) return ret; - dev_info(&client->dev, "flash led reset successfully\n"); if (!pdata->gpio_strobe) return -EINVAL; -- 2.28.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: media: atomisp: Don't abort on error in module exit path
The function lm3554_remove() checks for the return code for
lm3554_gpio_uninit() even though this is on the exit path and exits the
function, leaving the variable flash unfreed. Print a warning instead
and free flash unconditionally.
Signed-off-by: Alex Dewar
---
.../staging/media/atomisp/i2c/atomisp-lm3554.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
index cca10a4c2db0..621555b0cf6b 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
@@ -783,7 +783,7 @@ static int lm3554_gpio_init(struct i2c_client *client)
return 0;
}
-static int lm3554_gpio_uninit(struct i2c_client *client)
+static void lm3554_gpio_uninit(struct i2c_client *client)
{
struct v4l2_subdev *sd = i2c_get_clientdata(client);
struct lm3554 *flash = to_lm3554(sd);
@@ -792,13 +792,13 @@ static int lm3554_gpio_uninit(struct i2c_client *client)
ret = gpiod_direction_output(pdata->gpio_strobe, 0);
if (ret < 0)
- return ret;
+ dev_err(&client->dev,
+ "gpio request/direction_output fail for gpio_strobe");
ret = gpiod_direction_output(pdata->gpio_reset, 0);
if (ret < 0)
- return ret;
-
- return 0;
+ dev_err(&client->dev,
+ "gpio request/direction_output fail for gpio_reset");
}
static void *lm3554_platform_data_func(struct i2c_client *client)
@@ -918,16 +918,11 @@ static int lm3554_remove(struct i2c_client *client)
del_timer_sync(&flash->flash_off_delay);
- ret = lm3554_gpio_uninit(client);
- if (ret < 0)
- goto fail;
+ lm3554_gpio_uninit(client);
kfree(flash);
return 0;
-fail:
- dev_err(&client->dev, "gpio request/direction_output fail");
- return ret;
}
static const struct dev_pm_ops lm3554_pm_ops = {
--
2.28.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset
On Wed, Sep 2, 2020 at 8:52 PM Nathan Chancellor wrote: > > On Wed, Sep 02, 2020 at 05:36:29PM -0700, Florian Fainelli wrote: > > > > > > On 9/2/2020 3:38 PM, Nathan Chancellor wrote: > > [snip] > > > > Hello Nathan, > > > > > > > > Can you tell me how much memory your RPI has and if all of it is > > > > > > This is the 4GB version. > > > > > > > accessible by the PCIe device? Could you also please include the DTS > > > > of the PCIe node? IIRC, the RPI firmware does some mangling of the > > > > PCIe DT before Linux boots -- could you describe what is going on > > > > there? > > > > > > Unfortunately, I am not familiar with how to get this information. If > > > you could provide some instructions for how to do so, I am more than > > > happy to. I am not very knowleagable about the inner working of the Pi, > > > I mainly use it as a test platform for making sure that LLVM does not > > > cause problems on real devices. > > > > Can you bring the dtc application to your Pi root filesystem, and if so, can > > you run the following: > > > > dtc -I fs -O dtb /proc/device-tree -f > /tmp/device.dtb > > Sure, the result is attached. > > > or cat /sys/firmware/fdt > device.dtb > > > > and attach the resulting file? > > > > > > > > > Finally, can you attach the text of the full boot log? > > > > > > I have attached a working and broken boot log. Thank you for the quick > > > response! > > > > Is it possible for you to rebuild your kernel with CONFIG_MMC_DEBUG by any > > chance? > > Of course. A new log is attached with the debug output from that config. Hi Nicolas, Can you please help us out here? It appears that your commit 3d2cbb644836 "ARM: dts: bcm2711: Move emmc2 into its own bus" added the following line to the bcm2711.dtsi file: + dma-ranges = <0x0 0xc000 0x0 0x 0x4000>; and for some reason the eMMC is not operating properly w/ my commit.. Regardless, the only difference that my commit should make is to enforce the bounds of the dma_window (whereas the previous code adds/subs the pfn_offset everywhere). Thanks, Jim Quinlan PS If you would like to talk, let me know and we can make arrangements. > > > > I have a suspicion that this part of the DTS for the bcm2711.dtsi platform > > is at fault: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/bcm2711.dtsi#n264 > > > > and the resulting dma-ranges parsing is just not working for reasons to be > > determined. > > -- > > Florian > > Let me know if you need anything else out of me. > > Cheers, > Nathan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Congratulation: You're a winner!
CONGRATULATION!!! Reference to the 1,320th Euro-Millions draw that took place on Friday 15th May 2020 at 21:00 CEST (20:00 BST) through the Euro-Millions draw system were your E-mail address emerged among lucky winners with the drawn winning numbers: Lucky numbers 11-23-32-34-39 Star Number 1-6 Millionaire Maker: VBRL70371 serial number MZNS47038 Prize credited to file EURO/86169/2020 An official letter was sent to your address. Your email address has been awarded the sum of Ј2,274,805.30 GB pounds. Kindly, confirm receipt of this notification by contacting your claims officer Mr. Kennith William on wken...@zohomail.com for more details. visit the link https://www.euro-millions.com/results/15-05-2020 to view your winning details as published on the Euro-Millions site. Euro-Millions prizes must be claimed within 90 days of the draw date. This is a confidential mail sent to ONLY winners of this draws. If you have any questions, please contact our customer support. Kind regards, Peter Jones Customer Support EURO-MILLIONS Customer Service UK Regional Office Acorns Oakwood Park Business Center Fountains Road Bishop Thornton, Harrogate HG3 3BF, UK. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Remove unused variables
A few unused variables that were defined were found and removed.
Signed-off-by: Anant Thazhemadam
---
drivers/staging/comedi/drivers/dt2814.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/dt2814.c
b/drivers/staging/comedi/drivers/dt2814.c
index bcf4d5444faf..1a36d8612d9f 100644
--- a/drivers/staging/comedi/drivers/dt2814.c
+++ b/drivers/staging/comedi/drivers/dt2814.c
@@ -190,7 +190,6 @@ static irqreturn_t dt2814_interrupt(int irq, void *d)
struct comedi_device *dev = d;
struct dt2814_private *devpriv = dev->private;
struct comedi_subdevice *s = dev->read_subdev;
- int data;
if (!dev->attached) {
dev_err(dev->class_dev, "spurious interrupt\n");
@@ -229,7 +228,6 @@ static int dt2814_attach(struct comedi_device *dev, struct
comedi_devconfig *it)
struct dt2814_private *devpriv;
struct comedi_subdevice *s;
int ret;
- int i;
ret = comedi_request_region(dev, it->options[0], 0x2);
if (ret)
--
2.25.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Remove unused variables
This patch was generated, and sent by a script I was working on (and
apparently, still am), that would run sparse,
detect unused variables and eliminate them. However, upon manual
inspection, it became clear
to me that these functions are in fact necessary. I'm sorry this
mistake happened. Kindly ignore and discard this patch.
My sincere apologies.
Thanks,
Anant
On Fri, Sep 4, 2020 at 6:04 AM Anant Thazhemadam
wrote:
>
> A few unused variables that were defined were found and removed.
>
> Signed-off-by: Anant Thazhemadam
> ---
> drivers/staging/comedi/drivers/dt2814.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/dt2814.c
> b/drivers/staging/comedi/drivers/dt2814.c
> index bcf4d5444faf..1a36d8612d9f 100644
> --- a/drivers/staging/comedi/drivers/dt2814.c
> +++ b/drivers/staging/comedi/drivers/dt2814.c
> @@ -190,7 +190,6 @@ static irqreturn_t dt2814_interrupt(int irq, void *d)
> struct comedi_device *dev = d;
> struct dt2814_private *devpriv = dev->private;
> struct comedi_subdevice *s = dev->read_subdev;
> - int data;
>
> if (!dev->attached) {
> dev_err(dev->class_dev, "spurious interrupt\n");
> @@ -229,7 +228,6 @@ static int dt2814_attach(struct comedi_device *dev,
> struct comedi_devconfig *it)
> struct dt2814_private *devpriv;
> struct comedi_subdevice *s;
> int ret;
> - int i;
>
> ret = comedi_request_region(dev, it->options[0], 0x2);
> if (ret)
> --
> 2.25.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: media: atomisp: Fix error path in lm3554_probe()
On Thu, Sep 03, 2020 at 07:24:51PM +0100, Alex Dewar wrote: > + > + ret = atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH); > + if (!ret) > + return 0; Ugh!!! This is a a special case of the "success handling instead of failure handling" anti-pattern where the last condition in the function is different. I just fixed a bug caused by this on Wed. https://www.spinics.net/lists/netdev/msg680226.html But it doesn't cause any problems here so whatever... Reviewed-by: Dan Carpenter regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
