[PATCH 1/6] spi: spi-s3c64xx: Remove unused driver data structure tgl_spi field

[PATCH 1/6] spi: spi-s3c64xx: Remove unused driver data structure tgl_spi field

[Sylwester Nawrocki]

The tgl_spi pointer is now unused so remove it.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index baa3a9fa2638..f7ca7ed33e1e 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -156,7 +156,6 @@ struct s3c64xx_spi_port_config {
  * @ioclk: Pointer to the i/o clock between master and slave
  * @master: Pointer to the SPI Protocol master.
  * @cntrlr_info: Platform specific data for the controller this driver manages.
- * @tgl_spi: Pointer to the last CS left untoggled by the cs_change hint.
  * @lock: Controller specific lock.
  * @state: Set of FLAGS to indicate status.
  * @rx_dmach: Controller's DMA channel for Rx.
@@ -177,7 +176,6 @@ struct s3c64xx_spi_driver_data {
 	struct platform_device          *pdev;
 	struct spi_master               *master;
 	struct s3c64xx_spi_info  *cntrlr_info;
-	struct spi_device               *tgl_spi;
 	spinlock_t                      lock;
 	unsigned long                   sfr_start;
 	struct completion               xfer_completion;
-- 
2.14.2

[PATCH 1/6] spi: spi-s3c64xx: Remove unused driver data structure tgl_spi field

[Sylwester Nawrocki]

The tgl_spi pointer is now unused so remove it.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index baa3a9fa2638..f7ca7ed33e1e 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -156,7 +156,6 @@ struct s3c64xx_spi_port_config {
  * @ioclk: Pointer to the i/o clock between master and slave
  * @master: Pointer to the SPI Protocol master.
  * @cntrlr_info: Platform specific data for the controller this driver manages.
- * @tgl_spi: Pointer to the last CS left untoggled by the cs_change hint.
  * @lock: Controller specific lock.
  * @state: Set of FLAGS to indicate status.
  * @rx_dmach: Controller's DMA channel for Rx.
@@ -177,7 +176,6 @@ struct s3c64xx_spi_driver_data {
 	struct platform_device          *pdev;
 	struct spi_master               *master;
 	struct s3c64xx_spi_info  *cntrlr_info;
-	struct spi_device               *tgl_spi;
 	spinlock_t                      lock;
 	unsigned long                   sfr_start;
 	struct completion               xfer_completion;
-- 
2.14.2

[PATCH 2/6] spi: spi-s3c64xx: Remove unused s3c64xx_spi_hwinit() function argument

[Sylwester Nawrocki]

The channel argument is not used and anyway it could be retrieved from
the passed driver data structure.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index f7ca7ed33e1e..b7333a883442 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -889,7 +889,7 @@ static irqreturn_t s3c64xx_spi_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd, int channel)
+static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd)
 {
 	struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
 	void __iomem *regs = sdd->regs;
@@ -1143,7 +1143,7 @@ static int s3c64xx_spi_probe(struct platform_device *pdev)
 	pm_runtime_get_sync(&pdev->dev);
 
 	/* Setup Deufult Mode */
-	s3c64xx_spi_hwinit(sdd, sdd->port_id);
+	s3c64xx_spi_hwinit(sdd);
 
 	spin_lock_init(&sdd->lock);
 	init_completion(&sdd->xfer_completion);
@@ -1258,7 +1258,7 @@ static int s3c64xx_spi_resume(struct device *dev)
 	if (ret < 0)
 		return ret;
 
-	s3c64xx_spi_hwinit(sdd, sdd->port_id);
+	s3c64xx_spi_hwinit(sdd);
 
 	return spi_master_resume(master);
 }
-- 
2.14.2

[PATCH 2/6] spi: spi-s3c64xx: Remove unused s3c64xx_spi_hwinit() function argument

[Sylwester Nawrocki]

The channel argument is not used and anyway it could be retrieved from
the passed driver data structure.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index f7ca7ed33e1e..b7333a883442 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -889,7 +889,7 @@ static irqreturn_t s3c64xx_spi_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd, int channel)
+static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd)
 {
 	struct s3c64xx_spi_info *sci = sdd->cntrlr_info;
 	void __iomem *regs = sdd->regs;
@@ -1143,7 +1143,7 @@ static int s3c64xx_spi_probe(struct platform_device *pdev)
 	pm_runtime_get_sync(&pdev->dev);
 
 	/* Setup Deufult Mode */
-	s3c64xx_spi_hwinit(sdd, sdd->port_id);
+	s3c64xx_spi_hwinit(sdd);
 
 	spin_lock_init(&sdd->lock);
 	init_completion(&sdd->xfer_completion);
@@ -1258,7 +1258,7 @@ static int s3c64xx_spi_resume(struct device *dev)
 	if (ret < 0)
 		return ret;
 
-	s3c64xx_spi_hwinit(sdd, sdd->port_id);
+	s3c64xx_spi_hwinit(sdd);
 
 	return spi_master_resume(master);
 }
-- 
2.14.2

Re: [PATCH 2/6] spi: spi-s3c64xx: Remove unused s3c64xx_spi_hwinit() function argument

[andi]

Hi Sylwester,

On 17.04.2018 00:40, Sylwester Nawrocki wrote:
> The channel argument is not used and anyway it could be retrieved from
> the passed driver data structure.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

Reviewed-by: Andi Shyti <andi@etezian.org>

Thanks,
Andi

[PATCH 2/6] spi: spi-s3c64xx: Remove unused s3c64xx_spi_hwinit() function argument

[andi at etezian.org]

Hi Sylwester,

