lkml.org 
[lkml]   [2011]   [Dec]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [20/80] ALSA: lx6464es - fix device communication via command bus
    On Wed, Dec 7, 2011 at 8:06 AM, Greg KH <gregkh@suse.de> wrote:
    >
    > From: Tim Blechmann <tim@klingt.org>
    >
    > commit a29878553a9a7b4c06f93c7e383527cf014d4ceb upstream.
    >
    > commit 6175ddf06b6172046a329e3abfd9c901a43efd2e optimized the mem*io
    > functions that have been used to send commands to the device. these
    > optimizations somehow corrupted the communication with the lx6464es,
    > that resulted the device to be unusable with kernels after 2.6.33.

    Uhhuh. Looking at this, I understand why the driver can't really use
    memcpy_toio/fromio any more, and I never noticed this problem because
    it was worked around in drivers instead (or being discussed elsewhere
    and I just overlooked it)

    That commit (6175ddf06b61) is really bad and buggy. It probably wasn't
    horribly bad back when it was done, but it's getting increasingly bad
    as we change how "memcpy()" works.

    For example, memcpy some day will be just "rep movsb" on newer CPU's
    ("enhanced fast string memcpy"), which can be optimal for memory. But
    it is completely unacceptable for IO devices, so saying that "Iomem
    has no special significance on x86" is just total crap.

    iomem can work with regular accessors, yes, but it still has
    significance even on x86: iomem is *not* RAM.

    And using memcpy() is wrong, wrong, wrong. It's wrong even aside from
    any future "rep movsb" issues - it's already wrong due to the crazy
    "optimized x86 memcpy" that works around Atom crap. Copying stuff
    backwards is not what memcpy_toio/fromio is supposed to do, partly
    because it can confuse devices, but partly simply because it can
    easily destroy things like PCI bursting etc. I also would not be
    totally shocked to hear that some devices dislike 8-byte writes.

    So I understand why the driver works around the issue, but I think the
    real fix is to undo much of that broken commit in the first place.

    So we should probably make memcpy_fromio/toio do the
    "__inline_memcpy()" loop that we used to use: "rep movsl" followed by
    conditional movsw/movsb. No, it's not necessarily optimal, but it's
    *safe*, and it's quite likely *more* optimal than memcpy that may well
    end up doing it a byte at a time!

    So I think we should just re-instate "arch/x86/lib/io.c, copy the
    "inline_memcpy" thing in there, and add a similar "rep stosl" version
    for the "memset_io()". IOW, undo most of 6175ddf06b61, just make it
    use the same code for 32-bit and 64-bit.

    Ingo, Peter, Thomas? Comments?

    Linus


    \
     
     \ /
      Last update: 2011-12-07 18:51    [W:0.025 / U:0.016 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site