Hi Konrad,

On Wednesday 20 August 2014 09:02:50 Konrad Rzeszutek Wilk wrote:

On Tue, Aug 19, 2014 at 10:52:46PM +0200, Laurent Pinchart wrote:

quoted

On Tuesday 19 August 2014 11:40:24 Olav Haugan wrote:

quoted

On 8/19/2014 9:11 AM, Laurent Pinchart wrote:

quoted

On Tuesday 19 August 2014 13:59:54 Joerg Roedel wrote:

[snip]

quoted

quoted

quoted

quoted

Okay, since you add these call-backs to all drivers I think I can
live with not doing a pointer check here.

I suggested doing a

if (ops is not NULL)
	return ops();
else
	return default_ops();

to avoid modifying all drivers. I'm not sure why that wasn't received
with much enthusiasm.

Both Thierry R. and Konrad W. argued for modifying the drivers instead
so I implemented what the majority wanted. :-)

I'm not blaming you :-) I was just wondering what their rationale was.

a). To be inline with the usage of this API.

    For this particular case the other functions (but maybe I missed some)
    follow the idea that they implement the ops->func for every operation
    and they don't have an fallback.

That's because the other functions are mandatory, while this particular one is 
just an optimization and can be implemented generically.

b)  Follow what x86 maintainers prefer (based on other API calls,
    such as x86_init or any other platform overrides).

c). When there are new IOMMUs it has to take in-to account all of the
    function ops. If they fail to implement all of them the kernel
    crashes instead of working (but maybe working incorrectly).

I don't think that applies in this case, the generic implementation should 
work in all cases.

d). Lastly, we also want the bloat of kernel. If the compiler decides
    to roll in the fallback implementation for 'iommu_map_sg' in - it will
    needlessly expand the kernel wherever we use 'iommu_map_sg' call with
    a fallback implementation (which might not be called 99% of time).

    Having the little inline static just do 'func_ops->func' will
    stop the compiler from optimizing too much and convert this just
    to a simple function.

That's an argument I can buy.

-- 
Regards,

Laurent Pinchart