All of lore.kernel.org
 help / color / mirror / Atom feed
From: Finn Thain <fthain@telegraphics.com.au>
To: Michael Schmitz <schmitzmic@gmail.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "David S. Miller" <davem@davemloft.net>,
	linux-m68k <linux-m68k@vger.kernel.org>,
	netdev <netdev@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH net] macmace: Set platform device coherent_dma_mask
Date: Fri, 11 May 2018 15:28:58 +1000 (AEST)	[thread overview]
Message-ID: <alpine.LNX.2.21.1805111451220.8@nippy.intranet> (raw)
In-Reply-To: <0b13a08d-ba2d-b9dc-4fd9-1f6bad5cd1ee@gmail.com>

On Fri, 11 May 2018, Michael Schmitz wrote:

> 
> I'm afraid using platform_device_register() (which you already use for 
> the SCC devices) is the only option handling this on a per-device basis 
> without touching platform core code, while at the same time keeping the 
> DMA mask setup out of device drivers

I don't think that will fly. If you call platform_device_register() and 
follow that with a dma mask assignment, you could race with the bus 
matching and driver probe, and we are back to the same WARNING message.

If you want to use platform_device_register(), you'd have to implement 
arch_setup_pdev_archdata() and use that to set up the dma mask.

> (I can see Geert's point there - device driver code might be shared 
> across implementations of the device on platforms with different DMA 
> mask requirements,, something the driver can't be expected to know 
> about).

As I said, these drivers might be expected to be portable between Macs and 
early PowerMacs, but the same dma mask would apply AFAIK.

If a platform driver isn't expected to be portable, I think either method 
is reasonable: arch_setup_pdev_archdata() or the method in the patch.

Anyway, there is this in arch/powerpc/kernel/setup-common.c:

void arch_setup_pdev_archdata(struct platform_device *pdev)
{
        pdev->archdata.dma_mask = DMA_BIT_MASK(32);
        pdev->dev.dma_mask = &pdev->archdata.dma_mask;
	...

I'm inclined to propose something similar for m68k. That should fix the 
problem, since arch_setup_pdev_archdata() is already in the call chain:

	platform_device_register_simple()
		platform_device_register_resndata()
			platform_device_register_full()
				platform_device_alloc()
					arch_setup_pdev_archdata()

Thoughts? Will this have nasty side effects for m68k platforms that use 
smaller dma masks?

-- 

> 
> Cheers,
> 
> 	Michael
> 

WARNING: multiple messages have this Message-ID (diff)
From: Finn Thain <fthain@telegraphics.com.au>
To: Michael Schmitz <schmitzmic@gmail.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "David S. Miller" <davem@davemloft.net>,
	linux-m68k <linux-m68k@lists.linux-m68k.org>,
	netdev <netdev@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH net] macmace: Set platform device coherent_dma_mask
Date: Fri, 11 May 2018 15:28:58 +1000 (AEST)	[thread overview]
Message-ID: <alpine.LNX.2.21.1805111451220.8@nippy.intranet> (raw)
In-Reply-To: <0b13a08d-ba2d-b9dc-4fd9-1f6bad5cd1ee@gmail.com>

On Fri, 11 May 2018, Michael Schmitz wrote:

> 
> I'm afraid using platform_device_register() (which you already use for 
> the SCC devices) is the only option handling this on a per-device basis 
> without touching platform core code, while at the same time keeping the 
> DMA mask setup out of device drivers

I don't think that will fly. If you call platform_device_register() and 
follow that with a dma mask assignment, you could race with the bus 
matching and driver probe, and we are back to the same WARNING message.

If you want to use platform_device_register(), you'd have to implement 
arch_setup_pdev_archdata() and use that to set up the dma mask.

> (I can see Geert's point there - device driver code might be shared 
> across implementations of the device on platforms with different DMA 
> mask requirements,, something the driver can't be expected to know 
> about).

As I said, these drivers might be expected to be portable between Macs and 
early PowerMacs, but the same dma mask would apply AFAIK.

If a platform driver isn't expected to be portable, I think either method 
is reasonable: arch_setup_pdev_archdata() or the method in the patch.

Anyway, there is this in arch/powerpc/kernel/setup-common.c:

void arch_setup_pdev_archdata(struct platform_device *pdev)
{
        pdev->archdata.dma_mask = DMA_BIT_MASK(32);
        pdev->dev.dma_mask = &pdev->archdata.dma_mask;
	...

I'm inclined to propose something similar for m68k. That should fix the 
problem, since arch_setup_pdev_archdata() is already in the call chain:

	platform_device_register_simple()
		platform_device_register_resndata()
			platform_device_register_full()
				platform_device_alloc()
					arch_setup_pdev_archdata()

Thoughts? Will this have nasty side effects for m68k platforms that use 
smaller dma masks?

-- 

> 
> Cheers,
> 
> 	Michael
> 

  reply	other threads:[~2018-05-11  5:29 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <S1751632AbeECEYA/20180503042400Z+254@vger.kernel.org>
2018-05-03  7:25 ` [PATCH net] macmace: Set platform device coherent_dma_mask Geert Uytterhoeven
2018-05-03  7:25   ` Geert Uytterhoeven
2018-05-03  8:38   ` Finn Thain
2018-05-03  8:38     ` Finn Thain
2018-05-03  8:46     ` Geert Uytterhoeven
2018-05-03  8:46       ` Geert Uytterhoeven
2018-05-03  8:51       ` Christoph Hellwig
2018-05-03  8:51         ` Christoph Hellwig
2018-05-03 20:24         ` Michael Schmitz
2018-05-03 20:24           ` Michael Schmitz
2018-05-04  7:24           ` Geert Uytterhoeven
2018-05-04  7:24             ` Geert Uytterhoeven
2018-05-04  8:16             ` Michael Schmitz
2018-05-04  8:16               ` Michael Schmitz
2018-05-10  1:25         ` Finn Thain
2018-05-10  1:25           ` Finn Thain
2018-05-10  1:25       ` Finn Thain
2018-05-10  1:25         ` Finn Thain
2018-05-10 20:27         ` Michael Schmitz
2018-05-10 20:27           ` Michael Schmitz
2018-05-10 23:55           ` Finn Thain
2018-05-10 23:55             ` Finn Thain
2018-05-11  2:11             ` Michael Schmitz
2018-05-11  2:11               ` Michael Schmitz
2018-05-11  3:28               ` Finn Thain
2018-05-11  3:28                 ` Finn Thain
2018-05-11  4:18                 ` Michael Schmitz
2018-05-11  4:18                   ` Michael Schmitz
2018-05-11  5:28                   ` Finn Thain [this message]
2018-05-11  5:28                     ` Finn Thain
2018-05-11  9:30                     ` Michael Schmitz
2018-05-11  9:30                       ` Michael Schmitz
2018-05-11 10:06                       ` Finn Thain
2018-05-11 10:06                         ` Finn Thain
2018-05-11 22:02                         ` Michael Schmitz
2018-05-11 22:02                           ` Michael Schmitz
2018-05-03  4:23 Finn Thain
  -- strict thread matches above, loose matches on Subject: below --
2018-05-03  4:23 Finn Thain
2018-05-03  4:23 Finn Thain
2018-05-03  4:23 Finn Thain
2018-05-03  4:23 Finn Thain

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=alpine.LNX.2.21.1805111451220.8@nippy.intranet \
    --to=fthain@telegraphics.com.au \
    --cc=davem@davemloft.net \
    --cc=geert@linux-m68k.org \
    --cc=hch@lst.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=schmitzmic@gmail.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.