Re: i2c-gpio performance issue w/MPC8xx @ 50 MHz

["Håvard Skinnemoen" <hskinnemoen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On Mon, May 2, 2011 at 1:48 AM, Eran Duchan <pavius-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Hmm. i2c-gpio overrides the udelay specified by the platform? That
>> shouldn't happen. It does try to come up with some reasonable defaults
>> if the platform doesn't specify any though.
>
> Yep, only this scheme explicitly disallows a udelay of 0. It's not too
> problematic, I guess, seeing how close 1 is to 0 in terms of timing.

Hmm, yeah. Not sure how to allow 0 while still being backwards compatible.

>> I haven't investigated this in depth, but I'd say it's probably caused
>> by the extra indirection added by gpiolib, assuming the gpio framework
>> on your platform uses it. I guess it's one of the downsides of greatly
>> reducing the amount of platform-specific code.
>
> Indeed. Tracing the platform gpio code does not show it to be overly
> complex at all; simply a matter of mapping the gpio # to a chip (which
> is mapped to a port bank), mapping to the specific bit in the bank,
> some locking, setting and unlocking. The only problem is that while
> this is straightforward, it does introduce unwanted latency. I did not
> get to profiling the code, though.

Well, not really complex as such, but when you want to do all that
every 10 microseconds, it adds up really quickly.

>>> While I don't expect all users of i2c-algo-bit to be converted to such
>>> a platform driver, it may indeed be useful in some cases, so I have no
>>> objection.
>
> Nor would I expect this to be the case. i2c-algo-bit obviously works
> well on most platforms when paired to the gpio facility. It just isn't
> tight enough for environments low on processing power.
>
>> You can probably create a minimal i2c-platform driver which just
>> passes the platform-specific callbacks to i2c-algo-bit. I think it
>> should be a new bus driver though...
>
> This is where I kind of lost you. I implemented a bus driver specific
> to my platform (compiled in in drivers/i2c/busses) and then
> instantiated it in my platform module. I thought about adding a
> generic bus driver that would receive callbacks from the platform
> module (for example, drivers/i2c/busses/i2c-gpio-direct, or something)
> and simply pass these on to i2c-bit-algo. This would allow other
> platforms to do the same without having to create a platform specific
> bus driver each time like I did. I can submit something generic like
> this, if you think this it conforms to the model.

That sounds exactly like what I had in mind, except for the naming. If
the driver doesn't use the generic GPIO facilities at all, I don't
think it should have "gpio" in its name. What the driver does,
however, is taking platform-specific callbacks and passes them to
i2c-algo-bit. So I think "i2c-platform" would be a better name.

I don't really know whether it conforms to the "model" (or which model
we're talking about for that matter), but I can't see any problems
with it. Platforms for which i2c-gpio doesn't perform well enough can
get better performance at the cost of a little bit more
platform-specific code if they switch to i2c-platform.

Havard