How do I choose an arbitrary minor number for my tty device?

How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Arnd and Greg,

I'm working on a TTY driver for a virtual device that we call "byte channels".
This are likes pipes, but they go through an ePAPR hypervisor.

The hypervisor declares byte channels as nodes in a device tree.  Each byte
channel has a unique 32-bit number called a "handle", and this handle is
specified in the node for that device tree.  Applications are expected to scan
the device tree to look for the node they want, and then extract the handle from
that node.

The problem I have is that the handles are, from Linux's perspective, arbitrary
and sparsely assigned.  For example, we could have four byte channels with
handles of 2, 8, 73, and 74.

What I would like is for the minor number for each tty device to be the byte
channel handle.  Or the byte channel could be in the /dev name.  Either way,
applications can figure out which /dev entry to open in order to communicate
with a given byte channel.

Unfortunately, the only way I know how to do this is to create a separate tty
driver instance for each byte channel.  This is because of this code in
tty_register_driver:

	if (!(driver->flags & TTY_DRIVER_DEVPTS_MEM) && driver->num) {
		p = kzalloc(driver->num * 2 * sizeof(void *), GFP_KERNEL);
		if (!p)
			return -ENOMEM;
	}

The 'index' passed to tty_register_device() is added to minor_start to create
the /dev entry, and it's added to name_base to create the name.  If I specify a
very large number for driver->num, then tty_register_driver will create a large
but sparsely-populated array.

Would you be okay with creating a separate driver for each byte channel, or is
there a better way to do what I want?

--
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Wed, Nov 17, 2010 at 03:37:31PM -0600, Timur Tabi wrote:
> Arnd and Greg,
> 
> I'm working on a TTY driver for a virtual device that we call "byte channels".
> This are likes pipes, but they go through an ePAPR hypervisor.

What is ePAPR?

> The hypervisor declares byte channels as nodes in a device tree.  Each byte
> channel has a unique 32-bit number called a "handle", and this handle is
> specified in the node for that device tree.  Applications are expected to scan
> the device tree to look for the node they want, and then extract the handle from
> that node.
> 
> The problem I have is that the handles are, from Linux's perspective, arbitrary
> and sparsely assigned.  For example, we could have four byte channels with
> handles of 2, 8, 73, and 74.
> 
> What I would like is for the minor number for each tty device to be the byte
> channel handle.  Or the byte channel could be in the /dev name.  Either way,
> applications can figure out which /dev entry to open in order to communicate
> with a given byte channel.

Why would you need this mapping?  Just do a first-come-first serve
assignment of tty minor devices like all other subsystems do (usb,
serial, acm, etc.)

sysfs will show the representation between your ePAPR device "handle"
and the tty device minor just fine, as it does today for those other
types of devices.

Bonus being that udev will create a persistant device id for your tty
device based on that handle so you can just open that if you want to, no
need to get the kernel involved in sparse minor mappings at all.

Hope this helps,

greg k-h

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Greg KH wrote:

> What is ePAPR?

It's a specification for an interface between the boot loader and the operating
system.  It's based on the device tree model that exists on PowerPC today.
ePAPR defines a bunch of extensions, including one for hypervisors.  The byte
channel concept is an example of that.

> Why would you need this mapping?  Just do a first-come-first serve
> assignment of tty minor devices like all other subsystems do (usb,
> serial, acm, etc.)

Without some kind of mapping, there's no way for an application to know which
/dev entry to open.  Each byte channel goes to a different

> 
> sysfs will show the representation between your ePAPR device "handle"
> and the tty device minor just fine, as it does today for those other
> types of devices.

I don't see how.  A byte channel node defines several properties, one of which
could be a text string that acts as a label.  So if an application is looking
for the "channel-to-partition-two" byte channel, it can search for that string
in the device tree.  Once it finds the matching node, it can extract the byte
channel handle.

At this point, the application will want to open a /dev entry that corresponds
to that byte channel handle.  This is the piece I'm missing with the tty layer.

If I want to create a regular character device, I can do this:

bc->dev_id = MKDEV(MAJOR(dev_id), MINOR(dev_id) + i);
device_create(ehv_bc_class, NULL, bc->dev_id, bc, "bc%u", bc->handle);

Here, I control the name of the /dev entry via "bc%u".  I want something similar
for tty devices.

> Bonus being that udev will create a persistant device id for your tty
> device based on that handle so you can just open that if you want to, no
> need to get the kernel involved in sparse minor mappings at all.

I'm not sure I understand that.  In order for udev to do this, I need to tell it
what the byte channel handle actually is.  How do I do that using the tty layer?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Wed, Nov 17, 2010 at 04:10:21PM -0600, Timur Tabi wrote:
> Greg KH wrote:
> 
> > What is ePAPR?
> 
> It's a specification for an interface between the boot loader and the operating
> system.  It's based on the device tree model that exists on PowerPC today.
> ePAPR defines a bunch of extensions, including one for hypervisors.  The byte
> channel concept is an example of that.
> 
> > Why would you need this mapping?  Just do a first-come-first serve
> > assignment of tty minor devices like all other subsystems do (usb,
> > serial, acm, etc.)
> 
> Without some kind of mapping, there's no way for an application to know which
> /dev entry to open.  Each byte channel goes to a different
> 
> > 
> > sysfs will show the representation between your ePAPR device "handle"
> > and the tty device minor just fine, as it does today for those other
> > types of devices.
> 
> I don't see how.  A byte channel node defines several properties, one of which
> could be a text string that acts as a label.  So if an application is looking
> for the "channel-to-partition-two" byte channel, it can search for that string
> in the device tree.  Once it finds the matching node, it can extract the byte
> channel handle.
> 
> At this point, the application will want to open a /dev entry that corresponds
> to that byte channel handle.  This is the piece I'm missing with the tty layer.
> 
> If I want to create a regular character device, I can do this:
> 
> bc->dev_id = MKDEV(MAJOR(dev_id), MINOR(dev_id) + i);
> device_create(ehv_bc_class, NULL, bc->dev_id, bc, "bc%u", bc->handle);
> 
> Here, I control the name of the /dev entry via "bc%u".  I want something similar
> for tty devices.

No, you want to have a tty device attached to your "byte channel
device".  That will give you the correct mapping here.  Your tty device
number is sequencial and has nothing in its name to do with your "byte
channel device number" just like ttyS1 has nothing in its pci device id
that it lives on with my multi-port serial card.

> > Bonus being that udev will create a persistant device id for your tty
> > device based on that handle so you can just open that if you want to, no
> > need to get the kernel involved in sparse minor mappings at all.
> 
> I'm not sure I understand that.  In order for udev to do this, I need to tell it
> what the byte channel handle actually is.  How do I do that using the tty layer?

You just create your tty device and assign the parent of it to be your
"byte channel device".  Just like we do for PCI, USB, and all other bus
device types.

I think you are forgetting that your byte channel devices must be
"devices" in the system here, right?  There is a 'struct bus_id" for
your bus that these devices live on.  Then you create a tty device in
your tty driver that attaches to the byte channel that shows up as a tty
device on your bus.

Does that help explain things a bit better?

thanks,

greg k-h

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Greg KH wrote:
> I think you are forgetting that your byte channel devices must be
> "devices" in the system here, right?  There is a 'struct bus_id" for
> your bus that these devices live on.

Do you mean "struct bus_type"?  I don't have any concept of a "bus" in my
driver.  I didn't know I needed to create one, and I'm not sure how, either.

I guess my real problem is that I'm not really sure what I should be doing.  I
already have a plain character driver that creates devices for each byte
channel.  First I call alloc_chrdev_region() to get a started dev_id.  Then I
iterate over the byte channels and call device_create() for each one, with NULL
for the parent.  At the end, I call cdev_init() and cdev_add().

Now I'm trying to register these byte channels as tty devices, because I'd like
to use them for the system console, and not just plain character devices.  My
plan was to just call tty_register_device() instead of device_create(), so I
don't understand where the "bus" comes in.  What bus?

>  Then you create a tty device in
> your tty driver that attaches to the byte channel that shows up as a tty
> device on your bus.

Are you saying that when I call tty_register_device(), the third parameter
should be the return value from the previous call to device_create(), like this:

bc->dev_id = MKDEV(MAJOR(dev_id), MINOR(dev_id) + i);
bc->dev = device_create(ehv_bc_class, NULL, bc->dev_id, bc, "bc%u", bc->handle);
tty_register_device(ehv_bc_driver, MINOR(bc->dev_id), bc->dev);

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Wed, Nov 17, 2010 at 04:42:22PM -0600, Timur Tabi wrote:
> Greg KH wrote:
> > I think you are forgetting that your byte channel devices must be
> > "devices" in the system here, right?  There is a 'struct bus_id" for
> > your bus that these devices live on.
> 
> Do you mean "struct bus_type"?  I don't have any concept of a "bus" in my
> driver.  I didn't know I needed to create one, and I'm not sure how, either.

You need to create one as you really have a "bus" here, as you
described.

> I guess my real problem is that I'm not really sure what I should be doing.  I
> already have a plain character driver that creates devices for each byte
> channel.  First I call alloc_chrdev_region() to get a started dev_id.  Then I
> iterate over the byte channels and call device_create() for each one, with NULL
> for the parent.  At the end, I call cdev_init() and cdev_add().

Ick, no.

Please take a step back and go to your original message where you
described something like "we have a bus and devices we discover on the
bus and they have ids".  Because of that, you should just create a bus,
create the devices on the bus, and then, attach specific types of
drivers to those devices (like your tty driver.)

And your bus is automatically discoverable, right?  So you don't need
any interface for userspace to "create" devices, so you should be fine.

If it's not discoverable, go kick some firmware programmers butt and
make it so, as that is unforgivable in this age.  Seriously, that's flat
out broken and wrong, get it fixed first.

thanks,

greg k-h

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Greg KH wrote:

> You need to create one as you really have a "bus" here, as you
> described.

What about the fact that it's an Open Firmware driver?  My driver gets probed
for each byte channel node in the device tree.