On 17.04.2018 00:40, Sylwester Nawrocki wrote:
> The channel argument is not used and anyway it could be retrieved from
> the passed driver data structure.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

Reviewed-by: Andi Shyti <andi@etezian.org>

Thanks,
Andi

Re: [PATCH 2/6] spi: spi-s3c64xx: Remove unused s3c64xx_spi_hwinit() function argument

[Krzysztof Kozlowski]

On Mon, Apr 16, 2018 at 5:40 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> The channel argument is not used and anyway it could be retrieved from
> the passed driver data structure.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

[PATCH 2/6] spi: spi-s3c64xx: Remove unused s3c64xx_spi_hwinit() function argument

[Krzysztof Kozlowski]

On Mon, Apr 16, 2018 at 5:40 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> The channel argument is not used and anyway it could be retrieved from
> the passed driver data structure.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

[PATCH 3/6] spi: spi-s3c64xx: Drop unused enable_datapath() function argument

[Sylwester Nawrocki]

The spi pointer argument is not used now so remove it. While at it
s3c64xx_ prefix is added to the function name.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index b7333a883442..e90e60bc1ecc 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -348,9 +348,8 @@ static bool s3c64xx_spi_can_dma(struct spi_master *master,
 	return xfer->len > (FIFO_LVL_MASK(sdd) >> 1) + 1;
 }

-static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
-				struct spi_device *spi,
-				struct spi_transfer *xfer, int dma_mode)
+static void s3c64xx_enable_datapath(struct s3c64xx_spi_driver_data *sdd,
+				    struct spi_transfer *xfer, int dma_mode)
 {
 	void __iomem *regs = sdd->regs;
 	u32 modecfg, chcfg;
@@ -665,7 +664,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	sdd->state &= ~RXBUSY;
 	sdd->state &= ~TXBUSY;

-	enable_datapath(sdd, spi, xfer, use_dma);
+	s3c64xx_enable_datapath(sdd, xfer, use_dma);

 	/* Start the signals */
 	s3c64xx_spi_set_cs(spi, true);
--
2.14.2

[PATCH 3/6] spi: spi-s3c64xx: Drop unused enable_datapath() function argument

[Sylwester Nawrocki]

The spi pointer argument is not used now so remove it. While at it
s3c64xx_ prefix is added to the function name.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index b7333a883442..e90e60bc1ecc 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -348,9 +348,8 @@ static bool s3c64xx_spi_can_dma(struct spi_master *master,
 	return xfer->len > (FIFO_LVL_MASK(sdd) >> 1) + 1;
 }

-static void enable_datapath(struct s3c64xx_spi_driver_data *sdd,
-				struct spi_device *spi,
-				struct spi_transfer *xfer, int dma_mode)
+static void s3c64xx_enable_datapath(struct s3c64xx_spi_driver_data *sdd,
+				    struct spi_transfer *xfer, int dma_mode)
 {
 	void __iomem *regs = sdd->regs;
 	u32 modecfg, chcfg;
@@ -665,7 +664,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	sdd->state &= ~RXBUSY;
 	sdd->state &= ~TXBUSY;

-	enable_datapath(sdd, spi, xfer, use_dma);
+	s3c64xx_enable_datapath(sdd, xfer, use_dma);

 	/* Start the signals */
 	s3c64xx_spi_set_cs(spi, true);
--
2.14.2

Re: [PATCH 3/6] spi: spi-s3c64xx: Drop unused enable_datapath() function argument

[andi]

Hi Sylwester,

On 17.04.2018 00:40, Sylwester Nawrocki wrote:
> The spi pointer argument is not used now so remove it. While at it
> s3c64xx_ prefix is added to the function name.

These for me are two different patches, but in any case it's trivial
so that I don't mind it too much.

> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

Reviewed-by: Andi Shyti <andi@etezian.org>

Thanks,
Andi

[PATCH 3/6] spi: spi-s3c64xx: Drop unused enable_datapath() function argument

[andi at etezian.org]

Hi Sylwester,

On 17.04.2018 00:40, Sylwester Nawrocki wrote:
> The spi pointer argument is not used now so remove it. While at it
> s3c64xx_ prefix is added to the function name.

These for me are two different patches, but in any case it's trivial
so that I don't mind it too much.

> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

Reviewed-by: Andi Shyti <andi@etezian.org>

Thanks,
Andi

Re: [PATCH 3/6] spi: spi-s3c64xx: Drop unused enable_datapath() function argument

[Sylwester Nawrocki]

Hi Andi,

On 04/16/2018 09:09 PM, andi@etezian.org wrote:
> On 17.04.2018 00:40, Sylwester Nawrocki wrote:
>> The spi pointer argument is not used now so remove it. While at it
>> s3c64xx_ prefix is added to the function name.
>
> These for me are two different patches, but in any case it's trivial
> so that I don't mind it too much.

I am going to drop renaming from this patch. There are 2 more functions
(wait_for_pio(), wait_for_dma()) without prefix, should we rename them
too or rather leave all 3 as they are now?

-- 
Regards,
Sylwester

[PATCH 3/6] spi: spi-s3c64xx: Drop unused enable_datapath() function argument

[Sylwester Nawrocki]

Hi Andi,

On 04/16/2018 09:09 PM, andi at etezian.org wrote:
> On 17.04.2018 00:40, Sylwester Nawrocki wrote:
>> The spi pointer argument is not used now so remove it. While at it
>> s3c64xx_ prefix is added to the function name.
>
> These for me are two different patches, but in any case it's trivial
> so that I don't mind it too much.

I am going to drop renaming from this patch. There are 2 more functions
(wait_for_pio(), wait_for_dma()) without prefix, should we rename them
too or rather leave all 3 as they are now?

-- 
Regards,
Sylwester

Re: [PATCH 3/6] spi: spi-s3c64xx: Drop unused enable_datapath() function argument

[Krzysztof Kozlowski]

On Tue, Apr 17, 2018 at 12:02 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> Hi Andi,
>
> On 04/16/2018 09:09 PM, andi@etezian.org wrote:
>> On 17.04.2018 00:40, Sylwester Nawrocki wrote:
>>> The spi pointer argument is not used now so remove it. While at it
>>> s3c64xx_ prefix is added to the function name.
>>
>> These for me are two different patches, but in any case it's trivial
>> so that I don't mind it too much.
>
> I am going to drop renaming from this patch. There are 2 more functions
> (wait_for_pio(), wait_for_dma()) without prefix, should we rename them
> too or rather leave all 3 as they are now?

Indeed it is kind of mixed up - some purely local functions (not even
passed through pointer to some ops) have prefix, some do not. If you
ask me, I would prefer to have the prefix on all of them because:
1. If they appear in call trace (where backtrace pointers are not
telling anything) it is easy to understand their location,
2. having generic name like "wait_for_dma" sounds for me like generic,
Linux-wide macro (but it is not).

However I do not insist on that.

Best regards,
Krzysztof

[PATCH 3/6] spi: spi-s3c64xx: Drop unused enable_datapath() function argument

[Krzysztof Kozlowski]

On Tue, Apr 17, 2018 at 12:02 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> Hi Andi,
>
> On 04/16/2018 09:09 PM, andi at etezian.org wrote:
>> On 17.04.2018 00:40, Sylwester Nawrocki wrote:
>>> The spi pointer argument is not used now so remove it. While at it
>>> s3c64xx_ prefix is added to the function name.
>>
>> These for me are two different patches, but in any case it's trivial
>> so that I don't mind it too much.
>
> I am going to drop renaming from this patch. There are 2 more functions
> (wait_for_pio(), wait_for_dma()) without prefix, should we rename them
> too or rather leave all 3 as they are now?

Indeed it is kind of mixed up - some purely local functions (not even
passed through pointer to some ops) have prefix, some do not. If you
ask me, I would prefer to have the prefix on all of them because:
1. If they appear in call trace (where backtrace pointers are not
telling anything) it is easy to understand their location,
2. having generic name like "wait_for_dma" sounds for me like generic,
Linux-wide macro (but it is not).

However I do not insist on that.

Best regards,
Krzysztof

Re: [PATCH 3/6] spi: spi-s3c64xx: Drop unused enable_datapath() function argument

[Krzysztof Kozlowski]

On Mon, Apr 16, 2018 at 5:40 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> The spi pointer argument is not used now so remove it. While at it
> s3c64xx_ prefix is added to the function name.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)


Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

[PATCH 3/6] spi: spi-s3c64xx: Drop unused enable_datapath() function argument

[Krzysztof Kozlowski]

On Mon, Apr 16, 2018 at 5:40 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> The spi pointer argument is not used now so remove it. While at it
> s3c64xx_ prefix is added to the function name.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)


Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

