linux-kernel - Re: [PATCH v2 2/2] dmaengine: uniphier-mdmac: add UniPhier MIO DMAC driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 12 Sep 2018 14:25:38 +0900
From:   Masahiro Yamada <yamada.masahiro@...ionext.com>
To:     Vinod <vkoul@...nel.org>
Cc:     "open list:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM" 
        <dmaengine@...r.kernel.org>, DTML <devicetree@...r.kernel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Jassi Brar <jaswinder.singh@...aro.org>,
        Dan Williams <dan.j.williams@...el.com>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 2/2] dmaengine: uniphier-mdmac: add UniPhier MIO DMAC driver

2018-09-12 13:35 GMT+09:00 Vinod <vkoul@...nel.org>:
> On 12-09-18, 12:01, Masahiro Yamada wrote:
>> Hi Vinod,
>>
>>
>> 2018-09-11 16:00 GMT+09:00 Vinod <vkoul@...nel.org>:
>> > On 24-08-18, 10:41, Masahiro Yamada wrote:
>> >
>> >> +/* mc->vc.lock must be held by caller */
>> >> +static u32 __uniphier_mdmac_get_residue(struct uniphier_mdmac_desc *md)
>> >> +{
>> >> +     u32 residue = 0;
>> >> +     int i;
>> >> +
>> >> +     for (i = md->sg_cur; i < md->sg_len; i++)
>> >> +             residue += sg_dma_len(&md->sgl[i]);
>> >
>> > so if the descriptor is submitted to hardware, we return the descriptor
>> > length, which is not correct.
>> >
>> > Two cases are required to be handled:
>> > 1. Descriptor is in queue (IMO above logic is fine for that, but it can
>> > be calculated at descriptor submit and looked up here)
>>
>> Where do you want it to be calculated?
>
> where is it calculated now?


Please see __uniphier_mdmac_handle().


It gets the address and size by sg_dma_address(), sg_dma_len()
just before setting them to the hardware registers.


       sg = &md->sgl[md->sg_cur];

       if (md->dir == DMA_MEM_TO_DEV) {
               src_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_INC;
               src_addr = sg_dma_address(sg);
               dest_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_FIXED;
               dest_addr = 0;
       } else {
               src_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_FIXED;
               src_addr = 0;
               dest_mode = UNIPHIER_MDMAC_CH_MODE__ADDR_INC;
               dest_addr = sg_dma_address(sg);
       }







>> This hardware provides only simple registers (address and size)
>> for one-shot transfer instead of descriptors.
>>
>> So, I used sgl as-is because I did not see a good reason
>> to transform sgl to another data structure.
>
>
>> > this seems missing stuff. Where do you do register calculation for the
>> > descriptor and where is slave_config here, how do you know where to
>> > send/receive data form/to (peripheral)
>>
>>
>> This dmac is really simple, and un-flexible.
>>
>> The peripheral address to send/receive data from/to is hard-weird.
>> cfg->{src_addr,dst_addr} is not configurable.
>>
>> Look at __uniphier_mdmac_handle().
>> 'dest_addr' and 'src_addr' must be set to 0 for the peripheral.
>
> Fair enough, what about other values like addr_width and maxburst?


None of them is configurable.



-- 
Best Regards
Masahiro Yamada