Hi Samuel,
just a few things i wanted to clarify. My queries are inline.
On Fri, Jun 24, 2011 at 2:59 AM, Samuel Ortiz <sa...@linux.intel.com> wrote:
> Hi Alok,
>
> On Mon, Jun 20, 2011 at 02:06:15PM +0300, Alok Barsode wrote:
> > From: Alok Barsode <alok.bars...@linux.intel.com>
> >
> > Implement a command queue per device. It can queue power enable/disable
> > requests when device is busy. device_queue_add() can be used to add a
> > command in the queue, if the command is already queued, it is ignored.
> > device_queue_process() executes the queue.
> This looks good, thanks. Here are my comments:
>
> > @@ -245,11 +263,11 @@ int __connman_device_enable(struct connman_device
> *device)
> > if (device->blocked == TRUE)
> > return -ENOLINK;
> >
> > - /* There is an ongoing power disable request.
> > - * It would be great if we can queue such requests
> > - * rather than replying with a -EBUSY */
> > - if (device->powered_pending == PENDING_DISABLE)
> > - return -EBUSY;
> > + /* Ongoing power disable request, queue command. */
> > + if (device->powered_pending == PENDING_DISABLE) {
> > + device_queue_add(device, "enable");
> > + return -EINPROGRESS;
> > + }
> So the main point of this command queue was to cleanly get rid of the
> powered_pending stuff. And my idea was the following one: If the queue is
> empty, then we queue the command and we return the driver->enable return
> value. If the queue is not empty, then we just return -EINPROGRESS and we
> queue the command.
>
So we don't restrict the queue size, right?
Then, whenever we dequeue, we can compare the command and the actual device
> state (when we are dequeuing, the device is not in a pending state) and if
> they're identical then we just skip the command.
>
what if they are not identical? for example the respective daemon toggles
the power state ?
With this scheme, we won't need any powered_pending flag, but we'd still
> need
> your power operation timeout.
>
> Cheers,
> Samuel.
>
> --
> Intel Open Source Technology Centre
> http://oss.intel.com/
>
Cheers,
Alok.
_______________________________________________
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
