On Thu, May 28, 2020 at 6:52 PM Serge Semin
[off-list ref] wrote:

On Thu, May 28, 2020 at 05:56:30PM +0300, Andy Shevchenko wrote:

quoted

On Wed, May 27, 2020 at 01:50:21AM +0300, Serge Semin wrote:

...

quoted

In principal I agree, one nit below.
If you are okay with it, feel free to add my Rb tag.

quoted

quoted

+   /*
+    * It might be crucial for some devices to have the hardware
+    * accelerated multi-block transfers supported, aka LLPs in DW DMAC
+    * notation. So if LLPs are supported then max_sg_nents is set to
+    * zero which means unlimited number of SG entries can be handled in a
+    * single DMA transaction, otherwise it's just one SG entry.
+    */

quoted

+   caps->max_sg_nents = dwc->nollp;

quoted

To be on the safer side I would explicitly do it like

      if (dwc->nollp)
       /* your nice comment */
       = 1;
      else
       /* Unlimited */
       = 0;

type or content of nollp theoretically can be changed and this will affect maximum segments.

Agree. Though I don't like formatting you suggested. If I add my nice comment
between if-statement and assignment the the former will be look detached from
the if-statement, which seems a bit ugly. So I'd leave the comment above the
whole if-else statement, especially seeing I've already mentioned there about
the unlimited number of SG entries there.

        /*
         * It might be crucial for some devices to have the hardware
         * accelerated multi-block transfers supported, aka LLPs in DW DMAC
         * notation. So if LLPs are supported then max_sg_nents is set to
         * zero which means unlimited number of SG entries can be handled in a
         * single DMA transaction, otherwise it's just one SG entry.
         */
        if (dwc->nollp)
                caps->max_sg_nents = 1;
        else
                caps->max_sg_nents = 0;

Fine with me, thanks!

-- 
With Best Regards,
Andy Shevchenko