[PATCH v2 02/21] spi: dw: Add DWC SSI capability

2020-09-30 Thread Serge Semin
Currently DWC SSI core is supported by means of setting up the
core-specific update_cr0() callback. It isn't suitable for multiple
reasons. First of all having exported several methods doing the same thing
but for different chips makes the code harder to maintain. Secondly the
spi-dw-core driver exports the methods, then the spi-dw-mmio driver sets
the private data callback with one of them so to be called by the core
driver again. That makes the code logic too complicated. Thirdly using
callbacks for just updating the CR0 register is problematic, since in case
if the register needed to be updated from different parts of the code,
we'd have to create another callback (for instance the SPI device-specific
parameters don't need to be calculated each time the SPI transfer is
submitted, so it's better to pre-calculate the CR0 data at the SPI-device
setup stage).

So keeping all the above in mind let's discard the update_cr0() callbacks,
define a generic and static dw_spi_update_cr0() method and create the
DW_SPI_CAP_DWC_SSI capability, which when enabled would activate the
alternative CR0 register layout.

While at it add the comments to the code path of the normal DW APB SSI
controller setup to make the dw_spi_update_cr0() method looking coherent.

Signed-off-by: Serge Semin 
---
 drivers/spi/spi-dw-core.c | 80 ++-
 drivers/spi/spi-dw-mmio.c | 20 +-
 drivers/spi/spi-dw.h  |  9 +
 3 files changed, 40 insertions(+), 69 deletions(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 3a7fdca8d335..be16fdaf7ce0 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -228,60 +228,56 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
return dws->transfer_handler(dws);
 }
 
-/* Configure CTRLR0 for DW_apb_ssi */
-u32 dw_spi_update_cr0(struct spi_controller *master, struct spi_device *spi,
- struct spi_transfer *transfer)
+static void dw_spi_update_cr0(struct dw_spi *dws, struct spi_device *spi,
+ struct spi_transfer *transfer)
 {
struct chip_data *chip = spi_get_ctldata(spi);
u32 cr0;
 
-   /* Default SPI mode is SCPOL = 0, SCPH = 0 */
-   cr0 = (transfer->bits_per_word - 1)
-   | (SSI_MOTO_SPI << SPI_FRF_OFFSET)
-   | spi->mode & SPI_CPOL) ? 1 : 0) << SPI_SCOL_OFFSET) |
-  (((spi->mode & SPI_CPHA) ? 1 : 0) << SPI_SCPH_OFFSET) |
-  (((spi->mode & SPI_LOOP) ? 1 : 0) << SPI_SRL_OFFSET))
-   | (chip->tmode << SPI_TMOD_OFFSET);
+   /* CTRLR0[ 4/3: 0] Data Frame Size */
+   cr0 = (transfer->bits_per_word - 1);
 
