linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Brownell <david-b@pacbell.net>
To: Bryan Wu <bryan.wu@analog.com>
Cc: spi-devel-general@lists.sourceforge.net,
	linux-kernel@vger.kernel.org,
	Sonic Zhang <sonic.zhang@analog.com>
Subject: Re: [PATCH 09/14] Blackfin SPI driver: Fix SPI driver to work with SPI flash ST25P16 on bf548
Date: Tue, 30 Oct 2007 13:05:02 -0700	[thread overview]
Message-ID: <200710301305.02640.david-b@pacbell.net> (raw)
In-Reply-To: <1193735885-8202-10-git-send-email-bryan.wu@analog.com>

On Tuesday 30 October 2007, Bryan Wu wrote:
> Current SPI driver enables SPI controller and set the SPI baud register
> for each SPI transfer. But, they should never be changed within a SPI
> message session, in which seveal SPI transfers are pumped.

That's actually not true.  If a driver sets spi_transfer.max_speed_hz
to a nonzero value that's different from the previous bit rate (which
may be spi_device.max_speed_hz), it should be updated before that
transfer segment.  Example, sometimes data can't be clocked out at
the same rate commands can be clocked in.

Similarly with spi_transfer.bits_per_word ... again, it's very possible
that commands and data have different sizes.

Of course, if those values don't change, there'd be no point in
reconfiguring any aspect of those communications parameters...


I'll be forwarding this patch, since this looks like another case
where the main effect of the patch doesn't match its description
and since this patch series has taken too long already.  (Does this
patch even really relate primarily to working with an ST M25P16
flash part??)  Though it'd be reasonable to be more hard-nosed
about this and insist on another go-around for thesse patches.
(Making this the fifth one??)

But I *STRONGLY* suggest someone revisit the issue of whether those
two per-transfer options are now being handled correctly.  As well
as update procedures so that the patch comments start to have a
direct correspondence to what the patches have changed...

- Dave


> This patch 
> move move SPI setting to the begining of a message session. And never
> disables SPI controller until an error occurs.

  reply	other threads:[~2007-10-30 20:05 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-30  9:17 [PATCH 00/14] Blackfin on-chip SPI controller driver updates and bug-fixing Bryan Wu
     [not found] ` <1193735885-8202-1-git-send-email-bryan.wu-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
2007-10-30  9:17   ` [PATCH 01/14] Blackfin SPI driver: Initial supporting BF54x in SPI driver Bryan Wu
2007-10-30  9:17   ` [PATCH 02/14] Blackfin SPI driver: use new GPIO API and add error handling Bryan Wu
2007-10-30  9:17   ` [PATCH 03/14] Blackfin SPI driver: add error handing Bryan Wu
2007-10-30  9:17   ` [PATCH 04/14] Blackfin SPI driver: Blackfin SPI driver does not respect the per-transfer cs_change field Bryan Wu
2007-10-30  9:17   ` [PATCH 05/14] Blackfin SPI driver: prevent people from setting bits in ctl_reg Bryan Wu
2007-10-30  9:17   ` [PATCH 06/14] Blackfin SPI driver: update spi driver to support multi-ports Bryan Wu
2007-10-30  9:17   ` [PATCH 07/14] Blackfin SPI driver: Add SPI master controller platform device 1 Bryan Wu
2007-10-30 19:08     ` David Brownell
     [not found]       ` <200710301208.13367.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-10-31  4:18         ` Bryan Wu
2007-10-30  9:17   ` [PATCH 08/14] Blackfin SPI driver: Move GPIO config to setup and cleanup Bryan Wu
2007-10-30  9:18   ` [PATCH 09/14] Blackfin SPI driver: Fix SPI driver to work with SPI flash ST25P16 on bf548 Bryan Wu
2007-10-30 20:05     ` David Brownell [this message]
2007-10-31  6:50       ` Bryan Wu
2007-10-31  7:11         ` David Brownell
     [not found]           ` <200710310011.33545.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-10-31  7:35             ` Bryan Wu
2007-10-31  8:02               ` David Brownell
     [not found]                 ` <200710310102.36542.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-10-31  8:52                   ` Bryan Wu
2007-10-31  8:33             ` Mike Frysinger
     [not found]               ` <8bd0f97a0710310133m7b5cbbf3j545a489c401185df-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2007-10-31 19:01                 ` David Brownell
     [not found]                   ` <200710311201.29643.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-10-31 19:16                     ` Mike Frysinger
     [not found]                       ` <8bd0f97a0710311216l25f89b37v356fd44d7c1999da-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2007-10-31 20:14                         ` David Brownell
2007-10-30  9:18   ` [PATCH 10/14] Blackfin SPI driver: Clean up useless wait in bfin SPI driver Bryan Wu
2007-10-30  9:18   ` [PATCH 11/14] Blackfin SPI driver: Move global SPI regs_base and dma_ch to struct driver_data Bryan Wu
2007-10-30  9:18   ` [PATCH 12/14] Blackfin SPI driver: Fix bug in u16_cs_chg_reader to read data_len-2 bytes data firstly, then read out the last 2 bytes data Bryan Wu
2007-10-30  9:18   ` [PATCH 13/14] Blackfin SPI driver: Move cs_chg_udelay to cs_deactive to fix bug when some SPI LCD driver needs delay after cs_deactive Bryan Wu
     [not found]     ` <1193735885-8202-14-git-send-email-bryan.wu-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
2007-10-30 20:18       ` David Brownell
     [not found]         ` <200710301319.00071.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-10-31  6:30           ` Bryan Wu
2007-10-31 19:04             ` Cameron Barfield
2007-10-30  9:18   ` [PATCH 14/14] Blackfin SPI driver: set correct baud for spi mmc and enable SPI after DMA Bryan Wu
2007-10-30 20:24   ` [PATCH 00/14] Blackfin on-chip SPI controller driver updates and bug-fixing David Brownell
     [not found]     ` <200710301324.07691.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-10-30 20:29       ` Mike Frysinger
     [not found]         ` <8bd0f97a0710301329i8a21b22ncc9f71a81bc06543-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2007-10-30 20:42           ` David Brownell
     [not found]             ` <200710301342.18559.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-10-30 20:54               ` David Brownell

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=200710301305.02640.david-b@pacbell.net \
    --to=david-b@pacbell.net \
    --cc=bryan.wu@analog.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sonic.zhang@analog.com \
    --cc=spi-devel-general@lists.sourceforge.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).