Hi,

Thanks for having a look.

On Wed, Jun 3, 2020 at 12:52 PM Manivannan Sadhasivam
[off-list ref] wrote:

Individual comments for these enums?

I was expecting this comment , and thought these fields are self explanatory
But if you prefer to have description about it, I would have it in next version.

quoted

+enum owl_dmadesc_offsets {
+      OWL_DMADESC_NEXT_LLI = 0,
+      OWL_DMADESC_SADDR,
+      OWL_DMADESC_DADDR,
+      OWL_DMADESC_FLEN,
+      OWL_DMADESC_SRC_STRIDE,
+      OWL_DMADESC_DST_STRIDE,
+      OWL_DMADESC_CTRLA,
+      OWL_DMADESC_CTRLB,
+      OWL_DMADESC_CONST_NUM,
+      OWL_DMADESC_SIZE
};

/**

@@ -153,7 +144,7 @@ struct owl_dma_lli_hw {
 * @node: node for txd's lli_list
 */

struct owl_dma_lli {
-      struct  owl_dma_lli_hw  hw;
+      u32                     hw[OWL_DMADESC_SIZE];
      dma_addr_t              phys;
      struct list_head        node;
};

@@ -320,6 +311,11 @@ static inline u32 llc_hw_ctrlb(u32 int_ctl)
      return ctl;

}

+static u32 llc_hw_flen(struct owl_dma_lli *lli)
+{
+      return lli->hw[OWL_DMADESC_FLEN] & GENMASK(19, 0);
+}
+
static void owl_dma_free_lli(struct owl_dma *od,
                           struct owl_dma_lli *lli)
{

@@ -351,8 +347,9 @@ static struct owl_dma_lli *owl_dma_add_lli(struct

owl_dma_txd *txd,
              list_add_tail(&next->node, &txd->lli_list);

      if (prev) {
-              prev->hw.next_lli = next->phys;
-              prev->hw.ctrla |= llc_hw_ctrla(OWL_DMA_MODE_LME, 0);
+              prev->hw[OWL_DMADESC_NEXT_LLI] = next->phys;
+              prev->hw[OWL_DMADESC_CTRLA] |=
+                                      llc_hw_ctrla(OWL_DMA_MODE_LME, 0);
      }

      return next;

@@ -365,8 +362,7 @@ static inline int owl_dma_cfg_lli(struct

owl_dma_vchan *vchan,
                                struct dma_slave_config *sconfig,
                                bool is_cyclic)
{
-      struct owl_dma_lli_hw *hw = &lli->hw;
-      u32 mode;
+      u32 mode, ctrlb;

      mode = OWL_DMA_MODE_PW(0);

@@ -407,22 +403,22 @@ static inline int owl_dma_cfg_lli(struct

owl_dma_vchan *vchan,
              return -EINVAL;
      }

-      hw->next_lli = 0; /* One link list by default */
-      hw->saddr = src;
-      hw->daddr = dst;
-
-      hw->fcnt = 1; /* Frame count fixed as 1 */
-      hw->flen = len; /* Max frame length is 1MB */
-      hw->src_stride = 0;
-      hw->dst_stride = 0;
-      hw->ctrla = llc_hw_ctrla(mode,
-                               OWL_DMA_LLC_SAV_LOAD_NEXT |
-                               OWL_DMA_LLC_DAV_LOAD_NEXT);
+      lli->hw[OWL_DMADESC_CTRLA] = llc_hw_ctrla(mode,
+                                                OWL_DMA_LLC_SAV_LOAD_NEXT |
+                                                OWL_DMA_LLC_DAV_LOAD_NEXT);

      if (is_cyclic)
-              hw->ctrlb = llc_hw_ctrlb(OWL_DMA_INTCTL_BLOCK);
+              ctrlb = llc_hw_ctrlb(OWL_DMA_INTCTL_BLOCK);
      else
-              hw->ctrlb = llc_hw_ctrlb(OWL_DMA_INTCTL_SUPER_BLOCK);
+              ctrlb = llc_hw_ctrlb(OWL_DMA_INTCTL_SUPER_BLOCK);
+
+      lli->hw[OWL_DMADESC_NEXT_LLI] = 0;

Again, please preserve the old comments.

Sure, would do it.

quoted

+      lli->hw[OWL_DMADESC_SADDR] = src;
+      lli->hw[OWL_DMADESC_DADDR] = dst;
+      lli->hw[OWL_DMADESC_SRC_STRIDE] = 0;
+      lli->hw[OWL_DMADESC_DST_STRIDE] = 0;
+      lli->hw[OWL_DMADESC_FLEN] = len | FCNT_VAL << 20;

Please explain what you're doing here.

Actually , in next the patch 2/10 there is comment that explains a bit
about it.

        /*
         * S700 put flen and fcnt at offset 0x0c and 0x1c respectively,
         * whereas S900 put flen and fcnt at offset 0x0c.
         */

Shall I add more details to it in the next patch 02/10 ?

Thanks
-Amit.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel