[linux-sunxi] Re: [PATCH v7] DMA: sun6i: Add driver for the Allwinner A31 DMA controller

[Vinod Koul]

On Thu, Apr 24, 2014 at 04:22:44PM +0200, Maxime Ripard wrote:
 The Allwinner A31 has a 16 channels DMA controller that it shares with the
 newer A23. Although sharing some similarities with the DMA controller of the
 older Allwinner SoCs, it's significantly different, I don't expect it to be
 possible to share the driver for these two.
 
 The A31 Controller is able to memory-to-memory or memory-to-device transfers 
 on
 the 16 channels in parallel.

Overall driver looks in good shape, few comments though...

 +This driver follows the generic DMA bindings defined in dma.txt.
 +
 +Required properties:
 +
 +- compatible:Must be allwinner,sun6i-a31-dma
 +- reg:   Should contain the registers base address and length
 +- interrupts:Should contain a reference to the interrupt used by 
 this device
 +- clocks:Should contain a reference to the parent AHB clock
 +- resets:Should contain a reference to the reset controller asserting
 + this device in reset
 +- #dma-cells :   Should be 1, a single cell holding a line request number
 +
 +Example:
 + dma: dma-controller@01c02000 {
 + compatible = allwinner,sun6i-a31-dma;
 + reg = 0x01c02000 0x1000;
 + interrupts = 0 50 4;
 + clocks = ahb1_gates 6;
 + resets = ahb1_rst 6;
 + #dma-cells = 1;
 + };
 +
 +Clients:
 +
 +DMA clients connected to the A31 DMA controller must use the format
 +described in the dma.txt file, using a two-cell specifier for each
 +channel: a phandle plus one integer cells.
 +The two cells in order are:
 +
 +1. A phandle pointing to the DMA controller.
 +2. The port ID as specified in the datasheet
 +
 +Example:
 +spi2: spi@01c6a000 {
 + compatible = allwinner,sun6i-a31-spi;
 + reg = 0x01c6a000 0x1000;
 + interrupts = 0 67 4;
 + clocks = ahb1_gates 22, spi2_clk;
 + clock-names = ahb, mod;
 + dmas = dma 25, dma 25;
 + dma-names = rx, tx;
 + resets = ahb1_rst 22;
 +};
Ideally binding should be a separate patch

 +static inline u8 convert_burst(u8 maxburst)
 +{
 + if (maxburst == 1 || maxburst  16)
 + return 0;
are these valid configurations?

 +
 + return fls(maxburst) - 1;
 +}
 +
 +static inline u8 convert_buswidth(enum dma_slave_buswidth addr_width)
 +{
 + switch (addr_width) {
 + case DMA_SLAVE_BUSWIDTH_2_BYTES:
 + return 1;
 + case DMA_SLAVE_BUSWIDTH_4_BYTES:
 + return 2;
return (addr_width  1); ..??
since DMA_SLAVE_BUSWIDTH_2_BYTES is numeric 2 and DMA_SLAVE_BUSWIDTH_4_BYTES
numeric 4. 
 + default:
 + return 0;
error?

 +static inline void sun6i_dma_cfg_lli(struct sun6i_dma_lli *lli,
 +  dma_addr_t src,
 +  dma_addr_t dst, u32 len,
 +  struct dma_slave_config *config)
 +{
 + u32 src_width, dst_width, src_burst, dst_burst;
 +
 + if (!config)
 + return;
 +
 + src_burst = convert_burst(config-src_maxburst);
 + dst_burst = convert_burst(config-dst_maxburst);
 +
 + src_width = convert_buswidth(config-src_addr_width);
 + dst_width = convert_buswidth(config-dst_addr_width);
is 0 a valid case then?

 +
 +static int sun6i_dma_terminate_all(struct sun6i_vchan *vchan)
 +{
 + struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(vchan-vc.chan.device);
 + struct sun6i_pchan *pchan = vchan-phy;
 + unsigned long flags;
 + LIST_HEAD(head);
 +
 + spin_lock(sdev-lock);
 + list_del_init(vchan-node);
 + spin_unlock(sdev-lock);
 +
 + spin_lock_irqsave(vchan-vc.lock, flags);
 +
 + vchan_get_all_descriptors(vchan-vc, head);
 +
 + if (pchan) {
 + writel(DMA_CHAN_ENABLE_STOP, pchan-base + DMA_CHAN_ENABLE);
 + writel(DMA_CHAN_PAUSE_RESUME, pchan-base + DMA_CHAN_PAUSE);
 +
 + vchan-phy = NULL;
 + pchan-vchan = NULL;
 + pchan-desc = NULL;
 + pchan-done = NULL;
 + }
 +
 + spin_unlock_irqrestore(vchan-vc.lock, flags);
 +
 + vchan_dma_desc_free_list(vchan-vc, head);

shouldn't you kill the tasklet as well here?

 +static inline void sun6i_dma_free(struct sun6i_dma_dev *sdc)
 +{
 + int i;
 +
 + for (i = 0; i  NR_MAX_VCHANS; i++) {
 + struct sun6i_vchan *vchan = sdc-vchans[i];
 +
 + list_del(vchan-vc.chan.device_node);
 + tasklet_kill(vchan-vc.task);
 + }
 +
 + tasklet_kill(sdc-task);
This is again not good. see http://lwn.net/Articles/588457/
At this point HW can still generate interrupts or you can have irq running!

 +
 +static int sun6i_dma_probe(struct platform_device *pdev)
 +{
 + struct sun6i_dma_dev *sdc;
 + struct resource *res;
 + struct clk *mux, *pll6;
 + int irq;
 + int ret, i;
 +
 + sdc = devm_kzalloc(pdev-dev, sizeof(*sdc), GFP_KERNEL);
 + if (!sdc)
 + return -ENOMEM;
 +
 + res = 

[linux-sunxi] Re: [PATCH v7] DMA: sun6i: Add driver for the Allwinner A31 DMA controller

[Vinod Koul]

On Wed, Apr 30, 2014 at 02:53:22PM -0700, Maxime Ripard wrote:
 Hi Vinod,
 
 On Wed, Apr 30, 2014 at 12:34:08PM +0530, Vinod Koul wrote:
  On Thu, Apr 24, 2014 at 04:22:44PM +0200, Maxime Ripard wrote:
   +static inline void sun6i_dma_free(struct sun6i_dma_dev *sdc)
   +{
   + int i;
   +
   + for (i = 0; i  NR_MAX_VCHANS; i++) {
   + struct sun6i_vchan *vchan = sdc-vchans[i];
   +
   + list_del(vchan-vc.chan.device_node);
   + tasklet_kill(vchan-vc.task);
   + }
   +
   + tasklet_kill(sdc-task);
  This is again not good. see http://lwn.net/Articles/588457/
  At this point HW can still generate interrupts or you can have irq running!
 
 I'm not sure to fully understand the issue here, but what is not good?
 the first or the second tasklet_kill calls, or both?
 
 From what I understood, the issue is only there whenever you are
 calling tasklet_disable without making sure that no one will schedule
 your tasklet before disabling it.
 
 But the point is I don't actually use either _enable/_disable. I might
 be wrong in not using those functions, but I don't really see how I
 can be impacted.

Well that was one part of it. How do you ensure the tasklet is not scheduled
while and after you are killing it. You need to ensure irq is disabled and 
pending irqs
have finished processing. I dont see that bit.

-- 
~Vinod



signature.asc
Description: Digital signature

[linux-sunxi] Re: [PATCH v7] DMA: sun6i: Add driver for the Allwinner A31 DMA controller

[Vinod Koul]

On Wed, May 07, 2014 at 10:19:49PM -0500, Maxime Ripard wrote:
 On Fri, May 02, 2014 at 10:04:29PM +0530, Vinod Koul wrote:
  On Wed, Apr 30, 2014 at 02:53:22PM -0700, Maxime Ripard wrote:
   Hi Vinod,
   
   On Wed, Apr 30, 2014 at 12:34:08PM +0530, Vinod Koul wrote:
On Thu, Apr 24, 2014 at 04:22:44PM +0200, Maxime Ripard wrote:
 +static inline void sun6i_dma_free(struct sun6i_dma_dev *sdc)
 +{
 + int i;
 +
 + for (i = 0; i  NR_MAX_VCHANS; i++) {
 + struct sun6i_vchan *vchan = sdc-vchans[i];
 +
 + list_del(vchan-vc.chan.device_node);
 + tasklet_kill(vchan-vc.task);
 + }
 +
 + tasklet_kill(sdc-task);
This is again not good. see http://lwn.net/Articles/588457/
At this point HW can still generate interrupts or you can have irq 
running!
   
   I'm not sure to fully understand the issue here, but what is not good?
   the first or the second tasklet_kill calls, or both?
   
   From what I understood, the issue is only there whenever you are
   calling tasklet_disable without making sure that no one will schedule
   your tasklet before disabling it.
   
   But the point is I don't actually use either _enable/_disable. I might
   be wrong in not using those functions, but I don't really see how I
   can be impacted.
  
  Well that was one part of it. How do you ensure the tasklet is not scheduled
  while and after you are killing it. You need to ensure irq is disabled and 
  pending irqs
  have finished processing. I dont see that bit.
 
 Ok. I'll change that.
 
 Do you want me to use tasklet_enable and tasklet_disable as well?
I dont think it will help in this usage.

-- 
~Vinod


signature.asc
Description: Digital signature

[linux-sunxi] Re: [PATCH v7] DMA: sun6i: Add driver for the Allwinner A31 DMA controller

[Vinod Koul]

On Tue, May 13, 2014 at 03:42:58PM +0200, Maxime Ripard wrote:
 Hi Vinod,
 
 On Wed, Apr 30, 2014 at 12:34:08PM +0530, Vinod Koul wrote:
   +
   +static int sun6i_dma_terminate_all(struct sun6i_vchan *vchan)
   +{
   + struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(vchan-vc.chan.device);
   + struct sun6i_pchan *pchan = vchan-phy;
   + unsigned long flags;
   + LIST_HEAD(head);
   +
   + spin_lock(sdev-lock);
   + list_del_init(vchan-node);
   + spin_unlock(sdev-lock);
   +
   + spin_lock_irqsave(vchan-vc.lock, flags);
   +
   + vchan_get_all_descriptors(vchan-vc, head);
   +
   + if (pchan) {
   + writel(DMA_CHAN_ENABLE_STOP, pchan-base + DMA_CHAN_ENABLE);
   + writel(DMA_CHAN_PAUSE_RESUME, pchan-base + DMA_CHAN_PAUSE);
   +
   + vchan-phy = NULL;
   + pchan-vchan = NULL;
   + pchan-desc = NULL;
   + pchan-done = NULL;
   + }
   +
   + spin_unlock_irqrestore(vchan-vc.lock, flags);
   +
   + vchan_dma_desc_free_list(vchan-vc, head);
  
  shouldn't you kill the tasklet as well here?
 
 Just to be clear, which tasklet? vchan's or the driver's?
You need to take care of both. But I suspect if we ensure irq is not triggered
and any pending ones are completed you can simply kill both of the tasklets
happily. See the fixes merged in dmaengine last cycle (hint: patchlog shows what
we need to do)

Btw just noticed, you *should* use dmaengine: as the subsytem name on the patch
series...

-- 
~Vinod


signature.asc
Description: Digital signature

[linux-sunxi] Re: [PATCH v11 0/2] Add support for the Allwinner A31 DMA Controller

[Vinod Koul]

On Thu, Jul 24, 2014 at 04:44:20PM -0700, Andrew Morton wrote:
 On Thu, 24 Jul 2014 14:13:15 +0200 Maxime Ripard 
 maxime.rip...@free-electrons.com wrote:
 
  On Thu, Jul 17, 2014 at 09:46:14PM +0200, Maxime Ripard wrote:
   Hi,
   
   This patchset adds support for the DMA controller found in the
   Allwinner A31 and A23 SoCs.
   
   This has been tested using the newly introduced SPI driver on an A31
   EVK. Support for DMA-driven SPI transfers will be the subject of
   another patch serie.
   
   This has been around for around 5 monthes now, and didn't get any
   review but nitpicks for three versions, so I feel like it could be
   merged quite quickly.
  
  Ok, so, who should I bribe to get this merged?
 
 Turns out I'm easily bribed.  The code looks pretty clean and simple
 and is refreshingly free of comments, which only confuse people anyway.
Sorry for the delay, I will get this done today and merge to slave-dmaengine
next

 I think we could do this as a single patch - is there any benefit to
 splitting it apart like this?
Typically DT bindings are sent as aseprate patch and review independent of
teh driver. Though not many from DT-list have looked up slowing down patches

 The combinations of spin_lock()/spin_lock_irq() and spin_lock_irqsave()
 are a bit scary - it's easy to get these optimisations wrong.  Has it
 been thoroughly tested with lockdep enabled?
Since the dma is handled in the irq, iut is actaully essential to have
irqsave versions for dmaenegine driver

Thanks
-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v11 0/2] Add support for the Allwinner A31 DMA Controller

[Vinod Koul]

On Fri, Jul 25, 2014 at 11:41:30AM +0530, Vinod Koul wrote:
 On Thu, Jul 24, 2014 at 04:44:20PM -0700, Andrew Morton wrote:
  On Thu, 24 Jul 2014 14:13:15 +0200 Maxime Ripard 
  maxime.rip...@free-electrons.com wrote:
  
   On Thu, Jul 17, 2014 at 09:46:14PM +0200, Maxime Ripard wrote:
Hi,

This patchset adds support for the DMA controller found in the
Allwinner A31 and A23 SoCs.

This has been tested using the newly introduced SPI driver on an A31
EVK. Support for DMA-driven SPI transfers will be the subject of
another patch serie.

This has been around for around 5 monthes now, and didn't get any
review but nitpicks for three versions, so I feel like it could be
merged quite quickly.
   
   Ok, so, who should I bribe to get this merged?
  
  Turns out I'm easily bribed.  The code looks pretty clean and simple
  and is refreshingly free of comments, which only confuse people anyway.
 Sorry for the delay, I will get this done today and merge to slave-dmaengine
 next
And looks like Andrew merged them. Do you mind if I take these up thru
dmaengine tree?

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v11 0/2] Add support for the Allwinner A31 DMA Controller

[Vinod Koul]

On Thu, Jul 17, 2014 at 09:46:14PM +0200, Maxime Ripard wrote:
 Hi,
 
 This patchset adds support for the DMA controller found in the
 Allwinner A31 and A23 SoCs.
 
 This has been tested using the newly introduced SPI driver on an A31
 EVK. Support for DMA-driven SPI transfers will be the subject of
 another patch serie.
 
 This has been around for around 5 monthes now, and didn't get any
 review but nitpicks for three versions, so I feel like it could be
 merged quite quickly.
I have applied this now.

Can you please send follow patches for these:
- don't recall if I pointed earlier, but can we use direct conversion for
  calculating convert_burst() and convert_buswidth(), latter one at least
  seem doable
- don't use devm_request_irq(). You have irq enabled and you have killed
  tasklet. This is too racy. You need to ensure no irqs can be generated before 
killing
  tasklets.
- use synchronize_irq() before killing tasklet

Thanks

-- 

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v11 0/2] Add support for the Allwinner A31 DMA Controller

[Vinod Koul]

On Fri, Jul 25, 2014 at 06:37:46PM +0200, Maxime Ripard wrote:
  Can you please send follow patches for these:
  - don't recall if I pointed earlier, but can we use direct conversion for
calculating convert_burst() and convert_buswidth(), latter one at least
seem doable
 
 Ok. Do you still want the error reporting for the invalid width and
 burst size?
I think for convert_buswidth() we can do away with a switch case and convert
the numbers directly

  - don't use devm_request_irq(). You have irq enabled and you have killed
tasklet. This is too racy. You need to ensure no irqs can be generated 
  before killing
tasklets.
 
 Ok, would calling disable_irq before killing the tasklet an option for
 you ? that would allow to keep the devm_request_irq.
disable_irq also does syncronize, so it might work. But then again if the
probe fails after registering irq due to some other reason, and you get into
races with unitialized driver as well.

I think it might be easier to amnge by working with plain old request_irq()

 I'll also send patches for the various breakages and warnings spotted
 by the autobuilders.
Yes please, I havent seen the reports in detail yet, but they are reporting
mainly due COMPILE_TEST. DO you really want that?

Yes it did help me to compile for ARM, i didnt see any defconfig which has
this MACH in arm/configs

-- 
~Vinod


signature.asc
Description: Digital signature

[linux-sunxi] Re: [PATCH v11 0/2] Add support for the Allwinner A31 DMA Controller

[Vinod Koul]

On Mon, Jul 28, 2014 at 12:14:02PM +0200, Maxime Ripard wrote:
 On Fri, Jul 25, 2014 at 10:12:18PM +0530, Vinod Koul wrote:
  On Fri, Jul 25, 2014 at 06:37:46PM +0200, Maxime Ripard wrote:
Can you please send follow patches for these:
- don't recall if I pointed earlier, but can we use direct conversion 
for
  calculating convert_burst() and convert_buswidth(), latter one at 
least
  seem doable
   
   Ok. Do you still want the error reporting for the invalid width and
   burst size?
  I think for convert_buswidth() we can do away with a switch case and convert
  the numbers directly
 
 Well, it's already using a switch. Do you want me to remove completely
 the function and move the switch where it's used? It's used two times
 now, so I'd like to avoid duplicating it.
You can probably remove switch in these function by converting them
arithmetically..

- don't use devm_request_irq(). You have irq enabled and you have killed
  tasklet. This is too racy. You need to ensure no irqs can be 
generated before killing
  tasklets.
   
   Ok, would calling disable_irq before killing the tasklet an option for
   you ? that would allow to keep the devm_request_irq.
  disable_irq also does syncronize, so it might work. But then again if the
  probe fails after registering irq due to some other reason, and you get into
  races with unitialized driver as well.
  
  I think it might be easier to amnge by working with plain old request_irq()
 
 Yes, devm_free_irq like Russell suggested seems to both address your
 concerns, while not having to care about it at probe.
OK, pls send the fix

-- 
~Vinod


signature.asc
Description: Digital signature

[linux-sunxi] Re: [PATCH 7/7] dmaengine: sun6i: Remove obsolete clk muxing code

[Vinod Koul]

On Sat, Sep 06, 2014 at 06:47:28PM +0800, Chen-Yu Tsai wrote:
 The sun6i DMA controller requires the AHB1 bus clock to be
 clocked from PLL6. This was originally done by the dmaengine
 driver during probe time. The AHB1 clock driver has since been
 unified, so the original code does not work.
 
 Remove the clk muxing code, and replace it with DT clk default
 properties.

Applied, thanks

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v2 0/3] ARM: sun8i: Add DMA controller support

[Vinod Koul]

On Fri, Sep 26, 2014 at 11:06:01AM +0800, Chen-Yu Tsai wrote:
 On Thu, Sep 18, 2014 at 11:24 AM, Chen-Yu Tsai w...@csie.org wrote:
  Hi everyone,
 
  This is v2 of my sun8i DMA controller support series. This series
  adds support for the DMA controller found in the Allwinner A23 SoC.
  It is the same hardware as found in the A31 (sun6i) SoC. In addition
  to reduced physical channels and endpoints, the controller in the A23
  requires an undocumented register to be toggled. That seems to allow
  memory bus access.
 
  This series is based on my earlier clk: sun6i: Unify AHB1 clock and
  fix rate calculation series, which removes the clock muxing calls from
  the sun6i-dma driver. The default PLL6 pre-divider for AHB1 on the A23
  results in an exceedingly high clock rate for AHB1, and the system hangs.
  Also, on the A23, the dma controller happily works even when AHB1 is
  clocked from AXI.
 
 
  Patch 1 changes the channel count macros into runtime data binded to
  the DT compatible strings. It also gets rid of some hardcoded values
  in the interrupt handler.
 
  Patch 2 adds the channel number data for the A23 (sun8i), as well as
  the undocumented register quirk.
 
 Hi, Vinod,
 
 Any chance we can get patches 1  2 merged? You've already merged the
 prerequisite patch dmaengine: sun6i: Remove obsolete clk muxing code
 a few days ago, and patch 3 is already in arm-soc.

Hi ChenYu,

The patches look fine to me so I tried applying them but they fail to apply,
can you please rebase these two and resend

Thanks

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v2 0/3] ARM: sun8i: Add DMA controller support

[Vinod Koul]

On Thu, Nov 06, 2014 at 03:54:35PM +0800, Chen-Yu Tsai wrote:
  The patches look fine to me so I tried applying them but they fail to apply,
  can you please rebase these two and resend
 
 Hi Vinod,
 
 Could you publish an updated slave-dma/next branch? Currently I see it at
 v3.18-rc1. And the series rebases cleanly onto it with no differences.
 
 I'm guessing some of Maxime's cleanup work is the source of conflict.
Its pushed now..

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v4] dma: sun4i: Add support for the DMA engine on sun[457]i SoCs

[Vinod Koul]

On Sat, Jan 31, 2015 at 07:58:44PM -0300, Emilio López wrote:

 +/** Dedicated DMA register layout **/
 +
 +/* Dedicated DMA configuration register layout */
 +#define DDMA_CFG_LOADING BIT(31)
 +#define DDMA_CFG_BUSYBIT(30)
 +#define DDMA_CFG_CONT_MODE   BIT(29)
 +#define DDMA_CFG_DEST_NON_SECURE BIT(28)
 +#define DDMA_CFG_DEST_DATA_WIDTH(width)  ((width)  25)
 +#define DDMA_CFG_DEST_BURST_LENGTH(len)  ((len)  23)
 +#define DDMA_CFG_DEST_ADDR_MODE(mode)((mode)  21)
 +#define DDMA_CFG_DEST_DRQ_TYPE(type) ((type)  16)
 +#define DDMA_CFG_BYTE_COUNT_MODE_REMAIN  BIT(15)
 +#define DDMA_CFG_SRC_NON_SECURE  BIT(12)
 +#define DDMA_CFG_SRC_DATA_WIDTH(width)   ((width)  9)
 +#define DDMA_CFG_SRC_BURST_LENGTH(len)   ((len)  7)
 +#define DDMA_CFG_SRC_ADDR_MODE(mode) ((mode)  5)
 +#define DDMA_CFG_SRC_DRQ_TYPE(type)  ((type)  0)
 +
 +/* Dedicated DMA parameter register layout */
 +#define DDMA_PARA_DEST_DATA_BLK_SIZE(n)  (((n) - 1)  24)
 +#define DDMA_PARA_DEST_WAIT_CYCLES(n)(((n) - 1)  16)
 +#define DDMA_PARA_SRC_DATA_BLK_SIZE(n)   (((n) - 1)  8)
 +#define DDMA_PARA_SRC_WAIT_CYCLES(n) (((n) - 1)  0)
 +
 +/** DMA register offsets **/
 +
 +/* General register offsets */
 +#define DMA_IRQ_ENABLE_REG   0x0
 +#define DMA_IRQ_PENDING_STATUS_REG   0x4
 +
 +/* Normal DMA register offsets */
 +#define NDMA_CHANNEL_REG_BASE(n) (0x100 + (n) * 0x20)
 +#define NDMA_CFG_REG 0x0
 +#define NDMA_SRC_ADDR_REG0x4
 +#define NDMA_DEST_ADDR_REG   0x8
 +#define NDMA_BYTE_COUNT_REG  0xC
 +
 +/* Dedicated DMA register offsets */
 +#define DDMA_CHANNEL_REG_BASE(n) (0x300 + (n) * 0x20)
 +#define DDMA_CFG_REG 0x0
 +#define DDMA_SRC_ADDR_REG0x4
 +#define DDMA_DEST_ADDR_REG   0x8
 +#define DDMA_BYTE_COUNT_REG  0xC
 +#define DDMA_PARA_REG0x18
All these should be namespaced to avoid future conflicts

 +
 +/** DMA Driver **/
 +
 +/*
 + * Normal DMA has 8 channels, and Dedicated DMA has another 8, so that's
 + * 16 channels. As for endpoints, there's 29 and 21 respectively. Given
 + * that the Normal DMA endpoints (other than SDRAM) can be used as tx/rx,
 + * we need 78 vchans in total
 + */
 +#define NDMA_NR_MAX_CHANNELS 8
 +#define DDMA_NR_MAX_CHANNELS 8
 +#define DMA_NR_MAX_CHANNELS  (NDMA_NR_MAX_CHANNELS + DDMA_NR_MAX_CHANNELS)
 +#define NDMA_NR_MAX_VCHANS   (29 * 2 - 1)
 +#define DDMA_NR_MAX_VCHANS   21
 +#define DMA_NR_MAX_VCHANS(NDMA_NR_MAX_VCHANS + DDMA_NR_MAX_VCHANS)
 +
 +/* This set of DDMA timing parameters were found experimentally while
 + * working with the SPI driver and seem to make it behave correctly */
 +#define DDMA_MAGIC_SPI_PARAMETERS(DDMA_PARA_DEST_DATA_BLK_SIZE(1) | \
 + DDMA_PARA_SRC_DATA_BLK_SIZE(1) | \
 + DDMA_PARA_DEST_WAIT_CYCLES(2) | \
 + DDMA_PARA_SRC_WAIT_CYCLES(2))
This should ideally be configurable from SPI

 +
 +struct sun4i_dma_pchan {
 + /* Register base of channel */
 + void __iomem*base;
 + /* vchan currently being serviced */
 + struct sun4i_dma_vchan  *vchan;
 + /* Is this a dedicated pchan? */
 + int is_dedicated;
 +};
nitpick, it would be better if you follow kerneldoc for this struct comments
or add to same line. This is bit non intuitive here, or use empty lines


 +
 +static struct device *chan2dev(struct dma_chan *chan)
 +{
 + return chan-dev-device;
 +}
This should be in core..


 +static int __execute_vchan_pending(struct sun4i_dma_dev *priv,
 +struct sun4i_dma_vchan *vchan)
 +{
 + struct sun4i_dma_promise *promise = NULL;
 + struct sun4i_dma_contract *contract = NULL;
 + struct sun4i_dma_pchan *pchan;
 + struct virt_dma_desc *vd;
 + int ret;
 +
 + lockdep_assert_held(vchan-vc.lock);
 +
 + /* We need a pchan to do anything, so secure one if available */
 + pchan = find_and_use_pchan(priv, vchan);
 + if (!pchan)
 + return -EBUSY;
 +
 + /*
 +  * Channel endpoints must not be repeated, so if this vchan
 +  * has already submitted some work, we can't do anything else
 +  */
and shouldn't the search take care of this for you...
 +static struct sun4i_dma_promise *
 +generate_ndma_promise(struct dma_chan *chan, dma_addr_t src, dma_addr_t dest,
 +   size_t len, struct dma_slave_config *sconfig)
 +{
 + struct sun4i_dma_promise *promise;
 + int ret;
 +
 + promise = kzalloc(sizeof(*promise), GFP_NOWAIT);
 + if 

[linux-sunxi] Re: [PATCH 4/6] dmaengine: sun6i: Add support for Allwinner H3 (sun8i) variant

[Vinod Koul]

On Wed, May 06, 2015 at 12:13:42PM +0200, Maxime Ripard wrote:
 On Wed, May 06, 2015 at 11:31:31AM +0200, Jens Kuske wrote:
  The H3 SoC has the same dma engine as the A31 (sun6i), with a
  reduced amount of endpoints and physical channels. Add the proper
  config data and compatible string to support it.
  
  Signed-off-by: Jens Kuske jensku...@gmail.com

This looks fine to me, I think can be merged now. Do you guys want the
mainatainers to pick up patches to their subsystem or merge them tgether,
though don't see any dependency though

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 4/6] dmaengine: sun6i: Add support for Allwinner H3 (sun8i) variant

[Vinod Koul]

On Wed, May 06, 2015 at 11:31:31AM +0200, Jens Kuske wrote:
 The H3 SoC has the same dma engine as the A31 (sun6i), with a
 reduced amount of endpoints and physical channels. Add the proper
 config data and compatible string to support it.
Applied, thanks

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v6] dma: sun4i: Add support for the DMA engine on sun[457]i SoCs

[Vinod Koul]

On Mon, May 11, 2015 at 03:27:32PM +0200, Maxime Ripard wrote:
 +
 +/** Normal DMA register values **/
 +
 +/* Normal DMA source/destination data request type values */
 +#define NDMA_DRQ_TYPE_SDRAM  0x16
 +#define NDMA_DRQ_TYPE_LIMIT  (0x1F + 1)
 +
 +/** Normal DMA register layout **/
 +
 +/* Normal DMA configuration register layout */
 +#define NDMA_CFG_LOADING BIT(31)
 +#define NDMA_CFG_CONT_MODE   BIT(30)
 +#define NDMA_CFG_WAIT_STATE(n)   ((n)  27)
 +#define NDMA_CFG_DEST_DATA_WIDTH(width)  ((width)  25)
 +#define NDMA_CFG_DEST_BURST_LENGTH(len)  ((len)  23)
 +#define NDMA_CFG_DEST_NON_SECURE BIT(22)
 +#define NDMA_CFG_DEST_FIXED_ADDR BIT(21)
 +#define NDMA_CFG_DEST_DRQ_TYPE(type) ((type)  16)
 +#define NDMA_CFG_BYTE_COUNT_MODE_REMAIN  BIT(15)
 +#define NDMA_CFG_SRC_DATA_WIDTH(width)   ((width)  9)
 +#define NDMA_CFG_SRC_BURST_LENGTH(len)   ((len)  7)
 +#define NDMA_CFG_SRC_NON_SECURE  BIT(6)
 +#define NDMA_CFG_SRC_FIXED_ADDR  BIT(5)
 +#define NDMA_CFG_SRC_DRQ_TYPE(type)  ((type)  0)
 +
 +/** Dedicated DMA register values **/
 +
 +/* Dedicated DMA source/destination address mode values */
 +#define DDMA_ADDR_MODE_LINEAR0
 +#define DDMA_ADDR_MODE_IO1
 +#define DDMA_ADDR_MODE_HORIZONTAL_PAGE   2
 +#define DDMA_ADDR_MODE_VERTICAL_PAGE 3
 +
 +/* Dedicated DMA source/destination data request type values */
 +#define DDMA_DRQ_TYPE_SDRAM  0x1
 +#define DDMA_DRQ_TYPE_LIMIT  (0x1F + 1)
 +
 +/** Dedicated DMA register layout **/
 +
 +/* Dedicated DMA configuration register layout */
 +#define DDMA_CFG_LOADING BIT(31)
 +#define DDMA_CFG_BUSYBIT(30)
 +#define DDMA_CFG_CONT_MODE   BIT(29)
 +#define DDMA_CFG_DEST_NON_SECURE BIT(28)
 +#define DDMA_CFG_DEST_DATA_WIDTH(width)  ((width)  25)
 +#define DDMA_CFG_DEST_BURST_LENGTH(len)  ((len)  23)
 +#define DDMA_CFG_DEST_ADDR_MODE(mode)((mode)  21)
 +#define DDMA_CFG_DEST_DRQ_TYPE(type) ((type)  16)
 +#define DDMA_CFG_BYTE_COUNT_MODE_REMAIN  BIT(15)
 +#define DDMA_CFG_SRC_NON_SECURE  BIT(12)
 +#define DDMA_CFG_SRC_DATA_WIDTH(width)   ((width)  9)
 +#define DDMA_CFG_SRC_BURST_LENGTH(len)   ((len)  7)
 +#define DDMA_CFG_SRC_ADDR_MODE(mode) ((mode)  5)
 +#define DDMA_CFG_SRC_DRQ_TYPE(type)  ((type)  0)
 +
 +/* Dedicated DMA parameter register layout */
 +#define DDMA_PARA_DEST_DATA_BLK_SIZE(n)  (((n) - 1)  24)
 +#define DDMA_PARA_DEST_WAIT_CYCLES(n)(((n) - 1)  16)
 +#define DDMA_PARA_SRC_DATA_BLK_SIZE(n)   (((n) - 1)  8)
 +#define DDMA_PARA_SRC_WAIT_CYCLES(n) (((n) - 1)  0)
 +
 +/** DMA register offsets **/
 +
 +/* General register offsets */
 +#define DMA_IRQ_ENABLE_REG   0x0
 +#define DMA_IRQ_PENDING_STATUS_REG   0x4
 +
 +/* Normal DMA register offsets */
 +#define NDMA_CHANNEL_REG_BASE(n) (0x100 + (n) * 0x20)
 +#define NDMA_CFG_REG 0x0
 +#define NDMA_SRC_ADDR_REG0x4
 +#define NDMA_DEST_ADDR_REG   0x8
 +#define NDMA_BYTE_COUNT_REG  0xC
 +
 +/* Dedicated DMA register offsets */
 +#define DDMA_CHANNEL_REG_BASE(n) (0x300 + (n) * 0x20)
 +#define DDMA_CFG_REG 0x0
 +#define DDMA_SRC_ADDR_REG0x4
 +#define DDMA_DEST_ADDR_REG   0x8
 +#define DDMA_BYTE_COUNT_REG  0xC
 +#define DDMA_PARA_REG0x18
the defines here NDMA_, DDMA_, DMA_ are too generic names and not specific to
this driver, they can cause collisions in future with others, so lets make
them future proof please

 +static int choose_optimal_buswidth(dma_addr_t addr)
 +{
 + /* On 32 bit aligned addresses, we can use a 32 bit bus width */
 + if (addr % 4 == 0)
 + return DMA_SLAVE_BUSWIDTH_4_BYTES;
 + /* On 16 bit aligned addresses, we can use a 16 bit bus width */
 + else if (addr % 2 == 0)
 + return DMA_SLAVE_BUSWIDTH_2_BYTES;
 +
 + /* Worst-case scenario, we need to do byte aligned reads */
 + return DMA_SLAVE_BUSWIDTH_1_BYTE;
am not sure if that is right way, dmaengine drivers shouldn't assume and use
a parameter as they will conflict with client FIFO settings and make whole
data garbage

 +static struct sun4i_dma_promise *
 +generate_ndma_promise(struct dma_chan *chan, dma_addr_t src, dma_addr_t dest,
 +   size_t len, struct dma_slave_config *sconfig)
 +{
 + struct sun4i_dma_promise *promise;
 + int ret;
 +
 + promise = kzalloc(sizeof(*promise), GFP_NOWAIT);
 

[linux-sunxi] Re: [PATCH v7] dma: sun4i: Add support for the DMA engine on sun[457]i SoCs

[Vinod Koul]

On Mon, Jun 08, 2015 at 11:12:25PM +0200, Maxime Ripard wrote:

 +
 +static int sanitize_config(struct dma_slave_config *sconfig)
 +{
 + switch (sconfig-direction) {
all was fine but this part is not :( the dma_slave_config direction is
deprecated, so here you should honour the direction passed in _prepare call
and use that

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v6] dma: sun4i: Add support for the DMA engine on sun[457]i SoCs

[Vinod Koul]

On Thu, May 21, 2015 at 09:47:35AM +0200, Maxime Ripard wrote:
 On Mon, May 18, 2015 at 02:16:14PM +0530, Vinod Koul wrote:
   +static enum dma_status sun4i_dma_tx_status(struct dma_chan *chan,
   +dma_cookie_t cookie,
   +struct dma_tx_state *state)
   +{
   + struct sun4i_dma_vchan *vchan = to_sun4i_dma_vchan(chan);
   + struct sun4i_dma_pchan *pchan = vchan-pchan;
   + struct sun4i_dma_contract *contract;
   + struct sun4i_dma_promise *promise;
   + struct virt_dma_desc *vd;
   + unsigned long flags;
   + enum dma_status ret;
   + size_t bytes = 0;
   +
   + ret = dma_cookie_status(chan, cookie, state);
   + if (ret == DMA_COMPLETE)
   + return ret;
 
  Pls check if state is valid before progressing ahead
 
 Just one more question about this one. What should we return in the
 case where state would be NULL ? DMA_ERROR?
whatever dma_cookie_status returns

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v6] dma: sun4i: Add support for the DMA engine on sun[457]i SoCs

[Vinod Koul]

On Thu, May 21, 2015 at 01:45:27PM -0300, Emilio López wrote:
 Hi Maxime, Vinod,
 
 El 20/05/15 a las 18:17, Maxime Ripard escibió:
 +static struct dma_async_tx_descriptor *
 +sun4i_dma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf, size_t 
 len,
 +size_t period_len, enum dma_transfer_direction dir,
 +unsigned long flags)
 +{
 +  struct sun4i_dma_vchan *vchan = to_sun4i_dma_vchan(chan);
 +  struct dma_slave_config *sconfig = vchan-cfg;
 +  struct sun4i_dma_promise *promise;
 +  struct sun4i_dma_contract *contract;
 +  dma_addr_t src, dest;
 +  u32 endpoints;
 +  int nr_periods, offset, plength, i;
 +
 +  if (!is_slave_direction(dir)) {
 +  dev_err(chan2dev(chan), Invalid DMA direction\n);
 +  return NULL;
 +  }
 +
 +  if (vchan-is_dedicated) {
 +  /*
 +   * As we are using this just for audio data, we need to use
 +   * normal DMA. There is nothing stopping us from supporting
 +   * dedicated DMA here as well, so if a client comes up and
 +   * requires it, it will be simple to implement it.
 +   */
 +  dev_err(chan2dev(chan),
 +  Cyclic transfers are only supported on Normal DMA\n);
 +  return NULL;
 +  }
 +
 +  contract = generate_dma_contract();
 +  if (!contract)
 +  return NULL;
 +
 +  contract-is_cyclic = 1;
 +
 +  /* Figure out the endpoints and the address we need */
 +  if (dir == DMA_MEM_TO_DEV) {
 +  src = buf;
 +  dest = sconfig-dst_addr;
 +  endpoints = NDMA_CFG_SRC_DRQ_TYPE(NDMA_DRQ_TYPE_SDRAM) |
 +  NDMA_CFG_DEST_DRQ_TYPE(vchan-endpoint) |
 +  NDMA_CFG_DEST_FIXED_ADDR;
 +  } else {
 +  src = sconfig-src_addr;
 +  dest = buf;
 +  endpoints = NDMA_CFG_SRC_DRQ_TYPE(vchan-endpoint) |
 +  NDMA_CFG_SRC_FIXED_ADDR |
 +  NDMA_CFG_DEST_DRQ_TYPE(NDMA_DRQ_TYPE_SDRAM);
 +  }
 +
 +  /*
 +   * We will be using half done interrupts to make two periods
 +   * out of a promise, so we need to program the DMA engine less
 +   * often
 +   */
 +  nr_periods = DIV_ROUND_UP(len / period_len, 2);
 
 and why is that.. why don't you use actual period count here?
 
 I must admit I don't really know on this one.
 
 Emilio?
 
 You mean why is it that I chose to divide len / period_len (is
 there some other way to get period count that I'm missing?) by 2 and
 use half done interrupts? The engine can interrupt on half-transfer,
 so we can use this feature to program the engine half as often as if
 we didn't use it (keep in mind the hardware doesn't support linked
 lists).
 
 Say you have a set of periods (| marks the start/end, I for
 interrupt, P for programming the engine to do a new transfer), the
 easy but slow way would be to do
 
  |---|---|---|---| (periods / promises)
  P  I,P I,P I,P  I
 
 Using half transfer interrupts you can do
 
  |---|---| (promises as configured on hw)
  |---|---|---|---| (periods)
  P   I  I,P  I   I
 
 Which requires half the engine programming for the same functionality.
 
 Feel free to include these drawings on the comment if you think
 they'll help.
That explains it then, please do add this bit to driver documentation

 
 +static struct dma_async_tx_descriptor *
 +sun4i_dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 +  unsigned int sg_len, enum dma_transfer_direction dir,
 +  unsigned long flags, void *context)
 +{
 +  struct sun4i_dma_vchan *vchan = to_sun4i_dma_vchan(chan);
 +  struct dma_slave_config *sconfig = vchan-cfg;
 +  struct sun4i_dma_promise *promise;
 +  struct sun4i_dma_contract *contract;
 +  struct scatterlist *sg;
 +  dma_addr_t srcaddr, dstaddr;
 +  u32 endpoints, para;
 +  int i;
 +
 +  if (!sgl)
 +  return NULL;
 +
 +  if (!is_slave_direction(dir)) {
 +  dev_err(chan2dev(chan), Invalid DMA direction\n);
 +  return NULL;
 +  }
 +
 +  contract = generate_dma_contract();
 +  if (!contract)
 +  return NULL;
 +
 +  /* Figure out endpoints */
 +  if (vchan-is_dedicated  dir == DMA_MEM_TO_DEV) {
 +  endpoints = DDMA_CFG_SRC_DRQ_TYPE(DDMA_DRQ_TYPE_SDRAM) |
 +  DDMA_CFG_SRC_ADDR_MODE(DDMA_ADDR_MODE_LINEAR) |
 +  DDMA_CFG_DEST_DRQ_TYPE(vchan-endpoint) |
 +  DDMA_CFG_DEST_ADDR_MODE(DDMA_ADDR_MODE_IO);
 +  } else if (!vchan-is_dedicated  dir == DMA_MEM_TO_DEV) {
 +  endpoints = NDMA_CFG_SRC_DRQ_TYPE(NDMA_DRQ_TYPE_SDRAM) |
 +  NDMA_CFG_DEST_DRQ_TYPE(vchan-endpoint) |
 +  NDMA_CFG_DEST_FIXED_ADDR;
 +  } else if (vchan-is_dedicated) {
 +  endpoints = DDMA_CFG_SRC_DRQ_TYPE(vchan-endpoint) |
 +  DDMA_CFG_SRC_ADDR_MODE(DDMA_ADDR_MODE_IO) |
 +  DDMA_CFG_DEST_DRQ_TYPE(DDMA_DRQ_TYPE_SDRAM) |
 +  

[linux-sunxi] Re: [PATCH] dma: sun4i: expose block size and wait cycle configuration to DMA users

[Vinod Koul]

On Fri, Mar 11, 2016 at 10:40:55AM +0100, Boris Brezillon wrote:
> On Fri, 11 Mar 2016 11:54:52 +0530
> Vinod Koul <vinod.k...@intel.com> wrote:
> 
> > On Wed, Mar 09, 2016 at 11:14:34AM +0100, Boris Brezillon wrote:
> > > > > > > > + * struct sun4i_dma_chan_config - DMA channel config
> > > > > > > > + *
> > > > > > > > + * @para: contains information about block size and time 
> > > > > > > > before checking
> > > > > > > > + *   DRQ line. This is device specific and only applicable 
> > > > > > > > to dedicated
> > > > > > > > + *   DMA channels
> > > > > > > 
> > > > > > > What information, can you elobrate.. And why can't you use 
> > > > > > > existing
> > > > > > > dma_slave_config for this?
> > > > > > 
> > > > > > Block size is related to the device FIFO size. I guess it allows the
> > > > > > DMA channel to launch a transfer of X bytes without having to check 
> > > > > > the
> > > > > > DRQ line (the line telling the DMA engine it can transfer more data
> > > > > > to/from the device). The wait cycles information is apparently 
> > > > > > related
> > > > > > to the number of clks the engine should wait before polling/checking
> > > > > > the DRQ line status between each block transfer. I'm not sure what 
> > > > > > it
> > > > > > saves to put WAIT_CYCLES() to something != 1, but in their BSP,
> > > > > > Allwinner tweak that depending on the device.
> > > > 
> > > > we already have block size aka src/dst_maxburst, why not use that one.
> > > 
> > > Okay, but then remains the question "how should we choose the real burst
> > > size?". The block size described in Allwinner datasheet is not the
> > > number of words you will transmit without being preempted by other
> > > master -> slave requests, it's the number of bytes that can be
> > > transmitted without checking the DRQ line.
> > > IOW, block_size = burst_size * X
> > 
> > Thats fine, API expects words for this and also a width value. Client shoudl
> > pass both and for programming you should use bytes converted from words and
> > width.
> > 
> 
> Not sure I get what you mean. Are you suggesting to add new fields to
> the dma_slave_config struct to describe this block concept, or should

No

> we pass it through ->xxx_burstsize, and try to guess the real burstsize?

Pass the real burstsize in words

> In the latter case, you still haven't answered my question: how should
> we choose the burstsize?

>From word value convert to bytes and program HW

burst(in bytes) = burst (in words ) * buswidth;

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH] dma: sun4i: expose block size and wait cycle configuration to DMA users

[Vinod Koul]

On Fri, Mar 11, 2016 at 10:45:52AM +0100, Boris Brezillon wrote:
> On Fri, 11 Mar 2016 11:56:07 +0530
> Vinod Koul <vinod.k...@intel.com> wrote:
> 
> > On Wed, Mar 09, 2016 at 12:06:27PM +0100, Boris Brezillon wrote:
> > > On Tue, 8 Mar 2016 08:25:47 +0530
> > > Vinod Koul <vinod.k...@intel.com> wrote:
> > > > 
> > > > Why does dmaengine need to wait? Can you explain that
> > > 
> > > I don't have an answer for that one, but when I set WAIT_CYCLES to 1
> > > for the NAND use case it does not work. So I guess it is somehow
> > > related to how the DRQ line is controlled on the device side...
> > 
> > Is the WAIT cycle different for different usages or same for all
> > usages/channels?
> > 
> 
> In Allwinner BSP they adapt it on a per slave device basis, but since
> DMA channels are dynamically allocated, you can't know in advance which
> physical channel will be attached to a specific device.

And we have the correct values availble in datasheet for all usages

> Another option I considered was adding a new cell to the sun4i DT
> binding to encode these WAIT_CYCLES and BLOCK_SIZE information. But I'm
> not sure adding that to the DT is a good idea (not to mention that it
> would break DT ABI again, and given the last discussions on this topic,
> I'm not sure it's a good idea :-/).

Yes i was veering towards DT as well. This is a new property so ABI rules
wont break as long as driver still works with old properties.

But this nees to be property for clients and not driver. Client can then
program these

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH] dma: sun4i: expose block size and wait cycle configuration to DMA users

[Vinod Koul]

On Fri, Mar 11, 2016 at 11:55:49AM +0100, Maxime Ripard wrote:
> On Fri, Mar 11, 2016 at 03:39:02PM +0530, Vinod Koul wrote:
> > On Fri, Mar 11, 2016 at 10:45:52AM +0100, Boris Brezillon wrote:
> > > On Fri, 11 Mar 2016 11:56:07 +0530
> > > Vinod Koul <vinod.k...@intel.com> wrote:
> > > 
> > > > On Wed, Mar 09, 2016 at 12:06:27PM +0100, Boris Brezillon wrote:
> > > > > On Tue, 8 Mar 2016 08:25:47 +0530
> > > > > Vinod Koul <vinod.k...@intel.com> wrote:
> > > > > > 
> > > > > > Why does dmaengine need to wait? Can you explain that
> > > > > 
> > > > > I don't have an answer for that one, but when I set WAIT_CYCLES to 1
> > > > > for the NAND use case it does not work. So I guess it is somehow
> > > > > related to how the DRQ line is controlled on the device side...
> > > > 
> > > > Is the WAIT cycle different for different usages or same for all
> > > > usages/channels?
> > > > 
> > > 
> > > In Allwinner BSP they adapt it on a per slave device basis, but since
> > > DMA channels are dynamically allocated, you can't know in advance which
> > > physical channel will be attached to a specific device.
> > 
> > And we have the correct values availble in datasheet for all usages
> 
> No, we don't.
> 
> If you look at the datasheet in question, page 169.
> https://github.com/allwinner-zh/documents/blob/master/A20/A20_User_Manual_v1.4_20150510.pdf
> 
> This is the only documentation we have. And as you can see, it is very
> sparse (and that's an understament).
> 
> So we cannot make that assumption, so far the values have been found
> through trial and error for the devices in question.
> 
> > > Another option I considered was adding a new cell to the sun4i DT
> > > binding to encode these WAIT_CYCLES and BLOCK_SIZE information. But I'm
> > > not sure adding that to the DT is a good idea (not to mention that it
> > > would break DT ABI again, and given the last discussions on this topic,
> > > I'm not sure it's a good idea :-/).
> > 
> > Yes i was veering towards DT as well. This is a new property so ABI rules
> > wont break as long as driver still works with old properties.
> 
> Yeah, we can always default to our current hardcoded value if the
> property is missing. And since no-one is using the engine at the
> moment anyway, so it's not really a big deal.
> 
> > But this nees to be property for clients and not driver. Client can then
> > program these
> 
> Yes, totally. The question here is how the clients give that
> information to the driver.

For this part am not worried. If we can generalize this then we add to
dma_slave_config. Otherwise an exported symbol from driver should be fine.


-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: Digital signature

[linux-sunxi] Re: [PATCH] dma: sun4i: expose block size and wait cycle configuration to DMA users

[Vinod Koul]

On Fri, Mar 11, 2016 at 11:26:31AM +0100, Boris Brezillon wrote:
> On Fri, 11 Mar 2016 15:36:07 +0530
> Vinod Koul <vinod.k...@intel.com> wrote:
> 
> > On Fri, Mar 11, 2016 at 10:40:55AM +0100, Boris Brezillon wrote:
> > > On Fri, 11 Mar 2016 11:54:52 +0530
> > > Vinod Koul <vinod.k...@intel.com> wrote:
> > > 
> > > > On Wed, Mar 09, 2016 at 11:14:34AM +0100, Boris Brezillon wrote:
> > > > > > > > > > + * struct sun4i_dma_chan_config - DMA channel config
> > > > > > > > > > + *
> > > > > > > > > > + * @para: contains information about block size and time 
> > > > > > > > > > before checking
> > > > > > > > > > + *   DRQ line. This is device specific and only applicable 
> > > > > > > > > > to dedicated
> > > > > > > > > > + *   DMA channels
> > > > > > > > > 
> > > > > > > > > What information, can you elobrate.. And why can't you use 
> > > > > > > > > existing
> > > > > > > > > dma_slave_config for this?
> > > > > > > > 
> > > > > > > > Block size is related to the device FIFO size. I guess it 
> > > > > > > > allows the
> > > > > > > > DMA channel to launch a transfer of X bytes without having to 
> > > > > > > > check the
> > > > > > > > DRQ line (the line telling the DMA engine it can transfer more 
> > > > > > > > data
> > > > > > > > to/from the device). The wait cycles information is apparently 
> > > > > > > > related
> > > > > > > > to the number of clks the engine should wait before 
> > > > > > > > polling/checking
> > > > > > > > the DRQ line status between each block transfer. I'm not sure 
> > > > > > > > what it
> > > > > > > > saves to put WAIT_CYCLES() to something != 1, but in their BSP,
> > > > > > > > Allwinner tweak that depending on the device.
> > > > > > 
> > > > > > we already have block size aka src/dst_maxburst, why not use that 
> > > > > > one.
> > > > > 
> > > > > Okay, but then remains the question "how should we choose the real 
> > > > > burst
> > > > > size?". The block size described in Allwinner datasheet is not the
> > > > > number of words you will transmit without being preempted by other
> > > > > master -> slave requests, it's the number of bytes that can be
> > > > > transmitted without checking the DRQ line.
> > > > > IOW, block_size = burst_size * X
> > > > 
> > > > Thats fine, API expects words for this and also a width value. Client 
> > > > shoudl
> > > > pass both and for programming you should use bytes converted from words 
> > > > and
> > > > width.
> > > > 
> > > 
> > > Not sure I get what you mean. Are you suggesting to add new fields to
> > > the dma_slave_config struct to describe this block concept, or should
> > 
> > No
> > 
> > > we pass it through ->xxx_burstsize, and try to guess the real burstsize?
> > 
> > Pass the real burstsize in words
> > 
> > > In the latter case, you still haven't answered my question: how should
> > > we choose the burstsize?
> > 
> > From word value convert to bytes and program HW
> > 
> > burst(in bytes) = burst (in words ) * buswidth;
> > 
> 
> 
> Except, as already explained, the blocksize and burstsize concepts are
> not exactly the same, and the sunxi engine expect both to be defined.
> So let's take a real example to illustrate my question:
> 
> For the NAND use case, here is my DMA channel setup:
> 
> buswidth (or wordsize) = 4 bytes
> burstsize = 4 words (32 bytes)
> blocksize = 128 bytes
> 
> Here, you can see that blocksize = 4 * burstsize, and again, burstsize
> and blocksize are not encoding the same thing. So, assuming we use
> ->src/dst_burstsize to encode the blocksize in our case, how should we
> deduce the real burstsize (which still needs to be configured in the
> engine).

Oh, i was somehow under the impression they are same! Then we can't use
blocksize here, pls pass burst and width properly.

How is block size calculated?

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH] dma: sun4i: expose block size and wait cycle configuration to DMA users

[Vinod Koul]

On Mon, Mar 14, 2016 at 12:46:41PM +0100, Maxime Ripard wrote:
> On Fri, Mar 11, 2016 at 04:48:26PM +0530, Vinod Koul wrote:
> > > > But this nees to be property for clients and not driver. Client can then
> > > > program these
> > > 
> > > Yes, totally. The question here is how the clients give that
> > > information to the driver.
> > 
> > For this part am not worried. If we can generalize this then we add to
> > dma_slave_config. Otherwise an exported symbol from driver should be fine.
> 
> It's actually what we would like to avoid.
> 
> We have two potential provider driver that would need such an
> interface, and we have customer drivers that would be able to use any
> of these two, depending on which SoCs we're talking about.
> 
> Maintaining some logic in each and every driver in that case to know
> which one of this symbol is to be called seems counterproductive and
> painful.

You didn't specify which one you want to avoid, and my guess is latter
choice and not former :)

As I said, if it's something we can use in few examples and describe
generically I do not mind adding to dma_slave_config

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: Digital signature

[linux-sunxi] Re: [PATCH] dma: sun4i: expose block size and wait cycle configuration to DMA users

[Vinod Koul]

On Wed, Mar 09, 2016 at 11:14:34AM +0100, Boris Brezillon wrote:
> > > > > > + * struct sun4i_dma_chan_config - DMA channel config
> > > > > > + *
> > > > > > + * @para: contains information about block size and time before 
> > > > > > checking
> > > > > > + *   DRQ line. This is device specific and only applicable to 
> > > > > > dedicated
> > > > > > + *   DMA channels
> > > > > 
> > > > > What information, can you elobrate.. And why can't you use existing
> > > > > dma_slave_config for this?
> > > > 
> > > > Block size is related to the device FIFO size. I guess it allows the
> > > > DMA channel to launch a transfer of X bytes without having to check the
> > > > DRQ line (the line telling the DMA engine it can transfer more data
> > > > to/from the device). The wait cycles information is apparently related
> > > > to the number of clks the engine should wait before polling/checking
> > > > the DRQ line status between each block transfer. I'm not sure what it
> > > > saves to put WAIT_CYCLES() to something != 1, but in their BSP,
> > > > Allwinner tweak that depending on the device.
> > 
> > we already have block size aka src/dst_maxburst, why not use that one.
> 
> Okay, but then remains the question "how should we choose the real burst
> size?". The block size described in Allwinner datasheet is not the
> number of words you will transmit without being preempted by other
> master -> slave requests, it's the number of bytes that can be
> transmitted without checking the DRQ line.
> IOW, block_size = burst_size * X

Thats fine, API expects words for this and also a width value. Client shoudl
pass both and for programming you should use bytes converted from words and
width.

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH] dma: sun4i: expose block size and wait cycle configuration to DMA users

[Vinod Koul]

On Wed, Mar 09, 2016 at 12:06:27PM +0100, Boris Brezillon wrote:
> On Tue, 8 Mar 2016 08:25:47 +0530
> Vinod Koul <vinod.k...@intel.com> wrote:
> > 
> > Why does dmaengine need to wait? Can you explain that
> 
> I don't have an answer for that one, but when I set WAIT_CYCLES to 1
> for the NAND use case it does not work. So I guess it is somehow
> related to how the DRQ line is controlled on the device side...

Is the WAIT cycle different for different usages or same for all
usages/channels?

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH] dma: sun4i: expose block size and wait cycle configuration to DMA users

[Vinod Koul]

On Mon, Mar 07, 2016 at 09:30:24PM +0100, Maxime Ripard wrote:
> On Mon, Mar 07, 2016 at 04:08:57PM +0100, Boris Brezillon wrote:
> > Hi Vinod,
> > 
> > On Mon, 7 Mar 2016 20:24:29 +0530
> > Vinod Koul <vinod.k...@intel.com> wrote:
> > 
> > > On Mon, Mar 07, 2016 at 10:59:31AM +0100, Boris Brezillon wrote:
> > > > +/* Dedicated DMA parameter register layout */
> > > > +#define SUN4I_DDMA_PARA_DST_DATA_BLK_SIZE(n)   (((n) - 1) << 24)
> > > > +#define SUN4I_DDMA_PARA_DST_WAIT_CYCLES(n) (((n) - 1) << 16)
> > > > +#define SUN4I_DDMA_PARA_SRC_DATA_BLK_SIZE(n)   (((n) - 1) << 8)
> > > > +#define SUN4I_DDMA_PARA_SRC_WAIT_CYCLES(n) (((n) - 1) << 0)
> > > > +
> > > > +/**
> > > > + * struct sun4i_dma_chan_config - DMA channel config
> > > > + *
> > > > + * @para: contains information about block size and time before 
> > > > checking
> > > > + *   DRQ line. This is device specific and only applicable to 
> > > > dedicated
> > > > + *   DMA channels
> > > 
> > > What information, can you elobrate.. And why can't you use existing
> > > dma_slave_config for this?
> > 
> > Block size is related to the device FIFO size. I guess it allows the
> > DMA channel to launch a transfer of X bytes without having to check the
> > DRQ line (the line telling the DMA engine it can transfer more data
> > to/from the device). The wait cycles information is apparently related
> > to the number of clks the engine should wait before polling/checking
> > the DRQ line status between each block transfer. I'm not sure what it
> > saves to put WAIT_CYCLES() to something != 1, but in their BSP,
> > Allwinner tweak that depending on the device.

we already have block size aka src/dst_maxburst, why not use that one.

Why does dmaengine need to wait? Can you explain that

> > Note that I'd be happy if the above configuration could go into the
> > generic dma_slave_config struct. This way we could avoid per-engine
> > specific APIs.
> 
> And I'd really like to avoid that too. That will avoid to cripple the
> consumer drivers that might be using any of the two.

If it is fairly genric property we should add, otherwise yes we don't want
that

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: Digital signature

[linux-sunxi] Re: [PATCH] dma: sun4i: expose block size and wait cycle configuration to DMA users

[Vinod Koul]

On Tue, Mar 08, 2016 at 08:25:47AM +0530, Vinod Koul wrote:
> On Mon, Mar 07, 2016 at 09:30:24PM +0100, Maxime Ripard wrote:
> > On Mon, Mar 07, 2016 at 04:08:57PM +0100, Boris Brezillon wrote:

Also just noticed the subsystem name on this is not correct, pls fix that in
subsequent posting

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: Digital signature

[linux-sunxi] Re: [PATCH] dma: sun4i: expose block size and wait cycle configuration to DMA users

[Vinod Koul]

On Tue, Mar 08, 2016 at 09:46:25AM +0100, Boris Brezillon wrote:
> On Tue, 8 Mar 2016 08:51:31 +0100
> Maxime Ripard <maxime.rip...@free-electrons.com> wrote:
> 
> > On Tue, Mar 08, 2016 at 08:25:47AM +0530, Vinod Koul wrote:
> > > On Mon, Mar 07, 2016 at 09:30:24PM +0100, Maxime Ripard wrote:
> > > > On Mon, Mar 07, 2016 at 04:08:57PM +0100, Boris Brezillon wrote:
> > > > > Hi Vinod,
> > > > > 
> > > > > On Mon, 7 Mar 2016 20:24:29 +0530
> > > > > Vinod Koul <vinod.k...@intel.com> wrote:
> > > > > 
> > > > > > On Mon, Mar 07, 2016 at 10:59:31AM +0100, Boris Brezillon wrote:
> > > > > > > +/* Dedicated DMA parameter register layout */
> > > > > > > +#define SUN4I_DDMA_PARA_DST_DATA_BLK_SIZE(n) (((n) - 1) << 
> > > > > > > 24)
> > > > > > > +#define SUN4I_DDMA_PARA_DST_WAIT_CYCLES(n)   (((n) - 1) << 
> > > > > > > 16)
> > > > > > > +#define SUN4I_DDMA_PARA_SRC_DATA_BLK_SIZE(n) (((n) - 1) << 8)
> > > > > > > +#define SUN4I_DDMA_PARA_SRC_WAIT_CYCLES(n)   (((n) - 1) << 0)
> > > > > > > +
> > > > > > > +/**
> > > > > > > + * struct sun4i_dma_chan_config - DMA channel config
> > > > > > > + *
> > > > > > > + * @para: contains information about block size and time before 
> > > > > > > checking
> > > > > > > + * DRQ line. This is device specific and only applicable 
> > > > > > > to dedicated
> > > > > > > + * DMA channels
> > > > > > 
> > > > > > What information, can you elobrate.. And why can't you use existing
> > > > > > dma_slave_config for this?
> > > > > 
> > > > > Block size is related to the device FIFO size. I guess it allows the
> > > > > DMA channel to launch a transfer of X bytes without having to check 
> > > > > the
> > > > > DRQ line (the line telling the DMA engine it can transfer more data
> > > > > to/from the device). The wait cycles information is apparently related
> > > > > to the number of clks the engine should wait before polling/checking
> > > > > the DRQ line status between each block transfer. I'm not sure what it
> > > > > saves to put WAIT_CYCLES() to something != 1, but in their BSP,
> > > > > Allwinner tweak that depending on the device.
> > > 
> > > we already have block size aka src/dst_maxburst, why not use that one.
> > 
> > I'm not sure it's really the same thing. The DMA controller also has a
> > burst parameter, that is either 1 byte or 8 bytes, and ends up being
> > different from this one.
> 
> Well, that's what I understood to, but when reading more carefully the
> src/dst_maxburst description, it seems to match the block_size concept
> exposed by the sun4i dmaengine. But how should we choose the real burst
> size then.

maxburst is block size as you describe in this context

> IIRC, in most documentation/datasheets, burst size is referred as the
> maximum number of words (word size depends on the selected width) a
> master is allowed to transfer to a slave through the bus without
> being interrupted by other master requests.
> Am I correct?

maxburst is defined as words not bytes. Word is specfied with the
src/dst_addr_width.

> 
> > 
> > > Why does dmaengine need to wait? Can you explain that
> > 
> > We have no idea, we thought you might have one :)
> 
> Yes, it's really unclear to us why this is needed. There might be some
> kind of contention, or maybe the slave device takes some time to put
> DRQ line to low state, and without these wait_cycles the dmaengine
> would assume some data are still available in the FIFO while there's
> actually no more data to retrieve.
> 
> > 
> > It doesn't really makes sense to us, but it does have a significant
> > impact on the throughput.
> 
> I wouldn't say significant impact, but tweaking those parameters has
> some impact on the performances, and since it's not that complicated to
> implement, I thought it was worth a try, but maybe I'm wrong.

Can you guys check with HW folks and see why it is required, if that is a
possiblity!

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH] dma: sun4i: expose block size and wait cycle configuration to DMA users

[Vinod Koul]

On Tue, Mar 08, 2016 at 09:42:31AM +0100, Hans de Goede wrote:
> 
> 
> I see 2 possible reasons why waiting till checking for drq can help:
> 
> 1) A lot of devices have an internal fifo hooked up to a single mmio data
> register which gets read using the general purpose dma-engine, it allows
> this fifo to fill, and thus do burst transfers
> (We've seen similar issues with the scanout engine for the display which
>  has its own dma engine, and doing larger transfers helps a lot).
> 
> 2) Physical memory on the sunxi SoCs is (often) divided into banks
> with a shared data / address bus doing bank-switches is expensive, so
> this wait cycles may introduce latency which allows a user of another
> bank to complete its RAM accesses before the dma engine forces a
> bank switch, which ends up avoiding a lot of (interleaved) bank switches
> while both try to access a different banj and thus waiting makes things
> (much) faster in the end (again a known problem with the display
> scanout engine).
> 
> 
> 
> Note the differences these kinda tweaks make can be quite dramatic,
> when using a 1920x1080p60 hdmi output on the A10 SoC with a 16 bit
> memory bus (real world worst case scenario), the memory bandwidth
> left for userspace processes (measured through memset) almost doubles
> from 48 MB/s to 85 MB/s, source:
> http://ssvb.github.io/2014/11/11/revisiting-fullhd-x11-desktop-performance-of-the-allwinner-a10.html
> 
> TL;DR: Waiting before starting DMA allows for doing larger burst
> transfers which ends up making things more efficient.
> 
> Given this, I really expect there to be other dma-engines which
> have some option to wait a bit before starting/unpausing a transfer
> instead of starting it as soon as (more) data is available, so I think
> this would make a good addition to dma_slave_config.

I tend to agree but before we do that I would like this hypothesis to be
confirmed :)

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH] dma: sun4i: expose block size and wait cycle configuration to DMA users

[Vinod Koul]

On Tue, Mar 08, 2016 at 08:51:31AM +0100, Maxime Ripard wrote:
> > > > > > + * struct sun4i_dma_chan_config - DMA channel config
> > > > > > + *
> > > > > > + * @para: contains information about block size and time before 
> > > > > > checking
> > > > > > + *   DRQ line. This is device specific and only applicable to 
> > > > > > dedicated
> > > > > > + *   DMA channels
> > > > > 
> > > > > What information, can you elobrate.. And why can't you use existing
> > > > > dma_slave_config for this?
> > > > 
> > > > Block size is related to the device FIFO size. I guess it allows the
> > > > DMA channel to launch a transfer of X bytes without having to check the
> > > > DRQ line (the line telling the DMA engine it can transfer more data
> > > > to/from the device). The wait cycles information is apparently related
> > > > to the number of clks the engine should wait before polling/checking
> > > > the DRQ line status between each block transfer. I'm not sure what it
> > > > saves to put WAIT_CYCLES() to something != 1, but in their BSP,
> > > > Allwinner tweak that depending on the device.
> > 
> > we already have block size aka src/dst_maxburst, why not use that one.
> 
> I'm not sure it's really the same thing. The DMA controller also has a
> burst parameter, that is either 1 byte or 8 bytes, and ends up being
> different from this one.

Nope that is buswidth. maxburst is words which cna be sent to device FIFO.
> 
> > Why does dmaengine need to wait? Can you explain that
> 
> We have no idea, we thought you might have one :)

Well that is hardware dependent. From DMAengine API usage we dont ahve to
wait at all. We should submit next descriptor as soon as possible.

> It doesn't really makes sense to us, but it does have a significant
> impact on the throughput.

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: Digital signature

[linux-sunxi] Re: [PATCH 5/7] mtd: provide helper to prepare buffers for DMA operations

[Vinod Koul]

On Tue, Mar 08, 2016 at 12:15:13PM +0100, Boris Brezillon wrote:
>  
> +#ifdef CONFIG_HAS_DMA

Shouldn't this be CONFIG_DMA_ENGINE as you are preparing these descriptors
for DMA transfer?

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH] dma: sun4i: expose block size and wait cycle configuration to DMA users

[Vinod Koul]

On Mon, Mar 07, 2016 at 10:59:31AM +0100, Boris Brezillon wrote:
> +/* Dedicated DMA parameter register layout */
> +#define SUN4I_DDMA_PARA_DST_DATA_BLK_SIZE(n) (((n) - 1) << 24)
> +#define SUN4I_DDMA_PARA_DST_WAIT_CYCLES(n)   (((n) - 1) << 16)
> +#define SUN4I_DDMA_PARA_SRC_DATA_BLK_SIZE(n) (((n) - 1) << 8)
> +#define SUN4I_DDMA_PARA_SRC_WAIT_CYCLES(n)   (((n) - 1) << 0)
> +
> +/**
> + * struct sun4i_dma_chan_config - DMA channel config
> + *
> + * @para: contains information about block size and time before checking
> + * DRQ line. This is device specific and only applicable to dedicated
> + * DMA channels

What information, can you elobrate.. And why can't you use existing
dma_slave_config for this?

> + */
> +struct sun4i_dma_chan_config {
> + u32 para;
> +};
> +
> +int sun4i_dma_set_chan_config(struct dma_chan *dchan,
> +   const struct sun4i_dma_chan_config *cfg);
> +
> +#endif /* _SUN4I_DMA_H */

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v5 2/3] dmaengine: sun6i: Set default maxburst size and bus width

