[PATCH] spi: dw: support 4-16 bits per word

[PATCH] spi: dw: support 4-16 bits per word

[Simon Goldschmidt]

The spi-dw driver currently only supports 8 or 16 bits per word.

Since the hardware supports 4-16 bits per word, adapt the driver
to also support this.

Tested on socfpga cyclone5 with a 9-bit SPI display.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---
 drivers/spi/spi-dw.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index f693bfe95ab9..2ecbb6b19cea 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -307,14 +307,14 @@ static int dw_spi_transfer_one(struct spi_controller *master,
 		dws->current_freq = transfer->speed_hz;
 		spi_set_clk(dws, chip->clk_div);
 	}
-	if (transfer->bits_per_word == 8) {
+	if ((transfer->bits_per_word < 4) || (transfer->bits_per_word > 16))
+		return -EINVAL;
+	if (transfer->bits_per_word <= 8) {
 		dws->n_bytes = 1;
 		dws->dma_width = 1;
-	} else if (transfer->bits_per_word == 16) {
+	} else {
 		dws->n_bytes = 2;
 		dws->dma_width = 2;
-	} else {
-		return -EINVAL;
 	}
 	/* Default SPI mode is SCPOL = 0, SCPH = 0 */
 	cr0 = (transfer->bits_per_word - 1)
@@ -493,7 +493,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 	}
 
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP;
-	master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(16);
+	master->bits_per_word_mask =  SPI_BPW_RANGE_MASK(4, 16);
 	master->bus_num = dws->bus_num;
 	master->num_chipselect = dws->num_cs;
 	master->setup = dw_spi_setup;
-- 
2.17.1

Re: [PATCH] spi: dw: support 4-16 bits per word

[Andy Shevchenko]

On Wed, Aug 8, 2018 at 10:14 AM, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
> The spi-dw driver currently only supports 8 or 16 bits per word.
>
> Since the hardware supports 4-16 bits per word, adapt the driver
> to also support this.
>
> Tested on socfpga cyclone5 with a 9-bit SPI display.

> +       if ((transfer->bits_per_word < 4) || (transfer->bits_per_word > 16))
> +               return -EINVAL;

> +       if (transfer->bits_per_word <= 8) {
>                 dws->n_bytes = 1;
>                 dws->dma_width = 1;
> -       } else if (transfer->bits_per_word == 16) {
> +       } else {
>                 dws->n_bytes = 2;
>                 dws->dma_width = 2;
>         }

Now these can be just

n_bytes = round_up(8);
dma_width = round_up(8);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] spi: dw: support 4-16 bits per word

[Simon Goldschmidt]

On Wed, Aug 8, 2018 at 11:45 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, Aug 8, 2018 at 10:14 AM, Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
> > The spi-dw driver currently only supports 8 or 16 bits per word.
> >
> > Since the hardware supports 4-16 bits per word, adapt the driver
> > to also support this.
> >
> > Tested on socfpga cyclone5 with a 9-bit SPI display.
>
> > +       if ((transfer->bits_per_word < 4) || (transfer->bits_per_word > 16))
> > +               return -EINVAL;
>
> > +       if (transfer->bits_per_word <= 8) {
> >                 dws->n_bytes = 1;
> >                 dws->dma_width = 1;
> > -       } else if (transfer->bits_per_word == 16) {
> > +       } else {
> >                 dws->n_bytes = 2;
> >                 dws->dma_width = 2;
> >         }
>
> Now these can be just
>
> n_bytes = round_up(8);
> dma_width = round_up(8);

I guess you mean:

n_bytes = round_up(transfer->bits_per_word, 8);

But that would yield 8 or 16 where we need 1 or 2.

Reading spi-imx.c, this might work:

n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);


Simon

Re: [PATCH] spi: dw: support 4-16 bits per word

[Andy Shevchenko]

On Wed, Aug 8, 2018 at 2:58 PM, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
> On Wed, Aug 8, 2018 at 11:45 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Wed, Aug 8, 2018 at 10:14 AM, Simon Goldschmidt
>> <simon.k.r.goldschmidt@gmail.com> wrote:

