* [PATCH 2/7] af9015 Remove call to get config from probe.
@ 2011-11-12 15:55 Malcolm Priestley
2011-11-12 16:18 ` Antti Palosaari
0 siblings, 1 reply; 6+ messages in thread
From: Malcolm Priestley @ 2011-11-12 15:55 UTC (permalink / raw)
To: linux-media
Remove get config from probe and move to identify_state.
intf->cur_altsetting->desc.bInterfaceNumber is always expected to be zero, so there
no point in checking for it.
Calling from probe seems to cause a race condition with some USB controllers.
The first call fails as the device appears to busy with USB sub system
control calls.
Signed-off-by: Malcolm Priestley <tvboxspy@gmail.com>
---
drivers/media/dvb/dvb-usb/af9015.c | 87 ++++++++++++++---------------------
1 files changed, 35 insertions(+), 52 deletions(-)
diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
index dc6e4ec..eb464c8 100644
--- a/drivers/media/dvb/dvb-usb/af9015.c
+++ b/drivers/media/dvb/dvb-usb/af9015.c
@@ -820,7 +820,8 @@ static void af9015_set_remote_config(struct usb_device *udev,
return;
}
-static int af9015_read_config(struct usb_device *udev)
+static int af9015_read_config(struct usb_device *udev,
+ struct dvb_usb_device_properties *props)
{
int ret;
u8 val, i, offset = 0;
@@ -842,12 +843,10 @@ static int af9015_read_config(struct usb_device *udev)
goto error;
deb_info("%s: IR mode:%d\n", __func__, val);
- for (i = 0; i < af9015_properties_count; i++) {
- if (val == AF9015_IR_MODE_DISABLED)
- af9015_properties[i].rc.core.rc_codes = NULL;
- else
- af9015_set_remote_config(udev, &af9015_properties[i]);
- }
+ if (val == AF9015_IR_MODE_DISABLED)
+ props->rc.core.rc_codes = NULL;
+ else
+ af9015_set_remote_config(udev, props);
/* TS mode - one or two receivers */
req.addr = AF9015_EEPROM_TS_MODE;
@@ -859,18 +858,16 @@ static int af9015_read_config(struct usb_device *udev)
/* Set adapter0 buffer size according to USB port speed, adapter1 buffer
size can be static because it is enabled only USB2.0 */
- for (i = 0; i < af9015_properties_count; i++) {
- /* USB1.1 set smaller buffersize and disable 2nd adapter */
- if (udev->speed == USB_SPEED_FULL) {
- af9015_properties[i].adapter[0].fe[0].stream.u.bulk.buffersize
- = TS_USB11_FRAME_SIZE;
- /* disable 2nd adapter because we don't have
+ /* USB1.1 set smaller buffersize and disable 2nd adapter */
+ if (udev->speed == USB_SPEED_FULL) {
+ props->adapter[0].fe[0].stream.u.bulk.buffersize
+ = TS_USB11_FRAME_SIZE;
+ /* disable 2nd adapter because we don't have
PID-filters */
- af9015_config.dual_mode = 0;
- } else {
- af9015_properties[i].adapter[0].fe[0].stream.u.bulk.buffersize
- = TS_USB20_FRAME_SIZE;
- }
+ af9015_config.dual_mode = 0;
+ } else {
+ props->adapter[0].fe[0].stream.u.bulk.buffersize
+ = TS_USB20_FRAME_SIZE;
}
if (af9015_config.dual_mode) {
@@ -882,16 +879,11 @@ static int af9015_read_config(struct usb_device *udev)
af9015_af9013_config[1].demod_address = val;
/* enable 2nd adapter */
- for (i = 0; i < af9015_properties_count; i++)
- af9015_properties[i].num_adapters = 2;
+ props->num_adapters = 2;
+ } else /* disable 2nd adapter */
+ props->num_adapters = 1;
- } else {
- /* disable 2nd adapter */
- for (i = 0; i < af9015_properties_count; i++)
- af9015_properties[i].num_adapters = 1;
- }
-
- for (i = 0; i < af9015_properties[0].num_adapters; i++) {
+ for (i = 0; i < props->num_adapters; i++) {
if (i == 1)
offset = AF9015_EEPROM_OFFSET;
/* xtal */
@@ -995,8 +987,7 @@ error:
/* disable dual mode */
af9015_config.dual_mode = 0;
/* disable 2nd adapter */
- for (i = 0; i < af9015_properties_count; i++)
- af9015_properties[i].num_adapters = 1;
+ props->num_adapters = 1;
/* set correct IF */
af9015_af9013_config[0].tuner_if = 4570;
@@ -1014,6 +1005,10 @@ static int af9015_identify_state(struct usb_device *udev,
u8 reply;
struct req_t req = {GET_CONFIG, 0, 0, 0, 0, 1, &reply};
+ ret = af9015_read_config(udev, props);
+ if (ret)
+ return ret;
+
ret = af9015_rw_udev(udev, &req);
if (ret)
return ret;
@@ -1675,33 +1670,21 @@ static int af9015_usb_probe(struct usb_interface *intf,
{
int ret = 0;
struct dvb_usb_device *d = NULL;
- struct usb_device *udev = interface_to_usbdev(intf);
u8 i;
- deb_info("%s: interface:%d\n", __func__,
- intf->cur_altsetting->desc.bInterfaceNumber);
-
- /* interface 0 is used by DVB-T receiver and
- interface 1 is for remote controller (HID) */
- if (intf->cur_altsetting->desc.bInterfaceNumber == 0) {
- ret = af9015_read_config(udev);
- if (ret)
- return ret;
-
- for (i = 0; i < af9015_properties_count; i++) {
- ret = dvb_usb_device_init(intf, &af9015_properties[i],
- THIS_MODULE, &d, adapter_nr);
- if (!ret)
- break;
- if (ret != -ENODEV)
- return ret;
- }
- if (ret)
+ for (i = 0; i < af9015_properties_count; i++) {
+ ret = dvb_usb_device_init(intf, &af9015_properties[i],
+ THIS_MODULE, &d, adapter_nr);
+ if (!ret)
+ break;
+ if (ret != -ENODEV)
return ret;
-
- if (d)
- ret = af9015_init(d);
}
+ if (ret)
+ return ret;
+
+ if (d)
+ ret = af9015_init(d);
return ret;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/7] af9015 Remove call to get config from probe.
2011-11-12 15:55 [PATCH 2/7] af9015 Remove call to get config from probe Malcolm Priestley
@ 2011-11-12 16:18 ` Antti Palosaari
2011-11-12 18:22 ` Malcolm Priestley
0 siblings, 1 reply; 6+ messages in thread
From: Antti Palosaari @ 2011-11-12 16:18 UTC (permalink / raw)
To: Malcolm Priestley; +Cc: linux-media
On 11/12/2011 05:55 PM, Malcolm Priestley wrote:
> Remove get config from probe and move to identify_state.
>
> intf->cur_altsetting->desc.bInterfaceNumber is always expected to be zero, so there
> no point in checking for it.
Are you sure? IIRC there is HID remote on interface 1 or 2 or so (some
other than 0). Please double check.
> Calling from probe seems to cause a race condition with some USB controllers.
Why?
Unless reasons are not clear it should not be moved.
Antti
> The first call fails as the device appears to busy with USB sub system
> control calls.
>
> Signed-off-by: Malcolm Priestley<tvboxspy@gmail.com>
> ---
> drivers/media/dvb/dvb-usb/af9015.c | 87 ++++++++++++++---------------------
> 1 files changed, 35 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/media/dvb/dvb-usb/af9015.c b/drivers/media/dvb/dvb-usb/af9015.c
> index dc6e4ec..eb464c8 100644
> --- a/drivers/media/dvb/dvb-usb/af9015.c
> +++ b/drivers/media/dvb/dvb-usb/af9015.c
> @@ -820,7 +820,8 @@ static void af9015_set_remote_config(struct usb_device *udev,
> return;
> }
>
> -static int af9015_read_config(struct usb_device *udev)
> +static int af9015_read_config(struct usb_device *udev,
> + struct dvb_usb_device_properties *props)
> {
> int ret;
> u8 val, i, offset = 0;
> @@ -842,12 +843,10 @@ static int af9015_read_config(struct usb_device *udev)
> goto error;
>
> deb_info("%s: IR mode:%d\n", __func__, val);
> - for (i = 0; i< af9015_properties_count; i++) {
> - if (val == AF9015_IR_MODE_DISABLED)
> - af9015_properties[i].rc.core.rc_codes = NULL;
> - else
> - af9015_set_remote_config(udev,&af9015_properties[i]);
> - }
> + if (val == AF9015_IR_MODE_DISABLED)
> + props->rc.core.rc_codes = NULL;
> + else
> + af9015_set_remote_config(udev, props);
>
> /* TS mode - one or two receivers */
> req.addr = AF9015_EEPROM_TS_MODE;
> @@ -859,18 +858,16 @@ static int af9015_read_config(struct usb_device *udev)
>
> /* Set adapter0 buffer size according to USB port speed, adapter1 buffer
> size can be static because it is enabled only USB2.0 */
> - for (i = 0; i< af9015_properties_count; i++) {
> - /* USB1.1 set smaller buffersize and disable 2nd adapter */
> - if (udev->speed == USB_SPEED_FULL) {
> - af9015_properties[i].adapter[0].fe[0].stream.u.bulk.buffersize
> - = TS_USB11_FRAME_SIZE;
> - /* disable 2nd adapter because we don't have
> + /* USB1.1 set smaller buffersize and disable 2nd adapter */
> + if (udev->speed == USB_SPEED_FULL) {
> + props->adapter[0].fe[0].stream.u.bulk.buffersize
> + = TS_USB11_FRAME_SIZE;
> + /* disable 2nd adapter because we don't have
> PID-filters */
> - af9015_config.dual_mode = 0;
> - } else {
> - af9015_properties[i].adapter[0].fe[0].stream.u.bulk.buffersize
> - = TS_USB20_FRAME_SIZE;
> - }
> + af9015_config.dual_mode = 0;
> + } else {
> + props->adapter[0].fe[0].stream.u.bulk.buffersize
> + = TS_USB20_FRAME_SIZE;
> }
>
> if (af9015_config.dual_mode) {
> @@ -882,16 +879,11 @@ static int af9015_read_config(struct usb_device *udev)
> af9015_af9013_config[1].demod_address = val;
>
> /* enable 2nd adapter */
> - for (i = 0; i< af9015_properties_count; i++)
> - af9015_properties[i].num_adapters = 2;
> + props->num_adapters = 2;
> + } else /* disable 2nd adapter */
> + props->num_adapters = 1;
>
> - } else {
> - /* disable 2nd adapter */
> - for (i = 0; i< af9015_properties_count; i++)
> - af9015_properties[i].num_adapters = 1;
> - }
> -
> - for (i = 0; i< af9015_properties[0].num_adapters; i++) {
> + for (i = 0; i< props->num_adapters; i++) {
> if (i == 1)
> offset = AF9015_EEPROM_OFFSET;
> /* xtal */
> @@ -995,8 +987,7 @@ error:
> /* disable dual mode */
> af9015_config.dual_mode = 0;
> /* disable 2nd adapter */
> - for (i = 0; i< af9015_properties_count; i++)
> - af9015_properties[i].num_adapters = 1;
> + props->num_adapters = 1;
>
> /* set correct IF */
> af9015_af9013_config[0].tuner_if = 4570;
> @@ -1014,6 +1005,10 @@ static int af9015_identify_state(struct usb_device *udev,
> u8 reply;
> struct req_t req = {GET_CONFIG, 0, 0, 0, 0, 1,&reply};
>
> + ret = af9015_read_config(udev, props);
> + if (ret)
> + return ret;
> +
> ret = af9015_rw_udev(udev,&req);
> if (ret)
> return ret;
> @@ -1675,33 +1670,21 @@ static int af9015_usb_probe(struct usb_interface *intf,
> {
> int ret = 0;
> struct dvb_usb_device *d = NULL;
> - struct usb_device *udev = interface_to_usbdev(intf);
> u8 i;
>
> - deb_info("%s: interface:%d\n", __func__,
> - intf->cur_altsetting->desc.bInterfaceNumber);
> -
> - /* interface 0 is used by DVB-T receiver and
> - interface 1 is for remote controller (HID) */
> - if (intf->cur_altsetting->desc.bInterfaceNumber == 0) {
> - ret = af9015_read_config(udev);
> - if (ret)
> - return ret;
> -
> - for (i = 0; i< af9015_properties_count; i++) {
> - ret = dvb_usb_device_init(intf,&af9015_properties[i],
> - THIS_MODULE,&d, adapter_nr);
> - if (!ret)
> - break;
> - if (ret != -ENODEV)
> - return ret;
> - }
> - if (ret)
> + for (i = 0; i< af9015_properties_count; i++) {
> + ret = dvb_usb_device_init(intf,&af9015_properties[i],
> + THIS_MODULE,&d, adapter_nr);
> + if (!ret)
> + break;
> + if (ret != -ENODEV)
> return ret;
> -
> - if (d)
> - ret = af9015_init(d);
> }
> + if (ret)
> + return ret;
> +
> + if (d)
> + ret = af9015_init(d);
>
> return ret;
> }
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/7] af9015 Remove call to get config from probe.
2011-11-12 16:18 ` Antti Palosaari
@ 2011-11-12 18:22 ` Malcolm Priestley
2011-11-12 19:36 ` Antti Palosaari
0 siblings, 1 reply; 6+ messages in thread
From: Malcolm Priestley @ 2011-11-12 18:22 UTC (permalink / raw)
To: Antti Palosaari; +Cc: linux-media
On Sat, 2011-11-12 at 18:18 +0200, Antti Palosaari wrote:
> On 11/12/2011 05:55 PM, Malcolm Priestley wrote:
> > Remove get config from probe and move to identify_state.
> >
> > intf->cur_altsetting->desc.bInterfaceNumber is always expected to be zero, so there
> > no point in checking for it.
>
> Are you sure? IIRC there is HID remote on interface 1 or 2 or so (some
> other than 0). Please double check.
>
> > Calling from probe seems to cause a race condition with some USB controllers.
>
> Why?
>
Is some other module going to claim the device?
Would it not be better use usb_set_interface to set it back to 0?
Rather than failing it back to the user.
Regards
Malcolm
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/7] af9015 Remove call to get config from probe.
2011-11-12 18:22 ` Malcolm Priestley
@ 2011-11-12 19:36 ` Antti Palosaari
2011-11-13 12:04 ` Malcolm Priestley
0 siblings, 1 reply; 6+ messages in thread
From: Antti Palosaari @ 2011-11-12 19:36 UTC (permalink / raw)
To: Malcolm Priestley; +Cc: linux-media
On 11/12/2011 08:22 PM, Malcolm Priestley wrote:
> On Sat, 2011-11-12 at 18:18 +0200, Antti Palosaari wrote:
>> On 11/12/2011 05:55 PM, Malcolm Priestley wrote:
>>> Remove get config from probe and move to identify_state.
>>>
>>> intf->cur_altsetting->desc.bInterfaceNumber is always expected to be zero, so there
>>> no point in checking for it.
>>
>> Are you sure? IIRC there is HID remote on interface 1 or 2 or so (some
>> other than 0). Please double check.
>>
>>> Calling from probe seems to cause a race condition with some USB controllers.
>>
>> Why?
>>
> Is some other module going to claim the device?
>
> Would it not be better use usb_set_interface to set it back to 0?
>
> Rather than failing it back to the user.
Maybe I don't understand what you mean.
But here is the copy&paste from code:
/* interface 0 is used by DVB-T receiver and
interface 1 is for remote controller (HID) */
if (intf->cur_altsetting->desc.bInterfaceNumber == 0) {
As it says (comment in the code), interface 1 is for remote controller.
I added that to prevent call all that attach stuff twice. I have done
such check for some other drivers too where is multiple interfaces.
IIRC .probe() is called once per each interface device has. So it will
call that two times (if there is HID remote activated), resulting same
device appears two times in bad luck.
Just returning 0 for those probes we don't need sounds good approach for me.
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/7] af9015 Remove call to get config from probe.
2011-11-12 19:36 ` Antti Palosaari
@ 2011-11-13 12:04 ` Malcolm Priestley
2011-11-13 12:34 ` Antti Palosaari
0 siblings, 1 reply; 6+ messages in thread
From: Malcolm Priestley @ 2011-11-13 12:04 UTC (permalink / raw)
To: Antti Palosaari; +Cc: linux-media
On Sat, 2011-11-12 at 21:36 +0200, Antti Palosaari wrote:
> On 11/12/2011 08:22 PM, Malcolm Priestley wrote:
> > On Sat, 2011-11-12 at 18:18 +0200, Antti Palosaari wrote:
> >> On 11/12/2011 05:55 PM, Malcolm Priestley wrote:
> >>> Remove get config from probe and move to identify_state.
> >>>
> >>> intf->cur_altsetting->desc.bInterfaceNumber is always expected to be zero, so there
> >>> no point in checking for it.
> >>
> >> Are you sure? IIRC there is HID remote on interface 1 or 2 or so (some
> >> other than 0). Please double check.
> >>
> >>> Calling from probe seems to cause a race condition with some USB controllers.
> >>
> >> Why?
> >>
> > Is some other module going to claim the device?
> >
> > Would it not be better use usb_set_interface to set it back to 0?
> >
I spoke too soon, there is someone else about.
On boot input claims interface 1
Nov 13 11:43:36 tvbox kernel: [ 1.830276] input: Afatech DVB-T 2 as /devices/pci0000:00/0000:00:06.1/usb2/2-4/2-4:1.1/input/input2
Nov 13 11:43:36 tvbox kernel: [ 1.830367] generic-usb 0003:1B80:E399.0001: input,hidraw0: USB HID v1.01 Keyboard [Afatech DVB-T 2] on usb-0000:00:06.1-4/input1
...
Nov 13 11:43:38 tvbox kernel: [ 19.151700] dvb-usb: found a 'KWorld PlusTV Dual DVB-T Stick (DVB-T 399U)' in cold state, will try to load a firmware
...
Nov 13 11:43:39 tvbox kernel: [ 20.313483] input: IR-receiver inside an USB DVB receiver as /devices/pci0000:00/0000:00:06.1/usb2/2-4/rc/rc0/input8
Nov 13 11:43:39 tvbox kernel: [ 20.313528] rc0: IR-receiver inside an USB DVB receiver as /devices/pci0000:00/0000:00:06.1/usb2/2-4/rc/rc0
Nov 13 11:43:39 tvbox kernel: [ 20.313530] dvb-usb: schedule remote query interval to 500 msecs.
Nov 13 11:43:39 tvbox kernel: [ 20.313534] dvb-usb: KWorld PlusTV Dual DVB-T Stick (DVB-T 399U) successfully initialized and connected.
So, a usb_set_interface is needed to make sure we on interface 0 and remain there.
Regards
Malcolm
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/7] af9015 Remove call to get config from probe.
2011-11-13 12:04 ` Malcolm Priestley
@ 2011-11-13 12:34 ` Antti Palosaari
0 siblings, 0 replies; 6+ messages in thread
From: Antti Palosaari @ 2011-11-13 12:34 UTC (permalink / raw)
To: Malcolm Priestley; +Cc: linux-media
On 11/13/2011 02:04 PM, Malcolm Priestley wrote:
> On Sat, 2011-11-12 at 21:36 +0200, Antti Palosaari wrote:
>> On 11/12/2011 08:22 PM, Malcolm Priestley wrote:
>>> On Sat, 2011-11-12 at 18:18 +0200, Antti Palosaari wrote:
>>>> On 11/12/2011 05:55 PM, Malcolm Priestley wrote:
>>>>> Remove get config from probe and move to identify_state.
>>>>>
>>>>> intf->cur_altsetting->desc.bInterfaceNumber is always expected to be zero, so there
>>>>> no point in checking for it.
>>>>
>>>> Are you sure? IIRC there is HID remote on interface 1 or 2 or so (some
>>>> other than 0). Please double check.
>>>>
>>>>> Calling from probe seems to cause a race condition with some USB controllers.
>>>>
>>>> Why?
>>>>
>>> Is some other module going to claim the device?
>>>
>>> Would it not be better use usb_set_interface to set it back to 0?
>>>
>
> I spoke too soon, there is someone else about.
>
> On boot input claims interface 1
>
> Nov 13 11:43:36 tvbox kernel: [ 1.830276] input: Afatech DVB-T 2 as /devices/pci0000:00/0000:00:06.1/usb2/2-4/2-4:1.1/input/input2
> Nov 13 11:43:36 tvbox kernel: [ 1.830367] generic-usb 0003:1B80:E399.0001: input,hidraw0: USB HID v1.01 Keyboard [Afatech DVB-T 2] on usb-0000:00:06.1-4/input1
> ...
> Nov 13 11:43:38 tvbox kernel: [ 19.151700] dvb-usb: found a 'KWorld PlusTV Dual DVB-T Stick (DVB-T 399U)' in cold state, will try to load a firmware
> ...
> Nov 13 11:43:39 tvbox kernel: [ 20.313483] input: IR-receiver inside an USB DVB receiver as /devices/pci0000:00/0000:00:06.1/usb2/2-4/rc/rc0/input8
> Nov 13 11:43:39 tvbox kernel: [ 20.313528] rc0: IR-receiver inside an USB DVB receiver as /devices/pci0000:00/0000:00:06.1/usb2/2-4/rc/rc0
> Nov 13 11:43:39 tvbox kernel: [ 20.313530] dvb-usb: schedule remote query interval to 500 msecs.
> Nov 13 11:43:39 tvbox kernel: [ 20.313534] dvb-usb: KWorld PlusTV Dual DVB-T Stick (DVB-T 399U) successfully initialized and connected.
>
> So, a usb_set_interface is needed to make sure we on interface 0 and remain there.
Sorry, didn't still understand what you mean.
What I would like to say check for interface was set for HID remote and
since we just want to leave HID without any driver help we will just
skip it.
Do you try to say it should be always set usb_set_interface 0 just for sure?
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-11-13 12:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-12 15:55 [PATCH 2/7] af9015 Remove call to get config from probe Malcolm Priestley
2011-11-12 16:18 ` Antti Palosaari
2011-11-12 18:22 ` Malcolm Priestley
2011-11-12 19:36 ` Antti Palosaari
2011-11-13 12:04 ` Malcolm Priestley
2011-11-13 12:34 ` Antti Palosaari
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox