Re: [PATCH v2 1/5] V4L2: I2C: ML86V7667 video decoder driver
On Sun April 21 2013 20:40:30 Sergei Shtylyov wrote: From: Vladimir Barinov vladimir.bari...@cogentembedded.com Add OKI Semiconductor ML86V7667 video decoder driver. Signed-off-by: Vladimir Barinov vladimir.bari...@cogentembedded.com [Sergei: added v4l2_device_unregister_subdev() call to the error cleanup path of ml86v7667_probe(); some cleanup.] Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com --- Changes since the original posting: - fixed ACCC_CHROMA_CB_MASK; - got rid from the autodetection feature; - removed querystd() method calls from other methods; - removed deprecated g_chip_ident() method. drivers/media/i2c/Kconfig |9 drivers/media/i2c/Makefile|1 drivers/media/i2c/ml86v7667.c | 473 ++ 3 files changed, 483 insertions(+) Index: renesas/drivers/media/i2c/Kconfig === --- renesas.orig/drivers/media/i2c/Kconfig +++ renesas/drivers/media/i2c/Kconfig @@ -227,6 +227,15 @@ config VIDEO_KS0127 To compile this driver as a module, choose M here: the module will be called ks0127. +config VIDEO_ML86V7667 + tristate OKI ML86V7667 video decoder + depends on VIDEO_V4L2 I2C + ---help--- + Support for the OKI Semiconductor ML86V7667 video decoder. + + To compile this driver as a module, choose M here: the + module will be called ml86v7667. + config VIDEO_SAA7110 tristate Philips SAA7110 video decoder depends on VIDEO_V4L2 I2C Index: renesas/drivers/media/i2c/Makefile === --- renesas.orig/drivers/media/i2c/Makefile +++ renesas/drivers/media/i2c/Makefile @@ -64,3 +64,4 @@ obj-$(CONFIG_VIDEO_AS3645A) += as3645a.o obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o obj-$(CONFIG_VIDEO_AK881X) += ak881x.o obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o +obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o Index: renesas/drivers/media/i2c/ml86v7667.c === --- /dev/null +++ renesas/drivers/media/i2c/ml86v7667.c @@ -0,0 +1,473 @@ +/* + * OKI Semiconductor ML86V7667 video decoder driver + * + * Author: Vladimir Barinov sou...@cogentembedded.com + * Copyright (C) 2013 Cogent Embedded, Inc. + * Copyright (C) 2013 Renesas Solutions Corp. + * + * 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 linux/init.h +#include linux/module.h +#include linux/i2c.h +#include linux/slab.h +#include linux/videodev2.h +#include media/v4l2-chip-ident.h This include should be removed as well. +#include media/v4l2-subdev.h +#include media/v4l2-device.h +#include media/v4l2-ioctl.h +#include media/v4l2-ctrls.h + +#define DRV_NAME ml86v7667 + +/* Subaddresses */ +#define MRA_REG 0x00 /* Mode Register A */ +#define MRC_REG 0x02 /* Mode Register C */ +#define LUMC_REG 0x0C /* Luminance Control */ +#define CLC_REG 0x10 /* Contrast level control */ +#define SSEPL_REG0x11 /* Sync separation level */ +#define CHRCA_REG0x12 /* Chrominance Control A */ +#define ACCC_REG 0x14 /* ACC Loop filter Chrominance control */ +#define ACCRC_REG0x15 /* ACC Reference level control */ +#define HUE_REG 0x16 /* Hue control */ +#define ADC2_REG 0x1F /* ADC Register 2 */ +#define PLLR1_REG0x20 /* PLL Register 1 */ +#define STATUS_REG 0x2C /* STATUS Register */ + +/* Mode Register A register bits */ +#define MRA_OUTPUT_MODE_MASK (3 6) +#define MRA_ITUR_BT601 (1 6) +#define MRA_ITUR_BT656 (0 6) +#define MRA_INPUT_MODE_MASK (7 3) +#define MRA_PAL_BT601(4 3) +#define MRA_NTSC_BT601 (0 3) +#define MRA_REGISTER_MODE(1 0) + +/* Mode Register C register bits */ +#define MRC_AUTOSELECT (1 7) + +/* Luminance Control register bits */ +#define LUMC_ONOFF_SHIFT 7 +#define LUMC_ONOFF_MASK (1 7) + +/* Contrast level control register bits */ +#define CLC_CONTRAST_ONOFF (1 7) +#define CLC_CONTRAST_MASK0x0F + +/* Sync separation level register bits */ +#define SSEPL_LUMINANCE_ONOFF(1 7) +#define SSEPL_LUMINANCE_MASK 0x7F + +/* Chrominance Control A register bits */ +#define CHRCA_MODE_SHIFT 6 +#define CHRCA_MODE_MASK (1 6) + +/* ACC Loop filter Chrominance control register bits */ +#define ACCC_CHROMA_CR_SHIFT 3 +#define ACCC_CHROMA_CR_MASK (7 3) +#define ACCC_CHROMA_CB_SHIFT 0
Re: [PATCH RFCv3 01/10] [media] Add initial SDR support at V4L2 API
On Sun April 21 2013 21:00:30 Mauro Carvalho Chehab wrote: Adds the basic API bits for Software Digital Radio (SDR) at the V4L2 API. A normal radio device is actually radio and hardware demod. As the demod is in hardware, several things that are required for the demodulate the signal (IF, bandwidth, sample rate, RF/IF filters, etc) are internal to the device and aren't part of the API. SDR radio, on the other hand, requires that every control needed by the tuner to be exposed on userspace, as userspace needs to adjust the software decoder to match it. As proposed at: http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/63123 Add a new device node for SDR devices, and a VIDIOC_QUERYCAP capability (V4L2_CAP_SDR) to indicate that a devnode is SDR. The stream output format also needs to be different, as it should output sample data, instead of video streams. As we need to document SDR, add one initial format there that will be later be used. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com --- Documentation/DocBook/media/v4l/common.xml | 35 ++ Documentation/DocBook/media/v4l/pixfmt.xml | 41 ++ Documentation/DocBook/media/v4l/v4l2.xml | 1 + .../DocBook/media/v4l/vidioc-querycap.xml | 7 drivers/media/v4l2-core/v4l2-dev.c | 3 ++ include/media/v4l2-dev.h | 3 +- include/uapi/linux/videodev2.h | 11 ++ 7 files changed, 100 insertions(+), 1 deletion(-) diff --git a/Documentation/DocBook/media/v4l/common.xml b/Documentation/DocBook/media/v4l/common.xml index 1ddf354..f59c67d 100644 --- a/Documentation/DocBook/media/v4l/common.xml +++ b/Documentation/DocBook/media/v4l/common.xml @@ -513,6 +513,41 @@ the v4l2-capability; returned by the VIDIOC-QUERYCAP; ioctl when the device has one or more modulators./para /section + section id=sdr_tuner +titleSoftware Digital Radio (SDR) Tuners and Modulators/title + +paraThose devices are special types of Radio devices that don't s/don't/do not/ 'don't' is a bit too informal IMHO. +have any analog demodulator. Instead, it samples the radio IF or baseband +and sends the samples for userspace to demodulate./para s/for/to/ +section Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFCv3 02/10] [media] videodev2.h: Remove the unused old V4L1 buffer types
On Sun April 21 2013 21:00:31 Mauro Carvalho Chehab wrote: Those aren't used anywhere for a long time. Drop it. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com Acked-by: Hans Verkuil hans.verk...@cisco.com --- include/uapi/linux/videodev2.h | 21 - 1 file changed, 21 deletions(-) diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 4aa24c3..5d8ee92 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -72,27 +72,6 @@ #define VIDEO_MAX_FRAME 32 #define VIDEO_MAX_PLANES 8 -#ifndef __KERNEL__ - -/* These defines are V4L1 specific and should not be used with the V4L2 API! - They will be removed from this header in the future. */ - -#define VID_TYPE_CAPTURE 1 /* Can capture */ -#define VID_TYPE_TUNER 2 /* Can tune */ -#define VID_TYPE_TELETEXT4 /* Does teletext */ -#define VID_TYPE_OVERLAY 8 /* Overlay onto frame buffer */ -#define VID_TYPE_CHROMAKEY 16 /* Overlay by chromakey */ -#define VID_TYPE_CLIPPING32 /* Can clip */ -#define VID_TYPE_FRAMERAM64 /* Uses the frame buffer memory */ -#define VID_TYPE_SCALES 128 /* Scalable */ -#define VID_TYPE_MONOCHROME 256 /* Monochrome only */ -#define VID_TYPE_SUBCAPTURE 512 /* Can capture subareas of the image */ -#define VID_TYPE_MPEG_DECODER1024/* Can decode MPEG streams */ -#define VID_TYPE_MPEG_ENCODER2048/* Can encode MPEG streams */ -#define VID_TYPE_MJPEG_DECODER 4096/* Can decode MJPEG streams */ -#define VID_TYPE_MJPEG_ENCODER 8192/* Can encode MJPEG streams */ -#endif - /* * M I S C E L L A N E O U S */ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFCv3 03/10] [media] V4L2 api: Add a buffer capture type for SDR
On Sun April 21 2013 21:00:32 Mauro Carvalho Chehab wrote: As SDR devices are not video, VBI or RDS devices, it needs its own buffer type for capture. Also, as discussed at: http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/63123 It should be a way to enumerate and select the formats that the hardware supports, as one hardware may accept more than one format (cx88, for example, can likely support several different formats, as it will depend on how the RISC code for it will be written). So, add a a new stream type (V4L2_BUF_TYPE_SDR_CAPTURE) at the V4L2 API, and the corresponding buffer function handlers. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com --- Documentation/DocBook/media/v4l/dev-capture.xml | 26 Documentation/DocBook/media/v4l/io.xml | 6 + drivers/media/v4l2-core/v4l2-ioctl.c| 32 + include/media/v4l2-ioctl.h | 8 +++ include/uapi/linux/videodev2.h | 3 ++- 5 files changed, 64 insertions(+), 11 deletions(-) diff --git a/Documentation/DocBook/media/v4l/dev-capture.xml b/Documentation/DocBook/media/v4l/dev-capture.xml index e1c5f94..7797d2d 100644 --- a/Documentation/DocBook/media/v4l/dev-capture.xml +++ b/Documentation/DocBook/media/v4l/dev-capture.xml @@ -44,7 +44,7 @@ all video capture devices./para /section section -titleImage Format Negotiation/title +titleStreaming Format Negotiation/title paraThe result of a capture operation is determined by cropping and image format parameters. The former select an area of the @@ -65,13 +65,18 @@ linkend=crop /./para paraTo query the current image format applications set the structfieldtype/structfield field of a v4l2-format; to -constantV4L2_BUF_TYPE_VIDEO_CAPTURE/constant or -constantV4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE/constant and call the -VIDIOC-G-FMT; ioctl with a pointer to this structure. Drivers fill -the v4l2-pix-format; structfieldpix/structfield or the -v4l2-pix-format-mplane; structfieldpix_mp/structfield member of the +constantV4L2_BUF_TYPE_VIDEO_CAPTURE/constant, +constantV4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE/constant or +constantV4L2_BUF_TYPE_VIDEO_SDR_CAPTURE/constant or s/V4L2_BUF_TYPE_VIDEO_SDR_CAPTURE/V4L2_BUF_TYPE_SDR_CAPTURE/ +and call the VIDIOC-G-FMT; ioctl with a pointer to this structure./para + Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFCv3 04/10] [media] V4L2 sdr API: Add fields for VIDIOC_[G|S]_TUNER
On Sun April 21 2013 21:00:33 Mauro Carvalho Chehab wrote: As discussed at: http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/63123 SDR radio require some other things at to set. VIDIOC_[G|S]_TUNER is currently not enough. I proposed there to create an extra ioctl for it, but the thing is that VIDIOC_[G|S]_TUNER is meant to set what's needed to configure the tuner. So, add there the missing bits to: - set/get the tuner sampling rate; - set/get the tuner low pass bandwidth filter; - get the tuner IF. VIDIOC_[G|S]_TUNER already provides: - tuner input switch; - tuner input name; - tuner capability flags; - tuner range; - signal strength; - afc. There are still 3 u32 reserved fields there that may be used for SDR in the future, and may be neded at the final version. For example, it may make sense to add a SDR flag with: - bandwidth inversion flag; - tuner PLL lock flag (or reuse signal strength for that). It also makes sense to add sample_rate range there. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com --- Documentation/DocBook/media/v4l/vidioc-g-tuner.xml | 30 ++--- drivers/media/tuners/tuner-xc2028.c| 2 ++ include/uapi/linux/videodev2.h | 38 -- 3 files changed, 63 insertions(+), 7 deletions(-) diff --git a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml index 6cc8201..b8a3bcf 100644 --- a/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml +++ b/Documentation/DocBook/media/v4l/vidioc-g-tuner.xml @@ -200,9 +200,10 @@ audio/entry constant_SAP/constant flag is cleared in the structfieldcapability/structfield field, the corresponding constantV4L2_TUNER_SUB_/constant flag must not be set -here./paraparaThis field is valid only if this is the tuner of the +here./para +paraThis field is valid only for if this is the tuner of the current video input, or when the structure refers to a radio -tuner./para/entry +tuner. This field is not used by SDR tuners./para/entry /row row entry__u32/entry @@ -216,7 +217,7 @@ unless the requested mode is invalid or unsupported. See xref the selected and received audio programs do not match./paraparaCurrently this is the only field of struct structnamev4l2_tuner/structname applications can -change./para/entry +change. This field is not used by SDR tuners./para/entry /row row entry__u32/entry @@ -234,7 +235,28 @@ settles at zero, ie; range is what? --/entry /row row entry__u32/entry - entrystructfieldreserved/structfield[4]/entry + entrystructfieldsample_rate/structfield/entry + entry spanname=hspanSampling rate used by a SDR tuner, in Hz. + This value is valid only for SDR tuners./entry Are frequencies 4GHz ever likely for SDR? If so, then a u64 might be needed here. + /row + row + entry__u32/entry + entrystructfieldbandwidth/structfield/entry + entry spanname=hspanBandwidth allowed by the SDR tuner + low-pass saw filter, in Hz. This value is valid only for + SDR tuners./entry + /row + row + entry__u32/entry + entrystructfieldint_freq/structfield/entry + entry spanname=hspanIntermediate Frequency (IF) used by + the tuner, in Hz. This value is valid only for + constantVIDIOC_G_TUNER/constant, and it is valid only + on SDR tuners./entry + /row + row + entry__u32/entry + entrystructfieldreserved/structfield[3]/entry It's not clear from this doc that this is an anonymous union. See e.g. the decoder_cmd documentation how that is done elsewhere. entry spanname=hspanReserved for future extensions. Drivers and applications must set the array to zero./entry /row diff --git a/drivers/media/tuners/tuner-xc2028.c b/drivers/media/tuners/tuner-xc2028.c index 878d2c4..c61163f 100644 --- a/drivers/media/tuners/tuner-xc2028.c +++ b/drivers/media/tuners/tuner-xc2028.c @@ -1020,6 +1020,8 @@ static int generic_set_freq(struct dvb_frontend *fe, u32 freq /* in HZ */, * Maybe this might also be needed for DTV. */ switch (new_type) { + default:/* SDR currently not supported */ + goto ret; case V4L2_TUNER_ANALOG_TV: rc = send_seq(priv, {0x00, 0x00}); diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 974c49d..c002030 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -160,6 +160,22 @@ enum v4l2_tuner_type { V4L2_TUNER_RADIO = 1, V4L2_TUNER_ANALOG_TV = 2,
Re: [PATCH v9 02/20] V4L2: support asynchronous subdevice registration
Hi Guennadi, Thanks for the patch! On Fri, Apr 12, 2013 at 9:10 PM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Currently bridge device drivers register devices for all subdevices synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor is attached to a video bridge device, the bridge driver will create an I2C device and wait for the respective I2C driver to probe. This makes linking of devices straight forward, but this approach cannot be used with intrinsically asynchronous and unordered device registration systems like the Flattened Device Tree. To support such systems this patch adds an asynchronous subdevice registration framework to V4L2. To use it respective (e.g. I2C) subdevice drivers must register themselves with the framework. A bridge driver on the other hand must register notification callbacks, that will be called upon various related events. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de with this https://patchwork.linuxtv.org/patch/18096/ patch applied, yo can add my Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com Tested-by: Lad, Prabhakar prabhakar.cse...@gmail.com Regards, --Prabhakar -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFCv3 07/10] [media] tuner-core: add SDR support for g_tuner
On Sun April 21 2013 21:00:36 Mauro Carvalho Chehab wrote: Properly initialize the fields for VIDIOC_G_TUNER, if the device is in SDR mode. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com --- drivers/media/v4l2-core/tuner-core.c | 29 + 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c index b97ec63..e54b5ae 100644 --- a/drivers/media/v4l2-core/tuner-core.c +++ b/drivers/media/v4l2-core/tuner-core.c @@ -1190,7 +1190,31 @@ static int tuner_g_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt) } /* radio mode */ - if (vt-type == t-mode) { + vt-capability |= V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO; + + if (V4L2_TUNER_IS_SDR(vt-type)) { + vt-rangelow = tv_range[0] * 16000; + vt-rangehigh = tv_range[1] * 16000; Why use tv_range for SDR? It's a bit odd for something called SD 'Radio'. Regards, Hans + else { + vt-rangelow = radio_range[0] * 16000; + vt-rangehigh = radio_range[1] * 16000; + } + /* Check if the radio device is active */ + if (vt-type != t-mode) + return 0; + + if (V4L2_TUNER_IS_SDR(vt-type)) { + if (fe_tuner_ops-get_bandwidth) + fe_tuner_ops-get_bandwidth(t-fe, + vt-bandwidth); + if (fe_tuner_ops-get_if_frequency) + fe_tuner_ops-get_if_frequency(t-fe, + vt-int_freq); + /* + * Sampe rate is not a tuner props - it is up to the + * bridge driver to fill it. + */ + } else { vt-rxsubchans = V4L2_TUNER_SUB_MONO | V4L2_TUNER_SUB_STEREO; if (fe_tuner_ops-get_status) { u32 tuner_status; @@ -1203,9 +1227,6 @@ static int tuner_g_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt) } vt-audmode = t-audmode; } - vt-capability |= V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO; - vt-rangelow = radio_range[0] * 16000; - vt-rangehigh = radio_range[1] * 16000; return 0; } -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFCv3 08/10] [media] tuner-core: store tuner ranges at tuner struct
On Sun April 21 2013 21:00:37 Mauro Carvalho Chehab wrote: Instead of using global values for tuner ranges, store them internally. That fixes the need of using a different range for SDR radio, and will help to latter add a tuner ops to retrieve the tuner range for SDR mode. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com --- drivers/media/v4l2-core/tuner-core.c | 59 ++-- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c index e54b5ae..abdcda4 100644 --- a/drivers/media/v4l2-core/tuner-core.c +++ b/drivers/media/v4l2-core/tuner-core.c @@ -67,8 +67,8 @@ static char secam[] = --; static char ntsc[] = -; module_param_named(debug, tuner_debug, int, 0644); -module_param_array(tv_range, int, NULL, 0644); -module_param_array(radio_range, int, NULL, 0644); +module_param_array(tv_range, int, NULL, 0444); +module_param_array(radio_range, int, NULL, 0444); Shouldn't we add a sdr_range here as well? module_param_string(pal, pal, sizeof(pal), 0644); module_param_string(secam, secam, sizeof(secam), 0644); module_param_string(ntsc, ntsc, sizeof(ntsc), 0644); @@ -134,6 +134,8 @@ struct tuner { unsigned inttype; /* chip type id */ void*config; const char *name; + + u32 radio_range[2], tv_range[2], sdr_range[2]; }; /* @@ -266,7 +268,7 @@ static void set_type(struct i2c_client *c, unsigned int type, struct dvb_tuner_ops *fe_tuner_ops = t-fe.ops.tuner_ops; struct analog_demod_ops *analog_ops = t-fe.ops.analog_ops; unsigned char buffer[4]; - int tune_now = 1; + int i, tune_now = 1; if (type == UNSET || type == TUNER_ABSENT) { tuner_dbg(tuner 0x%02x: Tuner type absent\n, c-addr); @@ -451,6 +453,13 @@ static void set_type(struct i2c_client *c, unsigned int type, set_tv_freq(c, t-tv_freq); } + /* Initializes the tuner ranges from modprobe parameters */ + for (i = 0; i 2; i++) { + t-radio_range[i] = radio_range[i] * 16000; + t-sdr_range[i] = tv_range[i] * 16000; + t-tv_range[i] = tv_range[i] * 16; + } + tuner_dbg(%s %s I2C addr 0x%02x with type %d used for 0x%02x\n, c-adapter-name, c-driver-driver.name, c-addr 1, type, t-mode_mask); @@ -831,16 +840,16 @@ static void set_tv_freq(struct i2c_client *c, unsigned int freq) tuner_warn(Tuner has no way to set tv freq\n); return; } - if (freq tv_range[0] * 16 || freq tv_range[1] * 16) { + if (freq t-tv_range[0] || freq t-tv_range[1]) { tuner_dbg(TV freq (%d.%02d) out of range (%d-%d)\n, -freq / 16, freq % 16 * 100 / 16, tv_range[0], -tv_range[1]); +freq / 16, freq % 16 * 100 / 16, t-tv_range[0] / 16, +t-tv_range[1] / 16); /* V4L2 spec: if the freq is not possible then the closest possible value should be selected */ - if (freq tv_range[0] * 16) - freq = tv_range[0] * 16; + if (freq t-tv_range[0]) + freq = t-tv_range[0]; else - freq = tv_range[1] * 16; + freq = t-tv_range[1]; } params.frequency = freq; tuner_dbg(tv freq set to %d.%02d\n, @@ -957,7 +966,7 @@ static void set_radio_freq(struct i2c_client *c, unsigned int freq) { struct tuner *t = to_tuner(i2c_get_clientdata(c)); struct analog_demod_ops *analog_ops = t-fe.ops.analog_ops; - + u32 *range; struct analog_parameters params = { .mode = t-mode, .audmode = t-audmode, @@ -972,16 +981,22 @@ static void set_radio_freq(struct i2c_client *c, unsigned int freq) tuner_warn(tuner has no way to set radio frequency\n); return; } - if (freq radio_range[0] * 16000 || freq radio_range[1] * 16000) { + + if (V4L2_TUNER_IS_SDR(t-mode)) + range = t-sdr_range; + else + range = t-radio_range; + + if (freq range[0] || freq range[1]) { tuner_dbg(radio freq (%d.%02d) out of range (%d-%d)\n, freq / 16000, freq % 16000 * 100 / 16000, -radio_range[0], radio_range[1]); +range[0] / 16000, range[1] / 16000); /* V4L2 spec: if the freq is not possible then the closest possible value should be selected */ - if (freq radio_range[0] * 16000) - freq = radio_range[0] * 16000; + if (freq range[0]) + freq = range[0]; else -
[PATCH] [media] hdpvr: error handling and alloc abuse cleanup.
Removed unnecessary use of kzalloc() in get_video_info(). Removed unnecessary get_video_info() call from hdpvr_device_init(). Cleaned up error handling in hdpvr_start_streaming() to ensure caller gets a failure status if device is not functioning properly. Cleaned up error handling in vidioc_querystd(), vidioc_query_dv_timings() and vidioc_g_fmt_vid_cap(). This change also causes vidioc_g_fmt_vid_cap() not to return -EFAULT when video lock is not detected, but return empty width/height fields (legacy mode only). This new behavior is supported by MythTV. Signed-off-by: Leonid Kegulskiy l...@lumanate.com --- drivers/media/usb/hdpvr/hdpvr-control.c | 20 ++--- drivers/media/usb/hdpvr/hdpvr-core.c|8 drivers/media/usb/hdpvr/hdpvr-video.c | 70 +-- drivers/media/usb/hdpvr/hdpvr.h |2 +- 4 files changed, 43 insertions(+), 57 deletions(-) diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c b/drivers/media/usb/hdpvr/hdpvr-control.c index ae8f229..16d2d64 100644 --- a/drivers/media/usb/hdpvr/hdpvr-control.c +++ b/drivers/media/usb/hdpvr/hdpvr-control.c @@ -45,20 +45,10 @@ int hdpvr_config_call(struct hdpvr_device *dev, uint value, u8 valbuf) return ret 0 ? ret : 0; } -struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) +int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf) { - struct hdpvr_video_info *vidinf = NULL; -#ifdef HDPVR_DEBUG - char print_buf[15]; -#endif int ret; - vidinf = kzalloc(sizeof(struct hdpvr_video_info), GFP_KERNEL); - if (!vidinf) { - v4l2_err(dev-v4l2_dev, out of memory\n); - goto err; - } - mutex_lock(dev-usbc_mutex); ret = usb_control_msg(dev-udev, usb_rcvctrlpipe(dev-udev, 0), @@ -74,6 +64,7 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) #ifdef HDPVR_DEBUG if (hdpvr_debug MSG_INFO) { + char print_buf[15]; hex_dump_to_buffer(dev-usbc_buf, 5, 16, 1, print_buf, sizeof(print_buf), 0); v4l2_dbg(MSG_INFO, hdpvr_debug, dev-v4l2_dev, @@ -82,12 +73,7 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) #endif mutex_unlock(dev-usbc_mutex); - if (!vidinf-width || !vidinf-height || !vidinf-fps) { - kfree(vidinf); - vidinf = NULL; - } -err: - return vidinf; + return ret 0 ? ret : 0; } int get_input_lines_info(struct hdpvr_device *dev) diff --git a/drivers/media/usb/hdpvr/hdpvr-core.c b/drivers/media/usb/hdpvr/hdpvr-core.c index 8247c19..cb69405 100644 --- a/drivers/media/usb/hdpvr/hdpvr-core.c +++ b/drivers/media/usb/hdpvr/hdpvr-core.c @@ -220,7 +220,6 @@ static int hdpvr_device_init(struct hdpvr_device *dev) { int ret; u8 *buf; - struct hdpvr_video_info *vidinf; if (device_authorization(dev)) return -EACCES; @@ -242,13 +241,6 @@ static int hdpvr_device_init(struct hdpvr_device *dev) control request returned %d\n, ret); mutex_unlock(dev-usbc_mutex); - vidinf = get_video_info(dev); - if (!vidinf) - v4l2_dbg(MSG_INFO, hdpvr_debug, dev-v4l2_dev, - no valid video signal or device init failed\n); - else - kfree(vidinf); - /* enable fan and bling leds */ mutex_lock(dev-usbc_mutex); buf[0] = 0x1; diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c index 774ba0e..5e8d6c2 100644 --- a/drivers/media/usb/hdpvr/hdpvr-video.c +++ b/drivers/media/usb/hdpvr/hdpvr-video.c @@ -277,20 +277,21 @@ error: static int hdpvr_start_streaming(struct hdpvr_device *dev) { int ret; - struct hdpvr_video_info *vidinf; + struct hdpvr_video_info vidinf; if (dev-status == STATUS_STREAMING) return 0; else if (dev-status != STATUS_IDLE) return -EAGAIN; - vidinf = get_video_info(dev); + ret = get_video_info(dev, vidinf); + if (ret)/* device is dead */ + return ret; /* let the caller know */ - if (vidinf) { + if (vidinf.width vidinf.height) { v4l2_dbg(MSG_BUFFER, hdpvr_debug, dev-v4l2_dev, -video signal: %dx%d@%dhz\n, vidinf-width, -vidinf-height, vidinf-fps); - kfree(vidinf); +video signal: %dx%d@%dhz\n, vidinf.width, +vidinf.height, vidinf.fps); /* start streaming 2 request */ ret = usb_control_msg(dev-udev, @@ -298,8 +299,12 @@ static int hdpvr_start_streaming(struct hdpvr_device *dev) 0xb8, 0x38, 0x1, 0, NULL, 0, 8000); v4l2_dbg(MSG_BUFFER, hdpvr_debug,
Re: [PATCH RFCv3 00/10] Add SDR at V4L2 API
On Sun April 21 2013 21:00:29 Mauro Carvalho Chehab wrote: This is a version 3 of the V4L2 API bits to support Software Digital Radio (SDR). Changes from version 2: - Patches got better described and named; - Merged all SDR analog TV into just one field; - Documented what parts of the [RFC v2013-04-11] SDR API REQUIREMENT SPECIFICATION are being addressed on the patches that touch on the V4L2 external API. With regards to the [RFC v2013-04-11] SDR requirements spec[1], v3 implements: - operation mode inquire (Rx/Tx) - implemented via VIDIOC_QUERYCAP; - get/set doesn't make sense, as different devnodes are used for RX and TX; - sampling resolution - implemented via VIDIOC_ENUM_FMT/VIDIOC_G_FMT/VIDIOC_S_FMT. - sampling rate - get/set - implemented via VIDIOC_G_TUNER/VIDIOC_S_TUNER. - inquire HW - TODO (planning is to also use the same ioctl's for it); - It may make sense to move it out of TUNER ioctl, as this is actually ADC/DAC settings. For the initial tests, I'll likely use as is, but IMO, the beter is to split it out of VIDIOC_G_TUNER/VIDIOC_S_TUNER. - inversion - TODO. If needed, probably will use VIDIOC_G_TUNER. - RF frequency - implemented using VIDIOC_S_FREQUENCY/VIDIOC_G_FREQUENCY - scale: 62.5 Hz. - TODO: add a lower scale, for very low freqs. - IF frequency - implemented via VIDIOC_G_TUNER/VIDIOC_S_TUNER. - tuner lock (frequency synthesizer / PLL) - TODO. May eventually use tuner-strength - tuner gains - TODO (V4L2 controls). - enable/disable auto gain - TODO (V4L2 controls). - tuner filters - TODO (V4L2 controls). - pass RF standard to tuner? - Partially implemented. It doesn't pass V4L2 analog TV types yet. - TODO: add VIDIOC_S_STD support for SDR radio - antenna switch - implemented via VIDIOC_G_TUNER/VIDIOC_S_TUNER. - external LNA - TODO (maybe there's already a V4L2 control for it) - device locking between multiple APIs - TODO (well, there are resource locking already between DVB and V4L at driver level) I have some patches here: http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/tuner adding support for tuner ownership (as discussed in the past) to V4L2. It's untested, and it needs to be integrated in DVB as well. The actual struct dealing with the tuner ownership is not tied to V4L2 so I am hoping it can be integrated in DVB without too much effort. As I understand it a pure DVB driver does not have to deal with tuner ownership, so this is a problem only if V4L2 is involved as well. I think that adding a struct tuner_owners pointer to some DVB internal struct would be sufficient, and the DVB core can check if it is non-NULL and attempt to take tuner ownership using it. With yet another tuner mode we need something like this. I don't have time to work further on this for the next few weeks, so I am hoping you might find it useful. I reviewed part of your patch series, but I did not go in-depth with the tuner-core patches. [1] http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/63323 TODO: - check if VB2 require changes (might require trivial ones, due to the new buffer type); - add a SDR driver testing the new features - likely a cx88 driver, but as cx88 doesn't use VB2, Of course, you can also convert cx88 to vb2 :-) I might use some other driver. The advantage of using cx88 is that it has 2 ADC, one baseband and another IF that can be used at the same time, 8-bits or 10-bits and up to ~35 MHz of sampling rate. Nice work! Regards, Hans Mauro Carvalho Chehab (10): [media] Add initial SDR support at V4L2 API [media] videodev2.h: Remove the unused old V4L1 buffer types [media] V4L2 api: Add a buffer capture type for SDR [media] V4L2 sdr API: Add fields for VIDIOC_[G|S]_TUNER [media] v4l2-ioctl: Add tuner ioctl support for SDR radio type [media] tuner-core: consider SDR as radio [media] tuner-core: add SDR support for g_tuner [media] tuner-core: store tuner ranges at tuner struct [media] tuner-core: add support to get the tuner frequency range [media] tuner-core: add support for SDR set_tuner Documentation/DocBook/media/v4l/common.xml | 35 +++ Documentation/DocBook/media/v4l/dev-capture.xml| 26 +- Documentation/DocBook/media/v4l/io.xml | 6 + Documentation/DocBook/media/v4l/pixfmt.xml | 41 +++ Documentation/DocBook/media/v4l/v4l2.xml | 1 +
Re: [PATCH] [media] hdpvr: error handling and alloc abuse cleanup.
On Mon April 22 2013 09:23:03 Leonid Kegulskiy wrote: Removed unnecessary use of kzalloc() in get_video_info(). Removed unnecessary get_video_info() call from hdpvr_device_init(). Cleaned up error handling in hdpvr_start_streaming() to ensure caller gets a failure status if device is not functioning properly. Cleaned up error handling in vidioc_querystd(), vidioc_query_dv_timings() and vidioc_g_fmt_vid_cap(). This change also causes vidioc_g_fmt_vid_cap() not to return -EFAULT when video lock is not detected, but return empty width/height fields (legacy mode only). This new behavior is supported by MythTV. Hi Leonid, Can you split this patch up in smaller pieces? Basically in the order of the changes you mention in your commit log. That makes it a lot easier to review. In particular the last change (-EFAULT) needs its own patch. That makes it easy to revert should that become necessary. Regards, Hans Signed-off-by: Leonid Kegulskiy l...@lumanate.com --- drivers/media/usb/hdpvr/hdpvr-control.c | 20 ++--- drivers/media/usb/hdpvr/hdpvr-core.c|8 drivers/media/usb/hdpvr/hdpvr-video.c | 70 +-- drivers/media/usb/hdpvr/hdpvr.h |2 +- 4 files changed, 43 insertions(+), 57 deletions(-) diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c b/drivers/media/usb/hdpvr/hdpvr-control.c index ae8f229..16d2d64 100644 --- a/drivers/media/usb/hdpvr/hdpvr-control.c +++ b/drivers/media/usb/hdpvr/hdpvr-control.c @@ -45,20 +45,10 @@ int hdpvr_config_call(struct hdpvr_device *dev, uint value, u8 valbuf) return ret 0 ? ret : 0; } -struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) +int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf) { - struct hdpvr_video_info *vidinf = NULL; -#ifdef HDPVR_DEBUG - char print_buf[15]; -#endif int ret; - vidinf = kzalloc(sizeof(struct hdpvr_video_info), GFP_KERNEL); - if (!vidinf) { - v4l2_err(dev-v4l2_dev, out of memory\n); - goto err; - } - mutex_lock(dev-usbc_mutex); ret = usb_control_msg(dev-udev, usb_rcvctrlpipe(dev-udev, 0), @@ -74,6 +64,7 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) #ifdef HDPVR_DEBUG if (hdpvr_debug MSG_INFO) { + char print_buf[15]; hex_dump_to_buffer(dev-usbc_buf, 5, 16, 1, print_buf, sizeof(print_buf), 0); v4l2_dbg(MSG_INFO, hdpvr_debug, dev-v4l2_dev, @@ -82,12 +73,7 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) #endif mutex_unlock(dev-usbc_mutex); - if (!vidinf-width || !vidinf-height || !vidinf-fps) { - kfree(vidinf); - vidinf = NULL; - } -err: - return vidinf; + return ret 0 ? ret : 0; } int get_input_lines_info(struct hdpvr_device *dev) diff --git a/drivers/media/usb/hdpvr/hdpvr-core.c b/drivers/media/usb/hdpvr/hdpvr-core.c index 8247c19..cb69405 100644 --- a/drivers/media/usb/hdpvr/hdpvr-core.c +++ b/drivers/media/usb/hdpvr/hdpvr-core.c @@ -220,7 +220,6 @@ static int hdpvr_device_init(struct hdpvr_device *dev) { int ret; u8 *buf; - struct hdpvr_video_info *vidinf; if (device_authorization(dev)) return -EACCES; @@ -242,13 +241,6 @@ static int hdpvr_device_init(struct hdpvr_device *dev) control request returned %d\n, ret); mutex_unlock(dev-usbc_mutex); - vidinf = get_video_info(dev); - if (!vidinf) - v4l2_dbg(MSG_INFO, hdpvr_debug, dev-v4l2_dev, - no valid video signal or device init failed\n); - else - kfree(vidinf); - /* enable fan and bling leds */ mutex_lock(dev-usbc_mutex); buf[0] = 0x1; diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c index 774ba0e..5e8d6c2 100644 --- a/drivers/media/usb/hdpvr/hdpvr-video.c +++ b/drivers/media/usb/hdpvr/hdpvr-video.c @@ -277,20 +277,21 @@ error: static int hdpvr_start_streaming(struct hdpvr_device *dev) { int ret; - struct hdpvr_video_info *vidinf; + struct hdpvr_video_info vidinf; if (dev-status == STATUS_STREAMING) return 0; else if (dev-status != STATUS_IDLE) return -EAGAIN; - vidinf = get_video_info(dev); + ret = get_video_info(dev, vidinf); + if (ret)/* device is dead */ + return ret; /* let the caller know */ - if (vidinf) { + if (vidinf.width vidinf.height) { v4l2_dbg(MSG_BUFFER, hdpvr_debug, dev-v4l2_dev, - video signal: %dx%d@%dhz\n, vidinf-width, - vidinf-height, vidinf-fps); - kfree(vidinf); + video signal:
Re: [RFC] Motion Detection API
On Sun April 21 2013 14:04:26 Ismael Luceno wrote: On Fri, 12 Apr 2013 17:36:16 +0200 Hans Verkuil hverk...@xs4all.nl wrote: This RFC looks at adding support for motion detection to V4L2. This is the main missing piece that prevents the go7007 and solo6x10 drivers from being moved into mainline from the staging directory. ... Comment? Questions? +1. I like it :). Cool. Now all I need is time to actually implement this :-) Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL FOR 3.10] DiBxxxx: fixes and improvements
Hi Mauro, These patches contains some fixes and changes for the DiBcom demods and SIPs. Please merge for 3.10 if possible. The following changes since commit 60d509fa6a9c4653a86ad830e4c4b30360b23f0e: Linux 3.9-rc8 (2013-04-21 14:38:45 -0700) are available in the git repository at: git://git.linuxtv.org/pb/media_tree.git/ master for you to fetch changes up to 7e39d1958b186e5af259b9fde1a006853b4663ab: [media] dib8000: do not freeze AGCs by default (2013-04-22 10:06:48 +0200) Olivier Grenie (6): [media] dib8000: enhancement [media] dib7000p: enhancement [media] dib0090: enhancement [media] dib8096: enhancement [media] dib7090p: remove the support for the dib7090E [media] dib7090p: improve the support of the dib7090 and dib7790 Patrick Boettcher (1): [media] dib8000: do not freeze AGCs by default drivers/media/dvb-core/dvb-usb-ids.h |3 +- drivers/media/dvb-frontends/dib0090.c| 438 ++--- drivers/media/dvb-frontends/dib7000p.c | 17 +- drivers/media/dvb-frontends/dib7000p.h |7 + drivers/media/dvb-frontends/dib8000.c| 2239 +- drivers/media/dvb-frontends/dib8000.h|6 +- drivers/media/dvb-frontends/dibx000_common.h |3 +- drivers/media/usb/dvb-usb/dib0700_devices.c | 465 +++-- 8 files changed, 1780 insertions(+), 1398 deletions(-) regards, -- Patrick -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] V4L2: I2C: ML86V7667 video decoder driver
Hi Hans, Thank you for the review. Hans Verkuil wrote: +#include media/v4l2-chip-ident.h This include should be removed as well. ok + +static int ml86v7667_querystd(struct v4l2_subdev *sd, v4l2_std_id *std) +{ + struct ml86v7667_priv *priv = to_ml86v7667(sd); + + *std = priv-std; That's not right. querystd should attempt to detect the standard, that's what it is for. It should just return the detected standard, not actually change it. Ok. I've mixed the things up with your review on removing the autoselection feature and detection. Thx for pointing on this. +*/ + val = i2c_smbus_read_byte_data(client, STATUS_REG); + if (val 0) + return val; + + priv-std = val STATUS_NTSCPAL ? V4L2_STD_PAL : V4L2_STD_NTSC; Shouldn't this be 50 Hz vs 60 Hz formats? There are 60 Hz PAL standards and usually these devices detect 50 Hz vs 60 Hz, not NTSC vs PAL. In the reference manual it is not mentioned about 50/60Hz input format selection/detection but it mentioned just PAL/NTSC. The 50hz formats can be ether PAL and NTSC formats variants. The same is applied to 60Hz. In the ML86V7667 datasheet the description for STATUS register detection bit is just PAL/NTSC: $2C/STATUS [2] NTSC/PAL identification 0: NTSC /1: PAL If you assure me that I must judge their description as 50 vs 60Hz formats and not PAL/NTSC then I will make the change. Regards, Vladimir -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] V4L2: I2C: ML86V7667 video decoder driver
On Mon April 22 2013 10:39:42 Vladimir Barinov wrote: Hi Hans, Thank you for the review. Hans Verkuil wrote: +#include media/v4l2-chip-ident.h This include should be removed as well. ok + +static int ml86v7667_querystd(struct v4l2_subdev *sd, v4l2_std_id *std) +{ + struct ml86v7667_priv *priv = to_ml86v7667(sd); + + *std = priv-std; That's not right. querystd should attempt to detect the standard, that's what it is for. It should just return the detected standard, not actually change it. Ok. I've mixed the things up with your review on removing the autoselection feature and detection. Thx for pointing on this. + */ + val = i2c_smbus_read_byte_data(client, STATUS_REG); + if (val 0) + return val; + + priv-std = val STATUS_NTSCPAL ? V4L2_STD_PAL : V4L2_STD_NTSC; Shouldn't this be 50 Hz vs 60 Hz formats? There are 60 Hz PAL standards and usually these devices detect 50 Hz vs 60 Hz, not NTSC vs PAL. In the reference manual it is not mentioned about 50/60Hz input format selection/detection but it mentioned just PAL/NTSC. The 50hz formats can be ether PAL and NTSC formats variants. The same is applied to 60Hz. In the ML86V7667 datasheet the description for STATUS register detection bit is just PAL/NTSC: $2C/STATUS [2] NTSC/PAL identification 0: NTSC /1: PAL If you assure me that I must judge their description as 50 vs 60Hz formats and not PAL/NTSC then I will make the change. I can't judge that. Are there no status bits anywhere that tell you something about the number of lines per frame or the framerate? Are you able to test with a PAL-M or PAL-N(c) input? Can you ask the manufacturer for more information? If the answer to all of these is 'no', then stick to STD_PAL/NTSC. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] drm/exynos: prepare FIMD clocks
On Sunday 21 of April 2013 22:36:08 Inki Dae wrote: 2013/4/21 Tomasz Figa tomasz.f...@gmail.com Hi, On Monday 08 of April 2013 16:41:54 Viresh Kumar wrote: On 8 April 2013 16:37, Vikas Sajjan vikas.saj...@linaro.org wrote: While migrating to common clock framework (CCF), I found that the FIMD clocks were pulled down by the CCF. If CCF finds any clock(s) which has NOT been claimed by any of the drivers, then such clock(s) are PULLed low by CCF. Calling clk_prepare() for FIMD clocks fixes the issue. This patch also replaces clk_disable() with clk_unprepare() during exit, since clk_prepare() is called in fimd_probe(). I asked you about fixing your commit log too.. It still looks incorrect to me. This patch doesn't have anything to do with CCF pulling clocks down, but calling clk_prepare() before clk_enable() is must now.. that's it.. nothing more. I fully agree. The message should be something like: Common Clock Framework introduced the need to prepare clocks before enabling them, otherwise clk_enable() fails. This patch adds clk_prepare calls to the driver. and that's all. What you are observing as CCF pulling clocks down is the fact that clk_enable() fails if the clock is not prepared and so the clock is not enabled in result. Another thing is that CCF is not pulling anything down. GPIO pins can be pulled down (or up or not pulled), but clocks can be masked, gated or simply disabled - this does not imply their signal level. Signed-off-by: Vikas Sajjan vikas.saj...@linaro.org --- Changes since v3: - added clk_prepare() in fimd_probe() and clk_unprepare() in fimd_remove() as suggested by Viresh Kumar viresh.ku...@linaro.org Changes since v2: - moved clk_prepare_enable() and clk_disable_unprepare() from fimd_probe() to fimd_clock() as suggested by Inki Dae inki@samsung.com Changes since v1: - added error checking for clk_prepare_enable() and also replaced clk_disable() with clk_disable_unprepare() during exit. --- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 9537761..aa22370 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -934,6 +934,16 @@ static int fimd_probe(struct platform_device *pdev) return ret; } + ret = clk_prepare(ctx-bus_clk); + if (ret 0) + return ret; + + ret = clk_prepare(ctx-lcd_clk); + if (ret 0) { + clk_unprepare(ctx-bus_clk); + return ret; + } + Why not just simply use clk_prepare_enable() instead of all calls to clk_enable() in the driver? Same goes for s/clk_disable/clk_disable_unprepare/ . I agree with you. Using clk_prepare_enable() is more clear. Actually I had already commented on this. Please see the patch v2. But this way also looks good to me. Well, both versions are technically correct and will have the same effect for Exynos SoC clocks, since only enable/disable ops change hardware state. However if we look at general meaning of those generic ops, the clock will remain prepared for all the time the driver is loaded, even if the device Right, so I said previous one is more clear. I gonna revert current one and then merge previous one(v3) is runtime suspended. Again on Exynos SoCs this won't have any effect, but I think we should respect general Common Clock Framework semantics anyway. ctx-vidcon0 = pdata-vidcon0; ctx-vidcon1 = pdata-vidcon1; ctx-default_win = pdata-default_win; @@ -981,8 +991,8 @@ static int fimd_remove(struct platform_device *pdev) if (ctx-suspended) goto out; - clk_disable(ctx-lcd_clk); - clk_disable(ctx-bus_clk); + clk_unprepare(ctx-lcd_clk); + clk_unprepare(ctx-bus_clk); This looks wrong again.. You still need to call clk_disable() to make clk enabled count zero... Viresh is right again here. Ok, you two guys say together this looks wrong so I'd like to take more checking. I thought that clk-clk_enable is 1 at here and it would be 0 by pm_runtimg_put_sync(). Is there any my missing
Re: [PATCH v4] drm/exynos: prepare FIMD clocks
On Monday 22 of April 2013 10:44:00 Viresh Kumar wrote: On 21 April 2013 20:13, Tomasz Figa tomasz.f...@gmail.com wrote: 3) after those two changes, all that remains is to fix compliance with Common Clock Framework, in other words: s/clk_enable/clk_prepare_enable/ and s/clk_disable/clk_disable_unprepare/ We don't have to call clk_{un}prepare() everytime for your platform as you aren't doing anything in it. So just call them once at probe/remove and call clk_enable/disable everywhere else. Can you assure that in future SoCs, on which this driver will be used, this assumption will still hold true or even that in current Exynos driver this behavior won't be changed? Best regards, -- Tomasz Figa Samsung Poland RD Center SW Solution Development, Kernel and System Framework -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL FOR v3.10] Camera sensors patches
On Mon, Apr 22, 2013 at 01:14:07AM +0200, Laurent Pinchart wrote: I think that Mark's point was that the regulators should be provided by platform code (in the generic sense, it could be DT on ARM, board code, or a USB bridge driver for a webcam that uses the mt9p031 sensor) and used by the sensor driver. That's exactly what my mt9p031 patch does. Yes, you understood me perfectly - to a good approximation the matching up should be done by whatever the chip is soldered down to. signature.asc Description: Digital signature
Re: [PATCH v4] drm/exynos: prepare FIMD clocks
On 04/22/2013 11:56 AM, Tomasz Figa wrote: On Monday 22 of April 2013 10:44:00 Viresh Kumar wrote: On 21 April 2013 20:13, Tomasz Figa tomasz.f...@gmail.com wrote: 3) after those two changes, all that remains is to fix compliance with Common Clock Framework, in other words: s/clk_enable/clk_prepare_enable/ and s/clk_disable/clk_disable_unprepare/ We don't have to call clk_{un}prepare() everytime for your platform as you aren't doing anything in it. So just call them once at probe/remove and call clk_enable/disable everywhere else. Yes, I agree with that. Additionally clk_(un)prepare must not be called in atomic context, so some drivers will have to work like this anyway. Or the clocks could be prepared/unprepared in the device open/close file op for instance. Can you assure that in future SoCs, on which this driver will be used, this assumption will still hold true or even that in current Exynos driver this behavior won't be changed? -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] drm/exynos: prepare FIMD clocks
On 04/22/2013 12:03 PM, Inki Dae wrote: Also looks good to me. But what if power domain was disabled without pm runtime? In this case, you must enable the power domain at machine code or bootloader somewhere. This way would not only need some hard codes to turn the power domain on but also not manage power management fully. This is same as only the use of pm runtime interface(needing some hard codes without pm runtime) so I don't prefer to add clk_enable/disable to fimd probe(). I quite tend to force only the use of pm runtime as possible. So please add the hard codes to machine code or bootloader like you did for power domain if you want to use drm fimd without pm runtime. That's not how the runtime PM, clock subsystems work: 1) When CONFIG_PM_RUNTIME is disabled, all the used hardware must be kept powered on all the time. 2) Common Clock Framework will always gate all clocks that have zero enable_count. Note that CCF support for Exynos is already merged for 3.10 and it will be the only available clock support method for Exynos. AFAIK, drivers must work correctly in both cases, with CONFIG_PM_RUNTIME enabled and disabled. Then is the driver worked correctly if the power domain to this device was disabled at bootloader without CONFIG_PM_RUNTIME and with clk_enable()? I think, in this case, the device wouldn't be worked correctly because the power of the device remains off. So you must enable the power domain somewhere. What is the difference between these two cases? How about making the driver dependant on PM_RUNTIME and making it always use pm_runtime_* API, regardless if the platform actually implements runtime PM or not ? Is there any issue in using the Runtime PM core always, rather than coding any workarounds in drivers when PM_RUNTIME is disabled ? Thanks, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 0/4] media: davinci: vpif: capture/display support for async subdevice probing
From: Lad, Prabhakar prabhakar.cse...@gmail.com This patch series adds support for vpif capture and display driver to support asynchronously register subdevices. The first two patches adds asynchronous probing for adv7343 and tvp514x respectively. Need for this support: Currently bridge device drivers register devices for all subdevices synchronously, typically, during their probing. E.g. if an I2C CMOS sensor is attached to a video bridge device, the bridge driver will create an I2C device and wait for the respective I2C driver to probe. This makes linking of devices straight forward, but this approach cannot be used with intrinsically asynchronous and unordered device registration systems like the Flattened Device Tree. This series is dependent on following patches: 1: https://patchwork.kernel.org/patch/2437111/ 2: https://patchwork.linuxtv.org/patch/18096/ Changes for v2: 1: added support v4l-async support for vpif display driver. 2: added asynchronous probing for adv7343. Lad, Prabhakar (4): media: i2c: adv7343: add support for asynchronous probing media: i2c: tvp514x: add support for asynchronous probing media: davinci: vpif: capture: add V4L2-async support media: davinci: vpif: display: add V4L2-async support drivers/media/i2c/adv7343.c | 17 ++- drivers/media/i2c/tvp514x.c | 23 ++- drivers/media/platform/davinci/vpif_capture.c | 151 -- drivers/media/platform/davinci/vpif_capture.h |2 + drivers/media/platform/davinci/vpif_display.c | 219 +++-- drivers/media/platform/davinci/vpif_display.h |3 +- include/media/davinci/vpif_types.h|4 + 7 files changed, 273 insertions(+), 146 deletions(-) -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 1/4] media: i2c: adv7343: add support for asynchronous probing
From: Lad, Prabhakar prabhakar.cse...@gmail.com Both synchronous and asynchronous adv7343 subdevice probing is supported by this patch. Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Hans Verkuil hverk...@xs4all.nl Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Mauro Carvalho Chehab mche...@redhat.com --- drivers/media/i2c/adv7343.c | 17 + 1 files changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/adv7343.c b/drivers/media/i2c/adv7343.c index 9fc2b98..5b1417b 100644 --- a/drivers/media/i2c/adv7343.c +++ b/drivers/media/i2c/adv7343.c @@ -27,6 +27,7 @@ #include linux/uaccess.h #include media/adv7343.h +#include media/v4l2-async.h #include media/v4l2-device.h #include media/v4l2-chip-ident.h #include media/v4l2-ctrls.h @@ -44,6 +45,7 @@ struct adv7343_state { struct v4l2_subdev sd; struct v4l2_ctrl_handler hdl; const struct adv7343_platform_data *pdata; + struct v4l2_async_subdev_list asdl; u8 reg00; u8 reg01; u8 reg02; @@ -455,16 +457,22 @@ static int adv7343_probe(struct i2c_client *client, ADV7343_GAIN_DEF); state-sd.ctrl_handler = state-hdl; if (state-hdl.error) { - int err = state-hdl.error; - - v4l2_ctrl_handler_free(state-hdl); - return err; + err = state-hdl.error; + goto done; } v4l2_ctrl_handler_setup(state-hdl); err = adv7343_initialize(state-sd); if (err) + goto done; + + state-sd.dev = client-dev; + err = v4l2_async_register_subdev(state-sd); + +done: + if (err 0) v4l2_ctrl_handler_free(state-hdl); + return err; } @@ -473,6 +481,7 @@ static int adv7343_remove(struct i2c_client *client) struct v4l2_subdev *sd = i2c_get_clientdata(client); struct adv7343_state *state = to_state(sd); + v4l2_async_unregister_subdev(state-sd); v4l2_device_unregister_subdev(sd); v4l2_ctrl_handler_free(state-hdl); -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 2/4] media: i2c: tvp514x: add support for asynchronous probing
From: Lad, Prabhakar prabhakar.cse...@gmail.com Both synchronous and asynchronous tvp514x subdevice probing is supported by this patch. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Hans Verkuil hverk...@xs4all.nl Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Mauro Carvalho Chehab mche...@redhat.com --- drivers/media/i2c/tvp514x.c | 23 --- 1 files changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c index ab8f3fe..887bd93 100644 --- a/drivers/media/i2c/tvp514x.c +++ b/drivers/media/i2c/tvp514x.c @@ -36,6 +36,7 @@ #include linux/module.h #include linux/v4l2-mediabus.h +#include media/v4l2-async.h #include media/v4l2-device.h #include media/v4l2-common.h #include media/v4l2-mediabus.h @@ -1109,9 +1110,9 @@ tvp514x_probe(struct i2c_client *client, const struct i2c_device_id *id) /* Register with V4L2 layer as slave device */ sd = decoder-sd; v4l2_i2c_subdev_init(sd, client, tvp514x_ops); - strlcpy(sd-name, TVP514X_MODULE_NAME, sizeof(sd-name)); #if defined(CONFIG_MEDIA_CONTROLLER) + strlcpy(sd-name, TVP514X_MODULE_NAME, sizeof(sd-name)); decoder-pad.flags = MEDIA_PAD_FL_SOURCE; decoder-sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; decoder-sd.entity.flags |= MEDIA_ENT_T_V4L2_SUBDEV_DECODER; @@ -1138,16 +1139,23 @@ tvp514x_probe(struct i2c_client *client, const struct i2c_device_id *id) sd-ctrl_handler = decoder-hdl; if (decoder-hdl.error) { ret = decoder-hdl.error; - - v4l2_ctrl_handler_free(decoder-hdl); - return ret; + goto done; } v4l2_ctrl_handler_setup(decoder-hdl); - v4l2_info(sd, %s decoder driver registered !!\n, sd-name); - - return 0; + decoder-sd.dev = client-dev; + ret = v4l2_async_register_subdev(decoder-sd); + if (!ret) + v4l2_info(sd, %s decoder driver registered !!\n, sd-name); +done: + if (ret 0) { + v4l2_ctrl_handler_free(decoder-hdl); +#if defined(CONFIG_MEDIA_CONTROLLER) + media_entity_cleanup(decoder-sd.entity); +#endif + } + return ret; } /** @@ -1162,6 +1170,7 @@ static int tvp514x_remove(struct i2c_client *client) struct v4l2_subdev *sd = i2c_get_clientdata(client); struct tvp514x_decoder *decoder = to_decoder(sd); + v4l2_async_unregister_subdev(decoder-sd); v4l2_device_unregister_subdev(sd); #if defined(CONFIG_MEDIA_CONTROLLER) media_entity_cleanup(decoder-sd.entity); -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC v2 3/4] media: davinci: vpif: capture: add V4L2-async support
From: Lad, Prabhakar prabhakar.cse...@gmail.com Add support for asynchronous subdevice probing, using the v4l2-async API. The legacy synchronous mode is still supported too, which allows to gradually update drivers and platforms. Signed-off-by: Prabhakar Lad prabhakar.cse...@gmail.com Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de Cc: Hans Verkuil hans.verk...@cisco.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Mauro Carvalho Chehab mche...@redhat.com --- drivers/media/platform/davinci/vpif_capture.c | 151 + drivers/media/platform/davinci/vpif_capture.h |2 + include/media/davinci/vpif_types.h|2 + 3 files changed, 108 insertions(+), 47 deletions(-) diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c index a1b42b0..d723b58 100644 --- a/drivers/media/platform/davinci/vpif_capture.c +++ b/drivers/media/platform/davinci/vpif_capture.c @@ -24,6 +24,7 @@ #include linux/platform_device.h #include linux/slab.h +#include media/v4l2-async.h #include media/v4l2-chip-ident.h #include media/v4l2-ioctl.h @@ -2045,6 +2046,76 @@ vpif_init_free_channel_objects: return err; } +static int vpif_async_bound(struct v4l2_async_notifier *notifier, + struct v4l2_async_subdev_list *asdl) +{ + struct v4l2_subdev *subdev = v4l2_async_to_subdev(asdl); + int i = 0; + + for (i = 0; i vpif_obj.config-subdev_count; i++) + if (!strcmp(vpif_obj.config-subdev_info[i].name, + subdev-name)) { + vpif_obj.sd[i] = subdev; + return 0; + } + + return -EINVAL; +} + +static int vpif_probe_complete(void) +{ + struct common_obj *common; + struct channel_obj *ch; + int i, j, err, k; + + for (j = 0; j VPIF_CAPTURE_MAX_DEVICES; j++) { + ch = vpif_obj.dev[j]; + ch-channel_id = j; + common = (ch-common[VPIF_VIDEO_INDEX]); + spin_lock_init(common-irqlock); + mutex_init(common-lock); + ch-video_dev-lock = common-lock; + /* Initialize prio member of channel object */ + v4l2_prio_init(ch-prio); + video_set_drvdata(ch-video_dev, ch); + + /* select input 0 */ + err = vpif_set_input(vpif_obj.config, ch, 0); + if (err) + goto probe_out; + + err = video_register_device(ch-video_dev, + VFL_TYPE_GRABBER, (j ? 1 : 0)); + if (err) + goto probe_out; + } + + v4l2_info(vpif_obj.v4l2_dev, VPIF capture driver initialized\n); + return 0; + +probe_out: + for (k = 0; k j; k++) { + /* Get the pointer to the channel object */ + ch = vpif_obj.dev[k]; + /* Unregister video device */ + video_unregister_device(ch-video_dev); + } + kfree(vpif_obj.sd); + for (i = 0; i VPIF_CAPTURE_MAX_DEVICES; i++) { + ch = vpif_obj.dev[i]; + /* Note: does nothing if ch-video_dev == NULL */ + video_device_release(ch-video_dev); + } + v4l2_device_unregister(vpif_obj.v4l2_dev); + + return err; +} + +static int vpif_async_complete(struct v4l2_async_notifier *notifier) +{ + return vpif_probe_complete(); +} + /** * vpif_probe : This function probes the vpif capture driver * @pdev: platform device pointer @@ -2055,12 +2126,10 @@ vpif_init_free_channel_objects: static __init int vpif_probe(struct platform_device *pdev) { struct vpif_subdev_info *subdevdata; - struct vpif_capture_config *config; - int i, j, k, err; + int i, j, err; int res_idx = 0; struct i2c_adapter *i2c_adap; struct channel_obj *ch; - struct common_obj *common; struct video_device *vfd; struct resource *res; int subdev_count; @@ -2137,10 +2206,9 @@ static __init int vpif_probe(struct platform_device *pdev) } } - i2c_adap = i2c_get_adapter(1); - config = pdev-dev.platform_data; + vpif_obj.config = pdev-dev.platform_data; - subdev_count = config-subdev_count; + subdev_count = vpif_obj.config-subdev_count; vpif_obj.sd = kzalloc(sizeof(struct v4l2_subdev *) * subdev_count, GFP_KERNEL); if (vpif_obj.sd == NULL) { @@ -2149,53 +2217,42 @@ static __init int vpif_probe(struct platform_device *pdev) goto vpif_sd_error; } - for (i = 0; i subdev_count; i++) { - subdevdata = config-subdev_info[i]; - vpif_obj.sd[i] = - v4l2_i2c_new_subdev_board(vpif_obj.v4l2_dev, -
[PATCH RFC v2 4/4] media: davinci: vpif: display: add V4L2-async support
From: Lad, Prabhakar prabhakar.cse...@gmail.com Add support for asynchronous subdevice probing, using the v4l2-async API. The legacy synchronous mode is still supported too, which allows to gradually update drivers and platforms. Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de Cc: Hans Verkuil hans.verk...@cisco.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Mauro Carvalho Chehab mche...@redhat.com --- drivers/media/platform/davinci/vpif_display.c | 219 +++-- drivers/media/platform/davinci/vpif_display.h |3 +- include/media/davinci/vpif_types.h|2 + 3 files changed, 136 insertions(+), 88 deletions(-) diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c index 7b17368..4f53730 100644 --- a/drivers/media/platform/davinci/vpif_display.c +++ b/drivers/media/platform/davinci/vpif_display.c @@ -19,6 +19,7 @@ #include linux/platform_device.h #include linux/slab.h +#include media/v4l2-async.h #include media/v4l2-chip-ident.h #include media/v4l2-ioctl.h @@ -1684,6 +1685,105 @@ vpif_init_free_channel_objects: return err; } +static int vpif_async_bound(struct v4l2_async_notifier *notifier, + struct v4l2_async_subdev_list *asdl) +{ + struct v4l2_subdev *subdev = v4l2_async_to_subdev(asdl); + int i = 0; + + for (i = 0; i vpif_obj.config-subdev_count; i++) + if (!strcmp(vpif_obj.config-subdevinfo[i].name, + subdev-name)) { + vpif_obj.sd[i] = subdev; + vpif_obj.sd[i]-grp_id = 1 i; + return 0; + } + + return -EINVAL; +} + +static int vpif_probe_complete(void) +{ + struct common_obj *common; + struct channel_obj *ch; + int j, err, k; + + for (j = 0; j VPIF_DISPLAY_MAX_DEVICES; j++) { + ch = vpif_obj.dev[j]; + /* Initialize field of the channel objects */ + atomic_set(ch-usrs, 0); + for (k = 0; k VPIF_NUMOBJECTS; k++) { + ch-common[k].numbuffers = 0; + common = ch-common[k]; + common-io_usrs = 0; + common-started = 0; + spin_lock_init(common-irqlock); + mutex_init(common-lock); + common-numbuffers = 0; + common-set_addr = NULL; + common-ytop_off = 0; + common-ybtm_off = 0; + common-ctop_off = 0; + common-cbtm_off = 0; + common-cur_frm = NULL; + common-next_frm = NULL; + memset(common-fmt, 0, sizeof(common-fmt)); + common-numbuffers = config_params.numbuffers[k]; + } + ch-initialized = 0; + if (vpif_obj.config-subdev_count) + ch-sd = vpif_obj.sd[0]; + ch-channel_id = j; + if (j 2) + ch-common[VPIF_VIDEO_INDEX].numbuffers = + config_params.numbuffers[ch-channel_id]; + else + ch-common[VPIF_VIDEO_INDEX].numbuffers = 0; + + memset(ch-vpifparams, 0, sizeof(ch-vpifparams)); + + /* Initialize prio member of channel object */ + v4l2_prio_init(ch-prio); + ch-common[VPIF_VIDEO_INDEX].fmt.type = + V4L2_BUF_TYPE_VIDEO_OUTPUT; + ch-video_dev-lock = common-lock; + video_set_drvdata(ch-video_dev, ch); + + /* select output 0 */ + err = vpif_set_output(vpif_obj.config, ch, 0); + if (err) + goto probe_out; + + /* register video device */ + vpif_dbg(1, debug, channel=%x,channel-video_dev=%x\n, +(int)ch, (int)ch-video_dev); + + err = video_register_device(ch-video_dev, + VFL_TYPE_GRABBER, (j ? 3 : 2)); + if (err 0) + goto probe_out; + } + + v4l2_info(vpif_obj.v4l2_dev, + VPIF display driver initialized\n); + + return 0; + +probe_out: + for (k = 0; k j; k++) { + ch = vpif_obj.dev[k]; + video_unregister_device(ch-video_dev); + video_device_release(ch-video_dev); + ch-video_dev = NULL; + } + return err; +} + +static int vpif_async_complete(struct v4l2_async_notifier *notifier) +{ + return vpif_probe_complete(); +} + /* * vpif_probe: This function creates device entries by register itself to the * V4L2 driver and
Re: [PATCH v4] drm/exynos: prepare FIMD clocks
On 22 April 2013 15:26, Tomasz Figa t.f...@samsung.com wrote: Can you assure that in future SoCs, on which this driver will be used, this assumption will still hold true or even that in current Exynos driver this behavior won't be changed? Probably yes.. Registers for enabling/disabling these clocks should always be on AMBA bus and not on SPI/I2C, i.e. on-soc... and so this will hold true. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] drm/exynos: prepare FIMD clocks
On Monday 22 of April 2013 12:17:39 Sylwester Nawrocki wrote: On 04/22/2013 12:03 PM, Inki Dae wrote: Also looks good to me. But what if power domain was disabled without pm runtime? In this case, you must enable the power domain at machine code or bootloader somewhere. This way would not only need some hard codes to turn the power domain on but also not manage power management fully. This is same as only the use of pm runtime interface(needing some hard codes without pm runtime) so I don't prefer to add clk_enable/disable to fimd probe(). I quite tend to force only the use of pm runtime as possible. So please add the hard codes to machine code or bootloader like you did for power domain if you want to use drm fimd without pm runtime. That's not how the runtime PM, clock subsystems work: 1) When CONFIG_PM_RUNTIME is disabled, all the used hardware must be kept powered on all the time. 2) Common Clock Framework will always gate all clocks that have zero enable_count. Note that CCF support for Exynos is already merged for 3.10 and it will be the only available clock support method for Exynos. AFAIK, drivers must work correctly in both cases, with CONFIG_PM_RUNTIME enabled and disabled. Then is the driver worked correctly if the power domain to this device was disabled at bootloader without CONFIG_PM_RUNTIME and with clk_enable()? I think, in this case, the device wouldn't be worked correctly because the power of the device remains off. So you must enable the power domain somewhere. What is the difference between these two cases? How about making the driver dependant on PM_RUNTIME and making it always use pm_runtime_* API, regardless if the platform actually implements runtime PM or not ? Is there any issue in using the Runtime PM core always, rather than coding any workarounds in drivers when PM_RUNTIME is disabled ? I don't think this is a good idea. This would mean that any user that from some reasons don't want to use PM_RUNTIME, would not be able to use the driver anymore. Rafael, Kevin, do you have any opinion on this? Best regards, -- Tomasz Figa Samsung Poland RD Center SW Solution Development, Kernel and System Framework -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 1/4] media: i2c: adv7343: add support for asynchronous probing
Hi Prabhakar On Mon, 22 Apr 2013, Prabhakar Lad wrote: From: Lad, Prabhakar prabhakar.cse...@gmail.com Both synchronous and asynchronous adv7343 subdevice probing is supported by this patch. Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Hans Verkuil hverk...@xs4all.nl Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Mauro Carvalho Chehab mche...@redhat.com --- drivers/media/i2c/adv7343.c | 17 + 1 files changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/adv7343.c b/drivers/media/i2c/adv7343.c index 9fc2b98..5b1417b 100644 --- a/drivers/media/i2c/adv7343.c +++ b/drivers/media/i2c/adv7343.c @@ -27,6 +27,7 @@ #include linux/uaccess.h #include media/adv7343.h +#include media/v4l2-async.h #include media/v4l2-device.h #include media/v4l2-chip-ident.h #include media/v4l2-ctrls.h @@ -44,6 +45,7 @@ struct adv7343_state { struct v4l2_subdev sd; struct v4l2_ctrl_handler hdl; const struct adv7343_platform_data *pdata; + struct v4l2_async_subdev_list asdl; Do you still need this? Don't think it's needed any more with the latest V4L2-async version. Thanks Guennadi u8 reg00; u8 reg01; u8 reg02; @@ -455,16 +457,22 @@ static int adv7343_probe(struct i2c_client *client, ADV7343_GAIN_DEF); state-sd.ctrl_handler = state-hdl; if (state-hdl.error) { - int err = state-hdl.error; - - v4l2_ctrl_handler_free(state-hdl); - return err; + err = state-hdl.error; + goto done; } v4l2_ctrl_handler_setup(state-hdl); err = adv7343_initialize(state-sd); if (err) + goto done; + + state-sd.dev = client-dev; + err = v4l2_async_register_subdev(state-sd); + +done: + if (err 0) v4l2_ctrl_handler_free(state-hdl); + return err; } @@ -473,6 +481,7 @@ static int adv7343_remove(struct i2c_client *client) struct v4l2_subdev *sd = i2c_get_clientdata(client); struct adv7343_state *state = to_state(sd); + v4l2_async_unregister_subdev(state-sd); v4l2_device_unregister_subdev(sd); v4l2_ctrl_handler_free(state-hdl); -- 1.7.4.1 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC v2 1/4] media: i2c: adv7343: add support for asynchronous probing
Hi Guennadi, Thanks for the quick review. On Mon, Apr 22, 2013 at 4:08 PM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Hi Prabhakar On Mon, 22 Apr 2013, Prabhakar Lad wrote: From: Lad, Prabhakar prabhakar.cse...@gmail.com Both synchronous and asynchronous adv7343 subdevice probing is supported by this patch. Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Hans Verkuil hverk...@xs4all.nl Cc: Sakari Ailus sakari.ai...@iki.fi Cc: Mauro Carvalho Chehab mche...@redhat.com --- drivers/media/i2c/adv7343.c | 17 + 1 files changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/adv7343.c b/drivers/media/i2c/adv7343.c index 9fc2b98..5b1417b 100644 --- a/drivers/media/i2c/adv7343.c +++ b/drivers/media/i2c/adv7343.c @@ -27,6 +27,7 @@ #include linux/uaccess.h #include media/adv7343.h +#include media/v4l2-async.h #include media/v4l2-device.h #include media/v4l2-chip-ident.h #include media/v4l2-ctrls.h @@ -44,6 +45,7 @@ struct adv7343_state { struct v4l2_subdev sd; struct v4l2_ctrl_handler hdl; const struct adv7343_platform_data *pdata; + struct v4l2_async_subdev_list asdl; Do you still need this? Don't think it's needed any more with the latest V4L2-async version. Yes not required missed to remove :) Regards, --Prabhakar Thanks Guennadi u8 reg00; u8 reg01; u8 reg02; @@ -455,16 +457,22 @@ static int adv7343_probe(struct i2c_client *client, ADV7343_GAIN_DEF); state-sd.ctrl_handler = state-hdl; if (state-hdl.error) { - int err = state-hdl.error; - - v4l2_ctrl_handler_free(state-hdl); - return err; + err = state-hdl.error; + goto done; } v4l2_ctrl_handler_setup(state-hdl); err = adv7343_initialize(state-sd); if (err) + goto done; + + state-sd.dev = client-dev; + err = v4l2_async_register_subdev(state-sd); + +done: + if (err 0) v4l2_ctrl_handler_free(state-hdl); + return err; } @@ -473,6 +481,7 @@ static int adv7343_remove(struct i2c_client *client) struct v4l2_subdev *sd = i2c_get_clientdata(client); struct adv7343_state *state = to_state(sd); + v4l2_async_unregister_subdev(state-sd); v4l2_device_unregister_subdev(sd); v4l2_ctrl_handler_free(state-hdl); -- 1.7.4.1 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] V4L2: fix subdevice matching in asynchronous probing
On Mon, 22 Apr 2013, Laurent Pinchart wrote: Hi Guennadi, On Friday 19 April 2013 16:41:02 Guennadi Liakhovetski wrote: A wrapped list iterating loop hasn't been correctly recognised in v4l2_async_belongs(), which led to false positives. Fix the bug by verifying the loop termination condition. Reported-by: Prabhakar Lad prabhakar.cse...@gmail.com Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- Prabhakar, please, check, whether this fixes your problem. drivers/media/v4l2-core/v4l2-async.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index 5d6b428..5631944 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -76,6 +76,10 @@ static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier * break; } + if (asd-list == notifier-waiting) + /* Wrapped - no match found */ + return NULL; + return asd; } What about just if (match match(sd-dev, hw)) return asd; } return NULL; } That's a bit simpler. Well, if it were simpler, it would've occurred to me instead of the other more complex solution, tight? ;-) No, sure, looks better, thanks! As soon as the actual patches are approved in principle, I'll merge this into them. Regards Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Patch update notification: 2 patches updated
Hi Mauro, On Sat, Apr 20, 2013 at 5:43 PM, Patchwork patchw...@linuxtv.org wrote: Hello, The following patches (submitted by you) have been updated in patchwork: * [2/2] media: davinci: vpif_display: move displaying of error to approppraite place - http://patchwork.linuxtv.org/patch/18092/ was: Under Review now: Accepted * [1/2] media: davinci: vpif: remove unwanted header file inclusion - http://patchwork.linuxtv.org/patch/18093/ was: Under Review now: Accepted The above patches have been marked as 'Accepted', However I haven’t issued a pull request nor I find the patches in your master branch. Something wrong while updating patchwork ? Regards, --Prabhakar -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] drm/exynos: prepare FIMD clocks
On Monday, April 22, 2013 12:37:36 PM Tomasz Figa wrote: On Monday 22 of April 2013 12:17:39 Sylwester Nawrocki wrote: On 04/22/2013 12:03 PM, Inki Dae wrote: Also looks good to me. But what if power domain was disabled without pm runtime? In this case, you must enable the power domain at machine code or bootloader somewhere. This way would not only need some hard codes to turn the power domain on but also not manage power management fully. This is same as only the use of pm runtime interface(needing some hard codes without pm runtime) so I don't prefer to add clk_enable/disable to fimd probe(). I quite tend to force only the use of pm runtime as possible. So please add the hard codes to machine code or bootloader like you did for power domain if you want to use drm fimd without pm runtime. That's not how the runtime PM, clock subsystems work: 1) When CONFIG_PM_RUNTIME is disabled, all the used hardware must be kept powered on all the time. 2) Common Clock Framework will always gate all clocks that have zero enable_count. Note that CCF support for Exynos is already merged for 3.10 and it will be the only available clock support method for Exynos. AFAIK, drivers must work correctly in both cases, with CONFIG_PM_RUNTIME enabled and disabled. Then is the driver worked correctly if the power domain to this device was disabled at bootloader without CONFIG_PM_RUNTIME and with clk_enable()? I think, in this case, the device wouldn't be worked correctly because the power of the device remains off. So you must enable the power domain somewhere. What is the difference between these two cases? How about making the driver dependant on PM_RUNTIME and making it always use pm_runtime_* API, regardless if the platform actually implements runtime PM or not ? Is there any issue in using the Runtime PM core always, rather than coding any workarounds in drivers when PM_RUNTIME is disabled ? I don't think this is a good idea. This would mean that any user that from some reasons don't want to use PM_RUNTIME, would not be able to use the driver anymore. Rafael, Kevin, do you have any opinion on this? I agree. Drivers should work for CONFIG_PM_RUNTIME unset too and static inline stubs for all runtime PM helpers are available in that case. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 02/20] V4L2: support asynchronous subdevice registration
Hi Guennadi and Sylwester, On Monday 15 April 2013 13:57:17 Sylwester Nawrocki wrote: On 04/12/2013 05:40 PM, Guennadi Liakhovetski wrote: Currently bridge device drivers register devices for all subdevices synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor is attached to a video bridge device, the bridge driver will create an I2C device and wait for the respective I2C driver to probe. This makes linking of devices straight forward, but this approach cannot be used with intrinsically asynchronous and unordered device registration systems like the Flattened Device Tree. To support such systems this patch adds an asynchronous subdevice registration framework to V4L2. To use it respective (e.g. I2C) subdevice drivers must register themselves with the framework. A bridge driver on the other hand must register notification callbacks, that will be called upon various related events. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- v9: addressed Laurent's comments (thanks) 1. moved valid hw-bus_type check 2. made v4l2_async_unregister() void 3. renamed struct v4l2_async_hw_device to struct v4l2_async_hw_info 4. merged struct v4l2_async_subdev_list into struct v4l2_subdev 5. fixed a typo 6. made subdev_num unsigned Remembering the media controller days, I know how it feels to reach v9. Please keep on with the good work, we're getting there :-) drivers/media/v4l2-core/Makefile |3 +- drivers/media/v4l2-core/v4l2-async.c | 284 + include/media/v4l2-async.h | 99 include/media/v4l2-subdev.h | 10 ++ 4 files changed, 395 insertions(+), 1 deletions(-) create mode 100644 drivers/media/v4l2-core/v4l2-async.c create mode 100644 include/media/v4l2-async.h [snip] diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c new file mode 100644 index 000..98db2e0 --- /dev/null +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -0,0 +1,284 @@ +/* + * V4L2 asynchronous subdevice registration API + * + * Copyright (C) 2012-2013, Guennadi Liakhovetski g.liakhovet...@gmx.de + * + * 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. + */ + [...] +static void v4l2_async_cleanup(struct v4l2_async_subdev_list *asdl) +{ + struct v4l2_subdev *sd = v4l2_async_to_subdev(asdl); + + v4l2_device_unregister_subdev(sd); + /* Subdevice driver will reprobe and put asdl back onto the list */ + list_del_init(asdl-list); + asdl-asd = NULL; + sd-dev = NULL; +} + +static void v4l2_async_unregister(struct v4l2_async_subdev_list *asdl) +{ + struct v4l2_subdev *sd = v4l2_async_to_subdev(asdl); + + v4l2_async_cleanup(asdl); + + /* If we handled USB devices, we'd have to lock the parent too */ + device_release_driver(sd-dev); +} + +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev, +struct v4l2_async_notifier *notifier) +{ + struct v4l2_async_subdev_list *asdl, *tmp; + struct v4l2_async_subdev *asd; + int i; + + notifier-v4l2_dev = v4l2_dev; + INIT_LIST_HEAD(notifier-waiting); + INIT_LIST_HEAD(notifier-done); + + for (i = 0; i notifier-subdev_num; i++) { + asd = notifier-subdev[i]; + + switch (asd-hw.bus_type) { + case V4L2_ASYNC_BUS_CUSTOM: + case V4L2_ASYNC_BUS_PLATFORM: + case V4L2_ASYNC_BUS_I2C: + break; + default: + dev_err(notifier-v4l2_dev ? notifier-v4l2_dev-dev : NULL, + Invalid bus-type %u on %p\n, + asd-hw.bus_type, asd); + return -EINVAL; + } + list_add_tail(asd-list, notifier-waiting); + } + + mutex_lock(list_lock); + + /* Keep also completed notifiers on the list */ + list_add(notifier-list, notifier_list); + + list_for_each_entry_safe(asdl, tmp, subdev_list, list) { + int ret; + + asd = v4l2_async_belongs(notifier, asdl); + if (!asd) + continue; + + ret = v4l2_async_test_notify(notifier, asdl, asd); + if (ret 0) { + mutex_unlock(list_lock); + return ret; + } + } + + mutex_unlock(list_lock); + + return 0; +} +EXPORT_SYMBOL(v4l2_async_notifier_register); + +void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier) +{ + struct v4l2_async_subdev_list *asdl, *tmp; + int i = 0; + struct device **dev = kcalloc(notifier-subdev_num, + sizeof(*dev), GFP_KERNEL); + if
Re: [PATCH RFC] media: videobuf2: fix the length check for mmap
Em 21-04-2013 19:38, Laurent Pinchart escreveu: Hi Mauro, On Friday 19 April 2013 08:18:01 Mauro Carvalho Chehab wrote: Em Fri, 19 Apr 2013 15:16:56 +0530 Prabhakar lad escreveu: From: Lad, Prabhakar prabhakar.cse...@gmail.com From commit 068a0df76023926af958a336a78bef60468d2033 [media] media: vb2: add length check for mmap patch verifies that the mmap() size requested by userspace doesn't exceed the buffer size. As the mmap() size is rounded up to the next page boundary the check will fail for buffer sizes that are not multiple of the page size. This patch fixes the check by aligning the buffer size to page size during the check. Alongside fixes the vmalloc allocator to round up the size. Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Marek Szyprowski m.szyprow...@samsung.com Cc: Seung-Woo Kim sw0312@samsung.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Mauro Carvalho Chehab mche...@redhat.com --- drivers/media/v4l2-core/videobuf2-core.c|2 +- drivers/media/v4l2-core/videobuf2-vmalloc.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 58c1744..223fcd4 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -1886,7 +1886,7 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma) vb = q-bufs[buffer]; - if (vb-v4l2_planes[plane].length (vma-vm_end - vma-vm_start)) { + if (PAGE_ALIGN(vb-v4l2_planes[plane].length) (vma-vm_end - vma-vm_start)) { dprintk(1, Invalid length\n); return -EINVAL; } That is tricky, as it assumes that vb-v4l2_planes[plane].length was round up to PAGE_SIZE at each memops driver, but the vb2 core doesn't enforce it. IMO, it would be cleaner to round vb-v4l2_planes[plane].length up at VB2 core, before calling the memops alloc functions at the drivers. I don't think we should round vb-v4l2_planes[plane].length up. That variable stores the buffer length required by the driver, and will be used to perform size checks when importing a dmabuf buffer. We don't want to prevent a buffer large enough for the driver but not page size aligned to be imported. What we could do is round in the core the size passed to the alloc function, without storing the rounded value in vb-v4l2_planes[plane].length. And, reading down, I realize that this is exactly what you meant :-) Yes. Touching at .length would have side effects, but touching at buffer's .size seem ok and VB2 dma contig code already does that with the current code. The proposed patch looks good to me. Good. Also, VB2 is already complex enough to put it there without proper comments (and there's a minor codingstyle issue there: line is bigger than 80 cols). A comment is definitely a good idea. diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c index 313d977..bf3b95c 100644 --- a/drivers/media/v4l2-core/videobuf2-vmalloc.c +++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c @@ -44,7 +44,7 @@ static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_fl return NULL; buf-size = size; - buf-vaddr = vmalloc_user(buf-size); + buf-vaddr = vmalloc_user(PAGE_ALIGN(buf-size)); See? You needed to put an alignment here as well, not because vmalloc needs it, but because this is needed by VB2 core. Also, on the other drivers, buf-size is stored page aligned, while here, you're doing different, without any documented reason for doing that, instead of doing the same as on the other memops drivers. That mistake reflects, for example, when the driver prints the failure: if (!buf-vaddr) { pr_debug(vmalloc of size %ld failed\n, buf-size); as it will show a different size than what you actually required. As those memory starving errors can also produce a dump at the mm core, the size there won't match the size on the above printed message. Also, it is a very bad idea to delegate the core's requirement of do page alignment from the core to the memops drivers, as other patches may change the logic there, or a new memops could be added, and the same problem will hit again (and unnoticed, as the check routine do page alignments). Agreed. The memory allocator shouldn't need to guess the core requirements. buf-handler.refcount = buf-refcount; buf-handler.put = vb2_vmalloc_put; buf-handler.arg = buf; IMO, a cleaner version would be the following (untested) code. - [media] videobuf2: fix the length check for mmap Memory maps typically require that the buffer size to be page aligned. Currently, two memops drivers do such alignment internally, but videobuf-vmalloc doesn't. Also, the buffer overflow check doesn't take it into account. So,
Re: [PATCH RFCv3 07/10] [media] tuner-core: add SDR support for g_tuner
Em 22-04-2013 04:18, Hans Verkuil escreveu: On Sun April 21 2013 21:00:36 Mauro Carvalho Chehab wrote: Properly initialize the fields for VIDIOC_G_TUNER, if the device is in SDR mode. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com --- drivers/media/v4l2-core/tuner-core.c | 29 + 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c index b97ec63..e54b5ae 100644 --- a/drivers/media/v4l2-core/tuner-core.c +++ b/drivers/media/v4l2-core/tuner-core.c @@ -1190,7 +1190,31 @@ static int tuner_g_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt) } /* radio mode */ - if (vt-type == t-mode) { + vt-capability |= V4L2_TUNER_CAP_LOW | V4L2_TUNER_CAP_STEREO; + + if (V4L2_TUNER_IS_SDR(vt-type)) { + vt-rangelow = tv_range[0] * 16000; + vt-rangehigh = tv_range[1] * 16000; Why use tv_range for SDR? It's a bit odd for something called SD 'Radio'. Because it is the widest known range, and it covers already the FM range. A latter patch will improve the range, by adding a tuner callback to query about what's the real supported range, with will be typically broader than the TV one. Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/24] V4L2: allow dummy file-handle initialisation by v4l2_fh_init()
Hi Hans, On Friday 19 April 2013 09:22:50 Hans Verkuil wrote: On Thu April 18 2013 23:35:26 Guennadi Liakhovetski wrote: v4l2_fh_init() can be used to initialise dummy file-handles with vdev == NULL. Why would you want that? The reason is that subdev pad operations require a file handle and use it as a context to store the try rectangles. The wrappers thus need to create a dummy file handle. Anyway, this would definitely have to be documented as well in v4l2-fh.h. I'm still going through your patch series so there may be a good reason for allowing this, but it definitely doesn't make me happy. Regards, Hans Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- drivers/media/v4l2-core/v4l2-fh.c |8 +--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-fh.c b/drivers/media/v4l2-core/v4l2-fh.c index e57c002..7ae608b 100644 --- a/drivers/media/v4l2-core/v4l2-fh.c +++ b/drivers/media/v4l2-core/v4l2-fh.c @@ -33,10 +33,12 @@ void v4l2_fh_init(struct v4l2_fh *fh, struct video_device *vdev) { fh-vdev = vdev; - /* Inherit from video_device. May be overridden by the driver. */ - fh-ctrl_handler = vdev-ctrl_handler; + if (vdev) { + /* Inherit from video_device. May be overridden by the driver. */ + fh-ctrl_handler = vdev-ctrl_handler; + set_bit(V4L2_FL_USES_V4L2_FH, fh-vdev-flags); + } INIT_LIST_HEAD(fh-list); - set_bit(V4L2_FL_USES_V4L2_FH, fh-vdev-flags); fh-prio = V4L2_PRIORITY_UNSET; init_waitqueue_head(fh-wait); INIT_LIST_HEAD(fh-available); -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFCv3 08/10] [media] tuner-core: store tuner ranges at tuner struct
Em 22-04-2013 04:22, Hans Verkuil escreveu: On Sun April 21 2013 21:00:37 Mauro Carvalho Chehab wrote: Instead of using global values for tuner ranges, store them internally. That fixes the need of using a different range for SDR radio, and will help to latter add a tuner ops to retrieve the tuner range for SDR mode. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com --- drivers/media/v4l2-core/tuner-core.c | 59 ++-- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c index e54b5ae..abdcda4 100644 --- a/drivers/media/v4l2-core/tuner-core.c +++ b/drivers/media/v4l2-core/tuner-core.c @@ -67,8 +67,8 @@ static char secam[] = --; static char ntsc[] = -; module_param_named(debug, tuner_debug, int, 0644); -module_param_array(tv_range, int, NULL, 0644); -module_param_array(radio_range, int, NULL, 0644); +module_param_array(tv_range, int, NULL, 0444); +module_param_array(radio_range, int, NULL, 0444); Shouldn't we add a sdr_range here as well? I don't think it is needed to have a modprobe parameter for that. If user wants to change the range, VIDIOC_S_TUNER can be used. Btw, I was tempted to even remove those ;) module_param_string(pal, pal, sizeof(pal), 0644); module_param_string(secam, secam, sizeof(secam), 0644); module_param_string(ntsc, ntsc, sizeof(ntsc), 0644); @@ -134,6 +134,8 @@ struct tuner { unsigned inttype; /* chip type id */ void*config; const char *name; + + u32 radio_range[2], tv_range[2], sdr_range[2]; }; /* @@ -266,7 +268,7 @@ static void set_type(struct i2c_client *c, unsigned int type, struct dvb_tuner_ops *fe_tuner_ops = t-fe.ops.tuner_ops; struct analog_demod_ops *analog_ops = t-fe.ops.analog_ops; unsigned char buffer[4]; - int tune_now = 1; + int i, tune_now = 1; if (type == UNSET || type == TUNER_ABSENT) { tuner_dbg(tuner 0x%02x: Tuner type absent\n, c-addr); @@ -451,6 +453,13 @@ static void set_type(struct i2c_client *c, unsigned int type, set_tv_freq(c, t-tv_freq); } + /* Initializes the tuner ranges from modprobe parameters */ + for (i = 0; i 2; i++) { + t-radio_range[i] = radio_range[i] * 16000; + t-sdr_range[i] = tv_range[i] * 16000; + t-tv_range[i] = tv_range[i] * 16; + } + tuner_dbg(%s %s I2C addr 0x%02x with type %d used for 0x%02x\n, c-adapter-name, c-driver-driver.name, c-addr 1, type, t-mode_mask); @@ -831,16 +840,16 @@ static void set_tv_freq(struct i2c_client *c, unsigned int freq) tuner_warn(Tuner has no way to set tv freq\n); return; } - if (freq tv_range[0] * 16 || freq tv_range[1] * 16) { + if (freq t-tv_range[0] || freq t-tv_range[1]) { tuner_dbg(TV freq (%d.%02d) out of range (%d-%d)\n, - freq / 16, freq % 16 * 100 / 16, tv_range[0], - tv_range[1]); + freq / 16, freq % 16 * 100 / 16, t-tv_range[0] / 16, + t-tv_range[1] / 16); /* V4L2 spec: if the freq is not possible then the closest possible value should be selected */ - if (freq tv_range[0] * 16) - freq = tv_range[0] * 16; + if (freq t-tv_range[0]) + freq = t-tv_range[0]; else - freq = tv_range[1] * 16; + freq = t-tv_range[1]; } params.frequency = freq; tuner_dbg(tv freq set to %d.%02d\n, @@ -957,7 +966,7 @@ static void set_radio_freq(struct i2c_client *c, unsigned int freq) { struct tuner *t = to_tuner(i2c_get_clientdata(c)); struct analog_demod_ops *analog_ops = t-fe.ops.analog_ops; - + u32 *range; struct analog_parameters params = { .mode = t-mode, .audmode = t-audmode, @@ -972,16 +981,22 @@ static void set_radio_freq(struct i2c_client *c, unsigned int freq) tuner_warn(tuner has no way to set radio frequency\n); return; } - if (freq radio_range[0] * 16000 || freq radio_range[1] * 16000) { + + if (V4L2_TUNER_IS_SDR(t-mode)) + range = t-sdr_range; + else + range = t-radio_range; + + if (freq range[0] || freq range[1]) { tuner_dbg(radio freq (%d.%02d) out of range (%d-%d)\n, freq / 16000, freq % 16000 * 100 / 16000, - radio_range[0], radio_range[1]); + range[0] / 16000, range[1] / 16000); /* V4L2 spec: if the freq is not possible then the closest
Re: [PATCH 18/24] V4L2: mt9p031: power down the sensor if no supported device has been detected
Hi Guennadi, Thanks for the patch. On Thursday 18 April 2013 23:35:39 Guennadi Liakhovetski wrote: The mt9p031 driver first accesses the I2C device in its .registered() method. While doing that it furst powers the device up, but if probing s/furst/first/ fails, it doesn't power the chip back down. This patch fixes that bug. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- drivers/media/i2c/mt9p031.c | 10 ++ 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index eb2de22..70f4525 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -844,7 +844,7 @@ static int mt9p031_registered(struct v4l2_subdev *subdev) ret = mt9p031_power_on(mt9p031); if (ret 0) { dev_err(client-dev, MT9P031 power up failed\n); - return ret; + goto done; Not here. If power on fails, there's no need to power off. } /* Read out the chip version register */ @@ -852,13 +852,15 @@ static int mt9p031_registered(struct v4l2_subdev *subdev) if (data != MT9P031_CHIP_VERSION_VALUE) { dev_err(client-dev, MT9P031 not detected, wrong version 0x%04x\n, data); - return -ENODEV; + ret = -ENODEV; } +done: mt9p031_power_off(mt9p031); - dev_info(client-dev, MT9P031 detected at address 0x%02x\n, - client-addr); + if (!ret) + dev_info(client-dev, MT9P031 detected at address 0x%02x\n, + client-addr); return ret; } It would be easier to just move the power off line right after the mt9p031_read() call and leave the rest unchanged. diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 28cf95b..8de84c0 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -849,18 +849,18 @@ static int mt9p031_registered(struct v4l2_subdev *subdev) /* Read out the chip version register */ data = mt9p031_read(client, MT9P031_CHIP_VERSION); + mt9p031_power_off(mt9p031); + if (data != MT9P031_CHIP_VERSION_VALUE) { dev_err(client-dev, MT9P031 not detected, wrong version 0x%04x\n, data); return -ENODEV; } - mt9p031_power_off(mt9p031); - dev_info(client-dev, MT9P031 detected at address 0x%02x\n, client-addr); - return ret; + return 0; } static int mt9p031_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh) If you're happy with that there's no need to resubmit, I'll apply the patch to my tree for v3.11. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] drm/exynos: prepare FIMD clocks
On Monday 22 of April 2013 12:05:49 Sylwester Nawrocki wrote: On 04/22/2013 11:56 AM, Tomasz Figa wrote: On Monday 22 of April 2013 10:44:00 Viresh Kumar wrote: On 21 April 2013 20:13, Tomasz Figa tomasz.f...@gmail.com wrote: 3) after those two changes, all that remains is to fix compliance with Common Clock Framework, in other words: s/clk_enable/clk_prepare_enable/ and s/clk_disable/clk_disable_unprepare/ We don't have to call clk_{un}prepare() everytime for your platform as you aren't doing anything in it. So just call them once at probe/remove and call clk_enable/disable everywhere else. Yes, I agree with that. Additionally clk_(un)prepare must not be called in atomic context, so some drivers will have to work like this anyway. Or the clocks could be prepared/unprepared in the device open/close file op for instance. Well, I don't think drivers should make any assumptions how particular clk ops are implemented on particular platform. Instead, generic semantics of Common Clock Framework should be obeyed, which AFAIK are: 1) Each clock must be prepared before enabling. 2) clk_prepare() can not be called from atomic contexts. 3) clk_prepare_enable() can be used instead of clk_prepare() + clk_enable() when the driver does not need to enable the clock from atomic context. Since the Exynos DRM FIMD driver does not need to do call any clock operations in atomic contexts, the approach keeping the clock handling as simple as possible would be to just replace all clk_{enable,disable} with clk_{prepare_enable,disable_unprepare}, as I suggested. CCing Mike, the maintainer of Common Clock Framework, since he's the right person to pass any judgements when it is about clocks. Best regards, -- Tomasz Figa Samsung Poland RD Center SW Solution Development, Kernel and System Framework -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 23/24] V4L2: mt9p031: add struct v4l2_subdev_platform_data to platform data
Hi Guennadi, On Thursday 18 April 2013 23:47:26 Guennadi Liakhovetski wrote: On Thu, 18 Apr 2013, Guennadi Liakhovetski wrote: Adding struct v4l2_subdev_platform_data to mt9p031's platform data allows the driver to use generic functions to manage sensor power supplies. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de A small addition to this one too: to be absolutely honest, I also had to replace 12-bit formats with their 8-bit counterparts, because only 8 data lanes are connected to my camera host. We'll need to somehow properly solve this too. That information should be conveyed by platform/DT data for the host, and be used to convert the 12-bit media bus code into a 8-bit media bus code in the host (a core helper function would probably be helpful). --- drivers/media/i2c/mt9p031.c |1 + include/media/mt9p031.h |3 +++ 2 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 70f4525..ca2cc6e 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -1048,6 +1048,7 @@ static int mt9p031_probe(struct i2c_client *client, goto done; mt9p031-subdev.dev = client-dev; + mt9p031-subdev.pdata = pdata-sd_pdata; ret = v4l2_async_register_subdev(mt9p031-subdev); done: diff --git a/include/media/mt9p031.h b/include/media/mt9p031.h index 0c97b19..7bf7b53 100644 --- a/include/media/mt9p031.h +++ b/include/media/mt9p031.h @@ -1,6 +1,8 @@ #ifndef MT9P031_H #define MT9P031_H +#include media/v4l2-subdev.h + struct v4l2_subdev; /* @@ -15,6 +17,7 @@ struct mt9p031_platform_data { int reset; int ext_freq; int target_freq; + struct v4l2_subdev_platform_data sd_pdata; }; #endif -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/24] V4L2: mt9p031: power down the sensor if no supported device has been detected
Hi Laurent On Mon, 22 Apr 2013, Laurent Pinchart wrote: Hi Guennadi, Thanks for the patch. On Thursday 18 April 2013 23:35:39 Guennadi Liakhovetski wrote: The mt9p031 driver first accesses the I2C device in its .registered() method. While doing that it furst powers the device up, but if probing s/furst/first/ fails, it doesn't power the chip back down. This patch fixes that bug. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- drivers/media/i2c/mt9p031.c | 10 ++ 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index eb2de22..70f4525 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -844,7 +844,7 @@ static int mt9p031_registered(struct v4l2_subdev *subdev) ret = mt9p031_power_on(mt9p031); if (ret 0) { dev_err(client-dev, MT9P031 power up failed\n); - return ret; + goto done; Not here. If power on fails, there's no need to power off. Oops, right. } /* Read out the chip version register */ @@ -852,13 +852,15 @@ static int mt9p031_registered(struct v4l2_subdev *subdev) if (data != MT9P031_CHIP_VERSION_VALUE) { dev_err(client-dev, MT9P031 not detected, wrong version 0x%04x\n, data); - return -ENODEV; + ret = -ENODEV; } +done: mt9p031_power_off(mt9p031); - dev_info(client-dev, MT9P031 detected at address 0x%02x\n, -client-addr); + if (!ret) + dev_info(client-dev, MT9P031 detected at address 0x%02x\n, +client-addr); return ret; } It would be easier to just move the power off line right after the mt9p031_read() call and leave the rest unchanged. Sure, please, do. Thanks Guennadi diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 28cf95b..8de84c0 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -849,18 +849,18 @@ static int mt9p031_registered(struct v4l2_subdev *subdev) /* Read out the chip version register */ data = mt9p031_read(client, MT9P031_CHIP_VERSION); + mt9p031_power_off(mt9p031); + if (data != MT9P031_CHIP_VERSION_VALUE) { dev_err(client-dev, MT9P031 not detected, wrong version 0x%04x\n, data); return -ENODEV; } - mt9p031_power_off(mt9p031); - dev_info(client-dev, MT9P031 detected at address 0x%02x\n, client-addr); - return ret; + return 0; } static int mt9p031_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh) If you're happy with that there's no need to resubmit, I'll apply the patch to my tree for v3.11. -- Regards, Laurent Pinchart --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] ARM: shmobile: r8a7779: add VIN support
Hello. On 22-04-2013 8:57, Simon Horman wrote: From: Vladimir Barinov vladimir.bari...@cogentembedded.com Add VIN clocks and platform devices for R8A7779 SoC; add function to register the VIN platform devices. Signed-off-by: Vladimir Barinov vladimir.bari...@cogentembedded.com [Sergei: added 'id' parameter check to r8a7779_add_vin_device(), renamed some variables.] Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com [...] Index: renesas/arch/arm/mach-shmobile/setup-r8a7779.c === --- renesas.orig/arch/arm/mach-shmobile/setup-r8a7779.c +++ renesas/arch/arm/mach-shmobile/setup-r8a7779.c @@ -559,6 +559,33 @@ static struct resource ether_resources[] }, }; +#define R8A7779_VIN(idx) \ +static struct resource vin##idx##_resources[] = { \ + DEFINE_RES_MEM(0xffc5 + 0x1000 * (idx), 0x1000),\ + DEFINE_RES_IRQ(gic_iid(0x5f + (idx))), \ +}; \ + \ +static struct platform_device_info vin##idx##_info = { \ Hm, probably should have marked this as '__initdata'... maybe the resources too. That doesn't seem to be the case for other devices in that or other shmobile files. Am I missing something or should numerous other devices be updated? If the device is registered using platform_device_register_*(), it seems worth marking the resources, the platfrom data and 'struct platform_device_info' as '__initdata' as they're copied to the memory allocated from heap anyway and hence not needed past the init phase... WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 23/24] V4L2: mt9p031: add struct v4l2_subdev_platform_data to platform data
On Mon, 22 Apr 2013, Laurent Pinchart wrote: Hi Guennadi, On Thursday 18 April 2013 23:47:26 Guennadi Liakhovetski wrote: On Thu, 18 Apr 2013, Guennadi Liakhovetski wrote: Adding struct v4l2_subdev_platform_data to mt9p031's platform data allows the driver to use generic functions to manage sensor power supplies. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de A small addition to this one too: to be absolutely honest, I also had to replace 12-bit formats with their 8-bit counterparts, because only 8 data lanes are connected to my camera host. We'll need to somehow properly solve this too. That information should be conveyed by platform/DT data for the host, and be used to convert the 12-bit media bus code into a 8-bit media bus code in the host (a core helper function would probably be helpful). Yes, and we discussed this before too, I think. I proposed based then to implement some compatibility table of trivial transformations, like a 12-bit Bayer, right-shifted by 4 bits, produces a respective 8-bit Bayer etc. Such transformations would fit nicely in soc_mediabus.c ;-) This just needs to be implemented... Sure, I'd be happy to move soc_mediabus.c to drivers/media/v4l2-core/v4l2-mediabus.c. Thanks Guennadi --- drivers/media/i2c/mt9p031.c |1 + include/media/mt9p031.h |3 +++ 2 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 70f4525..ca2cc6e 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -1048,6 +1048,7 @@ static int mt9p031_probe(struct i2c_client *client, goto done; mt9p031-subdev.dev = client-dev; + mt9p031-subdev.pdata = pdata-sd_pdata; ret = v4l2_async_register_subdev(mt9p031-subdev); done: diff --git a/include/media/mt9p031.h b/include/media/mt9p031.h index 0c97b19..7bf7b53 100644 --- a/include/media/mt9p031.h +++ b/include/media/mt9p031.h @@ -1,6 +1,8 @@ #ifndef MT9P031_H #define MT9P031_H +#include media/v4l2-subdev.h + struct v4l2_subdev; /* @@ -15,6 +17,7 @@ struct mt9p031_platform_data { int reset; int ext_freq; int target_freq; + struct v4l2_subdev_platform_data sd_pdata; }; #endif -- Regards, Laurent Pinchart --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 23/24] V4L2: mt9p031: add struct v4l2_subdev_platform_data to platform data
Hi Guennadi, On Thursday 18 April 2013 23:35:44 Guennadi Liakhovetski wrote: Adding struct v4l2_subdev_platform_data to mt9p031's platform data allows the driver to use generic functions to manage sensor power supplies. The mt9p031 driver now handles its regulators explicitly, please see commit 97f212767a4d0fbddbf4786ccedacb47fc210548 Author: Laurent Pinchart laurent.pinch...@ideasonboard.com Date: Tue May 8 10:10:36 2012 -0300 [media] mt9p031: Add support for regulators Enable the regulators when powering the sensor up, and disable them when powering it down. The regulators are mandatory. Boards that don't allow controlling the sensor power lines must provide fixed voltage regulators. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- drivers/media/i2c/mt9p031.c |1 + include/media/mt9p031.h |3 +++ 2 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 70f4525..ca2cc6e 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -1048,6 +1048,7 @@ static int mt9p031_probe(struct i2c_client *client, goto done; mt9p031-subdev.dev = client-dev; + mt9p031-subdev.pdata = pdata-sd_pdata; ret = v4l2_async_register_subdev(mt9p031-subdev); done: diff --git a/include/media/mt9p031.h b/include/media/mt9p031.h index 0c97b19..7bf7b53 100644 --- a/include/media/mt9p031.h +++ b/include/media/mt9p031.h @@ -1,6 +1,8 @@ #ifndef MT9P031_H #define MT9P031_H +#include media/v4l2-subdev.h + struct v4l2_subdev; /* @@ -15,6 +17,7 @@ struct mt9p031_platform_data { int reset; int ext_freq; int target_freq; + struct v4l2_subdev_platform_data sd_pdata; }; #endif -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL FOR v3.10] Camera sensors patches
Em 22-04-2013 07:03, Mark Brown escreveu: On Mon, Apr 22, 2013 at 01:14:07AM +0200, Laurent Pinchart wrote: I think that Mark's point was that the regulators should be provided by platform code (in the generic sense, it could be DT on ARM, board code, or a USB bridge driver for a webcam that uses the mt9p031 sensor) and used by the sensor driver. That's exactly what my mt9p031 patch does. Yes, you understood me perfectly - to a good approximation the matching up should be done by whatever the chip is soldered down to. That doesn't make any sense to me. I2C devices can be used anywere, as they can be soldered either internally on an USB webcam without any regulators or any other platform code on it or could be soldered to some platform-specific bus. Also, what best describes soldered here is the binding between an I2C driver and the I2C adapter. The I2C adapter is a platform driver on embedded devices, where, on an usual USB camera, it is just a USB-I2C bridge. Also, requiring that simple USB cameras to have regulators will prevent its usual usage, as non-platform distros don't set config REGULATOR, and it shouldn't. Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 23/24] V4L2: mt9p031: add struct v4l2_subdev_platform_data to platform data
On Monday 22 April 2013 14:39:57 Guennadi Liakhovetski wrote: On Mon, 22 Apr 2013, Laurent Pinchart wrote: On Thursday 18 April 2013 23:47:26 Guennadi Liakhovetski wrote: On Thu, 18 Apr 2013, Guennadi Liakhovetski wrote: Adding struct v4l2_subdev_platform_data to mt9p031's platform data allows the driver to use generic functions to manage sensor power supplies. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de A small addition to this one too: to be absolutely honest, I also had to replace 12-bit formats with their 8-bit counterparts, because only 8 data lanes are connected to my camera host. We'll need to somehow properly solve this too. That information should be conveyed by platform/DT data for the host, and be used to convert the 12-bit media bus code into a 8-bit media bus code in the host (a core helper function would probably be helpful). Yes, and we discussed this before too, I think. I proposed based then to implement some compatibility table of trivial transformations, like a 12-bit Bayer, right-shifted by 4 bits, produces a respective 8-bit Bayer etc. Such transformations would fit nicely in soc_mediabus.c ;-) This just needs to be implemented... Sure, I'd be happy to move soc_mediabus.c to drivers/media/v4l2-core/v4l2-mediabus.c. And the OMAP3 ISP driver has something similiar in drivers/media/platform/omap3isp/ispvideo.c --- drivers/media/i2c/mt9p031.c |1 + include/media/mt9p031.h |3 +++ 2 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 70f4525..ca2cc6e 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -1048,6 +1048,7 @@ static int mt9p031_probe(struct i2c_client *client, goto done; mt9p031-subdev.dev = client-dev; + mt9p031-subdev.pdata = pdata-sd_pdata; ret = v4l2_async_register_subdev(mt9p031-subdev); done: diff --git a/include/media/mt9p031.h b/include/media/mt9p031.h index 0c97b19..7bf7b53 100644 --- a/include/media/mt9p031.h +++ b/include/media/mt9p031.h @@ -1,6 +1,8 @@ #ifndef MT9P031_H #define MT9P031_H +#include media/v4l2-subdev.h + struct v4l2_subdev; /* @@ -15,6 +17,7 @@ struct mt9p031_platform_data { int reset; int ext_freq; int target_freq; + struct v4l2_subdev_platform_data sd_pdata; }; #endif -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL FOR v3.10] Camera sensors patches
Em 22-04-2013 07:03, Mark Brown escreveu: On Mon, Apr 22, 2013 at 01:14:07AM +0200, Laurent Pinchart wrote: I think that Mark's point was that the regulators should be provided by platform code (in the generic sense, it could be DT on ARM, board code, or a USB bridge driver for a webcam that uses the mt9p031 sensor) and used by the sensor driver. That's exactly what my mt9p031 patch does. Yes, you understood me perfectly - to a good approximation the matching up should be done by whatever the chip is soldered down to. That doesn't make any sense to me. I2C devices can be used anywere, as they can be soldered either internally on an USB webcam without any regulators or any other platform code on it or could be soldered to some platform-specific bus. Also, what best describes soldered here is the binding between an I2C driver and the I2C adapter. The I2C adapter is a platform driver on embedded devices, where, on an usual USB camera, it is just a USB-I2C bridge. Also, requiring that simple USB cameras to have regulators will prevent its usual usage, as non-platform distros don't set config REGULATOR (and they shouldn't, as that would just increase the Kernel's footprint for a code that will never ever be needed there). Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL FOR v3.10] Camera sensors patches
On Mon, Apr 22, 2013 at 09:46:07AM -0300, Mauro Carvalho Chehab wrote: Em 22-04-2013 07:03, Mark Brown escreveu: Yes, you understood me perfectly - to a good approximation the matching up should be done by whatever the chip is soldered down to. That doesn't make any sense to me. I2C devices can be used anywere, as they can be soldered either internally on an USB webcam without any regulators or any other platform code on it or could be soldered to some platform-specific bus. If it's running on Linux on a visible I2C bus it ought to be shown as an I2C bus on Linux and the thing doing that plumbing ought to be worrying about hooking up anything the driver needs. Also, what best describes soldered here is the binding between an I2C driver and the I2C adapter. The I2C adapter is a platform driver on embedded devices, where, on an usual USB camera, it is just a USB-I2C bridge. Sure, but there's no meaningful difference between these things as far as plumbing things together goes. Also, requiring that simple USB cameras to have regulators will prevent its usual usage, as non-platform distros don't set config REGULATOR, and it shouldn't. No problem there, the regulator API stubs itself out if it's not enabled. signature.asc Description: Digital signature
Re: [PATCH RFC] media: videobuf2: fix the length check for mmap
Hi Mauro, On Friday 19 April 2013 08:18:01 Mauro Carvalho Chehab wrote: [snip] [media] videobuf2: fix the length check for mmap Memory maps typically require that the buffer size to be page s/that the/the/ aligned. Currently, two memops drivers do such alignment internally, but videobuf-vmalloc doesn't. Also, the buffer overflow check doesn't take it into account. So, instead of doing it at each memops driver, enforce it at VB2 core. Reported-by: Prabhakar lad prabhakar.cse...@gmail.com Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 58c1744..7d833ee 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -54,10 +54,15 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb) void *mem_priv; int plane; - /* Allocate memory for all planes in this buffer */ + /* + * Allocate memory for all planes in this buffer + * NOTE: mmapped areas should be page aligned + */ for (plane = 0; plane vb-num_planes; ++plane) { + unsigned long size = PAGE_ALIGN(q-plane_sizes[plane]); + mem_priv = call_memop(q, alloc, q-alloc_ctx[plane], - q-plane_sizes[plane], q-gfp_flags); + size, q-gfp_flags); if (IS_ERR_OR_NULL(mem_priv)) goto free; @@ -1852,6 +1857,7 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma) struct vb2_buffer *vb; unsigned int buffer, plane; int ret; + unsigned long length; if (q-memory != V4L2_MEMORY_MMAP) { dprintk(1, Queue is not currently set up for mmap\n); @@ -1886,8 +1892,15 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma) vb = q-bufs[buffer]; - if (vb-v4l2_planes[plane].length (vma-vm_end - vma-vm_start)) { - dprintk(1, Invalid length\n); + /* + * MMAP requires page_aligned buffers. + * The buffer length was page_aligned at __vb2_buf_mem_alloc(), + * so, we need to do the same here. + */ + length = PAGE_ALIGN(vb-v4l2_planes[plane].length); + if (length (vma-vm_end - vma-vm_start)) { + dprintk(1, + MMAP invalid, as it would overflow buffer length\n); return -EINVAL; } diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c index ae35d25..fd56f25 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c @@ -162,9 +162,6 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_flags) if (!buf) return ERR_PTR(-ENOMEM); - /* align image size to PAGE_SIZE */ - size = PAGE_ALIGN(size); - buf-vaddr = dma_alloc_coherent(dev, size, buf-dma_addr, GFP_KERNEL | gfp_flags); if (!buf-vaddr) { diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c index 59522b2..16ae3dc 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c @@ -55,7 +55,8 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_fla buf-write = 0; buf-offset = 0; buf-sg_desc.size = size; - buf-sg_desc.num_pages = (size + PAGE_SIZE - 1) PAGE_SHIFT; + /* size is already page aligned */ + buf-sg_desc.num_pages = size PAGE_SHIFT; buf-sg_desc.sglist = vzalloc(buf-sg_desc.num_pages * sizeof(*buf-sg_desc.sglist)); -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/12] exynos4-is driver fixes
This patch series includes fixes for several issues found during testing all exynos4-is device drivers build as modules. The exynos4-is build with all sub-drivers as 'M' is hopefully now free of all serious issues, but one. I.e. the requirement now is to have all sub-device drivers, including the sensor subdev drivers, built as modules. The problem when some of the sub-device drivers is statically linked is that the media links of a media entity just unregistered from the media device are not fully cleaned up in the media controller API. This means other entities can have dangling pointers to the links array owned by en entity just removed and freed. The problem is not existent when all media entites are registered/unregistred together. In such a case it doesn't hurt that media_entity_cleanup() function just frees the links array. I will post a separate RFC patch to address this issue, since it is not trivial where the link references should be removed from all involved media entities. I verified that adding a call to media_entity_remove_links() as in patch [1] to the v4l2_sdubdev_unregister_function() eliminates all weird crashes present before, when inserting/removing all the host driver modules while the sensor driver stays loaded. [1] http://git.linuxtv.org/snawrocki/samsung.git/commitdiff/f7007880a37c28beef845aa0787696aa8cead1cd Sylwester Nawrocki (12): s5c73m3: Fix remove() callback to free requested resources s5c73m3: Add missing subdev .unregistered callback exynos4-is: Remove redundant MODULE_DEVICE_TABLE entries exynos4-is: Fix initialization of subdev 'flags' field exynos4-is: Fix regulator/gpio resource releasing on the driver removal exynos4-is: Don't overwrite subdevdata in the fimc-is sensor driver exynos4-is: Unregister fimc-is subdevs from the media device properly exynos4-is: Set fimc-lite subdev subdev owner module exynos4-is: Remove redundant module_put() for MIPI-CSIS module exynos4-is: Remove debugfs entries properly exynos4-is: Change function call order in fimc_is_module_exit() exynos4-is: Fix runtime PM handling on fimc-is probe error path drivers/media/i2c/s5c73m3/s5c73m3-core.c | 21 +--- drivers/media/platform/exynos4-is/fimc-capture.c |2 +- drivers/media/platform/exynos4-is/fimc-is-i2c.c|3 -- drivers/media/platform/exynos4-is/fimc-is-sensor.c | 35 +--- drivers/media/platform/exynos4-is/fimc-is-sensor.h |6 drivers/media/platform/exynos4-is/fimc-is.c| 15 - drivers/media/platform/exynos4-is/fimc-isp.c |2 +- drivers/media/platform/exynos4-is/fimc-lite.c |3 +- drivers/media/platform/exynos4-is/media-dev.c |5 ++- 9 files changed, 46 insertions(+), 46 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/12] s5c73m3: Fix remove() callback to free requested resources
Make sure v4l2_device_unregister_subdev() is called for both: oif and sensor subdev and both media entities are freed on driver removal. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/i2c/s5c73m3/s5c73m3-core.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c b/drivers/media/i2c/s5c73m3/s5c73m3-core.c index b353c50..ce8fcf2 100644 --- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c +++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c @@ -1668,13 +1668,17 @@ out_err1: static int s5c73m3_remove(struct i2c_client *client) { - struct v4l2_subdev *sd = i2c_get_clientdata(client); - struct s5c73m3 *state = sensor_sd_to_s5c73m3(sd); + struct v4l2_subdev *oif_sd = i2c_get_clientdata(client); + struct s5c73m3 *state = oif_sd_to_s5c73m3(oif_sd); + struct v4l2_subdev *sensor_sd = state-sensor_sd; - v4l2_device_unregister_subdev(sd); + v4l2_device_unregister_subdev(oif_sd); - v4l2_ctrl_handler_free(sd-ctrl_handler); - media_entity_cleanup(sd-entity); + v4l2_ctrl_handler_free(oif_sd-ctrl_handler); + media_entity_cleanup(oif_sd-entity); + + v4l2_device_unregister_subdev(sensor_sd); + media_entity_cleanup(sensor_sd-entity); s5c73m3_unregister_spi_driver(state); s5c73m3_free_gpios(state); -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/12] s5c73m3: Add missing subdev .unregistered callback
This is needed to free any resources requested in the .registered subdev op. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/i2c/s5c73m3/s5c73m3-core.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c b/drivers/media/i2c/s5c73m3/s5c73m3-core.c index ce8fcf2..cb52438 100644 --- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c +++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c @@ -1457,6 +1457,12 @@ static int s5c73m3_oif_registered(struct v4l2_subdev *sd) return ret; } +static void s5c73m3_oif_unregistered(struct v4l2_subdev *sd) +{ + struct s5c73m3 *state = oif_sd_to_s5c73m3(sd); + v4l2_device_unregister_subdev(state-sensor_sd); +} + static const struct v4l2_subdev_internal_ops s5c73m3_internal_ops = { .open = s5c73m3_open, }; @@ -1474,6 +1480,7 @@ static const struct v4l2_subdev_ops s5c73m3_subdev_ops = { static const struct v4l2_subdev_internal_ops oif_internal_ops = { .registered = s5c73m3_oif_registered, + .unregistered = s5c73m3_oif_unregistered, .open = s5c73m3_oif_open, }; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/12] exynos4-is: Remove redundant MODULE_DEVICE_TABLE entries
Remove unneeded MODULE_DEVICE_TABLE(of,...) instances from files that are linked into same module. This fixes following error when building as a module: LD [M] drivers/media/platform/exynos4-is/s5p-fimc.o drivers/media/platform/exynos4-is/fimc-is-sensor.o: In function `.LANCHOR1': fimc-is-sensor.c:(.rodata+0x48): multiple definition of `__mod_of_device_table' drivers/media/platform/exynos4-is/fimc-is.o:fimc-is.c:(.rodata+0x174): first defined here drivers/media/platform/exynos4-is/fimc-is-i2c.o:(.rodata+0x5c): multiple definition of `__mod_of_device_table' drivers/media/platform/exynos4-is/fimc-is.o:fimc-is.c:(.rodata+0x174): first defined here make[4]: *** [drivers/media/platform/exynos4-is/exynos-fimc-is.o] Error 1 Also remove exporting fimc_is_(un)register_i2c_driver functions, it is not needed since these functions should be called only from our module. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/exynos4-is/fimc-is-i2c.c|3 --- drivers/media/platform/exynos4-is/fimc-is-sensor.c |1 - 2 files changed, 4 deletions(-) diff --git a/drivers/media/platform/exynos4-is/fimc-is-i2c.c b/drivers/media/platform/exynos4-is/fimc-is-i2c.c index 1ec6b3c..c39 100644 --- a/drivers/media/platform/exynos4-is/fimc-is-i2c.c +++ b/drivers/media/platform/exynos4-is/fimc-is-i2c.c @@ -103,7 +103,6 @@ static const struct of_device_id fimc_is_i2c_of_match[] = { { .compatible = FIMC_IS_I2C_COMPATIBLE }, { }, }; -MODULE_DEVICE_TABLE(of, fimc_is_i2c_of_match); static struct platform_driver fimc_is_i2c_driver = { .probe = fimc_is_i2c_probe, @@ -120,10 +119,8 @@ int fimc_is_register_i2c_driver(void) { return platform_driver_register(fimc_is_i2c_driver); } -EXPORT_SYMBOL(fimc_is_register_i2c_driver); void fimc_is_unregister_i2c_driver(void) { platform_driver_unregister(fimc_is_i2c_driver); } -EXPORT_SYMBOL(fimc_is_unregister_i2c_driver); diff --git a/drivers/media/platform/exynos4-is/fimc-is-sensor.c b/drivers/media/platform/exynos4-is/fimc-is-sensor.c index 02b2719..6b3ea54 100644 --- a/drivers/media/platform/exynos4-is/fimc-is-sensor.c +++ b/drivers/media/platform/exynos4-is/fimc-is-sensor.c @@ -294,7 +294,6 @@ static const struct of_device_id fimc_is_sensor_of_match[] = { }, { } }; -MODULE_DEVICE_TABLE(of, fimc_is_sensor_of_match); static struct i2c_driver fimc_is_sensor_driver = { .driver = { -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/12] exynos4-is: Fix initialization of subdev 'flags' field
Ensure the value of struct v4l2_subdev::flags field as set in v4l2_subdev_init() is preserved when initializing it in the subdev drivers. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/exynos4-is/fimc-capture.c |2 +- drivers/media/platform/exynos4-is/fimc-isp.c |2 +- drivers/media/platform/exynos4-is/fimc-lite.c|2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c b/drivers/media/platform/exynos4-is/fimc-capture.c index 72c516a..558c528 100644 --- a/drivers/media/platform/exynos4-is/fimc-capture.c +++ b/drivers/media/platform/exynos4-is/fimc-capture.c @@ -1869,7 +1869,7 @@ int fimc_initialize_capture_subdev(struct fimc_dev *fimc) int ret; v4l2_subdev_init(sd, fimc_subdev_ops); - sd-flags = V4L2_SUBDEV_FL_HAS_DEVNODE; + sd-flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; snprintf(sd-name, sizeof(sd-name), FIMC.%d, fimc-id); fimc-vid_cap.sd_pads[FIMC_SD_PAD_SINK_CAM].flags = MEDIA_PAD_FL_SINK; diff --git a/drivers/media/platform/exynos4-is/fimc-isp.c b/drivers/media/platform/exynos4-is/fimc-isp.c index 3b9a664..d63947f 100644 --- a/drivers/media/platform/exynos4-is/fimc-isp.c +++ b/drivers/media/platform/exynos4-is/fimc-isp.c @@ -621,7 +621,7 @@ int fimc_isp_subdev_create(struct fimc_isp *isp) v4l2_subdev_init(sd, fimc_is_subdev_ops); sd-grp_id = GRP_ID_FIMC_IS; - sd-flags = V4L2_SUBDEV_FL_HAS_DEVNODE; + sd-flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; snprintf(sd-name, sizeof(sd-name), FIMC-IS-ISP); isp-subdev_pads[FIMC_ISP_SD_PAD_SINK].flags = MEDIA_PAD_FL_SINK; diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c index 661d0d1..7ecf4e7 100644 --- a/drivers/media/platform/exynos4-is/fimc-lite.c +++ b/drivers/media/platform/exynos4-is/fimc-lite.c @@ -1377,7 +1377,7 @@ static int fimc_lite_create_capture_subdev(struct fimc_lite *fimc) int ret; v4l2_subdev_init(sd, fimc_lite_subdev_ops); - sd-flags = V4L2_SUBDEV_FL_HAS_DEVNODE; + sd-flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; snprintf(sd-name, sizeof(sd-name), FIMC-LITE.%d, fimc-index); fimc-subdev_pads[FLITE_SD_PAD_SINK].flags = MEDIA_PAD_FL_SINK; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/12] exynos4-is: Fix regulator/gpio resource releasing on the driver removal
Remove regulator_bulk_free() calls as devm_regulator_bulk_get() function is used to get the regulators so those will be freed automatically while the driver is removed. Missing gpio free is fixed by requesting a gpio with the devm_* API. All that is done now in the I2C client driver remove() callback is the media entity cleanup call. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/exynos4-is/fimc-is-sensor.c | 26 ++-- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/drivers/media/platform/exynos4-is/fimc-is-sensor.c b/drivers/media/platform/exynos4-is/fimc-is-sensor.c index 6b3ea54..035fa14 100644 --- a/drivers/media/platform/exynos4-is/fimc-is-sensor.c +++ b/drivers/media/platform/exynos4-is/fimc-is-sensor.c @@ -216,7 +216,8 @@ static int fimc_is_sensor_probe(struct i2c_client *client, gpio = of_get_gpio_flags(dev-of_node, 0, NULL); if (gpio_is_valid(gpio)) { - ret = gpio_request_one(gpio, GPIOF_OUT_INIT_LOW, DRIVER_NAME); + ret = devm_gpio_request_one(dev, gpio, GPIOF_OUT_INIT_LOW, + DRIVER_NAME); if (ret 0) return ret; } @@ -228,13 +229,11 @@ static int fimc_is_sensor_probe(struct i2c_client *client, ret = devm_regulator_bulk_get(client-dev, SENSOR_NUM_SUPPLIES, sensor-supplies); if (ret 0) - goto err_gpio; + return ret; of_id = of_match_node(fimc_is_sensor_of_match, dev-of_node); - if (!of_id) { - ret = -ENODEV; - goto err_reg; - } + if (!of_id) + return -ENODEV; sensor-drvdata = of_id-data; sensor-dev = dev; @@ -251,28 +250,19 @@ static int fimc_is_sensor_probe(struct i2c_client *client, sensor-pad.flags = MEDIA_PAD_FL_SOURCE; ret = media_entity_init(sd-entity, 1, sensor-pad, 0); if (ret 0) - goto err_reg; + return ret; v4l2_set_subdevdata(sd, sensor); pm_runtime_no_callbacks(dev); pm_runtime_enable(dev); - return 0; -err_reg: - regulator_bulk_free(SENSOR_NUM_SUPPLIES, sensor-supplies); -err_gpio: - if (gpio_is_valid(sensor-gpio_reset)) - gpio_free(sensor-gpio_reset); return ret; } static int fimc_is_sensor_remove(struct i2c_client *client) { - struct fimc_is_sensor *sensor; - - regulator_bulk_free(SENSOR_NUM_SUPPLIES, sensor-supplies); - media_entity_cleanup(sensor-subdev.entity); - + struct v4l2_subdev *sd = i2c_get_clientdata(client); + media_entity_cleanup(sd-entity); return 0; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/12] exynos4-is: Don't overwrite subdevdata in the fimc-is sensor driver
It's an I2C client driver and it must not overwrite the struct v4l2_subdev dev_priv field, which is used by the v4l2 core to store a pointer to struct i2c_client. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/exynos4-is/fimc-is-sensor.c |8 +--- drivers/media/platform/exynos4-is/fimc-is-sensor.h |6 ++ drivers/media/platform/exynos4-is/fimc-is.c|2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/media/platform/exynos4-is/fimc-is-sensor.c b/drivers/media/platform/exynos4-is/fimc-is-sensor.c index 035fa14..6647421 100644 --- a/drivers/media/platform/exynos4-is/fimc-is-sensor.c +++ b/drivers/media/platform/exynos4-is/fimc-is-sensor.c @@ -40,11 +40,6 @@ static const struct v4l2_mbus_framefmt fimc_is_sensor_formats[] = { } }; -static struct fimc_is_sensor *sd_to_fimc_is_sensor(struct v4l2_subdev *sd) -{ - return container_of(sd, struct fimc_is_sensor, subdev); -} - static const struct v4l2_mbus_framefmt *find_sensor_format( struct v4l2_mbus_framefmt *mf) { @@ -147,7 +142,7 @@ static const struct v4l2_subdev_internal_ops fimc_is_sensor_sd_internal_ops = { static int fimc_is_sensor_s_power(struct v4l2_subdev *sd, int on) { - struct fimc_is_sensor *sensor = v4l2_get_subdevdata(sd); + struct fimc_is_sensor *sensor = sd_to_fimc_is_sensor(sd); int gpio = sensor-gpio_reset; int ret; @@ -252,7 +247,6 @@ static int fimc_is_sensor_probe(struct i2c_client *client, if (ret 0) return ret; - v4l2_set_subdevdata(sd, sensor); pm_runtime_no_callbacks(dev); pm_runtime_enable(dev); diff --git a/drivers/media/platform/exynos4-is/fimc-is-sensor.h b/drivers/media/platform/exynos4-is/fimc-is-sensor.h index 50b8e4d..6036d49 100644 --- a/drivers/media/platform/exynos4-is/fimc-is-sensor.h +++ b/drivers/media/platform/exynos4-is/fimc-is-sensor.h @@ -77,6 +77,12 @@ struct fimc_is_sensor { struct v4l2_mbus_framefmt format; }; +static inline +struct fimc_is_sensor *sd_to_fimc_is_sensor(struct v4l2_subdev *sd) +{ + return container_of(sd, struct fimc_is_sensor, subdev); +} + int fimc_is_register_sensor_driver(void); void fimc_is_unregister_sensor_driver(void); diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c index 3c81c88..c4049d4 100644 --- a/drivers/media/platform/exynos4-is/fimc-is.c +++ b/drivers/media/platform/exynos4-is/fimc-is.c @@ -220,7 +220,7 @@ static int fimc_is_register_subdevs(struct fimc_is *is) if (WARN_ON(is-sensor)) continue; - is-sensor = v4l2_get_subdevdata(sd); + is-sensor = sd_to_fimc_is_sensor(sd); if (fimc_is_parse_sensor_config(is-sensor, child)) { dev_warn(is-pdev-dev, DT parse error: %s\n, -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/12] exynos4-is: Unregister fimc-is subdevs from the media device properly
Add missing v4l2_device_unregister_subdev() call for the FIMC-IS subdevs (currently there is only the FIMC-IS-ISP subdev) so corresponding resources are properly freed upon the media device driver module removal. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyugmin Park kyungmin.p...@samsung.com --- drivers/media/platform/exynos4-is/media-dev.c |4 1 file changed, 4 insertions(+) diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c index 1dbd554..a371ee5 100644 --- a/drivers/media/platform/exynos4-is/media-dev.c +++ b/drivers/media/platform/exynos4-is/media-dev.c @@ -823,6 +823,10 @@ static void fimc_md_unregister_entities(struct fimc_md *fmd) fimc_md_unregister_sensor(fmd-sensor[i].subdev); fmd-sensor[i].subdev = NULL; } + + if (fmd-fimc_is) + v4l2_device_unregister_subdev(fmd-fimc_is-isp.subdev); + v4l2_info(fmd-v4l2_dev, Unregistered all entities\n); } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/12] exynos4-is: Set fimc-lite subdev subdev owner module
The FIMC-LITE.n subdevs have currently sd-owner field not set, the exynos-fimc-lite module can be removed at any time, regardless it is in use by other modules. When this module is unloaded the kernel can crash easily by accessing video or media device nodes. Cc: sta...@vger.kernel.org Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/exynos4-is/fimc-lite.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c index 7ecf4e7..14bb7bc 100644 --- a/drivers/media/platform/exynos4-is/fimc-lite.c +++ b/drivers/media/platform/exynos4-is/fimc-lite.c @@ -1399,6 +1399,7 @@ static int fimc_lite_create_capture_subdev(struct fimc_lite *fimc) sd-ctrl_handler = handler; sd-internal_ops = fimc_lite_subdev_internal_ops; sd-entity.ops = fimc_lite_subdev_media_ops; + sd-owner = THIS_MODULE; v4l2_set_subdevdata(sd, fimc); return 0; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/12] exynos4-is: Remove redundant module_put() for MIPI-CSIS module
Currently there is unbalanced module_put() on the s5p-csis module which prevents it from being unloaded. The subdev's owner module has reference count decremented in v4l2_device_unregister_subdev() so just remove this erroneous call. Cc: sta...@vger.kernel.org # 3.8 Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/exynos4-is/media-dev.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c index a371ee5..15ef8f2 100644 --- a/drivers/media/platform/exynos4-is/media-dev.c +++ b/drivers/media/platform/exynos4-is/media-dev.c @@ -814,7 +814,6 @@ static void fimc_md_unregister_entities(struct fimc_md *fmd) if (fmd-csis[i].sd == NULL) continue; v4l2_device_unregister_subdev(fmd-csis[i].sd); - module_put(fmd-csis[i].sd-owner); fmd-csis[i].sd = NULL; } for (i = 0; i fmd-num_sensors; i++) { -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/12] exynos4-is: Remove debugfs entries properly
Ensure both debugfs: fimc_is directory and the fw_log file are properly removed in the driver cleanup sequence. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/exynos4-is/fimc-is.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c index c4049d4..ca72b02 100644 --- a/drivers/media/platform/exynos4-is/fimc-is.c +++ b/drivers/media/platform/exynos4-is/fimc-is.c @@ -766,7 +766,7 @@ static const struct file_operations fimc_is_debugfs_fops = { static void fimc_is_debugfs_remove(struct fimc_is *is) { - debugfs_remove(is-debugfs_entry); + debugfs_remove_recursive(is-debugfs_entry); is-debugfs_entry = NULL; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/12] exynos4-is: Fix runtime PM handling on fimc-is probe error path
Ensure there is no unbalanced pm_runtime_put(). Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/media/platform/exynos4-is/fimc-is.c |7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c index 5e89077..47c6363 100644 --- a/drivers/media/platform/exynos4-is/fimc-is.c +++ b/drivers/media/platform/exynos4-is/fimc-is.c @@ -847,16 +847,17 @@ static int fimc_is_probe(struct platform_device *pdev) goto err_irq; ret = fimc_is_setup_clocks(is); + pm_runtime_put_sync(dev); + if (ret 0) goto err_irq; - pm_runtime_put_sync(dev); is-clk_init = true; is-alloc_ctx = vb2_dma_contig_init_ctx(dev); if (IS_ERR(is-alloc_ctx)) { ret = PTR_ERR(is-alloc_ctx); - goto err_pm; + goto err_irq; } /* * Register FIMC-IS V4L2 subdevs to this driver. The video nodes @@ -885,8 +886,6 @@ err_sd: fimc_is_unregister_subdevs(is); err_irq: free_irq(is-irq, is); -err_pm: - pm_runtime_put(dev); err_clk: fimc_is_put_clocks(is); return ret; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/12] exynos4-is driver fixes
On 04/22/2013 04:03 PM, Sylwester Nawrocki wrote: This patch series includes fixes for several issues found during testing all exynos4-is device drivers build as modules. The exynos4-is build with all sub-drivers as 'M' is hopefully now free of all serious issues, but one. I.e. the requirement now is to have all sub-device drivers, including the sensor subdev drivers, built as modules. Hmm, to avoid issues all drivers must now be either statically linked or build as modules and all need to be inserted, the all removed. Leaving any one loaded all time may lead to a disaster... This is not a new issue and and is related to all drivers using MC framework, thus I plan to address it for 3.11. The problem when some of the sub-device drivers is statically linked is that the media links of a media entity just unregistered from the media device are not fully cleaned up in the media controller API. This means other entities can have dangling pointers to the links array owned by en entity just removed and freed. The problem is not existent when all media entites are registered/unregistred together. In such a case it doesn't hurt that media_entity_cleanup() function just frees the links array. I will post a separate RFC patch to address this issue, since it is not trivial where the link references should be removed from all involved media entities. I verified that adding a call to media_entity_remove_links() as in patch [1] to the v4l2_sdubdev_unregister_function() eliminates all weird crashes present before, when inserting/removing all the host driver modules while the sensor driver stays loaded. [1] http://git.linuxtv.org/snawrocki/samsung.git/commitdiff/f7007880a37c28beef845aa0787696aa8cead1cd -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2] video: ARM CLCD: Add DT CDF support
On Thu, Apr 18, 2013 at 06:33:21PM +0100, Pawel Moll wrote: This patch adds basic DT bindings for the PL11x CLCD cells and make their fbdev driver use them, together with the Common Display Framework. The DT provides information about the hardware configuration and limitations (eg. the largest supported resolution) but the video modes come exclusively from the Common Display Framework drivers, referenced to by the standard CDF binding. Signed-off-by: Pawel Moll pawel.m...@arm.com Much better. I will point out though that there be all sorts of worms here when you come to the previous ARM evaluation boards (which is why the capabilities stuff got written in the first place) where there's a horrid mixture of BGR/RGB ordering at various levels of the system - some of which must be set correctly because the CLCD output isn't strictly used as R bits G bits and B bits (to support different formats from the CLCD's native formats.) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: WARNINGS
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: Mon Apr 22 19:00:21 CEST 2013 git branch: test git hash: 6695be6863b75620ffa6d422965680ce785cb7c8 gcc version:i686-linux-gcc (GCC) 4.7.2 host hardware: x86_64 host os:3.8-3.slh.2-amd64 linux-git-arm-davinci: OK linux-git-arm-exynos: WARNINGS linux-git-arm-omap: WARNINGS linux-git-blackfin: WARNINGS 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.31.14-i686: WARNINGS linux-2.6.32.27-i686: WARNINGS linux-2.6.33.7-i686: WARNINGS linux-2.6.34.7-i686: WARNINGS linux-2.6.35.9-i686: WARNINGS 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: WARNINGS linux-3.5.7-i686: WARNINGS linux-3.6.11-i686: WARNINGS linux-3.7.4-i686: WARNINGS linux-3.8-i686: OK linux-3.9-rc1-i686: OK linux-2.6.31.14-x86_64: WARNINGS linux-2.6.32.27-x86_64: WARNINGS linux-2.6.33.7-x86_64: WARNINGS linux-2.6.34.7-x86_64: WARNINGS linux-2.6.35.9-x86_64: WARNINGS 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: WARNINGS 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: OK linux-3.9-rc1-x86_64: OK apps: WARNINGS spec-git: OK sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] media/rc/imon.c: do not try to register 2nd intf if 1st intf failed
This bug could be triggered if 1st interface configuration fails: Apr 8 18:20:30 homeserver kernel: usb 5-1: new low-speed USB device number 2 using ohci_hcd Apr 8 18:20:30 homeserver kernel: input: iMON Panel, Knob and Mouse(15c2:0036) as /devices/pci:00/:00:13.0/usb5/5-1/5-1:1.0/input/input2 Apr 8 18:20:30 homeserver kernel: Registered IR keymap rc-imon-pad Apr 8 18:20:30 homeserver kernel: input: iMON Remote (15c2:0036) as /devices/pci:00/:00:13.0/usb5/5-1/5-1:1.0/rc/rc0/input3 Apr 8 18:20:30 homeserver kernel: rc0: iMON Remote (15c2:0036) as /devices/pci:00/:00:13.0/usb5/5-1/5-1:1.0/rc/rc0 Apr 8 18:20:30 homeserver kernel: imon:send_packet: packet tx failed (-32) Apr 8 18:20:30 homeserver kernel: imon 5-1:1.0: remote input dev register failed Apr 8 18:20:30 homeserver kernel: imon 5-1:1.0: imon_init_intf0: rc device setup failed Apr 8 18:20:30 homeserver kernel: imon 5-1:1.0: unable to initialize intf0, err 0 Apr 8 18:20:30 homeserver kernel: imon:imon_probe: failed to initialize context! Apr 8 18:20:30 homeserver kernel: imon 5-1:1.0: unable to register, err -19 Apr 8 18:20:30 homeserver kernel: BUG: unable to handle kernel NULL pointer dereference at 0014 Apr 8 18:20:30 homeserver kernel: IP: [c05c4e4c] mutex_lock+0xc/0x30 Apr 8 18:20:30 homeserver kernel: *pde = Apr 8 18:20:30 homeserver kernel: Oops: 0002 [#1] PREEMPT SMP Apr 8 18:20:30 homeserver kernel: Modules linked in: Apr 8 18:20:30 homeserver kernel: Pid: 367, comm: khubd Not tainted 3.8.3-htpc-2-g79b1403 #23 Unknow Unknow/RS780-SB700 Apr 8 18:20:30 homeserver kernel: EIP: 0060:[c05c4e4c] EFLAGS: 00010296 CPU: 1 Apr 8 18:20:30 homeserver kernel: EIP is at mutex_lock+0xc/0x30 Apr 8 18:20:30 homeserver kernel: EAX: 0014 EBX: 0014 ECX: EDX: f590e480 Apr 8 18:20:30 homeserver kernel: ESI: f5deac00 EDI: f590e480 EBP: f5f3ee00 ESP: f6577c28 Apr 8 18:20:30 homeserver kernel: DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 Apr 8 18:20:30 homeserver kernel: CR0: 8005003b CR2: 0014 CR3: 0081b000 CR4: 07d0 Apr 8 18:20:30 homeserver kernel: DR0: DR1: DR2: DR3: Apr 8 18:20:30 homeserver kernel: DR6: 0ff0 DR7: 0400 Apr 8 18:20:30 homeserver kernel: Process khubd (pid: 367, ti=f6576000 task=f649ea00 task.ti=f6576000) Apr 8 18:20:30 homeserver kernel: Stack: Apr 8 18:20:30 homeserver kernel: f5deac00 c0448de4 f59714c0 f5deac64 c03b8ad2 f6577c90 0004 Apr 8 18:20:30 homeserver kernel: f649ea00 c0205142 f6779820 a1ff7f08 f5deac00 0001 f5f3ee1c 0014 Apr 8 18:20:30 homeserver kernel: 0004 0202 15c20036 c07a03e8 fffee0ca f6795c00 f5f3ee1c f5deac00 Apr 8 18:20:30 homeserver kernel: Call Trace: Apr 8 18:20:30 homeserver kernel: [c0448de4] ? imon_probe+0x494/0xde0 Apr 8 18:20:30 homeserver kernel: [c03b8ad2] ? rpm_resume+0xb2/0x4f0 Apr 8 18:20:30 homeserver kernel: [c0205142] ? sysfs_addrm_finish+0x12/0x90 Apr 8 18:20:30 homeserver kernel: [c04170e9] ? usb_probe_interface+0x169/0x240 Apr 8 18:20:30 homeserver kernel: [c03b0ca0] ? __driver_attach+0x80/0x80 Apr 8 18:20:30 homeserver kernel: [c03b0ca0] ? __driver_attach+0x80/0x80 Apr 8 18:20:30 homeserver kernel: [c03b0a94] ? driver_probe_device+0x54/0x1e0 Apr 8 18:20:30 homeserver kernel: [c0416abe] ? usb_device_match+0x4e/0x80 Apr 8 18:20:30 homeserver kernel: [c03af314] ? bus_for_each_drv+0x34/0x70 Apr 8 18:20:30 homeserver kernel: [c03b0a0b] ? device_attach+0x7b/0x90 Apr 8 18:20:30 homeserver kernel: [c03b0ca0] ? __driver_attach+0x80/0x80 Apr 8 18:20:30 homeserver kernel: [c03b00ff] ? bus_probe_device+0x5f/0x80 Apr 8 18:20:30 homeserver kernel: [c03aeab7] ? device_add+0x567/0x610 Apr 8 18:20:30 homeserver kernel: [c041a7bc] ? usb_create_ep_devs+0x7c/0xd0 Apr 8 18:20:30 homeserver kernel: [c0413837] ? create_intf_ep_devs+0x47/0x70 Apr 8 18:20:30 homeserver kernel: [c04156c4] ? usb_set_configuration+0x454/0x750 Apr 8 18:20:30 homeserver kernel: [c03b0ca0] ? __driver_attach+0x80/0x80 Apr 8 18:20:30 homeserver kernel: [c041de8a] ? generic_probe+0x2a/0x80 Apr 8 18:20:30 homeserver kernel: [c03b0ca0] ? __driver_attach+0x80/0x80 Apr 8 18:20:30 homeserver kernel: [c0205aff] ? sysfs_create_link+0xf/0x20 Apr 8 18:20:30 homeserver kernel: [c04171db] ? usb_probe_device+0x1b/0x40 Apr 8 18:20:30 homeserver kernel: [c03b0a94] ? driver_probe_device+0x54/0x1e0 Apr 8 18:20:30 homeserver kernel: [c03af314] ? bus_for_each_drv+0x34/0x70 Apr 8 18:20:30 homeserver kernel: [c03b0a0b] ? device_attach+0x7b/0x90 Apr 8 18:20:30 homeserver kernel: [c03b0ca0] ? __driver_attach+0x80/0x80 Apr 8 18:20:30 homeserver kernel: [c03b00ff] ? bus_probe_device+0x5f/0x80 Apr 8 18:20:30 homeserver kernel: [c03aeab7] ? device_add+0x567/0x610 Apr 8 18:20:30 homeserver kernel: [c040e6df] ? usb_new_device+0x12f/0x1e0 Apr 8 18:20:30 homeserver kernel: [c040f4d8] ? hub_thread+0x458/0x1230 Apr 8 18:20:30 homeserver kernel: [c015554f] ?
[PATCH 2/4] media/rc/imon.c: make send_packet() delay larger for 15c2:0036 [v2]
Imon device 15c2:0036 need a higher delay between send_packet() calls. Also use interruptible wait to avoid load average going too high (and let caller handle signals). Signed-off-by: Kevin Baradon kevin.bara...@gmail.com --- drivers/media/rc/imon.c | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c index e5d1c0d..624fd33 100644 --- a/drivers/media/rc/imon.c +++ b/drivers/media/rc/imon.c @@ -112,6 +112,7 @@ struct imon_context { bool tx_control; unsigned char usb_rx_buf[8]; unsigned char usb_tx_buf[8]; + unsigned int send_packet_delay; struct tx_t { unsigned char data_buf[35]; /* user data buffer */ @@ -185,6 +186,10 @@ enum { IMON_KEY_PANEL = 2, }; +enum { + IMON_NEED_20MS_PKT_DELAY = 1 +}; + /* * USB Device ID for iMON USB Control Boards * @@ -215,7 +220,7 @@ static struct usb_device_id imon_usb_id_table[] = { /* SoundGraph iMON OEM Touch LCD (IR 4.3 VGA LCD) */ { USB_DEVICE(0x15c2, 0x0035) }, /* SoundGraph iMON OEM VFD (IR VFD) */ - { USB_DEVICE(0x15c2, 0x0036) }, + { USB_DEVICE(0x15c2, 0x0036), .driver_info = IMON_NEED_20MS_PKT_DELAY }, /* device specifics unknown */ { USB_DEVICE(0x15c2, 0x0037) }, /* SoundGraph iMON OEM LCD (IR LCD) */ @@ -535,12 +540,12 @@ static int send_packet(struct imon_context *ictx) kfree(control_req); /* -* Induce a mandatory 5ms delay before returning, as otherwise, +* Induce a mandatory delay before returning, as otherwise, * send_packet can get called so rapidly as to overwhelm the device, * particularly on faster systems and/or those with quirky usb. */ - timeout = msecs_to_jiffies(5); - set_current_state(TASK_UNINTERRUPTIBLE); + timeout = msecs_to_jiffies(ictx-send_packet_delay); + set_current_state(TASK_INTERRUPTIBLE); schedule_timeout(timeout); return retval; @@ -2088,7 +2093,8 @@ static bool imon_find_endpoints(struct imon_context *ictx, } -static struct imon_context *imon_init_intf0(struct usb_interface *intf) +static struct imon_context *imon_init_intf0(struct usb_interface *intf, + const struct usb_device_id *id) { struct imon_context *ictx; struct urb *rx_urb; @@ -2128,6 +2134,10 @@ static struct imon_context *imon_init_intf0(struct usb_interface *intf) ictx-vendor = le16_to_cpu(ictx-usbdev_intf0-descriptor.idVendor); ictx-product = le16_to_cpu(ictx-usbdev_intf0-descriptor.idProduct); + /* default send_packet delay is 5ms but some devices need more */ + ictx-send_packet_delay = id-driver_info IMON_NEED_20MS_PKT_DELAY ? + 20 : 5; + ret = -ENODEV; iface_desc = intf-cur_altsetting; if (!imon_find_endpoints(ictx, iface_desc)) { @@ -2306,7 +2316,7 @@ static int imon_probe(struct usb_interface *interface, first_if_ctx = usb_get_intfdata(first_if); if (ifnum == 0) { - ictx = imon_init_intf0(interface); + ictx = imon_init_intf0(interface, id); if (!ictx) { pr_err(failed to initialize context!\n); ret = -ENODEV; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] media/rc/imon.c: kill urb when send_packet() is interrupted
This avoids: Apr 12 23:52:16 homeserver kernel: imon:send_packet: task interrupted Apr 12 23:52:16 homeserver kernel: [ cut here ] Apr 12 23:52:16 homeserver kernel: WARNING: at drivers/usb/core/urb.c:327 usb_submit_urb+0x353/0x370() Apr 12 23:52:16 homeserver kernel: Hardware name: Unknow Apr 12 23:52:16 homeserver kernel: URB f64b6f00 submitted while active Apr 12 23:52:16 homeserver kernel: Modules linked in: Apr 12 23:52:16 homeserver kernel: Pid: 3154, comm: LCDd Not tainted 3.8.6-htpc-5-g9e6fc5e #26 Apr 12 23:52:16 homeserver kernel: Call Trace: Apr 12 23:52:16 homeserver kernel: [c012d778] ? warn_slowpath_common+0x78/0xb0 Apr 12 23:52:16 homeserver kernel: [c04136c3] ? usb_submit_urb+0x353/0x370 Apr 12 23:52:16 homeserver kernel: [c04136c3] ? usb_submit_urb+0x353/0x370 Apr 12 23:52:16 homeserver kernel: [c0447010] ? imon_ir_change_protocol+0x150/0x150 Apr 12 23:52:16 homeserver kernel: [c012d843] ? warn_slowpath_fmt+0x33/0x40 Apr 12 23:52:16 homeserver kernel: [c04136c3] ? usb_submit_urb+0x353/0x370 Apr 12 23:52:16 homeserver kernel: [c0446c67] ? send_packet+0x97/0x270 Apr 12 23:52:16 homeserver kernel: [c0446cfe] ? send_packet+0x12e/0x270 Apr 12 23:52:16 homeserver kernel: [c05c5743] ? do_nanosleep+0xa3/0xd0 Apr 12 23:52:16 homeserver kernel: [c044760e] ? vfd_write+0xae/0x250 Apr 12 23:52:16 homeserver kernel: [c0447560] ? lcd_write+0x180/0x180 Apr 12 23:52:16 homeserver kernel: [c01b2b19] ? vfs_write+0x89/0x140 Apr 12 23:52:16 homeserver kernel: [c01b2dda] ? sys_write+0x4a/0x90 Apr 12 23:52:16 homeserver kernel: [c05c7c45] ? sysenter_do_call+0x12/0x26 Apr 12 23:52:16 homeserver kernel: ---[ end trace a0b6f0fcfd2f9a1d ]--- Apr 12 23:52:16 homeserver kernel: imon:send_packet: error submitting urb(-16) Apr 12 23:52:16 homeserver kernel: imon:vfd_write: send packet #3 failed Apr 12 23:52:16 homeserver kernel: imon:send_packet: error submitting urb(-16) Apr 12 23:52:16 homeserver kernel: imon:vfd_write: send packet #0 failed Signed-off-by: Kevin Baradon kevin.bara...@gmail.com --- drivers/media/rc/imon.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c index 3af7bb6..72e3fa6 100644 --- a/drivers/media/rc/imon.c +++ b/drivers/media/rc/imon.c @@ -528,8 +528,10 @@ static int send_packet(struct imon_context *ictx) mutex_unlock(ictx-lock); retval = wait_for_completion_interruptible( ictx-tx.finished); - if (retval) + if (retval) { + usb_kill_urb(ictx-tx_urb); pr_err_ratelimited(task interrupted\n); + } mutex_lock(ictx-lock); retval = ictx-tx.status; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] Revert media/rc/imon.c: make send_packet() delay larger for 15c2:0036
This reverts commit d92f150f9cb80b4df56331d1f42442da78e351f0. It seems send_packet() is used during initialization, before send_packet_delay is set. This will be fixed by another patch. Signed-off-by: Kevin Baradon kevin.bara...@gmail.com --- drivers/media/rc/imon.c | 17 - 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/drivers/media/rc/imon.c b/drivers/media/rc/imon.c index b8f9f85..e5d1c0d 100644 --- a/drivers/media/rc/imon.c +++ b/drivers/media/rc/imon.c @@ -112,7 +112,6 @@ struct imon_context { bool tx_control; unsigned char usb_rx_buf[8]; unsigned char usb_tx_buf[8]; - unsigned int send_packet_delay; struct tx_t { unsigned char data_buf[35]; /* user data buffer */ @@ -186,10 +185,6 @@ enum { IMON_KEY_PANEL = 2, }; -enum { - IMON_NEED_20MS_PKT_DELAY = 1 -}; - /* * USB Device ID for iMON USB Control Boards * @@ -220,7 +215,7 @@ static struct usb_device_id imon_usb_id_table[] = { /* SoundGraph iMON OEM Touch LCD (IR 4.3 VGA LCD) */ { USB_DEVICE(0x15c2, 0x0035) }, /* SoundGraph iMON OEM VFD (IR VFD) */ - { USB_DEVICE(0x15c2, 0x0036), .driver_info = IMON_NEED_20MS_PKT_DELAY }, + { USB_DEVICE(0x15c2, 0x0036) }, /* device specifics unknown */ { USB_DEVICE(0x15c2, 0x0037) }, /* SoundGraph iMON OEM LCD (IR LCD) */ @@ -540,12 +535,12 @@ static int send_packet(struct imon_context *ictx) kfree(control_req); /* -* Induce a mandatory delay before returning, as otherwise, +* Induce a mandatory 5ms delay before returning, as otherwise, * send_packet can get called so rapidly as to overwhelm the device, * particularly on faster systems and/or those with quirky usb. */ - timeout = msecs_to_jiffies(ictx-send_packet_delay); - set_current_state(TASK_INTERRUPTIBLE); + timeout = msecs_to_jiffies(5); + set_current_state(TASK_UNINTERRUPTIBLE); schedule_timeout(timeout); return retval; @@ -2329,10 +2324,6 @@ static int imon_probe(struct usb_interface *interface, } - /* default send_packet delay is 5ms but some devices need more */ - ictx-send_packet_delay = id-driver_info IMON_NEED_20MS_PKT_DELAY ? - 20 : 5; - usb_set_intfdata(interface, ictx); if (ifnum == 0) { -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] Revert buggy patch and fix other issues with imon driver
Hi, Following patchset fixes several issues with imon driver and reverts one (previously applied) buggy patch. Mauro, could you please apply this on top of your tree? Best regards, Kevin Baradon (4): Revert media/rc/imon.c: make send_packet() delay larger for 15c2:0036 media/rc/imon.c: make send_packet() delay larger for 15c2:0036 [v2] media/rc/imon.c: do not try to register 2nd intf if 1st intf failed media/rc/imon.c: kill urb when send_packet() is interrupted drivers/media/rc/imon.c | 26 ++ 1 file changed, 18 insertions(+), 8 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] V4L2: I2C: ML86V7667 video decoder driver
Hi Hans, Hans Verkuil wrote: +*/ + val = i2c_smbus_read_byte_data(client, STATUS_REG); + if (val 0) + return val; + + priv-std = val STATUS_NTSCPAL ? V4L2_STD_PAL : V4L2_STD_NTSC; Shouldn't this be 50 Hz vs 60 Hz formats? There are 60 Hz PAL standards and usually these devices detect 50 Hz vs 60 Hz, not NTSC vs PAL. In the reference manual it is not mentioned about 50/60Hz input format selection/detection but it mentioned just PAL/NTSC. The 50hz formats can be ether PAL and NTSC formats variants. The same is applied to 60Hz. In the ML86V7667 datasheet the description for STATUS register detection bit is just PAL/NTSC: $2C/STATUS [2] NTSC/PAL identification 0: NTSC /1: PAL If you assure me that I must judge their description as 50 vs 60Hz formats and not PAL/NTSC then I will make the change. I can't judge that. Are there no status bits anywhere that tell you something about the number of lines per frame or the framerate? You are right. I've found a relationship table with description of number of total H/V pixels vs Video Modes mentioned in datasheet. It's NTSC has Odd/263 and Even/262 vertical lines. The PAL has Odd/312 and Even/313. So I will change the standard per your suggestion. Are you able to test with a PAL-M or PAL-N(c) input? Unfortunately I cannot. I have a couple of different cameras and all of them mention PAL output with number of lines in the technical manual. All of them are 625 lines. Can you ask the manufacturer for more information? It can take a while for waiting their feedback since OKI was significantly reorganized. Than you very much for your valuable feedback/review. Regards, Vladimir -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] V4L2: I2C: ML86V7667 video decoder driver
Vladimir Barinov wrote: Hi Hans, Hans Verkuil wrote: + */ +val = i2c_smbus_read_byte_data(client, STATUS_REG); +if (val 0) +return val; + +priv-std = val STATUS_NTSCPAL ? V4L2_STD_PAL : V4L2_STD_NTSC; Shouldn't this be 50 Hz vs 60 Hz formats? There are 60 Hz PAL standards and usually these devices detect 50 Hz vs 60 Hz, not NTSC vs PAL. In the reference manual it is not mentioned about 50/60Hz input format selection/detection but it mentioned just PAL/NTSC. The 50hz formats can be ether PAL and NTSC formats variants. The same is applied to 60Hz. In the ML86V7667 datasheet the description for STATUS register detection bit is just PAL/NTSC: $2C/STATUS [2] NTSC/PAL identification 0: NTSC /1: PAL If you assure me that I must judge their description as 50 vs 60Hz formats and not PAL/NTSC then I will make the change. I can't judge that. Are there no status bits anywhere that tell you something about the number of lines per frame or the framerate? You are right. I've found a relationship table with description of number of total H/V pixels vs Video Modes mentioned in datasheet. It's NTSC has Odd/263 and Even/262 vertical lines. The PAL has Odd/312 and Even/313. A little clarification: it's NTSC relates to 485 active lines (after rejection of blank lines) and PAL relates to 578 active lines. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] ARM: shmobile: r8a7779: add VIN support
On Mon, Apr 22, 2013 at 04:37:33PM +0400, Sergei Shtylyov wrote: Hello. On 22-04-2013 8:57, Simon Horman wrote: From: Vladimir Barinov vladimir.bari...@cogentembedded.com Add VIN clocks and platform devices for R8A7779 SoC; add function to register the VIN platform devices. Signed-off-by: Vladimir Barinov vladimir.bari...@cogentembedded.com [Sergei: added 'id' parameter check to r8a7779_add_vin_device(), renamed some variables.] Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com [...] Index: renesas/arch/arm/mach-shmobile/setup-r8a7779.c === --- renesas.orig/arch/arm/mach-shmobile/setup-r8a7779.c +++ renesas/arch/arm/mach-shmobile/setup-r8a7779.c @@ -559,6 +559,33 @@ static struct resource ether_resources[] }, }; +#define R8A7779_VIN(idx) \ +static struct resource vin##idx##_resources[] = { \ + DEFINE_RES_MEM(0xffc5 + 0x1000 * (idx), 0x1000),\ + DEFINE_RES_IRQ(gic_iid(0x5f + (idx))), \ +};\ + \ +static struct platform_device_info vin##idx##_info = {\ Hm, probably should have marked this as '__initdata'... maybe the resources too. That doesn't seem to be the case for other devices in that or other shmobile files. Am I missing something or should numerous other devices be updated? If the device is registered using platform_device_register_*(), it seems worth marking the resources, the platfrom data and 'struct platform_device_info' as '__initdata' as they're copied to the memory allocated from heap anyway and hence not needed past the init phase... Thanks for the explanation, that make sense. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] V4L2: soc_camera: Renesas R-Car VIN driver
Hi Sergei, Thanks for the patch. From: Sergei Shtylyov sergei.shtyl...@cogentembedded.com Date: Sat, 20 Apr 2013 02:31:31 +0400 From: Vladimir Barinov vladimir.bari...@cogentembedded.com Add Renesas R-Car VIN (Video In) V4L2 driver. Based on the patch by Phil Edworthy phil.edwor...@renesas.com. Signed-off-by: Vladimir Barinov vladimir.bari...@cogentembedded.com [Sergei: removed deprecated IRQF_DISABLED flag.] Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com --- Changes since the original posting: - added IRQF_SHARED flag in devm_request_irq() call (since on R8A7778 VIN0/1 share the same IRQ) and removed deprecated IRQF_DISABLED flag. drivers/media/platform/soc_camera/Kconfig|7 drivers/media/platform/soc_camera/Makefile |1 drivers/media/platform/soc_camera/rcar_vin.c | 1784 +++ include/linux/platform_data/camera-rcar.h| 25 4 files changed, 1817 insertions(+) Index: renesas/drivers/media/platform/soc_camera/Kconfig === --- renesas.orig/drivers/media/platform/soc_camera/Kconfig +++ renesas/drivers/media/platform/soc_camera/Kconfig @@ -45,6 +45,13 @@ config VIDEO_PXA27x ---help--- This is a v4l2 driver for the PXA27x Quick Capture Interface +config VIDEO_RCAR_VIN + tristate R-Car Video Input (VIN) support + depends on VIDEO_DEV SOC_CAMERA (ARCH_R8A7778 || ARCH_R8A7779) + select VIDEOBUF2_DMA_CONTIG + ---help--- + This is a v4l2 driver for the R-Car VIN Interface + config VIDEO_SH_MOBILE_CSI2 tristate SuperH Mobile MIPI CSI-2 Interface driver depends on VIDEO_DEV SOC_CAMERA HAVE_CLK Index: renesas/drivers/media/platform/soc_camera/Makefile === --- renesas.orig/drivers/media/platform/soc_camera/Makefile +++ renesas/drivers/media/platform/soc_camera/Makefile @@ -10,5 +10,6 @@ obj-$(CONFIG_VIDEO_OMAP1) += omap1_came obj-$(CONFIG_VIDEO_PXA27x) += pxa_camera.o obj-$(CONFIG_VIDEO_SH_MOBILE_CEU)+= sh_mobile_ceu_camera.o obj-$(CONFIG_VIDEO_SH_MOBILE_CSI2) += sh_mobile_csi2.o +obj-$(CONFIG_VIDEO_RCAR_VIN) += rcar_vin.o ccflags-y += -I$(srctree)/drivers/media/i2c/soc_camera Index: renesas/drivers/media/platform/soc_camera/rcar_vin.c === --- /dev/null +++ renesas/drivers/media/platform/soc_camera/rcar_vin.c @@ -0,0 +1,1784 @@ +/* + * SoC-camera host driver for Renesas R-Car VIN unit + * + * Copyright (C) 2011-2013 Renesas Solutions Corp. + * Copyright (C) 2013 Cogent Embedded, Inc., sou...@cogentembedded.com + * + * Based on V4L2 Driver for SuperH Mobile CEU interface sh_mobile_ceu_camera.c + * + * Copyright (C) 2008 Magnus Damm + * + * 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 linux/kernel.h +#include linux/module.h +#include linux/interrupt.h +#include linux/slab.h +#include linux/delay.h +#include linux/platform_device.h +#include linux/pm_runtime.h +#include linux/platform_data/camera-rcar.h + +#include media/videobuf2-dma-contig.h +#include media/soc_camera.h +#include media/soc_mediabus.h + +#define DRV_NAME rcar_vin + +/* Register offsets for R-Сar VIN */ Are you using a 2-byte character in the string 'R-Car'? +#define VNMC_REG 0x00/* Video n Main Control Register */ +#define VNMS_REG 0x04/* Video n Module Status Register */ +#define VNFC_REG 0x08/* Video n Frame Capture Register */ +#define VNSLPRC_REG 0x0C/* Video n Start Line Pre-Clip Register */ +#define VNELPRC_REG 0x10/* Video n End Line Pre-Clip Register */ +#define VNSPPRC_REG 0x14/* Video n Start Pixel Pre-Clip Register */ +#define VNEPPRC_REG 0x18/* Video n End Pixel Pre-Clip Register */ +#define VNSLPOC_REG 0x1C/* Video n Start Line Post-Clip Register */ +#define VNELPOC_REG 0x20/* Video n End Line Post-Clip Register */ +#define VNSPPOC_REG 0x24/* Video n Start Pixel Post-Clip Register */ +#define VNEPPOC_REG 0x28/* Video n End Pixel Post-Clip Register */ +#define VNIS_REG 0x2C/* Video n Image Stride Register */ +#define VNMB_REG(m) (0x30 + ((m) 2)) /* Video n Memory Base m Register */ +#define VNIE_REG 0x40/* Video n Interrupt Enable Register */ +#define VNINTS_REG 0x44/* Video n Interrupt Status Register */ +#define VNSI_REG 0x48/* Video n Scanline Interrupt Register */ +#define VNMTC_REG0x4C/* Video n Memory Transfer Control Register */ +#define VNYS_REG 0x50/* Video n Y Scale Register */ +#define VNXS_REG 0x54/* Video n X Scale
Re: [PATCH v2 1/4] V4L2: soc_camera: Renesas R-Car VIN driver
Hello. On 04/23/2013 07:08 AM, Katsuya MATSUBARA wrote: Hi Sergei, Thanks for the patch. It's not mine. :-) From: Vladimir Barinov vladimir.bari...@cogentembedded.com Add Renesas R-Car VIN (Video In) V4L2 driver. Based on the patch by Phil Edworthy phil.edwor...@renesas.com. Signed-off-by: Vladimir Barinov vladimir.bari...@cogentembedded.com [Sergei: removed deprecated IRQF_DISABLED flag.] Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com [...] Index: renesas/drivers/media/platform/soc_camera/rcar_vin.c === --- /dev/null +++ renesas/drivers/media/platform/soc_camera/rcar_vin.c @@ -0,0 +1,1784 @@ +/* + * SoC-camera host driver for Renesas R-Car VIN unit + * + * Copyright (C) 2011-2013 Renesas Solutions Corp. + * Copyright (C) 2013 Cogent Embedded, Inc., sou...@cogentembedded.com + * + * Based on V4L2 Driver for SuperH Mobile CEU interface sh_mobile_ceu_camera.c + * + * Copyright (C) 2008 Magnus Damm + * + * 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 linux/kernel.h +#include linux/module.h +#include linux/interrupt.h +#include linux/slab.h +#include linux/delay.h +#include linux/platform_device.h +#include linux/pm_runtime.h +#include linux/platform_data/camera-rcar.h + +#include media/videobuf2-dma-contig.h +#include media/soc_camera.h +#include media/soc_mediabus.h + +#define DRV_NAME rcar_vin + +/* Register offsets for R-Сar VIN */ Are you using a 2-byte character in the string 'R-Car'? Hm, you have surprised me: indeed KMail chose UTF-8 and, even worse, quoted-printable encoding. I played some with the settings, let's see what will it do... [...] + +/* Register bit fields for R-Сar VIN */ s/R-Сar/R-Car/ Sorry, I see no difference. :-) + +#define is_continuous_transfer(priv) (priv-vb_count MAX_BUFFER_NUM ? \ +true : false) + [...] + + /* Number of hardware slots */ + if (priv-vb_count MAX_BUFFER_NUM) You can use the is_continuous_transfer() macro here. You're right. [...] + + /* input interface */ + switch (icd-current_fmt-code) { + case V4L2_MBUS_FMT_YUYV8_1X16: + /* BT.601/BT.1358 16bit YCbCr422 */ + vnmc |= VNMC_INF_YUV16; + input_is_yuv = 1; + break; + case V4L2_MBUS_FMT_YUYV8_2X8: + input_is_yuv = 1; + /* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */ + vnmc |= priv-pdata-flags RCAR_VIN_BT656 ? + VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601; + default: + break; + } Does this (first) implemenation unsupport RGB inputs yet though the h/w supports them? Yes, we have only tested BT.601/656 input. If so, 'is_input_yuv' could be useless since it must be set to 1 in any case. The *default* label doesn't set it, does it? + + /* output format */ + switch (icd-current_fmt-host_fmt-fourcc) { + case V4L2_PIX_FMT_NV16: + iowrite32(((cam-width * cam-height) + 0x7f) ~0x7f, + priv-base + VNUVAOF_REG); + dmr = VNDMR_DTMD_YCSEP; + output_is_yuv = 1; + break; + case V4L2_PIX_FMT_YUYV: + dmr = VNDMR_BPSM; + output_is_yuv = 1; + break; + case V4L2_PIX_FMT_UYVY: + dmr = 0; + output_is_yuv = 1; + break; + case V4L2_PIX_FMT_RGB555X: + dmr = VNDMR_DTMD_ARGB1555; + break; + case V4L2_PIX_FMT_RGB565: + dmr = 0; + break; + case V4L2_PIX_FMT_RGB32: + dmr = VNDMR_EXRGB; + break; VIN in R8A7778(R-CarM1A) does not support the RGB32 output, but R8A7779(R-CarH1) and uPD35004(R-CarE1) ones support it. Indeed... + + if (!priv-request_to_stop) { + if (is_continuous_transfer(priv)) + slot = (ioread32(priv-base + VNMS_REG) + VNMS_FBS_MASK) VNMS_FBS_SHIFT; + else + slot = 0; + + priv-queue_buf[slot]-v4l2_buf.field = priv-field; + priv-queue_buf[slot]-v4l2_buf.sequence = priv-sequence++; + do_gettimeofday(priv-queue_buf[slot]-v4l2_buf.timestamp); + vb2_buffer_done(priv-queue_buf[slot], VB2_BUF_STATE_DONE); + priv-queue_buf[slot] = NULL; + + if (priv-state != STOPPING) + can_run = rcar_vin_fill_hw_slot(priv); + + if (hw_stopped || !can_run) + priv-state = STOPPED; The continuous capturing
Re: [PATCH v2 1/4] V4L2: soc_camera: Renesas R-Car VIN driver
Hi, From: Sergei Shtylyov sergei.shtyl...@cogentembedded.com Date: Tue, 23 Apr 2013 08:38:35 +0400 On 04/23/2013 07:08 AM, Katsuya MATSUBARA wrote: From: Vladimir Barinov vladimir.bari...@cogentembedded.com (snip) +/* Register offsets for R-Сar VIN */ Are you using a 2-byte character in the string 'R-Car'? Hm, you have surprised me: indeed KMail chose UTF-8 and, even worse, quoted-printable encoding. I played some with the settings, let's see what will it do... [...] + +/* Register bit fields for R-Сar VIN */ s/R-Сar/R-Car/ Sorry, I see no difference. :-) Replacing the UTF-8 character 'C'(0xd0 0xa1) in the two above comments in your patch with ASCII character 'C' (0x43) can solve the encoding issue. 0cf0 2b 2f 2a 20 52 65 67 69 73 74 65 72 20 6f 66 66 |+/* Register off| 0d00 73 65 74 73 20 66 6f 72 20 52 2d d0 a1 61 72 20 |sets for R-..ar | ^ 12a0 74 65 72 20 2a 2f 0a 2b 0a 2b 2f 2a 20 52 65 67 |ter */.+.+/* Reg| 12b0 69 73 74 65 72 20 62 69 74 20 66 69 65 6c 64 73 |ister bit fields| 12c0 20 66 6f 72 20 52 2d d0 a1 61 72 20 56 49 4e 20 | for R-..ar VIN | ^^ Thanks, --- Katsuya Matsubara / IGEL Co., Ltd ma...@igel.co.jp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html