[PATCH 4/6] spi: spi-s3c64xx: Do not ignore timeout errors in polling I/O mode

[Sylwester Nawrocki]

Currently timeout errors in polling I/O mode transfer are silently ignored.
Fix it by returning an error when we time out waiting on the RX FIFO level
to reach the transfer length.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index e90e60bc1ecc..d87fa7d9fad3 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -502,6 +502,8 @@ static int wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
 		status = readl(regs + S3C64XX_SPI_STATUS);
 	} while (RX_FIFO_LVL(status, sdd) < xfer->len && --val);
 
+	if (!val)
+		return -EIO;
 
 	/* If it was only Tx */
 	if (!xfer->rx_buf) {
-- 
2.14.2

[PATCH 4/6] spi: spi-s3c64xx: Do not ignore timeout errors in polling I/O mode

[Sylwester Nawrocki]

Currently timeout errors in polling I/O mode transfer are silently ignored.
Fix it by returning an error when we time out waiting on the RX FIFO level
to reach the transfer length.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index e90e60bc1ecc..d87fa7d9fad3 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -502,6 +502,8 @@ static int wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
 		status = readl(regs + S3C64XX_SPI_STATUS);
 	} while (RX_FIFO_LVL(status, sdd) < xfer->len && --val);
 
+	if (!val)
+		return -EIO;
 
 	/* If it was only Tx */
 	if (!xfer->rx_buf) {
-- 
2.14.2

Re: [PATCH 4/6] spi: spi-s3c64xx: Do not ignore timeout errors in polling I/O mode

[andi]

Hi Sylwester,

On 17.04.2018 00:40, Sylwester Nawrocki wrote:
> Currently timeout errors in polling I/O mode transfer are silently 
> ignored.
> Fix it by returning an error when we time out waiting on the RX FIFO 
> level
> to reach the transfer length.

yes, makes sense.

> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

Reviewed-by: Andi Shyti <andi@etezian.org>

Thanks,
Andi

[PATCH 4/6] spi: spi-s3c64xx: Do not ignore timeout errors in polling I/O mode

[andi at etezian.org]

Hi Sylwester,

On 17.04.2018 00:40, Sylwester Nawrocki wrote:
> Currently timeout errors in polling I/O mode transfer are silently 
> ignored.
> Fix it by returning an error when we time out waiting on the RX FIFO 
> level
> to reach the transfer length.

yes, makes sense.

> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

Reviewed-by: Andi Shyti <andi@etezian.org>

Thanks,
Andi

Re: [PATCH 4/6] spi: spi-s3c64xx: Do not ignore timeout errors in polling I/O mode

[Krzysztof Kozlowski]

On Mon, Apr 16, 2018 at 5:40 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> Currently timeout errors in polling I/O mode transfer are silently ignored.
> Fix it by returning an error when we time out waiting on the RX FIFO level
> to reach the transfer length.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 2 ++
>  1 file changed, 2 insertions(+)
>

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

[PATCH 4/6] spi: spi-s3c64xx: Do not ignore timeout errors in polling I/O mode

[Krzysztof Kozlowski]

On Mon, Apr 16, 2018 at 5:40 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> Currently timeout errors in polling I/O mode transfer are silently ignored.
> Fix it by returning an error when we time out waiting on the RX FIFO level
> to reach the transfer length.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 2 ++
>  1 file changed, 2 insertions(+)
>

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH 4/6] spi: spi-s3c64xx: Do not ignore timeout errors in polling I/O mode

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 336 bytes --]