-   return cr0;
-}
-EXPORT_SYMBOL_GPL(dw_spi_update_cr0);
+   if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) {
+   /* CTRLR0[ 5: 4] Frame Format */
+   cr0 |= SSI_MOTO_SPI << SPI_FRF_OFFSET;
 
-/* Configure CTRLR0 for DWC_ssi */
-u32 dw_spi_update_cr0_v1_01a(struct spi_controller *master,
-struct spi_device *spi,
-struct spi_transfer *transfer)
-{
-   struct dw_spi *dws = spi_controller_get_devdata(master);
-   struct chip_data *chip = spi_get_ctldata(spi);
-   u32 cr0;
+   /*
+* SPI mode (SCPOL|SCPH)
+* CTRLR0[ 6] Serial Clock Phase
+* CTRLR0[ 7] Serial Clock Polarity
+*/
+   cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << SPI_SCOL_OFFSET;
+   cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << SPI_SCPH_OFFSET;
 
-   /* CTRLR0[ 4: 0] Data Frame Size */
-   cr0 = (transfer->bits_per_word - 1);
+   /* CTRLR0[11] Shift Register Loop */
+   cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << SPI_SRL_OFFSET;
 
-   /* CTRLR0[ 7: 6] Frame Format */
-   cr0 |= SSI_MOTO_SPI << DWC_SSI_CTRLR0_FRF_OFFSET;
+   /* CTRLR0[ 9:8] Transfer Mode */
+   cr0 |= chip->tmode << SPI_TMOD_OFFSET;
+   } else {
+   /* CTRLR0[ 7: 6] Frame Format */
+   cr0 |= SSI_MOTO_SPI << DWC_SSI_CTRLR0_FRF_OFFSET;
 
-   /*
-* SPI mode (SCPOL|SCPH)
-* CTRLR0[ 8] Serial Clock Phase
-* CTRLR0[ 9] Serial Clock Polarity
-*/
-   cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << DWC_SSI_CTRLR0_SCPOL_OFFSET;
-   cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << DWC_SSI_CTRLR0_SCPH_OFFSET;
+   /*
+* SPI mode (SCPOL|SCPH)
+* CTRLR0[ 8] Serial Clock Phase
+* CTRLR0[ 9] Serial Clock Polarity
+*/
+   cr0 |= ((spi->mode & SPI_CPOL) ? 1 : 0) << 
DWC_SSI_CTRLR0_SCPOL_OFFSET;
+   cr0 |= ((spi->mode & SPI_CPHA) ? 1 : 0) << 
DWC_SSI_CTRLR0_SCPH_OFFSET;
 
-   /* CTRLR0[11:10] Transfer Mode */
-   cr0 |= chip->tmode << DWC_SSI_CTRLR0_TMOD_OFFSET;
+   /* CTRLR0[13] Shift Register Loop */
+   cr0 |= ((spi->

Re: [PATCH v2 02/21] spi: dw: Add DWC SSI capability

2020-10-01 Thread Mark Brown
On Wed, Sep 30, 2020 at 09:55:26PM +0300, Serge Semin wrote:
> Currently DWC SSI core is supported by means of setting up the
> core-specific update_cr0() callback. It isn't suitable for multiple
> reasons. First of all having exported several methods doing the same thing
> but for different chips makes the code harder to maintain. Secondly the
> spi-dw-core driver exports the methods, then the spi-dw-mmio driver sets

This doesn't build with current code in an x86 defconfig, please check
and resend (looks like you forgot to update dw-pci):

mnt/kernel/drivers/spi/spi-dw-pci.c: In function 'spi_mid_init':
/mnt/kernel/drivers/spi/spi-dw-pci.c:52:5: error: 'struct dw_spi' has no member 
named 'update_cr0'
  dws->update_cr0 = dw_spi_update_cr0;
 ^~
/mnt/kernel/drivers/spi/spi-dw-pci.c:52:20: error: 'dw_spi_update_cr0' 
undeclared (first use in this function); did you mean 'dw_spi_set_cs'?
  dws->update_cr0 = dw_spi_update_cr0;
^
dw_spi_set_cs
/mnt/kernel/drivers/spi/spi-dw-pci.c:52:20: note: each undeclared identifier is 
reported only once for each function it appears in
/mnt/kernel/drivers/spi/spi-dw-pci.c: In function 'spi_generic_init':
/mnt/kernel/drivers/spi/spi-dw-pci.c:62:5: error: 'struct dw_spi' has no member 
named 'update_cr0'
  dws->update_cr0 = dw_spi_update_cr0;
 ^~
/mnt/kernel/drivers/spi/spi-dw-pci.c:62:20: error: 'dw_spi_update_cr0' 
undeclared (first use in this function); did you mean 'dw_spi_set_cs'?
  dws->update_cr0 = dw_spi_update_cr0;
^
dw_spi_set_cs
make[3]: *** [/mnt/kernel/scripts/Makefile.build:283: drivers/spi/spi-dw-pci.o] 
Error 1
make[2]: *** [/mnt/kernel/scripts/Makefile.build:500: drivers/spi] Error 2
make[2]: *** Waiting for unfinished jobs
make[1]: *** [/mnt/kernel/Makefile:1788: drivers] Error 2
make[1]: *** Waiting for unfinished jobs
make: *** [Makefile:185: __sub-make] Error 2


signature.asc
Description: PGP signature


Re: [PATCH v2 02/21] spi: dw: Add DWC SSI capability

2020-10-01 Thread Serge Semin
On Thu, Oct 01, 2020 at 10:51:05PM +0100, Mark Brown wrote:
> On Wed, Sep 30, 2020 at 09:55:26PM +0300, Serge Semin wrote:
> > Currently DWC SSI core is supported by means of setting up the
> > core-specific update_cr0() callback. It isn't suitable for multiple
> > reasons. First of all having exported several methods doing the same thing
> > but for different chips makes the code harder to maintain. Secondly the
> > spi-dw-core driver exports the methods, then the spi-dw-mmio driver sets
> 

> This doesn't build with current code in an x86 defconfig, please check
> and resend (looks like you forgot to update dw-pci):
> 
> mnt/kernel/drivers/spi/spi-dw-pci.c: In function 'spi_mid_init':
> /mnt/kernel/drivers/spi/spi-dw-pci.c:52:5: error: 'struct dw_spi' has no 
> member named 'update_cr0'
>   dws->update_cr0 = dw_spi_update_cr0;
>  ^~
> /mnt/kernel/drivers/spi/spi-dw-pci.c:52:20: error: 'dw_spi_update_cr0' 
> undeclared (first use in this function); did you mean 'dw_spi_set_cs'?
>   dws->update_cr0 = dw_spi_update_cr0;
> ^
> dw_spi_set_cs
> /mnt/kernel/drivers/spi/spi-dw-pci.c:52:20: note: each undeclared identifier 
> is reported only once for each function it appears in
> /mnt/kernel/drivers/spi/spi-dw-pci.c: In function 'spi_generic_init':
> /mnt/kernel/drivers/spi/spi-dw-pci.c:62:5: error: 'struct dw_spi' has no 
> member named 'update_cr0'
>   dws->update_cr0 = dw_spi_update_cr0;
>  ^~
> /mnt/kernel/drivers/spi/spi-dw-pci.c:62:20: error: 'dw_spi_update_cr0' 
> undeclared (first use in this function); did you mean 'dw_spi_set_cs'?
>   dws->update_cr0 = dw_spi_update_cr0;
> ^
> dw_spi_set_cs
> make[3]: *** [/mnt/kernel/scripts/Makefile.build:283: 
> drivers/spi/spi-dw-pci.o] Error 1
> make[2]: *** [/mnt/kernel/scripts/Makefile.build:500: drivers/spi] Error 2
> make[2]: *** Waiting for unfinished jobs
> make[1]: *** [/mnt/kernel/Makefile:1788: drivers] Error 2
> make[1]: *** Waiting for unfinished jobs
> make: *** [Makefile:185: __sub-make] Error 2

Oh, thanks for noticing this. I'll fix it straight away and resend. Sorry for
the inconvenience.

-Sergey