[Vinod Koul]

On Fri, Apr 22, 2016 at 08:48:40AM +0200, Jean-Francois Moine wrote:
> Some DMA clients, as audio, don't set the maxburst size and bus width
> on the memory side when starting DMA transfers.
> This patch prevents such transfers to be aborted by providing system
> default values to the lacking ones.
> 
> Signed-off-by: Jean-Francois Moine 
> ---
>  drivers/dma/sun6i-dma.c | 23 +++
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index b08245e..821fc4f 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -470,10 +470,25 @@ static int set_config(struct sun6i_dma_dev *sdev,
>  {
>   s8 src_width, dst_width, src_burst, dst_burst;
>  
> - src_burst = convert_burst(sconfig->src_maxburst);
> - src_width = convert_buswidth(sconfig->src_addr_width);
> - dst_burst = convert_burst(sconfig->dst_maxburst);
> - dst_width = convert_buswidth(sconfig->dst_addr_width);
> + if (direction == DMA_MEM_TO_DEV) {
> + src_burst = convert_burst(sconfig->src_maxburst ?
> + sconfig->src_maxburst : 8);
> + src_width = convert_buswidth(sconfig->src_addr_width !=
> + DMA_SLAVE_BUSWIDTH_UNDEFINED ?
> + sconfig->src_addr_width :
> + DMA_SLAVE_BUSWIDTH_4_BYTES);
> + dst_burst = convert_burst(sconfig->dst_maxburst);
> + dst_width = convert_buswidth(sconfig->dst_addr_width);
> + } else {/* DMA_DEV_TO_MEM */

Else can be any other direction, I would prefer we check that. Also swicth
would be better

> + src_burst = convert_burst(sconfig->src_maxburst);
> + src_width = convert_buswidth(sconfig->src_addr_width);
> + dst_burst = convert_burst(sconfig->dst_maxburst ?
> + sconfig->dst_maxburst : 8);
> + dst_width = convert_buswidth(sconfig->dst_addr_width !=
> + DMA_SLAVE_BUSWIDTH_UNDEFINED ?
> + sconfig->dst_addr_width :
> + DMA_SLAVE_BUSWIDTH_4_BYTES);
> + }
>  
>   if (src_burst < 0)
>   return src_burst;

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH RESEND v3 0/2] dmaengine: sun6i: Fixes

[Vinod Koul]

On Fri, Apr 22, 2016 at 08:22:56AM +0200, Jean-Francois Moine wrote:
> This patch series replaces part of the previous series
> 'dmaengine: sun6i: Fixes and upgrade for audio transfers'.
> It contains only fixes for a normal use of the DMA driver.

Applied, thanks


-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v5 3/3] dmaengine: sun6i: Add cyclic capability

