linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 7/8] uio: bind uio_dmem_genirq via OF
Date: Fri, 15 Jul 2016 17:52:39 +0100	[thread overview]
Message-ID: <20160715165239.GG19840@leverpostej> (raw)
In-Reply-To: <CAALAos9KVS3SveQzGWGhcdxZNPapWnmTwZe+4y2g3LNGP3muDA@mail.gmail.com>

On Fri, Jul 15, 2016 at 09:57:28PM +0530, Anup Patel wrote:
> On Fri, Jul 15, 2016 at 6:58 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Fri, Jul 15, 2016 at 02:34:02PM +0530, Anup Patel wrote:
> >> +static char uio_of_dma_bits_prop[128] = "uio,dma-bits";
> >> +static char uio_of_dmem_count_prop[128] = "uio,number-of-dynamic-regions";
> >> +static char uio_of_dmem_sizes_prop[128] = "uio,dynamic-regions-sizes";
> >
> > What are these proeprties? What is a "dynamic region" in hardware terms?
> 
> "Dynamic region" is DMAble memory alloced using dma_alloc_xxx() APIs
> for the device.
> 
> For DMA-capable devices accessed from user-space, we need DMAble
> memory available to user-space. Sometime such devices are also
> cache-coherent (or IO-coherent) so in such case user-space will need
> cacheable DMAble memory.
> 
> Number of "Dynamic regions" required by UIO dmem device depends
> on the type of HW device hence we describe number of "Dynamic regions"
> and size of "Dynamic regions" via DT attributes.

So this is something akin to the number of DMA channels a device might
use.

Either that's implicit knowledge of the device (if fixed), which doesn't
need to be in the DT, or a real property that should be described
explicitly in the device binding, that has nothing to do with UIO.

In general, a device which you might want to use with UIO may not
require such properties, so this is imposing a requirement on DTBs
purely for the sake of userspace software.

> > Why must they be in the DT, and why are the *property names* arbitrarily
> > overridable as module parameters? What exactly do you expect there to be
> > in a DT?
> 
> They must be in DT for platform devices created using DT probing. The
> number of "Dynamic regions" required by UIO device will depend on the
> nature of device. We cannot have same number and sizes of "Dynamic
> regions" for different UIO dmem devices.

If the devices are so different, they should not be handled by the same
driver that has no knowledge of those differences. Platform VFIO has the
same problem, and requires device-specific reset drivers.

> The UIO dmem driver is a generic driver and not specific to any type of
> HW. In fact, UIO dmem driver is generic enough to access variety of
> platform devices from user-space using UIO framework.
> 
> Based on this Rob had previously suggested to not have any (or enforce
> any) DT bindings for UIO dmem driver.
> 
> Refer, https://lkml.org/lkml/2016/5/18/457

I don't follow. Here Rob stated:

  DT describes h/w. UIO is not a h/w block, so this does not belong in DT. A
  UIO vs. kernel driver is purely a kernel decision which shouldn't
  require a DT change.

  The properties should be part of match data for a compatible string that
  needs them set. Or if they can be defined in a way that is actually a
  property of the h/w, then it would be acceptible. You'd still need to
  define compatible strings that the properties apply to.

Above, you have come up with a default set of property names which you
go looking for, and you expect a class of property, even if you don't
expect specific names. That constitutes a binding, and a poorly-defined
one at that.

Per my reading, Rob's point in https://lkml.org/lkml/2016/5/23/539 was
that providing the *values* as module parameters was fine, not providing
the *property names*.

> Link to original Jan's patchset is https://lkml.org/lkml/2016/5/17/141
> 
> We had two options:
> 1) Get details of "Dynamic regions" via module parameter but this
> would mean using same number and sizes of "Dynamic regions" for
> all UIO dmem devices.
> 2) Pass names of DT attributes used by UIO dmem driver as
> module parameters.
> 
> > DT bindings need documentation, but there was none as part of this series.
> >
> > I do not think this makes sense from a DT perspective.
> 
> As explained above, we are not fixing DT compatible string and
> DT attributes names for UIO dmem driver instead we pass all
> these as module parameters (preferable via kernel args or at
> time of module insertion). Due to this we don't require DT bindings
> document.

If you're looking for properties in the DT, you're assuming a binding of
some sort, regardless of whether you expect particular names.

To be clear, NAK to this as it stands.

Thanks,
Mark.

  reply	other threads:[~2016-07-15 16:52 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-15  9:03 [PATCH 0/8] Cache-coherent DMA access using UIO Anup Patel
2016-07-15  9:03 ` [PATCH 1/8] uio: code style cleanup Anup Patel
2016-07-15  9:03 ` [PATCH 2/8] uio: Add new UIO_MEM_PHYS_CACHE type for mem regions Anup Patel
2016-07-15  9:03 ` [PATCH 3/8] uio: Add new UIO_MEM_DEVICE " Anup Patel
2016-07-15  9:03 ` [PATCH 4/8] Documentation: Update documentation for UIO_MEM_PHYS_CACHE and UIO_MEM_DEVICE Anup Patel
2016-07-15  9:04 ` [PATCH 5/8] uio: fix dmem_region_start computation Anup Patel
2016-07-15 11:32   ` Greg Kroah-Hartman
2016-07-15 12:02     ` Jan Viktorin
2016-07-15  9:04 ` [PATCH 6/8] uio: UIO_IRQ_NONE is a valid option for uioinfo->irq Anup Patel
2016-07-15  9:04 ` [PATCH 7/8] uio: bind uio_dmem_genirq via OF Anup Patel
2016-07-15  9:45   ` Russell King - ARM Linux
2016-07-15 10:47     ` Anup Patel
2016-07-15 13:28   ` Mark Rutland
2016-07-15 16:27     ` Anup Patel
2016-07-15 16:52       ` Mark Rutland [this message]
2016-07-18  3:17         ` Anup Patel
2016-07-15  9:04 ` [PATCH 8/8] uio: Use new memtypes in uio_dmem_genirq Anup Patel

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=20160715165239.GG19840@leverpostej \
    --to=mark.rutland@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).