>> n_bytes = round_up(8);
>> dma_width = round_up(8);
>
> I guess you mean:
>
> n_bytes = round_up(transfer->bits_per_word, 8);
>
> But that would yield 8 or 16 where we need 1 or 2.

Indeed.

> Reading spi-imx.c, this might work:
>
> n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);

Yes.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2] spi: dw: support 4-16 bits per word

[Simon Goldschmidt]

The spi-dw driver currently only supports 8 or 16 bits per word.

Since the hardware supports 4-16 bits per word, adapt the driver
to also support this.

Tested on socfpga cyclone5 with a 9-bit SPI display.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

Changes in v2:
- use DIV_ROUND_UP to calculate number of bytes per word instead of
  if/else range checks

 drivers/spi/spi-dw.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index f693bfe95ab9..58a7caf31d59 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -307,15 +307,13 @@ static int dw_spi_transfer_one(struct spi_controller *master,
 		dws->current_freq = transfer->speed_hz;
 		spi_set_clk(dws, chip->clk_div);
 	}
-	if (transfer->bits_per_word == 8) {
-		dws->n_bytes = 1;
-		dws->dma_width = 1;
-	} else if (transfer->bits_per_word == 16) {
-		dws->n_bytes = 2;
-		dws->dma_width = 2;
-	} else {
+
+	if ((transfer->bits_per_word < 4) || (transfer->bits_per_word > 16))
 		return -EINVAL;
-	}
+
+	dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
+	dws->dma_width = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
+
 	/* Default SPI mode is SCPOL = 0, SCPH = 0 */
 	cr0 = (transfer->bits_per_word - 1)
 		| (chip->type << SPI_FRF_OFFSET)
@@ -493,7 +491,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 	}
 
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP;
-	master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(16);
+	master->bits_per_word_mask =  SPI_BPW_RANGE_MASK(4, 16);
 	master->bus_num = dws->bus_num;
 	master->num_chipselect = dws->num_cs;
 	master->setup = dw_spi_setup;
-- 
2.17.1

Re: [PATCH v2] spi: dw: support 4-16 bits per word

[Andy Shevchenko]

On Fri, Aug 17, 2018 at 10:01 AM, Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
> The spi-dw driver currently only supports 8 or 16 bits per word.
>
> Since the hardware supports 4-16 bits per word, adapt the driver
> to also support this.
>
> Tested on socfpga cyclone5 with a 9-bit SPI display.
>

LGTM,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
>
> Changes in v2:
> - use DIV_ROUND_UP to calculate number of bytes per word instead of
>   if/else range checks
>
>  drivers/spi/spi-dw.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
> index f693bfe95ab9..58a7caf31d59 100644
> --- a/drivers/spi/spi-dw.c
> +++ b/drivers/spi/spi-dw.c
> @@ -307,15 +307,13 @@ static int dw_spi_transfer_one(struct spi_controller *master,
>                 dws->current_freq = transfer->speed_hz;
>                 spi_set_clk(dws, chip->clk_div);
>         }
> -       if (transfer->bits_per_word == 8) {
> -               dws->n_bytes = 1;
> -               dws->dma_width = 1;
> -       } else if (transfer->bits_per_word == 16) {
> -               dws->n_bytes = 2;
> -               dws->dma_width = 2;
> -       } else {
> +
> +       if ((transfer->bits_per_word < 4) || (transfer->bits_per_word > 16))
>                 return -EINVAL;
> -       }
> +
> +       dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
> +       dws->dma_width = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
> +
>         /* Default SPI mode is SCPOL = 0, SCPH = 0 */
>         cr0 = (transfer->bits_per_word - 1)
>                 | (chip->type << SPI_FRF_OFFSET)
> @@ -493,7 +491,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
>         }
>
>         master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP;
> -       master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(16);
> +       master->bits_per_word_mask =  SPI_BPW_RANGE_MASK(4, 16);
>         master->bus_num = dws->bus_num;
>         master->num_chipselect = dws->num_cs;
>         master->setup = dw_spi_setup;
> --
> 2.17.1
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] spi: dw: support 4-16 bits per word

[Trent Piepho]

On Fri, 2018-08-17 at 09:01 +0200, Simon Goldschmidt wrote:
> The spi-dw driver currently only supports 8 or 16 bits per word.
> 
> Since the hardware supports 4-16 bits per word, adapt the driver
> to also support this.
> 
> 