[Vinod Koul]

On Fri, Apr 22, 2016 at 08:49:55AM +0200, Jean-Francois Moine wrote:
> DMA cyclic transfers are required by audio streaming.
> 
> Acked-by: Maxime Ripard 
> Signed-off-by: Jean-Francois Moine 
> ---
>  drivers/dma/sun6i-dma.c | 129 
> +---
>  1 file changed, 122 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 821fc4f..f0c4a53 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -146,6 +146,8 @@ struct sun6i_vchan {
>   struct dma_slave_config cfg;
>   struct sun6i_pchan  *phy;
>   u8  port;
> + u8  irq_type;
> + boolcyclic;
>  };
>  
>  struct sun6i_dma_dev {
> @@ -254,6 +256,30 @@ static inline s8 convert_buswidth(enum 
> dma_slave_buswidth addr_width)
>   return addr_width >> 1;
>  }
>  
> +static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
> +{
> + struct sun6i_desc *txd = pchan->desc;
> + struct sun6i_dma_lli *lli;
> + size_t bytes;
> + dma_addr_t pos;
> +
> + pos = readl(pchan->base + DMA_CHAN_LLI_ADDR);
> + bytes = readl(pchan->base + DMA_CHAN_CUR_CNT);
> +
> + if (pos == LLI_LAST_ITEM)
> + return bytes;
> +
> + for (lli = txd->v_lli; lli; lli = lli->v_lli_next) {
> + if (lli->p_lli_next == pos) {
> + for (lli = lli->v_lli_next; lli; lli = lli->v_lli_next)
> + bytes += lli->len;
> + break;
> + }
> + }
> +
> + return bytes;
> +}
> +
>  static void *sun6i_dma_lli_add(struct sun6i_dma_lli *prev,
>  struct sun6i_dma_lli *next,
>  dma_addr_t next_phy,
> @@ -342,8 +368,12 @@ static int sun6i_dma_start_desc(struct sun6i_vchan 
> *vchan)
>   irq_reg = pchan->idx / DMA_IRQ_CHAN_NR;
>   irq_offset = pchan->idx % DMA_IRQ_CHAN_NR;
>  
> + vchan->irq_type = vchan->cyclic ? DMA_IRQ_PKG : DMA_IRQ_QUEUE;
> +
>   irq_val = readl(sdev->base + DMA_IRQ_EN(irq_reg));
> - irq_val |= DMA_IRQ_QUEUE << (irq_offset * DMA_IRQ_CHAN_WIDTH);
> + irq_val &= ~((DMA_IRQ_HALF | DMA_IRQ_PKG | DMA_IRQ_QUEUE) <<
> + (irq_offset * DMA_IRQ_CHAN_WIDTH));
> + irq_val |= vchan->irq_type << (irq_offset * DMA_IRQ_CHAN_WIDTH);
>   writel(irq_val, sdev->base + DMA_IRQ_EN(irq_reg));
>  
>   writel(pchan->desc->p_lli, pchan->base + DMA_CHAN_LLI_ADDR);
> @@ -440,11 +470,12 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void 
> *dev_id)
>   writel(status, sdev->base + DMA_IRQ_STAT(i));
>  
>   for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
> - if (status & DMA_IRQ_QUEUE) {
> - pchan = sdev->pchans + j;
> - vchan = pchan->vchan;
> -
> - if (vchan) {
> + pchan = sdev->pchans + j;
> + vchan = pchan->vchan;
> + if (vchan && (status & vchan->irq_type)) {
> + if (vchan->cyclic) {
> + vchan_cyclic_callback(>desc->vd);
> + } else {
>   spin_lock(>vc.lock);
>   vchan_cookie_complete(>desc->vd);
>   pchan->done = pchan->desc;
> @@ -650,6 +681,78 @@ err_lli_free:
>   return NULL;
>  }
>  
> +static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_cyclic(
> + struct dma_chan *chan,
> + dma_addr_t buf_addr,
> + size_t buf_len,
> + size_t period_len,
> + enum dma_transfer_direction dir,
> + unsigned long flags)
> +{
> + struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
> + struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
> + struct dma_slave_config *sconfig = >cfg;
> + struct sun6i_dma_lli *v_lli, *prev = NULL;
> + struct sun6i_desc *txd;
> + dma_addr_t p_lli;
> + u32 lli_cfg;
> + unsigned int i, periods = buf_len / period_len;
> + int ret;

typically we check if direction is slave or not..

> +
> + ret = set_config(sdev, sconfig, dir, _cfg);
> + if (ret) {
> + dev_err(chan2dev(chan), "Invalid DMA configuration\n");
> + return NULL;
> + }
> +
> + txd = kzalloc(sizeof(*txd), GFP_NOWAIT);
> + if (!txd)
> + return NULL;
> +
> + for (i = 0; i < periods; i++) {
> + v_lli = dma_pool_alloc(sdev->pool, GFP_NOWAIT, _lli);
> + if (!v_lli) {
> + dev_err(sdev->slave.dev, "Failed to alloc lli 
> 

[linux-sunxi] Re: [PATCH v5 1/3] dmaengine: sun6i: Simplify lli setting

[Vinod Koul]

On Fri, Apr 22, 2016 at 08:47:29AM +0200, Jean-Francois Moine wrote:
> Checking the DMA config before setting the lli list avoids to do tests
> inside the setting loop.

Applied, thanks

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v6 0/3] dmaengine: sun6i: Upgrade for audio transfers

[Vinod Koul]

On Thu, Apr 28, 2016 at 05:19:11PM +0200, Jean-Francois Moine wrote:
> This patch series contains most of the remaining changes to
> the sun6i DMA driver for audio streaming (tested in
> a Allwinner H3 - Orange PI 2).
> The lacking patch (Add 4 as a possible burst value for H3) will be
> submitted when a consensus with Maxime Ripard will be found.
> This series is based on the previous patches applied by Vinod Koul.

Applied, thanks

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v4 0/3] dmaengine: sun6i: Upgrade for audio transfers

[Vinod Koul]

On Sat, Apr 02, 2016 at 11:49:13AM +0200, Jean-Francois Moine wrote:
> This patch series contains most of the required changes to
> the sun6i DMA driver for audio streaming (tested in
> a Allwinner H3 - Orange PI 2).
> It is based on the previous series 'dmaengine: sun6i: Fixes'
> (2016-03-18).

The series looks good but had one checkpatch error :(

I dont seem to have these fixes which explains why this fails for me. Can
you please resend the fixes one and rebase this and fix the checkpatch for
this and send me

Thanks
-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 08/10] dmaengine: sun6i: allow build on ARM64 platforms (sun50i)

[Vinod Koul]

On Sun, Jan 29, 2017 at 10:33:29AM +0800, Icenowy Zheng wrote:
> As 64-bit Allwinner H5 SoC has the same DMA engine with H3, the DMA
> driver should be allowed to be built for ARM64, in order to make it work on 
> H5.

Applied, thanks

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 08/10] dmaengine: sun6i: allow build on ARM64 platforms (sun50i)

[Vinod Koul]

On Tue, Jan 31, 2017 at 02:23:55AM +0800, Icenowy Zheng wrote:
> 
> 
> 31.01.2017, 00:41, "Vinod Koul" <vinod.k...@intel.com>:
> > On Sun, Jan 29, 2017 at 10:33:29AM +0800, Icenowy Zheng wrote:
> >>  As 64-bit Allwinner H5 SoC has the same DMA engine with H3, the DMA
> >>  driver should be allowed to be built for ARM64, in order to make it work 
> >> on H5.
> >>
> >>  Signed-off-by: Icenowy Zheng <icen...@aosc.xyz>
> >>  Acked-by: Maxime Ripard <maxime.rip...@free-electrons.com>
> >>  Acked-by: Chen-Yu Tsai <w...@csie.org>
> >>  ---
> >>  Patch introduced between v1 and v2, to satisfy the newly added H3/H5 audio
> >>  codec support.
> >>
> >>   drivers/dma/Kconfig | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>  diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> >>  index 0d6a96ee9fc7..d01d59812cf3 100644
> >>  --- a/drivers/dma/Kconfig
> >>  +++ b/drivers/dma/Kconfig
> >>  @@ -157,7 +157,7 @@ config DMA_SUN4I
> >>
> >>   config DMA_SUN6I
> >>   tristate "Allwinner A31 SoCs DMA support"
> >>  - depends on MACH_SUN6I || MACH_SUN8I || COMPILE_TEST
> >>  + depends on MACH_SUN6I || MACH_SUN8I || (ARM64 && ARCH_SUNXI) || 
> >> COMPILE_TEST
> >
> > Do we really need ARM64 here? also looking at others I wonder why isn't
> > this MACH_SUNXI...?
> 
> You mean directly place "ARCH_SUNXI" here?
> 
> SUN4I/SUN5I/SUN7I do not use DMA_SUN6I, they have different DMA
> controllers.

No my question was different..

We have MACH_SUNxx for 6I and 8I, so why do we have ARCH_SUNXI and if its an
arch SUNXI, X means it can take any value...

This schema looks pretty confusing while reading

Also I had a question on usage of ARM64..

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH] dmaengine: sun6i: Add support for Allwinner A83T (sun8i) variant

[Vinod Koul]

On Sun, Sep 18, 2016 at 09:59:50AM +0200, Jean-Francois Moine wrote:
> The A83T SoC has the same dma engine as the A31 (sun6i), with a reduced
> amount of endpoints and physical channels.

Applied, thanks

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4

[Vinod Koul]

On Tue, Oct 04, 2016 at 03:46:51PM +0200, Jean-Francois Moine wrote:
> On Tue, 4 Oct 2016 14:12:21 +0200
> Thomas Petazzoni  wrote:
> 
> > > > Add the case of a burst of 4 which is handled by the SoC.
> > > > 
> > > > Signed-off-by: Mylčne Josserand 
> > > > ---
> > > >  drivers/dma/sun6i-dma.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > > > index 8346199..0485204 100644
> > > > --- a/drivers/dma/sun6i-dma.c
> > > > +++ b/drivers/dma/sun6i-dma.c
> > > > @@ -240,6 +240,8 @@ static inline s8 convert_burst(u32 maxburst)
> > > > switch (maxburst) {
> > > > case 1:
> > > > return 0;
> > > > +   case 4:
> > > > +   return 1;
> > > > case 8:
> > > > return 2;
> > > > default:
> > > > -- 
> > > > 2.9.3  
> > > 
> > > This patch has already been rejected by Maxime in the threads
> > >   http://www.spinics.net/lists/dmaengine/msg08610.html
> > > and
> > >   http://www.spinics.net/lists/dmaengine/msg08719.html
> > > 
> > > I hope you will find the way he wants for this maxburst to be added.
> > 
> > I was about to reply to Mylene's e-mail, suggesting that she should add
> > a comment in the code (and maybe in the commit log) to explain why this
> > addition is needed, and also that even though the schematics say that
> > value "1" (max burst size of 4 bytes) is reserved, it is in fact
> > incorrect. The Allwinner BSP code is really using this value, and it's
> > the value that makes audio work, so we believe the datasheet is simply
> > incorrect.
> > 
> > We already discussed it with Maxime, so I believe he should agree this
> > time. But I would suggest to have such details explained in the commit
> > log and in a comment in the code.
> 
> Strange. Looking at the datasheets of the A23, A31, A33, A83T and H3
> (these are the SoCs using the DMA sun6i), only the H3 can have 4 as the
> burst size (the doc is unclear for the A31).
> 
> Well, I was submitting for the H3, Mylčne is submitting for the A33.
> So, what about the A23, A31 and A83T?

Since these are device properties, I feel we should move this to DT. That
way any new controller can have any variation based on the mood of hw
designer that day and we can hopefully cope with it :-)

But yes we would need to set the bursts supported in driver and allow above
for supported bursts only.

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4

[Vinod Koul]

On Tue, Nov 01, 2016 at 10:55:13PM +0800, Chen-Yu Tsai wrote:

> >>  * @src_maxburst: the maximum number of words (note: words, as in
> >>  * units of the src_addr_width member, not bytes) that can be sent
> >>  * in one burst to the device. Typically something like half the
> >>  * FIFO depth on I/O peripherals so you don't overflow it. This
> >>  * may or may not be applicable on memory sources.
> >>  * @dst_maxburst: same as src_maxburst but for destination target
> >>  * mutatis mutandis.
> >>
> >> The DMA engine driver should be free to select whatever burst size
> >> that doesn't exceed this. So for max_burst = 4, the driver can select
> >> burst = 4 for controllers that do support it, or burst = 1 for those
> >> that don't, and do more bursts.
> >
> > Nope, the client configures these parameters and dmaengine driver
> > validates and programs
> 
> Shouldn't we just name it "burst_size" then if it's meant to be what
> the client specifically asks for?

Well if for some reason we program lesser than than max it would work
technically. But a larger burst wont work at all, so thats why maxburst is
significant.

> My understanding is that the client configures its own parameters,
> such as the trigger level for the DRQ, like raise DRQ when level < 1/4
> FIFO depth, request maxburst = 1/4 or 1/2 FIFO depth, so as not to
> overrun the FIFO. When the DRQ is raised, the DMA engine will do a
> burst, and after the burst the DRQ would be low again, so the DMA
> engine will wait. So the DMA engine driver should be free to
> program the actual burst size to something less than maxburst, shouldn't
> it?

Yup but not more that max..

> >> This also means we can increase max_burst for the audio codec, as
> >> the FIFO is 64 samples deep for stereo, or 128 samples for mono.
> >
> > Beware that higher bursts means chance of underrun of FIFO. This value
> > is selected with consideration of power and performance required. Lazy
> > allocation would be half of FIFO size..
> 
> You mean underrun if its the source right? So the client setting maxburst
> should take the DRQ trigger level into account for this.

Yes

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v2 00/12] Add support for the audio codec on Allwinner V3s

[Vinod Koul]

On Sun, Mar 05, 2017 at 09:36:57PM +0800, Icenowy Zheng wrote:
> Allwinner V3s features a audio codec with dedicated digital and analog parts,
> like the ones on A23/H3, but much simpler (lack of MIC2, LINE IN and MBIAS).
> 
> Add support for it.
> 
> In order to make the codec usable, DMA support is also added in this series;
> the support of Lichee Pi Zero's dock board is also added here, as it's the
> only board hackable that come with ports connected to the codec.
> 
> Patch 1~3 split out parts that is not available on V3s in the analog codec.
> 
> Patch 4/5 adds support for V3s in analog/digital codec.
> 
> Patch 6 add the gate bit as a common quirk of sun6i-dma driver, as V3s also
> needs it.
> 
> Patch 7 really adds support for V3s in DMA engine.
> 
> Patch 8 restores the inclusion of CCU headers in the DTSI file of V3s, as
> it's removed when merging.
> 
> Patch 9/10/11 adds three parts of V3s: DMA engine, codec support and pinmux
> of mmc1 (used on Lichee Pi Zero dock).

And are these dependent upon rest, if not can you please send them
separately!

> 
> Patch 12 adds support for Lichee Pi Zero dock, with support of mmc1 and
> codec.
> 
> Icenowy Zheng (12):
>   ASoC: sun8i-codec-analog: split out mic2
>   ASoC: sun8i-codec-analog: split out line in
>   ASoC: sun8i-codec-analog: split out mbias
>   ASoC: sun8i-codec-analog: add support for V3s SoC
>   ASoC: sun4i-codec: Add support for V3s codec
>   dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk
>   dmaengine: sun6i: support V3s SoC variant
>   ARM: dts: sun8i: restore the inclusion of ccu headers in V3s DTSI
>   ARM: dts: sun8i: add DMA engine in V3s DTSI
>   ARM: dts: sun8i: add audio codec support into V3s DTSI
>   ARM: dts: sun8i: add pinmux for V3s mmc1
>   ARM: dts: sun8i: add device tree for Lichee Pi Zero with Dock
> 
>  .../devicetree/bindings/dma/sun6i-dma.txt  |   1 +
>  .../devicetree/bindings/sound/sun4i-codec.txt  |  11 +-
>  .../bindings/sound/sun8i-codec-analog.txt  |   1 +
>  arch/arm/boot/dts/Makefile |   3 +-
>  arch/arm/boot/dts/sun8i-v3s-licheepi-zero-dock.dts |  67 +++
>  arch/arm/boot/dts/sun8i-v3s.dtsi   | 103 ++
>  drivers/dma/sun6i-dma.c|  22 ++-
>  sound/soc/sunxi/sun4i-codec.c  |  59 ++
>  sound/soc/sunxi/sun8i-codec-analog.c   | 211 
> +
>  9 files changed, 405 insertions(+), 73 deletions(-)
>  create mode 100644 arch/arm/boot/dts/sun8i-v3s-licheepi-zero-dock.dts
> 
> -- 
> 2.11.1
> 

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 0/9] Add support for the audio codec on Allwinner V3s

[Vinod Koul]

Fixed Arnd email now..

On Wed, May 24, 2017 at 04:10:08PM +0530, Vinod Koul wrote:
> On Wed, May 24, 2017 at 06:05:58PM +0800, Icenowy Zheng wrote:
> > Allwinner V3s features a audio codec with dedicated digital and analog 
> > parts,
> > like the ones on A23/H3, but much simpler (lack of MIC2, LINE IN and MBIAS).
> > 
> > Add support for it.
> > 
> > In order to make the codec usable, DMA support is also added in this series.
> > 
> > Patch 1 split out MBIAS in analog codec driver.
> > 
> > Patch 2 prepared a set of objects that have MIC2 and LINEIN stripped out
> > for V3s.
> > 
> > Patch 3/4 adds support for V3s in analog/digital codec.
> > 
> > Patch 5 add the gate bit as a common quirk of sun6i-dma driver, as V3s also
> > needs it.
> > 
> > Patch 6 really adds support for V3s in DMA engine.
> > 
> > Patch 7/8/9 adds three parts of device tree: DMA engine, codec support
> > and enable the codec for Lichee Pi Zero dock.
> > 
> > Icenowy Zheng (9):
> >   ASoC: sun8i-codec-analog: split out mbias
> >   ASoC: sun8i-codec-analog: prepare a mixer control/widget/route set for
> > V3s
> >   ASoC: sun8i-codec-analog: add support for V3s SoC
> >   ASoC: sun4i-codec: Add support for V3s codec
> >   dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk
> >   dmaengine: sun6i: support V3s SoC variant
> >   ARM: dts: sun8i: add DMA engine in V3s DTSI
> >   ARM: dts: sun8i: add audio codec support into V3s DTSI
> >   ARM: sun8i: v3s: enable audio on Lichee Pi Zero Dock board
> 
> And why do we need the cross tree submission?? I have asked before and was
> met with silence! [1]
> 
> Can you please split it up per subsystem to make it easy for everyone and
> clearly spell out dependencies.
> 
> [1]: https://www.spinics.net/lists/dmaengine/msg12884.html

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 0/9] Add support for the audio codec on Allwinner V3s

