linux-kernel - Re: [PATCH v7 09/12] dmaengine: ti: New driver for K3 UDMA

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] [day] [month] [year] [list]

Message-ID: <20191223112623.GF2536@vkoul-mobl>
Date:   Mon, 23 Dec 2019 16:56:23 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     Peter Ujfalusi <peter.ujfalusi@...com>
Cc:     robh+dt@...nel.org, nm@...com, ssantosh@...nel.org,
        dan.j.williams@...el.com, dmaengine@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, grygorii.strashko@...com,
        lokeshvutla@...com, t-kristo@...com, tony@...mide.com,
        j-keerthy@...com, vigneshr@...com
Subject: Re: [PATCH v7 09/12] dmaengine: ti: New driver for K3 UDMA

On 23-12-19, 10:59, Peter Ujfalusi wrote:

> >> +static void udma_reset_counters(struct udma_chan *uc)
> >> +{
> >> +	u32 val;
> >> +
> >> +	if (uc->tchan) {
> >> +		val = udma_tchanrt_read(uc->tchan, UDMA_TCHAN_RT_BCNT_REG);
> >> +		udma_tchanrt_write(uc->tchan, UDMA_TCHAN_RT_BCNT_REG, val);
> > 
> > so you read back from UDMA_TCHAN_RT_BCNT_REG and write same value to
> > it??
> 
> Yes, that's correct. This is how we can reset it. The counter is
> decremented with the value you have written to the register.

aha, with so many read+write back I would have added a helper.. Not a
big deal though can be updated later

> >> +static struct udma_desc *udma_alloc_tr_desc(struct udma_chan *uc,
> >> +					    size_t tr_size, int tr_count,
> >> +					    enum dma_transfer_direction dir)
> >> +{
> >> +	struct udma_hwdesc *hwdesc;
> >> +	struct cppi5_desc_hdr_t *tr_desc;
> >> +	struct udma_desc *d;
> >> +	u32 reload_count = 0;
> >> +	u32 ring_id;
> >> +
> >> +	switch (tr_size) {
> >> +	case 16:
> >> +	case 32:
> >> +	case 64:
> >> +	case 128:
> >> +		break;
> >> +	default:
> >> +		dev_err(uc->ud->dev, "Unsupported TR size of %zu\n", tr_size);
> >> +		return NULL;
> >> +	}
> >> +
> >> +	/* We have only one descriptor containing multiple TRs */
> >> +	d = kzalloc(sizeof(*d) + sizeof(d->hwdesc[0]), GFP_ATOMIC);
> > 
> > this is invoked from prep_ so should use GFP_NOWAIT, we dont use
> > GFP_ATOMIC :)
> 
> Ok. btw: EDMA and sDMA driver is using GFP_ATOMIC :o

heh, we made sure to document this bit :)

> >> +static int udma_configure_statictr(struct udma_chan *uc, struct udma_desc *d,
> >> +				   enum dma_slave_buswidth dev_width,
> >> +				   u16 elcnt)
> >> +{
> >> +	if (uc->ep_type != PSIL_EP_PDMA_XY)
> >> +		return 0;
> >> +
> >> +	/* Bus width translates to the element size (ES) */
> >> +	switch (dev_width) {
> >> +	case DMA_SLAVE_BUSWIDTH_1_BYTE:
> >> +		d->static_tr.elsize = 0;
> >> +		break;
> >> +	case DMA_SLAVE_BUSWIDTH_2_BYTES:
> >> +		d->static_tr.elsize = 1;
> >> +		break;
> >> +	case DMA_SLAVE_BUSWIDTH_3_BYTES:
> >> +		d->static_tr.elsize = 2;
> >> +		break;
> >> +	case DMA_SLAVE_BUSWIDTH_4_BYTES:
> >> +		d->static_tr.elsize = 3;
> >> +		break;
> >> +	case DMA_SLAVE_BUSWIDTH_8_BYTES:
> >> +		d->static_tr.elsize = 4;
> > 
> > seems like ffs(dev_width) to me?
> 
> Not really:
> ffs(DMA_SLAVE_BUSWIDTH_1_BYTE) = 1
> ffs(DMA_SLAVE_BUSWIDTH_2_BYTES) = 2
> ffs(DMA_SLAVE_BUSWIDTH_3_BYTES) = 1

I missed this!

> ffs(DMA_SLAVE_BUSWIDTH_4_BYTES) = 3
> ffs(DMA_SLAVE_BUSWIDTH_8_BYTES) = 4

Otherwise you are ffs() - 1

-- 
~Vinod