mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: Sascha Hauer <sha@pengutronix.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH 11/20] dma: support marking SRAM for coherent DMA use
Date: Mon, 7 Jun 2021 09:40:23 +0200
Message-ID: <38eb100f-1905-ea86-2a2d-630c589e5cc7@pengutronix.de> (raw)
In-Reply-To: <20210607073428.GE26174@pengutronix.de>

Hi,

On 07.06.21 09:34, Sascha Hauer wrote:
> On Mon, May 31, 2021 at 09:38:12AM +0200, Ahmad Fatoum wrote:
>> The RISC-V architecture allows overriding the dma_alloc_coherent and
>> dma_free_coherent. Allow this to be controlled by device tree.
>>
>> Cache-coherent SoCs won't need this, but incoherent ones that have
>> uncached regions can register them here.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> +static void *pool_alloc_coherent(size_t size, dma_addr_t *dma_handle)
>> +{
>> +	struct dma_coherent_pool *pool;
>> +	void *ret = NULL;
>> +
>> +	list_for_each_entry(pool, &pools, list) {
>> +		ret = tlsf_memalign(pool->handle, DMA_ALIGNMENT, size);
>> +		if (!ret)
>> +			continue;
>> +	}
>> +
>> +	BUG_ON(!ret);
> 
> Being out of memory is no bug, no?

It's for dma_alloc_coherent. Other archs use xmemalign and have no handling for
error cases.

> 
>> +
>> +	memset(ret, 0, size);
>> +
>> +	if (dma_handle)
>> +		*dma_handle = (dma_addr_t)ret;
>> +
>> +	pr_debug("alloc(%zu) == %p\n", size, ret);
>> +
>> +	return ret;
>> +}
>> +
>> +static void pool_free_coherent(void *vaddr, dma_addr_t dma_handle, size_t size)
>> +{
>> +	resource_size_t addr = (resource_size_t)vaddr;
>> +	struct dma_coherent_pool *pool;
>> +
>> +	list_for_each_entry(pool, &pools, list) {
>> +		if (pool->resource->start <= addr && addr <= pool->resource->end) {
> 
> Nice :)
> I would have written if (addr >= start && addr <= end), but the way you
> have written it makes it visually clear from the first sight that addr
> should be in that specific range.

Since I posted this series, someone nudged me into a better direction:
Set dma_addr to the cached alias address, which is < 32-bit and return
from alloc_coherent a > 32-bit address within the uncached alias.

As the devices aren't cache coherent, it doesn't matter that the alias they
get is the uncached one.

> 
>> +			tlsf_free(pool->handle, vaddr);
>> +			return;
>> +		}
>> +	}
>> +
>> +	pr_warn("freeing invalid region: %p\n", vaddr);
>> +}
>> +
>> +static const struct dma_coherent_ops pool_ops = {
>> +	.alloc = pool_alloc_coherent,
>> +	.free = pool_free_coherent,
>> +};
>> +
>> +static int compare_pool_sizes(struct list_head *_a, struct list_head *_b)
>> +{
>> +	struct dma_coherent_pool *a = list_entry(_a, struct dma_coherent_pool, list);
>> +	struct dma_coherent_pool *b = list_entry(_b, struct dma_coherent_pool, list);
>> +
>> +	if (resource_size(a->resource) > resource_size(b->resource))
>> +		return 1;
>> +	if (resource_size(a->resource) < resource_size(b->resource))
>> +		return -1;
>> +	return 0;
>> +}
>> +
>> +static int dma_declare_coherent_pool(const struct resource *res)
>> +{
>> +	struct dma_coherent_pool *pool;
>> +	tlsf_t handle;
>> +
>> +	handle = tlsf_create_with_pool((void *)res->start, resource_size(res));
>> +	if (!handle)
>> +		return -EINVAL;
>> +
>> +	pool = xmalloc(sizeof(*pool));
> 
> Better xzalloc()? It's too easy to add some element to a structure and
> assume that it's initialized.
> 
>> +	pool->handle = handle;
>> +	pool->resource = res;
>> +
>> +	list_add_sort(&pool->list, &pools, compare_pool_sizes);
> 
> The pools are sorted by their size, but is this a good criterion for the
> pools priority?

The idea was to have some fixed order, so issues are easier to debug.
With the changes described above, this commit can be replaced.
(The dma_set_ops one before will remain).

Cheers,
Ahmad

> 
> Sascha
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


  reply	other threads:[~2021-06-07  7:42 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-31  7:38 [PATCH 00/20] RISC-V: prepare for BeagleV pre-production board support Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 01/20] RISC-V: socs: add Kconfig entry for StarFive JH7100 Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 02/20] net: designware: add support for IP integrated into StarFive SoC Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 03/20] mfd: add TI TPS65086 PMIC restart driver Ahmad Fatoum
2021-06-07  6:44   ` Sascha Hauer
2021-05-31  7:38 ` [PATCH 04/20] mtd: spi-nor: cadence: fix 64-bit issues Ahmad Fatoum
2021-06-07  6:51   ` Sascha Hauer
2021-05-31  7:38 ` [PATCH 05/20] nvmem: add StarFive OTP support Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 06/20] RISC-V: dma: support multiple dma_alloc_coherent backends Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 07/20] RISC-V: support incoherent I-Cache Ahmad Fatoum
2021-05-31  7:40   ` Ahmad Fatoum
2021-06-07  7:33     ` Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 08/20] soc: add support for StarFive JH7100 incoherent interconnect Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 09/20] soc: sifive: l2_cache: enable maximum available cache ways Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 10/20] net: designware: fix 64-bit incompatibilities Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 11/20] dma: support marking SRAM for coherent DMA use Ahmad Fatoum
2021-06-07  7:34   ` Sascha Hauer
2021-06-07  7:40     ` Ahmad Fatoum [this message]
2021-06-07  7:39   ` Sascha Hauer
2021-05-31  7:38 ` [PATCH 12/20] mci: allocate DMA-able memory Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 13/20] mci: allocate sector_buf on demand Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 14/20] dma: allocate 32-byte aligned buffers by default Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 15/20] mci: dw_mmc: enable use on 64-bit CPUs Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 16/20] mci: dw_mmc: match against generic "snps, dw-mshc" compatible Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 17/20] clk: add initial StarFive clock support Ahmad Fatoum
2021-05-31  8:41   ` Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 18/20] reset: add StarFive reset controller driver Ahmad Fatoum
2021-06-07  8:00   ` Sascha Hauer
2021-05-31  7:38 ` [PATCH 19/20] watchdog: add StarFive watchdog driver Ahmad Fatoum
2021-05-31  7:38 ` [PATCH 20/20] hw_random: add driver for RNG on StarFive SoC Ahmad Fatoum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=38eb100f-1905-ea86-2a2d-630c589e5cc7@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=sha@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

mail archive of the barebox mailing list

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lore.barebox.org/barebox/0 barebox/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 barebox barebox/ https://lore.barebox.org/barebox \
		barebox@lists.infradead.org
	public-inbox-index barebox

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git