* [PATCH 2/9] staging: media: atomisp: declare 'struct device' before using it
2020-05-29 20:00 [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check Arnd Bergmann
@ 2020-05-29 20:00 ` Arnd Bergmann
2020-05-30 2:55 ` Nathan Chancellor
2020-05-29 20:00 ` [PATCH 3/9] staging: media: atomisp: annotate an unused function Arnd Bergmann
` (7 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel,
clang-built-linux, Arnd Bergmann
In some configurations, including this header leads to a warning:
drivers/staging/media/atomisp//pci/sh_css_firmware.h:41:38: error: declaration of 'struct device' will not be visible outside of this function [-Werror,-Wvisibility]
Make sure the struct tag is known before declaring a function
that uses it as an argument.
Fixes: 9d4fa1a16b28 ("media: atomisp: cleanup directory hierarchy")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/media/atomisp/pci/sh_css_firmware.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.h b/drivers/staging/media/atomisp/pci/sh_css_firmware.h
index f6253392a6c9..317559c7689f 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_firmware.h
+++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.h
@@ -37,6 +37,7 @@ extern unsigned int sh_css_num_binaries;
char
*sh_css_get_fw_version(void);
+struct device;
bool
sh_css_check_firmware_version(struct device *dev, const char *fw_data);
--
2.26.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 2/9] staging: media: atomisp: declare 'struct device' before using it
2020-05-29 20:00 ` [PATCH 2/9] staging: media: atomisp: declare 'struct device' before using it Arnd Bergmann
@ 2020-05-30 2:55 ` Nathan Chancellor
0 siblings, 0 replies; 21+ messages in thread
From: Nathan Chancellor @ 2020-05-30 2:55 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mauro Carvalho Chehab, Sakari Ailus, linux-media, devel,
linux-kernel, clang-built-linux
On Fri, May 29, 2020 at 10:00:24PM +0200, Arnd Bergmann wrote:
> In some configurations, including this header leads to a warning:
>
> drivers/staging/media/atomisp//pci/sh_css_firmware.h:41:38: error: declaration of 'struct device' will not be visible outside of this function [-Werror,-Wvisibility]
>
> Make sure the struct tag is known before declaring a function
> that uses it as an argument.
>
> Fixes: 9d4fa1a16b28 ("media: atomisp: cleanup directory hierarchy")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> drivers/staging/media/atomisp/pci/sh_css_firmware.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.h b/drivers/staging/media/atomisp/pci/sh_css_firmware.h
> index f6253392a6c9..317559c7689f 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_firmware.h
> +++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.h
> @@ -37,6 +37,7 @@ extern unsigned int sh_css_num_binaries;
> char
> *sh_css_get_fw_version(void);
>
> +struct device;
> bool
> sh_css_check_firmware_version(struct device *dev, const char *fw_data);
>
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/9] staging: media: atomisp: annotate an unused function
2020-05-29 20:00 [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check Arnd Bergmann
2020-05-29 20:00 ` [PATCH 2/9] staging: media: atomisp: declare 'struct device' before using it Arnd Bergmann
@ 2020-05-29 20:00 ` Arnd Bergmann
2020-05-30 2:56 ` Nathan Chancellor
2020-05-29 20:00 ` [PATCH 4/9] staging: media: atomisp: fix a type conversion warning Arnd Bergmann
` (6 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel,
clang-built-linux, Arnd Bergmann
atomisp_mrfld_power() has no more callers and produces
a warning:
drivers/staging/media/atomisp/pci/atomisp_v4l2.c:764:12: error: unused function 'atomisp_mrfld_power' [-Werror,-Wunused-function]
Mark the function as unused while the PM code is being
debugged, expecting that it will be used again in the
future and should not just be removed.
Fixes: 95d1f398c4dc ("media: atomisp: keep the ISP powered on when setting it")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 694268d133c0..10abb35ba0e0 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -761,7 +761,8 @@ static void punit_ddr_dvfs_enable(bool enable)
pr_info("DDR DVFS, door bell is not cleared within 3ms\n");
}
-static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
+static __attribute__((unused)) int
+atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
{
unsigned long timeout;
u32 val = enable ? MRFLD_ISPSSPM0_IUNIT_POWER_ON :
--
2.26.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 3/9] staging: media: atomisp: annotate an unused function
2020-05-29 20:00 ` [PATCH 3/9] staging: media: atomisp: annotate an unused function Arnd Bergmann
@ 2020-05-30 2:56 ` Nathan Chancellor
0 siblings, 0 replies; 21+ messages in thread
From: Nathan Chancellor @ 2020-05-30 2:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mauro Carvalho Chehab, Sakari Ailus, linux-media, devel,
linux-kernel, clang-built-linux
On Fri, May 29, 2020 at 10:00:25PM +0200, Arnd Bergmann wrote:
> atomisp_mrfld_power() has no more callers and produces
> a warning:
>
> drivers/staging/media/atomisp/pci/atomisp_v4l2.c:764:12: error: unused function 'atomisp_mrfld_power' [-Werror,-Wunused-function]
>
> Mark the function as unused while the PM code is being
> debugged, expecting that it will be used again in the
> future and should not just be removed.
>
> Fixes: 95d1f398c4dc ("media: atomisp: keep the ISP powered on when setting it")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Mauro fixed this in his experimental branch:
https://git.linuxtv.org/mchehab/experimental.git/commit/?id=dbcee8118fc9283401df9dbe8ba03ab9d17433ff
> ---
> drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> index 694268d133c0..10abb35ba0e0 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> @@ -761,7 +761,8 @@ static void punit_ddr_dvfs_enable(bool enable)
> pr_info("DDR DVFS, door bell is not cleared within 3ms\n");
> }
>
> -static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
> +static __attribute__((unused)) int
> +atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
> {
> unsigned long timeout;
> u32 val = enable ? MRFLD_ISPSSPM0_IUNIT_POWER_ON :
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/9] staging: media: atomisp: fix a type conversion warning
2020-05-29 20:00 [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check Arnd Bergmann
2020-05-29 20:00 ` [PATCH 2/9] staging: media: atomisp: declare 'struct device' before using it Arnd Bergmann
2020-05-29 20:00 ` [PATCH 3/9] staging: media: atomisp: annotate an unused function Arnd Bergmann
@ 2020-05-29 20:00 ` Arnd Bergmann
2020-05-29 20:00 ` [PATCH 5/9] staging: media: atomisp: fix stack overflow in init_pipe_defaults() Arnd Bergmann
` (5 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel,
clang-built-linux, Arnd Bergmann
clang complains that the type conversion in the MAX() macro
contains an implied integer overflow to a signed number:
drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c:129:35:
error: implicit conversion from 'unsigned long' to 'int32_t' (aka 'int') changes value from 18446744073709543424 to -8192 [-Werror,-Wconstant-conversion]
As the conversion is clearly intended here, use an explicit cast.
Fixes: 9a0d7fb5ece6 ("media: atomisp: simplify math_support.h")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
.../atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c
index a9db6366d20b..613bace0522a 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c
@@ -126,7 +126,7 @@ compute_blending(int strength)
* exactly as s0.11 fixed point, but -1.0 can.
*/
isp_strength = -(((strength * isp_scale) + offset) / host_scale);
- return MAX(MIN(isp_strength, 0), -XNR_BLENDING_SCALE_FACTOR);
+ return MAX(MIN(isp_strength, 0), -(unsigned int)XNR_BLENDING_SCALE_FACTOR);
}
void
--
2.26.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 5/9] staging: media: atomisp: fix stack overflow in init_pipe_defaults()
2020-05-29 20:00 [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check Arnd Bergmann
` (2 preceding siblings ...)
2020-05-29 20:00 ` [PATCH 4/9] staging: media: atomisp: fix a type conversion warning Arnd Bergmann
@ 2020-05-29 20:00 ` Arnd Bergmann
2020-05-30 2:57 ` Nathan Chancellor
2020-05-29 20:00 ` [PATCH 6/9] staging: media: atomisp: fix type mismatch Arnd Bergmann
` (4 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel,
clang-built-linux, Arnd Bergmann
When building with clang, multiple copies of the structures to be
initialized are passed around on the stack and copied locally, using an
insane amount of stack space:
drivers/staging/media/atomisp/pci/sh_css.c:2371:1: error: stack frame size of 26864 bytes in function 'create_pipe' [-Werror,-Wframe-larger-than=]
Use constantly-allocated variables plus an explicit memcpy()
to avoid that.
Fixes: 6dc9a2568f84 ("media: atomisp: convert default struct values to use compound-literals with designated initializers")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/media/atomisp/pci/sh_css.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index e91c6029c651..1e8b9d637116 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -2264,6 +2264,12 @@ static enum ia_css_err
init_pipe_defaults(enum ia_css_pipe_mode mode,
struct ia_css_pipe *pipe,
bool copy_pipe) {
+ static const struct ia_css_pipe default_pipe = IA_CSS_DEFAULT_PIPE;
+ static const struct ia_css_preview_settings preview = IA_CSS_DEFAULT_PREVIEW_SETTINGS;
+ static const struct ia_css_capture_settings capture = IA_CSS_DEFAULT_CAPTURE_SETTINGS;
+ static const struct ia_css_video_settings video = IA_CSS_DEFAULT_VIDEO_SETTINGS;
+ static const struct ia_css_yuvpp_settings yuvpp = IA_CSS_DEFAULT_YUVPP_SETTINGS;
+
if (!pipe)
{
IA_CSS_ERROR("NULL pipe parameter");
@@ -2271,14 +2277,14 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
}
/* Initialize pipe to pre-defined defaults */
- *pipe = IA_CSS_DEFAULT_PIPE;
+ memcpy(pipe, &default_pipe, sizeof(default_pipe));
/* TODO: JB should not be needed, but temporary backward reference */
switch (mode)
{
case IA_CSS_PIPE_MODE_PREVIEW:
pipe->mode = IA_CSS_PIPE_ID_PREVIEW;
- pipe->pipe_settings.preview = IA_CSS_DEFAULT_PREVIEW_SETTINGS;
+ memcpy(&pipe->pipe_settings.preview, &preview, sizeof(preview));
break;
case IA_CSS_PIPE_MODE_CAPTURE:
if (copy_pipe) {
@@ -2286,11 +2292,11 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
} else {
pipe->mode = IA_CSS_PIPE_ID_CAPTURE;
}
- pipe->pipe_settings.capture = IA_CSS_DEFAULT_CAPTURE_SETTINGS;
+ memcpy(&pipe->pipe_settings.capture, &capture, sizeof(capture));
break;
case IA_CSS_PIPE_MODE_VIDEO:
pipe->mode = IA_CSS_PIPE_ID_VIDEO;
- pipe->pipe_settings.video = IA_CSS_DEFAULT_VIDEO_SETTINGS;
+ memcpy(&pipe->pipe_settings.video, &video, sizeof(video));
break;
case IA_CSS_PIPE_MODE_ACC:
pipe->mode = IA_CSS_PIPE_ID_ACC;
@@ -2300,7 +2306,7 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
break;
case IA_CSS_PIPE_MODE_YUVPP:
pipe->mode = IA_CSS_PIPE_ID_YUVPP;
- pipe->pipe_settings.yuvpp = IA_CSS_DEFAULT_YUVPP_SETTINGS;
+ memcpy(&pipe->pipe_settings.yuvpp, &yuvpp, sizeof(yuvpp));
break;
default:
return IA_CSS_ERR_INVALID_ARGUMENTS;
--
2.26.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 5/9] staging: media: atomisp: fix stack overflow in init_pipe_defaults()
2020-05-29 20:00 ` [PATCH 5/9] staging: media: atomisp: fix stack overflow in init_pipe_defaults() Arnd Bergmann
@ 2020-05-30 2:57 ` Nathan Chancellor
0 siblings, 0 replies; 21+ messages in thread
From: Nathan Chancellor @ 2020-05-30 2:57 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mauro Carvalho Chehab, Sakari Ailus, linux-media, devel,
linux-kernel, clang-built-linux
On Fri, May 29, 2020 at 10:00:27PM +0200, Arnd Bergmann wrote:
> When building with clang, multiple copies of the structures to be
> initialized are passed around on the stack and copied locally, using an
> insane amount of stack space:
>
> drivers/staging/media/atomisp/pci/sh_css.c:2371:1: error: stack frame size of 26864 bytes in function 'create_pipe' [-Werror,-Wframe-larger-than=]
>
> Use constantly-allocated variables plus an explicit memcpy()
> to avoid that.
>
> Fixes: 6dc9a2568f84 ("media: atomisp: convert default struct values to use compound-literals with designated initializers")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> drivers/staging/media/atomisp/pci/sh_css.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> index e91c6029c651..1e8b9d637116 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -2264,6 +2264,12 @@ static enum ia_css_err
> init_pipe_defaults(enum ia_css_pipe_mode mode,
> struct ia_css_pipe *pipe,
> bool copy_pipe) {
> + static const struct ia_css_pipe default_pipe = IA_CSS_DEFAULT_PIPE;
> + static const struct ia_css_preview_settings preview = IA_CSS_DEFAULT_PREVIEW_SETTINGS;
> + static const struct ia_css_capture_settings capture = IA_CSS_DEFAULT_CAPTURE_SETTINGS;
> + static const struct ia_css_video_settings video = IA_CSS_DEFAULT_VIDEO_SETTINGS;
> + static const struct ia_css_yuvpp_settings yuvpp = IA_CSS_DEFAULT_YUVPP_SETTINGS;
> +
> if (!pipe)
> {
> IA_CSS_ERROR("NULL pipe parameter");
> @@ -2271,14 +2277,14 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
> }
>
> /* Initialize pipe to pre-defined defaults */
> - *pipe = IA_CSS_DEFAULT_PIPE;
> + memcpy(pipe, &default_pipe, sizeof(default_pipe));
>
> /* TODO: JB should not be needed, but temporary backward reference */
> switch (mode)
> {
> case IA_CSS_PIPE_MODE_PREVIEW:
> pipe->mode = IA_CSS_PIPE_ID_PREVIEW;
> - pipe->pipe_settings.preview = IA_CSS_DEFAULT_PREVIEW_SETTINGS;
> + memcpy(&pipe->pipe_settings.preview, &preview, sizeof(preview));
> break;
> case IA_CSS_PIPE_MODE_CAPTURE:
> if (copy_pipe) {
> @@ -2286,11 +2292,11 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
> } else {
> pipe->mode = IA_CSS_PIPE_ID_CAPTURE;
> }
> - pipe->pipe_settings.capture = IA_CSS_DEFAULT_CAPTURE_SETTINGS;
> + memcpy(&pipe->pipe_settings.capture, &capture, sizeof(capture));
> break;
> case IA_CSS_PIPE_MODE_VIDEO:
> pipe->mode = IA_CSS_PIPE_ID_VIDEO;
> - pipe->pipe_settings.video = IA_CSS_DEFAULT_VIDEO_SETTINGS;
> + memcpy(&pipe->pipe_settings.video, &video, sizeof(video));
> break;
> case IA_CSS_PIPE_MODE_ACC:
> pipe->mode = IA_CSS_PIPE_ID_ACC;
> @@ -2300,7 +2306,7 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
> break;
> case IA_CSS_PIPE_MODE_YUVPP:
> pipe->mode = IA_CSS_PIPE_ID_YUVPP;
> - pipe->pipe_settings.yuvpp = IA_CSS_DEFAULT_YUVPP_SETTINGS;
> + memcpy(&pipe->pipe_settings.yuvpp, &yuvpp, sizeof(yuvpp));
> break;
> default:
> return IA_CSS_ERR_INVALID_ARGUMENTS;
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 6/9] staging: media: atomisp: fix type mismatch
2020-05-29 20:00 [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check Arnd Bergmann
` (3 preceding siblings ...)
2020-05-29 20:00 ` [PATCH 5/9] staging: media: atomisp: fix stack overflow in init_pipe_defaults() Arnd Bergmann
@ 2020-05-29 20:00 ` Arnd Bergmann
2020-05-29 20:00 ` [PATCH 7/9] staging: media: atomisp: fix enum type mixups Arnd Bergmann
` (3 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel,
clang-built-linux, Arnd Bergmann
The caller passes a variable of a different enum type:
drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c:1707:64: error: implicit conversion from enumeration type 'const enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion]
binary_supports_input_format(xcandidate, req_in_info->format));
An earlier patch tried to address this by changing the type
of the function argument, but as the caller passes two different
arguments, there is still a warning.
Assume that the last patch was correct and change the other caller
as well.
Fixes: 0116b8df1c9e ("media: staging: atomisp: stop duplicating input format types")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
index 2a23b7c6aeeb..10d698295daf 100644
--- a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
+++ b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
@@ -1704,7 +1704,7 @@ ia_css_binary_find(struct ia_css_binary_descr *descr,
ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
"ia_css_binary_find() [%d] continue: !%d\n",
__LINE__,
- binary_supports_input_format(xcandidate, req_in_info->format));
+ binary_supports_input_format(xcandidate, descr->stream_format));
continue;
}
#endif
--
2.26.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 7/9] staging: media: atomisp: fix enum type mixups
2020-05-29 20:00 [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check Arnd Bergmann
` (4 preceding siblings ...)
2020-05-29 20:00 ` [PATCH 6/9] staging: media: atomisp: fix type mismatch Arnd Bergmann
@ 2020-05-29 20:00 ` Arnd Bergmann
2020-05-30 3:00 ` Nathan Chancellor
2020-05-29 20:00 ` [PATCH 8/9] staging: media: atomisp: disable all custom formats Arnd Bergmann
` (2 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel,
clang-built-linux, Arnd Bergmann
Some function calls pass an incorrect enum type:
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:858:16: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
gp_device_rst(INPUT_SYSTEM0_ID);
~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:860:19: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
input_switch_rst(INPUT_SYSTEM0_ID);
~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:876:27: error: implicit conversion from enumeration type 'input_system_cfg_flag_t' to different enumeration type 'input_system_connection_t' [-Werror,-Wenum-conversion]
config.multicast[i] = INPUT_SYSTEM_CFG_FLAG_RESET;
~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:1326:32: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
input_selector_cfg_for_sensor(INPUT_SYSTEM0_ID);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:1329:19: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
input_switch_cfg(INPUT_SYSTEM0_ID, &config.input_switch_cfg);
~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
INPUT_SYSTEM0_ID is zero, so use the corresponding zero-value
of the expected types instead.
Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
.../pci/hive_isp_css_common/host/input_system.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
index 2114cf4f3fda..aa0f0fca9346 100644
--- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
+++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
@@ -855,9 +855,9 @@ input_system_error_t input_system_configuration_reset(void)
input_system_network_rst(INPUT_SYSTEM0_ID);
- gp_device_rst(INPUT_SYSTEM0_ID);
+ gp_device_rst(GP_DEVICE0_ID);
- input_switch_rst(INPUT_SYSTEM0_ID);
+ input_switch_rst(GP_DEVICE0_ID);
//target_rst();
@@ -873,7 +873,7 @@ input_system_error_t input_system_configuration_reset(void)
for (i = 0; i < N_CSI_PORTS; i++) {
config.csi_buffer_flags[i] = INPUT_SYSTEM_CFG_FLAG_RESET;
- config.multicast[i] = INPUT_SYSTEM_CFG_FLAG_RESET;
+ config.multicast[i] = INPUT_SYSTEM_DISCARD_ALL;
}
config.source_type_flags = INPUT_SYSTEM_CFG_FLAG_RESET;
@@ -1323,10 +1323,10 @@ static input_system_error_t configuration_to_registers(void)
} // end of switch (source_type)
// Set input selector.
- input_selector_cfg_for_sensor(INPUT_SYSTEM0_ID);
+ input_selector_cfg_for_sensor(GP_DEVICE0_ID);
// Set input switch.
- input_switch_cfg(INPUT_SYSTEM0_ID, &config.input_switch_cfg);
+ input_switch_cfg(GP_DEVICE0_ID, &config.input_switch_cfg);
// Set input formatters.
// AM: IF are set dynamically.
--
2.26.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 7/9] staging: media: atomisp: fix enum type mixups
2020-05-29 20:00 ` [PATCH 7/9] staging: media: atomisp: fix enum type mixups Arnd Bergmann
@ 2020-05-30 3:00 ` Nathan Chancellor
0 siblings, 0 replies; 21+ messages in thread
From: Nathan Chancellor @ 2020-05-30 3:00 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mauro Carvalho Chehab, Sakari Ailus, linux-media, devel,
linux-kernel, clang-built-linux
On Fri, May 29, 2020 at 10:00:29PM +0200, Arnd Bergmann wrote:
> Some function calls pass an incorrect enum type:
>
> drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:858:16: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
> gp_device_rst(INPUT_SYSTEM0_ID);
> ~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
> drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:860:19: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
> input_switch_rst(INPUT_SYSTEM0_ID);
> ~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
> drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:876:27: error: implicit conversion from enumeration type 'input_system_cfg_flag_t' to different enumeration type 'input_system_connection_t' [-Werror,-Wenum-conversion]
> config.multicast[i] = INPUT_SYSTEM_CFG_FLAG_RESET;
> ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:1326:32: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
> input_selector_cfg_for_sensor(INPUT_SYSTEM0_ID);
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
> drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:1329:19: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
> input_switch_cfg(INPUT_SYSTEM0_ID, &config.input_switch_cfg);
> ~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
>
> INPUT_SYSTEM0_ID is zero, so use the corresponding zero-value
> of the expected types instead.
>
> Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Huh weird that I did not see this warning but you do randconfigs so
that's expected.
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> .../pci/hive_isp_css_common/host/input_system.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
> index 2114cf4f3fda..aa0f0fca9346 100644
> --- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
> +++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
> @@ -855,9 +855,9 @@ input_system_error_t input_system_configuration_reset(void)
>
> input_system_network_rst(INPUT_SYSTEM0_ID);
>
> - gp_device_rst(INPUT_SYSTEM0_ID);
> + gp_device_rst(GP_DEVICE0_ID);
>
> - input_switch_rst(INPUT_SYSTEM0_ID);
> + input_switch_rst(GP_DEVICE0_ID);
>
> //target_rst();
>
> @@ -873,7 +873,7 @@ input_system_error_t input_system_configuration_reset(void)
>
> for (i = 0; i < N_CSI_PORTS; i++) {
> config.csi_buffer_flags[i] = INPUT_SYSTEM_CFG_FLAG_RESET;
> - config.multicast[i] = INPUT_SYSTEM_CFG_FLAG_RESET;
> + config.multicast[i] = INPUT_SYSTEM_DISCARD_ALL;
> }
>
> config.source_type_flags = INPUT_SYSTEM_CFG_FLAG_RESET;
> @@ -1323,10 +1323,10 @@ static input_system_error_t configuration_to_registers(void)
> } // end of switch (source_type)
>
> // Set input selector.
> - input_selector_cfg_for_sensor(INPUT_SYSTEM0_ID);
> + input_selector_cfg_for_sensor(GP_DEVICE0_ID);
>
> // Set input switch.
> - input_switch_cfg(INPUT_SYSTEM0_ID, &config.input_switch_cfg);
> + input_switch_cfg(GP_DEVICE0_ID, &config.input_switch_cfg);
>
> // Set input formatters.
> // AM: IF are set dynamically.
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 8/9] staging: media: atomisp: disable all custom formats
2020-05-29 20:00 [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check Arnd Bergmann
` (5 preceding siblings ...)
2020-05-29 20:00 ` [PATCH 7/9] staging: media: atomisp: fix enum type mixups Arnd Bergmann
@ 2020-05-29 20:00 ` Arnd Bergmann
2020-05-30 3:03 ` Nathan Chancellor
2020-05-29 20:00 ` [PATCH 9/9] staging: media: atomisp: add PMIC_OPREGION dependency Arnd Bergmann
2020-05-29 20:04 ` [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check Nick Desaulniers
8 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel,
clang-built-linux, Arnd Bergmann
clang points out the usage of an incorrect enum type in the
list of supported image formats:
drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:65: error: implicit conversion from enumeration type 'enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion]
{ V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 },
drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:39: error: implicit conversion from enumeration type 'enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion]
{ V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 },
{ V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, CSS_FRAME_FORMAT_NV12, 0, CSS_FRAME_FORMAT_NV12 },
{ MEDIA_BUS_FMT_JPEG_1X8, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0, ATOMISP_INPUT_FORMAT_BINARY_8 },
Checking the git history, I found a commit that disabled one such case
because it did not work. It seems likely that the incorrect enum was
part of the original problem and that the others do not work either,
or have never been tested.
Disable all the ones that cause a warning.
Fixes: cb02ae3d71ea ("media: staging: atomisp: Disable custom format for now")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/media/atomisp/pci/atomisp_subdev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
index 46590129cbe3..8bce466cc128 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
@@ -44,9 +44,11 @@ const struct atomisp_in_fmt_conv atomisp_in_fmt_conv[] = {
{ MEDIA_BUS_FMT_SRGGB12_1X12, 12, 12, ATOMISP_INPUT_FORMAT_RAW_12, CSS_BAYER_ORDER_RGGB, CSS_FORMAT_RAW_12 },
{ MEDIA_BUS_FMT_UYVY8_1X16, 8, 8, ATOMISP_INPUT_FORMAT_YUV422_8, 0, ATOMISP_INPUT_FORMAT_YUV422_8 },
{ MEDIA_BUS_FMT_YUYV8_1X16, 8, 8, ATOMISP_INPUT_FORMAT_YUV422_8, 0, ATOMISP_INPUT_FORMAT_YUV422_8 },
+#if 0
{ MEDIA_BUS_FMT_JPEG_1X8, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0, ATOMISP_INPUT_FORMAT_BINARY_8 },
{ V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, CSS_FRAME_FORMAT_NV12, 0, CSS_FRAME_FORMAT_NV12 },
{ V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 },
+#endif
{ V4L2_MBUS_FMT_CUSTOM_YUV420, 12, 12, ATOMISP_INPUT_FORMAT_YUV420_8_LEGACY, 0, ATOMISP_INPUT_FORMAT_YUV420_8_LEGACY },
#if 0
{ V4L2_MBUS_FMT_CUSTOM_M10MO_RAW, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0, ATOMISP_INPUT_FORMAT_BINARY_8 },
--
2.26.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 8/9] staging: media: atomisp: disable all custom formats
2020-05-29 20:00 ` [PATCH 8/9] staging: media: atomisp: disable all custom formats Arnd Bergmann
@ 2020-05-30 3:03 ` Nathan Chancellor
0 siblings, 0 replies; 21+ messages in thread
From: Nathan Chancellor @ 2020-05-30 3:03 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mauro Carvalho Chehab, Sakari Ailus, linux-media, devel,
linux-kernel, clang-built-linux
On Fri, May 29, 2020 at 10:00:30PM +0200, Arnd Bergmann wrote:
> clang points out the usage of an incorrect enum type in the
> list of supported image formats:
>
> drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:65: error: implicit conversion from enumeration type 'enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion]
> { V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 },
> drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:39: error: implicit conversion from enumeration type 'enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion]
> { V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 },
> { V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, CSS_FRAME_FORMAT_NV12, 0, CSS_FRAME_FORMAT_NV12 },
> { MEDIA_BUS_FMT_JPEG_1X8, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0, ATOMISP_INPUT_FORMAT_BINARY_8 },
>
> Checking the git history, I found a commit that disabled one such case
> because it did not work. It seems likely that the incorrect enum was
> part of the original problem and that the others do not work either,
> or have never been tested.
>
> Disable all the ones that cause a warning.
>
> Fixes: cb02ae3d71ea ("media: staging: atomisp: Disable custom format for now")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
I have this patch in my local tree and debated sending it myself. I
think that this is the right fix for now, as the driver is being cleaned
up. Maybe add a FIXME like the rest of this driver?
Regardless of that last point:
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> drivers/staging/media/atomisp/pci/atomisp_subdev.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> index 46590129cbe3..8bce466cc128 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
> @@ -44,9 +44,11 @@ const struct atomisp_in_fmt_conv atomisp_in_fmt_conv[] = {
> { MEDIA_BUS_FMT_SRGGB12_1X12, 12, 12, ATOMISP_INPUT_FORMAT_RAW_12, CSS_BAYER_ORDER_RGGB, CSS_FORMAT_RAW_12 },
> { MEDIA_BUS_FMT_UYVY8_1X16, 8, 8, ATOMISP_INPUT_FORMAT_YUV422_8, 0, ATOMISP_INPUT_FORMAT_YUV422_8 },
> { MEDIA_BUS_FMT_YUYV8_1X16, 8, 8, ATOMISP_INPUT_FORMAT_YUV422_8, 0, ATOMISP_INPUT_FORMAT_YUV422_8 },
> +#if 0
> { MEDIA_BUS_FMT_JPEG_1X8, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0, ATOMISP_INPUT_FORMAT_BINARY_8 },
> { V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, CSS_FRAME_FORMAT_NV12, 0, CSS_FRAME_FORMAT_NV12 },
> { V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 },
> +#endif
> { V4L2_MBUS_FMT_CUSTOM_YUV420, 12, 12, ATOMISP_INPUT_FORMAT_YUV420_8_LEGACY, 0, ATOMISP_INPUT_FORMAT_YUV420_8_LEGACY },
> #if 0
> { V4L2_MBUS_FMT_CUSTOM_M10MO_RAW, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0, ATOMISP_INPUT_FORMAT_BINARY_8 },
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 9/9] staging: media: atomisp: add PMIC_OPREGION dependency
2020-05-29 20:00 [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check Arnd Bergmann
` (6 preceding siblings ...)
2020-05-29 20:00 ` [PATCH 8/9] staging: media: atomisp: disable all custom formats Arnd Bergmann
@ 2020-05-29 20:00 ` Arnd Bergmann
2020-05-30 3:11 ` Nathan Chancellor
2020-05-29 20:04 ` [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check Nick Desaulniers
8 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel,
clang-built-linux, Arnd Bergmann
Without that driver, there is a link failure in
ERROR: modpost: "intel_soc_pmic_exec_mipi_pmic_seq_element"
[drivers/staging/media/atomisp/pci/atomisp_gmin_platform.ko] undefined!
Add an explicit Kconfig dependency.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/media/atomisp/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/media/atomisp/Kconfig b/drivers/staging/media/atomisp/Kconfig
index c4f3049b0706..e86311c14329 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -11,6 +11,7 @@ menuconfig INTEL_ATOMISP
config VIDEO_ATOMISP
tristate "Intel Atom Image Signal Processor Driver"
depends on VIDEO_V4L2 && INTEL_ATOMISP
+ depends on PMIC_OPREGION
select IOSF_MBI
select VIDEOBUF_VMALLOC
---help---
--
2.26.2
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 9/9] staging: media: atomisp: add PMIC_OPREGION dependency
2020-05-29 20:00 ` [PATCH 9/9] staging: media: atomisp: add PMIC_OPREGION dependency Arnd Bergmann
@ 2020-05-30 3:11 ` Nathan Chancellor
2020-05-30 5:25 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 21+ messages in thread
From: Nathan Chancellor @ 2020-05-30 3:11 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mauro Carvalho Chehab, Sakari Ailus, linux-media, devel,
linux-kernel, clang-built-linux
On Fri, May 29, 2020 at 10:00:31PM +0200, Arnd Bergmann wrote:
> Without that driver, there is a link failure in
>
> ERROR: modpost: "intel_soc_pmic_exec_mipi_pmic_seq_element"
> [drivers/staging/media/atomisp/pci/atomisp_gmin_platform.ko] undefined!
>
> Add an explicit Kconfig dependency.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
It'd be interesting to know if this is strictly required for the driver
to work properly. The call to intel_soc_pmic_exec_mipi_pmic_seq_element
has some error handling after it, maybe that should just be surrounded
by an #ifdef or IS_ENABLED for PMIC_OPREGION, like some other drivers
do.
Regardless of that:
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> drivers/staging/media/atomisp/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/media/atomisp/Kconfig b/drivers/staging/media/atomisp/Kconfig
> index c4f3049b0706..e86311c14329 100644
> --- a/drivers/staging/media/atomisp/Kconfig
> +++ b/drivers/staging/media/atomisp/Kconfig
> @@ -11,6 +11,7 @@ menuconfig INTEL_ATOMISP
> config VIDEO_ATOMISP
> tristate "Intel Atom Image Signal Processor Driver"
> depends on VIDEO_V4L2 && INTEL_ATOMISP
> + depends on PMIC_OPREGION
> select IOSF_MBI
> select VIDEOBUF_VMALLOC
> ---help---
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 9/9] staging: media: atomisp: add PMIC_OPREGION dependency
2020-05-30 3:11 ` Nathan Chancellor
@ 2020-05-30 5:25 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2020-05-30 5:25 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Arnd Bergmann, Sakari Ailus, linux-media, devel, linux-kernel,
clang-built-linux
Em Fri, 29 May 2020 20:11:29 -0700
Nathan Chancellor <natechancellor@gmail.com> escreveu:
> On Fri, May 29, 2020 at 10:00:31PM +0200, Arnd Bergmann wrote:
> > Without that driver, there is a link failure in
> >
> > ERROR: modpost: "intel_soc_pmic_exec_mipi_pmic_seq_element"
> > [drivers/staging/media/atomisp/pci/atomisp_gmin_platform.ko] undefined!
> >
> > Add an explicit Kconfig dependency.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> It'd be interesting to know if this is strictly required for the driver
> to work properly.
It is. Without OpRegion, the driver can't power on the camera sensors.
> The call to intel_soc_pmic_exec_mipi_pmic_seq_element
> has some error handling after it, maybe that should just be surrounded
> by an #ifdef or IS_ENABLED for PMIC_OPREGION, like some other drivers
> do.
>
> Regardless of that:
>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
>
> > ---
> > drivers/staging/media/atomisp/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/staging/media/atomisp/Kconfig b/drivers/staging/media/atomisp/Kconfig
> > index c4f3049b0706..e86311c14329 100644
> > --- a/drivers/staging/media/atomisp/Kconfig
> > +++ b/drivers/staging/media/atomisp/Kconfig
> > @@ -11,6 +11,7 @@ menuconfig INTEL_ATOMISP
> > config VIDEO_ATOMISP
> > tristate "Intel Atom Image Signal Processor Driver"
> > depends on VIDEO_V4L2 && INTEL_ATOMISP
> > + depends on PMIC_OPREGION
> > select IOSF_MBI
> > select VIDEOBUF_VMALLOC
> > ---help---
> > --
> > 2.26.2
> >
Thanks,
Mauro
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check
2020-05-29 20:00 [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check Arnd Bergmann
` (7 preceding siblings ...)
2020-05-29 20:00 ` [PATCH 9/9] staging: media: atomisp: add PMIC_OPREGION dependency Arnd Bergmann
@ 2020-05-29 20:04 ` Nick Desaulniers
2020-05-29 20:23 ` Arnd Bergmann
8 siblings, 1 reply; 21+ messages in thread
From: Nick Desaulniers @ 2020-05-29 20:04 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Mauro Carvalho Chehab, Sakari Ailus, linux-media, devel, LKML,
clang-built-linux, Nathan Chancellor
See also Nathan's 7 patch series.
https://lore.kernel.org/lkml/20200527071150.3381228-1-natechancellor@gmail.com/
Might be some overlap between series?
On Fri, May 29, 2020 at 1:00 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Checking the pointer to a member of a struct against NULL
> is pointless as clang points out:
>
> drivers/staging/media/atomisp/pci/atomisp_cmd.c:4278:17: error: address of 'config->info' will always evaluate to 'true'
>
> Check the original pointer instead, which may also be
> unnecessary here, but makes a little more sense.
>
> Fixes: 9d4fa1a16b28 ("media: atomisp: cleanup directory hierarchy")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/staging/media/atomisp/pci/atomisp_cmd.c | 2 +-
> drivers/staging/media/atomisp/pci/sh_css.c | 4 ++--
> drivers/staging/media/atomisp/pci/sh_css_sp.c | 4 ++--
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
> index 5be690f876c1..342fc3b34fe0 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
> @@ -4275,7 +4275,7 @@ int atomisp_param(struct atomisp_sub_device *asd, int flag,
> atomisp_css_get_dvs_grid_info(
> &asd->params.curr_grid_info);
>
> - if (!&config->info) {
> + if (!config) {
> dev_err(isp->dev, "ERROR: NULL pointer in grid_info\n");
> return -EINVAL;
> }
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> index d77432254a2c..e91c6029c651 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -8534,7 +8534,7 @@ ia_css_pipe_load_extension(struct ia_css_pipe *pipe,
>
> if (firmware->info.isp.type == IA_CSS_ACC_OUTPUT)
> {
> - if (&pipe->output_stage)
> + if (pipe)
> append_firmware(&pipe->output_stage, firmware);
> else {
> IA_CSS_LEAVE_ERR_PRIVATE(IA_CSS_ERR_INTERNAL_ERROR);
> @@ -8542,7 +8542,7 @@ ia_css_pipe_load_extension(struct ia_css_pipe *pipe,
> }
> } else if (firmware->info.isp.type == IA_CSS_ACC_VIEWFINDER)
> {
> - if (&pipe->vf_stage)
> + if (pipe)
> append_firmware(&pipe->vf_stage, firmware);
> else {
> IA_CSS_LEAVE_ERR_PRIVATE(IA_CSS_ERR_INTERNAL_ERROR);
> diff --git a/drivers/staging/media/atomisp/pci/sh_css_sp.c b/drivers/staging/media/atomisp/pci/sh_css_sp.c
> index e574396ad0f4..c0e579c1705f 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css_sp.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css_sp.c
> @@ -1022,7 +1022,7 @@ sh_css_sp_init_stage(struct ia_css_binary *binary,
> if (!pipe)
> return IA_CSS_ERR_INTERNAL_ERROR;
> ia_css_get_crop_offsets(pipe, &args->in_frame->info);
> - } else if (&binary->in_frame_info)
> + } else if (binary)
> {
> pipe = find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num);
> if (!pipe)
> @@ -1036,7 +1036,7 @@ sh_css_sp_init_stage(struct ia_css_binary *binary,
> if (!pipe)
> return IA_CSS_ERR_INTERNAL_ERROR;
> ia_css_get_crop_offsets(pipe, &args->in_frame->info);
> - } else if (&binary->in_frame_info) {
> + } else if (binary) {
> pipe = find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num);
> if (!pipe)
> return IA_CSS_ERR_INTERNAL_ERROR;
> --
> 2.26.2
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200529200031.4117841-1-arnd%40arndb.de.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check
2020-05-29 20:04 ` [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check Nick Desaulniers
@ 2020-05-29 20:23 ` Arnd Bergmann
2020-05-29 20:31 ` Arnd Bergmann
0 siblings, 1 reply; 21+ messages in thread
From: Arnd Bergmann @ 2020-05-29 20:23 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Mauro Carvalho Chehab, Sakari Ailus, Linux Media Mailing List,
driverdevel, LKML, clang-built-linux, Nathan Chancellor
On Fri, May 29, 2020 at 10:04 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> See also Nathan's 7 patch series.
> https://lore.kernel.org/lkml/20200527071150.3381228-1-natechancellor@gmail.com/
>
> Might be some overlap between series?
>
Probably. I really should have checked when I saw the number of warnings.
At least this gives Mauro a chance to double-check the changes and see if
Nathan and I came to different conclusions on any of them.
Arnd
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check
2020-05-29 20:23 ` Arnd Bergmann
@ 2020-05-29 20:31 ` Arnd Bergmann
2020-05-30 2:49 ` Nathan Chancellor
2020-05-30 9:22 ` Mauro Carvalho Chehab
0 siblings, 2 replies; 21+ messages in thread
From: Arnd Bergmann @ 2020-05-29 20:31 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Mauro Carvalho Chehab, Sakari Ailus, Linux Media Mailing List,
driverdevel, LKML, clang-built-linux, Nathan Chancellor
On Fri, May 29, 2020 at 10:23 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Fri, May 29, 2020 at 10:04 PM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@googlegroups.com> wrote:
> >
> > See also Nathan's 7 patch series.
> > https://lore.kernel.org/lkml/20200527071150.3381228-1-natechancellor@gmail.com/
> >
> > Might be some overlap between series?
> >
>
> Probably. I really should have checked when I saw the number of warnings.
>
> At least this gives Mauro a chance to double-check the changes and see if
> Nathan and I came to different conclusions on any of them.
I checked now and found that the overlap is smaller than I expected.
In each case, Nathans' solution seems more complete than mine,
so this patch ("staging: media: atomisp: fix incorrect NULL pointer check")
and also "staging: media: atomisp: fix a type conversion warning" can be
dropped, but I think the others are still needed.
Arnd
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check
2020-05-29 20:31 ` Arnd Bergmann
@ 2020-05-30 2:49 ` Nathan Chancellor
2020-05-30 9:22 ` Mauro Carvalho Chehab
1 sibling, 0 replies; 21+ messages in thread
From: Nathan Chancellor @ 2020-05-30 2:49 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Nick Desaulniers, Mauro Carvalho Chehab, Sakari Ailus,
Linux Media Mailing List, driverdevel, LKML, clang-built-linux
On Fri, May 29, 2020 at 10:31:44PM +0200, Arnd Bergmann wrote:
> On Fri, May 29, 2020 at 10:23 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Fri, May 29, 2020 at 10:04 PM 'Nick Desaulniers' via Clang Built
> > Linux <clang-built-linux@googlegroups.com> wrote:
> > >
> > > See also Nathan's 7 patch series.
> > > https://lore.kernel.org/lkml/20200527071150.3381228-1-natechancellor@gmail.com/
> > >
> > > Might be some overlap between series?
> > >
> >
> > Probably. I really should have checked when I saw the number of warnings.
> >
> > At least this gives Mauro a chance to double-check the changes and see if
> > Nathan and I came to different conclusions on any of them.
>
> I checked now and found that the overlap is smaller than I expected.
> In each case, Nathans' solution seems more complete than mine,
> so this patch ("staging: media: atomisp: fix incorrect NULL pointer check")
> and also "staging: media: atomisp: fix a type conversion warning" can be
> dropped, but I think the others are still needed.
>
> Arnd
Thanks for double checking! I will read through the rest of the series
and review as I can.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check
2020-05-29 20:31 ` Arnd Bergmann
2020-05-30 2:49 ` Nathan Chancellor
@ 2020-05-30 9:22 ` Mauro Carvalho Chehab
1 sibling, 0 replies; 21+ messages in thread
From: Mauro Carvalho Chehab @ 2020-05-30 9:22 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Nick Desaulniers, Sakari Ailus, Linux Media Mailing List,
driverdevel, LKML, clang-built-linux, Nathan Chancellor
Em Fri, 29 May 2020 22:31:44 +0200
Arnd Bergmann <arnd@arndb.de> escreveu:
> On Fri, May 29, 2020 at 10:23 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Fri, May 29, 2020 at 10:04 PM 'Nick Desaulniers' via Clang Built
> > Linux <clang-built-linux@googlegroups.com> wrote:
> > >
> > > See also Nathan's 7 patch series.
> > > https://lore.kernel.org/lkml/20200527071150.3381228-1-natechancellor@gmail.com/
> > >
> > > Might be some overlap between series?
> > >
> >
> > Probably. I really should have checked when I saw the number of warnings.
> >
> > At least this gives Mauro a chance to double-check the changes and see if
> > Nathan and I came to different conclusions on any of them.
>
> I checked now and found that the overlap is smaller than I expected.
> In each case, Nathans' solution seems more complete than mine,
> so this patch ("staging: media: atomisp: fix incorrect NULL pointer check")
> and also "staging: media: atomisp: fix a type conversion warning" can be
> dropped, but I think the others are still needed.
Hi Arnd,
I applied most of the patches from you. I had to add two extra patches
before this one:
[PATCH 5/9] staging: media: atomisp: fix stack overflow in init_pipe_defaults()
And rebase it, because otherwise gcc would fail to compile here.
I'm placing the patches I have so far ready for atomisp on this
tree:
https://git.linuxtv.org/mchehab/media-next.git/log/
Thanks,
Mauro
^ permalink raw reply [flat|nested] 21+ messages in thread