* [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size
@ 2015-03-12 2:31 ` Nobuhiro Iwamatsu
0 siblings, 0 replies; 14+ messages in thread
From: Nobuhiro Iwamatsu @ 2015-03-12 2:31 UTC (permalink / raw)
To: broonie; +Cc: linux-sh, linux-spi, yoshihiro.shimoda.uh, Nobuhiro Iwamatsu
This driver is the case of tx_buf was set or SPI_MASTER_MUST_TX is set to
master_flags, will be transmit mode. However, the current code, when
transmit mode and buffer is NULL, will be set to value of receive mode
size.
This is when the SPI_MASTER_MUST_TX is set to master_flags, sets to
transmit and receive either small size of FIFO, so as not to set the
actual size larger than value of FIFO.
Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
---
drivers/spi/spi-sh-msiof.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index e57eec0..260f433 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -606,12 +606,26 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
{
int fifo_shift;
int ret;
+ int rx_words = min_t(int, words, p->rx_fifo_size);
+ int tx_words = min_t(int, words, p->tx_fifo_size);
- /* limit maximum word transfer to rx/tx fifo size */
- if (tx_buf)
- words = min_t(int, words, p->tx_fifo_size);
- if (rx_buf)
- words = min_t(int, words, p->rx_fifo_size);
+ /*
+ * limit maximum word transfer to rx/tx fifo size.
+ *
+ * If SPI_MASTER_MUST_TX was enabled in master_flags, words was
+ * set to small value of FIFO.
+ */
+ if (p->chipdata->master_flags & SPI_MASTER_MUST_TX) {
+ if (rx_words > tx_words)
+ words = tx_words;
+ else
+ words = rx_words;
+ } else {
+ if (tx_buf)
+ words = tx_words;
+ if (rx_buf)
+ words = rx_words;
+ }
/* the fifo contents need shifting */
fifo_shift = 32 - bits;
--
2.1.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size
@ 2015-03-12 2:31 ` Nobuhiro Iwamatsu
0 siblings, 0 replies; 14+ messages in thread
From: Nobuhiro Iwamatsu @ 2015-03-12 2:31 UTC (permalink / raw)
To: broonie; +Cc: linux-sh, linux-spi, yoshihiro.shimoda.uh, Nobuhiro Iwamatsu
This driver is the case of tx_buf was set or SPI_MASTER_MUST_TX is set to
master_flags, will be transmit mode. However, the current code, when
transmit mode and buffer is NULL, will be set to value of receive mode
size.
This is when the SPI_MASTER_MUST_TX is set to master_flags, sets to
transmit and receive either small size of FIFO, so as not to set the
actual size larger than value of FIFO.
Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
---
drivers/spi/spi-sh-msiof.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index e57eec0..260f433 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -606,12 +606,26 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
{
int fifo_shift;
int ret;
+ int rx_words = min_t(int, words, p->rx_fifo_size);
+ int tx_words = min_t(int, words, p->tx_fifo_size);
- /* limit maximum word transfer to rx/tx fifo size */
- if (tx_buf)
- words = min_t(int, words, p->tx_fifo_size);
- if (rx_buf)
- words = min_t(int, words, p->rx_fifo_size);
+ /*
+ * limit maximum word transfer to rx/tx fifo size.
+ *
+ * If SPI_MASTER_MUST_TX was enabled in master_flags, words was
+ * set to small value of FIFO.
+ */
+ if (p->chipdata->master_flags & SPI_MASTER_MUST_TX) {
+ if (rx_words > tx_words)
+ words = tx_words;
+ else
+ words = rx_words;
+ } else {
+ if (tx_buf)
+ words = tx_words;
+ if (rx_buf)
+ words = rx_words;
+ }
/* the fifo contents need shifting */
fifo_shift = 32 - bits;
--
2.1.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size
2015-03-12 2:31 ` Nobuhiro Iwamatsu
(?)
@ 2015-03-12 11:37 ` Mark Brown
-1 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2015-03-12 11:37 UTC (permalink / raw)
To: Nobuhiro Iwamatsu; +Cc: linux-sh, linux-spi, yoshihiro.shimoda.uh
[-- Attachment #1: Type: text/plain, Size: 313 bytes --]
On Thu, Mar 12, 2015 at 11:31:16AM +0900, Nobuhiro Iwamatsu wrote:
> This driver is the case of tx_buf was set or SPI_MASTER_MUST_TX is set to
> master_flags, will be transmit mode. However, the current code, when
> transmit mode and buffer is NULL, will be set to value of receive mode
> size.
Applied, thanks.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size
2015-03-12 2:31 ` Nobuhiro Iwamatsu
@ 2015-03-12 12:35 ` Geert Uytterhoeven
-1 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2015-03-12 12:35 UTC (permalink / raw)
To: Nobuhiro Iwamatsu; +Cc: Mark Brown, Linux-sh list, linux-spi, Yoshihiro Shimoda
Hi Iwamatsu-san,
On Thu, Mar 12, 2015 at 3:31 AM, Nobuhiro Iwamatsu
<nobuhiro.iwamatsu.yj@renesas.com> wrote:
> This driver is the case of tx_buf was set or SPI_MASTER_MUST_TX is set to
> master_flags, will be transmit mode. However, the current code, when
> transmit mode and buffer is NULL, will be set to value of receive mode
> size.
If SPI_MASTER_MUST_TX is set, tx_buf will never be NULL.
If an SPI slave driver submits a receive-only request, the SPI core will
provide a dummy buffer (spi_master.dummy_tx).
> This is when the SPI_MASTER_MUST_TX is set to master_flags, sets to
> transmit and receive either small size of FIFO, so as not to set the
> actual size larger than value of FIFO.
>
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
> ---
> drivers/spi/spi-sh-msiof.c | 24 +++++++++++++++++++-----
> 1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index e57eec0..260f433 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -606,12 +606,26 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
> {
> int fifo_shift;
> int ret;
> + int rx_words = min_t(int, words, p->rx_fifo_size);
> + int tx_words = min_t(int, words, p->tx_fifo_size);
>
> - /* limit maximum word transfer to rx/tx fifo size */
> - if (tx_buf)
> - words = min_t(int, words, p->tx_fifo_size);
> - if (rx_buf)
> - words = min_t(int, words, p->rx_fifo_size);
> + /*
> + * limit maximum word transfer to rx/tx fifo size.
> + *
> + * If SPI_MASTER_MUST_TX was enabled in master_flags, words was
> + * set to small value of FIFO.
> + */
> + if (p->chipdata->master_flags & SPI_MASTER_MUST_TX) {
> + if (rx_words > tx_words)
> + words = tx_words;
> + else
> + words = rx_words;
> + } else {
> + if (tx_buf)
> + words = tx_words;
> + if (rx_buf)
> + words = rx_words;
> + }
>
> /* the fifo contents need shifting */
> fifo_shift = 32 - bits;
Sorry, I fail to see what exactly this is fixing.
If SPI_MASTER_MUST_TX is set, all hardware SPI transfers are either
a) transmit-only.
b) bidirectional (transmit buffer may be a dummy, provided by the SPI core).
For case a, only the TX FIFO size matters.
- The original code ignored the RX FIFO size (rx_buf = NULL),
- After your change, it always uses the minimum of the TX and RX FIFO sizes
(granted, the RX FIFO size is larger, so this doesn't make a difference on
current hardware).
For case b, both FIFO sizes matter, and the original code handled that fine
(tx_buf != NULL, rx_buf != NULL).
What am I missing?
Are you using a backport with broken SPI_MASTER_MUST_TX handling in the SPI
core?
I've just verified that with today's tree (renesas-drivers-2015-03-12-v4.0-rc3),
SPI_MASTER_MUST_TX works fine, and a dummy tx_buf is passed when needed.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size
@ 2015-03-12 12:35 ` Geert Uytterhoeven
0 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2015-03-12 12:35 UTC (permalink / raw)
To: Nobuhiro Iwamatsu; +Cc: Mark Brown, Linux-sh list, linux-spi, Yoshihiro Shimoda
Hi Iwamatsu-san,
On Thu, Mar 12, 2015 at 3:31 AM, Nobuhiro Iwamatsu
<nobuhiro.iwamatsu.yj@renesas.com> wrote:
> This driver is the case of tx_buf was set or SPI_MASTER_MUST_TX is set to
> master_flags, will be transmit mode. However, the current code, when
> transmit mode and buffer is NULL, will be set to value of receive mode
> size.
If SPI_MASTER_MUST_TX is set, tx_buf will never be NULL.
If an SPI slave driver submits a receive-only request, the SPI core will
provide a dummy buffer (spi_master.dummy_tx).
> This is when the SPI_MASTER_MUST_TX is set to master_flags, sets to
> transmit and receive either small size of FIFO, so as not to set the
> actual size larger than value of FIFO.
>
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
> ---
> drivers/spi/spi-sh-msiof.c | 24 +++++++++++++++++++-----
> 1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index e57eec0..260f433 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -606,12 +606,26 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
> {
> int fifo_shift;
> int ret;
> + int rx_words = min_t(int, words, p->rx_fifo_size);
> + int tx_words = min_t(int, words, p->tx_fifo_size);
>
> - /* limit maximum word transfer to rx/tx fifo size */
> - if (tx_buf)
> - words = min_t(int, words, p->tx_fifo_size);
> - if (rx_buf)
> - words = min_t(int, words, p->rx_fifo_size);
> + /*
> + * limit maximum word transfer to rx/tx fifo size.
> + *
> + * If SPI_MASTER_MUST_TX was enabled in master_flags, words was
> + * set to small value of FIFO.
> + */
> + if (p->chipdata->master_flags & SPI_MASTER_MUST_TX) {
> + if (rx_words > tx_words)
> + words = tx_words;
> + else
> + words = rx_words;
> + } else {
> + if (tx_buf)
> + words = tx_words;
> + if (rx_buf)
> + words = rx_words;
> + }
>
> /* the fifo contents need shifting */
> fifo_shift = 32 - bits;
Sorry, I fail to see what exactly this is fixing.
If SPI_MASTER_MUST_TX is set, all hardware SPI transfers are either
a) transmit-only.
b) bidirectional (transmit buffer may be a dummy, provided by the SPI core).
For case a, only the TX FIFO size matters.
- The original code ignored the RX FIFO size (rx_buf == NULL),
- After your change, it always uses the minimum of the TX and RX FIFO sizes
(granted, the RX FIFO size is larger, so this doesn't make a difference on
current hardware).
For case b, both FIFO sizes matter, and the original code handled that fine
(tx_buf != NULL, rx_buf != NULL).
What am I missing?
Are you using a backport with broken SPI_MASTER_MUST_TX handling in the SPI
core?
I've just verified that with today's tree (renesas-drivers-2015-03-12-v4.0-rc3),
SPI_MASTER_MUST_TX works fine, and a dummy tx_buf is passed when needed.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size
2015-03-12 12:35 ` Geert Uytterhoeven
(?)
@ 2015-03-12 16:00 ` Mark Brown
-1 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2015-03-12 16:00 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Nobuhiro Iwamatsu, Linux-sh list, linux-spi, Yoshihiro Shimoda
[-- Attachment #1: Type: text/plain, Size: 1491 bytes --]
On Thu, Mar 12, 2015 at 01:35:10PM +0100, Geert Uytterhoeven wrote:
> On Thu, Mar 12, 2015 at 3:31 AM, Nobuhiro Iwamatsu
> > This driver is the case of tx_buf was set or SPI_MASTER_MUST_TX is set to
> > master_flags, will be transmit mode. However, the current code, when
> > transmit mode and buffer is NULL, will be set to value of receive mode
> > size.
> If SPI_MASTER_MUST_TX is set, tx_buf will never be NULL.
> If an SPI slave driver submits a receive-only request, the SPI core will
> provide a dummy buffer (spi_master.dummy_tx).
Right, my reading of the changelog was that this was a red herring.
> > + int rx_words = min_t(int, words, p->rx_fifo_size);
> > + int tx_words = min_t(int, words, p->tx_fifo_size);
> Sorry, I fail to see what exactly this is fixing.
> If SPI_MASTER_MUST_TX is set, all hardware SPI transfers are either
> a) transmit-only.
> b) bidirectional (transmit buffer may be a dummy, provided by the SPI core).
> For case a, only the TX FIFO size matters.
> - The original code ignored the RX FIFO size (rx_buf == NULL),
> - After your change, it always uses the minimum of the TX and RX FIFO sizes
> (granted, the RX FIFO size is larger, so this doesn't make a difference on
> current hardware).
...
> What am I missing?
> Are you using a backport with broken SPI_MASTER_MUST_TX handling in the SPI
> core?
My understanding was that it was just about making sure that the driver
picked the minimum of the two FIFO sizes.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size
[not found] ` <CAMuHMdXmr7QH8jSkyH2q6xCise7ew5YzyAFc6Zb0psHZt3mGrg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-03-16 1:18 ` Nobuhiro Iwamatsu
0 siblings, 0 replies; 14+ messages in thread
From: Nobuhiro Iwamatsu @ 2015-03-16 1:18 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Mark Brown, Linux-sh list, linux-spi, Yoshihiro Shimoda
Hi,
Thanks for your comment.
2015-03-12 21:35 GMT+09:00 Geert Uytterhoeven <geert@linux-m68k.org>:
> Hi Iwamatsu-san,
>
> On Thu, Mar 12, 2015 at 3:31 AM, Nobuhiro Iwamatsu
> <nobuhiro.iwamatsu.yj@renesas.com> wrote:
>> This driver is the case of tx_buf was set or SPI_MASTER_MUST_TX is set to
>> master_flags, will be transmit mode. However, the current code, when
>> transmit mode and buffer is NULL, will be set to value of receive mode
>> size.
>
> If SPI_MASTER_MUST_TX is set, tx_buf will never be NULL.
> If an SPI slave driver submits a receive-only request, the SPI core will
> provide a dummy buffer (spi_master.dummy_tx).
I see.
>
>> This is when the SPI_MASTER_MUST_TX is set to master_flags, sets to
>> transmit and receive either small size of FIFO, so as not to set the
>> actual size larger than value of FIFO.
>>
>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
>> ---
>> drivers/spi/spi-sh-msiof.c | 24 +++++++++++++++++++-----
>> 1 file changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>> index e57eec0..260f433 100644
>> --- a/drivers/spi/spi-sh-msiof.c
>> +++ b/drivers/spi/spi-sh-msiof.c
>> @@ -606,12 +606,26 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
>> {
>> int fifo_shift;
>> int ret;
>> + int rx_words = min_t(int, words, p->rx_fifo_size);
>> + int tx_words = min_t(int, words, p->tx_fifo_size);
>>
>> - /* limit maximum word transfer to rx/tx fifo size */
>> - if (tx_buf)
>> - words = min_t(int, words, p->tx_fifo_size);
>> - if (rx_buf)
>> - words = min_t(int, words, p->rx_fifo_size);
>> + /*
>> + * limit maximum word transfer to rx/tx fifo size.
>> + *
>> + * If SPI_MASTER_MUST_TX was enabled in master_flags, words was
>> + * set to small value of FIFO.
>> + */
>> + if (p->chipdata->master_flags & SPI_MASTER_MUST_TX) {
>> + if (rx_words > tx_words)
>> + words = tx_words;
>> + else
>> + words = rx_words;
>> + } else {
>> + if (tx_buf)
>> + words = tx_words;
>> + if (rx_buf)
>> + words = rx_words;
>> + }
>>
>> /* the fifo contents need shifting */
>> fifo_shift = 32 - bits;
>
> Sorry, I fail to see what exactly this is fixing.
>
> If SPI_MASTER_MUST_TX is set, all hardware SPI transfers are either
> a) transmit-only.
> b) bidirectional (transmit buffer may be a dummy, provided by the SPI core).
>
> For case a, only the TX FIFO size matters.
> - The original code ignored the RX FIFO size (rx_buf = NULL),
> - After your change, it always uses the minimum of the TX and RX FIFO sizes
> (granted, the RX FIFO size is larger, so this doesn't make a difference on
> current hardware).
Yes.
>
> For case b, both FIFO sizes matter, and the original code handled that fine
> (tx_buf != NULL, rx_buf != NULL).
>
> What am I missing?
> Are you using a backport with broken SPI_MASTER_MUST_TX handling in the SPI
> core?
When tx_buf != NULL and rx_buf != NULL, current code uses FIFO size of
rx_buffer.
Since TX FIFO size is smaller than RX FIFO size, corrent code set the
wrong value to SITMDR2 register.
Therefore, this patch selects a smaller FIFO size, adds the function to set.
Does this has become a description?
>
> I've just verified that with today's tree (renesas-drivers-2015-03-12-v4.0-rc3),
> SPI_MASTER_MUST_TX works fine, and a dummy tx_buf is passed when needed.
I think correctly work on hardware. However, driver has been set to
the correct register value?
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
> Geert
Best regards,
Nobuhiro
--
Nobuhiro Iwamatsu
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size
@ 2015-03-16 1:18 ` Nobuhiro Iwamatsu
0 siblings, 0 replies; 14+ messages in thread
From: Nobuhiro Iwamatsu @ 2015-03-16 1:18 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Mark Brown, Linux-sh list, linux-spi, Yoshihiro Shimoda
Hi,
Thanks for your comment.
2015-03-12 21:35 GMT+09:00 Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>:
> Hi Iwamatsu-san,
>
> On Thu, Mar 12, 2015 at 3:31 AM, Nobuhiro Iwamatsu
> <nobuhiro.iwamatsu.yj-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
>> This driver is the case of tx_buf was set or SPI_MASTER_MUST_TX is set to
>> master_flags, will be transmit mode. However, the current code, when
>> transmit mode and buffer is NULL, will be set to value of receive mode
>> size.
>
> If SPI_MASTER_MUST_TX is set, tx_buf will never be NULL.
> If an SPI slave driver submits a receive-only request, the SPI core will
> provide a dummy buffer (spi_master.dummy_tx).
I see.
>
>> This is when the SPI_MASTER_MUST_TX is set to master_flags, sets to
>> transmit and receive either small size of FIFO, so as not to set the
>> actual size larger than value of FIFO.
>>
>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>> ---
>> drivers/spi/spi-sh-msiof.c | 24 +++++++++++++++++++-----
>> 1 file changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>> index e57eec0..260f433 100644
>> --- a/drivers/spi/spi-sh-msiof.c
>> +++ b/drivers/spi/spi-sh-msiof.c
>> @@ -606,12 +606,26 @@ static int sh_msiof_spi_txrx_once(struct sh_msiof_spi_priv *p,
>> {
>> int fifo_shift;
>> int ret;
>> + int rx_words = min_t(int, words, p->rx_fifo_size);
>> + int tx_words = min_t(int, words, p->tx_fifo_size);
>>
>> - /* limit maximum word transfer to rx/tx fifo size */
>> - if (tx_buf)
>> - words = min_t(int, words, p->tx_fifo_size);
>> - if (rx_buf)
>> - words = min_t(int, words, p->rx_fifo_size);
>> + /*
>> + * limit maximum word transfer to rx/tx fifo size.
>> + *
>> + * If SPI_MASTER_MUST_TX was enabled in master_flags, words was
>> + * set to small value of FIFO.
>> + */
>> + if (p->chipdata->master_flags & SPI_MASTER_MUST_TX) {
>> + if (rx_words > tx_words)
>> + words = tx_words;
>> + else
>> + words = rx_words;
>> + } else {
>> + if (tx_buf)
>> + words = tx_words;
>> + if (rx_buf)
>> + words = rx_words;
>> + }
>>
>> /* the fifo contents need shifting */
>> fifo_shift = 32 - bits;
>
> Sorry, I fail to see what exactly this is fixing.
>
> If SPI_MASTER_MUST_TX is set, all hardware SPI transfers are either
> a) transmit-only.
> b) bidirectional (transmit buffer may be a dummy, provided by the SPI core).
>
> For case a, only the TX FIFO size matters.
> - The original code ignored the RX FIFO size (rx_buf == NULL),
> - After your change, it always uses the minimum of the TX and RX FIFO sizes
> (granted, the RX FIFO size is larger, so this doesn't make a difference on
> current hardware).
Yes.
>
> For case b, both FIFO sizes matter, and the original code handled that fine
> (tx_buf != NULL, rx_buf != NULL).
>
> What am I missing?
> Are you using a backport with broken SPI_MASTER_MUST_TX handling in the SPI
> core?
When tx_buf != NULL and rx_buf != NULL, current code uses FIFO size of
rx_buffer.
Since TX FIFO size is smaller than RX FIFO size, corrent code set the
wrong value to SITMDR2 register.
Therefore, this patch selects a smaller FIFO size, adds the function to set.
Does this has become a description?
>
> I've just verified that with today's tree (renesas-drivers-2015-03-12-v4.0-rc3),
> SPI_MASTER_MUST_TX works fine, and a dummy tx_buf is passed when needed.
I think correctly work on hardware. However, driver has been set to
the correct register value?
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
> Geert
Best regards,
Nobuhiro
--
Nobuhiro Iwamatsu
--
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size
2015-03-16 1:18 ` Nobuhiro Iwamatsu
@ 2015-03-16 7:50 ` Geert Uytterhoeven
-1 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2015-03-16 7:50 UTC (permalink / raw)
To: Nobuhiro Iwamatsu; +Cc: Mark Brown, Linux-sh list, linux-spi, Yoshihiro Shimoda
Hi Iwamatsu-san,
On Mon, Mar 16, 2015 at 2:18 AM, Nobuhiro Iwamatsu
<nobuhiro.iwamatsu.yj@renesas.com> wrote:
> 2015-03-12 21:35 GMT+09:00 Geert Uytterhoeven <geert@linux-m68k.org>:
>> On Thu, Mar 12, 2015 at 3:31 AM, Nobuhiro Iwamatsu
>> <nobuhiro.iwamatsu.yj@renesas.com> wrote:
>>> - /* limit maximum word transfer to rx/tx fifo size */
>>> - if (tx_buf)
>>> - words = min_t(int, words, p->tx_fifo_size);
>>> - if (rx_buf)
>>> - words = min_t(int, words, p->rx_fifo_size);
>> Sorry, I fail to see what exactly this is fixing.
>>
>> If SPI_MASTER_MUST_TX is set, all hardware SPI transfers are either
>> a) transmit-only.
>> b) bidirectional (transmit buffer may be a dummy, provided by the SPI core).
>>
>> For case a, only the TX FIFO size matters.
>> - The original code ignored the RX FIFO size (rx_buf = NULL),
>> - After your change, it always uses the minimum of the TX and RX FIFO sizes
>> (granted, the RX FIFO size is larger, so this doesn't make a difference on
>> current hardware).
>
> Yes.
>
>>
>> For case b, both FIFO sizes matter, and the original code handled that fine
>> (tx_buf != NULL, rx_buf != NULL).
>>
>> What am I missing?
>> Are you using a backport with broken SPI_MASTER_MUST_TX handling in the SPI
>> core?
>
> When tx_buf != NULL and rx_buf != NULL, current code uses FIFO size of
> rx_buffer.
> Since TX FIFO size is smaller than RX FIFO size, corrent code set the
> wrong value to SITMDR2 register.
if tx_buf != NULL and rx_buf != NULL, current code does:
words = min_t(int, words, p->tx_fifo_size);
words = min_t(int, words, p->rx_fifo_size);
Hence words will be the minimum of the original value of words, tx_fifo_size,
and rx_fifo_size. What's wrong about that?
> Therefore, this patch selects a smaller FIFO size, adds the function to set.
>
> Does this has become a description?
>
>>
>> I've just verified that with today's tree (renesas-drivers-2015-03-12-v4.0-rc3),
>> SPI_MASTER_MUST_TX works fine, and a dummy tx_buf is passed when needed.
>
> I think correctly work on hardware. However, driver has been set to
> the correct register value?
I printed the value of words, which is passed to sh_msiof_spi_set_mode_regs().
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size
@ 2015-03-16 7:50 ` Geert Uytterhoeven
0 siblings, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2015-03-16 7:50 UTC (permalink / raw)
To: Nobuhiro Iwamatsu; +Cc: Mark Brown, Linux-sh list, linux-spi, Yoshihiro Shimoda
Hi Iwamatsu-san,
On Mon, Mar 16, 2015 at 2:18 AM, Nobuhiro Iwamatsu
<nobuhiro.iwamatsu.yj@renesas.com> wrote:
> 2015-03-12 21:35 GMT+09:00 Geert Uytterhoeven <geert@linux-m68k.org>:
>> On Thu, Mar 12, 2015 at 3:31 AM, Nobuhiro Iwamatsu
>> <nobuhiro.iwamatsu.yj@renesas.com> wrote:
>>> - /* limit maximum word transfer to rx/tx fifo size */
>>> - if (tx_buf)
>>> - words = min_t(int, words, p->tx_fifo_size);
>>> - if (rx_buf)
>>> - words = min_t(int, words, p->rx_fifo_size);
>> Sorry, I fail to see what exactly this is fixing.
>>
>> If SPI_MASTER_MUST_TX is set, all hardware SPI transfers are either
>> a) transmit-only.
>> b) bidirectional (transmit buffer may be a dummy, provided by the SPI core).
>>
>> For case a, only the TX FIFO size matters.
>> - The original code ignored the RX FIFO size (rx_buf == NULL),
>> - After your change, it always uses the minimum of the TX and RX FIFO sizes
>> (granted, the RX FIFO size is larger, so this doesn't make a difference on
>> current hardware).
>
> Yes.
>
>>
>> For case b, both FIFO sizes matter, and the original code handled that fine
>> (tx_buf != NULL, rx_buf != NULL).
>>
>> What am I missing?
>> Are you using a backport with broken SPI_MASTER_MUST_TX handling in the SPI
>> core?
>
> When tx_buf != NULL and rx_buf != NULL, current code uses FIFO size of
> rx_buffer.
> Since TX FIFO size is smaller than RX FIFO size, corrent code set the
> wrong value to SITMDR2 register.
if tx_buf != NULL and rx_buf != NULL, current code does:
words = min_t(int, words, p->tx_fifo_size);
words = min_t(int, words, p->rx_fifo_size);
Hence words will be the minimum of the original value of words, tx_fifo_size,
and rx_fifo_size. What's wrong about that?
> Therefore, this patch selects a smaller FIFO size, adds the function to set.
>
> Does this has become a description?
>
>>
>> I've just verified that with today's tree (renesas-drivers-2015-03-12-v4.0-rc3),
>> SPI_MASTER_MUST_TX works fine, and a dummy tx_buf is passed when needed.
>
> I think correctly work on hardware. However, driver has been set to
> the correct register value?
I printed the value of words, which is passed to sh_msiof_spi_set_mode_regs().
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size
2015-03-16 7:50 ` Geert Uytterhoeven
@ 2015-03-18 3:50 ` Nobuhiro Iwamatsu
-1 siblings, 0 replies; 14+ messages in thread
From: Nobuhiro Iwamatsu @ 2015-03-18 3:50 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Mark Brown, Linux-sh list, linux-spi, Yoshihiro Shimoda
Hi,
(2015/03/16 16:50), Geert Uytterhoeven wrote:
> Hi Iwamatsu-san,
>
> On Mon, Mar 16, 2015 at 2:18 AM, Nobuhiro Iwamatsu
> <nobuhiro.iwamatsu.yj@renesas.com> wrote:
>> 2015-03-12 21:35 GMT+09:00 Geert Uytterhoeven<geert@linux-m68k.org>:
>>> On Thu, Mar 12, 2015 at 3:31 AM, Nobuhiro Iwamatsu
>>> <nobuhiro.iwamatsu.yj@renesas.com> wrote:
>
>>>> - /* limit maximum word transfer to rx/tx fifo size */
>>>> - if (tx_buf)
>>>> - words = min_t(int, words, p->tx_fifo_size);
>>>> - if (rx_buf)
>>>> - words = min_t(int, words, p->rx_fifo_size);
>
>>> Sorry, I fail to see what exactly this is fixing.
>>>
>>> If SPI_MASTER_MUST_TX is set, all hardware SPI transfers are either
>>> a) transmit-only.
>>> b) bidirectional (transmit buffer may be a dummy, provided by the SPI core).
>>>
>>> For case a, only the TX FIFO size matters.
>>> - The original code ignored the RX FIFO size (rx_buf = NULL),
>>> - After your change, it always uses the minimum of the TX and RX FIFO sizes
>>> (granted, the RX FIFO size is larger, so this doesn't make a difference on
>>> current hardware).
>>
>> Yes.
>>
>>>
>>> For case b, both FIFO sizes matter, and the original code handled that fine
>>> (tx_buf != NULL, rx_buf != NULL).
>>>
>>> What am I missing?
>>> Are you using a backport with broken SPI_MASTER_MUST_TX handling in the SPI
>>> core?
>>
>> When tx_buf != NULL and rx_buf != NULL, current code uses FIFO size of
>> rx_buffer.
>> Since TX FIFO size is smaller than RX FIFO size, corrent code set the
>> wrong value to SITMDR2 register.
>
> if tx_buf != NULL and rx_buf != NULL, current code does:
>
> words = min_t(int, words, p->tx_fifo_size);
> words = min_t(int, words, p->rx_fifo_size);
>
> Hence words will be the minimum of the original value of words, tx_fifo_size,
> and rx_fifo_size. What's wrong about that?
>
I see. I understood about this.
Sorry, my bad.
Mark, if you do not apply this patch to your repository, could you ignore this?
>> Therefore, this patch selects a smaller FIFO size, adds the function to set.
>>
>> Does this has become a description?
>>
>>>
>>> I've just verified that with today's tree (renesas-drivers-2015-03-12-v4.0-rc3),
>>> SPI_MASTER_MUST_TX works fine, and a dummy tx_buf is passed when needed.
>>
>> I think correctly work on hardware. However, driver has been set to
>> the correct register value?
>
> I printed the value of words, which is passed to sh_msiof_spi_set_mode_regs().
>
> Gr{oetje,eeting}s,
>
> Geert
>
Best regards,
Nobuhiro
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size
@ 2015-03-18 3:50 ` Nobuhiro Iwamatsu
0 siblings, 0 replies; 14+ messages in thread
From: Nobuhiro Iwamatsu @ 2015-03-18 3:50 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Mark Brown, Linux-sh list, linux-spi, Yoshihiro Shimoda
Hi,
(2015/03/16 16:50), Geert Uytterhoeven wrote:
> Hi Iwamatsu-san,
>
> On Mon, Mar 16, 2015 at 2:18 AM, Nobuhiro Iwamatsu
> <nobuhiro.iwamatsu.yj@renesas.com> wrote:
>> 2015-03-12 21:35 GMT+09:00 Geert Uytterhoeven<geert@linux-m68k.org>:
>>> On Thu, Mar 12, 2015 at 3:31 AM, Nobuhiro Iwamatsu
>>> <nobuhiro.iwamatsu.yj@renesas.com> wrote:
>
>>>> - /* limit maximum word transfer to rx/tx fifo size */
>>>> - if (tx_buf)
>>>> - words = min_t(int, words, p->tx_fifo_size);
>>>> - if (rx_buf)
>>>> - words = min_t(int, words, p->rx_fifo_size);
>
>>> Sorry, I fail to see what exactly this is fixing.
>>>
>>> If SPI_MASTER_MUST_TX is set, all hardware SPI transfers are either
>>> a) transmit-only.
>>> b) bidirectional (transmit buffer may be a dummy, provided by the SPI core).
>>>
>>> For case a, only the TX FIFO size matters.
>>> - The original code ignored the RX FIFO size (rx_buf == NULL),
>>> - After your change, it always uses the minimum of the TX and RX FIFO sizes
>>> (granted, the RX FIFO size is larger, so this doesn't make a difference on
>>> current hardware).
>>
>> Yes.
>>
>>>
>>> For case b, both FIFO sizes matter, and the original code handled that fine
>>> (tx_buf != NULL, rx_buf != NULL).
>>>
>>> What am I missing?
>>> Are you using a backport with broken SPI_MASTER_MUST_TX handling in the SPI
>>> core?
>>
>> When tx_buf != NULL and rx_buf != NULL, current code uses FIFO size of
>> rx_buffer.
>> Since TX FIFO size is smaller than RX FIFO size, corrent code set the
>> wrong value to SITMDR2 register.
>
> if tx_buf != NULL and rx_buf != NULL, current code does:
>
> words = min_t(int, words, p->tx_fifo_size);
> words = min_t(int, words, p->rx_fifo_size);
>
> Hence words will be the minimum of the original value of words, tx_fifo_size,
> and rx_fifo_size. What's wrong about that?
>
I see. I understood about this.
Sorry, my bad.
Mark, if you do not apply this patch to your repository, could you ignore this?
>> Therefore, this patch selects a smaller FIFO size, adds the function to set.
>>
>> Does this has become a description?
>>
>>>
>>> I've just verified that with today's tree (renesas-drivers-2015-03-12-v4.0-rc3),
>>> SPI_MASTER_MUST_TX works fine, and a dummy tx_buf is passed when needed.
>>
>> I think correctly work on hardware. However, driver has been set to
>> the correct register value?
>
> I printed the value of words, which is passed to sh_msiof_spi_set_mode_regs().
>
> Gr{oetje,eeting}s,
>
> Geert
>
Best regards,
Nobuhiro
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size
[not found] ` <5508F607.7010104-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
@ 2015-03-18 10:51 ` Mark Brown
0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2015-03-18 10:51 UTC (permalink / raw)
To: Nobuhiro Iwamatsu
Cc: Geert Uytterhoeven, Linux-sh list, linux-spi, Yoshihiro Shimoda
[-- Attachment #1: Type: text/plain, Size: 269 bytes --]
On Wed, Mar 18, 2015 at 12:50:31PM +0900, Nobuhiro Iwamatsu wrote:
> (2015/03/16 16:50), Geert Uytterhoeven wrote:
> I see. I understood about this.
> Sorry, my bad.
> Mark, if you do not apply this patch to your repository, could you ignore this?
OK, I'll drop it.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size
@ 2015-03-18 10:51 ` Mark Brown
0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2015-03-18 10:51 UTC (permalink / raw)
To: Nobuhiro Iwamatsu
Cc: Geert Uytterhoeven, Linux-sh list, linux-spi, Yoshihiro Shimoda
[-- Attachment #1: Type: text/plain, Size: 269 bytes --]
On Wed, Mar 18, 2015 at 12:50:31PM +0900, Nobuhiro Iwamatsu wrote:
> (2015/03/16 16:50), Geert Uytterhoeven wrote:
> I see. I understood about this.
> Sorry, my bad.
> Mark, if you do not apply this patch to your repository, could you ignore this?
OK, I'll drop it.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-03-18 10:51 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-12 2:31 [PATCH] spi: sh-msiof: Fix limit maximum word transfer size of FIFO size Nobuhiro Iwamatsu
2015-03-12 2:31 ` Nobuhiro Iwamatsu
2015-03-12 11:37 ` Mark Brown
2015-03-12 12:35 ` Geert Uytterhoeven
2015-03-12 12:35 ` Geert Uytterhoeven
2015-03-12 16:00 ` Mark Brown
[not found] ` <CAMuHMdXmr7QH8jSkyH2q6xCise7ew5YzyAFc6Zb0psHZt3mGrg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-03-16 1:18 ` Nobuhiro Iwamatsu
2015-03-16 1:18 ` Nobuhiro Iwamatsu
2015-03-16 7:50 ` Geert Uytterhoeven
2015-03-16 7:50 ` Geert Uytterhoeven
2015-03-18 3:50 ` Nobuhiro Iwamatsu
2015-03-18 3:50 ` Nobuhiro Iwamatsu
[not found] ` <5508F607.7010104-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
2015-03-18 10:51 ` Mark Brown
2015-03-18 10:51 ` Mark Brown
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.