I'm still not sure why you think I have a bus.  I don't see where the UART
drivers register a bus, and there's not a whole lot different between a byte
channel and a UART.

> Please take a step back and go to your original message where you
> described something like "we have a bus and devices we discover on the
> bus and they have ids". 

I never said we have a bus.  There is no entity that conglomerates the byte
channels.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Thu, Nov 18, 2010 at 09:31:17AM -0600, Timur Tabi wrote:
> Greg KH wrote:
> 
> > You need to create one as you really have a "bus" here, as you
> > described.
> 
> What about the fact that it's an Open Firmware driver?  My driver gets probed
> for each byte channel node in the device tree.
> 
> I'm still not sure why you think I have a bus.  I don't see where the UART
> drivers register a bus, and there's not a whole lot different between a byte
> channel and a UART.

But they are obviously two different things, right?

> > Please take a step back and go to your original message where you
> > described something like "we have a bus and devices we discover on the
> > bus and they have ids". 
> 
> I never said we have a bus.  There is no entity that conglomerates the byte
> channels.

Why not?  It sounds like there should be, right?  That would make things
much easier for you in the end from what I can tell.

thanks,

greg k-h

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Greg KH wrote:
>> > 
>> > I'm still not sure why you think I have a bus.  I don't see where the UART
>> > drivers register a bus, and there's not a whole lot different between a byte
>> > channel and a UART.

> But they are obviously two different things, right?

Well, sure.  A byte channel is just a software concept.  You can send data,
receive data, and poll a byte channel.  You can't set the baud rate, or send
break signals or even do flow control.  I debated making the driver act like a
fake serial device, which probably would have been easier but not "correct".

Unfortunately, I seem to lack some fundamental understanding of tty drivers that
is making my life difficult.  I've been reading all the documentation that I can
get my hands on, as well as studying a lot of source code, but I still can't
find any example to base *my* code on.  I just don't know if what I'm doing is
right.

I still don't know how to connect the byte channel handle with the /dev entry.
That's the question my original post asked, and I still don't have an answer to it.

> Why not?  It sounds like there should be, right?  That would make things
> much easier for you in the end from what I can tell.

I'm not so sure.  Like I said, I still don't see where there's a bus.  I have a
single driver that has multiple devices.  It sounds to me like one call to
tty_register_driver() and multiple calls to tty_register_device() would be
sufficient.

For instance, there is no code in drivers/char/ that makes a call to
bus_register(), so I don't see any precedent for a tty driver to register a bus
first.

Also, this is an Open Firmware driver.  I already have a mechanism whereby I get
probed for each instance of a byte channel.  Isn't that my "bus"?

I'm really trying to do the right thing here, Greg, but every time I try to
solve one problem, I'm being told that I need to make things way more
complicated first.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Thu, Nov 18, 2010 at 10:03:12AM -0600, Timur Tabi wrote:
> Greg KH wrote:
> >> > 
> >> > I'm still not sure why you think I have a bus.  I don't see where the UART
> >> > drivers register a bus, and there's not a whole lot different between a byte
> >> > channel and a UART.
> 
> > But they are obviously two different things, right?
> 
> Well, sure.  A byte channel is just a software concept.  You can send data,
> receive data, and poll a byte channel.  You can't set the baud rate, or send
> break signals or even do flow control.  I debated making the driver act like a
> fake serial device, which probably would have been easier but not "correct".
> 
> Unfortunately, I seem to lack some fundamental understanding of tty drivers that
> is making my life difficult.  I've been reading all the documentation that I can
> get my hands on, as well as studying a lot of source code, but I still can't
> find any example to base *my* code on.  I just don't know if what I'm doing is
> right.
> 
> I still don't know how to connect the byte channel handle with the /dev entry.
> That's the question my original post asked, and I still don't have an answer to it.

Ok, I think I'm getting totally confused here.

If all you want is a tty driver, then just write a tty driver.  No need
for character device nodes or any of that other stuff.  Just assign
minor numbers to the device, create it, attach it to the tty core, and
away you go.

I think you are going to have to show code here to get any kind of a
better response out of me as I don't think we are understanding each
other here, sorry.

greg k-h

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Greg KH wrote:
> If all you want is a tty driver, then just write a tty driver.  No need
> for character device nodes or any of that other stuff.  Just assign
> minor numbers to the device, create it, attach it to the tty core, and
> away you go.

What I would like to do is have some control over the name or minor number of
the /dev entry.

> I think you are going to have to show code here to get any kind of a
> better response out of me as I don't think we are understanding each
> other here, sorry.

I'll have to email it to you directly, since it's not authorized for public
distribution just quite yet.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Thu, Nov 18, 2010 at 10:36:59AM -0600, Timur Tabi wrote:
> Greg KH wrote:
> > If all you want is a tty driver, then just write a tty driver.  No need
> > for character device nodes or any of that other stuff.  Just assign
> > minor numbers to the device, create it, attach it to the tty core, and
> > away you go.
> 
> What I would like to do is have some control over the name or minor number of
> the /dev entry.

Why?  Again, it doesn't matter, and no other tty driver does it.

Actually you do have control over it, if you really want it, but again,
don't do that :)

> > I think you are going to have to show code here to get any kind of a
> > better response out of me as I don't think we are understanding each
> > other here, sorry.
> 
> I'll have to email it to you directly, since it's not authorized for public
> distribution just quite yet.

Sorry, I can't do code review, or accept code that is not allowed to be
sent to the public, as, surprise, I'm public :)

So you just sent me an illegal file, great, should I expect to hear from
your lawyers now?

{sigh}

I deal enough with lawyers as it is, please don't do this...

greg k-h

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Greg KH wrote:
> Why?  Again, it doesn't matter, and no other tty driver does it.
> 
> Actually you do have control over it, if you really want it, but again,
> don't do that :)

Because the only way to know which byte channel tty you want is via the byte
channel handle.

Think of the situation where you have two serial ports on the back of your
computer.  One of them is /dev/ttyS0, and the other is /dev/ttyS1.  But which
one is which?  The only way to find out is try one and see if it works.

Now that might be acceptable for serial ports that are fixed physically.  But
byte channels handles are completely arbitrary and easily change with even the
slightest re-configuration of the partitions under the hypervisor.  I need to
have some way to tell userspace that /dev/ttybc0 is byte channel handle 73.

> Sorry, I can't do code review, or accept code that is not allowed to be
> sent to the public, as, surprise, I'm public :)

Well, ok.  I didn't want to spam the mailing list with something that only you
asked for, but here it is:

/* ePAPR hypervisor byte channel device driver
 *
 * Copyright 2009-2010 Freescale Semiconductor, Inc.
 *
 * Author: Timur Tabi <timur@freescale.com>
 *
 * This file is licensed under the terms of the GNU General Public License
 * version 2.  This program is licensed "as is" without any warranty of any
 * kind, whether express or implied.
 *
 * This driver support four distinct interfaces, all of which are related to
 * ePAPR hypervisor byte channels.
 *
 * 1) An early-console (udbg) driver.  This provides early console output
 * through a byte channel.  The byte channel handle must be specified in a
 * Kconfig option.
 *
 * 2) A normal console driver.  Output is sent to the byte channel designated
 * for stdout in the device tree.
 *
 * 3) A tty driver.
 *
 * 4) A byte channel character driver.  This driver creates a /dev/bcXX
 * character device for each byte channel.  The "XX" is the byte channel
 * handle.
 */

#include <linux/module.h>
#include <linux/init.h>
#include <linux/err.h>
#include <linux/interrupt.h>
#include <linux/fs.h>
#include <linux/poll.h>
#include <asm/epapr_hcalls.h>
#include <linux/of.h>
#include <linux/cdev.h>
#include <linux/console.h>
#include <linux/tty.h>
#include <linux/tty_flip.h>
#include <asm/udbg.h>

/* Byte channel handle for stdout (and stdin), taken from device tree */
static unsigned int stdout_bc;

/* Virtual IRQ for the byte channel handle for stdin, taken from device tree */
static unsigned int stdout_irq;

/**************************** SUPPORT FUNCTIONS ****************************/

/*
 * return TRUE if we're running under FSL hypervisor
 *
 * This function checks to see if we're running under the Freescale
 * hypervisor, and returns zero if we're not, or non-zero if we are.
 *
 * First, it checks if MSR[GS]==1, which means we're running under some
 * hypervisor.  Then it checks if there is a hypervisor node in the device
 * tree.  Currently, that means there needs to be a node in the root called
 * "hypervisor" and which has a property named "fsl,hv-version".
 */
static int has_fsl_hypervisor(void)
{
	struct device_node *node;
	int ret;

	if (!(mfmsr() & MSR_GS))
		return 0;

	node = of_find_node_by_path("/hypervisor");
	if (!node)
		return 0;

	ret = of_find_property(node, "fsl,hv-version", NULL) != NULL;

	of_node_put(node);

	return ret;
}

/*
 * find the byte channel handle to use for the console
 *
 * The byte channel to be used for the console is specified via a "stdout"
 * property in the /chosen node.
 *
 * For compatible with legacy device trees, we also look for a "stdout" alias.
 */
static void find_console_handle(void)
{
	struct device_node *np;
	const char *sprop = NULL;
	const uint32_t *iprop;

	np = of_find_node_by_path("/chosen");
	if (np)
		sprop = of_get_property(np, "stdout", NULL);

	if (!np || !sprop) {
		of_node_put(np);
		np = of_find_node_by_name(NULL, "aliases");
		if (np)
			sprop = of_get_property(np, "stdout", NULL);
	}

	of_node_put(np);
	if (!sprop)
		return;

	/* We don't care what the aliased node is actually called.  We only
	 * care if it's compatible with "epapr,hv-byte-channel", because that
	 * indicates that it's a byte channel node.
	 */
	np = of_find_node_by_path(sprop);
	if (!np) {
		pr_warning("ehv-bc: stdout node '%s' does not exist\n", sprop);
		return;
	}

	/* Is it a byte channel? */
	if (!of_device_is_compatible(np, "epapr,hv-byte-channel"))
		goto exit;

	stdout_irq = irq_of_parse_and_map(np, 0);
	if (stdout_irq == NO_IRQ) {
		pr_err("ehv-bc: no 'interrupts' property in %s node\n", sprop);
		goto exit;
	}

	iprop = of_get_property(np, "reg", NULL);
	if (!iprop) {
		pr_err("ehv-bc: no 'reg' property in %s node\n", np->name);
		goto exit;
	}
	stdout_bc = be32_to_cpu(*iprop);

exit:
	of_node_put(np);
}

