Re: [PATCH][resend] media: ti-vpe: cal: use of_graph_get_remote_endpoint()
Hi Hans > > From: Kuninori Morimoto > > > > Now, we can use of_graph_get_remote_endpoint(). Let's use it. > > I'm not sure why this is resent. It's part of a pending pull request > so I expect it to be merged this week. Sorry, I didn't know that. Thank you for your help Best regards --- Kuninori Morimoto
Re: [PATCH] keytable: ensure udev rule fires on rc input device
Hi Sean! On Sun, Aug 06, 2017 at 09:56:55AM +0100, Sean Young wrote: > The rc device is created before the input device, so if ir-keytable runs > too early the input device does not exist yet. > > Ensure that rule fires on creation of a rc device's input device. > > Note that this also prevents udev from starting ir-keytable on an > transmit only device, which has no input device. > > Signed-off-by: Sean Young Signed-off-by: Matthias Reichl One comment though, see below > --- > utils/keytable/70-infrared.rules | 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) > > Matthias, can I have your Signed-off-by please? Thank you. > > > diff --git a/utils/keytable/70-infrared.rules > b/utils/keytable/70-infrared.rules > index afffd951..b3531727 100644 > --- a/utils/keytable/70-infrared.rules > +++ b/utils/keytable/70-infrared.rules > @@ -1,4 +1,12 @@ > # Automatically load the proper keymaps after the Remote Controller device > # creation. The keycode tables rules should be at /etc/rc_maps.cfg > > -ACTION=="add", SUBSYSTEM=="rc", RUN+="/usr/bin/ir-keytable -a > /etc/rc_maps.cfg -s $name" > +ACTION!="add", SUBSYSTEMS!="rc", GOTO="rc_dev_end" This line doesn't quite what we want it to do. As SUBSYSTEMS!="rc" is basically a no-op and would only be evaluated on change/remove events anyways that line boils down to ACTION!="add", GOTO="rc_dev_end" and the following rules are evaluated on all add events. While that'll still work it'll do unnecessary work, like importing rc_sydev for all input devices and could bite us (or users) later if we change/extend the ruleset. Better do it like in my original comment (using positive logic and a GOTO="begin") or use ACTION!="add", GOTO="rc_dev_end" and add SUBSYSTEMS=="rc" to the IMPORT and RUN rules below. so long, Hias > + > +SUBSYSTEM=="rc", ENV{rc_sysdev}="$name" > + > +SUBSYSTEM=="input", IMPORT{parent}="rc_sysdev" > + > +KERNEL=="event[0-9]*", ENV{rc_sysdev}=="?*", RUN+="/usr/bin/ir-keytable -a > /etc/rc_maps.cfg -s $env{rc_sysdev}" > + > +LABEL="rc_dev_end" > -- > 2.11.0 >
Re: [PATCH] media: i2c: OV5647: gate clock lane before stream on
Hi all, 2017-07-25 10:34 GMT+08:00 Jacob Chen : > According to datasheet, BIT5 in reg-0x4800 are used to > enable/disable clock lane gate. > > It's wrong to make clock lane free running before > sensor stream on was called, while the mipi phy > are not initialized. > > Signed-off-by: Jacob Chen > --- > drivers/media/i2c/ov5647.c | 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c > index 95ce90f..d3e6fd0 100644 > --- a/drivers/media/i2c/ov5647.c > +++ b/drivers/media/i2c/ov5647.c > @@ -253,6 +253,10 @@ static int ov5647_stream_on(struct v4l2_subdev *sd) > { > int ret; > > + ret = ov5647_write(sd, 0x4800, 0x04); > + if (ret < 0) > + return ret; > + > ret = ov5647_write(sd, 0x4202, 0x00); > if (ret < 0) > return ret; > @@ -264,6 +268,10 @@ static int ov5647_stream_off(struct v4l2_subdev *sd) > { > int ret; > > + ret = ov5647_write(sd, 0x4800, 0x25); > + if (ret < 0) > + return ret; > + > ret = ov5647_write(sd, 0x4202, 0x0f); > if (ret < 0) > return ret; > @@ -320,7 +328,7 @@ static int __sensor_init(struct v4l2_subdev *sd) > return ret; > } > > - return ov5647_write(sd, 0x4800, 0x04); > + return ov5647_stream_off(sd); > } > > static int ov5647_sensor_power(struct v4l2_subdev *sd, int on) > -- > 2.7.4 > Can anyone comment on it? I saw there is a same discussion in https://patchwork.kernel.org/patch/9569031/ There is a comment in i.MX CSI2 driver. " Configure MIPI Camera Sensor to put all Tx lanes in LP-11 state. This must be carried out by the MIPI sensor's s_power(ON) subdev op. " That's what this patch do, sensor driver should make sure that clock lanes are in stop state while not streaming.
[PATCHv2] cec-api: log the reason for the -EINVAL in cec_s_mode
If cec_debug >= 1 then log why the requested mode returned -EINVAL. It can be hard to debug this since -EINVAL can be returned for many reasons. So this should help. Signed-off-by: Hans Verkuil --- Changes since v1: Removed some error injection code that inadvertently ended up in the patch I posted. --- drivers/media/cec/cec-api.c | 25 +++-- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/drivers/media/cec/cec-api.c b/drivers/media/cec/cec-api.c index 8dd16e263452..00d43d74020f 100644 --- a/drivers/media/cec/cec-api.c +++ b/drivers/media/cec/cec-api.c @@ -357,34 +357,47 @@ static long cec_s_mode(struct cec_adapter *adap, struct cec_fh *fh, if (copy_from_user(&mode, parg, sizeof(mode))) return -EFAULT; - if (mode & ~(CEC_MODE_INITIATOR_MSK | CEC_MODE_FOLLOWER_MSK)) + if (mode & ~(CEC_MODE_INITIATOR_MSK | CEC_MODE_FOLLOWER_MSK)) { + dprintk(1, "%s: invalid mode bits set\n", __func__); return -EINVAL; + } mode_initiator = mode & CEC_MODE_INITIATOR_MSK; mode_follower = mode & CEC_MODE_FOLLOWER_MSK; if (mode_initiator > CEC_MODE_EXCL_INITIATOR || - mode_follower > CEC_MODE_MONITOR_ALL) + mode_follower > CEC_MODE_MONITOR_ALL) { + dprintk(1, "%s: unknown mode\n", __func__); return -EINVAL; + } if (mode_follower == CEC_MODE_MONITOR_ALL && - !(adap->capabilities & CEC_CAP_MONITOR_ALL)) + !(adap->capabilities & CEC_CAP_MONITOR_ALL)) { + dprintk(1, "%s: MONITOR_ALL not supported\n", __func__); return -EINVAL; + } if (mode_follower == CEC_MODE_MONITOR_PIN && - !(adap->capabilities & CEC_CAP_MONITOR_PIN)) + !(adap->capabilities & CEC_CAP_MONITOR_PIN)) { + dprintk(1, "%s: MONITOR_PIN not supported\n", __func__); return -EINVAL; + } /* Follower modes should always be able to send CEC messages */ if ((mode_initiator == CEC_MODE_NO_INITIATOR || !(adap->capabilities & CEC_CAP_TRANSMIT)) && mode_follower >= CEC_MODE_FOLLOWER && - mode_follower <= CEC_MODE_EXCL_FOLLOWER_PASSTHRU) + mode_follower <= CEC_MODE_EXCL_FOLLOWER_PASSTHRU) { + dprintk(1, "%s: cannot transmit\n", __func__); return -EINVAL; + } /* Monitor modes require CEC_MODE_NO_INITIATOR */ - if (mode_initiator && mode_follower >= CEC_MODE_MONITOR_PIN) + if (mode_initiator && mode_follower >= CEC_MODE_MONITOR_PIN) { + dprintk(1, "%s: monitor modes require NO_INITIATOR\n", + __func__); return -EINVAL; + } /* Monitor modes require CAP_NET_ADMIN */ if (mode_follower >= CEC_MODE_MONITOR_PIN && !capable(CAP_NET_ADMIN)) -- 2.13.2
[GIT PULL FOR v4.14] cec: little improvements
Better logging, better documentation, add a convenience define. Regards, Hans The following changes since commit da48c948c263c9d87dfc64566b3373a858cc8aa2: media: fix warning on v4l2_subdev_call() result interpreted as bool (2017-07-26 13:43:17 -0400) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v4.14f for you to fetch changes up to b52870ca523e37c424f0267694101905b6256bb8: cec-api: log the reason for the -EINVAL in cec_s_mode (2017-08-07 09:31:15 +0200) Hans Verkuil (6): cec-funcs.h: cec_ops_report_features: set *dev_features to NULL media/cec.h: add CEC_CAP_DEFAULTS adv*/vivid/pulse8/rainshadow: cec: use CEC_CAP_DEFAULTS cec-ioc-adap-g-log-addrs.rst: fix wrong quotes cec-ioc-g-mode.rst: improve description of message, processing cec-api: log the reason for the -EINVAL in cec_s_mode Documentation/media/uapi/cec/cec-ioc-adap-g-log-addrs.rst | 2 +- Documentation/media/uapi/cec/cec-ioc-g-mode.rst | 61 -- drivers/media/cec/cec-api.c | 25 ++- drivers/media/i2c/adv7511.c | 3 +-- drivers/media/i2c/adv7604.c | 3 +-- drivers/media/i2c/adv7842.c | 3 +-- drivers/media/platform/vivid/vivid-cec.c | 3 +-- drivers/media/usb/pulse8-cec/pulse8-cec.c | 3 +-- drivers/media/usb/rainshadow-cec/rainshadow-cec.c | 3 +-- include/media/cec.h | 3 +++ include/uapi/linux/cec-funcs.h| 1 + 11 files changed, 67 insertions(+), 43 deletions(-)
Re: [PATCH] media: i2c: OV5647: gate clock lane before stream on
Hi Jacob, On Mon, 2017-08-07 at 15:11 +0800, Jacob Chen wrote: > Hi all, > > 2017-07-25 10:34 GMT+08:00 Jacob Chen : > > According to datasheet, BIT5 in reg-0x4800 are used to > > enable/disable clock lane gate. > > > > It's wrong to make clock lane free running before > > sensor stream on was called, while the mipi phy > > are not initialized. > > > > Signed-off-by: Jacob Chen >> > > --- > > drivers/media/i2c/ov5647.c | 10 +- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c > > index 95ce90f..d3e6fd0 100644 > > --- a/drivers/media/i2c/ov5647.c > > +++ b/drivers/media/i2c/ov5647.c > > @@ -253,6 +253,10 @@ static int ov5647_stream_on(struct v4l2_subdev *sd) > > { > > int ret; > > > > + ret = ov5647_write(sd, 0x4800, 0x04); > > + if (ret < 0) > > + return ret; > > + > > ret = ov5647_write(sd, 0x4202, 0x00); > > if (ret < 0) > > return ret; > > @@ -264,6 +268,10 @@ static int ov5647_stream_off(struct v4l2_subdev *sd) > > { > > int ret; > > > > + ret = ov5647_write(sd, 0x4800, 0x25); > > + if (ret < 0) > > + return ret; > > + > > ret = ov5647_write(sd, 0x4202, 0x0f); > > if (ret < 0) > > return ret; > > @@ -320,7 +328,7 @@ static int __sensor_init(struct v4l2_subdev *sd) > > return ret; > > } > > > > - return ov5647_write(sd, 0x4800, 0x04); > > + return ov5647_stream_off(sd); > > } > > > > static int ov5647_sensor_power(struct v4l2_subdev *sd, int on) > > -- > > 2.7.4 > > > > Can anyone comment on it? > > I saw there is a same discussion in > https://patchwork.kernel.org/patch/9569031/ > There is a comment in i.MX CSI2 driver. > " > Configure MIPI Camera Sensor to put all Tx lanes in LP-11 state. > This must be carried out by the MIPI sensor's s_power(ON) subdev > op. > " > That's what this patch do, sensor driver should make sure that clock > lanes are in stop state while not streaming. This is not the same, as far as I can tell. BIT(5) is just clock lane gating, as you describe above. To put the bus into LP-11 state, BIT(2) needs to be set. regards Philipp
Re: [PATCH 06/12] [media] exynos-gsc: constify v4l2_m2m_ops structures
On 08/06/2017 10:25 AM, Julia Lawall wrote: The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall Reviewed-by: Sylwester Nawrocki
Re: [PATCH 11/12] [media] exynos4-is: constify v4l2_m2m_ops structures
On 08/06/2017 10:25 AM, Julia Lawall wrote: The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall Reviewed-by: Sylwester Nawrocki
Re: [PATCH 03/12] [media] s5p-g2d: constify v4l2_m2m_ops structures
On 08/06/2017 10:25 AM, Julia Lawall wrote: The v4l2_m2m_ops structures are only passed as the only argument to v4l2_m2m_init, which is declared as const. Thus the v4l2_m2m_ops structures themselves can be const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall Reviewed-by: Sylwester Nawrocki
[PATCH] [media] staging/imx: always select VIDEOBUF2_DMA_CONTIG
I ran into a rare build error during randconfig testing: drivers/staging/media/imx/imx-media-capture.o: In function `capture_stop_streaming': imx-media-capture.c:(.text+0x224): undefined reference to `vb2_buffer_done' drivers/staging/media/imx/imx-media-capture.o: In function `imx_media_capture_device_register': imx-media-capture.c:(.text+0xe60): undefined reference to `vb2_queue_init' imx-media-capture.c:(.text+0xfa0): undefined reference to `vb2_dma_contig_memops' While VIDEOBUF2_DMA_CONTIG was already selected by the camera driver, it wasn't necessarily there with just the base driver enabled. This moves the 'select' statement to the top-level option to make sure it's always available. Fixes: 64b5a49df486 ("[media] media: imx: Add Capture Device Interface") Signed-off-by: Arnd Bergmann --- drivers/staging/media/imx/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/imx/Kconfig b/drivers/staging/media/imx/Kconfig index 719508fcb0e9..2be921cd0d55 100644 --- a/drivers/staging/media/imx/Kconfig +++ b/drivers/staging/media/imx/Kconfig @@ -2,6 +2,7 @@ config VIDEO_IMX_MEDIA tristate "i.MX5/6 V4L2 media core driver" depends on MEDIA_CONTROLLER && VIDEO_V4L2 && ARCH_MXC && IMX_IPUV3_CORE depends on VIDEO_V4L2_SUBDEV_API + select VIDEOBUF2_DMA_CONTIG select V4L2_FWNODE ---help--- Say yes here to enable support for video4linux media controller @@ -13,7 +14,6 @@ menu "i.MX5/6 Media Sub devices" config VIDEO_IMX_CSI tristate "i.MX5/6 Camera Sensor Interface driver" depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C - select VIDEOBUF2_DMA_CONTIG default y ---help--- A video4linux camera sensor interface driver for i.MX5/6. -- 2.9.0
[PATCH] [media] staging: atomisp: imx: remove dead code
Making some functions 'static' has uncovered a few functions that have no caller, through the gcc warnings: atomisp/i2c/imx/imx.c::12: error: 'imx_t_focus_vcm' defined but not used [-Werror=unused-function] atomisp/i2c/imx/imx.c:1103:12: error: 'imx_vcm_init' defined but not used [-Werror=unused-function] atomisp/i2c/imx/imx.c:1095:12: error: 'imx_vcm_power_down' defined but not used [-Werror=unused-function] atomisp/i2c/imx/imx.c:1087:12: error: 'imx_vcm_power_up' defined but not used [-Werror=unused-function] All four of these can be removed. Since they call indirect functions, I also looked at how those are used in turn: - The power_up/power_down callbacks are called from other functions and are still needed. - The t_focus_vcm callbacks pointers are completely unused and can be removed in both imx and ov8858. Some of the handlers are called directly and can now be marked static, the others are dummy implemntations that we can remove. - vcm_init is unused in imx, but dw9718_vcm_init is used in ov8858, but is not used in imx, so that one needs to stay around. The callback pointers in imx can be removed. Fixes: 9a5a6911aa3f ("staging: imx: fix non-static declarations") Signed-off-by: Arnd Bergmann --- drivers/staging/media/atomisp/i2c/imx/ad5816g.c | 11 +--- drivers/staging/media/atomisp/i2c/imx/drv201.c| 11 +--- drivers/staging/media/atomisp/i2c/imx/dw9714.c| 14 +- drivers/staging/media/atomisp/i2c/imx/dw9718.c| 5 drivers/staging/media/atomisp/i2c/imx/dw9719.c| 11 drivers/staging/media/atomisp/i2c/imx/imx.c | 32 --- drivers/staging/media/atomisp/i2c/imx/imx.h | 29 drivers/staging/media/atomisp/i2c/ov5693/ov5693.c | 2 +- drivers/staging/media/atomisp/i2c/ov8858.h| 3 --- drivers/staging/media/atomisp/i2c/ov8858_btns.h | 3 --- 10 files changed, 4 insertions(+), 117 deletions(-) diff --git a/drivers/staging/media/atomisp/i2c/imx/ad5816g.c b/drivers/staging/media/atomisp/i2c/imx/ad5816g.c index d68ebb49f002..558dcdf135d9 100644 --- a/drivers/staging/media/atomisp/i2c/imx/ad5816g.c +++ b/drivers/staging/media/atomisp/i2c/imx/ad5816g.c @@ -136,7 +136,7 @@ int ad5816g_vcm_power_down(struct v4l2_subdev *sd) } -int ad5816g_t_focus_vcm(struct v4l2_subdev *sd, u16 val) +static int ad5816g_t_focus_vcm(struct v4l2_subdev *sd, u16 val) { struct i2c_client *client = v4l2_get_subdevdata(sd); u16 data = val & VCM_CODE_MASK; @@ -214,12 +214,3 @@ int ad5816g_t_vcm_timing(struct v4l2_subdev *sd, s32 value) { return 0; } - -int ad5816g_vcm_init(struct v4l2_subdev *sd) -{ - ad5816g_dev.platform_data = camera_get_af_platform_data(); - return (NULL == ad5816g_dev.platform_data) ? -ENODEV : 0; - -} - - diff --git a/drivers/staging/media/atomisp/i2c/imx/drv201.c b/drivers/staging/media/atomisp/i2c/imx/drv201.c index 915e4019cfeb..6d9d4c968722 100644 --- a/drivers/staging/media/atomisp/i2c/imx/drv201.c +++ b/drivers/staging/media/atomisp/i2c/imx/drv201.c @@ -128,7 +128,7 @@ int drv201_vcm_power_down(struct v4l2_subdev *sd) } -int drv201_t_focus_vcm(struct v4l2_subdev *sd, u16 val) +static int drv201_t_focus_vcm(struct v4l2_subdev *sd, u16 val) { struct i2c_client *client = v4l2_get_subdevdata(sd); u16 data = val & VCM_CODE_MASK; @@ -207,12 +207,3 @@ int drv201_t_vcm_timing(struct v4l2_subdev *sd, s32 value) { return 0; } - -int drv201_vcm_init(struct v4l2_subdev *sd) -{ - drv201_dev.platform_data = camera_get_af_platform_data(); - return (NULL == drv201_dev.platform_data) ? -ENODEV : 0; -} - - - diff --git a/drivers/staging/media/atomisp/i2c/imx/dw9714.c b/drivers/staging/media/atomisp/i2c/imx/dw9714.c index b7dee1b6bb37..6397a7ee0af6 100644 --- a/drivers/staging/media/atomisp/i2c/imx/dw9714.c +++ b/drivers/staging/media/atomisp/i2c/imx/dw9714.c @@ -56,7 +56,7 @@ int dw9714_vcm_power_down(struct v4l2_subdev *sd) } -int dw9714_t_focus_vcm(struct v4l2_subdev *sd, u16 val) +static int dw9714_t_focus_vcm(struct v4l2_subdev *sd, u16 val) { struct i2c_client *client = v4l2_get_subdevdata(sd); int ret = -EINVAL; @@ -221,15 +221,3 @@ int dw9714_t_vcm_timing(struct v4l2_subdev *sd, s32 value) return 0; } - -int dw9714_vcm_init(struct v4l2_subdev *sd) -{ - - /* set VCM to home position and vcm mode to direct*/ - dw9714_dev.vcm_mode = DW9714_DIRECT; - dw9714_dev.vcm_settings.update = false; - dw9714_dev.platform_data = camera_get_af_platform_data(); - return (NULL == dw9714_dev.platform_data) ? -ENODEV : 0; - -} - diff --git a/drivers/staging/media/atomisp/i2c/imx/dw9718.c b/drivers/staging/media/atomisp/i2c/imx/dw9718.c index 65a1fcf187d5..c02b9f0a2440 100644 --- a/drivers/staging/media/atomisp/i2c/imx/dw9718.c +++ b/drivers/staging/media/atomisp/i2c/imx/dw9718.c @@ -204,11 +204,6 @@ int dw9718_q_focus_status(struct v4l2_subdev *sd, s32 *
Re: [PATCHv3 3/4] drm/bridge: dw-hdmi: add cec driver
On 08/03/2017 12:11 AM, Hans Verkuil wrote: From: Russell King Add a CEC driver for the dw-hdmi hardware. Queued to drm-misc-next after fixing up some minor Makefile/Kconfig conflicts. Thanks, Archit Reviewed-by: Neil Armstrong Signed-off-by: Russell King [hans.verkuil: unsigned -> unsigned int] [hans.verkuil: cec_transmit_done -> cec_transmit_attempt_done] [hans.verkuil: add missing CEC_CAP_PASSTHROUGH] Acked-by: Hans Verkuil Tested-by: Hans Verkuil Tested-by: Laurent Pinchart --- drivers/gpu/drm/bridge/synopsys/Kconfig | 9 + drivers/gpu/drm/bridge/synopsys/Makefile | 1 + drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 327 ++ drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h | 19 ++ drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 42 +++- drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 1 + 6 files changed, 398 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c create mode 100644 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h diff --git a/drivers/gpu/drm/bridge/synopsys/Kconfig b/drivers/gpu/drm/bridge/synopsys/Kconfig index 351db000599a..d34c3dc04ba9 100644 --- a/drivers/gpu/drm/bridge/synopsys/Kconfig +++ b/drivers/gpu/drm/bridge/synopsys/Kconfig @@ -23,3 +23,12 @@ config DRM_DW_HDMI_I2S_AUDIO help Support the I2S Audio interface which is part of the Synopsys Designware HDMI block. + +config DRM_DW_HDMI_CEC + tristate "Synopsis Designware CEC interface" + depends on DRM_DW_HDMI + select CEC_CORE + select CEC_NOTIFIER + help + Support the CE interface which is part of the Synopsys + Designware HDMI block. diff --git a/drivers/gpu/drm/bridge/synopsys/Makefile b/drivers/gpu/drm/bridge/synopsys/Makefile index 17aa7a65b57e..6fe415903668 100644 --- a/drivers/gpu/drm/bridge/synopsys/Makefile +++ b/drivers/gpu/drm/bridge/synopsys/Makefile @@ -3,3 +3,4 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o obj-$(CONFIG_DRM_DW_HDMI_I2S_AUDIO) += dw-hdmi-i2s-audio.o +obj-$(CONFIG_DRM_DW_HDMI_CEC) += dw-hdmi-cec.o diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c new file mode 100644 index ..6c323510f128 --- /dev/null +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c @@ -0,0 +1,327 @@ +/* + * Designware HDMI CEC driver + * + * Copyright (C) 2015-2017 Russell King. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#include +#include +#include +#include +#include +#include + +#include + +#include +#include + +#include "dw-hdmi-cec.h" + +enum { + HDMI_IH_CEC_STAT0 = 0x0106, + HDMI_IH_MUTE_CEC_STAT0 = 0x0186, + + HDMI_CEC_CTRL = 0x7d00, + CEC_CTRL_START = BIT(0), + CEC_CTRL_FRAME_TYP = 3 << 1, + CEC_CTRL_RETRY = 0 << 1, + CEC_CTRL_NORMAL = 1 << 1, + CEC_CTRL_IMMED = 2 << 1, + + HDMI_CEC_STAT = 0x7d01, + CEC_STAT_DONE = BIT(0), + CEC_STAT_EOM= BIT(1), + CEC_STAT_NACK = BIT(2), + CEC_STAT_ARBLOST= BIT(3), + CEC_STAT_ERROR_INIT = BIT(4), + CEC_STAT_ERROR_FOLL = BIT(5), + CEC_STAT_WAKEUP = BIT(6), + + HDMI_CEC_MASK = 0x7d02, + HDMI_CEC_POLARITY = 0x7d03, + HDMI_CEC_INT= 0x7d04, + HDMI_CEC_ADDR_L = 0x7d05, + HDMI_CEC_ADDR_H = 0x7d06, + HDMI_CEC_TX_CNT = 0x7d07, + HDMI_CEC_RX_CNT = 0x7d08, + HDMI_CEC_TX_DATA0 = 0x7d10, + HDMI_CEC_RX_DATA0 = 0x7d20, + HDMI_CEC_LOCK = 0x7d30, + HDMI_CEC_WKUPCTRL = 0x7d31, +}; + +struct dw_hdmi_cec { + struct dw_hdmi *hdmi; + const struct dw_hdmi_cec_ops *ops; + u32 addresses; + struct cec_adapter *adap; + struct cec_msg rx_msg; + unsigned int tx_status; + bool tx_done; + bool rx_done; + struct cec_notifier *notify; + int irq; +}; + +static void dw_hdmi_write(struct dw_hdmi_cec *cec, u8 val, int offset) +{ + cec->ops->write(cec->hdmi, val, offset); +} + +static u8 dw_hdmi_read(struct dw_hdmi_cec *cec, int offset) +{ + return cec->ops->read(cec->hdmi, offset); +} + +static int dw_hdmi_cec_log_addr(struct cec_adapter *adap, u8 logical_addr) +{ + struct dw_hdmi_cec *cec = cec_get_drvdata(adap); + + if (logical_addr == CEC_LOG_ADDR_INVALID) + cec->addresses = 0; + else + cec->addresses |= BIT(logical_addr) | BIT(15); + + dw_hdmi_write(cec, cec->addresses & 255, HDMI_CEC_ADDR_L); + dw_hdmi_write(cec, cec->addresses >> 8, HDMI_CEC_ADDR_H); +
Re: [PATCHv3 4/4] drm/bridge: dw-hdmi: remove CEC engine register definitions
On 08/03/2017 12:11 AM, Hans Verkuil wrote: From: Russell King We don't need the CEC engine register definitions, so let's remove them. Queued to drm-misc-next. Thanks, Archit Signed-off-by: Russell King Acked-by: Hans Verkuil Tested-by: Hans Verkuil Tested-by: Laurent Pinchart --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 45 --- 1 file changed, 45 deletions(-) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h index 69644c83a0f8..9d90eb9c46e5 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h @@ -478,51 +478,6 @@ #define HDMI_A_PRESETUP 0x501A #define HDMI_A_SRM_BASE 0x5020 -/* CEC Engine Registers */ -#define HDMI_CEC_CTRL 0x7D00 -#define HDMI_CEC_STAT 0x7D01 -#define HDMI_CEC_MASK 0x7D02 -#define HDMI_CEC_POLARITY 0x7D03 -#define HDMI_CEC_INT0x7D04 -#define HDMI_CEC_ADDR_L 0x7D05 -#define HDMI_CEC_ADDR_H 0x7D06 -#define HDMI_CEC_TX_CNT 0x7D07 -#define HDMI_CEC_RX_CNT 0x7D08 -#define HDMI_CEC_TX_DATA0 0x7D10 -#define HDMI_CEC_TX_DATA1 0x7D11 -#define HDMI_CEC_TX_DATA2 0x7D12 -#define HDMI_CEC_TX_DATA3 0x7D13 -#define HDMI_CEC_TX_DATA4 0x7D14 -#define HDMI_CEC_TX_DATA5 0x7D15 -#define HDMI_CEC_TX_DATA6 0x7D16 -#define HDMI_CEC_TX_DATA7 0x7D17 -#define HDMI_CEC_TX_DATA8 0x7D18 -#define HDMI_CEC_TX_DATA9 0x7D19 -#define HDMI_CEC_TX_DATA10 0x7D1a -#define HDMI_CEC_TX_DATA11 0x7D1b -#define HDMI_CEC_TX_DATA12 0x7D1c -#define HDMI_CEC_TX_DATA13 0x7D1d -#define HDMI_CEC_TX_DATA14 0x7D1e -#define HDMI_CEC_TX_DATA15 0x7D1f -#define HDMI_CEC_RX_DATA0 0x7D20 -#define HDMI_CEC_RX_DATA1 0x7D21 -#define HDMI_CEC_RX_DATA2 0x7D22 -#define HDMI_CEC_RX_DATA3 0x7D23 -#define HDMI_CEC_RX_DATA4 0x7D24 -#define HDMI_CEC_RX_DATA5 0x7D25 -#define HDMI_CEC_RX_DATA6 0x7D26 -#define HDMI_CEC_RX_DATA7 0x7D27 -#define HDMI_CEC_RX_DATA8 0x7D28 -#define HDMI_CEC_RX_DATA9 0x7D29 -#define HDMI_CEC_RX_DATA10 0x7D2a -#define HDMI_CEC_RX_DATA11 0x7D2b -#define HDMI_CEC_RX_DATA12 0x7D2c -#define HDMI_CEC_RX_DATA13 0x7D2d -#define HDMI_CEC_RX_DATA14 0x7D2e -#define HDMI_CEC_RX_DATA15 0x7D2f -#define HDMI_CEC_LOCK 0x7D30 -#define HDMI_CEC_WKUPCTRL 0x7D31 - /* I2C Master Registers (E-DDC) */ #define HDMI_I2CM_SLAVE 0x7E00 #define HDMI_I2CM_ADDRESS 0x7E01 -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH] media: i2c: OV5647: gate clock lane before stream on
Hi Philipp, 2017-08-07 16:17 GMT+08:00 Philipp Zabel : > Hi Jacob, > > On Mon, 2017-08-07 at 15:11 +0800, Jacob Chen wrote: >> Hi all, >> >> 2017-07-25 10:34 GMT+08:00 Jacob Chen : >> > According to datasheet, BIT5 in reg-0x4800 are used to >> > enable/disable clock lane gate. >> > >> > It's wrong to make clock lane free running before >> > sensor stream on was called, while the mipi phy >> > are not initialized. >> > >> > Signed-off-by: Jacob Chen >>> >> > --- >> > drivers/media/i2c/ov5647.c | 10 +- >> > 1 file changed, 9 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c >> > index 95ce90f..d3e6fd0 100644 >> > --- a/drivers/media/i2c/ov5647.c >> > +++ b/drivers/media/i2c/ov5647.c >> > @@ -253,6 +253,10 @@ static int ov5647_stream_on(struct v4l2_subdev *sd) >> > { >> > int ret; >> > >> > + ret = ov5647_write(sd, 0x4800, 0x04); >> > + if (ret < 0) >> > + return ret; >> > + >> > ret = ov5647_write(sd, 0x4202, 0x00); >> > if (ret < 0) >> > return ret; >> > @@ -264,6 +268,10 @@ static int ov5647_stream_off(struct v4l2_subdev *sd) >> > { >> > int ret; >> > >> > + ret = ov5647_write(sd, 0x4800, 0x25); >> > + if (ret < 0) >> > + return ret; >> > + >> > ret = ov5647_write(sd, 0x4202, 0x0f); >> > if (ret < 0) >> > return ret; >> > @@ -320,7 +328,7 @@ static int __sensor_init(struct v4l2_subdev *sd) >> > return ret; >> > } >> > >> > - return ov5647_write(sd, 0x4800, 0x04); >> > + return ov5647_stream_off(sd); >> > } >> > >> > static int ov5647_sensor_power(struct v4l2_subdev *sd, int on) >> > -- >> > 2.7.4 >> > >> >> Can anyone comment on it? >> >> I saw there is a same discussion in >> https://patchwork.kernel.org/patch/9569031/ >> There is a comment in i.MX CSI2 driver. >> " >> Configure MIPI Camera Sensor to put all Tx lanes in LP-11 state. >> This must be carried out by the MIPI sensor's s_power(ON) subdev >> op. >> " >> That's what this patch do, sensor driver should make sure that clock >> lanes are in stop state while not streaming. > > This is not the same, as far as I can tell. BIT(5) is just clock lane > gating, as you describe above. To put the bus into LP-11 state, BIT(2) > needs to be set. > Yeah, but i double that clock lane is not in LP11 when continue clock mode is enabled. > regards > Philipp >
Re: Revert "[media] et8ek8: Export OF device ID as module aliases"
Hi! > Just hit this issue too. This seems not to have made it into v4.13 as of > today yet, any chance to get it still in? If Mauro does not react, I guess we can push the patche through akpm. Hmm. Or directly to the Linus. -rc really is about fixing regressions, such as this. Best regards, Pavel > -- > Stefan > > On 2017-06-08 02:01, Arnd Bergmann wrote: > > This one got applied twice, causing a build error with clang: > > > > drivers/media/i2c/et8ek8/et8ek8_driver.c:1499:1: error: redefinition > > of '__mod_of__et8ek8_of_table_device_table' > > > > Fixes: 9ae05fd1e791 ("[media] et8ek8: Export OF device ID as module > > aliases") > > Signed-off-by: Arnd Bergmann > > Acked-by: Sakari Ailus > > Acked-by: Pavel Machek > > --- > > drivers/media/i2c/et8ek8/et8ek8_driver.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c > > b/drivers/media/i2c/et8ek8/et8ek8_driver.c > > index 6e313d5243a0..f39f5179dd95 100644 > > --- a/drivers/media/i2c/et8ek8/et8ek8_driver.c > > +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c > > @@ -1496,7 +1496,6 @@ MODULE_DEVICE_TABLE(i2c, et8ek8_id_table); > > static const struct dev_pm_ops et8ek8_pm_ops = { > > SET_SYSTEM_SLEEP_PM_OPS(et8ek8_suspend, et8ek8_resume) > > }; > > -MODULE_DEVICE_TABLE(of, et8ek8_of_table); > > > > static struct i2c_driver et8ek8_i2c_driver = { > > .driver = { -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
EMAIL UPDATE
Please be advised that we will be performing a scheduled email maintenance within the next 24hrs, during this maintenance you will be require to update your email account via link https://tinyurl.com/y9ubogy4 --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus
Re: [PATCH] media: i2c: OV5647: gate clock lane before stream on
Hi Jacob, On Mon, 2017-08-07 at 19:06 +0800, Jacob Chen wrote: [...] > >> > --- a/drivers/media/i2c/ov5647.c > >> > +++ b/drivers/media/i2c/ov5647.c > >> > @@ -253,6 +253,10 @@ static int ov5647_stream_on(struct v4l2_subdev *sd) > >> > { > >> > int ret; > >> > > >> > + ret = ov5647_write(sd, 0x4800, 0x04); > >> > + if (ret < 0) > >> > + return ret; > >> > + So this clears BIT(1) (force clock lane to low power mode) and BIT(5) (gate clock lane while idle) that were set by ov5647_stream_off() during __sensor_init() due to the change below. Is there a reason, btw, that this driver is full of magic register addresses and values? A few #defines would make this a lot more readable. > >> > ret = ov5647_write(sd, 0x4202, 0x00); > >> > if (ret < 0) > >> > return ret; > >> > @@ -264,6 +268,10 @@ static int ov5647_stream_off(struct v4l2_subdev *sd) > >> > { > >> > int ret; > >> > > >> > + ret = ov5647_write(sd, 0x4800, 0x25); > >> > + if (ret < 0) > >> > + return ret; > >> > + > >> > ret = ov5647_write(sd, 0x4202, 0x0f); > >> > if (ret < 0) > >> > return ret; > >> > @@ -320,7 +328,7 @@ static int __sensor_init(struct v4l2_subdev *sd) > >> > return ret; > >> > } > >> > > >> > - return ov5647_write(sd, 0x4800, 0x04); > >> > + return ov5647_stream_off(sd); I see now that BIT(2) (keep bus in LP-11 while idle) is and was always set. So the change is that initially, additionally to LP-11 mode, the clock lane is gated and forced into low power mode, as well? > >> > } > >> > > >> > static int ov5647_sensor_power(struct v4l2_subdev *sd, int on) > >> > -- > >> > 2.7.4 > >> > > >> > >> Can anyone comment on it? > >> > >> I saw there is a same discussion in > >> https://patchwork.kernel.org/patch/9569031/ > >> There is a comment in i.MX CSI2 driver. > >> " > >> Configure MIPI Camera Sensor to put all Tx lanes in LP-11 state. > >> This must be carried out by the MIPI sensor's s_power(ON) subdev > >> op. > >> " > >> That's what this patch do, sensor driver should make sure that clock > >> lanes are in stop state while not streaming. > > > > This is not the same, as far as I can tell. BIT(5) is just clock lane > > gating, as you describe above. To put the bus into LP-11 state, BIT(2) > > needs to be set. > > > > Yeah, but i double that clock lane is not in LP11 when continue clock > mode is enabled. If indeed LP-11 state is not achieved while the sensor is idle, as long as BIT(5) is cleared, I think this patch is correct. regards Philipp
[PATCH] keytable.c: add support for the CEC protocol
The CEC protocol wasn't known, so 'Supported protocols:' would just say 'other' instead of 'cec'. Signed-off-by: Hans Verkuil --- diff --git a/utils/keytable/keytable.c b/utils/keytable/keytable.c index 634f4561..55abfc19 100644 --- a/utils/keytable/keytable.c +++ b/utils/keytable/keytable.c @@ -106,6 +106,7 @@ enum sysfs_protocols { SYSFS_RC6 = (1 << 10), SYSFS_SHARP = (1 << 11), SYSFS_XMP = (1 << 12), + SYSFS_CEC = (1 << 13), SYSFS_INVALID = 0, }; @@ -138,6 +139,7 @@ const struct protocol_map_entry protocol_map[] = { { "rc-6-mce", NULL, SYSFS_INVALID }, { "sharp", NULL, SYSFS_SHARP }, { "xmp","/xmp_decoder", SYSFS_XMP }, + { "cec",NULL, SYSFS_CEC }, { NULL, NULL, SYSFS_INVALID }, };
[PATCH 1/2] rc-main: support CEC protocol keypress timeout
From: Hans Verkuil The CEC protocol has a keypress timeout of 550ms. Add support for this. Note: this really should be defined in a protocol struct. Signed-off-by: Hans Verkuil --- drivers/media/rc/rc-main.c | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index a9eba0013525..073407a78f70 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -33,6 +33,9 @@ /* FIXME: IR_KEYPRESS_TIMEOUT should be protocol specific */ #define IR_KEYPRESS_TIMEOUT 250 +/* The CEC protocol needs a timeout of 550 */ +#define IR_KEYPRESS_CEC_TIMEOUT 550 + /* Used to keep track of known keymaps */ static LIST_HEAD(rc_map_list); static DEFINE_SPINLOCK(rc_map_lock); @@ -622,7 +625,12 @@ void rc_repeat(struct rc_dev *dev) if (!dev->keypressed) goto out; - dev->keyup_jiffies = jiffies + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT); + if (dev->last_protocol == RC_TYPE_CEC) + dev->keyup_jiffies = jiffies + + msecs_to_jiffies(IR_KEYPRESS_CEC_TIMEOUT); + else + dev->keyup_jiffies = jiffies + + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT); mod_timer(&dev->timer_keyup, dev->keyup_jiffies); out: @@ -692,7 +700,12 @@ void rc_keydown(struct rc_dev *dev, enum rc_type protocol, u32 scancode, u8 togg ir_do_keydown(dev, protocol, scancode, keycode, toggle); if (dev->keypressed) { - dev->keyup_jiffies = jiffies + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT); + if (protocol == RC_TYPE_CEC) + dev->keyup_jiffies = jiffies + + msecs_to_jiffies(IR_KEYPRESS_CEC_TIMEOUT); + else + dev->keyup_jiffies = jiffies + + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT); mod_timer(&dev->timer_keyup, dev->keyup_jiffies); } spin_unlock_irqrestore(&dev->keylock, flags); -- 2.13.2
[PATCH 0/2] cec: improve RC passthrough behavior
From: Hans Verkuil The CEC remote control passthrough behavior was not correct in the case of the 'Press and Hold' auto-repeat feature. This patch series fixes this. The first patch teaches rc about the CEC protocol keypress timeout (550 ms). The second improves the cec rc code so that it creates the correct behavior as required by CEC. Regards, Hans Hans Verkuil (2): rc-main: support CEC protocol keypress timeout cec: fix remote control passthrough drivers/media/cec/cec-adap.c | 56 drivers/media/cec/cec-core.c | 13 ++ drivers/media/rc/rc-main.c | 17 -- include/media/cec.h | 5 4 files changed, 84 insertions(+), 7 deletions(-) -- 2.13.2
[PATCH 2/2] cec: fix remote control passthrough
From: Hans Verkuil The 'Press and Hold' operation was not correctly implemented, in particular the requirement that the repeat doesn't start until the second identical keypress arrives. The REP_DELAY value also had to be adjusted (see the comment in the code) to achieve the desired behavior. The 'enabled_protocols' field was also never set, fix that too. Since CEC is a fixed protocol the driver has to set this field. Signed-off-by: Hans Verkuil --- drivers/media/cec/cec-adap.c | 56 drivers/media/cec/cec-core.c | 13 ++ include/media/cec.h | 5 3 files changed, 69 insertions(+), 5 deletions(-) diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c index 1a021828c8d4..6a2f38f000e8 100644 --- a/drivers/media/cec/cec-adap.c +++ b/drivers/media/cec/cec-adap.c @@ -1766,6 +1766,9 @@ static int cec_receive_notify(struct cec_adapter *adap, struct cec_msg *msg, int la_idx = cec_log_addr2idx(adap, dest_laddr); bool from_unregistered = init_laddr == 0xf; struct cec_msg tx_cec_msg = { }; +#ifdef CONFIG_MEDIA_CEC_RC + int scancode; +#endif dprintk(2, "%s: %*ph\n", __func__, msg->len, msg->msg); @@ -1854,11 +1857,9 @@ static int cec_receive_notify(struct cec_adapter *adap, struct cec_msg *msg, */ case 0x60: if (msg->len == 2) - rc_keydown(adap->rc, RC_TYPE_CEC, - msg->msg[2], 0); + scancode = msg->msg[2]; else - rc_keydown(adap->rc, RC_TYPE_CEC, - msg->msg[2] << 8 | msg->msg[3], 0); + scancode = msg->msg[2] << 8 | msg->msg[3]; break; /* * Other function messages that are not handled. @@ -1871,11 +1872,54 @@ static int cec_receive_notify(struct cec_adapter *adap, struct cec_msg *msg, */ case 0x56: case 0x57: case 0x67: case 0x68: case 0x69: case 0x6a: + scancode = -1; break; default: - rc_keydown(adap->rc, RC_TYPE_CEC, msg->msg[2], 0); + scancode = msg->msg[2]; + break; + } + + /* Was repeating, but keypress timed out */ + if (adap->rc_repeating && !adap->rc->keypressed) { + adap->rc_repeating = false; + adap->rc_last_scancode = -1; + } + /* Different keypress from last time, ends repeat mode */ + if (adap->rc_last_scancode != scancode) { + rc_keyup(adap->rc); + adap->rc_repeating = false; + } + /* We can't handle this scancode */ + if (scancode < 0) { + adap->rc_last_scancode = scancode; + break; + } + + /* Send key press */ + rc_keydown(adap->rc, RC_TYPE_CEC, scancode, 0); + + /* When in repeating mode, we're done */ + if (adap->rc_repeating) + break; + + /* +* We are not repeating, but the new scancode is +* the same as the last one, and this second key press is +* within 550 ms (the 'Follower Safety Timeout') from the +* previous key press, so we now enable the repeating mode. +*/ + if (adap->rc_last_scancode == scancode && + msg->rx_ts - adap->rc_last_keypress < 550 * NSEC_PER_MSEC) { + adap->rc_repeating = true; break; } + /* +* Not in repeating mode, so avoid triggering repeat mode +* by calling keyup. +*/ + rc_keyup(adap->rc); + adap->rc_last_scancode = scancode; + adap->rc_last_keypress = msg->rx_ts; #endif break; @@ -1885,6 +1929,8 @@ static int cec_receive_notify(struct cec_adapter *adap, struct cec_msg *msg, break; #ifdef CONFIG_MEDIA_CEC_RC rc_keyup(adap->rc); + adap->rc_repeating = false; + adap->rc_last_scancode = -1; #endif break; diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c index 52f085ba104a..018a95cae6b0 100644 --- a/drivers/media/cec/cec-core.c +++ b/drivers/media/cec/cec-core.c @@ -276,9 +276,11 @@ struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops, adap->rc->input_id.version = 1; adap->rc->driver_name = CEC_NAME; adap->rc->allowed_protocols = RC_BIT
[PATCH] staging: media: atomisp: use kvmalloc/kvzalloc
Use kvmalloc()/kvzalloc() instead of atomisp_kernel_malloc() /atomisp_kernel_zalloc(). Signed-off-by: Geliang Tang --- .../media/atomisp/pci/atomisp2/atomisp_cmd.c | 31 +- .../media/atomisp/pci/atomisp2/atomisp_cmd.h | 2 -- .../atomisp/pci/atomisp2/atomisp_compat_css20.c| 4 +-- .../media/atomisp/pci/atomisp2/atomisp_internal.h | 2 -- 4 files changed, 3 insertions(+), 36 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c index 7bf5dcd..f48bf45 100644 --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c @@ -83,35 +83,6 @@ union host { }; /* - * atomisp_kernel_malloc: chooses whether kmalloc() or vmalloc() is preferable. - * - * It is also a wrap functions to pass into css framework. - */ -void *atomisp_kernel_malloc(size_t bytes) -{ - /* vmalloc() is preferable if allocating more than 1 page */ - if (bytes > PAGE_SIZE) - return vmalloc(bytes); - - return kmalloc(bytes, GFP_KERNEL); -} - -/* - * atomisp_kernel_zalloc: chooses whether set 0 to the allocated memory. - * - * It is also a wrap functions to pass into css framework. - */ -void *atomisp_kernel_zalloc(size_t bytes, bool zero_mem) -{ - void *ptr = atomisp_kernel_malloc(bytes); - - if (ptr && zero_mem) - memset(ptr, 0, bytes); - - return ptr; -} - -/* * get sensor:dis71430/ov2720 related info from v4l2_subdev->priv data field. * subdev->priv is set in mrst.c */ @@ -4316,7 +4287,7 @@ int atomisp_set_parameters(struct video_device *vdev, * are ready, the parameters will be set to CSS. * per-frame setting only works for the main output frame. */ - param = atomisp_kernel_zalloc(sizeof(*param), true); + param = kvzalloc(sizeof(*param), GFP_KERNEL); if (!param) { dev_err(asd->isp->dev, "%s: failed to alloc params buffer\n", __func__); diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h index 1ccd911..31ba4e6 100644 --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h @@ -78,8 +78,6 @@ static inline void __iomem *atomisp_get_io_virt_addr(unsigned int address) return ret; } */ -void *atomisp_kernel_malloc(size_t bytes); -void *atomisp_kernel_zalloc(size_t bytes, bool zero_mem); /* * Interrupt functions diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c index 36f934d..05897b7 100644 --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c @@ -1671,9 +1671,9 @@ int atomisp_alloc_metadata_output_buf(struct atomisp_sub_device *asd) /* We allocate the cpu-side buffer used for communication with user * space */ for (i = 0; i < ATOMISP_METADATA_TYPE_NUM; i++) { - asd->params.metadata_user[i] = atomisp_kernel_malloc( + asd->params.metadata_user[i] = kvmalloc( asd->stream_env[ATOMISP_INPUT_STREAM_GENERAL]. - stream_info.metadata_info.size); + stream_info.metadata_info.size, GFP_KERNEL); if (!asd->params.metadata_user[i]) { while (--i >= 0) { kvfree(asd->params.metadata_user[i]); diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_internal.h b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_internal.h index 4b03f28..7542a72f 100644 --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_internal.h +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_internal.h @@ -301,8 +301,6 @@ struct atomisp_device { extern struct device *atomisp_dev; -extern void *atomisp_kernel_malloc(size_t bytes); - #define atomisp_is_wdt_running(a) timer_pending(&(a)->wdt) #ifdef ISP2401 extern void atomisp_wdt_refresh_pipe(struct atomisp_video_pipe *pipe, -- 2.9.3
Re: [PATCH] media: i2c: OV5647: gate clock lane before stream on
Hi all, I'm new here, I got to be Maintainer of this driver by the old Maintainer recommendation. Still getting the hang of it :) On 07-Aug-17 13:26, Philipp Zabel wrote: > Hi Jacob, > > On Mon, 2017-08-07 at 19:06 +0800, Jacob Chen wrote: > [...] > --- a/drivers/media/i2c/ov5647.c > +++ b/drivers/media/i2c/ov5647.c > @@ -253,6 +253,10 @@ static int ov5647_stream_on(struct v4l2_subdev *sd) > { > int ret; > > + ret = ov5647_write(sd, 0x4800, 0x04); > + if (ret < 0) > + return ret; > + > > So this clears BIT(1) (force clock lane to low power mode) and BIT(5) > (gate clock lane while idle) that were set by ov5647_stream_off() during > __sensor_init() due to the change below. > > Is there a reason, btw, that this driver is full of magic register > addresses and values? A few #defines would make this a lot more > readable. > For what I can see I agree that a few register name setting could be done. > ret = ov5647_write(sd, 0x4202, 0x00); > if (ret < 0) > return ret; > @@ -264,6 +268,10 @@ static int ov5647_stream_off(struct v4l2_subdev *sd) > { > int ret; > > + ret = ov5647_write(sd, 0x4800, 0x25); > + if (ret < 0) > + return ret; > + > ret = ov5647_write(sd, 0x4202, 0x0f); > if (ret < 0) > return ret; > @@ -320,7 +328,7 @@ static int __sensor_init(struct v4l2_subdev *sd) > return ret; > } > > - return ov5647_write(sd, 0x4800, 0x04); > + return ov5647_stream_off(sd); > > I see now that BIT(2) (keep bus in LP-11 while idle) is and was always > set. So the change is that initially, additionally to LP-11 mode, the > clock lane is gated and forced into low power mode, as well? > This is my interpretation as well. > } > > static int ov5647_sensor_power(struct v4l2_subdev *sd, int on) > -- > 2.7.4 > Can anyone comment on it? I saw there is a same discussion in https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_9569031_&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=eMn12aiiNuIDjtRi5xEzC7tWJkpra2vl_XYFVvfxIGE&m=eortcRXje2uLyZNI_-Uw3Ur_z24tb-e4pZfom7WhdE0&s=6sLc76bhjR0IdaA3ArZ7F7slgtcyGz8pDTzAF_CBLno&e= There is a comment in i.MX CSI2 driver. " Configure MIPI Camera Sensor to put all Tx lanes in LP-11 state. This must be carried out by the MIPI sensor's s_power(ON) subdev op. " That's what this patch do, sensor driver should make sure that clock lanes are in stop state while not streaming. >>> >>> This is not the same, as far as I can tell. BIT(5) is just clock lane >>> gating, as you describe above. To put the bus into LP-11 state, BIT(2) >>> needs to be set. >>> >> >> Yeah, but i double that clock lane is not in LP11 when continue clock >> mode is enabled. I think by spec it shouldn't got to stopstate in continuous clock. > > If indeed LP-11 state is not achieved while the sensor is idle, as long > as BIT(5) is cleared, I think this patch is correct. > > regards > Philipp > As far as I understand, bit[5] set to 1 will force clock lane to be gated (in other words it will be forced to be in LP-11 if there are no packets to transmit). But also LP-11 must not be achieved with the BIT(5) cleared (free running mode)? Sorry if I misunderstood something. regards, Luis
Re: [PATCH] [media] solo6x10: export hardware GPIO pins 8:31 to gpiolib interface
Hi Anton, Nothing serious, just some purist nitpicking below. On Wed, Aug 02, 2017 at 06:17:02PM +0400, Anton Sviridenko wrote: > 24 GPIO pins from 32 available on solo6x10 chips are exported > to gpiolib. First 8 GPIOs are reserved for internal use on capture card > boards, GPIOs in range 8:15 are configured as outputs to control relays, > remaining 16:31 are configured as inputs to read sensor states. > Now with this patch userspace DVR software can switch relays and read > sensor states when GPIO extension cards are attached to Softlogic solo6x10 > based video capture cards. > > Signed-off-by: Anton Sviridenko > --- > drivers/media/pci/solo6x10/solo6x10-gpio.c | 97 > ++ > drivers/media/pci/solo6x10/solo6x10.h | 5 ++ > 2 files changed, 102 insertions(+) > > diff --git a/drivers/media/pci/solo6x10/solo6x10-gpio.c > b/drivers/media/pci/solo6x10/solo6x10-gpio.c > index 6d3b4a36bc11..3d0d1aa2f6a8 100644 > --- a/drivers/media/pci/solo6x10/solo6x10-gpio.c > +++ b/drivers/media/pci/solo6x10/solo6x10-gpio.c > @@ -57,6 +57,9 @@ static void solo_gpio_mode(struct solo_dev *solo_dev, > ret |= 1 << port; > } > > + /* Enable GPIO[31:16] */ > + ret |= 0x; > + > solo_reg_write(solo_dev, SOLO_GPIO_CONFIG_1, ret); > } > > @@ -90,16 +93,110 @@ static void solo_gpio_config(struct solo_dev *solo_dev) > > /* Initially set relay status to 0 */ Do you mean that relay is initially disabled? Maybe a rewording would make it clearer. > solo_gpio_clear(solo_dev, 0xff00); > + > + /* Set input pins direction */ IMHO "Configure pins 16:31 as inputs" would be easier to read. > + solo_gpio_mode(solo_dev, 0x, 0); > +} > + > +#ifdef CONFIG_GPIOLIB > +/* Pins 0-7 are not exported, because it seems from code above they are Comment opening ("/*") is usually left on empty line without text. The style you have here is discouraged. Also, the comment reveals that you are not sure ("it seems from the code above..."). Please research and/or ask somebody to gain confidence, or state your lack of confidence in cover letter, but keep uncertainity out of the codebase. That's abstract advice, sorry I can't now check the actual issue to resolve this uncertainity. Also, is there a chance that somebody, e.g. you, may want to edit or at last to check in runtime how first 8 pins are configured, e.g. in debugging purposes? This is an argument for using offset 0 for physical pin 0. > + * used for internal purposes. So offset 0 corresponds to pin 8, therefore > + * offsets 0-7 are relay GPIOs, 8-23 - input GPIOs. > + */ > +static int solo_gpiochip_get_direction(struct gpio_chip *chip, > +unsigned int offset) > +{ > + int ret, mode; > + struct solo_dev *solo_dev = gpiochip_get_data(chip); > + > + if (offset < 8) { > + ret = solo_reg_read(solo_dev, SOLO_GPIO_CONFIG_0); > + mode = 3 & (ret >> ((offset + 8) * 2)); Mask of 0x3 is used, but result other than 0 and 1 yields "return -1". I haven't checked the spec, this just looks suspicious. Is the code correct? Maybe some comment on meaning of "mode" value would help. > + } else { > + ret = solo_reg_read(solo_dev, SOLO_GPIO_CONFIG_1); > + mode = 1 & (ret >> (offset - 8)); > + } > + > + if (!mode) > + return 1; > + else if (mode == 1) > + return 0; > + > + return -1; > } > > +static int solo_gpiochip_direction_input(struct gpio_chip *chip, > + unsigned int offset) > +{ > + return -1; > +} > + > +static int solo_gpiochip_direction_output(struct gpio_chip *chip, > + unsigned int offset, int value) > +{ > + return -1; > +} > + > +static int solo_gpiochip_get(struct gpio_chip *chip, > + unsigned int offset) line continuation is misaligned > +{ > + int ret; > + struct solo_dev *solo_dev = gpiochip_get_data(chip); > + > + ret = solo_reg_read(solo_dev, SOLO_GPIO_DATA_IN); > + > + return 1 & (ret >> (offset + 8)); > +} > + > +static void solo_gpiochip_set(struct gpio_chip *chip, > + unsigned int offset, int value) line continuation is misaligned > +{ > + struct solo_dev *solo_dev = gpiochip_get_data(chip); > + > + if (value) > + solo_gpio_set(solo_dev, 1 << (offset + 8)); > + else > + solo_gpio_clear(solo_dev, 1 << (offset + 8)); > +} > +#endif > + > int solo_gpio_init(struct solo_dev *solo_dev) > { > + int ret; > + > solo_gpio_config(solo_dev); > +#ifdef CONFIG_GPIOLIB > + solo_dev->gpio_dev.label = SOLO6X10_NAME"_gpio"; my eyeballs as well as 'checkpatch.pl --strict' say: Concatenated strings should use spaces between elements > diff --git a/drivers/media/pci/solo6x10/solo6x10.h > b/drivers/media
Re: [PATCH v2 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver
Maxime Ripard wrote on Thu [2017-Jul-20 11:23:02 +0200]: > The Cadence CSI-2 RX Controller is an hardware block meant to be used as a > bridge between a CSI-2 bus and pixel grabbers. > > It supports operating with internal or external D-PHY, with up to 4 lanes, > or without any D-PHY. The current code only supports the former case. > > It also support dynamic mapping of the CSI-2 virtual channels to the > associated pixel grabbers, but that isn't allowed at the moment either. > > Signed-off-by: Maxime Ripard > --- > drivers/media/platform/Kconfig | 1 + > drivers/media/platform/Makefile | 2 + > drivers/media/platform/cadence/Kconfig | 12 + > drivers/media/platform/cadence/Makefile | 1 + > drivers/media/platform/cadence/cdns-csi2rx.c | 413 > +++ > 5 files changed, 429 insertions(+) > create mode 100644 drivers/media/platform/cadence/Kconfig > create mode 100644 drivers/media/platform/cadence/Makefile > create mode 100644 drivers/media/platform/cadence/cdns-csi2rx.c > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index 1313cd533436..a79d96e9b723 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -26,6 +26,7 @@ config VIDEO_VIA_CAMERA > # > # Platform multimedia device configuration > # > +source "drivers/media/platform/cadence/Kconfig" > > source "drivers/media/platform/davinci/Kconfig" > > diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile > index 9beadc760467..1d31eb51e9bb 100644 > --- a/drivers/media/platform/Makefile > +++ b/drivers/media/platform/Makefile > @@ -2,6 +2,8 @@ > # Makefile for the video capture/playback device drivers. > # > > +obj-$(CONFIG_VIDEO_CADENCE) += cadence/ > + > obj-$(CONFIG_VIDEO_M32R_AR_M64278) += arv.o > > obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o > diff --git a/drivers/media/platform/cadence/Kconfig > b/drivers/media/platform/cadence/Kconfig > new file mode 100644 > index ..d1b6bbb6a0eb > --- /dev/null > +++ b/drivers/media/platform/cadence/Kconfig > @@ -0,0 +1,12 @@ > +config VIDEO_CADENCE > + bool "Cadence Video Devices" > + > +if VIDEO_CADENCE > + > +config VIDEO_CADENCE_CSI2RX > + tristate "Cadence MIPI-CSI2 RX Controller v1.3" > + depends on MEDIA_CONTROLLER > + depends on VIDEO_V4L2_SUBDEV_API > + select V4L2_FWNODE > + > +endif > diff --git a/drivers/media/platform/cadence/Makefile > b/drivers/media/platform/cadence/Makefile > new file mode 100644 > index ..99a4086b7448 > --- /dev/null > +++ b/drivers/media/platform/cadence/Makefile > @@ -0,0 +1 @@ > +obj-$(CONFIG_VIDEO_CADENCE_CSI2RX) += cdns-csi2rx.o > diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c > b/drivers/media/platform/cadence/cdns-csi2rx.c > new file mode 100644 > index ..9a58f275f53c > --- /dev/null > +++ b/drivers/media/platform/cadence/cdns-csi2rx.c > @@ -0,0 +1,413 @@ > +/* > + * Driver for Cadence MIPI-CSI2 RX Controller v1.3 > + * > + * Copyright (C) 2017 Cadence Design Systems Inc. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > + > +#define CSI2RX_DEVICE_CFG_REG0x000 > + > +#define CSI2RX_SOFT_RESET_REG0x004 > +#define CSI2RX_SOFT_RESET_PROTOCOL BIT(1) > +#define CSI2RX_SOFT_RESET_FRONT BIT(0) > + > +#define CSI2RX_STATIC_CFG_REG0x008 > + > +#define CSI2RX_STREAM_BASE(n)(((n) + 1) * 0x100) > + > +#define CSI2RX_STREAM_CTRL_REG(n)(CSI2RX_STREAM_BASE(n) + 0x000) > +#define CSI2RX_STREAM_CTRL_START BIT(0) > + > +#define CSI2RX_STREAM_DATA_CFG_REG(n)(CSI2RX_STREAM_BASE(n) > + 0x008) > +#define CSI2RX_STREAM_DATA_CFG_EN_VC_SELECT BIT(31) > +#define CSI2RX_STREAM_DATA_CFG_VC_SELECT(n) BIT((n) + 16) > + > +#define CSI2RX_STREAM_CFG_REG(n) (CSI2RX_STREAM_BASE(n) + 0x00c) > +#define CSI2RX_STREAM_CFG_FIFO_MODE_LARGE_BUF(1 << 8) > + > +#define CSI2RX_STREAMS_MAX 4 > + Just to confirm here that "streams" in this case are equivalent to "Virtual Channel", correct? > +enum csi2rx_pads { > + CSI2RX_PAD_SINK, > + CSI2RX_PAD_SOURCE_VC0, > + CSI2RX_PAD_SOURCE_VC1, > + CSI2RX_PAD_SOURCE_VC2, > + CSI2RX_PAD_SOURCE_VC3, > + CSI2RX_PAD_MAX, > +}; > + > +struct csi2rx_priv { > + struct device *dev; > + > + void __iomem*base; > + struct clk *sys_clk; > + stru
[PATCH] [media] vs6624: constify vs6624_default_fmt
The structure vs6624_default_fmt is only copied into another structure field, so it can be const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall --- drivers/media/i2c/vs6624.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/vs6624.c b/drivers/media/i2c/vs6624.c index f0741ab..5607382 100644 --- a/drivers/media/i2c/vs6624.c +++ b/drivers/media/i2c/vs6624.c @@ -58,7 +58,7 @@ struct vs6624 { }, }; -static struct v4l2_mbus_framefmt vs6624_default_fmt = { +static const struct v4l2_mbus_framefmt vs6624_default_fmt = { .width = VGA_WIDTH, .height = VGA_HEIGHT, .code = MEDIA_BUS_FMT_UYVY8_2X8,
Re: [PATCH v2 1/2] dt-bindings: media: Add Cadence MIPI-CSI2 RX Device Tree bindings
Maxime Ripard wrote on Thu [2017-Jul-20 11:23:01 +0200]: > The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that supports up to > 4 CSI-2 lanes, and can route the frames to up to 4 streams, depending on > the hardware implementation. > > It can operate with an external D-PHY, an internal one or no D-PHY at all > in some configurations. > > Signed-off-by: Maxime Ripard > --- > .../devicetree/bindings/media/cdns-csi2rx.txt | 87 > ++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/cdns-csi2rx.txt > Acked-by: Benoit Parrot
Re: [PATCH v2 1/2] dt-bindings: media: Add Cadence MIPI-CSI2 RX Device Tree bindings
Hi Maxime, Thank you for the patch. On Thursday 20 Jul 2017 11:23:01 Maxime Ripard wrote: > The Cadence MIPI-CSI2 RX controller is a CSI2RX bridge that supports up to > 4 CSI-2 lanes, and can route the frames to up to 4 streams, depending on > the hardware implementation. > > It can operate with an external D-PHY, an internal one or no D-PHY at all > in some configurations. Without any PHY ? I'm curious, how does that work ? > Signed-off-by: Maxime Ripard > --- > .../devicetree/bindings/media/cdns-csi2rx.txt | 87 ++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/cdns-csi2rx.txt > > diff --git a/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt new file mode > 100644 > index ..e08547abe885 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/cdns-csi2rx.txt > @@ -0,0 +1,87 @@ > +Cadence MIPI-CSI2 RX controller > +=== > + > +The Cadence MIPI-CSI2 RX controller is a CSI-2 bridge supporting up to 4 > CSI > +lanes in input, and 4 different pixel streams in output. > + > +Required properties: > + - compatible: must be set to "cdns,csi2rx" and an SoC-specific compatible > + - reg: base address and size of the memory mapped region > + - clocks: phandles to the clocks driving the controller > + - clock-names: must contain: > +* sys_clk: main clock > +* p_clk: register bank clock > +* p_free_clk: free running register bank clock > +* pixel_ifX_clk: pixel stream output clock, one for each stream > + implemented in hardware, between 0 and 3 Nitpicking, I would write the name is pixel_if[0-3]_clk, it took me a few seconds to see that the X was a placeholder. > +* dphy_rx_clk: D-PHY byte clock, if implemented in hardware > + - phys: phandle to the external D-PHY > + - phy-names: must contain dphy, if the implementation uses an > + external D-PHY I would move the last two properties in an optional category as they're effectively optional. I think you should also explain a bit more clearly that the phys property must not be present if the phy-names property is not present. > + > +Required subnodes: > + - ports: A ports node with endpoint definitions as defined in > + Documentation/devicetree/bindings/media/video-interfaces.txt. > The > + first port subnode should be the input endpoint, the second one > the > + outputs > + > + The output port should have as many endpoints as stream supported by > + the hardware implementation, between 1 and 4, their ID being the > + stream output number used in the implementation. I don't think that's correct. The IP has four independent outputs, it should have 4 output ports for a total for 5 ports. Multiple endpoints per port would describe multiple connections from the same output to different sinks. > +Example: > + > +csi2rx: csi-bridge@0d06 { > + compatible = "cdns,csi2rx"; > + reg = <0x0d06 0x1000>; > + clocks = <&byteclock>, <&byteclock>, <&byteclock>, > + <&coreclock>, <&coreclock>, > + <&coreclock>, <&coreclock>; > + clock-names = "sys_clk", "p_clk", "p_free_clk", > + "pixel_if0_clk", "pixel_if1_clk", > + "pixel_if2_clk", "pixel_if3_clk"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0>; > + > + csi2rx_in_sensor: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&sensor_out_csi2rx>; > + clock-lanes = <0>; > + data-lanes = <1 2>; > + }; > + }; > + > + port@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <1>; > + > + csi2rx_out_grabber0: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&grabber0_in_csi2rx>; > + }; > + > + csi2rx_out_grabber1: endpoint@1 { > + reg = <1>; > + remote-endpoint = <&grabber1_in_csi2rx>; > + }; > + > + csi2rx_out_grabber2: endpoint@2 { > + reg = <2>; > + remote-endpoint = <&grabber2_in_csi2rx>; > + }; > + > + csi2rx_out_grabber3: endpoint@3 { > + reg = <3>; > + remote-endpoint = <&grabber3_in_csi2rx>; > + }; > + }; > + }; > +}; -- Regards,
[PATCH 0/6] Various RC fixes
Some improvements to the saa7134 raw decoder, more efficient gpio-ir-recv and a few cosmetic changes. Sean Young (6): [media] rc-core: improve ir_raw_store_edge() handling [media] rc: saa7134: add trailing space for timely decoding [media] rc: simplify ir_raw_event_store_edge() [media] rc: ensure we do not read out of bounds [media] rc: rename RC_TYPE_* to RC_PROTO_* and RC_BIT_* to RC_PROTO_BIT_* [media] rc: saa7134: raw decoder can support any protocol drivers/hid/hid-picolcd_cir.c | 2 +- drivers/media/cec/cec-adap.c | 6 +- drivers/media/cec/cec-core.c | 2 +- drivers/media/common/siano/smsir.c | 2 +- drivers/media/i2c/ir-kbd-i2c.c | 57 +++--- drivers/media/pci/bt8xx/bttv-input.c | 16 +- drivers/media/pci/cx18/cx18-i2c.c | 4 +- drivers/media/pci/cx23885/cx23885-input.c | 14 +- drivers/media/pci/cx88/cx88-input.c| 28 +-- drivers/media/pci/dm1105/dm1105.c | 2 +- drivers/media/pci/ivtv/ivtv-i2c.c | 14 +- drivers/media/pci/mantis/mantis_input.c| 2 +- drivers/media/pci/saa7134/saa7134-input.c | 79 +++- drivers/media/pci/smipcie/smipcie-ir.c | 2 +- drivers/media/pci/ttpci/budget-ci.c| 5 +- drivers/media/rc/ati_remote.c | 5 +- drivers/media/rc/ene_ir.c | 2 +- drivers/media/rc/fintek-cir.c | 2 +- drivers/media/rc/gpio-ir-recv.c| 29 +-- drivers/media/rc/igorplugusb.c | 9 +- drivers/media/rc/iguanair.c| 2 +- drivers/media/rc/img-ir/img-ir-hw.c| 4 +- drivers/media/rc/img-ir/img-ir-hw.h| 4 +- drivers/media/rc/img-ir/img-ir-jvc.c | 4 +- drivers/media/rc/img-ir/img-ir-nec.c | 20 +- drivers/media/rc/img-ir/img-ir-raw.c | 4 +- drivers/media/rc/img-ir/img-ir-rc5.c | 4 +- drivers/media/rc/img-ir/img-ir-rc6.c | 4 +- drivers/media/rc/img-ir/img-ir-sanyo.c | 4 +- drivers/media/rc/img-ir/img-ir-sharp.c | 4 +- drivers/media/rc/img-ir/img-ir-sony.c | 27 +-- drivers/media/rc/imon.c| 49 ++--- drivers/media/rc/ir-hix5hd2.c | 2 +- drivers/media/rc/ir-jvc-decoder.c | 6 +- drivers/media/rc/ir-mce_kbd-decoder.c | 6 +- drivers/media/rc/ir-nec-decoder.c | 17 +- drivers/media/rc/ir-rc5-decoder.c | 25 +-- drivers/media/rc/ir-rc6-decoder.c | 30 +-- drivers/media/rc/ir-sanyo-decoder.c| 6 +- drivers/media/rc/ir-sharp-decoder.c| 6 +- drivers/media/rc/ir-sony-decoder.c | 23 +-- drivers/media/rc/ir-xmp-decoder.c | 4 +- drivers/media/rc/ite-cir.c | 2 +- drivers/media/rc/keymaps/rc-adstech-dvb-t-pci.c| 8 +- drivers/media/rc/keymaps/rc-alink-dtu-m.c | 8 +- drivers/media/rc/keymaps/rc-anysee.c | 8 +- drivers/media/rc/keymaps/rc-apac-viewcomp.c| 8 +- drivers/media/rc/keymaps/rc-asus-pc39.c| 8 +- drivers/media/rc/keymaps/rc-asus-ps3-100.c | 8 +- drivers/media/rc/keymaps/rc-ati-tv-wonder-hd-600.c | 8 +- drivers/media/rc/keymaps/rc-ati-x10.c | 8 +- drivers/media/rc/keymaps/rc-avermedia-a16d.c | 8 +- drivers/media/rc/keymaps/rc-avermedia-cardbus.c| 8 +- drivers/media/rc/keymaps/rc-avermedia-dvbt.c | 8 +- drivers/media/rc/keymaps/rc-avermedia-m135a.c | 8 +- .../media/rc/keymaps/rc-avermedia-m733a-rm-k6.c| 8 +- drivers/media/rc/keymaps/rc-avermedia-rm-ks.c | 8 +- drivers/media/rc/keymaps/rc-avermedia.c| 8 +- drivers/media/rc/keymaps/rc-avertv-303.c | 8 +- drivers/media/rc/keymaps/rc-azurewave-ad-tu700.c | 8 +- drivers/media/rc/keymaps/rc-behold-columbus.c | 8 +- drivers/media/rc/keymaps/rc-behold.c | 8 +- drivers/media/rc/keymaps/rc-budget-ci-old.c| 8 +- drivers/media/rc/keymaps/rc-cec.c | 2 +- drivers/media/rc/keymaps/rc-cinergy-1400.c | 8 +- drivers/media/rc/keymaps/rc-cinergy.c | 8 +- drivers/media/rc/keymaps/rc-d680-dmb.c | 8 +- drivers/media/rc/keymaps/rc-delock-61959.c | 8 +- drivers/media/rc/keymaps/rc-dib0700-nec.c | 8 +- drivers/media/rc/keymaps/rc-dib0700-rc5.c | 8 +- drivers/media/rc/keymaps/rc-digitalnow-tinytwin.c | 8 +- drivers/media/rc/keymaps/rc-digittrade.c | 8 +- drivers/media/rc/keymaps/rc-dm1105-nec.c | 8 +- drivers/media/rc/keymaps/rc-dntv-live-dvb-t.c
[PATCH 4/6] [media] rc: ensure we do not read out of bounds
If rc_validate_filter() is called for CEC or XMP, then we would read beyond the end of the array. Suggested-by: Hans Verkuil Signed-off-by: Sean Young --- drivers/media/rc/rc-main.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index f306e67b8b66..7aaf28bcb01e 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -733,7 +733,7 @@ EXPORT_SYMBOL_GPL(rc_keydown_notimeout); static int rc_validate_filter(struct rc_dev *dev, struct rc_scancode_filter *filter) { - static u32 masks[] = { + static const u32 masks[] = { [RC_TYPE_RC5] = 0x1f7f, [RC_TYPE_RC5X_20] = 0x1f7f3f, [RC_TYPE_RC5_SZ] = 0x2fff, @@ -757,6 +757,9 @@ static int rc_validate_filter(struct rc_dev *dev, u32 s = filter->data; enum rc_type protocol = dev->wakeup_protocol; + if (protocol >= ARRAY_SIZE(masks)) + return -EINVAL; + switch (protocol) { case RC_TYPE_NECX: if s >> 16) ^ ~(s >> 8)) & 0xff) == 0) -- 2.13.4
[PATCH 2/6] [media] rc: saa7134: add trailing space for timely decoding
The gpio-ir-recv driver adds timeouts which the saa7134 lacks; this causes keypress not to arrive, and to only arrive once more IR is received. This is what the commit below calls "ghost keypresses", and it does not solve the issue completely. This makes the IR on the HVR-1150 much more reliable and responsive. Fixes: 3f5c4c73322e ("[media] rc: fix ghost keypresses with certain hw") Signed-off-by: Sean Young --- drivers/media/pci/saa7134/saa7134-input.c | 3 +++ drivers/media/rc/gpio-ir-recv.c | 21 - drivers/media/rc/rc-ir-raw.c | 19 ++- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c index 81e27ddcf6df..4b58c129be92 100644 --- a/drivers/media/pci/saa7134/saa7134-input.c +++ b/drivers/media/pci/saa7134/saa7134-input.c @@ -873,6 +873,9 @@ int saa7134_input_init1(struct saa7134_dev *dev) rc->dev.parent = &dev->pci->dev; rc->map_name = ir_codes; rc->driver_name = MODULE_NAME; + rc->min_timeout = 1; + rc->timeout = IR_DEFAULT_TIMEOUT; + rc->max_timeout = 10 * IR_DEFAULT_TIMEOUT; err = rc_register_device(rc); if (err) diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c index 561c27a4be64..512e31593a77 100644 --- a/drivers/media/rc/gpio-ir-recv.c +++ b/drivers/media/rc/gpio-ir-recv.c @@ -30,7 +30,6 @@ struct gpio_rc_dev { struct rc_dev *rcdev; int gpio_nr; bool active_low; - struct timer_list flush_timer; }; #ifdef CONFIG_OF @@ -94,26 +93,10 @@ static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id) if (rc < 0) goto err_get_value; - mod_timer(&gpio_dev->flush_timer, - jiffies + nsecs_to_jiffies(gpio_dev->rcdev->timeout)); - - ir_raw_event_handle(gpio_dev->rcdev); - err_get_value: return IRQ_HANDLED; } -static void flush_timer(unsigned long arg) -{ - struct gpio_rc_dev *gpio_dev = (struct gpio_rc_dev *)arg; - DEFINE_IR_RAW_EVENT(ev); - - ev.timeout = true; - ev.duration = gpio_dev->rcdev->timeout; - ir_raw_event_store(gpio_dev->rcdev, &ev); - ir_raw_event_handle(gpio_dev->rcdev); -} - static int gpio_ir_recv_probe(struct platform_device *pdev) { struct gpio_rc_dev *gpio_dev; @@ -171,9 +154,6 @@ static int gpio_ir_recv_probe(struct platform_device *pdev) gpio_dev->gpio_nr = pdata->gpio_nr; gpio_dev->active_low = pdata->active_low; - setup_timer(&gpio_dev->flush_timer, flush_timer, - (unsigned long)gpio_dev); - rc = gpio_request(pdata->gpio_nr, "gpio-ir-recv"); if (rc < 0) goto err_gpio_request; @@ -216,7 +196,6 @@ static int gpio_ir_recv_remove(struct platform_device *pdev) struct gpio_rc_dev *gpio_dev = platform_get_drvdata(pdev); free_irq(gpio_to_irq(gpio_dev->gpio_nr), gpio_dev); - del_timer_sync(&gpio_dev->flush_timer); rc_unregister_device(gpio_dev->rcdev); gpio_free(gpio_dev->gpio_nr); kfree(gpio_dev); diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c index 07a694298119..ef5efd994eef 100644 --- a/drivers/media/rc/rc-ir-raw.c +++ b/drivers/media/rc/rc-ir-raw.c @@ -134,9 +134,13 @@ int ir_raw_event_store_edge(struct rc_dev *dev, enum raw_event_type type) dev->raw->last_event = now; dev->raw->last_type = type; - if (!timer_pending(&dev->raw->edge_handle)) + /* timer could be set to timeout (125ms by default) */ + if (!timer_pending(&dev->raw->edge_handle) || + time_after(dev->raw->edge_handle.expires, + jiffies + msecs_to_jiffies(15))) { mod_timer(&dev->raw->edge_handle, jiffies + msecs_to_jiffies(15)); + } return rc; } @@ -491,6 +495,19 @@ EXPORT_SYMBOL(ir_raw_encode_scancode); static void edge_handle(unsigned long arg) { struct rc_dev *dev = (struct rc_dev *)arg; + ktime_t interval = ktime_get() - dev->raw->last_event; + + if (interval >= dev->timeout) { + DEFINE_IR_RAW_EVENT(ev); + + ev.timeout = true; + ev.duration = interval; + + ir_raw_event_store(dev, &ev); + } else { + mod_timer(&dev->raw->edge_handle, + jiffies + nsecs_to_jiffies(dev->timeout - interval)); + } ir_raw_event_handle(dev); } -- 2.13.4
[PATCH 6/6] [media] rc: saa7134: raw decoder can support any protocol
Any protocol for which we have a software decoder, can be enabled. Without this only the protocol of the default keymap can be used (rc-5). Signed-off-by: Sean Young --- drivers/media/pci/saa7134/saa7134-input.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c index b1f3769eac45..9337e4615519 100644 --- a/drivers/media/pci/saa7134/saa7134-input.c +++ b/drivers/media/pci/saa7134/saa7134-input.c @@ -860,8 +860,10 @@ int saa7134_input_init1(struct saa7134_dev *dev) rc->priv = dev; rc->open = saa7134_ir_open; rc->close = saa7134_ir_close; - if (raw_decode) + if (raw_decode) { rc->driver_type = RC_DRIVER_IR_RAW; + rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER; + } rc->device_name = ir->name; rc->input_phys = ir->phys; -- 2.13.4
[PATCH 3/6] [media] rc: simplify ir_raw_event_store_edge()
Since commit 12749b198fa4 ("[media] rc: saa7134: add trailing space for timely decoding"), the workaround of inserting reset events is no longer needed. Verified on a HVR-1150. Fixes: 3f5c4c73322e ("[media] rc: fix ghost keypresses with certain hw") Signed-off-by: Sean Young --- drivers/media/pci/saa7134/saa7134-input.c | 2 +- drivers/media/rc/gpio-ir-recv.c | 6 +- drivers/media/rc/img-ir/img-ir-raw.c | 4 ++-- drivers/media/rc/rc-core-priv.h | 1 - drivers/media/rc/rc-ir-raw.c | 31 +-- include/media/rc-core.h | 10 +- 6 files changed, 10 insertions(+), 44 deletions(-) diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c index 4b58c129be92..e7b386ee3ff9 100644 --- a/drivers/media/pci/saa7134/saa7134-input.c +++ b/drivers/media/pci/saa7134/saa7134-input.c @@ -1055,7 +1055,7 @@ static int saa7134_raw_decode_irq(struct saa7134_dev *dev) saa_clearb(SAA7134_GPIO_GPMODE3, SAA7134_GPIO_GPRESCAN); saa_setb(SAA7134_GPIO_GPMODE3, SAA7134_GPIO_GPRESCAN); space = saa_readl(SAA7134_GPIO_GPSTATUS0 >> 2) & ir->mask_keydown; - ir_raw_event_store_edge(dev->remote->dev, space ? IR_SPACE : IR_PULSE); + ir_raw_event_store_edge(dev->remote->dev, !space); return 1; } diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c index 512e31593a77..24c7ac8f1b82 100644 --- a/drivers/media/rc/gpio-ir-recv.c +++ b/drivers/media/rc/gpio-ir-recv.c @@ -76,7 +76,6 @@ static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id) struct gpio_rc_dev *gpio_dev = dev_id; int gval; int rc = 0; - enum raw_event_type type = IR_SPACE; gval = gpio_get_value(gpio_dev->gpio_nr); @@ -86,10 +85,7 @@ static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id) if (gpio_dev->active_low) gval = !gval; - if (gval == 1) - type = IR_PULSE; - - rc = ir_raw_event_store_edge(gpio_dev->rcdev, type); + rc = ir_raw_event_store_edge(gpio_dev->rcdev, gval == 1); if (rc < 0) goto err_get_value; diff --git a/drivers/media/rc/img-ir/img-ir-raw.c b/drivers/media/rc/img-ir/img-ir-raw.c index 7f23a863310c..64714efc1145 100644 --- a/drivers/media/rc/img-ir/img-ir-raw.c +++ b/drivers/media/rc/img-ir/img-ir-raw.c @@ -40,9 +40,9 @@ static void img_ir_refresh_raw(struct img_ir_priv *priv, u32 irq_status) /* report the edge to the IR raw decoders */ if (ir_status) /* low */ - ir_raw_event_store_edge(rc_dev, IR_SPACE); + ir_raw_event_store_edge(rc_dev, false); else /* high */ - ir_raw_event_store_edge(rc_dev, IR_PULSE); + ir_raw_event_store_edge(rc_dev, true); ir_raw_event_handle(rc_dev); } diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h index cae13efc1a88..5e5b10fbc47e 100644 --- a/drivers/media/rc/rc-core-priv.h +++ b/drivers/media/rc/rc-core-priv.h @@ -41,7 +41,6 @@ struct ir_raw_event_ctrl { /* fifo for the pulse/space durations */ DECLARE_KFIFO(kfifo, struct ir_raw_event, MAX_IR_EVENT_SIZE); ktime_t last_event; /* when last event occurred */ - enum raw_event_type last_type; /* last event type */ struct rc_dev *dev; /* pointer to the parent rc_dev */ /* edge driver */ struct timer_list edge_handle; diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c index ef5efd994eef..1761be8c7028 100644 --- a/drivers/media/rc/rc-ir-raw.c +++ b/drivers/media/rc/rc-ir-raw.c @@ -88,7 +88,7 @@ EXPORT_SYMBOL_GPL(ir_raw_event_store); /** * ir_raw_event_store_edge() - notify raw ir decoders of the start of a pulse/space * @dev: the struct rc_dev device descriptor - * @type: the type of the event that has occurred + * @pulse: true for pulse, false for space * * This routine (which may be called from an interrupt context) is used to * store the beginning of an ir pulse or space (or the start/end of ir @@ -96,43 +96,22 @@ EXPORT_SYMBOL_GPL(ir_raw_event_store); * hardware which does not provide durations directly but only interrupts * (or similar events) on state change. */ -int ir_raw_event_store_edge(struct rc_dev *dev, enum raw_event_type type) +int ir_raw_event_store_edge(struct rc_dev *dev, bool pulse) { ktime_t now; - s64 delta; /* ns */ DEFINE_IR_RAW_EVENT(ev); int rc = 0; - int delay; if (!dev->raw) return -EINVAL; now = ktime_get(); - delta = ktime_to_ns(ktime_sub(now, dev->raw->last_event)); - delay = MS_TO_NS(dev->input_dev->rep[REP_DELAY]); + ev.duration = ktime_sub(now, dev->raw->las
[PATCH 1/6] [media] rc-core: improve ir_raw_store_edge() handling
The gpio-ir-recv driver does many spurious wakeups (once per edge); the saa7134 driver has special handling to only wakeup 15ms after the first edge. Make this part of rc-core so gpio-ir-recv also benefits from this (so a rc-5 keypress now causes 3 wakeups rather than 24). Signed-off-by: Sean Young --- drivers/media/pci/saa7134/saa7134-input.c | 26 +- drivers/media/rc/rc-core-priv.h | 2 ++ drivers/media/rc/rc-ir-raw.c | 15 +++ 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c index ba1fc77a6f7b..81e27ddcf6df 100644 --- a/drivers/media/pci/saa7134/saa7134-input.c +++ b/drivers/media/pci/saa7134/saa7134-input.c @@ -452,13 +452,6 @@ static void saa7134_input_timer(unsigned long data) mod_timer(&ir->timer, jiffies + msecs_to_jiffies(ir->polling)); } -static void ir_raw_decode_timer_end(unsigned long data) -{ - struct saa7134_dev *dev = (struct saa7134_dev *)data; - - ir_raw_event_handle(dev->remote->dev); -} - static int __saa7134_ir_start(void *priv) { struct saa7134_dev *dev = priv; @@ -514,10 +507,6 @@ static int __saa7134_ir_start(void *priv) (unsigned long)dev); ir->timer.expires = jiffies + HZ; add_timer(&ir->timer); - } else if (ir->raw_decode) { - /* set timer_end for code completion */ - setup_timer(&ir->timer, ir_raw_decode_timer_end, - (unsigned long)dev); } return 0; @@ -535,7 +524,7 @@ static void __saa7134_ir_stop(void *priv) if (!ir->running) return; - if (ir->polling || ir->raw_decode) + if (ir->polling) del_timer_sync(&ir->timer); ir->running = false; @@ -1057,7 +1046,6 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) static int saa7134_raw_decode_irq(struct saa7134_dev *dev) { struct saa7134_card_ir *ir = dev->remote; - unsigned long timeout; int space; /* Generate initial event */ @@ -1066,17 +1054,5 @@ static int saa7134_raw_decode_irq(struct saa7134_dev *dev) space = saa_readl(SAA7134_GPIO_GPSTATUS0 >> 2) & ir->mask_keydown; ir_raw_event_store_edge(dev->remote->dev, space ? IR_SPACE : IR_PULSE); - /* -* Wait 15 ms from the start of the first IR event before processing -* the event. This time is enough for NEC protocol. May need adjustments -* to work with other protocols. -*/ - smp_mb(); - - if (!timer_pending(&ir->timer)) { - timeout = jiffies + msecs_to_jiffies(15); - mod_timer(&ir->timer, timeout); - } - return 1; } diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h index b3e7cac2c3ee..cae13efc1a88 100644 --- a/drivers/media/rc/rc-core-priv.h +++ b/drivers/media/rc/rc-core-priv.h @@ -43,6 +43,8 @@ struct ir_raw_event_ctrl { ktime_t last_event; /* when last event occurred */ enum raw_event_type last_type; /* last event type */ struct rc_dev *dev; /* pointer to the parent rc_dev */ + /* edge driver */ + struct timer_list edge_handle; /* raw decoder state follows */ struct ir_raw_event prev_ev; diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c index b6d256f03847..07a694298119 100644 --- a/drivers/media/rc/rc-ir-raw.c +++ b/drivers/media/rc/rc-ir-raw.c @@ -133,6 +133,11 @@ int ir_raw_event_store_edge(struct rc_dev *dev, enum raw_event_type type) dev->raw->last_event = now; dev->raw->last_type = type; + + if (!timer_pending(&dev->raw->edge_handle)) + mod_timer(&dev->raw->edge_handle, + jiffies + msecs_to_jiffies(15)); + return rc; } EXPORT_SYMBOL_GPL(ir_raw_event_store_edge); @@ -483,6 +488,13 @@ int ir_raw_encode_scancode(enum rc_type protocol, u32 scancode, } EXPORT_SYMBOL(ir_raw_encode_scancode); +static void edge_handle(unsigned long arg) +{ + struct rc_dev *dev = (struct rc_dev *)arg; + + ir_raw_event_handle(dev); +} + /* * Used to (un)register raw event clients */ @@ -504,6 +516,8 @@ int ir_raw_event_prepare(struct rc_dev *dev) dev->raw->dev = dev; dev->change_protocol = change_protocol; + setup_timer(&dev->raw->edge_handle, edge_handle, + (unsigned long)dev); INIT_KFIFO(dev->raw->kfifo); return 0; @@ -555,6 +569,7 @@ void ir_raw_event_unregister(struct rc_dev *dev) return; kthread_stop(dev->raw->thread); + del_timer_sync(&dev->raw->edge_handle); mutex_lock(&ir_raw_handler_lock); list_del(&dev->raw->list); -- 2.13.4
Re: [PATCH] keytable.c: add support for the CEC protocol
On Mon, Aug 07, 2017 at 02:52:01PM +0200, Hans Verkuil wrote: > The CEC protocol wasn't known, so 'Supported protocols:' would just say > 'other' instead of 'cec'. > > Signed-off-by: Hans Verkuil Acked-by: Sean Young > --- > diff --git a/utils/keytable/keytable.c b/utils/keytable/keytable.c > index 634f4561..55abfc19 100644 > --- a/utils/keytable/keytable.c > +++ b/utils/keytable/keytable.c > @@ -106,6 +106,7 @@ enum sysfs_protocols { > SYSFS_RC6 = (1 << 10), > SYSFS_SHARP = (1 << 11), > SYSFS_XMP = (1 << 12), > + SYSFS_CEC = (1 << 13), > SYSFS_INVALID = 0, > }; > > @@ -138,6 +139,7 @@ const struct protocol_map_entry protocol_map[] = { > { "rc-6-mce", NULL, SYSFS_INVALID }, > { "sharp", NULL, SYSFS_SHARP }, > { "xmp","/xmp_decoder", SYSFS_XMP }, > + { "cec",NULL, SYSFS_CEC }, > { NULL, NULL, SYSFS_INVALID }, > };
Re: [PATCH 1/2] rc-main: support CEC protocol keypress timeout
On Mon, Aug 07, 2017 at 03:31:23PM +0200, Hans Verkuil wrote: > From: Hans Verkuil > > The CEC protocol has a keypress timeout of 550ms. Add support for this. > > Note: this really should be defined in a protocol struct. That is a good point, the names of the protocol variants and scancode bits can also go into such a struct. This can be done at some point in the future. Acked-by: Sean Young > > Signed-off-by: Hans Verkuil > --- > drivers/media/rc/rc-main.c | 17 +++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c > index a9eba0013525..073407a78f70 100644 > --- a/drivers/media/rc/rc-main.c > +++ b/drivers/media/rc/rc-main.c > @@ -33,6 +33,9 @@ > /* FIXME: IR_KEYPRESS_TIMEOUT should be protocol specific */ > #define IR_KEYPRESS_TIMEOUT 250 > > +/* The CEC protocol needs a timeout of 550 */ > +#define IR_KEYPRESS_CEC_TIMEOUT 550 > + > /* Used to keep track of known keymaps */ > static LIST_HEAD(rc_map_list); > static DEFINE_SPINLOCK(rc_map_lock); > @@ -622,7 +625,12 @@ void rc_repeat(struct rc_dev *dev) > if (!dev->keypressed) > goto out; > > - dev->keyup_jiffies = jiffies + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT); > + if (dev->last_protocol == RC_TYPE_CEC) > + dev->keyup_jiffies = jiffies + > + msecs_to_jiffies(IR_KEYPRESS_CEC_TIMEOUT); > + else > + dev->keyup_jiffies = jiffies + > + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT); > mod_timer(&dev->timer_keyup, dev->keyup_jiffies); > > out: > @@ -692,7 +700,12 @@ void rc_keydown(struct rc_dev *dev, enum rc_type > protocol, u32 scancode, u8 togg > ir_do_keydown(dev, protocol, scancode, keycode, toggle); > > if (dev->keypressed) { > - dev->keyup_jiffies = jiffies + > msecs_to_jiffies(IR_KEYPRESS_TIMEOUT); > + if (protocol == RC_TYPE_CEC) > + dev->keyup_jiffies = jiffies + > + msecs_to_jiffies(IR_KEYPRESS_CEC_TIMEOUT); > + else > + dev->keyup_jiffies = jiffies + > + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT); > mod_timer(&dev->timer_keyup, dev->keyup_jiffies); > } > spin_unlock_irqrestore(&dev->keylock, flags); > -- > 2.13.2
Re: [PATCH v2 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver
Hi Maxime, Thank you for the patch. On Thursday 20 Jul 2017 11:23:02 Maxime Ripard wrote: > The Cadence CSI-2 RX Controller is an hardware block meant to be used as a > bridge between a CSI-2 bus and pixel grabbers. > > It supports operating with internal or external D-PHY, with up to 4 lanes, > or without any D-PHY. The current code only supports the former case. > > It also support dynamic mapping of the CSI-2 virtual channels to the > associated pixel grabbers, but that isn't allowed at the moment either. > > Signed-off-by: Maxime Ripard > --- > drivers/media/platform/Kconfig | 1 + > drivers/media/platform/Makefile | 2 + > drivers/media/platform/cadence/Kconfig | 12 + > drivers/media/platform/cadence/Makefile | 1 + > drivers/media/platform/cadence/cdns-csi2rx.c | 413 > 5 files changed, 429 insertions(+) > create mode 100644 drivers/media/platform/cadence/Kconfig > create mode 100644 drivers/media/platform/cadence/Makefile > create mode 100644 drivers/media/platform/cadence/cdns-csi2rx.c [snip] > diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c > b/drivers/media/platform/cadence/cdns-csi2rx.c new file mode 100644 > index ..9a58f275f53c > --- /dev/null > +++ b/drivers/media/platform/cadence/cdns-csi2rx.c > @@ -0,0 +1,413 @@ > +/* > + * Driver for Cadence MIPI-CSI2 RX Controller v1.3 > + * > + * Copyright (C) 2017 Cadence Design Systems Inc. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by > the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > + > +#define CSI2RX_DEVICE_CFG_REG0x000 > + > +#define CSI2RX_SOFT_RESET_REG0x004 > +#define CSI2RX_SOFT_RESET_PROTOCOL BIT(1) > +#define CSI2RX_SOFT_RESET_FRONT BIT(0) > + > +#define CSI2RX_STATIC_CFG_REG0x008 > + > +#define CSI2RX_STREAM_BASE(n)(((n) + 1) * 0x100) > + > +#define CSI2RX_STREAM_CTRL_REG(n)(CSI2RX_STREAM_BASE(n) + 0x000) > +#define CSI2RX_STREAM_CTRL_START BIT(0) > + > +#define CSI2RX_STREAM_DATA_CFG_REG(n)(CSI2RX_STREAM_BASE(n) + 0x008) > +#define CSI2RX_STREAM_DATA_CFG_EN_VC_SELECT BIT(31) > +#define CSI2RX_STREAM_DATA_CFG_VC_SELECT(n) BIT((n) + 16) > + > +#define CSI2RX_STREAM_CFG_REG(n) (CSI2RX_STREAM_BASE(n) + 0x00c) > +#define CSI2RX_STREAM_CFG_FIFO_MODE_LARGE_BUF(1 << 8) > + > +#define CSI2RX_STREAMS_MAX 4 > + > +enum csi2rx_pads { > + CSI2RX_PAD_SINK, > + CSI2RX_PAD_SOURCE_VC0, > + CSI2RX_PAD_SOURCE_VC1, > + CSI2RX_PAD_SOURCE_VC2, > + CSI2RX_PAD_SOURCE_VC3, > + CSI2RX_PAD_MAX, > +}; > + > +struct csi2rx_priv { > + struct device *dev; > + > + void __iomem*base; > + struct clk *sys_clk; > + struct clk *p_clk; > + struct clk *p_free_clk; > + struct clk *pixel_clk[CSI2RX_STREAMS_MAX]; > + struct clk *dphy_rx_clk; > + > + u8 lanes; > + u8 max_lanes; > + u8 max_streams; > + boolcdns_dphy; > + > + struct v4l2_subdev subdev; > + struct media_padpads[CSI2RX_PAD_MAX]; > + > + /* Remote sensor */ > + struct v4l2_async_subdevasd; > + struct device_node *sensor_node; > + struct v4l2_subdev *sensor_subdev; > + int sensor_pad; The remove subdev might not be a sensor. I would rename those three fields to source_* (don't forget to update the comment accordingly). > +}; > + > +static inline > +struct csi2rx_priv *v4l2_subdev_to_csi2rx(struct v4l2_subdev *subdev) > +{ > + return container_of(subdev, struct csi2rx_priv, subdev); > +} > + > +static void csi2rx_reset(struct csi2rx_priv *csi2rx) > +{ > + writel(CSI2RX_SOFT_RESET_PROTOCOL | CSI2RX_SOFT_RESET_FRONT, > +csi2rx->base + CSI2RX_SOFT_RESET_REG); > + > + udelay(10); > + > + writel(0, csi2rx->base + CSI2RX_SOFT_RESET_REG); > +} > + > +static int csi2rx_start(struct csi2rx_priv *csi2rx) > +{ > + u32 reg; > + int i; i is never negative, you can make it an unsigned int. > + csi2rx_reset(csi2rx); > + > + // TODO: modify the mapping of the DPHY lanes? The mapping should be read from DT and applied here. > + reg = readl(csi2rx->base + CSI2RX_STATIC_CFG_REG); > + reg &= ~GENMASK(11, 8); Could you define a macro for this register field ? > + writel(reg |
Re: [PATCH 2/2] cec: fix remote control passthrough
On Mon, Aug 07, 2017 at 03:31:24PM +0200, Hans Verkuil wrote: > From: Hans Verkuil > > The 'Press and Hold' operation was not correctly implemented, in > particular the requirement that the repeat doesn't start until > the second identical keypress arrives. The REP_DELAY value also > had to be adjusted (see the comment in the code) to achieve the > desired behavior. I'm afraid I've caused some confusion; I had not read your last message about autorepeat on irc correctly, when I said "exactly". So if the input layer has not received a key up event after a key down event, after REP_DELAY it will generate another key down event every REP_PERIOD. So for example, here I'm holding a button on a rc-5 device for some time. Comments on lines with parentheses. # ir-keytable -t Testing events. Please, press CTRL-C to abort. 1502138577.703695: event type EV_MSC(0x04): scancode = 0x1e11 (each time a driver receives something, scancode is reported.) 1502138577.703695: event type EV_KEY(0x01) key_down: KEY_VOLUMEDOWN(0x0072) 1502138577.703695: event type EV_SYN(0x00). 1502138577.817682: event type EV_MSC(0x04): scancode = 0x1e11 1502138577.817682: event type EV_SYN(0x00). (rc-5 repeats the command after 115ms). 1502138577.930676: event type EV_MSC(0x04): scancode = 0x1e11 1502138577.930676: event type EV_SYN(0x00). 1502138578.044682: event type EV_MSC(0x04): scancode = 0x1e11 1502138578.044682: event type EV_SYN(0x00). 1502138578.181690: event type EV_MSC(0x04): scancode = 0x1e11 1502138578.181690: event type EV_SYN(0x00). 1502138578.205667: event type EV_KEY(0x01) key_down: KEY_VOLUMEDOWN(0x0072) (this is 500ms after the initial key down, so this key down is generated by the input layer). 1502138578.205667: event type EV_SYN(0x00). 1502138578.333667: event type EV_KEY(0x01) key_down: KEY_VOLUMEDOWN(0x0072) (this is 500 + 125 ms, so another key down event generated by input layer). 1502138578.333667: event type EV_SYN(0x00). 1502138578.437662: event type EV_KEY(0x01) key_up: KEY_VOLUMEDOWN(0x0072) 1502138578.437662: event type EV_SYN(0x00). (key up generated by rc-core after 250ms after last scancode received) So I think the autorepeat can do exactly what you want, without cec having any special code for it. On a side note, here you can see that for rc-5 the IR_KEYPRESS_TIMEOUT should be 115ms (with a little extra margin). My apologies. Sean > > The 'enabled_protocols' field was also never set, fix that too. Since > CEC is a fixed protocol the driver has to set this field. > > Signed-off-by: Hans Verkuil > --- > drivers/media/cec/cec-adap.c | 56 > > drivers/media/cec/cec-core.c | 13 ++ > include/media/cec.h | 5 > 3 files changed, 69 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c > index 1a021828c8d4..6a2f38f000e8 100644 > --- a/drivers/media/cec/cec-adap.c > +++ b/drivers/media/cec/cec-adap.c > @@ -1766,6 +1766,9 @@ static int cec_receive_notify(struct cec_adapter *adap, > struct cec_msg *msg, > int la_idx = cec_log_addr2idx(adap, dest_laddr); > bool from_unregistered = init_laddr == 0xf; > struct cec_msg tx_cec_msg = { }; > +#ifdef CONFIG_MEDIA_CEC_RC > + int scancode; > +#endif > > dprintk(2, "%s: %*ph\n", __func__, msg->len, msg->msg); > > @@ -1854,11 +1857,9 @@ static int cec_receive_notify(struct cec_adapter > *adap, struct cec_msg *msg, >*/ > case 0x60: > if (msg->len == 2) > - rc_keydown(adap->rc, RC_TYPE_CEC, > -msg->msg[2], 0); > + scancode = msg->msg[2]; > else > - rc_keydown(adap->rc, RC_TYPE_CEC, > -msg->msg[2] << 8 | msg->msg[3], 0); > + scancode = msg->msg[2] << 8 | msg->msg[3]; > break; > /* >* Other function messages that are not handled. > @@ -1871,11 +1872,54 @@ static int cec_receive_notify(struct cec_adapter > *adap, struct cec_msg *msg, >*/ > case 0x56: case 0x57: > case 0x67: case 0x68: case 0x69: case 0x6a: > + scancode = -1; > break; > default: > - rc_keydown(adap->rc, RC_TYPE_CEC, msg->msg[2], 0); > + scancode = msg->msg[2]; > + break; > + } > + > + /* Was repeating, but keypress timed out */ > + if (adap->rc_repeating && !adap->rc->keypressed) { > + adap->rc_repeating = false; > + adap->rc_last_scancode = -1; > + } > + /* Different keypress from last time, ends repeat mode */ > + if (adap->rc_last_scancode != scancode) { > +
Re: [PATCH 2/2] v4l2-flash-led-class: Create separate sub-devices for indicators
On Wed, Jul 19, 2017 at 02:02:46PM +0200, Pavel Machek wrote: > On Tue 2017-07-18 21:41:07, Sakari Ailus wrote: > > The V4L2 flash interface allows controlling multiple LEDs through a single > > sub-devices if, and only if, these LEDs are of different types. This > > approach scales badly for flash controllers that drive multiple flash LEDs > > or for LED specific associations. Essentially, the original assumption of a > > LED driver chip that drives a single flash LED and an indicator LED is no > > longer valid. > > > > Address the matter by registering one sub-device per LED. > > > > Signed-off-by: Sakari Ailus > > Acked-by: Pavel Machek Thanks! > Does anything need to be done with drivers/media/i2c/adp1653.c ? No, it's stand-alone and does not use the V4L2 flash LED class framework-let. -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH] media: i2c: OV5647: gate clock lane before stream on
Hi all, 2017-08-07 22:48 GMT+08:00 Luis Oliveira : > Hi all, > > I'm new here, I got to be Maintainer of this driver by the old Maintainer > recommendation. Still getting the hang of it :) > > On 07-Aug-17 13:26, Philipp Zabel wrote: >> Hi Jacob, >> >> On Mon, 2017-08-07 at 19:06 +0800, Jacob Chen wrote: >> [...] >> --- a/drivers/media/i2c/ov5647.c >> +++ b/drivers/media/i2c/ov5647.c >> @@ -253,6 +253,10 @@ static int ov5647_stream_on(struct v4l2_subdev *sd) >> { >> int ret; >> >> + ret = ov5647_write(sd, 0x4800, 0x04); >> + if (ret < 0) >> + return ret; >> + >> >> So this clears BIT(1) (force clock lane to low power mode) and BIT(5) >> (gate clock lane while idle) that were set by ov5647_stream_off() during >> __sensor_init() due to the change below. >> >> Is there a reason, btw, that this driver is full of magic register >> addresses and values? A few #defines would make this a lot more >> readable. >> > > For what I can see I agree that a few register name setting could be done. > >> ret = ov5647_write(sd, 0x4202, 0x00); >> if (ret < 0) >> return ret; >> @@ -264,6 +268,10 @@ static int ov5647_stream_off(struct v4l2_subdev *sd) >> { >> int ret; >> >> + ret = ov5647_write(sd, 0x4800, 0x25); >> + if (ret < 0) >> + return ret; >> + >> ret = ov5647_write(sd, 0x4202, 0x0f); >> if (ret < 0) >> return ret; >> @@ -320,7 +328,7 @@ static int __sensor_init(struct v4l2_subdev *sd) >> return ret; >> } >> >> - return ov5647_write(sd, 0x4800, 0x04); >> + return ov5647_stream_off(sd); >> >> I see now that BIT(2) (keep bus in LP-11 while idle) is and was always >> set. So the change is that initially, additionally to LP-11 mode, the >> clock lane is gated and forced into low power mode, as well? >> > > This is my interpretation as well. > BIT(0) are not necessary, just i saw many driver have set it both with BIT(5). >> } >> >> static int ov5647_sensor_power(struct v4l2_subdev *sd, int on) >> -- >> 2.7.4 >> > > Can anyone comment on it? > > I saw there is a same discussion in > https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_9569031_&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=eMn12aiiNuIDjtRi5xEzC7tWJkpra2vl_XYFVvfxIGE&m=eortcRXje2uLyZNI_-Uw3Ur_z24tb-e4pZfom7WhdE0&s=6sLc76bhjR0IdaA3ArZ7F7slgtcyGz8pDTzAF_CBLno&e= > There is a comment in i.MX CSI2 driver. > " > Configure MIPI Camera Sensor to put all Tx lanes in LP-11 state. > This must be carried out by the MIPI sensor's s_power(ON) subdev > op. > " > That's what this patch do, sensor driver should make sure that clock > lanes are in stop state while not streaming. This is not the same, as far as I can tell. BIT(5) is just clock lane gating, as you describe above. To put the bus into LP-11 state, BIT(2) needs to be set. >>> >>> Yeah, but i double that clock lane is not in LP11 when continue clock >>> mode is enabled. > > I think by spec it shouldn't got to stopstate in continuous clock. > >> >> If indeed LP-11 state is not achieved while the sensor is idle, as long >> as BIT(5) is cleared, I think this patch is correct. >> >> regards >> Philipp >> > > As far as I understand, bit[5] set to 1 will force clock lane to be gated (in > other words it will be forced to be in LP-11 if there are no packets to > transmit). But also LP-11 must not be achieved with the BIT(5) cleared (free > running mode)? > > Sorry if I misunderstood something. > I do some experiments. I didn't have instruments to test, so i just observe it through phy registers. If BIT(5) are cleared in "ov5647_sensor_power" and do nothing about it in "ov5647_stream_on", Phy didn't get a SoT from sensor in "ov5647_stream_on" and it keep its clock lane in lp mode. if BIT(5) are set in "ov5647_sensor_power", and cleared in "ov5647_stream_on". Phy will get a SoT and the clock lane will enter hs mode. So i'm pretty sure that LP-11 must not be achieved with the BIT(5) cleared. > regards, > Luis >
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Tue Aug 8 05:00:16 CEST 2017 media-tree git hash:da48c948c263c9d87dfc64566b3373a858cc8aa2 media_build git hash: f01a9176bb03f22e3cd3b70282bd7fd272e504ae v4l-utils git hash: 172b663ea1fd16909a390f508851098a58061d9b gcc version:i686-linux-gcc (GCC) 7.1.0 sparse version: v0.5.0 smatch version: v0.5.0-3553-g78b2ea6 host hardware: x86_64 host os:4.11.0-164 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: WARNINGS linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-blackfin-bf561: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.36.4-i686: WARNINGS linux-2.6.37.6-i686: WARNINGS linux-2.6.38.8-i686: WARNINGS linux-2.6.39.4-i686: WARNINGS linux-3.0.60-i686: WARNINGS linux-3.1.10-i686: WARNINGS linux-3.2.37-i686: WARNINGS linux-3.3.8-i686: WARNINGS linux-3.4.27-i686: ERRORS linux-3.5.7-i686: WARNINGS linux-3.6.11-i686: WARNINGS linux-3.7.4-i686: WARNINGS linux-3.8-i686: WARNINGS linux-3.9.2-i686: WARNINGS linux-3.10.1-i686: WARNINGS linux-3.11.1-i686: WARNINGS linux-3.12.67-i686: WARNINGS linux-3.13.11-i686: WARNINGS linux-3.14.9-i686: WARNINGS linux-3.15.2-i686: WARNINGS linux-3.16.7-i686: WARNINGS linux-3.17.8-i686: ERRORS linux-3.18.7-i686: ERRORS linux-3.19-i686: WARNINGS linux-4.0.9-i686: WARNINGS linux-4.1.33-i686: WARNINGS linux-4.2.8-i686: WARNINGS linux-4.3.6-i686: WARNINGS linux-4.4.22-i686: WARNINGS linux-4.5.7-i686: WARNINGS linux-4.6.7-i686: WARNINGS linux-4.7.5-i686: WARNINGS linux-4.8-i686: OK linux-4.9.26-i686: OK linux-4.10.14-i686: OK linux-4.11-i686: OK linux-4.12.1-i686: OK linux-2.6.36.4-x86_64: WARNINGS linux-2.6.37.6-x86_64: WARNINGS linux-2.6.38.8-x86_64: WARNINGS linux-2.6.39.4-x86_64: WARNINGS linux-3.0.60-x86_64: WARNINGS linux-3.1.10-x86_64: WARNINGS linux-3.2.37-x86_64: WARNINGS linux-3.3.8-x86_64: WARNINGS linux-3.4.27-x86_64: ERRORS linux-3.5.7-x86_64: WARNINGS linux-3.6.11-x86_64: WARNINGS linux-3.7.4-x86_64: WARNINGS linux-3.8-x86_64: WARNINGS linux-3.9.2-x86_64: WARNINGS linux-3.10.1-x86_64: WARNINGS linux-3.11.1-x86_64: WARNINGS linux-3.12.67-x86_64: WARNINGS linux-3.13.11-x86_64: WARNINGS linux-3.14.9-x86_64: WARNINGS linux-3.15.2-x86_64: WARNINGS linux-3.16.7-x86_64: WARNINGS linux-3.17.8-x86_64: WARNINGS linux-3.18.7-x86_64: WARNINGS linux-3.19-x86_64: WARNINGS linux-4.0.9-x86_64: WARNINGS linux-4.1.33-x86_64: WARNINGS linux-4.2.8-x86_64: WARNINGS linux-4.3.6-x86_64: WARNINGS linux-4.4.22-x86_64: WARNINGS linux-4.5.7-x86_64: WARNINGS linux-4.6.7-x86_64: WARNINGS linux-4.7.5-x86_64: WARNINGS linux-4.8-x86_64: WARNINGS linux-4.9.26-x86_64: WARNINGS linux-4.10.14-x86_64: WARNINGS linux-4.11-x86_64: WARNINGS linux-4.12.1-x86_64: WARNINGS apps: WARNINGS spec-git: OK sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html