From: Gerhard Sittig <gsi-ynQEQJNshbs@public.gmane.org>
To: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
Cc: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Feng Tang <feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
Jean-Hugues Deschenes
<jean-hugues.deschenes-YGVykHU+fedBDgjK7y7TUQ@public.gmane.org>
Subject: Re: [PATCH 4/4] spi: dw: drop unused struct dw_spi field
Date: Thu, 26 Dec 2013 12:42:58 +0100 [thread overview]
Message-ID: <20131226114258.GH8064@book.gsilab.sittig.org> (raw)
In-Reply-To: <134cfa84519ca84268fc0ef502fba12773224133.1388040447.git.baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
On Thu, Dec 26, 2013 at 09:00 +0200, Baruch Siach wrote:
>
> diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
> index c3354e8..4e01019 100644
> --- a/drivers/spi/spi-dw.c
> +++ b/drivers/spi/spi-dw.c
> @@ -427,7 +427,6 @@ static void pump_transfers(unsigned long data)
> dws->tx_end = dws->tx + transfer->len;
> dws->rx = transfer->rx_buf;
> dws->rx_end = dws->rx + transfer->len;
> - dws->cs_change = transfer->cs_change;
> dws->len = dws->cur_transfer->len;
> if (chip != dws->prev_chip)
> cs_change = 1;
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 1ccfc18..cb52cfe 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -135,7 +135,6 @@ struct dw_spi {
> u8 n_bytes; /* current is a 1/2 bytes op */
> u8 max_bits_per_word; /* maxim is 16b */
> u32 dma_width;
> - int cs_change;
> irqreturn_t (*transfer_handler)(struct dw_spi *dws);
> void (*cs_control)(u32 command);
>
> --
This looks suspicious. Are you (the driver) ignoring the
cs_change information which the caller (the code which emits the
SPI message transfer call) provides? This appears to be a
deficiency in the SPI master's code then. Callers should be able
to control CS behaviour between the partial transfers of an SPI
message. It's part of the API.
While you are checking whether the SPI master obeys the caller's
CS change spec, can you check the optional delay between partial
transfers as well (I assume that both get handled in the same
spots of the code path)? ISTR that these two aspects (and the
lack of GPIO backed chip selects given how the hardware CS line
acts) were real obstacles when trying to use this DW SPI master.
Thank you for looking at it (and for having GPIO CS lines in your
queue, which would unbreak the worst blocker)!
virtually yours
Gerhard Sittig
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office-ynQEQJNshbs@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2013-12-26 11:42 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-26 7:00 [PATCH 0/4] spi: dw: fixes, and manages resources migration Baruch Siach
[not found] ` <cover.1388040447.git.baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
2013-12-26 7:00 ` [PATCH 1/4] spi: dw: use managed resources Baruch Siach
[not found] ` <406b0c452cfa4e58b8d4c565beb861da30ebead7.1388040447.git.baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
2013-12-30 13:24 ` Mark Brown
[not found] ` <20131230132420.GZ31886-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-12-30 14:48 ` Baruch Siach
2013-12-30 16:36 ` Baruch Siach
2013-12-26 7:00 ` [PATCH 2/4] spi: dw-mmio: prepare the clock before enabling Baruch Siach
[not found] ` <1553b4e9cc24fb9fe1eb81b3ee77cdd2e1d3bc69.1388040447.git.baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
2013-12-30 13:25 ` Mark Brown
2013-12-26 7:00 ` [PATCH 3/4] spi: dw: fix memory leak on error path Baruch Siach
[not found] ` <4ed40da8c93766ac05ee8d3507ae1d9fdc0a68a7.1388040447.git.baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
2013-12-30 13:08 ` Mark Brown
[not found] ` <20131230130814.GY31886-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-12-30 13:24 ` Baruch Siach
2013-12-26 7:00 ` [PATCH 4/4] spi: dw: drop unused struct dw_spi field Baruch Siach
[not found] ` <134cfa84519ca84268fc0ef502fba12773224133.1388040447.git.baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
2013-12-26 11:42 ` Gerhard Sittig [this message]
[not found] ` <20131226114258.GH8064-kDjWylLy9wD0K7fsECOQyeGNnDKD8DIp@public.gmane.org>
2013-12-26 11:58 ` Baruch Siach
[not found] ` <20131226115826.GY11794-MwjkAAnuF3khR1HGirfZ1z4kX+cae0hd@public.gmane.org>
2013-12-26 15:33 ` Gerhard Sittig
[not found] ` <20131226153346.GN8064-kDjWylLy9wD0K7fsECOQyeGNnDKD8DIp@public.gmane.org>
2013-12-26 20:25 ` Baruch Siach
2014-01-01 16:21 ` Feng Tang
2014-01-01 17:32 ` Mark Brown
[not found] ` <20140101173249.GS31886-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2014-01-01 19:13 ` Baruch Siach
2014-01-02 2:04 ` Feng Tang
2013-12-30 13:24 ` Mark Brown
2013-12-26 11:55 ` [PATCH 0/4] spi: dw: fixes, and manages resources migration Gerhard Sittig
[not found] ` <20131226115507.GI8064-kDjWylLy9wD0K7fsECOQyeGNnDKD8DIp@public.gmane.org>
2013-12-26 12:12 ` Baruch Siach
[not found] ` <20131226121224.GA11794-MwjkAAnuF3khR1HGirfZ1z4kX+cae0hd@public.gmane.org>
2013-12-26 15:42 ` Gerhard Sittig
[not found] ` <20131226154250.GO8064-kDjWylLy9wD0K7fsECOQyeGNnDKD8DIp@public.gmane.org>
2013-12-26 20:34 ` Baruch Siach
2013-12-31 12:34 ` Gerhard Sittig
[not found] ` <20131231123425.GP8064-kDjWylLy9wD0K7fsECOQyeGNnDKD8DIp@public.gmane.org>
2014-01-01 5:01 ` Baruch Siach
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=20131226114258.GH8064@book.gsilab.sittig.org \
--to=gsi-ynqeqjnshbs@public.gmane.org \
--cc=baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org \
--cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=feng.tang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
--cc=jean-hugues.deschenes-YGVykHU+fedBDgjK7y7TUQ@public.gmane.org \
--cc=linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
/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).