All of lore.kernel.org
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Huacai Chen <chenhuacai@gmail.com>
Cc: Ralf Baechle <ralf@linux-mips.org>,
	linux-mips@linux-mips.org, linux-kernel@vger.kernel.org,
	Fuxin Zhang <zhangfx@lemote.com>,
	Zhangjin Wu <wuzhangjin@gmail.com>,
	Huacai Chen <chenhc@lemote.com>, Hongliang Tao <taohl@lemote.com>,
	Hua Yan <yanh@lemote.com>
Subject: Re: [PATCH V5 09/18] MIPS: Loongson: Add swiotlb to support big memory (>4GB).
Date: Mon, 13 Aug 2012 13:54:47 -0400	[thread overview]
Message-ID: <20120813175447.GB26088@phenom.dumpdata.com> (raw)
In-Reply-To: <1344677543-22591-10-git-send-email-chenhc@lemote.com>

> +static void *loongson_dma_alloc_coherent(struct device *dev, size_t size,
> +				dma_addr_t *dma_handle, gfp_t gfp, struct dma_attrs *attrs)
> +{
> +	void *ret;
> +
> +	if (dma_alloc_from_coherent(dev, size, dma_handle, &ret))
> +		return ret;
> +
> +	/* ignore region specifiers */
> +	gfp &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
> +
> +#ifdef CONFIG_ZONE_DMA
> +	if (dev == NULL)
> +		gfp |= __GFP_DMA;

When would this happen? dev == NULL?

> +	else if (dev->coherent_dma_mask <= DMA_BIT_MASK(24))
> +		gfp |= __GFP_DMA;
> +	else
> +#endif
> +#ifdef CONFIG_ZONE_DMA32
> +	if (dev->coherent_dma_mask <= DMA_BIT_MASK(32))
> +		gfp |= __GFP_DMA32;
> +	else

Why the 'else'
> +#endif
> +	;

why?
> +	gfp |= __GFP_NORETRY;
> +
> +	ret = swiotlb_alloc_coherent(dev, size, dma_handle, gfp);
> +	mb();

Why the 'mb()' ? Can you just do
	return swiotlb_alloc_coherent(...) 

> +	return ret;
> +}
> +
> +static void loongson_dma_free_coherent(struct device *dev, size_t size,
> +				void *vaddr, dma_addr_t dma_handle, struct dma_attrs *attrs)
> +{
> +	int order = get_order(size);
> +
> +	if (dma_release_from_coherent(dev, order, vaddr))
> +		return;
> +
> +	swiotlb_free_coherent(dev, size, vaddr, dma_handle);
> +}
> +
> +static dma_addr_t loongson_dma_map_page(struct device *dev, struct page *page,
> +				unsigned long offset, size_t size,
> +				enum dma_data_direction dir,
> +				struct dma_attrs *attrs)
> +{
> +	dma_addr_t daddr = swiotlb_map_page(dev, page, offset, size,
> +					dir, attrs);
> +	mb();

Please do 'return swiotlb_map_page(..)'..

But if you are doing that why don't you just set the dma_ops.map_page = swiotlb_map_page
?


> +	return daddr;
> +}
> +
> +static int loongson_dma_map_sg(struct device *dev, struct scatterlist *sg,
> +				int nents, enum dma_data_direction dir,
> +				struct dma_attrs *attrs)
> +{
> +	int r = swiotlb_map_sg_attrs(dev, sg, nents, dir, NULL);
> +	mb();
> +
> +	return r;
> +}
> +
> +static void loongson_dma_sync_single_for_device(struct device *dev,
> +				dma_addr_t dma_handle, size_t size,
> +				enum dma_data_direction dir)
> +{
> +	swiotlb_sync_single_for_device(dev, dma_handle, size, dir);
> +	mb();
> +}
> +
> +static void loongson_dma_sync_sg_for_device(struct device *dev,
> +				struct scatterlist *sg, int nents,
> +				enum dma_data_direction dir)
> +{
> +	swiotlb_sync_sg_for_device(dev, sg, nents, dir);
> +	mb();
> +}
> +

I am not really sure why you have these extra functions, when you could
just modify the dma_ops to point to the swiotlb ones