/*************************** EARLY CONSOLE DRIVER ***************************/

#ifdef CONFIG_PPC_EARLY_DEBUG_EHV_BC

/*
 * send a byte to a byte channel, wait if necessary
 *
 * This function sends a byte to a byte channel, and it waits and
 * retries if the byte channel is full.  It returns if the character
 * has been sent, or if some error has occurred.
 *
 */
static void byte_channel_spin_send(const char data)
{
	int ret, count;

	do {
		count = 1;
		ret = ev_byte_channel_send(CONFIG_PPC_EARLY_DEBUG_EHV_BC_HANDLE,
					   &count, &data);
	} while (ret == EV_EAGAIN);
}

/*
 * The udbg subsystem calls this function to display a single character.
 * We convert CR to a CR/LF.
 */
static void ehv_bc_udbg_putc(char c)
{
	byte_channel_spin_send(c);
	if (c == '\n')
		byte_channel_spin_send('\r');
}

/*
 * early console initialization
 *
 * PowerPC kernels support an early printk console, also known as udbg.
 * This function must be called via the ppc_md.init_early function pointer.
 * At this point, the device tree has been unflattened, so we can obtain the
 * byte channel handle for stdout.
 *
 * We only support displaying of characters (putc).  We do not support
 * keyboard input.
 */
void __init udbg_init_ehv_bc(void)
{
	unsigned int rx_count, tx_count;
	unsigned int ret;

	/* Check if we're running as a guest of a hypervisor */
	if (!(mfmsr() & MSR_GS))
		return;

	/* Verify the byte channel handle */
	ret = ev_byte_channel_poll(CONFIG_PPC_EARLY_DEBUG_EHV_BC_HANDLE,
				   &rx_count, &tx_count);
	if (ret)
		return;

	udbg_putc = ehv_bc_udbg_putc;
	register_early_udbg_console();

	udbg_printf("ehv-bc: early console using byte channel handle %u\n",
		    CONFIG_PPC_EARLY_DEBUG_EHV_BC_HANDLE);
}

#endif

/****************************** CONSOLE DRIVER ******************************/

static struct tty_driver *ehv_bc_driver;

/*
 * Byte channel console sending worker function.
 *
 * For consoles, if the output buffer is full, we should just spin until it
 * clears.
 */
static int ehv_bc_console_byte_channel_send(unsigned int handle, const char *s,
			     unsigned int count)
{
	unsigned int len;
	int ret = 0;

	while (count) {
		len = min_t(unsigned int, count, 16);
		do {
			ret = ev_byte_channel_send(handle, &len, s);
		} while (ret == EV_EAGAIN);
		count -= len;
		s += len;
	}

	return ret;
}

static void ehv_bc_console_write(struct console *co, const char *s,
				 unsigned int count)
{
	unsigned int handle = (unsigned int)co->data;

	while (count) {
		unsigned int len;
		const char *p;
		int ret;

		/* No more than 16 characters can be sent at a time */
		len = min_t(unsigned int, count, 16);

		/* If there's a \n, we need to inject a \r */
		p = strnchr(s, len, '\n');
		if (p)
			len = p - s + 1;

		ret = ehv_bc_console_byte_channel_send(handle, s, len);

		/* Now send a \r if the last character was a \n */
		if (!ret && p)
			ret = ehv_bc_console_byte_channel_send(handle, "\r", 1);

		if (ret)
			/* Any non-zero return code here indicates failure */
			return;

		s += len;
		count -= len;
	}
}

static struct tty_driver *ehv_bc_console_device(struct console *c, int *index)
{
	*index = c->index;

	return ehv_bc_driver;
}

static int ehv_bc_tty_write(struct tty_struct *tty, const unsigned char *s,
				int count)
{
	int ret;

	ret = ehv_bc_console_byte_channel_send(stdout_bc, s, count);
	if (ret < 0)
		return ret;

	return count;
}

static struct console ehv_bc_console = {
	.name		= "ttyEHV",
	.write		= ehv_bc_console_write,
	.device		= ehv_bc_console_device,
	.flags		= CON_PRINTBUFFER | CON_ENABLED,
};

/*
 * Console initialization
 *
 * This is the first function that is called after the device tree is
 * available, so here is where we determine the byte channel handle and IRQ for
 * stdout/stdin, even though that information is used by the tty and character
 * drivers.
 */
static int __init ehv_bc_console_init(void)
{
	find_console_handle();
	if (!stdout_bc) {
		/* stdout is not a byte channel */
		pr_debug("ehv-bc: stdout is not a byte channel\n");
		return -ENODEV;
	}

#ifdef CONFIG_PPC_EARLY_DEBUG_EHV_BC
	/* Print a friendly warning if the user chose the wrong */
	if (stdout_bc != CONFIG_PPC_EARLY_DEBUG_EHV_BC_HANDLE)
		pr_warning("ehv-bc: udbg handle %u is not the stdout handle\n",
			   CONFIG_PPC_EARLY_DEBUG_EHV_BC_HANDLE);
#endif

	ehv_bc_console.data = (void *)stdout_bc;

	add_preferred_console(ehv_bc_console.name, 0, NULL);
	register_console(&ehv_bc_console);

	pr_info("ehv-bc: registered console driver for byte channel %u\n",
		stdout_bc);

	return 0;
}
console_initcall(ehv_bc_console_init);

/******************************** TTY DRIVER ********************************/

/* The tty port object for the console.  There doesn't appear to be a way for
 * ehv_bc_init() to create a port object and pass it to ehv_bc_tty_open(), so
 * we have to make this a global variable.
 */
static struct tty_port ttyport;

/*
 * byte channel receive interupt handler
 *
 * This ISR is called whenever data is available on a byte channel.
 */
static irqreturn_t ehv_bc_tty_rx_isr(int irq, void *data)
{
	struct tty_struct *tty = tty_port_tty_get(data);
	unsigned int rx_count, tx_count;
	unsigned int count, len;
	u8 buffer[16];
	int ret;

	/* Find out how much data is available to be read, and then ask the
	 * TTY layer if it can handle that much.  We want to ensure that every
	 * byte we read from the byte channel will be accepted by the TTY layer.
	 */
	ev_byte_channel_poll(stdout_bc, &rx_count, &tx_count);
	count = tty_buffer_request_room(tty, rx_count);

	/* 'count' is the maximum amount of data the TTY layer can accept at
	 * this time.  However, during testing, I was never able to get 'count'
	 * to be less than 'rx_count'
	 */

	while (count) {
		len = min_t(unsigned int, count, sizeof(buffer));

		ev_byte_channel_receive(stdout_bc, &len, buffer);

		/* 'len' is now the amount of data that's been received. 'len'
		   can't be zero, and most likely it's equal to one. */

		ret = tty_insert_flip_string(tty, buffer, len);

		/* 'ret' is the number of bytes that the TTY layer accepted.
		 * If it's not equal to 'len', then it means the buffer is
		 * full, which should never happen.  If it does, we can exit
		 * gracefully, but we'll have to drop the remaining 'len - ret'
		 * characters we read from the byte channel.
		 */
		if (ret != len)
			break;

		count -= len;
	}

	tty_flip_buffer_push(tty);

	return IRQ_HANDLED;
}

static int ehv_bc_tty_open(struct tty_struct *ttys, struct file *filp)
{
	return tty_port_open(&ttyport, ttys, filp);
}

static void ehv_bc_tty_close(struct tty_struct *ttys, struct file *filp)
{
	tty_port_close(&ttyport, ttys, filp);
}

/*
 * ehv_bc_tty_write_room - return the amount of space in the output buffer
 *
 * This is actually a contract between the driver and the tty layer outlining
 * how much write room the driver can guarantee will be sent OR BUFFERED.  This
 * driver MUST honor the return value.
 */
static int ehv_bc_tty_write_room(struct tty_struct *ttys)
{
	unsigned int rx_count, tx_count;
	unsigned int ret;

	/* Returns the amount of free space in the TX buffer */
	ret = ev_byte_channel_poll(stdout_bc, &rx_count, &tx_count);
	if (ret)
		/* Failure can occur only if stdout_bc is wrong*/
		return -EINVAL;

	return tx_count;
}

/*
 * TTY driver operations
 *
 * If we could ask the hypervisor how much data is still in the TX buffer, then
 * we could implement the .wait_until_sent and .chars_in_buffer functions.
 */
static const struct tty_operations ehv_bc_ops = {
	.open            = ehv_bc_tty_open,
	.close           = ehv_bc_tty_close,
	.write           = ehv_bc_tty_write,
	.write_room      = ehv_bc_tty_write_room,
};

/*
 * initialize the TTY port
 *
 * This function will only be called once, no matter how many times
 * ehv_bc_tty_open() is called.  That's why we register the ISR here.
 */
static int ehv_bc_tty_port_activate(struct tty_port *port,
				    struct tty_struct *ttys)
{
	int ret;

	ret = request_irq(stdout_irq, ehv_bc_tty_rx_isr, 0, "ehv-bc", port);
	if (ret < 0)
		pr_err("ehv-bc: could not request rx irq %u (ret=%i)\n",
		      stdout_irq, ret);

	return ret;
}

static void ehv_bc_tty_port_shutdown(struct tty_port *port)
{
	free_irq(stdout_irq, port);
}