> @@ -307,15 +307,13 @@ static int dw_spi_transfer_one(struct spi_controller *master,
> +
> +	if ((transfer->bits_per_word < 4) || (transfer->bits_per_word > 16))
>  		return -EINVAL;
> -	}

You don't need this check as the spi core validates the transfer
against master->bits_per_word_mask.

>  	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP;
> -	master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(16);
> +	master->bits_per_word_mask =  SPI_BPW_RANGE_MASK(4, 16);
>  	master->bus_num = dws->bus_num;
> 

Re: [PATCH v2] spi: dw: support 4-16 bits per word

[Simon Goldschmidt]

On Fri, Aug 17, 2018 at 6:32 PM Trent Piepho <tpiepho@impinj.com> wrote:
>
> On Fri, 2018-08-17 at 09:01 +0200, Simon Goldschmidt wrote:
> > The spi-dw driver currently only supports 8 or 16 bits per word.
> >
> > Since the hardware supports 4-16 bits per word, adapt the driver
> > to also support this.
> >
> >
>
> > @@ -307,15 +307,13 @@ static int dw_spi_transfer_one(struct spi_controller *master,
> > +
> > +     if ((transfer->bits_per_word < 4) || (transfer->bits_per_word > 16))
> >               return -EINVAL;
> > -     }
>
> You don't need this check as the spi core validates the transfer
> against master->bits_per_word_mask.

Ok.

>
> >       master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP;
> > -     master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(16);
> > +     master->bits_per_word_mask =  SPI_BPW_RANGE_MASK(4, 16);
> >       master->bus_num = dws->bus_num;
> >

[PATCH v3] spi: dw: support 4-16 bits per word

[Simon Goldschmidt]

The spi-dw driver currently only supports 8 or 16 bits per word.

Since the hardware supports 4-16 bits per word, adapt the driver
to also support this.

Tested on socfpga cyclone5 with a 9-bit SPI display.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
Changes in v3
- remove the check for valid 'bits_per_word' from dw_spi_transfer_one()
- add reviewed-by tag

Changes in v2:
- use DIV_ROUND_UP to calculate number of bytes per word instead of
  if/else range checks

 drivers/spi/spi-dw.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index f693bfe95ab9..58a7caf31d59 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -307,15 +307,10 @@ static int dw_spi_transfer_one(struct spi_controller *master,
 		dws->current_freq = transfer->speed_hz;
 		spi_set_clk(dws, chip->clk_div);
 	}
-	if (transfer->bits_per_word == 8) {
-		dws->n_bytes = 1;
-		dws->dma_width = 1;
-	} else if (transfer->bits_per_word == 16) {
-		dws->n_bytes = 2;
-		dws->dma_width = 2;
-	} else {
-		return -EINVAL;
-	}
+
+	dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
+	dws->dma_width = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
+
 	/* Default SPI mode is SCPOL = 0, SCPH = 0 */
 	cr0 = (transfer->bits_per_word - 1)
 		| (chip->type << SPI_FRF_OFFSET)
@@ -493,7 +491,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 	}
 
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP;
-	master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(16);
+	master->bits_per_word_mask =  SPI_BPW_RANGE_MASK(4, 16);
 	master->bus_num = dws->bus_num;
 	master->num_chipselect = dws->num_cs;
 	master->setup = dw_spi_setup;
-- 
2.17.1

Re: [PATCH v3] spi: dw: support 4-16 bits per word

[Mark Brown]

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

On Tue, Sep 04, 2018 at 09:49:44PM +0200, Simon Goldschmidt wrote:
> The spi-dw driver currently only supports 8 or 16 bits per word.
> 
> Since the hardware supports 4-16 bits per word, adapt the driver
> to also support this.

Please don't send new patches in reply to old patch serieses, it makes
it harder to follow what the current version of things is and makes it
much easier for the patches to get lost in the old threads.

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

Re: [PATCH v3] spi: dw: support 4-16 bits per word

[Simon Goldschmidt]

