All of lore.kernel.org
 help / color / mirror / Atom feed
From: Antoine Tenart <antoine.tenart@bootlin.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Brian Brooks <brian.brooks@linaro.org>,
	David Miller <davem@davemloft.net>,
	antoine.tenart@bootlin.com, maxime.chevallier@bootlin.com,
	ymarkman@marvell.com, stefanc@marvell.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	bjorn.topel@intel.com, brian.brooks@arm.com
Subject: Re: [PATCH] net: mvpp2: avoid bouncing buffers
Date: Fri, 1 Mar 2019 15:26:02 +0100	[thread overview]
Message-ID: <20190301142602.GC3554@kwain> (raw)
In-Reply-To: <20180827154843.GA25821@infradead.org>

Hi Christoph,

I saw you sent this patch as part of another series back in August, but
it seems it was never applied and I can't find it even in -next. We made
some tests and this patch is helping a lot the PPv2 engine driver in
improving its performances.

Do you plan on re-sending it, or reworking it? Are there current issues
with it that prevent it from being merged upstream? Can we help in any
way?

Thanks!
Antoine

On Mon, Aug 27, 2018 at 08:48:43AM -0700, Christoph Hellwig wrote:
> WE should basically never have dev->dma_mask = &dev->coherent_dma_mask,
> so until that is the case you are doctoring around the symptoms and
> not the problem.
> 
> Does the patch below help your case?
> 
> ----
> From 6294e0e330851ee06e66ab85b348f1d92d375d7a Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@lst.de>
> Date: Mon, 27 Aug 2018 17:23:24 +0200
> Subject: driver core: initialize a default DMA mask for platform device
> 
> We still treat devices without a DMA mask as defaulting to 32-bits for
> both mask, but a few releases ago we've started warning about such
> cases, as they require special cases to work around this sloppyness.
> Add a dma_mask field to struct platform_object so that we can initialize
> the dma_mask pointer in struct device and initialize both masks to
> 32-bits by default.  Architectures can still override this in
> arch_setup_pdev_archdata if needed.
> 
> Note that the code looks a little odd with the various conditionals
> because we have to support platform_device structures that are
> statically allocated.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/base/platform.c         | 15 +++++++++++++--
>  include/linux/platform_device.h |  1 +
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index dff82a3c2caa..baf4b06cf2d9 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -225,6 +225,17 @@ struct platform_object {
>  	char name[];
>  };
>  
> +static void setup_pdev_archdata(struct platform_device *pdev)
> +{
> +	if (!pdev->dev.coherent_dma_mask)
> +		pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> +	if (!pdev->dma_mask)
> +		pdev->dma_mask = DMA_BIT_MASK(32);
> +	if (!pdev->dev.dma_mask)
> +		pdev->dev.dma_mask = &pdev->dma_mask;
> +	arch_setup_pdev_archdata(pdev);
> +};
> +
>  /**
>   * platform_device_put - destroy a platform device
>   * @pdev: platform device to free
> @@ -271,7 +282,7 @@ struct platform_device *platform_device_alloc(const char *name, int id)
>  		pa->pdev.id = id;
>  		device_initialize(&pa->pdev.dev);
>  		pa->pdev.dev.release = platform_device_release;
> -		arch_setup_pdev_archdata(&pa->pdev);
> +		setup_pdev_archdata(&pa->pdev);
>  	}
>  
>  	return pa ? &pa->pdev : NULL;
> @@ -472,7 +483,7 @@ EXPORT_SYMBOL_GPL(platform_device_del);
>  int platform_device_register(struct platform_device *pdev)
>  {
>  	device_initialize(&pdev->dev);
> -	arch_setup_pdev_archdata(pdev);
> +	setup_pdev_archdata(pdev);
>  	return platform_device_add(pdev);
>  }
>  EXPORT_SYMBOL_GPL(platform_device_register);
> diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
> index 1a9f38f27f65..d84ec1de6022 100644
> --- a/include/linux/platform_device.h
> +++ b/include/linux/platform_device.h
> @@ -25,6 +25,7 @@ struct platform_device {
>  	int		id;
>  	bool		id_auto;
>  	struct device	dev;
> +	dma_addr_t	dma_mask;
>  	u32		num_resources;
>  	struct resource	*resource;
>  
> -- 
> 2.18.0
> 

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

  parent reply	other threads:[~2019-03-01 14:26 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-20  2:47 [PATCH] net: mvpp2: avoid bouncing buffers Brian Brooks
2018-08-20  2:55 ` David Miller
2018-08-20  6:23   ` Christoph Hellwig
2018-08-20  7:02     ` Yan Markman
2018-08-27 13:55     ` Brian Brooks
2018-08-27 15:48       ` Christoph Hellwig
2018-08-27 15:48         ` Christoph Hellwig
2018-09-02  2:10         ` Brian Brooks
2019-03-01 14:26         ` Antoine Tenart [this message]
2019-03-11 15:46           ` Christoph Hellwig

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=20190301142602.GC3554@kwain \
    --to=antoine.tenart@bootlin.com \
    --cc=bjorn.topel@intel.com \
    --cc=brian.brooks@arm.com \
    --cc=brian.brooks@linaro.org \
    --cc=davem@davemloft.net \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime.chevallier@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=stefanc@marvell.com \
    --cc=ymarkman@marvell.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.