[PATCH 7/9] staging/atomisp: add ACPI dependency
Without ACPI, some of the code fails to build:
media/atomisp/platform/intel-mid/atomisp_gmin_platform.c: In function
'atomisp_register_i2c_module':
media/atomisp/platform/intel-mid/atomisp_gmin_platform.c:174:7: error:
dereferencing pointer to incomplete type 'struct acpi_device'
We could work around that in the code, but since we already have a hard
dependency on x86, adding the ACPI dependency seems to be the easiest
solution.
Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/atomisp/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/Kconfig
b/drivers/staging/media/atomisp/Kconfig
index 3af2acdc7e96..97ffa2fc5384 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -1,6 +1,6 @@
menuconfig INTEL_ATOMISP
bool "Enable support to Intel MIPI camera drivers"
-depends on X86 && PCI
+depends on X86 && PCI && ACPI
help
Enable support for the Intel ISP2 camera interfaces and MIPI
sensor drivers.
--
2.9.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/9] staging/atomisp: add PCI dependency
Without CONFIG_PCI, config space reads never return any data,
leading to undefined behavior that gcc warns about:
platform/intel-mid/intel_mid_pcihelpers.c: In function
'intel_mid_msgbus_read32_raw':
platform/intel-mid/intel_mid_pcihelpers.c:66:9: error: 'data' is used
uninitialized in this function [-Werror=uninitialized]
platform/intel-mid/intel_mid_pcihelpers.c: In function
'intel_mid_msgbus_read32_raw_ext':
platform/intel-mid/intel_mid_pcihelpers.c:84:9: error: 'data' is used
uninitialized in this function [-Werror=uninitialized]
platform/intel-mid/intel_mid_pcihelpers.c: In function
'intel_mid_msgbus_read32':
platform/intel-mid/intel_mid_pcihelpers.c:137:9: error: 'data' is used
uninitialized in this function [-Werror=uninitialized]
With a dependency on CONFIG_PCI, we don't get this warning. This seems
safe as PCI config space accessors should always return something
when PCI is enabled.
Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/atomisp/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/Kconfig
b/drivers/staging/media/atomisp/Kconfig
index f7d8a841c629..3af2acdc7e96 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -1,6 +1,6 @@
menuconfig INTEL_ATOMISP
bool "Enable support to Intel MIPI camera drivers"
-depends on X86
+depends on X86 && PCI
help
Enable support for the Intel ISP2 camera interfaces and MIPI
sensor drivers.
--
2.9.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/9] stating/atomisp: fix -Wold-style-definition warning
ia_css_dequeue_param_buffers does not have an arguement type, causing a warning:
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c: In function
'ia_css_dequeue_param_buffers':
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c:3728:6:
error: old-style function definition [-Werror=old-style-definition]
This adds a 'void' keywork to silence the warning.
Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c
index 9d51f1c653a2..1f346394c6b1 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c
@@ -3725,7 +3725,7 @@ static void sh_css_update_isp_mem_params_to_ddr(
IA_CSS_LEAVE_PRIVATE("void");
}
-void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/)
+void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/ void)
{
unsigned int i;
hrt_vaddress cpy;
--
2.9.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/9] staging/atomisp: fix empty-body warning
Defining a debug function to nothing causes a warning with an empty block
after if()/else():
drivers/staging/media/atomisp/i2c/ov2680.c: In function 'ov2680_s_stream':
drivers/staging/media/atomisp/i2c/ov2680.c:1208:55: error: suggest braces
around empty body in an 'else' statement [-Werror=empty-body]
This changes the empty debug statement to dev_dbg(), which by default also
does nothing, but avoids this warning and also checks the format string.
As a side-effect, we can now use dynamic debugging to turn on the
output at runtime.
Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/atomisp/i2c/ov2680.c | 37 +++---
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/ov2680.c
b/drivers/staging/media/atomisp/i2c/ov2680.c
index 58d2a075d436..c08dd0b18fbb 100644
--- a/drivers/staging/media/atomisp/i2c/ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/ov2680.c
@@ -35,7 +35,6 @@
#include "ov2680.h"
-#define ov2680_debug(...) //dev_err(__VA_ARGS__)
static int h_flag = 0;
static int v_flag = 0;
static enum atomisp_bayer_order ov2680_bayer_order_mapping[] = {
@@ -99,7 +98,7 @@ static int ov2680_read_reg(struct i2c_client *client,
*val = be16_to_cpu(*(u16 *)&data[0]);
else
*val = be32_to_cpu(*(u32 *)&data[0]);
- //ov2680_debug(&client->dev, "i2c read adr%x = %x\n", reg,*val);
+ //dev_dbg(&client->dev, "i2c read adr%x = %x\n", reg,*val);
return 0;
}
@@ -114,7 +113,7 @@ static int ov2680_i2c_write(struct i2c_client *client, u16
len, u8 *data)
msg.len = len;
msg.buf = data;
ret = i2c_transfer(client->adapter, &msg, 1);
- //ov2680_debug(&client->dev, "+++i2c write reg=%x->%x\n", data[0]*256
+data[1],data[2]);
+ //dev_dbg(&client->dev, "+++i2c write reg=%x->%x\n", data[0]*256
+data[1],data[2]);
return ret == num_msg ? 0 : -EIO;
}
@@ -235,7 +234,7 @@ static int ov2680_write_reg_array(struct i2c_client *client,
const struct ov2680_reg *next = reglist;
struct ov2680_write_ctrl ctrl;
int err;
- ov2680_debug(&client->dev, "write reg array\n");
+ dev_dbg(&client->dev, "write reg array\n");
ctrl.index = 0;
for (; next->type != OV2680_TOK_TERM; next++) {
switch (next->type & OV2680_TOK_MASK) {
@@ -250,7 +249,7 @@ static int ov2680_write_reg_array(struct i2c_client *client,
* If next address is not consecutive, data needs to be
* flushed before proceed.
*/
-ov2680_debug(&client->dev, "+++ov2680_write_reg_array
reg=%x->%x\n", next->reg,next->val);
+dev_dbg(&client->dev, "+++ov2680_write_reg_array
reg=%x->%x\n", next->reg,next->val);
if (!__ov2680_write_reg_is_consecutive(client, &ctrl,
next)) {
err = __ov2680_flush_reg_array(client, &ctrl);
@@ -296,7 +295,8 @@ static int ov2680_g_fnumber_range(struct v4l2_subdev *sd,
s32 *val)
static int ov2680_g_bin_factor_x(struct v4l2_subdev *sd, s32 *val)
{
struct ov2680_device *dev = to_ov2680_sensor(sd);
- ov2680_debug(dev, "ov2680_g_bin_factor_x\n");
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ dev_dbg(&client->dev, "ov2680_g_bin_factor_x\n");
*val = ov2680_res[dev->fmt_idx].bin_factor_x;
return 0;
@@ -305,9 +305,10 @@ static int ov2680_g_bin_factor_x(struct v4l2_subdev *sd,
s32 *val)
static int ov2680_g_bin_factor_y(struct v4l2_subdev *sd, s32 *val)
{
struct ov2680_device *dev = to_ov2680_sensor(sd);
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
*val = ov2680_res[dev->fmt_idx].bin_factor_y;
- ov2680_debug(dev, "ov2680_g_bin_factor_y\n");
+ dev_dbg(&client->dev, "ov2680_g_bin_factor_y\n");
return 0;
}
@@ -322,7 +323,7 @@ static int ov2680_get_intg_factor(struct i2c_client *client,
unsigned int pix_clk_freq_hz;
u16 reg_val;
int ret;
- ov2680_debug(dev, "ov2680_get_intg_factor\n");
+ dev_dbg(&client->dev, "ov2680_get_intg_factor\n");
if (!info)
return -EINVAL;
@@ -399,7 +400,7 @@ static long __ov2680_set_exposure(struct v4l2_subdev *sd,
int coarse_itg,
u16 vts,hts;
int ret,exp_val;
- ov2680_debug(dev, "+++__ov2680_set_exposure coarse_itg %d, gain %d,
digitgain %d++\n",coarse_itg, gain, digitgain);
+ dev_dbg(&client->dev, "+++__ov2680_set_exposure coarse_itg %d, gain
%d, digitgain %d++\n",coarse_itg, gain, digitgain);
hts = ov2680_res[dev->fmt_idx].pixels_per_line;
vts = ov2680_res[dev->fmt_idx].lines_per_frame;
@@ -
[PATCH 1/9] staging/atomisp: include linux/io.h where needed
The plat_clock implementation fails ot build in some configurations:
platform/clock/vlv2_plat_clock.c: In function 'vlv2_plat_set_clock_freq':
platform/clock/vlv2_plat_clock.c:88:2: error: implicit declaration of function
'writel';did you mean 'wrmsrl'? [-Werror=implicit-function-declaration]
platform/clock/vlv2_plat_clock.c:88:12: error: implicit declaration of function
'readl' [-Werror=implicit-function-declaration]
platform/clock/vlv2_plat_clock.c: In function 'vlv2_plat_clk_probe':
platform/clock/vlv2_plat_clock.c:193:13: error: implicit declaration of
function 'ioremap_nocache' [-Werror=implicit-function-declaration]
platform/clock/vlv2_plat_clock.c:193:11: error: assignment makes pointer from
integer without a cast [-Werror=int-conversion]
platform/clock/vlv2_plat_clock.c: In function 'vlv2_plat_clk_remove':
platform/clock/vlv2_plat_clock.c:209:2: error: implicit declaration of function
'iounmap' [-Werror=implicit-function-declaration]
This includes the required header file.
Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
b/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
index a8ca93dbfbb5..25e939c50aef 100644
--- a/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
+++ b/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c
@@ -20,6 +20,7 @@
*/
#include
+#include
#include
#include
#include "../../include/linux/vlv2_plat_clock.h"
--
2.9.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/9] staging/atomisp: remove sh_css_lace_stat code
I ran into a build warning on my randconfig build box:
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c: In function
'ia_css_lace_statistics_free':
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_params.c:2845:64:
error: parameter 'me' set but not used [-Werror=unused-but-set-parameter]
It turns out that not only the parameter is unused but the entire function has
no
caller. Let's just remove it.
Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann
---
.../staging/media/atomisp/pci/atomisp2/Makefile| 1 -
.../media/atomisp/pci/atomisp2/css2400/ia_css.h| 1 -
.../atomisp/pci/atomisp2/css2400/ia_css_buffer.h | 1 -
.../pci/atomisp2/css2400/ia_css_lace_stat.h| 37 --
.../atomisp/pci/atomisp2/css2400/sh_css_internal.h | 1 -
.../pci/atomisp2/css2400/sh_css_lace_stat.c| 16 --
.../atomisp/pci/atomisp2/css2400/sh_css_params.c | 15 -
7 files changed, 72 deletions(-)
delete mode 100644
drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_lace_stat.h
delete mode 100644
drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_lace_stat.c
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/Makefile
b/drivers/staging/media/atomisp/pci/atomisp2/Makefile
index f538e56ed1a7..463f84cca4d8 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/Makefile
+++ b/drivers/staging/media/atomisp/pci/atomisp2/Makefile
@@ -108,7 +108,6 @@ atomisp-objs += \
css2400/isp/kernels/ipu2_io_ls/bayer_io_ls/ia_css_bayer_io.host.o \
css2400/isp/kernels/ipu2_io_ls/yuv444_io_ls/ia_css_yuv444_io.host.o \
css2400/isp/kernels/s3a/s3a_1.0/ia_css_s3a.host.o \
- css2400/sh_css_lace_stat.o \
css2400/sh_css_pipe.o \
css2400/ia_css_device_access.o \
css2400/sh_css_host_data.o \
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css.h
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css.h
index f67626f5258c..2458b3767c90 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css.h
@@ -42,7 +42,6 @@
#include "ia_css_stream_format.h"
#include "ia_css_stream_public.h"
#include "ia_css_tpg.h"
-#include "ia_css_lace_stat.h"
#include "ia_css_version.h"
#include "ia_css_mmu.h"
#include "ia_css_morph.h"
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_buffer.h
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_buffer.h
index 26b16f469042..b2ecf3618c15 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_buffer.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_buffer.h
@@ -60,7 +60,6 @@ struct ia_css_buffer {
struct ia_css_isp_3a_statistics *stats_3a;/**< 3A
statistics & optionally RGBY statistics. */
struct ia_css_isp_dvs_statistics *stats_dvs; /**< DVS
statistics. */
struct ia_css_isp_skc_dvs_statistics *stats_skc_dvs; /**< SKC
DVS statistics. */
- struct ia_css_isp_lace_statistics *stats_lace; /**< LACE
statistics. */
struct ia_css_frame *frame; /**< Frame
buffer. */
struct ia_css_acc_param *custom_data; /**< Custom
buffer. */
struct ia_css_metadata *metadata;/**< Sensor
metadata. */
diff --git
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_lace_stat.h
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_lace_stat.h
deleted file mode 100644
index 6fee1e200a8a..
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_lace_stat.h
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * Support for Intel Camera Imaging ISP subsystem.
- * Copyright (c) 2015, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
- */
-
-#ifndef __IA_CSS_LACE_STAT_H
-#define __IA_CSS_LACE_STAT_H
-
-/** @file
- * This file contains types used for LACE statistics
- */
-
-struct ia_css_isp_lace_statistics;
-
-/** @brief Allocate mem for the LACE statistics on the ISP
- * @return Pointer to the allocated LACE statistics
- * buffer on the ISP
-*/
-struct ia_css_isp_lace_statistics *ia_css_lace_statistics_allocate(void);
-
-/** @brief Free the ACC LACE statistics memory on the isp
- * @param[in] me Pointer to the LACE statistics buffer on the
- * ISP.
- * @return None
-*/
-void ia_css_lace_statistics_free(struct ia_css_isp_lace_statistics *me);
-
-#endif /* __I
[PATCH 9/9] staging/atomisp: add EFI dependency
Without CONFIG_EFI, the driver fails to call efivar_entry_get:
drivers/staging/built-in.o: In function `gmin_get_config_var':
(.text+0x1e3b): undefined reference to `efivar_entry_get'
Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/atomisp/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/Kconfig
b/drivers/staging/media/atomisp/Kconfig
index f24ae1c8cc90..e0ae0c93f800 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -1,6 +1,6 @@
menuconfig INTEL_ATOMISP
bool "Enable support to Intel MIPI camera drivers"
-depends on X86 && PCI && ACPI && MEDIA_CONTROLLER
+depends on X86 && PCI && ACPI && EFI && MEDIA_CONTROLLER
help
Enable support for the Intel ISP2 camera interfaces and MIPI
sensor drivers.
--
2.9.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/9] staging/atomisp: add VIDEO_V4L2_SUBDEV_API dependency
The driver fails to build if this is disabled, so we need an explicit
Kconfig dependency:
drivers/staging/media/atomisp/pci/atomisp2/./atomisp_cmd.c:6085:48: error:
'struct v4l2_subdev_fh' has no member named 'pad'
Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/atomisp/pci/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/Kconfig
b/drivers/staging/media/atomisp/pci/Kconfig
index e8f67835d03d..a72421431c7a 100644
--- a/drivers/staging/media/atomisp/pci/Kconfig
+++ b/drivers/staging/media/atomisp/pci/Kconfig
@@ -4,7 +4,7 @@
config VIDEO_ATOMISP
tristate "Intel Atom Image Signal Processor Driver"
- depends on VIDEO_V4L2
+ depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
select VIDEOBUF_VMALLOC
---help---
Say Y here if your platform supports Intel Atom SoC
--
2.9.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 8/9] staging/atomisp: add MEDIA_CONTROLLER dependency globally
One i2c driver already gained a dependency, but the others are equally broken:
drivers/staging/media/atomisp/i2c/ap1302.c: In function 'ap1302_remove':
drivers/staging/media/atomisp/i2c/ap1302.c:1143:31: error: 'struct v4l2_subdev'
has no member named 'entity'
drivers/staging/media/atomisp/i2c/mt9m114.c: In function 'mt9m114_remove':
drivers/staging/media/atomisp/i2c/mt9m114.c:1850:31: error: 'struct
v4l2_subdev' has no member named 'entity'
drivers/staging/media/atomisp/i2c/gc0310.c: In function 'gc0310_remove':
drivers/staging/media/atomisp/i2c/gc0310.c:1372:31: error: 'struct v4l2_subdev'
has no member named 'entity'
drivers/staging/media/atomisp/i2c/gc0310.c: In function 'gc0310_probe':
drivers/staging/media/atomisp/i2c/gc0310.c:1422:9: error: 'struct v4l2_subdev'
has no member named 'entity'
drivers/staging/media/atomisp/i2c/ov2722.c: In function 'ov2722_remove':
drivers/staging/media/atomisp/i2c/ov2722.c:1253:31: error: 'struct v4l2_subdev'
has no member named 'entity'
Let's just require MEDIA_CONTROLLER for all of them.
Fixes: dd1c0f278b0e ("staging: media: atomisp: fix build error in ov5693
driver")
Signed-off-by: Arnd Bergmann
---
drivers/staging/media/atomisp/Kconfig| 2 +-
drivers/staging/media/atomisp/i2c/ov5693/Kconfig | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/atomisp/Kconfig
b/drivers/staging/media/atomisp/Kconfig
index 97ffa2fc5384..f24ae1c8cc90 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -1,6 +1,6 @@
menuconfig INTEL_ATOMISP
bool "Enable support to Intel MIPI camera drivers"
-depends on X86 && PCI && ACPI
+depends on X86 && PCI && ACPI && MEDIA_CONTROLLER
help
Enable support for the Intel ISP2 camera interfaces and MIPI
sensor drivers.
diff --git a/drivers/staging/media/atomisp/i2c/ov5693/Kconfig
b/drivers/staging/media/atomisp/i2c/ov5693/Kconfig
index 3954b8c65fd1..9fb1bffbe9b3 100644
--- a/drivers/staging/media/atomisp/i2c/ov5693/Kconfig
+++ b/drivers/staging/media/atomisp/i2c/ov5693/Kconfig
@@ -1,6 +1,6 @@
config VIDEO_OV5693
tristate "Omnivision ov5693 sensor support"
- depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+ depends on I2C && VIDEO_V4L2
---help---
This is a Video4Linux2 sensor-level driver for the Micron
ov5693 5 Mpixel camera.
--
2.9.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vc04_services: make BCM_VIDEOCORE tristate
Adding the 'bool' symbol brought back a randconfig build bug that
I had fixed before:
drivers/staging/built-in.o: In function `vchiq_probe':
(.text+0x1da30): undefined reference to `rpi_firmware_get'
drivers/staging/built-in.o: In function `vchiq_platform_init':
(.text+0x27494): undefined reference to `rpi_firmware_property'
The problem is that when RASPBERRYPI_FIRMWARE is a loadable module,
but BCM2835_VCHIQ can again be built-in. Making BCM_VIDEOCORE
itself tristate will make Kconfig honor the dependency correctly.
Fixes: 6bbfe4a76158 ("staging: vc04_services: Create new BCM_VIDEOCORE setting
for VideoCore services.")
Signed-off-by: Arnd Bergmann
---
drivers/staging/vc04_services/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/vc04_services/Kconfig
b/drivers/staging/vc04_services/Kconfig
index f905df2275cf..9e2763663ab8 100644
--- a/drivers/staging/vc04_services/Kconfig
+++ b/drivers/staging/vc04_services/Kconfig
@@ -1,5 +1,5 @@
menuconfig BCM_VIDEOCORE
- bool "Broadcom VideoCore support"
+ tristate "Broadcom VideoCore support"
depends on HAS_DMA
depends on OF
depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST &&
!RASPBERRYPI_FIRMWARE)
--
2.9.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: outreachy
Hi! > > > Hah! That's the joy of being a maintainer of a driver in staging. Even > > > if you filter out outreachy, you are going to get a lot of "basic > > > mistakes" and other type patches cc:ed to you. > > > > > > I strongly suggest, that if you all don't like this type of stuff, > > > either: > > > - work to get the code out of staging as soon as possible (i.e. > > > send me coding style fixes for everything right now, and then > > > fix up the rest of the stuff.) > > > - take yourself off the maintainer list for this code. > > > > > > It's your choice, outreachy right now is a lot of patches, but again, > > > it's not going to keep you from getting the "basic" stuff sent to you > > > in ways that is totally wrong. > > > > Could we get these trivial patches off the lkml? Yes, lkml already has > > a lot of traffic, but no, this is not useful :-(. > > The outreachy instructions say to use the -nol argument to > get_maintainers, which would prevent them from being sent to any mailing > list. However others thought that all patches should be sent to mailing > lists, and so I haven't enforced anything for people who have omitted > -nol. However I have tried to remove bcm maintainers from CC lists on > replies and reminded people not to send you patches, Wonderful :-(. Can we at least make those people put the word "outreachy" in the subject so the emails are easier to delete? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: outreachy
On Mon, Mar 20, 2017 at 11:20:32AM +0100, Pavel Machek wrote: > Hi! > > > > > Hah! That's the joy of being a maintainer of a driver in staging. Even > > > > if you filter out outreachy, you are going to get a lot of "basic > > > > mistakes" and other type patches cc:ed to you. > > > > > > > > I strongly suggest, that if you all don't like this type of stuff, > > > > either: > > > > - work to get the code out of staging as soon as possible (i.e. > > > > send me coding style fixes for everything right now, and then > > > > fix up the rest of the stuff.) > > > > - take yourself off the maintainer list for this code. > > > > > > > > It's your choice, outreachy right now is a lot of patches, but again, > > > > it's not going to keep you from getting the "basic" stuff sent to you > > > > in ways that is totally wrong. > > > > > > Could we get these trivial patches off the lkml? Yes, lkml already has > > > a lot of traffic, but no, this is not useful :-(. > > > > The outreachy instructions say to use the -nol argument to > > get_maintainers, which would prevent them from being sent to any mailing > > list. However others thought that all patches should be sent to mailing > > lists, and so I haven't enforced anything for people who have omitted > > -nol. However I have tried to remove bcm maintainers from CC lists on > > replies and reminded people not to send you patches, > > Wonderful :-(. > > Can we at least make those people put the word "outreachy" in the > subject so the emails are easier to delete? It's easy for you to filter away as-is if you want to to by just looking at the cc: list for outreachy right now, don't make a new step for people to jump through because you don't want to be bothered. You only have 10 more days of it if you want to just ignore any patch until then... greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] bcm2835-gpio-exp: Driver for GPIO expander via mailbox service
On Sat, Mar 18, 2017 at 09:42:43PM -0700, Michael Zoran wrote: > On Sat, 2017-03-18 at 14:23 +0100, Stefan Wahren wrote: > > > Michael Zoran hat am 17. März 2017 um 16:22 > > > geschrieben: > > > > > > > > > From: Dave Stevenson > > > > > > Pi3 and Compute Module 3 have a GPIO expander that the > > > VPU communicates with. > > > There is a mailbox service that now allows control of this > > > expander, so add a kernel driver that can make use of it. > > > > > > Pwr_led node added to device-tree for Pi3. > > > > Looks like debris > > > > > > > > Signed-off-by: Dave Stevenson > > > > > > Stripped off changes to Makefile and Kconfig > > > Also stripped off changes to raspberrypi-firmware.h > > > Moved to drivers/staging/vc04_services/bcm2835-firmware-gpio > > > > > > Signed-off-by: Michael Zoran > > > --- > > > > > Hi Stefan, > > For this particular patch, I simply cherry-picked the orignal version > of the driver in the github directory. It's a 100% original copy of > the initial version. > > So any grips/complaints should be sent to dave.steven...@raspberrypi.org. Nah. Fix all the things Stefan mentioned and resend. Just mention in the changelog that you made a few changes yourself from Dave's original. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] staging: bcm2835-firmware-gpio: Add a build system for the driver
On Sat, Mar 18, 2017 at 10:40:06PM -0700, Michael Zoran wrote: > On Sun, 2017-03-19 at 13:27 +0800, kbuild test robot wrote: > > Hi Michael, > > > > [auto build test WARNING on staging/staging-testing] > > [also build test WARNING on next-20170310] > > [cannot apply to linux-rpi/for-rpi-next v4.11-rc2] > > [if your patch is applied to the wrong git tree, please drop us a > > note to help improve the system] > > > > url: https://github.com/0day-ci/linux/commits/Michael-Zoran/stagin > > g-bcm2835-firmware-gpio-Initial-staging-commit/20170319-14 > > > > > > coccinelle warnings: (new ones prefixed by >>) > > > > > > drivers/staging/vc04_services/bcm2835-firmware-gpio/gpio-bcm- > > > > exp.c:271:3-8: No need to set .owner here. The core will do it. > > > > > Please review and possibly fold the followup patch. > > > > --- > > 0-DAY kernel test infrastructure Open Source > > Technology Center > > https://lists.01.org/pipermail/kbuild-all Intel > > Corporation > > OK, Some I'm completely confused here. Has this patch been applied, > and through which tree has it been applied to? > > I think I need to send a V2. So who should I be sending V2 to? kbuild grabs patches off the mailing list and automatically tests them. They haven't been applied. Just resend a normal v2 series. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/5] staging: vc04_services: camera driver maintance
I'm not going to review this because it has kbuild errors. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] staging: bcm2835-firmware-gpio: Add a build system for the driver
On Mon, 2017-03-20 at 13:43 +0300, Dan Carpenter wrote: > On Sat, Mar 18, 2017 at 10:40:06PM -0700, Michael Zoran wrote: > > On Sun, 2017-03-19 at 13:27 +0800, kbuild test robot wrote: > > > Hi Michael, > > > > > > [auto build test WARNING on staging/staging-testing] > > > [also build test WARNING on next-20170310] > > > [cannot apply to linux-rpi/for-rpi-next v4.11-rc2] > > > [if your patch is applied to the wrong git tree, please drop us a > > > note to help improve the system] > > > > > > url: https://github.com/0day-ci/linux/commits/Michael-Zoran/st > > > agin > > > g-bcm2835-firmware-gpio-Initial-staging-commit/20170319-14 > > > > > > > > > coccinelle warnings: (new ones prefixed by >>) > > > > > > > > drivers/staging/vc04_services/bcm2835-firmware-gpio/gpio-bcm- > > > > > exp.c:271:3-8: No need to set .owner here. The core will do > > > > > it. > > > Please review and possibly fold the followup patch. > > > > > > --- > > > 0-DAY kernel test infrastructure Open Source > > > Technology Center > > > https://lists.01.org/pipermail/kbuild-all Intel > > > Corporation > > > > OK, Some I'm completely confused here. Has this patch been > > applied, > > and through which tree has it been applied to? > > > > I think I need to send a V2. So who should I be sending V2 to? > > kbuild grabs patches off the mailing list and automatically tests > them. > > They haven't been applied. Just resend a normal v2 series. > > regards, > dan carpenter > Actually, Stefan and Eric I think this driver has more of your interest. I really don't know the ins and out of the DT world, and that's most of what it's going to take to make this happen. Perhaps it would be better for either of you to be the primary drivers of making this happen? I'm not sure this driver fits well in with VideoCore which is more my interest anyway which I think is becoming clearer that it's about vchiq with all the faults that implies. Things that are half DT don't seem to fit in well. It sounds like you guys want to fast track this driver out of staging as soon as possible. Or perhaps it shouldn't go through staging at all since the original code is rather clear to begin with. Everyone know where the original source of the driver is now, so no reason anybody even needs to start with my changes. Thanks. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver
Em Sun, 19 Mar 2017 22:11:07 -0300 Mauro Carvalho Chehab escreveu: > Em Sun, 19 Mar 2017 10:04:28 -0700 > Michael Zoran escreveu: > > > A working DT that I tried this morning with the current firmware is > > posted here: > > http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-March/005924 > > .html > > > > It even works with minecraft_pi! With the new firmware, sometime after booting, I'm getting an oops, caused by bcm2835_audio/vchiq: [ 298.788995] Unable to handle kernel NULL pointer dereference at virtual address 0034 [ 298.821458] pgd = ed004000 [ 298.832294] [0034] *pgd=2e5e9835, *pte=, *ppte= [ 298.857450] Internal error: Oops: 17 [#1] SMP ARM [ 298.876273] Modules linked in: cfg80211 hid_logitech_hidpp hid_logitech_dj snd_bcm2835(C) snd_pcm snd_timer snd soundcore vchiq(C) uio_pdrv_genirq uio fuse [ 298.932064] CPU: 3 PID: 847 Comm: pulseaudio Tainted: G C 4.11.0-rc1+ #56 [ 298.963774] Hardware name: Generic DT based system [ 298.982945] task: ef758580 task.stack: ee4c6000 [ 299.001080] PC is at mutex_lock+0x14/0x3c [ 299.017148] LR is at vchiq_add_service_internal+0x138/0x3a0 [vchiq] [ 299.042246] pc : []lr : []psr: 4013 sp : ee4c7ca8 ip : fp : ef709800 [ 299.088240] r10: r9 : ee3bffc0 r8 : 0034 [ 299.109153] r7 : 0003 r6 : r5 : ee4c7d00 r4 : ee1d8c00 [ 299.135291] r3 : ef758580 r2 : r1 : ffc8 r0 : 0034 [ 299.161431] Flags: nZcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 299.190008] Control: 10c5383d Table: 2d00406a DAC: 0051 [ 299.213011] Process pulseaudio (pid: 847, stack limit = 0xee4c6220) [ 299.238104] Stack: (0xee4c7ca8 to 0xee4c8000) [ 299.255539] 7ca0: c1403d54 80400040 ff7f0600 ff7f0660 bf06b578 ee3bffc0 [ 299.288301] 7cc0: ee3afd00 ee4c7d00 bf0640b4 bf066428 [ 299.321064] 7ce0: ee3afd00 ee3afd00 ee4c7d34 ee3af444 ee3bffc0 ee3af444 ee3bffc0 bf0662ec [ 299.353826] 7d00: 41554453 bf065db0 ee3afd00 00010002 bf0d7408 ee3af440 bf0d7408 [ 299.386587] 7d20: ee79bd80 bf0d5a04 ef709800 0020 0002 0001 41554453 [ 299.419349] 7d40: bf0d559c ee3af440 0001 0001 [ 299.452111] 7d60: ee24ac80 ee24ac80 ee1c4a00 ee79bd80 ee24ace8 0001 bf0d4dfc [ 299.484872] 7d80: 000b ee4b8c3c ee4c7dc8 ee4b8800 ee4b8c28 ee4c6000 [ 299.517635] 7da0: ee4b8c3c ed029e40 bf0c0404 ee4b8800 ee1c4a00 ee4b8800 ed029e40 [ 299.550398] 7dc0: bf0c0560 ee072340 ef758580 c0367b7c ee4b8c40 ee4b8c40 [ 299.583161] 7de0: ee4b8800 ed029e40 ee318f80 ed029e40 0006 ee318f80 bf0c0748 [ 299.615924] 7e00: bf0a3430 ee4f6180 c0428fe0 ee318f80 21b0 0026 ed029e40 [ 299.648697] 7e20: ee318f80 ed029e48 c0428f1c ee4c7e94 0006 c0421cc0 ee4c7ed0 [ 299.681464] 7e40: 0802 ee4c7e94 0006 ee318f80 c0432c8c ee4c7f40 c0433bc0 [ 299.714225] 7e60: ed029e40 0041 ed004000 ee4c6000 [ 299.746987] 7e80: eec69808 0005 0002 ee318f80 ef0d2910 ee924908 bf0ba284 [ 299.779750] 7ea0: ee31bbc0 bebb53c4 ee4e1d00 0011 ee4c7f74 0001 f000 c0308b04 [ 299.812512] 7ec0: ee4c6000 bebb5710 c0434578 ef0d2910 ee924908 73541c18 0008 [ 299.845274] 7ee0: ee4a7019 ee899bb0 ee318f80 0101 0002 0084 [ 299.878037] 7f00: ee4c7f10 ee318df8 ed029840 40045532 bebb53c4 [ 299.910799] 7f20: ee4c6000 ee4a7000 c1403ef8 bebb550c 0011 ee5eca00 0020 ee5eca18 [ 299.943562] 7f40: ee4a7000 00080802 0002 ff9c f000 0011 ff9c [ 299.976324] 7f60: ee4a7000 c0422e70 0002 c04359b0 ed029840 0802 ed02 0006 [ 300.009086] 7f80: 0100 0001 0004 b189d000 0005 c0308b04 [ 300.041848] 7fa0: ee4c6000 c0308940 0004 bebb550c 00080802 bebb53c4 00084b58 [ 300.074611] 7fc0: 0004 b189d000 0005 bebb550c 00099448 bebb5710 [ 300.107373] 7fe0: bebb53c8 b6c40da4 b6c24334 8010 bebb550c 2fffd861 2fffdc61 [ 300.140190] [] (mutex_lock) from [] (vchiq_add_service_internal+0x138/0x3a0 [vchiq]) [ 300.178237] [] (vchiq_add_service_internal [vchiq]) from [] (vchiq_open_service+0x58/0xf0 [vchiq]) [ 300.221152] [] (vchiq_open_service [vchiq]) from [] (vchi_service_open+0x74/0xa8 [vchiq]) [ 300.260919] [] (vchi_service_open [vchiq]) from [] (bcm2835_audio_open+0xe8/0x2d0 [snd_bcm2835]) [ 300.303111] [] (bcm2835_audio_open [snd_bcm2835]) from [] (snd_bcm2835_playback_open_generic+0xc0/0x1c4 [snd_bcm2835]) [ 300.352975] [] (snd_bcm2835_playback_open_generic [snd_bcm2835]) from [] (snd_pcm_open_substream+0x60/0x110 [snd_pcm]) [ 300.402848] [] (snd_pcm_open_substream [snd_pcm]) from [] (snd_pcm_open+0xac/0x1
[PATCH] staging: radio-bcm2048: Fix checkpatch warnings: WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
Signed-off-by: Eddie Youseph
---
drivers/staging/media/bcm2048/radio-bcm2048.c | 44 +--
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index d605c41..7d33bce 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -2020,27 +2020,27 @@ static irqreturn_t bcm2048_handler(int irq, void *dev)
return count; \
}
-DEFINE_SYSFS_PROPERTY(power_state, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(mute, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(audio_route, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(dac_output, unsigned, int, "%u", 0)
-
-DEFINE_SYSFS_PROPERTY(fm_hi_lo_injection, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_frequency, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_af_frequency, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_deemphasis, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_rds_mask, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_best_tune_mode, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_search_rssi_threshold, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_search_mode_direction, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(fm_search_tune_mode, unsigned, int, "%u", value > 3)
-
-DEFINE_SYSFS_PROPERTY(rds, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(rds_b_block_mask, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(rds_b_block_match, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(rds_pi_mask, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(rds_pi_match, unsigned, int, "%u", 0)
-DEFINE_SYSFS_PROPERTY(rds_wline, unsigned, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(power_state, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(mute, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(audio_route, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(dac_output, unsigned int, int, "%u", 0)
+
+DEFINE_SYSFS_PROPERTY(fm_hi_lo_injection, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_frequency, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_af_frequency, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_deemphasis, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_rds_mask, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_best_tune_mode, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_search_rssi_threshold, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_search_mode_direction, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(fm_search_tune_mode, unsigned int, int, "%u", value > 3)
+
+DEFINE_SYSFS_PROPERTY(rds, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(rds_b_block_mask, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(rds_b_block_match, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(rds_pi_mask, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(rds_pi_match, unsigned int, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(rds_wline, unsigned int, int, "%u", 0)
property_read(rds_pi, unsigned int, "%x")
property_str_read(rds_rt, (BCM2048_MAX_RDS_RT + 1))
property_str_read(rds_ps, (BCM2048_MAX_RDS_PS + 1))
@@ -2052,7 +2052,7 @@ static irqreturn_t bcm2048_handler(int irq, void *dev)
property_read(region_top_frequency, unsigned int, "%u")
property_signed_read(fm_carrier_error, int, "%d")
property_signed_read(fm_rssi, int, "%d")
-DEFINE_SYSFS_PROPERTY(region, unsigned, int, "%u", 0)
+DEFINE_SYSFS_PROPERTY(region, unsigned int, int, "%u", 0)
static struct device_attribute attrs[] = {
__ATTR(power_state, 0644, bcm2048_power_state_read,
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] staging: atomisp: remove else statement after return
It doesn't need to have else statement after return.
Signed-off-by: Daeseok Youn
---
drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index d97a8df..8bdb224 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -2958,11 +2958,11 @@ int atomisp_get_metadata(struct atomisp_sub_device
*asd, int flag,
dev_err(isp->dev, "copy to user failed: copied %d bytes\n",
ret);
return -EFAULT;
- } else {
- list_del_init(&md_buf->list);
- list_add_tail(&md_buf->list, &asd->metadata[md_type]);
}
+ list_del_init(&md_buf->list);
+ list_add_tail(&md_buf->list, &asd->metadata[md_type]);
+
dev_dbg(isp->dev, "%s: HAL de-queued metadata type %d with exp_id %d\n",
__func__, md_type, md->exp_id);
return 0;
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()
If v4l2_subdev_call() gets the global frame interval values,
it returned 0 and it could be checked whether numerator is zero or not.
If the numerator is not zero, the fps could be calculated in this function.
If not, it just returns 0.
Signed-off-by: Daeseok Youn
---
.../media/atomisp/pci/atomisp2/atomisp_cmd.c | 22 ++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 8bdb224..6bdd19e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct
video_device *dev)
static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device *asd)
{
- struct v4l2_subdev_frame_interval frame_interval;
+ struct v4l2_subdev_frame_interval fi;
struct atomisp_device *isp = asd->isp;
- unsigned short fps;
- if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
- video, g_frame_interval, &frame_interval)) {
- fps = 0;
- } else {
- if (frame_interval.interval.numerator)
- fps = frame_interval.interval.denominator /
- frame_interval.interval.numerator;
- else
- fps = 0;
- }
+ unsigned short fps = 0;
+ int ret;
+
+ ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
+ video, g_frame_interval, &fi);
+
+ if (!ret && fi.interval.numerator)
+ fps = fi.interval.denominator / fi.interval.numerator;
+
return fps;
}
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] staging: atomisp: remove useless condition in if-statements
The css_pipe_id was checked with 'CSS_PIPE_ID_COPY' in previous if-
statement. In this case, if the css_pipe_id equals to 'CSS_PIPE_ID_COPY',
it could not enter the next if-statement. But the "next" if-statement
has the condition to check whether the css_pipe_id equals to
'CSS_PIPE_ID_COPY' or not. It should be removed.
Signed-off-by: Daeseok Youn
---
drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 6bdd19e..929ed80 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -910,8 +910,7 @@ static struct atomisp_video_pipe *__atomisp_get_pipe(
} else if (asd->run_mode->val == ATOMISP_RUN_MODE_VIDEO) {
/* For online video or SDV video pipe. */
if (css_pipe_id == CSS_PIPE_ID_VIDEO ||
- css_pipe_id == CSS_PIPE_ID_COPY ||
- css_pipe_id == CSS_PIPE_ID_YUVPP) {
+ css_pipe_id == CSS_PIPE_ID_COPY) {
if (buf_type == CSS_BUFFER_TYPE_OUTPUT_FRAME)
return &asd->video_out_video_capture;
return &asd->video_out_preview;
@@ -919,8 +918,7 @@ static struct atomisp_video_pipe *__atomisp_get_pipe(
} else if (asd->run_mode->val == ATOMISP_RUN_MODE_PREVIEW) {
/* For online preview or ZSL preview pipe. */
if (css_pipe_id == CSS_PIPE_ID_PREVIEW ||
- css_pipe_id == CSS_PIPE_ID_COPY ||
- css_pipe_id == CSS_PIPE_ID_YUVPP)
+ css_pipe_id == CSS_PIPE_ID_COPY)
return &asd->video_out_preview;
}
/* For capture pipe. */
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] staging: atomisp: remove redudant condition in if-statement
The V4L2_FIELD_ANY is zero, so the (!field) is same meaning
with (field == V4L2_FIELD_ANY) in if-statement.
Signed-off-by: Daeseok Youn
---
drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 929ed80..2437162 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -5084,7 +5084,7 @@ int atomisp_try_fmt(struct video_device *vdev, struct
v4l2_format *f,
depth = get_pixel_depth(pixelformat);
- if (!field || field == V4L2_FIELD_ANY)
+ if (field == V4L2_FIELD_ANY)
field = V4L2_FIELD_NONE;
else if (field != V4L2_FIELD_NONE) {
dev_err(isp->dev, "Wrong output field\n");
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/5] staging: vc04_services: camera driver maintance
On Mon, 2017-03-20 at 13:55 +0300, Dan Carpenter wrote: > I'm not going to review this because it has kbuild errors. > > regards, > dan carpenter > Hi, can you e-mail out the errors or send them to me. It worked when I submitted it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver
On Mon, 2017-03-20 at 07:58 -0300, Mauro Carvalho Chehab wrote: > Em Sun, 19 Mar 2017 22:11:07 -0300 > Mauro Carvalho Chehab escreveu: > > > Em Sun, 19 Mar 2017 10:04:28 -0700 > > Michael Zoran escreveu: > > > > > A working DT that I tried this morning with the current firmware > > > is > > > posted here: > > > http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-March/ > > > 005924 > > > .html > > > > > > It even works with minecraft_pi! > > With the new firmware, sometime after booting, I'm getting an oops, > caused > by bcm2835_audio/vchiq: > > [ 298.788995] Unable to handle kernel NULL pointer dereference at > virtual address 0034 > [ 298.821458] pgd = ed004000 > [ 298.832294] [0034] *pgd=2e5e9835, *pte=, > *ppte= > [ 298.857450] Internal error: Oops: 17 [#1] SMP ARM > [ 298.876273] Modules linked in: cfg80211 hid_logitech_hidpp > hid_logitech_dj snd_bcm2835(C) snd_pcm snd_timer snd soundcore > vchiq(C) uio_pdrv_genirq uio fuse > [ 298.932064] CPU: 3 PID: 847 Comm: pulseaudio Tainted: > G C 4.11.0-rc1+ #56 > [ 298.963774] Hardware name: Generic DT based system > [ 298.982945] task: ef758580 task.stack: ee4c6000 > [ 299.001080] PC is at mutex_lock+0x14/0x3c > [ 299.017148] LR is at vchiq_add_service_internal+0x138/0x3a0 > [vchiq] > [ 299.042246] pc : []lr : []psr: > 4013 > sp : ee4c7ca8 ip : fp : ef709800 > [ 299.088240] r10: r9 : ee3bffc0 r8 : 0034 > [ 299.109153] r7 : 0003 r6 : r5 : ee4c7d00 r4 : > ee1d8c00 > [ 299.135291] r3 : ef758580 r2 : r1 : ffc8 r0 : > 0034 > [ 299.161431] Flags: nZcv IRQs on FIQs on Mode SVC_32 ISA > ARM Segment none > [ 299.190008] Control: 10c5383d Table: 2d00406a DAC: 0051 > [ 299.213011] Process pulseaudio (pid: 847, stack limit = > 0xee4c6220) > [ 299.238104] Stack: (0xee4c7ca8 to 0xee4c8000) > [ 299.255539] 7ca0: c1403d54 80400040 ff7f0600 > ff7f0660 bf06b578 ee3bffc0 > [ 299.288301] 7cc0: ee3afd00 ee4c7d00 > bf0640b4 bf066428 > [ 299.321064] 7ce0: ee3afd00 ee3afd00 ee4c7d34 ee3af444 ee3bffc0 > ee3af444 ee3bffc0 bf0662ec > [ 299.353826] 7d00: 41554453 bf065db0 ee3afd00 00010002 bf0d7408 > ee3af440 bf0d7408 > [ 299.386587] 7d20: ee79bd80 bf0d5a04 ef709800 0020 > 0002 0001 41554453 > [ 299.419349] 7d40: bf0d559c ee3af440 > 0001 0001 > [ 299.452111] 7d60: ee24ac80 ee24ac80 ee1c4a00 ee79bd80 > ee24ace8 0001 bf0d4dfc > [ 299.484872] 7d80: 000b ee4b8c3c ee4c7dc8 > ee4b8800 ee4b8c28 ee4c6000 > [ 299.517635] 7da0: ee4b8c3c ed029e40 bf0c0404 ee4b8800 > ee1c4a00 ee4b8800 ed029e40 > [ 299.550398] 7dc0: bf0c0560 ee072340 ef758580 > c0367b7c ee4b8c40 ee4b8c40 > [ 299.583161] 7de0: ee4b8800 ed029e40 ee318f80 ed029e40 > 0006 ee318f80 bf0c0748 > [ 299.615924] 7e00: bf0a3430 ee4f6180 c0428fe0 ee318f80 > 21b0 0026 ed029e40 > [ 299.648697] 7e20: ee318f80 ed029e48 c0428f1c ee4c7e94 0006 > c0421cc0 ee4c7ed0 > [ 299.681464] 7e40: 0802 ee4c7e94 0006 ee318f80 > c0432c8c ee4c7f40 c0433bc0 > [ 299.714225] 7e60: ed029e40 0041 > ed004000 ee4c6000 > [ 299.746987] 7e80: eec69808 0005 0002 ee318f80 > ef0d2910 ee924908 bf0ba284 > [ 299.779750] 7ea0: ee31bbc0 bebb53c4 ee4e1d00 0011 ee4c7f74 > 0001 f000 c0308b04 > [ 299.812512] 7ec0: ee4c6000 bebb5710 c0434578 ef0d2910 > ee924908 73541c18 0008 > [ 299.845274] 7ee0: ee4a7019 ee899bb0 ee318f80 > 0101 0002 0084 > [ 299.878037] 7f00: ee4c7f10 ee318df8 > ed029840 40045532 bebb53c4 > [ 299.910799] 7f20: ee4c6000 ee4a7000 c1403ef8 bebb550c 0011 > ee5eca00 0020 ee5eca18 > [ 299.943562] 7f40: ee4a7000 00080802 0002 ff9c > f000 0011 ff9c > [ 299.976324] 7f60: ee4a7000 c0422e70 0002 c04359b0 ed029840 > 0802 ed02 0006 > [ 300.009086] 7f80: 0100 0001 0004 > b189d000 0005 c0308b04 > [ 300.041848] 7fa0: ee4c6000 c0308940 0004 bebb550c > 00080802 bebb53c4 00084b58 > [ 300.074611] 7fc0: 0004 b189d000 0005 > bebb550c 00099448 bebb5710 > [ 300.107373] 7fe0: bebb53c8 b6c40da4 b6c24334 8010 > bebb550c 2fffd861 2fffdc61 > [ 300.140190] [] (mutex_lock) from [] > (vchiq_add_service_internal+0x138/0x3a0 [vchiq]) > [ 300.178237] [] (vchiq_add_service_internal [vchiq]) from > [] (vchiq_open_service+0x58/0xf0 [vchiq]) > [ 300.221152] [] (vchiq_open_service [vchiq]) from > [] (vchi_service_open+0x74/0xa8 [vchiq]) > [ 300.260919] [] (vchi_service_open [vchiq]) from > [] (bcm2835_audio_open+0xe8/0x2d0 [snd_bcm2835]) > [ 300.303111] [] (bcm2835_audio_open [snd_bcm2835]) from > [] (snd_
Re: [PATCH 3/4] staging: atomisp: remove useless condition in if-statements
On Mon, Mar 20, 2017 at 08:00:06PM +0900, Daeseok Youn wrote: > The css_pipe_id was checked with 'CSS_PIPE_ID_COPY' in previous if- > statement. In this case, if the css_pipe_id equals to 'CSS_PIPE_ID_COPY', > it could not enter the next if-statement. But the "next" if-statement > has the condition to check whether the css_pipe_id equals to > 'CSS_PIPE_ID_COPY' or not. It should be removed. > > Signed-off-by: Daeseok Youn The patch is correct but the changelog is not. s/CSS_PIPE_ID_COPY/CSS_PIPE_ID_YUVPP/ regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline
On 03/17/2017 12:55 PM, Sakari Ailus wrote: > Hi Russell, > > On 03/17/17 13:42, Russell King - ARM Linux wrote: >> On Tue, Mar 14, 2017 at 08:55:36AM +0100, Hans Verkuil wrote: >>> We're all very driver-development-driven, and userspace gets very little >>> attention in general. So before just throwing in the towel we should take >>> a good look at the reasons why there has been little or no development: is >>> it because of fundamental design defects, or because nobody paid attention >>> to it? >>> >>> I strongly suspect it is the latter. >>> >>> In addition, I suspect end-users of these complex devices don't really care >>> about a plugin: they want full control and won't typically use generic >>> applications. If they would need support for that, we'd have seen much more >>> interest. The main reason for having a plugin is to simplify testing and >>> if this is going to be used on cheap hobbyist devkits. >> >> I think you're looking at it with a programmers hat on, not a users hat. >> >> Are you really telling me that requiring users to 'su' to root, and then >> use media-ctl to manually configure the capture device is what most >> users "want" ? > > It depends on who the user is. I don't think anyone is suggesting a > regular end user is the user of all these APIs: it is either an > application tailored for that given device, a skilled user with his test > scripts or as suggested previously, a libv4l plugin knowing the device > or a generic library geared towards providing best effort service. The > last one of this list does not exist yet and the second last item > requires help. > > Typically this class of devices is simply not up to provide the level of > service you're requesting without additional user space control library > which is responsible for automatic white balance, exposure and focus. > > Making use of the full potential of the hardware requires using a more > expressive interface. That's what the kernel interface must provide. If > we decide to limit ourselves to a small sub-set of that potential on the > level of the kernel interface, we have made a wrong decision. It's as > simple as that. This is why the functionality (and which requires taking > a lot of policy decisions) belongs to the user space. We cannot have > multiple drivers providing multiple kernel interfaces for the same hardware. Right. With my Cisco hat on I can tell you that Cisco would want full low-level control. If the driver would limit us we would not be able to use it. Same with anyone who wants to put Android CameraHAL on top of a V4L2 driver: they would need full control. Some simplified interface would be unacceptable. > > That said, I'm not trying to provide an excuse for not having libraries > available to help the user to configure and control the device more or > less automatically even in terms of best effort. It's something that > does require attention, a lot more of it than it has received in recent > few years. Right. Regards, Hans ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: Fix sparse warning in wilc_wfi_cfgoperations.c
On Sun, Mar 19, 2017 at 11:17:08PM -0700, Jacob Zachariah wrote: > Fix the following warning reported by sparse: > > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c:2006:51: warning: incorrect > type in assignment (different base types) > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c:2006:51:expected > unsigned short [unsigned] [assigned] [usertype] ht_capa_info > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c:2006:51:got restricted > __le16 const [usertype] cap_info > This is probably correct but there is another just a few lines down. Fix that as well. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt
On Sun, 2017-03-19 at 12:08 -0700, Steve Longerbeam wrote: > > On 03/19/2017 08:22 AM, Russell King - ARM Linux wrote: > > On Thu, Mar 09, 2017 at 08:53:18PM -0800, Steve Longerbeam wrote: > >> From: Philipp Zabel > >> > >> The csi_try_crop call in set_fmt should compare the cropping rectangle > >> to the currently set input format, not to the previous input format. > > Are we really sure that the cropping support is implemented correctly? > > > > I came across this while looking at what we're doing with the > > V4L2_SEL_FLAG_KEEP_CONFIG flag. > > > > Documentation/media/uapi/v4l/dev-subdev.rst defines the behaviour of > > the user API, and "Order of configuration and format propagation" says: > > > >The coordinates to a step always refer to the actual size of the > >previous step. The exception to this rule is the source compose > >rectangle, which refers to the sink compose bounds rectangle --- if it > >is supported by the hardware. > > > >1. Sink pad format. The user configures the sink pad format. This format > > defines the parameters of the image the entity receives through the > > pad for further processing. > > > >2. Sink pad actual crop selection. The sink pad crop defines the crop > > performed to the sink pad format. > > > >3. Sink pad actual compose selection. The size of the sink pad compose > > rectangle defines the scaling ratio compared to the size of the sink > > pad crop rectangle. The location of the compose rectangle specifies > > the location of the actual sink compose rectangle in the sink compose > > bounds rectangle. > > > >4. Source pad actual crop selection. Crop on the source pad defines crop > > performed to the image in the sink compose bounds rectangle. > > > >5. Source pad format. The source pad format defines the output pixel > > format of the subdev, as well as the other parameters with the > > exception of the image width and height. Width and height are defined > > by the size of the source pad actual crop selection. > > > >Accessing any of the above rectangles not supported by the subdev will > >return ``EINVAL``. Any rectangle referring to a previous unsupported > >rectangle coordinates will instead refer to the previous supported > >rectangle. For example, if sink crop is not supported, the compose > >selection will refer to the sink pad format dimensions instead. > > > > Note step 3 above: scaling is defined by the ratio of the _sink_ crop > > rectangle to the _sink_ compose rectangle. The above paragraph suggests we skip any rectangles that are not supported. In our case that would be 3. and 4., since the CSI can't compose into a larger frame. I hadn't realised that the crop selection currently happens on the source pad. The hardware actually only supports cropping of the input (the crop rectangle we write into the window registers are before downscaling). So the crop rectangle should be moved to the sink pad. > > So, lets say that the camera produces a 1280x720 image, and the sink > > pad format is configured with 1280x720. That's step 1. > > > > The sink crop operates within that rectangle, cropping it to an area. > > Let's say we're only interested in its centre, so we'd chose 640x360 > > with the top-left as 320,180. This is step 2. >> > > Then, if we want to down-scale by a factor of two, we'd set the sink > > compose selection to 320x180. Except when composing is not supported. If the sink compose and source crop rectangles are not supported, the source pad format takes their place in determining the scaling output resolution. At least that's how I read the documentation. > > This seems to be at odds with how the scaling is done in CSI at > > present: the selection implementations all reject attempts to > > configure the sink pad, instead only supporting crop rectangles on > > the source, > > Correct. Currently cropping is only supported at the source pad > (step 4). > > Initially the CSI didn't support down-scaling, so step 3 is not supported, > so the sink pad format/crop selection rectangle/crop compose rectangle > are collapsed into the same sink pad format rectangle. > > Philipp later added support for /2 downscaling, but we didn't put this in > the correct API, looks like this needs to move into the selection API at > step 3 (sink pad compose rectangle). I am not sure about this. Wouldn't moving the input crop to the sink pad be enough? If we added support for the sink pad compose rectangle, that wouldn't actually allow to compose the CSI output into a larger frame. Since the subdevice can't compose, I'd leave the sink compose rectangle disabled. > > and we use the source crop rectangle to define the > > down-scaling. We use the source pad format to define the downscaling relative to the source crop rectangle (which is wrong, it should be relative to the sink crop rectangle). > Yes. And maybe th
Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver
Hi Mauro, Am 20.03.2017 um 11:58 schrieb Mauro Carvalho Chehab: Em Sun, 19 Mar 2017 22:11:07 -0300 Mauro Carvalho Chehab escreveu: Em Sun, 19 Mar 2017 10:04:28 -0700 Michael Zoran escreveu: A working DT that I tried this morning with the current firmware is posted here: http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-March/005924 .html It even works with minecraft_pi! With the new firmware, sometime after booting, I'm getting an oops, caused by bcm2835_audio/vchiq: [ 298.788995] Unable to handle kernel NULL pointer dereference at virtual address 0034 [ 298.821458] pgd = ed004000 [ 298.832294] [0034] *pgd=2e5e9835, *pte=, *ppte= [ 298.857450] Internal error: Oops: 17 [#1] SMP ARM [ 298.876273] Modules linked in: cfg80211 hid_logitech_hidpp hid_logitech_dj snd_bcm2835(C) snd_pcm snd_timer snd soundcore vchiq(C) uio_pdrv_genirq uio fuse [ 298.932064] CPU: 3 PID: 847 Comm: pulseaudio Tainted: G C 4.11.0-rc1+ #56 [ 298.963774] Hardware name: Generic DT based system [ 298.982945] task: ef758580 task.stack: ee4c6000 [ 299.001080] PC is at mutex_lock+0x14/0x3c [ 299.017148] LR is at vchiq_add_service_internal+0x138/0x3a0 [vchiq] [ 299.042246] pc : []lr : []psr: 4013 sp : ee4c7ca8 ip : fp : ef709800 [ 299.088240] r10: r9 : ee3bffc0 r8 : 0034 [ 299.109153] r7 : 0003 r6 : r5 : ee4c7d00 r4 : ee1d8c00 [ 299.135291] r3 : ef758580 r2 : r1 : ffc8 r0 : 0034 [ 299.161431] Flags: nZcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 299.190008] Control: 10c5383d Table: 2d00406a DAC: 0051 [ 299.213011] Process pulseaudio (pid: 847, stack limit = 0xee4c6220) [ 299.238104] Stack: (0xee4c7ca8 to 0xee4c8000) [ 299.255539] 7ca0: c1403d54 80400040 ff7f0600 ff7f0660 bf06b578 ee3bffc0 [ 299.288301] 7cc0: ee3afd00 ee4c7d00 bf0640b4 bf066428 [ 299.321064] 7ce0: ee3afd00 ee3afd00 ee4c7d34 ee3af444 ee3bffc0 ee3af444 ee3bffc0 bf0662ec [ 299.353826] 7d00: 41554453 bf065db0 ee3afd00 00010002 bf0d7408 ee3af440 bf0d7408 [ 299.386587] 7d20: ee79bd80 bf0d5a04 ef709800 0020 0002 0001 41554453 [ 299.419349] 7d40: bf0d559c ee3af440 0001 0001 [ 299.452111] 7d60: ee24ac80 ee24ac80 ee1c4a00 ee79bd80 ee24ace8 0001 bf0d4dfc [ 299.484872] 7d80: 000b ee4b8c3c ee4c7dc8 ee4b8800 ee4b8c28 ee4c6000 [ 299.517635] 7da0: ee4b8c3c ed029e40 bf0c0404 ee4b8800 ee1c4a00 ee4b8800 ed029e40 [ 299.550398] 7dc0: bf0c0560 ee072340 ef758580 c0367b7c ee4b8c40 ee4b8c40 [ 299.583161] 7de0: ee4b8800 ed029e40 ee318f80 ed029e40 0006 ee318f80 bf0c0748 [ 299.615924] 7e00: bf0a3430 ee4f6180 c0428fe0 ee318f80 21b0 0026 ed029e40 [ 299.648697] 7e20: ee318f80 ed029e48 c0428f1c ee4c7e94 0006 c0421cc0 ee4c7ed0 [ 299.681464] 7e40: 0802 ee4c7e94 0006 ee318f80 c0432c8c ee4c7f40 c0433bc0 [ 299.714225] 7e60: ed029e40 0041 ed004000 ee4c6000 [ 299.746987] 7e80: eec69808 0005 0002 ee318f80 ef0d2910 ee924908 bf0ba284 [ 299.779750] 7ea0: ee31bbc0 bebb53c4 ee4e1d00 0011 ee4c7f74 0001 f000 c0308b04 [ 299.812512] 7ec0: ee4c6000 bebb5710 c0434578 ef0d2910 ee924908 73541c18 0008 [ 299.845274] 7ee0: ee4a7019 ee899bb0 ee318f80 0101 0002 0084 [ 299.878037] 7f00: ee4c7f10 ee318df8 ed029840 40045532 bebb53c4 [ 299.910799] 7f20: ee4c6000 ee4a7000 c1403ef8 bebb550c 0011 ee5eca00 0020 ee5eca18 [ 299.943562] 7f40: ee4a7000 00080802 0002 ff9c f000 0011 ff9c [ 299.976324] 7f60: ee4a7000 c0422e70 0002 c04359b0 ed029840 0802 ed02 0006 [ 300.009086] 7f80: 0100 0001 0004 b189d000 0005 c0308b04 [ 300.041848] 7fa0: ee4c6000 c0308940 0004 bebb550c 00080802 bebb53c4 00084b58 [ 300.074611] 7fc0: 0004 b189d000 0005 bebb550c 00099448 bebb5710 [ 300.107373] 7fe0: bebb53c8 b6c40da4 b6c24334 8010 bebb550c 2fffd861 2fffdc61 [ 300.140190] [] (mutex_lock) from [] (vchiq_add_service_internal+0x138/0x3a0 [vchiq]) [ 300.178237] [] (vchiq_add_service_internal [vchiq]) from [] (vchiq_open_service+0x58/0xf0 [vchiq]) [ 300.221152] [] (vchiq_open_service [vchiq]) from [] (vchi_service_open+0x74/0xa8 [vchiq]) [ 300.260919] [] (vchi_service_open [vchiq]) from [] (bcm2835_audio_open+0xe8/0x2d0 [snd_bcm2835]) [ 300.303111] [] (bcm2835_audio_open [snd_bcm2835]) from [] (snd_bcm2835_playback_open_generic+0xc0/0x1c4 [snd_bcm2835]) [ 300.352975] [] (snd_bcm2835_playback_open_generic [snd_bcm2835]) from [] (snd_pcm_open_substream+0x60/0x110 [snd_pcm]) [ 300.402848] [] (snd_pcm_open_substream [snd_
Re: [PATCH] staging: radio-bcm2048: Fix checkpatch warnings: WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
The subject is too long. On Mon, Mar 20, 2017 at 10:02:33PM +1100, unknown wrote: > Signed-off-by: Eddie Youseph You need to have a changelog. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()
Am 20.03.2017 11:59, schrieb Daeseok Youn:
> If v4l2_subdev_call() gets the global frame interval values,
> it returned 0 and it could be checked whether numerator is zero or not.
>
> If the numerator is not zero, the fps could be calculated in this function.
> If not, it just returns 0.
>
> Signed-off-by: Daeseok Youn
> ---
> .../media/atomisp/pci/atomisp2/atomisp_cmd.c | 22
> ++
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> index 8bdb224..6bdd19e 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> @@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct
> video_device *dev)
>
> static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device *asd)
> {
> - struct v4l2_subdev_frame_interval frame_interval;
> + struct v4l2_subdev_frame_interval fi;
> struct atomisp_device *isp = asd->isp;
> - unsigned short fps;
>
> - if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
> - video, g_frame_interval, &frame_interval)) {
> - fps = 0;
> - } else {
> - if (frame_interval.interval.numerator)
> - fps = frame_interval.interval.denominator /
> - frame_interval.interval.numerator;
> - else
> - fps = 0;
> - }
> + unsigned short fps = 0;
> + int ret;
> +
> + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
> +video, g_frame_interval, &fi);
> +
> + if (!ret && fi.interval.numerator)
> + fps = fi.interval.denominator / fi.interval.numerator;
> +
> return fps;
> }
do you need to check ret at all ? if an error occurs can fi.interval.numerator
be something else than 0 ?
if ret is an ERRNO it would be wise to return ret not fps, but this may require
changes at other places also.
re,
wh
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: radio-bcm2048: Fix checkpatch warnings: WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
Also fix your From header. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt
On Mon, Mar 20, 2017 at 12:55:26PM +0100, Philipp Zabel wrote: > The above paragraph suggests we skip any rectangles that are not > supported. In our case that would be 3. and 4., since the CSI can't > compose into a larger frame. I hadn't realised that the crop selection > currently happens on the source pad. I'd recommend viewing the documentation in its post-processed version, because then you get the examples as pictures, and they say that a picture is worth 1000 words. See https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/dev-subdev.html There is almost an exact example of what we're trying to do - it's figure 4.6. Here, we have a sink pad with a cropping rectangle on the input, which is then scaled to a composition rectangle (there's no bounds rectangle, and it's specified that in such a case the top,left of the composition rectangle will always be 0,0 - see quote below). Where it differs is that the example also supports source cropping for two source pads. We don't support that. The same document says: Scaling support is optional. When supported by a subdev, the crop rectangle on the subdev's sink pad is scaled to the size configured using the :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the subdev supports scaling but not composing, the top and left values are not used and must always be set to zero. which in itself describes _exactly_ our hardware here as far as the cropping and scaling the hardware supports. > Except when composing is not supported. If the sink compose and source > crop rectangles are not supported, the source pad format takes their > place in determining the scaling output resolution. At least that's how > I read the documentation. This isn't how I read it. Scaling is the relationship between the size of the sink crop and sink compose rectangle. Composition requires that there be a composition bounds rectangle to define the composition space, and the top,left of the composition rectangle be adjustable to place the composition rectangle within that space. The above quoted paragraph from the documentation backs up my view in its final sentence - it doesn't say "if the subdev supports scaling but not composing, there is no composition rectangle" but says that there _is_ one but its top,left coordinates are always zero. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] staging: sm750fb: Code readability is improved
On Sun, Mar 19, 2017 at 09:19:20PM +0530, Arushi Singhal wrote:
> New variables are added to make the code more readable.
>
> Signed-off-by: Arushi Singhal
> ---
> changes in v4
> -try to make the code much more readable.
> - defined the variable at the top of a block.
> ---
> drivers/staging/sm750fb/ddk750_mode.c | 57
> +++
> 1 file changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/staging/sm750fb/ddk750_mode.c
> b/drivers/staging/sm750fb/ddk750_mode.c
> index eea5aef2956f..6517e770e0a7 100644
> --- a/drivers/staging/sm750fb/ddk750_mode.c
> +++ b/drivers/staging/sm750fb/ddk750_mode.c
> @@ -76,38 +76,43 @@ static int programModeRegisters(mode_parameter_t
> *pModeParam, struct pll_value *
> {
> int ret = 0;
> int cnt = 0;
> - unsigned int tmp, reg;
> + unsigned int tmp, reg, temp;
Let's not have "tmp" and "temp" both. Generally "tmp" is better because
you can't confuse it with temperature.
>
> if (pll->clockType == SECONDARY_PLL) {
> /* programe secondary pixel clock */
> poke32(CRT_PLL_CTRL, sm750_format_pll_reg(pll));
> - poke32(CRT_HORIZONTAL_TOTAL,
> -(((pModeParam->horizontal_total - 1) <<
> - CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
> - CRT_HORIZONTAL_TOTAL_TOTAL_MASK) |
> -((pModeParam->horizontal_display_end - 1) &
> - CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK));
> -
> - poke32(CRT_HORIZONTAL_SYNC,
> -((pModeParam->horizontal_sync_width <<
> - CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) &
> - CRT_HORIZONTAL_SYNC_WIDTH_MASK) |
> -((pModeParam->horizontal_sync_start - 1) &
> - CRT_HORIZONTAL_SYNC_START_MASK));
>
> - poke32(CRT_VERTICAL_TOTAL,
> -(((pModeParam->vertical_total - 1) <<
> - CRT_VERTICAL_TOTAL_TOTAL_SHIFT) &
> - CRT_VERTICAL_TOTAL_TOTAL_MASK) |
> -((pModeParam->vertical_display_end - 1) &
> - CRT_VERTICAL_TOTAL_DISPLAY_END_MASK));
> + temp = (pModeParam->horizontal_total - 1) <<
> + CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT;
> + temp = temp & CRT_HORIZONTAL_TOTAL_TOTAL_MASK;
These two lines are really part of the same thing and they should be
done together.
tmp = ((pModeParam->horizontal_total - 1) <<
CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
CRT_HORIZONTAL_TOTAL_TOTAL_MASK;
> + temp = temp | ((pModeParam->horizontal_display_end - 1) &
> + CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK);
tmp |= (pModeParam->horizontal_display_end - 1) &
CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK;
Use |=. Remove the extra parenthesis.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
On 03/19/2017 01:14 PM, Russell King - ARM Linux wrote: > On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote: >> On 03/18/2017 12:22 PM, Russell King - ARM Linux wrote: >>> 0:00:01.955927879 20954 0x15ffe90 INFOv4l2 >>> gstv4l2object.c:3811:gst_v4l2_object_get_caps: probed caps: >>> video/x-bayer, format=(string)rggb, framerate=(fraction)3/1001, >>> width=(int)816, height=(int)616, pixel-aspect-ratio=(fraction)1/1; >>> video/x-raw, format=(string)I420, framerate=(fraction)3/1001, >>> width=(int)816, height=(int)616, interlace-mode=(string)progressive, >>> pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)YV12, >>> framerate=(fraction)3/1001, width=(int)816, height=(int)616, >>> interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; >>> video/x-raw, format=(string)BGR, framerate=(fraction)3/1001, >>> width=(int)816, height=(int)616, interlace-mode=(string)progressive, >>> pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)RGB, >>> framerate=(fraction)3/1001, width=(int)816, height=(int)616, >>> interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1 >>> >>>despite the media pipeline actually being configured for 60fps. >>> >>>Forcing it by adjusting the pipeline only results in gstreamer >>>failing, because it believes that v4l2 is unable to operate at >>>60fps. >>> >>>Also note the complaints from v4l2src about the non-compliance... >> >> Thanks, I've fixed most of v4l2-compliance issues, but this is not >> done yet. Is that something you can help with? > > I've looked at this, and IMHO it's yet another media control API mess. > > - media-ctl itself allows setting the format on subdev pads via > struct v4l2_subdev_format. > > - struct v4l2_subdev_format contains a struct v4l2_mbus_framefmt. > > - struct v4l2_mbus_framefmt contains: > * @width: frame width > * @height: frame height > * @code: data format code (from enum v4l2_mbus_pixelcode) > * @field: used interlacing type (from enum v4l2_field) > * @colorspace: colorspace of the data (from enum v4l2_colorspace) > * @ycbcr_enc: YCbCr encoding of the data (from enum v4l2_ycbcr_encoding) > * @quantization: quantization of the data (from enum v4l2_quantization) > * @xfer_func: transfer function of the data (from enum v4l2_xfer_func) > > - media-ctl sets width, height, code and field, but nothing else. > > We're already agreed that the fields that media-ctl are part of the > format negotiation between the ultimate source, flowing down to the > capture device. However, there's no support in media-ctl to deal > with these other fields - so media-ctl in itself is only half- > implemented. Correct. The colorspace et al fields are in practice unimportant for sensors. For HDMI/DP they are very important, though. It's the reason why nobody worked on adding support for this to media-ctl, it's almost exclusively used with sensors. Not saying that it is right that it hasn't been added to media-ctl, just that it never had a high enough prio. Regards, Hans > > From what I can tell, _we_ are doing the right thing in imx-media-capture. > > However, I think part of the problem is the set_fmt implementation. > When a source pad is configured via set_fmt(), any fields that can > not be altered (eg, because the subdev doesn't support colorspace > conversion) need to be preserved from the subdev's sink pad. > > Right now, CSI doesn't do that - it only looks at the width, height, > code, and field. > > I think we've got other bugs though that haven't been picked up by any > review - csi_try_fmt() adjusts the format using the _current_ > configuration of the sink pad, even when using V4L2_SUBDEV_FORMAT_TRY. > This seems wrong according to the docs: the purpose of the try > mechanism is to be able to setup the _entire_ pipeline using the TRY > mechanism to work out whether the configuration works, before then > setting for real. If we're validating the TRY formats against the > live configuration, then we're not doing that. > > There's calls for: > > v4l2_subdev_get_try_format > v4l2_subdev_get_try_crop > v4l2_subdev_get_try_compose > > to get the try configuration - we hardly make use of all of these. I > would suggest that we change the approach to implementing the various > subdevs such that: > > 1) like __csi_get_fmt(), we have accessors that gets a pointer to the >correct state for the TRY/live settings. > > 2) everywhere we're asked to get or set parameters that can be TRY/live, >we use these accessors to retrieve a pointer to the correct state to >not only read, but also modify. > > 3) when we're evaluating parameters against another pad, we use these >accessors to obtain the other pad's configuration, rather than poking >about in the state saved in the subdev's priv-> (which is irrelevant >for the TRY variant.) > > 4) ensure that all paramete
Re: [PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()
2017-03-20 21:04 GMT+09:00 walter harms :
>
>
> Am 20.03.2017 11:59, schrieb Daeseok Youn:
>> If v4l2_subdev_call() gets the global frame interval values,
>> it returned 0 and it could be checked whether numerator is zero or not.
>>
>> If the numerator is not zero, the fps could be calculated in this function.
>> If not, it just returns 0.
>>
>> Signed-off-by: Daeseok Youn
>> ---
>> .../media/atomisp/pci/atomisp2/atomisp_cmd.c | 22
>> ++
>> 1 file changed, 10 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> index 8bdb224..6bdd19e 100644
>> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> @@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct
>> video_device *dev)
>>
>> static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device *asd)
>> {
>> - struct v4l2_subdev_frame_interval frame_interval;
>> + struct v4l2_subdev_frame_interval fi;
>> struct atomisp_device *isp = asd->isp;
>> - unsigned short fps;
>>
>> - if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>> - video, g_frame_interval, &frame_interval)) {
>> - fps = 0;
>> - } else {
>> - if (frame_interval.interval.numerator)
>> - fps = frame_interval.interval.denominator /
>> - frame_interval.interval.numerator;
>> - else
>> - fps = 0;
>> - }
>> + unsigned short fps = 0;
>> + int ret;
>> +
>> + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>> +video, g_frame_interval, &fi);
>> +
>> + if (!ret && fi.interval.numerator)
>> + fps = fi.interval.denominator / fi.interval.numerator;
>> +
>> return fps;
>> }
>
>
>
> do you need to check ret at all ? if an error occurs can fi.interval.numerator
> be something else than 0 ?
the return value from the v4l2_subdev_call() function is zero when it
is done without any error. and also I checked
the ret value whether is 0 or not. if the ret is 0 then the value of
numerator should be checked to avoid for dividing by 0.
>
> if ret is an ERRNO it would be wise to return ret not fps, but this may
> require
> changes at other places also.
hmm.., yes, you are right. but I think it is ok because the
atomisp_get_sensor_fps() function is needed to get fps value.
(originally, zero or calculated fps value was returned.)
>
> re,
> wh
>
>>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v7] staging: adis16060_core: Replace mlock with buf_lock and refactor code
The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
In this driver, mlock was being used to protect hardware state
changes. Replace it with buf_lock in the devices global data.
As buf_lock protects both the adis16060_spi_write() and
adis16060_spi_read() functions and both are always called in
pair. First write, then read. Thus, refactor the code to have
one single function adis16060_spi_write_than_read() which is
protected by the existing buf_lock.
Removed nested locks as the function adis16060_read_raw call
a lock on &st->buf_lock and then calls the function
adis16060_spi_write which again tries to get hold
of the same lock.
The locks in adis16060_read_raw are dropped and now have a
single safe function adis16060_spi_write_than_read
with the locks inside it being called.
Signed-off-by: simran singhal
---
v7:
-Change subject
-Remove lock from read_raw instead of from
function adis16060_spi_write_than_read().
drivers/staging/iio/gyro/adis16060_core.c | 38 +--
1 file changed, 11 insertions(+), 27 deletions(-)
diff --git a/drivers/staging/iio/gyro/adis16060_core.c
b/drivers/staging/iio/gyro/adis16060_core.c
index c9d46e7..193587c 100644
--- a/drivers/staging/iio/gyro/adis16060_core.c
+++ b/drivers/staging/iio/gyro/adis16060_core.c
@@ -40,25 +40,18 @@ struct adis16060_state {
static struct iio_dev *adis16060_iio_dev;
-static int adis16060_spi_write(struct iio_dev *indio_dev, u8 val)
+static int adis16060_spi_write_than_read(struct iio_dev *indio_dev,
+u8 conf, u16 *val)
{
int ret;
struct adis16060_state *st = iio_priv(indio_dev);
mutex_lock(&st->buf_lock);
- st->buf[2] = val; /* The last 8 bits clocked in are latched */
+ st->buf[2] = conf; /* The last 8 bits clocked in are latched */
ret = spi_write(st->us_w, st->buf, 3);
- mutex_unlock(&st->buf_lock);
-
- return ret;
-}
-
-static int adis16060_spi_read(struct iio_dev *indio_dev, u16 *val)
-{
- int ret;
- struct adis16060_state *st = iio_priv(indio_dev);
- mutex_lock(&st->buf_lock);
+ if (ret < 0)
+ return ret;
ret = spi_read(st->us_r, st->buf, 3);
@@ -69,8 +62,8 @@ static int adis16060_spi_read(struct iio_dev *indio_dev, u16
*val)
*/
if (!ret)
*val = ((st->buf[0] & 0x3) << 12) |
- (st->buf[1] << 4) |
- ((st->buf[2] >> 4) & 0xF);
+(st->buf[1] << 4) |
+((st->buf[2] >> 4) & 0xF);
mutex_unlock(&st->buf_lock);
return ret;
@@ -83,20 +76,15 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
{
u16 tval = 0;
int ret;
+ struct adis16060_state *st = iio_priv(indio_dev);
switch (mask) {
case IIO_CHAN_INFO_RAW:
- /* Take the iio_dev status lock */
- mutex_lock(&indio_dev->mlock);
- ret = adis16060_spi_write(indio_dev, chan->address);
+ ret = adis16060_spi_write_than_read(indio_dev,
+ chan->address, &tval);
if (ret < 0)
- goto out_unlock;
+ return ret;
- ret = adis16060_spi_read(indio_dev, &tval);
- if (ret < 0)
- goto out_unlock;
-
- mutex_unlock(&indio_dev->mlock);
*val = tval;
return IIO_VAL_INT;
case IIO_CHAN_INFO_OFFSET:
@@ -110,10 +98,6 @@ static int adis16060_read_raw(struct iio_dev *indio_dev,
}
return -EINVAL;
-
-out_unlock:
- mutex_unlock(&indio_dev->mlock);
- return ret;
}
static const struct iio_info adis16060_info = {
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
On 03/19/2017 06:54 PM, Steve Longerbeam wrote:
>
>
> On 03/19/2017 03:38 AM, Russell King - ARM Linux wrote:
>> On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote:
>>> Right, imx-media-capture.c (the "standard" v4l2 user interface module)
>>> is not implementing VIDIOC_ENUM_FRAMESIZES. It should, but it can only
>>> return the single frame size that the pipeline has configured (the mbus
>>> format of the attached source pad).
>> I now have a set of patches that enumerate the frame sizes and intervals
>> from the source pad of the first subdev (since you're setting the formats
>> etc there from the capture device, it seems sensible to return what it
>> can support.) This means my patch set doesn't add to non-CSI subdevs.
>>
>>> Can you share your gstreamer pipeline? For now, until
>>> VIDIOC_ENUM_FRAMESIZES is implemented, try a pipeline that
>>> does not attempt to specify a frame rate. I use the attached
>>> script for testing, which works for me.
>> Note that I'm not specifying a frame rate on gstreamer - I'm setting
>> the pipeline up for 60fps, but gstreamer in its wisdom is unable to
>> enumerate the frame sizes, and therefore is unable to enumerate the
>> frame intervals (frame intervals depend on frame sizes), so it
>> falls back to the "tvnorms" which are basically 25/1 and 3/1001.
>>
>> It sees 60fps via G_PARM, and then decides to set 3/1001 via S_PARM.
>> So, we end up with most of the pipeline operating at 60fps, with CSI
>> doing frame skipping to reduce the frame rate to 30fps.
>>
>> gstreamer doesn't complain, doesn't issue any warnings, the only way
>> you can spot this is to enable debugging and look through the copious
>> debug log, or use -v and check the pad capabilities.
>>
>> Testing using gstreamer, and only using "does it produce video" is a
>> good simple test, but it's just that - it's a simple test. It doesn't
>> tell you that what you're seeing is what you intended to see (such as
>> video at the frame rate you expected) without more work.
>>
>>> Thanks, I've fixed most of v4l2-compliance issues, but this is not
>>> done yet. Is that something you can help with?
>> What did you do with:
>>
>> ioctl(3, VIDIOC_REQBUFS, {count=0, type=0 /* V4L2_BUF_TYPE_??? */, memory=0
>> /* V4L2_MEMORY_??? */}) = -1 EINVAL (Invalid argument)
>> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>> ioctl(3, VIDIOC_EXPBUF, 0xbef405bc) = -1 EINVAL (Invalid argument)
>> fail: v4l2-test-buffers.cpp(571): q.has_expbuf(node)
This is really a knock-on effect from an earlier issue where the compliance test
didn't detect support for MEMORY_MMAP.
>> test VIDIOC_EXPBUF: FAIL
>>
>> To me, this looks like a bug in v4l2-compliance (I'm using 1.10.0).
Always build from the master repo. 1.10 is pretty old.
>> I'm not sure what buffer VIDIOC_EXPBUF is expected to export, since
>> afaics no buffers have been allocated, so of course it's going to fail.
It just tests if EXPBUF is supported.
I think I will modify v4l2-compliance to bail out if it doesn't find support
for MEMORY_MMAP. Even though in theory support for this is optional, in practice
all applications expect that it is supported. That should fix this
hard-to-understand error.
>> Either that, or the v4l2 core vb2 code is non-compliant with v4l2's
>> interface requirements.
>>
>> In any case, it doesn't look like the buffer management is being
>> tested at all by v4l2-compliance - we know that gstreamer works, so
>> buffers _can_ be allocated, and I've also used dmabufs with gstreamer,
>> so I also know that VIDIOC_EXPBUF works there.
To test actual streaming you need to provide the -s option.
Note: v4l2-compliance has been developed for 'regular' video devices,
not MC devices. It may or may not work with the -s option.
As I think I mentioned somewhere else, creating a compliance test for
MC devices would help enormously in verifying drivers. I'm not sure if
it is better to create a new test or integrate it in v4l2-compliance.
I'm leaning towards the latter since there is a lot of overlap.
>>
>
> I wouldn't be surprised if you hit on a bug in v4l2-compliance. I
> stopped with v4l2-compliance
> at a different test failure that also didn't make sense to me:
>
> Streaming ioctls:
> test read/write: OK (Not Supported)
> Video Capture:
> Buffer: 0 Sequence: 0 Field: Any Timestamp: 41.664259s
> fail:
> .../v4l-utils-1.6.2/utils/v4l2-compliance/v4l2-test-buffers.cpp(281):
> !(g_flags() & (V4L2_BUF_FLAG_DONE | V4L2_BUF_FLAG_ERROR))
> fail:
> .../v4l-utils-1.6.2/utils/v4l2-compliance/v4l2-test-buffers.cpp(610):
> buf.check(q, last_seq)
> fail:
> .../v4l-utils-1.6.2/utils/v4l2-compliance/v4l2-test-buffers.cpp(883):
> captureBufs(node, q, m2m_q, frame_count, false)
> test MMAP: FAIL
> test USERPTR: OK (Not Supported)
> test DMABUF: Cannot test, specify --expbuf-device
>
> Total: 42, Succeeded: 38, Failed: 4,
Re: [PATCH v4] staging: sm750fb: Code readability is improved
On Mon, Mar 20, 2017 at 06:25:19PM +0530, Arushi Singhal wrote:
> On Mon, Mar 20, 2017 at 5:58 PM, Dan Carpenter
> wrote:
>
> > On Sun, Mar 19, 2017 at 09:19:20PM +0530, Arushi Singhal wrote:
> > > New variables are added to make the code more readable.
> > >
> > > Signed-off-by: Arushi Singhal
> > > ---
> > > changes in v4
> > > -try to make the code much more readable.
> > > - defined the variable at the top of a block.
> > > ---
> > > drivers/staging/sm750fb/ddk750_mode.c | 57
> > +++
> > > 1 file changed, 31 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/drivers/staging/sm750fb/ddk750_mode.c
> > b/drivers/staging/sm750fb/ddk750_mode.c
> > > index eea5aef2956f..6517e770e0a7 100644
> > > --- a/drivers/staging/sm750fb/ddk750_mode.c
> > > +++ b/drivers/staging/sm750fb/ddk750_mode.c
> > > @@ -76,38 +76,43 @@ static int programModeRegisters(mode_parameter_t
> > *pModeParam, struct pll_value *
> > > {
> > > int ret = 0;
> > > int cnt = 0;
> > > - unsigned int tmp, reg;
> > > + unsigned int tmp, reg, temp;
> >
> > Let's not have "tmp" and "temp" both. Generally "tmp" is better because
> > you can't confuse it with temperature.
> >
>
> Hi Dan
> I have not added the tmp variable.
> So is it good to use any other variable like "a" instead of temp.
Just re-use "tmp". No need to add "temp".
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline
On 03/17/2017 03:37 PM, Russell King - ARM Linux wrote: > On Fri, Mar 17, 2017 at 02:51:10PM +0100, Philipp Zabel wrote: >> On Fri, 2017-03-17 at 10:24 -0300, Mauro Carvalho Chehab wrote: >> [...] >>> The big question, waiting for an answer on the last 8 years is >>> who would do that? Such person would need to have several different >>> hardware from different vendors, in order to ensure that it has >>> a generic solution. >>> >>> It is a way more feasible that the Kernel developers that already >>> have a certain hardware on their hands to add support inside the >>> driver to forward the controls through the pipeline and to setup >>> a "default" pipeline that would cover the common use cases at >>> driver's probe. >> >> Actually, would setting pipeline via libv4l2 plugin and letting drivers >> provide a sane enabled default pipeline configuration be mutually >> exclusive? Not sure about the control forwarding, but at least a simple >> link setup and format forwarding would also be possible in the kernel >> without hindering userspace from doing it themselves later. > > I think this is the exact same problem as controls in ALSA. > > When ALSA started off in life, the requirement was that all controls > shall default to minimum, and the user is expected to adjust controls > after the system is running. > > After OSS, this gave quite a marked change in system behaviour, and > led to a lot of "why doesn't my sound work anymore" problems, because > people then had to figure out which combination of controls had to be > set to get sound out of their systems. > > Now it seems to be much better, where install Linux on a platform, and > you have a working sound system (assuming that the drivers are all there > which is generally the case for x86.) > > However, it's still possible to adjust all the controls from userspace. > All that's changed is the defaults. > > Why am I mentioning this - because from what I understand Mauro saying, > it's no different from this situation. Userspace will still have the > power to disable all links and setup its own. The difference is that > there will be a default configuration that the kernel sets up at boot > time that will be functional, rather than the current default > configuration where the system is completely non-functional until > manually configured. > > However, at the end of the day, I don't care _where_ the usability > problems are solved, only that there is some kind of solution. It's not > the _where_ that's the real issue here, but the _how_, and discussion of > the _how_ is completely missing. > > So, let's try kicking off a discussion about _how_ to do things. > > _How_ do we setup a media controller system so that we end up with a > usable configuration - let's start with the obvious bit... which links > should be enabled. > > I think the first pre-requisit is that we stop exposing capture devices > that can never be functional for the hardware that's present on the board, > so that there isn't this plentora of useless /dev/video* nodes and useless > subdevices. > > One possible solution to finding a default path may be "find the shortest > path between the capture device and the sensor and enable intervening > links". > > Then we need to try configuring that path with format/resolution > information. > > However, what if something in the shortest path can't handle the format > that the sensor produces? I think at that point, we'd need to drop that > subdev out of the path resolution, re-run the "find the shortest path" > algorithm, and try again. > > Repeat until success or no path between the capture and sensor exists. > > This works fine if you have just one sensor visible from a capture device, > but not if there's more than one (which I suspect is the case with the > Sabrelite board with its two cameras and video receiver.) That breaks > the "find the shortest path" algorithm. > > So, maybe it's a lot better to just let the board people provide via DT > a default setup for the connectivity of the modules somehow - certainly > one big step forward would be to disable in DT parts of the capture > system that can never be used (remembering that boards like the RPi / > Hummingboard may end up using DT overlays to describe this for different > cameras, so the capture setup may change after initial boot.) The MC was developed before the device tree came along. But now that the DT is here, I think this could be a sensible idea to let the DT provide an initial path. Sakari, Laurent, Mauro: any opinions? Regards, Hans ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()
Am 20.03.2017 13:51, schrieb DaeSeok Youn:
> 2017-03-20 21:04 GMT+09:00 walter harms :
>>
>>
>> Am 20.03.2017 11:59, schrieb Daeseok Youn:
>>> If v4l2_subdev_call() gets the global frame interval values,
>>> it returned 0 and it could be checked whether numerator is zero or not.
>>>
>>> If the numerator is not zero, the fps could be calculated in this function.
>>> If not, it just returns 0.
>>>
>>> Signed-off-by: Daeseok Youn
>>> ---
>>> .../media/atomisp/pci/atomisp2/atomisp_cmd.c | 22
>>> ++
>>> 1 file changed, 10 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>>> b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>>> index 8bdb224..6bdd19e 100644
>>> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>>> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>>> @@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct
>>> video_device *dev)
>>>
>>> static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device
>>> *asd)
>>> {
>>> - struct v4l2_subdev_frame_interval frame_interval;
>>> + struct v4l2_subdev_frame_interval fi;
>>> struct atomisp_device *isp = asd->isp;
>>> - unsigned short fps;
>>>
>>> - if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>>> - video, g_frame_interval, &frame_interval)) {
>>> - fps = 0;
>>> - } else {
>>> - if (frame_interval.interval.numerator)
>>> - fps = frame_interval.interval.denominator /
>>> - frame_interval.interval.numerator;
>>> - else
>>> - fps = 0;
>>> - }
>>> + unsigned short fps = 0;
>>> + int ret;
>>> +
>>> + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>>> +video, g_frame_interval, &fi);
>>> +
>>> + if (!ret && fi.interval.numerator)
>>> + fps = fi.interval.denominator / fi.interval.numerator;
>>> +
>>> return fps;
>>> }
>>
>>
>>
>> do you need to check ret at all ? if an error occurs can
>> fi.interval.numerator
>> be something else than 0 ?
> the return value from the v4l2_subdev_call() function is zero when it
> is done without any error. and also I checked
> the ret value whether is 0 or not. if the ret is 0 then the value of
> numerator should be checked to avoid for dividing by 0.
>>
>> if ret is an ERRNO it would be wise to return ret not fps, but this may
>> require
>> changes at other places also.
> hmm.., yes, you are right. but I think it is ok because the
> atomisp_get_sensor_fps() function is needed to get fps value.
> (originally, zero or calculated fps value was returned.)
maybe its better to divide this in:
if (ret)
return 0; // error case
return (fi.interval.numerator>0)?fi.interval.denominator /
fi.interval.numerator:0;
So there is a chance that someone will a) understand and b) fix the error
return.
re,
wh
>
>>
>> re,
>> wh
>>
>>>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
On Sat, 2017-03-18 at 12:58 -0700, Steve Longerbeam wrote: > > On 03/18/2017 12:22 PM, Russell King - ARM Linux wrote: > > Hi Steve, > > > > I've just been trying to get gstreamer to capture and h264 encode > > video from my camera at various frame rates, and what I've discovered > > does not look good. > > > > 1) when setting frame rates, media-ctl _always_ calls > > VIDIOC_SUBDEV_S_FRAME_INTERVAL with pad=0. To allow setting pad > 0: https://patchwork.linuxtv.org/patch/39348/ > > 2) media-ctl never retrieves the frame interval information, so there's > > no way to read it back with standard tools, and no indication that > > this is going on... > > I think Philipp Zabel submitted a patch which addresses these > in media-ctl. Check with him. To read back and propagate the frame interval: https://patchwork.linuxtv.org/patch/39349/ https://patchwork.linuxtv.org/patch/39351/ regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
On Sun, 2017-03-19 at 12:14 +, Russell King - ARM Linux wrote: > On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote: > > On 03/18/2017 12:22 PM, Russell King - ARM Linux wrote: > > >0:00:01.955927879 20954 0x15ffe90 INFOv4l2 > > >gstv4l2object.c:3811:gst_v4l2_object_get_caps: probed caps: > > >video/x-bayer, format=(string)rggb, framerate=(fraction)3/1001, > > >width=(int)816, height=(int)616, pixel-aspect-ratio=(fraction)1/1; > > >video/x-raw, format=(string)I420, framerate=(fraction)3/1001, > > >width=(int)816, height=(int)616, interlace-mode=(string)progressive, > > >pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)YV12, > > >framerate=(fraction)3/1001, width=(int)816, height=(int)616, > > >interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1; > > >video/x-raw, format=(string)BGR, framerate=(fraction)3/1001, > > >width=(int)816, height=(int)616, interlace-mode=(string)progressive, > > >pixel-aspect-ratio=(fraction)1/1; video/x-raw, format=(string)RGB, > > >framerate=(fraction)3/1001, width=(int)816, height=(int)616, > > >interlace-mode=(string)progressive, pixel-aspect-ratio=(fraction)1/1 > > > > > >despite the media pipeline actually being configured for 60fps. > > > > > >Forcing it by adjusting the pipeline only results in gstreamer > > >failing, because it believes that v4l2 is unable to operate at > > >60fps. > > > > > >Also note the complaints from v4l2src about the non-compliance... > > > > Thanks, I've fixed most of v4l2-compliance issues, but this is not > > done yet. Is that something you can help with? > > I've looked at this, and IMHO it's yet another media control API mess. > > - media-ctl itself allows setting the format on subdev pads via > struct v4l2_subdev_format. > > - struct v4l2_subdev_format contains a struct v4l2_mbus_framefmt. > > - struct v4l2_mbus_framefmt contains: > * @width: frame width > * @height: frame height > * @code: data format code (from enum v4l2_mbus_pixelcode) > * @field: used interlacing type (from enum v4l2_field) > * @colorspace: colorspace of the data (from enum v4l2_colorspace) > * @ycbcr_enc: YCbCr encoding of the data (from enum v4l2_ycbcr_encoding) > * @quantization: quantization of the data (from enum v4l2_quantization) > * @xfer_func: transfer function of the data (from enum v4l2_xfer_func) > > - media-ctl sets width, height, code and field, but nothing else. > > We're already agreed that the fields that media-ctl are part of the > format negotiation between the ultimate source, flowing down to the > capture device. However, there's no support in media-ctl to deal > with these other fields - so media-ctl in itself is only half- > implemented. To set and read colorimetry information: https://patchwork.linuxtv.org/patch/39350/ regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline
On 03/14/2017 11:21 AM, Mauro Carvalho Chehab wrote: > Em Tue, 14 Mar 2017 08:55:36 +0100 > Hans Verkuil escreveu: > >> On 03/14/2017 04:45 AM, Mauro Carvalho Chehab wrote: >>> Hi Sakari, >>> >>> I started preparing a long argument about it, but gave up in favor of a >>> simpler one. >>> >>> Em Mon, 13 Mar 2017 14:46:22 +0200 >>> Sakari Ailus escreveu: >>> Drivers are written to support hardware, not particular use case. >>> >>> No, it is just the reverse: drivers and hardware are developed to >>> support use cases. >>> >>> Btw, you should remember that the hardware is the full board, not just the >>> SoC. In practice, the board do limit the use cases: several provide a >>> single physical CSI connector, allowing just one sensor. >>> > This situation is there since 2009. If I remember well, you tried to write > such generic plugin in the past, but never finished it, apparently because > it is too complex. Others tried too over the years. I'd argue I know better what happened with that attempt than you do. I had a prototype of a generic pipeline configuration library but due to various reasons I haven't been able to continue working on that since around 2012. >>> >>> ... >>> > The last trial was done by Jacek, trying to cover just the exynos4 > driver. > Yet, even such limited scope plugin was not good enough, as it was never > merged upstream. Currently, there's no such plugins upstream. > > If we can't even merge a plugin that solves it for just *one* driver, > I have no hope that we'll be able to do it for the generic case. I believe Jacek ceased to work on that plugin in his day job; other than that, there are some matters left to be addressed in his latest patchset. >>> >>> The two above basically summaries the issue: the task of doing a generic >>> plugin on userspace, even for a single driver is complex enough to >>> not cover within a reasonable timeline. >>> >>> From 2009 to 2012, you were working on it, but didn't finish it. >>> >>> Apparently, nobody worked on it between 2013-2014 (but I may be wrong, as >>> I didn't check when the generic plugin interface was added to libv4l). >>> >>> In the case of Jacek's work, the first patch I was able to find was >>> written in Oct, 2014: >>> https://patchwork.kernel.org/patch/5098111/ >>> (not sure what happened with the version 1). >>> >>> The last e-mail about this subject was issued in Dec, 2016. >>> >>> In summary, you had this on your task for 3 years for an OMAP3 >>> plugin (where you have a good expertise), and Jacek for 2 years, >>> for Exynos 4, where he should also have a good knowledge. >>> >>> Yet, with all that efforts, no concrete results were achieved, as none >>> of the plugins got merged. >>> >>> Even if they were merged, if we keep the same mean time to develop a >>> libv4l plugin, that would mean that a plugin for i.MX6 could take 2-3 >>> years to be developed. >>> >>> There's a clear message on it: >>> - we shouldn't keep pushing for a solution via libv4l. >> >> Or: >> >> - userspace plugin development had a very a low priority and >>never got the attention it needed. > > The end result is the same: we can't count on it. > >> >> I know that's *my* reason. I rarely if ever looked at it. I always assumed >> Sakari and/or Laurent would look at it. If this reason is also valid for >> Sakari and Laurent, then it is no wonder nothing has happened in all that >> time. >> >> We're all very driver-development-driven, and userspace gets very little >> attention in general. So before just throwing in the towel we should take >> a good look at the reasons why there has been little or no development: is >> it because of fundamental design defects, or because nobody paid attention >> to it? > > No. We should look it the other way: basically, there are patches > for i.MX6 driver that sends control from videonode to subdevs. > > If we nack apply it, who will write the userspace plugin? When > such change will be merged upstream? > > If we don't have answers to any of the above questions, we should not > nack it. > > That's said, that doesn't prevent merging a libv4l plugin if/when > someone can find time/interest to develop it. I don't think this control inheritance patch will magically prevent you from needed a plugin. > >> I strongly suspect it is the latter. >> >> In addition, I suspect end-users of these complex devices don't really care >> about a plugin: they want full control and won't typically use generic >> applications. If they would need support for that, we'd have seen much more >> interest. The main reason for having a plugin is to simplify testing and >> if this is going to be used on cheap hobbyist devkits. > > What are the needs for a cheap hobbyist devkit owner? Do we currently > satisfy those needs? I'd say that having a functional driver when > compiled without the subdev A
Re: [PATCH v5 00/39] i.MX Media Driver
On Mon, Mar 20, 2017 at 02:01:58PM +0100, Hans Verkuil wrote:
> On 03/19/2017 06:54 PM, Steve Longerbeam wrote:
> >
> >
> > On 03/19/2017 03:38 AM, Russell King - ARM Linux wrote:
> >> What did you do with:
> >>
> >> ioctl(3, VIDIOC_REQBUFS, {count=0, type=0 /* V4L2_BUF_TYPE_??? */,
> >> memory=0 /* V4L2_MEMORY_??? */}) = -1 EINVAL (Invalid argument)
> >> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> >> ioctl(3, VIDIOC_EXPBUF, 0xbef405bc) = -1 EINVAL (Invalid argument)
> >> fail: v4l2-test-buffers.cpp(571): q.has_expbuf(node)
>
> This is really a knock-on effect from an earlier issue where the compliance
> test
> didn't detect support for MEMORY_MMAP.
So why does it succeed when I fix the compliance errors with VIDIOC_G_FMT?
With that fixed, I now get:
Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK (Not Supported)
Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test VIDIOC_EXPBUF: OK
The reason is, if you look at the code, VIDIOC_G_FMT populates a list
of possible buffer formats "node->valid_buftypes". If the VIDIOC_G_FMT
test fails, then node->valid_buftypes is zero.
This causes testReqBufs() to only check for the all-zeroed VIDIOC_REQBUFS
and declare it conformant, without creating any buffers (it can't, it
doesn't know which formats are supported.)
This causes node->valid_memorytype to be zero.
We then go on to testExpBuf(), and valid_memorytype zero, claiming (falsely)
that MMAP is not supported. The reality is that it _is_ supported, but
it's just the non-compliant VICIOC_G_FMT call (due to the colorspace
issue) causes the sequence of tests to fail.
> Always build from the master repo. 1.10 is pretty old.
It's what I have - remember, not everyone is happy to constantly replace
their distro packages with random new stuff.
> >> In any case, it doesn't look like the buffer management is being
> >> tested at all by v4l2-compliance - we know that gstreamer works, so
> >> buffers _can_ be allocated, and I've also used dmabufs with gstreamer,
> >> so I also know that VIDIOC_EXPBUF works there.
>
> To test actual streaming you need to provide the -s option.
>
> Note: v4l2-compliance has been developed for 'regular' video devices,
> not MC devices. It may or may not work with the -s option.
Right, and it exists to verify that the establised v4l2 API is correctly
implemented. If the v4l2 API is being offered to user applications,
then it must be conformant, otherwise it's not offering the v4l2 API.
(That's very much a definition statement in itself.)
So, are we really going to say MC devices do not offer the v4l2 API to
userspace, but something that might work? We've already seen today
one user say that they're not going to use mainline because of the
crud surrounding MC.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
On 03/20/2017 02:29 PM, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 02:01:58PM +0100, Hans Verkuil wrote:
>> On 03/19/2017 06:54 PM, Steve Longerbeam wrote:
>>>
>>>
>>> On 03/19/2017 03:38 AM, Russell King - ARM Linux wrote:
What did you do with:
ioctl(3, VIDIOC_REQBUFS, {count=0, type=0 /* V4L2_BUF_TYPE_??? */,
memory=0 /* V4L2_MEMORY_??? */}) = -1 EINVAL (Invalid argument)
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
ioctl(3, VIDIOC_EXPBUF, 0xbef405bc) = -1 EINVAL (Invalid argument)
fail: v4l2-test-buffers.cpp(571): q.has_expbuf(node)
>>
>> This is really a knock-on effect from an earlier issue where the compliance
>> test
>> didn't detect support for MEMORY_MMAP.
>
> So why does it succeed when I fix the compliance errors with VIDIOC_G_FMT?
> With that fixed, I now get:
>
> Format ioctls:
> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
> test VIDIOC_G/S_PARM: OK
> test VIDIOC_G_FBUF: OK (Not Supported)
> test VIDIOC_G_FMT: OK
> test VIDIOC_TRY_FMT: OK
> test VIDIOC_S_FMT: OK
> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
> test Cropping: OK (Not Supported)
> test Composing: OK (Not Supported)
> test Scaling: OK (Not Supported)
>
> Buffer ioctls:
> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> test VIDIOC_EXPBUF: OK
>
> The reason is, if you look at the code, VIDIOC_G_FMT populates a list
> of possible buffer formats "node->valid_buftypes". If the VIDIOC_G_FMT
> test fails, then node->valid_buftypes is zero.
>
> This causes testReqBufs() to only check for the all-zeroed VIDIOC_REQBUFS
> and declare it conformant, without creating any buffers (it can't, it
> doesn't know which formats are supported.)
>
> This causes node->valid_memorytype to be zero.
It should fail on this and return a more understandable error message.
>
> We then go on to testExpBuf(), and valid_memorytype zero, claiming (falsely)
> that MMAP is not supported. The reality is that it _is_ supported, but
> it's just the non-compliant VICIOC_G_FMT call (due to the colorspace
> issue) causes the sequence of tests to fail.
Yeah, you're not the first to complain about this. I plan on fixing this this
week.
>
>> Always build from the master repo. 1.10 is pretty old.
>
> It's what I have - remember, not everyone is happy to constantly replace
> their distro packages with random new stuff.
This is a compliance test, which is continuously developed in tandem with
new kernel versions. If you are working with an upstream kernel, then you
should also use the corresponding v4l2-compliance test. What's the point
of using an old one?
I will not support driver developers that use an old version of the
compliance test, that's a waste of my time.
>
In any case, it doesn't look like the buffer management is being
tested at all by v4l2-compliance - we know that gstreamer works, so
buffers _can_ be allocated, and I've also used dmabufs with gstreamer,
so I also know that VIDIOC_EXPBUF works there.
>>
>> To test actual streaming you need to provide the -s option.
>>
>> Note: v4l2-compliance has been developed for 'regular' video devices,
>> not MC devices. It may or may not work with the -s option.
>
> Right, and it exists to verify that the establised v4l2 API is correctly
> implemented. If the v4l2 API is being offered to user applications,
> then it must be conformant, otherwise it's not offering the v4l2 API.
> (That's very much a definition statement in itself.)
>
> So, are we really going to say MC devices do not offer the v4l2 API to
> userspace, but something that might work? We've already seen today
> one user say that they're not going to use mainline because of the
> crud surrounding MC.
>
Actually, my understanding was that he was stuck on the old kernel code.
In the case of v4l2-compliance, I never had the time to make it work with
MC devices. Same for that matter of certain memory to memory devices.
Just like MC devices these too behave differently. They are partially
supported in v4l2-compliance, but not fully.
Why? NO TIME.
Be glad there *is* a v4l2-compliance test at all! It's really, really useful
already, but it took *years* to develop, little bit by little bit. And yes,
I would really like to update it to fully support codecs and MC devices.
And with a bit of luck I hope to get permission from my boss to work on
this (among others) later in the year.
Complaining about this really won't help. We know it's a problem and unless
someone (me perhaps?) manages to get paid to work on this it's unlikely to
change for now.
Regards,
Hans
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/d
Re: [PATCH 0/5] staging: vc04_services: camera driver maintance
On Mon, Mar 20, 2017 at 04:06:00AM -0700, Michael Zoran wrote: > On Mon, 2017-03-20 at 13:55 +0300, Dan Carpenter wrote: > > I'm not going to review this because it has kbuild errors. > > > > regards, > > dan carpenter > > > > Hi, can you e-mail out the errors or send them to me. It worked when > I submitted it. > The problem is that you added a function only for ARM but the camera can build on i386. Anyway, we need to figure out why you aren't getting the kbuild emails and fix that. I forwarded the first email to you. The other is basically the same. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt
On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote: > On Mon, Mar 20, 2017 at 12:55:26PM +0100, Philipp Zabel wrote: > > The above paragraph suggests we skip any rectangles that are not > > supported. In our case that would be 3. and 4., since the CSI can't > > compose into a larger frame. I hadn't realised that the crop selection > > currently happens on the source pad. > > I'd recommend viewing the documentation in its post-processed version, > because then you get the examples as pictures, and they say that a > picture is worth 1000 words. See > > https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/dev-subdev.html > > There is almost an exact example of what we're trying to do - it's > figure 4.6. Here, we have a sink pad with a cropping rectangle on > the input, which is then scaled to a composition rectangle (there's > no bounds rectangle, and it's specified that in such a case the > top,left of the composition rectangle will always be 0,0 - see quote > below). > > Where it differs is that the example also supports source cropping > for two source pads. We don't support that. > > The same document says: > > Scaling support is optional. When supported by a subdev, the crop > rectangle on the subdev's sink pad is scaled to the size configured > using the > :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL > using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the > subdev supports scaling but not composing, the top and left values are > not used and must always be set to zero. Right, this sentence does imply that when scaling is supported, there must be a sink compose rectangle, even when composing is not. I have previously set up scaling like this: media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]" media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]" Does this mean, it should work like this instead? media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]" media-ctl --set-v4l2 "'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]" media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]" I suppose setting the source pad format should not be allowed to modify the sink compose rectangle. regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5] staging:sm750fb: Code readability is improved.
New variables are added to make the code more readable.
Signed-off-by: Arushi Singhal
---
changes in v5
-try to make the code much more readable.
- defined the variable at the top of a block.
- change the variable from temp to tmp.
drivers/staging/sm750fb/ddk750_mode.c | 55 +++
1 file changed, 30 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/sm750fb/ddk750_mode.c
b/drivers/staging/sm750fb/ddk750_mode.c
index eea5aef2956f..37b5d4850fb9 100644
--- a/drivers/staging/sm750fb/ddk750_mode.c
+++ b/drivers/staging/sm750fb/ddk750_mode.c
@@ -81,33 +81,38 @@ static int programModeRegisters(mode_parameter_t
*pModeParam, struct pll_value *
if (pll->clockType == SECONDARY_PLL) {
/* programe secondary pixel clock */
poke32(CRT_PLL_CTRL, sm750_format_pll_reg(pll));
- poke32(CRT_HORIZONTAL_TOTAL,
- (((pModeParam->horizontal_total - 1) <<
-CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
- CRT_HORIZONTAL_TOTAL_TOTAL_MASK) |
- ((pModeParam->horizontal_display_end - 1) &
- CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK));
-
- poke32(CRT_HORIZONTAL_SYNC,
- ((pModeParam->horizontal_sync_width <<
-CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) &
- CRT_HORIZONTAL_SYNC_WIDTH_MASK) |
- ((pModeParam->horizontal_sync_start - 1) &
- CRT_HORIZONTAL_SYNC_START_MASK));
- poke32(CRT_VERTICAL_TOTAL,
- (((pModeParam->vertical_total - 1) <<
-CRT_VERTICAL_TOTAL_TOTAL_SHIFT) &
- CRT_VERTICAL_TOTAL_TOTAL_MASK) |
- ((pModeParam->vertical_display_end - 1) &
- CRT_VERTICAL_TOTAL_DISPLAY_END_MASK));
+ tmp = ((pModeParam->horizontal_total - 1) <<
+ CRT_HORIZONTAL_TOTAL_TOTAL_SHIFT) &
+CRT_HORIZONTAL_TOTAL_TOTAL_MASK;
+ tmp |= (pModeParam->horizontal_display_end - 1) &
+ CRT_HORIZONTAL_TOTAL_DISPLAY_END_MASK;
- poke32(CRT_VERTICAL_SYNC,
- ((pModeParam->vertical_sync_height <<
-CRT_VERTICAL_SYNC_HEIGHT_SHIFT) &
- CRT_VERTICAL_SYNC_HEIGHT_MASK) |
- ((pModeParam->vertical_sync_start - 1) &
- CRT_VERTICAL_SYNC_START_MASK));
+ poke32(CRT_HORIZONTAL_TOTAL, tmp);
+
+ tmp = (pModeParam->horizontal_sync_width <<
+ CRT_HORIZONTAL_SYNC_WIDTH_SHIFT) &
+CRT_HORIZONTAL_SYNC_WIDTH_MASK;
+ tmp |= (pModeParam->horizontal_sync_start - 1) &
+ CRT_HORIZONTAL_SYNC_START_MASK;
+
+ poke32(CRT_HORIZONTAL_SYNC, tmp);
+
+ tmp = ((pModeParam->vertical_total - 1) <<
+ CRT_VERTICAL_TOTAL_TOTAL_SHIFT) &
+CRT_VERTICAL_TOTAL_TOTAL_MASK;
+ tmp |= (pModeParam->vertical_display_end - 1) &
+ CRT_VERTICAL_TOTAL_DISPLAY_END_MASK;
+
+ poke32(CRT_VERTICAL_TOTAL, tmp);
+
+ tmp = ((pModeParam->vertical_sync_height <<
+ CRT_VERTICAL_SYNC_HEIGHT_SHIFT)) &
+CRT_VERTICAL_SYNC_HEIGHT_MASK;
+ tmp |= (pModeParam->vertical_sync_start - 1) &
+ CRT_VERTICAL_SYNC_START_MASK;
+
+ poke32(CRT_VERTICAL_SYNC, tmp);
tmp = DISPLAY_CTRL_TIMING | DISPLAY_CTRL_PLANE;
if (pModeParam->vertical_sync_polarity)
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vc04_services: fix NULL pointer dereference on pointer 'service'
From: Colin Ian King
Currently, if pservice is null then service is set to NULL and immediately
afterwards service is dereferenced causing a null pointer dereference. Fix
this by bailing out early of the function with a null return.
Detected by CoverityScan, CID#1419681 ("Explicit null dereferenced")
Signed-off-by: Colin Ian King
---
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index dc9f85c2a5da..4f9e738abddf 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -2673,7 +2673,7 @@ vchiq_add_service_internal(VCHIQ_STATE_T *state,
if (!pservice) {
kfree(service);
- service = NULL;
+ return NULL;
}
service_quota = &state->service_quotas[service->localport];
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: adc: Replace mlock with driver private lock
The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
In this driver, mlock was being used to protect hardware state
changes. Replace it with a lock in the devices global data.
Signed-off-by: Arushi Singhal
---
drivers/staging/iio/adc/ad7606.c | 8
drivers/staging/iio/adc/ad7606.h | 2 ++
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 9dbfa64b1e53..9f529b34e018 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -208,7 +208,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_SCALE:
ret = -EINVAL;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
for (i = 0; i < ARRAY_SIZE(scale_avail); i++)
if (val2 == scale_avail[i][1]) {
gpiod_set_value(st->gpio_range, i);
@@ -217,7 +217,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
ret = 0;
break;
}
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return ret;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -231,11 +231,11 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
values[1] = (ret >> 1) & 1;
values[2] = (ret >> 2) & 1;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
values);
st->oversampling = val;
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return 0;
default:
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 746f9553d2ba..5d59bdd78727 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -14,6 +14,7 @@
* @name: identification string for chip
* @channels: channel specification
* @num_channels: number of channels
+ * @lock protect sensor state
*/
struct ad7606_chip_info {
@@ -37,6 +38,7 @@ struct ad7606_state {
booldone;
void __iomem*base_address;
+ struct mutexlock; /* protect sensor state */
struct gpio_desc*gpio_convst;
struct gpio_desc*gpio_reset;
struct gpio_desc*gpio_range;
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
On Mon, Mar 20, 2017 at 02:57:03PM +0100, Hans Verkuil wrote: > On 03/20/2017 02:29 PM, Russell King - ARM Linux wrote: > > It's what I have - remember, not everyone is happy to constantly replace > > their distro packages with random new stuff. > > This is a compliance test, which is continuously developed in tandem with > new kernel versions. If you are working with an upstream kernel, then you > should also use the corresponding v4l2-compliance test. What's the point > of using an old one? > > I will not support driver developers that use an old version of the > compliance test, that's a waste of my time. The reason that people may _not_ wish to constantly update v4l-utils is that it changes the libraries installed on their systems. So, the solution to that is not to complain about developers not using the latest version, but instead to de-couple it from the rest of the package, and provide it as a separate, stand-alone package that doesn't come with all the extra baggage. Now, there's two possible answers to that: 1. it depends on the libv4l2 version. If that's so, then you are insisting that people constantly move to the latest libv4l2 because of API changes, and those API changes may upset applications they're using. So this isn't really on. 2. it doesn't depend on libv4l2 version, in which case there's no reason for it to be packaged with v4l-utils. The reality is that v4l2-compliance links with libv4l2, so I'm not sure which it is. What I am sure of is that I don't want to upgrade libv4l2 on an ad-hoc basis, potentially causing issues with applications. > >> To test actual streaming you need to provide the -s option. > >> > >> Note: v4l2-compliance has been developed for 'regular' video devices, > >> not MC devices. It may or may not work with the -s option. > > > > Right, and it exists to verify that the establised v4l2 API is correctly > > implemented. If the v4l2 API is being offered to user applications, > > then it must be conformant, otherwise it's not offering the v4l2 API. > > (That's very much a definition statement in itself.) > > > > So, are we really going to say MC devices do not offer the v4l2 API to > > userspace, but something that might work? We've already seen today > > one user say that they're not going to use mainline because of the > > crud surrounding MC. > > > > Actually, my understanding was that he was stuck on the old kernel code. Err, sorry, I really don't follow. Who is "he"? _I_ was the one who reported the EXPBUF problem. Your comment makes no sense. > In the case of v4l2-compliance, I never had the time to make it work with > MC devices. Same for that matter of certain memory to memory devices. > > Just like MC devices these too behave differently. They are partially > supported in v4l2-compliance, but not fully. It seems you saying that the API provided by /dev/video* for a MC device breaks the v4l2-compliance tests? _No one_ has mentioned using v4l2-compliance on the subdevs. > Complaining about this really won't help. We know it's a problem and unless > someone (me perhaps?) manages to get paid to work on this it's unlikely to > change for now. Like the above comment, your comment makes no sense. I'm not complaining, I'm trying to find out the details. Yes, MC stuff sucks big time right now, the documentation is poor, there's a lack of understanding on all sides of the issues (which can be seen by the different opinions that people hold.) The only way to resolve these differences is via discussion, and if you're going to start thinking that everyone is complaining, then there's not going to be any forward progress. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt
On Mon, Mar 20, 2017 at 03:00:51PM +0100, Philipp Zabel wrote: > On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote: > > The same document says: > > > > Scaling support is optional. When supported by a subdev, the crop > > rectangle on the subdev's sink pad is scaled to the size configured > > using the > > :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL > > using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the > > subdev supports scaling but not composing, the top and left values are > > not used and must always be set to zero. > > Right, this sentence does imply that when scaling is supported, there > must be a sink compose rectangle, even when composing is not. > > I have previously set up scaling like this: > > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]" > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]" > > Does this mean, it should work like this instead? > > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]" > media-ctl --set-v4l2 > "'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]" > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]" > > I suppose setting the source pad format should not be allowed to modify > the sink compose rectangle. That is what I believe having read these documents several times, but we need v4l2 people to confirm. Note that setting the format on 'ipu1_csi0':0 should already be done by the previous media-ctl command, so it should be possible to simplify that to: media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]" media-ctl --set-v4l2 "'ipu1_csi0':0[compose:(0,0)/960x540]" media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]" I have tripped over a bug in media-ctl when specifying both a crop and compose rectangle - the --help output suggests that "," should be used to separate them. media-ctl rejects that, telling me the character at the "," should be "]". Replacing the "," with " " allows media-ctl to accept it and set both rectangles, so it sounds like a parser bug - I've not looked into this any further yet. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vc04_services: fix NULL pointer dereference on pointer 'service'
[add Mauro to CC]
Am 20.03.2017 um 15:08 schrieb Colin King:
From: Colin Ian King
Currently, if pservice is null then service is set to NULL and immediately
afterwards service is dereferenced causing a null pointer dereference. Fix
this by bailing out early of the function with a null return.
Detected by CoverityScan, CID#1419681 ("Explicit null dereferenced")
Signed-off-by: Colin Ian King
Acked-by: Stefan Wahren
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE:
Are you in need of a loan for business / personal loan? Apply now by email, note, this offer is for serious minded people Only: web.loanf...@gmail.com ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/5] staging: vc04_services: camera driver maintance
On Mon, 2017-03-20 at 16:57 +0300, Dan Carpenter wrote: > On Mon, Mar 20, 2017 at 04:06:00AM -0700, Michael Zoran wrote: > > On Mon, 2017-03-20 at 13:55 +0300, Dan Carpenter wrote: > > > I'm not going to review this because it has kbuild errors. > > > > > > regards, > > > dan carpenter > > > > > > > Hi, can you e-mail out the errors or send them to me. It worked > > when > > I submitted it. > > > > The problem is that you added a function only for ARM but the camera > can build on i386. > > Anyway, we need to figure out why you aren't getting the kbuild > emails > and fix that. I forwarded the first email to you. The other is > basically the same. > > regards, > dan carpenter > OK, thanks for sending it to me. I don't quite understand how that is possible in all since the whole subtree depends on RASPBERRYPI_FIRMWARE which I had nothing to do with setting up. Sounds to me like the real issue here is that the RASPBERRYPI_FIRMARE driver wasn't setup correctly in the first place. I also noticed when I was trying to understand how the raspberrypi driver works in the first place in that it doesn't always dump the firmware revision correctly if the DT node mbox handle gets pointed to who knows where. I've been complaining about very selective e-mails on so called public e-mail lists for a very long time now, and nobody has done anything about it. Sometimes I don't get any e-mail for a week and then I'll get a flood of e-mail thats 2 weeks old. And who knows which e-mails lists changes are going to. Some changes are getting in under the radar by others and other changes have every line criticized. I have been honestly testing all the changes I have submitted, and most of them have been small. But they have caused things to work when they don't work at all before. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver
Em Mon, 20 Mar 2017 04:08:21 -0700 Michael Zoran escreveu: > On Mon, 2017-03-20 at 07:58 -0300, Mauro Carvalho Chehab wrote: > > Em Sun, 19 Mar 2017 22:11:07 -0300 > > Mauro Carvalho Chehab escreveu: > > > > > Em Sun, 19 Mar 2017 10:04:28 -0700 > > > Michael Zoran escreveu: > > > > > > > A working DT that I tried this morning with the current firmware > > > > is > > > > posted here: > > > > http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-March/ > > > > 005924 > > > > .html > > > > > > > > It even works with minecraft_pi! > > > > With the new firmware, sometime after booting, I'm getting an oops, > > caused > > by bcm2835_audio/vchiq: > > > > [ 298.788995] Unable to handle kernel NULL pointer dereference at > > virtual address 0034 > > [ 298.821458] pgd = ed004000 > > [ 298.832294] [0034] *pgd=2e5e9835, *pte=, > > *ppte= > > [ 298.857450] Internal error: Oops: 17 [#1] SMP ARM > > [ 298.876273] Modules linked in: cfg80211 hid_logitech_hidpp > > hid_logitech_dj snd_bcm2835(C) snd_pcm snd_timer snd soundcore > > vchiq(C) uio_pdrv_genirq uio fuse > > [ 298.932064] CPU: 3 PID: 847 Comm: pulseaudio Tainted: > > G C 4.11.0-rc1+ #56 > > [ 298.963774] Hardware name: Generic DT based system > > [ 298.982945] task: ef758580 task.stack: ee4c6000 > > [ 299.001080] PC is at mutex_lock+0x14/0x3c > > [ 299.017148] LR is at vchiq_add_service_internal+0x138/0x3a0 > > [vchiq] > > [ 299.042246] pc : []lr : []psr: > > 4013 > > sp : ee4c7ca8 ip : fp : ef709800 > > [ 299.088240] r10: r9 : ee3bffc0 r8 : 0034 > > [ 299.109153] r7 : 0003 r6 : r5 : ee4c7d00 r4 : > > ee1d8c00 > > [ 299.135291] r3 : ef758580 r2 : r1 : ffc8 r0 : > > 0034 > > [ 299.161431] Flags: nZcv IRQs on FIQs on Mode SVC_32 ISA > > ARM Segment none > > [ 299.190008] Control: 10c5383d Table: 2d00406a DAC: 0051 > > [ 299.213011] Process pulseaudio (pid: 847, stack limit = > > 0xee4c6220) > > [ 299.238104] Stack: (0xee4c7ca8 to 0xee4c8000) > > [ 299.255539] 7ca0: c1403d54 80400040 ff7f0600 > > ff7f0660 bf06b578 ee3bffc0 > > [ 299.288301] 7cc0: ee3afd00 ee4c7d00 > > bf0640b4 bf066428 > > [ 299.321064] 7ce0: ee3afd00 ee3afd00 ee4c7d34 ee3af444 ee3bffc0 > > ee3af444 ee3bffc0 bf0662ec > > [ 299.353826] 7d00: 41554453 bf065db0 ee3afd00 00010002 bf0d7408 > > ee3af440 bf0d7408 > > [ 299.386587] 7d20: ee79bd80 bf0d5a04 ef709800 0020 > > 0002 0001 41554453 > > [ 299.419349] 7d40: bf0d559c ee3af440 > > 0001 0001 > > [ 299.452111] 7d60: ee24ac80 ee24ac80 ee1c4a00 ee79bd80 > > ee24ace8 0001 bf0d4dfc > > [ 299.484872] 7d80: 000b ee4b8c3c ee4c7dc8 > > ee4b8800 ee4b8c28 ee4c6000 > > [ 299.517635] 7da0: ee4b8c3c ed029e40 bf0c0404 ee4b8800 > > ee1c4a00 ee4b8800 ed029e40 > > [ 299.550398] 7dc0: bf0c0560 ee072340 ef758580 > > c0367b7c ee4b8c40 ee4b8c40 > > [ 299.583161] 7de0: ee4b8800 ed029e40 ee318f80 ed029e40 > > 0006 ee318f80 bf0c0748 > > [ 299.615924] 7e00: bf0a3430 ee4f6180 c0428fe0 ee318f80 > > 21b0 0026 ed029e40 > > [ 299.648697] 7e20: ee318f80 ed029e48 c0428f1c ee4c7e94 0006 > > c0421cc0 ee4c7ed0 > > [ 299.681464] 7e40: 0802 ee4c7e94 0006 ee318f80 > > c0432c8c ee4c7f40 c0433bc0 > > [ 299.714225] 7e60: ed029e40 0041 > > ed004000 ee4c6000 > > [ 299.746987] 7e80: eec69808 0005 0002 ee318f80 > > ef0d2910 ee924908 bf0ba284 > > [ 299.779750] 7ea0: ee31bbc0 bebb53c4 ee4e1d00 0011 ee4c7f74 > > 0001 f000 c0308b04 > > [ 299.812512] 7ec0: ee4c6000 bebb5710 c0434578 ef0d2910 > > ee924908 73541c18 0008 > > [ 299.845274] 7ee0: ee4a7019 ee899bb0 ee318f80 > > 0101 0002 0084 > > [ 299.878037] 7f00: ee4c7f10 ee318df8 > > ed029840 40045532 bebb53c4 > > [ 299.910799] 7f20: ee4c6000 ee4a7000 c1403ef8 bebb550c 0011 > > ee5eca00 0020 ee5eca18 > > [ 299.943562] 7f40: ee4a7000 00080802 0002 ff9c > > f000 0011 ff9c > > [ 299.976324] 7f60: ee4a7000 c0422e70 0002 c04359b0 ed029840 > > 0802 ed02 0006 > > [ 300.009086] 7f80: 0100 0001 0004 > > b189d000 0005 c0308b04 > > [ 300.041848] 7fa0: ee4c6000 c0308940 0004 bebb550c > > 00080802 bebb53c4 00084b58 > > [ 300.074611] 7fc0: 0004 b189d000 0005 > > bebb550c 00099448 bebb5710 > > [ 300.107373] 7fe0: bebb53c8 b6c40da4 b6c24334 8010 > > bebb550c 2fffd861 2fffdc61 > > [ 300.140190] [] (mutex_lock) from [] > > (vchiq_add_service_internal+0x138/0x3a0 [vchiq]) > > [ 300.178237] [] (vchiq_add_service_internal [vchiq]) from > > [] (vchiq_open_service+0x58/0x
Re: [PATCH v5 02/39] [media] dt-bindings: Add bindings for i.MX media driver
+Ramiro
On Thu, Mar 09, 2017 at 08:52:42PM -0800, Steve Longerbeam wrote:
> Add bindings documentation for the i.MX media driver.
>
> Signed-off-by: Steve Longerbeam
> ---
> Documentation/devicetree/bindings/media/imx.txt | 74
> +
> 1 file changed, 74 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/imx.txt
>
> diff --git a/Documentation/devicetree/bindings/media/imx.txt
> b/Documentation/devicetree/bindings/media/imx.txt
> new file mode 100644
> index 000..3059c06
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/imx.txt
> @@ -0,0 +1,74 @@
> +Freescale i.MX Media Video Device
> +=
> +
> +Video Media Controller node
> +---
> +
> +This is the media controller node for video capture support. It is a
> +virtual device that lists the camera serial interface nodes that the
> +media device will control.
> +
> +Required properties:
> +- compatible : "fsl,imx-capture-subsystem";
> +- ports : Should contain a list of phandles pointing to camera
> + sensor interface ports of IPU devices
> +
> +example:
> +
> +capture-subsystem {
> + compatible = "fsl,imx-capture-subsystem";
> + ports = <&ipu1_csi0>, <&ipu1_csi1>;
> +};
> +
> +fim child node
> +--
> +
> +This is an optional child node of the ipu_csi port nodes. If present and
> +available, it enables the Frame Interval Monitor. Its properties can be
> +used to modify the method in which the FIM measures frame intervals.
> +Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
> +Frame Interval Monitor.
> +
> +Optional properties:
> +- fsl,input-capture-channel: an input capture channel and channel flags,
> + specified as . The channel number
> + must be 0 or 1. The flags can be
> + IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, or
> + IRQ_TYPE_EDGE_BOTH, and specify which input
> + capture signal edge will trigger the input
> + capture event. If an input capture channel is
> + specified, the FIM will use this method to
> + measure frame intervals instead of via the EOF
> + interrupt. The input capture method is much
> + preferred over EOF as it is not subject to
> + interrupt latency errors. However it requires
> + routing the VSYNC or FIELD output signals of
> + the camera sensor to one of the i.MX input
> + capture pads (SD1_DAT0, SD1_DAT1), which also
> + gives up support for SD1.
> +
> +
> +mipi_csi2 node
> +--
> +
> +This is the device node for the MIPI CSI-2 Receiver, required for MIPI
> +CSI-2 sensors.
> +
> +Required properties:
> +- compatible : "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";
Ramiro is also working on a binding for DW MIPI CSI2 block[1]. We need 1
binding for that.
> +- reg : physical base address and length of the register set;
> +- clocks : the MIPI CSI-2 receiver requires three clocks: hsi_tx
> + (the D-PHY clock), video_27m (D-PHY PLL reference
> + clock), and eim_podf;
> +- clock-names: must contain "dphy", "ref", "pix";
> +- port@*: five port nodes must exist, containing endpoints
> + connecting to the source and sink devices according to
> + of_graph bindings. The first port is an input port,
> + connecting with a MIPI CSI-2 source, and ports 1
> + through 4 are output ports connecting with parallel
> + bus sink endpoint nodes and correspond to the four
> + MIPI CSI-2 virtual channel outputs.
> +
> +Optional properties:
> +- interrupts : must contain two level-triggered interrupts,
> + in order: 100 and 101;
> --
> 2.7.4
>
[1] https://lkml.org/lkml/2017/3/7/395
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/9] stating/atomisp: fix -Wold-style-definition warning
On Mon, 20 Mar 2017 10:32:19 +0100
Arnd Bergmann wrote:
> -void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/)
> +void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/ void)
> {
Why keep the comment?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline
Em Mon, 20 Mar 2017 14:10:30 +0100 Hans Verkuil escreveu: > On 03/17/2017 03:37 PM, Russell King - ARM Linux wrote: > > On Fri, Mar 17, 2017 at 02:51:10PM +0100, Philipp Zabel wrote: > >> On Fri, 2017-03-17 at 10:24 -0300, Mauro Carvalho Chehab wrote: > >> [...] > >>> The big question, waiting for an answer on the last 8 years is > >>> who would do that? Such person would need to have several different > >>> hardware from different vendors, in order to ensure that it has > >>> a generic solution. > >>> > >>> It is a way more feasible that the Kernel developers that already > >>> have a certain hardware on their hands to add support inside the > >>> driver to forward the controls through the pipeline and to setup > >>> a "default" pipeline that would cover the common use cases at > >>> driver's probe. > >> > >> Actually, would setting pipeline via libv4l2 plugin and letting drivers > >> provide a sane enabled default pipeline configuration be mutually > >> exclusive? Not sure about the control forwarding, but at least a simple > >> link setup and format forwarding would also be possible in the kernel > >> without hindering userspace from doing it themselves later. > > > > I think this is the exact same problem as controls in ALSA. > > > > When ALSA started off in life, the requirement was that all controls > > shall default to minimum, and the user is expected to adjust controls > > after the system is running. > > > > After OSS, this gave quite a marked change in system behaviour, and > > led to a lot of "why doesn't my sound work anymore" problems, because > > people then had to figure out which combination of controls had to be > > set to get sound out of their systems. > > > > Now it seems to be much better, where install Linux on a platform, and > > you have a working sound system (assuming that the drivers are all there > > which is generally the case for x86.) > > > > However, it's still possible to adjust all the controls from userspace. > > All that's changed is the defaults. > > > > Why am I mentioning this - because from what I understand Mauro saying, > > it's no different from this situation. Userspace will still have the > > power to disable all links and setup its own. The difference is that > > there will be a default configuration that the kernel sets up at boot > > time that will be functional, rather than the current default > > configuration where the system is completely non-functional until > > manually configured. > > > > However, at the end of the day, I don't care _where_ the usability > > problems are solved, only that there is some kind of solution. It's not > > the _where_ that's the real issue here, but the _how_, and discussion of > > the _how_ is completely missing. > > > > So, let's try kicking off a discussion about _how_ to do things. > > > > _How_ do we setup a media controller system so that we end up with a > > usable configuration - let's start with the obvious bit... which links > > should be enabled. > > > > I think the first pre-requisit is that we stop exposing capture devices > > that can never be functional for the hardware that's present on the board, > > so that there isn't this plentora of useless /dev/video* nodes and useless > > subdevices. > > > > One possible solution to finding a default path may be "find the shortest > > path between the capture device and the sensor and enable intervening > > links". > > > > Then we need to try configuring that path with format/resolution > > information. > > > > However, what if something in the shortest path can't handle the format > > that the sensor produces? I think at that point, we'd need to drop that > > subdev out of the path resolution, re-run the "find the shortest path" > > algorithm, and try again. > > > > Repeat until success or no path between the capture and sensor exists. > > > > This works fine if you have just one sensor visible from a capture device, > > but not if there's more than one (which I suspect is the case with the > > Sabrelite board with its two cameras and video receiver.) That breaks > > the "find the shortest path" algorithm. > > > > So, maybe it's a lot better to just let the board people provide via DT > > a default setup for the connectivity of the modules somehow - certainly > > one big step forward would be to disable in DT parts of the capture > > system that can never be used (remembering that boards like the RPi / > > Hummingboard may end up using DT overlays to describe this for different > > cameras, so the capture setup may change after initial boot.) > > The MC was developed before the device tree came along. But now that the DT > is here, I think this could be a sensible idea to let the DT provide an > initial path. > > Sakari, Laurent, Mauro: any opinions? It makes perfect sense to me. By setting the pipeline via DT on boards with simple configurations, e. g. just one CSI physical interface, it can create just one devnode (e. g. /dev/vi
Re: [PATCH v5 03/39] [media] dt/bindings: Add bindings for OV5640
On Thu, Mar 09, 2017 at 08:52:43PM -0800, Steve Longerbeam wrote: > Add device tree binding documentation for the OV5640 camera sensor. > > Signed-off-by: Steve Longerbeam > --- > .../devicetree/bindings/media/i2c/ov5640.txt | 45 > ++ > 1 file changed, 45 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5640.txt Acked-by: Rob Herring ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver
On Mon, 2017-03-20 at 11:58 -0300, Mauro Carvalho Chehab wrote: > Em Mon, 20 Mar 2017 04:08:21 -0700 > Michael Zoran escreveu: > > > On Mon, 2017-03-20 at 07:58 -0300, Mauro Carvalho Chehab wrote: > > > Em Sun, 19 Mar 2017 22:11:07 -0300 > > > Mauro Carvalho Chehab escreveu: > > > > > > > Em Sun, 19 Mar 2017 10:04:28 -0700 > > > > Michael Zoran escreveu: > > > > > > > > > A working DT that I tried this morning with the current > > > > > firmware > > > > > is > > > > > posted here: > > > > > http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-Ma > > > > > rch/ > > > > > 005924 > > > > > .html > > > > > > > > > > It even works with minecraft_pi! > > > > > > > > > > Hi, can you e-mail out your config.txt? Do you have audio enabled > > in > > config.txt? > > yes, I have this: > > $ cat config.txt |grep -i audio > # uncomment to force a HDMI mode rather than DVI. This can make audio > work in > # Enable audio (loads snd_bcm2835) > dtparam=audio=on > > Full config attached. > > Thanks, > Mauro > Are you using Eric Anholt's HDMI Audio driver that's included in VC4? That could well be incompatible with the firmware driver. Or are you using a half mode of VC4 for audio and VCHIQ for video? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: ade7754: Move header content to implementation file
The contents of ade7754.h are only used in ade7754.c.
Move the header contents to the implementation file,
and delete the header file.
Signed-off-by: simran singhal
---
drivers/staging/iio/meter/ade7754.c | 87 ++-
drivers/staging/iio/meter/ade7754.h | 90 -
2 files changed, 86 insertions(+), 91 deletions(-)
delete mode 100644 drivers/staging/iio/meter/ade7754.h
diff --git a/drivers/staging/iio/meter/ade7754.c
b/drivers/staging/iio/meter/ade7754.c
index 024463a..42f7b06 100644
--- a/drivers/staging/iio/meter/ade7754.c
+++ b/drivers/staging/iio/meter/ade7754.c
@@ -21,7 +21,92 @@
#include
#include
#include "meter.h"
-#include "ade7754.h"
+
+#define ADE7754_AENERGY 0x01
+#define ADE7754_RAENERGY 0x02
+#define ADE7754_LAENERGY 0x03
+#define ADE7754_VAENERGY 0x04
+#define ADE7754_RVAENERGY 0x05
+#define ADE7754_LVAENERGY 0x06
+#define ADE7754_PERIOD0x07
+#define ADE7754_TEMP 0x08
+#define ADE7754_WFORM 0x09
+#define ADE7754_OPMODE0x0A
+#define ADE7754_MMODE 0x0B
+#define ADE7754_WAVMODE 0x0C
+#define ADE7754_WATMODE 0x0D
+#define ADE7754_VAMODE0x0E
+#define ADE7754_IRQEN 0x0F
+#define ADE7754_STATUS0x10
+#define ADE7754_RSTATUS 0x11
+#define ADE7754_ZXTOUT0x12
+#define ADE7754_LINCYC0x13
+#define ADE7754_SAGCYC0x14
+#define ADE7754_SAGLVL0x15
+#define ADE7754_VPEAK 0x16
+#define ADE7754_IPEAK 0x17
+#define ADE7754_GAIN 0x18
+#define ADE7754_AWG 0x19
+#define ADE7754_BWG 0x1A
+#define ADE7754_CWG 0x1B
+#define ADE7754_AVAG 0x1C
+#define ADE7754_BVAG 0x1D
+#define ADE7754_CVAG 0x1E
+#define ADE7754_APHCAL0x1F
+#define ADE7754_BPHCAL0x20
+#define ADE7754_CPHCAL0x21
+#define ADE7754_AAPOS 0x22
+#define ADE7754_BAPOS 0x23
+#define ADE7754_CAPOS 0x24
+#define ADE7754_CFNUM 0x25
+#define ADE7754_CFDEN 0x26
+#define ADE7754_WDIV 0x27
+#define ADE7754_VADIV 0x28
+#define ADE7754_AIRMS 0x29
+#define ADE7754_BIRMS 0x2A
+#define ADE7754_CIRMS 0x2B
+#define ADE7754_AVRMS 0x2C
+#define ADE7754_BVRMS 0x2D
+#define ADE7754_CVRMS 0x2E
+#define ADE7754_AIRMSOS 0x2F
+#define ADE7754_BIRMSOS 0x30
+#define ADE7754_CIRMSOS 0x31
+#define ADE7754_AVRMSOS 0x32
+#define ADE7754_BVRMSOS 0x33
+#define ADE7754_CVRMSOS 0x34
+#define ADE7754_AAPGAIN 0x35
+#define ADE7754_BAPGAIN 0x36
+#define ADE7754_CAPGAIN 0x37
+#define ADE7754_AVGAIN0x38
+#define ADE7754_BVGAIN0x39
+#define ADE7754_CVGAIN0x3A
+#define ADE7754_CHKSUM0x3E
+#define ADE7754_VERSION 0x3F
+
+#define ADE7754_READ_REG(a)a
+#define ADE7754_WRITE_REG(a) ((a) | 0x80)
+
+#define ADE7754_MAX_TX4
+#define ADE7754_MAX_RX4
+#define ADE7754_STARTUP_DELAY 1000
+
+#define ADE7754_SPI_SLOW (u32)(300 * 1000)
+#define ADE7754_SPI_BURST (u32)(1000 * 1000)
+#define ADE7754_SPI_FAST (u32)(2000 * 1000)
+
+/**
+ * struct ade7754_state - device instance specific data
+ * @us:actual spi_device
+ * @buf_lock: mutex to protect tx and rx
+ * @tx:transmit buffer
+ * @rx:receive buffer
+ **/
+struct ade7754_state {
+ struct spi_device *us;
+ struct mutexbuf_lock;
+ u8 tx[ADE7754_MAX_TX] cacheline_aligned;
+ u8 rx[ADE7754_MAX_RX];
+};
static int ade7754_spi_write_reg_8(struct device *dev, u8 reg_address, u8 val)
{
diff --git a/drivers/staging/iio/meter/ade7754.h
b/drivers/staging/iio/meter/ade7754.h
deleted file mode 100644
index 28f71c2..000
--- a/drivers/staging/iio/meter/ade7754.h
+++ /dev/null
@@ -1,90 +0,0 @@
-#ifndef _ADE7754_H
-#define _ADE7754_H
-
-#define ADE7754_AENERGY 0x01
-#define ADE7754_RAENERGY 0x02
-#define ADE7754_LAENERGY 0x03
-#define ADE7754_VAENERGY 0x04
-#define ADE7754_RVAENERGY 0x05
-#define ADE7754_LVAENERGY 0x06
-#define ADE7754_PERIOD0x07
-#define ADE7754_TEMP 0x08
-#define ADE7754_WFORM 0x09
-#define ADE7754_OPMODE0x0A
-#define ADE7754_MMODE 0x0B
-#define ADE7754_WAVMODE 0x0C
-#define ADE7754_WATMODE 0x0D
-#define ADE7754_VAMODE0x0E
-#define ADE7754_IRQEN 0x0F
-#define ADE7754_STATUS0x10
-#define ADE7754_RSTATUS 0x11
-#define ADE7754_ZXTOUT0x12
-#define ADE7754_LINCYC0x13
-#define ADE7754_SAGCYC0x14
-#define ADE7754_SAGLVL0x15
-#define ADE7754_VPEAK 0x16
-#define ADE7754_IPEAK 0x17
-#define ADE7754_GAIN 0x18
-#define ADE7754_AWG 0x19
-#define ADE7754_BWG 0x1A
-#define ADE7754_CWG 0x1B
-#define ADE7754_AVAG 0x1C
-#define ADE7754_BVAG 0x1D
-#define ADE7754_CVAG 0x1E
-#define ADE7754_APHCAL0x1F
-#define ADE7754_BPHCAL0x20
-#define ADE7754_CPHCAL0x21
-#define ADE7754_AAPOS 0x22
-#define ADE7754_BAPOS 0x23
-#define ADE7754_CAPOS 0x24
-#define ADE7754_CFNUM 0x25
-#defi
[PATCH 0/2] staging: iio: ade7754: Header file maintenance
Collapse header file into source, and clean up includes in implementation. simran singhal (2): staging: ade7754: Move header content to implementation file staging: ade7754: Clean up #includes drivers/staging/iio/meter/ade7754.c | 98 ++--- drivers/staging/iio/meter/ade7754.h | 90 -- 2 files changed, 91 insertions(+), 97 deletions(-) delete mode 100644 drivers/staging/iio/meter/ade7754.h -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: ade7754: Clean up #includes
Alphabetize and separate kernel and subsystem headers. Signed-off-by: simran singhal --- drivers/staging/iio/meter/ade7754.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/staging/iio/meter/ade7754.c b/drivers/staging/iio/meter/ade7754.c index 42f7b06..fea93d7 100644 --- a/drivers/staging/iio/meter/ade7754.c +++ b/drivers/staging/iio/meter/ade7754.c @@ -6,18 +6,17 @@ * Licensed under the GPL-2 or later. */ -#include -#include #include -#include #include +#include +#include #include +#include +#include +#include #include #include #include -#include -#include - #include #include #include "meter.h" -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RESEND PATCH] staging: ad7606: Replace mlock with driver private lock
The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
In this driver, mlock was being used to protect hardware state
changes. Replace it with a lock in the devices global data.
Signed-off-by: Arushi Singhal
---
drivers/staging/iio/adc/ad7606.c | 8
drivers/staging/iio/adc/ad7606.h | 2 ++
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 9dbfa64b1e53..9f529b34e018 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -208,7 +208,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_SCALE:
ret = -EINVAL;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
for (i = 0; i < ARRAY_SIZE(scale_avail); i++)
if (val2 == scale_avail[i][1]) {
gpiod_set_value(st->gpio_range, i);
@@ -217,7 +217,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
ret = 0;
break;
}
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return ret;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -231,11 +231,11 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
values[1] = (ret >> 1) & 1;
values[2] = (ret >> 2) & 1;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
values);
st->oversampling = val;
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return 0;
default:
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 746f9553d2ba..5d59bdd78727 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -14,6 +14,7 @@
* @name: identification string for chip
* @channels: channel specification
* @num_channels: number of channels
+ * @lock protect sensor state
*/
struct ad7606_chip_info {
@@ -37,6 +38,7 @@ struct ad7606_state {
booldone;
void __iomem*base_address;
+ struct mutexlock; /* protect sensor state */
struct gpio_desc*gpio_convst;
struct gpio_desc*gpio_reset;
struct gpio_desc*gpio_range;
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver
Em Mon, 20 Mar 2017 08:11:41 -0700
Michael Zoran escreveu:
> On Mon, 2017-03-20 at 11:58 -0300, Mauro Carvalho Chehab wrote:
> > Em Mon, 20 Mar 2017 04:08:21 -0700
> > Michael Zoran escreveu:
> >
> > > On Mon, 2017-03-20 at 07:58 -0300, Mauro Carvalho Chehab wrote:
> > > > Em Sun, 19 Mar 2017 22:11:07 -0300
> > > > Mauro Carvalho Chehab escreveu:
> > > >
> > > > > Em Sun, 19 Mar 2017 10:04:28 -0700
> > > > > Michael Zoran escreveu:
> > > > >
> > > > > > A working DT that I tried this morning with the current
> > > > > > firmware
> > > > > > is
> > > > > > posted here:
> > > > > > http://lists.infradead.org/pipermail/linux-rpi-kernel/2017-Ma
> > > > > > rch/
> > > > > > 005924
> > > > > > .html
> > > > > >
> > > > > > It even works with minecraft_pi!
> > > >
> > > >
> > >
> > > Hi, can you e-mail out your config.txt? Do you have audio enabled
> > > in
> > > config.txt?
> >
> > yes, I have this:
> >
> > $ cat config.txt |grep -i audio
> > # uncomment to force a HDMI mode rather than DVI. This can make audio
> > work in
> > # Enable audio (loads snd_bcm2835)
> > dtparam=audio=on
> >
> > Full config attached.
> >
> > Thanks,
> > Mauro
> >
>
> Are you using Eric Anholt's HDMI Audio driver that's included in VC4?
> That could well be incompatible with the firmware driver. Or are you
> using a half mode of VC4 for audio and VCHIQ for video?
I'm using vanilla staging Kernel, from Greg's tree:
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?h=staging-next&id=7bc49cb9b9b8bad32536c4b6d1aff1824c1adc6c
Plus the DWC2 fixup I wrote and DT changes you pointed
(see enclosed).
I can disable the audio overlay here, as I don't have anything
connected to audio inputs/outputs.
Regards,
Mauro
---
diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi
b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index 38e6050035bc..1f42190e8558 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -27,6 +27,14 @@
firmware = <&firmware>;
#power-domain-cells = <1>;
};
+
+ vchiq {
+ compatible = "brcm,bcm2835-vchiq";
+ reg = <0x7e00b840 0xf>;
+ interrupts = <0 2>;
+ cache-line-size = <32>;
+ firmware = <&firmware>;
+ };
};
};
diff --git a/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
b/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
index c309633a1e87..7e8d42904022 100644
--- a/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
+++ b/arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
@@ -17,6 +17,45 @@
gpios = <&gpio 47 0>;
};
};
+
+
+ soc {
+
+// hvs at 7e40 {
+// status = "disabled";
+// };
+
+// v3d: v3d at 7ec0 {
+// status = "disabled";
+// };
+
+ vc4: gpu {
+ status = "disabled";
+ };
+
+ fb: fb {
+ status = "disabled";
+ };
+
+ vchiq: vchiq {
+ compatible = "brcm,bcm2835-vchiq";
+ reg = <0x7e00b840 0xf>;
+ interrupts = <0 2>;
+ cache-line-size = <32>;
+ firmware = <&firmware>;
+ };
+
+ audio: audio {
+ compatible = "brcm,bcm2835-audio";
+ brcm,pwm-channels = <8>;
+ };
+
+ };
+
+ __overrides__ {
+ cache_line_size = <&vchiq>, "cache-line-size:0";
+ };
+
};
Thanks,
Mauro
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline
Em Mon, 20 Mar 2017 14:24:25 +0100 Hans Verkuil escreveu: > On 03/14/2017 11:21 AM, Mauro Carvalho Chehab wrote: > > Em Tue, 14 Mar 2017 08:55:36 +0100 > > Hans Verkuil escreveu: > > > >> On 03/14/2017 04:45 AM, Mauro Carvalho Chehab wrote: > >>> Hi Sakari, > >>> > >> We're all very driver-development-driven, and userspace gets very little > >> attention in general. So before just throwing in the towel we should take > >> a good look at the reasons why there has been little or no development: is > >> it because of fundamental design defects, or because nobody paid attention > >> to it? > > > > No. We should look it the other way: basically, there are patches > > for i.MX6 driver that sends control from videonode to subdevs. > > > > If we nack apply it, who will write the userspace plugin? When > > such change will be merged upstream? > > > > If we don't have answers to any of the above questions, we should not > > nack it. > > > > That's said, that doesn't prevent merging a libv4l plugin if/when > > someone can find time/interest to develop it. > > I don't think this control inheritance patch will magically prevent you > from needed a plugin. Yeah, it is not just control inheritance. The driver needs to work without subdev API, e. g. mbus settings should also be done via the video devnode. Btw, Sakari made a good point on IRC: what happens if some app try to change the pipeline or subdev settings while another application is using the device? The driver should block such changes, maybe using the V4L2 priority mechanism. > This is literally the first time we have to cater to a cheap devkit. We > were always aware of this issue, but nobody really needed it. That's true. Now that we have a real need for that, we need to provide a solution. > > I'd say that we should not care anymore on providing a solution for > > generic applications to run on boards like OMAP3[1]. For hardware that > > are currently available that have Kernel driver and boards developed > > to be used as "cheap hobbyist devkit", I'd say we should implement > > a Kernel solution that would allow them to be used without subdev > > API, e. g. having all ioctls needed by generic applications to work > > functional, after some external application sets the pipeline. > > I liked Russell's idea of having the DT set up an initial video path. > This would (probably) make it much easier to provide a generic plugin since > there is already an initial valid path when the driver is loaded, and it > doesn't require custom code in the driver since this is left to the DT > which really knows about the HW. Setting the device via DT indeed makes easier (either for a kernel or userspace solution), but things like resolution changes should be possible without needing to change DT. Also, as MC and subdev changes should be blocked while a V4L2 app is using the device, using a mechanism like calling VIDIOC_S_PRIORITY ioctl via the V4l2 interface, Kernel will require changes, anyway. My suggestion is to touch on one driver to make it work with a generic application. As we currently have efforts and needs for the i.MX6 to do it, I'd say that the best would be to make it work on such driver. Once the work is done, we can see if the approach taken there would work at libv4l or not. In parallel, someone has to fix libv4l for it to be default on applications like gstreamer, e. g. adding support for DMABUF and fixing other issues that are preventing it to be used by default. Nicolas, Why libv4l is currently disabled at gstreamer's default settings? > > [1] Yet, I might eventually do that for fun, an OMAP3 board with tvp5150 > > just arrived here last week. It would be nice to have xawtv3 running on it > > :-) > > So, if I have a lot of spare time (with is very unlikely), I might > > eventually > > do something for it to work. > > > >> I know it took me a very long time before I had a working omap3. > > > > My first OMAP3 board with working V4L2 source just arrived last week :-) > > > >> So I am not at all surprised that little progress has been made. > > > > I'm not surprised, but I'm disappointed, as I tried to push toward a > > solution for this problem since when we had our initial meetings about > > it. > > So many things to do, so little time. Sounds corny, but really, that's what > this is all about. There were always (and frankly, still are) more important > things that needed to be done. What's most important for some developer may not be so important for another developer. My understanding here is that there are developers wanting/needing to have standard V4L2 apps support for (some) i.MX6 devices. Those are the ones that may/will allocate some time for it to happen. Thanks, Mauro ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver
On Mon, 2017-03-20 at 12:33 -0300, Mauro Carvalho Chehab wrote: > Em Mon, 20 Mar 2017 08:11:41 -0700 > Michael Zoran escreveu: > > > On Mon, 2017-03-20 at 11:58 -0300, Mauro Carvalho Chehab wrote: > > > Em Mon, 20 Mar 2017 04:08:21 -0700 > > > Michael Zoran escreveu: > > > > > > > On Mon, 2017-03-20 at 07:58 -0300, Mauro Carvalho Chehab > > > > wrote: > > > > > Em Sun, 19 Mar 2017 22:11:07 -0300 > > > > > Mauro Carvalho Chehab escreveu: > > > > > > > > > > > Em Sun, 19 Mar 2017 10:04:28 -0700 > > > > > > Michael Zoran escreveu: > > > > > > > > > > > > > A working DT that I tried this morning with the current > > > > > > > firmware > > > > > > > is > > > > > > > posted here: > > > > > > > http://lists.infradead.org/pipermail/linux-rpi-kernel/201 > > > > > > > 7-Ma > > > > > > > rch/ > > > > > > > 005924 > > > > > > > .html > > > > > > > > > > > > > > It even works with minecraft_pi! > > > > > > > > > > > > > > > > > > Hi, can you e-mail out your config.txt? Do you have audio > > > > enabled > > > > in > > > > config.txt? > > > > > > yes, I have this: > > > > > > $ cat config.txt |grep -i audio > > > # uncomment to force a HDMI mode rather than DVI. This can make > > > audio > > > work in > > > # Enable audio (loads snd_bcm2835) > > > dtparam=audio=on > > > > > > Full config attached. > > > > > > Thanks, > > > Mauro > > > > > > > Are you using Eric Anholt's HDMI Audio driver that's included in > > VC4? > > That could well be incompatible with the firmware driver. Or are > > you > > using a half mode of VC4 for audio and VCHIQ for video? > > I'm using vanilla staging Kernel, from Greg's tree: > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging. > git/commit/?h=staging- > next&id=7bc49cb9b9b8bad32536c4b6d1aff1824c1adc6c > > Plus the DWC2 fixup I wrote and DT changes you pointed > (see enclosed). > > I can disable the audio overlay here, as I don't have anything > connected to audio inputs/outputs. > > Regards, > Mauro > Why is the vchiq node in the tree twice? For me to even respond anymore you you going to have to include your entire dtb(whatever you are using) run through dtc -I dtb -O dts. You are also going to have to include your exact .config file you used for building, and exactly what these DWC2 fixeups are. You don't even state exactly what platform you are using, Is it even an RPI of some kind. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
On Mon, Mar 20, 2017 at 02:20:16PM +0100, Philipp Zabel wrote:
> To set and read colorimetry information:
> https://patchwork.linuxtv.org/patch/39350/
Thanks, I've applied all four of your patches, but there's a side effect
from that. Old media-ctl (modified by me):
- entity 53: imx219 0-0010 (2 pads, 2 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev9
pad0: Source
[fmt:SRGGB8/816x616 field:none
frame_interval:1/25]
-> "imx6-mipi-csi2":0 [ENABLED]
pad1: Sink
[fmt:SRGGB10/3280x2464 field:none
crop.bounds:(0,0)/3280x2464
crop:(0,0)/3264x2464
compose.bounds:(0,0)/3264x2464
compose:(0,0)/816x616]
<- "imx219 pixel 0-0010":0 [ENABLED,IMMUTABLE]
New media-ctl:
- entity 53: imx219 0-0010 (2 pads, 2 links)
type V4L2 subdev subtype Unknown flags 0
device node name /dev/v4l-subdev9
pad0: Source
[fmt:SRGGB8_1X8/816x616@1/25 field:none colorspace:srgb
xfer:srgb]
-> "imx6-mipi-csi2":0 [ENABLED]
pad1: Sink
<- "imx219 pixel 0-0010":0 [ENABLED,IMMUTABLE]
It looks like we successfully retrieve the frame interval for pad 0
and print it, but when we try to retrieve the frame interval for pad 1,
we get EINVAL (because that's what I'm returning, but I'm wondering if
that's the correct thing to do...) and that prevents _all_ format
information being output.
Maybe something like the following would be a better idea?
utils/media-ctl/media-ctl.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
index f61963a..a50a559 100644
--- a/utils/media-ctl/media-ctl.c
+++ b/utils/media-ctl/media-ctl.c
@@ -81,22 +81,22 @@ static void v4l2_subdev_print_format(struct media_entity
*entity,
struct v4l2_mbus_framefmt format;
struct v4l2_fract interval = { 0, 0 };
struct v4l2_rect rect;
- int ret;
+ int ret, err_fi;
ret = v4l2_subdev_get_format(entity, &format, pad, which);
if (ret != 0)
return;
- ret = v4l2_subdev_get_frame_interval(entity, &interval, pad);
- if (ret != 0 && ret != -ENOTTY)
- return;
+ err_fi = v4l2_subdev_get_frame_interval(entity, &interval, pad);
printf("\t\t[fmt:%s/%ux%u",
v4l2_subdev_pixelcode_to_string(format.code),
format.width, format.height);
- if (interval.numerator || interval.denominator)
+ if (err_fi == 0 && (interval.numerator || interval.denominator))
printf("@%u/%u", interval.numerator, interval.denominator);
+ else if (err_fi != -ENOTTY)
+ printf("@", strerror(-err_fi));
if (format.field)
printf(" field:%s", v4l2_subdev_field_to_string(format.field));
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
On 03/20/2017 03:11 PM, Russell King - ARM Linux wrote: > On Mon, Mar 20, 2017 at 02:57:03PM +0100, Hans Verkuil wrote: >> On 03/20/2017 02:29 PM, Russell King - ARM Linux wrote: >>> It's what I have - remember, not everyone is happy to constantly replace >>> their distro packages with random new stuff. >> >> This is a compliance test, which is continuously developed in tandem with >> new kernel versions. If you are working with an upstream kernel, then you >> should also use the corresponding v4l2-compliance test. What's the point >> of using an old one? >> >> I will not support driver developers that use an old version of the >> compliance test, that's a waste of my time. > > The reason that people may _not_ wish to constantly update v4l-utils > is that it changes the libraries installed on their systems. > > So, the solution to that is not to complain about developers not using > the latest version, but instead to de-couple it from the rest of the > package, and provide it as a separate, stand-alone package that doesn't > come with all the extra baggage. > > Now, there's two possible answers to that: > > 1. it depends on the libv4l2 version. If that's so, then you are >insisting that people constantly move to the latest libv4l2 because >of API changes, and those API changes may upset applications they're >using. So this isn't really on. > > 2. it doesn't depend on libv4l2 version, in which case there's no reason >for it to be packaged with v4l-utils. Run configure with --disable-v4l2-compliance-libv4l. This avoids linking with libv4l and allows you to build it stand-alone. Perhaps I should invert this option since in most cases you don't want to run v4l2-compliance with libv4l (it's off by default unless you pass the -w option to v4l2-compliance). > > The reality is that v4l2-compliance links with libv4l2, so I'm not sure > which it is. What I am sure of is that I don't want to upgrade libv4l2 > on an ad-hoc basis, potentially causing issues with applications. > To test actual streaming you need to provide the -s option. Note: v4l2-compliance has been developed for 'regular' video devices, not MC devices. It may or may not work with the -s option. >>> >>> Right, and it exists to verify that the establised v4l2 API is correctly >>> implemented. If the v4l2 API is being offered to user applications, >>> then it must be conformant, otherwise it's not offering the v4l2 API. >>> (That's very much a definition statement in itself.) >>> >>> So, are we really going to say MC devices do not offer the v4l2 API to >>> userspace, but something that might work? We've already seen today >>> one user say that they're not going to use mainline because of the >>> crud surrounding MC. >>> >> >> Actually, my understanding was that he was stuck on the old kernel code. > > Err, sorry, I really don't follow. Who is "he"? "one user say that they're not going to use mainline because of the crud surrounding MC." > > _I_ was the one who reported the EXPBUF problem. Your comment makes no > sense. > >> In the case of v4l2-compliance, I never had the time to make it work with >> MC devices. Same for that matter of certain memory to memory devices. >> >> Just like MC devices these too behave differently. They are partially >> supported in v4l2-compliance, but not fully. > > It seems you saying that the API provided by /dev/video* for a MC device > breaks the v4l2-compliance tests? There may be tests in the compliance suite that do not apply for MC devices and for which I never check. The compliance suite was never written with MC devices in mind, and it certainly hasn't been tested much with such devices. It's only very recent that I even got hardware that has MC support... >From what I can tell from the feedback I got it seems to be OKish, but I just can't guarantee that the compliance utility is correct for such devices. In particular I doubt the streaming tests (-s, -f, etc.) will work. The -s test *might* work if the pipeline is configured correctly before running v4l2-compliance. I can't imagine that the -f option would work at all since I would expect pipeline validation errors. I've been gently pushing Helen Koike to finish her virtual MC driver (https://patchwork.kernel.org/patch/9312783/) since having a virtual driver makes writing compliance tests much easier. > _No one_ has mentioned using v4l2-compliance on the subdevs. > >> Complaining about this really won't help. We know it's a problem and unless >> someone (me perhaps?) manages to get paid to work on this it's unlikely to >> change for now. > > Like the above comment, your comment makes no sense. I'm not complaining, > I'm trying to find out the details. Must be me then, it did feel like complaining... > Yes, MC stuff sucks big time right now, the documentation is poor, there's > a lack of understanding on all sides of the issues (which can be seen by > the different opinions that people hold.) T
[PATCH] staging: ad7759: Replace mlock with driver private lock
The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
In this driver, mlock was being used to protect hardware state
changes. Replace it with a lock in the devices global data.
Signed-off-by: Arushi Singhal
---
drivers/staging/iio/meter/ade7759.c | 4 ++--
drivers/staging/iio/meter/ade7759.h | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/iio/meter/ade7759.c
b/drivers/staging/iio/meter/ade7759.c
index 944ee3401029..1d1e0b33021f 100644
--- a/drivers/staging/iio/meter/ade7759.c
+++ b/drivers/staging/iio/meter/ade7759.c
@@ -381,7 +381,7 @@ static ssize_t ade7759_write_frequency(struct device *dev,
if (!val)
return -EINVAL;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->buf_lock);
t = 27900 / val;
if (t > 0)
@@ -402,7 +402,7 @@ static ssize_t ade7759_write_frequency(struct device *dev,
ret = ade7759_spi_write_reg_16(dev, ADE7759_MODE, reg);
out:
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->buf_lock);
return ret ? ret : len;
}
diff --git a/drivers/staging/iio/meter/ade7759.h
b/drivers/staging/iio/meter/ade7759.h
index f0716d2fdf8e..4f69bb93cc45 100644
--- a/drivers/staging/iio/meter/ade7759.h
+++ b/drivers/staging/iio/meter/ade7759.h
@@ -42,6 +42,7 @@
* @buf_lock: mutex to protect tx and rx
* @tx:transmit buffer
* @rx:receive buffer
+ * @lock protect sensor state
**/
struct ade7759_state {
struct spi_device *us;
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline
On Mon, Mar 20, 2017 at 12:39:38PM -0300, Mauro Carvalho Chehab wrote: > Em Mon, 20 Mar 2017 14:24:25 +0100 > Hans Verkuil escreveu: > > I don't think this control inheritance patch will magically prevent you > > from needed a plugin. > > Yeah, it is not just control inheritance. The driver needs to work > without subdev API, e. g. mbus settings should also be done via the > video devnode. > > Btw, Sakari made a good point on IRC: what happens if some app > try to change the pipeline or subdev settings while another > application is using the device? The driver should block such > changes, maybe using the V4L2 priority mechanism. My understanding is that there are already mechanisms in place to prevent that, but it's driver dependent - certainly several of the imx driver subdevs check whether they have an active stream, and refuse (eg) all set_fmt calls with -EBUSY if that is so. (That statement raises another question in my mind: if the subdev is streaming, should it refuse all set_fmt, even for the TRY stuff?) > In parallel, someone has to fix libv4l for it to be default on > applications like gstreamer, e. g. adding support for DMABUF > and fixing other issues that are preventing it to be used by > default. Hmm, not sure what you mean there - I've used dmabuf with gstreamer's v4l2src linked to libv4l2, importing the buffers into etnaviv using a custom plugin. There are distros around (ubuntu) where the v4l2 plugin is built against libv4l2. > My understanding here is that there are developers wanting/needing > to have standard V4L2 apps support for (some) i.MX6 devices. Those are > the ones that may/will allocate some time for it to happen. Quite - but we need to first know what is acceptable to the v4l2 community before we waste a lot of effort coding something up that may not be suitable. Like everyone else, there's only a limited amount of effort available, so using it wisely is a very good idea. Up until recently, it seemed that the only solution was to solve the userspace side of the media controller API via v4l2 plugins and the like. Much of my time that I have available to look at the imx6 capture stuff at the moment is taken up by triping over UAPI issues with the current code (such as the ones about CSI scaling, colorimetry, etc) and trying to get concensus on what the right solution to fix those issues actually is, and at the moment I don't have spare time to start addressing any kind of v4l2 plugin for user controls nor any other solution. Eg, I spent much of this last weekend sorting out my IMX219 camera sensor driver for my new understanding about how scaling is supposed to work, the frame enumeration issue (which I've posted patches for) and the CSI scaling issue (which I've some half-baked patch for at the moment, but probably by the time I've finished sorting that, Philipp or Steve will already have a solution.) That said, my new understanding of the scaling impacts the four patches I posted, and probably makes the frame size enumeration in CSI (due to its scaling) rather obsolete. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: outreachy
On Mon 2017-03-20 11:30:08, Greg KH wrote: > On Mon, Mar 20, 2017 at 11:20:32AM +0100, Pavel Machek wrote: > > Hi! > > > > > > > Hah! That's the joy of being a maintainer of a driver in staging. > > > > > Even > > > > > if you filter out outreachy, you are going to get a lot of "basic > > > > > mistakes" and other type patches cc:ed to you. > > > > > > > > > > I strongly suggest, that if you all don't like this type of stuff, > > > > > either: > > > > > - work to get the code out of staging as soon as possible (i.e. > > > > > send me coding style fixes for everything right now, and then > > > > > fix up the rest of the stuff.) > > > > > - take yourself off the maintainer list for this code. > > > > > > > > > > It's your choice, outreachy right now is a lot of patches, but again, > > > > > it's not going to keep you from getting the "basic" stuff sent to you > > > > > in ways that is totally wrong. > > > > > > > > Could we get these trivial patches off the lkml? Yes, lkml already has > > > > a lot of traffic, but no, this is not useful :-(. > > > > > > The outreachy instructions say to use the -nol argument to > > > get_maintainers, which would prevent them from being sent to any mailing > > > list. However others thought that all patches should be sent to mailing > > > lists, and so I haven't enforced anything for people who have omitted > > > -nol. However I have tried to remove bcm maintainers from CC lists on > > > replies and reminded people not to send you patches, > > > > Wonderful :-(. > > > > Can we at least make those people put the word "outreachy" in the > > subject so the emails are easier to delete? > > It's easy for you to filter away as-is if you want to to by just looking > at the cc: list for outreachy right now, don't make a new step for > people to jump through because you don't want to be bothered. You only > have 10 more days of it if you want to just ignore any patch until > then... 10 more days... I can survive 10 more days. I was afraid we would be stuck with it "forever". Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/9] stating/atomisp: fix -Wold-style-definition warning
On Mon, Mar 20, 2017 at 4:05 PM, Stephen Hemminger
wrote:
> On Mon, 20 Mar 2017 10:32:19 +0100
> Arnd Bergmann wrote:
>
>> -void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/)
>> +void ia_css_dequeue_param_buffers(/*unsigned int pipe_num*/ void)
>> {
> Why keep the comment?
The comment matches one later in the file when this function gets called.
I thought about cleaning up both at the same time, but couldn't figure out
how the comment ended up in there or why it was left behind in the first
place, so I ended up leaving both for another patch on top. If you prefer,
I could resend the patch and do both at once.
Arnd
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
On Mon, 2017-03-20 at 15:43 +, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 02:20:16PM +0100, Philipp Zabel wrote:
> > To set and read colorimetry information:
> > https://patchwork.linuxtv.org/patch/39350/
>
> Thanks, I've applied all four of your patches, but there's a side effect
> from that. Old media-ctl (modified by me):
>
> - entity 53: imx219 0-0010 (2 pads, 2 links)
> type V4L2 subdev subtype Unknown flags 0
> device node name /dev/v4l-subdev9
> pad0: Source
> [fmt:SRGGB8/816x616 field:none
> frame_interval:1/25]
> -> "imx6-mipi-csi2":0 [ENABLED]
> pad1: Sink
> [fmt:SRGGB10/3280x2464 field:none
> crop.bounds:(0,0)/3280x2464
> crop:(0,0)/3264x2464
> compose.bounds:(0,0)/3264x2464
> compose:(0,0)/816x616]
> <- "imx219 pixel 0-0010":0 [ENABLED,IMMUTABLE]
>
> New media-ctl:
>
> - entity 53: imx219 0-0010 (2 pads, 2 links)
> type V4L2 subdev subtype Unknown flags 0
> device node name /dev/v4l-subdev9
> pad0: Source
> [fmt:SRGGB8_1X8/816x616@1/25 field:none colorspace:srgb
> xfer:srgb]
> -> "imx6-mipi-csi2":0 [ENABLED]
> pad1: Sink
> <- "imx219 pixel 0-0010":0 [ENABLED,IMMUTABLE]
>
> It looks like we successfully retrieve the frame interval for pad 0
> and print it, but when we try to retrieve the frame interval for pad 1,
> we get EINVAL (because that's what I'm returning, but I'm wondering if
> that's the correct thing to do...) and that prevents _all_ format
> information being output.
According to the documentation [1], you are doing the right thing:
The struct v4l2_subdev_frame_interval pad references a non-existing
pad, or the pad doesn’t support frame intervals.
But v4l2_subdev_call returns -ENOIOCTLCMD if the g_frame_interval op is
not implemented at all, which is turned into -ENOTTY by video_usercopy.
[1]
https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/vidioc-subdev-g-frame-interval.html#return-value
> Maybe something like the following would be a better idea?
>
> utils/media-ctl/media-ctl.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
> index f61963a..a50a559 100644
> --- a/utils/media-ctl/media-ctl.c
> +++ b/utils/media-ctl/media-ctl.c
> @@ -81,22 +81,22 @@ static void v4l2_subdev_print_format(struct media_entity
> *entity,
> struct v4l2_mbus_framefmt format;
> struct v4l2_fract interval = { 0, 0 };
> struct v4l2_rect rect;
> - int ret;
> + int ret, err_fi;
>
> ret = v4l2_subdev_get_format(entity, &format, pad, which);
> if (ret != 0)
> return;
>
> - ret = v4l2_subdev_get_frame_interval(entity, &interval, pad);
> - if (ret != 0 && ret != -ENOTTY)
> - return;
> + err_fi = v4l2_subdev_get_frame_interval(entity, &interval, pad);
Not supporting frame intervals doesn't warrant a visible error message,
I think -EINVAL should also be ignored above, if the spec is to be
believed.
>
> printf("\t\t[fmt:%s/%ux%u",
> v4l2_subdev_pixelcode_to_string(format.code),
> format.width, format.height);
>
> - if (interval.numerator || interval.denominator)
> + if (err_fi == 0 && (interval.numerator || interval.denominator))
> printf("@%u/%u", interval.numerator, interval.denominator);
> + else if (err_fi != -ENOTTY)
> + printf("@", strerror(-err_fi));
Or here.
>
> if (format.field)
> printf(" field:%s", v4l2_subdev_field_to_string(format.field));
>
>
regards
Philipp
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/39] i.MX Media Driver
On Mon, Mar 20, 2017 at 05:29:07PM +0100, Philipp Zabel wrote:
> According to the documentation [1], you are doing the right thing:
>
> The struct v4l2_subdev_frame_interval pad references a non-existing
> pad, or the pad doesn’t support frame intervals.
>
> But v4l2_subdev_call returns -ENOIOCTLCMD if the g_frame_interval op is
> not implemented at all, which is turned into -ENOTTY by video_usercopy.
>
> [1]
> https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/vidioc-subdev-g-frame-interval.html#return-value
Thanks for confirming.
> > Maybe something like the following would be a better idea?
> >
> > utils/media-ctl/media-ctl.c | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
> > index f61963a..a50a559 100644
> > --- a/utils/media-ctl/media-ctl.c
> > +++ b/utils/media-ctl/media-ctl.c
> > @@ -81,22 +81,22 @@ static void v4l2_subdev_print_format(struct
> > media_entity *entity,
> > struct v4l2_mbus_framefmt format;
> > struct v4l2_fract interval = { 0, 0 };
> > struct v4l2_rect rect;
> > - int ret;
> > + int ret, err_fi;
> >
> > ret = v4l2_subdev_get_format(entity, &format, pad, which);
> > if (ret != 0)
> > return;
> >
> > - ret = v4l2_subdev_get_frame_interval(entity, &interval, pad);
> > - if (ret != 0 && ret != -ENOTTY)
> > - return;
> > + err_fi = v4l2_subdev_get_frame_interval(entity, &interval, pad);
>
> Not supporting frame intervals doesn't warrant a visible error message,
> I think -EINVAL should also be ignored above, if the spec is to be
> believed.
>
> >
> > printf("\t\t[fmt:%s/%ux%u",
> >v4l2_subdev_pixelcode_to_string(format.code),
> >format.width, format.height);
> >
> > - if (interval.numerator || interval.denominator)
> > + if (err_fi == 0 && (interval.numerator || interval.denominator))
> > printf("@%u/%u", interval.numerator, interval.denominator);
> > + else if (err_fi != -ENOTTY)
> > + printf("@", strerror(-err_fi));
>
> Or here.
I don't mind which - I could change this to:
else if (err_fi != -ENOTTY && err_fi != -EINVAL)
Or an alternative would be to print an error (ignoring ENOTTY and EINVAL)
to stderr at the "v4l2_subdev_get_frame_interval" callsite and continue
on (ensuring that interval is zeroed).
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RESEND PATCH] staging: ad7606: Replace mlock with driver private lock
On 03/20/2017 04:22 PM, Arushi Singhal wrote:
> The IIO subsystem is redefining iio_dev->mlock to be used by
> the IIO core only for protecting device operating mode changes.
> ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
>
> In this driver, mlock was being used to protect hardware state
> changes. Replace it with a lock in the devices global data.
>
> Signed-off-by: Arushi Singhal
Hi,
Looks good, but mutex_init() is missing.
- Lars
> ---
> drivers/staging/iio/adc/ad7606.c | 8
> drivers/staging/iio/adc/ad7606.h | 2 ++
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7606.c
> b/drivers/staging/iio/adc/ad7606.c
> index 9dbfa64b1e53..9f529b34e018 100644
> --- a/drivers/staging/iio/adc/ad7606.c
> +++ b/drivers/staging/iio/adc/ad7606.c
> @@ -208,7 +208,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
> switch (mask) {
> case IIO_CHAN_INFO_SCALE:
> ret = -EINVAL;
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock(&st->lock);
> for (i = 0; i < ARRAY_SIZE(scale_avail); i++)
> if (val2 == scale_avail[i][1]) {
> gpiod_set_value(st->gpio_range, i);
> @@ -217,7 +217,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
> ret = 0;
> break;
> }
> - mutex_unlock(&indio_dev->mlock);
> + mutex_unlock(&st->lock);
>
> return ret;
> case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> @@ -231,11 +231,11 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
> values[1] = (ret >> 1) & 1;
> values[2] = (ret >> 2) & 1;
>
> - mutex_lock(&indio_dev->mlock);
> + mutex_lock(&st->lock);
> gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
> values);
> st->oversampling = val;
> - mutex_unlock(&indio_dev->mlock);
> + mutex_unlock(&st->lock);
>
> return 0;
> default:
> diff --git a/drivers/staging/iio/adc/ad7606.h
> b/drivers/staging/iio/adc/ad7606.h
> index 746f9553d2ba..5d59bdd78727 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -14,6 +14,7 @@
> * @name:identification string for chip
> * @channels:channel specification
> * @num_channels:number of channels
> + * @lock protect sensor state
> */
>
> struct ad7606_chip_info {
> @@ -37,6 +38,7 @@ struct ad7606_state {
> booldone;
> void __iomem*base_address;
>
> + struct mutexlock; /* protect sensor state */
> struct gpio_desc*gpio_convst;
> struct gpio_desc*gpio_reset;
> struct gpio_desc*gpio_range;
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt
On Mon, Mar 20, 2017 at 02:17:05PM +, Russell King - ARM Linux wrote: > On Mon, Mar 20, 2017 at 03:00:51PM +0100, Philipp Zabel wrote: > > On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote: > > > The same document says: > > > > > > Scaling support is optional. When supported by a subdev, the crop > > > rectangle on the subdev's sink pad is scaled to the size configured > > > using the > > > :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL > > > using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the > > > subdev supports scaling but not composing, the top and left values are > > > not used and must always be set to zero. > > > > Right, this sentence does imply that when scaling is supported, there > > must be a sink compose rectangle, even when composing is not. > > > > I have previously set up scaling like this: > > > > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]" > > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]" > > > > Does this mean, it should work like this instead? > > > > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]" > > media-ctl --set-v4l2 > > "'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]" > > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]" > > > > I suppose setting the source pad format should not be allowed to modify > > the sink compose rectangle. > > That is what I believe having read these documents several times, but > we need v4l2 people to confirm. > > Note that setting the format on 'ipu1_csi0':0 should already be done by > the previous media-ctl command, so it should be possible to simplify > that to: > > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]" > media-ctl --set-v4l2 "'ipu1_csi0':0[compose:(0,0)/960x540]" > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]" There is a slightly puzzling bit in the documentation. If we consider the CSI, and the sink compose rectangle size has to always match the the source output pad format size (since in hardware they are one of the same), then: Inside subdevs, the order of image processing steps will always be from the sink pad towards the source pad. This is also reflected in the order in which the configuration must be performed by the user: the changes made will be propagated to any subsequent stages. If this behaviour is not desired, the user must set ``V4L2_SEL_FLAG_KEEP_CONFIG`` flag. This flag causes no propagation of the changes are allowed in any circumstances. This may also cause the accessed rectangle to be adjusted by the driver, depending on the properties of the underlying hardware. ^^ presumably, this means if we get a request to change the source compose rectangle with V4L2_SEL_FLAG_KEEP_CONFIG set, we need to force its size to be the current output format size. I don't think we can do anything else - because the above makes it very clear that any following stages shall not be updated. The last sentence appears to give permission to do that. This also has implications when changing the sink crop - the sink crop (eg) must not be smaller than the sink compose, as we don't support scaling up in CSI. It seems to me that V4L2_SEL_FLAG_KEEP_CONFIG in practice changes the way validation of the request works. So, rather than validating the request against the upstream rectangle and propagating downstream, it needs to be validated against both the upstream and downstream rectangles instead. It seems there's many subtleties to this... -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] bcm2835-gpio-exp: Driver for GPIO expander via mailbox service
Michael Zoran writes: >> > Since the API is completely documented, I see no reason we or >> > anybody >> > couldn't essentially rewrite the driver while it's in staging. I >> > just >> > think it would be best for everyone if the new version was a drop >> > in >> > replacement for the original version. Essential an enhancement >> > rather >> > then a competitor. >> >> I think my comments weren't fundamental changes, but you surely mean >> the devicetree ABI? I like to see this driver ASAP out of staging and >> i'm not interested to maintain 2 functional identical driver only to >> keep compability with the Foundation tree. Currently i'm afraid that >> we build up many drivers in staging, which need a complete rewrite >> later if they should come out of staging. It would be nice if we >> could avoid the situation we have with the thermal driver. >> >> Stefan > > The API I'm talking about here is the mailbox API that is used to talk > to the firmware. The numbers and structures to pass are documented. > Nothing prevents anybody from rewriting this driver and submitting it > to the appropriate subsystems. It's certainly small enough. > > If you really want working thermal or cpu speed drivers today, nothing > stops anybody from submitting the downstream drivers after doing some > minor touchups and submitting them to staging. That would at least get > things working while people argue about what the correct DT nodes > should be. > > I would also like to point out that the RPI 3 has been out for over a > year and nobody has been able to get working video out of it through > VC4 on a mainline tree. At least until now. So I'm not sure the best > way to go is for the expander driver to go under the GPIO subtree. Excuse me? Display works fine on my Pi3. VC4 uses DDC to probe for connection when the GPIO line isn't present in the DT. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt
Hi Steve, Russell,
On Mon, 2017-03-20 at 14:17 +, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 03:00:51PM +0100, Philipp Zabel wrote:
> > On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote:
> > > The same document says:
> > >
> > > Scaling support is optional. When supported by a subdev, the crop
> > > rectangle on the subdev's sink pad is scaled to the size configured
> > > using the
> > > :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL
> > > using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the
> > > subdev supports scaling but not composing, the top and left values are
> > > not used and must always be set to zero.
> >
> > Right, this sentence does imply that when scaling is supported, there
> > must be a sink compose rectangle, even when composing is not.
> >
> > I have previously set up scaling like this:
> >
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> >
> > Does this mean, it should work like this instead?
> >
> > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]"
> > media-ctl --set-v4l2
> > "'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]"
> > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]"
> >
> > I suppose setting the source pad format should not be allowed to modify
> > the sink compose rectangle.
>
> That is what I believe having read these documents several times, but
> we need v4l2 people to confirm.
What do you think of this:
--8<--
>From 2830aebc404bdfc9d7fc1ec94e5282d0b668e8f6 Mon Sep 17 00:00:00 2001
From: Philipp Zabel
Date: Mon, 20 Mar 2017 17:10:21 +0100
Subject: [PATCH] media: imx: csi: add sink selection rectangles
Move the crop rectangle to the sink pad and add a sink compose rectangle
to configure scaling. Also propagate rectangles from sink pad to crop
rectangle, to compose rectangle, and to the source pads both in ACTIVE
and TRY variants of set_fmt/selection, and initialize the default crop
and compose rectangles.
Signed-off-by: Philipp Zabel
---
drivers/staging/media/imx/imx-media-csi.c | 216 +-
1 file changed, 152 insertions(+), 64 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 560da3abdd70b..b026a5d602ddf 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -79,6 +79,7 @@ struct csi_priv {
const struct imx_media_pixfmt *cc[CSI_NUM_PADS];
struct v4l2_fract frame_interval;
struct v4l2_rect crop;
+ struct v4l2_rect compose;
const struct csi_skip_desc *skip[CSI_NUM_PADS - 1];
/* active vb2 buffers to send to video dev sink */
@@ -574,8 +575,8 @@ static int csi_setup(struct csi_priv *priv)
ipu_csi_set_window(priv->csi, &priv->crop);
ipu_csi_set_downsize(priv->csi,
-priv->crop.width == 2 * outfmt->width,
-priv->crop.height == 2 * outfmt->height);
+priv->crop.width == 2 * priv->compose.width,
+priv->crop.height == 2 * priv->compose.height);
ipu_csi_init_interface(priv->csi, &sensor_mbus_cfg, &if_fmt);
@@ -1001,6 +1002,27 @@ __csi_get_fmt(struct csi_priv *priv, struct
v4l2_subdev_pad_config *cfg,
return &priv->format_mbus[pad];
}
+static struct v4l2_rect *
+__csi_get_crop(struct csi_priv *priv, struct v4l2_subdev_pad_config *cfg,
+ enum v4l2_subdev_format_whence which)
+{
+ if (which == V4L2_SUBDEV_FORMAT_TRY)
+ return v4l2_subdev_get_try_crop(&priv->sd, cfg, CSI_SINK_PAD);
+ else
+ return &priv->crop;
+}
+
+static struct v4l2_rect *
+__csi_get_compose(struct csi_priv *priv, struct v4l2_subdev_pad_config *cfg,
+ enum v4l2_subdev_format_whence which)
+{
+ if (which == V4L2_SUBDEV_FORMAT_TRY)
+ return v4l2_subdev_get_try_compose(&priv->sd, cfg,
+ CSI_SINK_PAD);
+ else
+ return &priv->compose;
+}
+
static void csi_try_crop(struct csi_priv *priv,
struct v4l2_rect *crop,
struct v4l2_subdev_pad_config *cfg,
@@ -1159,6 +1181,7 @@ static void csi_try_fmt(struct csi_priv *priv,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *sdformat,
struct v4l2_rect *crop,
+ struct v4l2_rect *compose,
const struct imx_media_pixfmt **cc)
{
const struct imx_media_pixfmt *incc;
@@ -1173,15 +1196,8 @@ static void csi_try_fmt(struct csi_priv *priv,
incc = imx_media_find_mbus_format(infmt->code,
CS_SEL_ANY, true);
-
Re: [PATCH 1/6] bcm2835-gpio-exp: Driver for GPIO expander via mailbox service
On Mon, 2017-03-20 at 10:22 -0700, Eric Anholt wrote: > Michael Zoran writes: > > > > > Since the API is completely documented, I see no reason we or > > > > anybody > > > > couldn't essentially rewrite the driver while it's in > > > > staging. I > > > > just > > > > think it would be best for everyone if the new version was a > > > > drop > > > > in > > > > replacement for the original version. Essential an enhancement > > > > rather > > > > then a competitor. > > > > > > I think my comments weren't fundamental changes, but you surely > > > mean > > > the devicetree ABI? I like to see this driver ASAP out of staging > > > and > > > i'm not interested to maintain 2 functional identical driver only > > > to > > > keep compability with the Foundation tree. Currently i'm afraid > > > that > > > we build up many drivers in staging, which need a complete > > > rewrite > > > later if they should come out of staging. It would be nice if we > > > could avoid the situation we have with the thermal driver. > > > > > > Stefan > > > > The API I'm talking about here is the mailbox API that is used to > > talk > > to the firmware. The numbers and structures to pass are > > documented. > > Nothing prevents anybody from rewriting this driver and submitting > > it > > to the appropriate subsystems. It's certainly small enough. > > > > If you really want working thermal or cpu speed drivers today, > > nothing > > stops anybody from submitting the downstream drivers after doing > > some > > minor touchups and submitting them to staging. That would at least > > get > > things working while people argue about what the correct DT nodes > > should be. > > > > I would also like to point out that the RPI 3 has been out for over > > a > > year and nobody has been able to get working video out of it > > through > > VC4 on a mainline tree. At least until now. So I'm not sure the > > best > > way to go is for the expander driver to go under the GPIO subtree. > > Excuse me? Display works fine on my Pi3. VC4 uses DDC to probe for > connection when the GPIO line isn't present in the DT. Is this arm32 or arm64 modes? And is this through the simplefb that was adding for testing is the VC4 driver fully driving the display. Is VC4 still in the .config that you used? Is this a standard version of VC4, or does it include modifications for testing purposes? If it works so well as is, why do you need or want the expander? You tried to submit a very similar version a year ago? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline
Em Mon, 20 Mar 2017 16:10:03 + Russell King - ARM Linux escreveu: > On Mon, Mar 20, 2017 at 12:39:38PM -0300, Mauro Carvalho Chehab wrote: > > Em Mon, 20 Mar 2017 14:24:25 +0100 > > Hans Verkuil escreveu: > > > I don't think this control inheritance patch will magically prevent you > > > from needed a plugin. > > > > Yeah, it is not just control inheritance. The driver needs to work > > without subdev API, e. g. mbus settings should also be done via the > > video devnode. > > > > Btw, Sakari made a good point on IRC: what happens if some app > > try to change the pipeline or subdev settings while another > > application is using the device? The driver should block such > > changes, maybe using the V4L2 priority mechanism. > > My understanding is that there are already mechanisms in place to > prevent that, but it's driver dependent - certainly several of the > imx driver subdevs check whether they have an active stream, and > refuse (eg) all set_fmt calls with -EBUSY if that is so. > > (That statement raises another question in my mind: if the subdev is > streaming, should it refuse all set_fmt, even for the TRY stuff?) Returning -EBUSY only when streaming is too late, as ioctl's may be changing the pipeline configuration and/or buffer allocation, while the application is sending other ioctls in order to prepare for streaming. V4L2 has a mechanism of blocking other apps to change such parameters via VIDIOC_S_PRIORITY[1]. If an application sets priority to V4L2_PRIORITY_RECORD, any other application attempting to change the device via some other file descriptor will fail. So, it is a sort of "exclusive write access". On a quick look at V4L2 core, currently, sending a VIDIOC_S_PRIORITY ioctl to a /dev/video device doesn't seem to have any effect at either MC or V4L2 subdev API for the subdevs connected to it. We'll likely need to add some code at v4l2_prio_change() for it to notify the subdevs for them to return -EBUSY if one would try to change something there, while the device is priorized. [1] https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/vidioc-g-priority.html > > In parallel, someone has to fix libv4l for it to be default on > > applications like gstreamer, e. g. adding support for DMABUF > > and fixing other issues that are preventing it to be used by > > default. > > Hmm, not sure what you mean there - I've used dmabuf with gstreamer's > v4l2src linked to libv4l2, importing the buffers into etnaviv using > a custom plugin. There are distros around (ubuntu) where the v4l2 > plugin is built against libv4l2. Hmm... I guess some gst developer mentioned that there are/where some restrictions at libv4l2 with regards to DMABUF. I may be wrong. > > > My understanding here is that there are developers wanting/needing > > to have standard V4L2 apps support for (some) i.MX6 devices. Those are > > the ones that may/will allocate some time for it to happen. > > Quite - but we need to first know what is acceptable to the v4l2 > community before we waste a lot of effort coding something up that > may not be suitable. Like everyone else, there's only a limited > amount of effort available, so using it wisely is a very good idea. Sure. That's why we're discussing here :-) > Up until recently, it seemed that the only solution was to solve the > userspace side of the media controller API via v4l2 plugins and the > like. > > Much of my time that I have available to look at the imx6 capture > stuff at the moment is taken up by triping over UAPI issues with the > current code (such as the ones about CSI scaling, colorimetry, etc) > and trying to get concensus on what the right solution to fix those > issues actually is, and at the moment I don't have spare time to > start addressing any kind of v4l2 plugin for user controls nor any > other solution. I hear you. A solution via libv4l could be more elegant, but it doesn't seem simple, as nobody did it before, and depends on the libv4l plugin mechanism, with is currently unused. Also, I think it is easier to provide a solution using DT and some driver and/or core support for it, specially since, AFAICT, currently there's no way request exclusive access to MC and subdevs. It is probably not possible do to that exclusively in userspace. > Eg, I spent much of this last weekend sorting out my IMX219 camera > sensor driver for my new understanding about how scaling is supposed > to work, the frame enumeration issue (which I've posted patches for) > and the CSI scaling issue (which I've some half-baked patch for at the > moment, but probably by the time I've finished sorting that, Philipp > or Steve will already have a solution.) > > That said, my new understanding of the scaling impacts the four patches > I posted, and probably makes the frame size enumeration in CSI (due to > its scaling) rather obsolete. Yeah, when there's no scaler, it should report just the resolution(s) supported by the sensor (actually, at the CSI) via V4L2_
Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt
On Mon, 2017-03-20 at 14:17 +, Russell King - ARM Linux wrote: > On Mon, Mar 20, 2017 at 03:00:51PM +0100, Philipp Zabel wrote: > > On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote: > > > The same document says: > > > > > > Scaling support is optional. When supported by a subdev, the crop > > > rectangle on the subdev's sink pad is scaled to the size configured > > > using the > > > :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL > > > using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the > > > subdev supports scaling but not composing, the top and left values are > > > not used and must always be set to zero. > > > > Right, this sentence does imply that when scaling is supported, there > > must be a sink compose rectangle, even when composing is not. > > > > I have previously set up scaling like this: > > > > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]" > > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]" > > > > Does this mean, it should work like this instead? > > > > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]" > > media-ctl --set-v4l2 > > "'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]" > > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]" > > > > I suppose setting the source pad format should not be allowed to modify > > the sink compose rectangle. > > That is what I believe having read these documents several times, but > we need v4l2 people to confirm. > > Note that setting the format on 'ipu1_csi0':0 should already be done by > the previous media-ctl command, so it should be possible to simplify > that to: > > media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]" > media-ctl --set-v4l2 "'ipu1_csi0':0[compose:(0,0)/960x540]" > media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]" Thanks, that works, too. > I have tripped over a bug in media-ctl when specifying both a crop and > compose rectangle - the --help output suggests that "," should be used > to separate them. media-ctl rejects that, telling me the character at > the "," should be "]". Replacing the "," with " " allows media-ctl to > accept it and set both rectangles, so it sounds like a parser bug - I've > not looked into this any further yet. I can confirm this. I don't see any place in v4l2_subdev_parse_pad_format that handles the "," separator. There's just whitespace skipping between the v4l2-properties. regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4.9 52/93] x86/hyperv: Handle unknown NMIs on one CPU when unknown_nmi_panic
4.9-stable review patch. If anyone has any objections, please let me know.
--
From: Vitaly Kuznetsov
[ Upstream commit 59107e2f48831daedc46973ce4988605ab066de3 ]
There is a feature in Hyper-V ('Debug-VM --InjectNonMaskableInterrupt')
which injects NMI to the guest. We may want to crash the guest and do kdump
on this NMI by enabling unknown_nmi_panic. To make kdump succeed we need to
allow the kdump kernel to re-establish VMBus connection so it will see
VMBus devices (storage, network,..).
To properly unload VMBus making it possible to start over during kdump we
need to do the following:
- Send an 'unload' message to the hypervisor. This can be done on any CPU
so we do this the crashing CPU.
- Receive the 'unload finished' reply message. WS2012R2 delivers this
message to the CPU which was used to establish VMBus connection during
module load and this CPU may differ from the CPU sending 'unload'.
Receiving a VMBus message means the following:
- There is a per-CPU slot in memory for one message. This slot can in
theory be accessed by any CPU.
- We get an interrupt on the CPU when a message was placed into the slot.
- When we read the message we need to clear the slot and signal the fact
to the hypervisor. In case there are more messages to this CPU pending
the hypervisor will deliver the next message. The signaling is done by
writing to an MSR so this can only be done on the appropriate CPU.
To avoid doing cross-CPU work on crash we have vmbus_wait_for_unload()
function which checks message slots for all CPUs in a loop waiting for the
'unload finished' messages. However, there is an issue which arises when
these conditions are met:
- We're crashing on a CPU which is different from the one which was used
to initially contact the hypervisor.
- The CPU which was used for the initial contact is blocked with interrupts
disabled and there is a message pending in the message slot.
In this case we won't be able to read the 'unload finished' message on the
crashing CPU. This is reproducible when we receive unknown NMIs on all CPUs
simultaneously: the first CPU entering panic() will proceed to crash and
all other CPUs will stop themselves with interrupts disabled.
The suggested solution is to handle unknown NMIs for Hyper-V guests on the
first CPU which gets them only. This will allow us to rely on VMBus
interrupt handler being able to receive the 'unload finish' message in
case it is delivered to a different CPU.
The issue is not reproducible on WS2016 as Debug-VM delivers NMI to the
boot CPU only, WS2012R2 and earlier Hyper-V versions are affected.
Signed-off-by: Vitaly Kuznetsov
Acked-by: K. Y. Srinivasan
Cc: de...@linuxdriverproject.org
Cc: Haiyang Zhang
Link: http://lkml.kernel.org/r/20161202100720.28121-1-vkuzn...@redhat.com
Signed-off-by: Thomas Gleixner
Signed-off-by: Ingo Molnar
Signed-off-by: Sasha Levin
Signed-off-by: Greg Kroah-Hartman
---
arch/x86/kernel/cpu/mshyperv.c | 24
1 file changed, 24 insertions(+)
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
struct ms_hyperv_info ms_hyperv;
EXPORT_SYMBOL_GPL(ms_hyperv);
@@ -158,6 +159,26 @@ static unsigned char hv_get_nmi_reason(v
return 0;
}
+#ifdef CONFIG_X86_LOCAL_APIC
+/*
+ * Prior to WS2016 Debug-VM sends NMIs to all CPUs which makes
+ * it dificult to process CHANNELMSG_UNLOAD in case of crash. Handle
+ * unknown NMI on the first CPU which gets it.
+ */
+static int hv_nmi_unknown(unsigned int val, struct pt_regs *regs)
+{
+ static atomic_t nmi_cpu = ATOMIC_INIT(-1);
+
+ if (!unknown_nmi_panic)
+ return NMI_DONE;
+
+ if (atomic_cmpxchg(&nmi_cpu, -1, raw_smp_processor_id()) != -1)
+ return NMI_HANDLED;
+
+ return NMI_DONE;
+}
+#endif
+
static void __init ms_hyperv_init_platform(void)
{
/*
@@ -183,6 +204,9 @@ static void __init ms_hyperv_init_platfo
pr_info("HyperV: LAPIC Timer Frequency: %#x\n",
lapic_timer_frequency);
}
+
+ register_nmi_handler(NMI_UNKNOWN, hv_nmi_unknown, NMI_FLAG_FIRST,
+"hv_nmi_unknown");
#endif
if (ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt
On Mon, Mar 20, 2017 at 06:40:21PM +0100, Philipp Zabel wrote:
> On Mon, 2017-03-20 at 14:17 +, Russell King - ARM Linux wrote:
> > I have tripped over a bug in media-ctl when specifying both a crop and
> > compose rectangle - the --help output suggests that "," should be used
> > to separate them. media-ctl rejects that, telling me the character at
> > the "," should be "]". Replacing the "," with " " allows media-ctl to
> > accept it and set both rectangles, so it sounds like a parser bug - I've
> > not looked into this any further yet.
>
> I can confirm this. I don't see any place in
> v4l2_subdev_parse_pad_format that handles the "," separator. There's
> just whitespace skipping between the v4l2-properties.
Maybe this is the easiest solution:
utils/media-ctl/options.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
index 83ca1ca..8b97874 100644
--- a/utils/media-ctl/options.c
+++ b/utils/media-ctl/options.c
@@ -65,7 +65,7 @@ static void usage(const char *argv0)
printf("\tentity = entity-number | ( '\"' entity-name '\"' )
;\n");
printf("\n");
printf("\tv4l2= pad '[' v4l2-properties ']' ;\n");
- printf("\tv4l2-properties = v4l2-property { ',' v4l2-property } ;\n");
+ printf("\tv4l2-properties = v4l2-property { ' '* v4l2-property } ;\n");
printf("\tv4l2-property = v4l2-mbusfmt | v4l2-crop |
v4l2-interval\n");
printf("\t| v4l2-compose | v4l2-interval ;\n");
printf("\tv4l2-mbusfmt= 'fmt:' fcc '/' size ; { 'field:' v4l2-field
; } { 'colorspace:' v4l2-colorspace ; }\n");
;)
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] bcm2835-gpio-exp: Driver for GPIO expander via mailbox service
On Mon, 2017-03-20 at 10:28 -0700, Michael Zoran wrote: > On Mon, 2017-03-20 at 10:22 -0700, Eric Anholt wrote: > > Michael Zoran writes: > > > > > > > Since the API is completely documented, I see no reason we or > > > > > anybody > > > > > couldn't essentially rewrite the driver while it's in > > > > > staging. I > > > > > just > > > > > think it would be best for everyone if the new version was a > > > > > drop > > > > > in > > > > > replacement for the original version. Essential an > > > > > enhancement > > > > > rather > > > > > then a competitor. > > > > > > > > I think my comments weren't fundamental changes, but you surely > > > > mean > > > > the devicetree ABI? I like to see this driver ASAP out of > > > > staging > > > > and > > > > i'm not interested to maintain 2 functional identical driver > > > > only > > > > to > > > > keep compability with the Foundation tree. Currently i'm afraid > > > > that > > > > we build up many drivers in staging, which need a complete > > > > rewrite > > > > later if they should come out of staging. It would be nice if > > > > we > > > > could avoid the situation we have with the thermal driver. > > > > > > > > Stefan > > > > > > The API I'm talking about here is the mailbox API that is used to > > > talk > > > to the firmware. The numbers and structures to pass are > > > documented. > > > Nothing prevents anybody from rewriting this driver and > > > submitting > > > it > > > to the appropriate subsystems. It's certainly small enough. > > > > > > If you really want working thermal or cpu speed drivers today, > > > nothing > > > stops anybody from submitting the downstream drivers after doing > > > some > > > minor touchups and submitting them to staging. That would at > > > least > > > get > > > things working while people argue about what the correct DT nodes > > > should be. > > > > > > I would also like to point out that the RPI 3 has been out for > > > over > > > a > > > year and nobody has been able to get working video out of it > > > through > > > VC4 on a mainline tree. At least until now. So I'm not sure the > > > best > > > way to go is for the expander driver to go under the GPIO > > > subtree. > > > > Excuse me? Display works fine on my Pi3. VC4 uses DDC to probe > > for > > connection when the GPIO line isn't present in the DT. > > Is this arm32 or arm64 modes? And is this through the simplefb that > was > adding for testing is the VC4 driver fully driving the display. Is > VC4 > still in the .config that you used? Is this a standard version of > VC4, > or does it include modifications for testing purposes? > > If it works so well as is, why do you need or want the expander? You > tried to submit a very similar version a year ago? Since Stephan seems to have taken this sudden intense interest in vc04_services all of a sudden, perhaps now would be a excellent time to actually acomplish one of the TODOs and get the DT bindings in for this driver. Since it is working an all. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] staging: ad7606: Replace mlock with driver private lock
The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
In this driver, mlock was being used to protect hardware state
changes. Replace it with a lock in the devices global data.
Signed-off-by: Arushi Singhal
---
changes in v3
- Add mutex_init.
drivers/staging/iio/adc/ad7606.c | 9 +
drivers/staging/iio/adc/ad7606.h | 2 ++
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 9dbfa64b1e53..18f5f139117e 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -208,7 +208,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_SCALE:
ret = -EINVAL;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
for (i = 0; i < ARRAY_SIZE(scale_avail); i++)
if (val2 == scale_avail[i][1]) {
gpiod_set_value(st->gpio_range, i);
@@ -217,7 +217,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
ret = 0;
break;
}
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return ret;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -231,11 +231,11 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
values[1] = (ret >> 1) & 1;
values[2] = (ret >> 2) & 1;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
values);
st->oversampling = val;
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return 0;
default:
@@ -413,6 +413,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem
*base_address,
st = iio_priv(indio_dev);
st->dev = dev;
+ mutex_init(&st->lock);
st->bops = bops;
st->base_address = base_address;
/* tied to logic low, analog input range is +/- 5V */
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 746f9553d2ba..5d59bdd78727 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -14,6 +14,7 @@
* @name: identification string for chip
* @channels: channel specification
* @num_channels: number of channels
+ * @lock protect sensor state
*/
struct ad7606_chip_info {
@@ -37,6 +38,7 @@ struct ad7606_state {
booldone;
void __iomem*base_address;
+ struct mutexlock; /* protect sensor state */
struct gpio_desc*gpio_convst;
struct gpio_desc*gpio_reset;
struct gpio_desc*gpio_range;
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: ad7606: Replace mlock with driver private lock
The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
In this driver, mlock was being used to protect hardware state
changes. Replace it with a lock in the devices global data.
Signed-off-by: Arushi Singhal
---
changes in v2
- add mutex_init.
drivers/staging/iio/adc/ad7606.c | 9 +
drivers/staging/iio/adc/ad7606.h | 2 ++
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 9dbfa64b1e53..b58641369596 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -208,7 +208,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_SCALE:
ret = -EINVAL;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
for (i = 0; i < ARRAY_SIZE(scale_avail); i++)
if (val2 == scale_avail[i][1]) {
gpiod_set_value(st->gpio_range, i);
@@ -217,7 +217,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
ret = 0;
break;
}
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return ret;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -231,11 +231,11 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
values[1] = (ret >> 1) & 1;
values[2] = (ret >> 2) & 1;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
values);
st->oversampling = val;
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return 0;
default:
@@ -413,6 +413,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem
*base_address,
st = iio_priv(indio_dev);
st->dev = dev;
+ mutex_init(&chip->state_lock);
st->bops = bops;
st->base_address = base_address;
/* tied to logic low, analog input range is +/- 5V */
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 746f9553d2ba..5d59bdd78727 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -14,6 +14,7 @@
* @name: identification string for chip
* @channels: channel specification
* @num_channels: number of channels
+ * @lock protect sensor state
*/
struct ad7606_chip_info {
@@ -37,6 +38,7 @@ struct ad7606_state {
booldone;
void __iomem*base_address;
+ struct mutexlock; /* protect sensor state */
struct gpio_desc*gpio_convst;
struct gpio_desc*gpio_reset;
struct gpio_desc*gpio_range;
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] bcm2835-gpio-exp: Driver for GPIO expander via mailbox service
On Mon, 2017-03-20 at 11:54 -0700, Michael Zoran wrote: > On Mon, 2017-03-20 at 10:28 -0700, Michael Zoran wrote: > > On Mon, 2017-03-20 at 10:22 -0700, Eric Anholt wrote: > > > Michael Zoran writes: > > > > > > > > > Since the API is completely documented, I see no reason we > > > > > > or > > > > > > anybody > > > > > > couldn't essentially rewrite the driver while it's in > > > > > > staging. I > > > > > > just > > > > > > think it would be best for everyone if the new version was > > > > > > a > > > > > > drop > > > > > > in > > > > > > replacement for the original version. Essential an > > > > > > enhancement > > > > > > rather > > > > > > then a competitor. > > > > > > > > > > I think my comments weren't fundamental changes, but you > > > > > surely > > > > > mean > > > > > the devicetree ABI? I like to see this driver ASAP out of > > > > > staging > > > > > and > > > > > i'm not interested to maintain 2 functional identical driver > > > > > only > > > > > to > > > > > keep compability with the Foundation tree. Currently i'm > > > > > afraid > > > > > that > > > > > we build up many drivers in staging, which need a complete > > > > > rewrite > > > > > later if they should come out of staging. It would be nice if > > > > > we > > > > > could avoid the situation we have with the thermal driver. > > > > > > > > > > Stefan > > > > > > > > The API I'm talking about here is the mailbox API that is used > > > > to > > > > talk > > > > to the firmware. The numbers and structures to pass are > > > > documented. > > > > Nothing prevents anybody from rewriting this driver and > > > > submitting > > > > it > > > > to the appropriate subsystems. It's certainly small enough. > > > > > > > > If you really want working thermal or cpu speed drivers today, > > > > nothing > > > > stops anybody from submitting the downstream drivers after > > > > doing > > > > some > > > > minor touchups and submitting them to staging. That would at > > > > least > > > > get > > > > things working while people argue about what the correct DT > > > > nodes > > > > should be. > > > > > > > > I would also like to point out that the RPI 3 has been out for > > > > over > > > > a > > > > year and nobody has been able to get working video out of it > > > > through > > > > VC4 on a mainline tree. At least until now. So I'm not sure > > > > the > > > > best > > > > way to go is for the expander driver to go under the GPIO > > > > subtree. > > > > > > Excuse me? Display works fine on my Pi3. VC4 uses DDC to probe > > > for > > > connection when the GPIO line isn't present in the DT. > > > > Is this arm32 or arm64 modes? And is this through the simplefb that > > was > > adding for testing is the VC4 driver fully driving the display. Is > > VC4 > > still in the .config that you used? Is this a standard version of > > VC4, > > or does it include modifications for testing purposes? > > > > If it works so well as is, why do you need or want the > > expander? You > > tried to submit a very similar version a year ago? > > Since Stephan seems to have taken this sudden intense interest in > vc04_services all of a sudden, perhaps now would be a excellent time > to > actually acomplish one of the TODOs and get the DT bindings in for > this > driver. Since it is working an all. Since Stefan always has such interesting comments on patches and I've submitting changes to a driver that doesn't even offically load, perhaps someone can help me get one of the TODO items removed and make real progress getting this driver out of staging by sending me a real e-mail address of who I should send an actual working DT for the RPI 3 to that will actually be seriously considered. I'll leave it at that. Thanks. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: ad7606: Replace mlock with driver private lock
On 03/20/2017 07:56 PM, Arushi Singhal wrote:
[...]
> @@ -413,6 +413,7 @@ int ad7606_probe(struct device *dev, int irq, void
> __iomem *base_address,
> st = iio_priv(indio_dev);
>
> st->dev = dev;
> + mutex_init(&st->lock);
This is nitpicking, but putting this in the middle of the assignments has a
bit of a weired flow it. Either put it above or below the assignments.
> st->bops = bops;
> st->base_address = base_address;
> /* tied to logic low, analog input range is +/- 5V */
> diff --git a/drivers/staging/iio/adc/ad7606.h
> b/drivers/staging/iio/adc/ad7606.h
> index 746f9553d2ba..5d59bdd78727 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -14,6 +14,7 @@
> * @name:identification string for chip
> * @channels:channel specification
> * @num_channels:number of channels
> + * @lock protect sensor state
This documentation is for struct ad7607_chip_info, but you are adding the
field to struct ad7606_state.
> */
>
> struct ad7606_chip_info {
> @@ -37,6 +38,7 @@ struct ad7606_state {
> booldone;
> void __iomem*base_address;
>
> + struct mutexlock; /* protect sensor state */
> struct gpio_desc*gpio_convst;
> struct gpio_desc*gpio_reset;
> struct gpio_desc*gpio_range;
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt
On 03/20/2017 07:00 AM, Philipp Zabel wrote: On Mon, 2017-03-20 at 12:08 +, Russell King - ARM Linux wrote: On Mon, Mar 20, 2017 at 12:55:26PM +0100, Philipp Zabel wrote: The above paragraph suggests we skip any rectangles that are not supported. In our case that would be 3. and 4., since the CSI can't compose into a larger frame. I hadn't realised that the crop selection currently happens on the source pad. I'd recommend viewing the documentation in its post-processed version, because then you get the examples as pictures, and they say that a picture is worth 1000 words. See https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/dev-subdev.html There is almost an exact example of what we're trying to do - it's figure 4.6. Here, we have a sink pad with a cropping rectangle on the input, which is then scaled to a composition rectangle (there's no bounds rectangle, and it's specified that in such a case the top,left of the composition rectangle will always be 0,0 - see quote below). Where it differs is that the example also supports source cropping for two source pads. We don't support that. The same document says: Scaling support is optional. When supported by a subdev, the crop rectangle on the subdev's sink pad is scaled to the size configured using the :ref:`VIDIOC_SUBDEV_S_SELECTION ` IOCTL using ``V4L2_SEL_TGT_COMPOSE`` selection target on the same pad. If the subdev supports scaling but not composing, the top and left values are not used and must always be set to zero. Right, this sentence does imply that when scaling is supported, there must be a sink compose rectangle, even when composing is not. Ok, this all makes consistent sense to me too. So: - the CSI hardware cropping rectangle should be specified via the sink pad crop selection. - the CSI hardware /2 downscaler should be specified via the sink pad compose selection. - the final source pad rectangle is the same as the sink pad compose rectangle. So that leaves only step 4 (source pad crop selection) as unsupported. Steve I have previously set up scaling like this: media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]" media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]" Does this mean, it should work like this instead? media-ctl --set-v4l2 "'ipu1_csi0_mux':2[fmt:UYVY2X8/1920x1080@1/60]" media-ctl --set-v4l2 "'ipu1_csi0':0[fmt:UYVY2X8/1920x1080@1/60,compose:(0,0)/960x540]" media-ctl --set-v4l2 "'ipu1_csi0':2[fmt:AYUV32/960x540@1/30]" I suppose setting the source pad format should not be allowed to modify the sink compose rectangle. regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2] vmbus: fix missed ring events on boot
> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Saturday, March 18, 2017 9:55 PM
> To: gre...@linuxfoundation.org; KY Srinivasan ;
> Haiyang Zhang
> Cc: sta...@vger.kernel.org; de...@linuxdriverproject.org; Stephen
> Hemminger
> Subject: [PATCH v2] vmbus: fix missed ring events on boot
>
> During initialization, the channel initialization code schedules the
> tasklet to scan the VMBUS receive event page (i.e. simulates an
> interrupt). The problem was that it invokes the tasklet on a different
> CPU from where it normally runs and therefore if an event is present,
> it will clear the bit but not find the associated channel.
>
> This can lead to missed events, typically stuck tasks, during bootup
> when sub channels are being initialized. Typically seen as stuck
> boot with 8 or more CPU's.
>
> This patch is not necessary for upstream (4.11 and later) since
> commit 631e63a9f346 ("vmbus: change to per channel tasklet").
> This changed vmbus code to get rid of common tasklet which
> caused the problem.
>
> Cc: sta...@vger.kernel.org
> Fixes: 638fea33aee8 ("Drivers: hv: vmbus: fix the race when querying &
> updating the percpu list")
> Signed-off-by: Stephen Hemminger
> ---
> v2 - simplified version (only need to change one function).
> also don't need to wait for tasklet to be scheduled on other CPU
> add Fixes tag.
>
> drivers/hv/channel_mgmt.c | 15 +--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 0af7e39006c8..63c903b00a58 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -361,8 +361,19 @@ void hv_event_tasklet_enable(struct
> vmbus_channel *channel)
> tasklet = hv_context.event_dpc[channel->target_cpu];
> tasklet_enable(tasklet);
>
> - /* In case there is any pending event */
> - tasklet_schedule(tasklet);
> + /*
> + * In case there is any pending event schedule a rescan
> + * but must be on the correct CPU for the channel.
> + */
> + if (channel->target_cpu == get_cpu()) {
> + put_cpu();
We could be preempted here and end up scheduling the taklet on the wrong CPU.
> + tasklet_schedule(tasklet);
> + } else {
> + smp_call_function_single(channel->target_cpu,
> + (smp_call_func_t)tasklet_schedule,
> + tasklet, false);
> + put_cpu();
> + }
Why not call put_cpu() at the end of the block to close the preemption window
we have
earlier.
K. Y
> }
>
> void hv_process_channel_removal(struct vmbus_channel *channel, u32
> relid)
> --
> 2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] staging: ad7606: Replace mlock with driver private lock
The IIO subsystem is redefining iio_dev->mlock to be used by
the IIO core only for protecting device operating mode changes.
ie. Changes between INDIO_DIRECT_MODE, INDIO_BUFFER_* modes.
In this driver, mlock was being used to protect hardware state
changes. Replace it with a lock in the devices global data.
Signed-off-by: Arushi Singhal
---
changes in v3
- Add mutex_init.
- correct the Documentation.
drivers/staging/iio/adc/ad7606.c | 9 +
drivers/staging/iio/adc/ad7606.h | 3 +++
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index 9dbfa64b1e53..18f5f139117e 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -208,7 +208,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_SCALE:
ret = -EINVAL;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
for (i = 0; i < ARRAY_SIZE(scale_avail); i++)
if (val2 == scale_avail[i][1]) {
gpiod_set_value(st->gpio_range, i);
@@ -217,7 +217,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
ret = 0;
break;
}
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return ret;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -231,11 +231,11 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
values[1] = (ret >> 1) & 1;
values[2] = (ret >> 2) & 1;
- mutex_lock(&indio_dev->mlock);
+ mutex_lock(&st->lock);
gpiod_set_array_value(ARRAY_SIZE(values), st->gpio_os->desc,
values);
st->oversampling = val;
- mutex_unlock(&indio_dev->mlock);
+ mutex_unlock(&st->lock);
return 0;
default:
@@ -413,6 +413,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem
*base_address,
st = iio_priv(indio_dev);
st->dev = dev;
+ mutex_init(&st->lock);
st->bops = bops;
st->base_address = base_address;
/* tied to logic low, analog input range is +/- 5V */
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index 746f9553d2ba..acaed8d5379c 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -14,6 +14,7 @@
* @name: identification string for chip
* @channels: channel specification
* @num_channels: number of channels
+ * @lock protect sensor state
*/
struct ad7606_chip_info {
@@ -23,6 +24,7 @@ struct ad7606_chip_info {
/**
* struct ad7606_state - driver instance specific data
+ * @lock protect sensor state
*/
struct ad7606_state {
@@ -37,6 +39,7 @@ struct ad7606_state {
booldone;
void __iomem*base_address;
+ struct mutexlock; /* protect sensor state */
struct gpio_desc*gpio_convst;
struct gpio_desc*gpio_reset;
struct gpio_desc*gpio_range;
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] vmbus: fix missed ring events on boot
During initialization, the channel initialization code schedules the
tasklet to scan the VMBUS receive event page (i.e. simulates an
interrupt). The problem was that it invokes the tasklet on a different
CPU from where it normally runs and therefore if an event is present,
it will clear the bit but not find the associated channel.
This can lead to missed events, typically stuck tasks, during bootup
when sub channels are being initialized. Typically seen as stuck
boot with 8 or more CPU's.
This patch is for 4.10 and 4.9; but was introduced by fix to solve
a race on updates. It is not necessary for 4.11 and later since
commit 631e63a9f346 ("vmbus: change to per channel tasklet").
which eliminated the common tasklet which caused the problem.
Cc: sta...@vger.kernel.org
Fixes: 638fea33aee8 ("Drivers: hv: vmbus: fix the race when querying & updating
the percpu list")
Signed-off-by: Stephen Hemminger
---
Patch is against linux-4.10.y branch
v3 - avoid possible preempt race
v2 - simplify only change hv_event_tasklet_enable
drivers/hv/channel_mgmt.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 0af7e39006c8..74afef6271f0 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -361,8 +361,17 @@ void hv_event_tasklet_enable(struct vmbus_channel *channel)
tasklet = hv_context.event_dpc[channel->target_cpu];
tasklet_enable(tasklet);
- /* In case there is any pending event */
- tasklet_schedule(tasklet);
+ /*
+* In case there is any pending event schedule a rescan
+* but must be on the correct CPU for the channel.
+*/
+ if (channel->target_cpu == get_cpu())
+ tasklet_schedule(tasklet);
+ else
+ smp_call_function_single(channel->target_cpu,
+(smp_call_func_t)tasklet_schedule,
+tasklet, false);
+ put_cpu();
}
void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/5] staging: vc04_services: camera driver maintance
On Mon, Mar 20, 2017 at 07:53:18AM -0700, Michael Zoran wrote: > On Mon, 2017-03-20 at 16:57 +0300, Dan Carpenter wrote: > > On Mon, Mar 20, 2017 at 04:06:00AM -0700, Michael Zoran wrote: > > > On Mon, 2017-03-20 at 13:55 +0300, Dan Carpenter wrote: > > > > I'm not going to review this because it has kbuild errors. > > > > > > > > regards, > > > > dan carpenter > > > > > > > > > > Hi, can you e-mail out the errors or send them to me. It worked > > > when > > > I submitted it. > > > > > > > The problem is that you added a function only for ARM but the camera > > can build on i386. > > > > Anyway, we need to figure out why you aren't getting the kbuild > > emails > > and fix that. I forwarded the first email to you. The other is > > basically the same. > > > > regards, > > dan carpenter > > > > OK, thanks for sending it to me. > > I don't quite understand how that is possible in all since the whole > subtree depends on RASPBERRYPI_FIRMWARE which I had nothing to do with > setting up. Sounds to me like the real issue here is that the > RASPBERRYPI_FIRMARE driver wasn't setup correctly in the first place. It depends RASPBERRYPI_FIRMARE or COMPILE_TEST. The kbuild bot has COMPILE_TEST enabled so that solves the mystery of why the build failed. > I also noticed when I was trying to understand how the raspberrypi > driver works in the first place in that it doesn't always dump the > firmware revision correctly if the DT node mbox handle gets pointed to > who knows where. > > I've been complaining about very selective e-mails on so called public > e-mail lists for a very long time now, and nobody has done anything > about it. I have no idea what your talking about but I try to forget emails right away so that I'll be able to answer honestly that, "I don't recall" in case I am ever asked to testify in front of the United States senate. > Sometimes I don't get any e-mail for a week and then I'll get > a flood of e-mail thats 2 weeks old. And who knows which e-mails lists > changes are going to. This business with multiple lists *is* very annoying. Patches to do with kernel code should be sent to driver-devel. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt
On Mon, Mar 20, 2017 at 06:23:24PM +0100, Philipp Zabel wrote:
> --8<--
> >From 2830aebc404bdfc9d7fc1ec94e5282d0b668e8f6 Mon Sep 17 00:00:00 2001
> From: Philipp Zabel
> Date: Mon, 20 Mar 2017 17:10:21 +0100
> Subject: [PATCH] media: imx: csi: add sink selection rectangles
>
> Move the crop rectangle to the sink pad and add a sink compose rectangle
> to configure scaling. Also propagate rectangles from sink pad to crop
> rectangle, to compose rectangle, and to the source pads both in ACTIVE
> and TRY variants of set_fmt/selection, and initialize the default crop
> and compose rectangles.
Looks fine for the most part.
> - /*
> - * Modifying the crop rectangle always changes the format on the source
> - * pad. If the KEEP_CONFIG flag is set, just return the current crop
> - * rectangle.
> - */
> - if (sel->flags & V4L2_SEL_FLAG_KEEP_CONFIG) {
> - sel->r = priv->crop;
> - if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
> - cfg->try_crop = sel->r;
> + infmt = __csi_get_fmt(priv, cfg, CSI_SINK_PAD, sel->which);
> + crop = __csi_get_crop(priv, cfg, sel->which);
> + compose = __csi_get_compose(priv, cfg, sel->which);
> +
> + switch (sel->target) {
> + case V4L2_SEL_TGT_CROP:
> + /*
> + * Modifying the crop rectangle always changes the format on
> + * the source pads. If the KEEP_CONFIG flag is set, just return
> + * the current crop rectangle.
> + */
> + if (sel->flags & V4L2_SEL_FLAG_KEEP_CONFIG) {
> + sel->r = priv->crop;
My understanding of KEEP_CONFIG is that the only thing we're not
allowed to do is to propagate the change downstream.
Since downstream of the crop is the compose, that means the only
restriction here is that the width and height of the crop window must
be either equal to the compose width/height, or double the compose
width/height. (Anything else would necessitate the compose window
changing.)
However, the crop window can move position within the crop bounds,
provided it's entirely contained within those crop bounds.
The problem is that this becomes rather more complex it deal with
(as I'm finding out in my imx219 camera driver) and I'm thinking
that some of this complexity should probably be in a helper in
generic v4l2 code.
I don't know whether this applies (I hope it doesn't) but there's a
pile of guidelines in Documentation/media/uapi/v4l/vidioc-g-selection.rst
which describe how a crop/compose rectangle should be adjusted. As
I say, I hope they don't apply, because if they do, we would _really_
need helpers for this stuff, as I don't think having each driver
implement all these rules would be too successful!
> + if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
> + *crop = sel->r;
> + goto out;
> + }
> +
> + csi_try_crop(priv, &sel->r, cfg, infmt, sensor);
> +
> + *crop = sel->r;
> +
> + /* Reset scaling to 1:1 */
> + compose->width = crop->width;
> + compose->height = crop->height;
> + break;
> + case V4L2_SEL_TGT_COMPOSE:
> + /*
> + * Modifying the compose rectangle always changes the format on
> + * the source pads. If the KEEP_CONFIG flag is set, just return
> + * the current compose rectangle.
> + */
> + if (sel->flags & V4L2_SEL_FLAG_KEEP_CONFIG) {
> + sel->r = priv->compose;
I think, with my understanding of how the KEEP_CONFIG flag works, this
should be:
sel->r = *compose;
because if we change the compose rectangle width/height, we would need
to propagate this to the source pad, and the KEEP_CONFIG description
says we're not allowed to do that.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] HV: properly delay KVP packets when negotiation is in progress
> -Original Message-
> From: Long Li
> Sent: Sunday, March 19, 2017 7:49 PM
> To: 'Vitaly Kuznetsov'
> Cc: KY Srinivasan ; Haiyang Zhang
> ; Stephen Hemminger
> ; de...@linuxdriverproject.org; linux-
> ker...@vger.kernel.org
> Subject: RE: [PATCH] HV: properly delay KVP packets when negotiation is in
> progress
>
>
>
> > -Original Message-
> > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> > Sent: Friday, March 17, 2017 9:16 AM
> > To: Long Li
> > Cc: KY Srinivasan ; Haiyang Zhang
> > ; Stephen Hemminger
> ;
> > de...@linuxdriverproject.org; linux- ker...@vger.kernel.org
> > Subject: Re: [PATCH] HV: properly delay KVP packets when negotiation
> > is in progress
> >
> > Long Li writes:
> >
> > > The host may send multiple KVP packets before the negotiation with
> > > daemon is finished. We need to keep those packets in ring buffer
> > > until the daemon is negotiated and connected.
> >
> > The patch looks OK but previously we always presumed that this can't
> > happen for util drivers and host will never send a new request before
> > we answer to the previous one. If this is not true we may have more
> > issues which need fixing as all three drivers we have are written in a
> 'transaction'
> > fashion.
> >
> > So my question would be: can the host send multiple (KVP) packets
> > _after_ the negotiation with daemon is finished?
>
> Thanks Vitaly. I'm checking with Windows guys and will update soon.
It's possible that hosts may send a number of staged KVP requests as soon as
negotiation is done. The current KVP code can deal with a number of pending KVP
requests, and respond to them one by one.
To summarize the issue this patch tries to fix:
1. When host sends a negotiation request, and it times out, the host will send
another negotiation request, and so on.
2. The guest can respond to all negotiation requests from the host. All
subsequent response (except for the 1st response) are ignored by the host.
3. Before negotiation is done, the host may have staged a number of pending KVP
requests.
4. As soon as negotiation is done, the host sends all KVP requests to guest.
There is a corner case that if there is only one pending KVP request after the
2nd (or 3rd etc) negotiation, it may get lost. I'm testing the following code
to address this condition:
@@ -705,6 +706,7 @@ void hv_kvp_onchannelcallback(void *context)
VM_PKT_DATA_INBAND, 0);
host_negotiatied = NEGO_FINISHED;
+ hv_poll_channel(kvp_transaction.recv_channel, kvp_poll_wrapper);
}
}
Please drop this patch. I'll send V2.
>
> >
> >
> > >
> > > This patch is based on the work of Nick Meier
> > >
> > >
> > > Signed-off-by: Long Li
> > > ---
> > > drivers/hv/hv_kvp.c | 9 +
> > > 1 file changed, 5 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c index
> > > de26371..b9f928d 100644
> > > --- a/drivers/hv/hv_kvp.c
> > > +++ b/drivers/hv/hv_kvp.c
> > > @@ -628,16 +628,17 @@ void hv_kvp_onchannelcallback(void *context)
> > >NEGO_IN_PROGRESS,
> > >NEGO_FINISHED} host_negotiatied =
> > NEGO_NOT_STARTED;
> > >
> > > - if (host_negotiatied == NEGO_NOT_STARTED &&
> > > - kvp_transaction.state < HVUTIL_READY) {
> > > + if (kvp_transaction.state < HVUTIL_READY) {
> > > /*
> > >* If userspace daemon is not connected and host is asking
> > >* us to negotiate we need to delay to not lose messages.
> > >* This is important for Failover IP setting.
> > >*/
> > > - host_negotiatied = NEGO_IN_PROGRESS;
> > > - schedule_delayed_work(&kvp_host_handshake_work,
> > > + if (host_negotiatied == NEGO_NOT_STARTED) {
> > > + host_negotiatied = NEGO_IN_PROGRESS;
> > > +
> > schedule_delayed_work(&kvp_host_handshake_work,
> > > HV_UTIL_NEGO_TIMEOUT * HZ);
> > > + }
> > > return;
> > > }
> > > if (kvp_transaction.state > HVUTIL_READY)
> >
> > --
> > Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()
2017-03-20 22:11 GMT+09:00 walter harms :
>
>
> Am 20.03.2017 13:51, schrieb DaeSeok Youn:
>> 2017-03-20 21:04 GMT+09:00 walter harms :
>>>
>>>
>>> Am 20.03.2017 11:59, schrieb Daeseok Youn:
If v4l2_subdev_call() gets the global frame interval values,
it returned 0 and it could be checked whether numerator is zero or not.
If the numerator is not zero, the fps could be calculated in this function.
If not, it just returns 0.
Signed-off-by: Daeseok Youn
---
.../media/atomisp/pci/atomisp2/atomisp_cmd.c | 22
++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 8bdb224..6bdd19e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct
video_device *dev)
static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device
*asd)
{
- struct v4l2_subdev_frame_interval frame_interval;
+ struct v4l2_subdev_frame_interval fi;
struct atomisp_device *isp = asd->isp;
- unsigned short fps;
- if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
- video, g_frame_interval, &frame_interval)) {
- fps = 0;
- } else {
- if (frame_interval.interval.numerator)
- fps = frame_interval.interval.denominator /
- frame_interval.interval.numerator;
- else
- fps = 0;
- }
+ unsigned short fps = 0;
+ int ret;
+
+ ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
+video, g_frame_interval, &fi);
+
+ if (!ret && fi.interval.numerator)
+ fps = fi.interval.denominator / fi.interval.numerator;
+
return fps;
}
>>>
>>>
>>>
>>> do you need to check ret at all ? if an error occurs can
>>> fi.interval.numerator
>>> be something else than 0 ?
>> the return value from the v4l2_subdev_call() function is zero when it
>> is done without any error. and also I checked
>> the ret value whether is 0 or not. if the ret is 0 then the value of
>> numerator should be checked to avoid for dividing by 0.
>>>
>>> if ret is an ERRNO it would be wise to return ret not fps, but this may
>>> require
>>> changes at other places also.
>> hmm.., yes, you are right. but I think it is ok because the
>> atomisp_get_sensor_fps() function is needed to get fps value.
>> (originally, zero or calculated fps value was returned.)
>
> maybe its better to divide this in:
> if (ret)
>return 0; // error case
>
> return (fi.interval.numerator>0)?fi.interval.denominator /
> fi.interval.numerator:0;
>
> So there is a chance that someone will a) understand and b) fix the error
> return.
yes, it looks better than mine. I will update it and resend it.
Thanks walter,
Regards,
Daeseok Youn.
>
> re,
> wh
>
>>
>>>
>>> re,
>>> wh
>>>
>>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4 V2] staging: atomisp: remove else statement after return
It doesn't need to have else statement after return.
Signed-off-by: Daeseok Youn
---
V2: one(2/4) of this series was updated so I tried to send them again.
drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index d97a8df..8bdb224 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -2958,11 +2958,11 @@ int atomisp_get_metadata(struct atomisp_sub_device
*asd, int flag,
dev_err(isp->dev, "copy to user failed: copied %d bytes\n",
ret);
return -EFAULT;
- } else {
- list_del_init(&md_buf->list);
- list_add_tail(&md_buf->list, &asd->metadata[md_type]);
}
+ list_del_init(&md_buf->list);
+ list_add_tail(&md_buf->list, &asd->metadata[md_type]);
+
dev_dbg(isp->dev, "%s: HAL de-queued metadata type %d with exp_id %d\n",
__func__, md_type, md->exp_id);
return 0;
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