On Thu, Sep 6, 2018 at 1:09 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, Sep 04, 2018 at 09:49:44PM +0200, Simon Goldschmidt wrote:
> > The spi-dw driver currently only supports 8 or 16 bits per word.
> >
> > Since the hardware supports 4-16 bits per word, adapt the driver
> > to also support this.
>
> Please don't send new patches in reply to old patch serieses, it makes
> it harder to follow what the current version of things is and makes it
> much easier for the patches to get lost in the old threads.

Ok, no problem and thanks for the hint! Where does this requirement
come from? Patchwork or mail sorting habits?

Anyway, how does this continue, will you pick the patch or do I need
to somehow collect yet more reviews?

Regards,
Simon

Re: [PATCH v3] spi: dw: support 4-16 bits per word

[Mark Brown]

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

On Thu, Sep 06, 2018 at 01:23:34PM +0200, Simon Goldschmidt wrote:
> On Thu, Sep 6, 2018 at 1:09 PM Mark Brown <broonie@kernel.org> wrote:

> > Please don't send new patches in reply to old patch serieses, it makes
> > it harder to follow what the current version of things is and makes it
> > much easier for the patches to get lost in the old threads.

> Ok, no problem and thanks for the hint! Where does this requirement
> come from? Patchwork or mail sorting habits?

Mail sorting.  It can mean that you get things like someone deleting a
thread and the new patch getting caught up in a thread delete command
and hence missed.

> Anyway, how does this continue, will you pick the patch or do I need
> to somehow collect yet more reviews?

You should've got a mail at about the same time saying it's been
applied.

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

Re: [PATCH v3] spi: dw: support 4-16 bits per word

[Simon Goldschmidt]

On Thu, Sep 6, 2018 at 3:23 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Sep 06, 2018 at 01:23:34PM +0200, Simon Goldschmidt wrote:
> > On Thu, Sep 6, 2018 at 1:09 PM Mark Brown <broonie@kernel.org> wrote:
>
> > > Please don't send new patches in reply to old patch serieses, it makes
> > > it harder to follow what the current version of things is and makes it
> > > much easier for the patches to get lost in the old threads.
>
> > Ok, no problem and thanks for the hint! Where does this requirement
> > come from? Patchwork or mail sorting habits?
>
> Mail sorting.  It can mean that you get things like someone deleting a
> thread and the new patch getting caught up in a thread delete command
> and hence missed.
>
> > Anyway, how does this continue, will you pick the patch or do I need
> > to somehow collect yet more reviews?
>
> You should've got a mail at about the same time saying it's been
> applied.

Right, got it. Thanks again.

Applied "spi: dw: support 4-16 bits per word" to the spi tree

[Mark Brown]

The patch

   spi: dw: support 4-16 bits per word

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From af060b3f72b801962033f75a2fda25fff992796d Mon Sep 17 00:00:00 2001
From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
Date: Tue, 4 Sep 2018 21:49:44 +0200
Subject: [PATCH] spi: dw: support 4-16 bits per word

The spi-dw driver currently only supports 8 or 16 bits per word.

Since the hardware supports 4-16 bits per word, adapt the driver
to also support this.

Tested on socfpga cyclone5 with a 9-bit SPI display.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-dw.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index 1736612ee86b..3e205ab60cd4 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -308,15 +308,10 @@ static int dw_spi_transfer_one(struct spi_controller *master,
 		dws->current_freq = transfer->speed_hz;
 		spi_set_clk(dws, chip->clk_div);
 	}
-	if (transfer->bits_per_word == 8) {
-		dws->n_bytes = 1;
-		dws->dma_width = 1;
-	} else if (transfer->bits_per_word == 16) {
-		dws->n_bytes = 2;
-		dws->dma_width = 2;
-	} else {
-		return -EINVAL;
-	}
+
+	dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
+	dws->dma_width = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
+
 	/* Default SPI mode is SCPOL = 0, SCPH = 0 */
 	cr0 = (transfer->bits_per_word - 1)
 		| (chip->type << SPI_FRF_OFFSET)
@@ -496,7 +491,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 	}
 
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP;
-	master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(16);
+	master->bits_per_word_mask =  SPI_BPW_RANGE_MASK(4, 16);
 	master->bus_num = dws->bus_num;
 	master->num_chipselect = dws->num_cs;
 	master->setup = dw_spi_setup;
-- 
2.19.0.rc1