[Vinod Koul]

On Wed, May 24, 2017 at 06:05:58PM +0800, Icenowy Zheng wrote:
> Allwinner V3s features a audio codec with dedicated digital and analog parts,
> like the ones on A23/H3, but much simpler (lack of MIC2, LINE IN and MBIAS).
> 
> Add support for it.
> 
> In order to make the codec usable, DMA support is also added in this series.
> 
> Patch 1 split out MBIAS in analog codec driver.
> 
> Patch 2 prepared a set of objects that have MIC2 and LINEIN stripped out
> for V3s.
> 
> Patch 3/4 adds support for V3s in analog/digital codec.
> 
> Patch 5 add the gate bit as a common quirk of sun6i-dma driver, as V3s also
> needs it.
> 
> Patch 6 really adds support for V3s in DMA engine.
> 
> Patch 7/8/9 adds three parts of device tree: DMA engine, codec support
> and enable the codec for Lichee Pi Zero dock.
> 
> Icenowy Zheng (9):
>   ASoC: sun8i-codec-analog: split out mbias
>   ASoC: sun8i-codec-analog: prepare a mixer control/widget/route set for
> V3s
>   ASoC: sun8i-codec-analog: add support for V3s SoC
>   ASoC: sun4i-codec: Add support for V3s codec
>   dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk
>   dmaengine: sun6i: support V3s SoC variant
>   ARM: dts: sun8i: add DMA engine in V3s DTSI
>   ARM: dts: sun8i: add audio codec support into V3s DTSI
>   ARM: sun8i: v3s: enable audio on Lichee Pi Zero Dock board