On Mon, Apr 16, 2018 at 05:40:19PM +0200, Sylwester Nawrocki wrote:
> Currently timeout errors in polling I/O mode transfer are silently ignored.
> Fix it by returning an error when we time out waiting on the RX FIFO level
> to reach the transfer length.

This would probably have been better sent as a fix before the cosmetic
updates.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/6] spi: spi-s3c64xx: Do not ignore timeout errors in polling I/O mode

[Mark Brown]

On Mon, Apr 16, 2018 at 05:40:19PM +0200, Sylwester Nawrocki wrote:
> Currently timeout errors in polling I/O mode transfer are silently ignored.
> Fix it by returning an error when we time out waiting on the RX FIFO level
> to reach the transfer length.

This would probably have been better sent as a fix before the cosmetic
updates.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180417/f8e2d91c/attachment.sig>

[PATCH 5/6] spi: spi-s3c64xx: Fix indentation in the register offset definitions

[Sylwester Nawrocki]

Change indentation so register address offset and register bit definitions
are aligned to same column.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index d87fa7d9fad3..2723fa6bed8b 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -28,15 +28,15 @@

 #define S3C64XX_SPI_CH_CFG		0x00
 #define S3C64XX_SPI_CLK_CFG		0x04
-#define S3C64XX_SPI_MODE_CFG	0x08
-#define S3C64XX_SPI_SLAVE_SEL	0x0C
+#define S3C64XX_SPI_MODE_CFG		0x08
+#define S3C64XX_SPI_SLAVE_SEL		0x0C
 #define S3C64XX_SPI_INT_EN		0x10
 #define S3C64XX_SPI_STATUS		0x14
 #define S3C64XX_SPI_TX_DATA		0x18
 #define S3C64XX_SPI_RX_DATA		0x1C
-#define S3C64XX_SPI_PACKET_CNT	0x20
-#define S3C64XX_SPI_PENDING_CLR	0x24
-#define S3C64XX_SPI_SWAP_CFG	0x28
+#define S3C64XX_SPI_PACKET_CNT		0x20
+#define S3C64XX_SPI_PENDING_CLR		0x24
+#define S3C64XX_SPI_SWAP_CFG		0x28
 #define S3C64XX_SPI_FB_CLK		0x2C

 #define S3C64XX_SPI_CH_HS_EN		(1<<6)	/* High Speed Enable */
@@ -77,9 +77,9 @@
 #define S3C64XX_SPI_INT_TX_FIFORDY_EN		(1<<0)

 #define S3C64XX_SPI_ST_RX_OVERRUN_ERR		(1<<5)
-#define S3C64XX_SPI_ST_RX_UNDERRUN_ERR	(1<<4)
+#define S3C64XX_SPI_ST_RX_UNDERRUN_ERR		(1<<4)
 #define S3C64XX_SPI_ST_TX_OVERRUN_ERR		(1<<3)
-#define S3C64XX_SPI_ST_TX_UNDERRUN_ERR	(1<<2)
+#define S3C64XX_SPI_ST_TX_UNDERRUN_ERR		(1<<2)
 #define S3C64XX_SPI_ST_RX_FIFORDY		(1<<1)
 #define S3C64XX_SPI_ST_TX_FIFORDY		(1<<0)

@@ -100,7 +100,7 @@
 #define S3C64XX_SPI_SWAP_TX_BIT			(1<<1)
 #define S3C64XX_SPI_SWAP_TX_EN			(1<<0)

-#define S3C64XX_SPI_FBCLK_MSK		(3<<0)
+#define S3C64XX_SPI_FBCLK_MSK			(3<<0)

 #define FIFO_LVL_MASK(i) ((i)->port_conf->fifo_lvl_mask[i->port_id])
 #define S3C64XX_SPI_ST_TX_DONE(v, i) (((v) & \
--
2.14.2

[PATCH 5/6] spi: spi-s3c64xx: Fix indentation in the register offset definitions

[Sylwester Nawrocki]

Change indentation so register address offset and register bit definitions
are aligned to same column.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index d87fa7d9fad3..2723fa6bed8b 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -28,15 +28,15 @@

 #define S3C64XX_SPI_CH_CFG		0x00
 #define S3C64XX_SPI_CLK_CFG		0x04
-#define S3C64XX_SPI_MODE_CFG	0x08
-#define S3C64XX_SPI_SLAVE_SEL	0x0C
+#define S3C64XX_SPI_MODE_CFG		0x08
+#define S3C64XX_SPI_SLAVE_SEL		0x0C
 #define S3C64XX_SPI_INT_EN		0x10
 #define S3C64XX_SPI_STATUS		0x14
 #define S3C64XX_SPI_TX_DATA		0x18
 #define S3C64XX_SPI_RX_DATA		0x1C