static const struct tty_port_operations ehv_bc_tty_port_ops = {
	.activate = ehv_bc_tty_port_activate,
	.shutdown = ehv_bc_tty_port_shutdown,
};

static int __init ehv_bc_tty_init(unsigned int count)
{
	int ret;

	ehv_bc_driver = alloc_tty_driver(1);
	if (!ehv_bc_driver)
		return -ENOMEM;

	ehv_bc_driver->owner        = THIS_MODULE;
	ehv_bc_driver->driver_name  = "ehv-bc";
	ehv_bc_driver->name         = "ttyEHV";
	ehv_bc_driver->num          = count;
	ehv_bc_driver->type         = TTY_DRIVER_TYPE_CONSOLE;
	ehv_bc_driver->subtype      = SYSTEM_TYPE_CONSOLE;
	ehv_bc_driver->init_termios = tty_std_termios;
	ehv_bc_driver->flags        = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;

	tty_set_operations(ehv_bc_driver, &ehv_bc_ops);

	ret = tty_register_driver(ehv_bc_driver);
	if (ret) {
		pr_err("ehv-bc: could not register tty driver (ret=%i)\n", ret);
		goto error;
	}

	tty_port_init(&ttyport);
	ttyport.ops = &ehv_bc_tty_port_ops;

	pr_info("ehv-bc: registered tty driver for byte channel %u\n",
		stdout_bc);

	return 0;

error:
	put_tty_driver(ehv_bc_driver);
	return ret;
}

static void __exit ehv_bc_tty_exit(void)
{
	tty_unregister_driver(ehv_bc_driver);
	put_tty_driver(ehv_bc_driver);
}

/*********************** BYTE CHANNEL CHARACTER DRIVER ***********************/

/* Per-byte channel private data */
struct ehv_bc_data {
	struct device *dev;
	struct device *tty_dev;
	dev_t dev_id;
	uint32_t handle;
	unsigned int rx_irq;
	unsigned int tx_irq;
	wait_queue_head_t rx_wait;
	wait_queue_head_t tx_wait;
	int rx_ready;
	int tx_ready;
};

/**
 * ehv_bc_rx_isr - byte channel receive interupt handler
 *
 * This ISR is called whenever data is available on a byte channel.
 */
static irqreturn_t ehv_bc_rx_isr(int irq, void *data)
{
	struct ehv_bc_data *bc = data;

	bc->rx_ready = 1;
	wake_up_interruptible(&bc->rx_wait);

	return IRQ_HANDLED;
}

/**
 * ehv_bc_tx_isr - byte channel transmit interupt handler
 *
 * This ISR is called whenever space is available on a byte channel.
 */
static irqreturn_t ehv_bc_tx_isr(int irq, void *data)
{
	struct ehv_bc_data *bc = data;

	bc->tx_ready = 1;
	wake_up_interruptible(&bc->tx_wait);

	return IRQ_HANDLED;
}

/**
 * ehv_bc_poll - query the byte channel to see if data is available
 *
 * Returns a bitmask indicating whether a read will block
 */
static unsigned int ehv_bc_poll(struct file *filp, struct poll_table_struct *p)
{
	struct ehv_bc_data *bc = filp->private_data;
	unsigned int rx_count, tx_count;
	unsigned int mask = 0;
	int ret;

	ret = ev_byte_channel_poll(bc->handle, &rx_count, &tx_count);
	if (ret)
		return POLLERR;

	poll_wait(filp, &bc->rx_wait, p);
	poll_wait(filp, &bc->tx_wait, p);

	if (rx_count)
		mask |= POLLIN | POLLRDNORM;
	if (tx_count)
		mask |= POLLOUT | POLLWRNORM;

	return mask;
}

/**
 * ehv_bc_read - read from a byte channel into the user's buffer
 *
 * Returns the total number of bytes read, or error
 */
static ssize_t ehv_bc_read(struct file *filp, char __user *buf, size_t len,
			   loff_t *off)
{
	struct ehv_bc_data *bc = filp->private_data;
	unsigned int count;
	unsigned int total = 0;
	char buffer[16];
	int ret;

	/* Make sure we stop when the user buffer is full. */
	while (len) {
		/* Don't ask for more than we can receive */
		count = min(len, sizeof(buffer));

		/* Reset the RX status here so that we don't need a spinlock
		 * around the hyerpcall.  It won't matter if the ISR is called
		 * before the receive() returns, because 'count' will be
		 * non-zero, so we won't test rx_ready.
		 */
		bc->rx_ready = 0;

		/* Non-blocking */
		ret = ev_byte_channel_receive(bc->handle, &count, buffer);
		if (ret)
			return -EIO;

		/* If the byte channel is empty, then either we're done or we
		 * need to block.
		 */
		if (!count) {
			if (total)
				/* We did read some chars, so we're done. */
				return total;

			/* If the application specified O_NONBLOCK, then we
			 * return the appropriate error code.
			 */
			if (filp->f_flags & O_NONBLOCK)
				return -EAGAIN;

			/* Wait until some data is available */
			if (wait_event_interruptible(bc->rx_wait, bc->rx_ready))
				return -ERESTARTSYS;

			/* Data is available, so loop around and read it */
			continue;
		}

		copy_to_user(buf, buffer, count);

		buf += count;
		len -= count;
		total += count;
	}

	return total;
}

/**
 * ehv_bc_write - write to a byte channel from the user's buffer
 *
 * Returns the total number of bytes written, or error
 */
static ssize_t ehv_bc_write(struct file *filp, const char __user *buf,
			    size_t len, loff_t *off)
{
	struct ehv_bc_data *bc = filp->private_data;
	unsigned int count;
	unsigned int total = 0;
	char buffer[16];
	int ret;

	while (len) {
		count = min(len, sizeof(buffer));
		copy_from_user(buffer, buf, count);

		bc->tx_ready = 0;

		ret = ev_byte_channel_send(bc->handle, &count, buffer);
		if (ret) {
			if (total)
				/* We did write some chars, so we're done. */
				return total;

			/* If the application specified O_NONBLOCK, then we
			 * return the appropriate error code.
			 */
			if (filp->f_flags & O_NONBLOCK)
				return -EAGAIN;

			/* Wait until some data is available */
			if (wait_event_interruptible(bc->tx_wait, bc->tx_ready))
				return -ERESTARTSYS;

			continue;
		}

		buf += count;
		len -= count;
		total += count;
	}

	return total;
}

/* Array of byte channel objects */
static struct ehv_bc_data *bcs;

/* Number of elements in bcs[] */
static unsigned int count;

/**
 * ehv_bc_open - open the driver
 */
static int ehv_bc_open(struct inode *inode, struct file *filp)
{
	unsigned int minor = iminor(inode);
	struct ehv_bc_data *bc = &bcs[minor];

	filp->private_data = bc;

	return 0;
}

static const struct file_operations ehv_bc_fops = {
	.owner = THIS_MODULE,
	.open = ehv_bc_open,
	.poll = ehv_bc_poll,
	.read = ehv_bc_read,
	.write = ehv_bc_write,
};

static struct class *ehv_bc_class;
static dev_t dev_id;
static struct cdev cdev;

/**
 * ehv_bc_init - ePAPR hypervisor byte channel driver initialization
 *
 * This function is called when this module is loaded.
 */
static int __init ehv_bc_init(void)
{
	struct device_node *np;
	const uint32_t *reg;
	struct ehv_bc_data *bc;
	unsigned int i;
	int ret;

	pr_info("ePAPR hypervisor byte channel driver\n");

	if (!has_fsl_hypervisor()) {
		pr_info("ehv-bc: no hypervisor found\n");
		return -ENODEV;
	}

	/* Count the number of byte channels */
	for_each_compatible_node(np, NULL, "epapr,hv-byte-channel") {
		reg = of_get_property(np, "reg", NULL);
		if (reg && (be32_to_cpu(*reg) != stdout_bc))
			count++;
	}

	/* If stdout is directed to a byte channel, then initialize the tty */
	if (stdout_bc) {
		ret = ehv_bc_tty_init(count + 1);
		if (ret)
			return ret;
	}

	if (!count) {
		if (stdout_bc)
			return 0;

		pr_info("ehv-bc: no byte channels\n");
		return -ENODEV;
	}

	bcs = kzalloc(count * sizeof(struct ehv_bc_data), GFP_KERNEL);
	if (!bcs)
		return -ENOMEM;

	ret = alloc_chrdev_region(&dev_id, 0, count, "ehv-bc");
	if (ret < 0) {
		pr_err("ehv-bc: could not register character device "
		       " (ret=%i)\n", ret);
		goto error_nomem;
	}

	/* Create our class in sysfs */
	ehv_bc_class = class_create(THIS_MODULE, "ehv-bc");

	i = 0;
	for_each_compatible_node(np, NULL, "epapr,hv-byte-channel") {
		reg = of_get_property(np, "reg", NULL);
		if (!reg) {
			pr_err("ehv-bc: no 'reg' property in %s node\n",
				np->name);
			continue;
		}
		if (be32_to_cpu(*reg) == stdout_bc)
			/* Skip the stdout byte channel */
			continue;

		bc = &bcs[i];

		init_waitqueue_head(&bc->rx_wait);
		init_waitqueue_head(&bc->tx_wait);
		bc->handle = be32_to_cpu(*reg);

		bc->rx_irq = irq_of_parse_and_map(np, 0);
		if (bc->rx_irq == NO_IRQ) {
			pr_err("ehv-bc: no 'interrupts' property in %s node\n",
				np->name);
			continue;
		}
		ret = request_irq(bc->rx_irq, ehv_bc_rx_isr, 0, np->name, bc);
		if (ret < 0) {
			pr_err("ehv-bc: could not request rx irq %u\n",
			       bc->rx_irq);
			continue;
		}

		bc->tx_irq = irq_of_parse_and_map(np, 1);
		if (bc->tx_irq == NO_IRQ) {
			pr_err("ehv-bc: no 'interrupts' property in %s node\n",
				np->name);
			continue;
		}
		ret = request_irq(bc->tx_irq, ehv_bc_tx_isr, 0, np->name, bc);
		if (ret < 0) {
			pr_err("ehv-bc: could not request tx irq %u\n",
			       bc->tx_irq);
			continue;
		}

		/* Create the 'dev' entry in sysfs */
		bc->dev_id = MKDEV(MAJOR(dev_id), MINOR(dev_id) + i);
		bc->dev = device_create(ehv_bc_class, NULL, bc->dev_id, bc,
					"bc%u", bc->handle);
		if (IS_ERR(bc->dev)) {
			pr_err("ehv-bc: could not register byte channel %u\n",
				bc->handle);
			continue;
		}
		bc->tty_dev = tty_register_device(ehv_bc_driver, i, bc->dev);
		printk(KERN_INFO "%s:%u tty_dev=%p\n", __func__, __LINE__, bc->tty_dev);

		pr_info("ehv-bc: registered byte channel %u\n", bc->handle);

		i++;
	}

	cdev_init(&cdev, &ehv_bc_fops);
	cdev.owner = THIS_MODULE;

	ret = cdev_add(&cdev, dev_id, count);
	if (ret < 0) {
		pr_err("ehv-bc: could not add cdev\n");
		goto error;
	}

	return 0;

error:
	for (i = 0; i < count; i++) {
		device_destroy(ehv_bc_class, bcs[i].dev_id);
		free_irq(bcs[i].rx_irq, &bcs[i]);
		free_irq(bcs[i].tx_irq, &bcs[i]);
	}

	unregister_chrdev_region(dev_id, count);

error_nomem:
	kfree(bcs);

	return ret;
}


