On 05/08/13 14:26, Archit Taneja wrote:
> On Monday 05 August 2013 01:43 PM, Tomi Valkeinen wrote:
>>> +/*
>>> + * submit a list of DMA descriptors to the VPE VPDMA, do not wait
>>> for completion
>>> + */
>>> +int vpdma_submit_descs(struct vpdma_data *vpdma, struct
>>> vpdma_desc_list *list)
>>> +{
>>> + /* we always use the first list */
>>> + int list_num = 0;
>>> + int list_size;
>>> +
>>> + if (vpdma_list_busy(vpdma, list_num))
>>> + return -EBUSY;
>>> +
>>> + /* 16-byte granularity */
>>> + list_size = (list->next - list->buf.addr) >> 4;
>>> +
>>> + write_reg(vpdma, VPDMA_LIST_ADDR, (u32) list->buf.dma_addr);
>>> + wmb();
>>
>> What is the wmb() for?
>
> VPDMA_LIST_ADDR needs to be written before VPDMA_LIST_ATTR, otherwise
> VPDMA doesn't work. wmb() ensures the ordering.
Are you sure it's needed? Here's an interesting thread about writing and
reading to registers: http://marc.info/?t=130588594900002&r=1&w=2
>>> +
>>> + for (i = 0; i < 100; i++) { /* max 1 second */
>>> + msleep_interruptible(10);
>>
>> You call interruptible version here, but you don't handle the
>> interrupted case. I believe the loop will just continue looping, even if
>> the user interrupted.
>
> Okay. I think I don't understand the interruptible version correctly. We
> don't need to msleep_interruptible here, we aren't waiting on any wake
> up event, we just want to wait till a bit gets set.
Well, I think the interruptible versions should be used when the user
(wel, userspace program) initiates the action. The user should have the
option to interrupt a possibly long running operation, which is what
msleep_interruptible() makes possible.
Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
