* [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size @ 2016-10-26 7:00 Alexandru Gagniuc 2016-10-26 7:00 ` [PATCH] " Alexandru Gagniuc ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Alexandru Gagniuc @ 2016-10-26 7:00 UTC (permalink / raw) To: broonie Cc: maxime.ripard, wens, linux-spi, linux-arm-kernel, linux-kernel, Alexandru Gagniuc This is the seventh attempt to get this patch in. I was prompted to look into this again as someone recently remarked: " cool, to bad spi does not work properly on allwinner..." Two and a half years ago, we were told this will all be handled with DMA. A year or so ago, we were told this will all be handled with DMA. A week or so ago, we were told this will all be handled with DMA. See the pattern? When DMA finally takes over, this fallback path is not mutually exclusive. Changes since V6: Rebased to make sure it applies on top of 4.9-rc. Also tested on actual hardware. Alexandru Gagniuc (1): spi: sun4i: Allow transfers larger than FIFO size drivers/spi/spi-sun4i.c | 75 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 67 insertions(+), 8 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] spi: sun4i: Allow transfers larger than FIFO size 2016-10-26 7:00 [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size Alexandru Gagniuc @ 2016-10-26 7:00 ` Alexandru Gagniuc 2016-10-29 18:12 ` Applied "spi: sun4i: Allow transfers larger than FIFO size" to the spi tree Mark Brown 2016-10-26 8:55 ` [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size Maxime Ripard 2016-10-26 10:07 ` Mark Brown 2 siblings, 1 reply; 11+ messages in thread From: Alexandru Gagniuc @ 2016-10-26 7:00 UTC (permalink / raw) To: broonie Cc: maxime.ripard, wens, linux-spi, linux-arm-kernel, linux-kernel, Alexandru Gagniuc, Olliver Schinagl SPI transfers were limited to one FIFO depth, which is 64 bytes. This was an artificial limitation, however, as the hardware can handle much larger bursts. To accommodate this, we enable the interrupt when the Rx FIFO is 3/4 full, and drain the FIFO within the interrupt handler. The 3/4 ratio was chosen arbitrarily, with the intention to reduce the potential number of interrupts. Since the SUN4I_CTL_TP bit is set, the hardware will pause transmission whenever the FIFO is full, so there is no risk of losing data if we can't service the interrupt in time. For the Tx side, enable and use the Tx FIFO 3/4 empty interrupt to replenish the FIFO on large SPI bursts. This requires more care in when the interrupt is left enabled, as this interrupt will continually trigger when the FIFO is less than 1/4 full, even though we acknowledge it. Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com> --- drivers/spi/spi-sun4i.c | 75 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 67 insertions(+), 8 deletions(-) diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c index 4969dc1..c5cd635 100644 --- a/drivers/spi/spi-sun4i.c +++ b/drivers/spi/spi-sun4i.c @@ -46,6 +46,8 @@ #define SUN4I_CTL_TP BIT(18) #define SUN4I_INT_CTL_REG 0x0c +#define SUN4I_INT_CTL_RF_F34 BIT(4) +#define SUN4I_INT_CTL_TF_E34 BIT(12) #define SUN4I_INT_CTL_TC BIT(16) #define SUN4I_INT_STA_REG 0x10 @@ -61,11 +63,14 @@ #define SUN4I_CLK_CTL_CDR1(div) (((div) & SUN4I_CLK_CTL_CDR1_MASK) << 8) #define SUN4I_CLK_CTL_DRS BIT(12) +#define SUN4I_MAX_XFER_SIZE 0xffffff + #define SUN4I_BURST_CNT_REG 0x20 -#define SUN4I_BURST_CNT(cnt) ((cnt) & 0xffffff) +#define SUN4I_BURST_CNT(cnt) ((cnt) & SUN4I_MAX_XFER_SIZE) #define SUN4I_XMIT_CNT_REG 0x24 -#define SUN4I_XMIT_CNT(cnt) ((cnt) & 0xffffff) +#define SUN4I_XMIT_CNT(cnt) ((cnt) & SUN4I_MAX_XFER_SIZE) + #define SUN4I_FIFO_STA_REG 0x28 #define SUN4I_FIFO_STA_RF_CNT_MASK 0x7f @@ -96,6 +101,31 @@ static inline void sun4i_spi_write(struct sun4i_spi *sspi, u32 reg, u32 value) writel(value, sspi->base_addr + reg); } +static inline u32 sun4i_spi_get_tx_fifo_count(struct sun4i_spi *sspi) +{ + u32 reg = sun4i_spi_read(sspi, SUN4I_FIFO_STA_REG); + + reg >>= SUN4I_FIFO_STA_TF_CNT_BITS; + + return reg & SUN4I_FIFO_STA_TF_CNT_MASK; +} + +static inline void sun4i_spi_enable_interrupt(struct sun4i_spi *sspi, u32 mask) +{ + u32 reg = sun4i_spi_read(sspi, SUN4I_INT_CTL_REG); + + reg |= mask; + sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, reg); +} + +static inline void sun4i_spi_disable_interrupt(struct sun4i_spi *sspi, u32 mask) +{ + u32 reg = sun4i_spi_read(sspi, SUN4I_INT_CTL_REG); + + reg &= ~mask; + sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, reg); +} + static inline void sun4i_spi_drain_fifo(struct sun4i_spi *sspi, int len) { u32 reg, cnt; @@ -118,10 +148,13 @@ static inline void sun4i_spi_drain_fifo(struct sun4i_spi *sspi, int len) static inline void sun4i_spi_fill_fifo(struct sun4i_spi *sspi, int len) { + u32 cnt; u8 byte; - if (len > sspi->len) - len = sspi->len; + /* See how much data we can fit */ + cnt = SUN4I_FIFO_DEPTH - sun4i_spi_get_tx_fifo_count(sspi); + + len = min3(len, (int)cnt, sspi->len); while (len--) { byte = sspi->tx_buf ? *sspi->tx_buf++ : 0; @@ -184,10 +217,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 > SUN4I_MAX_XFER_SIZE) return -EMSGSIZE; - if (tfr->tx_buf && tfr->len >= SUN4I_FIFO_DEPTH) + if (tfr->tx_buf && tfr->len >= SUN4I_MAX_XFER_SIZE) return -EMSGSIZE; reinit_completion(&sspi->done); @@ -286,7 +319,11 @@ static int sun4i_spi_transfer_one(struct spi_master *master, sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH - 1); /* Enable the interrupts */ - sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, SUN4I_INT_CTL_TC); + sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TC | + SUN4I_INT_CTL_RF_F34); + /* Only enable Tx FIFO interrupt if we really need it */ + if (tx_len > SUN4I_FIFO_DEPTH) + sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TF_E34); /* Start the transfer */ reg = sun4i_spi_read(sspi, SUN4I_CTL_REG); @@ -306,7 +343,6 @@ static int sun4i_spi_transfer_one(struct spi_master *master, goto out; } - sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH); out: sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0); @@ -322,10 +358,33 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id) /* Transfer complete */ if (status & SUN4I_INT_CTL_TC) { sun4i_spi_write(sspi, SUN4I_INT_STA_REG, SUN4I_INT_CTL_TC); + sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH); complete(&sspi->done); return IRQ_HANDLED; } + /* Receive FIFO 3/4 full */ + if (status & SUN4I_INT_CTL_RF_F34) { + sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH); + /* Only clear the interrupt _after_ draining the FIFO */ + sun4i_spi_write(sspi, SUN4I_INT_STA_REG, SUN4I_INT_CTL_RF_F34); + return IRQ_HANDLED; + } + + /* Transmit FIFO 3/4 empty */ + if (status & SUN4I_INT_CTL_TF_E34) { + sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH); + + if (!sspi->len) + /* nothing left to transmit */ + sun4i_spi_disable_interrupt(sspi, SUN4I_INT_CTL_TF_E34); + + /* Only clear the interrupt _after_ re-seeding the FIFO */ + sun4i_spi_write(sspi, SUN4I_INT_STA_REG, SUN4I_INT_CTL_TF_E34); + + return IRQ_HANDLED; + } + return IRQ_NONE; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Applied "spi: sun4i: Allow transfers larger than FIFO size" to the spi tree 2016-10-26 7:00 ` [PATCH] " Alexandru Gagniuc @ 2016-10-29 18:12 ` Mark Brown 0 siblings, 0 replies; 11+ messages in thread From: Mark Brown @ 2016-10-29 18:12 UTC (permalink / raw) To: Alexandru Gagniuc Cc: Maxime Ripard, Olliver Schinagl, Mark Brown, broonie, maxime.ripard, wens, linux-spi, linux-arm-kernel, linux-kernel, Olliver Schinagl, linux-spi The patch spi: sun4i: Allow transfers larger than FIFO size has been applied to the spi tree at git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark >From 196737912da5eab055489e0635662a3b7adef6eb Mon Sep 17 00:00:00 2001 From: Alexandru Gagniuc <mr.nuke.me@gmail.com> Date: Wed, 26 Oct 2016 00:00:31 -0700 Subject: [PATCH] spi: sun4i: Allow transfers larger than FIFO size SPI transfers were limited to one FIFO depth, which is 64 bytes. This was an artificial limitation, however, as the hardware can handle much larger bursts. To accommodate this, we enable the interrupt when the Rx FIFO is 3/4 full, and drain the FIFO within the interrupt handler. The 3/4 ratio was chosen arbitrarily, with the intention to reduce the potential number of interrupts. Since the SUN4I_CTL_TP bit is set, the hardware will pause transmission whenever the FIFO is full, so there is no risk of losing data if we can't service the interrupt in time. For the Tx side, enable and use the Tx FIFO 3/4 empty interrupt to replenish the FIFO on large SPI bursts. This requires more care in when the interrupt is left enabled, as this interrupt will continually trigger when the FIFO is less than 1/4 full, even though we acknowledge it. Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> Signed-off-by: Olliver Schinagl <o.schinagl@ultimaker.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- drivers/spi/spi-sun4i.c | 75 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 67 insertions(+), 8 deletions(-) diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c index 4969dc10684a..c5cd635c28f3 100644 --- a/drivers/spi/spi-sun4i.c +++ b/drivers/spi/spi-sun4i.c @@ -46,6 +46,8 @@ #define SUN4I_CTL_TP BIT(18) #define SUN4I_INT_CTL_REG 0x0c +#define SUN4I_INT_CTL_RF_F34 BIT(4) +#define SUN4I_INT_CTL_TF_E34 BIT(12) #define SUN4I_INT_CTL_TC BIT(16) #define SUN4I_INT_STA_REG 0x10 @@ -61,11 +63,14 @@ #define SUN4I_CLK_CTL_CDR1(div) (((div) & SUN4I_CLK_CTL_CDR1_MASK) << 8) #define SUN4I_CLK_CTL_DRS BIT(12) +#define SUN4I_MAX_XFER_SIZE 0xffffff + #define SUN4I_BURST_CNT_REG 0x20 -#define SUN4I_BURST_CNT(cnt) ((cnt) & 0xffffff) +#define SUN4I_BURST_CNT(cnt) ((cnt) & SUN4I_MAX_XFER_SIZE) #define SUN4I_XMIT_CNT_REG 0x24 -#define SUN4I_XMIT_CNT(cnt) ((cnt) & 0xffffff) +#define SUN4I_XMIT_CNT(cnt) ((cnt) & SUN4I_MAX_XFER_SIZE) + #define SUN4I_FIFO_STA_REG 0x28 #define SUN4I_FIFO_STA_RF_CNT_MASK 0x7f @@ -96,6 +101,31 @@ static inline void sun4i_spi_write(struct sun4i_spi *sspi, u32 reg, u32 value) writel(value, sspi->base_addr + reg); } +static inline u32 sun4i_spi_get_tx_fifo_count(struct sun4i_spi *sspi) +{ + u32 reg = sun4i_spi_read(sspi, SUN4I_FIFO_STA_REG); + + reg >>= SUN4I_FIFO_STA_TF_CNT_BITS; + + return reg & SUN4I_FIFO_STA_TF_CNT_MASK; +} + +static inline void sun4i_spi_enable_interrupt(struct sun4i_spi *sspi, u32 mask) +{ + u32 reg = sun4i_spi_read(sspi, SUN4I_INT_CTL_REG); + + reg |= mask; + sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, reg); +} + +static inline void sun4i_spi_disable_interrupt(struct sun4i_spi *sspi, u32 mask) +{ + u32 reg = sun4i_spi_read(sspi, SUN4I_INT_CTL_REG); + + reg &= ~mask; + sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, reg); +} + static inline void sun4i_spi_drain_fifo(struct sun4i_spi *sspi, int len) { u32 reg, cnt; @@ -118,10 +148,13 @@ static inline void sun4i_spi_drain_fifo(struct sun4i_spi *sspi, int len) static inline void sun4i_spi_fill_fifo(struct sun4i_spi *sspi, int len) { + u32 cnt; u8 byte; - if (len > sspi->len) - len = sspi->len; + /* See how much data we can fit */ + cnt = SUN4I_FIFO_DEPTH - sun4i_spi_get_tx_fifo_count(sspi); + + len = min3(len, (int)cnt, sspi->len); while (len--) { byte = sspi->tx_buf ? *sspi->tx_buf++ : 0; @@ -184,10 +217,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 > SUN4I_MAX_XFER_SIZE) return -EMSGSIZE; - if (tfr->tx_buf && tfr->len >= SUN4I_FIFO_DEPTH) + if (tfr->tx_buf && tfr->len >= SUN4I_MAX_XFER_SIZE) return -EMSGSIZE; reinit_completion(&sspi->done); @@ -286,7 +319,11 @@ static int sun4i_spi_transfer_one(struct spi_master *master, sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH - 1); /* Enable the interrupts */ - sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, SUN4I_INT_CTL_TC); + sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TC | + SUN4I_INT_CTL_RF_F34); + /* Only enable Tx FIFO interrupt if we really need it */ + if (tx_len > SUN4I_FIFO_DEPTH) + sun4i_spi_enable_interrupt(sspi, SUN4I_INT_CTL_TF_E34); /* Start the transfer */ reg = sun4i_spi_read(sspi, SUN4I_CTL_REG); @@ -306,7 +343,6 @@ static int sun4i_spi_transfer_one(struct spi_master *master, goto out; } - sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH); out: sun4i_spi_write(sspi, SUN4I_INT_CTL_REG, 0); @@ -322,10 +358,33 @@ static irqreturn_t sun4i_spi_handler(int irq, void *dev_id) /* Transfer complete */ if (status & SUN4I_INT_CTL_TC) { sun4i_spi_write(sspi, SUN4I_INT_STA_REG, SUN4I_INT_CTL_TC); + sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH); complete(&sspi->done); return IRQ_HANDLED; } + /* Receive FIFO 3/4 full */ + if (status & SUN4I_INT_CTL_RF_F34) { + sun4i_spi_drain_fifo(sspi, SUN4I_FIFO_DEPTH); + /* Only clear the interrupt _after_ draining the FIFO */ + sun4i_spi_write(sspi, SUN4I_INT_STA_REG, SUN4I_INT_CTL_RF_F34); + return IRQ_HANDLED; + } + + /* Transmit FIFO 3/4 empty */ + if (status & SUN4I_INT_CTL_TF_E34) { + sun4i_spi_fill_fifo(sspi, SUN4I_FIFO_DEPTH); + + if (!sspi->len) + /* nothing left to transmit */ + sun4i_spi_disable_interrupt(sspi, SUN4I_INT_CTL_TF_E34); + + /* Only clear the interrupt _after_ re-seeding the FIFO */ + sun4i_spi_write(sspi, SUN4I_INT_STA_REG, SUN4I_INT_CTL_TF_E34); + + return IRQ_HANDLED; + } + return IRQ_NONE; } -- 2.10.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size 2016-10-26 7:00 [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size Alexandru Gagniuc 2016-10-26 7:00 ` [PATCH] " Alexandru Gagniuc @ 2016-10-26 8:55 ` Maxime Ripard 2016-10-27 11:14 ` Mark Brown 2016-10-26 10:07 ` Mark Brown 2 siblings, 1 reply; 11+ messages in thread From: Maxime Ripard @ 2016-10-26 8:55 UTC (permalink / raw) To: Alexandru Gagniuc Cc: broonie, wens, linux-spi, linux-arm-kernel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 976 bytes --] On Wed, Oct 26, 2016 at 12:00:30AM -0700, Alexandru Gagniuc wrote: > This is the seventh attempt to get this patch in. I was prompted to look > into this again as someone recently remarked: > > " cool, to bad spi does not work properly on allwinner..." > > Two and a half years ago, we were told this will all be handled with DMA. > A year or so ago, we were told this will all be handled with DMA. > A week or so ago, we were told this will all be handled with DMA. > > See the pattern? > When DMA finally takes over, this fallback path is not mutually exclusive. I definitely agree, and we had this patch in the CHIP kernel for quite some time, working like a charm. I was planning to respin it in the next few days, glad to see you took care of it :) Mark, any comments on this? For the record, it already has my Acked-by. Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size 2016-10-26 8:55 ` [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size Maxime Ripard @ 2016-10-27 11:14 ` Mark Brown 2016-10-27 21:27 ` Maxime Ripard 0 siblings, 1 reply; 11+ messages in thread From: Mark Brown @ 2016-10-27 11:14 UTC (permalink / raw) To: Maxime Ripard Cc: Alexandru Gagniuc, wens, linux-spi, linux-arm-kernel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1096 bytes --] On Wed, Oct 26, 2016 at 10:55:28AM +0200, Maxime Ripard wrote: > On Wed, Oct 26, 2016 at 12:00:30AM -0700, Alexandru Gagniuc wrote: > > When DMA finally takes over, this fallback path is not mutually exclusive. > I definitely agree, and we had this patch in the CHIP kernel for quite > some time, working like a charm. > I was planning to respin it in the next few days, glad to see you took > care of it :) > Mark, any comments on this? For the record, it already has my Acked-by. Without knowing what the previous discussion was it's hard to comment, it sounds like some prior review comments are just being ignored here but since I'm not turning up anything with this subject line I've no idea what that might have been (and that's very concerning in itself given that this is apparently v7...). I'm also concerned that there isn't a version of this for sun6i, it's going to make all the cut'n'pasting between the two drivers harder if we make changes in one and not the other. If the concern from the previous reviews to do with not using DMA is there some reason it's hard to do DMA? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size 2016-10-27 11:14 ` Mark Brown @ 2016-10-27 21:27 ` Maxime Ripard 2016-10-27 22:47 ` Mark Brown 0 siblings, 1 reply; 11+ messages in thread From: Maxime Ripard @ 2016-10-27 21:27 UTC (permalink / raw) To: Mark Brown Cc: Alexandru Gagniuc, wens, linux-spi, linux-arm-kernel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2132 bytes --] Hi Mark, On Thu, Oct 27, 2016 at 12:14:19PM +0100, Mark Brown wrote: > On Wed, Oct 26, 2016 at 10:55:28AM +0200, Maxime Ripard wrote: > > On Wed, Oct 26, 2016 at 12:00:30AM -0700, Alexandru Gagniuc wrote: > > > > When DMA finally takes over, this fallback path is not mutually exclusive. > > > I definitely agree, and we had this patch in the CHIP kernel for quite > > some time, working like a charm. > > > I was planning to respin it in the next few days, glad to see you took > > care of it :) > > > Mark, any comments on this? For the record, it already has my Acked-by. > > Without knowing what the previous discussion was it's hard to comment, > it sounds like some prior review comments are just being ignored here > but since I'm not turning up anything with this subject line I've no > idea what that might have been (and that's very concerning in itself > given that this is apparently v7...). v4 was here: https://patchwork.kernel.org/patch/3893371/ v5: https://patchwork.kernel.org/patch/5455381/ v6: https://patchwork.kernel.org/patch/6975871/ So basically, I really have no idea why, but it really seems like it was just falling through the cracks, repeatedly (I'm not puting the blame on anyone though, it just happened). Maybe it was just because of the lack of comments :) > I'm also concerned that there isn't a version of this for sun6i, > it's going to make all the cut'n'pasting between the two drivers > harder if we make changes in one and not the other. I think I'll give reg_field a shot though, and try to merge the sun6i driver into this one and see the results. If it can help your decision. > If the concern from the previous reviews to do with not using DMA is > there some reason it's hard to do DMA? I think just like Alexandru that it is orthogonal. But to really answer, no, it's not difficult. There's just been some fundamental disagreement on whether DMA was supposed to be optional or not that stalled everything I guess. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size 2016-10-27 21:27 ` Maxime Ripard @ 2016-10-27 22:47 ` Mark Brown 2016-11-02 17:47 ` Maxime Ripard 0 siblings, 1 reply; 11+ messages in thread From: Mark Brown @ 2016-10-27 22:47 UTC (permalink / raw) To: Maxime Ripard Cc: Alexandru Gagniuc, wens, linux-spi, linux-arm-kernel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1899 bytes --] On Thu, Oct 27, 2016 at 11:27:27PM +0200, Maxime Ripard wrote: > On Thu, Oct 27, 2016 at 12:14:19PM +0100, Mark Brown wrote: > > but since I'm not turning up anything with this subject line I've no > > idea what that might have been (and that's very concerning in itself > > given that this is apparently v7...). > v4 was here: https://patchwork.kernel.org/patch/3893371/ > v5: https://patchwork.kernel.org/patch/5455381/ > v6: https://patchwork.kernel.org/patch/6975871/ > So basically, I really have no idea why, but it really seems like it > was just falling through the cracks, repeatedly (I'm not puting the > blame on anyone though, it just happened). Maybe it was just because > of the lack of comments :) Oh, those subject lines were all starting ARM: rather than spi: - there's a good chance I didn't look at the patches if I was busy thinking they were changes for arch/arm rather than the SPI driver. > > I'm also concerned that there isn't a version of this for sun6i, > > it's going to make all the cut'n'pasting between the two drivers > > harder if we make changes in one and not the other. > I think I'll give reg_field a shot though, and try to merge the sun6i > driver into this one and see the results. If it can help your > decision. It would definitely be nice given the level of duplication. > > If the concern from the previous reviews to do with not using DMA is > > there some reason it's hard to do DMA? > I think just like Alexandru that it is orthogonal. But to really > answer, no, it's not difficult. There's just been some fundamental > disagreement on whether DMA was supposed to be optional or not that > stalled everything I guess. Oh, I seem to remember some patches adding DMA support that were doing some strange special snowflake thing with ignoring errors now that I think about it but that's not this one... why did nobody ever follow up on those? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size 2016-10-27 22:47 ` Mark Brown @ 2016-11-02 17:47 ` Maxime Ripard 0 siblings, 0 replies; 11+ messages in thread From: Maxime Ripard @ 2016-11-02 17:47 UTC (permalink / raw) To: Mark Brown Cc: Alexandru Gagniuc, wens, linux-spi, linux-arm-kernel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 933 bytes --] Hi Mark, On Thu, Oct 27, 2016 at 11:47:12PM +0100, Mark Brown wrote: > > > If the concern from the previous reviews to do with not using DMA is > > > there some reason it's hard to do DMA? > > > I think just like Alexandru that it is orthogonal. But to really > > answer, no, it's not difficult. There's just been some fundamental > > disagreement on whether DMA was supposed to be optional or not that > > stalled everything I guess. > > Oh, I seem to remember some patches adding DMA support that were doing > some strange special snowflake thing with ignoring errors now that I > think about it but that's not this one... why did nobody ever follow up > on those? I have no idea. I think last time we discussed this was last July. Especially with this in now, there's probably no rush anyway. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size 2016-10-26 7:00 [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size Alexandru Gagniuc 2016-10-26 7:00 ` [PATCH] " Alexandru Gagniuc 2016-10-26 8:55 ` [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size Maxime Ripard @ 2016-10-26 10:07 ` Mark Brown 2016-10-26 16:06 ` Alex Gagniuc 2 siblings, 1 reply; 11+ messages in thread From: Mark Brown @ 2016-10-26 10:07 UTC (permalink / raw) To: Alexandru Gagniuc Cc: maxime.ripard, wens, linux-spi, linux-arm-kernel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 499 bytes --] On Wed, Oct 26, 2016 at 12:00:30AM -0700, Alexandru Gagniuc wrote: > This is the seventh attempt to get this patch in. I was prompted to look > into this again as someone recently remarked: Please don't send cover letters for single patches, if there is anything that needs saying put it in the changelog of the patch or after the --- if it's administrative stuff. This reduces mail volume and ensures that any important information is recorded in the changelog rather than being lost. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size 2016-10-26 10:07 ` Mark Brown @ 2016-10-26 16:06 ` Alex Gagniuc 2016-10-26 16:45 ` Mark Brown 0 siblings, 1 reply; 11+ messages in thread From: Alex Gagniuc @ 2016-10-26 16:06 UTC (permalink / raw) To: Mark Brown Cc: Maxime Ripard, Chen-Yu Tsai, linux-spi, linux-arm-kernel, linux-kernel On Wed, Oct 26, 2016 at 3:07 AM, Mark Brown <broonie@kernel.org> wrote: > On Wed, Oct 26, 2016 at 12:00:30AM -0700, Alexandru Gagniuc wrote: >> This is the seventh attempt to get this patch in. I was prompted to look >> into this again as someone recently remarked: > > Please don't send cover letters for single patches, if there is anything > that needs saying put it in the changelog of the patch or after the --- > if it's administrative stuff. This reduces mail volume and ensures that > any important information is recorded in the changelog rather than being > lost. Thanks for the heads up! Sorry about that. Do you want me to resend this as a single mail? Alex ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size 2016-10-26 16:06 ` Alex Gagniuc @ 2016-10-26 16:45 ` Mark Brown 0 siblings, 0 replies; 11+ messages in thread From: Mark Brown @ 2016-10-26 16:45 UTC (permalink / raw) To: Alex Gagniuc Cc: Maxime Ripard, Chen-Yu Tsai, linux-spi, linux-arm-kernel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 179 bytes --] On Wed, Oct 26, 2016 at 09:06:31AM -0700, Alex Gagniuc wrote: > Thanks for the heads up! Sorry about that. Do you want me to resend > this as a single mail? It's fine, no need. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-11-02 17:47 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-26 7:00 [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size Alexandru Gagniuc 2016-10-26 7:00 ` [PATCH] " Alexandru Gagniuc 2016-10-29 18:12 ` Applied "spi: sun4i: Allow transfers larger than FIFO size" to the spi tree Mark Brown 2016-10-26 8:55 ` [PATCH v7] spi: sun4i: Allow transfers larger than FIFO size Maxime Ripard 2016-10-27 11:14 ` Mark Brown 2016-10-27 21:27 ` Maxime Ripard 2016-10-27 22:47 ` Mark Brown 2016-11-02 17:47 ` Maxime Ripard 2016-10-26 10:07 ` Mark Brown 2016-10-26 16:06 ` Alex Gagniuc 2016-10-26 16:45 ` Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).