[PATCH 1/3] spi-topcliff-pch: Transform noisy message to dev_dbg

[PATCH 1/3] spi-topcliff-pch: Transform noisy message to dev_dbg

[Alexander Stein]

If during a SPI transfer with len larger than PCH_MAX_FIFO_DEPTH and the
IRQ handler happens to be called when the transmit FIFO is already empty,
and SPSR_FI_BIT is set consequently, the message
"spi_master spi32766: pch_spi_handler_sub : Transfer is not completed"
is spammed to the systemlog, because tx_index has already increased
further due to the next bytes to be written. This case is uncritical as
new bytes have already been written.

Signed-off-by: Alexander Stein <alexander.stein-93q1YBGzJSMe9JSWTWOYM3xStJ4P+DSV@public.gmane.org>
---
 drivers/spi/spi-topcliff-pch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-topcliff-pch.c b/drivers/spi/spi-topcliff-pch.c
index 4461313..d038a7b 100644
--- a/drivers/spi/spi-topcliff-pch.c
+++ b/drivers/spi/spi-topcliff-pch.c
@@ -332,7 +332,7 @@ static void pch_spi_handler_sub(struct pch_spi_data *data, u32 reg_spsr_val,
 				data->transfer_active = false;
 				wake_up(&data->wait);
 			} else {
-				dev_err(&data->master->dev,
+				dev_dbg(&data->master->dev,
 					"%s : Transfer is not completed",
 					__func__);
 			}
-- 
1.8.3.2

--
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

[PATCH 2/3] spi-topcliff-pch: Fix DMA channel

[Alexander Stein]

bus_num might be asigned dynamically to e.g. 32766. In this case the
calculated DMA channel based on SPI bus number is bogus. Use SPI channel
number instead for calculation.

Signed-off-by: Alexander Stein <alexander.stein-93q1YBGzJSMe9JSWTWOYM3xStJ4P+DSV@public.gmane.org>
---
 drivers/spi/spi-topcliff-pch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-topcliff-pch.c b/drivers/spi/spi-topcliff-pch.c
index d038a7b..7cb71b0 100644
--- a/drivers/spi/spi-topcliff-pch.c
+++ b/drivers/spi/spi-topcliff-pch.c
@@ -921,7 +921,7 @@ static void pch_spi_request_dma(struct pch_spi_data *data, int bpw)
 	/* Set Tx DMA */
 	param = &dma->param_tx;
 	param->dma_dev = &dma_dev->dev;
-	param->chan_id = data->master->bus_num * 2; /* Tx = 0, 2 */
+	param->chan_id = data->ch * 2; /* Tx = 0, 2 */;
 	param->tx_reg = data->io_base_addr + PCH_SPDWR;
 	param->width = width;
 	chan = dma_request_channel(mask, pch_spi_filter, param);
@@ -936,7 +936,7 @@ static void pch_spi_request_dma(struct pch_spi_data *data, int bpw)
 	/* Set Rx DMA */
 	param = &dma->param_rx;
 	param->dma_dev = &dma_dev->dev;
-	param->chan_id = data->master->bus_num * 2 + 1; /* Rx = Tx + 1 */
+	param->chan_id = data->ch * 2 + 1; /* Rx = Tx + 1 */;
 	param->rx_reg = data->io_base_addr + PCH_SPDRR;
 	param->width = width;
 	chan = dma_request_channel(mask, pch_spi_filter, param);
-- 
1.8.3.2

--
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

Re: [PATCH 2/3] spi-topcliff-pch: Fix DMA channel

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 263 bytes --]

On Wed, Feb 19, 2014 at 10:36:08AM +0100, Alexander Stein wrote:
> bus_num might be asigned dynamically to e.g. 32766. In this case the
> calculated DMA channel based on SPI bus number is bogus. Use SPI channel
> number instead for calculation.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 3/3] spi-topcliff-pch: Fix debug message

[Alexander Stein]

Signed-off-by: Alexander Stein <alexander.stein-93q1YBGzJSMe9JSWTWOYM3xStJ4P+DSV@public.gmane.org>
---
 drivers/spi/spi-topcliff-pch.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/spi/spi-topcliff-pch.c b/drivers/spi/spi-topcliff-pch.c
index 7cb71b0..c2a3435 100644
--- a/drivers/spi/spi-topcliff-pch.c
+++ b/drivers/spi/spi-topcliff-pch.c
@@ -1157,8 +1157,7 @@ static void pch_spi_handle_dma(struct pch_spi_data *data, int *bpw)
 	dma->nent = num;
 	dma->desc_tx = desc_tx;
 
-	dev_dbg(&data->master->dev, "\n%s:Pulling down SSN low - writing "
-		"0x2 to SSNXCR\n", __func__);
+	dev_dbg(&data->master->dev, "%s:Pulling down SSN low - writing 0x2 to SSNXCR\n", __func__);
 
 	spin_lock_irqsave(&data->lock, flags);
 	pch_spi_writereg(data->master, PCH_SSNXCR, SSN_LOW);