And why do we need the cross tree submission?? I have asked before and was
met with silence! [1]

Can you please split it up per subsystem to make it easy for everyone and
clearly spell out dependencies.

[1]: https://www.spinics.net/lists/dmaengine/msg12884.html

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk

[Vinod Koul]

On Mon, Jun 05, 2017 at 08:33:47PM +0800, Icenowy Zheng wrote:
> From: Icenowy Zheng 
> 
> Originally we enable a special gate bit when the compatible indicates
> A23/33.
> 
> But according to BSP sources and user manuals, more SoCs will need this
> gate bit.
> 
> So make it a common quirk configured in the config struct.
> 
> Signed-off-by: Icenowy Zheng 
> ---
> Changes since original codec patchset v3:
> - Refactored comments to cover some words found in official documents.
> - Removed the comments when toggling the gate bit.
> 
>  drivers/dma/sun6i-dma.c | 20 +---
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index a2358780ab2c..252b59c1d1d5 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -101,6 +101,17 @@ struct sun6i_dma_config {
>   u32 nr_max_channels;
>   u32 nr_max_requests;
>   u32 nr_max_vchans;
> + /*
> +  * In the datasheets/user manuals of newer Allwinner SoCs, a special
> +  * bit (bit 2 at register 0x20) is present.
> +  * It's named "DMA MCLK interface circuit auto gating bit" in the
> +  * documents, and the footnote of this register says that this bit
> +  * should be set up when initializing the DMA controller.
> +  * Allwinner A23/A33 user manuals do not have this bit documented,
> +  * however these SoCs really have and need this bit, as seen in the
> +  * BSP kernel source code.
> +  */
> + bool gate_needed;

Since this is a hw property, why is this not added as an optional DT
property?

>  };
>  
>  /*
> @@ -1009,6 +1020,7 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
>   .nr_max_channels = 8,
>   .nr_max_requests = 24,
>   .nr_max_vchans   = 37,
> + .gate_needed = true,
>  };
>  
>  static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
> @@ -1174,13 +1186,7 @@ static int sun6i_dma_probe(struct platform_device 
> *pdev)
>   goto err_dma_unregister;
>   }
>  
> - /*
> -  * sun8i variant requires us to toggle a dma gating register,
> -  * as seen in Allwinner's SDK. This register is not documented
> -  * in the A23 user manual.
> -  */
> - if (of_device_is_compatible(pdev->dev.of_node,
> - "allwinner,sun8i-a23-dma"))
> + if (sdc->cfg->gate_needed)
>   writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
>  
>   return 0;
> -- 
> 2.12.2
> 

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH 1/2] dmaengine: sun6i: make gate bit in sun8i's DMA engines a common quirk

