All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Earl Chew <echew@ixiacom.com>
Cc: "Hans J. Koch" <hjk@hansjkoch.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Hans J. Koch" <hjk@linutronix.de>
Subject: Re: RFC: UIO  null parent for __uio_register_device and uio_device_name()
Date: Wed, 19 Jan 2011 08:30:05 -0800	[thread overview]
Message-ID: <20110119163005.GA12979@kroah.com> (raw)
In-Reply-To: <4D370654.7020400@ixiacom.com>

On Wed, Jan 19, 2011 at 07:42:12AM -0800, Earl Chew wrote:
> Hans,
> 
> Thanks for the quick response.
> 
> >> o Allow a null parent during uio_register_device. We've had
> >>   situations where there was no convenient parent. We could
> >>   concoct a parent, but it seemed to make sense to allow
> >>   for a null.
> > 
> > Hmm. I saw many UIO drivers over the years but we never had that
> > problem. In the probe() function of your UIO driver you get a
> > suitable device as parameter. Why don't you just use it?
> 
> We build custom hardware, and UIO has served us well by providing
> a way out of kernel space.
> 
> [ I'm calling the driver we write the "UIO client driver" to
>   avoid confusion with the Linux UIO driver. ]
> 
> In some implementations, the HW binds directly to the CPU
> physical address space. In particular, it is not accessed via
> PCI space. Thus there is no probe() function required at the
> UIO client driver.
> 
> The physical address assignments for the device are fixed
> at system design time, and the UIO client driver (for the custom hardware)
> has those addresses built into it.
> 
> No probing is required in this scenario.

Great, then create a platform device for your "hardware device" and you
should be fine, right?

> > Yes, we intentionally defined struct uio_device in uio.c and not in
> > a header file to prevent driver authors from using it ;-)
> 
> Yes. This is a good pattern. I like using it too  ;-)
> 
> >> +const char *uio_device_name(struct uio_info *info)
> >> +{
> >> +       return dev_name(info->uio_dev->dev);
> > 
> > No NULL checks? Note that info->uio_dev can be NULL if uio_register_device()
> > failed...
> 
> Apologies. I've tightened up the code.  See below.
> 
> > The function above returns strings like "uio0", "uio1", and so on. What's
> > the value of that? To identify a certain driver or distinguish several
> > instances of it, the "name" member of struct uio_info should be used.
> > 
> > But maybe I just miss the point. What's your use case?
> 
> This is how our code currently works.
> 
> 1. UIO client driver registers with Linux UIO driver.
> 2. UIO client driver advertises the name of the Linux UIO driver
>    instance to which it is bound (eg "uio2") to our custom
>    application
> 3. Our custom application queries for the name of the driver
>    to use (eg "uio2")
> 4. Our custom application issues open("/dev/uio2") to access hardware.
> 
> Working on the implementation of our new system last night I realise
> that our new configuration runs very lean, and I now do not have the
> use of udev to populate /dev/uio[0-9].

Then what populates your other device nodes?

> This new configuration runs so lean that I don't have access to sysfs.

I really find this hard to believe.  Why would you not configure sysfs
in your system?

> I've asked about including sysfs, but there are good reasons not to
> include it.

What are they?

> This means that /sys/class/uio[0-9]/... will not be available.

Nor will lots of other things that you will end up needing.

> The result is that I need to figure out the major/minor device number in
> order to access the Linux UIO device. I will advertise this through /proc/
> entries specific to the UIO client driver. We can then use this information
> to mknod /dev/uio[0-9].

Heh, you have /proc/ but not /sys/?  Someone needs to move out of the
90's :)

> I've added uio_device_chrdev() alongside uio_device_name() to allow
> us to figure out the coordinates of the Linux UIO device.

Ick, no, please use the sysfs files that are alredy present for such a
thing, don't reinvent the wheel, I will not accept such a change.

thanks,

greg k-h

  reply	other threads:[~2011-01-19 16:30 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-19  3:56 RFC: UIO null parent for __uio_register_device and uio_device_name() Earl Chew
2011-01-19 14:56 ` Hans J. Koch
2011-01-19 15:42   ` Earl Chew
2011-01-19 16:30     ` Greg KH [this message]
2011-01-19 17:07       ` Earl Chew
2011-01-19 17:11         ` Earl Chew
2011-01-19 17:24           ` Greg KH
2011-01-19 17:22         ` Greg KH
2011-01-19 20:52         ` Hans J. Koch
2011-01-19 22:06           ` Earl Chew
2011-01-19 22:41             ` Greg KH

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=20110119163005.GA12979@kroah.com \
    --to=greg@kroah.com \
    --cc=echew@ixiacom.com \
    --cc=hjk@hansjkoch.de \
    --cc=hjk@linutronix.de \
    --cc=linux-kernel@vger.kernel.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 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.