> +static dma_addr_t loongson_unity_phys_to_dma(struct device *dev, phys_addr_t paddr)
> +{
> +	return (paddr < 0x10000000) ?
> +			(paddr | 0x0000000080000000) : paddr;
> +}
> +
> +static phys_addr_t loongson_unity_dma_to_phys(struct device *dev, dma_addr_t daddr)
> +{
> +	return (daddr < 0x90000000 && daddr >= 0x80000000) ?
> +			(daddr & 0x0fffffff) : daddr;
> +}
> +
> +struct loongson_dma_map_ops {
> +	struct dma_map_ops dma_map_ops;
> +	dma_addr_t (*phys_to_dma)(struct device *dev, phys_addr_t paddr);
> +	phys_addr_t (*dma_to_phys)(struct device *dev, dma_addr_t daddr);
> +};
> +
> +dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
> +{
> +	struct loongson_dma_map_ops *ops = container_of(get_dma_ops(dev),
> +					struct loongson_dma_map_ops, dma_map_ops);
> +
> +	return ops->phys_to_dma(dev, paddr);
> +}
> +
> +phys_addr_t dma_to_phys(struct device *dev, dma_addr_t daddr)
> +{
> +	struct loongson_dma_map_ops *ops = container_of(get_dma_ops(dev),
> +					struct loongson_dma_map_ops, dma_map_ops);
> +
> +	return ops->dma_to_phys(dev, daddr);
> +}
> +
> +static int loongson_dma_set_mask(struct device *dev, u64 mask)
> +{
> +	/* Loongson doesn't support DMA above 32-bit */
> +	if (mask > DMA_BIT_MASK(32))
> +		return -EIO;
> +
> +	*dev->dma_mask = mask;
> +
> +	return 0;
> +}
> +
> +static struct loongson_dma_map_ops loongson_linear_dma_map_ops = {
> +	.dma_map_ops = {
> +		.alloc = loongson_dma_alloc_coherent,
> +		.free = loongson_dma_free_coherent,
> +		.map_page = loongson_dma_map_page,

But why not 'swiotlb_map_page'?

> +		.unmap_page = swiotlb_unmap_page,
> +		.map_sg = loongson_dma_map_sg,
> +		.unmap_sg = swiotlb_unmap_sg_attrs,
> +		.sync_single_for_cpu = swiotlb_sync_single_for_cpu,
> +		.sync_single_for_device = loongson_dma_sync_single_for_device,
> +		.sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
> +		.sync_sg_for_device = loongson_dma_sync_sg_for_device,
> +		.mapping_error = swiotlb_dma_mapping_error,
> +		.dma_supported = swiotlb_dma_supported,
> +		.set_dma_mask = loongson_dma_set_mask
> +	},
> +	.phys_to_dma = loongson_unity_phys_to_dma,
> +	.dma_to_phys = loongson_unity_dma_to_phys

Why do you need these? I am not seeing it being used here by any external code?

> +};
> +
> +void __init plat_swiotlb_setup(void)
> +{
> +	swiotlb_init(1);
> +	mips_dma_map_ops = &loongson_linear_dma_map_ops.dma_map_ops;
> +}
> diff --git a/arch/mips/mm/dma-default.c b/arch/mips/mm/dma-default.c
> index 3fab204..122f4f8 100644
> --- a/arch/mips/mm/dma-default.c
> +++ b/arch/mips/mm/dma-default.c
> @@ -42,6 +42,13 @@ static inline int cpu_is_noncoherent_r10000(struct device *dev)
>  	       current_cpu_type() == CPU_R12000);
>  }
>  
> +static inline int cpu_is_noncoherent_loongson(struct device *dev)
> +{
> +	return !plat_device_is_coherent(dev) &&
> +			(current_cpu_type() == CPU_LOONGSON2 ||
> +			 current_cpu_type() == CPU_LOONGSON3);
> +}
> +
>  static gfp_t massage_gfp_flags(const struct device *dev, gfp_t gfp)
>  {
>  	gfp_t dma_flag;
> @@ -209,7 +216,7 @@ static inline void __dma_sync(struct page *page,
>  static void mips_dma_unmap_page(struct device *dev, dma_addr_t dma_addr,
>  	size_t size, enum dma_data_direction direction, struct dma_attrs *attrs)
>  {
> -	if (cpu_is_noncoherent_r10000(dev))
> +	if (cpu_is_noncoherent_r10000(dev) || cpu_is_noncoherent_loongson(dev))
>  		__dma_sync(dma_addr_to_page(dev, dma_addr),
>  			   dma_addr & ~PAGE_MASK, size, direction);
>  
> @@ -260,7 +267,7 @@ static void mips_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
>  static void mips_dma_sync_single_for_cpu(struct device *dev,
>  	dma_addr_t dma_handle, size_t size, enum dma_data_direction direction)
>  {
> -	if (cpu_is_noncoherent_r10000(dev))
> +	if (cpu_is_noncoherent_r10000(dev) || cpu_is_noncoherent_loongson(dev))
>  		__dma_sync(dma_addr_to_page(dev, dma_handle),
>  			   dma_handle & ~PAGE_MASK, size, direction);
>  }
> @@ -281,7 +288,7 @@ static void mips_dma_sync_sg_for_cpu(struct device *dev,
>  
>  	/* Make sure that gcc doesn't leave the empty loop body.  */
>  	for (i = 0; i < nelems; i++, sg++) {
> -		if (cpu_is_noncoherent_r10000(dev))
> +		if (cpu_is_noncoherent_r10000(dev) || cpu_is_noncoherent_loongson(dev))
>  			__dma_sync(sg_page(sg), sg->offset, sg->length,
>  				   direction);
>  	}
> -- 
> 1.7.7.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

  parent reply	other threads:[~2012-08-13 18:04 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-11  9:32 [PATCH V5 00/16] MIPS: Add Loongson-3 based machines support Huacai Chen
2012-08-11  9:32 ` [PATCH V5 01/18] MIPS: Loongson: Add basic Loongson-3 definition Huacai Chen
2012-08-12  5:51   ` John Crispin
2012-08-15 19:36     ` Ralf Baechle
2012-08-16  0:39       ` Huacai Chen
2012-08-11  9:32 ` [PATCH V5 02/18] MIPS: Loongson: Add basic Loongson-3 CPU support Huacai Chen
2012-08-11  9:32 ` [PATCH V5 03/18] MIPS: Loongson 3: Add Lemote-3A machtypes definition Huacai Chen
2012-08-11  9:32 ` [PATCH V5 04/18] MIPS: Loongson: Make Loongson-3 to use BCD format for RTC Huacai Chen
2012-08-12  6:06   ` John Crispin
2012-08-11  9:32 ` [PATCH V5 05/18] MIPS: Loongson: Add UEFI-like firmware interface support Huacai Chen
2012-08-11  9:32 ` [PATCH V5 06/18] MIPS: Loongson 3: Add HT-linked PCI support Huacai Chen
2012-08-11  9:32 ` [PATCH V5 07/18] MIPS: Loongson 3: Add IRQ init and dispatch support Huacai Chen
2012-08-11  9:32 ` [PATCH V5 08/18] MIPS: Loongson 3: Add serial port support Huacai Chen
2012-08-11  9:32 ` [PATCH V5 09/18] MIPS: Loongson: Add swiotlb to support big memory (>4GB) Huacai Chen
2012-08-12  6:17   ` John Crispin
2012-08-13 17:54   ` Konrad Rzeszutek Wilk [this message]
2012-08-14  2:29     ` Huacai Chen
2012-08-14  5:57     ` Huacai Chen
2012-08-14 16:26       ` David Daney
2012-08-14 16:26         ` David Daney
2012-08-15  2:18     ` Huacai Chen
2012-08-15 20:24     ` Ralf Baechle
2012-08-16  3:19       ` Huacai Chen
2012-08-11  9:32 ` [PATCH V5 10/18] MIPS: Loongson: Add Loongson-3 Kconfig options Huacai Chen
2012-08-12  6:20   ` John Crispin
2012-08-11  9:32 ` [PATCH V5 11/18] drm/radeon: Include swiotlb.h if SWIOTLB configured Huacai Chen
2012-08-11  9:32 ` [PATCH V5 12/18] drm: Handle io prot correctly for MIPS Huacai Chen
2012-08-11  9:32 ` [PATCH V5 13/18] drm: Define SAREA_MAX for Loongson (PageSize = 16KB) Huacai Chen
2012-08-15 21:31   ` Ralf Baechle
2012-08-16  0:43     ` Huacai Chen
2012-08-16  1:58   ` Matt Turner
2012-08-16  1:58     ` Matt Turner
2012-08-16  3:20     ` Huacai Chen
2012-08-11  9:32 ` [PATCH V5 14/18] ALSA: HDA: Make hda sound card usable for Loongson Huacai Chen
2012-08-11  9:32   ` Huacai Chen
2012-08-13  8:00   ` [alsa-devel] " Takashi Iwai
2012-08-13  8:00     ` Takashi Iwai
2012-08-13  8:00     ` [alsa-devel] " Takashi Iwai
2012-08-13  8:22     ` Huacai Chen
2012-08-13  8:22       ` Huacai Chen
2012-08-11  9:32 ` [PATCH V5 15/18] MIPS: Loongson 3: Add Loongson-3 SMP support Huacai Chen
2012-08-11  9:32 ` [PATCH V5 16/18] MIPS: Loongson 3: Add CPU hotplug support Huacai Chen
2012-08-11  9:32 ` [PATCH V5 17/18] MIPS: Fix poweroff failure when HOTPLUG_CPU configured Huacai Chen
2012-08-11  9:32 ` [PATCH V5 18/18] MIPS: Loongson: Add a Loongson-3 default config file Huacai Chen
2012-08-12  6:13 ` [PATCH V5 00/16] MIPS: Add Loongson-3 based machines support John Crispin
2012-08-12  8:10   ` Huacai Chen

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=20120813175447.GB26088@phenom.dumpdata.com \
    --to=konrad.wilk@oracle.com \
    --cc=chenhc@lemote.com \
    --cc=chenhuacai@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=ralf@linux-mips.org \
    --cc=taohl@lemote.com \
    --cc=wuzhangjin@gmail.com \
    --cc=yanh@lemote.com \
    --cc=zhangfx@lemote.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.