| Message ID | 1397772242-4048-1-git-send-email-agross@codeaurora.org (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
On Thu, Apr 17, 2014 at 05:04:02PM -0500, Andy Gross wrote: > This patch adds APIs that allow for BAM hardware flags to be set per > descriptor. Each one of the new flags informs the attached peripheral of a > special behavior that is required. > > The EOT flag requests that the peripheral assert an end of transaction interrupt > when that descriptor is complete. It also results in special signaling protocol > that is used between the attached peripheral and the core using the DMA > controller. DMA_PREP_INTERRUPT ?? > > The NWD flag requests that the peripheral wait until the data has been fully > processed before signaling an interrupt. interrupt for transaction complete or DMA request?
On Fri, May 02, 2014 at 09:58:41PM +0530, Vinod Koul wrote: > On Thu, Apr 17, 2014 at 05:04:02PM -0500, Andy Gross wrote: > > This patch adds APIs that allow for BAM hardware flags to be set per > > descriptor. Each one of the new flags informs the attached peripheral of a > > special behavior that is required. > > > > The EOT flag requests that the peripheral assert an end of transaction interrupt > > when that descriptor is complete. It also results in special signaling protocol > > that is used between the attached peripheral and the core using the DMA > > controller. > DMA_PREP_INTERRUPT ?? I have 3 different IRQs that can be asserted based on the bit I set in the hardware descriptor. The normal IRQ is the INT bit. However, in some cases the peripheral protocol requires the use of the EOT or EOB interrupt instead. The DMA_PREP_INTERRUPT would only work if I had only 2 choices. > > > > > The NWD flag requests that the peripheral wait until the data has been fully > > processed before signaling an interrupt. > interrupt for transaction complete or DMA request? This is a special signaling mechanism that holds off the DMA interrupt until the peripheral actually acks that the data has been processed completely. This is required in many cases by the peripheral. One example is the SPI controller. At the end of a transaction you are supposed to set the NWD so that the chip select is de-asserted. <snip>
On Fri, May 02, 2014 at 01:08:27PM -0500, Andy Gross wrote: > On Fri, May 02, 2014 at 09:58:41PM +0530, Vinod Koul wrote: > > On Thu, Apr 17, 2014 at 05:04:02PM -0500, Andy Gross wrote: > > > This patch adds APIs that allow for BAM hardware flags to be set per > > > descriptor. Each one of the new flags informs the attached peripheral of a > > > special behavior that is required. > > > > > > The EOT flag requests that the peripheral assert an end of transaction interrupt > > > when that descriptor is complete. It also results in special signaling protocol > > > that is used between the attached peripheral and the core using the DMA > > > controller. > > DMA_PREP_INTERRUPT ?? > > I have 3 different IRQs that can be asserted based on the bit I set in the > hardware descriptor. The normal IRQ is the INT bit. However, in some cases the > peripheral protocol requires the use of the EOT or EOB interrupt instead. The > DMA_PREP_INTERRUPT would only work if I had only 2 choices. Thinking about this more, I could use the DMA_PREP_INTERRUPT to cover the EOT flag. However, I might get in a bind later if I need to support the EOB (end of block) interrupt. > > > > > > > > > The NWD flag requests that the peripheral wait until the data has been fully > > > processed before signaling an interrupt. > > interrupt for transaction complete or DMA request? > > This is a special signaling mechanism that holds off the DMA interrupt until the > peripheral actually acks that the data has been processed completely. This is > required in many cases by the peripheral. One example is the SPI controller. > At the end of a transaction you are supposed to set the NWD so that the chip > select is de-asserted. I'm not sure what flag I could map this to... maybe DMA_CTRL_ACK? or maybe the DMA_PREP_FENCE? I don't generally like overloading the flags and slightly twisting their intent. Could we add a flag to denote device ACK?
Hi Andy, On 15/05/14 18:32, Andy Gross wrote: > On Fri, May 02, 2014 at 01:08:27PM -0500, Andy Gross wrote: >> On Fri, May 02, 2014 at 09:58:41PM +0530, Vinod Koul wrote: >>> On Thu, Apr 17, 2014 at 05:04:02PM -0500, Andy Gross wrote: >>>> This patch adds APIs that allow for BAM hardware flags to be set per >>>> descriptor. Each one of the new flags informs the attached peripheral of a >>>> special behavior that is required. >>>> >>>> The EOT flag requests that the peripheral assert an end of transaction interrupt >>>> when that descriptor is complete. It also results in special signaling protocol >>>> that is used between the attached peripheral and the core using the DMA >>>> controller. >>> DMA_PREP_INTERRUPT ?? >> >> I have 3 different IRQs that can be asserted based on the bit I set in the >> hardware descriptor. The normal IRQ is the INT bit. However, in some cases the >> peripheral protocol requires the use of the EOT or EOB interrupt instead. The >> DMA_PREP_INTERRUPT would only work if I had only 2 choices. > > Thinking about this more, I could use the DMA_PREP_INTERRUPT to cover the EOT > flag. However, I might get in a bind later if I need to support the EOB (end of > block) interrupt. This is good start, mapping EOT to DMA_PREP_INTERRUPT seems to be much appropriate. This will also provide mmci driver with much more generic interface than the bam specific function calls. Only way forward is to tie up this descriptor specific flags to more generic flags. Having specific callbacks would introduce limitations in using generic device drivers. Not really sure how we can map EOB/NWD flags without really defining new flags. Mapping to other generic flags might be totally confusing to people using/interpreting those flags. Needs more discussion on this. --srini > >> >>> >>>> >>>> The NWD flag requests that the peripheral wait until the data has been fully >>>> processed before signaling an interrupt. >>> interrupt for transaction complete or DMA request? >> >> This is a special signaling mechanism that holds off the DMA interrupt until the >> peripheral actually acks that the data has been processed completely. This is >> required in many cases by the peripheral. One example is the SPI controller. >> At the end of a transaction you are supposed to set the NWD so that the chip >> select is de-asserted. > > I'm not sure what flag I could map this to... maybe DMA_CTRL_ACK? or maybe the > DMA_PREP_FENCE? I don't generally like overloading the flags and slightly > twisting their intent. Could we add a flag to denote device ACK? > -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 15, 2014 at 12:32:06PM -0500, Andy Gross wrote: > On Fri, May 02, 2014 at 01:08:27PM -0500, Andy Gross wrote: > > On Fri, May 02, 2014 at 09:58:41PM +0530, Vinod Koul wrote: > > > On Thu, Apr 17, 2014 at 05:04:02PM -0500, Andy Gross wrote: > > > > This patch adds APIs that allow for BAM hardware flags to be set per > > > > descriptor. Each one of the new flags informs the attached peripheral of a > > > > special behavior that is required. > > > > > > > > The EOT flag requests that the peripheral assert an end of transaction interrupt > > > > when that descriptor is complete. It also results in special signaling protocol > > > > that is used between the attached peripheral and the core using the DMA > > > > controller. > > > DMA_PREP_INTERRUPT ?? > > > > I have 3 different IRQs that can be asserted based on the bit I set in the > > hardware descriptor. The normal IRQ is the INT bit. However, in some cases the > > peripheral protocol requires the use of the EOT or EOB interrupt instead. The > > DMA_PREP_INTERRUPT would only work if I had only 2 choices. > > Thinking about this more, I could use the DMA_PREP_INTERRUPT to cover the EOT > flag. However, I might get in a bind later if I need to support the EOB (end of > block) interrupt. Sorry for delay in this. I think it would make sense to use DMA_PREP_INTERRUPT for EOB interrupt. The EOT should always be enabled for the cases where it is applicable instead of nomral irq. That should genrically ocvery your cases, or did we miss anything here > > > > > > > > > > > > > > The NWD flag requests that the peripheral wait until the data has been fully > > > > processed before signaling an interrupt. > > > interrupt for transaction complete or DMA request? > > > > This is a special signaling mechanism that holds off the DMA interrupt until the > > peripheral actually acks that the data has been processed completely. This is > > required in many cases by the peripheral. One example is the SPI controller. > > At the end of a transaction you are supposed to set the NWD so that the chip > > select is de-asserted. > > I'm not sure what flag I could map this to... maybe DMA_CTRL_ACK? or maybe the > DMA_PREP_FENCE? I don't generally like overloading the flags and slightly > twisting their intent. Could we add a flag to denote device ACK? Nope lets not override these...
On Thu, May 22, 2014 at 11:40:49AM +0530, Vinod Koul wrote: <snip> > > > I have 3 different IRQs that can be asserted based on the bit I set in the > > > hardware descriptor. The normal IRQ is the INT bit. However, in some cases the > > > peripheral protocol requires the use of the EOT or EOB interrupt instead. The > > > DMA_PREP_INTERRUPT would only work if I had only 2 choices. > > > > Thinking about this more, I could use the DMA_PREP_INTERRUPT to cover the EOT > > flag. However, I might get in a bind later if I need to support the EOB (end of > > block) interrupt. > Sorry for delay in this. > > I think it would make sense to use DMA_PREP_INTERRUPT for EOB interrupt. The EOT > should always be enabled for the cases where it is applicable instead of nomral > irq. > > That should genrically ocvery your cases, or did we miss anything here The EOT is not used for every transaction. It is part of a handshaking protocol with the attached peripheral, much like the NWD (notify when done). As near as I can tell today, no peripheral depends on the EOB, so we could drop it for now until it is needed and cross this bridge when we need to. > > > > > > > > > > > > > > > > > > > The NWD flag requests that the peripheral wait until the data has been fully > > > > > processed before signaling an interrupt. > > > > interrupt for transaction complete or DMA request? > > > > > > This is a special signaling mechanism that holds off the DMA interrupt until the > > > peripheral actually acks that the data has been processed completely. This is > > > required in many cases by the peripheral. One example is the SPI controller. > > > At the end of a transaction you are supposed to set the NWD so that the chip > > > select is de-asserted. > > > > I'm not sure what flag I could map this to... maybe DMA_CTRL_ACK? or maybe the > > DMA_PREP_FENCE? I don't generally like overloading the flags and slightly > > twisting their intent. Could we add a flag to denote device ACK? > Nope lets not override these... Then I need to add a flag. Something like DMA_PREP_DEVICE_ACK that denotes that the attached device needs to ACK the transfer. Also, one thing I forgot. For crypto and some of the other blocks, we have something called command descriptors that can be transferred to the blocks to provide programming or direction. The DMA controller has a flag that tells the attached peripheral that the incoming DMA contains command descriptors. The dma descriptors still point to a memory region to transfer, but the contents are interpreted differently. This is yet another flag in the descriptor flag section. I'd need another flag for this as well. Something like DMA_PREP_CMD. We are actively working on upstreaming our crypto block and this will be required for that to work properly.
On 22/05/14 16:09, Andy Gross wrote: > On Thu, May 22, 2014 at 11:40:49AM +0530, Vinod Koul wrote: > > <snip> > >>>> I have 3 different IRQs that can be asserted based on the bit I set in the >>>> hardware descriptor. The normal IRQ is the INT bit. However, in some cases the >>>> peripheral protocol requires the use of the EOT or EOB interrupt instead. The >>>> DMA_PREP_INTERRUPT would only work if I had only 2 choices. >>> >>> Thinking about this more, I could use the DMA_PREP_INTERRUPT to cover the EOT >>> flag. However, I might get in a bind later if I need to support the EOB (end of >>> block) interrupt. >> Sorry for delay in this. >> >> I think it would make sense to use DMA_PREP_INTERRUPT for EOB interrupt. The EOT >> should always be enabled for the cases where it is applicable instead of nomral >> irq. >> >> That should genrically ocvery your cases, or did we miss anything here > > The EOT is not used for every transaction. It is part of a handshaking > protocol with the attached peripheral, much like the NWD (notify when done). As > near as I can tell today, no peripheral depends on the EOB, so we could drop it > for now until it is needed and cross this bridge when we need to. As EOT behaviour is totally dependent on the attached peripheral(or channel), Can't we make this specific to channel by passing additional flags in the DT dma channel descriptors? This will be better abstraction for drivers as well. I know that EOT flag is part of descriptor but still some channels *must* have EOT to run there state-machine correctly. So making it optional for those channels might be wrong. Are there any use cases for particular *channel* where EOT requirement changes dynamically? --srini > >>> >>>> >>>>> >>>>>> >>>>>> The NWD flag requests that the peripheral wait until the data has been fully >>>>>> processed before signaling an interrupt. >>>>> interrupt for transaction complete or DMA request? >>>> >>>> This is a special signaling mechanism that holds off the DMA interrupt until the >>>> peripheral actually acks that the data has been processed completely. This is >>>> required in many cases by the peripheral. One example is the SPI controller. >>>> At the end of a transaction you are supposed to set the NWD so that the chip >>>> select is de-asserted. >>> >>> I'm not sure what flag I could map this to... maybe DMA_CTRL_ACK? or maybe the >>> DMA_PREP_FENCE? I don't generally like overloading the flags and slightly >>> twisting their intent. Could we add a flag to denote device ACK? >> Nope lets not override these... > > Then I need to add a flag. Something like DMA_PREP_DEVICE_ACK that denotes that the > attached device needs to ACK the transfer. > > Also, one thing I forgot. For crypto and some of the other blocks, we have > something called command descriptors that can be transferred to the blocks to > provide programming or direction. The DMA controller has a flag that tells the > attached peripheral that the incoming DMA contains command descriptors. The dma > descriptors still point to a memory region to transfer, but the contents are > interpreted differently. This is yet another flag in the descriptor flag > section. I'd need another flag for this as well. Something like DMA_PREP_CMD. > > We are actively working on upstreaming our crypto block and this will be > required for that to work properly. > -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 22, 2014 at 04:27:05PM +0100, Srinivas Kandagatla wrote: <snip> > > > >The EOT is not used for every transaction. It is part of a handshaking > >protocol with the attached peripheral, much like the NWD (notify when done). As > >near as I can tell today, no peripheral depends on the EOB, so we could drop it > >for now until it is needed and cross this bridge when we need to. > > As EOT behaviour is totally dependent on the attached peripheral(or > channel), Can't we make this specific to channel by passing > additional flags in the DT dma channel descriptors? This will be > better abstraction for drivers as well. Even for channels where you want to use EOT, you don't use it for every transaction. So a global channel flag isn't going to work. This is the same for NWD. It is a per descriptor choice. > > I know that EOT flag is part of descriptor but still some channels > *must* have EOT to run there state-machine correctly. So making it > optional for those channels might be wrong. > > Are there any use cases for particular *channel* where EOT > requirement changes dynamically? I2C is one example. You place EOT on the last transaction that makes up a write/read transaction. You may have multiple descriptors to send data, but the last one has EOT. And for read transactions, you place NWD on the last read transaction. <snip>
On 22/05/14 16:32, Andy Gross wrote: > On Thu, May 22, 2014 at 04:27:05PM +0100, Srinivas Kandagatla wrote: > > <snip> > >>> >>> The EOT is not used for every transaction. It is part of a handshaking >>> protocol with the attached peripheral, much like the NWD (notify when done). As >>> near as I can tell today, no peripheral depends on the EOB, so we could drop it >>> for now until it is needed and cross this bridge when we need to. >> >> As EOT behaviour is totally dependent on the attached peripheral(or >> channel), Can't we make this specific to channel by passing >> additional flags in the DT dma channel descriptors? This will be >> better abstraction for drivers as well. > > Even for channels where you want to use EOT, you don't use it for every > transaction. So a global channel flag isn't going to work. This is the same > for NWD. It is a per descriptor choice. Thanks Andy for explaining, I got it now. > >> >> I know that EOT flag is part of descriptor but still some channels >> *must* have EOT to run there state-machine correctly. So making it >> optional for those channels might be wrong. >> >> Are there any use cases for particular *channel* where EOT >> requirement changes dynamically? > > I2C is one example. You place EOT on the last transaction that makes up a > write/read transaction. You may have multiple descriptors to send data, but the > last one has EOT. And for read transactions, you place NWD on the last read > transaction. > > <snip> > -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/dma/qcom_bam_dma.c b/drivers/dma/qcom_bam_dma.c index 02f7fef..f6c8ef1 100644 --- a/drivers/dma/qcom_bam_dma.c +++ b/drivers/dma/qcom_bam_dma.c @@ -61,12 +61,18 @@ struct bam_desc_hw { #define DESC_FLAG_INT BIT(15) #define DESC_FLAG_EOT BIT(14) #define DESC_FLAG_EOB BIT(13) +#define DESC_FLAG_NWD BIT(12) +#define DESC_FLAG_CMD BIT(11) struct bam_async_desc { struct virt_dma_desc vd; u32 num_desc; u32 xfer_len; + + /* transaction flags, EOT|EOB|NWD|CMD */ + u16 flags; + struct bam_desc_hw *curr_desc; enum dma_transfer_direction dir; @@ -800,6 +806,34 @@ static void bam_apply_new_config(struct bam_chan *bchan, bchan->reconfigure = 0; } +void qcom_bam_set_desc_eot(struct dma_async_tx_descriptor *txd) +{ + struct bam_async_desc *async_desc = container_of(txd, + struct bam_async_desc, vd.tx); + + async_desc->flags |= DESC_FLAG_EOT; +} +EXPORT_SYMBOL(qcom_bam_set_desc_eot); + +void qcom_bam_set_desc_cmd(struct dma_async_tx_descriptor *txd) +{ + struct bam_async_desc *async_desc = container_of(txd, + struct bam_async_desc, vd.tx); + + async_desc->flags |= DESC_FLAG_CMD; +} +EXPORT_SYMBOL(qcom_bam_set_desc_cmd); + +void qcom_bam_set_desc_nwd(struct dma_async_tx_descriptor *txd) +{ + struct bam_async_desc *async_desc = container_of(txd, + struct bam_async_desc, vd.tx); + + async_desc->flags |= DESC_FLAG_NWD; +} +EXPORT_SYMBOL(qcom_bam_set_desc_nwd); + + /** * bam_start_dma - start next transaction * @bchan - bam dma channel @@ -812,6 +846,7 @@ static void bam_start_dma(struct bam_chan *bchan) struct bam_desc_hw *desc; struct bam_desc_hw *fifo = PTR_ALIGN(bchan->fifo_virt, sizeof(struct bam_desc_hw)); + int i; lockdep_assert_held(&bchan->vc.lock); @@ -838,8 +873,17 @@ static void bam_start_dma(struct bam_chan *bchan) else async_desc->xfer_len = async_desc->num_desc; - /* set INT on last descriptor */ - desc[async_desc->xfer_len - 1].flags |= DESC_FLAG_INT; + /* set command descriptor flag, if applicable */ + if (async_desc->flags & DESC_FLAG_CMD) + for (i = 0; i < async_desc->xfer_len; i++) + desc[i].flags |= DESC_FLAG_CMD; + + /* set EOT or INT based on flag settings and if final transaction */ + if (async_desc->flags & DESC_FLAG_EOT && + async_desc->num_desc == async_desc->xfer_len) + desc[async_desc->xfer_len - 1].flags |= DESC_FLAG_EOT; + else + desc[async_desc->xfer_len - 1].flags |= DESC_FLAG_INT; if (bchan->tail + async_desc->xfer_len > MAX_DESCRIPTORS) { u32 partial = MAX_DESCRIPTORS - bchan->tail; diff --git a/include/linux/dma/qcom_bam_dma.h b/include/linux/dma/qcom_bam_dma.h new file mode 100644 index 0000000..65a371eb --- /dev/null +++ b/include/linux/dma/qcom_bam_dma.h @@ -0,0 +1,23 @@ +#ifndef _QCOM_BAM_DMA_H_ +#define _QCOM_BAM_DMA_H_ +/* + * Copyright (c) 2014, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <linux/dmaengine.h> + +void qcom_bam_set_desc_cmd(struct dma_async_tx_descriptor *); +void qcom_bam_set_desc_eot(struct dma_async_tx_descriptor *); +void qcom_bam_set_desc_nwd(struct dma_async_tx_descriptor *); + +#endif
This patch adds APIs that allow for BAM hardware flags to be set per descriptor. Each one of the new flags informs the attached peripheral of a special behavior that is required. The EOT flag requests that the peripheral assert an end of transaction interrupt when that descriptor is complete. It also results in special signaling protocol that is used between the attached peripheral and the core using the DMA controller. The NWD flag requests that the peripheral wait until the data has been fully processed before signaling an interrupt. The CMD flag informs the peripheral that the descriptor payload contains command descriptors and not data descriptors. Signed-off-by: Andy Gross <agross@codeaurora.org> --- drivers/dma/qcom_bam_dma.c | 48 ++++++++++++++++++++++++++++++++++++-- include/linux/dma/qcom_bam_dma.h | 23 ++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 include/linux/dma/qcom_bam_dma.h