[Vinod Koul]

On Wed, Jun 14, 2017 at 04:32:57PM +0800, Icenowy Zheng wrote:
> 
> 
> 于 2017年6月14日 GMT+08:00 下午4:32:52, Vinod Koul <vinod.k...@intel.com> 写到:
> >On Mon, Jun 05, 2017 at 08:33:47PM +0800, Icenowy Zheng wrote:
> >> From: Icenowy Zheng <icen...@aosc.xyz>
> >> 
> >> Originally we enable a special gate bit when the compatible indicates
> >> A23/33.
> >> 
> >> But according to BSP sources and user manuals, more SoCs will need
> >this
> >> gate bit.
> >> 
> >> So make it a common quirk configured in the config struct.
> >> 
> >> Signed-off-by: Icenowy Zheng <icen...@aosc.xyz>
> >> ---
> >> Changes since original codec patchset v3:
> >> - Refactored comments to cover some words found in official
> >documents.
> >> - Removed the comments when toggling the gate bit.
> >> 
> >>  drivers/dma/sun6i-dma.c | 20 +---
> >>  1 file changed, 13 insertions(+), 7 deletions(-)
> >> 
> >> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> >> index a2358780ab2c..252b59c1d1d5 100644
> >> --- a/drivers/dma/sun6i-dma.c
> >> +++ b/drivers/dma/sun6i-dma.c
> >> @@ -101,6 +101,17 @@ struct sun6i_dma_config {
> >>u32 nr_max_channels;
> >>u32 nr_max_requests;
> >>u32 nr_max_vchans;
> >> +  /*
> >> +   * In the datasheets/user manuals of newer Allwinner SoCs, a
> >special
> >> +   * bit (bit 2 at register 0x20) is present.
> >> +   * It's named "DMA MCLK interface circuit auto gating bit" in the
> >> +   * documents, and the footnote of this register says that this bit
> >> +   * should be set up when initializing the DMA controller.
> >> +   * Allwinner A23/A33 user manuals do not have this bit documented,
> >> +   * however these SoCs really have and need this bit, as seen in the
> >> +   * BSP kernel source code.
> >> +   */
> >> +  bool gate_needed;
> >
> >Since this is a hw property, why is this not added as an optional DT
> >property?
> 
> As it's SoC-specified.
> 
> Some SoCs need it, and some don't.

and that is the reason it should be a property

> 
> SoC info is in compatible, so there's no reason to make it a property.

that's why it would need to be optional for the SoC's that needs these..

> 
> >
> >>  };
> >>  
> >>  /*
> >> @@ -1009,6 +1020,7 @@ static struct sun6i_dma_config
> >sun8i_a23_dma_cfg = {
> >>.nr_max_channels = 8,
> >>.nr_max_requests = 24,
> >>.nr_max_vchans   = 37,
> >> +  .gate_needed = true,
> >>  };
> >>  
> >>  static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
> >> @@ -1174,13 +1186,7 @@ static int sun6i_dma_probe(struct
> >platform_device *pdev)
> >>goto err_dma_unregister;
> >>}
> >>  
> >> -  /*
> >> -   * sun8i variant requires us to toggle a dma gating register,
> >> -   * as seen in Allwinner's SDK. This register is not documented
> >> -   * in the A23 user manual.
> >> -   */
> >> -  if (of_device_is_compatible(pdev->dev.of_node,
> >> -  "allwinner,sun8i-a23-dma"))
> >> +  if (sdc->cfg->gate_needed)
> >>writel(SUN8I_DMA_GATE_ENABLE, sdc->base + SUN8I_DMA_GATE);
> >>  
> >>return 0;
> >> -- 
> >> 2.12.2
> >> 

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v4 00/11] dmaengine: sun6i: Fixes for H3/A83T, enable A64

