Thread (25 messages) 25 messages, 4 authors, 2018-10-26

Re: [PATCH v9 6/9] i3c: master: Add driver for Cadence IP

From: Arnd Bergmann <arnd@arndb.de>
Date: 2018-10-26 13:21:37
Also in: linux-doc, linux-gpio, linux-i2c, lkml

On Fri, Oct 26, 2018 at 2:46 PM Boris Brezillon
[off-list ref] wrote:
On Fri, 26 Oct 2018 12:01:52 +0200
Arnd Bergmann [off-list ref] wrote:
quoted
On Fri, Oct 26, 2018 at 9:57 AM Boris Brezillon
[off-list ref] wrote:
quoted
On Fri, 26 Oct 2018 09:43:25 +0200
Arnd Bergmann [off-list ref] wrote:
quoted
On Thu, Oct 25, 2018 at 6:30 PM Boris Brezillon
[off-list ref] wrote:
quoted
On Thu, 25 Oct 2018 18:13:51 +0200 Arnd Bergmann [off-list ref] wrote:
On Thu, Oct 25, 2018 at 6:07 PM Boris Brezillon [off-list ref] wrote:
quoted
quoted
On Thu, 25 Oct 2018 17:30:26 +0200
This is apparently not allowed on ARC when 'buffer' is
unaligned. I think what we need here is to use
put_unaligned() instead of the pointer dereference.
For architectures that can do unaligned accesses,
the result is the same, but for ARC it will fix the problem.
Okay, so writesl()/readsl() should deal with unaligned pointers, and
default implementations should be fixed. I guess you'll send a patch to
use put/get_unaligned().
That's one way of doing it, though thinking about it some more,
this can also introduce overhead on machines that don't support
unaligned buffers and only work on drivers that are guaranteed
to see fully aligned data.

We could also override these specifically for ARC, and risk
running into the same problem elsewhere, rather than be sure
to fix everyone while risking to introduce noticeable performance
regressions in existing drivers.
quoted
quoted
quoted
One way to address this might be to always bounce any
messages that are less than a cache line through a
(pre-)kmallocated buffer, and require any longer messages
to be cache capable. This could also solve the issue with
readsl(), but it would be a rather confusing user interface.

Another option might be to have separate interfaces for
"short" and "long" messages at the API level and have
distinct rules for those: short would always be bounced
by the i3c code, and long puts restrictions on the buffer
location.
Hm, let's keep the API simple. I'll just mandate that all payload bufs
passed to i3c_master_send_ccc_cmd_locked() be dynamically allocated.
Ok. What about i2c commands sent to the same i3c controller
then?
Still not taken care of.
quoted
Do we need to copy those to satisfy the requirements
of the i3c layer?
I guess we should. The question is, should we do that unconditionally
or should we try to optimize thins with something like:

        if (!virt_addr_valid(xfer->buf) ||
            object_is_on_stack(xfer->buf))
                /* Alloc bounce buf. */
        else
                /* Use provided buf. */
There may be too many cases that we need to handle here that
are not DMA capable. To be on the safe side, I'd probably always
copy all data that is not a multiple of fully aligned cache lines,
as well as pointers that fails to meet some other requirements
(stack, vmalloc, kmap, ...)

        Arnd
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help