Re: [PATCH 0/3] ARM: multi_v7_defconfig: Enable more OMAP family platforms
Tony Lindgren t...@atomide.com writes: Ping, looks like these are still pending. Probably should be applied directly by the arm-soc maintainers. If these should go through arm-soc, please resend to:a...@kernel.org so they make it into our queue of stuff to be reviewed/applied. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On 08/07/2015 03:17 PM, Russell King - ARM Linux wrote: On Fri, Aug 07, 2015 at 02:35:45PM +0200, Sebastian Andrzej Siewior wrote: On 08/07/2015 12:55 PM, Russell King - ARM Linux wrote: On Fri, Aug 07, 2015 at 10:41:57AM +0200, Sebastian Andrzej Siewior wrote: This DMA driver is used by 8250-omap on DRA7-evm. There is one requirement that is to pause a transfer. This is currently used on the RX side. It is possible that the UART HW aborted the RX (UART's RX-timeout) but the DMA controller starts the transfer shortly after. Before we can manually purge the FIFO we need to pause the transfer, check how many bytes it already received and terminate the transfer without it making any progress. From testing on the TX side it seems that it is possible that we invoke pause once the transfer has completed which is indicated by the missing CCR_ENABLE bit but before the interrupt has been noticed. In that case the interrupt will come even after disabling it. How do you cope with the OMAP DMA hardware clearing its FIFO when you pause it? I don't ... and so you introduce a silent data loss bug into the driver. That's not very clever. Right now the 820-omap (8250-dma in general, too but they don't use this driver) pause only the RX transfer in an error condition. This means it is only device-to-mem transfer. I only mentioned the TX transfer here since this was easier to test. That may be how 8250 works, but 8250 is not everything. You can't ignore this problem. You have to deal with it - either by not allowing a channel that would loose data to be paused, or by recovering from that condition. You're not doing either in your patch. Therefore, I have no other option but to NAK your change. Sorry. Please fix this. Would it be okay if I only allow pause for RX-transfers? For TX-transfers, I would need to update the start-address so the transfers begins where it stopped. However based on your concern I can't really assume that the position reported by the HW is the correct one. Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On 08/07/2015 03:22 PM, Russell King - ARM Linux wrote: On Fri, Aug 07, 2015 at 12:36:14PM +0200, Sebastian Andrzej Siewior wrote: On 08/07/2015 11:44 AM, Peter Ujfalusi wrote: with a short testing audio did not broke (the only user of pause/resume) Some comments embedded. Cc: sta...@vger.kernel.org Why stable? This is not fixing any bugs since the PAUSE was not allowed for non cyclic transfers. Hmmm. The DRA7x was using pause before for UART. I just did not see it coming that it was not allowed here. John made a similar change to the edma driver and I assumed it went stable but now I see that it was just cherry-picked into the ti tree. This is *NOT* stable material. Pause of these channels is something that omap-dma has *never* supported. Therefore, it is *not* a regression. What you are doing is *adding* a feature to the omap-dma driver. That is not stable material in any sense. Stable is for bug fixes to existing code, not feature enhancements. I didn't consider this as a feature. If something else has been converted to pause channels and that is causing a problem, then _that_ conversion is where the bug lies, not the lack of support in the omap-dma. So we had the 8250-DMA doing pause and all its current users implement it. We have a DMA driver tree which is not used and it not implementing pause (not implementing pause at all). Later we get a combo of 8250-DMA + DMA driver that is broken because the lack of pause and this is noticed a few kernel releases later. The only way of fixing the bug is by implementing the pause feature. Now you are saying that even if I implement this missing feature in a newer kernel I am not allowed to mark it stable despite the fact that it fixes an existing problem in older kernels because it is not a regression. If it's a result of using some new driver with omap-dma, then the problem is with whatever introduced that new combination - it's not that omap-dma is buggy. Don't fix bugs in -stable by adding features. That's _no_ way to fix bugs. NAK on this feature patch having any kind of stable tag. I already accepted this. Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On Fri, Aug 07, 2015 at 03:42:06PM +0200, Sebastian Andrzej Siewior wrote: On 08/07/2015 03:22 PM, Russell King - ARM Linux wrote: On Fri, Aug 07, 2015 at 12:36:14PM +0200, Sebastian Andrzej Siewior wrote: On 08/07/2015 11:44 AM, Peter Ujfalusi wrote: with a short testing audio did not broke (the only user of pause/resume) Some comments embedded. Cc: sta...@vger.kernel.org Why stable? This is not fixing any bugs since the PAUSE was not allowed for non cyclic transfers. Hmmm. The DRA7x was using pause before for UART. I just did not see it coming that it was not allowed here. John made a similar change to the edma driver and I assumed it went stable but now I see that it was just cherry-picked into the ti tree. This is *NOT* stable material. Pause of these channels is something that omap-dma has *never* supported. Therefore, it is *not* a regression. What you are doing is *adding* a feature to the omap-dma driver. That is not stable material in any sense. Stable is for bug fixes to existing code, not feature enhancements. I didn't consider this as a feature. As it is something that the driver has _not_ supported, you are clearly adding a feature to an existing driver. It's not a bug fix. If something else has been converted to pause channels and that is causing a problem, then _that_ conversion is where the bug lies, not the lack of support in the omap-dma. So we had the 8250-DMA doing pause and all its current users implement it. We have a DMA driver tree which is not used and it not implementing pause (not implementing pause at all). Later we get a combo of 8250-DMA + DMA driver that is broken because the lack of pause and this is noticed a few kernel releases later. Right, so the patch which caused the regression is the one which arranged for the 8250-dma + omap-dma combination to work together, not the missing pause support in omap-dma. It doesn't matter that it's several releases old, it's that change caused the regression you have today. That change is incorrect today, and it was just as incorrect at the time that it was merged. The only way of fixing the bug is by implementing the pause feature. That's not the only way of fixing the bug. As the binding of drivers is controlled by DT, you can disable the binding of these two drivers there and 8250 will go back to using non-DMA mode - which is the situation which existed prior to the change which coupled the two drivers together. That's an acceptable change for -stable trees, because it's reverting the change which caused the regression, taking us back to a situation we _know_ worked previously. Then, in mainline during the next merge window, we can introduce the pause feature to omap-dma, and re-enable the 8250 driver to use it. _Once_ that's proven stable, we can then take a view whether those changes should _then_ be backported to stable kernels with greater confidence that backporting the feature addition won't itself cause a new regression. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On Fri, Aug 07, 2015 at 02:35:45PM +0200, Sebastian Andrzej Siewior wrote: On 08/07/2015 12:55 PM, Russell King - ARM Linux wrote: On Fri, Aug 07, 2015 at 10:41:57AM +0200, Sebastian Andrzej Siewior wrote: This DMA driver is used by 8250-omap on DRA7-evm. There is one requirement that is to pause a transfer. This is currently used on the RX side. It is possible that the UART HW aborted the RX (UART's RX-timeout) but the DMA controller starts the transfer shortly after. Before we can manually purge the FIFO we need to pause the transfer, check how many bytes it already received and terminate the transfer without it making any progress. From testing on the TX side it seems that it is possible that we invoke pause once the transfer has completed which is indicated by the missing CCR_ENABLE bit but before the interrupt has been noticed. In that case the interrupt will come even after disabling it. How do you cope with the OMAP DMA hardware clearing its FIFO when you pause it? I don't ... and so you introduce a silent data loss bug into the driver. That's not very clever. Right now the 820-omap (8250-dma in general, too but they don't use this driver) pause only the RX transfer in an error condition. This means it is only device-to-mem transfer. I only mentioned the TX transfer here since this was easier to test. That may be how 8250 works, but 8250 is not everything. You can't ignore this problem. You have to deal with it - either by not allowing a channel that would loose data to be paused, or by recovering from that condition. You're not doing either in your patch. Therefore, I have no other option but to NAK your change. Sorry. Please fix this. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On Fri, Aug 07, 2015 at 12:36:14PM +0200, Sebastian Andrzej Siewior wrote: On 08/07/2015 11:44 AM, Peter Ujfalusi wrote: with a short testing audio did not broke (the only user of pause/resume) Some comments embedded. Cc: sta...@vger.kernel.org Why stable? This is not fixing any bugs since the PAUSE was not allowed for non cyclic transfers. Hmmm. The DRA7x was using pause before for UART. I just did not see it coming that it was not allowed here. John made a similar change to the edma driver and I assumed it went stable but now I see that it was just cherry-picked into the ti tree. This is *NOT* stable material. Pause of these channels is something that omap-dma has *never* supported. Therefore, it is *not* a regression. What you are doing is *adding* a feature to the omap-dma driver. That is not stable material in any sense. Stable is for bug fixes to existing code, not feature enhancements. If something else has been converted to pause channels and that is causing a problem, then _that_ conversion is where the bug lies, not the lack of support in the omap-dma. If it's a result of using some new driver with omap-dma, then the problem is with whatever introduced that new combination - it's not that omap-dma is buggy. Don't fix bugs in -stable by adding features. That's _no_ way to fix bugs. NAK on this feature patch having any kind of stable tag. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On Fri, Aug 07, 2015 at 03:22:56PM +0200, Sebastian Andrzej Siewior wrote: On 08/07/2015 03:17 PM, Russell King - ARM Linux wrote: On Fri, Aug 07, 2015 at 02:35:45PM +0200, Sebastian Andrzej Siewior wrote: On 08/07/2015 12:55 PM, Russell King - ARM Linux wrote: On Fri, Aug 07, 2015 at 10:41:57AM +0200, Sebastian Andrzej Siewior wrote: This DMA driver is used by 8250-omap on DRA7-evm. There is one requirement that is to pause a transfer. This is currently used on the RX side. It is possible that the UART HW aborted the RX (UART's RX-timeout) but the DMA controller starts the transfer shortly after. Before we can manually purge the FIFO we need to pause the transfer, check how many bytes it already received and terminate the transfer without it making any progress. From testing on the TX side it seems that it is possible that we invoke pause once the transfer has completed which is indicated by the missing CCR_ENABLE bit but before the interrupt has been noticed. In that case the interrupt will come even after disabling it. How do you cope with the OMAP DMA hardware clearing its FIFO when you pause it? I don't ... and so you introduce a silent data loss bug into the driver. That's not very clever. Right now the 820-omap (8250-dma in general, too but they don't use this driver) pause only the RX transfer in an error condition. This means it is only device-to-mem transfer. I only mentioned the TX transfer here since this was easier to test. That may be how 8250 works, but 8250 is not everything. You can't ignore this problem. You have to deal with it - either by not allowing a channel that would loose data to be paused, or by recovering from that condition. You're not doing either in your patch. Therefore, I have no other option but to NAK your change. Sorry. Please fix this. Would it be okay if I only allow pause for RX-transfers? Yes, that satisfies one of my suggestions above. For TX-transfers, I would need to update the start-address so the transfers begins where it stopped. However based on your concern I can't really assume that the position reported by the HW is the correct one. Exactly - I don't believe that existing OMAP DMA hardware can ever support pausing a mem-to-device transfer without data loss as you have no idea how many bytes have been prefetched by the DMA, and therefore you have no idea how many bytes to unwind the hardware position. It gets worse than that when you have to cross into the previous descriptor. It's really not nice. So, disallowing pause for mem-to-device is entirely reasonable given the data loss implications. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] tty: serial: 8250_omap: do not use RX DMA if pause is not supported
The 8250-omap driver requires the DMA-engine driver to support the pause
command in order to properly turn off programmed RX transfer before the
driver stars manually reading from the FIFO.
The lacking support of the requirement has been discovered recently. In
order to stay safe here we disable support for RX-DMA as soon as we
notice that it does not work. This should happen very early.
If the user does not want to see this backtrace he can either disable
DMA support (completely or RX-only) or backport the required patches for
edma / omap-dma once they hit mainline.
Cc: sta...@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
drivers/tty/serial/8250/8250_omap.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_omap.c
b/drivers/tty/serial/8250/8250_omap.c
index 0340ee6ba970..07a11e0935e4 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -112,6 +112,7 @@ struct omap8250_priv {
struct work_struct qos_work;
struct uart_8250_dma omap8250_dma;
spinlock_t rx_dma_lock;
+ bool rx_dma_broken;
};
static u32 uart_read(struct uart_8250_port *up, u32 reg)
@@ -761,6 +762,7 @@ static void omap_8250_rx_dma_flush(struct uart_8250_port *p)
struct omap8250_priv*priv = p-port.private_data;
struct uart_8250_dma*dma = p-dma;
unsigned long flags;
+ int ret;
spin_lock_irqsave(priv-rx_dma_lock, flags);
@@ -769,7 +771,9 @@ static void omap_8250_rx_dma_flush(struct uart_8250_port *p)
return;
}
- dmaengine_pause(dma-rxchan);
+ ret = dmaengine_pause(dma-rxchan);
+ if (WARN_ON_ONCE(ret))
+ priv-rx_dma_broken = true;
spin_unlock_irqrestore(priv-rx_dma_lock, flags);
@@ -813,6 +817,9 @@ static int omap_8250_rx_dma(struct uart_8250_port *p,
unsigned int iir)
break;
}
+ if (priv-rx_dma_broken)
+ return -EINVAL;
+
spin_lock_irqsave(priv-rx_dma_lock, flags);
if (dma-rx_running)
--
2.5.0
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
On 6 August 2015 at 23:33, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Thu, Aug 06, 2015 at 06:14:00PM +0200, Geert Uytterhoeven wrote: On Thu, Aug 6, 2015 at 3:51 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Thu, Aug 06, 2015 at 05:55:23PM +0530, Vignesh R wrote: On the whole following are my requirements: 1. to be able to communicate with non -flash SPI devices via config port ( this functionality is supported by current driver, I dont want to break it). Or pump any spi_message on to SPI bus directly. 2. take advantage of memory mapped port in order to increase read throughput( and use dma in future) when the slave is a m25p80 type flash. 3. handle m25p80 as well as other slave on multiple chipselects. I just need to know whether the user that requested the transfer is m25p80 driver. If yes, ti-qspi driver can take advantage of memory mapped interface, else just use config port to access SPI bus directly. The problem with this approach is that it's an abomination. It's adding a SPI-user specific hack which is detected by a specific driver. That's really not sane - what happens when we have lots of these kinds of I'm an X SPI-user with drivers detecting that? It's not maintainable in the long term. Yes, your requirements _today_ seem simple and easy, but you're only thinking about today, not tomorrow when you've moved on and someone else has to maintain the mess left behind (or delete it from mainline because they're sick of dealing with a hack.) The spi_message that is received in transfer_one_message() is too generic to imply the slave device that is on the other side of the wire. IMO, the read command does not imply that the slave is m25p80 flash (besides the read opcodes vary across vendors of m25p80 and across modes). I can see both sides of the argument. Mark is saying: if the SPI driver detects that the message to be transmitted is a read command followed by the appropriate number of dummy bytes, and then the data being read _and_ it's using quad-mode access, and the hardware generates _exactly_ that in hardware using the memory mapped mode, there is no reason _not_ to use the hardware to achieve that SPI transaction. The bus activity will be identical to what happens when the SPI controller is used manually to achieve that bus sequence. You're saying: but the documentation says you can't use it for anything except m25p80. If you look at 24.5.4.1.2, it tells you what the SFI generates on the bus, which is: 1. CS active 2. Read command byte sent 3. 1-4 address bytes sent 4. 0-3 dummy bytes sent 5. data bytes read from bus 6. CS inactive So, Mark's point is if we can detect a transaction which fits _that_ bus activity, there's no reason not to use this acceleration for the transaction. What you're failing to counter with is: we don't have enough information in the SPI driver to know how many dummy bytes there are between the address bytes and the data read from the bus. Irrespective of the dummy bytes. What if the spi device is not a FLASH ROM, but some other device, which receives a data packet that accidentally looks like an m25p80 READ command? Well, for the most part it looks like it should still work, but there could be a gotcha, but first, let's get rid of a myth there. The QSPI is _not_ specific to the M25P80. The manual says nothing about being specific to that device. What it says is that it's for SPI NOR memory. It will work with bus widths of 1, 2 or 4 data lines, so it probably works with non-M25P80 SPI NOR devices too - and the fact that the read and write commands are completely programmable suggests that using it with SPI NOR devices which do not use the M25P80 read command value is intended. ... That much is good, but now is the problem - how does the SFI know that we're going to require to read 32 bytes? I think the answer to that is that it doesn't know, so it probably just reads the number of bytes which the access on the SoC bus is asking for, which makes it indeterminant from a software point of view to control how many bytes will be read without provoking another send 0x01, next address, dummy byte sequence. So, I'm now on the side of not parsing commands in the SPI driver, and back on the idea that this needs to be handled in some other manner which doesn't involve polluting the SPI core with flag-hacks. OK, so we can agree that using this hardware acceleration for any kind of transfer indiscriminately is not a very good idea. Now since the description is clearer it's obvious that ti-qspi cannot work fully mmapped as fsl-qspi does because the setup has to be done over normal spi access and using non-m25p80 devices on the same bus is a requirement. The place where it is known if a transfer can use the mmap access is m25p80.c So my suggestion is - add a new method for spi master that
[PATCH v2 03/22] memory: omap-gpmc: Introduce GPMC to NAND interface
The OMAP GPMC module has certain registers dedicated for NAND
access and some NAND bits mixed with other GPMC functionality.
For the NAND dedicated registers we have the struct gpmc_nand_regs.
The NAND driver needs to access NAND specific bits from the
following non-dedicated registers
1) FIFOEVENT and TERMCOUNT from GPMC_IRQENABLE and GPMC_IRQSTATUS
2) EMPTYWRITEBUFFERSTATUS from GPMC_STATUS
For accessing these bits we introduce the struct gpmc_nand_ops.
Rename the gpmc_update_nand_reg() API to gpmc_omap_get_nand_ops()
and make it return the gpmc_nand_ops along with updating the
gpmc_nand_regs. This API will be called by the OMAP NAND driver
to access the necessary bits in GPMC register space.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/memory/omap-gpmc.c | 21 +
include/linux/omap-gpmc.h | 42 --
2 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index 3a27a84..79d78ab 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -1099,6 +1099,27 @@ void gpmc_update_nand_reg(struct gpmc_nand_regs *reg,
int cs)
}
}
+static struct gpmc_nand_ops nand_ops;
+
+/**
+ * gpmc_omap_get_nand_ops - Get the GPMC NAND interface
+ * @regs: the GPMC NAND register map exclusive for NAND use.
+ * @cs: GPMC chip select number on which the NAND sits. The
+ * register map returned will be specific to this chip select.
+ *
+ * Returns NULL on error e.g. invalid cs.
+ */
+struct gpmc_nand_ops *gpmc_omap_get_nand_ops(struct gpmc_nand_regs *reg, int
cs)
+{
+ if (cs = gpmc_cs_num)
+ return NULL;
+
+ gpmc_update_nand_reg(reg, cs);
+
+ return nand_ops;
+}
+EXPORT_SYMBOL_GPL(gpmc_omap_get_nand_ops);
+
int gpmc_get_client_irq(unsigned irq_config)
{
int i;
diff --git a/include/linux/omap-gpmc.h b/include/linux/omap-gpmc.h
index 2dcef1c..a811c91 100644
--- a/include/linux/omap-gpmc.h
+++ b/include/linux/omap-gpmc.h
@@ -14,14 +14,52 @@
#define GPMC_IRQ_FIFOEVENTENABLE 0x01
#define GPMC_IRQ_COUNT_EVENT 0x02
+enum gpmc_nand_irq {
+ GPMC_NAND_IRQ_FIFOEVENT = 0,
+ GPMC_NAND_IRQ_TERMCOUNT,
+};
+
+/**
+ * gpmc_nand_ops - Interface between NAND and GPMC
+ * @nand_irq_enable: enable the requested GPMC NAND interrupt event.
+ * @nand_irq_disable: disable the requested GPMC NAND interrupt event.
+ * @nand_irq_clear: clears the GPMC NAND interrupt event status.
+ * @nand_irq_status: get the NAND interrupt event status.
+ * @nand_write_buffer_empty: get the NAND write buffer empty status.
+ */
+struct gpmc_nand_ops {
+ int (*nand_irq_enable)(enum gpmc_nand_irq irq);
+ int (*nand_irq_disable)(enum gpmc_nand_irq irq);
+ void (*nand_irq_clear)(enum gpmc_nand_irq irq);
+ u32 (*nand_irq_status)(void);
+ bool (*nand_writebuffer_empty)(void);
+};
+
+struct gpmc_nand_regs;
+
+#if IS_ENABLED(CONFIG_OMAP_GPMC)
+struct gpmc_nand_ops *gpmc_omap_get_nand_ops(struct gpmc_nand_regs *regs,
+int cs);
+#else
+static inline gpmc_nand_ops *gpmc_omap_get_nand_ops(struct gpmc_nand_regs
*regs,
+ int cs);
+{
+ return NULL;
+}
+#endif /* CONFIG_OMAP_GPMC */
+
+/**/
+
+/* deprecated APIs */
+extern void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int cs);
+/**/
+
extern int gpmc_calc_timings(struct gpmc_timings *gpmc_t,
struct gpmc_settings *gpmc_s,
struct gpmc_device_timings *dev_t);
-struct gpmc_nand_regs;
struct device_node;
-extern void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int cs);
extern int gpmc_get_client_irq(unsigned irq_config);
extern unsigned int gpmc_ticks_to_ns(unsigned int ticks);
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [4.2-rc1][PATCH] gpio: omap: add missed spin_unlock_irqrestore in omap_gpio_irq_type
Hi Tony,
On 08/07/2015 06:36 AM, Tony Lindgren wrote:
* Linus Walleij linus.wall...@linaro.org [150716 01:38]:
On Wed, Jun 24, 2015 at 4:54 PM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
From: Grygorii Strashko grygorii.stras...@linaro.org
Add missed spin_unlock_irqrestore in omap_gpio_irq_type when
omap_set_gpio_triggering() is failed.
It fixes static checker warning:
drivers/gpio/gpio-omap.c:523 omap_gpio_irq_type()
warn: inconsistent returns 'spin_lock:bank-lock'.
This fixes commit:
1562e4618ded ('gpio: omap: fix error handling in omap_gpio_irq_type')
Reported-by: Javier Martinez Canillas jav...@dowhile0.org
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
Patch applied for fixes.
Linus, looks like we now have a new build warning in Linux next
with the fixes and raw_spinlock_t change, so a merge or a commit
is needed. If you prefer a patch, here's one below.
Yes. It seems merge/rebase issue between fixes next:
- this patch went through fixes and RAW spinlock conversation
patch through -next, and without merge conflicts.
and patch has been posted already by Axel Lin:
http://www.spinics.net/lists/linux-omap/msg121031.html
8 ---
From: Tony Lindgren t...@atomide.com
Date: Thu, 6 Aug 2015 20:32:24 -0700
Subject: [PATCH] gpio: omap: Fix build warning for raw_spinlock_t conversion
and unlock fix
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Looks like we now have a new build warning in Linux next with fix
977bd8a94c40 (gpio: omap: add missed spin_unlock_irqrestore in
omap_gpio_irq_type) and raw_spinlock changes done in commit
4dbada2be460 (gpio: omap: use raw locks for locking):
drivers/gpio/gpio-omap.c: In function ‘omap_gpio_irq_type’:
drivers/gpio/gpio-omap.c:504:26: warning: passing argument 1 of
‘spin_unlock_irqrestore’ from incompatible pointer type
[-Wincompatible-pointer-types]
spin_unlock_irqrestore(bank-lock, flags);
^
In file included from include/linux/seqlock.h:35:0,
from include/linux/time.h:5,
from include/linux/stat.h:18,
from include/linux/module.h:10,
from drivers/gpio/gpio-omap.c:16:
include/linux/spinlock.h:370:122: note: expected
‘spinlock_t * {aka struct spinlock *}’ but argument is of type
‘raw_spinlock_t * {aka struct raw_spinlock *}’
Fix the issue with using raw_spinlock_t instead.
Signed-off-by: Tony Lindgren t...@atomide.com
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -501,7 +501,7 @@ static int omap_gpio_irq_type(struct irq_data *d, unsigned
type)
raw_spin_lock_irqsave(bank-lock, flags);
retval = omap_set_gpio_triggering(bank, offset, type);
if (retval) {
- spin_unlock_irqrestore(bank-lock, flags);
+ raw_spin_unlock_irqrestore(bank-lock, flags);
goto error;
}
omap_gpio_init_irq(bank, offset);
--
To unsubscribe from this list: send the line unsubscribe linux-gpio in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
regards,
-grygorii
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
On 8/6/2015 23:33, Russell King - ARM Linux wrote: On Thu, Aug 06, 2015 at 06:14:00PM +0200, Geert Uytterhoeven wrote: Irrespective of the dummy bytes. What if the spi device is not a FLASH ROM, but some other device, which receives a data packet that accidentally looks like an m25p80 READ command? Well, for the most part it looks like it should still work, but there could be a gotcha, but first, let's get rid of a myth there. The QSPI is _not_ specific to the M25P80. The manual says nothing about being specific to that device. What it says is that it's for SPI NOR memory. It will work with bus widths of 1, 2 or 4 data lines, so it probably works with non-M25P80 SPI NOR devices too - and the fact that the read and write commands are completely programmable suggests that using it with SPI NOR devices which do not use the M25P80 read command value is intended. The SFI is a state machine based translator which sits behind the SPI interface (look at the manual). It sequence sthe SPI bus through a series of standard SPI states which happen to be the states I detailed above. Now, the first byte of the SFI-generated SPI message can be programmed to any 8 bit value. So the first byte of the SPI message is totally under software control. The next one to four bytes which comprise the address can be controlled to by deciding where in the memory map to start reading from. Hence, the value of those bytes is also totally under software control. The number of dummy bytes can be programmed too. So far so good. So, if we know that we have a SPI message which says send 0x01 0x20 0x30, send one dummy byte, read 32 bytes, if we program the SFI to send a read command as 0x01, program an address length of 2 bytes with one dummy byte, and then read the next 32 bytes at the appropriate offset in the memory mapping to cause the next two bytes to be 0x20, 0x30, then what we end up with on the bus is: send 0x01, 0x20, 0x30 send one dummy byte That much is good, but now is the problem - how does the SFI know that we're going to require to read 32 bytes? I think the answer to that is that it doesn't know, so it probably just reads the number of bytes which the access on the SoC bus is asking for, which makes it indeterminant from a software point of view to control how many bytes will be read without provoking another send 0x01, next address, dummy byte sequence. So, I'm now on the side of not parsing commands in the SPI driver, and back on the idea that this needs to be handled in some other manner which doesn't involve polluting the SPI core with flag-hacks. So I see 2 distinct options: Have the nor driver modified to run SPI commands and then ask the SPI framework (and driver) to switch into mmap mode: Would probably look something like this inside the nor driver: /* lock spi bus for other activities */ spi_bus_lock(spi); /* send the configuration for mmap */ t[0].tx_buf = flash-command; t[0].len = m25p_cmdsz(nor); spi_message_add_tail(t[0], m); t[1].tx_buf = dummy_buffer; t[1].len = dummy; spi_message_add_tail(t[1], m); spi_sync(spi, m); /* switch to mmap mode */ spi-mode |= SPI_MMAP; spi_setup(spi); /* run the mmapped transfers bypassing the spi-layer */ memcpy(...) /* open questions here: which address range * and how to detect if transfer is done */ /* restore back to normal mode */ spi-mode = ~SPI_MMAP; spi_setup(spi); /* unlock spi bus for other activities */ spi_bus_unlock(spi); The downside is that it requires modification in several places (nor-framework, spi-framework plus the driver) and it would not be generic enough... IMO such a situation is feasible if we only got a single device on the spi-bus, but leaves a lot of questions open... Alternatively we could create an additional api. On the other end of spectrum could be a solution where the spi-master driverwould have the opportunity to query the device-tree for specific propertiesduring the spi_add_device phase - in this case querying the followingproperty in the device-tree: spi-master-XXX,use-mmap-cmd-mode = 0x08 0x38; to implement mmap-mode for commands 0x08 and 0x38. Maybe we would want to also encode the number of address bytes to send per command without hardcoding those values explicitly: so maybe something like: spi-master-XXX,use-mmap-cmd-mode = 0x08 2 0x38 3; Obviously these would need to get documented in the bindings documentation of that driver. Alternatively we could also introduce generic alternate modes for the driver(similar to GPIO - ALT modes), but that would be less transparent and more hard-coded... In the end this would mean that from the nor framework side therewould be no change at all - it still would be issuing something like this: /* send the normal block read command */ t[0].tx_buf = flash-command; /* note that the address would be encoded here */ t[0].len = m25p_cmdsz(nor); spi_message_add_tail(t[0], m);
Re: [PATCH 6/7] phy: omap-usb2: Add a new compatible string for USB2 PHY2
On 05/08/15 17:18, Kishon Vijay Abraham I wrote:
Hi Roger,
On Wednesday 05 August 2015 01:55 PM, Roger Quadros wrote:
On 05/08/15 11:23, Roger Quadros wrote:
On 04/08/15 18:20, Kishon Vijay Abraham I wrote:
The USB2 PHY2 has a different register map compared to USB2 PHY1
to power on/off the PHY. In order to handle it, add a new
compatible string.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
Documentation/devicetree/bindings/phy/ti-phy.txt |2 ++
drivers/phy/phy-omap-usb2.c |9 +
2 files changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt
b/Documentation/devicetree/bindings/phy/ti-phy.txt
index 49e5b0c..a061077 100644
--- a/Documentation/devicetree/bindings/phy/ti-phy.txt
+++ b/Documentation/devicetree/bindings/phy/ti-phy.txt
@@ -31,6 +31,8 @@ OMAP USB2 PHY
Required properties:
- compatible: Should be ti,omap-usb2
+ Should be ti,dra7x-usb2-phy2 for the 2nd instance of USB2 PHY
+ in DRA7x
- reg : Address and length of the register set for the device.
- #phy-cells: determine the number of cells that should be given in the
phandle while referencing this phy.
diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index b5c266a..2f7220f 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -159,6 +159,11 @@ static const struct usb_phy_data dra7x_usb2_data = {
.flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
};
+static const struct usb_phy_data dra7x_usb2_phy2_data = {
+ .label = dra7x_usb2_phy2,
+ .flags = OMAP_USB2_CALIBRATE_FALSE_DISCONNECT,
+};
+
static const struct usb_phy_data am437x_usb2_data = {
.label = am437x_usb2,
.flags = 0,
@@ -178,6 +183,10 @@ static const struct of_device_id omap_usb2_id_table[]
= {
.data = dra7x_usb2_data,
},
{
+ .compatible = ti,dra7x-usb2-phy2,
+ .data = dra7x_usb2_phy2_data,
Why is this needed? You can reuse dra7x_usb2_data as is.
OK. I see why we need it in the next patch.
Probably both patches could be squashed.
What does .label indicate?
it's actually used only in usb_add_phy() (drivers/usb/phy/phy.c). I don't
think
it's actually important unless you strongly feel so.
I leave it to you then :).
cheers,
-roger
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] memory: omap-gpmc: Don't try to save the GPMC context
On 05/08/15 15:24, Tomeu Vizoso wrote:
...if there isn't one already.
If for some reason the GPMC device hasn't been probed yet, gpmc_base is
going to be NULL. Because there's no context yet to be saved, just turn
these functions into no-ops until that device gets probed.
Unable to handle kernel NULL pointer dereference at virtual address 0010
pgd = c0204000
[0010] *pgd=
Internal error: Oops: 5 [#1] SMP ARM
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.2.0-rc5-next-20150804-05947-g23f38fe8eda9 #1
Hardware name: Generic OMAP3-GP (Flattened Device Tree)
task: c0e623e8 ti: c0e5c000 task.ti: c0e5c000
PC is at omap3_gpmc_save_context+0x8/0xc4
LR is at omap_sram_idle+0x154/0x23c
pc : [c087c7ac]lr : [c023262c]psr: 6193
sp : c0e5df40 ip : c0f92a80 fp : c0999eb0
r10: c0e57364 r9 : c0e66f14 r8 : 0003
r7 : r6 : 0003 r5 : r4 : c0f5f174
r3 : c0fa4fe8 r2 : r1 : r0 : fa200280
Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 10c5387d Table: 80204019 DAC: 0015
Process swapper/0 (pid: 0, stack limit = 0xc0e5c220)
Stack: (0xc0e5df40 to 0xc0e5e000)
df40: c0e66ef8 c0f5f1a4 0003 c02333a4 c3813822
df60: c0e5a5c8 cfb8a5d0 c07f0c44 0e4f1d7e
df80: c3813822 cfb8a5d0 c0e5e4e4 cfb8a5d0 c0e66f14 c0e5a5c8 c0e5e54c
dfa0: c0e5e544 c0e57364 c0999eb0 c0277758 00fa c0f5d000 c0d61c18
dfc0: c0d61674 c0df7a48 c0f5d5d4
dfe0: c0e5e4c0 c0df7a44 c0e634f8 80204059 8020807c
[c087c7ac] (omap3_gpmc_save_context) from [c023262c]
(omap_sram_idle+0x154/0x23c)
[c023262c] (omap_sram_idle) from [c02333a4]
(omap3_enter_idle_bm+0xec/0x1a8)
[c02333a4] (omap3_enter_idle_bm) from [c07f0c44]
(cpuidle_enter_state+0xbc/0x284)
[c07f0c44] (cpuidle_enter_state) from [c0277758]
(cpu_startup_entry+0x174/0x24c)
[c0277758] (cpu_startup_entry) from [c0d61c18] (start_kernel+0x358/0x3c0)
[c0d61c18] (start_kernel) from [8020807c] (0x8020807c)
Code: c0ccace8 c0ccacc0 e59f30b4 e5932000 (e5921010)
Signed-off-by: Tomeu Vizoso tomeu.viz...@collabora.com
Suggested-by: Javier Martinez Canillas jav...@dowhile0.org
Acked-by: Roger Quadros rog...@ti.com
cheers,
-roger
---
drivers/memory/omap-gpmc.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index 3a27a84ad3ec..9426276dbe14 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -2245,6 +2245,9 @@ void omap3_gpmc_save_context(void)
{
int i;
+ if (!gpmc_base)
+ return;
+
gpmc_context.sysconfig = gpmc_read_reg(GPMC_SYSCONFIG);
gpmc_context.irqenable = gpmc_read_reg(GPMC_IRQENABLE);
gpmc_context.timeout_ctrl = gpmc_read_reg(GPMC_TIMEOUT_CONTROL);
@@ -2277,6 +2280,9 @@ void omap3_gpmc_restore_context(void)
{
int i;
+ if (!gpmc_base)
+ return;
+
gpmc_write_reg(GPMC_SYSCONFIG, gpmc_context.sysconfig);
gpmc_write_reg(GPMC_IRQENABLE, gpmc_context.irqenable);
gpmc_write_reg(GPMC_TIMEOUT_CONTROL, gpmc_context.timeout_ctrl);
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 16/22] memory: omap-gpmc: Support general purpose input for WAITPINs
OMAPs can have 2 to 4 WAITPINs that can be used as general purpose
input if not used for memory wait state insertion.
The first user will be the OMAP NAND chip to get the NAND
read/busy status using gpiolib.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/memory/omap-gpmc.c | 122 +++--
1 file changed, 117 insertions(+), 5 deletions(-)
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index 30d9c21..264009d 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -21,6 +21,7 @@
#include linux/spinlock.h
#include linux/io.h
#include linux/module.h
+#include linux/gpio/driver.h
#include linux/interrupt.h
#include linux/platform_device.h
#include linux/of.h
@@ -223,6 +224,11 @@ struct omap3_gpmc_regs {
struct gpmc_cs_config cs_context[GPMC_CS_NUM];
};
+struct gpmc_device {
+ struct device *dev;
+ struct gpio_chip gpio_chip;
+};
+
static struct resource gpmc_mem_root;
static struct gpmc_cs_data gpmc_cs[GPMC_CS_NUM];
static DEFINE_SPINLOCK(gpmc_mem_lock);
@@ -1919,10 +1925,69 @@ err:
return ret;
}
+static int gpmc_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
+{
+ return 1; /* we're input only */
+}
+
+static int gpmc_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ return 0; /* we're input only */
+}
+
+static int gpmc_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
+ int value)
+{
+ return -EINVAL; /* we're input only */
+}
+
+static void gpmc_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+}
+
+static int gpmc_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ u32 reg;
+
+ offset += 8;
+
+ reg = gpmc_read_reg(GPMC_STATUS) BIT(offset);
+
+ return !!reg;
+}
+
+static int gpmc_gpio_init(struct gpmc_device *gpmc)
+{
+ int ret;
+
+ gpmc-gpio_chip.dev = gpmc-dev;
+ gpmc-gpio_chip.owner = THIS_MODULE;
+ gpmc-gpio_chip.label = DEVICE_NAME;
+ gpmc-gpio_chip.ngpio = gpmc_nr_waitpins;
+ gpmc-gpio_chip.get_direction = gpmc_gpio_get_direction;
+ gpmc-gpio_chip.direction_input = gpmc_gpio_direction_input;
+ gpmc-gpio_chip.direction_output = gpmc_gpio_direction_output;
+ gpmc-gpio_chip.set = gpmc_gpio_set;
+ gpmc-gpio_chip.get = gpmc_gpio_get;
+ gpmc-gpio_chip.base = -1;
+
+ ret = gpiochip_add(gpmc-gpio_chip);
+ if (ret 0) {
+ dev_err(gpmc-dev, could not register gpio chip: %d\n, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static void gpmc_gpio_exit(struct gpmc_device *gpmc)
+{
+ gpiochip_remove(gpmc-gpio_chip);
+}
+
static int gpmc_probe_dt(struct platform_device *pdev)
{
int ret;
- struct device_node *child;
const struct of_device_id *of_id =
of_match_device(gpmc_dt_ids, pdev-dev);
@@ -1950,6 +2015,17 @@ static int gpmc_probe_dt(struct platform_device *pdev)
return ret;
}
+ dev_info(pdev-dev, num-cs %d, num-wait %d\n,
+gpmc_cs_num, gpmc_nr_waitpins);
+
+ return 0;
+}
+
+static int gpmc_probe_dt_children(struct platform_device *pdev)
+{
+ int ret;
+ struct device_node *child;
+
for_each_available_child_of_node(pdev-dev.of_node, child) {
if (!child-name)
@@ -1959,6 +2035,9 @@ static int gpmc_probe_dt(struct platform_device *pdev)
ret = gpmc_probe_onenand_child(pdev, child);
else
ret = gpmc_probe_generic_child(pdev, child);
+
+ if (ret)
+ return ret;
}
return 0;
@@ -1968,6 +2047,11 @@ static int gpmc_probe_dt(struct platform_device *pdev)
{
return 0;
}
+
+static int gpmc_probe_dt_children(struct platform_device *pdev)
+{
+ return 0;
+}
#endif
static int gpmc_probe(struct platform_device *pdev)
@@ -1975,6 +2059,7 @@ static int gpmc_probe(struct platform_device *pdev)
int rc;
u32 l;
struct resource *res;
+ struct gpmc_device *gpmc;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (res == NULL)
@@ -2005,6 +2090,17 @@ static int gpmc_probe(struct platform_device *pdev)
return -EINVAL;
}
+ rc = gpmc_probe_dt(pdev);
+ if (rc)
+ return rc;
+
+ gpmc = devm_kzalloc(pdev-dev, sizeof(*gpmc), GFP_KERNEL);
+ if (!gpmc)
+ return -ENOMEM;
+
+ gpmc-dev = pdev-dev;
+ platform_set_drvdata(pdev, gpmc);
+
pm_runtime_enable(pdev-dev);
pm_runtime_get_sync(pdev-dev);
@@ -2032,24 +2128,40 @@ static int gpmc_probe(struct platform_device *pdev)
GPMC_REVISION_MINOR(l));
gpmc_mem_init();
+ rc = gpmc_gpio_init(gpmc);
+ if (rc)
+ goto gpio_init_failed;
+
[PATCH v2 18/22] memory: omap-gpmc: Add irqchip support to the gpiochip
The WAIT pins support falling edge interrupts so add irqchip
support to the gpiochip model.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/memory/omap-gpmc.c | 111 +
1 file changed, 111 insertions(+)
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index 0df70ab..417acce 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -13,6 +13,7 @@
* published by the Free Software Foundation.
*/
#include linux/irq.h
+#include linux/irqdomain.h
#include linux/kernel.h
#include linux/init.h
#include linux/err.h
@@ -227,6 +228,7 @@ struct omap3_gpmc_regs {
struct gpmc_device {
struct device *dev;
struct gpio_chip gpio_chip;
+ struct irq_chip irq_chip;
};
static struct resource gpmc_mem_root;
@@ -1944,6 +1946,79 @@ err:
return ret;
}
+static int gpmc_irq_endis(unsigned long hwirq, bool endis)
+{
+ u32 regval;
+
+ /* WAITPIN starts at BIT 8 */
+ hwirq += 8;
+
+ regval = gpmc_read_reg(GPMC_IRQENABLE);
+ if (endis)
+ regval |= BIT(hwirq);
+ else
+ regval = ~BIT(hwirq);
+ gpmc_write_reg(GPMC_IRQENABLE, regval);
+
+ return 0;
+}
+
+static void gpmc_irq_mask(struct irq_data *d)
+{
+ gpmc_irq_endis(d-hwirq, false);
+}
+
+static void gpmc_irq_unmask(struct irq_data *d)
+{
+ gpmc_irq_endis(d-hwirq, true);
+}
+
+static void gpmc_irq_ack(struct irq_data *d)
+{
+ unsigned hwirq = d-hwirq + 8;
+
+ /* Setting bit to 1 clears (or Acks) the interrupt */
+ gpmc_write_reg(GPMC_IRQSTATUS, BIT(hwirq));
+}
+
+static int gpmc_irq_set_type(struct irq_data *d, unsigned trigger)
+{
+ /* We only support falling edge interrupts */
+ if (trigger ~IRQ_TYPE_EDGE_FALLING)
+ return -EINVAL;
+
+ return 0;
+}
+
+static irqreturn_t gpmc_handle_irq(int irq, void *data)
+{
+ int hwirq, virq;
+ u32 regval;
+ struct gpmc_device *gpmc = data;
+
+ regval = gpmc_read_reg(GPMC_IRQSTATUS);
+ regval = 8; /* we're only interested in WAIT pins */
+
+ if (!regval)
+ return IRQ_NONE;
+
+ for (hwirq = 0; hwirq gpmc-gpio_chip.ngpio; hwirq++) {
+ if (regval BIT(hwirq)) {
+ virq = irq_find_mapping(gpmc-gpio_chip.irqdomain,
+ hwirq);
+ if (!virq) {
+ dev_warn(gpmc_dev,
+spurious irq detected hwirq %d, virq
%d\n,
+hwirq, virq);
+ }
+
+ generic_handle_irq(virq);
+ }
+ }
+
+ return IRQ_HANDLED;
+}
+
static int gpmc_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
{
return 1; /* we're input only */
@@ -1978,6 +2053,7 @@ static int gpmc_gpio_get(struct gpio_chip *chip, unsigned
offset)
static int gpmc_gpio_init(struct gpmc_device *gpmc)
{
int ret;
+ u32 regval;
gpmc-gpio_chip.dev = gpmc-dev;
gpmc-gpio_chip.owner = THIS_MODULE;
@@ -1996,7 +2072,42 @@ static int gpmc_gpio_init(struct gpmc_device *gpmc)
return ret;
}
+ /* Disable interrupts */
+ gpmc_write_reg(GPMC_IRQENABLE, 0);
+
+ /* clear interrupts */
+ regval = gpmc_read_reg(GPMC_IRQSTATUS);
+ gpmc_write_reg(GPMC_IRQSTATUS, regval);
+
+ gpmc-irq_chip.name = DEVICE_NAME;
+ gpmc-irq_chip.irq_ack = gpmc_irq_ack;
+ gpmc-irq_chip.irq_mask = gpmc_irq_mask;
+ gpmc-irq_chip.irq_unmask = gpmc_irq_unmask;
+ gpmc-irq_chip.irq_set_type = gpmc_irq_set_type;
+
+ ret = gpiochip_irqchip_add(gpmc-gpio_chip, gpmc-irq_chip, 0,
+ handle_edge_irq, IRQ_TYPE_NONE);
+
+ if (ret) {
+ dev_err(gpmc-dev, could not add irqchip to gpiochip: %d\n,
+ ret);
+ goto fail;
+ }
+
+ /* We're sharing this IRQ with OMAP NAND driver */
+ ret = request_irq(gpmc_irq, gpmc_handle_irq, IRQF_SHARED, gpmc, gpmc);
+ if (ret) {
+ dev_err(gpmc-dev, could not request gpmc irq (%d): %d\n,
+ gpmc_irq, ret);
+ goto fail;
+ }
+
return 0;
+
+fail:
+ gpiochip_remove(gpmc-gpio_chip);
+
+ return ret;
}
static void gpmc_gpio_exit(struct gpmc_device *gpmc)
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 14/22] ARM: dts: OMAP2+: Fix NAND device nodes.
Add compatible id, GPMC register resource and interrupt
resource to NAND controller nodes.
TODO: For now only dra7-evm and omap3-beagle are fixed.
Once series is reviewed I'll update this patch to
fix all omap boards.
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/boot/dts/dra7-evm.dts | 4 +++-
arch/arm/boot/dts/omap3-beagle.dts | 3 ++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index 096f68b..ce11b0f 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -569,9 +569,11 @@
status = okay;
pinctrl-names = default;
pinctrl-0 = nand_flash_x16;
- ranges = 0 0 0 0x0100;/* minimum GPMC partition = 16MB */
+ ranges = 0 0 0x0800 0x0100; /* minimum GPMC partition =
16MB */
nand@0,0 {
+ compatible = ti,omap2-nand;
reg = 0 0 4; /* device IO registers */
+ interrupts = GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH;
ti,nand-ecc-opt = bch8;
ti,elm-id = elm;
nand-bus-width = 16;
diff --git a/arch/arm/boot/dts/omap3-beagle.dts
b/arch/arm/boot/dts/omap3-beagle.dts
index a547411..34cf55e 100644
--- a/arch/arm/boot/dts/omap3-beagle.dts
+++ b/arch/arm/boot/dts/omap3-beagle.dts
@@ -384,7 +384,8 @@
/* Chip select 0 */
nand@0,0 {
- reg = 0 0 4; /* NAND I/O window, 4 bytes */
+ compatible = ti,omap2-nand;
+ reg = 0 0 4; /* CS0, offset 0, IO size 4 */
interrupts = 20;
ti,nand-ecc-opt = ham1;
nand-bus-width = 16;
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 11/22] mtd: nand: omap: Clean up device tree support
Move NAND specific device tree parsing to NAND driver.
The NAND controller node must have a compatible id, register space
resource and interrupt resource.
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/mach-omap2/gpmc-nand.c | 5 +-
drivers/memory/omap-gpmc.c | 135 ++
drivers/mtd/nand/omap2.c | 136 +++
include/linux/platform_data/mtd-nand-omap2.h | 3 +-
4 files changed, 147 insertions(+), 132 deletions(-)
diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index ffe646a..e07ca27 100644
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -95,10 +95,7 @@ int gpmc_nand_init(struct omap_nand_platform_data
*gpmc_nand_data,
gpmc_nand_res[1].start = gpmc_get_irq();
memset(s, 0, sizeof(struct gpmc_settings));
- if (gpmc_nand_data-of_node)
- gpmc_read_settings_dt(gpmc_nand_data-of_node, s);
- else
- gpmc_set_legacy(gpmc_nand_data, s);
+ gpmc_set_legacy(gpmc_nand_data, s);
s.device_nand = true;
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index 713d7af..1c87252 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -29,7 +29,6 @@
#include linux/of_device.h
#include linux/of_platform.h
#include linux/omap-gpmc.h
-#include linux/mtd/nand.h
#include linux/pm_runtime.h
#include linux/platform_data/mtd-nand-omap2.h
@@ -1716,105 +1715,6 @@ static void __maybe_unused gpmc_read_timings_dt(struct
device_node *np,
of_property_read_bool(np, gpmc,time-para-granularity);
}
-#if IS_ENABLED(CONFIG_MTD_NAND)
-
-static const char * const nand_xfer_types[] = {
- [NAND_OMAP_PREFETCH_POLLED] = prefetch-polled,
- [NAND_OMAP_POLLED] = polled,
- [NAND_OMAP_PREFETCH_DMA]= prefetch-dma,
- [NAND_OMAP_PREFETCH_IRQ]= prefetch-irq,
-};
-
-static int gpmc_probe_nand_child(struct platform_device *pdev,
-struct device_node *child)
-{
- u32 val;
- const char *s;
- struct gpmc_timings gpmc_t;
- struct omap_nand_platform_data *gpmc_nand_data;
-
- if (of_property_read_u32(child, reg, val) 0) {
- dev_err(pdev-dev, %s has no 'reg' property\n,
- child-full_name);
- return -ENODEV;
- }
-
- gpmc_nand_data = devm_kzalloc(pdev-dev, sizeof(*gpmc_nand_data),
- GFP_KERNEL);
- if (!gpmc_nand_data)
- return -ENOMEM;
-
- gpmc_nand_data-cs = val;
- gpmc_nand_data-of_node = child;
-
- /* Detect availability of ELM module */
- gpmc_nand_data-elm_of_node = of_parse_phandle(child, ti,elm-id, 0);
- if (gpmc_nand_data-elm_of_node == NULL)
- gpmc_nand_data-elm_of_node =
- of_parse_phandle(child, elm_id, 0);
-
- /* select ecc-scheme for NAND */
- if (of_property_read_string(child, ti,nand-ecc-opt, s)) {
- pr_err(%s: ti,nand-ecc-opt not found\n, __func__);
- return -ENODEV;
- }
-
- if (!strcmp(s, sw))
- gpmc_nand_data-ecc_opt = OMAP_ECC_HAM1_CODE_SW;
- else if (!strcmp(s, ham1) ||
-!strcmp(s, hw) || !strcmp(s, hw-romcode))
- gpmc_nand_data-ecc_opt =
- OMAP_ECC_HAM1_CODE_HW;
- else if (!strcmp(s, bch4))
- if (gpmc_nand_data-elm_of_node)
- gpmc_nand_data-ecc_opt =
- OMAP_ECC_BCH4_CODE_HW;
- else
- gpmc_nand_data-ecc_opt =
- OMAP_ECC_BCH4_CODE_HW_DETECTION_SW;
- else if (!strcmp(s, bch8))
- if (gpmc_nand_data-elm_of_node)
- gpmc_nand_data-ecc_opt =
- OMAP_ECC_BCH8_CODE_HW;
- else
- gpmc_nand_data-ecc_opt =
- OMAP_ECC_BCH8_CODE_HW_DETECTION_SW;
- else if (!strcmp(s, bch16))
- if (gpmc_nand_data-elm_of_node)
- gpmc_nand_data-ecc_opt =
- OMAP_ECC_BCH16_CODE_HW;
- else
- pr_err(%s: BCH16 requires ELM support\n, __func__);
- else
- pr_err(%s: ti,nand-ecc-opt invalid value\n, __func__);
-
- /* select data transfer mode for NAND controller */
- if (!of_property_read_string(child, ti,nand-xfer-type, s))
- for (val = 0; val ARRAY_SIZE(nand_xfer_types); val++)
- if (!strcasecmp(s, nand_xfer_types[val])) {
- gpmc_nand_data-xfer_type = val;
- break;
- }
-
-
[PATCH v2 09/22] mtd: nand: omap2: manage NAND interrupts
Manage NAND interrupts here using the GPMC IRQ ops.
This causes performance in prefetch-irq mode to be increased
from
[ 38.252811] mtd_speedtest: eraseblock write speed is 5576 KiB/s
[ 39.265259] mtd_speedtest: eraseblock read speed is 8192 KiB/s
to
[ 35.666446] mtd_speedtest: eraseblock write speed is 6537 KiB/s
[ 36.444842] mtd_speedtest: eraseblock read speed is 10680 KiB/s
Test results on dra7-evm using mtd_speedtest.ko
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/mtd/nand/omap2.c | 63 +++-
1 file changed, 30 insertions(+), 33 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 5c2f6df..cabc5ea 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -162,8 +162,7 @@ struct omap_nand_info {
enum omap_ecc ecc_opt;
struct completion comp;
struct dma_chan *dma;
- int gpmc_irq_fifo;
- int gpmc_irq_count;
+ int gpmc_irq;
enum {
OMAP_NAND_IO_READ = 0, /* read */
OMAP_NAND_IO_WRITE, /* write */
@@ -573,12 +572,17 @@ static irqreturn_t omap_nand_irq(int this_irq, void *dev)
{
struct omap_nand_info *info = (struct omap_nand_info *) dev;
u32 bytes;
+ u32 irqstatus;
+
+ irqstatus = info-ops-nand_irq_status();
+ if (!irqstatus)
+ return IRQ_NONE;
bytes = readl(info-reg.gpmc_prefetch_status);
bytes = PREFETCH_STATUS_FIFO_CNT(bytes);
bytes = bytes 0xFFFC; /* io in multiple of 4 bytes */
if (info-iomode == OMAP_NAND_IO_WRITE) { /* checks for write io */
- if (this_irq == info-gpmc_irq_count)
+ if (irqstatus GPMC_IRQENABLE_TERMCOUNT)
goto done;
if (info-buf_len (info-buf_len bytes))
@@ -595,17 +599,25 @@ static irqreturn_t omap_nand_irq(int this_irq, void *dev)
(u32 *)info-buf, bytes 2);
info-buf = info-buf + bytes;
- if (this_irq == info-gpmc_irq_count)
+ if (irqstatus GPMC_IRQENABLE_TERMCOUNT)
goto done;
}
+ /* Clear FIFOEVENT STATUS */
+ info-ops-nand_irq_clear(GPMC_NAND_IRQ_FIFOEVENT);
+
return IRQ_HANDLED;
done:
complete(info-comp);
- disable_irq_nosync(info-gpmc_irq_fifo);
- disable_irq_nosync(info-gpmc_irq_count);
+ /* Clear FIFOEVENT and TERMCOUNT STATUS */
+ info-ops-nand_irq_clear(GPMC_NAND_IRQ_FIFOEVENT);
+ info-ops-nand_irq_clear(GPMC_NAND_IRQ_TERMCOUNT);
+
+ /* Disable Interrupt generation */
+ info-ops-nand_irq_disable(GPMC_NAND_IRQ_FIFOEVENT);
+ info-ops-nand_irq_disable(GPMC_NAND_IRQ_TERMCOUNT);
return IRQ_HANDLED;
}
@@ -640,8 +652,9 @@ static void omap_read_buf_irq_pref(struct mtd_info *mtd,
u_char *buf, int len)
info-buf_len = len;
- enable_irq(info-gpmc_irq_count);
- enable_irq(info-gpmc_irq_fifo);
+ /* Enable Interrupt generation */
+ info-ops-nand_irq_enable(GPMC_NAND_IRQ_TERMCOUNT);
+ info-ops-nand_irq_enable(GPMC_NAND_IRQ_FIFOEVENT);
/* waiting for read to complete */
wait_for_completion(info-comp);
@@ -690,8 +703,9 @@ static void omap_write_buf_irq_pref(struct mtd_info *mtd,
info-buf_len = len;
- enable_irq(info-gpmc_irq_count);
- enable_irq(info-gpmc_irq_fifo);
+ /* Enable Interrupt generation */
+ info-ops-nand_irq_enable(GPMC_NAND_IRQ_TERMCOUNT);
+ info-ops-nand_irq_enable(GPMC_NAND_IRQ_FIFOEVENT);
/* waiting for write to complete */
wait_for_completion(info-comp);
@@ -1770,35 +1784,18 @@ static int omap_nand_probe(struct platform_device *pdev)
break;
case NAND_OMAP_PREFETCH_IRQ:
- info-gpmc_irq_fifo = platform_get_irq(pdev, 0);
- if (info-gpmc_irq_fifo = 0) {
- dev_err(pdev-dev, error getting fifo irq\n);
- err = -ENODEV;
- goto return_error;
- }
- err = devm_request_irq(pdev-dev, info-gpmc_irq_fifo,
- omap_nand_irq, IRQF_SHARED,
- gpmc-nand-fifo, info);
- if (err) {
- dev_err(pdev-dev, requesting irq(%d) error:%d,
- info-gpmc_irq_fifo, err);
- info-gpmc_irq_fifo = 0;
- goto return_error;
- }
-
- info-gpmc_irq_count = platform_get_irq(pdev, 1);
- if (info-gpmc_irq_count = 0) {
- dev_err(pdev-dev, error getting count irq\n);
+ info-gpmc_irq =
[PATCH v2 12/22] mtd: nand: omap: Update DT binding documentation
Add compatible id and interrupts. The NAND interrupts are
provided by the GPMC controller node.
Signed-off-by: Roger Quadros rog...@ti.com
---
Documentation/devicetree/bindings/mtd/gpmc-nand.txt | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
index fb733c4..253e6de 100644
--- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
@@ -13,7 +13,11 @@ Documentation/devicetree/bindings/mtd/nand.txt
Required properties:
- - reg:The CS line the peripheral is connected to
+ - compatible: ti,omap2-nand
+ - reg:range id (CS number), base offset and length of the
+ NAND I/O space
+ - interrupt-parent: must point to gpmc node
+ - interrupts: gpmc interrupt
Optional properties:
@@ -55,20 +59,24 @@ Example for an AM33xx board:
gpmc: gpmc@5000 {
compatible = ti,am3352-gpmc;
ti,hwmods = gpmc;
- reg = 0x5000 0x100;
+ reg = 0x5000 0x36c;
interrupts = 100;
gpmc,num-cs = 8;
gpmc,num-waitpins = 2;
#address-cells = 2;
#size-cells = 1;
- ranges = 0 0 0x0800 0x2000; /* CS0: NAND */
+ ranges = 0 0 0x0800 0x100;/* CS0 space, 16MB */
elm_id = elm;
nand@0,0 {
- reg = 0 0 0; /* CS0, offset 0 */
+ compatible = ti,omap2-nand;
+ reg = 0 0 4; /* CS0, offset 0, NAND I/O
window 4 */
+ interrupts = 100;
nand-bus-width = 16;
ti,nand-ecc-opt = bch8;
ti,nand-xfer-type = polled;
+ interrupt-parent = gpmc;
+ interrupts = 0, 1;
gpmc,sync-clk-ps = 0;
gpmc,cs-on-ns = 0;
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 10/22] mtd: nand: omap: Copy platform data parameters to omap_nand_info data
Copy all the platform data parameters to the driver's local data
structure 'omap_nand_info' and use it in the entire driver. This will
make it easer for device tree migration.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/mtd/nand/omap2.c | 26 ++
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index cabc5ea..589404c 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -152,14 +152,18 @@ static struct nand_hw_control omap_gpmc_controller = {
};
struct omap_nand_info {
- struct omap_nand_platform_data *pdata;
struct mtd_info mtd;
struct nand_chipnand;
struct platform_device *pdev;
int gpmc_cs;
- unsigned long phys_base;
+ booldev_ready;
+ enum nand_ioxfer_type;
+ int devsize;
enum omap_ecc ecc_opt;
+ struct device_node *elm_of_node;
+
+ unsigned long phys_base;
struct completion comp;
struct dma_chan *dma;
int gpmc_irq;
@@ -1650,7 +1654,7 @@ static bool omap2_nand_ecc_check(struct omap_nand_info
*info,
CONFIG_MTD_NAND_OMAP_BCH not enabled\n);
return false;
}
- if (ecc_needs_elm !is_elm_present(info, pdata-elm_of_node)) {
+ if (ecc_needs_elm !is_elm_present(info, info-elm_of_node)) {
dev_err(info-pdev-dev, ELM not available\n);
return false;
}
@@ -1695,6 +1699,11 @@ static int omap_nand_probe(struct platform_device *pdev)
info-gpmc_cs = pdata-cs;
info-of_node = pdata-of_node;
info-ecc_opt = pdata-ecc_opt;
+ info-dev_ready = pdata-dev_ready;
+ info-xfer_type = pdata-xfer_type;
+ info-devsize = pdata-devsize;
+ info-elm_of_node = pdata-elm_of_node;
+
mtd = info-mtd;
mtd-priv = info-nand;
mtd-name = dev_name(pdev-dev);
@@ -1721,7 +1730,7 @@ static int omap_nand_probe(struct platform_device *pdev)
* chip delay which is slightly more than tR (AC Timing) of the NAND
* device and read status register until you get a failure or success
*/
- if (pdata-dev_ready) {
+ if (info-dev_ready) {
nand_chip-dev_ready = omap_dev_ready;
nand_chip-chip_delay = 0;
} else {
@@ -1735,15 +1744,16 @@ static int omap_nand_probe(struct platform_device *pdev)
nand_chip-options |= NAND_SKIP_BBTSCAN;
/* scan NAND device connected to chip controller */
- nand_chip-options |= pdata-devsize NAND_BUSWIDTH_16;
+ nand_chip-options |= info-devsize NAND_BUSWIDTH_16;
if (nand_scan_ident(mtd, 1, NULL)) {
- dev_err(info-pdev-dev, scan failed, may be bus-width
mismatch\n);
+ dev_err(info-pdev-dev,
+ scan failed, may be bus-width mismatch\n);
err = -ENXIO;
goto return_error;
}
/* re-populate low-level callbacks based on xfer modes */
- switch (pdata-xfer_type) {
+ switch (info-xfer_type) {
case NAND_OMAP_PREFETCH_POLLED:
nand_chip-read_buf = omap_read_buf_pref;
nand_chip-write_buf = omap_write_buf_pref;
@@ -1806,7 +1816,7 @@ static int omap_nand_probe(struct platform_device *pdev)
default:
dev_err(pdev-dev,
- xfer_type(%d) not supported!\n, pdata-xfer_type);
+ xfer_type(%d) not supported!\n, info-xfer_type);
err = -EINVAL;
goto return_error;
}
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
On 08/07/2015 01:08 PM, Michal Suchanek wrote:
Now since the description is clearer it's obvious that ti-qspi cannot
work fully mmapped as fsl-qspi does because the setup has to be done
over normal spi access and using non-m25p80 devices on the same bus is
a requirement.
The place where it is known if a transfer can use the mmap access is m25p80.c
So my suggestion is
- add a new method for spi master that gets the read opcode, dummy
length, address, address length, buffer, buffer length and performs
read from the flash memory in a hardware-specific way
- add a check in m25p80.c that the master supports this feature and if
so use it (eg check that the method is non-null)
Presumably if some new SPI controllers with similar feature are
supported in the future they can use the same inteface because you
pass on everything the m25p80 read knows.
Ok... Do you mean something like this?
I will take m25p80 as example but can be expanded for any flash.
In include/linux/mtd.h:
struct spi_mtd_config_info {
struct spi_device *spi;
u32 page_size;
u8 addr_width;
u8 erase_opcode;
u8 read_opcode;
u8 read_dummy;
u8 program_opcode;
enum read_mode flash_read;
} /* subset of struct spi_nor */
In m25p80.c:
static int m25p80_read(struct spi_nor *nor, loff_t from,
size_t len, size_t *retlen,
u_char *buf)
{
struct spi_mtd_config_info info;
struct spi_device *spi;
if (spi-master-spi_mtd_mmap_read) {
/* Populate spi_mtd_config_info */
spi-master-spi_mtd_mmap_read(info, from, len,
retlen, buf);
}
else {
/* no mtd specific acceleration supported try normal
* SPI way of communicating with flash
* continue with current code
* set up spi_message and call spi_sync()
*/
}
}
In spi-ti-qspi.c:
Implement spi_mtd_mmap_read while holding master-bus_lock mutex.
--
Regards
Vignesh
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
This DMA driver is used by 8250-omap on DRA7-evm. There is one
requirement that is to pause a transfer. This is currently used on the RX
side. It is possible that the UART HW aborted the RX (UART's RX-timeout)
but the DMA controller starts the transfer shortly after.
Before we can manually purge the FIFO we need to pause the transfer,
check how many bytes it already received and terminate the transfer
without it making any progress.
From testing on the TX side it seems that it is possible that we invoke
pause once the transfer has completed which is indicated by the missing
CCR_ENABLE bit but before the interrupt has been noticed. In that case the
interrupt will come even after disabling it.
The AM572x manual says that we have to wait for the CCR_RD_ACTIVE
CCR_WR_ACTIVE bits to be gone before programming it again here is the
drain loop. Also it looks like without the drain the TX-transfer makes
sometimes progress.
One note: The pause + resume combo is broken because after resume the
the complete transfer will be programmed again. That means the already
transferred bytes (until the pause event) will be sent again. This is
currently not important for my UART user because it does only pause +
terminate.
Cc: sta...@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
drivers/dma/omap-dma.c | 54 ++
1 file changed, 41 insertions(+), 13 deletions(-)
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 249445c8a4c6..6b8497203caf 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -299,7 +299,7 @@ static void omap_dma_start(struct omap_chan *c, struct
omap_desc *d)
omap_dma_chan_write(c, CCR, d-ccr | CCR_ENABLE);
}
-static void omap_dma_stop(struct omap_chan *c)
+static int omap_dma_stop(struct omap_chan *c)
{
struct omap_dmadev *od = to_omap_dma_dev(c-vc.chan.device);
uint32_t val;
@@ -342,8 +342,26 @@ static void omap_dma_stop(struct omap_chan *c)
omap_dma_glbl_write(od, OCP_SYSCONFIG, sysconfig);
} else {
+ int i = 0;
+
+ if (!(val CCR_ENABLE))
+ return -EINVAL;
+
val = ~CCR_ENABLE;
omap_dma_chan_write(c, CCR, val);
+ do {
+ val = omap_dma_chan_read(c, CCR);
+ if (!(val (CCR_RD_ACTIVE | CCR_WR_ACTIVE)))
+ break;
+ if (i 100)
+ break;
+ udelay(5);
+ } while (1);
+
+ if (val (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
+ dev_err(c-vc.chan.device-dev,
+ DMA drain did not complete on lch %d\n,
+ c-dma_ch);
}
mb();
@@ -358,6 +376,7 @@ static void omap_dma_stop(struct omap_chan *c)
omap_dma_chan_write(c, CLNK_CTRL, val);
}
+ return 0;
}
static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d,
@@ -728,6 +747,8 @@ static enum dma_status omap_dma_tx_status(struct dma_chan
*chan,
} else {
txstate-residue = 0;
}
+ if (ret == DMA_IN_PROGRESS c-paused)
+ ret = DMA_PAUSED;
spin_unlock_irqrestore(c-vc.lock, flags);
return ret;
@@ -1053,28 +1074,33 @@ static int omap_dma_terminate_all(struct dma_chan *chan)
static int omap_dma_pause(struct dma_chan *chan)
{
struct omap_chan *c = to_omap_dma_chan(chan);
+ struct omap_dmadev *od = to_omap_dma_dev(chan-device);
+ unsigned long flags;
+ int ret = -EINVAL;
- /* Pause/Resume only allowed with cyclic mode */
- if (!c-cyclic)
- return -EINVAL;
+ spin_lock_irqsave(od-irq_lock, flags);
- if (!c-paused) {
- omap_dma_stop(c);
- c-paused = true;
+ if (!c-paused c-desc) {
+ ret = omap_dma_stop(c);
+ if (!ret)
+ c-paused = true;
}
- return 0;
+ spin_unlock_irqrestore(od-irq_lock, flags);
+
+ return ret;
}
static int omap_dma_resume(struct dma_chan *chan)
{
struct omap_chan *c = to_omap_dma_chan(chan);
+ struct omap_dmadev *od = to_omap_dma_dev(chan-device);
+ unsigned long flags;
+ int ret = -EINVAL;
- /* Pause/Resume only allowed with cyclic mode */
- if (!c-cyclic)
- return -EINVAL;
+ spin_lock_irqsave(od-irq_lock, flags);
- if (c-paused) {
+ if (c-paused c-desc) {
mb();
/* Restore channel link register */
@@ -1082,9 +1108,11 @@ static int omap_dma_resume(struct dma_chan *chan)
omap_dma_start(c, c-desc);
c-paused = false;
+ ret = 0;
}
+ spin_unlock_irqrestore(od-irq_lock, flags);
-
Re: [PATCH 3/8] rtc: omap: Add external clock enabling support
On Thursday 06 August 2015 03:21 PM, Tony Lindgren wrote:
* Alexandre Belloni alexandre.bell...@free-electrons.com [150806 02:50]:
On 06/08/2015 at 12:36:54 +0300, Grygorii Strashko wrote :
Pls, correct me if I'm not right. Is below what you propose?
Doard dts:
/ {
rtc_32k_ext_clk: rtc_osc_xi_clkin32_ext {
#clock-cells = 0;
compatible = fixed-clock;
clock-frequency = 32000;
clock-output-names = rtc_osc_xi_clkin32;
};
}
rtc {
status = okay;
clocks = sys_32k_ck, rtc_32k_ext_clk;
[optional] clock-names = int-clk, ext-clk;
};
Driver:
1) clk0 is mandatory, internal clock source
2) clk1 is optional, external clock source, so
if present - RTC driver can switch to use ext clock source
Thanks Grygorii. I will implement it this way.
Absolutely!
Sounds good to me too.
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 22/22] memory: omap-gpmc: Prevent GPMC_STATUS from being accessed via gpmc_regs
GPMC_STATUS register is private to the GPMC module and must not be
accessed directly by NAND driver through the gpmc_regs.
They must use gpmc_omap_get_nand_ops() instead.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/memory/omap-gpmc.c | 2 +-
include/linux/platform_data/mtd-nand-omap2.h | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index 417acce..bdd1533 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -1051,7 +1051,7 @@ void gpmc_update_nand_reg(struct gpmc_nand_regs *reg, int
cs)
{
int i;
- reg-gpmc_status = gpmc_base + GPMC_STATUS;
+ reg-gpmc_status = NULL;/* deprecated */
reg-gpmc_nand_command = gpmc_base + GPMC_CS0_OFFSET +
GPMC_CS_NAND_COMMAND + GPMC_CS_SIZE * cs;
reg-gpmc_nand_address = gpmc_base + GPMC_CS0_OFFSET +
diff --git a/include/linux/platform_data/mtd-nand-omap2.h
b/include/linux/platform_data/mtd-nand-omap2.h
index 19e509d..17d57a1 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -45,7 +45,6 @@ enum omap_ecc {
};
struct gpmc_nand_regs {
- void __iomem*gpmc_status;
void __iomem*gpmc_nand_command;
void __iomem*gpmc_nand_address;
void __iomem*gpmc_nand_data;
@@ -64,6 +63,8 @@ struct gpmc_nand_regs {
void __iomem*gpmc_bch_result4[GPMC_BCH_NUM_REMAINDER];
void __iomem*gpmc_bch_result5[GPMC_BCH_NUM_REMAINDER];
void __iomem*gpmc_bch_result6[GPMC_BCH_NUM_REMAINDER];
+ /* Deprecated. Do not use */
+ void __iomem*gpmc_status;
};
struct omap_nand_platform_data {
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 17/22] memory: omap-gpmc: Reserve WAITPIN if needed for WAIT monitoring
If the device attached to GPMC wants to use the WAIT pin
for WAIT monitoring then we reserve it internally for
exclusive use.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/memory/omap-gpmc.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index 264009d..0df70ab 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -1779,6 +1779,8 @@ static int gpmc_probe_generic_child(struct
platform_device *pdev,
const char *name;
int ret, cs;
u32 val;
+ struct gpio_desc *waitpin_desc = NULL;
+ struct gpmc_device *gpmc = platform_get_drvdata(pdev);
if (of_property_read_u32(child, reg, cs) 0) {
dev_err(pdev-dev, %s has no 'reg' property\n,
@@ -1880,15 +1882,28 @@ static int gpmc_probe_generic_child(struct
platform_device *pdev,
goto err;
}
+ /* Reserve wait pin if it is required and valid */
+ if (gpmc_s.wait_on_read || gpmc_s.wait_on_write) {
+ unsigned wait_pin = gpmc_s.wait_pin;
+
+ waitpin_desc = gpiochip_request_own_desc(gpmc-gpio_chip,
+wait_pin, WAITPIN);
+ if (IS_ERR(waitpin_desc)) {
+ dev_err(pdev-dev, invalid wait-pin: %d\n, wait_pin);
+ ret = PTR_ERR(waitpin_desc);
+ goto err;
+ }
+ }
+
ret = gpmc_cs_program_settings(cs, gpmc_s);
if (ret 0)
- goto err;
+ goto err_cs;
ret = gpmc_cs_set_timings(cs, gpmc_t, gpmc_s);
if (ret) {
dev_err(pdev-dev, failed to set gpmc timings for: %s\n,
child-name);
- goto err;
+ goto err_cs;
}
/* Clear limited address i.e. enable A26-A11 */
@@ -1919,6 +1934,10 @@ err_child_fail:
dev_err(pdev-dev, failed to create gpmc child %s\n, child-name);
ret = -ENODEV;
+err_cs:
+ if (waitpin_desc)
+ gpiochip_free_own_desc(waitpin_desc);
+
err:
gpmc_cs_free(cs);
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 13/22] memory: omap-gpmc: Prevent mapping into 1st 16MB
We have been preventing mapping GPMC children in the
first 1MB but really it has to be the first 16MB as
the minimum GPMC partition size is 16MB.
Also print an error message if CS mapping fails
due to DT requesting address outside the GPMC
map.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/memory/omap-gpmc.c | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index 1c87252..30d9c21 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -93,6 +93,14 @@
#define GPMC_CS_SIZE 0x30
#defineGPMC_BCH_SIZE 0x10
+/*
+ * The first 1MB of GPMC address space is typically mapped to
+ * the internal ROM. Never allocate the first page, to
+ * facilitate bug detection; even if we didn't boot from ROM.
+ * As GPMC minimum partition size is 16MB we can only start from
+ * there.
+ */
+#define GPMC_MEM_START 0x100
#define GPMC_MEM_END 0x3FFF
#define GPMC_CHUNK_SHIFT 24 /* 16 MB */
@@ -1171,12 +1179,7 @@ static void gpmc_mem_init(void)
{
int cs;
- /*
-* The first 1MB of GPMC address space is typically mapped to
-* the internal ROM. Never allocate the first page, to
-* facilitate bug detection; even if we didn't boot from ROM.
-*/
- gpmc_mem_root.start = SZ_1M;
+ gpmc_mem_root.start = GPMC_MEM_START;
gpmc_mem_root.end = GPMC_MEM_END;
/* Reserve all regions that has been set up by bootloader */
@@ -1830,6 +1833,15 @@ static int gpmc_probe_generic_child(struct
platform_device *pdev,
if (ret 0) {
dev_err(pdev-dev, cannot remap GPMC CS %d to %pa\n,
cs, res.start);
+ if (res.start GPMC_MEM_START) {
+ dev_info(pdev-dev,
+GPMC CS %d start cannot be lesser than
0x%x\n,
+cs, GPMC_MEM_START);
+ } else if (res.end GPMC_MEM_END) {
+ dev_info(pdev-dev,
+GPMC CS %d end cannot be greater than 0x%x\n,
+cs, GPMC_MEM_END);
+ }
goto err;
}
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 15/22] memory: omap-gpmc: Move device tree binding to correct location
omap-gpmc.c is a memory controller so move the binding to the
right place.
Signed-off-by: Roger Quadros rog...@ti.com
---
.../bindings/{bus/ti-gpmc.txt = memory-controllers/omap-gpmc.txt}| 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename Documentation/devicetree/bindings/{bus/ti-gpmc.txt =
memory-controllers/omap-gpmc.txt} (100%)
diff --git a/Documentation/devicetree/bindings/bus/ti-gpmc.txt
b/Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
similarity index 100%
rename from Documentation/devicetree/bindings/bus/ti-gpmc.txt
rename to Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/22] ARM: OMAP2+: gpmc: Add platform data
Add a platform data structure for GPMC. It contains all the necessary
platform information that needs to be passed from platform init code
to GPMC driver.
Signed-off-by: Roger Quadros rog...@ti.com
---
include/linux/omap-gpmc.h | 3 +--
include/linux/platform_data/gpmc-omap.h | 30 ++
2 files changed, 31 insertions(+), 2 deletions(-)
create mode 100644 include/linux/platform_data/gpmc-omap.h
diff --git a/include/linux/omap-gpmc.h b/include/linux/omap-gpmc.h
index 7dee0014..5c79190 100644
--- a/include/linux/omap-gpmc.h
+++ b/include/linux/omap-gpmc.h
@@ -7,8 +7,7 @@
* option) any later version.
*/
-/* Maximum Number of Chip Selects */
-#define GPMC_CS_NUM8
+#include linux/platform_data/gpmc-omap.h
#define GPMC_CONFIG_WP 0x0005
diff --git a/include/linux/platform_data/gpmc-omap.h
b/include/linux/platform_data/gpmc-omap.h
new file mode 100644
index 000..d32d9de
--- /dev/null
+++ b/include/linux/platform_data/gpmc-omap.h
@@ -0,0 +1,30 @@
+/*
+ * OMAP GPMC Platform data
+ *
+ * Copyright (C) 2014 Texas Instruments, Inc. - http://www.ti.com
+ * Roger Quadros rog...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+
+#ifndef _GPMC_OMAP_H_
+#define _GPMC_OMAP_H_
+
+/* Maximum Number of Chip Selects */
+#define GPMC_CS_NUM8
+
+/* Data for each chip select */
+struct gpmc_omap_cs_data {
+ bool valid; /* data is valid */
+ bool is_nand; /* device within this CS is NAND */
+ struct platform_device *pdev; /* device within this CS region */
+ unsigned pdata_size;
+};
+
+struct gpmc_omap_platform_data {
+ struct gpmc_omap_cs_data cs[GPMC_CS_NUM];
+};
+
+#endif /* _GPMC_OMAP_H */
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/22] ARM: OMAP2+: gpmc: Add gpmc timings and settings to platform data
Add device_timings, gpmc_timings and gpmc_setting to
gpmc platform data.
Signed-off-by: Roger Quadros rog...@ti.com
---
include/linux/omap-gpmc.h | 134 --
include/linux/platform_data/gpmc-omap.h | 139
2 files changed, 139 insertions(+), 134 deletions(-)
diff --git a/include/linux/omap-gpmc.h b/include/linux/omap-gpmc.h
index 5c79190..2dcef1c 100644
--- a/include/linux/omap-gpmc.h
+++ b/include/linux/omap-gpmc.h
@@ -14,140 +14,6 @@
#define GPMC_IRQ_FIFOEVENTENABLE 0x01
#define GPMC_IRQ_COUNT_EVENT 0x02
-#define GPMC_BURST_4 4 /* 4 word burst */
-#define GPMC_BURST_8 8 /* 8 word burst */
-#define GPMC_BURST_16 16 /* 16 word burst */
-#define GPMC_DEVWIDTH_8BIT 1 /* 8-bit device width */
-#define GPMC_DEVWIDTH_16BIT2 /* 16-bit device width */
-#define GPMC_MUX_AAD 1 /* Addr-Addr-Data multiplex */
-#define GPMC_MUX_AD2 /* Addr-Data multiplex */
-
-/* bool type time settings */
-struct gpmc_bool_timings {
- bool cycle2cyclediffcsen;
- bool cycle2cyclesamecsen;
- bool we_extra_delay;
- bool oe_extra_delay;
- bool adv_extra_delay;
- bool cs_extra_delay;
- bool time_para_granularity;
-};
-
-/*
- * Note that all values in this struct are in nanoseconds except sync_clk
- * (which is in picoseconds), while the register values are in gpmc_fck cycles.
- */
-struct gpmc_timings {
- /* Minimum clock period for synchronous mode (in picoseconds) */
- u32 sync_clk;
-
- /* Chip-select signal timings corresponding to GPMC_CS_CONFIG2 */
- u32 cs_on; /* Assertion time */
- u32 cs_rd_off; /* Read deassertion time */
- u32 cs_wr_off; /* Write deassertion time */
-
- /* ADV signal timings corresponding to GPMC_CONFIG3 */
- u32 adv_on; /* Assertion time */
- u32 adv_rd_off; /* Read deassertion time */
- u32 adv_wr_off; /* Write deassertion time */
-
- /* WE signals timings corresponding to GPMC_CONFIG4 */
- u32 we_on; /* WE assertion time */
- u32 we_off; /* WE deassertion time */
-
- /* OE signals timings corresponding to GPMC_CONFIG4 */
- u32 oe_on; /* OE assertion time */
- u32 oe_off; /* OE deassertion time */
-
- /* Access time and cycle time timings corresponding to GPMC_CONFIG5 */
- u32 page_burst_access; /* Multiple access word delay */
- u32 access; /* Start-cycle to first data valid delay */
- u32 rd_cycle; /* Total read cycle time */
- u32 wr_cycle; /* Total write cycle time */
-
- u32 bus_turnaround;
- u32 cycle2cycle_delay;
-
- u32 wait_monitoring;
- u32 clk_activation;
-
- /* The following are only on OMAP3430 */
- u32 wr_access; /* WRACCESSTIME */
- u32 wr_data_mux_bus;/* WRDATAONADMUXBUS */
-
- struct gpmc_bool_timings bool_timings;
-};
-
-/* Device timings in picoseconds */
-struct gpmc_device_timings {
- u32 t_ceasu;/* address setup to CS valid */
- u32 t_avdasu; /* address setup to ADV valid */
- /* XXX: try to combine t_avdp_r t_avdp_w. Issue is
-* of tusb using these timings even for sync whilst
-* ideally for adv_rd/(wr)_off it should have considered
-* t_avdh instead. This indirectly necessitates r/w
-* variations of t_avdp as it is possible to have one
-* sync other async
-*/
- u32 t_avdp_r; /* ADV low time (what about t_cer ?) */
- u32 t_avdp_w;
- u32 t_aavdh;/* address hold time */
- u32 t_oeasu;/* address setup to OE valid */
- u32 t_aa; /* access time from ADV assertion */
- u32 t_iaa; /* initial access time */
- u32 t_oe; /* access time from OE assertion */
- u32 t_ce; /* access time from CS asertion */
- u32 t_rd_cycle; /* read cycle time */
- u32 t_cez_r;/* read CS deassertion to high Z */
- u32 t_cez_w;/* write CS deassertion to high Z */
- u32 t_oez; /* OE deassertion to high Z */
- u32 t_weasu;/* address setup to WE valid */
- u32 t_wpl; /* write assertion time */
- u32 t_wph; /* write deassertion time */
- u32 t_wr_cycle; /* write cycle time */
-
- u32 clk;
- u32 t_bacc; /* burst access valid clock to output delay */
- u32 t_ces; /* CS setup time to clk */
- u32 t_avds; /* ADV setup time to clk */
- u32 t_avdh; /* ADV hold time from clk */
- u32 t_ach; /* address hold time from clk */
- u32 t_rdyo; /* clk to ready valid */
-
- u32 t_ce_rdyz; /* XXX: description ?, or use t_cez
[PATCH v2 00/22] memory: omap-gpmc: mtd: nand: Support GPMC NAND on non-OMAP platforms
Hi,
We do a couple of things in this series which result in
cleaner device tree implementation, faster perfomance and
multi-platform support. As an added bonus we get new GPI/Interrupt pins
for use in the system.
- Establish a custom interface between NAND and GPMC driver. This is
needed because all of the NAND registers sit in the GPMC register space.
Some bits like NAND IRQ are even shared with GPMC.
- Remove NAND IRQ handling from omap-gpmc driver, share the GPMC IRQ
with the omap2-nand driver and handle NAND IRQ events in the NAND driver.
This causes performance increase when using prefetch-irq mode.
30% increase in read, 17% increase in write in prefetch-irq mode.
- Clean up device tree support so that omap-gpmc IP and the omap2 NAND
driver can be used on non-OMAP platforms. e.g. Keystone.
- Implement GPIOCHIP + IRQCHIP for the GPMC WAITPINS. SoCs can contain
2 to 4 of these and most of them would be unused otherwise. It also
allows a cleaner implementation of NAND Ready pin status for the NAND driver.
- Implement GPIOlib based NAND ready pin checking for OMAP NAND driver.
NOTE: I've only adapted dra7.dtsi and dra7x-evms for this series.
I will adapt all other boards when the series is in a shape to be accepted.
cheers,
-roger
This is done in patches 1 to 14
Roger Quadros (22):
ARM: OMAP2+: gpmc: Add platform data
ARM: OMAP2+: gpmc: Add gpmc timings and settings to platform data
memory: omap-gpmc: Introduce GPMC to NAND interface
mtd: nand: omap2: Use gpmc_omap_get_nand_ops() to get NAND registers
memory: omap-gpmc: Add GPMC-NAND ops to get writebufferempty status
mtd: nand: omap2: Switch to using GPMC-NAND ops for writebuffer empty
check
memory: omap-gpmc: Remove NAND IRQ code
memory: omap-gpmc: Add IRQ ops for GPMC-NAND interface
mtd: nand: omap2: manage NAND interrupts
mtd: nand: omap: Copy platform data parameters to omap_nand_info data
mtd: nand: omap: Clean up device tree support
mtd: nand: omap: Update DT binding documentation
memory: omap-gpmc: Prevent mapping into 1st 16MB
ARM: dts: OMAP2+: Fix NAND device nodes.
memory: omap-gpmc: Move device tree binding to correct location
memory: omap-gpmc: Support general purpose input for WAITPINs
memory: omap-gpmc: Reserve WAITPIN if needed for WAIT monitoring
memory: omap-gpmc: Add irqchip support to the gpiochip
ARM: dts: dra7: Enable gpio interrupt controller for gpmc node
mtd: nand: omap2: Implement NAND ready using gpiolib
ARM: dts: dra7x-evm: Provide NAND ready pin
memory: omap-gpmc: Prevent GPMC_STATUS from being accessed via
gpmc_regs
.../omap-gpmc.txt} | 0
.../devicetree/bindings/mtd/gpmc-nand.txt | 16 +-
arch/arm/boot/dts/dra7-evm.dts | 6 +-
arch/arm/boot/dts/dra7.dtsi| 4 +
arch/arm/boot/dts/dra72-evm.dts| 2 +
arch/arm/boot/dts/omap3-beagle.dts | 3 +-
arch/arm/mach-omap2/gpmc-nand.c| 11 +-
drivers/memory/omap-gpmc.c | 610 -
drivers/mtd/nand/omap2.c | 261 ++---
include/linux/omap-gpmc.h | 172 ++
include/linux/platform_data/gpmc-omap.h| 169 ++
include/linux/platform_data/mtd-nand-omap2.h | 12 +-
12 files changed, 782 insertions(+), 484 deletions(-)
rename Documentation/devicetree/bindings/{bus/ti-gpmc.txt =
memory-controllers/omap-gpmc.txt} (100%)
create mode 100644 include/linux/platform_data/gpmc-omap.h
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] extcon: palmas: Support GPIO based USB ID detection
Hi Lee / Samuel,
On 03/08/15 17:40, Roger Quadros wrote:
Some palmas based chip variants do not have OTG based ID logic.
For these variants we rely on GPIO based USB ID detection.
These chips do have VBUS comparator for VBUS detection so we
continue to use the old way of detecting VBUS.
Acked-by: Chanwoo Choi cw00.c...@samsung.com
Signed-off-by: Roger Quadros rog...@ti.com
It seems the extcon maintainer Chanwoo needs either of your Ack
to accept this patch.
Can you please take a look and Ack if OK? Thanks.
cheers,
-roger
---
v4: updated gpio debounce logic to use delayed_workqueue.
.../devicetree/bindings/extcon/extcon-palmas.txt | 5 +-
drivers/extcon/extcon-palmas.c | 129
++---
drivers/extcon/extcon-usb-gpio.c | 1 +
include/linux/mfd/palmas.h | 7 ++
4 files changed, 126 insertions(+), 16 deletions(-)
diff --git a/Documentation/devicetree/bindings/extcon/extcon-palmas.txt
b/Documentation/devicetree/bindings/extcon/extcon-palmas.txt
index 45414bb..f61d5af 100644
--- a/Documentation/devicetree/bindings/extcon/extcon-palmas.txt
+++ b/Documentation/devicetree/bindings/extcon/extcon-palmas.txt
@@ -10,8 +10,11 @@ Required Properties:
Optional Properties:
- ti,wakeup : To enable the wakeup comparator in probe
- - ti,enable-id-detection: Perform ID detection.
+ - ti,enable-id-detection: Perform ID detection. If id-gpio is specified
+ it performs id-detection using GPIO else using OTG core.
- ti,enable-vbus-detection: Perform VBUS detection.
+ - id-gpio: gpio for GPIO ID detection. See gpio binding.
+ - debounce-delay-ms: debounce delay for GPIO ID pin in milliseconds.
palmas-usb {
compatible = ti,twl6035-usb, ti,palmas-usb;
diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
index 8933e7e..662e917 100644
--- a/drivers/extcon/extcon-palmas.c
+++ b/drivers/extcon/extcon-palmas.c
@@ -28,6 +28,10 @@
#include linux/mfd/palmas.h
#include linux/of.h
#include linux/of_platform.h
+#include linux/of_gpio.h
+#include linux/workqueue.h
+
+#define USB_GPIO_DEBOUNCE_MS 20 /* ms */
static const unsigned int palmas_extcon_cable[] = {
EXTCON_USB,
@@ -118,19 +122,54 @@ static irqreturn_t palmas_id_irq_handler(int irq, void
*_palmas_usb)
return IRQ_HANDLED;
}
+static void palmas_gpio_id_detect(struct work_struct *work)
+{
+ int id;
+ struct palmas_usb *palmas_usb = container_of(to_delayed_work(work),
+ struct palmas_usb,
+ wq_detectid);
+ struct extcon_dev *edev = palmas_usb-edev;
+
+ if (!palmas_usb-id_gpiod)
+ return;
+
+ id = gpiod_get_value_cansleep(palmas_usb-id_gpiod);
+
+ if (id) {
+ extcon_set_cable_state_(edev, EXTCON_USB_HOST, false);
+ dev_info(palmas_usb-dev, USB-HOST cable is detached\n);
+ } else {
+ extcon_set_cable_state_(edev, EXTCON_USB_HOST, true);
+ dev_info(palmas_usb-dev, USB-HOST cable is attached\n);
+ }
+}
+
+static irqreturn_t palmas_gpio_id_irq_handler(int irq, void *_palmas_usb)
+{
+ struct palmas_usb *palmas_usb = _palmas_usb;
+
+ queue_delayed_work(system_power_efficient_wq, palmas_usb-wq_detectid,
+palmas_usb-sw_debounce_jiffies);
+
+ return IRQ_HANDLED;
+}
+
static void palmas_enable_irq(struct palmas_usb *palmas_usb)
{
palmas_write(palmas_usb-palmas, PALMAS_USB_OTG_BASE,
PALMAS_USB_VBUS_CTRL_SET,
PALMAS_USB_VBUS_CTRL_SET_VBUS_ACT_COMP);
- palmas_write(palmas_usb-palmas, PALMAS_USB_OTG_BASE,
- PALMAS_USB_ID_CTRL_SET, PALMAS_USB_ID_CTRL_SET_ID_ACT_COMP);
+ if (palmas_usb-enable_id_detection) {
+ palmas_write(palmas_usb-palmas, PALMAS_USB_OTG_BASE,
+ PALMAS_USB_ID_CTRL_SET,
+ PALMAS_USB_ID_CTRL_SET_ID_ACT_COMP);
- palmas_write(palmas_usb-palmas, PALMAS_USB_OTG_BASE,
- PALMAS_USB_ID_INT_EN_HI_SET,
- PALMAS_USB_ID_INT_EN_HI_SET_ID_GND |
- PALMAS_USB_ID_INT_EN_HI_SET_ID_FLOAT);
+ palmas_write(palmas_usb-palmas, PALMAS_USB_OTG_BASE,
+ PALMAS_USB_ID_INT_EN_HI_SET,
+ PALMAS_USB_ID_INT_EN_HI_SET_ID_GND |
+ PALMAS_USB_ID_INT_EN_HI_SET_ID_FLOAT);
+ }
if (palmas_usb-enable_vbus_detection)
palmas_vbus_irq_handler(palmas_usb-vbus_irq, palmas_usb);
@@ -169,20 +208,36 @@ static int palmas_usb_probe(struct platform_device
*pdev)
palmas_usb-wakeup = pdata-wakeup;
}
+ palmas_usb-id_gpiod = devm_gpiod_get_optional(pdev-dev, id);
+ if (IS_ERR(palmas_usb-id_gpiod)) {
+
[PATCH v2 21/22] ARM: dts: dra7x-evm: Provide NAND ready pin
On these boards NAND ready pin status is avilable over GPMC_WAIT0 pin. Read speed increases from 13768 KiB/ to 17246 KiB/s. Write speed was unchanged at 7123 KiB/s. Measured using mtd_speedtest.ko. Signed-off-by: Roger Quadros rog...@ti.com --- arch/arm/boot/dts/dra7-evm.dts | 1 + arch/arm/boot/dts/dra72-evm.dts | 1 + 2 files changed, 2 insertions(+) diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts index 3fb1ced..b717fd0 100644 --- a/arch/arm/boot/dts/dra7-evm.dts +++ b/arch/arm/boot/dts/dra7-evm.dts @@ -575,6 +575,7 @@ reg = 0 0 4; /* device IO registers */ interrupt-parent = crossbar_mpu; interrupts = GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH; + ready-gpio = gpmc 0 GPIO_ACTIVE_HIGH; /* gpmc_wait0 pin */ ti,nand-ecc-opt = bch8; ti,elm-id = elm; nand-bus-width = 16; diff --git a/arch/arm/boot/dts/dra72-evm.dts b/arch/arm/boot/dts/dra72-evm.dts index fc0677a..c1a3397 100644 --- a/arch/arm/boot/dts/dra72-evm.dts +++ b/arch/arm/boot/dts/dra72-evm.dts @@ -387,6 +387,7 @@ */ reg = 0 0 4; /* device IO registers */ interrupt-parent = crossbar_mpu; + ready-gpio = gpmc 0 GPIO_ACTIVE_HIGH; /* gpmc_wait0 pin */ ti,nand-ecc-opt = bch8; ti,elm-id = elm; nand-bus-width = 16; -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 20/22] mtd: nand: omap2: Implement NAND ready using gpiolib
The GPMC WAIT pin status are now available over gpiolib.
Update the omap_dev_ready() function to use gpio instead of
directly accessing GPMC register space.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/mtd/nand/omap2.c | 29 +---
include/linux/platform_data/mtd-nand-omap2.h | 2 +-
2 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index b009d1d..8ace767 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -12,6 +12,7 @@
#include linux/dmaengine.h
#include linux/dma-mapping.h
#include linux/delay.h
+#include linux/gpio/consumer.h
#include linux/module.h
#include linux/interrupt.h
#include linux/jiffies.h
@@ -184,6 +185,8 @@ struct omap_nand_info {
/* fields specific for BCHx_HW ECC scheme */
struct device *elm_dev;
struct device_node *of_node;
+ /* NAND ready gpio */
+ struct gpio_desc*ready_gpiod;
};
/**
@@ -1041,22 +1044,17 @@ static int omap_wait(struct mtd_info *mtd, struct
nand_chip *chip)
}
/**
- * omap_dev_ready - calls the platform specific dev_ready function
+ * omap_dev_ready - checks the NAND Ready GPIO line
* @mtd: MTD device structure
+ *
+ * Returns true if ready and false if busy.
*/
static int omap_dev_ready(struct mtd_info *mtd)
{
- unsigned int val = 0;
struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
mtd);
- val = readl(info-reg.gpmc_status);
-
- if ((val 0x100) == 0x100) {
- return 1;
- } else {
- return 0;
- }
+ return gpiod_get_value(info-ready_gpiod);
}
/**
@@ -1776,7 +1774,9 @@ static int omap_nand_probe(struct platform_device *pdev)
info-reg = pdata-reg;
info-of_node = pdata-of_node;
info-ecc_opt = pdata-ecc_opt;
- info-dev_ready = pdata-dev_ready;
+ if (pdata-dev_ready)
+ dev_info(pdev-dev, pdata-dev_ready is
deprecated\n);
+
info-xfer_type = pdata-xfer_type;
info-devsize = pdata-devsize;
info-elm_of_node = pdata-elm_of_node;
@@ -1809,6 +1809,13 @@ static int omap_nand_probe(struct platform_device *pdev)
nand_chip-IO_ADDR_W = nand_chip-IO_ADDR_R;
nand_chip-cmd_ctrl = omap_hwcontrol;
+ info-ready_gpiod = devm_gpiod_get_optional(pdev-dev, ready,
+ GPIOD_IN);
+ if (IS_ERR(info-ready_gpiod)) {
+ dev_err(dev, failed to get ready gpio\n);
+ return PTR_ERR(info-ready_gpiod);
+ }
+
/*
* If RDY/BSY line is connected to OMAP then use the omap ready
* function and the generic nand_wait function which reads the status
@@ -1816,7 +1823,7 @@ static int omap_nand_probe(struct platform_device *pdev)
* chip delay which is slightly more than tR (AC Timing) of the NAND
* device and read status register until you get a failure or success
*/
- if (info-dev_ready) {
+ if (info-ready_gpiod) {
nand_chip-dev_ready = omap_dev_ready;
nand_chip-chip_delay = 0;
} else {
diff --git a/include/linux/platform_data/mtd-nand-omap2.h
b/include/linux/platform_data/mtd-nand-omap2.h
index ff27e5a..19e509d 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -70,7 +70,6 @@ struct omap_nand_platform_data {
int cs;
struct mtd_partition*parts;
int nr_parts;
- booldev_ready;
boolflash_bbt;
enum nand_ioxfer_type;
int devsize;
@@ -81,5 +80,6 @@ struct omap_nand_platform_data {
/* deprecated */
struct gpmc_nand_regs reg;
struct device_node *of_node;
+ booldev_ready;
};
#endif
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: dts: am335x-phycore-som: Move NAND partition table into board files
Partitions which are defined in the som file can not be deleted in the
board file.
Signed-off-by: Matthias Klein matthias.kl...@optimeas.de
---
arch/arm/boot/dts/am335x-phycore-som.dtsi | 37 -
arch/arm/boot/dts/am335x-wega.dtsi| 45 +++
2 files changed, 45 insertions(+), 37 deletions(-)
diff --git a/arch/arm/boot/dts/am335x-phycore-som.dtsi
b/arch/arm/boot/dts/am335x-phycore-som.dtsi
index 4d28fc3..8f12bd54 100644
--- a/arch/arm/boot/dts/am335x-phycore-som.dtsi
+++ b/arch/arm/boot/dts/am335x-phycore-som.dtsi
@@ -189,43 +189,6 @@
#address-cells = 1;
#size-cells = 1;
-
- partition@0 {
- label = xload;
- reg = 0x0 0x2;
- };
- partition@1 {
- label = xload_backup1;
- reg = 0x2 0x2;
- };
- partition@2 {
- label = xload_backup2;
- reg = 0x4 0x2;
- };
- partition@3 {
- label = xload_backup3;
- reg = 0x6 0x2;
- };
- partition@4 {
- label = barebox;
- reg = 0x8 0x8;
- };
- partition@5 {
- label = bareboxenv;
- reg = 0x10 0x4;
- };
- partition@6 {
- label = oftree;
- reg = 0x14 0x4;
- };
- partition@7 {
- label = kernel;
- reg = 0x18 0x80;
- };
- partition@8 {
- label = root;
- reg = 0x98 0x0;
- };
};
};
diff --git a/arch/arm/boot/dts/am335x-wega.dtsi
b/arch/arm/boot/dts/am335x-wega.dtsi
index 5e541bd..945a41d 100644
--- a/arch/arm/boot/dts/am335x-wega.dtsi
+++ b/arch/arm/boot/dts/am335x-wega.dtsi
@@ -149,3 +149,48 @@
usb1_phy {
status = okay;
};
+
+nandflash {
+ partition@0 {
+ label = xload;
+ reg = 0x0 0x2;
+ };
+
+ partition@1 {
+ label = xload_backup1;
+ reg = 0x2 0x2;
+ };
+
+ partition@2 {
+ label = xload_backup2;
+ reg = 0x4 0x2;
+ };
+
+ partition@3 {
+ label = xload_backup3;
+ reg = 0x6 0x2;
+ };
+
+ partition@4 {
+ label = barebox;
+ reg = 0x8 0x8;
+ };
+
+ partition@5 {
+ label = bareboxenv;
+ reg = 0x10 0x4;
+ };
+
+ partition@6 {
+ label = oftree;
+ reg = 0x14 0x4;
+ };
+ partition@7 {
+ label = kernel;
+ reg = 0x18 0x80;
+ };
+ partition@8 {
+ label = root;
+ reg = 0x98 0x0;
+ };
+};
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 19/22] ARM: dts: dra7: Enable gpio interrupt controller for gpmc node
The GPMC driver now implements gpiochip and irqchip so
enable gpio-controller and interrupt-controller properties.
With this the interrupt parent of NAND node changes so fix it
accordingly.
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/boot/dts/dra7-evm.dts | 1 +
arch/arm/boot/dts/dra7.dtsi | 4
arch/arm/boot/dts/dra72-evm.dts | 1 +
3 files changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
index ce11b0f..3fb1ced 100644
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -573,6 +573,7 @@
nand@0,0 {
compatible = ti,omap2-nand;
reg = 0 0 4; /* device IO registers */
+ interrupt-parent = crossbar_mpu;
interrupts = GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH;
ti,nand-ecc-opt = bch8;
ti,elm-id = elm;
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 8f1e25b..a4ab66d 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -1369,6 +1369,10 @@
gpmc,num-waitpins = 2;
#address-cells = 2;
#size-cells = 1;
+ gpio-controller;
+ #gpio-cells = 2;
+ interrupt-controller;
+ #interrupt-cells = 2;
status = disabled;
};
diff --git a/arch/arm/boot/dts/dra72-evm.dts b/arch/arm/boot/dts/dra72-evm.dts
index 8037384..fc0677a 100644
--- a/arch/arm/boot/dts/dra72-evm.dts
+++ b/arch/arm/boot/dts/dra72-evm.dts
@@ -386,6 +386,7 @@
* SW5.9 (GPMC_WPN) = OFF (HIGH)
*/
reg = 0 0 4; /* device IO registers */
+ interrupt-parent = crossbar_mpu;
ti,nand-ecc-opt = bch8;
ti,elm-id = elm;
nand-bus-width = 16;
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 07/22] memory: omap-gpmc: Remove NAND IRQ code
NAND IRQs will now be managed directly in the OMAP NAND driver
so remove the IRQchip model.
Another patch will add back GPIO-IRQchip code to handle the
WAITPIN interrupts.
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/mach-omap2/gpmc-nand.c | 4 +-
drivers/memory/omap-gpmc.c | 164 +---
include/linux/omap-gpmc.h | 5 +-
3 files changed, 5 insertions(+), 168 deletions(-)
diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index 04e6998..ffe646a 100644
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -80,7 +80,6 @@ int gpmc_nand_init(struct omap_nand_platform_data
*gpmc_nand_data,
struct resource gpmc_nand_res[] = {
{ .flags = IORESOURCE_MEM, },
{ .flags = IORESOURCE_IRQ, },
- { .flags = IORESOURCE_IRQ, },
};
BUG_ON(gpmc_nand_data-cs = GPMC_CS_NUM);
@@ -93,8 +92,7 @@ int gpmc_nand_init(struct omap_nand_platform_data
*gpmc_nand_data,
return err;
}
gpmc_nand_res[0].end = gpmc_nand_res[0].start + NAND_IO_SIZE - 1;
- gpmc_nand_res[1].start = gpmc_get_client_irq(GPMC_IRQ_FIFOEVENTENABLE);
- gpmc_nand_res[2].start = gpmc_get_client_irq(GPMC_IRQ_COUNT_EVENT);
+ gpmc_nand_res[1].start = gpmc_get_irq();
memset(s, 0, sizeof(struct gpmc_settings));
if (gpmc_nand_data-of_node)
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index 41df030..e70a8df 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -123,12 +123,6 @@
#define GPMC_CS_NAND_ADDRESS 0x20
#define GPMC_CS_NAND_DATA 0x24
-/* Control Commands */
-#define GPMC_CONFIG_RDY_BSY0x0001
-#define GPMC_CONFIG_DEV_SIZE 0x0002
-#define GPMC_CONFIG_DEV_TYPE 0x0003
-#define GPMC_SET_IRQ_STATUS0x0004
-
#define GPMC_CONFIG1_WRAPBURST_SUPP (1 31)
#define GPMC_CONFIG1_READMULTIPLE_SUPP (1 30)
#define GPMC_CONFIG1_READTYPE_ASYNC (0 29)
@@ -176,17 +170,11 @@
#define GPMC_CONFIG_WRITEPROTECT 0x0010
#define WR_RD_PIN_MONITORING 0x0060
-#define GPMC_ENABLE_IRQ0x000d
-
/* ECC commands */
#define GPMC_ECC_READ 0 /* Reset Hardware ECC for read */
#define GPMC_ECC_WRITE 1 /* Reset Hardware ECC for write */
#define GPMC_ECC_READSYN 2 /* Reset before syndrom is read back */
-/* XXX: Only NAND irq has been considered,currently these are the only ones
used
- */
-#defineGPMC_NR_IRQ 2
-
enum gpmc_clk_domain {
GPMC_CD_FCLK,
GPMC_CD_CLK
@@ -201,11 +189,6 @@ struct gpmc_cs_data {
struct resource mem;
};
-struct gpmc_client_irq {
- unsignedirq;
- u32 bitmask;
-};
-
/* Structure to save gpmc cs context */
struct gpmc_cs_config {
u32 config1;
@@ -233,10 +216,6 @@ struct omap3_gpmc_regs {
struct gpmc_cs_config cs_context[GPMC_CS_NUM];
};
-static struct gpmc_client_irq gpmc_client_irq[GPMC_NR_IRQ];
-static struct irq_chip gpmc_irq_chip;
-static int gpmc_irq_start;
-
static struct resource gpmc_mem_root;
static struct gpmc_cs_data gpmc_cs[GPMC_CS_NUM];
static DEFINE_SPINLOCK(gpmc_mem_lock);
@@ -244,15 +223,13 @@ static DEFINE_SPINLOCK(gpmc_mem_lock);
static unsigned int gpmc_cs_num = GPMC_CS_NUM;
static unsigned int gpmc_nr_waitpins;
static struct device *gpmc_dev;
-static int gpmc_irq;
+static int gpmc_irq = -EINVAL;
static resource_size_t phys_base, mem_size;
static unsigned gpmc_capability;
static void __iomem *gpmc_base;
static struct clk *gpmc_l3_clk;
-static irqreturn_t gpmc_handle_irq(int irq, void *dev);
-
static void gpmc_write_reg(int idx, u32 val)
{
writel_relaxed(val, gpmc_base + idx);
@@ -1037,14 +1014,6 @@ int gpmc_configure(int cmd, int wval)
u32 regval;
switch (cmd) {
- case GPMC_ENABLE_IRQ:
- gpmc_write_reg(GPMC_IRQENABLE, wval);
- break;
-
- case GPMC_SET_IRQ_STATUS:
- gpmc_write_reg(GPMC_IRQSTATUS, wval);
- break;
-
case GPMC_CONFIG_WP:
regval = gpmc_read_reg(GPMC_CONFIG);
if (wval)
@@ -1132,113 +1101,9 @@ struct gpmc_nand_ops *gpmc_omap_get_nand_ops(struct
gpmc_nand_regs *reg, int cs)
}
EXPORT_SYMBOL_GPL(gpmc_omap_get_nand_ops);
-int gpmc_get_client_irq(unsigned irq_config)
-{
- int i;
-
- if (hweight32(irq_config) 1)
- return 0;
-
- for (i = 0; i GPMC_NR_IRQ; i++)
- if (gpmc_client_irq[i].bitmask irq_config)
- return gpmc_client_irq[i].irq;
-
- return 0;
-}
-
-static int gpmc_irq_endis(unsigned irq, bool endis)
-{
- int i;
- u32 regval;
-
- for (i = 0; i GPMC_NR_IRQ; i++)
- if (irq == gpmc_client_irq[i].irq) {
- regval = gpmc_read_reg(GPMC_IRQENABLE);
-
[PATCH v2 08/22] memory: omap-gpmc: Add IRQ ops for GPMC-NAND interface
Provide functions to enable/disable NAND IRQs, get
NAND event status and clear NAND events.
The NAND events of interest are TERMCOUNT and FIFOEVENT.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/memory/omap-gpmc.c | 50 ++
include/linux/omap-gpmc.h | 4
2 files changed, 54 insertions(+)
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index e70a8df..713d7af 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -1078,8 +1078,58 @@ static bool gpmc_nand_writebuffer_empty(void)
return false;
}
+static int gpmc_nand_irq_enable(enum gpmc_nand_irq irq)
+{
+ u32 reg;
+
+ if (irq GPMC_NAND_IRQ_TERMCOUNT)
+ return -EINVAL;
+
+ reg = gpmc_read_reg(GPMC_IRQENABLE);
+ reg |= BIT(irq);
+ gpmc_write_reg(GPMC_IRQENABLE, reg);
+
+ return 0;
+}
+
+static int gpmc_nand_irq_disable(enum gpmc_nand_irq irq)
+{
+ u32 reg;
+
+ if (irq GPMC_NAND_IRQ_TERMCOUNT)
+ return -EINVAL;
+
+ reg = gpmc_read_reg(GPMC_IRQENABLE);
+ reg = ~BIT(irq);
+ gpmc_write_reg(GPMC_IRQENABLE, reg);
+
+ return 0;
+}
+
+static void gpmc_nand_irq_clear(enum gpmc_nand_irq irq)
+{
+ if (irq GPMC_NAND_IRQ_TERMCOUNT)
+ return;
+
+ /* setting bit to 1 clears the bit in IRQSTATUS */
+ gpmc_write_reg(GPMC_IRQSTATUS, BIT(irq));
+}
+
+static u32 gpmc_nand_irq_status(void)
+{
+ u32 reg = gpmc_read_reg(GPMC_IRQSTATUS);
+
+ /* Mask out non-NAND bits */
+ reg = GPMC_IRQENABLE_FIFOEVENT | GPMC_IRQENABLE_TERMCOUNT;
+ return reg;
+}
+
static struct gpmc_nand_ops nand_ops = {
.nand_writebuffer_empty = gpmc_nand_writebuffer_empty,
+ .nand_irq_enable = gpmc_nand_irq_enable,
+ .nand_irq_disable = gpmc_nand_irq_disable,
+ .nand_irq_clear = gpmc_nand_irq_clear,
+ .nand_irq_status = gpmc_nand_irq_status,
};
/**
diff --git a/include/linux/omap-gpmc.h b/include/linux/omap-gpmc.h
index 44e322f..063a84f 100644
--- a/include/linux/omap-gpmc.h
+++ b/include/linux/omap-gpmc.h
@@ -11,6 +11,10 @@
#define GPMC_CONFIG_WP 0x0005
+/* GPMC IRQENABLE/IRQSTATUS BIT defs */
+#define GPMC_IRQENABLE_FIFOEVENT BIT(0)
+#define GPMC_IRQENABLE_TERMCOUNT BIT(1)
+
enum gpmc_nand_irq {
GPMC_NAND_IRQ_FIFOEVENT = 0,
GPMC_NAND_IRQ_TERMCOUNT,
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/22] mtd: nand: omap2: Use gpmc_omap_get_nand_ops() to get NAND registers
Deprecate nand register passing via platform data and use
gpmc_omap_get_nand_ops() instead.
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/mach-omap2/gpmc-nand.c | 2 --
drivers/mtd/nand/omap2.c | 9 -
include/linux/platform_data/mtd-nand-omap2.h | 4 +++-
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-omap2/gpmc-nand.c b/arch/arm/mach-omap2/gpmc-nand.c
index 72918c4..04e6998 100644
--- a/arch/arm/mach-omap2/gpmc-nand.c
+++ b/arch/arm/mach-omap2/gpmc-nand.c
@@ -121,8 +121,6 @@ int gpmc_nand_init(struct omap_nand_platform_data
*gpmc_nand_data,
if (err 0)
goto out_free_cs;
- gpmc_update_nand_reg(gpmc_nand_data-reg, gpmc_nand_data-cs);
-
if (!gpmc_hwecc_bch_capable(gpmc_nand_data-ecc_opt)) {
pr_err(omap2-nand: Unsupported NAND ECC scheme selected\n);
err = -EINVAL;
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 60fa899..f214fe2 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -28,6 +28,7 @@
#include linux/mtd/nand_bch.h
#include linux/platform_data/elm.h
+#include linux/omap-gpmc.h
#include linux/platform_data/mtd-nand-omap2.h
#defineDRIVER_NAME omap2-nand
@@ -169,7 +170,9 @@ struct omap_nand_info {
} iomode;
u_char *buf;
int buf_len;
+ /* Interface to GPMC */
struct gpmc_nand_regs reg;
+ struct gpmc_nand_ops*ops;
/* generated at runtime depending on ECC algorithm and layout selected
*/
struct nand_ecclayout oobinfo;
/* fields specific for BCHx_HW ECC scheme */
@@ -1677,9 +1680,13 @@ static int omap_nand_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, info);
+ info-ops = gpmc_omap_get_nand_ops(info-reg, info-gpmc_cs);
+ if (!info-ops) {
+ dev_err(pdev-dev, Failed to get GPMC-NAND interface\n);
+ return -ENODEV;
+ }
info-pdev = pdev;
info-gpmc_cs = pdata-cs;
- info-reg = pdata-reg;
info-of_node = pdata-of_node;
info-ecc_opt = pdata-ecc_opt;
mtd = info-mtd;
diff --git a/include/linux/platform_data/mtd-nand-omap2.h
b/include/linux/platform_data/mtd-nand-omap2.h
index 090bbab..a067f58 100644
--- a/include/linux/platform_data/mtd-nand-omap2.h
+++ b/include/linux/platform_data/mtd-nand-omap2.h
@@ -75,10 +75,12 @@ struct omap_nand_platform_data {
enum nand_ioxfer_type;
int devsize;
enum omap_ecc ecc_opt;
- struct gpmc_nand_regs reg;
/* for passing the partitions */
struct device_node *of_node;
struct device_node *elm_of_node;
+
+ /* deprecated */
+ struct gpmc_nand_regs reg;
};
#endif
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/22] mtd: nand: omap2: Switch to using GPMC-NAND ops for writebuffer empty check
Instead of accessing the gpmc_status register directly start
using the gpmc_nand_ops-nand_writebuffer_empty() helper
to check write buffer empty status.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/mtd/nand/omap2.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index f214fe2..5c2f6df 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -289,15 +289,11 @@ static void omap_write_buf8(struct mtd_info *mtd, const
u_char *buf, int len)
struct omap_nand_info *info = container_of(mtd,
struct omap_nand_info, mtd);
u_char *p = (u_char *)buf;
- u32 status = 0;
while (len--) {
iowrite8(*p++, info-nand.IO_ADDR_W);
/* wait until buffer is available for write */
- do {
- status = readl(info-reg.gpmc_status)
- STATUS_BUFF_EMPTY;
- } while (!status);
+ while (info-ops-nand_writebuffer_empty());
}
}
@@ -325,17 +321,13 @@ static void omap_write_buf16(struct mtd_info *mtd, const
u_char * buf, int len)
struct omap_nand_info *info = container_of(mtd,
struct omap_nand_info, mtd);
u16 *p = (u16 *) buf;
- u32 status = 0;
/* FIXME try bursts of writesw() or DMA ... */
len = 1;
while (len--) {
iowrite16(*p++, info-nand.IO_ADDR_W);
/* wait until buffer is available for write */
- do {
- status = readl(info-reg.gpmc_status)
- STATUS_BUFF_EMPTY;
- } while (!status);
+ while (info-ops-nand_writebuffer_empty());
}
}
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/22] memory: omap-gpmc: Add GPMC-NAND ops to get writebufferempty status
This is needed by OMAP NAND driver to poll the empty status
of the writebuffer.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/memory/omap-gpmc.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c
index 79d78ab..41df030 100644
--- a/drivers/memory/omap-gpmc.c
+++ b/drivers/memory/omap-gpmc.c
@@ -81,6 +81,8 @@
#define GPMC_CONFIG_LIMITEDADDRESS BIT(1)
+#define GPMC_STATUS_EMPTYWRITEBUFFERSTATUS BIT(0)
+
#defineGPMC_CONFIG2_CSEXTRADELAY BIT(7)
#defineGPMC_CONFIG3_ADVEXTRADELAY BIT(7)
#defineGPMC_CONFIG4_OEEXTRADELAY BIT(7)
@@ -1099,7 +1101,17 @@ void gpmc_update_nand_reg(struct gpmc_nand_regs *reg,
int cs)
}
}
-static struct gpmc_nand_ops nand_ops;
+static bool gpmc_nand_writebuffer_empty(void)
+{
+ if (gpmc_read_reg(GPMC_STATUS) GPMC_STATUS_EMPTYWRITEBUFFERSTATUS)
+ return true;
+
+ return false;
+}
+
+static struct gpmc_nand_ops nand_ops = {
+ .nand_writebuffer_empty = gpmc_nand_writebuffer_empty,
+};
/**
* gpmc_omap_get_nand_ops - Get the GPMC NAND interface
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On Fri, Aug 07, 2015 at 10:41:57AM +0200, Sebastian Andrzej Siewior wrote: This DMA driver is used by 8250-omap on DRA7-evm. There is one requirement that is to pause a transfer. This is currently used on the RX side. It is possible that the UART HW aborted the RX (UART's RX-timeout) but the DMA controller starts the transfer shortly after. Before we can manually purge the FIFO we need to pause the transfer, check how many bytes it already received and terminate the transfer without it making any progress. From testing on the TX side it seems that it is possible that we invoke pause once the transfer has completed which is indicated by the missing CCR_ENABLE bit but before the interrupt has been noticed. In that case the interrupt will come even after disabling it. How do you cope with the OMAP DMA hardware clearing its FIFO when you pause it? The problem is that on mem-to-device transfers, the DMA hardware can prefetch some data from memory into its FIFO before the device wants the data. If you then disable the channel, the hardware clears the FIFO. It is unspecified whether the hardware updates the source address in this case, or by how much. So it's pretty hard to undo the prefetching in software. The net result is: data loss. This is why I explicitly did not implement pause/resume for non-cyclic channels. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] OMAPDSS: Fix omap_dss_find_output_by_port_node() port refcount decrement
Fix omap_dss_find_output_by_port_node() port parameter refcount
decrementation. The only user of dss_of_port_get_parent_device()
function is omap_dss_find_output_by_port_node() and it assumes the
refcount of the port parameter is not decremented by the call.
Signed-off-by: Jyri Sarha jsa...@ti.com
---
drivers/video/fbdev/omap2/dss/dss-of.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/omap2/dss/dss-of.c
b/drivers/video/fbdev/omap2/dss/dss-of.c
index ab6ef16..43f999d 100644
--- a/drivers/video/fbdev/omap2/dss/dss-of.c
+++ b/drivers/video/fbdev/omap2/dss/dss-of.c
@@ -95,7 +95,7 @@ struct device_node *dss_of_port_get_parent_device(struct
device_node *port)
if (!port)
return NULL;
- np = of_get_next_parent(port);
+ np = of_get_parent(port);
for (i = 0; i 2 np; ++i) {
struct property *prop;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
On 7 August 2015 at 10:25, Martin Sperl mar...@sperl.org wrote: On 8/6/2015 23:33, Russell King - ARM Linux wrote: On Thu, Aug 06, 2015 at 06:14:00PM +0200, Geert Uytterhoeven wrote: Irrespective of the dummy bytes. What if the spi device is not a FLASH ROM, but some other device, which receives a data packet that accidentally looks like an m25p80 READ command? Well, for the most part it looks like it should still work, but there could be a gotcha, but first, let's get rid of a myth there. The QSPI is _not_ specific to the M25P80. The manual says nothing about being specific to that device. What it says is that it's for SPI NOR memory. It will work with bus widths of 1, 2 or 4 data lines, so it probably works with non-M25P80 SPI NOR devices too - and the fact that the read and write commands are completely programmable suggests that using it with SPI NOR devices which do not use the M25P80 read command value is intended. The SFI is a state machine based translator which sits behind the SPI interface (look at the manual). It sequence sthe SPI bus through a series of standard SPI states which happen to be the states I detailed above. Now, the first byte of the SFI-generated SPI message can be programmed to any 8 bit value. So the first byte of the SPI message is totally under software control. The next one to four bytes which comprise the address can be controlled to by deciding where in the memory map to start reading from. Hence, the value of those bytes is also totally under software control. The number of dummy bytes can be programmed too. So far so good. So, if we know that we have a SPI message which says send 0x01 0x20 0x30, send one dummy byte, read 32 bytes, if we program the SFI to send a read command as 0x01, program an address length of 2 bytes with one dummy byte, and then read the next 32 bytes at the appropriate offset in the memory mapping to cause the next two bytes to be 0x20, 0x30, then what we end up with on the bus is: send 0x01, 0x20, 0x30 send one dummy byte That much is good, but now is the problem - how does the SFI know that we're going to require to read 32 bytes? I think the answer to that is that it doesn't know, so it probably just reads the number of bytes which the access on the SoC bus is asking for, which makes it indeterminant from a software point of view to control how many bytes will be read without provoking another send 0x01, next address, dummy byte sequence. So, I'm now on the side of not parsing commands in the SPI driver, and back on the idea that this needs to be handled in some other manner which doesn't involve polluting the SPI core with flag-hacks. So I see 2 distinct options: Have the nor driver modified to run SPI commands and then ask the SPI framework (and driver) to switch into mmap mode: Would probably look something like this inside the nor driver: /* lock spi bus for other activities */ spi_bus_lock(spi); /* send the configuration for mmap */ t[0].tx_buf = flash-command; t[0].len = m25p_cmdsz(nor); spi_message_add_tail(t[0], m); t[1].tx_buf = dummy_buffer; t[1].len = dummy; spi_message_add_tail(t[1], m); spi_sync(spi, m); /* switch to mmap mode */ spi-mode |= SPI_MMAP; Presumably you will do this *before* sending the read command so the SPI driver can reverse-engineer it according to the spi-nor read pattern. Then the mmap mode can be set in spi_sync() but you also have to lock the spi controller to prevent other transfers from resetting it. Or you can pass the read buffer to the SPI master driver as well to do the memcpy for you. Which you want to do anyway in case the SPI master can use DMA to fill the buffer rather than memcpy. spi_setup(spi); /* run the mmapped transfers bypassing the spi-layer */ memcpy(...) /* open questions here: which address range * and how to detect if transfer is done */ Presumably when the memcpy ends the transfer is done. /* restore back to normal mode */ spi-mode = ~SPI_MMAP; spi_setup(spi); /* unlock spi bus for other activities */ spi_bus_unlock(spi); Presumably you have to restore to non-mmap mode only when you receive a command that requires it. If the next command is read with same configuration you can just keep reading. The downside is that it requires modification in several places (nor-framework, spi-framework plus the driver) and it would not be generic enough... You only need modification to the SPI master driver and m25p80 driver. The SPI master driver is where the hardware-specific operation is executed and m25p80 is where you have the information so you can decide to take advantage of the hardware acceleration. IMO such a situation is feasible if we only got a single device on the spi-bus, but leaves a lot of questions open... Alternatively we could create an additional api. The spi master driver can track what
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On 08/07/2015 11:44 AM, Peter Ujfalusi wrote:
On 08/07/2015 11:41 AM, Sebastian Andrzej Siewior wrote:
This DMA driver is used by 8250-omap on DRA7-evm. There is one
requirement that is to pause a transfer. This is currently used on the RX
side. It is possible that the UART HW aborted the RX (UART's RX-timeout)
but the DMA controller starts the transfer shortly after.
Before we can manually purge the FIFO we need to pause the transfer,
check how many bytes it already received and terminate the transfer
without it making any progress.
From testing on the TX side it seems that it is possible that we invoke
pause once the transfer has completed which is indicated by the missing
CCR_ENABLE bit but before the interrupt has been noticed. In that case the
interrupt will come even after disabling it.
The AM572x manual says that we have to wait for the CCR_RD_ACTIVE
CCR_WR_ACTIVE bits to be gone before programming it again here is the
drain loop. Also it looks like without the drain the TX-transfer makes
sometimes progress.
One note: The pause + resume combo is broken because after resume the
the complete transfer will be programmed again. That means the already
transferred bytes (until the pause event) will be sent again. This is
currently not important for my UART user because it does only pause +
terminate.
with a short testing audio did not broke (the only user of pause/resume)
Some comments embedded.
Cc: sta...@vger.kernel.org
Why stable? This is not fixing any bugs since the PAUSE was not allowed for
non cyclic transfers.
Hmmm. The DRA7x was using pause before for UART. I just did not see it
coming that it was not allowed here. John made a similar change to the
edma driver and I assumed it went stable but now I see that it was just
cherry-picked into the ti tree.
If you are not comfortable it being stable material I can drop it.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
drivers/dma/omap-dma.c | 54
++
1 file changed, 41 insertions(+), 13 deletions(-)
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 249445c8a4c6..6b8497203caf 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -299,7 +299,7 @@ static void omap_dma_start(struct omap_chan *c, struct
omap_desc *d)
omap_dma_chan_write(c, CCR, d-ccr | CCR_ENABLE);
}
-static void omap_dma_stop(struct omap_chan *c)
+static int omap_dma_stop(struct omap_chan *c)
{
struct omap_dmadev *od = to_omap_dma_dev(c-vc.chan.device);
uint32_t val;
@@ -342,8 +342,26 @@ static void omap_dma_stop(struct omap_chan *c)
omap_dma_glbl_write(od, OCP_SYSCONFIG, sysconfig);
} else {
+int i = 0;
+
+if (!(val CCR_ENABLE))
+return -EINVAL;
+
val = ~CCR_ENABLE;
omap_dma_chan_write(c, CCR, val);
+do {
+val = omap_dma_chan_read(c, CCR);
+if (!(val (CCR_RD_ACTIVE | CCR_WR_ACTIVE)))
+break;
+if (i 100)
if (++i 100)
break;
to avoid infinite loop?
Ah. So I forgot to increment the counter. A few lines above there is
the same loop as a workaround for something. This is the same loop. I
could merge the loop + warning if you prefer. to have those things in
one place. I could also just increment i. Merging the two loops might
be better.
+break;
+udelay(5);
+} while (1);
+
+if (val (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
if (i 100) ?
While that would work, too I think it is more explicit to the reader if
you check for the condition that is important to you.
+dev_err(c-vc.chan.device-dev,
+DMA drain did not complete on lch %d\n,
+c-dma_ch);
}
mb();
Sebastian
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] OMAPDSS: of-dss: omap_dss_find_output_by_port_node() keep port refcount
Hi, On 06/08/15 21:41, Jyri Sarha wrote: The only user of dss_of_port_get_parent_device() function is omap_dss_find_output_by_port_node() and it assumes the refcount of the port parameter is not decremented by the call. The subject of the patch should contain dss-of, not of-dss. Although for both patches I think it's fine to use plain OMAPDSS: prefix. The subject should also say fix or such. The description above is kind of detached. A patch description should generally describe something in the lines of what the current behavior is, what the problem is, and what the patch does. And the desc should be independent of the subject. This particular case is rather simple, and it's clear that dss_of_port_get_parent_device() is not supposed to decrease the refcount of the port parameter (as that kind of behavior is normally a special case). And so it should be enough to say what the issue seen is and that you fix the function to not decrement the parameter's refcount. Tomi signature.asc Description: OpenPGP digital signature
Re: Linux 4.2.0-rc5: am335x: musb warnings
On Fri, Aug 7, 2015 at 12:16 PM, Yegor Yefremov yegorsli...@googlemail.com wrote: On Thu, Aug 6, 2015 at 4:21 PM, Felipe Balbi ba...@ti.com wrote: HI, On Thu, Aug 06, 2015 at 09:40:26AM +0200, Yegor Yefremov wrote: I performed a stress test with several FT4232H chips connected to a how many ? # lsusb -t /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M |__ Port 1: Dev 2, If 0, Class=, Driver=hub/4p, 480M |__ Port 1: Dev 3, If 0, Class=, Driver=ftdi_sio, 480M |__ Port 1: Dev 3, If 1, Class=, Driver=ftdi_sio, 480M |__ Port 1: Dev 3, If 2, Class=, Driver=ftdi_sio, 480M |__ Port 1: Dev 3, If 3, Class=, Driver=ftdi_sio, 480M |__ Port 2: Dev 4, If 0, Class=, Driver=ftdi_sio, 480M |__ Port 2: Dev 4, If 1, Class=, Driver=ftdi_sio, 480M |__ Port 2: Dev 4, If 2, Class=, Driver=ftdi_sio, 480M |__ Port 2: Dev 4, If 3, Class=, Driver=ftdi_sio, 480M |__ Port 3: Dev 5, If 0, Class=, Driver=ftdi_sio, 480M |__ Port 3: Dev 5, If 1, Class=, Driver=ftdi_sio, 480M |__ Port 3: Dev 5, If 2, Class=, Driver=ftdi_sio, 480M |__ Port 3: Dev 5, If 3, Class=, Driver=ftdi_sio, 480M 3 chips a 4-ports are attached. Warnings appear on another device (without internal hub) with only one FT4232H too: # lsusb -t /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M |__ Port 1: Dev 2, If 0, Class=, Driver=ftdi_sio, 480M |__ Port 1: Dev 2, If 1, Class=, Driver=ftdi_sio, 480M |__ Port 1: Dev 2, If 2, Class=, Driver=ftdi_sio, 480M |__ Port 1: Dev 2, If 3, Class=, Driver=ftdi_sio, 480M hub, that is attached to one of the musb ports. So far the test was successful for several hours. But I've seen following warnings: musb_host_rx 1973: Rx interrupt with no errors or packet! musb_ep_program 931: broken !rx_reinit, ep5 csr 0203 musb_host_rx 1973: Rx interrupt with no errors or packet! musb_host_rx 1973: Rx interrupt with no errors or packet! musb_host_rx 1973: Rx interrupt with no errors or packet! musb_host_rx 1973: Rx interrupt with no errors or packet! musb_ep_program 931: broken !rx_reinit, ep5 csr 0003 musb_host_rx 1973: Rx interrupt with no errors or packet! musb_ep_program 931: broken !rx_reinit, ep7 csr 0003 Is this expected behavior? no, that shouldn't happen, but it does and, apparently, in more than one implementation. Wondering if you're running into endpoint limitation due to MUSB's poor transfer scheduling for non-bulk endpoints. -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] OMAPDSS: dss-of: Fix node refcount leak in omapdss_of_get_next_port()
On 06/08/15 21:41, Jyri Sarha wrote: Signed-off-by: Jyri Sarha jsa...@ti.com Please always fill in the patch description. In simplest cases it may be the same as in the subject. --- drivers/video/fbdev/omap2/dss/dss-of.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/fbdev/omap2/dss/dss-of.c b/drivers/video/fbdev/omap2/dss/dss-of.c index 928ee63..ab6ef16 100644 --- a/drivers/video/fbdev/omap2/dss/dss-of.c +++ b/drivers/video/fbdev/omap2/dss/dss-of.c @@ -60,6 +60,7 @@ omapdss_of_get_next_port(const struct device_node *parent, } prev = port; } while (of_node_cmp(port-name, port) != 0); + of_node_put(ports); I think a blank line is needed above of_node_put(). Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On 08/07/2015 11:41 AM, Sebastian Andrzej Siewior wrote:
This DMA driver is used by 8250-omap on DRA7-evm. There is one
requirement that is to pause a transfer. This is currently used on the RX
side. It is possible that the UART HW aborted the RX (UART's RX-timeout)
but the DMA controller starts the transfer shortly after.
Before we can manually purge the FIFO we need to pause the transfer,
check how many bytes it already received and terminate the transfer
without it making any progress.
From testing on the TX side it seems that it is possible that we invoke
pause once the transfer has completed which is indicated by the missing
CCR_ENABLE bit but before the interrupt has been noticed. In that case the
interrupt will come even after disabling it.
The AM572x manual says that we have to wait for the CCR_RD_ACTIVE
CCR_WR_ACTIVE bits to be gone before programming it again here is the
drain loop. Also it looks like without the drain the TX-transfer makes
sometimes progress.
One note: The pause + resume combo is broken because after resume the
the complete transfer will be programmed again. That means the already
transferred bytes (until the pause event) will be sent again. This is
currently not important for my UART user because it does only pause +
terminate.
with a short testing audio did not broke (the only user of pause/resume)
Some comments embedded.
Cc: sta...@vger.kernel.org
Why stable? This is not fixing any bugs since the PAUSE was not allowed for
non cyclic transfers.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
drivers/dma/omap-dma.c | 54
++
1 file changed, 41 insertions(+), 13 deletions(-)
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 249445c8a4c6..6b8497203caf 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -299,7 +299,7 @@ static void omap_dma_start(struct omap_chan *c, struct
omap_desc *d)
omap_dma_chan_write(c, CCR, d-ccr | CCR_ENABLE);
}
-static void omap_dma_stop(struct omap_chan *c)
+static int omap_dma_stop(struct omap_chan *c)
{
struct omap_dmadev *od = to_omap_dma_dev(c-vc.chan.device);
uint32_t val;
@@ -342,8 +342,26 @@ static void omap_dma_stop(struct omap_chan *c)
omap_dma_glbl_write(od, OCP_SYSCONFIG, sysconfig);
} else {
+ int i = 0;
+
+ if (!(val CCR_ENABLE))
+ return -EINVAL;
+
val = ~CCR_ENABLE;
omap_dma_chan_write(c, CCR, val);
+ do {
+ val = omap_dma_chan_read(c, CCR);
+ if (!(val (CCR_RD_ACTIVE | CCR_WR_ACTIVE)))
+ break;
+ if (i 100)
if (++i 100)
break;
to avoid infinite loop?
+ break;
+ udelay(5);
+ } while (1);
+
+ if (val (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
if (i 100) ?
+ dev_err(c-vc.chan.device-dev,
+ DMA drain did not complete on lch %d\n,
+ c-dma_ch);
}
mb();
@@ -358,6 +376,7 @@ static void omap_dma_stop(struct omap_chan *c)
omap_dma_chan_write(c, CLNK_CTRL, val);
}
+ return 0;
}
static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d,
@@ -728,6 +747,8 @@ static enum dma_status omap_dma_tx_status(struct dma_chan
*chan,
} else {
txstate-residue = 0;
}
+ if (ret == DMA_IN_PROGRESS c-paused)
+ ret = DMA_PAUSED;
spin_unlock_irqrestore(c-vc.lock, flags);
return ret;
@@ -1053,28 +1074,33 @@ static int omap_dma_terminate_all(struct dma_chan
*chan)
static int omap_dma_pause(struct dma_chan *chan)
{
struct omap_chan *c = to_omap_dma_chan(chan);
+ struct omap_dmadev *od = to_omap_dma_dev(chan-device);
+ unsigned long flags;
+ int ret = -EINVAL;
- /* Pause/Resume only allowed with cyclic mode */
- if (!c-cyclic)
- return -EINVAL;
+ spin_lock_irqsave(od-irq_lock, flags);
- if (!c-paused) {
- omap_dma_stop(c);
- c-paused = true;
+ if (!c-paused c-desc) {
+ ret = omap_dma_stop(c);
+ if (!ret)
+ c-paused = true;
}
- return 0;
+ spin_unlock_irqrestore(od-irq_lock, flags);
+
+ return ret;
}
static int omap_dma_resume(struct dma_chan *chan)
{
struct omap_chan *c = to_omap_dma_chan(chan);
+ struct omap_dmadev *od = to_omap_dma_dev(chan-device);
+ unsigned long flags;
+ int ret = -EINVAL;
- /* Pause/Resume only allowed with cyclic mode */
- if (!c-cyclic)
- return -EINVAL;
+ spin_lock_irqsave(od-irq_lock, flags);
- if (c-paused) {
+ if (c-paused
Re: Linux 4.2.0-rc5: am335x: musb warnings
On Thu, Aug 6, 2015 at 4:21 PM, Felipe Balbi ba...@ti.com wrote: HI, On Thu, Aug 06, 2015 at 09:40:26AM +0200, Yegor Yefremov wrote: I performed a stress test with several FT4232H chips connected to a how many ? # lsusb -t /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M |__ Port 1: Dev 2, If 0, Class=, Driver=hub/4p, 480M |__ Port 1: Dev 3, If 0, Class=, Driver=ftdi_sio, 480M |__ Port 1: Dev 3, If 1, Class=, Driver=ftdi_sio, 480M |__ Port 1: Dev 3, If 2, Class=, Driver=ftdi_sio, 480M |__ Port 1: Dev 3, If 3, Class=, Driver=ftdi_sio, 480M |__ Port 2: Dev 4, If 0, Class=, Driver=ftdi_sio, 480M |__ Port 2: Dev 4, If 1, Class=, Driver=ftdi_sio, 480M |__ Port 2: Dev 4, If 2, Class=, Driver=ftdi_sio, 480M |__ Port 2: Dev 4, If 3, Class=, Driver=ftdi_sio, 480M |__ Port 3: Dev 5, If 0, Class=, Driver=ftdi_sio, 480M |__ Port 3: Dev 5, If 1, Class=, Driver=ftdi_sio, 480M |__ Port 3: Dev 5, If 2, Class=, Driver=ftdi_sio, 480M |__ Port 3: Dev 5, If 3, Class=, Driver=ftdi_sio, 480M 3 chips a 4-ports are attached. hub, that is attached to one of the musb ports. So far the test was successful for several hours. But I've seen following warnings: musb_host_rx 1973: Rx interrupt with no errors or packet! musb_ep_program 931: broken !rx_reinit, ep5 csr 0203 musb_host_rx 1973: Rx interrupt with no errors or packet! musb_host_rx 1973: Rx interrupt with no errors or packet! musb_host_rx 1973: Rx interrupt with no errors or packet! musb_host_rx 1973: Rx interrupt with no errors or packet! musb_ep_program 931: broken !rx_reinit, ep5 csr 0003 musb_host_rx 1973: Rx interrupt with no errors or packet! musb_ep_program 931: broken !rx_reinit, ep7 csr 0003 Is this expected behavior? no, that shouldn't happen, but it does and, apparently, in more than one implementation. Wondering if you're running into endpoint limitation due to MUSB's poor transfer scheduling for non-bulk endpoints. -- balbi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] OMAPDSS: Fix node refcount leak in omapdss_of_get_next_port()
Fix node refcount leak in omapdss_of_get_next_port(). Signed-off-by: Jyri Sarha jsa...@ti.com --- drivers/video/fbdev/omap2/dss/dss-of.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/fbdev/omap2/dss/dss-of.c b/drivers/video/fbdev/omap2/dss/dss-of.c index 928ee63..ab6ef16 100644 --- a/drivers/video/fbdev/omap2/dss/dss-of.c +++ b/drivers/video/fbdev/omap2/dss/dss-of.c @@ -60,6 +60,7 @@ omapdss_of_get_next_port(const struct device_node *parent, } prev = port; } while (of_node_cmp(port-name, port) != 0); + of_node_put(ports); } return port; -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/2] OMAPDSS: Fix of-node refcount issues
Changes since the first version: - Fix commit descriptions and subject according to Tomi's comments I found couple of refcounting issues related to OMAP DSS of-node handling. Second patch should fix the ERROR: Bad of_node_put() on /encoder@0/ports/port@1 -problem. In the long run it would make sense start using of_graph_*() functions in OMAP DSS too. However the semantics of of_graph_*() functions are quite different from omapdss_of_*() counterparts. So it better to just fix these bugs first. Jyri Sarha (2): OMAPDSS: Fix node refcount leak in omapdss_of_get_next_port() OMAPDSS: Fix omap_dss_find_output_by_port_node() port refcount decrement drivers/video/fbdev/omap2/dss/dss-of.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] dma: omap-dma: add support for pause of non-cyclic transfers
On Fri, Aug 07, 2015 at 05:36:05PM +0200, Sebastian Andrzej Siewior wrote: + /* + * We do not allow DMA_MEM_TO_DEV transfers to be paused. + * According to RMK the OMAP hardware might prefetch bytes from + * memory into its FIFO and not send it to the device due to the + * pause. The bytes in the FIFO are cleared on pause. It is + * unspecified by how many bytes the source address is updated + * if at all. Would you mind rewording the above. Please take the time to read the manuals for the device you are playing with. It's mostly documented in there. See the OMAP4430 ES2.x TRM, 16.4.18 and 16.4.19. 16.4.19 states that: When a source-synchronized channel is disabled during a transfer... In all other cases, the channel undergoes an abort. A source-synchronised channel is one where the fetching of data is under control of the device. In other words, a device-to-memory transfer. So, a destination-synchronised channel (which would be a memory-to-device transfer) undergoes an abort if you clear the enable bit. 16.4.20.4.6.2 defines what happens when a channel aborts: If an abort trigger occurs, the channel aborts immediately after completion of current read/write transactions and then FIFO is cleaned up. It doesn't define what cleaned up means, so that's open to some interpretation. That's why I contacted TI about this back in 2012: | What is the behaviour of the OMAP DMA hardware when the DMA4_CCRi[7] | enable bit is cleared and subsequently set without any additional | reprogramming? I'm thinking specifically about memory-to-device DMA | operations, in particular the UART ports. | | Will this allow a transfer to be temporarily paused, and then | subsequently resumed with out loss of data in the DMA hardware, as if | nothing had actually happened? The answer I received was to check that RD_ACTIVE and WR_ACTIVE are both clear _before_ disabling the channel, otherwise data loss will occur. The problem is that if the channel is active, then device activity can result in DMA activity starting between reading those as both clear and the write to DMA_CCR to clear the enable bit hitting the hardware. The explanation went on to say that if the DMA hardware can't drain the data in its FIFO to the destination, then data loss might occur. That will occur if we're destination-synchronised (eg) to a UART and the UART is not accepting any further data. Due to this, the OMAP DMA engine driver was submitted with this in the cover note: For the OMAP DMAengine driver, there's a few short-comings: 1. pause/resume support is not implemented; it's not clear whether the OMAP hardware is capable of supporting this sanely. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On Fri, Aug 07, 2015 at 06:20:44PM +0200, Sebastian Andrzej Siewior wrote:
On 08/07/2015 06:07 PM, Peter Hurley wrote:
If we look at what 8250-dma.c is doing:
if (dma-rx_running) {
dmaengine_pause(dma-rxchan);
It's 8250-dma.c which is silently _ignoring_ the return code, failing
to check that the operation it requested worked. Maybe this should be
WARN_ON(dmaengine_pause(dma-rxchan)) or at least it should print a
message?
Thanks for the suggestion; I'll hold on to that and push it after we add
the 8250 omap dma pause in mainline.
I have a patch ready with WARN_ON_ONCE() for 8250-omap and 8250-dma.
This warning would trigger on am335x/edma until v4.2-rc1 and omap-dma
based version is open. I could post it if you want me to.
Besides that those two, there are four other drivers ignoring the
return code dmaengine_pause().
It'll probably be a good idea if those are fixed too, and also have a
__must_check annotation on the declaration of dmaengine_pause() to
prevent future mishaps like this.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On Fri, Aug 07, 2015 at 11:08:48AM -0400, Peter Hurley wrote:
[ + Greg KH ]
On 08/07/2015 09:57 AM, Russell King - ARM Linux wrote:
As it is something that the driver has _not_ supported, you are clearly
adding a feature to an existing driver. It's not a bug fix.
If something else has been converted to pause channels and that is causing
a problem, then _that_ conversion is where the bug lies, not the lack of
support in the omap-dma.
FWIW, the actual bug is the api that silently does nothing.
Incorrect.
static int omap_dma_pause(struct dma_chan *chan)
{
struct omap_chan *c = to_omap_dma_chan(chan);
/* Pause/Resume only allowed with cyclic mode */
if (!c-cyclic)
return -EINVAL;
Asking for the channel to be paused will return an error code indicating
that the request failed. That will be propagated back through to the
return code of dmaengine_pause().
If we look at what 8250-dma.c is doing:
if (dma-rx_running) {
dmaengine_pause(dma-rxchan);
It's 8250-dma.c which is silently _ignoring_ the return code, failing
to check that the operation it requested worked. Maybe this should be
WARN_ON(dmaengine_pause(dma-rxchan)) or at least it should print a
message?
Right, so the patch which caused the regression is the one which arranged
for the 8250-dma + omap-dma combination to work together, not the missing
pause support in omap-dma.
That would be the original submission patch set for an entire driver,
the 8250_omap driver.
Well, that's where the bug lies, and I don't agree with your assessment
that it would mean reverting the whole thing.
The binding between the two drivers is controlled via DT. DT tells it
which DMA controller and which DMA input to use. So, as DMA is currently
broken on this, the solution is to break that link so that 8250-omap goes
back to using PIO only.
As the binding of drivers is controlled by DT, you can disable the binding
of these two drivers
No. 8250 dma is not a stand-alone driver. Even if it were, how would you go
back and fix DTs in the wild?
Well, we have reached an impass then.
I'm not going to accept a feature addition to a driver as a stable patch
without it being adequately tested over _several_ kernel revisions to
ensure that we don't end up cocking up some driver which uses the DMA
interfaces correctly. It's too big a risk.
So, I guess that means that older kernels will just have to remain broken -
all because the basic testing of the original code was never undertaken
to ensure that basic stuff like reception of characters worked properly.
Sorry, I have little sympathy here.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] omap: Fix broken pbias device creation
Hi Peter, On Friday 07 August 2015 05:18 PM, Peter Robinson wrote: On Wed, Aug 5, 2015 at 11:00 AM, Tony Lindgren t...@atomide.com wrote: * Grygorii Strashko grygorii.stras...@ti.com [150729 02:01]: On 07/27/2015 03:16 PM, Kishon Vijay Abraham I wrote: pbias device creation got broken once SCM cleanup got merged. This patch series re-enables device creation by adding simple-bus in the compatible property of the syscon dt node. validated this series in DRA72, OMAP4 PANDA and OMAP5 UEVM boards. Kishon Vijay Abraham I (4): ARM: dts: omap24xx: Fix broken pbias device creation ARM: dts: OMAP4: Fix broken pbias device creation ARM: dts: OMAP5: Fix broken pbias device creation ARM: dts: dra7: Fix broken pbias device creation arch/arm/boot/dts/dra7.dtsi |2 +- arch/arm/boot/dts/omap2430.dtsi |3 ++- arch/arm/boot/dts/omap4.dtsi|3 ++- arch/arm/boot/dts/omap5.dtsi|3 ++- 4 files changed, 7 insertions(+), 4 deletions(-) For dra7-evm: Tested-by: Grygorii Strashko grygorii.stras...@ti.com Applying all into omap-for-v4.2fixes-v2 thanks. Do we need the same for am33xx.dtsi too given commit e3bc5358e097 ? PBIAS is not present in am33xx. Thanks Kishon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On 08/07/2015 09:25 AM, Russell King - ARM Linux wrote: On Fri, Aug 07, 2015 at 03:22:56PM +0200, Sebastian Andrzej Siewior wrote: For TX-transfers, I would need to update the start-address so the transfers begins where it stopped. However based on your concern I can't really assume that the position reported by the HW is the correct one. Exactly - I don't believe that existing OMAP DMA hardware can ever support pausing a mem-to-device transfer without data loss as you have no idea how many bytes have been prefetched by the DMA, and therefore you have no idea how many bytes to unwind the hardware position. It gets worse than that when you have to cross into the previous descriptor. It's really not nice. So, disallowing pause for mem-to-device is entirely reasonable given the data loss implications. Thanks for adding your hard-won knowledge to this discussion, Russell. This saves us a bunch of wasted effort trying to fix x_char with DMA (and TCSANOW termios changes and throttling). Regards, Peter Hurley -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On 08/07/2015 06:07 PM, Peter Hurley wrote:
If we look at what 8250-dma.c is doing:
if (dma-rx_running) {
dmaengine_pause(dma-rxchan);
It's 8250-dma.c which is silently _ignoring_ the return code, failing
to check that the operation it requested worked. Maybe this should be
WARN_ON(dmaengine_pause(dma-rxchan)) or at least it should print a
message?
Thanks for the suggestion; I'll hold on to that and push it after we add
the 8250 omap dma pause in mainline.
I have a patch ready with WARN_ON_ONCE() for 8250-omap and 8250-dma.
This warning would trigger on am335x/edma until v4.2-rc1 and omap-dma
based version is open. I could post it if you want me to.
Besides that those two, there are four other drivers ignoring the
return code dmaengine_pause().
Sebastian
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On Fri, Aug 07, 2015 at 05:44:03PM +0200, Sebastian Andrzej Siewior wrote:
On 08/07/2015 05:29 PM, Russell King - ARM Linux wrote:
On Fri, Aug 07, 2015 at 11:08:48AM -0400, Peter Hurley wrote:
[ + Greg KH ]
On 08/07/2015 09:57 AM, Russell King - ARM Linux wrote:
As it is something that the driver has _not_ supported, you are clearly
adding a feature to an existing driver. It's not a bug fix.
If something else has been converted to pause channels and that is
causing
a problem, then _that_ conversion is where the bug lies, not the lack of
support in the omap-dma.
FWIW, the actual bug is the api that silently does nothing.
Incorrect.
static int omap_dma_pause(struct dma_chan *chan)
{
struct omap_chan *c = to_omap_dma_chan(chan);
/* Pause/Resume only allowed with cyclic mode */
if (!c-cyclic)
return -EINVAL;
Asking for the channel to be paused will return an error code indicating
that the request failed. That will be propagated back through to the
return code of dmaengine_pause().
If we look at what 8250-dma.c is doing:
if (dma-rx_running) {
dmaengine_pause(dma-rxchan);
It's 8250-dma.c which is silently _ignoring_ the return code, failing
to check that the operation it requested worked. Maybe this should be
WARN_ON(dmaengine_pause(dma-rxchan)) or at least it should print a
message?
I think this is what Peter meant by saying silently does nothing.
Maybe Peter should phrase his replies better. the actual bug is the
api that silently does nothing. to me means he is complaining that
dmaengine_pause() had no effect. _That_ is what I'm objecting to,
and claiming that Peter's comment is wrong.
He's now blaming me for snide remarks. I could call his one above an
incorrect snide remark against the quality of DMA engine code. He's
totally wrong, and been proven wrong by my analysis above, plain and
simple.
He should now accept that he's wrong and move along with sorting out
this mess _without_ requiring optional features to be implemented in
other subsystems to fix bugs in code he's supposed to be maintaining.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cmm files for omap4460
On 08/06/2015 11:38 PM, Ryan wrote: Hi, I am using a 4460 Based custom board i want to debug linux using Trace32. Could anyone share the cmm files required for debugging omap4460. I also request you to share any related documentation for debugging omap using trace32. You need to talk to lauterbach support. I have kind of always used the following and it seems to work.. ;t32id=C:\t32 t32id=/opt/t32 kdir=~/Src/opensource/linux print Connecting to OMAP 4460... SYStem.RESet system.mode down system.cpu omap4460 sys.jc RTCK system.mode attach core.select 1 system.MEMACCESS.DAP core.select 0 system.MEMACCESS.DAP TASK.RESet sYmbol.RESet sYmbol.AutoLoad.RESet ; reset autoloader list TRONCHIP.s undef ON TrOnchip.Set DABORT OFF TrOnchip.Set PABORT OFF TrOnchip.Set UNDEF OFF SYStem.Option DACR ON ; give Debugger global write permissions SETUP.IMASKASM OFF ; lock interrupts while single stepping SYSTEM.MEMACCESS DAP ; enable DAP access ;setup missing trace funnel and TPIU disable d.s eapb:0xd4164fb0 %l 0xc5acce55 d.s eapb:0xd4164000 %l 0x0301 ; ds.l 0x54160020 %1 1 print Connecting to OMAP 4460 Completed Successfully ;Resetting watchdog ;d.s e:0x4A314048 %l 0x ;wait 10.ms ;d.s e:0x4A314048 %l 0x system.option pwrdwn on setup.urate 3.s system.polling slow per.reprogram t32id\peromap4460app.per menu.reprogram t32id\menomap4460app.men data.load.elf kdir/vmlinux /nocode /gnu enddo -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] dma: omap-dma: add support for pause of non-cyclic transfers
This DMA driver is used by 8250-omap on DRA7-evm. There is one
requirement that is to pause a transfer. This is currently used on the RX
side. It is possible that the UART HW aborted the RX (UART's RX-timeout)
but the DMA controller starts the transfer shortly after.
Before we can manually purge the FIFO we need to pause the transfer,
check how many bytes it already received and terminate the transfer
without it making any progress.
From testing on the TX side it seems that it is possible that we invoke
pause once the transfer has completed which is indicated by the missing
CCR_ENABLE bit but before the interrupt has been noticed. In that case the
interrupt will come even after disabling it.
The AM572x manual says that we have to wait for the CCR_RD_ACTIVE
CCR_WR_ACTIVE bits to be gone before programming it again here is the
drain loop. Also it looks like without the drain the TX-transfer makes
sometimes progress.
One note: The pause + resume combo is broken because after resume the
the complete transfer will be programmed again. That means the already
transferred bytes (until the pause event) will be sent again. This is
currently not important for my UART user because it does only pause +
terminate.
v1…v2:
- move the drain loop into omap_dma_drain_chan() instead of having it
twice.
- allow pause only for DMA_DEV_TO_MEM transfers if non-cyclic. Add a
comment why DMA_MEM_TO_DEV not allowed.
- clear pause on terminate_all. Otherwise pause() + terminate_all()
will keep the pause bit set and we can't pause the following
transfer.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
drivers/dma/omap-dma.c | 107 +
1 file changed, 73 insertions(+), 34 deletions(-)
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 249445c8a4c6..6bbf089d2d7f 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -299,7 +299,30 @@ static void omap_dma_start(struct omap_chan *c, struct
omap_desc *d)
omap_dma_chan_write(c, CCR, d-ccr | CCR_ENABLE);
}
-static void omap_dma_stop(struct omap_chan *c)
+static void omap_dma_drain_chan(struct omap_chan *c)
+{
+ int i;
+ uint32_t val;
+
+ /* Wait for sDMA FIFO to drain */
+ for (i = 0; ; i++) {
+ val = omap_dma_chan_read(c, CCR);
+ if (!(val (CCR_RD_ACTIVE | CCR_WR_ACTIVE)))
+ break;
+
+ if (i 100)
+ break;
+
+ udelay(5);
+ }
+
+ if (val (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
+ dev_err(c-vc.chan.device-dev,
+ DMA drain did not complete on lch %d\n,
+ c-dma_ch);
+}
+
+static int omap_dma_stop(struct omap_chan *c)
{
struct omap_dmadev *od = to_omap_dma_dev(c-vc.chan.device);
uint32_t val;
@@ -312,7 +335,6 @@ static void omap_dma_stop(struct omap_chan *c)
val = omap_dma_chan_read(c, CCR);
if (od-plat-errata DMA_ERRATA_i541 val CCR_TRIGGER_SRC) {
uint32_t sysconfig;
- unsigned i;
sysconfig = omap_dma_glbl_read(od, OCP_SYSCONFIG);
val = sysconfig ~DMA_SYSCONFIG_MIDLEMODE_MASK;
@@ -323,27 +345,18 @@ static void omap_dma_stop(struct omap_chan *c)
val = ~CCR_ENABLE;
omap_dma_chan_write(c, CCR, val);
- /* Wait for sDMA FIFO to drain */
- for (i = 0; ; i++) {
- val = omap_dma_chan_read(c, CCR);
- if (!(val (CCR_RD_ACTIVE | CCR_WR_ACTIVE)))
- break;
-
- if (i 100)
- break;
-
- udelay(5);
- }
-
- if (val (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
- dev_err(c-vc.chan.device-dev,
- DMA drain did not complete on lch %d\n,
- c-dma_ch);
+ omap_dma_drain_chan(c);
omap_dma_glbl_write(od, OCP_SYSCONFIG, sysconfig);
} else {
+
+ if (!(val CCR_ENABLE))
+ return -EINVAL;
+
val = ~CCR_ENABLE;
omap_dma_chan_write(c, CCR, val);
+
+ omap_dma_drain_chan(c);
}
mb();
@@ -358,6 +371,7 @@ static void omap_dma_stop(struct omap_chan *c)
omap_dma_chan_write(c, CLNK_CTRL, val);
}
+ return 0;
}
static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d,
@@ -728,6 +742,8 @@ static enum dma_status omap_dma_tx_status(struct dma_chan
*chan,
} else {
txstate-residue = 0;
}
+ if (ret == DMA_IN_PROGRESS c-paused)
+ ret = DMA_PAUSED;
spin_unlock_irqrestore(c-vc.lock, flags);
return ret;
@@ -1038,10 +1054,8 @@ static int omap_dma_terminate_all(struct
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On 08/07/2015 11:29 AM, Russell King - ARM Linux wrote:
On Fri, Aug 07, 2015 at 11:08:48AM -0400, Peter Hurley wrote:
[ + Greg KH ]
On 08/07/2015 09:57 AM, Russell King - ARM Linux wrote:
As it is something that the driver has _not_ supported, you are clearly
adding a feature to an existing driver. It's not a bug fix.
If something else has been converted to pause channels and that is causing
a problem, then _that_ conversion is where the bug lies, not the lack of
support in the omap-dma.
FWIW, the actual bug is the api that silently does nothing.
Incorrect.
static int omap_dma_pause(struct dma_chan *chan)
{
struct omap_chan *c = to_omap_dma_chan(chan);
/* Pause/Resume only allowed with cyclic mode */
if (!c-cyclic)
return -EINVAL;
Asking for the channel to be paused will return an error code indicating
that the request failed. That will be propagated back through to the
return code of dmaengine_pause().
If we look at what 8250-dma.c is doing:
if (dma-rx_running) {
dmaengine_pause(dma-rxchan);
It's 8250-dma.c which is silently _ignoring_ the return code, failing
to check that the operation it requested worked. Maybe this should be
WARN_ON(dmaengine_pause(dma-rxchan)) or at least it should print a
message?
Thanks for the suggestion; I'll hold on to that and push it after we add
the 8250 omap dma pause in mainline.
Right, so the patch which caused the regression is the one which arranged
for the 8250-dma + omap-dma combination to work together, not the missing
pause support in omap-dma.
That would be the original submission patch set for an entire driver,
the 8250_omap driver.
Well, that's where the bug lies, and I don't agree with your assessment
that it would mean reverting the whole thing.
The binding between the two drivers is controlled via DT. DT tells it
which DMA controller and which DMA input to use. So, as DMA is currently
broken on this, the solution is to break that link so that 8250-omap goes
back to using PIO only.
As the binding of drivers is controlled by DT, you can disable the binding
of these two drivers
No. 8250 dma is not a stand-alone driver. Even if it were, how would you go
back and fix DTs in the wild?
Well, we have reached an impass then.
I'm not going to accept a feature addition to a driver as a stable patch
without it being adequately tested over _several_ kernel revisions to
ensure that we don't end up cocking up some driver which uses the DMA
interfaces correctly. It's too big a risk.
So, I guess that means that older kernels will just have to remain broken -
all because the basic testing of the original code was never undertaken
to ensure that basic stuff like reception of characters worked properly.
Well, instead of me saying something snide about the lack of upstream serial
driver unit tests, I'll say I've been working on cleaning up and organizing
my own tty/serial subsystem and driver units tests which I hope to upstream
in the fall.
Those include i/o validators that ran this driver for days at time without
error at max line rate. Unfortunately, that hardware does not exhibit the
same problem as the DRA7 noted in the changelog.
Sorry, I have little sympathy here.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] dma: omap-dma: add support for pause of non-cyclic transfers
* Russell King - ARM Linux | 2015-08-07 17:26:48 [+0100]: On Fri, Aug 07, 2015 at 05:36:05PM +0200, Sebastian Andrzej Siewior wrote: +/* + * We do not allow DMA_MEM_TO_DEV transfers to be paused. + * According to RMK the OMAP hardware might prefetch bytes from + * memory into its FIFO and not send it to the device due to the + * pause. The bytes in the FIFO are cleared on pause. It is + * unspecified by how many bytes the source address is updated + * if at all. Would you mind rewording the above. Please take the time to read the manuals for the device you are playing with. It's mostly documented in there. See the OMAP4430 ES2.x TRM, 16.4.18 and 16.4.19. I didn't connect the dots that well. Thank you. … I updated the comment to: /* * We do not allow DMA_MEM_TO_DEV transfers to be paused. * From the AM572x TRM, 16.1.4.18 Disabling a Channel During Transfer: * When a channel is disabled during a transfer, the channel undergoes * an abort, unless it is hardware-source-synchronized …. * A source-synchronised channel is one where the fetching of data is * under control of the device. In other words, a device-to-memory * transfer. So, a destination-synchronised channel (which would be a * memory-to-device transfer) undergoes an abort if the the CCR_ENABLE * bit is cleared. * From 16.1.4.20.4.6.2 Abort: If an abort trigger occurs, the channel * aborts immediately after completion of current read/write * transactions and then the FIFO is cleaned up. The term cleaned up * is not defined. TI recommends to check that RD_ACTIVE and WR_ACTIVE * are both clear _before_ disabling the channel, otherwise data loss * will occur. * The problem is that if the channel is active, then device activity * can result in DMA activity starting between reading those as both * clear and the write to DMA_CCR to clear the enable bit hitting the * hardware. If the DMA hardware can't drain the data in its FIFO to the * destination, then data loss might occur (say if we write to an UART * and the UART is not accepting any further data). */ would that be okay? Due to this, the OMAP DMA engine driver was submitted with this in the cover note: For the OMAP DMAengine driver, there's a few short-comings: 1. pause/resume support is not implemented; it's not clear whether the OMAP hardware is capable of supporting this sanely. If I google for it, I find it. pause/resume support for cyclic was added later without a note why it is only supported for cyclic. Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On Fri, Aug 07, 2015 at 02:21:59PM -0400, Peter Hurley wrote: [ + Heikki ] On 08/07/2015 12:33 PM, Russell King - ARM Linux wrote: What you have is a race condition in the code you a responsible for maintaining, caused by poorly implemented code. Fix it, rather than whinging about drivers outside of your subsystem having never implemented _optional_ things that you choose to merge broken code which relied upon it _without_ checking that the operation succeeded. It is _entirely_ your code which is wrong here. I will wait for that to be fixed before acking the omap-dma change since you obviously need something to test with. I'm not sure to what you're referring here. A WARNing fixes nothing. The warning can wait. If you mean some patch, as yet unwritten, that handles the dma cases when dmaengine_pause() is unimplemented without data loss, ok, but please confirm that's what you mean. But the regression needs fixing. However, at some point one must look at the api and wonder if the separation of concern has been drawn in the right place. It _is_ in the right place. dmaengine_pause() always has been permitted to fail. It's the responsibility of the user of this API to _check_ the return code to find out whether it had the desired effect. Not checking the return code is a bug in the caller's code. If that wasn't the case, dmaengine_pause() would have a void return type. It doesn't. It has an 'int' to allow failure, or to allow non- implementation for cases where the underlying hardware can't pause the channel without causing data loss. What would you think is better: an API which silently loses data, or one which refuses to stop the transfer and reports an error code back to the caller. You seem to be arguing for the former, and as such, there's no way I can take you seriously. In any case, Greg has now commented on the patch adding the feature, basically refusing it for stable tree inclusion. So the matter is settled: omap-dma isn't going to get the pause feature added in stable trees any time soon. So a different solution now needs to be found, which is what I've been saying all along... -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On Fri, Aug 07, 2015 at 01:23:20PM -0400, Peter Hurley wrote: The omap-serial driver which doesn't use dma is still the preferred stable driver for omap, for the moment. One of the main features of the 8250_omap integration was the addition of dma support. Without it, 8250_omap is ttyO in ttyS clothing. Correct. omap-serial used to have broken DMA support, and I had a series of patches fixing it. Unfortunately, despite TI asking me to work on this, TI went ahead and removed the DMA support behind my back, which totally screwed my patch series after I merged some of its pre-requists. It was then decided that DMA support was no longer an important feature, so I dropped the patch set. So, I'm well aware of the issues here, and the problems of using the OMAP sDMA with the UARTs. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On 08/07/2015 12:39 PM, Russell King - ARM Linux wrote:
On Fri, Aug 07, 2015 at 05:44:03PM +0200, Sebastian Andrzej Siewior wrote:
On 08/07/2015 05:29 PM, Russell King - ARM Linux wrote:
On Fri, Aug 07, 2015 at 11:08:48AM -0400, Peter Hurley wrote:
[ + Greg KH ]
On 08/07/2015 09:57 AM, Russell King - ARM Linux wrote:
As it is something that the driver has _not_ supported, you are clearly
adding a feature to an existing driver. It's not a bug fix.
If something else has been converted to pause channels and that is
causing
a problem, then _that_ conversion is where the bug lies, not the lack of
support in the omap-dma.
FWIW, the actual bug is the api that silently does nothing.
Incorrect.
static int omap_dma_pause(struct dma_chan *chan)
{
struct omap_chan *c = to_omap_dma_chan(chan);
/* Pause/Resume only allowed with cyclic mode */
if (!c-cyclic)
return -EINVAL;
Asking for the channel to be paused will return an error code indicating
that the request failed. That will be propagated back through to the
return code of dmaengine_pause().
If we look at what 8250-dma.c is doing:
if (dma-rx_running) {
dmaengine_pause(dma-rxchan);
It's 8250-dma.c which is silently _ignoring_ the return code, failing
to check that the operation it requested worked. Maybe this should be
WARN_ON(dmaengine_pause(dma-rxchan)) or at least it should print a
message?
I think this is what Peter meant by saying silently does nothing.
Maybe Peter should phrase his replies better. the actual bug is the
api that silently does nothing. to me means he is complaining that
dmaengine_pause() had no effect. _That_ is what I'm objecting to,
and claiming that Peter's comment is wrong.
Yes, I missed that the api included a return code which the 8250 dma
code should be checking.
He's now blaming me for snide remarks. I could call his one above an
incorrect snide remark against the quality of DMA engine code.
You'd be reading a lot into my statement.
He's
totally wrong, and been proven wrong by my analysis above, plain and
simple.
He should now accept that he's wrong
Done.
and move along with sorting out
this mess _without_ requiring optional features to be implemented in
other subsystems to fix bugs in code he's supposed to be maintaining.
This is simply a bug that flew under the radar, much like every other
bug that gets fixed daily in mainline.
The omap-serial driver which doesn't use dma is still the preferred
stable driver for omap, for the moment.
One of the main features of the 8250_omap integration was the addition
of dma support. Without it, 8250_omap is ttyO in ttyS clothing.
Regards,
Peter Hurley
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On Fri, Aug 07, 2015 at 10:41:57AM +0200, Sebastian Andrzej Siewior wrote: This DMA driver is used by 8250-omap on DRA7-evm. There is one requirement that is to pause a transfer. This is currently used on the RX side. It is possible that the UART HW aborted the RX (UART's RX-timeout) but the DMA controller starts the transfer shortly after. Before we can manually purge the FIFO we need to pause the transfer, check how many bytes it already received and terminate the transfer without it making any progress. From testing on the TX side it seems that it is possible that we invoke pause once the transfer has completed which is indicated by the missing CCR_ENABLE bit but before the interrupt has been noticed. In that case the interrupt will come even after disabling it. The AM572x manual says that we have to wait for the CCR_RD_ACTIVE CCR_WR_ACTIVE bits to be gone before programming it again here is the drain loop. Also it looks like without the drain the TX-transfer makes sometimes progress. One note: The pause + resume combo is broken because after resume the the complete transfer will be programmed again. That means the already transferred bytes (until the pause event) will be sent again. This is currently not important for my UART user because it does only pause + terminate. Cc: sta...@vger.kernel.org You don't get a add support patch into the stable tree unless it's a trivial device id or quirk table addition, please go re-read Documentation/stable_kernel_rules.txt thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
[ + Heikki ]
On 08/07/2015 12:33 PM, Russell King - ARM Linux wrote:
On Fri, Aug 07, 2015 at 12:07:11PM -0400, Peter Hurley wrote:
On 08/07/2015 11:29 AM, Russell King - ARM Linux wrote:
On Fri, Aug 07, 2015 at 11:08:48AM -0400, Peter Hurley wrote:
[ + Greg KH ]
On 08/07/2015 09:57 AM, Russell King - ARM Linux wrote:
As it is something that the driver has _not_ supported, you are clearly
adding a feature to an existing driver. It's not a bug fix.
If something else has been converted to pause channels and that is
causing
a problem, then _that_ conversion is where the bug lies, not the lack of
support in the omap-dma.
FWIW, the actual bug is the api that silently does nothing.
Incorrect.
static int omap_dma_pause(struct dma_chan *chan)
{
struct omap_chan *c = to_omap_dma_chan(chan);
/* Pause/Resume only allowed with cyclic mode */
if (!c-cyclic)
return -EINVAL;
Asking for the channel to be paused will return an error code indicating
that the request failed. That will be propagated back through to the
return code of dmaengine_pause().
If we look at what 8250-dma.c is doing:
if (dma-rx_running) {
dmaengine_pause(dma-rxchan);
It's 8250-dma.c which is silently _ignoring_ the return code, failing
to check that the operation it requested worked. Maybe this should be
WARN_ON(dmaengine_pause(dma-rxchan)) or at least it should print a
message?
Thanks for the suggestion; I'll hold on to that and push it after we add
the 8250 omap dma pause in mainline.
Why wait? You're hiding a data loss bug which is clearly the result of
code you allegedly maintain.
Because it will generate tons of unnecessary reports when the patch that
WARNs inevitably ends up in mainline a version earlier than the patch that
fixes it.
Well, instead of me saying something snide about the lack of upstream serial
driver unit tests, I'll say I've been working on cleaning up and organizing
my own tty/serial subsystem and driver units tests which I hope to upstream
in the fall.
Those include i/o validators that ran this driver for days at time without
error at max line rate. Unfortunately, that hardware does not exhibit the
same problem as the DRA7 noted in the changelog.
What you have is a race condition in the code you a responsible for
maintaining, caused by poorly implemented code. Fix it, rather than
whinging about drivers outside of your subsystem having never implemented
_optional_ things that you choose to merge broken code which relied upon
it _without_ checking that the operation succeeded.
It is _entirely_ your code which is wrong here.
I will wait for that to be fixed before acking the omap-dma change since
you obviously need something to test with.
I'm not sure to what you're referring here.
A WARNing fixes nothing.
If you mean some patch, as yet unwritten, that handles the dma cases when
dmaengine_pause() is unimplemented without data loss, ok, but please confirm
that's what you mean.
However, at some point one must look at the api and wonder if the separation
of concern has been drawn in the right place.
Regards,
Peter Hurley
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] dma: omap-dma: add support for pause of non-cyclic transfers
On Fri, Aug 07, 2015 at 07:55:48PM +0200, Sebastian Andrzej Siewior wrote: /* * We do not allow DMA_MEM_TO_DEV transfers to be paused. * From the AM572x TRM, 16.1.4.18 Disabling a Channel During Transfer: * When a channel is disabled during a transfer, the channel undergoes * an abort, unless it is hardware-source-synchronized …. * A source-synchronised channel is one where the fetching of data is * under control of the device. In other words, a device-to-memory * transfer. So, a destination-synchronised channel (which would be a * memory-to-device transfer) undergoes an abort if the the CCR_ENABLE * bit is cleared. * From 16.1.4.20.4.6.2 Abort: If an abort trigger occurs, the channel * aborts immediately after completion of current read/write * transactions and then the FIFO is cleaned up. The term cleaned up * is not defined. TI recommends to check that RD_ACTIVE and WR_ACTIVE * are both clear _before_ disabling the channel, otherwise data loss * will occur. * The problem is that if the channel is active, then device activity * can result in DMA activity starting between reading those as both * clear and the write to DMA_CCR to clear the enable bit hitting the * hardware. If the DMA hardware can't drain the data in its FIFO to the * destination, then data loss might occur (say if we write to an UART * and the UART is not accepting any further data). */ would that be okay? Better, if a tad verbose. I guess no one will miss that. :) If I google for it, I find it. pause/resume support for cyclic was added later without a note why it is only supported for cyclic. Try searching for the [PATCH 11/11] ASoC: omap-pcm: Convert to use dmaengine thread, which was the initial round of patches converting omap-pcm to DMA engine, and where there was some discussion of how to handle it at that time. The result of that was it was felt that the safest approach was to limit it to cyclic transfers for ASoC, and to use the method which had been well proven over previous years/decade on numerous different OMAP hardware. It's all entirely sensible given that omap-dma has to cope with many different hardware revisions with two major hardware versions and people having limited testing resources for validation. So you will understand, given the data loss issues here, why it was decided that omap-dma will only provide pause/resume support for cases where it has been proven to work sufficiently well and where data loss is not that a major issue (if a few samples of audio get lost over a suspend/resume, it's not going to corrupt your data.) -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/11] omap_hsmmc: voltage switching and tuning
Hi, On Thursday 06 August 2015 12:18 PM, Tony Lindgren wrote: * Kishon Vijay Abraham I kis...@ti.com [150805 08:03]: Hi, On Wednesday 05 August 2015 04:13 PM, Tony Lindgren wrote: * Kishon Vijay Abraham I kis...@ti.com [150730 00:49]: Patch series implements voltage switching and tuning for omap_hsmmc driver. Did basic read/write test in J6, J6 Eco, Beagle-x15, AM437x EVM, Beaglebone black, OMAP5 uEVM and OMAP4 PANDA. Your tests are missing omap3? I don't have one at my disposal :-( I'll try to find one and add omap3 tests. Great :) Beagle xm is a good one to test the USB PHY stuff on and also MMC. Having USB cable connected or EHCI loaded blocks the PM though, so NFSroot is not very usable for testing on it. I found a beagle xm. So I should be able to get back to this early next week. Thanks Kishon If you want to test PM over NFSroot, boards with the smsc911x known to work with PM are at least omap3-evm-37xx.dts and logicpd-torpedo-37xx-devkit.dts. Since you tinker with the USB PHYs, torpedo has MUSB working the same way as the beagle boards, EVM I think can be modified that way but by default has diffent PHY. Also overo tobi boards work with PM but only for retntion idle and not off idle, and map3-sbc-t3517.dts probably can be made to work with PM but I have mine as a gateway and have not been able to test with it. Probably also zoom3 boards with later processor boards can be made to work, the LDP has early omap34xx variants and can't be made to work reliably. I don't have omap3-lilly, but I'd assume that can also be made to work with PM if not already working. I suggest you add some omap3 tests in general as otherwise you are only testing a subset of the driver features and completely missing things like rutnime PM and save and restore for the deeper idle states. yeah, I'll do those tests and re-post the series. Thanks! Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
[ + Greg KH ] On 08/07/2015 09:57 AM, Russell King - ARM Linux wrote: On Fri, Aug 07, 2015 at 03:42:06PM +0200, Sebastian Andrzej Siewior wrote: On 08/07/2015 03:22 PM, Russell King - ARM Linux wrote: On Fri, Aug 07, 2015 at 12:36:14PM +0200, Sebastian Andrzej Siewior wrote: On 08/07/2015 11:44 AM, Peter Ujfalusi wrote: with a short testing audio did not broke (the only user of pause/resume) Some comments embedded. Cc: sta...@vger.kernel.org Why stable? This is not fixing any bugs since the PAUSE was not allowed for non cyclic transfers. Hmmm. The DRA7x was using pause before for UART. I just did not see it coming that it was not allowed here. John made a similar change to the edma driver and I assumed it went stable but now I see that it was just cherry-picked into the ti tree. This is *NOT* stable material. Pause of these channels is something that omap-dma has *never* supported. Therefore, it is *not* a regression. What you are doing is *adding* a feature to the omap-dma driver. That is not stable material in any sense. Stable is for bug fixes to existing code, not feature enhancements. I didn't consider this as a feature. As it is something that the driver has _not_ supported, you are clearly adding a feature to an existing driver. It's not a bug fix. If something else has been converted to pause channels and that is causing a problem, then _that_ conversion is where the bug lies, not the lack of support in the omap-dma. FWIW, the actual bug is the api that silently does nothing. So we had the 8250-DMA doing pause and all its current users implement it. We have a DMA driver tree which is not used and it not implementing pause (not implementing pause at all). Later we get a combo of 8250-DMA + DMA driver that is broken because the lack of pause and this is noticed a few kernel releases later. Right, so the patch which caused the regression is the one which arranged for the 8250-dma + omap-dma combination to work together, not the missing pause support in omap-dma. That would be the original submission patch set for an entire driver, the 8250_omap driver. It doesn't matter that it's several releases old, it's that change caused the regression you have today. That change is incorrect today, and it was just as incorrect at the time that it was merged. The only way of fixing the bug is by implementing the pause feature. That's not the only way of fixing the bug. As the binding of drivers is controlled by DT, you can disable the binding of these two drivers No. 8250 dma is not a stand-alone driver. Even if it were, how would you go back and fix DTs in the wild? The binding is built-in with a CONFIG_ switch. there and 8250 will go back to using non-DMA mode - which is the situation which existed prior to the change which coupled the two drivers together. That's an acceptable change for -stable trees, because it's reverting the change which caused the regression, taking us back to a situation we _know_ worked previously. What you're suggesting here is only possible if the entire 8250_omap driver is removed, so that's a non-starter. I suggest to wait on any solution until the correct fix is mainlined and backported, as you note below. Regards, Peter Hurley Then, in mainline during the next merge window, we can introduce the pause feature to omap-dma, and re-enable the 8250 driver to use it. _Once_ that's proven stable, we can then take a view whether those changes should _then_ be backported to stable kernels with greater confidence that backporting the feature addition won't itself cause a new regression. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On 08/07/2015 05:29 PM, Russell King - ARM Linux wrote:
On Fri, Aug 07, 2015 at 11:08:48AM -0400, Peter Hurley wrote:
[ + Greg KH ]
On 08/07/2015 09:57 AM, Russell King - ARM Linux wrote:
As it is something that the driver has _not_ supported, you are clearly
adding a feature to an existing driver. It's not a bug fix.
If something else has been converted to pause channels and that is causing
a problem, then _that_ conversion is where the bug lies, not the lack of
support in the omap-dma.
FWIW, the actual bug is the api that silently does nothing.
Incorrect.
static int omap_dma_pause(struct dma_chan *chan)
{
struct omap_chan *c = to_omap_dma_chan(chan);
/* Pause/Resume only allowed with cyclic mode */
if (!c-cyclic)
return -EINVAL;
Asking for the channel to be paused will return an error code indicating
that the request failed. That will be propagated back through to the
return code of dmaengine_pause().
If we look at what 8250-dma.c is doing:
if (dma-rx_running) {
dmaengine_pause(dma-rxchan);
It's 8250-dma.c which is silently _ignoring_ the return code, failing
to check that the operation it requested worked. Maybe this should be
WARN_ON(dmaengine_pause(dma-rxchan)) or at least it should print a
message?
I think this is what Peter meant by saying silently does nothing.
So, I guess that means that older kernels will just have to remain broken -
all because the basic testing of the original code was never undertaken
to ensure that basic stuff like reception of characters worked properly.
Hehe. I wouldn't describe testing at 3mbaud as basic. This reads as I
didn't do any kind of testing at all prior submitting the driver. This
was not the case.
Sebastian
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On Fri, Aug 07, 2015 at 12:07:11PM -0400, Peter Hurley wrote:
On 08/07/2015 11:29 AM, Russell King - ARM Linux wrote:
On Fri, Aug 07, 2015 at 11:08:48AM -0400, Peter Hurley wrote:
[ + Greg KH ]
On 08/07/2015 09:57 AM, Russell King - ARM Linux wrote:
As it is something that the driver has _not_ supported, you are clearly
adding a feature to an existing driver. It's not a bug fix.
If something else has been converted to pause channels and that is
causing
a problem, then _that_ conversion is where the bug lies, not the lack of
support in the omap-dma.
FWIW, the actual bug is the api that silently does nothing.
Incorrect.
static int omap_dma_pause(struct dma_chan *chan)
{
struct omap_chan *c = to_omap_dma_chan(chan);
/* Pause/Resume only allowed with cyclic mode */
if (!c-cyclic)
return -EINVAL;
Asking for the channel to be paused will return an error code indicating
that the request failed. That will be propagated back through to the
return code of dmaengine_pause().
If we look at what 8250-dma.c is doing:
if (dma-rx_running) {
dmaengine_pause(dma-rxchan);
It's 8250-dma.c which is silently _ignoring_ the return code, failing
to check that the operation it requested worked. Maybe this should be
WARN_ON(dmaengine_pause(dma-rxchan)) or at least it should print a
message?
Thanks for the suggestion; I'll hold on to that and push it after we add
the 8250 omap dma pause in mainline.
Why wait? You're hiding a data loss bug which is clearly the result of
code you allegedly maintain.
Well, instead of me saying something snide about the lack of upstream serial
driver unit tests, I'll say I've been working on cleaning up and organizing
my own tty/serial subsystem and driver units tests which I hope to upstream
in the fall.
Those include i/o validators that ran this driver for days at time without
error at max line rate. Unfortunately, that hardware does not exhibit the
same problem as the DRA7 noted in the changelog.
What you have is a race condition in the code you a responsible for
maintaining, caused by poorly implemented code. Fix it, rather than
whinging about drivers outside of your subsystem having never implemented
_optional_ things that you choose to merge broken code which relied upon
it _without_ checking that the operation succeeded.
It is _entirely_ your code which is wrong here.
I will wait for that to be fixed before acking the omap-dma change since
you obviously need something to test with.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] dma: add __must_check annotation for dmaengine_pause()
In 8250-omap I learned it the hard way that ignoring the return code
of dmaengine_pause() might be bad because the underlying DMA driver
might not support the function at all and so not doing what one is
expecting.
This patch adds the __must_check annotation as suggested by Russell King.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
include/linux/dmaengine.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 8ad9a4e839f6..4eac4716bded 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -825,7 +825,7 @@ static inline int dmaengine_terminate_all(struct dma_chan
*chan)
return -ENOSYS;
}
-static inline int dmaengine_pause(struct dma_chan *chan)
+static inline int __must_check dmaengine_pause(struct dma_chan *chan)
{
if (chan-device-device_pause)
return chan-device-device_pause(chan);
--
2.5.0
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
omap-dma + 8250_omap: fix the dmaengine_pause()
#1 is something that can go stable and disables RX-DMA should it notice that it does not work. #2 adds the anotation as suggest by Russell. #3 adds the missing feature to omap-dma so dmaengine_pause() works. Once this is merged, the warning from #1 disappears. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/3] dma: omap-dma: add support for pause of non-cyclic transfers
This DMA driver is used by 8250-omap on DRA7-evm. There is one
requirement that is to pause a transfer. This is currently used on the RX
side. It is possible that the UART HW aborted the RX (UART's RX-timeout)
but the DMA controller starts the transfer shortly after.
Before we can manually purge the FIFO we need to pause the transfer,
check how many bytes it already received and terminate the transfer
without it making any progress.
From testing on the TX side it seems that it is possible that we invoke
pause once the transfer has completed which is indicated by the missing
CCR_ENABLE bit but before the interrupt has been noticed. In that case the
interrupt will come even after disabling it.
The AM572x manual says that we have to wait for the CCR_RD_ACTIVE
CCR_WR_ACTIVE bits to be gone before programming it again here is the
drain loop. Also it looks like without the drain the TX-transfer makes
sometimes progress.
One note: The pause + resume combo is broken because after resume the
the complete transfer will be programmed again. That means the already
transferred bytes (until the pause event) will be sent again. This is
currently not important for my UART user because it does only pause +
terminate.
v2…v3:
- rephrase the comment based on Russell's information / feedback.
v1…v2:
- move the drain loop into omap_dma_drain_chan() instead of having it
twice.
- allow pause only for DMA_DEV_TO_MEM transfers if non-cyclic. Add a
comment why DMA_MEM_TO_DEV not allowed.
- clear pause on terminate_all. Otherwise pause() + terminate_all()
will keep the pause bit set and we can't pause the following
transfer.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
drivers/dma/omap-dma.c | 122 +++--
1 file changed, 88 insertions(+), 34 deletions(-)
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 249445c8a4c6..cb217fab472e 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -299,7 +299,30 @@ static void omap_dma_start(struct omap_chan *c, struct
omap_desc *d)
omap_dma_chan_write(c, CCR, d-ccr | CCR_ENABLE);
}
-static void omap_dma_stop(struct omap_chan *c)
+static void omap_dma_drain_chan(struct omap_chan *c)
+{
+ int i;
+ uint32_t val;
+
+ /* Wait for sDMA FIFO to drain */
+ for (i = 0; ; i++) {
+ val = omap_dma_chan_read(c, CCR);
+ if (!(val (CCR_RD_ACTIVE | CCR_WR_ACTIVE)))
+ break;
+
+ if (i 100)
+ break;
+
+ udelay(5);
+ }
+
+ if (val (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
+ dev_err(c-vc.chan.device-dev,
+ DMA drain did not complete on lch %d\n,
+ c-dma_ch);
+}
+
+static int omap_dma_stop(struct omap_chan *c)
{
struct omap_dmadev *od = to_omap_dma_dev(c-vc.chan.device);
uint32_t val;
@@ -312,7 +335,6 @@ static void omap_dma_stop(struct omap_chan *c)
val = omap_dma_chan_read(c, CCR);
if (od-plat-errata DMA_ERRATA_i541 val CCR_TRIGGER_SRC) {
uint32_t sysconfig;
- unsigned i;
sysconfig = omap_dma_glbl_read(od, OCP_SYSCONFIG);
val = sysconfig ~DMA_SYSCONFIG_MIDLEMODE_MASK;
@@ -323,27 +345,18 @@ static void omap_dma_stop(struct omap_chan *c)
val = ~CCR_ENABLE;
omap_dma_chan_write(c, CCR, val);
- /* Wait for sDMA FIFO to drain */
- for (i = 0; ; i++) {
- val = omap_dma_chan_read(c, CCR);
- if (!(val (CCR_RD_ACTIVE | CCR_WR_ACTIVE)))
- break;
-
- if (i 100)
- break;
-
- udelay(5);
- }
-
- if (val (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
- dev_err(c-vc.chan.device-dev,
- DMA drain did not complete on lch %d\n,
- c-dma_ch);
+ omap_dma_drain_chan(c);
omap_dma_glbl_write(od, OCP_SYSCONFIG, sysconfig);
} else {
+
+ if (!(val CCR_ENABLE))
+ return -EINVAL;
+
val = ~CCR_ENABLE;
omap_dma_chan_write(c, CCR, val);
+
+ omap_dma_drain_chan(c);
}
mb();
@@ -358,6 +371,7 @@ static void omap_dma_stop(struct omap_chan *c)
omap_dma_chan_write(c, CLNK_CTRL, val);
}
+ return 0;
}
static void omap_dma_start_sg(struct omap_chan *c, struct omap_desc *d,
@@ -728,6 +742,8 @@ static enum dma_status omap_dma_tx_status(struct dma_chan
*chan,
} else {
txstate-residue = 0;
}
+ if (ret == DMA_IN_PROGRESS c-paused)
+ ret = DMA_PAUSED;
spin_unlock_irqrestore(c-vc.lock, flags);
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On 08/07/2015 01:44 PM, Peter Ujfalusi wrote:
Cc: sta...@vger.kernel.org
Why stable? This is not fixing any bugs since the PAUSE was not allowed for
non cyclic transfers.
Hmmm. The DRA7x was using pause before for UART. I just did not see it
coming that it was not allowed here. John made a similar change to the
edma driver and I assumed it went stable but now I see that it was just
cherry-picked into the ti tree.
If you are not comfortable it being stable material I can drop it.
This change is needed for the UART DMA support if I'm not mistaken and this
mode is not really supported by older kernels, so having this to implement
something which is not going to be used in the stable kernels feels somehow
wrong.
We have the DT pieces since v3.19-rc1. And if I remember correctly I
tested this on am335x-evm and dra7-evm by I the time I posted the
patches. I agree that dra7 support was not the best back then but I am
almost sure that I had vanilla running for testing.
But I don't insist on the stable tag. Consider it dropped.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
drivers/dma/omap-dma.c | 54
++
1 file changed, 41 insertions(+), 13 deletions(-)
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 249445c8a4c6..6b8497203caf 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -299,7 +299,7 @@ static void omap_dma_start(struct omap_chan *c, struct
omap_desc *d)
omap_dma_chan_write(c, CCR, d-ccr | CCR_ENABLE);
}
-static void omap_dma_stop(struct omap_chan *c)
+static int omap_dma_stop(struct omap_chan *c)
{
struct omap_dmadev *od = to_omap_dma_dev(c-vc.chan.device);
uint32_t val;
@@ -342,8 +342,26 @@ static void omap_dma_stop(struct omap_chan *c)
omap_dma_glbl_write(od, OCP_SYSCONFIG, sysconfig);
} else {
+ int i = 0;
+
+ if (!(val CCR_ENABLE))
+ return -EINVAL;
+
val = ~CCR_ENABLE;
omap_dma_chan_write(c, CCR, val);
+ do {
+ val = omap_dma_chan_read(c, CCR);
+ if (!(val (CCR_RD_ACTIVE | CCR_WR_ACTIVE)))
+ break;
+ if (i 100)
if (++i 100)
break;
to avoid infinite loop?
Ah. So I forgot to increment the counter. A few lines above there is
the same loop as a workaround for something. This is the same loop. I
could merge the loop + warning if you prefer. to have those things in
one place. I could also just increment i. Merging the two loops might
be better.
The other loop is for handling the ERRATA i541 and the two loops can not be
merged since the errata handling also require to change in SYSCONFIG register.
yes, but I had in mind is to put the loop into one function so we gain:
+static void omap_dma_drain_chan(struct omap_chan *c)
+{
+ int i;
+ uint32_t val;
+
+ /* Wait for sDMA FIFO to drain */
+ for (i = 0; ; i++) {
+ val = omap_dma_chan_read(c, CCR);
+ if (!(val (CCR_RD_ACTIVE | CCR_WR_ACTIVE)))
+ break;
+
+ if (i 100)
+ break;
+
+ udelay(5);
+ }
+
+ if (val (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
+ dev_err(c-vc.chan.device-dev,
+ DMA drain did not complete on lch %d\n,
+ c-dma_ch);
+}
which is invoked by both parts of the if case (handling the errata not
not) instead of having the same loop twice.
+ break;
+ udelay(5);
+ } while (1);
+
+ if (val (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
if (i 100) ?
While that would work, too I think it is more explicit to the reader if
you check for the condition that is important to you.
Yeah, I see that the errata handling is doing the same, fine by me.
good.
Sebastian
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On 08/07/2015 01:36 PM, Sebastian Andrzej Siewior wrote:
On 08/07/2015 11:44 AM, Peter Ujfalusi wrote:
On 08/07/2015 11:41 AM, Sebastian Andrzej Siewior wrote:
This DMA driver is used by 8250-omap on DRA7-evm. There is one
requirement that is to pause a transfer. This is currently used on the RX
side. It is possible that the UART HW aborted the RX (UART's RX-timeout)
but the DMA controller starts the transfer shortly after.
Before we can manually purge the FIFO we need to pause the transfer,
check how many bytes it already received and terminate the transfer
without it making any progress.
From testing on the TX side it seems that it is possible that we invoke
pause once the transfer has completed which is indicated by the missing
CCR_ENABLE bit but before the interrupt has been noticed. In that case the
interrupt will come even after disabling it.
The AM572x manual says that we have to wait for the CCR_RD_ACTIVE
CCR_WR_ACTIVE bits to be gone before programming it again here is the
drain loop. Also it looks like without the drain the TX-transfer makes
sometimes progress.
One note: The pause + resume combo is broken because after resume the
the complete transfer will be programmed again. That means the already
transferred bytes (until the pause event) will be sent again. This is
currently not important for my UART user because it does only pause +
terminate.
with a short testing audio did not broke (the only user of pause/resume)
Some comments embedded.
Cc: sta...@vger.kernel.org
Why stable? This is not fixing any bugs since the PAUSE was not allowed for
non cyclic transfers.
Hmmm. The DRA7x was using pause before for UART. I just did not see it
coming that it was not allowed here. John made a similar change to the
edma driver and I assumed it went stable but now I see that it was just
cherry-picked into the ti tree.
If you are not comfortable it being stable material I can drop it.
This change is needed for the UART DMA support if I'm not mistaken and this
mode is not really supported by older kernels, so having this to implement
something which is not going to be used in the stable kernels feels somehow
wrong.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
drivers/dma/omap-dma.c | 54
++
1 file changed, 41 insertions(+), 13 deletions(-)
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 249445c8a4c6..6b8497203caf 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -299,7 +299,7 @@ static void omap_dma_start(struct omap_chan *c, struct
omap_desc *d)
omap_dma_chan_write(c, CCR, d-ccr | CCR_ENABLE);
}
-static void omap_dma_stop(struct omap_chan *c)
+static int omap_dma_stop(struct omap_chan *c)
{
struct omap_dmadev *od = to_omap_dma_dev(c-vc.chan.device);
uint32_t val;
@@ -342,8 +342,26 @@ static void omap_dma_stop(struct omap_chan *c)
omap_dma_glbl_write(od, OCP_SYSCONFIG, sysconfig);
} else {
+ int i = 0;
+
+ if (!(val CCR_ENABLE))
+ return -EINVAL;
+
val = ~CCR_ENABLE;
omap_dma_chan_write(c, CCR, val);
+ do {
+ val = omap_dma_chan_read(c, CCR);
+ if (!(val (CCR_RD_ACTIVE | CCR_WR_ACTIVE)))
+ break;
+ if (i 100)
if (++i 100)
break;
to avoid infinite loop?
Ah. So I forgot to increment the counter. A few lines above there is
the same loop as a workaround for something. This is the same loop. I
could merge the loop + warning if you prefer. to have those things in
one place. I could also just increment i. Merging the two loops might
be better.
The other loop is for handling the ERRATA i541 and the two loops can not be
merged since the errata handling also require to change in SYSCONFIG register.
+ break;
+ udelay(5);
+ } while (1);
+
+ if (val (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
if (i 100) ?
While that would work, too I think it is more explicit to the reader if
you check for the condition that is important to you.
Yeah, I see that the errata handling is doing the same, fine by me.
+ dev_err(c-vc.chan.device-dev,
+ DMA drain did not complete on lch %d\n,
+ c-dma_ch);
}
mb();
Sebastian
--
Péter
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] omap: Fix broken pbias device creation
On Wed, Aug 5, 2015 at 11:00 AM, Tony Lindgren t...@atomide.com wrote: * Grygorii Strashko grygorii.stras...@ti.com [150729 02:01]: On 07/27/2015 03:16 PM, Kishon Vijay Abraham I wrote: pbias device creation got broken once SCM cleanup got merged. This patch series re-enables device creation by adding simple-bus in the compatible property of the syscon dt node. validated this series in DRA72, OMAP4 PANDA and OMAP5 UEVM boards. Kishon Vijay Abraham I (4): ARM: dts: omap24xx: Fix broken pbias device creation ARM: dts: OMAP4: Fix broken pbias device creation ARM: dts: OMAP5: Fix broken pbias device creation ARM: dts: dra7: Fix broken pbias device creation arch/arm/boot/dts/dra7.dtsi |2 +- arch/arm/boot/dts/omap2430.dtsi |3 ++- arch/arm/boot/dts/omap4.dtsi|3 ++- arch/arm/boot/dts/omap5.dtsi|3 ++- 4 files changed, 7 insertions(+), 4 deletions(-) For dra7-evm: Tested-by: Grygorii Strashko grygorii.stras...@ti.com Applying all into omap-for-v4.2fixes-v2 thanks. Do we need the same for am33xx.dtsi too given commit e3bc5358e097 ? Peter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On 08/07/2015 12:55 PM, Russell King - ARM Linux wrote: On Fri, Aug 07, 2015 at 10:41:57AM +0200, Sebastian Andrzej Siewior wrote: This DMA driver is used by 8250-omap on DRA7-evm. There is one requirement that is to pause a transfer. This is currently used on the RX side. It is possible that the UART HW aborted the RX (UART's RX-timeout) but the DMA controller starts the transfer shortly after. Before we can manually purge the FIFO we need to pause the transfer, check how many bytes it already received and terminate the transfer without it making any progress. From testing on the TX side it seems that it is possible that we invoke pause once the transfer has completed which is indicated by the missing CCR_ENABLE bit but before the interrupt has been noticed. In that case the interrupt will come even after disabling it. How do you cope with the OMAP DMA hardware clearing its FIFO when you pause it? I don't The problem is that on mem-to-device transfers, the DMA hardware can prefetch some data from memory into its FIFO before the device wants the data. If you then disable the channel, the hardware clears the FIFO. It is unspecified whether the hardware updates the source address in this case, or by how much. So it's pretty hard to undo the prefetching in software. The net result is: data loss. This is why I explicitly did not implement pause/resume for non-cyclic channels. Right now the 820-omap (8250-dma in general, too but they don't use this driver) pause only the RX transfer in an error condition. This means it is only device-to-mem transfer. I only mentioned the TX transfer here since this was easier to test. When I first tested the 8250_omap + DMA on EDMA it seemed that once the UART hardware triggered an time-out interrupt the DMA transfer _never_ started. Based on this I could just cancel the transfer since the RX-DMA and assume zero bytes were transfer. Therefore I ignored the lack of pause support in EDMA. Things were fine until someone used 3mbaud instead 115200. Its been observed that at this speed the UART triggers the time-out interrupt and the DMA transfer just started. Without the support for pause we lost some bytes here and once pause support was added the problem was gone. Now I've been chasing another bug on Dra7 which uses this driver and the lack of pause support is a candidate for my current problem. So at this point, things can't get worse if we pause a transfer and the hardware reported the wrong number of received bytes. The situation can improve if we get the correct number :) The 8250_omap does not pause the TX/RX transfer for the fun of it. As I said, on the TX side we avoid it and on the RX side the transfer is only paused if we receive an error interrupt (= no way out). Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] tty: serial: 8250_omap: do not use RX DMA if pause is not supported
On 08/07/2015 04:00 PM, Sebastian Andrzej Siewior wrote:
The 8250-omap driver requires the DMA-engine driver to support the pause
command in order to properly turn off programmed RX transfer before the
driver stars manually reading from the FIFO.
The lacking support of the requirement has been discovered recently. In
order to stay safe here we disable support for RX-DMA as soon as we
notice that it does not work. This should happen very early.
If the user does not want to see this backtrace he can either disable
DMA support (completely or RX-only) or backport the required patches for
edma / omap-dma once they hit mainline.
Cc: sta...@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
drivers/tty/serial/8250/8250_omap.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_omap.c
b/drivers/tty/serial/8250/8250_omap.c
index 0340ee6ba970..07a11e0935e4 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -112,6 +112,7 @@ struct omap8250_priv {
struct work_struct qos_work;
struct uart_8250_dma omap8250_dma;
spinlock_t rx_dma_lock;
+ bool rx_dma_broken;
};
static u32 uart_read(struct uart_8250_port *up, u32 reg)
@@ -761,6 +762,7 @@ static void omap_8250_rx_dma_flush(struct uart_8250_port
*p)
struct omap8250_priv*priv = p-port.private_data;
struct uart_8250_dma*dma = p-dma;
unsigned long flags;
+ int ret;
spin_lock_irqsave(priv-rx_dma_lock, flags);
@@ -769,7 +771,9 @@ static void omap_8250_rx_dma_flush(struct uart_8250_port
*p)
return;
}
- dmaengine_pause(dma-rxchan);
+ ret = dmaengine_pause(dma-rxchan);
+ if (WARN_ON_ONCE(ret))
+ priv-rx_dma_broken = true;
No offense, Sebastian, but it boggles my mind that anyone could defend this
as solid api design. We're in the middle of an interrupt handler and the
slave dma driver is /just/ telling us now that it doesn't implement this
functionality?!!?
The dmaengine api has _so much_ setup and none of it contemplates telling the
consumer that critical functionality is missing?
Even dma_get_slave_caps() returns _true_ for cmd_pause support; ok, that
interface is pointless.
Rather than losing /critical data/ here, the interrupt handler should just
busy-wait until dmaengine_tx_status() returns DMA_COMPLETE for the rx_cookie.
Regards,
Peter Hurley
spin_unlock_irqrestore(priv-rx_dma_lock, flags);
@@ -813,6 +817,9 @@ static int omap_8250_rx_dma(struct uart_8250_port *p,
unsigned int iir)
break;
}
+ if (priv-rx_dma_broken)
+ return -EINVAL;
+
spin_lock_irqsave(priv-rx_dma_lock, flags);
if (dma-rx_running)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] dma: add __must_check annotation for dmaengine_pause()
On 08/07/2015 04:00 PM, Sebastian Andrzej Siewior wrote:
In 8250-omap I learned it the hard way that ignoring the return code
of dmaengine_pause() might be bad because the underlying DMA driver
might not support the function at all and so not doing what one is
expecting.
This patch adds the __must_check annotation as suggested by Russell King.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
include/linux/dmaengine.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 8ad9a4e839f6..4eac4716bded 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -825,7 +825,7 @@ static inline int dmaengine_terminate_all(struct dma_chan
*chan)
return -ENOSYS;
}
-static inline int dmaengine_pause(struct dma_chan *chan)
+static inline int __must_check dmaengine_pause(struct dma_chan *chan)
{
if (chan-device-device_pause)
return chan-device-device_pause(chan);
Not that this is your responsibility, Sebastian, but considering there are
fewer than 20 users of dmaengine_pause() in the entire tree, we should add
WARN_ON_ONCE() around those uses with this patch to avoid a bunch needless
one-off fixes.
Regards,
Peter Hurley
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers
On 08/07/2015 02:32 PM, Russell King - ARM Linux wrote: On Fri, Aug 07, 2015 at 02:21:59PM -0400, Peter Hurley wrote: [ + Heikki ] On 08/07/2015 12:33 PM, Russell King - ARM Linux wrote: What you have is a race condition in the code you a responsible for maintaining, caused by poorly implemented code. Fix it, rather than whinging about drivers outside of your subsystem having never implemented _optional_ things that you choose to merge broken code which relied upon it _without_ checking that the operation succeeded. It is _entirely_ your code which is wrong here. I will wait for that to be fixed before acking the omap-dma change since you obviously need something to test with. I'm not sure to what you're referring here. A WARNing fixes nothing. The warning can wait. If you mean some patch, as yet unwritten, that handles the dma cases when dmaengine_pause() is unimplemented without data loss, ok, but please confirm that's what you mean. But the regression needs fixing. I too would prefer the bug to be fixed. But calling it a regression is incorrect. There is no previous SHA in which this problem didn't exist, except before either 8250_dma or 8250_omap was added. From the outset, both the 8250 dma code and the 8250_omap driver (mistakenly) relied on dmaengine_pause. However, at some point one must look at the api and wonder if the separation of concern has been drawn in the right place. It _is_ in the right place. dmaengine_pause() always has been permitted to fail. It's the responsibility of the user of this API to _check_ the return code to find out whether it had the desired effect. Not checking the return code is a bug in the caller's code. If that wasn't the case, dmaengine_pause() would have a void return type. It doesn't. It has an 'int' to allow failure A resource error is significantly different than ENOSYS or EINVAL. or to allow non- implementation for cases where the underlying hardware can't pause the channel without causing data loss. That's your assertion; I've seen no documentation to back that up (other than the de facto commit). And quite frankly, that's absurd. 1. No other driver implements _only some_ use-cases of dmaengine_pause(). 2. The number of users expecting dmaengine_pause to be implemented for non-cyclic dma transfers _dwarfs_ cyclic users. 3. There's a dedicated query interface, dma_get_slave_caps(), for which omap-dma returns /true/ -- not /maybe/ -- to indicate dmaengine_pause() is implemented. As a consumer of the api, I'd much rather opt-out at device initialization time knowing that a required feature is unimplemented, than discover it at i/o time when it's too late. What would you think is better: an API which silently loses data, or one which refuses to stop the transfer and reports an error code back to the caller. An api which provides a means of determining if necessary functionality is implemented _during setup_. That way the consumer of the api can determine if the feature is supportable. For example, dma_get_slave_caps() could differentiate * pause for cyclic support * pause for non-cyclic support * pause and resume support * pause and terminate support You seem to be arguing for the former, and as such, there's no way I can take you seriously. Leaping to conclusions. In any case, Greg has now commented on the patch adding the feature, basically refusing it for stable tree inclusion. So the matter is settled: omap-dma isn't going to get the pause feature added in stable trees any time soon. So a different solution now needs to be found, which is what I've been saying all along... While Sebastian's initial patch is a good first-cut at addressing 8250_omap's use of omap-dma, none of the patches address the general design problem I have outlined above; namely, that simply returning an error at use time for an unimplemented slave transaction is fundamentally flawed. Regards, Peter Hurley -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
