On Thu, May 28, 2020 at 6:52 PM Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx> wrote: > On Thu, May 28, 2020 at 05:56:30PM +0300, Andy Shevchenko wrote: > > On Wed, May 27, 2020 at 01:50:21AM +0300, Serge Semin wrote: ... > > In principal I agree, one nit below. > > If you are okay with it, feel free to add my Rb tag. > > > + /* > > > + * 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. > > > + */ > > > > > + caps->max_sg_nents = dwc->nollp; > > > > > 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