/**
 * ehv_bc_exit - ePAPR hypervisor byte channel driver termination
 *
 * This function is called when this driver is unloaded.
 */
static void __exit ehv_bc_exit(void)
{
	unsigned int i;

	cdev_del(&cdev);

	for (i = 0; i < count; i++) {
		device_destroy(ehv_bc_class, bcs[i].dev_id);
		free_irq(bcs[i].rx_irq, &bcs[i]);
		free_irq(bcs[i].tx_irq, &bcs[i]);
	}

	unregister_chrdev_region(dev_id, count);

	kfree(bcs);

	ehv_bc_tty_exit();
}

module_init(ehv_bc_init);
module_exit(ehv_bc_exit);

MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
MODULE_DESCRIPTION("ePAPR hypervisor byte channel driver");
MODULE_LICENSE("GPL");

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Thu, Nov 18, 2010 at 10:56:43AM -0600, Timur Tabi wrote:
> Greg KH wrote:
> > Why?  Again, it doesn't matter, and no other tty driver does it.
> > 
> > Actually you do have control over it, if you really want it, but again,
> > don't do that :)
> 
> Because the only way to know which byte channel tty you want is via the byte
> channel handle.
> 
> Think of the situation where you have two serial ports on the back of your
> computer.  One of them is /dev/ttyS0, and the other is /dev/ttyS1.  But which
> one is which?  The only way to find out is try one and see if it works.

No, you can use the /dev/serial/ links to determine exactly which is
which depending on the pci id, and other unique identifiers (serial
numbers, etc.)

> Now that might be acceptable for serial ports that are fixed physically.  But
> byte channels handles are completely arbitrary and easily change with even the
> slightest re-configuration of the partitions under the hypervisor.  I need to
> have some way to tell userspace that /dev/ttybc0 is byte channel handle 73.

Then again, just use the /dev/serial/ symlinks that are there for you.

> > Sorry, I can't do code review, or accept code that is not allowed to be
> > sent to the public, as, surprise, I'm public :)
> 
> Well, ok.  I didn't want to spam the mailing list with something that only you
> asked for, but here it is:

Is this somehow not public code?  What just changed in the past 15
minutes?

confused,

greg k-h

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Greg KH wrote:

> No, you can use the /dev/serial/ links to determine exactly which is
> which depending on the pci id, and other unique identifiers (serial
> numbers, etc.)

I just booted a Linux kernel with the driver I just emailed you, and there's no
/dev/serial/ directory.  The only directories under /dev/ are 'shm' and 'pts',
both of which are empty.

I'm also running a Fedora 13 x86 system, just to see if I need a full modern OS
to see these files.  Again, there is no /dev/serial/, even though I have serial
ports.

Also not that since I'm not registering the byte channels as serial devices, I
wouldn't expect anything in /dev/serial/ to reference them.

What does my driver need to do in order for these /dev/xxxx/ entries to contain
that information?

> Is this somehow not public code?  What just changed in the past 15
> minutes?

Sorry, when I said "not public", I didn't mean it in a legal sense.  Now that I
think about it, I guess that doesn't make much sense.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Thu, Nov 18, 2010 at 11:38:07AM -0600, Timur Tabi wrote:
> Greg KH wrote:
> 
> > No, you can use the /dev/serial/ links to determine exactly which is
> > which depending on the pci id, and other unique identifiers (serial
> > numbers, etc.)
> 
> I just booted a Linux kernel with the driver I just emailed you, and there's no
> /dev/serial/ directory.  The only directories under /dev/ are 'shm' and 'pts',
> both of which are empty.

Then plug in a serial port device and see what happens.  You didn't hook
everything up in your driver correctly it seems, do your devices show up
under /sys/class/tty?

> I'm also running a Fedora 13 x86 system, just to see if I need a full modern OS
> to see these files.  Again, there is no /dev/serial/, even though I have serial
> ports.

Dynamic ones like a usb to serial device?

> Also not that since I'm not registering the byte channels as serial devices, I
> wouldn't expect anything in /dev/serial/ to reference them.
> 
> What does my driver need to do in order for these /dev/xxxx/ entries to contain
> that information?

See the udev rules for details.

> > Is this somehow not public code?  What just changed in the past 15
> > minutes?
> 
> Sorry, when I said "not public", I didn't mean it in a legal sense.  Now that I
> think about it, I guess that doesn't make much sense.

Yes, it didn't :)

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Greg KH wrote:
>> I just booted a Linux kernel with the driver I just emailed you, and there's no
>> > /dev/serial/ directory.  The only directories under /dev/ are 'shm' and 'pts',
>> > both of which are empty.

> Then plug in a serial port device and see what happens.  You didn't hook
> everything up in your driver correctly it seems, do your devices show up
> under /sys/class/tty?

If I delete the call to device_create() in ehv_bc_init() (so that it creates the
TTY devices only, and not the character devices), I get this:

# ls -l /sys/class/tty/ttyEHV*
lrwxrwxrwx    1 root     root            0 Jan  1 00:04 /sys/class/tty/ttyEHV0
-> ../../devices/virtual/tty/ttyEHV0
lrwxrwxrwx    1 root     root            0 Jan  1 00:04 /sys/class/tty/ttyEHV1
-> ../../devices/virtual/tty/ttyEHV1
lrwxrwxrwx    1 root     root            0 Jan  1 00:04 /sys/class/tty/ttyEHV2
-> ../../devices/virtual/tty/ttyEHV2

>> > I'm also running a Fedora 13 x86 system, just to see if I need a full modern OS
>> > to see these files.  Again, there is no /dev/serial/, even though I have serial
>> > ports.
> Dynamic ones like a usb to serial device?

No, not dynamic ones.

>> > Also not that since I'm not registering the byte channels as serial devices, I
>> > wouldn't expect anything in /dev/serial/ to reference them.
>> > 
>> > What does my driver need to do in order for these /dev/xxxx/ entries to contain
>> > that information?

> See the udev rules for details.

udev rules still need some way for the driver to tell user-space that
/dev/ttyEHV0 is associated with byte channel handle 73.  I still don't know what
mechanism my driver is supposed to use to make that information available to
user space.

I could fake it by doing this:

for (i = 0; i < num_byte_channels; i++) {
	bc->handle = get_the_byte_channel_handle(i);
	ehv_bc_driver->name_base = bc->handle - i;
	tty_register_device(ehv_bc_driver, i, NULL);
}

This actually works and does what I want, but I seriously doubt it's acceptable.
 When I do this, I get:

# ls -l /dev/ttyEH*
crw-rw----    1 root     uucp     253,   0 Jan  1 00:00 /dev/ttyEHV73
crw-rw----    1 root     uucp     253,   1 Jan  1 00:00 /dev/ttyEHV76
crw-rw----    1 root     uucp     253,   2 Jan  1 00:00 /dev/ttyEHV79

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Thu, Nov 18, 2010 at 01:35:33PM -0600, Timur Tabi wrote:
> Greg KH wrote:
> >> I just booted a Linux kernel with the driver I just emailed you, and there's no
> >> > /dev/serial/ directory.  The only directories under /dev/ are 'shm' and 'pts',
> >> > both of which are empty.
> 
> > Then plug in a serial port device and see what happens.  You didn't hook
> > everything up in your driver correctly it seems, do your devices show up
> > under /sys/class/tty?
> 
> If I delete the call to device_create() in ehv_bc_init() (so that it creates the
> TTY devices only, and not the character devices), I get this:
> 
> # ls -l /sys/class/tty/ttyEHV*
> lrwxrwxrwx    1 root     root            0 Jan  1 00:04 /sys/class/tty/ttyEHV0
> -> ../../devices/virtual/tty/ttyEHV0
> lrwxrwxrwx    1 root     root            0 Jan  1 00:04 /sys/class/tty/ttyEHV1
> -> ../../devices/virtual/tty/ttyEHV1
> lrwxrwxrwx    1 root     root            0 Jan  1 00:04 /sys/class/tty/ttyEHV2
> -> ../../devices/virtual/tty/ttyEHV2
> 
> >> > I'm also running a Fedora 13 x86 system, just to see if I need a full modern OS
> >> > to see these files.  Again, there is no /dev/serial/, even though I have serial
> >> > ports.
> > Dynamic ones like a usb to serial device?
> 
> No, not dynamic ones.
> 
> >> > Also not that since I'm not registering the byte channels as serial devices, I
> >> > wouldn't expect anything in /dev/serial/ to reference them.
> >> > 
> >> > What does my driver need to do in order for these /dev/xxxx/ entries to contain
> >> > that information?
> 
> > See the udev rules for details.
> 
> udev rules still need some way for the driver to tell user-space that
> /dev/ttyEHV0 is associated with byte channel handle 73.  I still don't know what
> mechanism my driver is supposed to use to make that information available to
> user space.
> 
> I could fake it by doing this:
> 
> for (i = 0; i < num_byte_channels; i++) {
> 	bc->handle = get_the_byte_channel_handle(i);
> 	ehv_bc_driver->name_base = bc->handle - i;
> 	tty_register_device(ehv_bc_driver, i, NULL);

Why are you not setting up a parent device of your tty device?  That
should be essencial.

> }
> 
> This actually works and does what I want, but I seriously doubt it's acceptable.
>  When I do this, I get:
> 
> # ls -l /dev/ttyEH*
> crw-rw----    1 root     uucp     253,   0 Jan  1 00:00 /dev/ttyEHV73
> crw-rw----    1 root     uucp     253,   1 Jan  1 00:00 /dev/ttyEHV76
> crw-rw----    1 root     uucp     253,   2 Jan  1 00:00 /dev/ttyEHV79