-#define S3C64XX_SPI_PACKET_CNT	0x20
-#define S3C64XX_SPI_PENDING_CLR	0x24
-#define S3C64XX_SPI_SWAP_CFG	0x28
+#define S3C64XX_SPI_PACKET_CNT		0x20
+#define S3C64XX_SPI_PENDING_CLR		0x24
+#define S3C64XX_SPI_SWAP_CFG		0x28
 #define S3C64XX_SPI_FB_CLK		0x2C

 #define S3C64XX_SPI_CH_HS_EN		(1<<6)	/* High Speed Enable */
@@ -77,9 +77,9 @@
 #define S3C64XX_SPI_INT_TX_FIFORDY_EN		(1<<0)

 #define S3C64XX_SPI_ST_RX_OVERRUN_ERR		(1<<5)
-#define S3C64XX_SPI_ST_RX_UNDERRUN_ERR	(1<<4)
+#define S3C64XX_SPI_ST_RX_UNDERRUN_ERR		(1<<4)
 #define S3C64XX_SPI_ST_TX_OVERRUN_ERR		(1<<3)
-#define S3C64XX_SPI_ST_TX_UNDERRUN_ERR	(1<<2)
+#define S3C64XX_SPI_ST_TX_UNDERRUN_ERR		(1<<2)
 #define S3C64XX_SPI_ST_RX_FIFORDY		(1<<1)
 #define S3C64XX_SPI_ST_TX_FIFORDY		(1<<0)

@@ -100,7 +100,7 @@
 #define S3C64XX_SPI_SWAP_TX_BIT			(1<<1)
 #define S3C64XX_SPI_SWAP_TX_EN			(1<<0)

-#define S3C64XX_SPI_FBCLK_MSK		(3<<0)
+#define S3C64XX_SPI_FBCLK_MSK			(3<<0)

 #define FIFO_LVL_MASK(i) ((i)->port_conf->fifo_lvl_mask[i->port_id])
 #define S3C64XX_SPI_ST_TX_DONE(v, i) (((v) & \
--
2.14.2

Re: [PATCH 5/6] spi: spi-s3c64xx: Fix indentation in the register offset definitions

[andi]

Hi Sylwester,

On 17.04.2018 00:40, Sylwester Nawrocki wrote:
> Change indentation so register address offset and register bit 
> definitions
> are aligned to same column.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

Reviewed-by: Andi Shyti <andi@etezian.org>

Thanks,
Andi

[PATCH 5/6] spi: spi-s3c64xx: Fix indentation in the register offset definitions

[andi at etezian.org]

Hi Sylwester,

On 17.04.2018 00:40, Sylwester Nawrocki wrote:
> Change indentation so register address offset and register bit 
> definitions
> are aligned to same column.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

Reviewed-by: Andi Shyti <andi@etezian.org>

Thanks,
Andi

Re: [PATCH 5/6] spi: spi-s3c64xx: Fix indentation in the register offset definitions

[Krzysztof Kozlowski]

On Mon, Apr 16, 2018 at 5:40 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> Change indentation so register address offset and register bit definitions
> are aligned to same column.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

[PATCH 5/6] spi: spi-s3c64xx: Fix indentation in the register offset definitions

[Krzysztof Kozlowski]

On Mon, Apr 16, 2018 at 5:40 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> Change indentation so register address offset and register bit definitions
> are aligned to same column.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

[PATCH 6/6] spi: spi-s3c64xx: Allow higher transfer lengths in polling IO mode

[Sylwester Nawrocki]

Some variants of the SPI controller have no DMA support, in such case
SPI transfers longer than the FIFO length are not currently properly
handled by the driver. Fix it by doing multiple transfers in the
s3c64xx_spi_transfer_one() function if the SPI transfer length exceeds
the FIFO size.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 108 ++++++++++++++++++++++++++++++----------------
 1 file changed, 71 insertions(+), 37 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 2723fa6bed8b..b2e8ed354dac 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -349,7 +349,7 @@ static bool s3c64xx_spi_can_dma(struct spi_master *master,
 }
 
 static void s3c64xx_enable_datapath(struct s3c64xx_spi_driver_data *sdd,
-				    struct spi_transfer *xfer, int dma_mode)
+				    struct spi_transfer *xfer, bool dma_mode)
 {
 	void __iomem *regs = sdd->regs;
 	u32 modecfg, chcfg;
@@ -634,11 +634,15 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 				    struct spi_transfer *xfer)
 {
 	struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master);
+	const unsigned int fifo_len = (FIFO_LVL_MASK(sdd) >> 1) + 1;
+	const void *tx_buf = NULL;
+	void *rx_buf = NULL;
+	int target_len = 0, origin_len = 0;
+	bool use_dma = false;
 	int status;
 	u32 speed;
 	u8 bpw;
 	unsigned long flags;
-	int use_dma;
 
 	reinit_completion(&sdd->xfer_completion);
 
@@ -653,48 +657,78 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 		s3c64xx_spi_config(sdd);
 	}
 
-	/* Polling method for xfers not bigger than FIFO capacity */
-	use_dma = 0;
-	if (!is_polling(sdd) &&
-	    (sdd->rx_dma.ch && sdd->tx_dma.ch &&
-	     (xfer->len > ((FIFO_LVL_MASK(sdd) >> 1) + 1))))
-		use_dma = 1;
+	if (!is_polling(sdd) && (xfer->len > fifo_len) &&
+	    sdd->rx_dma.ch && sdd->tx_dma.ch) {
+		use_dma = true;
 
-	spin_lock_irqsave(&sdd->lock, flags);
+	} else if (is_polling(sdd) && xfer->len > fifo_len) {
+		tx_buf = xfer->tx_buf;
+		rx_buf = xfer->rx_buf;
+		origin_len = xfer->len;
 
-	/* Pending only which is to be done */
-	sdd->state &= ~RXBUSY;
-	sdd->state &= ~TXBUSY;
+		target_len = xfer->len;
+		if (xfer->len > fifo_len)
+			xfer->len = fifo_len;
+	}
+
+	do {
+		spin_lock_irqsave(&sdd->lock, flags);
 
-	s3c64xx_enable_datapath(sdd, xfer, use_dma);
+		/* Pending only which is to be done */
+		sdd->state &= ~RXBUSY;
+		sdd->state &= ~TXBUSY;
 
-	/* Start the signals */
-	s3c64xx_spi_set_cs(spi, true);
+		s3c64xx_enable_datapath(sdd, xfer, use_dma);
 
-	spin_unlock_irqrestore(&sdd->lock, flags);
+		/* Start the signals */
+		s3c64xx_spi_set_cs(spi, true);
 
-	if (use_dma)
-		status = wait_for_dma(sdd, xfer);
-	else
-		status = wait_for_pio(sdd, xfer);
-
-	if (status) {
-		dev_err(&spi->dev, "I/O Error: rx-%d tx-%d res:rx-%c tx-%c len-%d\n",
-			xfer->rx_buf ? 1 : 0, xfer->tx_buf ? 1 : 0,
-			(sdd->state & RXBUSY) ? 'f' : 'p',
-			(sdd->state & TXBUSY) ? 'f' : 'p',
-			xfer->len);
-
-		if (use_dma) {
-			if (xfer->tx_buf != NULL
-			    && (sdd->state & TXBUSY))
-				dmaengine_terminate_all(sdd->tx_dma.ch);
-			if (xfer->rx_buf != NULL
-			    && (sdd->state & RXBUSY))
-				dmaengine_terminate_all(sdd->rx_dma.ch);
+		spin_unlock_irqrestore(&sdd->lock, flags);
+
+		if (use_dma)
+			status = wait_for_dma(sdd, xfer);
+		else
+			status = wait_for_pio(sdd, xfer);
+
+		if (status) {
+			dev_err(&spi->dev,
+				"I/O Error: rx-%d tx-%d res:rx-%c tx-%c len-%d\n",
+				xfer->rx_buf ? 1 : 0, xfer->tx_buf ? 1 : 0,
+				(sdd->state & RXBUSY) ? 'f' : 'p',
+				(sdd->state & TXBUSY) ? 'f' : 'p',
+				xfer->len);
+
+			if (use_dma) {
+				if (xfer->tx_buf && (sdd->state & TXBUSY))
+					dmaengine_terminate_all(sdd->tx_dma.ch);
+				if (xfer->rx_buf && (sdd->state & RXBUSY))
+					dmaengine_terminate_all(sdd->rx_dma.ch);
+			}
+		} else {
+			flush_fifo(sdd);
 		}
-	} else {
-		flush_fifo(sdd);
+
+		if (target_len > 0) {
+			target_len -= xfer->len;
+
+			if (xfer->tx_buf)
+				xfer->tx_buf += xfer->len;
+
+			if (xfer->rx_buf)
+				xfer->rx_buf += xfer->len;
+
+			if (target_len > fifo_len)
+				xfer->len = fifo_len;
+			else
+				xfer->len = target_len;
+		}
+	} while (target_len > 0);
+
+	if (origin_len) {
+		/* Restore original xfer buffers and length */
+		xfer->tx_buf = tx_buf;
+		xfer->rx_buf = rx_buf;
+		xfer->len = origin_len;
 	}
 
 	return status;
-- 
2.14.2

[PATCH 6/6] spi: spi-s3c64xx: Allow higher transfer lengths in polling IO mode

[Sylwester Nawrocki]

Some variants of the SPI controller have no DMA support, in such case
SPI transfers longer than the FIFO length are not currently properly
handled by the driver. Fix it by doing multiple transfers in the
s3c64xx_spi_transfer_one() function if the SPI transfer length exceeds
the FIFO size.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 108 ++++++++++++++++++++++++++++++----------------
 1 file changed, 71 insertions(+), 37 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 2723fa6bed8b..b2e8ed354dac 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -349,7 +349,7 @@ static bool s3c64xx_spi_can_dma(struct spi_master *master,
 }
 
 static void s3c64xx_enable_datapath(struct s3c64xx_spi_driver_data *sdd,
-				    struct spi_transfer *xfer, int dma_mode)
+				    struct spi_transfer *xfer, bool dma_mode)
 {
 	void __iomem *regs = sdd->regs;
 	u32 modecfg, chcfg;
@@ -634,11 +634,15 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 				    struct spi_transfer *xfer)
 {
 	struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master);
+	const unsigned int fifo_len = (FIFO_LVL_MASK(sdd) >> 1) + 1;
+	const void *tx_buf = NULL;
+	void *rx_buf = NULL;
+	int target_len = 0, origin_len = 0;
+	bool use_dma = false;
 	int status;
 	u32 speed;
 	u8 bpw;
 	unsigned long flags;
-	int use_dma;
 
 	reinit_completion(&sdd->xfer_completion);
 
@@ -653,48 +657,78 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 		s3c64xx_spi_config(sdd);
 	}
 
