All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: David Brownell <david-b@pacbell.net>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	spi-devel-general@lists.sourceforge.net,
	lkml <linux-kernel@vger.kernel.org>,
	Vernon Sauder <vernoninhand@gmail.com>
Subject: Re: [patch 2.6.28-rc9] spi: spi_write_then_read() regression fix
Date: Tue, 23 Dec 2008 10:45:37 -0800 (PST)	[thread overview]
Message-ID: <alpine.LFD.2.00.0812231040080.3535@localhost.localdomain> (raw)
In-Reply-To: <200812211648.26699.david-b@pacbell.net>



On Sun, 21 Dec 2008, David Brownell wrote:

> On Sunday 21 December 2008, Linus Torvalds wrote:
> > 
> > Hmm. In addition, isn't this broken (in that same function):
> 
> No -- this is full duplex.  The write_then_read() helper is
> simplifying a common half-duplex idiom for short operations,
> but the harware still does full duplex.  Buffer layout is:
> 
>   Before:	WWWWW0000000
>   After:	xxxxxRRRRRRR

Then, the problem is this:

        x.tx_buf = local_buf;
        x.rx_buf = local_buf;

which makes no sense. It's not two separate "tx_buf"/"rx_buf" things, it's 
just a single "buf".

Having two separate pointers is not only confusing, but it's actively 
WRONG, since the "rx_buf" clearly does not start at local_buf.

But that's not how most spi drivers seem to do it. They literally set 
tx_bug and rx_buf to different values. So again, none of what you talk 
about makes sense, and none of your "explanations" are anything but just 
incoherent noise.

So please explain how it can _possibly_ make sense to

 - have two separate pointers

 - and have one of them point to what is clearly not the data it is 
   supposed to point to.

Hmm?

			Linus

WARNING: multiple messages have this Message-ID (diff)
From: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
To: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	Vernon Sauder
	<vernoninhand-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	lkml <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [patch 2.6.28-rc9] spi: spi_write_then_read() regression fix
Date: Tue, 23 Dec 2008 10:45:37 -0800 (PST)	[thread overview]
Message-ID: <alpine.LFD.2.00.0812231040080.3535@localhost.localdomain> (raw)
In-Reply-To: <200812211648.26699.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>



On Sun, 21 Dec 2008, David Brownell wrote:

> On Sunday 21 December 2008, Linus Torvalds wrote:
> > 
> > Hmm. In addition, isn't this broken (in that same function):
> 
> No -- this is full duplex.  The write_then_read() helper is
> simplifying a common half-duplex idiom for short operations,
> but the harware still does full duplex.  Buffer layout is:
> 
>   Before:	WWWWW0000000
>   After:	xxxxxRRRRRRR

Then, the problem is this:

        x.tx_buf = local_buf;
        x.rx_buf = local_buf;

which makes no sense. It's not two separate "tx_buf"/"rx_buf" things, it's 
just a single "buf".

Having two separate pointers is not only confusing, but it's actively 
WRONG, since the "rx_buf" clearly does not start at local_buf.

But that's not how most spi drivers seem to do it. They literally set 
tx_bug and rx_buf to different values. So again, none of what you talk 
about makes sense, and none of your "explanations" are anything but just 
incoherent noise.

So please explain how it can _possibly_ make sense to

 - have two separate pointers

 - and have one of them point to what is clearly not the data it is 
   supposed to point to.

Hmm?

			Linus

------------------------------------------------------------------------------

  parent reply	other threads:[~2008-12-23 18:46 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-21  7:32 [patch 2.6.28-rc9] spi: spi_write_then_read() regression fix David Brownell
2008-12-21  7:32 ` David Brownell
2008-12-21 23:46 ` Linus Torvalds
2008-12-21 23:46   ` Linus Torvalds
2008-12-22  0:48   ` David Brownell
2008-12-23  1:53     ` Ingo Oeser
2008-12-23 18:37       ` David Brownell
2008-12-23 18:37         ` David Brownell
2008-12-23 18:45     ` Linus Torvalds [this message]
2008-12-23 18:45       ` Linus Torvalds
2008-12-23 20:54       ` David Brownell
2008-12-23 20:54         ` David Brownell
     [not found]         ` <200812231254.55284.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-12-24  3:26           ` Tang, Feng
     [not found]             ` <EADF0A36011179459010BDF5142A45750458C7DD-Uz4Je35TzWuiAffOGbnezLfspsVTdybXVpNB7YpNyf8@public.gmane.org>
2008-12-24  3:53               ` David Brownell
     [not found]                 ` <200812231953.26407.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-12-24  4:20                   ` Tang, Feng
     [not found]                     ` <EADF0A36011179459010BDF5142A45750458C815-Uz4Je35TzWuiAffOGbnezLfspsVTdybXVpNB7YpNyf8@public.gmane.org>
2008-12-24  8:39                       ` David Brownell
     [not found]                         ` <200812240039.32008.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-12-24  8:58                           ` Baruch Siach
     [not found]                             ` <20081224085844.GA11197-l+PWtdWbHAs2zBO71LVbc/qBs+8SCbDb@public.gmane.org>
2008-12-24 19:43                               ` David Brownell
2008-12-25  7:03                           ` Tang, Feng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.2.00.0812231040080.3535@localhost.localdomain \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=david-b@pacbell.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=spi-devel-general@lists.sourceforge.net \
    --cc=vernoninhand@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.