Re: [PATCH v3 1/3] powerpc/512x: add LocalPlus Bus FIFO device driver

On 01.10.2015 20:11, Timur Tabi wrote: > On 09/30/2015 04:24 PM, Alexander Popov wrote: >>> Driver code that has to parse #address-cells or #size-cells >>> is usually wrong. >> >> I would not call it "parsing", I just check whether the dts-file is good. >> Anyway, could you give me a clue how to

Re: [PATCH v3 1/3] powerpc/512x: add LocalPlus Bus FIFO device driver

On 09/30/2015 04:24 PM, Alexander Popov wrote: Can you test for "!cs" here instead? +e = -EFAULT; +goto err_param; +} Unfortunately no: 0 is a valid value for Chip Select. Is it OK to leave it like that? Yes. +lpbfifo.ram_bus_addr = sg_dma_address(); /* For

Re: [PATCH v3 1/3] powerpc/512x: add LocalPlus Bus FIFO device driver

Hello Timur, thanks again for your review. On 25.09.2015 04:01, Timur Tabi wrote: > Alexander Popov wrote: >> + >> +for (i = 0; i < lpbfifo.cs_n; i++) { >> +phys_addr_t cs_start; >> +phys_addr_t cs_end; >> + >> +cs_start = lpbfifo.cs_ranges[i].addr; >> +cs_end

Re: [PATCH v3 1/3] powerpc/512x: add LocalPlus Bus FIFO device driver

On 28.09.2015 16:18, Timur Tabi wrote: > Alexander Popov wrote: >> The only question I have: why calling dma_unmap_single() from within >> a spinlock is a bad practice? > > I don't know, but usually functions that allocate or free memory cannot be > called from within a spinlock. You need to

Re: [PATCH v3 1/3] powerpc/512x: add LocalPlus Bus FIFO device driver

Hello, Timur, thanks a lot for your review. I'll fix all the issues you pointed at and return with v4. The only question I have: why calling dma_unmap_single() from within a spinlock is a bad practice? Best regards, Alexander ___ Linuxppc-dev mailing

Re: [PATCH v3 1/3] powerpc/512x: add LocalPlus Bus FIFO device driver

Alexander Popov wrote: The only question I have: why calling dma_unmap_single() from within a spinlock is a bad practice? I don't know, but usually functions that allocate or free memory cannot be called from within a spinlock. You need to check that. Since the MPC5121 is a single-core

[PATCH v3 1/3] powerpc/512x: add LocalPlus Bus FIFO device driver

This driver for Freescale MPC512x LocalPlus Bus FIFO (called SCLPC in the Reference Manual) allows Direct Memory Access transfers between RAM and peripheral devices on LocalPlus Bus. Signed-off-by: Alexander Popov --- arch/powerpc/configs/mpc512x_defconfig| 1 +

Re: [PATCH v3 1/3] powerpc/512x: add LocalPlus Bus FIFO device driver

Alexander Popov wrote: +struct mpc512x_lpbfifo_request { + phys_addr_t bus_phys; /* physical address of some device on LPB */ Is this a phys_addr_t or a dma_addr_t? It can't be both a physical address and a bus address. + void *ram_virt; /* virtual address of some