-	/* Polling method for xfers not bigger than FIFO capacity */
-	use_dma = 0;
-	if (!is_polling(sdd) &&
-	    (sdd->rx_dma.ch && sdd->tx_dma.ch &&
-	     (xfer->len > ((FIFO_LVL_MASK(sdd) >> 1) + 1))))
-		use_dma = 1;
+	if (!is_polling(sdd) && (xfer->len > fifo_len) &&
+	    sdd->rx_dma.ch && sdd->tx_dma.ch) {
+		use_dma = true;
 
-	spin_lock_irqsave(&sdd->lock, flags);
+	} else if (is_polling(sdd) && xfer->len > fifo_len) {
+		tx_buf = xfer->tx_buf;
+		rx_buf = xfer->rx_buf;
+		origin_len = xfer->len;
 
-	/* Pending only which is to be done */
-	sdd->state &= ~RXBUSY;
-	sdd->state &= ~TXBUSY;
+		target_len = xfer->len;
+		if (xfer->len > fifo_len)
+			xfer->len = fifo_len;
+	}
+
+	do {
+		spin_lock_irqsave(&sdd->lock, flags);
 
-	s3c64xx_enable_datapath(sdd, xfer, use_dma);
+		/* Pending only which is to be done */
+		sdd->state &= ~RXBUSY;
+		sdd->state &= ~TXBUSY;
 
-	/* Start the signals */
-	s3c64xx_spi_set_cs(spi, true);
+		s3c64xx_enable_datapath(sdd, xfer, use_dma);
 
-	spin_unlock_irqrestore(&sdd->lock, flags);
+		/* Start the signals */
+		s3c64xx_spi_set_cs(spi, true);
 
-	if (use_dma)
-		status = wait_for_dma(sdd, xfer);
-	else
-		status = wait_for_pio(sdd, xfer);
-
-	if (status) {
-		dev_err(&spi->dev, "I/O Error: rx-%d tx-%d res:rx-%c tx-%c len-%d\n",
-			xfer->rx_buf ? 1 : 0, xfer->tx_buf ? 1 : 0,
-			(sdd->state & RXBUSY) ? 'f' : 'p',
-			(sdd->state & TXBUSY) ? 'f' : 'p',
-			xfer->len);
-
-		if (use_dma) {
-			if (xfer->tx_buf != NULL
-			    && (sdd->state & TXBUSY))
-				dmaengine_terminate_all(sdd->tx_dma.ch);
-			if (xfer->rx_buf != NULL
-			    && (sdd->state & RXBUSY))
-				dmaengine_terminate_all(sdd->rx_dma.ch);
+		spin_unlock_irqrestore(&sdd->lock, flags);
+
+		if (use_dma)
+			status = wait_for_dma(sdd, xfer);
+		else
+			status = wait_for_pio(sdd, xfer);
+
+		if (status) {
+			dev_err(&spi->dev,
+				"I/O Error: rx-%d tx-%d res:rx-%c tx-%c len-%d\n",
+				xfer->rx_buf ? 1 : 0, xfer->tx_buf ? 1 : 0,
+				(sdd->state & RXBUSY) ? 'f' : 'p',
+				(sdd->state & TXBUSY) ? 'f' : 'p',
+				xfer->len);
+
+			if (use_dma) {
+				if (xfer->tx_buf && (sdd->state & TXBUSY))
+					dmaengine_terminate_all(sdd->tx_dma.ch);
+				if (xfer->rx_buf && (sdd->state & RXBUSY))
+					dmaengine_terminate_all(sdd->rx_dma.ch);
+			}
+		} else {
+			flush_fifo(sdd);
 		}
-	} else {
-		flush_fifo(sdd);
+
+		if (target_len > 0) {
+			target_len -= xfer->len;
+
+			if (xfer->tx_buf)
+				xfer->tx_buf += xfer->len;
+
+			if (xfer->rx_buf)
+				xfer->rx_buf += xfer->len;
+
+			if (target_len > fifo_len)
+				xfer->len = fifo_len;
+			else
+				xfer->len = target_len;
+		}
+	} while (target_len > 0);
+
+	if (origin_len) {
+		/* Restore original xfer buffers and length */
+		xfer->tx_buf = tx_buf;
+		xfer->rx_buf = rx_buf;
+		xfer->len = origin_len;
 	}
 
 	return status;
-- 
2.14.2

Re: [PATCH 6/6] spi: spi-s3c64xx: Allow higher transfer lengths in polling IO mode

[andi]

Hi Sylwester,

On 17.04.2018 00:40, Sylwester Nawrocki wrote:
> Some variants of the SPI controller have no DMA support, in such case
> SPI transfers longer than the FIFO length are not currently properly
> handled by the driver. Fix it by doing multiple transfers in the
> s3c64xx_spi_transfer_one() function if the SPI transfer length exceeds
> the FIFO size.

this patch is a mix of cosmetics and what you are actually describing
in the commit log...

> @@ -634,11 +634,15 @@ static int s3c64xx_spi_transfer_one(struct
> spi_master *master,
>  				    struct spi_transfer *xfer)
>  {
>  	struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master);
> +	const unsigned int fifo_len = (FIFO_LVL_MASK(sdd) >> 1) + 1;
> +	const void *tx_buf = NULL;
> +	void *rx_buf = NULL;
> +	int target_len = 0, origin_len = 0;
> +	bool use_dma = false;
>  	int status;
>  	u32 speed;
>  	u8 bpw;
>  	unsigned long flags;
> -	int use_dma;

... for example 'use_dma' bool instead of int or the 'fifo_len'. I agree
with these changes and, even though they are trivial, I would prefer
having them in separate patches.

Thanks,
Andi

[PATCH 6/6] spi: spi-s3c64xx: Allow higher transfer lengths in polling IO mode

[andi at etezian.org]

Hi Sylwester,

On 17.04.2018 00:40, Sylwester Nawrocki wrote:
> Some variants of the SPI controller have no DMA support, in such case
> SPI transfers longer than the FIFO length are not currently properly
> handled by the driver. Fix it by doing multiple transfers in the
> s3c64xx_spi_transfer_one() function if the SPI transfer length exceeds
> the FIFO size.

this patch is a mix of cosmetics and what you are actually describing
in the commit log...

> @@ -634,11 +634,15 @@ static int s3c64xx_spi_transfer_one(struct
> spi_master *master,
>  				    struct spi_transfer *xfer)
>  {
>  	struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master);
> +	const unsigned int fifo_len = (FIFO_LVL_MASK(sdd) >> 1) + 1;
> +	const void *tx_buf = NULL;
> +	void *rx_buf = NULL;
> +	int target_len = 0, origin_len = 0;
> +	bool use_dma = false;
>  	int status;
>  	u32 speed;
>  	u8 bpw;
>  	unsigned long flags;
> -	int use_dma;

... for example 'use_dma' bool instead of int or the 'fifo_len'. I agree
with these changes and, even though they are trivial, I would prefer
having them in separate patches.

Thanks,
Andi

Re: [PATCH 6/6] spi: spi-s3c64xx: Allow higher transfer lengths in polling IO mode

[Sylwester Nawrocki]

Hi Andi,

On 04/16/2018 09:19 PM, andi@etezian.org wrote:
> On 17.04.2018 00:40, Sylwester Nawrocki wrote:
>> Some variants of the SPI controller have no DMA support, in such case
>> SPI transfers longer than the FIFO length are not currently properly
>> handled by the driver. Fix it by doing multiple transfers in the
>> s3c64xx_spi_transfer_one() function if the SPI transfer length exceeds
>> the FIFO size.
> 
> this patch is a mix of cosmetics and what you are actually describing
> in the commit log...
> 
>> @@ -634,11 +634,15 @@ static int s3c64xx_spi_transfer_one(struct
>> spi_master *master,
>>  				    struct spi_transfer *xfer)
>>  {
>>  	struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master);
>> +	const unsigned int fifo_len = (FIFO_LVL_MASK(sdd) >> 1) + 1;
>> +	const void *tx_buf = NULL;
>> +	void *rx_buf = NULL;
>> +	int target_len = 0, origin_len = 0;
>> +	bool use_dma = false;
>>  	int status;
>>  	u32 speed;
>>  	u8 bpw;
>>  	unsigned long flags;
>> -	int use_dma;
> 
> ... for example 'use_dma' bool instead of int or the 'fifo_len'. I agree
> with these changes and, even though they are trivial, I would prefer
> having them in separate patches.

Thanks for your review! I will split that into 3 patches then, however
it seems a bit dubious to have the fifo_len change on its own.

-- 
Regards,
Sylwester

[PATCH 6/6] spi: spi-s3c64xx: Allow higher transfer lengths in polling IO mode

[Sylwester Nawrocki]

Hi Andi,

On 04/16/2018 09:19 PM, andi at etezian.org wrote:
> On 17.04.2018 00:40, Sylwester Nawrocki wrote:
>> Some variants of the SPI controller have no DMA support, in such case
>> SPI transfers longer than the FIFO length are not currently properly
>> handled by the driver. Fix it by doing multiple transfers in the
>> s3c64xx_spi_transfer_one() function if the SPI transfer length exceeds
>> the FIFO size.
> 
> this patch is a mix of cosmetics and what you are actually describing
> in the commit log...
> 
>> @@ -634,11 +634,15 @@ static int s3c64xx_spi_transfer_one(struct
>> spi_master *master,
>>  				    struct spi_transfer *xfer)
>>  {
>>  	struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master);
>> +	const unsigned int fifo_len = (FIFO_LVL_MASK(sdd) >> 1) + 1;
>> +	const void *tx_buf = NULL;
>> +	void *rx_buf = NULL;
>> +	int target_len = 0, origin_len = 0;
>> +	bool use_dma = false;
>>  	int status;
>>  	u32 speed;
>>  	u8 bpw;
>>  	unsigned long flags;
>> -	int use_dma;
> 
> ... for example 'use_dma' bool instead of int or the 'fifo_len'. I agree
> with these changes and, even though they are trivial, I would prefer
> having them in separate patches.

Thanks for your review! I will split that into 3 patches then, however
it seems a bit dubious to have the fifo_len change on its own.

-- 
Regards,
Sylwester

Re: [PATCH 1/6] spi: spi-s3c64xx: Remove unused driver data structure tgl_spi field

[andi]

Hi Sylwester,

On 17.04.2018 00:40, Sylwester Nawrocki wrote:
> The tgl_spi pointer is now unused so remove it.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

Reviewed-by: Andi Shyti <andi@etezian.org>

Thanks,
Andi

[PATCH 1/6] spi: spi-s3c64xx: Remove unused driver data structure tgl_spi field

[andi at etezian.org]

Hi Sylwester,

On 17.04.2018 00:40, Sylwester Nawrocki wrote:
> The tgl_spi pointer is now unused so remove it.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

Reviewed-by: Andi Shyti <andi@etezian.org>

Thanks,
Andi

Re: [PATCH 1/6] spi: spi-s3c64xx: Remove unused driver data structure tgl_spi field

[Krzysztof Kozlowski]

On Mon, Apr 16, 2018 at 5:40 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> The tgl_spi pointer is now unused so remove it.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

[PATCH 1/6] spi: spi-s3c64xx: Remove unused driver data structure tgl_spi field

[Krzysztof Kozlowski]

On Mon, Apr 16, 2018 at 5:40 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> The tgl_spi pointer is now unused so remove it.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof