On 09/10/2013 02:02 PM, Mark Brown wrote:
On Tue, Sep 10, 2013 at 01:23:45PM +0200, Łukasz Czerwiński wrote:
On 09/09/2013 04:45 PM, Mark Brown wrote:
This seems like a very low limit to have, consider things like firmware
downloads for example. It seems reasonable to have a preallocated small
buffer but there should be some fallback for larger transfer sizes.
I have tested my modification with different buffer sizes with
S5C73M3 driver (355560 B upload). I obtained following upload times:
16kB buffer:
- 83.972 ms
- 79.196 ms
- 79.432 ms
128kB buffer:
- 74.449 ms
- 80.719 ms
- 75.599 ms
For 256kB I obtained similar results as in 128kB case. Normally
non-interrupted SPI transfer should take 56ms (50MHz). Performance
loss is approximately about 6% between 16kB and 128KB buffer. I my
opinion there are no need to use bigger buffers.
You're missing the point here. This requires the client driver to know
the maximum transfer size that the controller supports and split the
transfer up for it. This isn't something that the SPI API supports now
and while it would be useful to add for controllers that are genuinely
limited in transfer size it doesn't seem sane to just randomly impose a
limit to make life easier for software. Not all applications will be
able to split their transfers up in the first place and it seems like
the wrong place to put that knowledge.
What I would expect to see the driver doing here is either allocating
a larger buffer on demand or (probably more reliably given the need for
contiguous physical memory) transparently splitting larger transfers.
A super smart implementation would overlap the copy of blocks with the
transfers of preceeding blocks, and there should be something we can do
to avoid having to copy data that's already in suitable memory.
Thanks for your comments.
I will try implement transparently splitting larger transfer.
Ideally this would all be factored out into the core based on limit
information but that's probably not required immediately. Though now I
write that I'm wondering how many other devices should be doing this but
aren't. The DMA mapping of transfer buffers doesn't seem terribly
device specific...
I propose add extra module parameter which allows to change buffer size.
No, module parameters are not sensible for modern code. If you wanted
runtime configuration a sysfs tunable would be better though I do think
that should be tuning optimisation, not a hard limit on the transfer
size.
ok
Thanks
Lukasz
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