[Vinod Koul]

On Thu, Sep 28, 2017 at 03:49:17AM +0200, Stefan Brüns wrote:
> Commit 3a03ea763a67 ("dmaengine: sun6i: Add support for Allwinner A83T
> (sun8i) variant") and commit f008db8c00c1 ("dmaengine: sun6i: Add support for
> Allwinner H3 (sun8i) variant") added support for the A83T resp. H3, but missed
> some differences between the original A31 and A83T/H3.

Applied 1 thru 8, 7 didn't applied though!

Thanks
-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v4 07/11] dmaengine: sun6i: Retrieve channel count/max request from devicetree

[Vinod Koul]

On Thu, Sep 28, 2017 at 03:49:24AM +0200, Stefan Brüns wrote:
> To avoid introduction of a new compatible for each small SoC/DMA controller
> variation, move the definition of the channel count to the devicetree.
> 
> The number of vchans is no longer explicit, but limited by the highest
> port/DMA request number. The result is a slight overallocation for SoCs
> with a sparse port mapping.

This doesnt apply for me, please check

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v4 07/11] dmaengine: sun6i: Retrieve channel count/max request from devicetree

[Vinod Koul]

On Mon, Oct 16, 2017 at 02:21:02PM +, Brüns, Stefan wrote:
> On Montag, 16. Oktober 2017 09:02:17 CEST Vinod Koul wrote:
> > On Thu, Sep 28, 2017 at 03:49:24AM +0200, Stefan Brüns wrote:
> > > To avoid introduction of a new compatible for each small SoC/DMA
> > > controller
> > > variation, move the definition of the channel count to the devicetree.
> > > 
> > > The number of vchans is no longer explicit, but limited by the highest
> > > port/DMA request number. The result is a slight overallocation for SoCs
> > > with a sparse port mapping.
> > 
> > This doesnt apply for me, please check
> 
> The hunk below fails after https://git.kernel.org/pub/scm/linux/kernel/git/
> vkoul/slave-dma.git/commit/?h=topic/
> sun=8f3b00347bf075fb457f90ce76573615f567e7bc
> , which removed the "const struct of_device_id *device;" declaration.
> 
> 
> > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > index 7fce976a13d8..b5906da2a975 100644
> > --- a/drivers/dma/sun6i-dma.c
> > +++ b/drivers/dma/sun6i-dma.c
> > @@ -42,6 +42,9 @@
> ..
> > @@ -1155,6 +1159,7 @@ MODULE_DEVICE_TABLE(of, sun6i_dma_match);
> >  static int sun6i_dma_probe(struct platform_device *pdev)
> >  {
> > const struct of_device_id *device;
> > +   struct device_node *np = pdev->dev.of_node;
> > struct sun6i_dma_dev *sdc;
> > struct resource *res;
> > int ret, i;
> 
> Can you fix this up, or should I send a new version of this patch?

rebased one please

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH RESEND 0/2] Allwinner V3s DMA support

[Vinod Koul]

On Tue, Aug 29, 2017 at 12:51:25PM +0800, Icenowy Zheng wrote:
> This is a dedicated patchset of Allwinner V3s DMA support, which used
> to be part of the audio codec support patchset.
> 
> It's a derivation of the DMA part of v3 of the codec patchset.

Applied, thanks

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v5 07/11] dmaengine: sun6i: Retrieve channel count/max request from devicetree

[Vinod Koul]

On Tue, Oct 17, 2017 at 01:06:34AM +0200, Stefan Brüns wrote:
> To avoid introduction of a new compatible for each small SoC/DMA controller
> variation, move the definition of the channel count to the devicetree.
> 
> The number of vchans is no longer explicit, but limited by the highest
> port/DMA request number. The result is a slight overallocation for SoCs
> with a sparse port mapping.

Applied, thanks. But ...


>  static int sun6i_dma_probe(struct platform_device *pdev)
>  {
> + struct device_node *np = pdev->dev.of_node;
>   struct sun6i_dma_dev *sdc;
>   struct resource *res;
>   int ret, i;
> @@ -1228,6 +1233,26 @@ static int sun6i_dma_probe(struct platform_device 
> *pdev)
>   sdc->num_vchans = sdc->cfg->nr_max_vchans;
>   sdc->max_request = sdc->cfg->nr_max_requests;
>  
> + ret = of_property_read_u32(np, "dma-channels", >num_pchans);
> + if (ret && !sdc->num_pchans) {
> + dev_err(>dev, "Can't get dma-channels.\n");
> + return ret;
> + }

 ... we should probably use device_read_xxx calls instead of of_xxx

-- 
~Vinod

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.