spi: spi-s3c64xx: Remove unused driver data structure tgl_spi field
Subsystems (3)
A message that is not in the archive (sent off-list or not yet received): CGME20180416154055epcas2p4c08bb2254e79368a29eb481cddc59855@epcas2p4.samsung.com
PATCH 1/6 spi: spi-s3c64xx: Remove unused driver data structure tgl_spi field
The tgl_spi pointer is now unused so remove it.
--- drivers/spi/spi-s3c64xx.c | 2 -- 1 file changed, 2 deletions(-)
drivers/spi/spi-s3c64xx.c +0 -2
@@ -156,7 +156,6 @@ struct s3c64xx_spi_port_config {* @ioclk: Pointer to the i/o clock between master and slave * @master: Pointer to the SPI Protocol master. * @cntrlr_info: Platform specific data for the controller this driver manages. -* @tgl_spi: Pointer to the last CS left untoggled by the cs_change hint. * @lock: Controller specific lock. * @state: Set of FLAGS to indicate status. * @rx_dmach: Controller's DMA channel for Rx.
@@ -177,7 +176,6 @@ struct s3c64xx_spi_driver_data {struct platform_device *pdev;struct spi_master *master;struct s3c64xx_spi_info *cntrlr_info; -struct spi_device *tgl_spi;spinlock_t lock;unsigned long sfr_start;struct completion xfer_completion;
Signature · 1 line
2.14.2
PATCH 2/6 spi: spi-s3c64xx: Remove unused s3c64xx_spi_hwinit() function argument
The channel argument is not used and anyway it could be retrieved from the passed driver data structure.
--- drivers/spi/spi-s3c64xx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/spi/spi-s3c64xx.c +3 -3
@@ -889,7 +889,7 @@ static irqreturn_t s3c64xx_spi_irq(int irq, void *data)return IRQ_HANDLED ; } -static void s3c64xx_spi_hwinit (struct s3c64xx_spi_driver_data *sdd,int channel) +static void s3c64xx_spi_hwinit (struct s3c64xx_spi_driver_data *sdd) {struct s3c64xx_spi_info *sci = sdd->cntrlr_info;void __iomem *regs = sdd->regs;
@@ -1143,7 +1143,7 @@ static int s3c64xx_spi_probe(struct platform_device *pdev)pm_runtime_get_sync (&pdev->dev);/* Setup Deufult Mode */ -s3c64xx_spi_hwinit (sdd, sdd->port_id); +s3c64xx_spi_hwinit (sdd);spin_lock_init (&sdd->lock);init_completion (&sdd->xfer_completion);
@@ -1258,7 +1258,7 @@ static int s3c64xx_spi_resume(struct device *dev)if (ret <0 )return ret; -s3c64xx_spi_hwinit (sdd, sdd->port_id); +s3c64xx_spi_hwinit (sdd);return spi_master_resume (master); }
Signature · 1 line
2.14.2
PATCH 2/6 spi: spi-s3c64xx: Remove unused s3c64xx_spi_hwinit() function argument
Hi Sylwester,
Quoted 4 lines · Sylwester Nawrocki
The channel argument is not used and anyway it could be retrieved from the passed driver data structure.
Signed-off-by Sylwester Nawrocki <[email protected]>
Thanks,
Andi
Krzysztof Kozlowski
maintainerPATCH 2/6 spi: spi-s3c64xx: Remove unused s3c64xx_spi_hwinit() function argument
In reply to Sylwester NawrockiQuoted 8 lines · Sylwester Nawrocki
The channel argument is not used and anyway it could be retrieved from the passed driver data structure.
Signed-off-by Sylwester Nawrocki <[email protected]>--- drivers/spi/spi-s3c64xx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Best regards,
Krzysztof
PATCH 3/6 spi: spi-s3c64xx: Drop unused enable_datapath() function argument
In reply to Sylwester NawrockiThe spi pointer argument is not used now so remove it. While at it s3c64xx_ prefix is added to the function name.
--- drivers/spi/spi-s3c64xx.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/spi/spi-s3c64xx.c +3 -4
@@ -348,9 +348,8 @@ static bool s3c64xx_spi_can_dma(struct spi_master *master,return xfer->len > (FIFO_LVL_MASK (sdd) >>1 ) +1 ; } -static void enable_datapath (struct s3c64xx_spi_driver_data *sdd, -struct spi_device *spi, -struct spi_transfer *xfer,int dma_mode) +static void s3c64xx_enable_datapath (struct s3c64xx_spi_driver_data *sdd, +struct spi_transfer *xfer,int dma_mode) {void __iomem *regs = sdd->regs;u32 modecfg, chcfg;
@@ -665,7 +664,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master, sdd->state &= ~RXBUSY ; sdd->state &= ~TXBUSY ; -enable_datapath (sdd, spi, xfer, use_dma); +s3c64xx_enable_datapath (sdd, xfer, use_dma);/* Start the signals */ s3c64xx_spi_set_cs (spi, true);
--
2.14.2
PATCH 3/6 spi: spi-s3c64xx: Drop unused enable_datapath() function argument
Hi Sylwester,
Quoted 2 lines · Sylwester Nawrocki
The spi pointer argument is not used now so remove it. While at it
s3c64xx_prefix is added to the function name.
These for me are two different patches, but in any case it's trivial so that I don't mind it too much.
Quoted 1 line
Signed-off-by Sylwester Nawrocki <[email protected]>
Thanks,
Andi
PATCH 3/6 spi: spi-s3c64xx: Drop unused enable_datapath() function argument
Hi Andi,
Quoted 6 lines · andi at etezian.org
Quoted 2 lines · Sylwester Nawrocki
The spi pointer argument is not used now so remove it. While at it
s3c64xx_prefix is added to the function name.These for me are two different patches, but in any case it's trivial so that I don't mind it too much.
I am going to drop renaming from this patch. There are 2 more functions (wait_for_pio(), wait_for_dma()) without prefix, should we rename them too or rather leave all 3 as they are now?
Signature · 2 lines
Regards, Sylwester
Krzysztof Kozlowski
maintainerPATCH 3/6 spi: spi-s3c64xx: Drop unused enable_datapath() function argument
Quoted 7 lines · Sylwester Nawrocki
Hi Andi,
Quoted 4 lines · andi at etezian.org
Quoted 2 lines · Sylwester Nawrocki
The spi pointer argument is not used now so remove it. While at it
s3c64xx_prefix is added to the function name.
Quoted 6 lines
Quoted 2 lines
These for me are two different patches, but in any case it's trivial so that I don't mind it too much.
I am going to drop renaming from this patch. There are 2 more functions (
wait_for_pio(),wait_for_dma()) without prefix, should we rename them too or rather leave all 3 as they are now?
Indeed it is kind of mixed up - some purely local functions (not even passed through pointer to some ops) have prefix, some do not. If you ask me, I would prefer to have the prefix on all of them because:
1. If they appear in call trace (where backtrace pointers are not telling anything) it is easy to understand their location,
2. having generic name like "wait_for_dma" sounds for me like generic, Linux-wide macro (but it is not).
However I do not insist on that.
Best regards,
Krzysztof
Krzysztof Kozlowski
maintainerPATCH 3/6 spi: spi-s3c64xx: Drop unused enable_datapath() function argument
In reply to Sylwester NawrockiQuoted 7 lines · Sylwester Nawrocki
The spi pointer argument is not used now so remove it. While at it
s3c64xx_prefix is added to the function name.Signed-off-by Sylwester Nawrocki <[email protected]>--- drivers/spi/spi-s3c64xx.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
Best regards,
Krzysztof
PATCH 4/6 spi: spi-s3c64xx: Do not ignore timeout errors in polling I/O mode
In reply to Sylwester NawrockiCurrently timeout errors in polling I/O mode transfer are silently ignored. Fix it by returning an error when we time out waiting on the RX FIFO level to reach the transfer length.
--- drivers/spi/spi-s3c64xx.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/spi/spi-s3c64xx.c +2 -0
@@ -502,6 +502,8 @@ static int wait_for_pio(struct s3c64xx_spi_driver_data *sdd, status =readl (regs +S3C64XX_SPI_STATUS ); }while (RX_FIFO_LVL (status, sdd) < xfer->len && --val); +if (!val) +return -EIO ;/* If it was only Tx */ if (!xfer->rx_buf) {
Signature · 1 line
2.14.2
PATCH 4/6 spi: spi-s3c64xx: Do not ignore timeout errors in polling I/O mode
Hi Sylwester,
Quoted 5 lines · Sylwester Nawrocki
Currently timeout errors in polling I/O mode transfer are silently ignored.
Fix it by returning an error when we time out waiting on the RX FIFO level
to reach the transfer length.
yes, makes sense.
Quoted 1 line
Signed-off-by Sylwester Nawrocki <[email protected]>
Thanks,
Andi
Krzysztof Kozlowski
maintainerPATCH 4/6 spi: spi-s3c64xx: Do not ignore timeout errors in polling I/O mode
In reply to Sylwester NawrockiQuoted 9 lines · Sylwester Nawrocki
Currently timeout errors in polling I/O mode transfer are silently ignored. Fix it by returning an error when we time out waiting on the RX FIFO level to reach the transfer length.
Signed-off-by Sylwester Nawrocki <[email protected]>--- drivers/spi/spi-s3c64xx.c | 2 ++ 1 file changed, 2 insertions(+)
Best regards,
Krzysztof
Mark Brown
maintainerPATCH 4/6 spi: spi-s3c64xx: Do not ignore timeout errors in polling I/O mode
In reply to Sylwester NawrockiQuoted 3 lines · Sylwester Nawrocki
Currently timeout errors in polling I/O mode transfer are silently ignored. Fix it by returning an error when we time out waiting on the RX FIFO level to reach the transfer length.
This would probably have been better sent as a fix before the cosmetic
updates.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180417/f8e2d91c/attachment.sig>
PATCH 5/6 spi: spi-s3c64xx: Fix indentation in the register offset definitions
In reply to Sylwester NawrockiChange indentation so register address offset and register bit definitions are aligned to same column.
--- drivers/spi/spi-s3c64xx.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/spi/spi-s3c64xx.c +8 -8
@@ -28,15 +28,15 @@#define S3C64XX_SPI_CH_CFG 0x00#define S3C64XX_SPI_CLK_CFG 0x04 -#define S3C64XX_SPI_MODE_CFG 0x08 -#define S3C64XX_SPI_SLAVE_SEL 0x0C +#define S3C64XX_SPI_MODE_CFG 0x08 +#define S3C64XX_SPI_SLAVE_SEL 0x0C#define S3C64XX_SPI_INT_EN 0x10#define S3C64XX_SPI_STATUS 0x14#define S3C64XX_SPI_TX_DATA 0x18#define S3C64XX_SPI_RX_DATA 0x1C -#define S3C64XX_SPI_PACKET_CNT 0x20 -#define S3C64XX_SPI_PENDING_CLR 0x24 -#define S3C64XX_SPI_SWAP_CFG 0x28 +#define S3C64XX_SPI_PACKET_CNT 0x20 +#define S3C64XX_SPI_PENDING_CLR 0x24 +#define S3C64XX_SPI_SWAP_CFG 0x28#define S3C64XX_SPI_FB_CLK 0x2C#define S3C64XX_SPI_CH_HS_EN (1<<6)/* High Speed Enable */
@@ -77,9 +77,9 @@#define S3C64XX_SPI_INT_TX_FIFORDY_EN (1<<0)#define S3C64XX_SPI_ST_RX_OVERRUN_ERR (1<<5) -#define S3C64XX_SPI_ST_RX_UNDERRUN_ERR (1<<4) +#define S3C64XX_SPI_ST_RX_UNDERRUN_ERR (1<<4)#define S3C64XX_SPI_ST_TX_OVERRUN_ERR (1<<3) -#define S3C64XX_SPI_ST_TX_UNDERRUN_ERR (1<<2) +#define S3C64XX_SPI_ST_TX_UNDERRUN_ERR (1<<2)#define S3C64XX_SPI_ST_RX_FIFORDY (1<<1)#define S3C64XX_SPI_ST_TX_FIFORDY (1<<0)
@@ -100,7 +100,7 @@#define S3C64XX_SPI_SWAP_TX_BIT (1<<1)#define S3C64XX_SPI_SWAP_TX_EN (1<<0) -#define S3C64XX_SPI_FBCLK_MSK (3<<0) +#define S3C64XX_SPI_FBCLK_MSK (3<<0)#define FIFO_LVL_MASK (i) ((i)->port_conf->fifo_lvl_mask[i->port_id])#define S3C64XX_SPI_ST_TX_DONE (v, i) (((v) & \
--
2.14.2
PATCH 5/6 spi: spi-s3c64xx: Fix indentation in the register offset definitions
Hi Sylwester,
Quoted 5 lines · Sylwester Nawrocki
Change indentation so register address offset and register bit definitions
are aligned to same column.Signed-off-by Sylwester Nawrocki <[email protected]>
Thanks,
Andi
Krzysztof Kozlowski
maintainerPATCH 5/6 spi: spi-s3c64xx: Fix indentation in the register offset definitions
In reply to Sylwester NawrockiQuoted 7 lines · Sylwester Nawrocki
Change indentation so register address offset and register bit definitions are aligned to same column.
Signed-off-by Sylwester Nawrocki <[email protected]>--- drivers/spi/spi-s3c64xx.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
Best regards,
Krzysztof
PATCH 6/6 spi: spi-s3c64xx: Allow higher transfer lengths in polling IO mode
In reply to Sylwester NawrockiSome variants of the SPI controller have no DMA support, in such case SPI transfers longer than the FIFO length are not currently properly handled by the driver. Fix it by doing multiple transfers in the s3c64xx_spi_transfer_one() function if the SPI transfer length exceeds the FIFO size.
--- drivers/spi/spi-s3c64xx.c | 108 ++++++++++++++++++++++++++++++---------------- 1 file changed, 71 insertions(+), 37 deletions(-)
drivers/spi/spi-s3c64xx.c +71 -37
@@ -349,7 +349,7 @@ static bool s3c64xx_spi_can_dma(struct spi_master *master, }static void s3c64xx_enable_datapath (struct s3c64xx_spi_driver_data *sdd, -struct spi_transfer *xfer,int dma_mode) +struct spi_transfer *xfer,bool dma_mode) {void __iomem *regs = sdd->regs;u32 modecfg, chcfg;
@@ -634,11 +634,15 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master, struct spi_transfer *xfer) {struct s3c64xx_spi_driver_data *sdd =spi_master_get_devdata (master); +const unsigned int fifo_len = (FIFO_LVL_MASK (sdd) >>1 ) +1 ; +const void *tx_buf =NULL ; +void *rx_buf =NULL ; +int target_len =0 , origin_len =0 ; +bool use_dma = false;int status;u32 speed;u8 bpw;unsigned long flags; -int use_dma;reinit_completion (&sdd->xfer_completion);
@@ -653,48 +657,78 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,s3c64xx_spi_config (sdd); } -/* Polling method for xfers not bigger than FIFO capacity */ - use_dma =0 ; -if (!is_polling (sdd) && - (sdd->rx_dma.ch && sdd->tx_dma.ch && - (xfer->len > ((FIFO_LVL_MASK (sdd) >>1 ) +1 )))) - use_dma =1 ; +if (!is_polling (sdd) && (xfer->len > fifo_len) && + sdd->rx_dma.ch && sdd->tx_dma.ch) { + use_dma = true; -spin_lock_irqsave (&sdd->lock, flags); + }else if (is_polling (sdd) && xfer->len > fifo_len) { + tx_buf = xfer->tx_buf; + rx_buf = xfer->rx_buf; + origin_len = xfer->len; -/* Pending only which is to be done */ - sdd->state &= ~RXBUSY ; - sdd->state &= ~TXBUSY ; + target_len = xfer->len; +if (xfer->len > fifo_len) + xfer->len = fifo_len; + } + +do { +spin_lock_irqsave (&sdd->lock, flags); -s3c64xx_enable_datapath (sdd, xfer, use_dma); +/* Pending only which is to be done */ + sdd->state &= ~RXBUSY ; + sdd->state &= ~TXBUSY ; -/* Start the signals */ -s3c64xx_spi_set_cs (spi, true); +s3c64xx_enable_datapath (sdd, xfer, use_dma); -spin_unlock_irqrestore (&sdd->lock, flags); +/* Start the signals */ +s3c64xx_spi_set_cs (spi, true); -if (use_dma) - status =wait_for_dma (sdd, xfer); -else - status =wait_for_pio (sdd, xfer); - -if (status) { -dev_err (&spi->dev,"I/O Error: rx-%d tx-%d res:rx-%c tx-%c len-%d\n" , - xfer->rx_buf ?1 :0 , xfer->tx_buf ?1 :0 , - (sdd->state &RXBUSY ) ?'f' :'p' , - (sdd->state &TXBUSY ) ?'f' :'p' , - xfer->len); - -if (use_dma) { -if (xfer->tx_buf !=NULL - && (sdd->state &TXBUSY )) -dmaengine_terminate_all (sdd->tx_dma.ch); -if (xfer->rx_buf !=NULL - && (sdd->state &RXBUSY )) -dmaengine_terminate_all (sdd->rx_dma.ch); +spin_unlock_irqrestore (&sdd->lock, flags); + +if (use_dma) + status =wait_for_dma (sdd, xfer); +else + status =wait_for_pio (sdd, xfer); + +if (status) { +dev_err (&spi->dev, +"I/O Error: rx-%d tx-%d res:rx-%c tx-%c len-%d\n" , + xfer->rx_buf ?1 :0 , xfer->tx_buf ?1 :0 , + (sdd->state &RXBUSY ) ?'f' :'p' , + (sdd->state &TXBUSY ) ?'f' :'p' , + xfer->len); + +if (use_dma) { +if (xfer->tx_buf && (sdd->state &TXBUSY )) +dmaengine_terminate_all (sdd->tx_dma.ch); +if (xfer->rx_buf && (sdd->state &RXBUSY )) +dmaengine_terminate_all (sdd->rx_dma.ch); + } + }else { +flush_fifo (sdd); } - }else { -flush_fifo (sdd); + +if (target_len >0 ) { + target_len -= xfer->len; + +if (xfer->tx_buf) + xfer->tx_buf += xfer->len; + +if (xfer->rx_buf) + xfer->rx_buf += xfer->len; + +if (target_len > fifo_len) + xfer->len = fifo_len; +else + xfer->len = target_len; + } + }while (target_len >0 ); + +if (origin_len) { +/* Restore original xfer buffers and length */ + xfer->tx_buf = tx_buf; + xfer->rx_buf = rx_buf; + xfer->len = origin_len; }return status;
Signature · 1 line
2.14.2
PATCH 6/6 spi: spi-s3c64xx: Allow higher transfer lengths in polling IO mode
Hi Sylwester,
Quoted 5 lines · Sylwester Nawrocki
Some variants of the SPI controller have no DMA support, in such case SPI transfers longer than the FIFO length are not currently properly handled by the driver. Fix it by doing multiple transfers in the
s3c64xx_spi_transfer_one()function if the SPI transfer length exceeds the FIFO size.
this patch is a mix of cosmetics and what you are actually describing in the commit log...
Quoted 5 lines
drivers/spi/spi-s3c64xx.c +0
-0@@ -634,11 +634,15 @@ static int s3c64xx_spi_transfer_one(structspi_master *master, struct spi_transfer *xfer) { struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master);
drivers/spi/spi-s3c64xx.c static int s3c64xx_spi_transfer_one(struct · line 642
Quoted 10 lines
drivers/spi/spi-s3c64xx.c +5
-1+const unsigned int fifo_len = (FIFO_LVL_MASK (sdd) >>1 ) +1 ; +const void *tx_buf =NULL ; +void *rx_buf =NULL ; +int target_len =0 , origin_len =0 ; +bool use_dma = false;int status;u32 speed;u8 bpw;unsigned long flags; -int use_dma;
... for example 'use_dma' bool instead of int or the 'fifo_len'. I agree with these changes and, even though they are trivial, I would prefer having them in separate patches.
Thanks,
Andi
PATCH 6/6 spi: spi-s3c64xx: Allow higher transfer lengths in polling IO mode
Hi Andi,
Quoted 21 lines · andi at etezian.org
Quoted 5 lines · Sylwester Nawrocki
Some variants of the SPI controller have no DMA support, in such case SPI transfers longer than the FIFO length are not currently properly handled by the driver. Fix it by doing multiple transfers in the
s3c64xx_spi_transfer_one()function if the SPI transfer length exceeds the FIFO size.this patch is a mix of cosmetics and what you are actually describing in the commit log...
Quoted 11 lines
drivers/spi/spi-s3c64xx.c +5
-0@@ -634,11 +634,15 @@ static int s3c64xx_spi_transfer_one(structspi_master *master, struct spi_transfer *xfer) { struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master); +const unsigned int fifo_len = (FIFO_LVL_MASK (sdd) >>1 ) +1 ; +const void *tx_buf =NULL ; +void *rx_buf =NULL ; +int target_len =0 , origin_len =0 ; +bool use_dma = false;int status;
Quoted 8 lines
drivers/spi/spi-s3c64xx.c static int s3c64xx_spi_transfer_one(struct spi_master *master, · line 641
Quoted 4 lines
drivers/spi/spi-s3c64xx.c +0
-1u32 speed;u8 bpw;unsigned long flags; -int use_dma;... for example '
use_dma' bool instead of int or the 'fifo_len'. I agree with these changes and, even though they are trivial, I would prefer having them in separate patches.
Thanks for your review! I will split that into 3 patches then, however it seems a bit dubious to have the fifo_len change on its own.
Signature · 2 lines
Regards, Sylwester
PATCH 1/6 spi: spi-s3c64xx: Remove unused driver data structure tgl_spi field
In reply to Sylwester NawrockiHi Sylwester,
Quoted 3 lines · Sylwester Nawrocki
The
tgl_spipointer is now unused so remove it.Signed-off-by Sylwester Nawrocki <[email protected]>
Thanks,
Andi
Krzysztof Kozlowski
maintainerPATCH 1/6 spi: spi-s3c64xx: Remove unused driver data structure tgl_spi field
In reply to Sylwester NawrockiQuoted 6 lines · Sylwester Nawrocki
The
tgl_spipointer is now unused so remove it.Signed-off-by Sylwester Nawrocki <[email protected]>--- drivers/spi/spi-s3c64xx.c | 2 -- 1 file changed, 2 deletions(-)
Best regards,
Krzysztof