serial8250_init and platform_device

serial8250_init and platform_device

[Kumar Gala]

I dont get how it is you dont have more platform_devices register than 
you should based on how serial8250_init works if you have additional 
code registering a serial8250 device.  For example, 
arch/arm/mach-s3c2410/mach-vr1000.c will register one serial8250 
device, and it appears to me that serial8250_init will register a 2nd.  
Is this the expected behavior or am I missing something?

- kumar

Re: serial8250_init and platform_device

[Russell King]

On Thu, Jan 20, 2005 at 01:14:49AM -0600, Kumar Gala wrote:
> I dont get how it is you dont have more platform_devices register than 
> you should based on how serial8250_init works if you have additional 
> code registering a serial8250 device.  For example, 
> arch/arm/mach-s3c2410/mach-vr1000.c will register one serial8250 
> device, and it appears to me that serial8250_init will register a 2nd.  
> Is this the expected behavior or am I missing something?

I don't understand what you're saying, sorry.

serial8250_init() registers an "ISA compatibility" 8250 device for those
architectures which haven't converted themselves to the new scheme.

It then creates all the 8250 ports from 0..UART_NR.  In the case of an
architecture which doesn't have any SERIAL_PORT_DFNS defined (eg, ARM)
these are just dummy placeholder registrations.

We then register the device driver, which allows us to pick up on the
platform devices.  This causes the placeholder registrations to be
reassigned to the platform devices on a first come first served basis
via the standard registration call serial8250_register_port().

While you can have both the "ISA compatibility" scheme and the
"platform device" scheme contain the same port description, you'll
only end up with just the one port registered in the end.  That just
happens to be correct and desired behaviour.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: serial8250_init and platform_device

[Kumar Gala]

I'm trying to convert some PPC embedded code from using the old "ISA 
compat" style with SERIAL_PORT_DFNS to using platform_device.

> On Thu, Jan 20, 2005 at 01:14:49AM -0600, Kumar Gala wrote:
>  > I dont get how it is you dont have more platform_devices register 
> than
> > you should based on how serial8250_init works if you have additional
> > code registering a serial8250 device.  For example,
> > arch/arm/mach-s3c2410/mach-vr1000.c will register one serial8250
> > device, and it appears to me that serial8250_init will register a 
> 2nd. 
> > Is this the expected behavior or am I missing something?
>
> I don't understand what you're saying, sorry.

No problem, let me try to clarify.  I'm trying to figure out in the ARM 
case if there are 2 platform_devices that are registered and if this is 
the desired behavior (and if so why?).

> serial8250_init() registers an "ISA compatibility" 8250 device for 
> those
>  architectures which haven't converted themselves to the new scheme.
>
> It then creates all the 8250 ports from 0..UART_NR.  In the case of an
>  architecture which doesn't have any SERIAL_PORT_DFNS defined (eg, ARM)
>  these are just dummy placeholder registrations.

In serial8250_init() we call platform_device_register_simple(), this 
will be one registration of a serial8250 device.  In my example of 
vr1000, arch/arm/mach-s3c2410/cpu.c:s3c_arch_init() calls 
platform_device_register, the 2nd time a serial8250 device is 
registered.

> We then register the device driver, which allows us to pick up on the
>  platform devices.  This causes the placeholder registrations to be
>  reassigned to the platform devices on a first come first served basis
>  via the standard registration call serial8250_register_port().

I'm not following you here, its not clear if you mean we have 2 
platform devices registered in the system, but only one actually has 
serial ports that are registered.  If you are using SERIAL_PORT_DFNS, 
it will be the platform_device created in serial8250_init(), if you are 
not it will be the platform_device created elsewhere?

> While you can have both the "ISA compatibility" scheme and the
>  "platform device" scheme contain the same port description, you'll
>  only end up with just the one port registered in the end.  That just
>  happens to be correct and desired behaviour.

- kumar

Re: serial8250_init and platform_device

[Russell King]

On Thu, Jan 20, 2005 at 09:23:56AM -0600, Kumar Gala wrote:
> No problem, let me try to clarify.  I'm trying to figure out in the ARM 
> case if there are 2 platform_devices that are registered and if this is 
> the desired behavior (and if so why?).

Yes.  The first (by serial8250_init) is the ISA compatibility platform
device, which registers the "old" static list of serial devices found
in include/asm-*/serial.h.

The second registers a platform device which contains the 8250 serial
devices for the particular platform.

> In serial8250_init() we call platform_device_register_simple(), this 
> will be one registration of a serial8250 device.  In my example of 
> vr1000, arch/arm/mach-s3c2410/cpu.c:s3c_arch_init() calls 
> platform_device_register, the 2nd time a serial8250 device is 
> registered.

Correct.

> > We then register the device driver, which allows us to pick up on the
> >  platform devices.  This causes the placeholder registrations to be
> >  reassigned to the platform devices on a first come first served basis
> >  via the standard registration call serial8250_register_port().
> 
> I'm not following you here, its not clear if you mean we have 2 
> platform devices registered in the system, but only one actually has 
> serial ports that are registered.

We have two platform devices registered - one representing the
compatibility devices, and one (or more) representing the platform's
own devices.

To illustrate this, let's assume your architecture always has ports at
a fixed set of addresses, and then has a set of platform specific ports.
You may wish to have one platform device for the platform common serial
devices which always gets registered.  Your platform specific
initialisation code could then register another platform device which
contains details of the platform specific serial devices.

> If you are using SERIAL_PORT_DFNS, 
> it will be the platform_device created in serial8250_init(), if you are 
> not it will be the platform_device created elsewhere?

Initially, all serial devices are attached to the platform device
created in serial8250_init(), whether or not they are listed in
SERIAL_PORT_DFNS or not.  Serial devices not listed in SERIAL_PORT_DFNS
remain in an unconfigured state.

When the 8250 platform device driver is registered, serial devices
are "stolen" from this "private" platform device, and are owned by
the platform device which registered them.

Lets take some examples:

1. SERIAL_PORT_DFNS contains port at 0x3f8, irq4, and we have space for
   4 serial ports.  No other platform devices are registered.

# cat /proc/tty/driver/serial
serinfo:1.0 driver revision:
0: uart:16550A port:000003F8 irq:4 tx:0 rx:0
1: uart:unknown port:00000000 irq:0
2: uart:unknown port:00000000 irq:0
3: uart:unknown port:00000000 irq:0
# tree /sys/class/tty/ttyS*
/sys/class/tty/ttyS0
|-- dev
`-- device -> ../../../devices/platform/serial8250
/sys/class/tty/ttyS1
|-- dev
`-- device -> ../../../devices/platform/serial8250
/sys/class/tty/ttyS2
|-- dev
`-- device -> ../../../devices/platform/serial8250
/sys/class/tty/ttyS3
|-- dev
`-- device -> ../../../devices/platform/serial8250

2. SERIAL_PORT_DFNS contains port at 0x3f8, irq4, and we have space for
   4 serial ports.  A platform device also describing 0x3f8, irq 4 is
   registered.

# cat /proc/tty/driver/serial
serinfo:1.0 driver revision:
0: uart:16550A port:000003F8 irq:4 tx:0 rx:0
1: uart:unknown port:00000000 irq:0
2: uart:unknown port:00000000 irq:0
3: uart:unknown port:00000000 irq:0
# tree /sys/class/tty/ttyS*
/sys/class/tty/ttyS0
|-- dev
|-- device -> ../../../devices/platform/serial82500
`-- driver -> ../../../bus/platform/drivers/serial8250
/sys/class/tty/ttyS1
|-- dev
`-- device -> ../../../devices/platform/serial8250
/sys/class/tty/ttyS2
|-- dev
`-- device -> ../../../devices/platform/serial8250
/sys/class/tty/ttyS3
|-- dev
`-- device -> ../../../devices/platform/serial8250

3. SERIAL_PORT_DFNS is empty, otherwise the same as case (2).  Results
   are identical to case (2).

HTH.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: serial8250_init and platform_device

[Kumar Gala]

Russell,

I think this all makes sense to me.  I'm just wondering why we would 
have a platform device register in a system for 'legacy ISA' when we 
know the system doesnt have any ports that will fit the category.

As you show in example #2 you have

.../devices/platform/serial82500
.../devices/platform/serial8250

why have the 'serial8250' if you know your system doesnt have any ports 
that will exist there?

- kumar

On Jan 20, 2005, at 9:44 AM, Russell King wrote:

