Some improvements/fixes
Subsystems (7)
cover Some improvements/fixes
I'm testing the Media Controller graph on some of my boards. While here, I noticed some troubles when producing the graphs.
This series address them.
PS.: I'm storing the graphs of the boards I've tested at:
https://mchehab.fedorapeople.org/mc-next-gen/?C=M;O=D
I improved the v4l2-utils tool that produces the graph (contrib/test/mc_nextgen_test) to add a title on each graph. So, the files after 2016-02-12 will contain the info provided by MEDIA_IOC_DEVICE_INFO ioctl.
Mauro Carvalho Chehab (11):
[media] v4l2-mc.h: prevent it for being included twice [media] v4l2-mc: add a routine to create USB media_device [media] rc-core: don't lock device at rc_register_device() [media] allow overriding the driver name
[media] use v4l2_mc_usb_media_device_init() on most USB devices [media] v4l2-mc: use usb_make_path() to provide bus info [media] em28xx-dvb: create RF connector on DVB-only mode [media] cx231xx: use v4l2 core function to create the MC graph [media] si2157: register as a tuner entity
[media] cx231xx, em28xx: pass media_device to si2157 [media] cx231xx: create connectors at the media graph
drivers/media/media-device.c | 6 ++- drivers/media/rc/rc-main.c | 45 ++++++++++++--------- drivers/media/tuners/si2157.c | 32 ++++++++++++++- drivers/media/tuners/si2157.h | 5 +++ drivers/media/tuners/si2157_priv.h | 8 ++++ drivers/media/usb/au0828/au0828-core.c | 19 ++------- drivers/media/usb/cx231xx/cx231xx-cards.c | 62 +++-------------------------- drivers/media/usb/cx231xx/cx231xx-dvb.c | 10 ++++- drivers/media/usb/cx231xx/cx231xx-video.c | 46 +++++++++++++++++++++ drivers/media/usb/cx231xx/cx231xx.h | 3 ++ drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 13 +----- drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 15 +------ drivers/media/usb/em28xx/em28xx-cards.c | 23 ++++------- drivers/media/usb/em28xx/em28xx-dvb.c | 13 +++++- drivers/media/usb/em28xx/em28xx-video.c | 3 +- drivers/media/usb/siano/smsusb.c | 1 + drivers/media/v4l2-core/v4l2-mc.c | 41 ++++++++++++++++++- include/media/media-device.h | 5 +++ include/media/rc-core.h | 2 + include/media/v4l2-mc.h | 44 +++++++++++++++++--- 20 files changed, 255 insertions(+), 141 deletions(-)
Signature · 1 line
2.5.0
PATCH 1/11 v4l2-mc.h: prevent it for being included twice
Don't let it be included twice, to avoid compiler issues.
--- include/media/v4l2-mc.h | 5 +++++ 1 file changed, 5 insertions(+)
include/media/v4l2-mc.h +5 -0
@@ -14,6 +14,9 @@* GNU General Public License for more details. */ +#ifndef _V4L2_MC_H +#define _V4L2_MC_H +#include <media/media-device.h> /**
@@ -136,3 +139,5 @@ struct media_device *v4l2_mc_pci_media_device_init(struct pci_dev *pci_dev, }#endif + +#endif
Signature · 1 line
2.5.0
Re: [PATCH 01/11] [media] v4l2-mc.h: prevent it for being included twice
Quoted 3 lines · Mauro Carvalho Chehab
Don't let it be included twice, to avoid compiler issues.
Signed-off-by Mauro Carvalho Chehab <[email protected]>
Thanks!
Hans
Quoted 25 lines
--- include/media/v4l2-mc.h | 5 +++++ 1 file changed, 5 insertions(+)include/media/v4l2-mc.h +5
-0@@ -14,6 +14,9 @@* GNU General Public License for more details. */ +#ifndef _V4L2_MC_H +#define _V4L2_MC_H +#include <media/media-device.h> /** @@ -136,3 +139,5 @@ struct media_device *v4l2_mc_pci_media_device_init(struct pci_dev *pci_dev, }#endif + +#endif
PATCH 2/11 v4l2-mc: add a routine to create USB media_device
In reply to Mauro Carvalho ChehabInstead of copying exactly the same code on all USB devices, add an ancillary routine that will create and fill the struct media_device with the values imported from the USB device.
--- drivers/media/v4l2-core/v4l2-mc.c | 41 ++++++++++++++++++++++++++++++++++++++- include/media/v4l2-mc.h | 39 ++++++++++++++++++++++++++++++++----- 2 files changed, 74 insertions(+), 6 deletions(-)
drivers/media/v4l2-core/v4l2-mc.c +40 -1
@@ -16,12 +16,13 @@#include <linux/module.h> #include <linux/pci.h> +#include <linux/usb.h> #include <media/media-entity.h> #include <media/v4l2-mc.h> struct media_device *v4l2_mc_pci_media_device_init (struct pci_dev *pci_dev, -char *name) +const char *name) {#ifdef CONFIG_PCI struct media_device *mdev;
@@ -53,6 +54,44 @@ struct media_device *v4l2_mc_pci_media_device_init(struct pci_dev *pci_dev, }EXPORT_SYMBOL_GPL (v4l2_mc_pci_media_device_init); +struct media_device *__v4l2_mc_usb_media_device_init (struct usb_device *udev, +const char *board_name, +const char *driver_name) +{ +#ifdef CONFIG_USB +struct media_device *mdev; + + mdev =kzalloc (sizeof (*mdev),GFP_KERNEL ); +if (!mdev) +return NULL ; + + mdev->dev = &udev->dev; + +if (driver_name) +strlcpy (mdev->driver_name, driver_name, +sizeof (mdev->driver_name)); + +if (board_name) +strlcpy (mdev->model, board_name,sizeof (mdev->model)); +else if (udev->product) +strlcpy (mdev->model, udev->product,sizeof (mdev->model)); +else +strlcpy (mdev->model,"unknown model" ,sizeof (mdev->model)); +if (udev->serial) +strlcpy (mdev->serial, udev->serial,sizeof (mdev->serial)); +strcpy (mdev->bus_info, udev->devpath); + mdev->hw_revision =le16_to_cpu (udev->descriptor.bcdDevice); + mdev->driver_version =LINUX_VERSION_CODE ; + +media_device_init (mdev); + +return mdev; +#else +return NULL ; +#endif +} +EXPORT_SYMBOL_GPL (__v4l2_mc_usb_media_device_init); +int v4l2_mc_create_media_graph (struct media_device *mdev) {
include/media/v4l2-mc.h +34 -5
@@ -95,8 +95,9 @@ enum demod_pad_index { DEMOD_NUM_PADS }; - -struct pci_dev ;/* We don't need to include pci.h here */ +/* We don't need to include pci.h or usb.h here */ +struct pci_dev ; +struct usb_device ;#ifdef CONFIG_MEDIA_CONTROLLER /**
@@ -124,8 +125,24 @@ int v4l2_mc_create_media_graph(struct media_device *mdev);* name for the pci device, given by pci_name() macro. */ struct media_device *v4l2_mc_pci_media_device_init (struct pci_dev *pci_dev, -char *name); - +const char *name); +/** +* __v4l2_mc_usb_media_device_init() - create and initialize a +* struct &media_device from a PCI device. +* +* @udev: pointer to struct usb_device +* @board_name: media device name. If %NULL, the routine will use the usb +* product name, if available. +* @driver_name: name of the driver. if %NULL, the routine will use the name +* given by udev->dev->driver->name, with is usually the wrong +* thing to do. +* +* NOTE: It is better to call v4l2_mc_usb_media_device_init() instead, as +* such macro fills driver_name with %KBUILD_MODNAME. +*/ +struct media_device *__v4l2_mc_usb_media_device_init (struct usb_device *udev, +const char *board_name, +const char *driver_name);#else static inline int v4l2_mc_create_media_graph (struct media_device *mdev)
@@ -133,11 +150,23 @@ static inline int v4l2_mc_create_media_graph(struct media_device *mdev)return 0 ; } +static inline struct media_device *v4l2_mc_pci_media_device_init (struct pci_dev *pci_dev, -char *name) { +char *name) +{return NULL ; } +static inline +struct media_device *__v4l2_mc_usb_media_device_init (struct usb_device *udev, +char *board_name, +char *driver_name) +{ +return NULL ; +}#endif +#define v4l2_mc_usb_media_device_init (udev, name) \ + __v4l2_mc_usb_media_device_init(udev, name, KBUILD_MODNAME) +#endif
Signature · 1 line
2.5.0
Re: [PATCH 02/11] [media] v4l2-mc: add a routine to create USB media_device
Quoted 50 lines · Mauro Carvalho Chehab
Instead of copying exactly the same code on all USB devices, add an ancillary routine that will create and fill the struct
media_devicewith the values imported from the USB device.Signed-off-by Mauro Carvalho Chehab <[email protected]>--- drivers/media/v4l2-core/v4l2-mc.c | 41 ++++++++++++++++++++++++++++++++++++++- include/media/v4l2-mc.h | 39 ++++++++++++++++++++++++++++++++----- 2 files changed, 74 insertions(+), 6 deletions(-)drivers/media/v4l2-core/v4l2-mc.c +18
-1@@ -16,12 +16,13 @@#include <linux/module.h> #include <linux/pci.h> +#include <linux/usb.h> #include <media/media-entity.h> #include <media/v4l2-mc.h> struct media_device *v4l2_mc_pci_media_device_init (struct pci_dev *pci_dev, -char *name) +const char *name) {#ifdef CONFIG_PCI struct media_device *mdev;@@ -53,6 +54,44 @@ struct media_device *v4l2_mc_pci_media_device_init(struct pci_dev *pci_dev, }EXPORT_SYMBOL_GPL (v4l2_mc_pci_media_device_init); +struct media_device *__v4l2_mc_usb_media_device_init (struct usb_device *udev, +const char *board_name, +const char *driver_name) +{ +#ifdef CONFIG_USB +struct media_device *mdev; + + mdev =kzalloc (sizeof (*mdev),GFP_KERNEL ); +if (!mdev) +return NULL ; + + mdev->dev = &udev->dev; + +if (driver_name) +strlcpy (mdev->driver_name, driver_name, +sizeof (mdev->driver_name));
drivers/media/v4l2-core/v4l2-mc.c struct media_device *v4l2_mc_pci_media_device_init(struct pci_dev *pci_dev, · line 82
Quoted 10 lines
drivers/media/v4l2-core/v4l2-mc.c +10
-0+ +if (board_name) +strlcpy (mdev->model, board_name,sizeof (mdev->model)); +else if (udev->product) +strlcpy (mdev->model, udev->product,sizeof (mdev->model)); +else +strlcpy (mdev->model,"unknown model" ,sizeof (mdev->model)); +if (udev->serial) +strlcpy (mdev->serial, udev->serial,sizeof (mdev->serial)); +strcpy (mdev->bus_info, udev->devpath);
I would use strlcpy here as well. Just to be safe.
I also noticed that in v4l2_mc_pci_media_device_init() you use sprintf for bus_info instead of snprintf. There too I would prefer to see snprintf.
Regards,
Hans
Quoted 84 lines
drivers/media/v4l2-core/v4l2-mc.c +12
-0+ mdev->hw_revision =le16_to_cpu (udev->descriptor.bcdDevice); + mdev->driver_version =LINUX_VERSION_CODE ; + +media_device_init (mdev); + +return mdev; +#else +return NULL ; +#endif +} +EXPORT_SYMBOL_GPL (__v4l2_mc_usb_media_device_init); +int v4l2_mc_create_media_graph (struct media_device *mdev) {include/media/v4l2-mc.h +34
-5@@ -95,8 +95,9 @@ enum demod_pad_index { DEMOD_NUM_PADS }; - -struct pci_dev ;/* We don't need to include pci.h here */ +/* We don't need to include pci.h or usb.h here */ +struct pci_dev ; +struct usb_device ;#ifdef CONFIG_MEDIA_CONTROLLER /** @@ -124,8 +125,24 @@ int v4l2_mc_create_media_graph(struct media_device *mdev);* name for the pci device, given by pci_name() macro. */ struct media_device *v4l2_mc_pci_media_device_init (struct pci_dev *pci_dev, -char *name); - +const char *name); +/** +* __v4l2_mc_usb_media_device_init() - create and initialize a +* struct &media_device from a PCI device. +* +* @udev: pointer to struct usb_device +* @board_name: media device name. If %NULL, the routine will use the usb +* product name, if available. +* @driver_name: name of the driver. if %NULL, the routine will use the name +* given by udev->dev->driver->name, with is usually the wrong +* thing to do. +* +* NOTE: It is better to call v4l2_mc_usb_media_device_init() instead, as +* such macro fills driver_name with %KBUILD_MODNAME. +*/ +struct media_device *__v4l2_mc_usb_media_device_init (struct usb_device *udev, +const char *board_name, +const char *driver_name);#else static inline int v4l2_mc_create_media_graph (struct media_device *mdev)@@ -133,11 +150,23 @@ static inline int v4l2_mc_create_media_graph(struct media_device *mdev)return 0 ; } +static inline struct media_device *v4l2_mc_pci_media_device_init (struct pci_dev *pci_dev, -char *name) { +char *name) +{return NULL ; } +static inline +struct media_device *__v4l2_mc_usb_media_device_init (struct usb_device *udev, +char *board_name, +char *driver_name) +{ +return NULL ; +}#endif +#define v4l2_mc_usb_media_device_init (udev, name) \ + __v4l2_mc_usb_media_device_init(udev, name, KBUILD_MODNAME) +#endif
PATCH 3/11 rc-core: don't lock device at rc_register_device()
In reply to Mauro Carvalho ChehabThe mutex lock at rc_register_device() was added by commit 08aeb7c9a42a ("[media] rc: add locking to fix register/show race").
It is meant to avoid race issues when trying to open a sysfs file while the RC register didn't complete.
Adding a lock there causes troubles, as detected by the Kernel lock debug instrumentation at the Kernel:
======================================================
[ INFO: possible circular locking dependency detected ]
4.5.0-rc3+ #46 Not tainted
-------------------------------------------------------
systemd-udevd/2681 is trying to acquire lock:
(s_active#171){++++.+}, at: [<ffffffff8171a115>] kernfs_remove_by_name_ns+0x45/0xa0
but task is already holding lock:
(&dev->lock){+.+.+.}, at: [<ffffffffa0724def>] rc_register_device+0xb2f/0x1450 [rc_core]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&dev->lock){+.+.+.}:
[<ffffffff8124817d>] lock_acquire+0x13d/0x320
[<ffffffff822de966>] mutex_lock_nested+0xb6/0x860
[<ffffffffa0721f2b>] show_protocols+0x3b/0x3f0 [rc_core]
[<ffffffff81cdaba5>] dev_attr_show+0x45/0xc0
[<ffffffff8171f1b3>] sysfs_kf_seq_show+0x203/0x3c0
[<ffffffff8171a6a1>] kernfs_seq_show+0x121/0x1b0
[<ffffffff81617c71>] seq_read+0x2f1/0x1160
[<ffffffff8171c911>] kernfs_fop_read+0x321/0x460
[<ffffffff815abc20>] __vfs_read+0xe0/0x3d0
[<ffffffff815ae90e>] vfs_read+0xde/0x2d0
[<ffffffff815b1d01>] SyS_read+0x111/0x230
[<ffffffff822e8636>] entry_SYSCALL_64_fastpath+0x16/0x76
-> #0 (s_active#171){++++.+}:
[<ffffffff81244f24>] __lock_acquire+0x4304/0x5990
[<ffffffff8124817d>] lock_acquire+0x13d/0x320
[<ffffffff81717d3a>] __kernfs_remove+0x58a/0x810
[<ffffffff8171a115>] kernfs_remove_by_name_ns+0x45/0xa0
[<ffffffff81721592>] remove_files.isra.0+0x72/0x190
[<ffffffff8172174b>] sysfs_remove_group+0x9b/0x150
[<ffffffff81721854>] sysfs_remove_groups+0x54/0xa0
[<ffffffff81cd97d0>] device_remove_attrs+0xb0/0x140
[<ffffffff81cdb27c>] device_del+0x38c/0x6b0
[<ffffffffa0724b8b>] rc_register_device+0x8cb/0x1450 [rc_core]
[<ffffffffa1326a7b>] dvb_usb_remote_init+0x66b/0x14d0 [dvb_usb]
[<ffffffffa1321c81>] dvb_usb_device_init+0xf21/0x1860 [dvb_usb]
[<ffffffffa13517dc>] dib0700_probe+0x14c/0x410 [dvb_usb_dib0700]
[<ffffffff81dbb1dd>] usb_probe_interface+0x45d/0x940
[<ffffffff81ce7e7a>] driver_probe_device+0x21a/0xc30
[<ffffffff81ce89b1>] __driver_attach+0x121/0x160
[<ffffffff81ce21bf>] bus_for_each_dev+0x11f/0x1a0
[<ffffffff81ce6cdd>] driver_attach+0x3d/0x50
[<ffffffff81ce5df9>] bus_add_driver+0x4c9/0x770
[<ffffffff81cea39c>] driver_register+0x18c/0x3b0
[<ffffffff81db6e98>] usb_register_driver+0x1f8/0x440
[<ffffffffa074001e>] dib0700_driver_init+0x1e/0x1000 [dvb_usb_dib0700]
[<ffffffff810021b1>] do_one_initcall+0x141/0x300
[<ffffffff8144d8eb>] do_init_module+0x1d0/0x5ad
[<ffffffff812f27b6>] load_module+0x6666/0x9ba0
[<ffffffff812f5fe8>] SyS_finit_module+0x108/0x130
[<ffffffff822e8636>] entry_SYSCALL_64_fastpath+0x16/0x76
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&dev->lock);
lock(s_active#171);
lock(&dev->lock);
lock(s_active#171);
*** DEADLOCK ***
3 locks held by systemd-udevd/2681:
#0: (&dev->mutex){......}, at: [<ffffffff81ce8933>] __driver_attach+0xa3/0x160
#1: (&dev->mutex){......}, at: [<ffffffff81ce8941>] __driver_attach+0xb1/0x160
#2: (&dev->lock){+.+.+.}, at: [<ffffffffa0724def>] rc_register_device+0xb2f/0x1450 [rc_core]In this specific case, some error happened during device init, causing IR to be disabled.
Let's fix it by adding a var that will tell when the device is initialized. Any calls before that will return a -EINVAL.
That should prevent the race issues.
--- drivers/media/rc/rc-main.c | 45 ++++++++++++++++++++++++++------------------- include/media/rc-core.h | 2 ++ 2 files changed, 28 insertions(+), 19 deletions(-)
drivers/media/rc/rc-main.c +26 -19
@@ -723,12 +723,18 @@ int rc_open(struct rc_dev *rdev)return -EINVAL ;mutex_lock (&rdev->lock); +if (!rdev->initialized) { + rval = -EINVAL ; + goto unlock; + } +if (!rdev->users++ && rdev->open !=NULL ) rval = rdev->open (rdev);if (rval) rdev->users--; +unlock:mutex_unlock (&rdev->lock);return rval;
@@ -874,6 +880,10 @@ static ssize_t show_protocols(struct device *device,return -EINVAL ;mutex_lock (&dev->lock); +if (!dev->initialized) { +mutex_unlock (&dev->lock); +return -EINVAL ; + }if (fattr->type ==RC_FILTER_NORMAL ) { enabled = dev->enabled_protocols;
@@ -1074,6 +1084,10 @@ static ssize_t store_protocols(struct device *device, }mutex_lock (&dev->lock); +if (!dev->initialized) { + rc = -EINVAL ; + goto out; + } old_protocols = *current_protocols; new_protocols = old_protocols;
@@ -1154,12 +1168,17 @@ static ssize_t show_filter(struct device *device,if (!dev)return -EINVAL ; +mutex_lock (&dev->lock); +if (!dev->initialized) { +mutex_unlock (&dev->lock); +return -EINVAL ; + } +if (fattr->type ==RC_FILTER_NORMAL ) filter = &dev->scancode_filter;else filter = &dev->scancode_wakeup_filter; -mutex_lock (&dev->lock);if (fattr->mask) val = filter->mask;else
@@ -1222,6 +1241,10 @@ static ssize_t store_filter(struct device *device,return -EINVAL ;mutex_lock (&dev->lock); +if (!dev->initialized) { + ret = -EINVAL ; + goto unlock; + } new_filter = *filter;if (fattr->mask)
@@ -1419,14 +1442,6 @@ int rc_register_device(struct rc_dev *dev) dev->sysfs_groups[attr++] = &rc_dev_wakeup_protocol_attr_grp; dev->sysfs_groups[attr++] =NULL ; -/* -* Take the lock here, as the device sysfs node will appear -* when device_add() is called, which may trigger an ir-keytable udev -* rule, which will in turn call show_protocols and access -* dev->enabled_protocols before it has been initialized. -*/ -mutex_lock (&dev->lock); - rc =device_add (&dev->dev);if (rc) goto out_unlock;
@@ -1440,13 +1455,7 @@ int rc_register_device(struct rc_dev *dev) dev->input_dev->phys = dev->input_phys; dev->input_dev->name = dev->input_name; -/* input_register_device can call ir_open, so unlock mutex here */ -mutex_unlock (&dev->lock); - rc =input_register_device (dev->input_dev); - -mutex_lock (&dev->lock); -if (rc) goto out_table;
@@ -1475,10 +1484,7 @@ int rc_register_device(struct rc_dev *dev)request_module_nowait ("ir-lirc-codec" ); raw_init = true; } -/* calls ir_register_device so unlock mutex here*/ -mutex_unlock (&dev->lock); rc =ir_raw_event_register (dev); -mutex_lock (&dev->lock);if (rc <0 ) goto out_input; }
@@ -1491,6 +1497,8 @@ int rc_register_device(struct rc_dev *dev) dev->enabled_protocols = rc_type; } +mutex_lock (&dev->lock); + dev->initialized = true;mutex_unlock (&dev->lock); IR_dprintk(1, "Registered rc%u (driver: %s, remote: %s, mode %s)\n",
@@ -1512,7 +1520,6 @@ out_table: out_dev:device_del (&dev->dev); out_unlock: -mutex_unlock (&dev->lock);ida_simple_remove (&rc_ida, minor);return rc; }
include/media/rc-core.h +2 -0
@@ -60,6 +60,7 @@ enum rc_filter_type {/** * struct rc_dev - represents a remote control device * @dev: driver model's view of this device +* @initialized: true if the device init has completed * @sysfs_groups: sysfs attribute groups * @input_name: name of the input child device * @input_phys: physical path to the input child device
@@ -121,6 +122,7 @@ enum rc_filter_type {*/ struct rc_dev {struct device dev; +bool initialized;const struct attribute_group *sysfs_groups[5 ];const char *input_name;const char *input_phys;
Signature · 1 line
2.5.0
PATCH 4/11 allow overriding the driver name
In reply to Mauro Carvalho ChehabOn USB drivers, the dev struct is usually filled with the USB device. That would mean that the name of the driver specified by media_device.dev.driver.name would be "usb", instead of the name of the actual driver that created the media entity.
Add an optional field at the internal struct to allow drivers to override the driver name.
--- drivers/media/media-device.c | 6 +++++- include/media/media-device.h | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-)
drivers/media/media-device.c +5 -1
@@ -55,7 +55,11 @@ static int media_device_get_info(struct media_device *dev,memset (&info,0 ,sizeof (info)); -strlcpy (info.driver, dev->dev->driver->name,sizeof (info.driver)); +if (dev->driver_name[0 ]) +strlcpy (info.driver, dev->driver_name,sizeof (info.driver)); +else +strlcpy (info.driver, dev->dev->driver->name,sizeof (info.driver)); +strlcpy (info.model, dev->model,sizeof (info.model));strlcpy (info.serial, dev->serial,sizeof (info.serial));strlcpy (info.bus_info, dev->bus_info,sizeof (info.bus_info));
include/media/media-device.h +5 -0
@@ -268,6 +268,10 @@ struct device;* struct media_device - Media device * @dev: Parent device * @devnode: Media device node +* @driver_name: Optional device driver name. If not set, calls to +* %MEDIA_IOC_DEVICE_INFO will return dev->driver->name. +* This is needed for USB drivers for example, as otherwise +* they'll all appear as if the driver name was "usb". * @model: Device model name * @serial: Device serial number (optional) * @bus_info: Unique and stable device location identifier
@@ -303,6 +307,7 @@ struct media_device {struct media_devnode devnode;char model[32 ]; +char driver_name[32 ];char serial[40 ];char bus_info[32 ];u32 hw_revision;
Signature · 1 line
2.5.0
PATCH 5/11 use v4l2_mc_usb_media_device_init() on most USB devices
In reply to Mauro Carvalho ChehabExcept for the usbuvc driver (with has an embedded media_device struct on it), the other drivers have a pointer to media_device.
On those drivers, replace their own implementation for the core one. That warrants that those subdev drivers will fill the media_device info the same way.
--- drivers/media/usb/au0828/au0828-core.c | 19 ++++--------------- drivers/media/usb/cx231xx/cx231xx-cards.c | 12 +----------- drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 13 ++----------- drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 15 ++------------- drivers/media/usb/em28xx/em28xx-cards.c | 23 ++++++++--------------- drivers/media/usb/siano/smsusb.c | 1 + 6 files changed, 18 insertions(+), 65 deletions(-)
drivers/media/usb/au0828/au0828-core.c +4 -15
@@ -191,23 +191,12 @@ static int au0828_media_device_init(struct au0828_dev *dev,#ifdef CONFIG_MEDIA_CONTROLLER struct media_device *mdev; - mdev =kzalloc (sizeof (*mdev),GFP_KERNEL ); -if (!mdev) -return -ENOMEM ; - - mdev->dev = &udev->dev; -if (!dev->board.name) -strlcpy (mdev->model,"unknown au0828" ,sizeof (mdev->model)); + mdev =v4l2_mc_usb_media_device_init (udev,"unknown au0828" );else -strlcpy (mdev->model, dev->board.name,sizeof (mdev->model)); -if (udev->serial) -strlcpy (mdev->serial, udev->serial,sizeof (mdev->serial)); -strcpy (mdev->bus_info, udev->devpath); - mdev->hw_revision =le16_to_cpu (udev->descriptor.bcdDevice); - mdev->driver_version =LINUX_VERSION_CODE ; - -media_device_init (mdev); + mdev =v4l2_mc_usb_media_device_init (udev, dev->board.name); +if (!mdev) +return -ENOMEM ; dev->media_dev = mdev;#endif
drivers/media/usb/cx231xx/cx231xx-cards.c +1 -11
@@ -1212,20 +1212,10 @@ static int cx231xx_media_device_init(struct cx231xx *dev,#ifdef CONFIG_MEDIA_CONTROLLER struct media_device *mdev; - mdev =kzalloc (sizeof (*mdev),GFP_KERNEL ); + mdev =v4l2_mc_usb_media_device_init (udev, dev->board.name);if (!mdev)return -ENOMEM ; - mdev->dev = dev->dev; -strlcpy (mdev->model, dev->board.name,sizeof (mdev->model)); -if (udev->serial) -strlcpy (mdev->serial, udev->serial,sizeof (mdev->serial)); -strcpy (mdev->bus_info, udev->devpath); - mdev->hw_revision =le16_to_cpu (udev->descriptor.bcdDevice); - mdev->driver_version =LINUX_VERSION_CODE ; - -media_device_init (mdev); - dev->media_dev = mdev;#endif return 0 ;
drivers/media/usb/dvb-usb-v2/dvb_usb_core.c +2 -11
@@ -20,6 +20,7 @@*/ #include "dvb_usb_common.h" +#include <media/v4l2-mc.h> static int dvb_usbv2_disable_rc_polling;module_param_named (disable_rc_polling, dvb_usbv2_disable_rc_polling, int,0644 );
@@ -407,20 +408,10 @@ static int dvb_usbv2_media_device_init(struct dvb_usb_adapter *adap)struct dvb_usb_device *d =adap_to_d (adap);struct usb_device *udev = d->udev; - mdev =kzalloc (sizeof (*mdev),GFP_KERNEL ); + mdev =v4l2_mc_usb_media_device_init (udev, d->name);if (!mdev)return -ENOMEM ; - mdev->dev = &udev->dev; -strlcpy (mdev->model, d->name,sizeof (mdev->model)); -if (udev->serial) -strlcpy (mdev->serial, udev->serial,sizeof (mdev->serial)); -strcpy (mdev->bus_info, udev->devpath); - mdev->hw_revision =le16_to_cpu (udev->descriptor.bcdDevice); - mdev->driver_version =LINUX_VERSION_CODE ; - -media_device_init (mdev); -dvb_register_media_controller (&adap->dvb_adap, mdev);dev_info (&d->udev->dev,"media controller created\n" );
drivers/media/usb/dvb-usb/dvb-usb-dvb.c +2 -13
@@ -7,6 +7,7 @@* linux-dvb API. */ #include "dvb-usb-common.h" +#include <media/v4l2-mc.h> /* does the complete input transfer handling */ static int dvb_usb_ctrl_feed (struct dvb_demux_feed *dvbdmxfeed,int onoff)
@@ -102,19 +103,7 @@ static int dvb_usb_media_device_init(struct dvb_usb_adapter *adap)struct dvb_usb_device *d = adap->dev;struct usb_device *udev = d->udev; - mdev =kzalloc (sizeof (*mdev),GFP_KERNEL ); -if (!mdev) -return -ENOMEM ; - - mdev->dev = &udev->dev; -strlcpy (mdev->model, d->desc->name,sizeof (mdev->model)); -if (udev->serial) -strlcpy (mdev->serial, udev->serial,sizeof (mdev->serial)); -strcpy (mdev->bus_info, udev->devpath); - mdev->hw_revision =le16_to_cpu (udev->descriptor.bcdDevice); - mdev->driver_version =LINUX_VERSION_CODE ; - -media_device_init (mdev); + mdev =v4l2_mc_usb_media_device_init (udev, d->desc->name);dvb_register_media_controller (&adap->dvb_adap, mdev);
drivers/media/usb/em28xx/em28xx-cards.c +8 -15
@@ -3019,24 +3019,17 @@ static int em28xx_media_device_init(struct em28xx *dev,#ifdef CONFIG_MEDIA_CONTROLLER struct media_device *mdev; - mdev =kzalloc (sizeof (*mdev),GFP_KERNEL ); +if (udev->product) { + mdev =v4l2_mc_usb_media_device_init (udev, udev->product); + }else if (udev->manufacturer) { + mdev =v4l2_mc_usb_media_device_init (udev, udev->manufacturer); + }else { + mdev =v4l2_mc_usb_media_device_init (udev, dev->name); + } +if (!mdev)return -ENOMEM ; - mdev->dev = &udev->dev; - -if (!dev->name) -strlcpy (mdev->model,"unknown em28xx" ,sizeof (mdev->model)); -else -strlcpy (mdev->model, dev->name,sizeof (mdev->model)); -if (udev->serial) -strlcpy (mdev->serial, udev->serial,sizeof (mdev->serial)); -strcpy (mdev->bus_info, udev->devpath); - mdev->hw_revision =le16_to_cpu (udev->descriptor.bcdDevice); - mdev->driver_version =LINUX_VERSION_CODE ; - -media_device_init (mdev); - dev->media_dev = mdev;#endif return 0 ;
drivers/media/usb/siano/smsusb.c +1 -0
@@ -27,6 +27,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.#include <linux/firmware.h> #include <linux/slab.h> #include <linux/module.h> +#include <media/v4l2-mc.h> #include "sms-cards.h" #include "smsendian.h"
Signature · 1 line
2.5.0
PATCH 6/11 v4l2-mc: use usb_make_path() to provide bus info
In reply to Mauro Carvalho ChehabReport the bus info on the same way as VIDIOC_QUERYCAP. Also, currently, it is reporting just PORT/DEV.
--- drivers/media/v4l2-core/v4l2-mc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/v4l2-core/v4l2-mc.c +1 -1
@@ -79,7 +79,7 @@ struct media_device * __v4l2_mc_usb_media_device_init(struct usb_device *udev,strlcpy (mdev->model,"unknown model" ,sizeof (mdev->model));if (udev->serial)strlcpy (mdev->serial, udev->serial,sizeof (mdev->serial)); -strcpy (mdev->bus_info, udev->devpath); +usb_make_path (udev, mdev->bus_info,sizeof (mdev->bus_info)); mdev->hw_revision =le16_to_cpu (udev->descriptor.bcdDevice); mdev->driver_version =LINUX_VERSION_CODE ;
Signature · 1 line
2.5.0
Re: [PATCH 06/11] [media] v4l2-mc: use usb_make_path() to provide bus info
Quoted 14 lines · Mauro Carvalho Chehab
Report the bus info on the same way as
VIDIOC_QUERYCAP. Also, currently, it is reporting just PORT/DEV.Signed-off-by Mauro Carvalho Chehab <[email protected]>--- drivers/media/v4l2-core/v4l2-mc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/media/v4l2-core/v4l2-mc.c +0
-0
drivers/media/v4l2-core/v4l2-mc.c struct media_device * __v4l2_mc_usb_media_device_init(struct usb_device *udev, · line 85
Quoted 8 lines
drivers/media/v4l2-core/v4l2-mc.c +1
-1@@ -79,7 +79,7 @@ struct media_device * __v4l2_mc_usb_media_device_init(struct usb_device *udev,strlcpy (mdev->model,"unknown model" ,sizeof (mdev->model));if (udev->serial)strlcpy (mdev->serial, udev->serial,sizeof (mdev->serial)); -strcpy (mdev->bus_info, udev->devpath); +usb_make_path (udev, mdev->bus_info,sizeof (mdev->bus_info)); mdev->hw_revision =le16_to_cpu (udev->descriptor.bcdDevice); mdev->driver_version =LINUX_VERSION_CODE ;
Please fold this in patch 02/11 and you can disregard my comment about strcpy vs strlcpy.
Regards,
Hans
PATCH 7/11 em28xx-dvb: create RF connector on DVB-only mode
In reply to Mauro Carvalho ChehabWhen in analog mode, the RF connector will be created by em28xx-video. However, when the device is in digital mode only, the RF connector is not shown. In this case, let the DVB core to create it for us.
--- drivers/media/usb/em28xx/em28xx-dvb.c | 7 ++++++- drivers/media/usb/em28xx/em28xx-video.c | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-)
drivers/media/usb/em28xx/em28xx-dvb.c +6 -1
@@ -905,6 +905,7 @@ static int em28xx_register_dvb(struct em28xx_dvb *dvb, struct module *module, struct em28xx *dev, struct device *device) { int result; +bool create_rf_connector = false;mutex_init (&dvb->lock);
@@ -998,7 +999,11 @@ static int em28xx_register_dvb(struct em28xx_dvb *dvb, struct module *module,/* register network adapter */ dvb_net_init (&dvb->adapter, &dvb->net, &dvb->demux.dmx); - result =dvb_create_media_graph (&dvb->adapter, false); +/* If the analog part won't create RF connectors, DVB will do it */ +if (!dev->has_video || (dev->tuner_type ==TUNER_ABSENT )) + create_rf_connector = true; + + result =dvb_create_media_graph (&dvb->adapter, create_rf_connector);if (result <0 ) goto fail_create_graph;
drivers/media/usb/em28xx/em28xx-video.c +2 -1
@@ -990,7 +990,8 @@ static void em28xx_v4l2_create_entities(struct em28xx *dev) ent->function =MEDIA_ENT_F_CONN_SVIDEO ;break ;default :/* EM28XX_VMUX_TELEVISION or EM28XX_RADIO */ - ent->function =MEDIA_ENT_F_CONN_RF ; +if (dev->tuner_type !=TUNER_ABSENT ) + ent->function =MEDIA_ENT_F_CONN_RF ;break ; }
Signature · 1 line
2.5.0
PATCH 8/11 cx231xx: use v4l2 core function to create the MC graph
In reply to Mauro Carvalho ChehabInstead of having its own routine, use the one defined at the core, as it is generic enough to handle the cx231xx usecases.
--- drivers/media/usb/cx231xx/cx231xx-cards.c | 47 +------------------------------ 1 file changed, 1 insertion(+), 46 deletions(-)
drivers/media/usb/cx231xx/cx231xx-cards.c +1 -46
@@ -1221,51 +1221,6 @@ static int cx231xx_media_device_init(struct cx231xx *dev,return 0 ; } -static int cx231xx_create_media_graph (struct cx231xx *dev) -{ -#ifdef CONFIG_MEDIA_CONTROLLER -struct media_device *mdev = dev->media_dev; -struct media_entity *entity; -struct media_entity *tuner =NULL , *decoder =NULL ; -int ret; - -if (!mdev) -return 0 ; - -media_device_for_each_entity (entity, mdev) { -switch (entity->function) { -case MEDIA_ENT_F_TUNER : - tuner = entity; -break ; -case MEDIA_ENT_F_ATV_DECODER : - decoder = entity; -break ; - } - } - -/* Analog setup, using tuner as a link */ - -if (!decoder) -return 0 ; - -if (tuner) { - ret =media_create_pad_link (tuner,TUNER_PAD_OUTPUT , decoder,0 , -MEDIA_LNK_FL_ENABLED ); -if (ret <0 ) -return ret; - } - ret =media_create_pad_link (decoder,1 , &dev->vdev.entity,0 , -MEDIA_LNK_FL_ENABLED ); -if (ret <0 ) -return ret; - ret =media_create_pad_link (decoder,2 , &dev->vbi_dev.entity,0 , -MEDIA_LNK_FL_ENABLED ); -if (ret <0 ) -return ret; -#endif -return 0 ; -} -/* * cx231xx_init_dev() * allocates and inits the device structs, registers i2c bus and v4l device
@@ -1729,7 +1684,7 @@ static int cx231xx_usb_probe(struct usb_interface *interface,/* load other modules required */ request_modules (dev); - retval =cx231xx_create_media_graph (dev); + retval =v4l2_mc_create_media_graph (dev->media_dev);if (retval <0 ) goto done;
Signature · 1 line
2.5.0
kbuild test robot
build botRe: [PATCH 08/11] [media] cx231xx: use v4l2 core function to create the MC graph
reproduce:
Hi Mauro,
[auto build test ERROR on linuxtv-media/master]
[cannot apply to v4.5-rc3 next-20160212]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Some-improvements-fixes/20160212-174848 base: git://linuxtv.org/media_tree.git master config: x86_64-rhel (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/media/usb/cx231xx/cx231xx-cards.c: In function 'cx231xx_usb_probe':
>> drivers/media/usb/cx231xx/cx231xx-cards.c:1687:41: error: 'struct cx231xx' has no member named 'media_dev'
retval = v4l2_mc_create_media_graph(dev->media_dev);
^vim +1687 drivers/media/usb/cx231xx/cx231xx-cards.c
1681 if (dev->model == CX231XX_BOARD_CNXT_RDE_253S)
1682 cx231xx_sleep_s5h1432(dev);
1683
1684 /* load other modules required */
1685 request_modules(dev);
1686
> 1687 retval = v4l2_mc_create_media_graph(dev->media_dev);
1688 if (retval < 0)
1689 goto done;
1690
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
PATCH 9/11 si2157: register as a tuner entity
In reply to Mauro Carvalho ChehabAs this tuner doesn't use the usual subdev interface, we need to register it manually.
--- drivers/media/tuners/si2157.c | 32 +++++++++++++++++++++++++++++++- drivers/media/tuners/si2157.h | 5 +++++ drivers/media/tuners/si2157_priv.h | 8 ++++++++ 3 files changed, 44 insertions(+), 1 deletion(-)
drivers/media/tuners/si2157.c +31 -1
@@ -403,7 +403,7 @@ err: }static int si2157_probe (struct i2c_client *client, -const struct i2c_device_id *id) +const struct i2c_device_id *id) {struct si2157_config *cfg = client->dev.platform_data;struct dvb_frontend *fe = cfg->fe;
@@ -438,6 +438,31 @@ static int si2157_probe(struct i2c_client *client,memcpy (&fe->ops.tuner_ops, &si2157_ops,sizeof (struct dvb_tuner_ops )); fe->tuner_priv = client; +#ifdef CONFIG_MEDIA_CONTROLLER +if (cfg->mdev) { + dev->mdev = cfg->mdev; + + dev->ent.name =KBUILD_MODNAME ; + dev->ent.function =MEDIA_ENT_F_TUNER ; + + dev->pad[TUNER_PAD_RF_INPUT ].flags =MEDIA_PAD_FL_SINK ; + dev->pad[TUNER_PAD_OUTPUT ].flags =MEDIA_PAD_FL_SOURCE ; + dev->pad[TUNER_PAD_AUD_OUT ].flags =MEDIA_PAD_FL_SOURCE ; + + ret =media_entity_pads_init (&dev->ent,TUNER_NUM_PADS , + &dev->pad[0 ]); + +if (ret) + goto err_kfree; + + ret =media_device_register_entity (cfg->mdev, &dev->ent); +if (ret) { +media_entity_cleanup (&dev->ent); + goto err_kfree; + } + } +#endif +dev_info (&client->dev,"Silicon Labs %s successfully attached\n" , dev->chiptype ==SI2157_CHIPTYPE_SI2146 ?"Si2146" :"Si2147/2148/2157/2158" );
@@ -461,6 +486,11 @@ static int si2157_remove(struct i2c_client *client)/* stop statistics polling */ cancel_delayed_work_sync (&dev->stat_work); +#ifdef CONFIG_MEDIA_CONTROLLER_DVB +if (dev->mdev) +media_device_unregister_entity (&dev->ent); +#endif +memset (&fe->ops.tuner_ops,0 ,sizeof (struct dvb_tuner_ops )); fe->tuner_priv =NULL ;kfree (dev);
drivers/media/tuners/si2157.h +5 -0
@@ -18,6 +18,7 @@#define SI2157_H #include <linux/kconfig.h> +#include <media/media-device.h> #include "dvb_frontend.h" /*
@@ -30,6 +31,10 @@ struct si2157_config {*/ struct dvb_frontend *fe; +#if defined(CONFIG_MEDIA_CONTROLLER ) +struct media_device *mdev; +#endif +/* * Spectral Inversion */
drivers/media/tuners/si2157_priv.h +8 -0
@@ -18,6 +18,7 @@#define SI2157_PRIV_H #include <linux/firmware.h> +#include <media/v4l2-mc.h> #include "si2157.h" /* state struct */
@@ -31,6 +32,13 @@ struct si2157_dev {u8 if_port;u32 if_frequency;struct delayed_work stat_work; + +#if defined(CONFIG_MEDIA_CONTROLLER ) +struct media_device *mdev; +struct media_entity ent; +struct media_pad pad[TUNER_NUM_PADS ]; +#endif + };#define SI2157_CHIPTYPE_SI2157 0
Signature · 1 line
2.5.0
PATCH 10/11 cx231xx, em28xx: pass media_device to si2157
In reply to Mauro Carvalho ChehabAs si2157 doesn't use the subdev, but has instead a binding logic that doesn't have any core framework, we need to manually pass the media_device struct via platform data on every place it is called.
This fixes support for HVR-955Q when MC is enabled.
--- drivers/media/usb/cx231xx/cx231xx-dvb.c | 6 ++++++ drivers/media/usb/em28xx/em28xx-dvb.c | 6 ++++++ 2 files changed, 12 insertions(+)
drivers/media/usb/cx231xx/cx231xx-dvb.c +6 -0
@@ -801,6 +801,9 @@ static int dvb_init(struct cx231xx *dev)/* attach tuner */ memset (&si2157_config,0 ,sizeof (si2157_config)); si2157_config.fe = dev->dvb->frontend; +#ifdef CONFIG_MEDIA_CONTROLLER + si2157_config.mdev = dev->media_dev; +#endif si2157_config.if_port =1 ; si2157_config.inversion = true;strlcpy (info.type,"si2157" ,I2C_NAME_SIZE );
@@ -857,6 +860,9 @@ static int dvb_init(struct cx231xx *dev)/* attach tuner */ memset (&si2157_config,0 ,sizeof (si2157_config)); si2157_config.fe = dev->dvb->frontend; +#ifdef CONFIG_MEDIA_CONTROLLER + si2157_config.mdev = dev->media_dev; +#endif si2157_config.if_port =1 ; si2157_config.inversion = true;strlcpy (info.type,"si2157" ,I2C_NAME_SIZE );
drivers/media/usb/em28xx/em28xx-dvb.c +6 -0
@@ -1671,6 +1671,9 @@ static int em28xx_dvb_init(struct em28xx *dev)memset (&si2157_config,0 ,sizeof (si2157_config)); si2157_config.fe = dvb->fe[0 ]; si2157_config.if_port =1 ; +#ifdef CONFIG_MEDIA_CONTROLLER + si2157_config.mdev = dev->media_dev; +#endif memset (&info,0 ,sizeof (struct i2c_board_info ));strlcpy (info.type,"si2157" ,I2C_NAME_SIZE ); info.addr =0x60 ;
@@ -1732,6 +1735,9 @@ static int em28xx_dvb_init(struct em28xx *dev)memset (&si2157_config,0 ,sizeof (si2157_config)); si2157_config.fe = dvb->fe[0 ]; si2157_config.if_port =0 ; +#ifdef CONFIG_MEDIA_CONTROLLER + si2157_config.mdev = dev->media_dev; +#endif memset (&info,0 ,sizeof (struct i2c_board_info ));strlcpy (info.type,"si2146" ,I2C_NAME_SIZE ); info.addr =0x60 ;
Signature · 1 line
2.5.0
PATCH 11/11 cx231xx: create connectors at the media graph
In reply to Mauro Carvalho ChehabWe need to add connectors to the cx231xx graph.
--- drivers/media/usb/cx231xx/cx231xx-cards.c | 3 ++ drivers/media/usb/cx231xx/cx231xx-dvb.c | 4 ++- drivers/media/usb/cx231xx/cx231xx-video.c | 46 +++++++++++++++++++++++++++++++ drivers/media/usb/cx231xx/cx231xx.h | 3 ++ 4 files changed, 55 insertions(+), 1 deletion(-)
drivers/media/usb/cx231xx/cx231xx-cards.c +3 -0
@@ -1684,6 +1684,9 @@ static int cx231xx_usb_probe(struct usb_interface *interface,/* load other modules required */ request_modules (dev); +/* Init entities at the Media Controller */ +cx231xx_v4l2_create_entities (dev); + retval =v4l2_mc_create_media_graph (dev->media_dev);if (retval <0 ) goto done;
drivers/media/usb/cx231xx/cx231xx-dvb.c +3 -1
@@ -25,6 +25,7 @@#include <media/v4l2-common.h> #include <media/videobuf-vmalloc.h> +#include <media/tuner.h> #include "xc5000.h" #include "s5h1432.h"
@@ -551,7 +552,8 @@ static int register_dvb(struct cx231xx_dvb *dvb,/* register network adapter */ dvb_net_init (&dvb->adapter, &dvb->net, &dvb->demux.dmx); - result =dvb_create_media_graph (&dvb->adapter, false); + result =dvb_create_media_graph (&dvb->adapter, + dev->tuner_type ==TUNER_ABSENT );if (result <0 ) goto fail_create_graph;
drivers/media/usb/cx231xx/cx231xx-video.c +46 -0
52 lines not on this page · raw message
drivers/media/usb/cx231xx/cx231xx.h +3 -0
@@ -663,6 +663,8 @@ struct cx231xx {#if defined(CONFIG_MEDIA_CONTROLLER )struct media_device *media_dev;struct media_pad video_pad, vbi_pad; +struct media_entity input_ent[MAX_CX231XX_INPUT ]; +struct media_pad input_pad[MAX_CX231XX_INPUT ];#endif unsigned char eedata[256 ];
@@ -943,6 +945,7 @@ int cx231xx_register_extension(struct cx231xx_ops *dev);void cx231xx_unregister_extension (struct cx231xx_ops *dev);void cx231xx_init_extension (struct cx231xx *dev);void cx231xx_close_extension (struct cx231xx *dev); +void cx231xx_v4l2_create_entities (struct cx231xx *dev);int cx231xx_querycap (struct file *file,void *priv,struct v4l2_capability *cap);int cx231xx_g_tuner (struct file *file,void *priv,struct v4l2_tuner *t);
Signature · 1 line
2.5.0