Does that work for you?  Looks fine to me :)

But again, you really should get the driver model portion right...

thanks,

greg k-h

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Greg KH wrote:
> Does that work for you?  Looks fine to me :)

Ok!

> But again, you really should get the driver model portion right...

I'll do that.

One last question.  If I create multiple TTY devices, how do I tell the kernel
which one should be used for the console?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Thu, Nov 18, 2010 at 02:06:14PM -0600, Timur Tabi wrote:
> Greg KH wrote:
> > Does that work for you?  Looks fine to me :)
> 
> Ok!
> 
> > But again, you really should get the driver model portion right...
> 
> I'll do that.
> 
> One last question.  If I create multiple TTY devices, how do I tell the kernel
> which one should be used for the console?

On the command line.

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Greg KH wrote:
>> > One last question.  If I create multiple TTY devices, how do I tell the kernel
>> > which one should be used for the console?

> On the command line.

Is there a way to do it in the driver itself?  Getting the boot loader to
identify the specific byte channel for stdout isn't trivial.  It'd be nice if
the driver could tell the tty layer, "BTW, this use this for the default console".

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Alan Cox]

On Thu, 18 Nov 2010 14:43:17 -0600
Timur Tabi <timur@freescale.com> wrote:

> Greg KH wrote:
> >> > One last question.  If I create multiple TTY devices, how do I tell the kernel
> >> > which one should be used for the console?
> 
> > On the command line.
> 
> Is there a way to do it in the driver itself?  Getting the boot loader to
> identify the specific byte channel for stdout isn't trivial.  It'd be nice if
> the driver could tell the tty layer, "BTW, this use this for the default console".

You can - provide the required method in your console driver and it'll
get used by /dev/console. Funnily enough a lot of other platform and
consoles need that too.

Alan

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Alan Cox wrote:
>> > Is there a way to do it in the driver itself?  Getting the boot loader to
>> > identify the specific byte channel for stdout isn't trivial.  It'd be nice if
>> > the driver could tell the tty layer, "BTW, this use this for the default console".

> You can - provide the required method in your console driver and it'll
> get used by /dev/console. Funnily enough a lot of other platform and
> consoles need that too.

Can you elaborate on that?  What is the "required method"?  My driver is mostly
working now, but I have to supply the command-line "console=ttyEHV73" in order
for the login prompt to show up.  Unfortunately, there's no way for the boot
loader to know that the primary byte channel for stdout is #73, so I need a way
for the driver to tell the kernel this.

Also, if I have a /dev/ttyEHV76 entry for a byte channel that's not the primary
stdout tty, is the following supposed to work:

	cat > /dev/ttyEHV76

That is, should I be able to use a TTY device as a normal character device,
where I can just write and read characters?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Timur Tabi wrote:
> Can you elaborate on that?  What is the "required method"?  My driver is mostly
> working now, but I have to supply the command-line "console=ttyEHV73" in order
> for the login prompt to show up.  Unfortunately, there's no way for the boot
> loader to know that the primary byte channel for stdout is #73, so I need a way
> for the driver to tell the kernel this.

Never mind .. I got it working.  For some reason, it took a while for me to get
add_preferred_console() working.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Alan Cox]

> > You can - provide the required method in your console driver and it'll
> > get used by /dev/console. Funnily enough a lot of other platform and
> > consoles need that too.
> 
> Can you elaborate on that?  What is the "required method"?  My driver is mostly
> working now, but I have to supply the command-line "console=ttyEHV73" in order
> for the login prompt to show up.  Unfortunately, there's no way for the boot
> loader to know that the primary byte channel for stdout is #73, so I need a way
> for the driver to tell the kernel this.

Your console driver provides a device method (see kernel/printk.c).
When /dev/console is opened the kernel iterates the console list looking
for one with ->device and then calls that method. On success it expects
the passed int * to contain the minor number to use.

I suspect in your case you probably want to attach the primary byte
channel to minor 0 in the driver (and reserve it for that), or some
similar rule.

> Also, if I have a /dev/ttyEHV76 entry for a byte channel that's not the primary
> stdout tty, is the following supposed to work:
> 
> 	cat > /dev/ttyEHV76
> 
> That is, should I be able to use a TTY device as a normal character device,
> where I can just write and read characters?

Sort of - processing gets done but you can disable the processing easily
enough. If you have channels that are not tty related you may want to tap
them directly to avoid the overhead of the tty layer if they are high
data rate.

Alan

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Alan Cox wrote:
> Your console driver provides a device method (see kernel/printk.c).
> When /dev/console is opened the kernel iterates the console list looking
> for one with ->device and then calls that method. On success it expects
> the passed int * to contain the minor number to use.

Are you talking about this:

static struct tty_driver *ehv_bc_console_device(struct console *co, int *index)
{
	*index = co->index;

	return ehv_bc_driver;
}

I never really understood this function, but almost everyone does the same
thing, and it seems to work for me.  Looking at console_device(), it appears
that all of the xxx_console_device functions are called in order until one of
them returns non-NULL.

How is this related to add_preferred_console()?  When I call this function, I
also specify the same index and the name from the struct console device:

static struct console ehv_bc_console = {
	.name		= "ttyEHV",
	.write		= ehv_bc_console_write,
	.device		= ehv_bc_console_device,
	.flags		= CON_PRINTBUFFER | CON_ENABLED,
};

add_preferred_console(ehv_bc_console.name, ehv_bc_console.index, NULL);
register_console(&ehv_bc_console);

> I suspect in your case you probably want to attach the primary byte
> channel to minor 0 in the driver (and reserve it for that), or some
> similar rule.

Yes, that's a good idea.  It does simplify things a lot.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Alan Cox]

> Are you talking about this:
> 
> static struct tty_driver *ehv_bc_console_device(struct console *co, int *index)
> {
> 	*index = co->index;
> 
> 	return ehv_bc_driver;
> }

Yes.

> I never really understood this function, but almost everyone does the same
> thing, and it seems to work for me.  Looking at console_device(), it appears
> that all of the xxx_console_device functions are called in order until one of
> them returns non-NULL.

*index is the minor number to use - so you can return whichever minor
matches your primary interface

> > I suspect in your case you probably want to attach the primary byte
> > channel to minor 0 in the driver (and reserve it for that), or some
> > similar rule.
> 
> Yes, that's a good idea.  It does simplify things a lot.

It's probably the cleanest and simplest solution and it fits the
"natural" order of things.

Re: How do I choose an arbitrary minor number for my tty device?

[Alan Cox]

> for (i = 0; i < num_byte_channels; i++) {
> 	bc->handle = get_the_byte_channel_handle(i);
> 	ehv_bc_driver->name_base = bc->handle - i;
> 	tty_register_device(ehv_bc_driver, i, NULL);
> }
> 
> This actually works and does what I want, but I seriously doubt it's acceptable.
>  When I do this, I get:
> 
> # ls -l /dev/ttyEH*
> crw-rw----    1 root     uucp     253,   0 Jan  1 00:00 /dev/ttyEHV73
> crw-rw----    1 root     uucp     253,   1 Jan  1 00:00 /dev/ttyEHV76
> crw-rw----    1 root     uucp     253,   2 Jan  1 00:00 /dev/ttyEHV79

Which looks the right approach to me.

Re: How do I choose an arbitrary minor number for my tty device?

[Scott Wood]

On Thu, 18 Nov 2010 10:03:12 -0600
Timur Tabi <timur@freescale.com> wrote:

> I'm not so sure.  Like I said, I still don't see where there's a bus.  I have a
> single driver that has multiple devices.  It sounds to me like one call to
> tty_register_driver() and multiple calls to tty_register_device() would be
> sufficient.
> 
> For instance, there is no code in drivers/char/ that makes a call to
> bus_register(), so I don't see any precedent for a tty driver to register a bus
> first.

The tty driver doesn't register the bus, but rather a driver for
some type of device on that bus.  The code to create the bus goes
elsewhere, and would not be specific to byte channels.

> Also, this is an Open Firmware driver.  I already have a mechanism whereby I get
> probed for each instance of a byte channel.  Isn't that my "bus"?

It would be if you actually had it -- but it looks like you just loop
over the nodes.

We should add a proper bus for the "handles" node.  Then sysfs should
show the link between the tty device and a device tree node -- which is
really what we're after, the handle is just a means to that end.

> I'm really trying to do the right thing here, Greg, but every time I try to
> solve one problem, I'm being told that I need to make things way more
> complicated first.

s/make things way more complicated/use the existing infrastructure
rather than reinvent the wheel/

And getting rid of the redundant chardev driver would be a
simplification...

-Scott

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Scott Wood wrote:

> The tty driver doesn't register the bus, but rather a driver for
> some type of device on that bus.  The code to create the bus goes
> elsewhere, and would not be specific to byte channels.