> On Thu, Jan 20, 2005 at 09:23:56AM -0600, Kumar Gala wrote:
>  > No problem, let me try to clarify.  I'm trying to figure out in the 
> ARM
>  > case if there are 2 platform_devices that are registered and if 
> this is
> > the desired behavior (and if so why?).
>
> Yes.  The first (by serial8250_init) is the ISA compatibility platform
>  device, which registers the "old" static list of serial devices found
>  in include/asm-*/serial.h.
>
> The second registers a platform device which contains the 8250 serial
>  devices for the particular platform.
>
> > In serial8250_init() we call platform_device_register_simple(), this
> > will be one registration of a serial8250 device.  In my example of
> > vr1000, arch/arm/mach-s3c2410/cpu.c:s3c_arch_init() calls
> > platform_device_register, the 2nd time a serial8250 device is
> > registered.
>
> Correct.
>
> > > We then register the device driver, which allows us to pick up on 
> the
>  > >  platform devices.  This causes the placeholder registrations to 
> be
>  > >  reassigned to the platform devices on a first come first served 
> basis
>  > >  via the standard registration call serial8250_register_port().
> >
> > I'm not following you here, its not clear if you mean we have 2
> > platform devices registered in the system, but only one actually has
> > serial ports that are registered.
>
> We have two platform devices registered - one representing the
>  compatibility devices, and one (or more) representing the platform's
>  own devices.
>
> To illustrate this, let's assume your architecture always has ports at
>  a fixed set of addresses, and then has a set of platform specific 
> ports.
>  You may wish to have one platform device for the platform common 
> serial
>  devices which always gets registered.  Your platform specific
>  initialisation code could then register another platform device which
>  contains details of the platform specific serial devices.
>
> > If you are using SERIAL_PORT_DFNS,
>  > it will be the platform_device created in serial8250_init(), if you 
> are
> > not it will be the platform_device created elsewhere?
>
> Initially, all serial devices are attached to the platform device
>  created in serial8250_init(), whether or not they are listed in
>  SERIAL_PORT_DFNS or not.  Serial devices not listed in 
> SERIAL_PORT_DFNS
> remain in an unconfigured state.
>
> When the 8250 platform device driver is registered, serial devices
>  are "stolen" from this "private" platform device, and are owned by
>  the platform device which registered them.
>
> Lets take some examples:
>
> 1. SERIAL_PORT_DFNS contains port at 0x3f8, irq4, and we have space for
>     4 serial ports.  No other platform devices are registered.
>
> # cat /proc/tty/driver/serial
> serinfo:1.0 driver revision:
>  0: uart:16550A port:000003F8 irq:4 tx:0 rx:0
>  1: uart:unknown port:00000000 irq:0
>  2: uart:unknown port:00000000 irq:0
>  3: uart:unknown port:00000000 irq:0
>  # tree /sys/class/tty/ttyS*
> /sys/class/tty/ttyS0
> |-- dev
>  `-- device -> ../../../devices/platform/serial8250
> /sys/class/tty/ttyS1
> |-- dev
>  `-- device -> ../../../devices/platform/serial8250
> /sys/class/tty/ttyS2
> |-- dev
>  `-- device -> ../../../devices/platform/serial8250
> /sys/class/tty/ttyS3
> |-- dev
>  `-- device -> ../../../devices/platform/serial8250
>
> 2. SERIAL_PORT_DFNS contains port at 0x3f8, irq4, and we have space for
>     4 serial ports.  A platform device also describing 0x3f8, irq 4 is
>     registered.
>
> # cat /proc/tty/driver/serial
> serinfo:1.0 driver revision:
>  0: uart:16550A port:000003F8 irq:4 tx:0 rx:0
>  1: uart:unknown port:00000000 irq:0
>  2: uart:unknown port:00000000 irq:0
>  3: uart:unknown port:00000000 irq:0
>  # tree /sys/class/tty/ttyS*
> /sys/class/tty/ttyS0
> |-- dev
>  |-- device -> ../../../devices/platform/serial82500
> `-- driver -> ../../../bus/platform/drivers/serial8250
> /sys/class/tty/ttyS1
> |-- dev
>  `-- device -> ../../../devices/platform/serial8250
> /sys/class/tty/ttyS2
> |-- dev
>  `-- device -> ../../../devices/platform/serial8250
> /sys/class/tty/ttyS3
> |-- dev
>  `-- device -> ../../../devices/platform/serial8250
>
> 3. SERIAL_PORT_DFNS is empty, otherwise the same as case (2).  Results
>     are identical to case (2).
>
> HTH.
>
> -- 
> Russell King
>   Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
>  maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
>                  2.6 Serial core

Re: serial8250_init and platform_device

[Russell King]

On Thu, Jan 20, 2005 at 01:06:55PM -0600, Kumar Gala wrote:
> Russell,
> 
> I think this all makes sense to me.  I'm just wondering why we would 
> have a platform device register in a system for 'legacy ISA' when we 
> know the system doesnt have any ports that will fit the category.
> 
> As you show in example #2 you have
> 
> .../devices/platform/serial82500
> .../devices/platform/serial8250
> 
> why have the 'serial8250' if you know your system doesnt have any ports 
> that will exist there?

In this case, it is a placeholder, and needs to be there if you're using
power management.

For instance, you may use setserial on /dev/ttyS2 to reconfigure it
to an address where you know a serial port is.  Without the "serial8250"
device, it isn't linked into the device model, and therefore doesn't
receive any power management notifications.

Once the SERIAL_PORT_DFNS are gone, and we have a more modern interface
than setserial for setting up random ports, this "serial8250" device
will vanish.

While we're here, you've reminded me about an annoying point about
platform device naming...

Greg - the name is constructed from "name" + "id num" thusly:

	serial8250
	serial82500
	serial82501
	serial82502

When "name" ends in a number, it gets rather confusing.  Can we have
an optional delimiter in there when we append the ID number, maybe
something like a '.' or ':' ?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: serial8250_init and platform_device

[Greg KH]

On Thu, Jan 20, 2005 at 07:38:45PM +0000, Russell King wrote:
> 
> Greg - the name is constructed from "name" + "id num" thusly:
> 
> 	serial8250
> 	serial82500
> 	serial82501
> 	serial82502
> 
> When "name" ends in a number, it gets rather confusing.  Can we have
> an optional delimiter in there when we append the ID number, maybe
> something like a '.' or ':' ?

Sure, that's fine with me.  Someone send me a patch :)

thanks,

greg k-h

Re: serial8250_init and platform_device

[Russell King]

On Thu, Jan 20, 2005 at 11:50:58AM -0800, Greg KH wrote:
> On Thu, Jan 20, 2005 at 07:38:45PM +0000, Russell King wrote:
> > 
> > Greg - the name is constructed from "name" + "id num" thusly:
> > 
> > 	serial8250
> > 	serial82500
> > 	serial82501
> > 	serial82502
> > 
> > When "name" ends in a number, it gets rather confusing.  Can we have
> > an optional delimiter in there when we append the ID number, maybe
> > something like a '.' or ':' ?
> 
> Sure, that's fine with me.  Someone send me a patch :)

Like this?
-

Separate platform device name from platform device number such that
names ending with numbers aren't confusing.

Signed-off-by: Russell King <rmk@arm.linux.org.uk>

--- orig/drivers/base/platform.c	Wed Jan 12 10:11:20 2005
+++ linux/drivers/base/platform.c	Thu Jan 20 20:08:53 2005
@@ -131,7 +131,7 @@ int platform_device_register(struct plat
 	pdev->dev.bus = &platform_bus_type;
 
 	if (pdev->id != -1)
-		snprintf(pdev->dev.bus_id, BUS_ID_SIZE, "%s%u", pdev->name, pdev->id);
+		snprintf(pdev->dev.bus_id, BUS_ID_SIZE, "%s.%u", pdev->name, pdev->id);
 	else
 		strlcpy(pdev->dev.bus_id, pdev->name, BUS_ID_SIZE);
 

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: serial8250_init and platform_device

[Kumar Gala]

agreed, the lack of a delimiter in the naming was annoying.

- kumar

On Jan 20, 2005, at 2:10 PM, Russell King wrote:

> On Thu, Jan 20, 2005 at 11:50:58AM -0800, Greg KH wrote:
>  > On Thu, Jan 20, 2005 at 07:38:45PM +0000, Russell King wrote:
>  > >
> > > Greg - the name is constructed from "name" + "id num" thusly:
>  > >
> > >     serial8250
>  > >     serial82500
>  > >     serial82501
>  > >     serial82502
>  > >
> > > When "name" ends in a number, it gets rather confusing.  Can we 
> have
>  > > an optional delimiter in there when we append the ID number, maybe
>  > > something like a '.' or ':' ?
>  >
> > Sure, that's fine with me.  Someone send me a patch :)
>
> Like this?
>  -
>
> Separate platform device name from platform device number such that
>  names ending with numbers aren't confusing.
>
> Signed-off-by: Russell King <rmk@arm.linux.org.uk>
>
> --- orig/drivers/base/platform.c        Wed Jan 12 10:11:20 2005
> +++ linux/drivers/base/platform.c       Thu Jan 20 20:08:53 2005
> @@ -131,7 +131,7 @@ int platform_device_register(struct plat
>          pdev->dev.bus = &platform_bus_type;
>  
>          if (pdev->id != -1)
>  -               snprintf(pdev->dev.bus_id, BUS_ID_SIZE, "%s%u", 
> pdev->name, pdev->id);
>  +               snprintf(pdev->dev.bus_id, BUS_ID_SIZE, "%s.%u", 
> pdev->name, pdev->id);
>          else
>                  strlcpy(pdev->dev.bus_id, pdev->name, BUS_ID_SIZE);
>  
>
> -- 
> Russell King
>   Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
>  maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
>                  2.6 Serial core

Re: serial8250_init and platform_device

[Kumar Gala]

Good, I can understand the need to maintain compatibility until we get 
ride of SERIAL_PORT_DFNS.

- kumar

On Jan 20, 2005, at 1:38 PM, Russell King wrote:

> On Thu, Jan 20, 2005 at 01:06:55PM -0600, Kumar Gala wrote:
>  > Russell,
>  >
> > I think this all makes sense to me.  I'm just wondering why we would
> > have a platform device register in a system for 'legacy ISA' when we
> > know the system doesnt have any ports that will fit the category.
>  >
> > As you show in example #2 you have
>  >
> > .../devices/platform/serial82500
> > .../devices/platform/serial8250
> >
> > why have the 'serial8250' if you know your system doesnt have any 
> ports
>  > that will exist there?
>
> In this case, it is a placeholder, and needs to be there if you're 
> using
>  power management.
>
> For instance, you may use setserial on /dev/ttyS2 to reconfigure it
>  to an address where you know a serial port is.  Without the 
> "serial8250"
>  device, it isn't linked into the device model, and therefore doesn't
>  receive any power management notifications.
>
> Once the SERIAL_PORT_DFNS are gone, and we have a more modern interface
>  than setserial for setting up random ports, this "serial8250" device
>  will vanish.
>
> While we're here, you've reminded me about an annoying point about
>  platform device naming...
>
> Greg - the name is constructed from "name" + "id num" thusly:
>
>         serial8250
>          serial82500
>          serial82501
>          serial82502
>
> When "name" ends in a number, it gets rather confusing.  Can we have
>  an optional delimiter in there when we append the ID number, maybe
>  something like a '.' or ':' ?
>
> -- 
> Russell King
>   Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
>  maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
>                  2.6 Serial core

Re: serial8250_init and platform_device

[Greg KH]

On Thu, Jan 20, 2005 at 08:10:59PM +0000, Russell King wrote:
> On Thu, Jan 20, 2005 at 11:50:58AM -0800, Greg KH wrote:
> > On Thu, Jan 20, 2005 at 07:38:45PM +0000, Russell King wrote:
> > > 
> > > Greg - the name is constructed from "name" + "id num" thusly:
> > > 
> > > 	serial8250
> > > 	serial82500
> > > 	serial82501
> > > 	serial82502
> > > 
> > > When "name" ends in a number, it gets rather confusing.  Can we have
> > > an optional delimiter in there when we append the ID number, maybe
> > > something like a '.' or ':' ?
> > 
> > Sure, that's fine with me.  Someone send me a patch :)
> 
> Like this?
> -
> 
> Separate platform device name from platform device number such that
> names ending with numbers aren't confusing.
> 
> Signed-off-by: Russell King <rmk@arm.linux.org.uk>

Looks good to me, applied.

thanks,

greg k-h