On Wednesday 17 March 2010 07:50:27 Pawel Osciak wrote: >>That is my opinion, yes. However, there is one case where this is actually >>useful. Take for example the function videobuf_to_dma in >>videobuf-dma-sg.c. This is called by drivers and it makes sense that that >>function should double-check that the videobuf_buffer is associated with >>the dma_sg memtype. >> >>But calling this 'magic' is a poor choice of name. There is nothing magic >>about it, in this case it is just an identifier of the memtype. And there >>may be better ways to do this check anyway. >> >>I have not done any analysis, but might be enough to check whether the >>int_ops field of videobuf_queue equals the sg_ops pointer. If so, then the >>whole magic handling can go away in this case. > > > Well... I see this discussion is dragging on a bit. > I will not be touching magics for now then, at least not until we arrive at > a consensus sometime in the future. You give up too easily :-) But I agree that it is probably best to start with checkpatch cleanups. I did a bit more research. The idea that I had to check the int_ops field doesn't work at the moment because videobuf_qtype_ops contains a wild variety of functions: some operate on a queue, some on a buffer. Some get a queue pointer, but really should have gotten a buffer pointer. Ops like copy_to_user and copy_stream shouldn't have been in the qtype at all, at least not in the current form. Anyway, it would help a lot if the qtype_ops are split into two structs: one for queues, one for buffers. Each queue or buffer struct then has a pointer to the ops. And that can be used by the qtype code as a check to detect whether it is called from the right context. Frankly, in the long term the queue-specific ops should probably be replaced by buffer-specific ops anyway since we want to have a lot more flexibility here and possible have different memory types per buffer (or even plane) on the same queue. It is my impression that all the queue-specific ops just walk over the buffers anyway, so they can easily be replaced by buffer-specific ops and the 'walk over' part can be moved to the core. So perhaps switching immediately to buffer-only ops might actually be better than create separate queue and buffer ops. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- 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