Which code to create the bus?  Are you saying that the driver should call
bus_register()?

>> Also, this is an Open Firmware driver.  I already have a mechanism whereby I get
>> probed for each instance of a byte channel.  Isn't that my "bus"?
> 
> It would be if you actually had it -- but it looks like you just loop
> over the nodes.

Well, ok, but I can change that.  If I drop the normal character driver
registration and register the byte channels only as tty devices, then I can make
it probe-able.  The reason I don't do it now is because, for a normal character
device, I need to call cdev_init() and cdev_add() after all devices have been
registered, which can't be done in an OF driver since I don't get told when
there are no more probes.

> We should add a proper bus for the "handles" node.  Then sysfs should
> show the link between the tty device and a device tree node -- which is
> really what we're after, the handle is just a means to that end.

How exactly do I do that?

> And getting rid of the redundant chardev driver would be a
> simplification...

I agree there, but so far people have been telling me, "just do this!" without
actually telling me how to do "this".


-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Thu, Nov 18, 2010 at 11:42:21AM -0600, Timur Tabi wrote:
> > And getting rid of the redundant chardev driver would be a
> > simplification...
> 
> I agree there, but so far people have been telling me, "just do this!" without
> actually telling me how to do "this".

We can't do your work for you :)

Re: How do I choose an arbitrary minor number for my tty device?

[Scott Wood]

On Thu, 18 Nov 2010 11:42:21 -0600
Timur Tabi <timur@freescale.com> wrote:

> Scott Wood wrote:
> 
> > The tty driver doesn't register the bus, but rather a driver for
> > some type of device on that bus.  The code to create the bus goes
> > elsewhere, and would not be specific to byte channels.
> 
> Which code to create the bus?  Are you saying that the driver should call
> bus_register()?

No, the bus code calls that (probably drivers/base/platform.c in this
case).

> >> Also, this is an Open Firmware driver.  I already have a mechanism whereby I get
> >> probed for each instance of a byte channel.  Isn't that my "bus"?
> > 
> > It would be if you actually had it -- but it looks like you just loop
> > over the nodes.
> 
> Well, ok, but I can change that.  If I drop the normal character driver
> registration and register the byte channels only as tty devices, then I can make
> it probe-able.

OK.

> > We should add a proper bus for the "handles" node.  Then sysfs should
> > show the link between the tty device and a device tree node -- which is
> > really what we're after, the handle is just a means to that end.
> 
> How exactly do I do that?

Pass the platform device pointer to tty_register_device.

Then, in the sysfs node, "driver" should be a symlink to
another sysfs node whose path looks lind of like an OF path.

Unfortunately, it's not an exact match, and the fact that reg doesn't
translate to a physical address means that AFAICT you'll currently get
something like "byte-channel.nnn", where "nnn" is an arbitrary
kernel-assigned number.

It would be nice if platform devices that are created from device tree
nodes included a link to the corresponding /proc/device-tree node in
their sysfs node.

Other than that, I guess you could add hv handle support to
of_device_make_bus_id.

-Scott

Re: How do I choose an arbitrary minor number for my tty device?

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

On Thu, 2010-11-18 at 12:13 -0600, Scott Wood wrote:
> On Thu, 18 Nov 2010 11:42:21 -0600
> Timur Tabi <timur@freescale.com> wrote:
> 
> > Scott Wood wrote:

> > > We should add a proper bus for the "handles" node.  Then sysfs should
> > > show the link between the tty device and a device tree node -- which is
> > > really what we're after, the handle is just a means to that end.
> > 
> > How exactly do I do that?
> 
> Pass the platform device pointer to tty_register_device.
> 
> Then, in the sysfs node, "driver" should be a symlink to
> another sysfs node whose path looks lind of like an OF path.
> 
> Unfortunately, it's not an exact match, and the fact that reg doesn't
> translate to a physical address means that AFAICT you'll currently get
> something like "byte-channel.nnn", where "nnn" is an arbitrary
> kernel-assigned number.

Can you not use device_rename() ?

> It would be nice if platform devices that are created from device tree
> nodes included a link to the corresponding /proc/device-tree node in
> their sysfs node.

It's not a link, but the OF path is in devspec, so you can work it out
fairly easily.

cheers






[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: How do I choose an arbitrary minor number for my tty device?

[Scott Wood]

On Wed, 24 Nov 2010 21:23:47 +1100
Michael Ellerman <michael@ellerman.id.au> wrote:

> On Thu, 2010-11-18 at 12:13 -0600, Scott Wood wrote:
> > Unfortunately, it's not an exact match, and the fact that reg doesn't
> > translate to a physical address means that AFAICT you'll currently get
> > something like "byte-channel.nnn", where "nnn" is an arbitrary
> > kernel-assigned number.
> 
> Can you not use device_rename() ?

Ah, didn't know about that.  Still, might be nice to add support for
"handle" nodes at the infrastructure level rather than in each driver.

> > It would be nice if platform devices that are created from device tree
> > nodes included a link to the corresponding /proc/device-tree node in
> > their sysfs node.
> 
> It's not a link, but the OF path is in devspec, so you can work it out
> fairly easily.

Hmm, I see a "devspec" in PCI devices, but not in devtree-probed
platform devices.  of_bus_type_init isn't being called from anywhere
but the ibmebus code.  It looks like this was a casualty of merging
of_platform with platform (commit eca3930163ba8884060ce9d9ff5ef0d9b7c7b00f).

-Scott

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Wed, Nov 24, 2010 at 12:08:18PM -0600, Scott Wood wrote:
> On Wed, 24 Nov 2010 21:23:47 +1100
> Michael Ellerman <michael@ellerman.id.au> wrote:
> 
> > On Thu, 2010-11-18 at 12:13 -0600, Scott Wood wrote:
> > > Unfortunately, it's not an exact match, and the fact that reg doesn't
> > > translate to a physical address means that AFAICT you'll currently get
> > > something like "byte-channel.nnn", where "nnn" is an arbitrary
> > > kernel-assigned number.
> > 
> > Can you not use device_rename() ?
> 
> Ah, didn't know about that.  Still, might be nice to add support for
> "handle" nodes at the infrastructure level rather than in each driver.

No, please never use that function, bad things will happen.

thanks,

greg k-h

Re: How do I choose an arbitrary minor number for my tty device?

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1690 bytes --]

On Wed, 2010-11-24 at 10:23 -0800, Greg KH wrote:
> On Wed, Nov 24, 2010 at 12:08:18PM -0600, Scott Wood wrote:
> > On Wed, 24 Nov 2010 21:23:47 +1100
> > Michael Ellerman <michael@ellerman.id.au> wrote:
> > 
> > > On Thu, 2010-11-18 at 12:13 -0600, Scott Wood wrote:
> > > > Unfortunately, it's not an exact match, and the fact that reg doesn't
> > > > translate to a physical address means that AFAICT you'll currently get
> > > > something like "byte-channel.nnn", where "nnn" is an arbitrary
> > > > kernel-assigned number.
> > > 
> > > Can you not use device_rename() ?
> > 
> > Ah, didn't know about that.  Still, might be nice to add support for
> > "handle" nodes at the infrastructure level rather than in each driver.
> 
> No, please never use that function, bad things will happen.

Why? The network & wireless code uses it, so presumably it can work? If
not please consider:

commit f470f680dfaad8731f079a033a50440082e20930
Author: Michael Ellerman <michael@ellerman.id.au>
Date:   Thu Nov 25 09:41:28 2010 +1100

    driver core: Document that device_rename() is not to be used
    
    Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 6ed6454..f1fac19 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1513,6 +1513,8 @@ EXPORT_SYMBOL_GPL(device_destroy);
  * exclusion between two different calls of device_rename
  * on the same device to ensure that new_name is valid and
  * won't conflict with other devices.
+ *
+ * "Never use this function, bad things will happen" - gregkh
  */
 int device_rename(struct device *dev, const char *new_name)
 {


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Thu, Nov 25, 2010 at 09:44:07AM +1100, Michael Ellerman wrote:
> On Wed, 2010-11-24 at 10:23 -0800, Greg KH wrote:
> > On Wed, Nov 24, 2010 at 12:08:18PM -0600, Scott Wood wrote:
> > > On Wed, 24 Nov 2010 21:23:47 +1100
> > > Michael Ellerman <michael@ellerman.id.au> wrote:
> > > 
> > > > On Thu, 2010-11-18 at 12:13 -0600, Scott Wood wrote:
> > > > > Unfortunately, it's not an exact match, and the fact that reg doesn't
> > > > > translate to a physical address means that AFAICT you'll currently get
> > > > > something like "byte-channel.nnn", where "nnn" is an arbitrary
> > > > > kernel-assigned number.
> > > > 
> > > > Can you not use device_rename() ?
> > > 
> > > Ah, didn't know about that.  Still, might be nice to add support for
> > > "handle" nodes at the infrastructure level rather than in each driver.
> > 
> > No, please never use that function, bad things will happen.
> 
> Why? The network & wireless code uses it, so presumably it can work?

Yes it can, but networking is the only code that should use this.

> If
> not please consider:
> 
> commit f470f680dfaad8731f079a033a50440082e20930
> Author: Michael Ellerman <michael@ellerman.id.au>
> Date:   Thu Nov 25 09:41:28 2010 +1100
> 
>     driver core: Document that device_rename() is not to be used
>     
>     Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 6ed6454..f1fac19 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1513,6 +1513,8 @@ EXPORT_SYMBOL_GPL(device_destroy);
>   * exclusion between two different calls of device_rename
>   * on the same device to ensure that new_name is valid and
>   * won't conflict with other devices.
> + *
> + * "Never use this function, bad things will happen" - gregkh

Nice, I like it :)

I'll go queue this up.

greg k-h

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Greg KH wrote:
>> > + * "Never use this function, bad things will happen" - gregkh
> Nice, I like it :)
> 
> I'll go queue this up.

How about something a little more descriptive?  You said that the networking
code can use it.  Could you add some text to explain what the networking code
does with it, and why it's the only allow caller of this code?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Mon, Nov 29, 2010 at 03:51:15PM -0600, Timur Tabi wrote:
> Greg KH wrote:
> >> > + * "Never use this function, bad things will happen" - gregkh
> > Nice, I like it :)
> > 
> > I'll go queue this up.
> 
> How about something a little more descriptive?  You said that the networking
> code can use it.  Could you add some text to explain what the networking code
> does with it, and why it's the only allow caller of this code?

I really don't want to get into it, but the networking code is the only
thing that allows this due to the userspace "want" to rename devices
because we can't create symlinks to network devices like we can for all
other devices (as they use device nodes.)

When this function is called, userspace had better know exactly what is
going on as it can get confused due to the lack of uevents happening.

thanks,

greg k-h

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Greg KH wrote:
> I really don't want to get into it, but the networking code is the only
> thing that allows this due to the userspace "want" to rename devices
> because we can't create symlinks to network devices like we can for all
> other devices (as they use device nodes.)
> 
> When this function is called, userspace had better know exactly what is
> going on as it can get confused due to the lack of uevents happening.

Thanks for the explanation, but I'm just concerned that you'll add a comment
that says, "don't call this function".  A quick grep will show that there are
users of this function, and we'll have this exact conversation all over again.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Mon, Nov 29, 2010 at 04:36:50PM -0600, Timur Tabi wrote:
> Greg KH wrote:
> > I really don't want to get into it, but the networking code is the only
> > thing that allows this due to the userspace "want" to rename devices
> > because we can't create symlinks to network devices like we can for all
> > other devices (as they use device nodes.)
> > 
> > When this function is called, userspace had better know exactly what is
> > going on as it can get confused due to the lack of uevents happening.
> 
> Thanks for the explanation, but I'm just concerned that you'll add a comment
> that says, "don't call this function".  A quick grep will show that there are
> users of this function, and we'll have this exact conversation all over again.

Care to turn the above into a patch?  I'll be glad to apply that as well
:)

thanks,

greg k-h

Re: How do I choose an arbitrary minor number for my tty device?

[Tabi Timur-B04825]

Greg KH wrote:
> Care to turn the above into a patch?  I'll be glad to apply that as well

Sure, I'll take care of it tomorrow.

-- 
Timur Tabi
Linux kernel developer

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Greg KH wrote:
>>> > > When this function is called, userspace had better know exactly what is
>>> > > going on as it can get confused due to the lack of uevents happening.
>> > 
>> > Thanks for the explanation, but I'm just concerned that you'll add a comment
>> > that says, "don't call this function".  A quick grep will show that there are
>> > users of this function, and we'll have this exact conversation all over again.
> Care to turn the above into a patch?  I'll be glad to apply that as well

One question: why are uevents not happening?

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Greg KH]

On Tue, Nov 30, 2010 at 01:33:52PM -0600, Timur Tabi wrote:
> Greg KH wrote:
> >>> > > When this function is called, userspace had better know exactly what is
> >>> > > going on as it can get confused due to the lack of uevents happening.
> >> > 
> >> > Thanks for the explanation, but I'm just concerned that you'll add a comment
> >> > that says, "don't call this function".  A quick grep will show that there are
> >> > users of this function, and we'll have this exact conversation all over again.
> > Care to turn the above into a patch?  I'll be glad to apply that as well
> 
> One question: why are uevents not happening?

What uevent would describe this?  Userspace asked for the rename to
happen, so it better be aware that it did.

We don't want to do a "remove" and then "add" sequence of events, that
would confuse everyone.

thanks,

greg k-h

Re: How do I choose an arbitrary minor number for my tty device?

[Kay Sievers]

On Wed, Dec 1, 2010 at 02:00, Greg KH <greg@kroah.com> wrote:
> On Tue, Nov 30, 2010 at 01:33:52PM -0600, Timur Tabi wrote:
>> Greg KH wrote:
>> >>> > > When this function is called, userspace had better know exactly what is
>> >>> > > going on as it can get confused due to the lack of uevents happening.
>> >> >
>> >> > Thanks for the explanation, but I'm just concerned that you'll add a comment
>> >> > that says, "don't call this function".  A quick grep will show that there are
>> >> > users of this function, and we'll have this exact conversation all over again.
>> > Care to turn the above into a patch?  I'll be glad to apply that as well
>>
>> One question: why are uevents not happening?
>
> What uevent would describe this?  Userspace asked for the rename to
> happen, so it better be aware that it did.
>
> We don't want to do a "remove" and then "add" sequence of events, that
> would confuse everyone.

A bit more details:

First: never rename anything! :)

It's racy at many levels, symlinks and other stuff are not replaced
atomically, you get a "move" uevent", but it's not easy to connect the
event to the old and new device. Device nodes are not renamed at all,
there isn't even support for that in the kernel now.

In the meantime during renaming, your target name might be taken by
another driver, creating conflicts. Or the old name is taken directly
after you renamed it -- then you get events for the same DEVPATH,
before you even seet the "move" event. It's just a mess, and nothing
new should ever rely on kernel device renaming. Besides that it's not
even implemented now for other things than (driver-core wise very
simple) network devices.

We are currently about to change network renaming in udev to
completely disallow renaming of devices in the same namespace as the
kernel uses, because we can't solve the problems properly, that arise
with swapping names of multiple interfaces without races. Means,
renaming of eth[0-9]* will only be allowed to some other name than
eth[0-9]*, for the mentioned reasons.

Make up a "real" name in the driver before you register anything, or
add some other attributes for userspace to find the device, or use
udev to add symlinks -- but never rename kernel devices later, it's a
complete mess. We don't even want to get into that and try to
implement the missing pieces in the core. We really have other pieces
to fix in the driver core mess. :)

Kay

Re: How do I choose an arbitrary minor number for my tty device?

[Timur Tabi]

Kay Sievers wrote:
> 
> It's racy at many levels, symlinks and other stuff are not replaced
> atomically, you get a "move" uevent", but it's not easy to connect the
> event to the old and new device. Device nodes are not renamed at all,
> there isn't even support for that in the kernel now.

Kay, thank you so much for this explanation.  I wish every function in the
kernel were described so thoroughly!  My life would be so much easier.

I put your text into a patch and posted it for review.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: How do I choose an arbitrary minor number for my tty device?

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 936 bytes --]

On Wed, 2010-11-24 at 12:08 -0600, Scott Wood wrote:
> On Wed, 24 Nov 2010 21:23:47 +1100
> Michael Ellerman <michael@ellerman.id.au> wrote:

> > > It would be nice if platform devices that are created from device tree
> > > nodes included a link to the corresponding /proc/device-tree node in
> > > their sysfs node.
> > 
> > It's not a link, but the OF path is in devspec, so you can work it out
> > fairly easily.
> 
> Hmm, I see a "devspec" in PCI devices, but not in devtree-probed
> platform devices.  of_bus_type_init isn't being called from anywhere
> but the ibmebus code.  It looks like this was a casualty of merging
> of_platform with platform (commit eca3930163ba8884060ce9d9ff5ef0d9b7c7b00f).

Ah crud, yeah I was looking at an old kernel.

That seems like a regression, but seemingly no one has complained so
perhaps it doesn't matter in practice. It was certainly a nice feature
though.

cheers


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: How do I choose an arbitrary minor number for my tty device?

[Grant Likely]

On Wed, Nov 24, 2010 at 3:46 PM, Michael Ellerman
<michael@ellerman.id.au> wrote:
> On Wed, 2010-11-24 at 12:08 -0600, Scott Wood wrote:
>> On Wed, 24 Nov 2010 21:23:47 +1100
>> Michael Ellerman <michael@ellerman.id.au> wrote:
>
>> > > It would be nice if platform devices that are created from device tree
>> > > nodes included a link to the corresponding /proc/device-tree node in
>> > > their sysfs node.
>> >
>> > It's not a link, but the OF path is in devspec, so you can work it out
>> > fairly easily.
>>
>> Hmm, I see a "devspec" in PCI devices, but not in devtree-probed
>> platform devices.  of_bus_type_init isn't being called from anywhere
>> but the ibmebus code.  It looks like this was a casualty of merging
>> of_platform with platform (commit eca3930163ba8884060ce9d9ff5ef0d9b7c7b00f).
>
> Ah crud, yeah I was looking at an old kernel.
>
> That seems like a regression, but seemingly no one has complained so
> perhaps it doesn't matter in practice. It was certainly a nice feature
> though.

Hmmm, I missed that when merging.  Oops.

devspec is easy enough to add back since there isn't a conflict.  It
can even be made system-wide (not just platform bus) since any device
can have an of_node now.  However, the modalias and name attributes
are a lot harder since there are name conflicts.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: How do I choose an arbitrary minor number for my tty device?

[Scott Wood]

[sorry for the resend, fixed the devtree list address]

On Wed, 24 Nov 2010 21:23:47 +1100
Michael Ellerman <michael@ellerman.id.au> wrote:

> On Thu, 2010-11-18 at 12:13 -0600, Scott Wood wrote:
> > Unfortunately, it's not an exact match, and the fact that reg doesn't
> > translate to a physical address means that AFAICT you'll currently get
> > something like "byte-channel.nnn", where "nnn" is an arbitrary
> > kernel-assigned number.
> 
> Can you not use device_rename() ?

Ah, didn't know about that.  Still, might be nice to add support for
"handle" nodes at the infrastructure level rather than in each driver.

> > It would be nice if platform devices that are created from device tree
> > nodes included a link to the corresponding /proc/device-tree node in
> > their sysfs node.
> 
> It's not a link, but the OF path is in devspec, so you can work it out
> fairly easily.

Hmm, I see a "devspec" in PCI devices, but not in devtree-probed
platform devices.  of_bus_type_init isn't being called from anywhere
but the ibmebus code.  It looks like this was a casualty of merging
of_platform with platform (commit eca3930163ba8884060ce9d9ff5ef0d9b7c7b00f).

-Scott