On De 25 2015 05:51, Stefan Richter wrote:
> On Dec 24 Takashi Sakamoto wrote:
>> --- a/sound/firewire/dice/dice.c
>> +++ b/sound/firewire/dice/dice.c
> [...]
>> static void dice_bus_reset(struct fw_unit *unit)
>> {
>> struct snd_dice *dice = dev_get_drvdata(&unit->device);
>>
>> + /* Postpone a workqueue for deferred registration. */
>> + if (!dice->registered) {
>> + schedule_registration(dice);
>> + return;
>> + }
>> +
>> /* The handler address register becomes initialized. */
>> snd_dice_transaction_reinit(dice);
>>
>
> In previous versions of the patch, you used
> schedule_delayed_work(&dice->dwork, delay);
> in dice_probe() and
> mod_delayed_work(dice->dwork.wq, &dice->dwork, delay);
> in dice_bus_reset().
>
> Now you are using schedule_delayed_work() in both. This means that
> dice_bus_reset() is now unable to further defer the work. Is this
> intentional?
No intention, just my mistake. Originally, I had an intention to use
'mod_delayed_work()' here.
https://www.kernel.org/doc/htmldocs/device-drivers/API-mod-delayed-work.html
> By the way, in drivers/firewire/core-cdev.c, I used a somewhat different
> workqueue scheduling scheme. Problem:
> - The first 1000 ms after a bus reset are to be used for re-allocations
> of previously allocated isochronous resources. Attempts for new iso
> resource allocations shall be deferred until after 1000 ms after the
> latest bus reset.
> My solution:
> - The work which is to perform allocations/ reallocations/ deallocations
> is scheduled immediately. But the worker function (iso_resource_work)
> reschedules itself if it notices that (1.) its job is to allocate and
> (2.) the last bus reset was less than 1 s ago.
>
> I used the same principle in drivers/firewire/core-card.c. Problem:
> - If system software wants to reset the bus, it shall wait at least
> 2000 ms until after the last bus reset. The reason is to allow for
> various bus reset handling protocols to be performed first (e.g.
> isochronous resource allocations, re-logins and the likes).
> My solution:
> - br_work() reschedules itself if it detects that the last bus reset was
> less than 2 s ago.
>
> Admittedly there is a small remaining window after the worker looked at
> card->reset_jiffies and before it performs its real work, and another bus
> rest could happen in this window. I suppose if I wanted to close even
> this window, I would have to couple card->reset_jiffies with a
> bus generation counter and then make the remaining work depended on this
> bus generation.
>
> Back to your patch: I am not sure how strictly you want to guarantee the
> delay between last reset and do_registration()'s execution. Maybe it
> would be beneficial to put the check for card->reset_jiffies and self-
> rescheduling into do_registration(), similar to the two examples from
> firewire-core, or maybe that's not really necessary for your purposes.
On Dec 25 2015 06:04, Stefan Richter wrote:
> Or as I wrote in the other thread:
> Maybe you can live with less precision and simply use a fixed relative
> delay, disregarding card->reset_jiffies altogether.
For the case that users execute insmod/rmmod by their own. In this case,
no need to be delay for the registration work.
Thanks
Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