-- 
1.8.3.2

--
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

Re: [PATCH 3/3] spi-topcliff-pch: Fix debug message

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 241 bytes --]

On Wed, Feb 19, 2014 at 10:36:09AM +0100, Alexander Stein wrote:
> Signed-off-by: Alexander Stein <alexander.stein-93q1YBGzJSMe9JSWTWOYM3xStJ4P+DSV@public.gmane.org>

Applied, thanks.  Please use subject lines consistent with the
subsystem.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/3] spi-topcliff-pch: Transform noisy message to dev_dbg

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 774 bytes --]

On Wed, Feb 19, 2014 at 10:36:07AM +0100, Alexander Stein wrote:
> If during a SPI transfer with len larger than PCH_MAX_FIFO_DEPTH and the
> IRQ handler happens to be called when the transmit FIFO is already empty,
> and SPSR_FI_BIT is set consequently, the message
> "spi_master spi32766: pch_spi_handler_sub : Transfer is not completed"
> is spammed to the systemlog, because tx_index has already increased
> further due to the next bytes to be written. This case is uncritical as
> new bytes have already been written.

When you say "spammed" does that mean "printed once" or "is continually
printed"?  I'd have assumed the latter in which case it might be better
to go as low as _vdbg() or even remove the message entirely if it's not
possible to suppress it entirely.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/3] spi-topcliff-pch: Transform noisy message to dev_dbg

[Alexander Stein]

Hello Mark,

On Wednesday 19 February 2014 21:41:50, Mark Brown wrote:
> On Wed, Feb 19, 2014 at 10:36:07AM +0100, Alexander Stein wrote:
> > If during a SPI transfer with len larger than PCH_MAX_FIFO_DEPTH and the
> > IRQ handler happens to be called when the transmit FIFO is already empty,
> > and SPSR_FI_BIT is set consequently, the message
> > "spi_master spi32766: pch_spi_handler_sub : Transfer is not completed"
> > is spammed to the systemlog, because tx_index has already increased
> > further due to the next bytes to be written. This case is uncritical as
> > new bytes have already been written.
> 
> When you say "spammed" does that mean "printed once" or "is continually
> printed"?  I'd have assumed the latter in which case it might be better
> to go as low as _vdbg() or even remove the message entirely if it's not
> possible to suppress it entirely.

It is printed continually during the whole SPI transfer, e.g. dumping a 256k AT25 EEPROM.
I don't know this hardware good enough to judge if it can be suppressed at all. Maybe if the interrupt thresholds are appropriately this doesn't happen. But maybe if someone changes that, maybe this driver can be changed to a queued driver instead which could simplify the driver.

Best regards,
Alexander

--
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

[PATCH v2] spi-topcliff-pch: Transform noisy message to dev_vdbg

[Alexander Stein]

If during a SPI transfer with len larger than PCH_MAX_FIFO_DEPTH and the
IRQ handler happens to be called when the transmit FIFO is already empty,
and SPSR_FI_BIT is set consequently, the message
"spi_master spi32766: pch_spi_handler_sub : Transfer is not completed"
is spammed to the systemlog, because tx_index has already increased
further due to the next bytes to be written. This case is uncritical as
new bytes have already been written.

Signed-off-by: Alexander Stein <alexander.stein-93q1YBGzJSMe9JSWTWOYM3xStJ4P+DSV@public.gmane.org>
---
Changes in v2:
* use dev_vdbg instead of dev_dbg

 drivers/spi/spi-topcliff-pch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-topcliff-pch.c b/drivers/spi/spi-topcliff-pch.c
index 88eb57e..667a1bb 100644
--- a/drivers/spi/spi-topcliff-pch.c
+++ b/drivers/spi/spi-topcliff-pch.c
@@ -332,7 +332,7 @@ static void pch_spi_handler_sub(struct pch_spi_data *data, u32 reg_spsr_val,
 				data->transfer_active = false;
 				wake_up(&data->wait);
 			} else {
-				dev_err(&data->master->dev,
+				dev_vdbg(&data->master->dev,
 					"%s : Transfer is not completed",
 					__func__);
 			}
-- 
1.8.3.2

--
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

Re: [PATCH v2] spi-topcliff-pch: Transform noisy message to dev_vdbg

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 355 bytes --]

On Tue, Mar 25, 2014 at 08:10:32AM +0100, Alexander Stein wrote:
> If during a SPI transfer with len larger than PCH_MAX_FIFO_DEPTH and the
> IRQ handler happens to be called when the transmit FIFO is already empty,
> and SPSR_FI_BIT is set consequently, the message

Applied, thanks.  Please use subject lines consistent with the style of
the subsystem.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]