* [PATCH 1/9] em28xx: Hauppauge DualHD second tuner functionality
2018-01-05 0:04 [PATCH 0/9] Hauppauge em28xx/lgdt3306a soloHD/DualHD support Brad Love
@ 2018-01-05 0:04 ` Brad Love
2018-01-05 0:21 ` Michael Ira Krufky
2018-01-05 0:04 ` [PATCH 2/9] em28xx: Bulk transfer implementation fix Brad Love
` (7 subsequent siblings)
8 siblings, 1 reply; 32+ messages in thread
From: Brad Love @ 2018-01-05 0:04 UTC (permalink / raw)
To: linux-media; +Cc: Brad Love
Implement use of secondary TS port on em28xx.
Adds has_dual_ts field, allows secondary demod/tuner to be
added to a single em28xx device.
Hauppauge DualHD models are configured to use this feature.
Signed-off-by: Brad Love <brad@nextdimension.cc>
---
drivers/media/usb/em28xx/em28xx-cards.c | 126 +++++++++++++++++++++++++++++++-
drivers/media/usb/em28xx/em28xx-core.c | 42 +++++++++--
drivers/media/usb/em28xx/em28xx-dvb.c | 33 +++++++--
drivers/media/usb/em28xx/em28xx.h | 12 +++
4 files changed, 196 insertions(+), 17 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 34e16f6a..7f5d0b28 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2402,6 +2402,7 @@ struct em28xx_board em28xx_boards[] = {
.tuner_type = TUNER_ABSENT,
.tuner_gpio = hauppauge_dualhd_dvb,
.has_dvb = 1,
+ .has_dual_ts = 1,
.ir_codes = RC_MAP_HAUPPAUGE,
.leds = hauppauge_dualhd_leds,
},
@@ -2417,6 +2418,7 @@ struct em28xx_board em28xx_boards[] = {
.tuner_type = TUNER_ABSENT,
.tuner_gpio = hauppauge_dualhd_dvb,
.has_dvb = 1,
+ .has_dual_ts = 1,
.ir_codes = RC_MAP_HAUPPAUGE,
.leds = hauppauge_dualhd_leds,
},
@@ -3239,7 +3241,8 @@ static void em28xx_release_resources(struct em28xx *dev)
em28xx_i2c_unregister(dev, 1);
em28xx_i2c_unregister(dev, 0);
- usb_put_dev(udev);
+ if (dev->ts == PRIMARY_TS)
+ usb_put_dev(udev);
/* Mark device as unused */
clear_bit(dev->devno, em28xx_devused);
@@ -3432,6 +3435,35 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
return 0;
}
+int em28xx_duplicate_dev(struct em28xx *dev)
+{
+ int nr;
+ struct em28xx *sec_dev = kzalloc(sizeof(*sec_dev), GFP_KERNEL);
+
+ if (sec_dev == NULL) {
+ dev->dev_next = NULL;
+ return -ENOMEM;
+ }
+ memcpy(sec_dev, dev, sizeof(sizeof(*sec_dev)));
+ /* Check to see next free device and mark as used */
+ do {
+ nr = find_first_zero_bit(em28xx_devused, EM28XX_MAXBOARDS);
+ if (nr >= EM28XX_MAXBOARDS) {
+ /* No free device slots */
+ dev_warn(&dev->intf->dev, ": Supports only %i em28xx boards.\n",
+ EM28XX_MAXBOARDS);
+ kfree(sec_dev);
+ dev->dev_next = NULL;
+ return -ENOMEM;
+ }
+ } while (test_and_set_bit(nr, em28xx_devused));
+ sec_dev->devno = nr;
+ snprintf(sec_dev->name, 28, "em28xx #%d", nr);
+ sec_dev->dev_next = NULL;
+ dev->dev_next = sec_dev;
+ return 0;
+}
+
/* high bandwidth multiplier, as encoded in highspeed endpoint descriptors */
#define hb_mult(wMaxPacketSize) (1 + (((wMaxPacketSize) >> 11) & 0x03))
@@ -3551,6 +3583,17 @@ static int em28xx_usb_probe(struct usb_interface *interface,
}
}
break;
+ case 0x85:
+ if (usb_endpoint_xfer_isoc(e)) {
+ if (size > dev->dvb_max_pkt_size_isoc_ts2) {
+ dev->dvb_ep_isoc_ts2 = e->bEndpointAddress;
+ dev->dvb_max_pkt_size_isoc_ts2 = size;
+ dev->dvb_alt_isoc = i;
+ }
+ } else {
+ dev->dvb_ep_bulk_ts2 = e->bEndpointAddress;
+ }
+ break;
}
}
/* NOTE:
@@ -3565,6 +3608,8 @@ static int em28xx_usb_probe(struct usb_interface *interface,
* 0x83 isoc* => audio
* 0x84 isoc => digital
* 0x84 bulk => analog or digital**
+ * 0x85 isoc => digital TS2
+ * 0x85 bulk => digital TS2
* (*: audio should always be isoc)
* (**: analog, if ep 0x82 is isoc, otherwise digital)
*
@@ -3632,6 +3677,10 @@ static int em28xx_usb_probe(struct usb_interface *interface,
dev->has_video = has_video;
dev->ifnum = ifnum;
+ dev->ts = PRIMARY_TS;
+ snprintf(dev->name, 28, "em28xx");
+ dev->dev_next = NULL;
+
if (has_vendor_audio) {
dev_err(&interface->dev,
"Audio interface %i found (Vendor Class)\n", ifnum);
@@ -3711,6 +3760,65 @@ static int em28xx_usb_probe(struct usb_interface *interface,
dev->dvb_xfer_bulk ? "bulk" : "isoc");
}
+ if (dev->board.has_dual_ts && em28xx_duplicate_dev(dev) == 0) {
+ dev->dev_next->ts = SECONDARY_TS;
+ dev->dev_next->alt = -1;
+ dev->dev_next->is_audio_only = has_vendor_audio &&
+ !(has_video || has_dvb);
+ dev->dev_next->has_video = false;
+ dev->dev_next->ifnum = ifnum;
+ dev->dev_next->model = id->driver_info;
+
+ mutex_init(&dev->dev_next->lock);
+ retval = em28xx_init_dev(dev->dev_next, udev, interface,
+ dev->dev_next->devno);
+ if (retval)
+ goto err_free;
+
+ dev->dev_next->board.ir_codes = NULL; /* No IR for 2nd tuner */
+ dev->dev_next->board.has_ir_i2c = 0; /* No IR for 2nd tuner */
+
+ if (usb_xfer_mode < 0) {
+ if (dev->dev_next->board.is_webcam)
+ try_bulk = 1;
+ else
+ try_bulk = 0;
+ } else {
+ try_bulk = usb_xfer_mode > 0;
+ }
+
+ /* Select USB transfer types to use */
+ if (has_dvb) {
+ if (!dev->dvb_ep_isoc_ts2 ||
+ (try_bulk && dev->dvb_ep_bulk_ts2))
+ dev->dev_next->dvb_xfer_bulk = 1;
+ dev_info(&dev->intf->dev, "dvb ts2 set to %s mode.\n",
+ dev->dev_next->dvb_xfer_bulk ? "bulk" : "isoc");
+ }
+
+ dev->dev_next->dvb_ep_isoc = dev->dvb_ep_isoc_ts2;
+ dev->dev_next->dvb_ep_bulk = dev->dvb_ep_bulk_ts2;
+ dev->dev_next->dvb_max_pkt_size_isoc = dev->dvb_max_pkt_size_isoc_ts2;
+ dev->dev_next->dvb_alt_isoc = dev->dvb_alt_isoc;
+
+ /* Configuare hardware to support TS2*/
+ if (dev->dvb_xfer_bulk) {
+ /* The ep4 and ep5 are configuared for BULK */
+ em28xx_write_reg(dev, 0x0b, 0x96);
+ mdelay(100);
+ em28xx_write_reg(dev, 0x0b, 0x80);
+ mdelay(100);
+ } else {
+ /* The ep4 and ep5 are configuared for ISO */
+ em28xx_write_reg(dev, 0x0b, 0x96);
+ mdelay(100);
+ em28xx_write_reg(dev, 0x0b, 0x82);
+ mdelay(100);
+ }
+
+ kref_init(&dev->dev_next->ref);
+ }
+
kref_init(&dev->ref);
request_modules(dev);
@@ -3753,15 +3861,29 @@ static void em28xx_usb_disconnect(struct usb_interface *interface)
if (!dev)
return;
+ if (dev->dev_next != NULL) {
+ dev->dev_next->disconnected = 1;
+ dev_info(&dev->intf->dev, "Disconnecting %s\n",
+ dev->dev_next->name);
+ flush_request_modules(dev->dev_next);
+ }
+
dev->disconnected = 1;
- dev_err(&dev->intf->dev, "Disconnecting\n");
+ dev_err(&dev->intf->dev, "Disconnecting %s\n", dev->name);
flush_request_modules(dev);
em28xx_close_extension(dev);
+ if (dev->dev_next != NULL)
+ em28xx_release_resources(dev->dev_next);
em28xx_release_resources(dev);
+
+ if (dev->dev_next != NULL) {
+ kref_put(&dev->dev_next->ref, em28xx_free_device);
+ dev->dev_next = NULL;
+ }
kref_put(&dev->ref, em28xx_free_device);
}
diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
index 1d0d8cc..ef38e56 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -638,10 +638,18 @@ int em28xx_capture_start(struct em28xx *dev, int start)
dev->chip_id == CHIP_ID_EM28174 ||
dev->chip_id == CHIP_ID_EM28178) {
/* The Transport Stream Enable Register moved in em2874 */
- rc = em28xx_write_reg_bits(dev, EM2874_R5F_TS_ENABLE,
- start ?
- EM2874_TS1_CAPTURE_ENABLE : 0x00,
- EM2874_TS1_CAPTURE_ENABLE);
+ if (dev->ts == PRIMARY_TS)
+ rc = em28xx_write_reg_bits(dev,
+ EM2874_R5F_TS_ENABLE,
+ start ?
+ EM2874_TS1_CAPTURE_ENABLE : 0x00,
+ EM2874_TS1_CAPTURE_ENABLE);
+ else
+ rc = em28xx_write_reg_bits(dev,
+ EM2874_R5F_TS_ENABLE,
+ start ?
+ EM2874_TS2_CAPTURE_ENABLE : 0x00,
+ EM2874_TS2_CAPTURE_ENABLE);
} else {
/* FIXME: which is the best order? */
/* video registers are sampled by VREF */
@@ -1077,7 +1085,11 @@ int em28xx_register_extension(struct em28xx_ops *ops)
mutex_lock(&em28xx_devlist_mutex);
list_add_tail(&ops->next, &em28xx_extension_devlist);
list_for_each_entry(dev, &em28xx_devlist, devlist) {
- ops->init(dev);
+ if (ops->init) {
+ ops->init(dev);
+ if (dev->dev_next != NULL)
+ ops->init(dev->dev_next);
+ }
}
mutex_unlock(&em28xx_devlist_mutex);
pr_info("em28xx: Registered (%s) extension\n", ops->name);
@@ -1091,7 +1103,11 @@ void em28xx_unregister_extension(struct em28xx_ops *ops)
mutex_lock(&em28xx_devlist_mutex);
list_for_each_entry(dev, &em28xx_devlist, devlist) {
- ops->fini(dev);
+ if (ops->fini) {
+ if (dev->dev_next != NULL)
+ ops->fini(dev->dev_next);
+ ops->fini(dev);
+ }
}
list_del(&ops->next);
mutex_unlock(&em28xx_devlist_mutex);
@@ -1106,8 +1122,11 @@ void em28xx_init_extension(struct em28xx *dev)
mutex_lock(&em28xx_devlist_mutex);
list_add_tail(&dev->devlist, &em28xx_devlist);
list_for_each_entry(ops, &em28xx_extension_devlist, next) {
- if (ops->init)
+ if (ops->init) {
ops->init(dev);
+ if (dev->dev_next != NULL)
+ ops->init(dev->dev_next);
+ }
}
mutex_unlock(&em28xx_devlist_mutex);
}
@@ -1118,8 +1137,11 @@ void em28xx_close_extension(struct em28xx *dev)
mutex_lock(&em28xx_devlist_mutex);
list_for_each_entry(ops, &em28xx_extension_devlist, next) {
- if (ops->fini)
+ if (ops->fini) {
+ if (dev->dev_next != NULL)
+ ops->fini(dev->dev_next);
ops->fini(dev);
+ }
}
list_del(&dev->devlist);
mutex_unlock(&em28xx_devlist_mutex);
@@ -1134,6 +1156,8 @@ int em28xx_suspend_extension(struct em28xx *dev)
list_for_each_entry(ops, &em28xx_extension_devlist, next) {
if (ops->suspend)
ops->suspend(dev);
+ if (dev->dev_next != NULL)
+ ops->suspend(dev->dev_next);
}
mutex_unlock(&em28xx_devlist_mutex);
return 0;
@@ -1148,6 +1172,8 @@ int em28xx_resume_extension(struct em28xx *dev)
list_for_each_entry(ops, &em28xx_extension_devlist, next) {
if (ops->resume)
ops->resume(dev);
+ if (dev->dev_next != NULL)
+ ops->resume(dev->dev_next);
}
mutex_unlock(&em28xx_devlist_mutex);
return 0;
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
index 8a81c94..9bc9576 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -199,7 +199,6 @@ static int em28xx_start_streaming(struct em28xx_dvb *dvb)
int rc;
struct em28xx_i2c_bus *i2c_bus = dvb->adapter.priv;
struct em28xx *dev = i2c_bus->dev;
- struct usb_device *udev = interface_to_usbdev(dev->intf);
int dvb_max_packet_size, packet_multiplier, dvb_alt;
if (dev->dvb_xfer_bulk) {
@@ -218,7 +217,6 @@ static int em28xx_start_streaming(struct em28xx_dvb *dvb)
dvb_alt = dev->dvb_alt_isoc;
}
- usb_set_interface(udev, dev->ifnum, dvb_alt);
rc = em28xx_set_mode(dev, EM28XX_DIGITAL_MODE);
if (rc < 0)
return rc;
@@ -1128,8 +1126,9 @@ static void em28xx_unregister_dvb(struct em28xx_dvb *dvb)
static int em28xx_dvb_init(struct em28xx *dev)
{
- int result = 0;
+ int result = 0, dvb_alt = 0;
struct em28xx_dvb *dvb;
+ struct usb_device *udev;
if (dev->is_audio_only) {
/* Shouldn't initialize IR for this interface */
@@ -1914,7 +1913,10 @@ static int em28xx_dvb_init(struct em28xx *dev)
si2168_config.ts_mode = SI2168_TS_SERIAL;
memset(&info, 0, sizeof(struct i2c_board_info));
strlcpy(info.type, "si2168", I2C_NAME_SIZE);
- info.addr = 0x64;
+ if (dev->ts == PRIMARY_TS)
+ info.addr = 0x64;
+ else
+ info.addr = 0x67;
info.platform_data = &si2168_config;
request_module(info.type);
client = i2c_new_device(&dev->i2c_adap[dev->def_i2c_bus], &info);
@@ -1940,7 +1942,10 @@ static int em28xx_dvb_init(struct em28xx *dev)
#endif
memset(&info, 0, sizeof(struct i2c_board_info));
strlcpy(info.type, "si2157", I2C_NAME_SIZE);
- info.addr = 0x60;
+ if (dev->ts == PRIMARY_TS)
+ info.addr = 0x60;
+ else
+ info.addr = 0x63;
info.platform_data = &si2157_config;
request_module(info.type);
client = i2c_new_device(adapter, &info);
@@ -1976,7 +1981,10 @@ static int em28xx_dvb_init(struct em28xx *dev)
lgdt3306a_config.fe = &dvb->fe[0];
lgdt3306a_config.i2c_adapter = &adapter;
strlcpy(info.type, "lgdt3306a", sizeof(info.type));
- info.addr = 0x59;
+ if (dev->ts == PRIMARY_TS)
+ info.addr = 0x59;
+ else
+ info.addr = 0x0e;
info.platform_data = &lgdt3306a_config;
request_module(info.type);
client = i2c_new_device(&dev->i2c_adap[dev->def_i2c_bus],
@@ -2003,7 +2011,10 @@ static int em28xx_dvb_init(struct em28xx *dev)
#endif
memset(&info, 0, sizeof(struct i2c_board_info));
strlcpy(info.type, "si2157", sizeof(info.type));
- info.addr = 0x60;
+ if (dev->ts == PRIMARY_TS)
+ info.addr = 0x60;
+ else
+ info.addr = 0x62;
info.platform_data = &si2157_config;
request_module(info.type);
@@ -2046,6 +2057,14 @@ static int em28xx_dvb_init(struct em28xx *dev)
if (result < 0)
goto out_free;
+ if (dev->dvb_xfer_bulk) {
+ dvb_alt = 0;
+ } else { /* isoc */
+ dvb_alt = dev->dvb_alt_isoc;
+ }
+
+ udev = interface_to_usbdev(dev->intf);
+ usb_set_interface(udev, dev->ifnum, dvb_alt);
dev_info(&dev->intf->dev, "DVB extension successfully initialized\n");
kref_get(&dev->ref);
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 88084f2..c85292c 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -217,6 +217,9 @@
/* max. number of button state polling addresses */
#define EM28XX_NUM_BUTTON_ADDRESSES_MAX 5
+#define PRIMARY_TS 0
+#define SECONDARY_TS 1
+
enum em28xx_mode {
EM28XX_SUSPEND,
EM28XX_ANALOG_MODE,
@@ -457,6 +460,7 @@ struct em28xx_board {
unsigned int mts_firmware:1;
unsigned int max_range_640_480:1;
unsigned int has_dvb:1;
+ unsigned int has_dual_ts:1;
unsigned int is_webcam:1;
unsigned int valid:1;
unsigned int has_ir_i2c:1;
@@ -621,6 +625,7 @@ struct em28xx {
unsigned int is_audio_only:1;
enum em28xx_int_audio_type int_audio_type;
enum em28xx_usb_audio_type usb_audio_type;
+ unsigned char name[32];
struct em28xx_board board;
@@ -682,6 +687,8 @@ struct em28xx {
u8 ifnum; /* number of the assigned usb interface */
u8 analog_ep_isoc; /* address of isoc endpoint for analog */
u8 analog_ep_bulk; /* address of bulk endpoint for analog */
+ u8 dvb_ep_isoc_ts2; /* address of isoc endpoint for DVB TS2*/
+ u8 dvb_ep_bulk_ts2; /* address of bulk endpoint for DVB TS2*/
u8 dvb_ep_isoc; /* address of isoc endpoint for DVB */
u8 dvb_ep_bulk; /* address of bulk endpoint for DVB */
int alt; /* alternate setting */
@@ -695,6 +702,8 @@ struct em28xx {
int dvb_alt_isoc; /* alternate setting for DVB isoc transfers */
unsigned int dvb_max_pkt_size_isoc; /* isoc max packet size of the
selected DVB ep at dvb_alt */
+ unsigned int dvb_max_pkt_size_isoc_ts2; /* isoc max packet size of the
+ selected DVB ep at dvb_alt */
unsigned int dvb_xfer_bulk:1; /* use bulk instead of isoc
transfers for DVB */
char urb_buf[URB_MAX_CTRL_SIZE]; /* urb control msg buffer */
@@ -726,6 +735,9 @@ struct em28xx {
struct media_entity input_ent[MAX_EM28XX_INPUT];
struct media_pad input_pad[MAX_EM28XX_INPUT];
#endif
+
+ struct em28xx *dev_next;
+ int ts;
};
#define kref_to_dev(d) container_of(d, struct em28xx, ref)
--
2.7.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 1/9] em28xx: Hauppauge DualHD second tuner functionality
2018-01-05 0:04 ` [PATCH 1/9] em28xx: Hauppauge DualHD second tuner functionality Brad Love
@ 2018-01-05 0:21 ` Michael Ira Krufky
0 siblings, 0 replies; 32+ messages in thread
From: Michael Ira Krufky @ 2018-01-05 0:21 UTC (permalink / raw)
To: Brad Love; +Cc: linux-media
On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
> Implement use of secondary TS port on em28xx.
> Adds has_dual_ts field, allows secondary demod/tuner to be
> added to a single em28xx device.
>
> Hauppauge DualHD models are configured to use this feature.
>
> Signed-off-by: Brad Love <brad@nextdimension.cc>
:+1
Reviewed-by: Michael Ira Krufky <mkrufky@linuxtv.org>
> ---
> drivers/media/usb/em28xx/em28xx-cards.c | 126 +++++++++++++++++++++++++++++++-
> drivers/media/usb/em28xx/em28xx-core.c | 42 +++++++++--
> drivers/media/usb/em28xx/em28xx-dvb.c | 33 +++++++--
> drivers/media/usb/em28xx/em28xx.h | 12 +++
> 4 files changed, 196 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
> index 34e16f6a..7f5d0b28 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -2402,6 +2402,7 @@ struct em28xx_board em28xx_boards[] = {
> .tuner_type = TUNER_ABSENT,
> .tuner_gpio = hauppauge_dualhd_dvb,
> .has_dvb = 1,
> + .has_dual_ts = 1,
> .ir_codes = RC_MAP_HAUPPAUGE,
> .leds = hauppauge_dualhd_leds,
> },
> @@ -2417,6 +2418,7 @@ struct em28xx_board em28xx_boards[] = {
> .tuner_type = TUNER_ABSENT,
> .tuner_gpio = hauppauge_dualhd_dvb,
> .has_dvb = 1,
> + .has_dual_ts = 1,
> .ir_codes = RC_MAP_HAUPPAUGE,
> .leds = hauppauge_dualhd_leds,
> },
> @@ -3239,7 +3241,8 @@ static void em28xx_release_resources(struct em28xx *dev)
> em28xx_i2c_unregister(dev, 1);
> em28xx_i2c_unregister(dev, 0);
>
> - usb_put_dev(udev);
> + if (dev->ts == PRIMARY_TS)
> + usb_put_dev(udev);
>
> /* Mark device as unused */
> clear_bit(dev->devno, em28xx_devused);
> @@ -3432,6 +3435,35 @@ static int em28xx_init_dev(struct em28xx *dev, struct usb_device *udev,
> return 0;
> }
>
> +int em28xx_duplicate_dev(struct em28xx *dev)
> +{
> + int nr;
> + struct em28xx *sec_dev = kzalloc(sizeof(*sec_dev), GFP_KERNEL);
> +
> + if (sec_dev == NULL) {
> + dev->dev_next = NULL;
> + return -ENOMEM;
> + }
> + memcpy(sec_dev, dev, sizeof(sizeof(*sec_dev)));
> + /* Check to see next free device and mark as used */
> + do {
> + nr = find_first_zero_bit(em28xx_devused, EM28XX_MAXBOARDS);
> + if (nr >= EM28XX_MAXBOARDS) {
> + /* No free device slots */
> + dev_warn(&dev->intf->dev, ": Supports only %i em28xx boards.\n",
> + EM28XX_MAXBOARDS);
> + kfree(sec_dev);
> + dev->dev_next = NULL;
> + return -ENOMEM;
> + }
> + } while (test_and_set_bit(nr, em28xx_devused));
> + sec_dev->devno = nr;
> + snprintf(sec_dev->name, 28, "em28xx #%d", nr);
> + sec_dev->dev_next = NULL;
> + dev->dev_next = sec_dev;
> + return 0;
> +}
> +
> /* high bandwidth multiplier, as encoded in highspeed endpoint descriptors */
> #define hb_mult(wMaxPacketSize) (1 + (((wMaxPacketSize) >> 11) & 0x03))
>
> @@ -3551,6 +3583,17 @@ static int em28xx_usb_probe(struct usb_interface *interface,
> }
> }
> break;
> + case 0x85:
> + if (usb_endpoint_xfer_isoc(e)) {
> + if (size > dev->dvb_max_pkt_size_isoc_ts2) {
> + dev->dvb_ep_isoc_ts2 = e->bEndpointAddress;
> + dev->dvb_max_pkt_size_isoc_ts2 = size;
> + dev->dvb_alt_isoc = i;
> + }
> + } else {
> + dev->dvb_ep_bulk_ts2 = e->bEndpointAddress;
> + }
> + break;
> }
> }
> /* NOTE:
> @@ -3565,6 +3608,8 @@ static int em28xx_usb_probe(struct usb_interface *interface,
> * 0x83 isoc* => audio
> * 0x84 isoc => digital
> * 0x84 bulk => analog or digital**
> + * 0x85 isoc => digital TS2
> + * 0x85 bulk => digital TS2
> * (*: audio should always be isoc)
> * (**: analog, if ep 0x82 is isoc, otherwise digital)
> *
> @@ -3632,6 +3677,10 @@ static int em28xx_usb_probe(struct usb_interface *interface,
> dev->has_video = has_video;
> dev->ifnum = ifnum;
>
> + dev->ts = PRIMARY_TS;
> + snprintf(dev->name, 28, "em28xx");
> + dev->dev_next = NULL;
> +
> if (has_vendor_audio) {
> dev_err(&interface->dev,
> "Audio interface %i found (Vendor Class)\n", ifnum);
> @@ -3711,6 +3760,65 @@ static int em28xx_usb_probe(struct usb_interface *interface,
> dev->dvb_xfer_bulk ? "bulk" : "isoc");
> }
>
> + if (dev->board.has_dual_ts && em28xx_duplicate_dev(dev) == 0) {
> + dev->dev_next->ts = SECONDARY_TS;
> + dev->dev_next->alt = -1;
> + dev->dev_next->is_audio_only = has_vendor_audio &&
> + !(has_video || has_dvb);
> + dev->dev_next->has_video = false;
> + dev->dev_next->ifnum = ifnum;
> + dev->dev_next->model = id->driver_info;
> +
> + mutex_init(&dev->dev_next->lock);
> + retval = em28xx_init_dev(dev->dev_next, udev, interface,
> + dev->dev_next->devno);
> + if (retval)
> + goto err_free;
> +
> + dev->dev_next->board.ir_codes = NULL; /* No IR for 2nd tuner */
> + dev->dev_next->board.has_ir_i2c = 0; /* No IR for 2nd tuner */
> +
> + if (usb_xfer_mode < 0) {
> + if (dev->dev_next->board.is_webcam)
> + try_bulk = 1;
> + else
> + try_bulk = 0;
> + } else {
> + try_bulk = usb_xfer_mode > 0;
> + }
> +
> + /* Select USB transfer types to use */
> + if (has_dvb) {
> + if (!dev->dvb_ep_isoc_ts2 ||
> + (try_bulk && dev->dvb_ep_bulk_ts2))
> + dev->dev_next->dvb_xfer_bulk = 1;
> + dev_info(&dev->intf->dev, "dvb ts2 set to %s mode.\n",
> + dev->dev_next->dvb_xfer_bulk ? "bulk" : "isoc");
> + }
> +
> + dev->dev_next->dvb_ep_isoc = dev->dvb_ep_isoc_ts2;
> + dev->dev_next->dvb_ep_bulk = dev->dvb_ep_bulk_ts2;
> + dev->dev_next->dvb_max_pkt_size_isoc = dev->dvb_max_pkt_size_isoc_ts2;
> + dev->dev_next->dvb_alt_isoc = dev->dvb_alt_isoc;
> +
> + /* Configuare hardware to support TS2*/
> + if (dev->dvb_xfer_bulk) {
> + /* The ep4 and ep5 are configuared for BULK */
> + em28xx_write_reg(dev, 0x0b, 0x96);
> + mdelay(100);
> + em28xx_write_reg(dev, 0x0b, 0x80);
> + mdelay(100);
> + } else {
> + /* The ep4 and ep5 are configuared for ISO */
> + em28xx_write_reg(dev, 0x0b, 0x96);
> + mdelay(100);
> + em28xx_write_reg(dev, 0x0b, 0x82);
> + mdelay(100);
> + }
> +
> + kref_init(&dev->dev_next->ref);
> + }
> +
> kref_init(&dev->ref);
>
> request_modules(dev);
> @@ -3753,15 +3861,29 @@ static void em28xx_usb_disconnect(struct usb_interface *interface)
> if (!dev)
> return;
>
> + if (dev->dev_next != NULL) {
> + dev->dev_next->disconnected = 1;
> + dev_info(&dev->intf->dev, "Disconnecting %s\n",
> + dev->dev_next->name);
> + flush_request_modules(dev->dev_next);
> + }
> +
> dev->disconnected = 1;
>
> - dev_err(&dev->intf->dev, "Disconnecting\n");
> + dev_err(&dev->intf->dev, "Disconnecting %s\n", dev->name);
>
> flush_request_modules(dev);
>
> em28xx_close_extension(dev);
>
> + if (dev->dev_next != NULL)
> + em28xx_release_resources(dev->dev_next);
> em28xx_release_resources(dev);
> +
> + if (dev->dev_next != NULL) {
> + kref_put(&dev->dev_next->ref, em28xx_free_device);
> + dev->dev_next = NULL;
> + }
> kref_put(&dev->ref, em28xx_free_device);
> }
>
> diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
> index 1d0d8cc..ef38e56 100644
> --- a/drivers/media/usb/em28xx/em28xx-core.c
> +++ b/drivers/media/usb/em28xx/em28xx-core.c
> @@ -638,10 +638,18 @@ int em28xx_capture_start(struct em28xx *dev, int start)
> dev->chip_id == CHIP_ID_EM28174 ||
> dev->chip_id == CHIP_ID_EM28178) {
> /* The Transport Stream Enable Register moved in em2874 */
> - rc = em28xx_write_reg_bits(dev, EM2874_R5F_TS_ENABLE,
> - start ?
> - EM2874_TS1_CAPTURE_ENABLE : 0x00,
> - EM2874_TS1_CAPTURE_ENABLE);
> + if (dev->ts == PRIMARY_TS)
> + rc = em28xx_write_reg_bits(dev,
> + EM2874_R5F_TS_ENABLE,
> + start ?
> + EM2874_TS1_CAPTURE_ENABLE : 0x00,
> + EM2874_TS1_CAPTURE_ENABLE);
> + else
> + rc = em28xx_write_reg_bits(dev,
> + EM2874_R5F_TS_ENABLE,
> + start ?
> + EM2874_TS2_CAPTURE_ENABLE : 0x00,
> + EM2874_TS2_CAPTURE_ENABLE);
> } else {
> /* FIXME: which is the best order? */
> /* video registers are sampled by VREF */
> @@ -1077,7 +1085,11 @@ int em28xx_register_extension(struct em28xx_ops *ops)
> mutex_lock(&em28xx_devlist_mutex);
> list_add_tail(&ops->next, &em28xx_extension_devlist);
> list_for_each_entry(dev, &em28xx_devlist, devlist) {
> - ops->init(dev);
> + if (ops->init) {
> + ops->init(dev);
> + if (dev->dev_next != NULL)
> + ops->init(dev->dev_next);
> + }
> }
> mutex_unlock(&em28xx_devlist_mutex);
> pr_info("em28xx: Registered (%s) extension\n", ops->name);
> @@ -1091,7 +1103,11 @@ void em28xx_unregister_extension(struct em28xx_ops *ops)
>
> mutex_lock(&em28xx_devlist_mutex);
> list_for_each_entry(dev, &em28xx_devlist, devlist) {
> - ops->fini(dev);
> + if (ops->fini) {
> + if (dev->dev_next != NULL)
> + ops->fini(dev->dev_next);
> + ops->fini(dev);
> + }
> }
> list_del(&ops->next);
> mutex_unlock(&em28xx_devlist_mutex);
> @@ -1106,8 +1122,11 @@ void em28xx_init_extension(struct em28xx *dev)
> mutex_lock(&em28xx_devlist_mutex);
> list_add_tail(&dev->devlist, &em28xx_devlist);
> list_for_each_entry(ops, &em28xx_extension_devlist, next) {
> - if (ops->init)
> + if (ops->init) {
> ops->init(dev);
> + if (dev->dev_next != NULL)
> + ops->init(dev->dev_next);
> + }
> }
> mutex_unlock(&em28xx_devlist_mutex);
> }
> @@ -1118,8 +1137,11 @@ void em28xx_close_extension(struct em28xx *dev)
>
> mutex_lock(&em28xx_devlist_mutex);
> list_for_each_entry(ops, &em28xx_extension_devlist, next) {
> - if (ops->fini)
> + if (ops->fini) {
> + if (dev->dev_next != NULL)
> + ops->fini(dev->dev_next);
> ops->fini(dev);
> + }
> }
> list_del(&dev->devlist);
> mutex_unlock(&em28xx_devlist_mutex);
> @@ -1134,6 +1156,8 @@ int em28xx_suspend_extension(struct em28xx *dev)
> list_for_each_entry(ops, &em28xx_extension_devlist, next) {
> if (ops->suspend)
> ops->suspend(dev);
> + if (dev->dev_next != NULL)
> + ops->suspend(dev->dev_next);
> }
> mutex_unlock(&em28xx_devlist_mutex);
> return 0;
> @@ -1148,6 +1172,8 @@ int em28xx_resume_extension(struct em28xx *dev)
> list_for_each_entry(ops, &em28xx_extension_devlist, next) {
> if (ops->resume)
> ops->resume(dev);
> + if (dev->dev_next != NULL)
> + ops->resume(dev->dev_next);
> }
> mutex_unlock(&em28xx_devlist_mutex);
> return 0;
> diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c
> index 8a81c94..9bc9576 100644
> --- a/drivers/media/usb/em28xx/em28xx-dvb.c
> +++ b/drivers/media/usb/em28xx/em28xx-dvb.c
> @@ -199,7 +199,6 @@ static int em28xx_start_streaming(struct em28xx_dvb *dvb)
> int rc;
> struct em28xx_i2c_bus *i2c_bus = dvb->adapter.priv;
> struct em28xx *dev = i2c_bus->dev;
> - struct usb_device *udev = interface_to_usbdev(dev->intf);
> int dvb_max_packet_size, packet_multiplier, dvb_alt;
>
> if (dev->dvb_xfer_bulk) {
> @@ -218,7 +217,6 @@ static int em28xx_start_streaming(struct em28xx_dvb *dvb)
> dvb_alt = dev->dvb_alt_isoc;
> }
>
> - usb_set_interface(udev, dev->ifnum, dvb_alt);
> rc = em28xx_set_mode(dev, EM28XX_DIGITAL_MODE);
> if (rc < 0)
> return rc;
> @@ -1128,8 +1126,9 @@ static void em28xx_unregister_dvb(struct em28xx_dvb *dvb)
>
> static int em28xx_dvb_init(struct em28xx *dev)
> {
> - int result = 0;
> + int result = 0, dvb_alt = 0;
> struct em28xx_dvb *dvb;
> + struct usb_device *udev;
>
> if (dev->is_audio_only) {
> /* Shouldn't initialize IR for this interface */
> @@ -1914,7 +1913,10 @@ static int em28xx_dvb_init(struct em28xx *dev)
> si2168_config.ts_mode = SI2168_TS_SERIAL;
> memset(&info, 0, sizeof(struct i2c_board_info));
> strlcpy(info.type, "si2168", I2C_NAME_SIZE);
> - info.addr = 0x64;
> + if (dev->ts == PRIMARY_TS)
> + info.addr = 0x64;
> + else
> + info.addr = 0x67;
> info.platform_data = &si2168_config;
> request_module(info.type);
> client = i2c_new_device(&dev->i2c_adap[dev->def_i2c_bus], &info);
> @@ -1940,7 +1942,10 @@ static int em28xx_dvb_init(struct em28xx *dev)
> #endif
> memset(&info, 0, sizeof(struct i2c_board_info));
> strlcpy(info.type, "si2157", I2C_NAME_SIZE);
> - info.addr = 0x60;
> + if (dev->ts == PRIMARY_TS)
> + info.addr = 0x60;
> + else
> + info.addr = 0x63;
> info.platform_data = &si2157_config;
> request_module(info.type);
> client = i2c_new_device(adapter, &info);
> @@ -1976,7 +1981,10 @@ static int em28xx_dvb_init(struct em28xx *dev)
> lgdt3306a_config.fe = &dvb->fe[0];
> lgdt3306a_config.i2c_adapter = &adapter;
> strlcpy(info.type, "lgdt3306a", sizeof(info.type));
> - info.addr = 0x59;
> + if (dev->ts == PRIMARY_TS)
> + info.addr = 0x59;
> + else
> + info.addr = 0x0e;
> info.platform_data = &lgdt3306a_config;
> request_module(info.type);
> client = i2c_new_device(&dev->i2c_adap[dev->def_i2c_bus],
> @@ -2003,7 +2011,10 @@ static int em28xx_dvb_init(struct em28xx *dev)
> #endif
> memset(&info, 0, sizeof(struct i2c_board_info));
> strlcpy(info.type, "si2157", sizeof(info.type));
> - info.addr = 0x60;
> + if (dev->ts == PRIMARY_TS)
> + info.addr = 0x60;
> + else
> + info.addr = 0x62;
> info.platform_data = &si2157_config;
> request_module(info.type);
>
> @@ -2046,6 +2057,14 @@ static int em28xx_dvb_init(struct em28xx *dev)
> if (result < 0)
> goto out_free;
>
> + if (dev->dvb_xfer_bulk) {
> + dvb_alt = 0;
> + } else { /* isoc */
> + dvb_alt = dev->dvb_alt_isoc;
> + }
> +
> + udev = interface_to_usbdev(dev->intf);
> + usb_set_interface(udev, dev->ifnum, dvb_alt);
> dev_info(&dev->intf->dev, "DVB extension successfully initialized\n");
>
> kref_get(&dev->ref);
> diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
> index 88084f2..c85292c 100644
> --- a/drivers/media/usb/em28xx/em28xx.h
> +++ b/drivers/media/usb/em28xx/em28xx.h
> @@ -217,6 +217,9 @@
> /* max. number of button state polling addresses */
> #define EM28XX_NUM_BUTTON_ADDRESSES_MAX 5
>
> +#define PRIMARY_TS 0
> +#define SECONDARY_TS 1
> +
> enum em28xx_mode {
> EM28XX_SUSPEND,
> EM28XX_ANALOG_MODE,
> @@ -457,6 +460,7 @@ struct em28xx_board {
> unsigned int mts_firmware:1;
> unsigned int max_range_640_480:1;
> unsigned int has_dvb:1;
> + unsigned int has_dual_ts:1;
> unsigned int is_webcam:1;
> unsigned int valid:1;
> unsigned int has_ir_i2c:1;
> @@ -621,6 +625,7 @@ struct em28xx {
> unsigned int is_audio_only:1;
> enum em28xx_int_audio_type int_audio_type;
> enum em28xx_usb_audio_type usb_audio_type;
> + unsigned char name[32];
>
> struct em28xx_board board;
>
> @@ -682,6 +687,8 @@ struct em28xx {
> u8 ifnum; /* number of the assigned usb interface */
> u8 analog_ep_isoc; /* address of isoc endpoint for analog */
> u8 analog_ep_bulk; /* address of bulk endpoint for analog */
> + u8 dvb_ep_isoc_ts2; /* address of isoc endpoint for DVB TS2*/
> + u8 dvb_ep_bulk_ts2; /* address of bulk endpoint for DVB TS2*/
> u8 dvb_ep_isoc; /* address of isoc endpoint for DVB */
> u8 dvb_ep_bulk; /* address of bulk endpoint for DVB */
> int alt; /* alternate setting */
> @@ -695,6 +702,8 @@ struct em28xx {
> int dvb_alt_isoc; /* alternate setting for DVB isoc transfers */
> unsigned int dvb_max_pkt_size_isoc; /* isoc max packet size of the
> selected DVB ep at dvb_alt */
> + unsigned int dvb_max_pkt_size_isoc_ts2; /* isoc max packet size of the
> + selected DVB ep at dvb_alt */
> unsigned int dvb_xfer_bulk:1; /* use bulk instead of isoc
> transfers for DVB */
> char urb_buf[URB_MAX_CTRL_SIZE]; /* urb control msg buffer */
> @@ -726,6 +735,9 @@ struct em28xx {
> struct media_entity input_ent[MAX_EM28XX_INPUT];
> struct media_pad input_pad[MAX_EM28XX_INPUT];
> #endif
> +
> + struct em28xx *dev_next;
> + int ts;
> };
>
> #define kref_to_dev(d) container_of(d, struct em28xx, ref)
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 2/9] em28xx: Bulk transfer implementation fix
2018-01-05 0:04 [PATCH 0/9] Hauppauge em28xx/lgdt3306a soloHD/DualHD support Brad Love
2018-01-05 0:04 ` [PATCH 1/9] em28xx: Hauppauge DualHD second tuner functionality Brad Love
@ 2018-01-05 0:04 ` Brad Love
2018-01-05 0:22 ` Michael Ira Krufky
` (2 more replies)
2018-01-05 0:04 ` [PATCH 3/9] em28xx: USB bulk packet size fix Brad Love
` (6 subsequent siblings)
8 siblings, 3 replies; 32+ messages in thread
From: Brad Love @ 2018-01-05 0:04 UTC (permalink / raw)
To: linux-media; +Cc: Brad Love
Set appropriate bulk/ISOC transfer multiplier on capture start.
This sets ISOC transfer to 940 bytes (188 * 5)
This sets bulk transfer to 48128 bytes (188 * 256)
The above values are maximum allowed according to Empia.
Signed-off-by: Brad Love <brad@nextdimension.cc>
---
drivers/media/usb/em28xx/em28xx-core.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
index ef38e56..67ed6a3 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -638,6 +638,18 @@ int em28xx_capture_start(struct em28xx *dev, int start)
dev->chip_id == CHIP_ID_EM28174 ||
dev->chip_id == CHIP_ID_EM28178) {
/* The Transport Stream Enable Register moved in em2874 */
+ if (dev->dvb_xfer_bulk) {
+ /* Max Tx Size = 188 * 256 = 48128 - LCM(188,512) * 2 */
+ em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
+ EM2874_R5D_TS1_PKT_SIZE :
+ EM2874_R5E_TS2_PKT_SIZE,
+ 0xFF);
+ } else {
+ /* TS2 Maximum Transfer Size = 188 * 5 */
+ em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
+ EM2874_R5D_TS1_PKT_SIZE :
+ EM2874_R5E_TS2_PKT_SIZE, 0x05);
+ }
if (dev->ts == PRIMARY_TS)
rc = em28xx_write_reg_bits(dev,
EM2874_R5F_TS_ENABLE,
--
2.7.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 2/9] em28xx: Bulk transfer implementation fix
2018-01-05 0:04 ` [PATCH 2/9] em28xx: Bulk transfer implementation fix Brad Love
@ 2018-01-05 0:22 ` Michael Ira Krufky
2018-01-05 14:20 ` Devin Heitmueller
2018-01-30 12:07 ` Mauro Carvalho Chehab
2018-02-01 22:04 ` [PATCH v2 " Brad Love
2 siblings, 1 reply; 32+ messages in thread
From: Michael Ira Krufky @ 2018-01-05 0:22 UTC (permalink / raw)
To: Brad Love; +Cc: linux-media
On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
> Set appropriate bulk/ISOC transfer multiplier on capture start.
> This sets ISOC transfer to 940 bytes (188 * 5)
> This sets bulk transfer to 48128 bytes (188 * 256)
>
> The above values are maximum allowed according to Empia.
>
> Signed-off-by: Brad Love <brad@nextdimension.cc>
:+1
Reviewed-by: Michael Ira Krufky <mkrufky@linuxtv.org>
> ---
> drivers/media/usb/em28xx/em28xx-core.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
> index ef38e56..67ed6a3 100644
> --- a/drivers/media/usb/em28xx/em28xx-core.c
> +++ b/drivers/media/usb/em28xx/em28xx-core.c
> @@ -638,6 +638,18 @@ int em28xx_capture_start(struct em28xx *dev, int start)
> dev->chip_id == CHIP_ID_EM28174 ||
> dev->chip_id == CHIP_ID_EM28178) {
> /* The Transport Stream Enable Register moved in em2874 */
> + if (dev->dvb_xfer_bulk) {
> + /* Max Tx Size = 188 * 256 = 48128 - LCM(188,512) * 2 */
> + em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
> + EM2874_R5D_TS1_PKT_SIZE :
> + EM2874_R5E_TS2_PKT_SIZE,
> + 0xFF);
> + } else {
> + /* TS2 Maximum Transfer Size = 188 * 5 */
> + em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
> + EM2874_R5D_TS1_PKT_SIZE :
> + EM2874_R5E_TS2_PKT_SIZE, 0x05);
> + }
> if (dev->ts == PRIMARY_TS)
> rc = em28xx_write_reg_bits(dev,
> EM2874_R5F_TS_ENABLE,
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 2/9] em28xx: Bulk transfer implementation fix
2018-01-05 0:22 ` Michael Ira Krufky
@ 2018-01-05 14:20 ` Devin Heitmueller
2018-01-05 15:21 ` Brad Love
2018-01-30 19:56 ` Brad Love
0 siblings, 2 replies; 32+ messages in thread
From: Devin Heitmueller @ 2018-01-05 14:20 UTC (permalink / raw)
To: Michael Ira Krufky; +Cc: Brad Love, linux-media
Hi Brad,
My documents indicate that Register 0x5D and 0x5E are read-only, and
populated based on the eeprom programming.
On your device, what is the value of those registers prior to you changing them?
If you write to those registers, do they reflect the new values if you
read them back?
Does changing these values result in any change to the device's
endpoint configuration (which is typically statically defined when the
device is probed)?
What precisely is the behavior you were seeing prior to this patch?
Devin
On Thu, Jan 4, 2018 at 7:22 PM, Michael Ira Krufky <mkrufky@linuxtv.org> wrote:
> On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
>> Set appropriate bulk/ISOC transfer multiplier on capture start.
>> This sets ISOC transfer to 940 bytes (188 * 5)
>> This sets bulk transfer to 48128 bytes (188 * 256)
>>
>> The above values are maximum allowed according to Empia.
>>
>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>
> :+1
>
> Reviewed-by: Michael Ira Krufky <mkrufky@linuxtv.org>
>
>> ---
>> drivers/media/usb/em28xx/em28xx-core.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
>> index ef38e56..67ed6a3 100644
>> --- a/drivers/media/usb/em28xx/em28xx-core.c
>> +++ b/drivers/media/usb/em28xx/em28xx-core.c
>> @@ -638,6 +638,18 @@ int em28xx_capture_start(struct em28xx *dev, int start)
>> dev->chip_id == CHIP_ID_EM28174 ||
>> dev->chip_id == CHIP_ID_EM28178) {
>> /* The Transport Stream Enable Register moved in em2874 */
>> + if (dev->dvb_xfer_bulk) {
>> + /* Max Tx Size = 188 * 256 = 48128 - LCM(188,512) * 2 */
>> + em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>> + EM2874_R5D_TS1_PKT_SIZE :
>> + EM2874_R5E_TS2_PKT_SIZE,
>> + 0xFF);
>> + } else {
>> + /* TS2 Maximum Transfer Size = 188 * 5 */
>> + em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>> + EM2874_R5D_TS1_PKT_SIZE :
>> + EM2874_R5E_TS2_PKT_SIZE, 0x05);
>> + }
>> if (dev->ts == PRIMARY_TS)
>> rc = em28xx_write_reg_bits(dev,
>> EM2874_R5F_TS_ENABLE,
>> --
>> 2.7.4
>>
--
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 2/9] em28xx: Bulk transfer implementation fix
2018-01-05 14:20 ` Devin Heitmueller
@ 2018-01-05 15:21 ` Brad Love
2018-01-30 19:56 ` Brad Love
1 sibling, 0 replies; 32+ messages in thread
From: Brad Love @ 2018-01-05 15:21 UTC (permalink / raw)
To: Devin Heitmueller, Michael Ira Krufky; +Cc: Brad Love, linux-media
On 2018-01-05 08:20, Devin Heitmueller wrote:
> Hi Brad,
>
> My documents indicate that Register 0x5D and 0x5E are read-only, and
> populated based on the eeprom programming.
>
> On your device, what is the value of those registers prior to you changing them?
>
> If you write to those registers, do they reflect the new values if you
> read them back?
>
> Does changing these values result in any change to the device's
> endpoint configuration (which is typically statically defined when the
> device is probed)?
>
> What precisely is the behavior you were seeing prior to this patch?
>
> Devin
Hey Devin,
We have devices programmed ISOC and bulk in eeprom, but we were seeing
before that bulk transfers were not happening as expected. This included
continuity errors and corrupted packets. After speaking with Empia they
supplied this patch to configure the multiplier explicitly. They also
suggested changing the usb configuration to match this multiplier. This
is done in patch 3/9. I will add some instrumentation to check out the
data you're looking for though. I can say offhand that modifying those
values does have tangible effects. On 'native' machines, there is little
difference, but the multiplier values are make or break in VMWare.
Will reply with the data later.
Cheers,
Brad
> On Thu, Jan 4, 2018 at 7:22 PM, Michael Ira Krufky <mkrufky@linuxtv.org> wrote:
>> On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
>>> Set appropriate bulk/ISOC transfer multiplier on capture start.
>>> This sets ISOC transfer to 940 bytes (188 * 5)
>>> This sets bulk transfer to 48128 bytes (188 * 256)
>>>
>>> The above values are maximum allowed according to Empia.
>>>
>>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>> :+1
>>
>> Reviewed-by: Michael Ira Krufky <mkrufky@linuxtv.org>
>>
>>> ---
>>> drivers/media/usb/em28xx/em28xx-core.c | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
>>> index ef38e56..67ed6a3 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-core.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-core.c
>>> @@ -638,6 +638,18 @@ int em28xx_capture_start(struct em28xx *dev, int start)
>>> dev->chip_id == CHIP_ID_EM28174 ||
>>> dev->chip_id == CHIP_ID_EM28178) {
>>> /* The Transport Stream Enable Register moved in em2874 */
>>> + if (dev->dvb_xfer_bulk) {
>>> + /* Max Tx Size = 188 * 256 = 48128 - LCM(188,512) * 2 */
>>> + em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>>> + EM2874_R5D_TS1_PKT_SIZE :
>>> + EM2874_R5E_TS2_PKT_SIZE,
>>> + 0xFF);
>>> + } else {
>>> + /* TS2 Maximum Transfer Size = 188 * 5 */
>>> + em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>>> + EM2874_R5D_TS1_PKT_SIZE :
>>> + EM2874_R5E_TS2_PKT_SIZE, 0x05);
>>> + }
>>> if (dev->ts == PRIMARY_TS)
>>> rc = em28xx_write_reg_bits(dev,
>>> EM2874_R5F_TS_ENABLE,
>>> --
>>> 2.7.4
>>>
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 2/9] em28xx: Bulk transfer implementation fix
2018-01-05 14:20 ` Devin Heitmueller
2018-01-05 15:21 ` Brad Love
@ 2018-01-30 19:56 ` Brad Love
1 sibling, 0 replies; 32+ messages in thread
From: Brad Love @ 2018-01-30 19:56 UTC (permalink / raw)
To: Devin Heitmueller, Michael Ira Krufky; +Cc: Brad Love, linux-media
On 2018-01-05 08:20, Devin Heitmueller wrote:
> Hi Brad,
>
> My documents indicate that Register 0x5D and 0x5E are read-only, and
> populated based on the eeprom programming.
>
> On your device, what is the value of those registers prior to you changing them?
>
> If you write to those registers, do they reflect the new values if you
> read them back?
>
> Does changing these values result in any change to the device's
> endpoint configuration (which is typically statically defined when the
> device is probed)?
>
> What precisely is the behavior you were seeing prior to this patch?
>
> Devin
Hi Devin,
I answered ISOC half of the question dealing with endpoint configuration
in Mauro's email, just now, apologies for not cc'ing you. Here I'll
answer about the bulk half.
These registers according to Empia and my own research are writeable. I
instrumented the driver to get before and after values. The registers
are indeed updated with the 0xff value written.
The behaviour encountered is as follows. The DualHD DVB bulk model I
have with me, after device init, has TS1_PKT_SIZE:0x05 and
TS2_PKT_SIZE:0x10. Since the devices are bulk models, this leads to TS1
producing bulk packets of 940B and TS2 producing bulk packets of 3008B.
With this patch both PKT_SIZE registers are set to 0xff, and verified
returning 0xff. After that the bulk packet size produced by the em28xx
is 48128B, or 256*188. Patch 3/9 sets the bulk dvb multiplier to 94, so
then (512*94==48128) is used when starting the USB transfer. For bulk
transfers, everything should be in sync.
Such small bulk multipliers, as currently used, completely destroys
performance on embedded devices and in VMWare. This patch makes both of
those use cases very happy, and, usable at all.
Cheers,
Brad
>
> On Thu, Jan 4, 2018 at 7:22 PM, Michael Ira Krufky <mkrufky@linuxtv.org> wrote:
>> On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
>>> Set appropriate bulk/ISOC transfer multiplier on capture start.
>>> This sets ISOC transfer to 940 bytes (188 * 5)
>>> This sets bulk transfer to 48128 bytes (188 * 256)
>>>
>>> The above values are maximum allowed according to Empia.
>>>
>>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>> :+1
>>
>> Reviewed-by: Michael Ira Krufky <mkrufky@linuxtv.org>
>>
>>> ---
>>> drivers/media/usb/em28xx/em28xx-core.c | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
>>> index ef38e56..67ed6a3 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-core.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-core.c
>>> @@ -638,6 +638,18 @@ int em28xx_capture_start(struct em28xx *dev, int start)
>>> dev->chip_id == CHIP_ID_EM28174 ||
>>> dev->chip_id == CHIP_ID_EM28178) {
>>> /* The Transport Stream Enable Register moved in em2874 */
>>> + if (dev->dvb_xfer_bulk) {
>>> + /* Max Tx Size = 188 * 256 = 48128 - LCM(188,512) * 2 */
>>> + em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>>> + EM2874_R5D_TS1_PKT_SIZE :
>>> + EM2874_R5E_TS2_PKT_SIZE,
>>> + 0xFF);
>>> + } else {
>>> + /* TS2 Maximum Transfer Size = 188 * 5 */
>>> + em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>>> + EM2874_R5D_TS1_PKT_SIZE :
>>> + EM2874_R5E_TS2_PKT_SIZE, 0x05);
>>> + }
>>> if (dev->ts == PRIMARY_TS)
>>> rc = em28xx_write_reg_bits(dev,
>>> EM2874_R5F_TS_ENABLE,
>>> --
>>> 2.7.4
>>>
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/9] em28xx: Bulk transfer implementation fix
2018-01-05 0:04 ` [PATCH 2/9] em28xx: Bulk transfer implementation fix Brad Love
2018-01-05 0:22 ` Michael Ira Krufky
@ 2018-01-30 12:07 ` Mauro Carvalho Chehab
2018-01-30 19:33 ` Brad Love
2018-02-01 22:04 ` [PATCH v2 " Brad Love
2 siblings, 1 reply; 32+ messages in thread
From: Mauro Carvalho Chehab @ 2018-01-30 12:07 UTC (permalink / raw)
To: Brad Love; +Cc: linux-media
Em Thu, 4 Jan 2018 18:04:12 -0600
Brad Love <brad@nextdimension.cc> escreveu:
> Set appropriate bulk/ISOC transfer multiplier on capture start.
> This sets ISOC transfer to 940 bytes (188 * 5)
> This sets bulk transfer to 48128 bytes (188 * 256)
>
> The above values are maximum allowed according to Empia.
>
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
> drivers/media/usb/em28xx/em28xx-core.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
> index ef38e56..67ed6a3 100644
> --- a/drivers/media/usb/em28xx/em28xx-core.c
> +++ b/drivers/media/usb/em28xx/em28xx-core.c
> @@ -638,6 +638,18 @@ int em28xx_capture_start(struct em28xx *dev, int start)
> dev->chip_id == CHIP_ID_EM28174 ||
> dev->chip_id == CHIP_ID_EM28178) {
> /* The Transport Stream Enable Register moved in em2874 */
> + if (dev->dvb_xfer_bulk) {
> + /* Max Tx Size = 188 * 256 = 48128 - LCM(188,512) * 2 */
> + em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
> + EM2874_R5D_TS1_PKT_SIZE :
> + EM2874_R5E_TS2_PKT_SIZE,
> + 0xFF);
> + } else {
> + /* TS2 Maximum Transfer Size = 188 * 5 */
> + em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
> + EM2874_R5D_TS1_PKT_SIZE :
> + EM2874_R5E_TS2_PKT_SIZE, 0x05);
> + }
Hmm... for ISOC, the USB descriptors inform the max transfer size, with
are detected at probe time, on this part of em28xx_usb_probe:
if (size > dev->dvb_max_pkt_size_isoc) {
has_dvb = true; /* see NOTE (~) */
dev->dvb_ep_isoc = e->bEndpointAddress;
dev->dvb_max_pkt_size_isoc = size;
dev->dvb_alt_isoc = i;
}
If we're touching TS PKT size register, it should somehow be
aligned what's there. I mean, we should either do:
em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
EM2874_R5D_TS1_PKT_SIZE :
EM2874_R5E_TS2_PKT_SIZE, dev->dvb_max_pkt_size_isoc / 188);
Or the other way around, setting dev->dvb_max_pkt_size_isoc after
writing to EM2874_R5D_TS1_PKT_SIZE or EM2874_R5E_TS2_PKT_SIZE.
Not sure what's more accurate here: the USB descriptors or the
contents of the TS size register. I doubt, I would stick with
the USB descriptor info.
Btw, I wander what happens if we write a bigger value than 5 to those
registers. Would it support a bigger transfer size than 940 for ISOCH?
Cheers,
Mauro
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH 2/9] em28xx: Bulk transfer implementation fix
2018-01-30 12:07 ` Mauro Carvalho Chehab
@ 2018-01-30 19:33 ` Brad Love
0 siblings, 0 replies; 32+ messages in thread
From: Brad Love @ 2018-01-30 19:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Brad Love; +Cc: linux-media
On 2018-01-30 06:07, Mauro Carvalho Chehab wrote:
> Em Thu, 4 Jan 2018 18:04:12 -0600
> Brad Love <brad@nextdimension.cc> escreveu:
>
>> Set appropriate bulk/ISOC transfer multiplier on capture start.
>> This sets ISOC transfer to 940 bytes (188 * 5)
>> This sets bulk transfer to 48128 bytes (188 * 256)
>>
>> The above values are maximum allowed according to Empia.
>>
>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>> ---
>> drivers/media/usb/em28xx/em28xx-core.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
>> index ef38e56..67ed6a3 100644
>> --- a/drivers/media/usb/em28xx/em28xx-core.c
>> +++ b/drivers/media/usb/em28xx/em28xx-core.c
>> @@ -638,6 +638,18 @@ int em28xx_capture_start(struct em28xx *dev, int start)
>> dev->chip_id == CHIP_ID_EM28174 ||
>> dev->chip_id == CHIP_ID_EM28178) {
>> /* The Transport Stream Enable Register moved in em2874 */
>> + if (dev->dvb_xfer_bulk) {
>> + /* Max Tx Size = 188 * 256 = 48128 - LCM(188,512) * 2 */
>> + em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>> + EM2874_R5D_TS1_PKT_SIZE :
>> + EM2874_R5E_TS2_PKT_SIZE,
>> + 0xFF);
>> + } else {
>> + /* TS2 Maximum Transfer Size = 188 * 5 */
>> + em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>> + EM2874_R5D_TS1_PKT_SIZE :
>> + EM2874_R5E_TS2_PKT_SIZE, 0x05);
>> + }
> Hmm... for ISOC, the USB descriptors inform the max transfer size, with
> are detected at probe time, on this part of em28xx_usb_probe:
>
> if (size > dev->dvb_max_pkt_size_isoc) {
> has_dvb = true; /* see NOTE (~) */
> dev->dvb_ep_isoc = e->bEndpointAddress;
> dev->dvb_max_pkt_size_isoc = size;
> dev->dvb_alt_isoc = i;
> }
>
> If we're touching TS PKT size register, it should somehow be
> aligned what's there. I mean, we should either do:
>
> em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
> EM2874_R5D_TS1_PKT_SIZE :
> EM2874_R5E_TS2_PKT_SIZE, dev->dvb_max_pkt_size_isoc / 188);
>
> Or the other way around, setting dev->dvb_max_pkt_size_isoc after
> writing to EM2874_R5D_TS1_PKT_SIZE or EM2874_R5E_TS2_PKT_SIZE.
>
> Not sure what's more accurate here: the USB descriptors or the
> contents of the TS size register. I doubt, I would stick with
> the USB descriptor info.
>
> Btw, I wander what happens if we write a bigger value than 5 to those
> registers. Would it support a bigger transfer size than 940 for ISOCH?
>
>
>
> Cheers,
> Mauro
Hi Mauro,
On the one ISOC device I have here, the usb endpoint
dvb_max_pkt_size_isoc is 940 during usb_probe and
EM2874_R5D_TS1_PKT_SIZE returns 5 when queried in start_streaming. I
just did a little checking and EM2874_R5D_TS1_PKT_SIZE accepted, and
returned the value I wrote all the way up to 32. The device is DVB
however, so I cannot test actual operation to see if it increases ISOC
packet size at all. We're just going on what Empia said here for the
maximum of 5.
I agree that this is probably more correct to use (dvb_max_pkt_size_isoc
/ 188) instead of a hardcoded 5. This at least would keep devices with
other multipliers happy. Your second method of querying the multiplier
before setting up the endpoint would require a re-organization of
usb_probe to move em28xx_init_dev higher.
I'll submit a v2 of this briefly.
Cheers,
Brad
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 2/9] em28xx: Bulk transfer implementation fix
2018-01-05 0:04 ` [PATCH 2/9] em28xx: Bulk transfer implementation fix Brad Love
2018-01-05 0:22 ` Michael Ira Krufky
2018-01-30 12:07 ` Mauro Carvalho Chehab
@ 2018-02-01 22:04 ` Brad Love
2 siblings, 0 replies; 32+ messages in thread
From: Brad Love @ 2018-02-01 22:04 UTC (permalink / raw)
To: linux-media; +Cc: Brad Love
Set appropriate bulk/ISOC transfer multiplier on capture start.
This sets ISOC transfer to USB endpoint configuration
This sets bulk transfer to 48128 bytes (188 * 256)
The bulk multiplier is maximum allowed according to Empia.
Signed-off-by: Brad Love <brad@nextdimension.cc>
---
Changes since v1
- use ISOC endpoint configuration instead of constant
- removed TS2 from comment
drivers/media/usb/em28xx/em28xx-core.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
index ef38e56..6727d14 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -638,6 +638,19 @@ int em28xx_capture_start(struct em28xx *dev, int start)
dev->chip_id == CHIP_ID_EM28174 ||
dev->chip_id == CHIP_ID_EM28178) {
/* The Transport Stream Enable Register moved in em2874 */
+ if (dev->dvb_xfer_bulk) {
+ /* Max Tx Size = 188 * 256 = 48128 - LCM(188,512) * 2 */
+ em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
+ EM2874_R5D_TS1_PKT_SIZE :
+ EM2874_R5E_TS2_PKT_SIZE,
+ 0xFF);
+ } else {
+ /* ISOC Maximum Transfer Size = 188 * 5 */
+ em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
+ EM2874_R5D_TS1_PKT_SIZE :
+ EM2874_R5E_TS2_PKT_SIZE,
+ dev->dvb_max_pkt_size_isoc / 188);
+ }
if (dev->ts == PRIMARY_TS)
rc = em28xx_write_reg_bits(dev,
EM2874_R5F_TS_ENABLE,
--
2.7.4
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 3/9] em28xx: USB bulk packet size fix
2018-01-05 0:04 [PATCH 0/9] Hauppauge em28xx/lgdt3306a soloHD/DualHD support Brad Love
2018-01-05 0:04 ` [PATCH 1/9] em28xx: Hauppauge DualHD second tuner functionality Brad Love
2018-01-05 0:04 ` [PATCH 2/9] em28xx: Bulk transfer implementation fix Brad Love
@ 2018-01-05 0:04 ` Brad Love
2018-01-05 0:23 ` Michael Ira Krufky
2018-01-05 0:04 ` [PATCH 4/9] em28xx: Increase max em28xx boards to max dvb adapters Brad Love
` (5 subsequent siblings)
8 siblings, 1 reply; 32+ messages in thread
From: Brad Love @ 2018-01-05 0:04 UTC (permalink / raw)
To: linux-media; +Cc: Brad Love
Hauppauge em28xx bulk devices exhibit continuity errors and corrupted
packets, when run in VMWare virtual machines. Unknown if other
manufacturers bulk models exhibit the same issue. KVM/Qemu is unaffected.
According to documentation the maximum packet multiplier for em28xx in bulk
transfer mode is 256 * 188 bytes. This changes the size of bulk transfers
to maximum supported value and have a bonus beneficial alignment.
Before:
# 512 * 384 = 196608
## 196608 % 188 != 0
After:
# 512 * 47 * 2 = 48128 (188 * 128 * 2)
## 48128 % 188 = 0
This sets up USB to expect just as many bytes as the em28xx is set to emit.
Successful usage under load afterwards natively and in both VMWare
and KVM/Qemu virtual machines.
Signed-off-by: Brad Love <brad@nextdimension.cc>
---
drivers/media/usb/em28xx/em28xx.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index c85292c..7be8ac9 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -191,7 +191,7 @@
USB 2.0 spec says bulk packet size is always 512 bytes
*/
#define EM28XX_BULK_PACKET_MULTIPLIER 384
-#define EM28XX_DVB_BULK_PACKET_MULTIPLIER 384
+#define EM28XX_DVB_BULK_PACKET_MULTIPLIER 94
#define EM28XX_INTERLACED_DEFAULT 1
--
2.7.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 3/9] em28xx: USB bulk packet size fix
2018-01-05 0:04 ` [PATCH 3/9] em28xx: USB bulk packet size fix Brad Love
@ 2018-01-05 0:23 ` Michael Ira Krufky
0 siblings, 0 replies; 32+ messages in thread
From: Michael Ira Krufky @ 2018-01-05 0:23 UTC (permalink / raw)
To: Brad Love; +Cc: linux-media
On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
> Hauppauge em28xx bulk devices exhibit continuity errors and corrupted
> packets, when run in VMWare virtual machines. Unknown if other
> manufacturers bulk models exhibit the same issue. KVM/Qemu is unaffected.
>
> According to documentation the maximum packet multiplier for em28xx in bulk
> transfer mode is 256 * 188 bytes. This changes the size of bulk transfers
> to maximum supported value and have a bonus beneficial alignment.
>
> Before:
> # 512 * 384 = 196608
> ## 196608 % 188 != 0
>
> After:
> # 512 * 47 * 2 = 48128 (188 * 128 * 2)
> ## 48128 % 188 = 0
>
> This sets up USB to expect just as many bytes as the em28xx is set to emit.
>
> Successful usage under load afterwards natively and in both VMWare
> and KVM/Qemu virtual machines.
>
> Signed-off-by: Brad Love <brad@nextdimension.cc>
:+1
Reviewed-by: Michael Ira Krufky <mkrufky@linuxtv.org>
> ---
> drivers/media/usb/em28xx/em28xx.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
> index c85292c..7be8ac9 100644
> --- a/drivers/media/usb/em28xx/em28xx.h
> +++ b/drivers/media/usb/em28xx/em28xx.h
> @@ -191,7 +191,7 @@
> USB 2.0 spec says bulk packet size is always 512 bytes
> */
> #define EM28XX_BULK_PACKET_MULTIPLIER 384
> -#define EM28XX_DVB_BULK_PACKET_MULTIPLIER 384
> +#define EM28XX_DVB_BULK_PACKET_MULTIPLIER 94
>
> #define EM28XX_INTERLACED_DEFAULT 1
>
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 4/9] em28xx: Increase max em28xx boards to max dvb adapters
2018-01-05 0:04 [PATCH 0/9] Hauppauge em28xx/lgdt3306a soloHD/DualHD support Brad Love
` (2 preceding siblings ...)
2018-01-05 0:04 ` [PATCH 3/9] em28xx: USB bulk packet size fix Brad Love
@ 2018-01-05 0:04 ` Brad Love
2018-01-05 0:23 ` Michael Ira Krufky
2018-01-05 0:04 ` [PATCH 5/9] em28xx: Add Hauppauge SoloHD/DualHD bulk models Brad Love
` (4 subsequent siblings)
8 siblings, 1 reply; 32+ messages in thread
From: Brad Love @ 2018-01-05 0:04 UTC (permalink / raw)
To: linux-media; +Cc: Brad Love
Maximum 4 em28xx boards is too low, this can be maxed out by two devices.
This allows all the dvb adapters in the system to be em28xx if so desired.
Signed-off-by: Brad Love <brad@nextdimension.cc>
---
drivers/media/usb/em28xx/em28xx.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 7be8ac9..3dbcc9d 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -166,7 +166,7 @@
#define EM28XX_STOP_AUDIO 0
/* maximum number of em28xx boards */
-#define EM28XX_MAXBOARDS 4 /*FIXME: should be bigger */
+#define EM28XX_MAXBOARDS DVB_MAX_ADAPTERS /* All adapters could be em28xx */
/* maximum number of frames that can be queued */
#define EM28XX_NUM_FRAMES 5
--
2.7.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 4/9] em28xx: Increase max em28xx boards to max dvb adapters
2018-01-05 0:04 ` [PATCH 4/9] em28xx: Increase max em28xx boards to max dvb adapters Brad Love
@ 2018-01-05 0:23 ` Michael Ira Krufky
0 siblings, 0 replies; 32+ messages in thread
From: Michael Ira Krufky @ 2018-01-05 0:23 UTC (permalink / raw)
To: Brad Love; +Cc: linux-media
On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
> Maximum 4 em28xx boards is too low, this can be maxed out by two devices.
> This allows all the dvb adapters in the system to be em28xx if so desired.
>
> Signed-off-by: Brad Love <brad@nextdimension.cc>
:+1
Reviewed-by: Michael Ira Krufky <mkrufky@linuxtv.org>
> ---
> drivers/media/usb/em28xx/em28xx.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
> index 7be8ac9..3dbcc9d 100644
> --- a/drivers/media/usb/em28xx/em28xx.h
> +++ b/drivers/media/usb/em28xx/em28xx.h
> @@ -166,7 +166,7 @@
> #define EM28XX_STOP_AUDIO 0
>
> /* maximum number of em28xx boards */
> -#define EM28XX_MAXBOARDS 4 /*FIXME: should be bigger */
> +#define EM28XX_MAXBOARDS DVB_MAX_ADAPTERS /* All adapters could be em28xx */
>
> /* maximum number of frames that can be queued */
> #define EM28XX_NUM_FRAMES 5
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 5/9] em28xx: Add Hauppauge SoloHD/DualHD bulk models
2018-01-05 0:04 [PATCH 0/9] Hauppauge em28xx/lgdt3306a soloHD/DualHD support Brad Love
` (3 preceding siblings ...)
2018-01-05 0:04 ` [PATCH 4/9] em28xx: Increase max em28xx boards to max dvb adapters Brad Love
@ 2018-01-05 0:04 ` Brad Love
2018-01-05 0:24 ` Michael Ira Krufky
2018-01-05 0:04 ` [PATCH 6/9] em28xx: Enable Hauppauge SoloHD rebranded 292e SE Brad Love
` (3 subsequent siblings)
8 siblings, 1 reply; 32+ messages in thread
From: Brad Love @ 2018-01-05 0:04 UTC (permalink / raw)
To: linux-media; +Cc: Brad Love
Add additional pids to driver list
Signed-off-by: Brad Love <brad@nextdimension.cc>
---
drivers/media/usb/em28xx/em28xx-cards.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 7f5d0b28..34c693a 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -507,8 +507,10 @@ static struct em28xx_reg_seq plex_px_bcud[] = {
};
/*
- * 2040:0265 Hauppauge WinTV-dualHD DVB
- * 2040:026d Hauppauge WinTV-dualHD ATSC/QAM
+ * 2040:0265 Hauppauge WinTV-dualHD DVB Isoc
+ * 2040:8265 Hauppauge WinTV-dualHD DVB Bulk
+ * 2040:026d Hauppauge WinTV-dualHD ATSC/QAM Isoc
+ * 2040:826d Hauppauge WinTV-dualHD ATSC/QAM Bulk
* reg 0x80/0x84:
* GPIO_0: Yellow LED tuner 1, 0=on, 1=off
* GPIO_1: Green LED tuner 1, 0=on, 1=off
@@ -2391,7 +2393,8 @@ struct em28xx_board em28xx_boards[] = {
.has_dvb = 1,
},
/*
- * 2040:0265 Hauppauge WinTV-dualHD (DVB version).
+ * 2040:0265 Hauppauge WinTV-dualHD (DVB version) Isoc.
+ * 2040:8265 Hauppauge WinTV-dualHD (DVB version) Bulk.
* Empia EM28274, 2x Silicon Labs Si2168, 2x Silicon Labs Si2157
*/
[EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_DVB] = {
@@ -2407,7 +2410,8 @@ struct em28xx_board em28xx_boards[] = {
.leds = hauppauge_dualhd_leds,
},
/*
- * 2040:026d Hauppauge WinTV-dualHD (model 01595 - ATSC/QAM).
+ * 2040:026d Hauppauge WinTV-dualHD (model 01595 - ATSC/QAM) Isoc.
+ * 2040:826d Hauppauge WinTV-dualHD (model 01595 - ATSC/QAM) Bulk.
* Empia EM28274, 2x LG LGDT3306A, 2x Silicon Labs Si2157
*/
[EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_01595] = {
@@ -2549,8 +2553,12 @@ struct usb_device_id em28xx_id_table[] = {
.driver_info = EM2883_BOARD_HAUPPAUGE_WINTV_HVR_850 },
{ USB_DEVICE(0x2040, 0x0265),
.driver_info = EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_DVB },
+ { USB_DEVICE(0x2040, 0x8265),
+ .driver_info = EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_DVB },
{ USB_DEVICE(0x2040, 0x026d),
.driver_info = EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_01595 },
+ { USB_DEVICE(0x2040, 0x826d),
+ .driver_info = EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_01595 },
{ USB_DEVICE(0x0438, 0xb002),
.driver_info = EM2880_BOARD_AMD_ATI_TV_WONDER_HD_600 },
{ USB_DEVICE(0x2001, 0xf112),
@@ -2611,7 +2619,11 @@ struct usb_device_id em28xx_id_table[] = {
.driver_info = EM28178_BOARD_PCTV_461E },
{ USB_DEVICE(0x2013, 0x025f),
.driver_info = EM28178_BOARD_PCTV_292E },
- { USB_DEVICE(0x2040, 0x0264), /* Hauppauge WinTV-soloHD */
+ { USB_DEVICE(0x2040, 0x0264), /* Hauppauge WinTV-soloHD Isoc */
+ .driver_info = EM28178_BOARD_PCTV_292E },
+ { USB_DEVICE(0x2040, 0x8264), /* Hauppauge OEM Generic WinTV-soloHD Bulk */
+ .driver_info = EM28178_BOARD_PCTV_292E },
+ { USB_DEVICE(0x2040, 0x8268), /* Hauppauge Retail WinTV-soloHD Bulk */
.driver_info = EM28178_BOARD_PCTV_292E },
{ USB_DEVICE(0x0413, 0x6f07),
.driver_info = EM2861_BOARD_LEADTEK_VC100 },
--
2.7.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 5/9] em28xx: Add Hauppauge SoloHD/DualHD bulk models
2018-01-05 0:04 ` [PATCH 5/9] em28xx: Add Hauppauge SoloHD/DualHD bulk models Brad Love
@ 2018-01-05 0:24 ` Michael Ira Krufky
0 siblings, 0 replies; 32+ messages in thread
From: Michael Ira Krufky @ 2018-01-05 0:24 UTC (permalink / raw)
To: Brad Love; +Cc: linux-media
On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
> Add additional pids to driver list
>
> Signed-off-by: Brad Love <brad@nextdimension.cc>
:+1
Reviewed-by: Michael Ira Krufky <mkrufky@linuxtv.org>
> ---
> drivers/media/usb/em28xx/em28xx-cards.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
> index 7f5d0b28..34c693a 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -507,8 +507,10 @@ static struct em28xx_reg_seq plex_px_bcud[] = {
> };
>
> /*
> - * 2040:0265 Hauppauge WinTV-dualHD DVB
> - * 2040:026d Hauppauge WinTV-dualHD ATSC/QAM
> + * 2040:0265 Hauppauge WinTV-dualHD DVB Isoc
> + * 2040:8265 Hauppauge WinTV-dualHD DVB Bulk
> + * 2040:026d Hauppauge WinTV-dualHD ATSC/QAM Isoc
> + * 2040:826d Hauppauge WinTV-dualHD ATSC/QAM Bulk
> * reg 0x80/0x84:
> * GPIO_0: Yellow LED tuner 1, 0=on, 1=off
> * GPIO_1: Green LED tuner 1, 0=on, 1=off
> @@ -2391,7 +2393,8 @@ struct em28xx_board em28xx_boards[] = {
> .has_dvb = 1,
> },
> /*
> - * 2040:0265 Hauppauge WinTV-dualHD (DVB version).
> + * 2040:0265 Hauppauge WinTV-dualHD (DVB version) Isoc.
> + * 2040:8265 Hauppauge WinTV-dualHD (DVB version) Bulk.
> * Empia EM28274, 2x Silicon Labs Si2168, 2x Silicon Labs Si2157
> */
> [EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_DVB] = {
> @@ -2407,7 +2410,8 @@ struct em28xx_board em28xx_boards[] = {
> .leds = hauppauge_dualhd_leds,
> },
> /*
> - * 2040:026d Hauppauge WinTV-dualHD (model 01595 - ATSC/QAM).
> + * 2040:026d Hauppauge WinTV-dualHD (model 01595 - ATSC/QAM) Isoc.
> + * 2040:826d Hauppauge WinTV-dualHD (model 01595 - ATSC/QAM) Bulk.
> * Empia EM28274, 2x LG LGDT3306A, 2x Silicon Labs Si2157
> */
> [EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_01595] = {
> @@ -2549,8 +2553,12 @@ struct usb_device_id em28xx_id_table[] = {
> .driver_info = EM2883_BOARD_HAUPPAUGE_WINTV_HVR_850 },
> { USB_DEVICE(0x2040, 0x0265),
> .driver_info = EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_DVB },
> + { USB_DEVICE(0x2040, 0x8265),
> + .driver_info = EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_DVB },
> { USB_DEVICE(0x2040, 0x026d),
> .driver_info = EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_01595 },
> + { USB_DEVICE(0x2040, 0x826d),
> + .driver_info = EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_01595 },
> { USB_DEVICE(0x0438, 0xb002),
> .driver_info = EM2880_BOARD_AMD_ATI_TV_WONDER_HD_600 },
> { USB_DEVICE(0x2001, 0xf112),
> @@ -2611,7 +2619,11 @@ struct usb_device_id em28xx_id_table[] = {
> .driver_info = EM28178_BOARD_PCTV_461E },
> { USB_DEVICE(0x2013, 0x025f),
> .driver_info = EM28178_BOARD_PCTV_292E },
> - { USB_DEVICE(0x2040, 0x0264), /* Hauppauge WinTV-soloHD */
> + { USB_DEVICE(0x2040, 0x0264), /* Hauppauge WinTV-soloHD Isoc */
> + .driver_info = EM28178_BOARD_PCTV_292E },
> + { USB_DEVICE(0x2040, 0x8264), /* Hauppauge OEM Generic WinTV-soloHD Bulk */
> + .driver_info = EM28178_BOARD_PCTV_292E },
> + { USB_DEVICE(0x2040, 0x8268), /* Hauppauge Retail WinTV-soloHD Bulk */
> .driver_info = EM28178_BOARD_PCTV_292E },
> { USB_DEVICE(0x0413, 0x6f07),
> .driver_info = EM2861_BOARD_LEADTEK_VC100 },
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 6/9] em28xx: Enable Hauppauge SoloHD rebranded 292e SE
2018-01-05 0:04 [PATCH 0/9] Hauppauge em28xx/lgdt3306a soloHD/DualHD support Brad Love
` (4 preceding siblings ...)
2018-01-05 0:04 ` [PATCH 5/9] em28xx: Add Hauppauge SoloHD/DualHD bulk models Brad Love
@ 2018-01-05 0:04 ` Brad Love
2018-01-05 0:24 ` Michael Ira Krufky
2018-01-05 0:04 ` [PATCH 7/9] lgdt3306a: Set fe ops.release to NULL if probed Brad Love
` (2 subsequent siblings)
8 siblings, 1 reply; 32+ messages in thread
From: Brad Love @ 2018-01-05 0:04 UTC (permalink / raw)
To: linux-media; +Cc: Brad Love
Add a missing device to the driver table.
Signed-off-by: Brad Love <brad@nextdimension.cc>
---
drivers/media/usb/em28xx/em28xx-cards.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 34c693a..66d4c3a 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2619,6 +2619,8 @@ struct usb_device_id em28xx_id_table[] = {
.driver_info = EM28178_BOARD_PCTV_461E },
{ USB_DEVICE(0x2013, 0x025f),
.driver_info = EM28178_BOARD_PCTV_292E },
+ { USB_DEVICE(0x2013, 0x0264), /* Hauppauge WinTV-soloHD 292e SE */
+ .driver_info = EM28178_BOARD_PCTV_292E },
{ USB_DEVICE(0x2040, 0x0264), /* Hauppauge WinTV-soloHD Isoc */
.driver_info = EM28178_BOARD_PCTV_292E },
{ USB_DEVICE(0x2040, 0x8264), /* Hauppauge OEM Generic WinTV-soloHD Bulk */
--
2.7.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 6/9] em28xx: Enable Hauppauge SoloHD rebranded 292e SE
2018-01-05 0:04 ` [PATCH 6/9] em28xx: Enable Hauppauge SoloHD rebranded 292e SE Brad Love
@ 2018-01-05 0:24 ` Michael Ira Krufky
0 siblings, 0 replies; 32+ messages in thread
From: Michael Ira Krufky @ 2018-01-05 0:24 UTC (permalink / raw)
To: Brad Love; +Cc: linux-media
On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
> Add a missing device to the driver table.
>
> Signed-off-by: Brad Love <brad@nextdimension.cc>
:+1
Reviewed-by: Michael Ira Krufky <mkrufky@linuxtv.org>
> ---
> drivers/media/usb/em28xx/em28xx-cards.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
> index 34c693a..66d4c3a 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -2619,6 +2619,8 @@ struct usb_device_id em28xx_id_table[] = {
> .driver_info = EM28178_BOARD_PCTV_461E },
> { USB_DEVICE(0x2013, 0x025f),
> .driver_info = EM28178_BOARD_PCTV_292E },
> + { USB_DEVICE(0x2013, 0x0264), /* Hauppauge WinTV-soloHD 292e SE */
> + .driver_info = EM28178_BOARD_PCTV_292E },
> { USB_DEVICE(0x2040, 0x0264), /* Hauppauge WinTV-soloHD Isoc */
> .driver_info = EM28178_BOARD_PCTV_292E },
> { USB_DEVICE(0x2040, 0x8264), /* Hauppauge OEM Generic WinTV-soloHD Bulk */
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 7/9] lgdt3306a: Set fe ops.release to NULL if probed
2018-01-05 0:04 [PATCH 0/9] Hauppauge em28xx/lgdt3306a soloHD/DualHD support Brad Love
` (5 preceding siblings ...)
2018-01-05 0:04 ` [PATCH 6/9] em28xx: Enable Hauppauge SoloHD rebranded 292e SE Brad Love
@ 2018-01-05 0:04 ` Brad Love
2018-01-05 0:19 ` Michael Ira Krufky
2018-01-05 0:04 ` [PATCH 8/9] lgdt3306a: QAM streaming improvement Brad Love
2018-01-05 0:04 ` [PATCH 9/9] lgdt3306a: Add QAM AUTO support Brad Love
8 siblings, 1 reply; 32+ messages in thread
From: Brad Love @ 2018-01-05 0:04 UTC (permalink / raw)
To: linux-media; +Cc: Brad Love
If release is part of frontend ops then it is called in the
course of dvb_frontend_detach. The process also decrements
the module usage count. The problem is if the lgdt3306a
driver is reached via i2c_new_device, then when it is
eventually destroyed remove is called, which further
decrements the module usage count to negative. After this
occurs the driver is in a bad state and no longer works.
Also fixed by NULLing out the release callback is a double
kfree of state, which introduces arbitrary oopses/GPF.
This problem is only currently reachable via the em28xx driver.
On disconnect of Hauppauge SoloHD before:
lsmod | grep lgdt3306a
lgdt3306a 28672 -1
i2c_mux 16384 1 lgdt3306a
On disconnect of Hauppauge SoloHD after:
lsmod | grep lgdt3306a
lgdt3306a 28672 0
i2c_mux 16384 1 lgdt3306a
Signed-off-by: Brad Love <brad@nextdimension.cc>
---
drivers/media/dvb-frontends/lgdt3306a.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c
index 6356815..d2477ed 100644
--- a/drivers/media/dvb-frontends/lgdt3306a.c
+++ b/drivers/media/dvb-frontends/lgdt3306a.c
@@ -2177,6 +2177,7 @@ static int lgdt3306a_probe(struct i2c_client *client,
i2c_set_clientdata(client, fe->demodulator_priv);
state = fe->demodulator_priv;
+ state->frontend.ops.release = NULL;
/* create mux i2c adapter for tuner */
state->muxc = i2c_mux_alloc(client->adapter, &client->dev,
--
2.7.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 7/9] lgdt3306a: Set fe ops.release to NULL if probed
2018-01-05 0:04 ` [PATCH 7/9] lgdt3306a: Set fe ops.release to NULL if probed Brad Love
@ 2018-01-05 0:19 ` Michael Ira Krufky
2018-01-05 1:37 ` Brad Love
2018-01-09 5:17 ` Matthias Schwarzott
0 siblings, 2 replies; 32+ messages in thread
From: Michael Ira Krufky @ 2018-01-05 0:19 UTC (permalink / raw)
To: Brad Love; +Cc: linux-media
On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
> If release is part of frontend ops then it is called in the
> course of dvb_frontend_detach. The process also decrements
> the module usage count. The problem is if the lgdt3306a
> driver is reached via i2c_new_device, then when it is
> eventually destroyed remove is called, which further
> decrements the module usage count to negative. After this
> occurs the driver is in a bad state and no longer works.
> Also fixed by NULLing out the release callback is a double
> kfree of state, which introduces arbitrary oopses/GPF.
> This problem is only currently reachable via the em28xx driver.
>
> On disconnect of Hauppauge SoloHD before:
>
> lsmod | grep lgdt3306a
> lgdt3306a 28672 -1
> i2c_mux 16384 1 lgdt3306a
>
> On disconnect of Hauppauge SoloHD after:
>
> lsmod | grep lgdt3306a
> lgdt3306a 28672 0
> i2c_mux 16384 1 lgdt3306a
>
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
> drivers/media/dvb-frontends/lgdt3306a.c | 1 +
> 1 file changed, 1 insertion(+)
>
Brad,
We won't be able to apply this one. The symptom that you're trying to
fix is indicative of some other problem, probably in the em28xx
driver. NULL'ing the release callback is not the right thing to do.
-Mike Krufky
> diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c
> index 6356815..d2477ed 100644
> --- a/drivers/media/dvb-frontends/lgdt3306a.c
> +++ b/drivers/media/dvb-frontends/lgdt3306a.c
> @@ -2177,6 +2177,7 @@ static int lgdt3306a_probe(struct i2c_client *client,
>
> i2c_set_clientdata(client, fe->demodulator_priv);
> state = fe->demodulator_priv;
> + state->frontend.ops.release = NULL;
>
> /* create mux i2c adapter for tuner */
> state->muxc = i2c_mux_alloc(client->adapter, &client->dev,
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 7/9] lgdt3306a: Set fe ops.release to NULL if probed
2018-01-05 0:19 ` Michael Ira Krufky
@ 2018-01-05 1:37 ` Brad Love
2018-01-09 5:17 ` Matthias Schwarzott
1 sibling, 0 replies; 32+ messages in thread
From: Brad Love @ 2018-01-05 1:37 UTC (permalink / raw)
To: Michael Ira Krufky; +Cc: linux-media
On 2018-01-04 18:19, Michael Ira Krufky wrote:
> On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
>> If release is part of frontend ops then it is called in the
>> course of dvb_frontend_detach. The process also decrements
>> the module usage count. The problem is if the lgdt3306a
>> driver is reached via i2c_new_device, then when it is
>> eventually destroyed remove is called, which further
>> decrements the module usage count to negative. After this
>> occurs the driver is in a bad state and no longer works.
>> Also fixed by NULLing out the release callback is a double
>> kfree of state, which introduces arbitrary oopses/GPF.
>> This problem is only currently reachable via the em28xx driver.
>>
>> On disconnect of Hauppauge SoloHD before:
>>
>> lsmod | grep lgdt3306a
>> lgdt3306a 28672 -1
>> i2c_mux 16384 1 lgdt3306a
>>
>> On disconnect of Hauppauge SoloHD after:
>>
>> lsmod | grep lgdt3306a
>> lgdt3306a 28672 0
>> i2c_mux 16384 1 lgdt3306a
>>
>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>> ---
>> drivers/media/dvb-frontends/lgdt3306a.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
> Brad,
>
> We won't be able to apply this one. The symptom that you're trying to
> fix is indicative of some other problem, probably in the em28xx
> driver. NULL'ing the release callback is not the right thing to do.
>
> -Mike Krufky
Hey Mike,
Redacting this patch to deal with individually. I will separately send
to the list an example of another bridge driver which exhibits the same
issue, when using the lgdt3306a driver similarly. I don't think this is
solely an em28xx issue. I will also send a patch fixing only the double
free, as that seems to be most important.
Cheers,
Brad
>> diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c
>> index 6356815..d2477ed 100644
>> --- a/drivers/media/dvb-frontends/lgdt3306a.c
>> +++ b/drivers/media/dvb-frontends/lgdt3306a.c
>> @@ -2177,6 +2177,7 @@ static int lgdt3306a_probe(struct i2c_client *client,
>>
>> i2c_set_clientdata(client, fe->demodulator_priv);
>> state = fe->demodulator_priv;
>> + state->frontend.ops.release = NULL;
>>
>> /* create mux i2c adapter for tuner */
>> state->muxc = i2c_mux_alloc(client->adapter, &client->dev,
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 7/9] lgdt3306a: Set fe ops.release to NULL if probed
2018-01-05 0:19 ` Michael Ira Krufky
2018-01-05 1:37 ` Brad Love
@ 2018-01-09 5:17 ` Matthias Schwarzott
2018-01-09 13:00 ` Michael Ira Krufky
1 sibling, 1 reply; 32+ messages in thread
From: Matthias Schwarzott @ 2018-01-09 5:17 UTC (permalink / raw)
To: Michael Ira Krufky, Brad Love; +Cc: linux-media
Am 05.01.2018 um 01:19 schrieb Michael Ira Krufky:
> On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
>> If release is part of frontend ops then it is called in the
>> course of dvb_frontend_detach. The process also decrements
>> the module usage count. The problem is if the lgdt3306a
>> driver is reached via i2c_new_device, then when it is
>> eventually destroyed remove is called, which further
>> decrements the module usage count to negative. After this
>> occurs the driver is in a bad state and no longer works.
>> Also fixed by NULLing out the release callback is a double
>> kfree of state, which introduces arbitrary oopses/GPF.
>> This problem is only currently reachable via the em28xx driver.
>>
>> On disconnect of Hauppauge SoloHD before:
>>
>> lsmod | grep lgdt3306a
>> lgdt3306a 28672 -1
>> i2c_mux 16384 1 lgdt3306a
>>
>> On disconnect of Hauppauge SoloHD after:
>>
>> lsmod | grep lgdt3306a
>> lgdt3306a 28672 0
>> i2c_mux 16384 1 lgdt3306a
>>
>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>> ---
>> drivers/media/dvb-frontends/lgdt3306a.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>
> Brad,
>
> We won't be able to apply this one. The symptom that you're trying to
> fix is indicative of some other problem, probably in the em28xx
> driver. NULL'ing the release callback is not the right thing to do.
>
Hi Mike,
Why do you nak this perfectly fine patch?
Let me start to explain.
dvb_attach style drivers have an attach function and for unloading a
.release callback.
i2c-style drivers have a probe and a remove function.
Mixed style drivers must be constructed, that either release or remove
is called, never both.
They both do the same thing, but with different signature.
Now checking lgdt3306a driver:
dvb attach style:
In lgdt3306a_attach the release callback is set to lgdt3306a_release and
no remove exists. Fine.
i2c style probe:
struct i2c_driver contains lgdt3306a_probe and lgdt3306a_remove.
lgdt3306a_probe shares code and calls lgdt3306a_attach, but afterwards
the release callback field must be set to NULL.
This is/was done exactly like this in multiple other drivers as long as
they have been multiple style attachable.
Regards
Matthias
>
>> diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c
>> index 6356815..d2477ed 100644
>> --- a/drivers/media/dvb-frontends/lgdt3306a.c
>> +++ b/drivers/media/dvb-frontends/lgdt3306a.c
>> @@ -2177,6 +2177,7 @@ static int lgdt3306a_probe(struct i2c_client *client,
>>
>> i2c_set_clientdata(client, fe->demodulator_priv);
>> state = fe->demodulator_priv;
>> + state->frontend.ops.release = NULL;
>>
>> /* create mux i2c adapter for tuner */
>> state->muxc = i2c_mux_alloc(client->adapter, &client->dev,
>> --
>> 2.7.4
>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 7/9] lgdt3306a: Set fe ops.release to NULL if probed
2018-01-09 5:17 ` Matthias Schwarzott
@ 2018-01-09 13:00 ` Michael Ira Krufky
0 siblings, 0 replies; 32+ messages in thread
From: Michael Ira Krufky @ 2018-01-09 13:00 UTC (permalink / raw)
To: Matthias Schwarzott; +Cc: Brad Love, linux-media
On Tue, Jan 9, 2018 at 12:17 AM, Matthias Schwarzott <zzam@gentoo.org> wrote:
> Am 05.01.2018 um 01:19 schrieb Michael Ira Krufky:
>> On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
>>> If release is part of frontend ops then it is called in the
>>> course of dvb_frontend_detach. The process also decrements
>>> the module usage count. The problem is if the lgdt3306a
>>> driver is reached via i2c_new_device, then when it is
>>> eventually destroyed remove is called, which further
>>> decrements the module usage count to negative. After this
>>> occurs the driver is in a bad state and no longer works.
>>> Also fixed by NULLing out the release callback is a double
>>> kfree of state, which introduces arbitrary oopses/GPF.
>>> This problem is only currently reachable via the em28xx driver.
>>>
>>> On disconnect of Hauppauge SoloHD before:
>>>
>>> lsmod | grep lgdt3306a
>>> lgdt3306a 28672 -1
>>> i2c_mux 16384 1 lgdt3306a
>>>
>>> On disconnect of Hauppauge SoloHD after:
>>>
>>> lsmod | grep lgdt3306a
>>> lgdt3306a 28672 0
>>> i2c_mux 16384 1 lgdt3306a
>>>
>>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>>> ---
>>> drivers/media/dvb-frontends/lgdt3306a.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>
>> Brad,
>>
>> We won't be able to apply this one. The symptom that you're trying to
>> fix is indicative of some other problem, probably in the em28xx
>> driver. NULL'ing the release callback is not the right thing to do.
>>
>
> Hi Mike,
> Why do you nak this perfectly fine patch?
> Let me start to explain.
>
> dvb_attach style drivers have an attach function and for unloading a
> .release callback.
>
> i2c-style drivers have a probe and a remove function.
>
> Mixed style drivers must be constructed, that either release or remove
> is called, never both.
> They both do the same thing, but with different signature.
>
>
> Now checking lgdt3306a driver:
>
> dvb attach style:
> In lgdt3306a_attach the release callback is set to lgdt3306a_release and
> no remove exists. Fine.
>
> i2c style probe:
> struct i2c_driver contains lgdt3306a_probe and lgdt3306a_remove.
> lgdt3306a_probe shares code and calls lgdt3306a_attach, but afterwards
> the release callback field must be set to NULL.
>
> This is/was done exactly like this in multiple other drivers as long as
> they have been multiple style attachable.
>
> Regards
> Matthias
>
>>
>>> diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c
>>> index 6356815..d2477ed 100644
>>> --- a/drivers/media/dvb-frontends/lgdt3306a.c
>>> +++ b/drivers/media/dvb-frontends/lgdt3306a.c
>>> @@ -2177,6 +2177,7 @@ static int lgdt3306a_probe(struct i2c_client *client,
>>>
>>> i2c_set_clientdata(client, fe->demodulator_priv);
>>> state = fe->demodulator_priv;
>>> + state->frontend.ops.release = NULL;
>>>
>>> /* create mux i2c adapter for tuner */
>>> state->muxc = i2c_mux_alloc(client->adapter, &client->dev,
>>> --
>>> 2.7.4
>>>
>>
>
Brad & Matthias,
It makes sense. This is just a result of a transitional period where
there are multiple APIs for attaching the driver. Hopefully we can
consolidate soon.
I will look at the other drivers when I have time later on, but you're
probably right, and we will likely end up merging this one after all.
I'll try to get a PR out to Mauro by the weekend.
-Mike
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 8/9] lgdt3306a: QAM streaming improvement
2018-01-05 0:04 [PATCH 0/9] Hauppauge em28xx/lgdt3306a soloHD/DualHD support Brad Love
` (6 preceding siblings ...)
2018-01-05 0:04 ` [PATCH 7/9] lgdt3306a: Set fe ops.release to NULL if probed Brad Love
@ 2018-01-05 0:04 ` Brad Love
2018-01-05 0:26 ` Michael Ira Krufky
2018-01-05 1:30 ` [PATCH v2 " Brad Love
2018-01-05 0:04 ` [PATCH 9/9] lgdt3306a: Add QAM AUTO support Brad Love
8 siblings, 2 replies; 32+ messages in thread
From: Brad Love @ 2018-01-05 0:04 UTC (permalink / raw)
To: linux-media; +Cc: Brad Love
Add some register updates required for stable viewing
on Cablevision in NY. Does not adversely affect other providers.
Signed-off-by: Brad Love <brad@nextdimension.cc>
---
drivers/media/dvb-frontends/lgdt3306a.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c
index d2477ed..2f540f1 100644
--- a/drivers/media/dvb-frontends/lgdt3306a.c
+++ b/drivers/media/dvb-frontends/lgdt3306a.c
@@ -598,6 +598,28 @@ static int lgdt3306a_set_qam(struct lgdt3306a_state *state, int modulation)
if (lg_chkerr(ret))
goto fail;
+ /* 5.1 V0.36 SRDCHKALWAYS : For better QAM detection */
+ ret = lgdt3306a_read_reg(state, 0x000A, &val);
+ val &= 0xFD;
+ val |= 0x02;
+ ret = lgdt3306a_write_reg(state, 0x000A, val);
+ if (lg_chkerr(ret))
+ goto fail;
+
+ /* 5.2 V0.36 Control of "no signal" detector function */
+ ret = lgdt3306a_read_reg(state, 0x2849, &val);
+ val &= 0xDF;
+ ret = lgdt3306a_write_reg(state, 0x2849, val);
+ if (lg_chkerr(ret))
+ goto fail;
+
+ /* 5.3 Fix for Blonder Tongue HDE-2H-QAM and AQM modulators */
+ ret = lgdt3306a_read_reg(state, 0x302B, &val);
+ val &= 0x7F; /* SELFSYNCFINDEN_CQS=0; disable auto reset */
+ ret = lgdt3306a_write_reg(state, 0x302B, val);
+ if (lg_chkerr(ret))
+ goto fail;
+
/* 6. Reset */
ret = lgdt3306a_soft_reset(state);
if (lg_chkerr(ret))
--
2.7.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 8/9] lgdt3306a: QAM streaming improvement
2018-01-05 0:04 ` [PATCH 8/9] lgdt3306a: QAM streaming improvement Brad Love
@ 2018-01-05 0:26 ` Michael Ira Krufky
2018-01-05 1:34 ` Brad Love
2018-01-05 1:30 ` [PATCH v2 " Brad Love
1 sibling, 1 reply; 32+ messages in thread
From: Michael Ira Krufky @ 2018-01-05 0:26 UTC (permalink / raw)
To: Brad Love; +Cc: linux-media
On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
> Add some register updates required for stable viewing
> on Cablevision in NY. Does not adversely affect other providers.
>
> Signed-off-by: Brad Love <brad@nextdimension.cc>
> ---
> drivers/media/dvb-frontends/lgdt3306a.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c
> index d2477ed..2f540f1 100644
> --- a/drivers/media/dvb-frontends/lgdt3306a.c
> +++ b/drivers/media/dvb-frontends/lgdt3306a.c
> @@ -598,6 +598,28 @@ static int lgdt3306a_set_qam(struct lgdt3306a_state *state, int modulation)
> if (lg_chkerr(ret))
> goto fail;
>
> + /* 5.1 V0.36 SRDCHKALWAYS : For better QAM detection */
> + ret = lgdt3306a_read_reg(state, 0x000A, &val);
> + val &= 0xFD;
> + val |= 0x02;
> + ret = lgdt3306a_write_reg(state, 0x000A, val);
> + if (lg_chkerr(ret))
> + goto fail;
> +
> + /* 5.2 V0.36 Control of "no signal" detector function */
> + ret = lgdt3306a_read_reg(state, 0x2849, &val);
> + val &= 0xDF;
> + ret = lgdt3306a_write_reg(state, 0x2849, val);
> + if (lg_chkerr(ret))
> + goto fail;
> +
> + /* 5.3 Fix for Blonder Tongue HDE-2H-QAM and AQM modulators */
> + ret = lgdt3306a_read_reg(state, 0x302B, &val);
> + val &= 0x7F; /* SELFSYNCFINDEN_CQS=0; disable auto reset */
> + ret = lgdt3306a_write_reg(state, 0x302B, val);
> + if (lg_chkerr(ret))
> + goto fail;
> +
> /* 6. Reset */
> ret = lgdt3306a_soft_reset(state);
> if (lg_chkerr(ret))
Brad,
The change looks good, but can you resubmit this using lowercase hex?
Cheers,
Michael Ira Krufky
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 8/9] lgdt3306a: QAM streaming improvement
2018-01-05 0:26 ` Michael Ira Krufky
@ 2018-01-05 1:34 ` Brad Love
0 siblings, 0 replies; 32+ messages in thread
From: Brad Love @ 2018-01-05 1:34 UTC (permalink / raw)
To: Michael Ira Krufky; +Cc: linux-media
On 2018-01-04 18:26, Michael Ira Krufky wrote:
> On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
>> Add some register updates required for stable viewing
>> on Cablevision in NY. Does not adversely affect other providers.
>>
>> Signed-off-by: Brad Love <brad@nextdimension.cc>
>> ---
>> drivers/media/dvb-frontends/lgdt3306a.c | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c
>> index d2477ed..2f540f1 100644
>> --- a/drivers/media/dvb-frontends/lgdt3306a.c
>> +++ b/drivers/media/dvb-frontends/lgdt3306a.c
>> @@ -598,6 +598,28 @@ static int lgdt3306a_set_qam(struct lgdt3306a_state *state, int modulation)
>> if (lg_chkerr(ret))
>> goto fail;
>>
>> + /* 5.1 V0.36 SRDCHKALWAYS : For better QAM detection */
>> + ret = lgdt3306a_read_reg(state, 0x000A, &val);
>> + val &= 0xFD;
>> + val |= 0x02;
>> + ret = lgdt3306a_write_reg(state, 0x000A, val);
>> + if (lg_chkerr(ret))
>> + goto fail;
>> +
>> + /* 5.2 V0.36 Control of "no signal" detector function */
>> + ret = lgdt3306a_read_reg(state, 0x2849, &val);
>> + val &= 0xDF;
>> + ret = lgdt3306a_write_reg(state, 0x2849, val);
>> + if (lg_chkerr(ret))
>> + goto fail;
>> +
>> + /* 5.3 Fix for Blonder Tongue HDE-2H-QAM and AQM modulators */
>> + ret = lgdt3306a_read_reg(state, 0x302B, &val);
>> + val &= 0x7F; /* SELFSYNCFINDEN_CQS=0; disable auto reset */
>> + ret = lgdt3306a_write_reg(state, 0x302B, val);
>> + if (lg_chkerr(ret))
>> + goto fail;
>> +
>> /* 6. Reset */
>> ret = lgdt3306a_soft_reset(state);
>> if (lg_chkerr(ret))
> Brad,
>
> The change looks good, but can you resubmit this using lowercase hex?
>
> Cheers,
>
> Michael Ira Krufky
Done, v2 submitted.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 8/9] lgdt3306a: QAM streaming improvement
2018-01-05 0:04 ` [PATCH 8/9] lgdt3306a: QAM streaming improvement Brad Love
2018-01-05 0:26 ` Michael Ira Krufky
@ 2018-01-05 1:30 ` Brad Love
2018-01-05 11:57 ` Michael Ira Krufky
1 sibling, 1 reply; 32+ messages in thread
From: Brad Love @ 2018-01-05 1:30 UTC (permalink / raw)
To: linux-media; +Cc: Brad Love
Add some register updates required for stable viewing
on Cablevision in NY. Does not adversely affect other providers.
Changes since v1:
- Change upper case hex to lower case.
Signed-off-by: Brad Love <brad@nextdimension.cc>
---
drivers/media/dvb-frontends/lgdt3306a.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c
index d2477ed..2f540f1 100644
--- a/drivers/media/dvb-frontends/lgdt3306a.c
+++ b/drivers/media/dvb-frontends/lgdt3306a.c
@@ -598,6 +598,28 @@ static int lgdt3306a_set_qam(struct lgdt3306a_state *state, int modulation)
if (lg_chkerr(ret))
goto fail;
+ /* 5.1 V0.36 SRDCHKALWAYS : For better QAM detection */
+ ret = lgdt3306a_read_reg(state, 0x000a, &val);
+ val &= 0xfd;
+ val |= 0x02;
+ ret = lgdt3306a_write_reg(state, 0x000a, val);
+ if (lg_chkerr(ret))
+ goto fail;
+
+ /* 5.2 V0.36 Control of "no signal" detector function */
+ ret = lgdt3306a_read_reg(state, 0x2849, &val);
+ val &= 0xdf;
+ ret = lgdt3306a_write_reg(state, 0x2849, val);
+ if (lg_chkerr(ret))
+ goto fail;
+
+ /* 5.3 Fix for Blonder Tongue HDE-2H-QAM and AQM modulators */
+ ret = lgdt3306a_read_reg(state, 0x302b, &val);
+ val &= 0x7f; /* SELFSYNCFINDEN_CQS=0; disable auto reset */
+ ret = lgdt3306a_write_reg(state, 0x302b, val);
+ if (lg_chkerr(ret))
+ goto fail;
+
/* 6. Reset */
ret = lgdt3306a_soft_reset(state);
if (lg_chkerr(ret))
--
2.7.4
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v2 8/9] lgdt3306a: QAM streaming improvement
2018-01-05 1:30 ` [PATCH v2 " Brad Love
@ 2018-01-05 11:57 ` Michael Ira Krufky
0 siblings, 0 replies; 32+ messages in thread
From: Michael Ira Krufky @ 2018-01-05 11:57 UTC (permalink / raw)
To: Brad Love; +Cc: linux-media
On Thu, Jan 4, 2018 at 8:30 PM, Brad Love <brad@nextdimension.cc> wrote:
> Add some register updates required for stable viewing
> on Cablevision in NY. Does not adversely affect other providers.
>
> Changes since v1:
> - Change upper case hex to lower case.
>
> Signed-off-by: Brad Love <brad@nextdimension.cc>
Looks good - Thanks.
Reviewed-by: Michael Ira Krufky <mkrufky@linuxtv.org>
> ---
> drivers/media/dvb-frontends/lgdt3306a.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c
> index d2477ed..2f540f1 100644
> --- a/drivers/media/dvb-frontends/lgdt3306a.c
> +++ b/drivers/media/dvb-frontends/lgdt3306a.c
> @@ -598,6 +598,28 @@ static int lgdt3306a_set_qam(struct lgdt3306a_state *state, int modulation)
> if (lg_chkerr(ret))
> goto fail;
>
> + /* 5.1 V0.36 SRDCHKALWAYS : For better QAM detection */
> + ret = lgdt3306a_read_reg(state, 0x000a, &val);
> + val &= 0xfd;
> + val |= 0x02;
> + ret = lgdt3306a_write_reg(state, 0x000a, val);
> + if (lg_chkerr(ret))
> + goto fail;
> +
> + /* 5.2 V0.36 Control of "no signal" detector function */
> + ret = lgdt3306a_read_reg(state, 0x2849, &val);
> + val &= 0xdf;
> + ret = lgdt3306a_write_reg(state, 0x2849, val);
> + if (lg_chkerr(ret))
> + goto fail;
> +
> + /* 5.3 Fix for Blonder Tongue HDE-2H-QAM and AQM modulators */
> + ret = lgdt3306a_read_reg(state, 0x302b, &val);
> + val &= 0x7f; /* SELFSYNCFINDEN_CQS=0; disable auto reset */
> + ret = lgdt3306a_write_reg(state, 0x302b, val);
> + if (lg_chkerr(ret))
> + goto fail;
> +
> /* 6. Reset */
> ret = lgdt3306a_soft_reset(state);
> if (lg_chkerr(ret))
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 9/9] lgdt3306a: Add QAM AUTO support
2018-01-05 0:04 [PATCH 0/9] Hauppauge em28xx/lgdt3306a soloHD/DualHD support Brad Love
` (7 preceding siblings ...)
2018-01-05 0:04 ` [PATCH 8/9] lgdt3306a: QAM streaming improvement Brad Love
@ 2018-01-05 0:04 ` Brad Love
2018-01-05 0:27 ` Michael Ira Krufky
2018-02-12 20:52 ` [PATCH v2 " Brad Love
8 siblings, 2 replies; 32+ messages in thread
From: Brad Love @ 2018-01-05 0:04 UTC (permalink / raw)
To: linux-media; +Cc: Brad Love
As configured currently, modulation in the driver is set to auto detect,
no matter what the user sets modulation to. This leads to both QAM64
and QAM256 having the same effect. QAM AUTO is explicitly added here for
compatibility with scanning software who can use AUTO instead of doing
essentially the same scan twice.
Also included is a module option to enforce a specific QAM modulation if
desired. The true modulation is read before calculating the snr.
Changes are backwards compatible with current behaviour.
Signed-off-by: Brad Love <brad@nextdimension.cc>
---
drivers/media/dvb-frontends/lgdt3306a.c | 42 ++++++++++++++++++++++++++-------
1 file changed, 33 insertions(+), 9 deletions(-)
diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c
index 2f540f1..111efb0 100644
--- a/drivers/media/dvb-frontends/lgdt3306a.c
+++ b/drivers/media/dvb-frontends/lgdt3306a.c
@@ -30,6 +30,17 @@ static int debug;
module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "set debug level (info=1, reg=2 (or-able))");
+/*
+ * Older drivers treated QAM64 and QAM256 the same; that is the HW always
+ * used "Auto" mode during detection. Setting "forced_manual"=1 allows
+ * the user to treat these modes as separate. For backwards compatibility,
+ * it's off by default. QAM_AUTO can now be specified to achive that
+ * effect even if "forced_manual"=1
+ */
+static int forced_manual;
+module_param(forced_manual, int, 0644);
+MODULE_PARM_DESC(forced_manual, "if set, QAM64 and QAM256 will only lock to modulation specified");
+
#define DBG_INFO 1
#define DBG_REG 2
#define DBG_DUMP 4 /* FGR - comment out to remove dump code */
@@ -566,7 +577,12 @@ static int lgdt3306a_set_qam(struct lgdt3306a_state *state, int modulation)
/* 3. : 64QAM/256QAM detection(manual, auto) */
ret = lgdt3306a_read_reg(state, 0x0009, &val);
val &= 0xfc;
- val |= 0x02; /* STDOPDETCMODE[1:0]=1=Manual 2=Auto */
+ /* Check for forced Manual modulation modes; otherwise always "auto" */
+ if(forced_manual && (modulation != QAM_AUTO)){
+ val |= 0x01; /* STDOPDETCMODE[1:0]= 1=Manual */
+ } else {
+ val |= 0x02; /* STDOPDETCMODE[1:0]= 2=Auto */
+ }
ret = lgdt3306a_write_reg(state, 0x0009, val);
if (lg_chkerr(ret))
goto fail;
@@ -642,10 +658,9 @@ static int lgdt3306a_set_modulation(struct lgdt3306a_state *state,
ret = lgdt3306a_set_vsb(state);
break;
case QAM_64:
- ret = lgdt3306a_set_qam(state, QAM_64);
- break;
case QAM_256:
- ret = lgdt3306a_set_qam(state, QAM_256);
+ case QAM_AUTO:
+ ret = lgdt3306a_set_qam(state, p->modulation);
break;
default:
return -EINVAL;
@@ -672,6 +687,7 @@ static int lgdt3306a_agc_setup(struct lgdt3306a_state *state,
break;
case QAM_64:
case QAM_256:
+ case QAM_AUTO:
break;
default:
return -EINVAL;
@@ -726,6 +742,7 @@ static int lgdt3306a_spectral_inversion(struct lgdt3306a_state *state,
break;
case QAM_64:
case QAM_256:
+ case QAM_AUTO:
/* Auto ok for QAM */
ret = lgdt3306a_set_inversion_auto(state, 1);
break;
@@ -749,6 +766,7 @@ static int lgdt3306a_set_if(struct lgdt3306a_state *state,
break;
case QAM_64:
case QAM_256:
+ case QAM_AUTO:
if_freq_khz = state->cfg->qam_if_khz;
break;
default:
@@ -1607,6 +1625,7 @@ static int lgdt3306a_read_status(struct dvb_frontend *fe,
switch (state->current_modulation) {
case QAM_256:
case QAM_64:
+ case QAM_AUTO:
if (lgdt3306a_qam_lock_poll(state) == LG3306_LOCK) {
*status |= FE_HAS_VITERBI;
*status |= FE_HAS_SYNC;
@@ -1650,6 +1669,7 @@ static int lgdt3306a_read_signal_strength(struct dvb_frontend *fe,
* Calculate some sort of "strength" from SNR
*/
struct lgdt3306a_state *state = fe->demodulator_priv;
+ u8 val;
u16 snr; /* snr_x10 */
int ret;
u32 ref_snr; /* snr*100 */
@@ -1662,11 +1682,15 @@ static int lgdt3306a_read_signal_strength(struct dvb_frontend *fe,
ref_snr = 1600; /* 16dB */
break;
case QAM_64:
- ref_snr = 2200; /* 22dB */
- break;
case QAM_256:
- ref_snr = 2800; /* 28dB */
- break;
+ case QAM_AUTO:
+ /* need to know actual modulation to set proper SNR baseline */
+ lgdt3306a_read_reg(state, 0x00a6, &val);
+ if(val & 0x04)
+ ref_snr = 2800; /* QAM-256 28dB */
+ else
+ ref_snr = 2200; /* QAM-64 22dB */
+ break;
default:
return -EINVAL;
}
@@ -2136,7 +2160,7 @@ static const struct dvb_frontend_ops lgdt3306a_ops = {
.frequency_min = 54000000,
.frequency_max = 858000000,
.frequency_stepsize = 62500,
- .caps = FE_CAN_QAM_64 | FE_CAN_QAM_256 | FE_CAN_8VSB
+ .caps = FE_CAN_QAM_AUTO | FE_CAN_QAM_64 | FE_CAN_QAM_256 | FE_CAN_8VSB
},
.i2c_gate_ctrl = lgdt3306a_i2c_gate_ctrl,
.init = lgdt3306a_init,
--
2.7.4
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [PATCH 9/9] lgdt3306a: Add QAM AUTO support
2018-01-05 0:04 ` [PATCH 9/9] lgdt3306a: Add QAM AUTO support Brad Love
@ 2018-01-05 0:27 ` Michael Ira Krufky
2018-02-12 20:52 ` [PATCH v2 " Brad Love
1 sibling, 0 replies; 32+ messages in thread
From: Michael Ira Krufky @ 2018-01-05 0:27 UTC (permalink / raw)
To: Brad Love; +Cc: linux-media
On Thu, Jan 4, 2018 at 7:04 PM, Brad Love <brad@nextdimension.cc> wrote:
> As configured currently, modulation in the driver is set to auto detect,
> no matter what the user sets modulation to. This leads to both QAM64
> and QAM256 having the same effect. QAM AUTO is explicitly added here for
> compatibility with scanning software who can use AUTO instead of doing
> essentially the same scan twice.
> Also included is a module option to enforce a specific QAM modulation if
> desired. The true modulation is read before calculating the snr.
> Changes are backwards compatible with current behaviour.
>
> Signed-off-by: Brad Love <brad@nextdimension.cc>
:+1
Reviewed-by: Michael Ira Krufky <mkrufky@linuxtv.org>
> ---
> drivers/media/dvb-frontends/lgdt3306a.c | 42 ++++++++++++++++++++++++++-------
> 1 file changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c
> index 2f540f1..111efb0 100644
> --- a/drivers/media/dvb-frontends/lgdt3306a.c
> +++ b/drivers/media/dvb-frontends/lgdt3306a.c
> @@ -30,6 +30,17 @@ static int debug;
> module_param(debug, int, 0644);
> MODULE_PARM_DESC(debug, "set debug level (info=1, reg=2 (or-able))");
>
> +/*
> + * Older drivers treated QAM64 and QAM256 the same; that is the HW always
> + * used "Auto" mode during detection. Setting "forced_manual"=1 allows
> + * the user to treat these modes as separate. For backwards compatibility,
> + * it's off by default. QAM_AUTO can now be specified to achive that
> + * effect even if "forced_manual"=1
> + */
> +static int forced_manual;
> +module_param(forced_manual, int, 0644);
> +MODULE_PARM_DESC(forced_manual, "if set, QAM64 and QAM256 will only lock to modulation specified");
> +
> #define DBG_INFO 1
> #define DBG_REG 2
> #define DBG_DUMP 4 /* FGR - comment out to remove dump code */
> @@ -566,7 +577,12 @@ static int lgdt3306a_set_qam(struct lgdt3306a_state *state, int modulation)
> /* 3. : 64QAM/256QAM detection(manual, auto) */
> ret = lgdt3306a_read_reg(state, 0x0009, &val);
> val &= 0xfc;
> - val |= 0x02; /* STDOPDETCMODE[1:0]=1=Manual 2=Auto */
> + /* Check for forced Manual modulation modes; otherwise always "auto" */
> + if(forced_manual && (modulation != QAM_AUTO)){
> + val |= 0x01; /* STDOPDETCMODE[1:0]= 1=Manual */
> + } else {
> + val |= 0x02; /* STDOPDETCMODE[1:0]= 2=Auto */
> + }
> ret = lgdt3306a_write_reg(state, 0x0009, val);
> if (lg_chkerr(ret))
> goto fail;
> @@ -642,10 +658,9 @@ static int lgdt3306a_set_modulation(struct lgdt3306a_state *state,
> ret = lgdt3306a_set_vsb(state);
> break;
> case QAM_64:
> - ret = lgdt3306a_set_qam(state, QAM_64);
> - break;
> case QAM_256:
> - ret = lgdt3306a_set_qam(state, QAM_256);
> + case QAM_AUTO:
> + ret = lgdt3306a_set_qam(state, p->modulation);
> break;
> default:
> return -EINVAL;
> @@ -672,6 +687,7 @@ static int lgdt3306a_agc_setup(struct lgdt3306a_state *state,
> break;
> case QAM_64:
> case QAM_256:
> + case QAM_AUTO:
> break;
> default:
> return -EINVAL;
> @@ -726,6 +742,7 @@ static int lgdt3306a_spectral_inversion(struct lgdt3306a_state *state,
> break;
> case QAM_64:
> case QAM_256:
> + case QAM_AUTO:
> /* Auto ok for QAM */
> ret = lgdt3306a_set_inversion_auto(state, 1);
> break;
> @@ -749,6 +766,7 @@ static int lgdt3306a_set_if(struct lgdt3306a_state *state,
> break;
> case QAM_64:
> case QAM_256:
> + case QAM_AUTO:
> if_freq_khz = state->cfg->qam_if_khz;
> break;
> default:
> @@ -1607,6 +1625,7 @@ static int lgdt3306a_read_status(struct dvb_frontend *fe,
> switch (state->current_modulation) {
> case QAM_256:
> case QAM_64:
> + case QAM_AUTO:
> if (lgdt3306a_qam_lock_poll(state) == LG3306_LOCK) {
> *status |= FE_HAS_VITERBI;
> *status |= FE_HAS_SYNC;
> @@ -1650,6 +1669,7 @@ static int lgdt3306a_read_signal_strength(struct dvb_frontend *fe,
> * Calculate some sort of "strength" from SNR
> */
> struct lgdt3306a_state *state = fe->demodulator_priv;
> + u8 val;
> u16 snr; /* snr_x10 */
> int ret;
> u32 ref_snr; /* snr*100 */
> @@ -1662,11 +1682,15 @@ static int lgdt3306a_read_signal_strength(struct dvb_frontend *fe,
> ref_snr = 1600; /* 16dB */
> break;
> case QAM_64:
> - ref_snr = 2200; /* 22dB */
> - break;
> case QAM_256:
> - ref_snr = 2800; /* 28dB */
> - break;
> + case QAM_AUTO:
> + /* need to know actual modulation to set proper SNR baseline */
> + lgdt3306a_read_reg(state, 0x00a6, &val);
> + if(val & 0x04)
> + ref_snr = 2800; /* QAM-256 28dB */
> + else
> + ref_snr = 2200; /* QAM-64 22dB */
> + break;
> default:
> return -EINVAL;
> }
> @@ -2136,7 +2160,7 @@ static const struct dvb_frontend_ops lgdt3306a_ops = {
> .frequency_min = 54000000,
> .frequency_max = 858000000,
> .frequency_stepsize = 62500,
> - .caps = FE_CAN_QAM_64 | FE_CAN_QAM_256 | FE_CAN_8VSB
> + .caps = FE_CAN_QAM_AUTO | FE_CAN_QAM_64 | FE_CAN_QAM_256 | FE_CAN_8VSB
> },
> .i2c_gate_ctrl = lgdt3306a_i2c_gate_ctrl,
> .init = lgdt3306a_init,
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 32+ messages in thread* [PATCH v2 9/9] lgdt3306a: Add QAM AUTO support
2018-01-05 0:04 ` [PATCH 9/9] lgdt3306a: Add QAM AUTO support Brad Love
2018-01-05 0:27 ` Michael Ira Krufky
@ 2018-02-12 20:52 ` Brad Love
1 sibling, 0 replies; 32+ messages in thread
From: Brad Love @ 2018-02-12 20:52 UTC (permalink / raw)
To: linux-media; +Cc: Brad Love
As configured currently, modulation in the driver is set to auto detect,
no matter what the user sets modulation to. This leads to both QAM64
and QAM256 having the same effect. QAM AUTO is explicitly added here for
compatibility with scanning software who can use AUTO instead of doing
essentially the same scan twice.
Also included is a module option to enforce a specific QAM modulation if
desired. The true modulation is read before calculating the snr.
Changes are backwards compatible with current behaviour.
Signed-off-by: Brad Love <brad@nextdimension.cc>
---
Changes since v1:
- fixed checkpatch complaint
drivers/media/dvb-frontends/lgdt3306a.c | 41 +++++++++++++++++++++++++--------
1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/media/dvb-frontends/lgdt3306a.c b/drivers/media/dvb-frontends/lgdt3306a.c
index 377271d..ed70369 100644
--- a/drivers/media/dvb-frontends/lgdt3306a.c
+++ b/drivers/media/dvb-frontends/lgdt3306a.c
@@ -30,6 +30,17 @@ static int debug;
module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "set debug level (info=1, reg=2 (or-able))");
+/*
+ * Older drivers treated QAM64 and QAM256 the same; that is the HW always
+ * used "Auto" mode during detection. Setting "forced_manual"=1 allows
+ * the user to treat these modes as separate. For backwards compatibility,
+ * it's off by default. QAM_AUTO can now be specified to achive that
+ * effect even if "forced_manual"=1
+ */
+static int forced_manual;
+module_param(forced_manual, int, 0644);
+MODULE_PARM_DESC(forced_manual, "if set, QAM64 and QAM256 will only lock to modulation specified");
+
#define DBG_INFO 1
#define DBG_REG 2
#define DBG_DUMP 4 /* FGR - comment out to remove dump code */
@@ -566,7 +577,11 @@ static int lgdt3306a_set_qam(struct lgdt3306a_state *state, int modulation)
/* 3. : 64QAM/256QAM detection(manual, auto) */
ret = lgdt3306a_read_reg(state, 0x0009, &val);
val &= 0xfc;
- val |= 0x02; /* STDOPDETCMODE[1:0]=1=Manual 2=Auto */
+ /* Check for forced Manual modulation modes; otherwise always "auto" */
+ if (forced_manual && (modulation != QAM_AUTO))
+ val |= 0x01; /* STDOPDETCMODE[1:0]= 1=Manual */
+ else
+ val |= 0x02; /* STDOPDETCMODE[1:0]= 2=Auto */
ret = lgdt3306a_write_reg(state, 0x0009, val);
if (lg_chkerr(ret))
goto fail;
@@ -642,10 +657,9 @@ static int lgdt3306a_set_modulation(struct lgdt3306a_state *state,
ret = lgdt3306a_set_vsb(state);
break;
case QAM_64:
- ret = lgdt3306a_set_qam(state, QAM_64);
- break;
case QAM_256:
- ret = lgdt3306a_set_qam(state, QAM_256);
+ case QAM_AUTO:
+ ret = lgdt3306a_set_qam(state, p->modulation);
break;
default:
return -EINVAL;
@@ -672,6 +686,7 @@ static int lgdt3306a_agc_setup(struct lgdt3306a_state *state,
break;
case QAM_64:
case QAM_256:
+ case QAM_AUTO:
break;
default:
return -EINVAL;
@@ -726,6 +741,7 @@ static int lgdt3306a_spectral_inversion(struct lgdt3306a_state *state,
break;
case QAM_64:
case QAM_256:
+ case QAM_AUTO:
/* Auto ok for QAM */
ret = lgdt3306a_set_inversion_auto(state, 1);
break;
@@ -749,6 +765,7 @@ static int lgdt3306a_set_if(struct lgdt3306a_state *state,
break;
case QAM_64:
case QAM_256:
+ case QAM_AUTO:
if_freq_khz = state->cfg->qam_if_khz;
break;
default:
@@ -1607,6 +1624,7 @@ static int lgdt3306a_read_status(struct dvb_frontend *fe,
switch (state->current_modulation) {
case QAM_256:
case QAM_64:
+ case QAM_AUTO:
if (lgdt3306a_qam_lock_poll(state) == LG3306_LOCK) {
*status |= FE_HAS_VITERBI;
*status |= FE_HAS_SYNC;
@@ -1650,6 +1668,7 @@ static int lgdt3306a_read_signal_strength(struct dvb_frontend *fe,
* Calculate some sort of "strength" from SNR
*/
struct lgdt3306a_state *state = fe->demodulator_priv;
+ u8 val;
u16 snr; /* snr_x10 */
int ret;
u32 ref_snr; /* snr*100 */
@@ -1662,11 +1681,15 @@ static int lgdt3306a_read_signal_strength(struct dvb_frontend *fe,
ref_snr = 1600; /* 16dB */
break;
case QAM_64:
- ref_snr = 2200; /* 22dB */
- break;
case QAM_256:
- ref_snr = 2800; /* 28dB */
- break;
+ case QAM_AUTO:
+ /* need to know actual modulation to set proper SNR baseline */
+ lgdt3306a_read_reg(state, 0x00a6, &val);
+ if (val & 0x04)
+ ref_snr = 2800; /* QAM-256 28dB */
+ else
+ ref_snr = 2200; /* QAM-64 22dB */
+ break;
default:
return -EINVAL;
}
@@ -2136,7 +2159,7 @@ static const struct dvb_frontend_ops lgdt3306a_ops = {
.frequency_min = 54000000,
.frequency_max = 858000000,
.frequency_stepsize = 62500,
- .caps = FE_CAN_QAM_64 | FE_CAN_QAM_256 | FE_CAN_8VSB
+ .caps = FE_CAN_QAM_AUTO | FE_CAN_QAM_64 | FE_CAN_QAM_256 | FE_CAN_8VSB
},
.i2c_gate_ctrl = lgdt3306a_i2c_gate_ctrl,
.init = lgdt3306a_init,
--
2.7.4
^ permalink raw reply related [flat|nested] 32+ messages in thread