On Fri, 2012-06-15 at 01:12 +0300, Antti Palosaari wrote:
> On 06/15/2012 12:33 AM, Malcolm Priestley wrote:
> > On Thu, 2012-06-14 at 23:31 +0300, Antti Palosaari wrote:
> >> On 06/14/2012 04:15 AM, Antti Palosaari wrote:
> >>> On 06/14/2012 03:44 AM, Malcolm Priestley wrote:
> >>>> On Thu, 2012-06-14 at 02:29 +0300, Antti Palosaari wrote:
> >>>>> Hi Malcolm,
> >>>>> I was really surprised someone has had interest to test that stuff at
> >>>>> that phase as I did not even advertised it yet :) It is likely happen
> >>>>> next Monday or so as there is some issues I would like to check / solve.
> >>>>>
> >>>>>
> >>>>> On 06/14/2012 01:24 AM, Malcolm Priestley wrote:
> >>>>>> Hi antti
> >>>>>>
> >>>>>> There some issues with dvb_usb_v2 with the lmedm04 driver.
> >>>>>>
> >>>>>> The first being this patch, no return value from
> >>>>>> dvb_usb_download_firmware
> >>>>>> causes system wide dead lock with COLD disconnect as system attempts
> >>>>>> to continue
> >>>>>> to warm state.
> >>>>>
> >>>>> Hmm, I did not understand what you mean. What I looked lmedm04 driver I
> >>>>> think it uses single USB ID (no cold + warm IDs). So it downloads
> >>>>> firmware and then reconnects itself from the USB bus?
> >>>>> For that scenario you should "return RECONNECTS_USB;" from the driver
> >>>>> .download_firmware().
> >>>>>
> >>>> If the device disconnects from the USB bus after the firmware download.
> >>>>
> >>>> In most cases the device is already gone.
> >>>>
> >>>> There is currently no way to insert RECONNECTS_USB into the return.
> >>>
> >>> Argh, I was blind! You are absolutely correct. It never returns value 1
> >>> (RECONNECTS_USB) from the .download_firmware().
> >>>
> >>> That patch is fine, I will apply it, thanks!
> >>>
> >>> I think that must be also changed to return immediately without
> >>> releasing the interface. Let the USB core release it when it detects
> >>> disconnect - otherwise it could crash as it tries to access potentially
> >>> resources that are already freed. Just for the timing issue if it
> >>> happens or not.
> >>>
> >>> } else if (ret == RECONNECTS_USB) {
> >>> ret = 0;
> >>> goto exit_usb_driver_release_interface;
> >>>
> >>> add return 0 here without releasing interface and test.
> >>>
> >>>
> >>>>> I tested it using one non-public Cypress FX2 device - it was changing
> >>>>> USB ID after the FX download, but from the driver perspective it does
> >>>>> not matter. It is always new device if it reconnects USB.
> >>>>>
> >>>>
> >>>> Have double checked that the thread is not continuing to write on the
> >>>> old ID?
> >>>
> >>> Nope, but likely delayed probe() is finished until it reconnects so I
> >>> cannot see problem. You device disconnects faster and thus USB core
> >>> traps .disconnect() earlier...
> >>>
> >>> Could you test returning 0 and if it works sent new patch.
> >>>
> >>>> The zero condition will lead to dvb_usb_init.
> >>>>
> >>>>> PS. as I looked that driver I saw many different firmwares. That is now
> >>>>> supported and you should use .get_firmware_name() (maybe you already did
> >>>>> it).
> >>>>>
> >>>> Yes, I have supported this in the driver.
> >>
> >> Malcolm,
> >>
> >> could you just test if returning from the routines after fw download is
> >> enough to fix all your problems?
> >>
> >> I mean those two fixes:
> >> dvb_usb_download_firmware()
> >> * return RECONNECTS_USB correctly
> >>
> >> dvb_usbv2_init_work()
> >> * return without releasing USB interface if RECONNECTS_USB
> > Hi Antti,
> >
> > Yes, I have tested it and there is no difference.
> >
> > My understanding is, if the interface is no longer bound it returns
> > anyway.
> >
> > It is best not to use it, other drivers in the dvb-usb tree may not like
> > to be forcibly unbound prior to their reset.
>
> I don't understand why this logic cannot work. Do you have some crash
> dump I can see likely functions in path?
Sorry, you misunderstood me, there is no crash.
I was only talking using usb_driver_release_interface() or not.
The skeleton code below would work fine :-)
Regards
Malcolm
>
> I draw it here as a skeleton code.
>
> dvb_usbv2_init_work()
> ret = download_firmware()
> if (ret == DEVICE_IS_WARM)
> init_device()
> return
> else if (ret == DEVICE_RECONECTS_USB)
> return
> else (ret == some error code)
> usb_driver_release_interface()
> return
>
> dvb_usbv2_probe()
> state = alloc()
> usb_set_intfdata(state)
> schedule_work(dvb_usbv2_init_work)
> return 0
>
> dvb_usbv2_disconnect()
> state = usb_get_intfdata()
> cancel_work_sync(dvb_usbv2_init_work)
> free(state)
>
> Anyhow, I have devices I know how to force reconnect USB (AF9015, EC168)
> when needed. So I will make some tests here.
>
> regards
> Antti
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
