sunxi spi fixes
Subsystems (5)
v3 cover sunxi spi fixes
Hello,
This is update of the sunxi spi patches that should give full-featured SPI driver.
First three patches fix issues with the current driver and can be of use for stable kernels so adding cc for those.
I merged the sun4i and sun6i driver because there several issues that need to be fixed in both separately and they are even out of sync wrt some fixes. I guess some of the merge patches can be squashed.
I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot tell if that side was broken by this patchset - especially the last patch that adds DMA was afaik never tested on sun6i.
Emilio L?pez (1):
spi: sun4i: add DMA support
Michal Suchanek (12):
spi: sunxi: set maximum and minimum speed of SPI master
spi: sunxi: fix transfer timeout
spi: sun4i: fix FIFO limit
spi: sunxi: expose maximum transfer size limit
spi: sun6i: update CS handling from spi-sun4i
spi: sunxi: rename sun4i,sun6i -> sunxi
spi: sunxi: rename constants to match between sun4i and sun6i
spi: sunxi: synchronize whitespace, comments, struct
spi: sunxi: use register map
spi: sunxi: merge sun4i and sun6i SPI driver
dt: spi: sun4i: merge sun4i and sun6i binding doc
spi: sunxi: remove CONFIG_SPI_SUN6I
.../devicetree/bindings/spi/spi-sun4i.txt | 21 +- .../devicetree/bindings/spi/spi-sun6i.txt | 24 - arch/arm/configs/multi_v7_defconfig | 1 - arch/arm/configs/sunxi_defconfig | 1 - drivers/spi/Kconfig | 10 +- drivers/spi/Makefile | 1 - drivers/spi/spi-sun4i.c | 728 +++++++++++++++++---- drivers/spi/spi-sun6i.c | 482 -------------- 8 files changed, 600 insertions(+), 668 deletions(-) delete mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt delete mode 100644 drivers/spi/spi-sun6i.c
Signature · 1 line
2.8.1
PATCH v3 1/13 spi: sunxi: set maximum and minimum speed of SPI master
The speed limits are unset in the sun4i and sun6i SPI drivers.
The maximum speed of SPI master is used when maximum speed of SPI slave is not specified. Also the __spi_validate function should check that transfer speeds do not exceed the master limits.
The user manual for A10 and A31 specifies maximum speed of the SPI clock as 100MHz and minimum as 3kHz.
Setting the SPI clock to out-of-spec values can lock up the SoC.
--
v2:
new patch
v3:
fix constant style
--- drivers/spi/spi-sun4i.c | 2 ++ drivers/spi/spi-sun6i.c | 2 ++ 2 files changed, 4 insertions(+)
drivers/spi/spi-sun4i.c +2 -0
@@ -387,6 +387,8 @@ static int sun4i_spi_probe(struct platform_device *pdev) } sspi->master = master; + master->max_speed_hz =100 *1000 *1000 ; + master->min_speed_hz =3 *1000 ; master->set_cs = sun4i_spi_set_cs; master->transfer_one = sun4i_spi_transfer_one; master->num_chipselect =4 ;
drivers/spi/spi-sun6i.c +2 -0
@@ -386,6 +386,8 @@ static int sun6i_spi_probe(struct platform_device *pdev) } sspi->master = master; + master->max_speed_hz =100 *1000 *1000 ; + master->min_speed_hz =3 *1000 ; master->set_cs = sun6i_spi_set_cs; master->transfer_one = sun6i_spi_transfer_one; master->num_chipselect =4 ;
Signature · 1 line
2.8.1
Re: [PATCH v3 01/13] spi: sunxi: set maximum and minimum speed of SPI master
Quoted 40 lines · Michal Suchanek
The speed limits are unset in the sun4i and sun6i SPI drivers.
The maximum speed of SPI master is used when maximum speed of SPI slave is not specified. Also the
__spi_validatefunction should check that transfer speeds do not exceed the master limits.The user manual for A10 and A31 specifies maximum speed of the SPI clock as 100MHz and minimum as 3kHz.
Setting the SPI clock to out-of-spec values can lock up the SoC.
Signed-off-by Michal Suchanek <[email protected]>--
v2:
new patch
v3:
fix constant style--- drivers/spi/spi-sun4i.c | 2 ++ drivers/spi/spi-sun6i.c | 2 ++ 2 files changed, 4 insertions(+)drivers/spi/spi-sun4i.c +2
-0@@ -387,6 +387,8 @@ static int sun4i_spi_probe(struct platform_device *pdev) } sspi->master = master; + master->max_speed_hz =100 *1000 *1000 ; + master->min_speed_hz =3 *1000 ; master->set_cs = sun4i_spi_set_cs; master->transfer_one = sun4i_spi_transfer_one; master->num_chipselect =4 ;drivers/spi/spi-sun6i.c +0
-0
drivers/spi/spi-sun6i.c static int sun6i_spi_probe(struct platform_device *pdev) · line 393
Quoted 8 lines
drivers/spi/spi-sun6i.c +2
-0@@ -386,6 +386,8 @@ static int sun6i_spi_probe(struct platform_device *pdev) } sspi->master = master; + master->max_speed_hz =100 *1000 *1000 ; + master->min_speed_hz =3 *1000 ; master->set_cs = sun6i_spi_set_cs; master->transfer_one = sun6i_spi_transfer_one; master->num_chipselect =4 ;
I really don't get why you want to do that kind of padding, when no one does in the rest of the driver, or the rest of the kernel.
Once properly changed,
Thanks,
Maxime
Signature · 10 lines
Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160613/95701f26/attachment.sig>
PATCH v3 2/13 spi: sunxi: fix transfer timeout
In reply to Michal SuchanekThe trasfer timeout is fixed at 1000 ms. Reading a 4Mbyte flash over 1MHz SPI bus takes way longer than that. Calculate the timeout from the actual time the transfer is supposed to take and multiply by 2 for good measure.
---
v2:
- fix build error
- use unsigned instead of int in max_t
- use tfr->speed_hz instead of sspi->max_speed_hz
- commit 47284e3e0f3c427c93f8583549b6c938e8a18015 changes the driver
to use tfr->speed_hz v3: - avoid max_t by using unsigned constant --- drivers/spi/spi-sun4i.c | 10 +++++++++- drivers/spi/spi-sun6i.c | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-)
drivers/spi/spi-sun4i.c +9 -1
@@ -173,6 +173,7 @@ static int sun4i_spi_transfer_one(struct spi_master *master, {struct sun4i_spi *sspi =spi_master_get_devdata (master);unsigned int mclk_rate, div, timeout; +unsigned int start, end, tx_time;unsigned int tx_len =0 ;int ret =0 ;u32 reg;
@@ -279,9 +280,16 @@ static int sun4i_spi_transfer_one(struct spi_master *master, reg =sun4i_spi_read (sspi,SUN4I_CTL_REG );sun4i_spi_write (sspi,SUN4I_CTL_REG , reg |SUN4I_CTL_XCH ); + tx_time =max (tfr->len *8 *2 / (tfr->speed_hz /1000 ),100U ); + start = jiffies; timeout =wait_for_completion_timeout (&sspi->done, -msecs_to_jiffies (1000 )); +msecs_to_jiffies (tx_time)); + end = jiffies;if (!timeout) { +dev_warn (&master->dev, +"%s: timeout transferring %u bytes@%iHz for %i(%i)ms" , +dev_name (&spi->dev), tfr->len, tfr->speed_hz, +jiffies_to_msecs (end - start), tx_time); ret = -ETIMEDOUT ; goto out; }
drivers/spi/spi-sun6i.c +9 -1
@@ -160,6 +160,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master, {struct sun6i_spi *sspi =spi_master_get_devdata (master);unsigned int mclk_rate, div, timeout; +unsigned int start, end, tx_time;unsigned int tx_len =0 ;int ret =0 ;u32 reg;
@@ -269,9 +270,16 @@ static int sun6i_spi_transfer_one(struct spi_master *master, reg =sun6i_spi_read (sspi,SUN6I_TFR_CTL_REG );sun6i_spi_write (sspi,SUN6I_TFR_CTL_REG , reg |SUN6I_TFR_CTL_XCH ); + tx_time =max (tfr->len *8 *2 / (tfr->speed_hz /1000 ),100U ); + start = jiffies; timeout =wait_for_completion_timeout (&sspi->done, -msecs_to_jiffies (1000 )); +msecs_to_jiffies (tx_time)); + end = jiffies;if (!timeout) { +dev_warn (&master->dev, +"%s: timeout transferring %u bytes@%iHz for %i(%i)ms" , +dev_name (&spi->dev), tfr->len, tfr->speed_hz, +jiffies_to_msecs (end - start), tx_time); ret = -ETIMEDOUT ; goto out; }
Signature · 1 line
2.8.1
Re: [PATCH v3 02/13] spi: sunxi: fix transfer timeout
Quoted 6 lines · Michal Suchanek
The trasfer timeout is fixed at 1000 ms. Reading a 4Mbyte flash over 1MHz SPI bus takes way longer than that. Calculate the timeout from the actual time the transfer is supposed to take and multiply by 2 for good measure.
Signed-off-by Michal Suchanek <[email protected]>
Maxime
Signature · 10 lines
Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160613/1f94e877/attachment-0001.sig>
PATCH v3 3/13 spi: sun4i: fix FIFO limit
In reply to Michal SuchanekWhen testing SPI without DMA I noticed that filling the FIFO on the spi controller causes timeout.
Always leave room for one byte in the FIFO.
---
v2:
use EMSGSIZE instead of EINVAL
v3:
fix comment style
--- drivers/spi/spi-sun4i.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
drivers/spi/spi-sun4i.c +10 -3
@@ -180,7 +180,10 @@ static int sun4i_spi_transfer_one(struct spi_master *master,/* We don't support transfer larger than the FIFO */ if (tfr->len >SUN4I_FIFO_DEPTH ) -return -EINVAL ; +return -EMSGSIZE ; + +if (tfr->tx_buf && tfr->len >=SUN4I_FIFO_DEPTH ) +return -EMSGSIZE ;reinit_completion (&sspi->done); sspi->tx_buf = tfr->tx_buf;
@@ -270,8 +273,12 @@ static int sun4i_spi_transfer_one(struct spi_master *master,sun4i_spi_write (sspi,SUN4I_BURST_CNT_REG ,SUN4I_BURST_CNT (tfr->len));sun4i_spi_write (sspi,SUN4I_XMIT_CNT_REG ,SUN4I_XMIT_CNT (tx_len)); -/* Fill the TX FIFO */ -sun4i_spi_fill_fifo (sspi,SUN4I_FIFO_DEPTH ); +/* +* Fill the TX FIFO +* Filling the FIFO fully causes timeout for some reason +* at least on spi2 on A10s +*/ +sun4i_spi_fill_fifo (sspi,SUN4I_FIFO_DEPTH -1 );/* Enable the interrupts */ sun4i_spi_write (sspi,SUN4I_INT_CTL_REG ,SUN4I_INT_CTL_TC );
Signature · 1 line
2.8.1
Re: [PATCH v3 03/13] spi: sun4i: fix FIFO limit
Quoted 6 lines · Michal Suchanek
When testing SPI without DMA I noticed that filling the FIFO on the spi controller causes timeout.
Always leave room for one byte in the FIFO.
Signed-off-by Michal Suchanek <[email protected]>
Thanks,
Maxime
Signature · 10 lines
Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160613/6b657bdc/attachment.sig>
PATCH v3 4/13 spi: sunxi: expose maximum transfer size limit
In reply to Michal SuchanekThe sun4i spi hardware can trasfer at most 63 bytes of data without DMA support so report the limitation. Same for sun6i.
--- drivers/spi/spi-sun4i.c | 6 ++++++ drivers/spi/spi-sun6i.c | 5 +++++ 2 files changed, 11 insertions(+)
drivers/spi/spi-sun4i.c +6 -0
@@ -167,6 +167,11 @@ static void sun4i_spi_set_cs(struct spi_device *spi, bool enable)sun4i_spi_write (sspi,SUN4I_CTL_REG , reg); } +static size_t sun4i_spi_max_transfer_size (struct spi_device *spi) +{ +return SUN4I_FIFO_DEPTH -1 ; +} +static int sun4i_spi_transfer_one (struct spi_master *master,struct spi_device *spi,struct spi_transfer *tfr)
@@ -411,6 +416,7 @@ static int sun4i_spi_probe(struct platform_device *pdev) master->bits_per_word_mask =SPI_BPW_MASK (8 ); master->dev.of_node = pdev->dev.of_node; master->auto_runtime_pm = true; + master->max_transfer_size = sun4i_spi_max_transfer_size; sspi->hclk =devm_clk_get (&pdev->dev,"ahb" );if (IS_ERR (sspi->hclk)) {
drivers/spi/spi-sun6i.c +5 -0
@@ -153,6 +153,10 @@ static void sun6i_spi_set_cs(struct spi_device *spi, bool enable)sun6i_spi_write (sspi,SUN6I_TFR_CTL_REG , reg); } +static size_t sun6i_spi_max_transfer_size (struct spi_device *spi) +{ +return SUN6I_FIFO_DEPTH -1 ; +} static int sun6i_spi_transfer_one(struct spi_master *master, struct spi_device *spi,
@@ -403,6 +407,7 @@ static int sun6i_spi_probe(struct platform_device *pdev) master->bits_per_word_mask =SPI_BPW_MASK (8 ); master->dev.of_node = pdev->dev.of_node; master->auto_runtime_pm = true; + master->max_transfer_size = sun6i_spi_max_transfer_size; sspi->hclk =devm_clk_get (&pdev->dev,"ahb" );if (IS_ERR (sspi->hclk)) {
Signature · 1 line
2.8.1
Re: [PATCH v3 04/13] spi: sunxi: expose maximum transfer size limit
Quoted 4 lines · Michal Suchanek
The sun4i spi hardware can trasfer at most 63 bytes of data without DMA support so report the limitation. Same for sun6i.
Signed-off-by Michal Suchanek <[email protected]>
Maxime
Signature · 10 lines
Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160613/429a3724/attachment.sig>
PATCH v3 5/13 spi: sun6i: update CS handling from spi-sun4i
In reply to Michal SuchanekWhile trying to merge sun4i and sun6i spi drivers I noticed the sun4i driver seems to have more reasonable CS handling.
Update sun6i to same.
--- drivers/spi/spi-sun6i.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
drivers/spi/spi-sun6i.c +19 -3
@@ -145,11 +145,30 @@ static void sun6i_spi_set_cs(struct spi_device *spi, bool enable) reg &= ~SUN6I_TFR_CTL_CS_MASK ; reg |=SUN6I_TFR_CTL_CS (spi->chip_select); +/* We want to control the chip select manually */ + reg |=SUN6I_TFR_CTL_CS_MANUAL ; +if (enable) reg |=SUN6I_TFR_CTL_CS_LEVEL ;else reg &= ~SUN6I_TFR_CTL_CS_LEVEL ; +/* +* Even though this looks irrelevant since we are supposed to +* be controlling the chip select manually, this bit also +* controls the levels of the chip select for inactive +* devices. +* +* If we don't set it, the chip select level will go low by +* default when the device is idle, which is not really +* expected in the common case where the chip select is active +* low. +*/ +if (spi->mode &SPI_CS_HIGH ) + reg &= ~SUN6I_TFR_CTL_SPOL ; +else + reg |=SUN6I_TFR_CTL_SPOL ; +sun6i_spi_write (sspi,SUN6I_TFR_CTL_REG , reg); }
@@ -215,9 +234,6 @@ static int sun6i_spi_transfer_one(struct spi_master *master,else reg |=SUN6I_TFR_CTL_DHB ; -/* We want to control the chip select manually */ - reg |=SUN6I_TFR_CTL_CS_MANUAL ; -sun6i_spi_write (sspi,SUN6I_TFR_CTL_REG , reg);/* Ensure that we have a parent clock fast enough */
Signature · 1 line
2.8.1
PATCH v3 6/13 spi: sunxi: rename sun4i,sun6i -> sunxi
In reply to Michal SuchanekPrepare to merge drivers.
--- drivers/spi/spi-sun4i.c | 274 +++++++++++++++++++++++----------------------- drivers/spi/spi-sun6i.c | 284 ++++++++++++++++++++++++------------------------ 2 files changed, 279 insertions(+), 279 deletions(-)
drivers/spi/spi-sun4i.c +137 -137
@@ -22,58 +22,58 @@#include <linux/spi/spi.h> -#define SUN4I_FIFO_DEPTH 64 - -#define SUN4I_RXDATA_REG 0x00 - -#define SUN4I_TXDATA_REG 0x04 - -#define SUN4I_CTL_REG 0x08 -#define SUN4I_CTL_ENABLE BIT(0) -#define SUN4I_CTL_MASTER BIT(1) -#define SUN4I_CTL_CPHA BIT(2) -#define SUN4I_CTL_CPOL BIT(3) -#define SUN4I_CTL_CS_ACTIVE_LOW BIT(4) -#define SUN4I_CTL_LMTF BIT(6) -#define SUN4I_CTL_TF_RST BIT(8) -#define SUN4I_CTL_RF_RST BIT(9) -#define SUN4I_CTL_XCH BIT(10) -#define SUN4I_CTL_CS_MASK 0x3000 -#define SUN4I_CTL_CS (cs) (((cs) << 12) & SUN4I_CTL_CS_MASK) -#define SUN4I_CTL_DHB BIT(15) -#define SUN4I_CTL_CS_MANUAL BIT(16) -#define SUN4I_CTL_CS_LEVEL BIT(17) -#define SUN4I_CTL_TP BIT(18) - -#define SUN4I_INT_CTL_REG 0x0c -#define SUN4I_INT_CTL_TC BIT(16) - -#define SUN4I_INT_STA_REG 0x10 - -#define SUN4I_DMA_CTL_REG 0x14 - -#define SUN4I_WAIT_REG 0x18 - -#define SUN4I_CLK_CTL_REG 0x1c -#define SUN4I_CLK_CTL_CDR2_MASK 0xff -#define SUN4I_CLK_CTL_CDR2 (div) ((div) & SUN4I_CLK_CTL_CDR2_MASK) -#define SUN4I_CLK_CTL_CDR1_MASK 0xf -#define SUN4I_CLK_CTL_CDR1 (div) (((div) & SUN4I_CLK_CTL_CDR1_MASK) << 8) -#define SUN4I_CLK_CTL_DRS BIT(12) - -#define SUN4I_BURST_CNT_REG 0x20 -#define SUN4I_BURST_CNT (cnt) ((cnt) & 0xffffff) - -#define SUN4I_XMIT_CNT_REG 0x24 -#define SUN4I_XMIT_CNT (cnt) ((cnt) & 0xffffff) - -#define SUN4I_FIFO_STA_REG 0x28 -#define SUN4I_FIFO_STA_RF_CNT_MASK 0x7f -#define SUN4I_FIFO_STA_RF_CNT_BITS 0 -#define SUN4I_FIFO_STA_TF_CNT_MASK 0x7f -#define SUN4I_FIFO_STA_TF_CNT_BITS 16 - -struct sun4i_spi { +#define SUNXI_FIFO_DEPTH 64 + +#define SUNXI_RXDATA_REG 0x00 + +#define SUNXI_TXDATA_REG 0x04 + +#define SUNXI_CTL_REG 0x08 +#define SUNXI_CTL_ENABLE BIT(0) +#define SUNXI_CTL_MASTER BIT(1) +#define SUNXI_CTL_CPHA BIT(2) +#define SUNXI_CTL_CPOL BIT(3) +#define SUNXI_CTL_CS_ACTIVE_LOW BIT(4) +#define SUNXI_CTL_LMTF BIT(6) +#define SUNXI_CTL_TF_RST BIT(8) +#define SUNXI_CTL_RF_RST BIT(9) +#define SUNXI_CTL_XCH BIT(10) +#define SUNXI_CTL_CS_MASK 0x3000 +#define SUNXI_CTL_CS (cs) (((cs) << 12) & SUNXI_CTL_CS_MASK) +#define SUNXI_CTL_DHB BIT(15) +#define SUNXI_CTL_CS_MANUAL BIT(16) +#define SUNXI_CTL_CS_LEVEL BIT(17) +#define SUNXI_CTL_TP BIT(18) + +#define SUNXI_INT_CTL_REG 0x0c +#define SUNXI_INT_CTL_TC BIT(16) + +#define SUNXI_INT_STA_REG 0x10 + +#define SUNXI_DMA_CTL_REG 0x14 + +#define SUNXI_WAIT_REG 0x18 + +#define SUNXI_CLK_CTL_REG 0x1c +#define SUNXI_CLK_CTL_CDR2_MASK 0xff +#define SUNXI_CLK_CTL_CDR2 (div) ((div) & SUNXI_CLK_CTL_CDR2_MASK) +#define SUNXI_CLK_CTL_CDR1_MASK 0xf +#define SUNXI_CLK_CTL_CDR1 (div) (((div) & SUNXI_CLK_CTL_CDR1_MASK) << 8) +#define SUNXI_CLK_CTL_DRS BIT(12) + +#define SUNXI_BURST_CNT_REG 0x20 +#define SUNXI_BURST_CNT (cnt) ((cnt) & 0xffffff) + +#define SUNXI_XMIT_CNT_REG 0x24 +#define SUNXI_XMIT_CNT (cnt) ((cnt) & 0xffffff) + +#define SUNXI_FIFO_STA_REG 0x28 +#define SUNXI_FIFO_STA_RF_CNT_MASK 0x7f +#define SUNXI_FIFO_STA_RF_CNT_BITS 0 +#define SUNXI_FIFO_STA_TF_CNT_MASK 0x7f +#define SUNXI_FIFO_STA_TF_CNT_BITS 16 + +struct sunxi_spi {struct spi_master *master;void __iomem *base_addr;struct clk *hclk;
@@ -86,37 +86,37 @@ struct sun4i_spi {int len; }; -static inline u32 sun4i_spi_read (struct sun4i_spi *sspi,u32 reg) +static inline u32 sunxi_spi_read (struct sunxi_spi *sspi,u32 reg) {return readl (sspi->base_addr + reg); } -static inline void sun4i_spi_write (struct sun4i_spi *sspi,u32 reg,u32 value) +static inline void sunxi_spi_write (struct sunxi_spi *sspi,u32 reg,u32 value) {writel (value, sspi->base_addr + reg); } -static inline void sun4i_spi_drain_fifo (struct sun4i_spi *sspi,int len) +static inline void sunxi_spi_drain_fifo (struct sunxi_spi *sspi,int len) {u32 reg, cnt;u8 byte;/* See how much data is available */ - reg =sun4i_spi_read (sspi,SUN4I_FIFO_STA_REG ); - reg &=SUN4I_FIFO_STA_RF_CNT_MASK ; - cnt = reg >>SUN4I_FIFO_STA_RF_CNT_BITS ; + reg =sunxi_spi_read (sspi,SUNXI_FIFO_STA_REG ); + reg &=SUNXI_FIFO_STA_RF_CNT_MASK ; + cnt = reg >>SUNXI_FIFO_STA_RF_CNT_BITS ;if (len > cnt) len = cnt;while (len--) { - byte =readb (sspi->base_addr +SUN4I_RXDATA_REG ); + byte =readb (sspi->base_addr +SUNXI_RXDATA_REG );if (sspi->rx_buf) *sspi->rx_buf++ = byte; } } -static inline void sun4i_spi_fill_fifo (struct sun4i_spi *sspi,int len) +static inline void sunxi_spi_fill_fifo (struct sunxi_spi *sspi,int len) {u8 byte;
@@ -125,28 +125,28 @@ static inline void sun4i_spi_fill_fifo(struct sun4i_spi *sspi, int len)while (len--) { byte = sspi->tx_buf ? *sspi->tx_buf++ :0 ; -writeb (byte, sspi->base_addr +SUN4I_TXDATA_REG ); +writeb (byte, sspi->base_addr +SUNXI_TXDATA_REG ); sspi->len--; } } -static void sun4i_spi_set_cs (struct spi_device *spi,bool enable) +static void sunxi_spi_set_cs (struct spi_device *spi,bool enable) { -struct sun4i_spi *sspi =spi_master_get_devdata (spi->master); +struct sunxi_spi *sspi =spi_master_get_devdata (spi->master);u32 reg; - reg =sun4i_spi_read (sspi,SUN4I_CTL_REG ); + reg =sunxi_spi_read (sspi,SUNXI_CTL_REG ); - reg &= ~SUN4I_CTL_CS_MASK ; - reg |=SUN4I_CTL_CS (spi->chip_select); + reg &= ~SUNXI_CTL_CS_MASK ; + reg |=SUNXI_CTL_CS (spi->chip_select);/* We want to control the chip select manually */ - reg |=SUN4I_CTL_CS_MANUAL ; + reg |=SUNXI_CTL_CS_MANUAL ;if (enable) - reg |=SUN4I_CTL_CS_LEVEL ; + reg |=SUNXI_CTL_CS_LEVEL ;else - reg &= ~SUN4I_CTL_CS_LEVEL ; + reg &= ~SUNXI_CTL_CS_LEVEL ;/* * Even though this looks irrelevant since we are supposed to
@@ -160,23 +160,23 @@ static void sun4i_spi_set_cs(struct spi_device *spi, bool enable)* low. */ if (spi->mode &SPI_CS_HIGH ) - reg &= ~SUN4I_CTL_CS_ACTIVE_LOW ; + reg &= ~SUNXI_CTL_CS_ACTIVE_LOW ;else - reg |=SUN4I_CTL_CS_ACTIVE_LOW ; + reg |=SUNXI_CTL_CS_ACTIVE_LOW ; -sun4i_spi_write (sspi,SUN4I_CTL_REG , reg); +sunxi_spi_write (sspi,SUNXI_CTL_REG , reg); } -static size_t sun4i_spi_max_transfer_size (struct spi_device *spi) +static size_t sunxi_spi_max_transfer_size (struct spi_device *spi) { -return SUN4I_FIFO_DEPTH -1 ; +return SUNXI_FIFO_DEPTH -1 ; } -static int sun4i_spi_transfer_one (struct spi_master *master, +static int sunxi_spi_transfer_one (struct spi_master *master,struct spi_device *spi,struct spi_transfer *tfr) { -struct sun4i_spi *sspi =spi_master_get_devdata (master); +struct sunxi_spi *sspi =spi_master_get_devdata (master);unsigned int mclk_rate, div, timeout;unsigned int start, end, tx_time;unsigned int tx_len =0 ;
@@ -184,10 +184,10 @@ static int sun4i_spi_transfer_one(struct spi_master *master,u32 reg;/* We don't support transfer larger than the FIFO */ -if (tfr->len >SUN4I_FIFO_DEPTH ) +if (tfr->len >SUNXI_FIFO_DEPTH )return -EMSGSIZE ; -if (tfr->tx_buf && tfr->len >=SUN4I_FIFO_DEPTH ) +if (tfr->tx_buf && tfr->len >=SUNXI_FIFO_DEPTH )return -EMSGSIZE ;reinit_completion (&sspi->done);
@@ -196,33 +196,33 @@ static int sun4i_spi_transfer_one(struct spi_master *master, sspi->len = tfr->len;/* Clear pending interrupts */ -sun4i_spi_write (sspi,SUN4I_INT_STA_REG , ~0 ); +sunxi_spi_write (sspi,SUNXI_INT_STA_REG , ~0 ); - reg =sun4i_spi_read (sspi,SUN4I_CTL_REG ); + reg =sunxi_spi_read (sspi,SUNXI_CTL_REG );/* Reset FIFOs */ -sun4i_spi_write (sspi,SUN4I_CTL_REG , - reg |SUN4I_CTL_RF_RST |SUN4I_CTL_TF_RST ); +sunxi_spi_write (sspi,SUNXI_CTL_REG , + reg |SUNXI_CTL_RF_RST |SUNXI_CTL_TF_RST );/* * Setup the transfer control register: Chip Select, * polarities, etc. */ if (spi->mode &SPI_CPOL ) - reg |=SUN4I_CTL_CPOL ; + reg |=SUNXI_CTL_CPOL ;else - reg &= ~SUN4I_CTL_CPOL ; + reg &= ~SUNXI_CTL_CPOL ;if (spi->mode &SPI_CPHA ) - reg |=SUN4I_CTL_CPHA ; + reg |=SUNXI_CTL_CPHA ;else - reg &= ~SUN4I_CTL_CPHA ; + reg &= ~SUNXI_CTL_CPHA ;if (spi->mode &SPI_LSB_FIRST ) - reg |=SUN4I_CTL_LMTF ; + reg |=SUNXI_CTL_LMTF ;else - reg &= ~SUN4I_CTL_LMTF ; + reg &= ~SUNXI_CTL_LMTF ;/*
@@ -230,11 +230,11 @@ static int sun4i_spi_transfer_one(struct spi_master *master,* FIFO with bogus data */ if (sspi->rx_buf) - reg &= ~SUN4I_CTL_DHB ; + reg &= ~SUNXI_CTL_DHB ;else - reg |=SUN4I_CTL_DHB ; + reg |=SUNXI_CTL_DHB ; -sun4i_spi_write (sspi,SUN4I_CTL_REG , reg); +sunxi_spi_write (sspi,SUNXI_CTL_REG , reg);/* Ensure that we have a parent clock fast enough */ mclk_rate =clk_get_rate (sspi->mclk);
@@ -258,39 +258,39 @@ static int sun4i_spi_transfer_one(struct spi_master *master,* frequency, fall back to CDR1. */ div = mclk_rate / (2 * tfr->speed_hz); -if (div <= (SUN4I_CLK_CTL_CDR2_MASK +1 )) { +if (div <= (SUNXI_CLK_CTL_CDR2_MASK +1 )) {if (div >0 ) div--; - reg =SUN4I_CLK_CTL_CDR2 (div) |SUN4I_CLK_CTL_DRS ; + reg =SUNXI_CLK_CTL_CDR2 (div) |SUNXI_CLK_CTL_DRS ; }else { div =ilog2 (mclk_rate) -ilog2 (tfr->speed_hz); - reg =SUN4I_CLK_CTL_CDR1 (div); + reg =SUNXI_CLK_CTL_CDR1 (div); } -sun4i_spi_write (sspi,SUN4I_CLK_CTL_REG , reg); +sunxi_spi_write (sspi,SUNXI_CLK_CTL_REG , reg);/* Setup the transfer now... */ if (sspi->tx_buf) tx_len = tfr->len;/* Setup the counters */ -sun4i_spi_write (sspi,SUN4I_BURST_CNT_REG ,SUN4I_BURST_CNT (tfr->len)); -sun4i_spi_write (sspi,SUN4I_XMIT_CNT_REG ,SUN4I_XMIT_CNT (tx_len)); +sunxi_spi_write (sspi,SUNXI_BURST_CNT_REG ,SUNXI_BURST_CNT (tfr->len)); +sunxi_spi_write (sspi,SUNXI_XMIT_CNT_REG ,SUNXI_XMIT_CNT (tx_len));/* * Fill the TX FIFO * Filling the FIFO fully causes timeout for some reason * at least on spi2 on A10s */ -sun4i_spi_fill_fifo (sspi,SUN4I_FIFO_DEPTH -1 ); +sunxi_spi_fill_fifo (sspi,SUNXI_FIFO_DEPTH -1 );/* Enable the interrupts */ -sun4i_spi_write (sspi,SUN4I_INT_CTL_REG ,SUN4I_INT_CTL_TC ); +sunxi_spi_write (sspi,SUNXI_INT_CTL_REG ,SUNXI_INT_CTL_TC );/* Start the transfer */ - reg =sun4i_spi_read (sspi,SUN4I_CTL_REG ); -sun4i_spi_write (sspi,SUN4I_CTL_REG , reg |SUN4I_CTL_XCH ); + reg =sunxi_spi_read (sspi,SUNXI_CTL_REG ); +sunxi_spi_write (sspi,SUNXI_CTL_REG , reg |SUNXI_CTL_XCH ); tx_time =max (tfr->len *8 *2 / (tfr->speed_hz /1000 ),100U ); start = jiffies;
@@ -306,22 +306,22 @@ static int sun4i_spi_transfer_one(struct spi_master *master, goto out; } -sun4i_spi_drain_fifo (sspi,SUN4I_FIFO_DEPTH ); +sunxi_spi_drain_fifo (sspi,SUNXI_FIFO_DEPTH ); out: -sun4i_spi_write (sspi,SUN4I_INT_CTL_REG ,0 ); +sunxi_spi_write (sspi,SUNXI_INT_CTL_REG ,0 );return ret; } -static irqreturn_t sun4i_spi_handler (int irq,void *dev_id) +static irqreturn_t sunxi_spi_handler (int irq,void *dev_id) { -struct sun4i_spi *sspi = dev_id; -u32 status =sun4i_spi_read (sspi,SUN4I_INT_STA_REG ); +struct sunxi_spi *sspi = dev_id; +u32 status =sunxi_spi_read (sspi,SUNXI_INT_STA_REG );/* Transfer complete */ -if (status &SUN4I_INT_CTL_TC ) { -sun4i_spi_write (sspi,SUN4I_INT_STA_REG ,SUN4I_INT_CTL_TC ); +if (status &SUNXI_INT_CTL_TC ) { +sunxi_spi_write (sspi,SUNXI_INT_STA_REG ,SUNXI_INT_CTL_TC );complete (&sspi->done);return IRQ_HANDLED ; }
@@ -329,10 +329,10 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id)return IRQ_NONE ; } -static int sun4i_spi_runtime_resume (struct device *dev) +static int sunxi_spi_runtime_resume (struct device *dev) {struct spi_master *master =dev_get_drvdata (dev); -struct sun4i_spi *sspi =spi_master_get_devdata (master); +struct sunxi_spi *sspi =spi_master_get_devdata (master);int ret; ret =clk_prepare_enable (sspi->hclk);
@@ -347,8 +347,8 @@ static int sun4i_spi_runtime_resume(struct device *dev) goto err; } -sun4i_spi_write (sspi,SUN4I_CTL_REG , -SUN4I_CTL_ENABLE |SUN4I_CTL_MASTER |SUN4I_CTL_TP ); +sunxi_spi_write (sspi,SUNXI_CTL_REG , +SUNXI_CTL_ENABLE |SUNXI_CTL_MASTER |SUNXI_CTL_TP );return 0 ;
@@ -358,10 +358,10 @@ out:return ret; } -static int sun4i_spi_runtime_suspend (struct device *dev) +static int sunxi_spi_runtime_suspend (struct device *dev) {struct spi_master *master =dev_get_drvdata (dev); -struct sun4i_spi *sspi =spi_master_get_devdata (master); +struct sunxi_spi *sspi =spi_master_get_devdata (master);clk_disable_unprepare (sspi->mclk);clk_disable_unprepare (sspi->hclk);
@@ -369,14 +369,14 @@ static int sun4i_spi_runtime_suspend(struct device *dev)return 0 ; } -static int sun4i_spi_probe (struct platform_device *pdev) +static int sunxi_spi_probe (struct platform_device *pdev) {struct spi_master *master; -struct sun4i_spi *sspi; +struct sunxi_spi *sspi;struct resource *res;int ret =0 , irq; - master =spi_alloc_master (&pdev->dev,sizeof (struct sun4i_spi )); + master =spi_alloc_master (&pdev->dev,sizeof (struct sunxi_spi ));if (!master) {dev_err (&pdev->dev,"Unable to allocate SPI Master\n" );return -ENOMEM ;
@@ -399,8 +399,8 @@ static int sun4i_spi_probe(struct platform_device *pdev) goto err_free_master; } - ret =devm_request_irq (&pdev->dev, irq, sun4i_spi_handler, -0 ,"sun4i-spi" , sspi); + ret =devm_request_irq (&pdev->dev, irq, sunxi_spi_handler, +0 ,"sunxi-spi" , sspi);if (ret) {dev_err (&pdev->dev,"Cannot request IRQ\n" ); goto err_free_master;
@@ -409,14 +409,14 @@ static int sun4i_spi_probe(struct platform_device *pdev) sspi->master = master; master->max_speed_hz =100 *1000 *1000 ; master->min_speed_hz =3 *1000 ; - master->set_cs = sun4i_spi_set_cs; - master->transfer_one = sun4i_spi_transfer_one; + master->set_cs = sunxi_spi_set_cs; + master->transfer_one = sunxi_spi_transfer_one; master->num_chipselect =4 ; master->mode_bits =SPI_CPOL |SPI_CPHA |SPI_CS_HIGH |SPI_LSB_FIRST ; master->bits_per_word_mask =SPI_BPW_MASK (8 ); master->dev.of_node = pdev->dev.of_node; master->auto_runtime_pm = true; - master->max_transfer_size = sun4i_spi_max_transfer_size; + master->max_transfer_size = sunxi_spi_max_transfer_size; sspi->hclk =devm_clk_get (&pdev->dev,"ahb" );if (IS_ERR (sspi->hclk)) {
@@ -438,7 +438,7 @@ static int sun4i_spi_probe(struct platform_device *pdev)* This wake-up/shutdown pattern is to be able to have the * device woken up, even if runtime_pm is disabled */ - ret =sun4i_spi_runtime_resume (&pdev->dev); + ret =sunxi_spi_runtime_resume (&pdev->dev);if (ret) {dev_err (&pdev->dev,"Couldn't resume the device\n" ); goto err_free_master;
@@ -458,40 +458,40 @@ static int sun4i_spi_probe(struct platform_device *pdev) err_pm_disable:pm_runtime_disable (&pdev->dev); -sun4i_spi_runtime_suspend (&pdev->dev); +sunxi_spi_runtime_suspend (&pdev->dev); err_free_master:spi_master_put (master);return ret; } -static int sun4i_spi_remove (struct platform_device *pdev) +static int sunxi_spi_remove (struct platform_device *pdev) {pm_runtime_disable (&pdev->dev);return 0 ; } -static const struct of_device_id sun4i_spi_match[] = { +static const struct of_device_id sunxi_spi_match[] = { { .compatible ="allwinner,sun4i-a10-spi" , }, {} }; -MODULE_DEVICE_TABLE (of, sun4i_spi_match); +MODULE_DEVICE_TABLE (of, sunxi_spi_match); -static const struct dev_pm_ops sun4i_spi_pm_ops = { - .runtime_resume = sun4i_spi_runtime_resume, - .runtime_suspend = sun4i_spi_runtime_suspend, +static const struct dev_pm_ops sunxi_spi_pm_ops = { + .runtime_resume = sunxi_spi_runtime_resume, + .runtime_suspend = sunxi_spi_runtime_suspend, }; -static struct platform_driver sun4i_spi_driver = { - .probe = sun4i_spi_probe, - .remove = sun4i_spi_remove, +static struct platform_driver sunxi_spi_driver = { + .probe = sunxi_spi_probe, + .remove = sunxi_spi_remove, .driver = { - .name ="sun4i-spi" , - .of_match_table = sun4i_spi_match, - .pm = &sun4i_spi_pm_ops, + .name ="sunxi-spi" , + .of_match_table = sunxi_spi_match, + .pm = &sunxi_spi_pm_ops, }, }; -module_platform_driver (sun4i_spi_driver); +module_platform_driver (sunxi_spi_driver);MODULE_AUTHOR ("Pan Nan <[email protected]>" );MODULE_AUTHOR ("Maxime Ripard <[email protected]>" );
drivers/spi/spi-sun6i.c +142 -142
511 lines not on this page · raw message
Signature · 1 line
2.8.1
PATCH v3 7/13 spi: sunxi: rename constants to match between sun4i and sun6i
In reply to Michal SuchanekSUNXI_CTL_ -> SUNXI_TFR_CTL_SUNXI_TFR_CTL_LMTF -> SUNXI_TFR_CTL_FBS SUNXI_TFR_CTL_CS_ACTIVE_LOW -> SUNXI_TFR_CTL_SPOL
and some SUNXI_???_CTL_ -> SUNXI_CTL_
for constants migrated to different registers between sun4i and sun6i
No functional change.
--- drivers/spi/spi-sun4i.c | 68 ++++++++++++++++++++++++------------------------- drivers/spi/spi-sun6i.c | 14 +++++----- 2 files changed, 41 insertions(+), 41 deletions(-)
drivers/spi/spi-sun4i.c +34 -34
@@ -28,21 +28,21 @@#define SUNXI_TXDATA_REG 0x04 -#define SUNXI_CTL_REG 0x08 +#define SUNXI_TFR_CTL_REG 0x08#define SUNXI_CTL_ENABLE BIT(0)#define SUNXI_CTL_MASTER BIT(1) -#define SUNXI_CTL_CPHA BIT(2) -#define SUNXI_CTL_CPOL BIT(3) -#define SUNXI_CTL_CS_ACTIVE_LOW BIT(4) -#define SUNXI_CTL_LMTF BIT(6) +#define SUNXI_TFR_CTL_CPHA BIT(2) +#define SUNXI_TFR_CTL_CPOL BIT(3) +#define SUNXI_TFR_CTL_SPOL BIT(4) +#define SUNXI_TFR_CTL_FBS BIT(6)#define SUNXI_CTL_TF_RST BIT(8)#define SUNXI_CTL_RF_RST BIT(9) -#define SUNXI_CTL_XCH BIT(10) -#define SUNXI_CTL_CS_MASK 0x3000 -#define SUNXI_CTL_CS (cs) (((cs) << 12) & SUNXI_CTL_CS_MASK) -#define SUNXI_CTL_DHB BIT(15) -#define SUNXI_CTL_CS_MANUAL BIT(16) -#define SUNXI_CTL_CS_LEVEL BIT(17) +#define SUNXI_TFR_CTL_XCH BIT(10) +#define SUNXI_TFR_CTL_CS_MASK 0x3000 +#define SUNXI_TFR_CTL_CS (cs) (((cs) << 12) & SUNXI_TFR_CTL_CS_MASK) +#define SUNXI_TFR_CTL_DHB BIT(15) +#define SUNXI_TFR_CTL_CS_MANUAL BIT(16) +#define SUNXI_TFR_CTL_CS_LEVEL BIT(17)#define SUNXI_CTL_TP BIT(18)#define SUNXI_INT_CTL_REG 0x0c
@@ -135,18 +135,18 @@ static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)struct sunxi_spi *sspi =spi_master_get_devdata (spi->master);u32 reg; - reg =sunxi_spi_read (sspi,SUNXI_CTL_REG ); + reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG ); - reg &= ~SUNXI_CTL_CS_MASK ; - reg |=SUNXI_CTL_CS (spi->chip_select); + reg &= ~SUNXI_TFR_CTL_CS_MASK ; + reg |=SUNXI_TFR_CTL_CS (spi->chip_select);/* We want to control the chip select manually */ - reg |=SUNXI_CTL_CS_MANUAL ; + reg |=SUNXI_TFR_CTL_CS_MANUAL ;if (enable) - reg |=SUNXI_CTL_CS_LEVEL ; + reg |=SUNXI_TFR_CTL_CS_LEVEL ;else - reg &= ~SUNXI_CTL_CS_LEVEL ; + reg &= ~SUNXI_TFR_CTL_CS_LEVEL ;/* * Even though this looks irrelevant since we are supposed to
@@ -160,11 +160,11 @@ static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)* low. */ if (spi->mode &SPI_CS_HIGH ) - reg &= ~SUNXI_CTL_CS_ACTIVE_LOW ; + reg &= ~SUNXI_TFR_CTL_SPOL ;else - reg |=SUNXI_CTL_CS_ACTIVE_LOW ; + reg |=SUNXI_TFR_CTL_SPOL ; -sunxi_spi_write (sspi,SUNXI_CTL_REG , reg); +sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , reg); }static size_t sunxi_spi_max_transfer_size (struct spi_device *spi)
@@ -199,10 +199,10 @@ static int sunxi_spi_transfer_one(struct spi_master *master,sunxi_spi_write (sspi,SUNXI_INT_STA_REG , ~0 ); - reg =sunxi_spi_read (sspi,SUNXI_CTL_REG ); + reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG );/* Reset FIFOs */ -sunxi_spi_write (sspi,SUNXI_CTL_REG , +sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , reg |SUNXI_CTL_RF_RST |SUNXI_CTL_TF_RST );/*
@@ -210,19 +210,19 @@ static int sunxi_spi_transfer_one(struct spi_master *master,* polarities, etc. */ if (spi->mode &SPI_CPOL ) - reg |=SUNXI_CTL_CPOL ; + reg |=SUNXI_TFR_CTL_CPOL ;else - reg &= ~SUNXI_CTL_CPOL ; + reg &= ~SUNXI_TFR_CTL_CPOL ;if (spi->mode &SPI_CPHA ) - reg |=SUNXI_CTL_CPHA ; + reg |=SUNXI_TFR_CTL_CPHA ;else - reg &= ~SUNXI_CTL_CPHA ; + reg &= ~SUNXI_TFR_CTL_CPHA ;if (spi->mode &SPI_LSB_FIRST ) - reg |=SUNXI_CTL_LMTF ; + reg |=SUNXI_TFR_CTL_FBS ;else - reg &= ~SUNXI_CTL_LMTF ; + reg &= ~SUNXI_TFR_CTL_FBS ;/*
@@ -230,11 +230,11 @@ static int sunxi_spi_transfer_one(struct spi_master *master,* FIFO with bogus data */ if (sspi->rx_buf) - reg &= ~SUNXI_CTL_DHB ; + reg &= ~SUNXI_TFR_CTL_DHB ;else - reg |=SUNXI_CTL_DHB ; + reg |=SUNXI_TFR_CTL_DHB ; -sunxi_spi_write (sspi,SUNXI_CTL_REG , reg); +sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , reg);/* Ensure that we have a parent clock fast enough */ mclk_rate =clk_get_rate (sspi->mclk);
@@ -289,8 +289,8 @@ static int sunxi_spi_transfer_one(struct spi_master *master,sunxi_spi_write (sspi,SUNXI_INT_CTL_REG ,SUNXI_INT_CTL_TC );/* Start the transfer */ - reg =sunxi_spi_read (sspi,SUNXI_CTL_REG ); -sunxi_spi_write (sspi,SUNXI_CTL_REG , reg |SUNXI_CTL_XCH ); + reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG ); +sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , reg |SUNXI_TFR_CTL_XCH ); tx_time =max (tfr->len *8 *2 / (tfr->speed_hz /1000 ),100U ); start = jiffies;
@@ -347,7 +347,7 @@ static int sunxi_spi_runtime_resume(struct device *dev) goto err; } -sunxi_spi_write (sspi,SUNXI_CTL_REG , +sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG ,SUNXI_CTL_ENABLE |SUNXI_CTL_MASTER |SUNXI_CTL_TP );return 0 ;
drivers/spi/spi-sun6i.c +7 -7
@@ -26,9 +26,9 @@#define SUNXI_FIFO_DEPTH 128#define SUNXI_GBL_CTL_REG 0x04 -#define SUNXI_GBL_CTL_BUS_ENABLE BIT(0) -#define SUNXI_GBL_CTL_MASTER BIT(1) -#define SUNXI_GBL_CTL_TP BIT(7) +#define SUNXI_CTL_ENABLE BIT(0) +#define SUNXI_CTL_MASTER BIT(1) +#define SUNXI_CTL_TP BIT(7)#define SUNXI_GBL_CTL_RST BIT(31)#define SUNXI_TFR_CTL_REG 0x08
@@ -50,8 +50,8 @@#define SUNXI_INT_STA_REG 0x14#define SUNXI_FIFO_CTL_REG 0x18 -#define SUNXI_FIFO_CTL_RF_RST BIT(15) -#define SUNXI_FIFO_CTL_TF_RST BIT(31) +#define SUNXI_CTL_RF_RST BIT(15) +#define SUNXI_CTL_TF_RST BIT(31)#define SUNXI_FIFO_STA_REG 0x1c#define SUNXI_FIFO_STA_RF_CNT_MASK 0x7f
@@ -202,7 +202,7 @@ static int sunxi_spi_transfer_one(struct spi_master *master,/* Reset FIFO */ sunxi_spi_write (sspi,SUNXI_FIFO_CTL_REG , -SUNXI_FIFO_CTL_RF_RST |SUNXI_FIFO_CTL_TF_RST ); +SUNXI_CTL_RF_RST |SUNXI_CTL_TF_RST );/* * Setup the transfer control register: Chip Select,
@@ -352,7 +352,7 @@ static int sunxi_spi_runtime_resume(struct device *dev) }sunxi_spi_write (sspi,SUNXI_GBL_CTL_REG , -SUNXI_GBL_CTL_BUS_ENABLE |SUNXI_GBL_CTL_MASTER |SUNXI_GBL_CTL_TP ); +SUNXI_CTL_ENABLE |SUNXI_CTL_MASTER |SUNXI_CTL_TP );return 0 ;
Signature · 1 line
2.8.1
Re: [linux-sunxi] [PATCH v3 07/13] spi: sunxi: rename constants to match between sun4i and sun6i
Hi Michal,
Quoted 2 lines · Michal Suchanek
SUNXI_CTL_->SUNXI_TFR_CTL_SUNXI_TFR_CTL_LMTF->SUNXI_TFR_CTL_FBS
I don't know these abbreviations, are they both referring to the same thing?
Quoted 1 line
SUNXI_TFR_CTL_CS_ACTIVE_LOW->SUNXI_TFR_CTL_SPOL
It looks like you're making the constant name less descriptive here. Is the old version (CS_ACTIVE_LOW) incorrect?
Quoted 53 lines
and some SUNXI_???_CTL_ ->
SUNXI_CTL_
for constants migrated to different registers between sun4i and sun6iNo functional change.
Signed-off-by Michal Suchanek <[email protected]>--- drivers/spi/spi-sun4i.c | 68 ++++++++++++++++++++++++------------------------- drivers/spi/spi-sun6i.c | 14 +++++----- 2 files changed, 41 insertions(+), 41 deletions(-)drivers/spi/spi-sun4i.c +11
-11@@ -28,21 +28,21 @@#define SUNXI_TXDATA_REG 0x04 -#define SUNXI_CTL_REG 0x08 +#define SUNXI_TFR_CTL_REG 0x08#define SUNXI_CTL_ENABLE BIT(0)#define SUNXI_CTL_MASTER BIT(1) -#define SUNXI_CTL_CPHA BIT(2) -#define SUNXI_CTL_CPOL BIT(3) -#define SUNXI_CTL_CS_ACTIVE_LOW BIT(4) -#define SUNXI_CTL_LMTF BIT(6) +#define SUNXI_TFR_CTL_CPHA BIT(2) +#define SUNXI_TFR_CTL_CPOL BIT(3) +#define SUNXI_TFR_CTL_SPOL BIT(4) +#define SUNXI_TFR_CTL_FBS BIT(6)#define SUNXI_CTL_TF_RST BIT(8)#define SUNXI_CTL_RF_RST BIT(9) -#define SUNXI_CTL_XCH BIT(10) -#define SUNXI_CTL_CS_MASK 0x3000 -#define SUNXI_CTL_CS (cs) (((cs) << 12) & SUNXI_CTL_CS_MASK) -#define SUNXI_CTL_DHB BIT(15) -#define SUNXI_CTL_CS_MANUAL BIT(16) -#define SUNXI_CTL_CS_LEVEL BIT(17) +#define SUNXI_TFR_CTL_XCH BIT(10) +#define SUNXI_TFR_CTL_CS_MASK 0x3000 +#define SUNXI_TFR_CTL_CS (cs) (((cs) << 12) & SUNXI_TFR_CTL_CS_MASK) +#define SUNXI_TFR_CTL_DHB BIT(15) +#define SUNXI_TFR_CTL_CS_MANUAL BIT(16) +#define SUNXI_TFR_CTL_CS_LEVEL BIT(17)#define SUNXI_CTL_TP BIT(18)#define SUNXI_INT_CTL_REG 0x0cdrivers/spi/spi-sun6i.c +0
-0
drivers/spi/spi-sun6i.c line 31
Quoted 9 lines
drivers/spi/spi-sun6i.c +3
-3@@ -26,9 +26,9 @@#define SUNXI_FIFO_DEPTH 128#define SUNXI_GBL_CTL_REG 0x04 -#define SUNXI_GBL_CTL_BUS_ENABLE BIT(0) -#define SUNXI_GBL_CTL_MASTER BIT(1) -#define SUNXI_GBL_CTL_TP BIT(7) +#define SUNXI_CTL_ENABLE BIT(0) +#define SUNXI_CTL_MASTER BIT(1) +#define SUNXI_CTL_TP BIT(7)
If these are bit definitions for the GBL register, why throw that information away?
drivers/spi/spi-sun6i.c line 54
Quoted 11 lines
drivers/spi/spi-sun6i.c +2
-2#define SUNXI_GBL_CTL_RST BIT(31)#define SUNXI_TFR_CTL_REG 0x08@@ -50,8 +50,8 @@#define SUNXI_INT_STA_REG 0x14#define SUNXI_FIFO_CTL_REG 0x18 -#define SUNXI_FIFO_CTL_RF_RST BIT(15) -#define SUNXI_FIFO_CTL_TF_RST BIT(31) +#define SUNXI_CTL_RF_RST BIT(15) +#define SUNXI_CTL_TF_RST BIT(31)
Same here with FIFO.
drivers/spi/spi-sun6i.c line 57
Quoted 3 lines
drivers/spi/spi-sun6i.c +0
-0#define SUNXI_FIFO_STA_REG 0x1c#define SUNXI_FIFO_STA_RF_CNT_MASK 0x7f
My gut feeling on this is that we have a lot of cases of a definition of a register offset, then definitions of the bits in that register with that register encoded into the constant's name. You appear to be throwing a lot of that information away which makes me worry.
Thanks,
Signature · 4 lines
Julian Calaby Email: julian.calaby at gmail.com Profile: http://www.google.com/profiles/julian.calaby/
Re: [linux-sunxi] [PATCH v3 07/13] spi: sunxi: rename constants to match between sun4i and sun6i
Hello,
Quoted 26 lines · Julian Calaby
Hi Michal,
Quoted 2 lines · Michal Suchanek
SUNXI_CTL_->SUNXI_TFR_CTL_SUNXI_TFR_CTL_LMTF->SUNXI_TFR_CTL_FBSI don't know these abbreviations, are they both referring to the same thing?
Quoted 1 line
SUNXI_TFR_CTL_CS_ACTIVE_LOW->SUNXI_TFR_CTL_SPOLIt looks like you're making the constant name less descriptive here. Is the old version (
CS_ACTIVE_LOW) incorrect?Quoted 13 lines
and some SUNXI_???_CTL_ ->
SUNXI_CTL_
for constants migrated to different registers between sun4i and sun6iNo functional change.
#define
SUNXI_INT_CTL_REG0x0cdrivers/spi/spi-sun6i.c +0
-0@@ -26,9 +26,9 @@#define SUNXI_FIFO_DEPTH 128
Quoted 10 lines
drivers/spi/spi-sun6i.c line 31
Quoted 7 lines
drivers/spi/spi-sun6i.c +3
-3#define SUNXI_GBL_CTL_REG 0x04 -#define SUNXI_GBL_CTL_BUS_ENABLE BIT(0) -#define SUNXI_GBL_CTL_MASTER BIT(1) -#define SUNXI_GBL_CTL_TP BIT(7) +#define SUNXI_CTL_ENABLE BIT(0) +#define SUNXI_CTL_MASTER BIT(1) +#define SUNXI_CTL_TP BIT(7)If these are bit definitions for the GBL register, why throw that information away?
Those bits are on the TFR register in the earlier IP so it makes perfect sense to me this way.
Thanks
Michal
PATCH v3 8/13 spi: sunxi: synchronize whitespace, comments, struct
In reply to Michal SuchanekMinimize differences between sun4i and sun6i driver without code changes.
--- drivers/spi/spi-sun4i.c | 11 ++++++----- drivers/spi/spi-sun6i.c | 8 +++----- 2 files changed, 9 insertions(+), 10 deletions(-)
drivers/spi/spi-sun4i.c +6 -5
@@ -19,6 +19,7 @@#include <linux/module.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> +#include <linux/reset.h> #include <linux/spi/spi.h>
@@ -56,7 +57,7 @@#define SUNXI_CLK_CTL_REG 0x1c#define SUNXI_CLK_CTL_CDR2_MASK 0xff -#define SUNXI_CLK_CTL_CDR2 (div) ((div) & SUNXI_CLK_CTL_CDR2_MASK) +#define SUNXI_CLK_CTL_CDR2 (div) (((div) & SUNXI_CLK_CTL_CDR2_MASK) << 0)#define SUNXI_CLK_CTL_CDR1_MASK 0xf#define SUNXI_CLK_CTL_CDR1 (div) (((div) & SUNXI_CLK_CTL_CDR1_MASK) << 8)#define SUNXI_CLK_CTL_DRS BIT(12)
@@ -78,6 +79,7 @@ struct sunxi_spi {void __iomem *base_addr;struct clk *hclk;struct clk *mclk; +struct reset_control *rstc;struct completion done;
@@ -136,7 +138,6 @@ static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)u32 reg; reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG ); - reg &= ~SUNXI_TFR_CTL_CS_MASK ; reg |=SUNXI_TFR_CTL_CS (spi->chip_select);
@@ -198,7 +199,6 @@ static int sunxi_spi_transfer_one(struct spi_master *master,/* Clear pending interrupts */ sunxi_spi_write (sspi,SUNXI_INT_STA_REG , ~0 ); - reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG );/* Reset FIFOs */
@@ -224,7 +224,6 @@ static int sunxi_spi_transfer_one(struct spi_master *master,else reg &= ~SUNXI_TFR_CTL_FBS ; -/* * If it's a TX only transfer, we don't want to fill the RX * FIFO with bogus data
@@ -248,7 +247,7 @@ static int sunxi_spi_transfer_one(struct spi_master *master,* * We have two choices there. Either we can use the clock * divide rate 1, which is calculated thanks to this formula: -* SPI_CLK = MOD_CLK / (2 ^ (cdr + 1)) +* SPI_CLK = MOD_CLK / (2 ^ cdr) * Or we can use CDR2, which is calculated with the formula: * SPI_CLK = MOD_CLK / (2 * (cdr + 1)) * Wether we use the former or the latter is set through the
@@ -352,6 +351,8 @@ static int sunxi_spi_runtime_resume(struct device *dev)return 0 ; +err2: +clk_disable_unprepare (sspi->mclk); err:clk_disable_unprepare (sspi->hclk); out:
drivers/spi/spi-sun6i.c +3 -5
@@ -29,7 +29,6 @@#define SUNXI_CTL_ENABLE BIT(0)#define SUNXI_CTL_MASTER BIT(1)#define SUNXI_CTL_TP BIT(7) -#define SUNXI_GBL_CTL_RST BIT(31)#define SUNXI_TFR_CTL_REG 0x08#define SUNXI_TFR_CTL_CPHA BIT(0)
@@ -44,7 +43,6 @@#define SUNXI_TFR_CTL_XCH BIT(31)#define SUNXI_INT_CTL_REG 0x10 -#define SUNXI_INT_CTL_RF_OVF BIT(8)#define SUNXI_INT_CTL_TC BIT(12)#define SUNXI_INT_STA_REG 0x14
@@ -200,7 +198,9 @@ static int sunxi_spi_transfer_one(struct spi_master *master,/* Clear pending interrupts */ sunxi_spi_write (sspi,SUNXI_INT_STA_REG , ~0 ); -/* Reset FIFO */ + reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG ); + +/* Reset FIFOs */ sunxi_spi_write (sspi,SUNXI_FIFO_CTL_REG ,SUNXI_CTL_RF_RST |SUNXI_CTL_TF_RST );
@@ -208,8 +208,6 @@ static int sunxi_spi_transfer_one(struct spi_master *master,* Setup the transfer control register: Chip Select, * polarities, etc. */ - reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG ); -if (spi->mode &SPI_CPOL ) reg |=SUNXI_TFR_CTL_CPOL ;else
Signature · 1 line
2.8.1
PATCH v3 9/13 spi: sunxi: use register map
In reply to Michal SuchanekThe driver code uses the fact that several bits are set in one register to set them at once.
Between hardware revisions these bits might migrate to different position or different register altogether but the logical bit groups that are used together end up in the same register.
--- drivers/spi/spi-sun4i.c | 227 ++++++++++++++++++++++++++++++-------------- drivers/spi/spi-sun6i.c | 243 ++++++++++++++++++++++++++++++++---------------- 2 files changed, 322 insertions(+), 148 deletions(-)
drivers/spi/spi-sun4i.c +157 -70
@@ -23,63 +23,118 @@#include <linux/spi/spi.h> -#define SUNXI_FIFO_DEPTH 64 +#define SUN4I_FIFO_DEPTH 64 +#define SUN6I_FIFO_DEPTH 128 -#define SUNXI_RXDATA_REG 0x00 +#define SUN4I_COMPATIBLE "allwinner,sun4i-a10-spi" +#define SUN6I_COMPATIBLE "allwinner,sun6i-a31-spi" -#define SUNXI_TXDATA_REG 0x04 +#define SUNXI_TFR_CTL_CS (bitmap, cs) (((cs) << \ + (bitmap)[SUNXI_TFR_CTL_CS_SHIFT]) \ + & (bitmap)[SUNXI_TFR_CTL_CS_MASK]) -#define SUNXI_TFR_CTL_REG 0x08 -#define SUNXI_CTL_ENABLE BIT(0) -#define SUNXI_CTL_MASTER BIT(1) -#define SUNXI_TFR_CTL_CPHA BIT(2) -#define SUNXI_TFR_CTL_CPOL BIT(3) -#define SUNXI_TFR_CTL_SPOL BIT(4) -#define SUNXI_TFR_CTL_FBS BIT(6) -#define SUNXI_CTL_TF_RST BIT(8) -#define SUNXI_CTL_RF_RST BIT(9) -#define SUNXI_TFR_CTL_XCH BIT(10) -#define SUNXI_TFR_CTL_CS_MASK 0x3000 -#define SUNXI_TFR_CTL_CS (cs) (((cs) << 12) & SUNXI_TFR_CTL_CS_MASK) -#define SUNXI_TFR_CTL_DHB BIT(15) -#define SUNXI_TFR_CTL_CS_MANUAL BIT(16) -#define SUNXI_TFR_CTL_CS_LEVEL BIT(17) -#define SUNXI_CTL_TP BIT(18) +#define SUNXI_CNT_MASK 0xffffff +#define SUNXI_XMIT_CNT (cnt) ((cnt) & SUNXI_CNT_MASK) +#define SUNXI_BURST_CNT (cnt) ((cnt) & SUNXI_CNT_MASK) +#define SUNXI_BURST_CTL_CNT_STC (cnt) ((cnt) & SUNXI_CNT_MASK) -#define SUNXI_INT_CTL_REG 0x0c -#define SUNXI_INT_CTL_TC BIT(16) - -#define SUNXI_INT_STA_REG 0x10 - -#define SUNXI_DMA_CTL_REG 0x14 - -#define SUNXI_WAIT_REG 0x18 - -#define SUNXI_CLK_CTL_REG 0x1c +#define SUNXI_CLK_CTL_DRS BIT(12)#define SUNXI_CLK_CTL_CDR2_MASK 0xff#define SUNXI_CLK_CTL_CDR2 (div) (((div) & SUNXI_CLK_CTL_CDR2_MASK) << 0)#define SUNXI_CLK_CTL_CDR1_MASK 0xf#define SUNXI_CLK_CTL_CDR1 (div) (((div) & SUNXI_CLK_CTL_CDR1_MASK) << 8) -#define SUNXI_CLK_CTL_DRS BIT(12) - -#define SUNXI_BURST_CNT_REG 0x20 -#define SUNXI_BURST_CNT (cnt) ((cnt) & 0xffffff) -#define SUNXI_XMIT_CNT_REG 0x24 -#define SUNXI_XMIT_CNT (cnt) ((cnt) & 0xffffff) - -#define SUNXI_FIFO_STA_REG 0x28#define SUNXI_FIFO_STA_RF_CNT_MASK 0x7f#define SUNXI_FIFO_STA_RF_CNT_BITS 0#define SUNXI_FIFO_STA_TF_CNT_MASK 0x7f#define SUNXI_FIFO_STA_TF_CNT_BITS 16 +enum SPI_SUNXI_TYPE { +SPI_SUN4I =1 , +SPI_SUN6I , +}; + +enum SUNXI_REG_ENUM { +SUNXI_RXDATA_REG , +SUNXI_TXDATA_REG , +SUNXI_TFR_CTL_REG , +SUNXI_INT_CTL_REG , +SUNXI_INT_STA_REG , +SUNXI_WAIT_REG , +SUNXI_CLK_CTL_REG , +SUNXI_BURST_CNT_REG , +SUNXI_XMIT_CNT_REG , +SUNXI_FIFO_STA_REG , +SUNXI_VERSION_REG , +SUNXI_GBL_CTL_REG , +SUNXI_FIFO_CTL_REG , +SUNXI_BURST_CTL_CNT_REG , +SUNXI_NUM_REGS +}; + +static int sun4i_regmap[SUNXI_NUM_REGS ] = { +/* SUNXI_RXDATA_REG */ 0x00 , +/* SUNXI_TXDATA_REG */ 0x04 , +/* SUNXI_TFR_CTL_REG */ 0x08 , +/* SUNXI_INT_CTL_REG */ 0x0c , +/* SUNXI_INT_STA_REG */ 0x10 , +/* SUNXI_WAIT_REG */ 0x18 , +/* SUNXI_CLK_CTL_REG */ 0x1c , +/* SUNXI_BURST_CNT_REG */ 0x20 , +/* SUNXI_XMIT_CNT_REG */ 0x24 , +/* SUNXI_FIFO_STA_REG */ 0x28 , +-1 ,-1 ,-1 ,-1 +}; + +enum SUNXI_BITMAP_ENUM { +SUNXI_CTL_ENABLE , +SUNXI_CTL_MASTER , +SUNXI_TFR_CTL_CPHA , +SUNXI_TFR_CTL_CPOL , +SUNXI_TFR_CTL_CS_ACTIVE_LOW , +SUNXI_TFR_CTL_FBS , +SUNXI_CTL_TF_RST , +SUNXI_CTL_RF_RST , +SUNXI_TFR_CTL_XCH , +SUNXI_TFR_CTL_CS_MASK , +SUNXI_TFR_CTL_CS_SHIFT , +SUNXI_TFR_CTL_DHB , +SUNXI_TFR_CTL_CS_MANUAL , +SUNXI_TFR_CTL_CS_LEVEL , +SUNXI_CTL_TP , +SUNXI_INT_CTL_TC , +SUNXI_BITMAP_SIZE +}; + +static int sun4i_bitmap[SUNXI_BITMAP_SIZE ] = { +/* SUNXI_CTL_ENABLE */ BIT (0 ), +/* SUNXI_CTL_MASTER */ BIT (1 ), +/* SUNXI_TFR_CTL_CPHA */ BIT (2 ), +/* SUNXI_TFR_CTL_CPOL */ BIT (3 ), +/* SUNXI_TFR_CTL_CS_ACTIVE_LOW */ BIT (4 ), +/* SUNXI_TFR_CTL_FBS */ BIT (6 ), +/* SUNXI_CTL_TF_RST */ BIT (8 ), +/* SUNXI_CTL_RF_RST */ BIT (9 ), +/* SUNXI_TFR_CTL_XCH */ BIT (10 ), +/* SUNXI_TFR_CTL_CS_MASK */ 0x3000 , +/* SUNXI_TFR_CTL_CS_SHIFT */ 12 , +/* SUNXI_TFR_CTL_DHB */ BIT (15 ), +/* SUNXI_TFR_CTL_CS_MANUAL */ BIT (16 ), +/* SUNXI_TFR_CTL_CS_LEVEL */ BIT (17 ), +/* SUNXI_CTL_TP */ BIT (18 ), +/* SUNXI_INT_CTL_TC */ BIT (16 ), +}; +struct sunxi_spi {struct spi_master *master;void __iomem *base_addr;struct clk *hclk;struct clk *mclk;struct reset_control *rstc; +int (*regmap)[SUNXI_NUM_REGS ]; +int (*bitmap)[SUNXI_BITMAP_SIZE ]; +int fifo_depth; +int type;struct completion done;
@@ -88,14 +143,31 @@ struct sunxi_spi {int len; }; -static inline u32 sunxi_spi_read (struct sunxi_spi *sspi,u32 reg) +static inline u32 sspi_reg (struct sunxi_spi *sspi,enum SUNXI_REG_ENUM name) +{ +BUG_ON ((name >=SUNXI_NUM_REGS ) || (name <0 ) || + (*sspi->regmap)[name] <0 ); +return (*sspi->regmap)[name]; +} + +static inline u32 sunxi_spi_read (struct sunxi_spi *sspi, +enum SUNXI_REG_ENUM name) { -return readl (sspi->base_addr + reg); +return readl (sspi->base_addr +sspi_reg (sspi, name)); } -static inline void sunxi_spi_write (struct sunxi_spi *sspi,u32 reg,u32 value) +static inline void sunxi_spi_write (struct sunxi_spi *sspi, +enum SUNXI_REG_ENUM name,u32 value) { -writel (value, sspi->base_addr + reg); +writel (value, sspi->base_addr +sspi_reg (sspi, name)); +} + +static inline u32 sspi_bits (struct sunxi_spi *sspi, +enum SUNXI_BITMAP_ENUM name) +{ +BUG_ON ((name >=SUNXI_BITMAP_SIZE ) || (name <0 ) || + (*sspi->bitmap)[name] <=0 ); +return (*sspi->bitmap)[name]; }static inline void sunxi_spi_drain_fifo (struct sunxi_spi *sspi,int len)
@@ -112,7 +184,8 @@ static inline void sunxi_spi_drain_fifo(struct sunxi_spi *sspi, int len) len = cnt;while (len--) { - byte =readb (sspi->base_addr +SUNXI_RXDATA_REG ); + byte =readb (sspi->base_addr + +sspi_reg (sspi,SUNXI_RXDATA_REG ));if (sspi->rx_buf) *sspi->rx_buf++ = byte; }
@@ -127,7 +200,8 @@ static inline void sunxi_spi_fill_fifo(struct sunxi_spi *sspi, int len)while (len--) { byte = sspi->tx_buf ? *sspi->tx_buf++ :0 ; -writeb (byte, sspi->base_addr +SUNXI_TXDATA_REG ); +writeb (byte, sspi->base_addr + +sspi_reg (sspi,SUNXI_TXDATA_REG )); sspi->len--; } }
@@ -138,16 +212,16 @@ static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)u32 reg; reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG ); - reg &= ~SUNXI_TFR_CTL_CS_MASK ; - reg |=SUNXI_TFR_CTL_CS (spi->chip_select); + reg &= ~sspi_bits (sspi,SUNXI_TFR_CTL_CS_MASK ); + reg |=SUNXI_TFR_CTL_CS (*sspi->bitmap, spi->chip_select);/* We want to control the chip select manually */ - reg |=SUNXI_TFR_CTL_CS_MANUAL ; + reg |=sspi_bits (sspi,SUNXI_TFR_CTL_CS_MANUAL );if (enable) - reg |=SUNXI_TFR_CTL_CS_LEVEL ; + reg |=sspi_bits (sspi,SUNXI_TFR_CTL_CS_LEVEL );else - reg &= ~SUNXI_TFR_CTL_CS_LEVEL ; + reg &= ~sspi_bits (sspi,SUNXI_TFR_CTL_CS_LEVEL );/* * Even though this looks irrelevant since we are supposed to
@@ -161,16 +235,19 @@ static void sunxi_spi_set_cs(struct spi_device *spi, bool enable)* low. */ if (spi->mode &SPI_CS_HIGH ) - reg &= ~SUNXI_TFR_CTL_SPOL ; + reg &= ~sspi_bits (sspi,SUNXI_TFR_CTL_CS_ACTIVE_LOW );else - reg |=SUNXI_TFR_CTL_SPOL ; + reg |=sspi_bits (sspi,SUNXI_TFR_CTL_CS_ACTIVE_LOW );sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , reg); }static size_t sunxi_spi_max_transfer_size (struct spi_device *spi) { -return SUNXI_FIFO_DEPTH -1 ; +struct spi_master *master = spi->master; +struct sunxi_spi *sspi =spi_master_get_devdata (master); + +return sspi->fifo_depth -1 ; } static int sunxi_spi_transfer_one(struct spi_master *master,
@@ -185,10 +262,10 @@ static int sunxi_spi_transfer_one(struct spi_master *master,u32 reg;/* We don't support transfer larger than the FIFO */ -if (tfr->len >SUNXI_FIFO_DEPTH ) +if (tfr->len > sspi->fifo_depth)return -EMSGSIZE ; -if (tfr->tx_buf && tfr->len >=SUNXI_FIFO_DEPTH ) +if (tfr->tx_buf && tfr->len >= sspi->fifo_depth)return -EMSGSIZE ;reinit_completion (&sspi->done);
@@ -203,35 +280,36 @@ static int sunxi_spi_transfer_one(struct spi_master *master,/* Reset FIFOs */ sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , - reg |SUNXI_CTL_RF_RST |SUNXI_CTL_TF_RST ); + reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST ));/* * Setup the transfer control register: Chip Select, * polarities, etc. */ if (spi->mode &SPI_CPOL ) - reg |=SUNXI_TFR_CTL_CPOL ; + reg |=sspi_bits (sspi,SUNXI_TFR_CTL_CPOL );else - reg &= ~SUNXI_TFR_CTL_CPOL ; + reg &= ~sspi_bits (sspi,SUNXI_TFR_CTL_CPOL );if (spi->mode &SPI_CPHA ) - reg |=SUNXI_TFR_CTL_CPHA ; + reg |=sspi_bits (sspi,SUNXI_TFR_CTL_CPHA );else - reg &= ~SUNXI_TFR_CTL_CPHA ; + reg &= ~sspi_bits (sspi,SUNXI_TFR_CTL_CPHA );if (spi->mode &SPI_LSB_FIRST ) - reg |=SUNXI_TFR_CTL_FBS ; + reg |=sspi_bits (sspi,SUNXI_TFR_CTL_FBS );else - reg &= ~SUNXI_TFR_CTL_FBS ; + reg &= ~sspi_bits (sspi,SUNXI_TFR_CTL_FBS );/* * If it's a TX only transfer, we don't want to fill the RX * FIFO with bogus data */ if (sspi->rx_buf) - reg &= ~SUNXI_TFR_CTL_DHB ; + reg &= ~sspi_bits (sspi,SUNXI_TFR_CTL_DHB );else - reg |=SUNXI_TFR_CTL_DHB ; + reg |=sspi_bits (sspi,SUNXI_TFR_CTL_DHB );sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , reg);
@@ -282,14 +360,16 @@ static int sunxi_spi_transfer_one(struct spi_master *master,* Filling the FIFO fully causes timeout for some reason * at least on spi2 on A10s */ -sunxi_spi_fill_fifo (sspi,SUNXI_FIFO_DEPTH -1 ); +sunxi_spi_fill_fifo (sspi, sspi->fifo_depth -1 );/* Enable the interrupts */ -sunxi_spi_write (sspi,SUNXI_INT_CTL_REG ,SUNXI_INT_CTL_TC ); +sunxi_spi_write (sspi,SUNXI_INT_CTL_REG , +sspi_bits (sspi,SUNXI_INT_CTL_TC ));/* Start the transfer */ reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG ); -sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , reg |SUNXI_TFR_CTL_XCH ); +sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , + reg |sspi_bits (sspi,SUNXI_TFR_CTL_XCH )); tx_time =max (tfr->len *8 *2 / (tfr->speed_hz /1000 ),100U ); start = jiffies;
@@ -305,7 +385,7 @@ static int sunxi_spi_transfer_one(struct spi_master *master, goto out; } -sunxi_spi_drain_fifo (sspi,SUNXI_FIFO_DEPTH ); +sunxi_spi_drain_fifo (sspi, sspi->fifo_depth); out:sunxi_spi_write (sspi,SUNXI_INT_CTL_REG ,0 );
@@ -319,8 +399,9 @@ static irqreturn_t sunxi_spi_handler(int irq, void *dev_id)u32 status =sunxi_spi_read (sspi,SUNXI_INT_STA_REG );/* Transfer complete */ -if (status &SUNXI_INT_CTL_TC ) { -sunxi_spi_write (sspi,SUNXI_INT_STA_REG ,SUNXI_INT_CTL_TC ); +if (status &sspi_bits (sspi,SUNXI_INT_CTL_TC )) { +sunxi_spi_write (sspi,SUNXI_INT_STA_REG , +sspi_bits (sspi,SUNXI_INT_CTL_TC ));complete (&sspi->done);return IRQ_HANDLED ; }
@@ -347,7 +428,9 @@ static int sunxi_spi_runtime_resume(struct device *dev) }sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , -SUNXI_CTL_ENABLE |SUNXI_CTL_MASTER |SUNXI_CTL_TP ); +sspi_bits (sspi,SUNXI_CTL_ENABLE ) | +sspi_bits (sspi,SUNXI_CTL_MASTER ) | +sspi_bits (sspi,SUNXI_CTL_TP ));return 0 ;
@@ -408,6 +491,10 @@ static int sunxi_spi_probe(struct platform_device *pdev) } sspi->master = master; + sspi->fifo_depth =SUN4I_FIFO_DEPTH ; + sspi->type =SPI_SUN4I ; + sspi->regmap = &sun4i_regmap; + sspi->bitmap = &sun4i_bitmap; master->max_speed_hz =100 *1000 *1000 ; master->min_speed_hz =3 *1000 ; master->set_cs = sunxi_spi_set_cs;
@@ -473,7 +560,7 @@ static int sunxi_spi_remove(struct platform_device *pdev) }static const struct of_device_id sunxi_spi_match[] = { - { .compatible ="allwinner,sun4i-a10-spi" , }, + { .compatible =SUN4I_COMPATIBLE , }, {} };MODULE_DEVICE_TABLE (of, sunxi_spi_match);
drivers/spi/spi-sun6i.c +165 -78
385 lines not on this page · raw message
Signature · 1 line
2.8.1
PATCH v3 10/13 spi: sunxi: merge sun4i and sun6i SPI driver
In reply to Michal SuchanekThe drivers are very similar and share multiple flaws which needed separate fixes for both drivers.
--- drivers/spi/Kconfig | 8 +- drivers/spi/Makefile | 1 - drivers/spi/spi-sun4i.c | 156 +++++++++++-- drivers/spi/spi-sun6i.c | 598 ------------------------------------------------ 4 files changed, 143 insertions(+), 620 deletions(-) delete mode 100644 drivers/spi/spi-sun6i.c
drivers/spi/Kconfig +4 -4
@@ -575,17 +575,17 @@ config SPI_ST_SSC4 this option, support will be included for the SSC driven SPI. config SPI_SUN4I - tristate "Allwinner A10 SoCs SPI controller" + tristate "Allwinner A1X/A2X/A31 SoCs SPI controller" depends on ARCH_SUNXI || COMPILE_TEST + depends on RESET_CONTROLLER help SPI driver for Allwinner sun4i, sun5i and sun7i SoCs config SPI_SUN6I - tristate "Allwinner A31 SPI controller" + tristate "Allwinner A31 SPI controller (deprecated compatibility option)" depends on ARCH_SUNXI || COMPILE_TEST depends on RESET_CONTROLLER - help - This enables using the SPI controller on the Allwinner A31 SoCs. + select SPI_SUN4I config SPI_MXS tristate "Freescale MXS SPI controller"
drivers/spi/Makefile +0 -1
@@ -83,7 +83,6 @@ obj-$(CONFIG_SPI_SH_SCI) += spi-sh-sci.o obj-$(CONFIG_SPI_SIRF) += spi-sirf.o obj-$(CONFIG_SPI_ST_SSC4) += spi-st-ssc4.o obj-$(CONFIG_SPI_SUN4I) += spi-sun4i.o -obj-$(CONFIG_SPI_SUN6I) += spi-sun6i.o obj-$(CONFIG_SPI_TEGRA114) += spi-tegra114.o obj-$(CONFIG_SPI_TEGRA20_SFLASH) += spi-tegra20-sflash.o obj-$(CONFIG_SPI_TEGRA20_SLINK) += spi-tegra20-slink.o
drivers/spi/spi-sun4i.c +139 -17
245 lines not on this page · raw message
drivers/spi/spi-sun6i.c deleted +0 -598
598 lines not on this page · raw message
Signature · 1 line
2.8.1
Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver
Hi Michal,
Quoted 21 lines · Michal Suchanek
The drivers are very similar and share multiple flaws which needed separate fixes for both drivers.
Signed-off-by Michal Suchanek <[email protected]>--- drivers/spi/Kconfig | 8 +- drivers/spi/Makefile | 1 - drivers/spi/spi-sun4i.c | 156 +++++++++++-- drivers/spi/spi-sun6i.c | 598 ------------------------------------------------ 4 files changed, 143 insertions(+), 620 deletions(-) delete mode 100644 drivers/spi/spi-sun6i.cdrivers/spi/spi-sun4i.c +0
-1@@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master, reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG );/* Reset FIFOs */ - sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG,
drivers/spi/spi-sun4i.c static int sunxi_spi_transfer_one(struct spi_master *master, · line 331
Quoted 10 lines
drivers/spi/spi-sun4i.c +8
-2- reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | - sspi_bits(sspi, SUNXI_CTL_TF_RST)); +if (sspi->type ==SPI_SUN4I ) +sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , + reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST )); +else +sunxi_spi_write (sspi,SUNXI_FIFO_CTL_REG , +sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST ));
If we're already doing different stuff for each generation of the IP, why not just use the register offsets and bit definitions directly?
Quoted 14 lines
drivers/spi/spi-sun4i.c +6
-1/* * Setup the transfer control register: Chip Select, @@ -427,7 +473,19 @@ static int sunxi_spi_runtime_resume(struct device *dev) goto err; } - sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, +if (sspi->rstc) { + ret =reset_control_deassert (sspi->rstc); +if (ret) { +dev_err (dev,"Couldn't deassert the device from reset\n" ); + goto err2; + }
drivers/spi/spi-sun4i.c static int sunxi_spi_runtime_resume(struct device *dev) · line 491
Quoted 10 lines
drivers/spi/spi-sun4i.c +7
-0+ } + +if (sspi->type ==SPI_SUN4I ) + reg =SUNXI_TFR_CTL_REG ; +else + reg =SUNXI_GBL_CTL_REG ; +sunxi_spi_write (sspi, reg,sspi_bits (sspi,SUNXI_CTL_ENABLE ) |sspi_bits (sspi,SUNXI_CTL_MASTER ) |sspi_bits (sspi,SUNXI_CTL_TP ));
Same here.
Quoted 9 lines
drivers/spi/spi-sun4i.c +1
-4@@ -491,10 +558,37 @@ static int sunxi_spi_probe(struct platform_device *pdev) } sspi->master = master; - sspi->fifo_depth =SUN4I_FIFO_DEPTH ; - sspi->type =SPI_SUN4I ; - sspi->regmap = &sun4i_regmap; - sspi->bitmap = &sun4i_bitmap; +if (of_device_is_compatible (pdev->dev.of_node,SUN4I_COMPATIBLE )) {
drivers/spi/spi-sun4i.c static int sunxi_spi_probe(struct platform_device *pdev) · line 571
Quoted 10 lines
drivers/spi/spi-sun4i.c +10
-0+ sspi->fifo_depth =SUN4I_FIFO_DEPTH ; + sspi->type =SPI_SUN4I ; + sspi->regmap = &sun4i_regmap; + sspi->bitmap = &sun4i_bitmap; + }else if (of_device_is_compatible (pdev->dev.of_node, +SUN6I_COMPATIBLE )) { + sspi->fifo_depth =SUN6I_FIFO_DEPTH ; + sspi->type =SPI_SUN6I ; + sspi->regmap = &sun6i_regmap; + sspi->bitmap = &sun6i_bitmap;
Can you store data in the match table instead of doing this?
Quoted 13 lines
drivers/spi/spi-sun4i.c +13
-0+ }else { +const char *str =NULL ; +int i =1 ; + +of_property_read_string (pdev->dev.of_node,"compatible" , &str); +dev_err (&pdev->dev,"Unknown device compatible %s" , str); +/* is there no sane way to print a string array property ? */ +if (of_property_count_strings (pdev->dev.of_node,"compatible" ) + >1 ) { +while (!of_property_read_string_index (pdev->dev.of_node, +"compatible" , i, + &str)) { +pr_err (", %s" , str);
drivers/spi/spi-sun4i.c static int sunxi_spi_probe(struct platform_device *pdev) · line 594
Quoted 10 lines
drivers/spi/spi-sun4i.c +7
-0+ i++; + } + } + ret = -EINVAL ; + goto err_free_master; + } + master->max_speed_hz =100 *1000 *1000 ; master->min_speed_hz =3 *1000 ; master->set_cs = sunxi_spi_set_cs;
Thanks,
Signature · 4 lines
Julian Calaby Email: julian.calaby at gmail.com Profile: http://www.google.com/profiles/julian.calaby/
Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver
Hello,
Quoted 22 lines · Julian Calaby
Hi Michal,
Quoted 19 lines · Michal Suchanek
The drivers are very similar and share multiple flaws which needed separate fixes for both drivers.
Signed-off-by Michal Suchanek <[email protected]>--- drivers/spi/Kconfig | 8 +- drivers/spi/Makefile | 1 - drivers/spi/spi-sun4i.c | 156 +++++++++++-- drivers/spi/spi-sun6i.c | 598 ------------------------------------------------ 4 files changed, 143 insertions(+), 620 deletions(-) delete mode 100644 drivers/spi/spi-sun6i.cdrivers/spi/spi-sun4i.c +0
-0@@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master, reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG );
Quoted 15 lines
drivers/spi/spi-sun4i.c static int sunxi_spi_transfer_one(struct spi_master *master, · line 331
Quoted 12 lines
drivers/spi/spi-sun4i.c +8
-3/* Reset FIFOs */ -sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , - reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | -sspi_bits (sspi,SUNXI_CTL_TF_RST )); +if (sspi->type ==SPI_SUN4I ) +sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , + reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST )); +else +sunxi_spi_write (sspi,SUNXI_FIFO_CTL_REG , +sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST ));If we're already doing different stuff for each generation of the IP, why not just use the register offsets and bit definitions directly?
Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place makes my eyes bleed and you cannot use the check that you are accessing a register that actually exists.
Quoted 13 lines
Quoted 13 lines
drivers/spi/spi-sun4i.c +5
-4@@ -491,10 +558,37 @@ static int sunxi_spi_probe(struct platform_device *pdev) } sspi->master = master; - sspi->fifo_depth =SUN4I_FIFO_DEPTH ; - sspi->type =SPI_SUN4I ; - sspi->regmap = &sun4i_regmap; - sspi->bitmap = &sun4i_bitmap; +if (of_device_is_compatible (pdev->dev.of_node,SUN4I_COMPATIBLE )) { + sspi->fifo_depth =SUN4I_FIFO_DEPTH ; + sspi->type =SPI_SUN4I ; + sspi->regmap = &sun4i_regmap; + sspi->bitmap = &sun4i_bitmap;
Quoted 8 lines
drivers/spi/spi-sun4i.c static int sunxi_spi_probe(struct platform_device *pdev) · line 571
Quoted 6 lines
drivers/spi/spi-sun4i.c +6
-0+ }else if (of_device_is_compatible (pdev->dev.of_node, +SUN6I_COMPATIBLE )) { + sspi->fifo_depth =SUN6I_FIFO_DEPTH ; + sspi->type =SPI_SUN6I ; + sspi->regmap = &sun6i_regmap; + sspi->bitmap = &sun6i_bitmap;Can you store data in the match table instead of doing this?
That might be nicer. Will look into this.
Thanks
Michal
Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver
Hi Michal,
Quoted 41 lines · Michal Suchanek
Hello,
Quoted 22 lines · Julian Calaby
Hi Michal,
Quoted 19 lines · Michal Suchanek
The drivers are very similar and share multiple flaws which needed separate fixes for both drivers.
Signed-off-by Michal Suchanek <[email protected]>--- drivers/spi/Kconfig | 8 +- drivers/spi/Makefile | 1 - drivers/spi/spi-sun4i.c | 156 +++++++++++-- drivers/spi/spi-sun6i.c | 598 ------------------------------------------------ 4 files changed, 143 insertions(+), 620 deletions(-) delete mode 100644 drivers/spi/spi-sun6i.cdrivers/spi/spi-sun4i.c +0
-0@@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master, reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG );Quoted 15 lines
drivers/spi/spi-sun4i.c static int sunxi_spi_transfer_one(struct spi_master *master, · line 331
Quoted 12 lines
drivers/spi/spi-sun4i.c +8
-3/* Reset FIFOs */ -sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , - reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | -sspi_bits (sspi,SUNXI_CTL_TF_RST )); +if (sspi->type ==SPI_SUN4I ) +sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , + reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST )); +else +sunxi_spi_write (sspi,SUNXI_FIFO_CTL_REG , +sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST ));If we're already doing different stuff for each generation of the IP, why not just use the register offsets and bit definitions directly?
Quoted 3 lines
Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place makes my eyes bleed and you cannot use the check that you are accessing a register that actually exists.
I mean removing SUNXI_CTL_RF_RST and SUNXI_CTL_TF_RST from all of the indirection you added and using them directly, i.e.
#define SUN4I_TFR_CTL_RF_RST BIT(x)
#define SUN4I_TFR_CTL_TF_RST BIT(x)
#define SUN6I_FIFO_CTL_RF_RST BIT(x)
#define SUN6I_FIFO_CTL_TF_RST BIT(x)
then
if (sspi->type == SPI_SUN4I)
sunxi_spi_write(sspi, SUNXI_TFR_CTL_REG, reg |SUN4I_TFR_CTL_RF_RST | SUN4I_TFR_CTL_TF_RST); else
sunxi_spi_write(sspi, SUNXI_FIFO_CTL_REG, reg |SUN6I_FIFO_CTL_RF_RST | SUN6I_FIFO_CTL_TF_RST);
I.e. the bits that need setting are in different registers, so you have to have an if statement and separate calls. Therefore there's no real benefit from the indirection you've introduced here, unless you're expecting the SUN8I variant to use different bits in one of those two registers.
Thanks,
Signature · 4 lines
Julian Calaby Email: julian.calaby at gmail.com Profile: http://www.google.com/profiles/julian.calaby/
Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver
Quoted 65 lines · Julian Calaby
Hi Michal,
Quoted 41 lines · Michal Suchanek
Hello,
Quoted 22 lines · Julian Calaby
Hi Michal,
Quoted 19 lines · Michal Suchanek
The drivers are very similar and share multiple flaws which needed separate fixes for both drivers.
Signed-off-by Michal Suchanek <[email protected]>--- drivers/spi/Kconfig | 8 +- drivers/spi/Makefile | 1 - drivers/spi/spi-sun4i.c | 156 +++++++++++-- drivers/spi/spi-sun6i.c | 598 ------------------------------------------------ 4 files changed, 143 insertions(+), 620 deletions(-) delete mode 100644 drivers/spi/spi-sun6i.cdrivers/spi/spi-sun4i.c +0
-0@@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master, reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG );Quoted 15 lines
drivers/spi/spi-sun4i.c static int sunxi_spi_transfer_one(struct spi_master *master, · line 331
Quoted 12 lines
drivers/spi/spi-sun4i.c +8
-3/* Reset FIFOs */ -sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , - reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | -sspi_bits (sspi,SUNXI_CTL_TF_RST )); +if (sspi->type ==SPI_SUN4I ) +sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , + reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST )); +else +sunxi_spi_write (sspi,SUNXI_FIFO_CTL_REG , +sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST ));If we're already doing different stuff for each generation of the IP, why not just use the register offsets and bit definitions directly?
Quoted 3 lines
Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place makes my eyes bleed and you cannot use the check that you are accessing a register that actually exists.
I mean removing
SUNXI_CTL_RF_RSTandSUNXI_CTL_TF_RSTfrom all of the indirection you added and using them directly, i.e.#define
SUN4I_TFR_CTL_RF_RSTBIT(x)
#defineSUN4I_TFR_CTL_TF_RSTBIT(x)
#defineSUN6I_FIFO_CTL_RF_RSTBIT(x)
#defineSUN6I_FIFO_CTL_TF_RSTBIT(x)then
if (
sspi->type==SPI_SUN4I)
sunxi_spi_write(sspi,SUNXI_TFR_CTL_REG, reg |SUN4I_TFR_CTL_RF_RST|SUN4I_TFR_CTL_TF_RST); else
sunxi_spi_write(sspi,SUNXI_FIFO_CTL_REG, reg |SUN6I_FIFO_CTL_RF_RST|SUN6I_FIFO_CTL_TF_RST);
Quoted 6 lines
I.e. the bits that need setting are in different registers, so you have to have an if statement and separate calls. Therefore there's no real benefit from the indirection you've introduced here, unless you're expecting the SUN8I variant to use different bits in one of those two registers.
That looks nice for this particular case.
Still you will have to remember that these bits are specified directly while other bits on the register are mapped which is not so nice.
Thanks
Michal
Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver
Hi Michal,
Quoted 66 lines · Michal Suchanek
Quoted 65 lines · Julian Calaby
Hi Michal,
Quoted 41 lines · Michal Suchanek
Hello,
Quoted 22 lines · Julian Calaby
Hi Michal,
Quoted 19 lines · Michal Suchanek
The drivers are very similar and share multiple flaws which needed separate fixes for both drivers.
Signed-off-by Michal Suchanek <[email protected]>--- drivers/spi/Kconfig | 8 +- drivers/spi/Makefile | 1 - drivers/spi/spi-sun4i.c | 156 +++++++++++-- drivers/spi/spi-sun6i.c | 598 ------------------------------------------------ 4 files changed, 143 insertions(+), 620 deletions(-) delete mode 100644 drivers/spi/spi-sun6i.cdrivers/spi/spi-sun4i.c +0
-0@@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master, reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG );Quoted 15 lines
drivers/spi/spi-sun4i.c static int sunxi_spi_transfer_one(struct spi_master *master, · line 331
Quoted 12 lines
drivers/spi/spi-sun4i.c +8
-3/* Reset FIFOs */ -sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , - reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | -sspi_bits (sspi,SUNXI_CTL_TF_RST )); +if (sspi->type ==SPI_SUN4I ) +sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , + reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST )); +else +sunxi_spi_write (sspi,SUNXI_FIFO_CTL_REG , +sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST ));If we're already doing different stuff for each generation of the IP, why not just use the register offsets and bit definitions directly?
Quoted 3 lines
Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place makes my eyes bleed and you cannot use the check that you are accessing a register that actually exists.
I mean removing
SUNXI_CTL_RF_RSTandSUNXI_CTL_TF_RSTfrom all of the indirection you added and using them directly, i.e.#define
SUN4I_TFR_CTL_RF_RSTBIT(x)
#defineSUN4I_TFR_CTL_TF_RSTBIT(x)
#defineSUN6I_FIFO_CTL_RF_RSTBIT(x)
#defineSUN6I_FIFO_CTL_TF_RSTBIT(x)then
if (
sspi->type==SPI_SUN4I)
sunxi_spi_write(sspi,SUNXI_TFR_CTL_REG, reg |SUN4I_TFR_CTL_RF_RST|SUN4I_TFR_CTL_TF_RST); else
sunxi_spi_write(sspi,SUNXI_FIFO_CTL_REG, reg |SUN6I_FIFO_CTL_RF_RST|SUN6I_FIFO_CTL_TF_RST);
Quoted 8 lines
Quoted 6 lines
I.e. the bits that need setting are in different registers, so you have to have an if statement and separate calls. Therefore there's no real benefit from the indirection you've introduced here, unless you're expecting the SUN8I variant to use different bits in one of those two registers.
That looks nice for this particular case.
There was another case I pointed out in this driver that could potentially benefit from this.
Quoted 2 lines
Still you will have to remember that these bits are specified directly while other bits on the register are mapped which is not so nice.
True. I'm not sure which path is best in this case. To my eye, your indirection scheme seems like the "heavy" solution, however I have no idea what form a "lighter" solution would take.
Other drivers I've seen which have tackled similar problems have used a large struct to hold all the variant-specific stuff, whether that's function pointers, register offsets, constants, etc. (so this code could theoretically be re-written as sunxi_spi_write(sspi, sspi->fifo_reg, reg | sspi->fifo_reset_arg) or sspi->reset_fifo()) however the driver I saw doing this (rtl8xxxu) was handling much more significant differences between device variants than you are.
Thanks,
Signature · 4 lines
Julian Calaby Email: julian.calaby at gmail.com Profile: http://www.google.com/profiles/julian.calaby/
Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver
Hello,
Quoted 88 lines · Julian Calaby
Hi Michal,
Quoted 66 lines · Michal Suchanek
Quoted 65 lines · Julian Calaby
Hi Michal,
Quoted 41 lines · Michal Suchanek
Hello,
Quoted 22 lines · Julian Calaby
Hi Michal,
Quoted 19 lines · Michal Suchanek
The drivers are very similar and share multiple flaws which needed separate fixes for both drivers.
Signed-off-by Michal Suchanek <[email protected]>--- drivers/spi/Kconfig | 8 +- drivers/spi/Makefile | 1 - drivers/spi/spi-sun4i.c | 156 +++++++++++-- drivers/spi/spi-sun6i.c | 598 ------------------------------------------------ 4 files changed, 143 insertions(+), 620 deletions(-) delete mode 100644 drivers/spi/spi-sun6i.cdrivers/spi/spi-sun4i.c +0
-0@@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master, reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG );Quoted 15 lines
drivers/spi/spi-sun4i.c static int sunxi_spi_transfer_one(struct spi_master *master, · line 331
Quoted 12 lines
drivers/spi/spi-sun4i.c +8
-3/* Reset FIFOs */ -sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , - reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | -sspi_bits (sspi,SUNXI_CTL_TF_RST )); +if (sspi->type ==SPI_SUN4I ) +sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , + reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST )); +else +sunxi_spi_write (sspi,SUNXI_FIFO_CTL_REG , +sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST ));If we're already doing different stuff for each generation of the IP, why not just use the register offsets and bit definitions directly?
Quoted 3 lines
Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place makes my eyes bleed and you cannot use the check that you are accessing a register that actually exists.
I mean removing
SUNXI_CTL_RF_RSTandSUNXI_CTL_TF_RSTfrom all of the indirection you added and using them directly, i.e.#define
SUN4I_TFR_CTL_RF_RSTBIT(x)
#defineSUN4I_TFR_CTL_TF_RSTBIT(x)
#defineSUN6I_FIFO_CTL_RF_RSTBIT(x)
#defineSUN6I_FIFO_CTL_TF_RSTBIT(x)then
if (
sspi->type==SPI_SUN4I)
sunxi_spi_write(sspi,SUNXI_TFR_CTL_REG, reg |SUN4I_TFR_CTL_RF_RST|SUN4I_TFR_CTL_TF_RST); else
sunxi_spi_write(sspi,SUNXI_FIFO_CTL_REG, reg |SUN6I_FIFO_CTL_RF_RST|SUN6I_FIFO_CTL_TF_RST);Quoted 8 lines
Quoted 6 lines
I.e. the bits that need setting are in different registers, so you have to have an if statement and separate calls. Therefore there's no real benefit from the indirection you've introduced here, unless you're expecting the SUN8I variant to use different bits in one of those two registers.
That looks nice for this particular case.
There was another case I pointed out in this driver that could potentially benefit from this.
Quoted 2 lines
Still you will have to remember that these bits are specified directly while other bits on the register are mapped which is not so nice.
True. I'm not sure which path is best in this case. To my eye, your indirection scheme seems like the "heavy" solution, however I have no idea what form a "lighter" solution would take.
Quoted 5 lines
Other drivers I've seen which have tackled similar problems have used a large struct to hold all the variant-specific stuff, whether that's function pointers, register offsets, constants, etc. (so this code could theoretically be re-written as
sunxi_spi_write(sspi, sspi->fifo_reg, reg | sspi->fifo_reset_arg)
This won't do. There is fifo_reg and tfr_reg on both IPs but some bits from one migrated to the other.
Quoted 1 line
or
sspi->reset_fifo())
This would work but would require more thorough rewrite and heavier adaptation layer.
As it is where different register is used in different IP revision it is specified explicitly in the code while register names to numbers are mapped in read() and write(). Value bits are mapped by explicitly calling a function on the name.
This gives nice 1:1 mapping to the old code and allows checking that both the register and the bit value in question exist on the IP.
Matching the value to register relies on the driver code, however.
Thanks
Michal
Re: [linux-sunxi] [PATCH v3 10/13] spi: sunxi: merge sun4i and sun6i SPI driver
Hi Michal,
Quoted 105 lines · Michal Suchanek
Hello,
Quoted 88 lines · Julian Calaby
Hi Michal,
Quoted 66 lines · Michal Suchanek
Quoted 65 lines · Julian Calaby
Hi Michal,
Quoted 41 lines · Michal Suchanek
Hello,
Quoted 22 lines · Julian Calaby
Hi Michal,
Quoted 19 lines · Michal Suchanek
The drivers are very similar and share multiple flaws which needed separate fixes for both drivers.
Signed-off-by Michal Suchanek <[email protected]>--- drivers/spi/Kconfig | 8 +- drivers/spi/Makefile | 1 - drivers/spi/spi-sun4i.c | 156 +++++++++++-- drivers/spi/spi-sun6i.c | 598 ------------------------------------------------ 4 files changed, 143 insertions(+), 620 deletions(-) delete mode 100644 drivers/spi/spi-sun6i.cdrivers/spi/spi-sun4i.c +0
-0@@ -279,9 +321,14 @@ static int sunxi_spi_transfer_one(struct spi_master *master, reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG );Quoted 15 lines
drivers/spi/spi-sun4i.c static int sunxi_spi_transfer_one(struct spi_master *master, · line 331
Quoted 12 lines
drivers/spi/spi-sun4i.c +8
-3/* Reset FIFOs */ -sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , - reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | -sspi_bits (sspi,SUNXI_CTL_TF_RST )); +if (sspi->type ==SPI_SUN4I ) +sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , + reg |sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST )); +else +sunxi_spi_write (sspi,SUNXI_FIFO_CTL_REG , +sspi_bits (sspi,SUNXI_CTL_RF_RST ) | +sspi_bits (sspi,SUNXI_CTL_TF_RST ));If we're already doing different stuff for each generation of the IP, why not just use the register offsets and bit definitions directly?
Quoted 3 lines
Because having (*sspi->regmap)[SUNXI_FIFO_CTL_REG] all over the place makes my eyes bleed and you cannot use the check that you are accessing a register that actually exists.
I mean removing
SUNXI_CTL_RF_RSTandSUNXI_CTL_TF_RSTfrom all of the indirection you added and using them directly, i.e.#define
SUN4I_TFR_CTL_RF_RSTBIT(x)
#defineSUN4I_TFR_CTL_TF_RSTBIT(x)
#defineSUN6I_FIFO_CTL_RF_RSTBIT(x)
#defineSUN6I_FIFO_CTL_TF_RSTBIT(x)then
if (
sspi->type==SPI_SUN4I)
sunxi_spi_write(sspi,SUNXI_TFR_CTL_REG, reg |SUN4I_TFR_CTL_RF_RST|SUN4I_TFR_CTL_TF_RST); else
sunxi_spi_write(sspi,SUNXI_FIFO_CTL_REG, reg |SUN6I_FIFO_CTL_RF_RST|SUN6I_FIFO_CTL_TF_RST);Quoted 8 lines
Quoted 6 lines
I.e. the bits that need setting are in different registers, so you have to have an if statement and separate calls. Therefore there's no real benefit from the indirection you've introduced here, unless you're expecting the SUN8I variant to use different bits in one of those two registers.
That looks nice for this particular case.
There was another case I pointed out in this driver that could potentially benefit from this.
Quoted 2 lines
Still you will have to remember that these bits are specified directly while other bits on the register are mapped which is not so nice.
True. I'm not sure which path is best in this case. To my eye, your indirection scheme seems like the "heavy" solution, however I have no idea what form a "lighter" solution would take.
Quoted 5 lines
Other drivers I've seen which have tackled similar problems have used a large struct to hold all the variant-specific stuff, whether that's function pointers, register offsets, constants, etc. (so this code could theoretically be re-written as
sunxi_spi_write(sspi, sspi->fifo_reg, reg | sspi->fifo_reset_arg)This won't do. There is
fifo_regandtfr_regon both IPs but some bits from one migrated to the other.Quoted 1 line
or
sspi->reset_fifo())This would work but would require more thorough rewrite and heavier adaptation layer.
Quoted 7 lines
As it is where different register is used in different IP revision it is specified explicitly in the code while register names to numbers are mapped in
read()andwrite(). Value bits are mapped by explicitly calling a function on the name.This gives nice 1:1 mapping to the old code and allows checking that both the register and the bit value in question exist on the IP.
And your method is almost beautiful from that perspective, however I still feel that it's too "heavy". That said, neither of my suggestions are much better. I provided them in the hope that they might be illuminating.
Quoted 1 line
Matching the value to register relies on the driver code, however.
Of course.
I'm not sure what the state-of-the-art for dealing with variants of devices where the manufacturer keeps scrambling the registers is, however I feel we can do better, though I'm not sure how.
Thanks,
Signature · 4 lines
Julian Calaby Email: julian.calaby at gmail.com Profile: http://www.google.com/profiles/julian.calaby/
PATCH v3 11/13 dt: spi: sun4i: merge sun4i and sun6i binding doc
In reply to Michal Suchanek--- .../devicetree/bindings/spi/spi-sun4i.txt | 21 ++++++++++--------- .../devicetree/bindings/spi/spi-sun6i.txt | 24 ---------------------- 2 files changed, 11 insertions(+), 34 deletions(-) delete mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt
Documentation/devicetree/bindings/spi/spi-sun4i.txt +11 -10
@@ -1,7 +1,8 @@ -Allwinner A10 SPI controller +Allwinner A10/A31 SPI controller Required properties: -- compatible: Should be "allwinner,sun4-a10-spi". +- compatible: Should be one of "allwinner,sun4i-a10-spi" and + "allwinner,sun6i-a31-spi" - reg: Should contain register location and length. - interrupts: Should contain interrupt. - clocks: phandle to the clocks feeding the SPI controller. Two are
@@ -9,16 +10,16 @@ Required properties: - "ahb": the gated AHB parent clock - "mod": the parent module clock - clock-names: Must contain the clock names described just above +- resets: (sun6i only) phandle to the reset controller asserting + this device in reset Example: -spi1: spi at 01c06000 { - compatible = "allwinner,sun4i-a10-spi"; - reg = <0x01c06000 0x1000>; - interrupts = <11>; - clocks = <&ahb_gates 21>, <&spi1_clk>; +spi1: spi at 01c69000 { + compatible = "allwinner,sun6i-a31-spi"; + reg = <0x01c69000 0x1000>; + interrupts = <0 66 4>; + clocks = <&ahb1_gates 21>, <&spi1_clk>; clock-names = "ahb", "mod"; - status = "disabled"; - #address-cells = <1>; - #size-cells = <0>; + resets = <&ahb1_rst 21>; };
Documentation/devicetree/bindings/spi/spi-sun6i.txt deleted +0 -24
@@ -1,24 +0,0 @@ -Allwinner A31 SPI controller - -Required properties: -- compatible: Should be "allwinner,sun6i-a31-spi". -- reg: Should contain register location and length. -- interrupts: Should contain interrupt. -- clocks: phandle to the clocks feeding the SPI controller. Two are - needed: - - "ahb": the gated AHB parent clock - - "mod": the parent module clock -- clock-names: Must contain the clock names described just above -- resets: phandle to the reset controller asserting this device in - reset - -Example: - -spi1: spi at 01c69000 { - compatible = "allwinner,sun6i-a31-spi"; - reg = <0x01c69000 0x1000>; - interrupts = <0 66 4>; - clocks = <&ahb1_gates 21>, <&spi1_clk>; - clock-names = "ahb", "mod"; - resets = <&ahb1_rst 21>; -};
Signature · 1 line
2.8.1
Re: [linux-sunxi] [PATCH v3 11/13] dt: spi: sun4i: merge sun4i and sun6i binding doc
Hi Michal,
Quoted 36 lines · Michal Suchanek
Signed-off-by Michal Suchanek <[email protected]>--- .../devicetree/bindings/spi/spi-sun4i.txt | 21 ++++++++++--------- .../devicetree/bindings/spi/spi-sun6i.txt | 24 ---------------------- 2 files changed, 11 insertions(+), 34 deletions(-) delete mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txtDocumentation/devicetree/bindings/spi/spi-sun4i.txt +5
-7@@ -1,7 +1,8 @@ -Allwinner A10 SPI controller +Allwinner A10/A31 SPI controller Required properties: -- compatible: Should be "allwinner,sun4-a10-spi". +- compatible: Should be one of "allwinner,sun4i-a10-spi" and + "allwinner,sun6i-a31-spi" - reg: Should contain register location and length. - interrupts: Should contain interrupt. - clocks: phandle to the clocks feeding the SPI controller. Two are@@ -9,16 +10,16 @@ Required properties: - "ahb": the gated AHB parent clock - "mod": the parent module clock - clock-names: Must contain the clock names described just above +- resets: (sun6i only) phandle to the reset controller asserting + this device in reset Example: -spi1: spi at 01c06000 { - compatible = "allwinner,sun4i-a10-spi"; - reg = <0x01c06000 0x1000>; - interrupts = <11>; - clocks = <&ahb_gates 21>, <&spi1_clk>;
Documentation/devicetree/bindings/spi/spi-sun4i.txt Required properties: · line 24
Quoted 10 lines
Documentation/devicetree/bindings/spi/spi-sun4i.txt +6
-3+spi1: spi at 01c69000 { + compatible = "allwinner,sun6i-a31-spi"; + reg = <0x01c69000 0x1000>; + interrupts = <0 66 4>; + clocks = <&ahb1_gates 21>, <&spi1_clk>; clock-names = "ahb", "mod"; - status = "disabled"; - #address-cells = <1>; - #size-cells = <0>; + resets = <&ahb1_rst 21>;
Why not have an example of each type?
Documentation/devicetree/bindings/spi/spi-sun4i.txt Required properties: · line 25
Quoted 1 line
Documentation/devicetree/bindings/spi/spi-sun4i.txt +0
-0};
Thanks,
Signature · 4 lines
Julian Calaby Email: julian.calaby at gmail.com Profile: http://www.google.com/profiles/julian.calaby/
Re: [linux-sunxi] [PATCH v3 11/13] dt: spi: sun4i: merge sun4i and sun6i binding doc
Quoted 43 lines · Julian Calaby
Hi Michal,
Quoted 40 lines · Michal Suchanek
Signed-off-by Michal Suchanek <[email protected]>--- .../devicetree/bindings/spi/spi-sun4i.txt | 21 ++++++++++--------- .../devicetree/bindings/spi/spi-sun6i.txt | 24 ---------------------- 2 files changed, 11 insertions(+), 34 deletions(-) delete mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txtDocumentation/devicetree/bindings/spi/spi-sun4i.txt +9
-7@@ -1,7 +1,8 @@ -Allwinner A10 SPI controller +Allwinner A10/A31 SPI controller Required properties: -- compatible: Should be "allwinner,sun4-a10-spi". +- compatible: Should be one of "allwinner,sun4i-a10-spi" and + "allwinner,sun6i-a31-spi" - reg: Should contain register location and length. - interrupts: Should contain interrupt. - clocks: phandle to the clocks feeding the SPI controller. Two are@@ -9,16 +10,16 @@ Required properties: - "ahb": the gated AHB parent clock - "mod": the parent module clock - clock-names: Must contain the clock names described just above +- resets: (sun6i only) phandle to the reset controller asserting + this device in reset Example: -spi1: spi at 01c06000 { - compatible = "allwinner,sun4i-a10-spi"; - reg = <0x01c06000 0x1000>; - interrupts = <11>; - clocks = <&ahb_gates 21>, <&spi1_clk>; +spi1: spi at 01c69000 { + compatible = "allwinner,sun6i-a31-spi"; + reg = <0x01c69000 0x1000>; + interrupts = <0 66 4>;
Quoted 8 lines
Documentation/devicetree/bindings/spi/spi-sun4i.txt Required properties: · line 24
Quoted 6 lines
Documentation/devicetree/bindings/spi/spi-sun4i.txt +2
-3+ clocks = <&ahb1_gates 21>, <&spi1_clk>; clock-names = "ahb", "mod"; - status = "disabled"; - #address-cells = <1>; - #size-cells = <0>; + resets = <&ahb1_rst 21>;Why not have an example of each type?
How many binding docs have examples of all types?
There are actual DTs using these so you can look at those as well.
This driver covers 3 types of bindings which look different in the DT:
sun4i IP with some Chinese interrupt controller, sun4i IP with GIC, and sun6i IP with GIC.
Thanks
Michal
Re: [linux-sunxi] [PATCH v3 11/13] dt: spi: sun4i: merge sun4i and sun6i binding doc
Hi Michal,
Quoted 53 lines · Michal Suchanek
Quoted 43 lines · Julian Calaby
Hi Michal,
Quoted 40 lines · Michal Suchanek
Signed-off-by Michal Suchanek <[email protected]>--- .../devicetree/bindings/spi/spi-sun4i.txt | 21 ++++++++++--------- .../devicetree/bindings/spi/spi-sun6i.txt | 24 ---------------------- 2 files changed, 11 insertions(+), 34 deletions(-) delete mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txtDocumentation/devicetree/bindings/spi/spi-sun4i.txt +9
-7@@ -1,7 +1,8 @@ -Allwinner A10 SPI controller +Allwinner A10/A31 SPI controller Required properties: -- compatible: Should be "allwinner,sun4-a10-spi". +- compatible: Should be one of "allwinner,sun4i-a10-spi" and + "allwinner,sun6i-a31-spi" - reg: Should contain register location and length. - interrupts: Should contain interrupt. - clocks: phandle to the clocks feeding the SPI controller. Two are@@ -9,16 +10,16 @@ Required properties: - "ahb": the gated AHB parent clock - "mod": the parent module clock - clock-names: Must contain the clock names described just above +- resets: (sun6i only) phandle to the reset controller asserting + this device in reset Example: -spi1: spi at 01c06000 { - compatible = "allwinner,sun4i-a10-spi"; - reg = <0x01c06000 0x1000>; - interrupts = <11>; - clocks = <&ahb_gates 21>, <&spi1_clk>; +spi1: spi at 01c69000 { + compatible = "allwinner,sun6i-a31-spi"; + reg = <0x01c69000 0x1000>; + interrupts = <0 66 4>;Quoted 8 lines
Documentation/devicetree/bindings/spi/spi-sun4i.txt Required properties: · line 24
Quoted 6 lines
Documentation/devicetree/bindings/spi/spi-sun4i.txt +2
-3+ clocks = <&ahb1_gates 21>, <&spi1_clk>; clock-names = "ahb", "mod"; - status = "disabled"; - #address-cells = <1>; - #size-cells = <0>; + resets = <&ahb1_rst 21>;Why not have an example of each type?
Quoted 1 line
How many binding docs have examples of all types?
I'm pretty sure that there's a few. This was only a suggestion, so if it's not to your taste, ignore it.
Quoted 6 lines
There are actual DTs using these so you can look at those as well.
This driver covers 3 types of bindings which look different in the DT:
sun4i IP with some Chinese interrupt controller, sun4i IP with GIC, and sun6i IP with GIC.
Fair point.
Thanks,
Signature · 4 lines
Julian Calaby Email: julian.calaby at gmail.com Profile: http://www.google.com/profiles/julian.calaby/
PATCH v3 12/13 spi: sunxi: remove CONFIG_SPI_SUN6I
In reply to Michal SuchanekThis is no longer used.
--- arch/arm/configs/multi_v7_defconfig | 1 - arch/arm/configs/sunxi_defconfig | 1 - drivers/spi/Kconfig | 6 ------ 3 files changed, 8 deletions(-)
arch/arm/configs/multi_v7_defconfig +0 -1
@@ -362,7 +362,6 @@ CONFIG_SPI_SH_MSIOF=m CONFIG_SPI_SH_HSPI=y CONFIG_SPI_SIRF=y CONFIG_SPI_SUN4I=y -CONFIG_SPI_SUN6I=y CONFIG_SPI_TEGRA114=y CONFIG_SPI_TEGRA20_SFLASH=y CONFIG_SPI_TEGRA20_SLINK=y
arch/arm/configs/sunxi_defconfig +0 -1
@@ -78,7 +78,6 @@ CONFIG_I2C_MV64XXX=y CONFIG_I2C_SUN6I_P2WI=y CONFIG_SPI=y CONFIG_SPI_SUN4I=y -CONFIG_SPI_SUN6I=y CONFIG_GPIO_SYSFS=y CONFIG_POWER_SUPPLY=y CONFIG_AXP20X_POWER=y
drivers/spi/Kconfig +0 -6
@@ -581,12 +581,6 @@ config SPI_SUN4I help SPI driver for Allwinner sun4i, sun5i and sun7i SoCs -config SPI_SUN6I - tristate "Allwinner A31 SPI controller (deprecated compatibility option)" - depends on ARCH_SUNXI || COMPILE_TEST - depends on RESET_CONTROLLER - select SPI_SUN4I - config SPI_MXS tristate "Freescale MXS SPI controller" depends on ARCH_MXS
Signature · 1 line
2.8.1
PATCH v3 13/13 spi: sun4i: add DMA support
In reply to Michal SuchanekFrom: Emilio L?pez <[email protected]>
This patch adds support for 64 byte or bigger transfers on the sun4i SPI controller. Said transfers will be performed via DMA.
---
v2:
- fallback to previous behaviour when DMA initialization fails
v3:
- adjust to merged driver
- add bit set/unset helpers
- add wait_for_dma (default=1) so driver does not randomly load without dma
- use SUNXI_CNT_MASK as transfer size limit
--- drivers/spi/spi-sun4i.c | 247 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 230 insertions(+), 17 deletions(-)
drivers/spi/spi-sun4i.c +230 -17
@@ -14,6 +14,8 @@#include <linux/clk.h> #include <linux/delay.h> #include <linux/device.h> +#include <linux/dmaengine.h> +#include <linux/dma-mapping.h> #include <linux/interrupt.h> #include <linux/io.h> #include <linux/module.h>
@@ -50,6 +52,12 @@#define SUNXI_FIFO_STA_TF_CNT_MASK 0x7f#define SUNXI_FIFO_STA_TF_CNT_BITS 16 +static int wait_for_dma =1 ; +module_param (wait_for_dma, int,0644 ); +MODULE_PARM_DESC (wait_for_dma, +"When acquiring a DMA channel returns EDEFER return and let kernel defer spi master probe.\n" +"Non-DMA operation is used otherwise (defaults to wait for DMA driver to load)." ); +enum SPI_SUNXI_TYPE {SPI_SUN4I =1 ,SPI_SUN6I ,
@@ -61,6 +69,7 @@ enum SUNXI_REG_ENUM {SUNXI_TFR_CTL_REG ,SUNXI_INT_CTL_REG ,SUNXI_INT_STA_REG , +SUNXI_DMA_CTL_REG ,SUNXI_WAIT_REG ,SUNXI_CLK_CTL_REG , SUNXI_BURST_CNT_REG,
@@ -79,6 +88,7 @@ static int sun4i_regmap[SUNXI_NUM_REGS] = {/* SUNXI_TFR_CTL_REG */ 0x08 ,/* SUNXI_INT_CTL_REG */ 0x0c ,/* SUNXI_INT_STA_REG */ 0x10 , +/* SUNXI_DMA_CTL_REG */ 0x14 ,/* SUNXI_WAIT_REG */ 0x18 ,/* SUNXI_CLK_CTL_REG */ 0x1c , /* SUNXI_BURST_CNT_REG */ 0x20,
@@ -93,6 +103,7 @@ static int sun6i_regmap[SUNXI_NUM_REGS] = {/* SUNXI_TFR_CTL_REG */ 0x08 ,/* SUNXI_INT_CTL_REG */ 0x10 ,/* SUNXI_INT_STA_REG */ 0x14 , +/* SUNXI_DMA_CTL_REG */ -1 ,/* SUNXI_WAIT_REG */ 0x20 ,/* SUNXI_CLK_CTL_REG */ 0x24 , /* SUNXI_BURST_CNT_REG */ 0x30,
@@ -110,6 +121,7 @@ enum SUNXI_BITMAP_ENUM {SUNXI_TFR_CTL_CPHA ,SUNXI_TFR_CTL_CPOL ,SUNXI_TFR_CTL_CS_ACTIVE_LOW , +SUNXI_CTL_DMA_DEDICATED ,SUNXI_TFR_CTL_FBS ,SUNXI_CTL_TF_RST , SUNXI_CTL_RF_RST,
@@ -121,6 +133,9 @@ enum SUNXI_BITMAP_ENUM {SUNXI_TFR_CTL_CS_LEVEL ,SUNXI_CTL_TP ,SUNXI_INT_CTL_TC , +SUNXI_CTL_DMA_RF_READY , +SUNXI_CTL_DMA_TF_NOT_FULL , +SUNXI_CTL_DMA_TF_HALF , SUNXI_BITMAP_SIZE };
@@ -130,6 +145,7 @@ static int sun4i_bitmap[SUNXI_BITMAP_SIZE] = {/* SUNXI_TFR_CTL_CPHA */ BIT (2 ),/* SUNXI_TFR_CTL_CPOL */ BIT (3 ),/* SUNXI_TFR_CTL_CS_ACTIVE_LOW */ BIT (4 ), +/* SUNXI_CTL_DMA_DEDICATED */ BIT (5 ),/* SUNXI_TFR_CTL_FBS */ BIT (6 ),/* SUNXI_CTL_TF_RST */ BIT (8 ), /* SUNXI_CTL_RF_RST */ BIT(9),
@@ -141,6 +157,9 @@ static int sun4i_bitmap[SUNXI_BITMAP_SIZE] = {/* SUNXI_TFR_CTL_CS_LEVEL */ BIT (17 ),/* SUNXI_CTL_TP */ BIT (18 ),/* SUNXI_INT_CTL_TC */ BIT (16 ), +/* SUNXI_CTL_DMA_RF_READY */ BIT (0 ), +/* SUNXI_CTL_DMA_TF_NOT_FULL */ BIT (10 ), +/* SUNXI_CTL_DMA_TF_HALF */ BIT(9), }; static int sun6i_bitmap[SUNXI_BITMAP_SIZE] = {
@@ -149,6 +168,12 @@ static int sun6i_bitmap[SUNXI_BITMAP_SIZE] = {/* SUNXI_TFR_CTL_CPHA */ BIT (0 ),/* SUNXI_TFR_CTL_CPOL */ BIT (1 ),/* SUNXI_TFR_CTL_CS_ACTIVE_LOW */ BIT (2 ), +/* +* Bit 9 is listed as dedicated dma control for rx. +* There is no dedicated dma control bit listed for tx and bit 25 +* on the logical position is listed as unused. +*/ +/* SUNXI_CTL_DMA_DEDICATED */ BIT (9 )|BIT (25 ),/* SUNXI_TFR_CTL_FBS */ BIT (12 ),/* SUNXI_CTL_TF_RST */ BIT (31 ), /* SUNXI_CTL_RF_RST */ BIT(15),
@@ -160,6 +185,15 @@ static int sun6i_bitmap[SUNXI_BITMAP_SIZE] = {/* SUNXI_TFR_CTL_CS_LEVEL */ BIT (7 ),/* SUNXI_CTL_TP */ BIT (7 ),/* SUNXI_INT_CTL_TC */ BIT (12 ), +/* +* On sun4i there are separate bits enabling request on different fifo levels. +* On sun6i there is a level field and enable bit which enables request on that +* FIFO level. Only one level is ever used so just pack the relevant bits as +* one constant. +*/ +/* SUNXI_CTL_DMA_RF_READY */ BIT (0 )|BIT (8 ), +/* SUNXI_CTL_DMA_TF_NOT_FULL */ (0x7f <<16 )|BIT (24 ), +/* SUNXI_CTL_DMA_TF_HALF */ BIT(23)|BIT(24), }; struct sunxi_spi {
@@ -207,6 +241,20 @@ static inline u32 sspi_bits(struct sunxi_spi *sspi,return (*sspi->bitmap)[name]; } +static inline void sunxi_spi_set (struct sunxi_spi *sspi,u32 reg,u32 value) +{ +u32 orig =sunxi_spi_read (sspi, reg); + +sunxi_spi_write (sspi, reg, orig | value); +} + +static inline void sunxi_spi_unset (struct sunxi_spi *sspi,u32 reg,u32 value) +{ +u32 orig =sunxi_spi_read (sspi, reg); + +sunxi_spi_write (sspi, reg, orig & ~value); +} +static inline void sunxi_spi_drain_fifo (struct sunxi_spi *sspi,int len) {u32 reg, cnt;
@@ -243,6 +291,15 @@ static inline void sunxi_spi_fill_fifo(struct sunxi_spi *sspi, int len) } } +static bool sunxi_spi_can_dma (struct spi_master *master, +struct spi_device *spi, +struct spi_transfer *tfr) +{ +struct sunxi_spi *sspi =spi_master_get_devdata (master); + +return tfr->len >= sspi->fifo_depth; +} +static void sunxi_spi_set_cs (struct spi_device *spi,bool enable) {struct sunxi_spi *sspi =spi_master_get_devdata (spi->master);
@@ -284,6 +341,8 @@ static size_t sunxi_spi_max_transfer_size(struct spi_device *spi)struct spi_master *master = spi->master;struct sunxi_spi *sspi =spi_master_get_devdata (master); +if (master->can_dma) +return SUNXI_CNT_MASK ;return sspi->fifo_depth -1 ; }
@@ -292,22 +351,27 @@ static int sunxi_spi_transfer_one(struct spi_master *master, struct spi_transfer *tfr) {struct sunxi_spi *sspi =spi_master_get_devdata (master); +struct dma_async_tx_descriptor *desc_tx =NULL , *desc_rx =NULL ;unsigned int mclk_rate, div, timeout;unsigned int start, end, tx_time;unsigned int tx_len =0 ;int ret =0 ; -u32 reg; +u32 reg, trigger =0 ; + +if (!master->can_dma) { +/* We don't support transfer larger than the FIFO */ +if (tfr->len > sspi->fifo_depth) +return -EMSGSIZE ; +/* +* Filling the FIFO fully causes timeout for some reason +* at least on spi2 on A10s +*/ +if ((sspi->type ==SPI_SUN4I ) && + tfr->tx_buf && tfr->len >= sspi->fifo_depth) +return -EMSGSIZE ; + } -/* We don't support transfer larger than the FIFO */ -if (tfr->len > sspi->fifo_depth) -return -EMSGSIZE ; - -/* -* Filling the FIFO fully causes timeout for some reason -* at least on spi2 on A10s -*/ -if ((sspi->type ==SPI_SUN4I ) && - tfr->tx_buf && tfr->len >= sspi->fifo_depth) +if (tfr->len >SUNXI_CNT_MASK )return -EMSGSIZE ;reinit_completion (&sspi->done);
@@ -405,17 +469,81 @@ static int sunxi_spi_transfer_one(struct spi_master *master,sunxi_spi_write (sspi,SUNXI_BURST_CTL_CNT_REG ,SUNXI_BURST_CTL_CNT_STC (tx_len)); -/* Fill the TX FIFO */ -sunxi_spi_fill_fifo (sspi, sspi->fifo_depth); +/* Setup transfer buffers */ +if (sunxi_spi_can_dma (master, spi, tfr)) { +dev_dbg (&sspi->master->dev,"Using DMA mode for transfer\n" ); + +if (sspi->tx_buf) { + desc_tx =dmaengine_prep_slave_sg (master->dma_tx, + tfr->tx_sg.sgl, tfr->tx_sg.nents, +DMA_TO_DEVICE , +DMA_PREP_INTERRUPT |DMA_CTRL_ACK ); +if (!desc_tx) { +dev_err (&sspi->master->dev, +"Couldn't prepare dma slave\n" ); + ret = -EIO ; + goto out; + } + +if (sspi->type ==SPI_SUN4I ) + trigger |=sspi_bits (sspi,SUNXI_CTL_DMA_TF_NOT_FULL ); +else + trigger |=sspi_bits (sspi,SUNXI_CTL_DMA_TF_HALF ); + +dmaengine_submit (desc_tx); +dma_async_issue_pending (master->dma_tx); + } + +if (sspi->rx_buf) { + desc_rx =dmaengine_prep_slave_sg (master->dma_rx, + tfr->rx_sg.sgl, tfr->rx_sg.nents, +DMA_FROM_DEVICE , +DMA_PREP_INTERRUPT |DMA_CTRL_ACK ); +if (!desc_rx) { +dev_err (&sspi->master->dev, +"Couldn't prepare dma slave\n" ); + ret = -EIO ; + goto out; + } + + trigger |=sspi_bits (sspi,SUNXI_CTL_DMA_RF_READY ); + +dmaengine_submit (desc_rx); +dma_async_issue_pending (master->dma_rx); + } + +/* Enable Dedicated DMA requests */ +if (sspi->type ==SPI_SUN4I ) { +sunxi_spi_set (sspi,SUNXI_TFR_CTL_REG , +sspi_bits (sspi,SUNXI_CTL_DMA_DEDICATED )); +sunxi_spi_write (sspi,SUNXI_DMA_CTL_REG , trigger); + }else { + trigger |=sspi_bits (sspi,SUNXI_CTL_DMA_DEDICATED ); +sunxi_spi_write (sspi,SUNXI_FIFO_CTL_REG , trigger); + } + }else { +dev_dbg (&sspi->master->dev,"Using PIO mode for transfer\n" ); + +/* Disable DMA requests */ +if (sspi->type ==SPI_SUN4I ) { +sunxi_spi_unset (sspi,SUNXI_TFR_CTL_REG , +sspi_bits (sspi,SUNXI_CTL_DMA_DEDICATED )); +sunxi_spi_write (sspi,SUNXI_DMA_CTL_REG ,0 ); + }else { +sunxi_spi_write (sspi,SUNXI_FIFO_CTL_REG ,0 ); + } + +/* Fill the TX FIFO */ +sunxi_spi_fill_fifo (sspi, sspi->fifo_depth); + }/* Enable the interrupts */ sunxi_spi_write (sspi,SUNXI_INT_CTL_REG ,sspi_bits (sspi,SUNXI_INT_CTL_TC ));/* Start the transfer */ - reg =sunxi_spi_read (sspi,SUNXI_TFR_CTL_REG ); -sunxi_spi_write (sspi,SUNXI_TFR_CTL_REG , - reg |sspi_bits (sspi,SUNXI_TFR_CTL_XCH )); +sunxi_spi_set (sspi,SUNXI_TFR_CTL_REG , +sspi_bits (sspi,SUNXI_TFR_CTL_XCH )); tx_time =max (tfr->len *8 *2 / (tfr->speed_hz /1000 ),100U ); start = jiffies;
@@ -431,9 +559,23 @@ static int sunxi_spi_transfer_one(struct spi_master *master, goto out; } +out: +if (ret <0 &&sunxi_spi_can_dma (master, spi, tfr)) { +dev_dbg (&master->dev,"DMA channel teardown" ); +if (sspi->tx_buf) +dmaengine_terminate_sync (master->dma_tx); +if (sspi->rx_buf) +dmaengine_terminate_sync (master->dma_rx); + } + +/* +* By this time either the transfer has ended and we have data in the +* FIFO buffer from a PIO RX transfer or the buffer is empty +* or something has failed. +* Empty the buffer either way to avoid leaving garbage around. +*/ sunxi_spi_drain_fifo (sspi, sspi->fifo_depth); -out:sunxi_spi_write (sspi,SUNXI_INT_CTL_REG ,0 );return ret;
@@ -515,6 +657,7 @@ static int sunxi_spi_runtime_suspend(struct device *dev)static int sunxi_spi_probe (struct platform_device *pdev) { +struct dma_slave_config dma_sconfig;struct spi_master *master;struct sunxi_spi *sspi;struct resource *res;
@@ -625,6 +768,54 @@ static int sunxi_spi_probe(struct platform_device *pdev) } } + master->dma_tx =dma_request_slave_channel_reason (&pdev->dev,"tx" ); +if (IS_ERR (master->dma_tx)) { +dev_err (&pdev->dev,"Unable to acquire DMA channel TX\n" ); + ret =PTR_ERR (master->dma_tx); + goto err_dma_chan; + } + + dma_sconfig.direction =DMA_MEM_TO_DEV ; + dma_sconfig.src_addr_width =DMA_SLAVE_BUSWIDTH_1_BYTE ; + dma_sconfig.dst_addr_width =DMA_SLAVE_BUSWIDTH_1_BYTE ; + dma_sconfig.dst_addr = res->start +sspi_reg (sspi,SUNXI_TXDATA_REG ); + dma_sconfig.src_maxburst =1 ; + dma_sconfig.dst_maxburst =1 ; + + ret =dmaengine_slave_config (master->dma_tx, &dma_sconfig); +if (ret) { +dev_err (&pdev->dev,"Unable to configure TX DMA slave\n" ); + goto err_tx_dma_release; + } + + master->dma_rx =dma_request_slave_channel_reason (&pdev->dev,"rx" ); +if (IS_ERR (master->dma_rx)) { +dev_err (&pdev->dev,"Unable to acquire DMA channel RX\n" ); + ret =PTR_ERR (master->dma_rx); + goto err_tx_dma_release; + } + + dma_sconfig.direction =DMA_DEV_TO_MEM ; + dma_sconfig.src_addr_width =DMA_SLAVE_BUSWIDTH_1_BYTE ; + dma_sconfig.dst_addr_width =DMA_SLAVE_BUSWIDTH_1_BYTE ; + dma_sconfig.src_addr = res->start +sspi_reg (sspi,SUNXI_RXDATA_REG ); + dma_sconfig.src_maxburst =1 ; + dma_sconfig.dst_maxburst =1 ; + + ret =dmaengine_slave_config (master->dma_rx, &dma_sconfig); +if (ret) { +dev_err (&pdev->dev,"Unable to configure RX DMA slave\n" ); + goto err_rx_dma_release; + } + +/* +* This is a bit dodgy. If you set can_dma then map_msg in spi.c +* apparently dereferences your dma channels if non-NULL even if your +* can_dma never returns true (and crashes if the channel is an error +* pointer). So just don't set can_dma unless both channels are valid. +*/ + master->can_dma = sunxi_spi_can_dma; +wakeup:/* * This wake-up/shutdown pattern is to be able to have the * device woken up, even if runtime_pm is disabled
@@ -665,18 +856,40 @@ static int sunxi_spi_probe(struct platform_device *pdev)return 0 ; +err_rx_dma_release: +dma_release_channel (master->dma_rx); +err_tx_dma_release: +dma_release_channel (master->dma_tx); +err_dma_chan: + master->dma_tx =NULL ; + master->dma_rx =NULL ; +if ((ret == -EPROBE_DEFER ) && wait_for_dma) + goto err_free_master; + goto wakeup; + err_pm_disable:pm_runtime_disable (&pdev->dev);sunxi_spi_runtime_suspend (&pdev->dev); err_free_master: +if (master->can_dma) { +dma_release_channel (master->dma_rx); +dma_release_channel (master->dma_tx); + }spi_master_put (master);return ret; }static int sunxi_spi_remove (struct platform_device *pdev) { +struct spi_master *master =platform_get_drvdata (pdev); +pm_runtime_disable (&pdev->dev); +if (master->can_dma) { +dma_release_channel (master->dma_rx); +dma_release_channel (master->dma_tx); + } +return 0 ; }
Signature · 1 line
2.8.1
Quoted 8 lines · Michal Suchanek
Hello,
This is update of the sunxi spi patches that should give full-featured SPI driver.
First three patches fix issues with the current driver and can be of use for stable kernels so adding cc for those.
Quoted 7 lines
I merged the sun4i and sun6i driver because there several issues that need to be fixed in both separately and they are even out of sync wrt some fixes. I guess some of the merge patches can be squashed.
I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot tell if that side was broken by this patchset - especially the last patch that adds DMA was afaik never tested on sun6i.
So, you didn't run that code through checkpatch and you rewrite the whole thing entirely without even testing it... Awesome.
For the record, I'm still very much opposed to such a merge.
The first fixes are very welcome though, can and should go in.
Maxime
Signature · 10 lines
Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160613/7904c14e/attachment.sig>
Hello,
Quoted 13 lines · Maxime Ripard
Quoted 12 lines · Michal Suchanek
Hello,
This is update of the sunxi spi patches that should give full-featured SPI driver.
First three patches fix issues with the current driver and can be of use for stable kernels so adding cc for those.
I merged the sun4i and sun6i driver because there several issues that need to be fixed in both separately and they are even out of sync wrt some fixes. I guess some of the merge patches can be squashed.
Quoted 6 lines
Quoted 3 lines
I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot tell if that side was broken by this patchset - especially the last patch that adds DMA was afaik never tested on sun6i.
So, you didn't run that code through checkpatch and you rewrite the whole thing entirely without even testing it... Awesome.
Aside from the DMA part this is not a rewrite.
And I did run the code through checkpatch. It still gives some warnings about BUG_ON and overly long lines, sure. II don't think those are that serious or that fixing them would improve the code.
Thanks
Michal
Hello,
Quoted 13 lines · Maxime Ripard
Quoted 12 lines · Michal Suchanek
Hello,
This is update of the sunxi spi patches that should give full-featured SPI driver.
First three patches fix issues with the current driver and can be of use for stable kernels so adding cc for those.
I merged the sun4i and sun6i driver because there several issues that need to be fixed in both separately and they are even out of sync wrt some fixes. I guess some of the merge patches can be squashed.
Quoted 8 lines
Quoted 3 lines
I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot tell if that side was broken by this patchset - especially the last patch that adds DMA was afaik never tested on sun6i.
For the record, I'm still very much opposed to such a merge.
What is the reason against the merge? I did not find the original discussion.
I tried to rename everything in the drivers from sun4i and sun6i to sunxi to look at a clean diff and found about 5 differences 2 of which look like a bug.
Thanks
Michal
Quoted 22 lines · Michal Suchanek
Hello,
Quoted 18 lines · Maxime Ripard
Quoted 15 lines · Michal Suchanek
Hello,
This is update of the sunxi spi patches that should give full-featured SPI driver.
First three patches fix issues with the current driver and can be of use for stable kernels so adding cc for those.
I merged the sun4i and sun6i driver because there several issues that need to be fixed in both separately and they are even out of sync wrt some fixes. I guess some of the merge patches can be squashed.
I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot tell if that side was broken by this patchset - especially the last patch that adds DMA was afaik never tested on sun6i.
Quoted 4 lines
Quoted 1 line
For the record, I'm still very much opposed to such a merge.
What is the reason against the merge? I did not find the original discussion.
I really prefer some code that is concise and clear but a little duplicated over some code that shares every possible lines of code but is a giant mess impossible to understand.
I just came across the reg_field stuff in regmap that would allow to partially address that problem though, there's still the bit indices issue to overcome though.
Quoted 3 lines
I tried to rename everything in the drivers from sun4i and sun6i to sunxi to look at a clean diff and found about 5 differences 2 of which look like a bug.
It's hard to tell without testing.
Maxime
Signature · 10 lines
Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160725/30b5e523/attachment.sig>
Hello,
Quoted 23 lines · Maxime Ripard
Quoted 22 lines · Michal Suchanek
Hello,
Quoted 18 lines · Maxime Ripard
Quoted 15 lines · Michal Suchanek
Hello,
This is update of the sunxi spi patches that should give full-featured SPI driver.
First three patches fix issues with the current driver and can be of use for stable kernels so adding cc for those.
I merged the sun4i and sun6i driver because there several issues that need to be fixed in both separately and they are even out of sync wrt some fixes. I guess some of the merge patches can be squashed.
I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot tell if that side was broken by this patchset - especially the last patch that adds DMA was afaik never tested on sun6i.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
For the record, I'm still very much opposed to such a merge.
What is the reason against the merge? I did not find the original discussion.
I really prefer some code that is concise and clear but a little duplicated over some code that shares every possible lines of code but is a giant mess impossible to understand.
Yes, it's been tried. In the case of this driver there is more duplication than differences. Also bitrot due to different variants receiving different updates and fixes crept in already.
Adding the remap layer certainly does not make the driver easier to understand but it's not becoming giant mess either.
Quoted 10 lines
I just came across the
reg_fieldstuff in regmap that would allow to partially address that problem though, there's still the bit indices issue to overcome though.Quoted 3 lines
I tried to rename everything in the drivers from sun4i and sun6i to sunxi to look at a clean diff and found about 5 differences 2 of which look like a bug.
It's hard to tell without testing.
I tested the merged (and separate) driver on a H3 board and found a type bug in the remap layer.
I also found I can transfer at most 68 bytes without DMA although the manuals claim 128 byte FIFO. DMA transfers weren't very reliable which may or may not be a wiring issue. I have seen similar issues intermittently with SPI on A10s.
Thanks
Michal
Quoted 36 lines · Michal Suchanek
Hello,
Quoted 23 lines · Maxime Ripard
Quoted 22 lines · Michal Suchanek
Hello,
Quoted 18 lines · Maxime Ripard
Quoted 15 lines · Michal Suchanek
Hello,
This is update of the sunxi spi patches that should give full-featured SPI driver.
First three patches fix issues with the current driver and can be of use for stable kernels so adding cc for those.
I merged the sun4i and sun6i driver because there several issues that need to be fixed in both separately and they are even out of sync wrt some fixes. I guess some of the merge patches can be squashed.
I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot tell if that side was broken by this patchset - especially the last patch that adds DMA was afaik never tested on sun6i.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
For the record, I'm still very much opposed to such a merge.
What is the reason against the merge? I did not find the original discussion.
I really prefer some code that is concise and clear but a little duplicated over some code that shares every possible lines of code but is a giant mess impossible to understand.
Quoted 6 lines
Yes, it's been tried. In the case of this driver there is more duplication than differences. Also bitrot due to different variants receiving different updates and fixes crept in already.
Adding the remap layer certainly does not make the driver easier to understand but it's not becoming giant mess either.
Well, when you say that you're not quite fond of using your structure all over the place because "it makes your eyes bleed", I wouldn't call that a good sales pitch.
But again, reg_field seems like a good solution for that.
Maxime
Signature · 10 lines
Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160729/3298104f/attachment.sig>
Quoted 41 lines · Maxime Ripard
Quoted 40 lines · Michal Suchanek
Hello,
Quoted 23 lines · Maxime Ripard
Quoted 22 lines · Michal Suchanek
Hello,
Quoted 18 lines · Maxime Ripard
Quoted 15 lines · Michal Suchanek
Hello,
This is update of the sunxi spi patches that should give full-featured SPI driver.
First three patches fix issues with the current driver and can be of use for stable kernels so adding cc for those.
I merged the sun4i and sun6i driver because there several issues that need to be fixed in both separately and they are even out of sync wrt some fixes. I guess some of the merge patches can be squashed.
I tested this with A10s Olinuxino Micro. I have no sun6i device so I cannot tell if that side was broken by this patchset - especially the last patch that adds DMA was afaik never tested on sun6i.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
For the record, I'm still very much opposed to such a merge.
What is the reason against the merge? I did not find the original discussion.
I really prefer some code that is concise and clear but a little duplicated over some code that shares every possible lines of code but is a giant mess impossible to understand.
Yes, it's been tried. In the case of this driver there is more duplication than differences. Also bitrot due to different variants receiving different updates and fixes crept in already.
Quoted 6 lines
Quoted 2 lines
Adding the remap layer certainly does not make the driver easier to understand but it's not becoming giant mess either.
Well, when you say that you're not quite fond of using your structure all over the place because "it makes your eyes bleed", I wouldn't call that a good sales pitch.
That's why put dereferencing it inside the read function. It reduces the noise throughout the code and make it easier to add checks or change the remap implementation.
Quoted 2 lines
But again,
reg_fieldseems like a good solution for that.
I will look at reg_field.
I saw some drivers using remap layers that are way more heavyweight than looking up an integer in a table so I wanted to avoid that.
Thanks
Michal