Re: [PATCH 1/2] media: meye: relax dependencies if COMPILE_TEST
Hi Mauro, I love your patch! Yet something to improve: [auto build test ERROR on linuxtv-media/master] [also build test ERROR on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/media-meye-relax-dependencies-if-COMPILE_TEST/20180407-073023 base: git://linuxtv.org/media_tree.git master config: ia64-allmodconfig (attached as .config) compiler: ia64-linux-gcc (GCC) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=ia64 All errors (new ones prefixed by >>): ERROR: "ia64_delay_loop" [drivers/spi/spi-thunderx.ko] undefined! ERROR: "__sw_hweight8" [drivers/net/wireless/mediatek/mt76/mt76.ko] undefined! ERROR: "ia64_delay_loop" [drivers/net/phy/mdio-cavium.ko] undefined! >> ERROR: "sony_pic_camera_command" [drivers/media/pci/meye/meye.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [RFCv9 PATCH 04/29] media-request: core request support
On 06/04/18 22:35, Sakari Ailus wrote:
> Hi Hans,
>
> Thanks for your work on this. A few comments below...
>
> On Wed, Mar 28, 2018 at 03:50:05PM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil
>>
>> Implement the core of the media request processing.
>>
>> Drivers can bind request objects to a request. These objects
>> can then be marked completed if the driver finished using them,
>> or just be unbound if the results do not need to be kept (e.g.
>> in the case of buffers).
>>
>> Once all objects that were added are either unbound or completed,
>> the request is marked 'complete' and a POLLPRI signal is sent
>> via poll.
>>
>> Both requests and request objects are refcounted.
>>
>> While a request is queued its refcount is incremented (since it
>> is in use by a driver). Once it is completed the refcount is
>> decremented. When the user closes the request file descriptor
>> the refcount is also decremented. Once it reaches 0 all request
>> objects in the request are unbound and put() and the request
>> itself is freed.
>>
>> Signed-off-by: Hans Verkuil
>> ---
>> drivers/media/media-request.c | 269
>> +-
>> include/media/media-request.h | 148 +++
>> 2 files changed, 416 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c
>> index ead78613fdbe..8135d9d32af9 100644
>> --- a/drivers/media/media-request.c
>> +++ b/drivers/media/media-request.c
>> @@ -16,8 +16,275 @@
>> #include
>> #include
>>
>> +static const char * const request_state[] = {
>> +"idle",
>> +"queueing",
>> +"queued",
>> +"complete",
>> +"cleaning",
>> +};
>> +
>> +static const char *
>> +media_request_state_str(enum media_request_state state)
>> +{
>> +if (WARN_ON(state >= ARRAY_SIZE(request_state)))
>> +return "unknown";
>> +return request_state[state];
>> +}
>> +
>> +static void media_request_clean(struct media_request *req)
>> +{
>> +struct media_request_object *obj, *obj_safe;
>> +
>> +WARN_ON(req->state != MEDIA_REQUEST_STATE_CLEANING);
>> +
>> +list_for_each_entry_safe(obj, obj_safe, &req->objects, list) {
>> +media_request_object_unbind(obj);
>> +media_request_object_put(obj);
>> +}
>> +
>> +req->num_incomplete_objects = 0;
>> +wake_up_interruptible(&req->poll_wait);
>> +}
>> +
>> +static void media_request_release(struct kref *kref)
>> +{
>> +struct media_request *req =
>> +container_of(kref, struct media_request, kref);
>> +struct media_device *mdev = req->mdev;
>> +unsigned long flags;
>> +
>> +dev_dbg(mdev->dev, "request: release %s\n", req->debug_str);
>> +
>> +spin_lock_irqsave(&req->lock, flags);
>> +req->state = MEDIA_REQUEST_STATE_CLEANING;
>> +spin_unlock_irqrestore(&req->lock, flags);
>> +
>> +media_request_clean(req);
>> +
>> +if (mdev->ops->req_free)
>> +mdev->ops->req_free(req);
>> +else
>> +kfree(req);
>> +}
>> +
>> +void media_request_put(struct media_request *req)
>> +{
>> +kref_put(&req->kref, media_request_release);
>> +}
>> +EXPORT_SYMBOL_GPL(media_request_put);
>> +
>> +static int media_request_close(struct inode *inode, struct file *filp)
>> +{
>> +struct media_request *req = filp->private_data;
>> +
>> +media_request_put(req);
>
> One more newline?
>
>> +return 0;
>> +}
>> +
>> +static unsigned int media_request_poll(struct file *filp,
>> + struct poll_table_struct *wait)
>> +{
>> +struct media_request *req = filp->private_data;
>> +unsigned long flags;
>> +enum media_request_state state;
>> +
>> +if (!(poll_requested_events(wait) & POLLPRI))
>> +return 0;
>> +
>> +spin_lock_irqsave(&req->lock, flags);
>> +state = req->state;
>> +spin_unlock_irqrestore(&req->lock, flags);
>> +
>> +if (state == MEDIA_REQUEST_STATE_COMPLETE)
>> +return POLLPRI;
>> +if (state == MEDIA_REQUEST_STATE_IDLE)
>
> Should this be
>
> if (state != MEDIA_REQUEST_STATE_QUEUED)
Good question. I'll take another look at this.
>
> ? (Also see my other comment on the QUEUEING state below.)
>
>> +return POLLERR;
>> +
>> +poll_wait(filp, &req->poll_wait, wait);
>
> Newline?
>
>> +return 0;
>> +}
>> +
>> +static long media_request_ioctl(struct file *filp, unsigned int cmd,
>> +unsigned long __arg)
>> +{
>> +return -ENOIOCTLCMD;
>> +}
>> +
>> +static const struct file_operations request_fops = {
>> +.owner = THIS_MODULE,
>> +.poll = media_request_poll,
>> +.unlocked_ioctl = media_request_ioctl,
>> +.release = media_request_close,
>> +};
>> +
>> int media_request_alloc(struct media_device *mdev,
>> struct media_request_alloc *alloc)
>> {
>> -return -ENOMEM;
>> +struct media_request *req;
>> +struct file *filp;
>> +char comm[TASK_COMM
Re: uvcvideo stopped working in 4.16
On 06/04/18 18:54, Damjan Georgievski wrote: > Since the 4.16 kernel my uvcvideo webcam on Thinkpad X1 Carbon (5th > gen) stopped working with gst-launch-1.0, kamoso (kde webcam app), > Firefox and Chromium on sites like appear.in, talky.io, Google > Hangouts and meet.jit.si. Do you see a /dev/v4l-touchX (X is probably 0) device? If so, then this patch will probably fix the issue: https://patchwork.linuxtv.org/patch/48417/ It will appear in a stable 4.16 release soon. Regards, Hans > > It works fine in 4.15 > > The camera is: > Bus 001 Device 004: ID 04f2:b5ce Chicony Electronics Co., Ltd > > After further investigation, if I rmmod the uvcvideo module, and then > load it again, the camera starts working normally. But I get this in > dmesg: > > [ 63.399362] usbcore: deregistering interface driver uvcvideo > [ 63.495659] WARNING: CPU: 1 PID: 858 at > drivers/media/v4l2-core/v4l2-dev.c:176 v4l2_device_release+0xe3/0x100 > [videodev] > [ 63.495662] Modules linked in: rfcomm joydev mousedev rmi_smbus > rmi_core bnep snd_hda_codec_hdmi snd_soc_skl snd_soc_skl_ipc > snd_hda_codec_conexant snd_hda_ext_core snd_hda_codec_generic > snd_soc_sst_dsp snd_soc_sst_ipc snd_soc_acpi snd_soc_core snd_compress > ac97_bus snd_pcm_dmaengine arc4 iTCO_wdt iTCO_vendor_support wmi_bmof > intel_wmi_thunderbolt intel_rapl iwlmvm x86_pkg_temp_thermal > intel_powerclamp coretemp kvm_intel snd_hda_intel mac80211 > snd_hda_codec kvm uvcvideo(-) btusb btrtl btbcm btintel snd_hda_core > videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 iwlwifi > videobuf2_common bluetooth snd_hwdep irqbypass e1000e intel_cstate > snd_pcm videodev intel_uncore qcserial intel_rapl_perf cdc_mbim ptp > usb_wwan psmouse pps_core snd_timer usbserial input_leds media pcspkr > cdc_wdm cdc_ncm cfg80211 usbnet > [ 63.495758] mii i2c_i801 rtsx_pci_ms memstick tpm_crb ecdh_generic > mei_me mei shpchp intel_pch_thermal ucsi_acpi typec_ucsi thinkpad_acpi > typec wmi nvram i2c_hid ac rfkill snd battery soundcore led_class > tpm_tis rtc_cmos tpm_tis_core hid tpm evdev rng_core mac_hid > usbip_host usbip_core sg scsi_mod crypto_user ip_tables x_tables ext4 > crc16 mbcache jbd2 fscrypto algif_skcipher af_alg dm_crypt dm_mod > crct10dif_pclmul crc32_pclmul crc32c_intel rtsx_pci_sdmmc > ghash_clmulni_intel pcbc serio_raw mmc_core atkbd libps2 xhci_pci > aesni_intel aes_x86_64 xhci_hcd crypto_simd glue_helper cryptd usbcore > rtsx_pci usb_common i8042 serio vfat fat i915 intel_gtt i2c_algo_bit > drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm > agpgart > [ 63.495882] CPU: 1 PID: 858 Comm: rmmod Not tainted 4.16.0+ #1 > [ 63.495885] Hardware name: LENOVO 20HQS0LV00/20HQS0LV00, BIOS > N1MET45W (1.30 ) 02/22/2018 > [ 63.495899] RIP: 0010:v4l2_device_release+0xe3/0x100 [videodev] > [ 63.495903] RSP: 0018:9c974505bd60 EFLAGS: 00010202 > [ 63.495907] RAX: RBX: 97d00b84a1d0 RCX: > > [ 63.495910] RDX: 97d00b84a018 RSI: 0286 RDI: > c0c7f0e0 > [ 63.495912] RBP: 97d008245ae0 R08: R09: > > [ 63.495915] R10: c823d22a9a00 R11: R12: > 97d00b84a019 > [ 63.495917] R13: 97d008245b90 R14: 97d00cbaf8f8 R15: > > [ 63.495921] FS: 7f3549e74b80() GS:97d01f48() > knlGS: > [ 63.495924] CS: 0010 DS: ES: CR0: 80050033 > [ 63.495928] CR2: 01bffe78 CR3: 00047d4e2003 CR4: > 003606e0 > [ 63.495930] Call Trace: > [ 63.495947] device_release+0x30/0x90 > [ 63.495958] kobject_put+0x85/0x1a0 > [ 63.495967] uvc_unregister_video+0x49/0x90 [uvcvideo] > [ 63.495995] usb_unbind_interface+0x85/0x280 [usbcore] > [ 63.496007] device_release_driver_internal+0x15a/0x220 > [ 63.496014] driver_detach+0x37/0x70 > [ 63.496021] bus_remove_driver+0x51/0xd0 > [ 63.496051] usb_deregister+0x6a/0xd0 [usbcore] > [ 63.496065] uvc_cleanup+0xc/0x36e [uvcvideo] > [ 63.496075] SyS_delete_module+0x16a/0x2d0 > [ 63.496088] do_syscall_64+0x74/0x190 > [ 63.496100] entry_SYSCALL_64_after_hwframe+0x3d/0xa2 > [ 63.496105] RIP: 0033:0x7f35495975d7 > [ 63.496108] RSP: 002b:7ffcbf653b88 EFLAGS: 0206 ORIG_RAX: > 00b0 > [ 63.496112] RAX: ffda RBX: RCX: > 7f35495975d7 > [ 63.496115] RDX: 000a RSI: 0800 RDI: > 01bf57b8 > [ 63.496117] RBP: 01bf5750 R08: 7ffcbf652b01 R09: > > [ 63.496119] R10: 08b2 R11: 0206 R12: > 7ffcbf655d95 > [ 63.496123] R13: R14: 01bf5750 R15: > 01bf5260 > [ 63.496127] Code: da ce 48 85 ed 74 1f 5b 48 89 ef 5d 41 5c e9 f5 > 6b 00 00 5b 5d 41 5c e9 6c 9c da ce 4c 89 e7 e8 04 80 f4 ff eb c6 5b > 5d 41 5c c3 <0f> 0b 5b 5d 41 5c 48 c7 c7 e0 f0 c7 c0 e9 cb e8 ac ce 90 > 66 2e > [ 63.496214] ---[ end trace
Re: [PATCH v7 1/2] uvcvideo: send a control event when a Control Change interrupt arrives
Hi Laurent,
Thanks for the review. As soon as the second patch has also been reviewed,
I'll work on both of them.
Thanks
Guennadi
On Fri, 23 Mar 2018, Laurent Pinchart wrote:
> Hi Guennadi,
>
> Thank you for the patch.
>
> On Friday, 23 March 2018 11:24:00 EET Laurent Pinchart wrote:
> > From: Guennadi Liakhovetski
> >
> > UVC defines a method of handling asynchronous controls, which sends a
> > USB packet over the interrupt pipe. This patch implements support for
> > such packets by sending a control event to the user. Since this can
> > involve USB traffic and, therefore, scheduling, this has to be done
> > in a work queue.
> >
> > Signed-off-by: Guennadi Liakhovetski
> > ---
> > drivers/media/usb/uvc/uvc_ctrl.c | 166 ++
> > drivers/media/usb/uvc/uvc_status.c | 111 ++---
> > drivers/media/usb/uvc/uvc_v4l2.c | 4 +-
> > drivers/media/usb/uvc/uvcvideo.h | 15 +++-
> > include/uapi/linux/uvcvideo.h | 2 +
> > 5 files changed, 269 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c
> > b/drivers/media/usb/uvc/uvc_ctrl.c index 4042cbdb721b..f4773c56438c 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -20,6 +20,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > @@ -1222,30 +1223,134 @@ static void uvc_ctrl_send_event(struct uvc_fh
> > *handle, {
> > struct v4l2_subscribed_event *sev;
> > struct v4l2_event ev;
> > + bool autoupdate;
> >
> > if (list_empty(&mapping->ev_subs))
> > return;
> >
> > + if (!handle) {
>
> In which circumstances does this happen ? Is it when the device reports a
> control change event without an prior control set ? Have you seen that
> happening in practice ?
>
> > + autoupdate = true;
> > + sev = list_first_entry(&mapping->ev_subs,
> > + struct v4l2_subscribed_event, node);
> > + handle = container_of(sev->fh, struct uvc_fh, vfh);
>
> There's a check below that guards against sev->fh being NULL. Could sev->fh
> be
> NULL here ?
>
> > + } else {
> > + autoupdate = false;
> > + }
> > +
> > uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping, value, changes);
> >
> > list_for_each_entry(sev, &mapping->ev_subs, node) {
> > if (sev->fh && (sev->fh != &handle->vfh ||
> > (sev->flags & V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK) ||
> > - (changes & V4L2_EVENT_CTRL_CH_FLAGS)))
> > + (changes & V4L2_EVENT_CTRL_CH_FLAGS) || autoupdate))
> > v4l2_event_queue_fh(sev->fh, &ev);
> > }
> > }
> >
> > -static void uvc_ctrl_send_slave_event(struct uvc_fh *handle,
> > - struct uvc_control *master, u32 slave_id,
> > - const struct v4l2_ext_control *xctrls, unsigned int xctrls_count)
> > +static void __uvc_ctrl_send_slave_event(struct uvc_fh *handle,
> > + struct uvc_control *master, u32 slave_id)
> > {
> > struct uvc_control_mapping *mapping = NULL;
> > struct uvc_control *ctrl = NULL;
> > u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
> > - unsigned int i;
> > s32 val = 0;
> >
> > + __uvc_find_control(master->entity, slave_id, &mapping, &ctrl, 0);
> > + if (ctrl == NULL)
> > + return;
> > +
> > + if (__uvc_ctrl_get(handle->chain, ctrl, mapping, &val) == 0)
> > + changes |= V4L2_EVENT_CTRL_CH_VALUE;
> > +
> > + uvc_ctrl_send_event(handle, ctrl, mapping, val, changes);
> > +}
> > +
> > +static void uvc_ctrl_status_event_work(struct work_struct *work)
> > +{
> > + struct uvc_device *dev = container_of(work, struct uvc_device,
> > + async_ctrl.work);
> > + struct uvc_video_chain *chain;
> > + struct uvc_ctrl_work *w = &dev->async_ctrl;
> > + struct uvc_control_mapping *mapping;
> > + struct uvc_control *ctrl;
> > + struct uvc_fh *handle;
> > + unsigned int i;
> > + u8 *data;
> > +
> > + spin_lock_irq(&w->lock);
> > + data = w->data;
> > + w->data = NULL;
> > + chain = w->chain;
> > + ctrl = w->ctrl;
> > + handle = ctrl->handle;
> > + ctrl->handle = NULL;
> > + spin_unlock_irq(&w->lock);
> > +
> > + if (mutex_lock_interruptible(&chain->ctrl_mutex))
> > + goto free;
>
> This will result in the event being lost, which isn't very nice (see below
> for
> additional comments on this topic). Can't we use mutex_lock() ?
>
> > +
> > + list_for_each_entry(mapping, &ctrl->info.mappings, list) {
> > + s32 value = mapping->get(mapping, UVC_GET_CUR, data);
> > +
> > + for (i = 0; i < ARRAY_SIZE(mapping->slave_ids); ++i) {
> > + if (!mapping->slave_ids[i])
> > + break;
> > +
> > + __uvc_ctrl_send_slave_event(handle, ctrl,
> > +
[PATCH for v4.17] cec: fix smatch error
drivers/media/cec/cec-pin-error-inj.c:231
cec_pin_error_inj_parse_line() error: uninitialized symbol 'pos'.
The tx-add-bytes command didn't check for the presence of an argument, and
also didn't check that it was > 0.
This should fix this error.
Signed-off-by: Hans Verkuil
---
diff --git a/drivers/media/cec/cec-pin-error-inj.c
b/drivers/media/cec/cec-pin-error-inj.c
index aaa899a175ce..7132a2758bd3 100644
--- a/drivers/media/cec/cec-pin-error-inj.c
+++ b/drivers/media/cec/cec-pin-error-inj.c
@@ -203,16 +203,18 @@ bool cec_pin_error_inj_parse_line(struct cec_adapter
*adap, char *line)
mode_mask = CEC_ERROR_INJ_MODE_MASK << mode_offset;
arg_idx = cec_error_inj_cmds[i].arg_idx;
- if (mode_offset == CEC_ERROR_INJ_RX_ARB_LOST_OFFSET ||
- mode_offset == CEC_ERROR_INJ_TX_ADD_BYTES_OFFSET)
- is_bit_pos = false;
-
if (mode_offset == CEC_ERROR_INJ_RX_ARB_LOST_OFFSET) {
if (has_op)
return false;
if (!has_pos)
pos = 0x0f;
+ is_bit_pos = false;
+ } else if (mode_offset == CEC_ERROR_INJ_TX_ADD_BYTES_OFFSET) {
+ if (!has_pos || !pos)
+ return false;
+ is_bit_pos = false;
}
+
if (arg_idx >= 0 && is_bit_pos) {
if (!has_pos || pos >= 160)
return false;
v4l2 configuration doubt
Hi All, Need to configure below parameter: Color Format: RGB888 Color Depth: 8 Pixels Per Clock: 2 Is there any macro available for this configuration like MEDIA_BUS_FMT_RGB888_1X24? How to configure pixel per clock in device tree and driver file? Thanks & Regards - Asad
Re: uvcvideo stopped working in 4.16
On 7 April 2018 at 11:11, Hans Verkuil wrote: > On 06/04/18 18:54, Damjan Georgievski wrote: >> Since the 4.16 kernel my uvcvideo webcam on Thinkpad X1 Carbon (5th >> gen) stopped working with gst-launch-1.0, kamoso (kde webcam app), >> Firefox and Chromium on sites like appear.in, talky.io, Google >> Hangouts and meet.jit.si. > > Do you see a /dev/v4l-touchX (X is probably 0) device? If so, then this > patch will probably fix the issue: > > https://patchwork.linuxtv.org/patch/48417/ > > It will appear in a stable 4.16 release soon. Thanks Hans, that patch indeed fixes my issue -- damjan
Re: [PATCH v7 1/2] uvcvideo: send a control event when a Control Change interrupt arrives
Hi Guennadi,
On Saturday, 7 April 2018 12:30:44 EEST Guennadi Liakhovetski wrote:
> Hi Laurent,
>
> Thanks for the review. As soon as the second patch has also been reviewed,
> I'll work on both of them.
I would have sworn I had sent the review for 2/2 as well. I'll get to it right
away.
> On Fri, 23 Mar 2018, Laurent Pinchart wrote:
> > Hi Guennadi,
> >
> > Thank you for the patch.
> >
> > On Friday, 23 March 2018 11:24:00 EET Laurent Pinchart wrote:
> > > From: Guennadi Liakhovetski
> > >
> > > UVC defines a method of handling asynchronous controls, which sends a
> > > USB packet over the interrupt pipe. This patch implements support for
> > > such packets by sending a control event to the user. Since this can
> > > involve USB traffic and, therefore, scheduling, this has to be done
> > > in a work queue.
> > >
> > > Signed-off-by: Guennadi Liakhovetski
> > > ---
> > >
> > > drivers/media/usb/uvc/uvc_ctrl.c | 166
> > > ++
> > > drivers/media/usb/uvc/uvc_status.c | 111 ++---
> > > drivers/media/usb/uvc/uvc_v4l2.c | 4 +-
> > > drivers/media/usb/uvc/uvcvideo.h | 15 +++-
> > > include/uapi/linux/uvcvideo.h | 2 +
> > > 5 files changed, 269 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c
> > > b/drivers/media/usb/uvc/uvc_ctrl.c index 4042cbdb721b..f4773c56438c
> > > 100644
> > > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > > @@ -20,6 +20,7 @@
> > >
> > > #include
> > > #include
> > > #include
> > >
> > > +#include
> > >
> > > #include
> > > #include
> > >
> > > @@ -1222,30 +1223,134 @@ static void uvc_ctrl_send_event(struct uvc_fh
> > > *handle, {
> > >
> > > struct v4l2_subscribed_event *sev;
> > > struct v4l2_event ev;
> > >
> > > + bool autoupdate;
> > >
> > > if (list_empty(&mapping->ev_subs))
> > >
> > > return;
> > >
> > > + if (!handle) {
> >
> > In which circumstances does this happen ? Is it when the device reports a
> > control change event without an prior control set ? Have you seen that
> > happening in practice ?
> >
> > > + autoupdate = true;
> > > + sev = list_first_entry(&mapping->ev_subs,
> > > +struct v4l2_subscribed_event, node);
> > > + handle = container_of(sev->fh, struct uvc_fh, vfh);
> >
> > There's a check below that guards against sev->fh being NULL. Could
> > sev->fh be NULL here ?
> >
> > > + } else {
> > > + autoupdate = false;
> > > + }
> > > +
> > >
> > > uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping, value,
> > > changes);
> > >
> > > list_for_each_entry(sev, &mapping->ev_subs, node) {
> > >
> > > if (sev->fh && (sev->fh != &handle->vfh ||
> > >
> > > (sev->flags & V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK) ||
> > >
> > > - (changes & V4L2_EVENT_CTRL_CH_FLAGS)))
> > > + (changes & V4L2_EVENT_CTRL_CH_FLAGS) || autoupdate))
> > >
> > > v4l2_event_queue_fh(sev->fh, &ev);
> > >
> > > }
> > >
> > > }
> > >
> > > -static void uvc_ctrl_send_slave_event(struct uvc_fh *handle,
> > > - struct uvc_control *master, u32 slave_id,
> > > - const struct v4l2_ext_control *xctrls, unsigned int xctrls_count)
> > > +static void __uvc_ctrl_send_slave_event(struct uvc_fh *handle,
> > > + struct uvc_control *master, u32 slave_id)
> > >
> > > {
> > >
> > > struct uvc_control_mapping *mapping = NULL;
> > > struct uvc_control *ctrl = NULL;
> > > u32 changes = V4L2_EVENT_CTRL_CH_FLAGS;
> > >
> > > - unsigned int i;
> > >
> > > s32 val = 0;
> > >
> > > + __uvc_find_control(master->entity, slave_id, &mapping, &ctrl, 0);
> > > + if (ctrl == NULL)
> > > + return;
> > > +
> > > + if (__uvc_ctrl_get(handle->chain, ctrl, mapping, &val) == 0)
> > > + changes |= V4L2_EVENT_CTRL_CH_VALUE;
> > > +
> > > + uvc_ctrl_send_event(handle, ctrl, mapping, val, changes);
> > > +}
> > > +
> > > +static void uvc_ctrl_status_event_work(struct work_struct *work)
> > > +{
> > > + struct uvc_device *dev = container_of(work, struct uvc_device,
> > > + async_ctrl.work);
> > > + struct uvc_video_chain *chain;
> > > + struct uvc_ctrl_work *w = &dev->async_ctrl;
> > > + struct uvc_control_mapping *mapping;
> > > + struct uvc_control *ctrl;
> > > + struct uvc_fh *handle;
> > > + unsigned int i;
> > > + u8 *data;
> > > +
> > > + spin_lock_irq(&w->lock);
> > > + data = w->data;
> > > + w->data = NULL;
> > > + chain = w->chain;
> > > + ctrl = w->ctrl;
> > > + handle = ctrl->handle;
> > > + ctrl->handle = NULL;
> > > + spin_unlock_irq(&w->lock);
> > > +
> > > + if (mutex_lock_interruptible(&chain->ctrl_mutex))
> > > + goto free;
> >
> > This will result in the event being lost, which isn't very nice (see below
> > for additional comments on this topic). Can't we use mu
Re: [PATCH v7 2/2] uvcvideo: handle control pipe protocol STALLs
Hi Guennadi,
Thank you for the patch.
On Friday, 23 March 2018 11:24:01 EEST Laurent Pinchart wrote:
> From: Guennadi Liakhovetski
>
> When a command ends up in a STALL on the control pipe, use the Request
> Error Code control to provide a more precise error information to the
> user.
This is the kind of change that I believe is completely right, but that
nonetheless worries me. All the years I've spent working with UVC webcams
taught me that lots of cameras have buggy firmware, and that any change in how
the host driver issues requests can have dire consequences for users. This is
especially true when the host driver issues a request that was never issued
before.
The UVC specification also doesn't clearly tell whether the request error code
control is mandatory for a device to implement. My interpretation of the
specification is that it is, but it would have been nice for the specification
to be explicit on this topic. Have you encountered devices that don't
implement this control ?
This being said, I don't claim that would be a reason not to use the request
error code control as proposed by this patch, but I would feel less worried if
I knew whether the Windows driver used that control as well. If it does
there's a high chance that cameras will implement it correctly, while if it
doesn't we will most certainly hit bugs with several cameras. I was thus
wondering if in the course of your UVC developments you would have happened to
find out whether this control is used by Windows.
I would also like to know a bit more about the purpose of this patch. Logging
the error code is certainly useful for diagnosis purpose. Have you also found
it useful to report different errors back to userspace ? If so, could you
explain your use cases ?
> Signed-off-by: Guennadi Liakhovetski
> ---
> drivers/media/usb/uvc/uvc_video.c | 59
> 1 file changed, 53 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c
> b/drivers/media/usb/uvc/uvc_video.c index aa0082fe5833..eb9e04a59427 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -34,15 +34,59 @@ static int __uvc_query_ctrl(struct uvc_device *dev, u8
> query, u8 unit, u8 intfnum, u8 cs, void *data, u16 size,
> int timeout)
> {
> - u8 type = USB_TYPE_CLASS | USB_RECIP_INTERFACE;
> + u8 type = USB_TYPE_CLASS | USB_RECIP_INTERFACE, tmp, error;
Would you mind declaring one variable per line to match the style of the rest
of the driver ?
> unsigned int pipe;
> + int ret;
>
> pipe = (query & 0x80) ? usb_rcvctrlpipe(dev->udev, 0)
> : usb_sndctrlpipe(dev->udev, 0);
>
> type |= (query & 0x80) ? USB_DIR_IN : USB_DIR_OUT;
>
> - return usb_control_msg(dev->udev, pipe, query, type, cs << 8,
> + ret = usb_control_msg(dev->udev, pipe, query, type, cs << 8,
> unit << 8 | intfnum, data, size, timeout);
> +
Nitpicking, you can remove the blank line here.
> + if (ret != -EPIPE)
> + return ret;
> +
> + tmp = *(u8 *)data;
> +
> + pipe = usb_rcvctrlpipe(dev->udev, 0);
> + type = USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN;
> + ret = usb_control_msg(dev->udev, pipe, UVC_GET_CUR, type,
> + UVC_VC_REQUEST_ERROR_CODE_CONTROL << 8,
> + unit << 8 | intfnum, data, 1, timeout);
Unless I'm mistaken the wIndex value should be "Zero and Interface" according
to both the UVC 1.1 and UVC 1.5 specifications. This should thus be
ret = usb_control_msg(dev->udev, pipe, UVC_GET_CUR, type,
UVC_VC_REQUEST_ERROR_CODE_CONTROL << 8,
intfnum, data, 1, timeout);
UVC_VC_REQUEST_ERROR_CODE_CONTROL is only applicable to requests to the
control interface, not to the streaming interfaces, while __uvc_query_ctrl()
is used for both. I think the code should thus be moved to uvc_query_ctrl().
This would allow calling __uvc_query_ctrl() for both the original request and
the error code control instead of calling usb_control_msg() manually.
> + error = *(u8 *)data;
> + *(u8 *)data = tmp;
> +
> + if (ret < 0)
> + return ret;
> +
> + if (!ret)
I wonder if this check should be if (ret != 1) as it would be an error if the
device returned more than 1 byte. I suppose this can't happen when calling
usb_control_msg() with the size set to 1, but I'd find the check easier to
understand.
> + return -EINVAL;
Should we return -EPIPE instead ? if ret != 1 we have failed reading the error
code, so reporting the STALL to the caller is the best we could do in my
opinion. -EINVAL would mean that this function was called with invalid
parameters, and we don't know that for sure.
> + uvc_trace(UVC_TRACE_CONTROL, "Control error %u\n", error);
> +
> + switch (error) {
> + case 0:
>
[PATCH] v4l: omap3isp: Enable driver compilation on ARM with COMPILE_TEST
The omap3isp driver can't be compiled on non-ARM platforms but has no compile-time dependency on OMAP. It however requires common clock framework support, which isn't provided by all ARM platforms. Drop the OMAP dependency when COMPILE_TEST is set and add ARM and COMMON_CLK dependencies. Signed-off-by: Laurent Pinchart --- drivers/media/platform/Kconfig | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) Hi Mauro, While we continue the discussions on whether the ARM IOMMU functions should be stubbed in the omap3isp driver itself or not, I propose already merging this patch that will extend build coverage for the omap3isp driver. Extending compilation to non-ARM platforms can then be added on top, depending on the result of the discussion. You might have noticed the 0-day build bot report reporting that the driver depends on the common clock framework (build failure on openrisc). The issue affects ARM as well as not all ARM platforms use the common clock framework. I've thus also added a dependency on COMMON_CLK. Note that this dependency can prevent compilation on x86 platforms. If you want to fix that, the definition of struct clk_hw in include/linux/clk-provider.h will need to be exposed even when CONFIG_COMMON_CLK isn't selected. I'll let you propose a fix for that issue to the clock maintainers if you think it should be addressed. diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index c7a1cf8a1b01..58aa233d3cf9 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -62,7 +62,10 @@ config VIDEO_MUX config VIDEO_OMAP3 tristate "OMAP 3 Camera support" - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API + depends on ARCH_OMAP3 || COMPILE_TEST + depends on ARM + depends on COMMON_CLK depends on HAS_DMA && OF depends on OMAP_IOMMU select ARM_DMA_USE_IOMMU -- Regards, Laurent Pinchart
Re: [PATCH 2/2] media: omapfb: relax compilation if COMPILE_TEST
Hi Mauro, Thank you for the patch. On Friday, 6 April 2018 18:33:20 EEST Mauro Carvalho Chehab wrote: > The dependency of DRM_OMAP = n can be relaxed for just > compilation test. > > This allows building the omap3isp driver with allyesconfig > on ARM. omapfb has nothing to do with omap3isp. I assume you meant omap_vout. There's a reason why both DRM_OMAP and FB_OMAP2 can't be compiled at the same time, they export identical symbols. I believe you will end up with link failures if you do so. > Signed-off-by: Mauro Carvalho Chehab > --- > drivers/video/fbdev/omap2/omapfb/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/video/fbdev/omap2/omapfb/Kconfig > b/drivers/video/fbdev/omap2/omapfb/Kconfig index e6226aeed17e..e42794a5e26c > 100644 > --- a/drivers/video/fbdev/omap2/omapfb/Kconfig > +++ b/drivers/video/fbdev/omap2/omapfb/Kconfig > @@ -4,7 +4,7 @@ config OMAP2_VRFB > menuconfig FB_OMAP2 > tristate "OMAP2+ frame buffer support" > depends on FB > -depends on DRM_OMAP = n > +depends on DRM_OMAP = n || COMPILE_TEST > > select FB_OMAP2_DSS > select OMAP2_VRFB if ARCH_OMAP2 || ARCH_OMAP3 -- Regards, Laurent Pinchart
Re: [PATCH 02/16] media: omap3isp: allow it to build with COMPILE_TEST
Hi Mauro,
On Thursday, 5 April 2018 22:44:44 EEST Mauro Carvalho Chehab wrote:
> Em Thu, 05 Apr 2018 21:30:27 +0300 Laurent Pinchart escreveu:
> > On Thursday, 5 April 2018 20:54:02 EEST Mauro Carvalho Chehab wrote:
> > > There aren't much things required for it to build with COMPILE_TEST.
> > > It just needs to provide stub for an arm-dependent include.
> > >
> > > Let's replicate the same solution used by ipmmu-vmsa, in order
> > > to allow building omap3 with COMPILE_TEST.
> > >
> > > The actual logic here came from this driver:
> > >drivers/iommu/ipmmu-vmsa.c
> > >
> > > Signed-off-by: Mauro Carvalho Chehab
> > > ---
> > >
> > > drivers/media/platform/Kconfig| 8
> > > drivers/media/platform/omap3isp/isp.c | 7 +++
> > > 2 files changed, 11 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/Kconfig
> > > b/drivers/media/platform/Kconfig index c7a1cf8a1b01..03c9dfeb7781
> > > 100644
> > > --- a/drivers/media/platform/Kconfig
> > > +++ b/drivers/media/platform/Kconfig
> > > @@ -62,12 +62,12 @@ config VIDEO_MUX
> > >
> > > config VIDEO_OMAP3
> > > tristate "OMAP 3 Camera support"
> > > - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3
> > > + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
> > > depends on HAS_DMA && OF
> > > - depends on OMAP_IOMMU
> > > - select ARM_DMA_USE_IOMMU
> > > + depends on ((ARCH_OMAP3 && OMAP_IOMMU) || COMPILE_TEST)
> > > + select ARM_DMA_USE_IOMMU if OMAP_IOMMU
> > > select VIDEOBUF2_DMA_CONTIG
> > > - select MFD_SYSCON
> > > + select MFD_SYSCON if ARCH_OMAP3
> > > select V4L2_FWNODE
> > > ---help---
> > > Driver for an OMAP 3 camera controller.
> > > diff --git a/drivers/media/platform/omap3isp/isp.c
> > > b/drivers/media/platform/omap3isp/isp.c index 8eb000e3d8fd..2a11a709aa4f
> > > 100644
> > > --- a/drivers/media/platform/omap3isp/isp.c
> > > +++ b/drivers/media/platform/omap3isp/isp.c
> > > @@ -61,7 +61,14 @@
> > > #include
> > > #include
> > >
> > > +#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
> > > #include
> > > +#else
> > > +#define arm_iommu_create_mapping(...)NULL
> > > +#define arm_iommu_attach_device(...) -ENODEV
> > > +#define arm_iommu_release_mapping(...) do {} while (0)
> > > +#define arm_iommu_detach_device(...) do {} while (0)
> > > +#endif
> >
> > I don't think it's the job of a driver to define those stubs, sorry.
> > Otherwise where do you stop ? If you have half of the code that is
> > architecture- dependent, would you stub it ? And what if the stubs you
> > define here generate warnings in static analyzers ?
>
> I agree that we should avoid doing that as a general case, but see
> below.
>
> > If you want to make drivers compile for all architectures, the APIs they
> > use must be defined in linux/, not in asm/. They can be stubbed there
> > when not implemented in a particular architecture, but not in the driver.
>
> In this specific case, the same approach taken here is already needed
> by the Renesas VMSA-compatible IPMMU driver, with, btw, is inside
> drivers/iommu:
>
> drivers/iommu/ipmmu-vmsa.c
The reason there is different, the driver is shared by ARM32 and ARM64
platforms. Furthermore, there's an effort (or at least there was) to move away
from those APIs in the driver, but I think it has stalled.
> Also, this API is used only by 3 drivers [1]:
>
> drivers/iommu/ipmmu-vmsa.c
> drivers/iommu/mtk_iommu_v1.c
> drivers/media/platform/omap3isp/isp.c
>
> [1] as blamed by
> git grep -l arm_iommu_create_mapping
The exynos driver also uses it.
> That hardly seems to be an arch-specific iommu solution, but, instead, some
> hack used by only three drivers or some legacy iommu binding.
It's more complex than that. There are multiple IOMMU-related APIs on ARM, so
more recent than others, with different feature sets. While I agree that
drivers should move away from arm_iommu_create_mapping(), doing so requires
coordination between the IOMMU driver and the bus master driver (for instance
the omap3isp driver). It's not a trivial matter, but I'd love if someone
submitted patches :-)
> The omap3isp is, btw, the only driver outside drivers/iommu that needs it.
>
> So, it sounds that other driver uses some other approach, but hardly
> it would be worth to change this driver to use something else.
>
> So, better to stick with the same solution the Renesas driver used.
I'm not responsible for that solution and I didn't think it was a good one at
the time it was introduced, but in any case it is not meant at all to support
COMPILE_TEST. I still don't think the omap3isp driver should declare stubs for
these functions for the purpose of supporting compile-testing on x86.
--
Regards,
Laurent Pinchart
Re: [PATCH 2/2] media: omapfb: relax compilation if COMPILE_TEST
Em Sat, 07 Apr 2018 14:46:56 +0300 Laurent Pinchart escreveu: > Hi Mauro, > > Thank you for the patch. > > On Friday, 6 April 2018 18:33:20 EEST Mauro Carvalho Chehab wrote: > > The dependency of DRM_OMAP = n can be relaxed for just > > compilation test. > > > > This allows building the omap3isp driver with allyesconfig > > on ARM. > > omapfb has nothing to do with omap3isp. I assume you meant omap_vout. > > There's a reason why both DRM_OMAP and FB_OMAP2 can't be compiled at the same > time, they export identical symbols. I believe you will end up with link > failures if you do so. Ah, OK. I'll just drop this patch. Thanks, Mauro
[PATCH] gpu: ipu-v3: Fix BT1120 interlaced CCIR codes
The BT1120 interlaced CCIR codes are the same as BT656 ones and different than BT656 progressive CCIR codes, fix this. Signed-off-by: Marek Vasut Cc: Steve Longerbeam Cc: Philipp Zabel --- drivers/gpu/ipu-v3/ipu-csi.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c index caa05b0702e1..301a729581ce 100644 --- a/drivers/gpu/ipu-v3/ipu-csi.c +++ b/drivers/gpu/ipu-v3/ipu-csi.c @@ -435,12 +435,16 @@ int ipu_csi_init_interface(struct ipu_csi *csi, break; case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR: case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR: - case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR: - case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR: ipu_csi_write(csi, 0x40030 | CSI_CCIR_ERR_DET_EN, CSI_CCIR_CODE_1); ipu_csi_write(csi, 0xFF, CSI_CCIR_CODE_3); break; + case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_DDR: + case IPU_CSI_CLK_MODE_CCIR1120_INTERLACED_SDR: + ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN, CSI_CCIR_CODE_1); + ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2); + ipu_csi_write(csi, 0xFF, CSI_CCIR_CODE_3); + break; case IPU_CSI_CLK_MODE_GATED_CLK: case IPU_CSI_CLK_MODE_NONGATED_CLK: ipu_csi_write(csi, 0, CSI_CCIR_CODE_1); -- 2.16.2
[PATCH] media: imx: Skip every second frame in VDIC DIRECT mode
In VDIC direct mode, the VDIC applies combing filter during and doubles the framerate, that is, after the first two half-frames are received and the first frame is emitted by the VDIC, every subsequent half-frame is patched into the result and a full frame is produced. The half-frame order in the full frames is as follows 12 32 34 54 etc. Drop every second frame to trim the framerate back to the original one of the signal and skip the odd patched frames. Signed-off-by: Marek Vasut Cc: Steve Longerbeam Cc: Philipp Zabel --- drivers/staging/media/imx/imx-media-vdic.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c index 482250d47e7c..b538bbebedc5 100644 --- a/drivers/staging/media/imx/imx-media-vdic.c +++ b/drivers/staging/media/imx/imx-media-vdic.c @@ -289,6 +289,7 @@ static int vdic_setup_direct(struct vdic_priv *priv) /* set VDIC to receive from CSI for direct path */ ipu_fsu_link(priv->ipu, IPUV3_CHANNEL_CSI_DIRECT, IPUV3_CHANNEL_CSI_VDI_PREV); + ipu_set_vdi_skip(priv->ipu, 0x2); return 0; } @@ -313,6 +314,8 @@ static int vdic_setup_indirect(struct vdic_priv *priv) const struct imx_media_pixfmt *incc; int in_size, ret; + ipu_set_vdi_skip(priv->ipu, 0x0); + infmt = &priv->format_mbus[VDIC_SINK_PAD_IDMAC]; incc = priv->cc[VDIC_SINK_PAD_IDMAC]; -- 2.16.2
[PATCH] media: staging/imx: Handle CSI->VDIC->PRPVF pipeline
In case the PRPVF is not connected directly to CSI, the PRPVF subdev
driver won't find the CSI subdev and will not configure the CSI input
mux. This is not noticable on the IPU1-CSI0 interface with parallel
camera, since the mux is set "correctly" by default and the parallel
camera will work just fine. This is however noticable on IPU2-CSI1,
where the mux is not set to the correct position by default and the
pipeline will fail.
Add similar code to what is in PRPVF to VDIC driver, so that the VDIC
can locate the CSI subdev and configure the mux correctly if the CSI
is connected to the VDIC. Make the PRPVF driver configure the CSI mux
only in case it's connected directly to CSI and not in case it is
connected to VDIC.
Signed-off-by: Marek Vasut
Cc: Philipp Zabel
Cc: Steve Longerbeam
---
drivers/staging/media/imx/imx-ic-prp.c | 6 ++
drivers/staging/media/imx/imx-media-vdic.c | 24
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prp.c
b/drivers/staging/media/imx/imx-ic-prp.c
index 98923fc844ce..84fa66dae21a 100644
--- a/drivers/staging/media/imx/imx-ic-prp.c
+++ b/drivers/staging/media/imx/imx-ic-prp.c
@@ -72,14 +72,12 @@ static inline struct prp_priv *sd_to_priv(struct
v4l2_subdev *sd)
static int prp_start(struct prp_priv *priv)
{
struct imx_ic_priv *ic_priv = priv->ic_priv;
- bool src_is_vdic;
priv->ipu = priv->md->ipu[ic_priv->ipu_id];
/* set IC to receive from CSI or VDI depending on source */
- src_is_vdic = !!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC);
-
- ipu_set_ic_src_mux(priv->ipu, priv->csi_id, src_is_vdic);
+ if (!(priv->src_sd->grp_id & IMX_MEDIA_GRP_ID_VDIC))
+ ipu_set_ic_src_mux(priv->ipu, priv->csi_id, false);
return 0;
}
diff --git a/drivers/staging/media/imx/imx-media-vdic.c
b/drivers/staging/media/imx/imx-media-vdic.c
index b538bbebedc5..e660911e7024 100644
--- a/drivers/staging/media/imx/imx-media-vdic.c
+++ b/drivers/staging/media/imx/imx-media-vdic.c
@@ -117,6 +117,9 @@ struct vdic_priv {
bool csi_direct; /* using direct CSI->VDIC->IC pipeline */
+ /* the CSI id at link validate */
+ int csi_id;
+
/* motion select control */
struct v4l2_ctrl_handler ctrl_hdlr;
enum ipu_motion_sel motion;
@@ -388,6 +391,9 @@ static int vdic_start(struct vdic_priv *priv)
if (ret)
return ret;
+ /* set IC to receive from CSI or VDI depending on source */
+ ipu_set_ic_src_mux(priv->ipu, priv->csi_id, true);
+
/*
* init the VDIC.
*
@@ -778,6 +784,7 @@ static int vdic_link_validate(struct v4l2_subdev *sd,
struct v4l2_subdev_format *sink_fmt)
{
struct vdic_priv *priv = v4l2_get_subdevdata(sd);
+ struct imx_media_subdev *csi;
int ret;
ret = v4l2_subdev_link_validate_default(sd, link,
@@ -785,6 +792,23 @@ static int vdic_link_validate(struct v4l2_subdev *sd,
if (ret)
return ret;
+ csi = imx_media_find_upstream_subdev(priv->md, priv->src,
+IMX_MEDIA_GRP_ID_CSI);
+ if (!IS_ERR(csi)) {
+ switch (csi->sd->grp_id) {
+ case IMX_MEDIA_GRP_ID_CSI0:
+ priv->csi_id = 0;
+ break;
+ case IMX_MEDIA_GRP_ID_CSI1:
+ priv->csi_id = 1;
+ break;
+ default:
+ ret = -EINVAL;
+ }
+ } else {
+ priv->csi_id = 0;
+ }
+
mutex_lock(&priv->lock);
if (priv->csi_direct && priv->motion != HIGH_MOTION) {
--
2.16.2
Re: [PATCH 02/16] media: omap3isp: allow it to build with COMPILE_TEST
Em Sat, 07 Apr 2018 14:56:59 +0300
Laurent Pinchart escreveu:
> Hi Mauro,
>
> On Thursday, 5 April 2018 22:44:44 EEST Mauro Carvalho Chehab wrote:
> > Em Thu, 05 Apr 2018 21:30:27 +0300 Laurent Pinchart escreveu:
> > > On Thursday, 5 April 2018 20:54:02 EEST Mauro Carvalho Chehab wrote:
> > > > There aren't much things required for it to build with COMPILE_TEST.
> > > > It just needs to provide stub for an arm-dependent include.
> > > >
> > > > Let's replicate the same solution used by ipmmu-vmsa, in order
> > > > to allow building omap3 with COMPILE_TEST.
> > > >
> > > > The actual logic here came from this driver:
> > > >drivers/iommu/ipmmu-vmsa.c
> > > >
> > > > Signed-off-by: Mauro Carvalho Chehab
> > > > ---
> > > >
> > > > drivers/media/platform/Kconfig| 8
> > > > drivers/media/platform/omap3isp/isp.c | 7 +++
> > > > 2 files changed, 11 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/media/platform/Kconfig
> > > > b/drivers/media/platform/Kconfig index c7a1cf8a1b01..03c9dfeb7781
> > > > 100644
> > > > --- a/drivers/media/platform/Kconfig
> > > > +++ b/drivers/media/platform/Kconfig
> > > > @@ -62,12 +62,12 @@ config VIDEO_MUX
> > > >
> > > > config VIDEO_OMAP3
> > > > tristate "OMAP 3 Camera support"
> > > > - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API &&
> > > > ARCH_OMAP3
> > > > + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
> > > > depends on HAS_DMA && OF
> > > > - depends on OMAP_IOMMU
> > > > - select ARM_DMA_USE_IOMMU
> > > > + depends on ((ARCH_OMAP3 && OMAP_IOMMU) || COMPILE_TEST)
> > > > + select ARM_DMA_USE_IOMMU if OMAP_IOMMU
> > > > select VIDEOBUF2_DMA_CONTIG
> > > > - select MFD_SYSCON
> > > > + select MFD_SYSCON if ARCH_OMAP3
> > > > select V4L2_FWNODE
> > > > ---help---
> > > > Driver for an OMAP 3 camera controller.
> > > > diff --git a/drivers/media/platform/omap3isp/isp.c
> > > > b/drivers/media/platform/omap3isp/isp.c index 8eb000e3d8fd..2a11a709aa4f
> > > > 100644
> > > > --- a/drivers/media/platform/omap3isp/isp.c
> > > > +++ b/drivers/media/platform/omap3isp/isp.c
> > > > @@ -61,7 +61,14 @@
> > > > #include
> > > > #include
> > > >
> > > > +#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
> > > > #include
> > > > +#else
> > > > +#define arm_iommu_create_mapping(...) NULL
> > > > +#define arm_iommu_attach_device(...) -ENODEV
> > > > +#define arm_iommu_release_mapping(...) do {} while (0)
> > > > +#define arm_iommu_detach_device(...) do {} while (0)
> > > > +#endif
> > >
> > > I don't think it's the job of a driver to define those stubs, sorry.
> > > Otherwise where do you stop ? If you have half of the code that is
> > > architecture- dependent, would you stub it ? And what if the stubs you
> > > define here generate warnings in static analyzers ?
> >
> > I agree that we should avoid doing that as a general case, but see
> > below.
> >
> > > If you want to make drivers compile for all architectures, the APIs they
> > > use must be defined in linux/, not in asm/. They can be stubbed there
> > > when not implemented in a particular architecture, but not in the driver.
> > >
> >
> > In this specific case, the same approach taken here is already needed
> > by the Renesas VMSA-compatible IPMMU driver, with, btw, is inside
> > drivers/iommu:
> >
> > drivers/iommu/ipmmu-vmsa.c
>
> The reason there is different, the driver is shared by ARM32 and ARM64
> platforms. Furthermore, there's an effort (or at least there was) to move
> away
> from those APIs in the driver, but I think it has stalled.
Anyway, the approach sticks at the driver. As this was accepted even
inside drivers/iommu, I fail to see why not doing the same on media,
specially since it really helps us to find bugs at omap3isp driver.
Even without pushing upstream (where Coverity would analyze it),
we got lots of problems by simply letting omap3isp to use the
usual tools we use for all other drivers.
> > Also, this API is used only by 3 drivers [1]:
> >
> > drivers/iommu/ipmmu-vmsa.c
> > drivers/iommu/mtk_iommu_v1.c
> > drivers/media/platform/omap3isp/isp.c
> >
> > [1] as blamed by
> > git grep -l arm_iommu_create_mapping
>
> The exynos driver also uses it.
>
> > That hardly seems to be an arch-specific iommu solution, but, instead, some
> > hack used by only three drivers or some legacy iommu binding.
>
> It's more complex than that. There are multiple IOMMU-related APIs on ARM, so
> more recent than others, with different feature sets. While I agree that
> drivers should move away from arm_iommu_create_mapping(), doing so requires
> coordination between the IOMMU driver and the bus master driver (for instance
> the omap3isp driver). It's not a trivial matter, but I'd love if someone
> submitted patches :-)
If someone steps up to do that, it would be really helpful, but we
Re: [PATCH] v4l: omap3isp: Enable driver compilation on ARM with COMPILE_TEST
Em Sat, 7 Apr 2018 14:40:08 +0300 Laurent Pinchart escreveu: > The omap3isp driver can't be compiled on non-ARM platforms but has no > compile-time dependency on OMAP. It however requires common clock > framework support, which isn't provided by all ARM platforms. > > Drop the OMAP dependency when COMPILE_TEST is set and add ARM and > COMMON_CLK dependencies. > > Signed-off-by: Laurent Pinchart > --- > drivers/media/platform/Kconfig | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > Hi Mauro, > > While we continue the discussions on whether the ARM IOMMU functions should be > stubbed in the omap3isp driver itself or not, I propose already merging this > patch that will extend build coverage for the omap3isp driver. Extending > compilation to non-ARM platforms can then be added on top, depending on the > result of the discussion. > > You might have noticed the 0-day build bot report reporting that the driver > depends on the common clock framework (build failure on openrisc). The issue > affects ARM as well as not all ARM platforms use the common clock framework. > I've thus also added a dependency on COMMON_CLK. Note that this dependency can > prevent compilation on x86 platforms. If you want to fix that, the > definition of struct clk_hw in include/linux/clk-provider.h will need to be > exposed even when CONFIG_COMMON_CLK isn't selected. I'll let you propose a fix > for that issue to the clock maintainers if you think it should be addressed. Weird, it built/linked fine on x86 without COMMON_CLK. Perhaps there are some stubs there that aren't working properly for openrisc arch. I'll take a look on it. > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index c7a1cf8a1b01..58aa233d3cf9 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -62,7 +62,10 @@ config VIDEO_MUX > > config VIDEO_OMAP3 > tristate "OMAP 3 Camera support" > - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 > + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API > + depends on ARCH_OMAP3 || COMPILE_TEST > + depends on ARM > + depends on COMMON_CLK > depends on HAS_DMA && OF > depends on OMAP_IOMMU > select ARM_DMA_USE_IOMMU Thanks, Mauro
Re: [PATCH v2 02/19] media: omap3isp: allow it to build with COMPILE_TEST
Hi Mauro,
Thank you for the patch.
On Thursday, 5 April 2018 23:29:29 EEST Mauro Carvalho Chehab wrote:
> There aren't much things required for it to build with COMPILE_TEST.
> It just needs to provide stub for an arm-dependent include.
>
> Let's replicate the same solution used by ipmmu-vmsa, in order
> to allow building omap3 with COMPILE_TEST.
>
> The actual logic here came from this driver:
>
>drivers/iommu/ipmmu-vmsa.c
>
> Signed-off-by: Mauro Carvalho Chehab
> ---
> drivers/media/platform/Kconfig| 8
> drivers/media/platform/omap3isp/isp.c | 7 +++
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index c7a1cf8a1b01..03c9dfeb7781 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -62,12 +62,12 @@ config VIDEO_MUX
>
> config VIDEO_OMAP3
> tristate "OMAP 3 Camera support"
> - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3
> + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
> depends on HAS_DMA && OF
> - depends on OMAP_IOMMU
> - select ARM_DMA_USE_IOMMU
> + depends on ((ARCH_OMAP3 && OMAP_IOMMU) || COMPILE_TEST)
> + select ARM_DMA_USE_IOMMU if OMAP_IOMMU
> select VIDEOBUF2_DMA_CONTIG
> - select MFD_SYSCON
> + select MFD_SYSCON if ARCH_OMAP3
Is this needed ? Can't MFD_SYSCON be selected on non-ARM platforms ?
> select V4L2_FWNODE
> ---help---
> Driver for an OMAP 3 camera controller.
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 8eb000e3d8fd..2a11a709aa4f
> 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -61,7 +61,14 @@
> #include
> #include
>
> +#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
> #include
> +#else
> +#define arm_iommu_create_mapping(...)NULL
> +#define arm_iommu_attach_device(...) -ENODEV
> +#define arm_iommu_release_mapping(...) do {} while (0)
> +#define arm_iommu_detach_device(...) do {} while (0)
> +#endif
>
> #include
> #include
As just commented on in a reply to v1 I still don't think this is a proper
solution. I'm all for extending compile-test coverage, but I don't believe we
need to make all drivers compile on every platform. ARM is certainly a
mainstream-enough platform nowadays to be worth having the media tree compiled
for.
--
Regards,
Laurent Pinchart
Re: [PATCH] v4l: omap3isp: Enable driver compilation on ARM with COMPILE_TEST
Hi Mauro, On Saturday, 7 April 2018 16:16:57 EEST Mauro Carvalho Chehab wrote: > Em Sat, 7 Apr 2018 14:40:08 +0300 Laurent Pinchart escreveu: > > The omap3isp driver can't be compiled on non-ARM platforms but has no > > compile-time dependency on OMAP. It however requires common clock > > framework support, which isn't provided by all ARM platforms. > > > > Drop the OMAP dependency when COMPILE_TEST is set and add ARM and > > COMMON_CLK dependencies. > > > > Signed-off-by: Laurent Pinchart > > --- > > > > drivers/media/platform/Kconfig | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > Hi Mauro, > > > > While we continue the discussions on whether the ARM IOMMU functions > > should be stubbed in the omap3isp driver itself or not, I propose already > > merging this patch that will extend build coverage for the omap3isp > > driver. Extending compilation to non-ARM platforms can then be added on > > top, depending on the result of the discussion. > > > > You might have noticed the 0-day build bot report reporting that the > > driver depends on the common clock framework (build failure on openrisc). > > The issue affects ARM as well as not all ARM platforms use the common > > clock framework. I've thus also added a dependency on COMMON_CLK. Note > > that this dependency can prevent compilation on x86 platforms. If you want > > to fix that, the definition of struct clk_hw in include/linux/clk- > > provider.h will need to be exposed even when CONFIG_COMMON_CLK isn't > > selected. I'll let you propose a fix for that issue to the clock > > maintainers if you think it should be addressed. > > Weird, it built/linked fine on x86 without COMMON_CLK. Perhaps there are > some stubs there that aren't working properly for openrisc arch. I'll > take a look on it. It's not function stubs this time, but structure definitions. I haven't checked why it might build on x86, but please note that some x86 platforms enable COMMON_CLK, that might be why you got it to build with an allyesconfig or allmodconfig configuration. Given that this patch extends compile-test coverage from ARCH_OMAP3 to ARM and fixes a dependency issue that would affect other architectures as well, could you consider taking it in your tree and base your compile-test coverage expansion for omap3isp on top of it ? > > diff --git a/drivers/media/platform/Kconfig > > b/drivers/media/platform/Kconfig index c7a1cf8a1b01..58aa233d3cf9 100644 > > --- a/drivers/media/platform/Kconfig > > +++ b/drivers/media/platform/Kconfig > > @@ -62,7 +62,10 @@ config VIDEO_MUX > > > > config VIDEO_OMAP3 > > tristate "OMAP 3 Camera support" > > - depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API && ARCH_OMAP3 > > + depends on VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API > > + depends on ARCH_OMAP3 || COMPILE_TEST > > + depends on ARM > > + depends on COMMON_CLK > > depends on HAS_DMA && OF > > depends on OMAP_IOMMU > > select ARM_DMA_USE_IOMMU -- Regards, Laurent Pinchart
[PATCH 4/6] media: ov772x: add media controller support
Create a source pad and set the media controller type to the sensor.
Cc: Jacopo Mondi
Cc: Laurent Pinchart
Cc: Hans Verkuil
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
Signed-off-by: Akinobu Mita
---
drivers/media/i2c/ov772x.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 4bb81ff..5e91fa1 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -425,6 +425,9 @@ struct ov772x_priv {
unsigned shortband_filter;
unsigned int fps;
int (*reg_read)(struct i2c_client *client, u8 addr);
+#ifdef CONFIG_MEDIA_CONTROLLER
+ struct media_pad pad;
+#endif
};
/*
@@ -1328,9 +1331,17 @@ static int ov772x_probe(struct i2c_client *client,
goto error_clk_put;
}
- ret = ov772x_video_probe(priv);
+#ifdef CONFIG_MEDIA_CONTROLLER
+ priv->pad.flags = MEDIA_PAD_FL_SOURCE;
+ priv->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+ ret = media_entity_pads_init(&priv->subdev.entity, 1, &priv->pad);
if (ret < 0)
goto error_gpio_put;
+#endif
+
+ ret = ov772x_video_probe(priv);
+ if (ret < 0)
+ goto error_entity_cleanup;
priv->cfmt = &ov772x_cfmts[0];
priv->win = &ov772x_win_sizes[0];
@@ -1338,11 +1349,15 @@ static int ov772x_probe(struct i2c_client *client,
ret = v4l2_async_register_subdev(&priv->subdev);
if (ret)
- goto error_gpio_put;
+ goto error_entity_cleanup;
return 0;
+error_entity_cleanup:
+#ifdef CONFIG_MEDIA_CONTROLLER
+ media_entity_cleanup(&priv->subdev.entity);
error_gpio_put:
+#endif
if (priv->pwdn_gpio)
gpiod_put(priv->pwdn_gpio);
error_clk_put:
@@ -1357,6 +1372,9 @@ static int ov772x_remove(struct i2c_client *client)
{
struct ov772x_priv *priv = to_ov772x(i2c_get_clientdata(client));
+#ifdef CONFIG_MEDIA_CONTROLLER
+ media_entity_cleanup(&priv->subdev.entity);
+#endif
clk_put(priv->clk);
if (priv->pwdn_gpio)
gpiod_put(priv->pwdn_gpio);
--
2.7.4
[PATCH 6/6] media: ov772x: support device tree probing
The ov772x driver currently only supports legacy platform data probe.
This change enables device tree probing.
Note that the platform data probe can select auto or manual edge control
mode, but the device tree probling can only select auto edge control mode
for now.
Cc: Jacopo Mondi
Cc: Laurent Pinchart
Cc: Hans Verkuil
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
Signed-off-by: Akinobu Mita
---
drivers/media/i2c/ov772x.c | 60 ++
1 file changed, 40 insertions(+), 20 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 5e91fa1..e67ec37 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -763,13 +763,13 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_VFLIP:
val = ctrl->val ? VFLIP_IMG : 0x00;
priv->flag_vflip = ctrl->val;
- if (priv->info->flags & OV772X_FLAG_VFLIP)
+ if (priv->info && (priv->info->flags & OV772X_FLAG_VFLIP))
val ^= VFLIP_IMG;
return ov772x_mask_set(client, COM3, VFLIP_IMG, val);
case V4L2_CID_HFLIP:
val = ctrl->val ? HFLIP_IMG : 0x00;
priv->flag_hflip = ctrl->val;
- if (priv->info->flags & OV772X_FLAG_HFLIP)
+ if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP))
val ^= HFLIP_IMG;
return ov772x_mask_set(client, COM3, HFLIP_IMG, val);
case V4L2_CID_BAND_STOP_FILTER:
@@ -928,19 +928,14 @@ static void ov772x_select_params(const struct
v4l2_mbus_framefmt *mf,
*win = ov772x_select_win(mf->width, mf->height);
}
-static int ov772x_set_params(struct ov772x_priv *priv,
-const struct ov772x_color_format *cfmt,
-const struct ov772x_win_size *win)
+static int ov772x_edgectrl(struct ov772x_priv *priv)
{
struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev);
- struct v4l2_fract tpf;
int ret;
- u8 val;
- /* Reset hardware. */
- ov772x_reset(client);
+ if (!priv->info)
+ return 0;
- /* Edge Ctrl. */
if (priv->info->edgectrl.strength & OV772X_MANUAL_EDGE_CTRL) {
/*
* Manual Edge Control Mode.
@@ -951,19 +946,19 @@ static int ov772x_set_params(struct ov772x_priv *priv,
ret = ov772x_mask_set(client, DSPAUTO, EDGE_ACTRL, 0x00);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
ret = ov772x_mask_set(client,
EDGE_TRSHLD, OV772X_EDGE_THRESHOLD_MASK,
priv->info->edgectrl.threshold);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
ret = ov772x_mask_set(client,
EDGE_STRNGT, OV772X_EDGE_STRENGTH_MASK,
priv->info->edgectrl.strength);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
} else if (priv->info->edgectrl.upper > priv->info->edgectrl.lower) {
/*
@@ -975,15 +970,35 @@ static int ov772x_set_params(struct ov772x_priv *priv,
EDGE_UPPER, OV772X_EDGE_UPPER_MASK,
priv->info->edgectrl.upper);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
ret = ov772x_mask_set(client,
EDGE_LOWER, OV772X_EDGE_LOWER_MASK,
priv->info->edgectrl.lower);
if (ret < 0)
- goto ov772x_set_fmt_error;
+ return ret;
}
+ return 0;
+}
+
+static int ov772x_set_params(struct ov772x_priv *priv,
+const struct ov772x_color_format *cfmt,
+const struct ov772x_win_size *win)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev);
+ struct v4l2_fract tpf;
+ int ret;
+ u8 val;
+
+ /* Reset hardware. */
+ ov772x_reset(client);
+
+ /* Edge Ctrl. */
+ ret = ov772x_edgectrl(priv);
+ if (ret < 0)
+ goto ov772x_set_fmt_error;
+
/* Format and window size. */
ret = ov772x_write(client, HSTART, win->rect.left >> 2);
if (ret < 0)
@@ -1284,11 +1299,6 @@ static int ov772x_probe(struct i2c_client *client,
struct i2c_adapter *adapter = client->adapter;
int ret;
- if (!client->dev.platform_data) {
- dev_err(&client->dev, "Missing ov772x platform data\n");
- return -E
[PATCH 1/6] media: ov772x: allow i2c controllers without I2C_FUNC_PROTOCOL_MANGLING
The ov772x driver only works when the i2c controller have
I2C_FUNC_PROTOCOL_MANGLING. However, many i2c controller drivers don't
support it.
The reason that the ov772x requires I2C_FUNC_PROTOCOL_MANGLING is that
it doesn't support repeated starts.
This change adds an alternative method for reading from ov772x register
which uses two separated i2c messages for the i2c controllers without
I2C_FUNC_PROTOCOL_MANGLING.
Cc: Jacopo Mondi
Cc: Laurent Pinchart
Cc: Hans Verkuil
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
Signed-off-by: Akinobu Mita
---
drivers/media/i2c/ov772x.c | 42 +-
1 file changed, 33 insertions(+), 9 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index b62860c..283ae2c 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -424,6 +424,7 @@ struct ov772x_priv {
/* band_filter = COM8[5] ? 256 - BDBASE : 0 */
unsigned shortband_filter;
unsigned int fps;
+ int (*reg_read)(struct i2c_client *client, u8 addr);
};
/*
@@ -542,11 +543,34 @@ static struct ov772x_priv *to_ov772x(struct v4l2_subdev
*sd)
return container_of(sd, struct ov772x_priv, subdev);
}
-static inline int ov772x_read(struct i2c_client *client, u8 addr)
+static int ov772x_read(struct i2c_client *client, u8 addr)
+{
+ struct v4l2_subdev *sd = i2c_get_clientdata(client);
+ struct ov772x_priv *priv = to_ov772x(sd);
+
+ return priv->reg_read(client, addr);
+}
+
+static int ov772x_reg_read(struct i2c_client *client, u8 addr)
{
return i2c_smbus_read_byte_data(client, addr);
}
+static int ov772x_reg_read_fallback(struct i2c_client *client, u8 addr)
+{
+ int ret;
+ u8 val;
+
+ ret = i2c_master_send(client, &addr, 1);
+ if (ret < 0)
+ return ret;
+ ret = i2c_master_recv(client, &val, 1);
+ if (ret < 0)
+ return ret;
+
+ return val;
+}
+
static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 value)
{
return i2c_smbus_write_byte_data(client, addr, value);
@@ -1255,20 +1279,20 @@ static int ov772x_probe(struct i2c_client *client,
return -EINVAL;
}
- if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
- I2C_FUNC_PROTOCOL_MANGLING)) {
- dev_err(&adapter->dev,
- "I2C-Adapter doesn't support SMBUS_BYTE_DATA or
PROTOCOL_MANGLING\n");
- return -EIO;
- }
- client->flags |= I2C_CLIENT_SCCB;
-
priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
priv->info = client->dev.platform_data;
+ if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
+I2C_FUNC_PROTOCOL_MANGLING))
+ priv->reg_read = ov772x_reg_read;
+ else
+ priv->reg_read = ov772x_reg_read_fallback;
+
+ client->flags |= I2C_CLIENT_SCCB;
+
v4l2_i2c_subdev_init(&priv->subdev, client, &ov772x_subdev_ops);
v4l2_ctrl_handler_init(&priv->hdl, 3);
v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops,
--
2.7.4
[PATCH 2/6] media: ov772x: add checks for register read errors
This change adds checks for register read errors and returns correct
error code.
Cc: Jacopo Mondi
Cc: Laurent Pinchart
Cc: Hans Verkuil
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
Signed-off-by: Akinobu Mita
---
drivers/media/i2c/ov772x.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 283ae2c..c56f910 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1169,8 +1169,15 @@ static int ov772x_video_probe(struct ov772x_priv *priv)
return ret;
/* Check and show product ID and manufacturer ID. */
- pid = ov772x_read(client, PID);
- ver = ov772x_read(client, VER);
+ ret = ov772x_read(client, PID);
+ if (ret < 0)
+ return ret;
+ pid = ret;
+
+ ret = ov772x_read(client, VER);
+ if (ret < 0)
+ return ret;
+ ver = ret;
switch (VERSION(pid, ver)) {
case OV7720:
--
2.7.4
[PATCH 5/6] media: ov772x: add device tree binding
This adds a device tree binding documentation for OV7720/OV7725 sensor.
Cc: Jacopo Mondi
Cc: Laurent Pinchart
Cc: Hans Verkuil
Cc: Sakari Ailus
Cc: Mauro Carvalho Chehab
Cc: Rob Herring
Signed-off-by: Akinobu Mita
---
.../devicetree/bindings/media/i2c/ov772x.txt | 36 ++
MAINTAINERS| 1 +
2 files changed, 37 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt
b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
new file mode 100644
index 000..9b0df3b
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
@@ -0,0 +1,36 @@
+* Omnivision OV7720/OV7725 CMOS sensor
+
+Required Properties:
+- compatible: shall be one of
+ "ovti,ov7720"
+ "ovti,ov7725"
+- clocks: reference to the xclk input clock.
+- clock-names: shall be "xclk".
+
+Optional Properties:
+- rstb-gpios: reference to the GPIO connected to the RSTB pin, if any.
+- pwdn-gpios: reference to the GPIO connected to the PWDN pin, if any.
+
+The device node shall contain one 'port' child node with one child 'endpoint'
+subnode for its digital output video port, in accordance with the video
+interface bindings defined in Documentation/devicetree/bindings/media/
+video-interfaces.txt.
+
+Example:
+
+&i2c0 {
+ ov772x: camera@21 {
+ compatible = "ovti,ov7725";
+ reg = <0x21>;
+ rstb-gpios = <&axi_gpio_0 0 GPIO_ACTIVE_LOW>;
+ pwdn-gpios = <&axi_gpio_0 1 GPIO_ACTIVE_LOW>;
+ clocks = <&xclk>;
+ clock-names = "xclk";
+
+ port {
+ ov772x_0: endpoint {
+ remote-endpoint = <&vcap1_in0>;
+ };
+ };
+ };
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 7e48624..3e0224a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10295,6 +10295,7 @@ T: git git://linuxtv.org/media_tree.git
S: Odd fixes
F: drivers/media/i2c/ov772x.c
F: include/media/i2c/ov772x.h
+F: Documentation/devicetree/bindings/media/i2c/ov772x.txt
OMNIVISION OV7740 SENSOR DRIVER
M: Wenyou Yang
--
2.7.4
[PATCH 0/6] media: ov772x: support media controller, device tree probing, etc.
This patchset includes support media controller, device tree probing and other miscellanuous changes for ov772x driver. Akinobu Mita (6): media: ov772x: allow i2c controllers without I2C_FUNC_PROTOCOL_MANGLING media: ov772x: add checks for register read errors media: ov772x: create subdevice device node media: ov772x: add media controller support media: ov772x: add device tree binding media: ov772x: support device tree probing .../devicetree/bindings/media/i2c/ov772x.txt | 36 ++ MAINTAINERS| 1 + drivers/media/i2c/ov772x.c | 136 - 3 files changed, 140 insertions(+), 33 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt Cc: Jacopo Mondi Cc: Laurent Pinchart Cc: Hans Verkuil Cc: Sakari Ailus Cc: Mauro Carvalho Chehab Cc: Rob Herring -- 2.7.4
[PATCH 3/6] media: ov772x: create subdevice device node
Set the V4L2_SUBDEV_FL_HAS_DEVNODE flag for the subdevice so that the subdevice device node is created. Cc: Jacopo Mondi Cc: Laurent Pinchart Cc: Hans Verkuil Cc: Sakari Ailus Cc: Mauro Carvalho Chehab Signed-off-by: Akinobu Mita --- drivers/media/i2c/ov772x.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c index c56f910..4bb81ff 100644 --- a/drivers/media/i2c/ov772x.c +++ b/drivers/media/i2c/ov772x.c @@ -1301,6 +1301,7 @@ static int ov772x_probe(struct i2c_client *client, client->flags |= I2C_CLIENT_SCCB; v4l2_i2c_subdev_init(&priv->subdev, client, &ov772x_subdev_ops); + priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; v4l2_ctrl_handler_init(&priv->hdl, 3); v4l2_ctrl_new_std(&priv->hdl, &ov772x_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); -- 2.7.4
wir bieten 2% Kredite
Sehr geehrte Damen und Herren, Sie brauchen Geld? Sie sind auf der suche nach einem Darlehnen? Seriös und unkompliziert? Dann sind Sie hier bei uns genau richtig. Durch unsere jahrelange Erfahrung und kompetente Beratung sind wir Europaweit tätig. Wir bieten jedem ein GÜNSTIGES Darlehnen zu TOP Konditionen an. Darlehnen zwischen 5000 CHF/Euro bis zu 20 Millionen CHF/Euro möglich. Wir erheben dazu 2% Zinssatz. Lassen Sie sich von unserem kompetenten Team beraten. Zögern Sie nicht und kontaktieren Sie mich unter für weitere Infos & Anfragen unter der eingeblendeten Email Adresse. Ich freue mich von Ihnen zu hören.
business Proposal / Geschäftsvorschlag
I have a business Proposal for you, contact me directly This business has a cash involvement of $250,000,000.00 Anders Karlsson Ich habe einen Geschäftsvorschlag für Sie, kontaktieren Sie mich direkt Dieses Unternehmen hat eine Beteiligung von $ 250.000.000,00 - [] Anders Karlsson
cron job: media_tree daily build: OK
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: Sun Apr 8 05:00:13 CEST 2018 media-tree git hash:17dec0a949153d9ac00760ba2f5b78cb583e995f media_build git hash: 541653bb52fcf7c34b8b83a4c8cc66b09c68ac37 v4l-utils git hash: 47d43b130dc6e9e0edc900759fb37649208371e4 gcc version:i686-linux-gcc (GCC) 7.3.0 sparse version: v0.5.2-rc1 smatch version: v0.5.0-4419-g3b5bf5c9 host hardware: x86_64 host os:4.14.0-3-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-multi: OK linux-git-arm-pxa: OK linux-git-arm-stm32: OK linux-git-arm64: OK linux-git-i686: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.36.4-i686: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-i686: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-i686: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-i686: OK linux-2.6.39.4-x86_64: OK linux-3.0.101-i686: OK linux-3.0.101-x86_64: OK linux-3.1.10-i686: OK linux-3.1.10-x86_64: OK linux-3.2.101-i686: OK linux-3.2.101-x86_64: OK linux-3.3.8-i686: OK linux-3.3.8-x86_64: OK linux-3.4.113-i686: OK linux-3.4.113-x86_64: OK linux-3.5.7-i686: OK linux-3.5.7-x86_64: OK linux-3.6.11-i686: OK linux-3.6.11-x86_64: OK linux-3.7.10-i686: OK linux-3.7.10-x86_64: OK linux-3.8.13-i686: OK linux-3.8.13-x86_64: OK linux-3.9.11-i686: OK linux-3.9.11-x86_64: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.56-i686: OK linux-3.16.56-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.102-i686: OK linux-3.18.102-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.51-i686: OK linux-4.1.51-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.109-i686: OK linux-4.4.109-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.91-i686: OK linux-4.9.91-x86_64: OK linux-4.14.31-i686: OK linux-4.14.31-x86_64: OK linux-4.15.14-i686: OK linux-4.15.14-x86_64: OK linux-4.16-i686: OK linux-4.16-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Sunday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Sunday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
