2006-01-11 06:59:10

by Dmitry Torokhov

[permalink] [raw]
Subject: [patch 6/6] serial8250: convert to the new platform device interface

serial8250: convert to the new platform device interface

Do not use platform_device_register_simple() as it is going away.
Also set up driver's owner to create link driver->module in sysfs.

Signed-off-by: Dmitry Torokhov <[email protected]>
---

drivers/serial/8250.c | 30 ++++++++++++++++++++----------
1 files changed, 20 insertions(+), 10 deletions(-)

Index: work/drivers/serial/8250.c
===================================================================
--- work.orig/drivers/serial/8250.c
+++ work/drivers/serial/8250.c
@@ -2453,6 +2453,7 @@ static struct platform_driver serial8250
.resume = serial8250_resume,
.driver = {
.name = "serial8250",
+ .owner = THIS_MODULE,
},
};

@@ -2593,21 +2594,30 @@ static int __init serial8250_init(void)
if (ret)
goto out;

- serial8250_isa_devs = platform_device_register_simple("serial8250",
- PLAT8250_DEV_LEGACY, NULL, 0);
- if (IS_ERR(serial8250_isa_devs)) {
- ret = PTR_ERR(serial8250_isa_devs);
- goto unreg;
+ ret = platform_driver_register(&serial8250_isa_driver);
+ if (ret)
+ goto unreg_uart_drv;
+
+ serial8250_isa_devs = platform_device_alloc("serial8250",
+ PLAT8250_DEV_LEGACY);
+ if (!serial8250_isa_devs) {
+ ret = -ENOMEM;
+ goto unreg_plat_drv;
}

+ ret = platform_device_add(serial8250_isa_devs);
+ if (ret)
+ goto put_dev;
+
serial8250_register_ports(&serial8250_reg, &serial8250_isa_devs->dev);

- ret = platform_driver_register(&serial8250_isa_driver);
- if (ret == 0)
- goto out;
+ goto out;

- platform_device_unregister(serial8250_isa_devs);
- unreg:
+ put_dev:
+ platform_device_put(serial8250_isa_devs);
+ unreg_plat_drv:
+ platform_driver_unregister(&serial8250_isa_driver);
+ unreg_uart_drv:
uart_unregister_driver(&serial8250_reg);
out:
return ret;


2006-01-16 22:27:26

by Kumar Gala

[permalink] [raw]
Subject: Re: [patch 6/6] serial8250: convert to the new platform device interface

This patch is breaking arch/ppc & arch/powerpc usage of 8250.c. The
issue appears to be with the order in which platform_driver_register
() is called vs platform_device_add().

arch/powerpc/kernel/legacy_serial.c registers an 8250 device on the
platform bus before 8250_init() gets called.

Changing the order of platform_driver_register() vs
platform_device_add() fixes the issue. I'm still not sure what the
correct solution to this is. Ideas? comments?

- kumar

On Jan 11, 2006, at 12:47 AM, Dmitry Torokhov wrote:

> serial8250: convert to the new platform device interface
>
> Do not use platform_device_register_simple() as it is going away.
> Also set up driver's owner to create link driver->module in sysfs.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
>
> drivers/serial/8250.c | 30 ++++++++++++++++++++----------
> 1 files changed, 20 insertions(+), 10 deletions(-)
>
> Index: work/drivers/serial/8250.c
> ===================================================================
> --- work.orig/drivers/serial/8250.c
> +++ work/drivers/serial/8250.c
> @@ -2453,6 +2453,7 @@ static struct platform_driver serial8250
> .resume = serial8250_resume,
> .driver = {
> .name = "serial8250",
> + .owner = THIS_MODULE,
> },
> };
>
> @@ -2593,21 +2594,30 @@ static int __init serial8250_init(void)
> if (ret)
> goto out;
>
> - serial8250_isa_devs = platform_device_register_simple("serial8250",
> - PLAT8250_DEV_LEGACY, NULL, 0);
> - if (IS_ERR(serial8250_isa_devs)) {
> - ret = PTR_ERR(serial8250_isa_devs);
> - goto unreg;
> + ret = platform_driver_register(&serial8250_isa_driver);
> + if (ret)
> + goto unreg_uart_drv;
> +
> + serial8250_isa_devs = platform_device_alloc("serial8250",
> + PLAT8250_DEV_LEGACY);
> + if (!serial8250_isa_devs) {
> + ret = -ENOMEM;
> + goto unreg_plat_drv;
> }
>
> + ret = platform_device_add(serial8250_isa_devs);
> + if (ret)
> + goto put_dev;
> +
> serial8250_register_ports(&serial8250_reg, &serial8250_isa_devs-
> >dev);
>
> - ret = platform_driver_register(&serial8250_isa_driver);
> - if (ret == 0)
> - goto out;
> + goto out;
>
> - platform_device_unregister(serial8250_isa_devs);
> - unreg:
> + put_dev:
> + platform_device_put(serial8250_isa_devs);
> + unreg_plat_drv:
> + platform_driver_unregister(&serial8250_isa_driver);
> + unreg_uart_drv:
> uart_unregister_driver(&serial8250_reg);
> out:
> return ret;
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-
> kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2006-01-16 23:31:56

by Russell King

[permalink] [raw]
Subject: Re: [patch 6/6] serial8250: convert to the new platform device interface

On Mon, Jan 16, 2006 at 04:27:17PM -0600, Kumar Gala wrote:
> This patch is breaking arch/ppc & arch/powerpc usage of 8250.c. The
> issue appears to be with the order in which platform_driver_register
> () is called vs platform_device_add().
>
> arch/powerpc/kernel/legacy_serial.c registers an 8250 device on the
> platform bus before 8250_init() gets called.
>
> Changing the order of platform_driver_register() vs
> platform_device_add() fixes the issue. I'm still not sure what the
> correct solution to this is. Ideas? comments?

Mea Culpa - should've spotted that - that patch is actually rather
broken. platform_driver_register() can't be moved from where it
initially was.

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -2595,15 +2595,11 @@ static int __init serial8250_init(void)
if (ret)
goto out;

- ret = platform_driver_register(&serial8250_isa_driver);
- if (ret)
- goto unreg_uart_drv;
-
serial8250_isa_devs = platform_device_alloc("serial8250",
PLAT8250_DEV_LEGACY);
if (!serial8250_isa_devs) {
ret = -ENOMEM;
- goto unreg_plat_drv;
+ goto unreg_uart_drv;
}

ret = platform_device_add(serial8250_isa_devs);
@@ -2612,12 +2608,13 @@ static int __init serial8250_init(void)

serial8250_register_ports(&serial8250_reg, &serial8250_isa_devs->dev);

- goto out;
+ ret = platform_driver_register(&serial8250_isa_driver);
+ if (ret == 0)
+ goto out;

+ platform_device_del(serial8250_isa_devs);
put_dev:
platform_device_put(serial8250_isa_devs);
- unreg_plat_drv:
- platform_driver_unregister(&serial8250_isa_driver);
unreg_uart_drv:
uart_unregister_driver(&serial8250_reg);
out:


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

2006-01-17 00:02:09

by Kumar Gala

[permalink] [raw]
Subject: Re: [patch 6/6] serial8250: convert to the new platform device interface

On Mon, 16 Jan 2006, Russell King wrote:

> On Mon, Jan 16, 2006 at 04:27:17PM -0600, Kumar Gala wrote:
> > This patch is breaking arch/ppc & arch/powerpc usage of 8250.c. The
> > issue appears to be with the order in which platform_driver_register
> > () is called vs platform_device_add().
> >
> > arch/powerpc/kernel/legacy_serial.c registers an 8250 device on the
> > platform bus before 8250_init() gets called.
> >
> > Changing the order of platform_driver_register() vs
> > platform_device_add() fixes the issue. I'm still not sure what the
> > correct solution to this is. Ideas? comments?
>
> Mea Culpa - should've spotted that - that patch is actually rather
> broken. platform_driver_register() can't be moved from where it
> initially was.

This seems to fix my issue on arch/powerpc and arch/ppc, please push to
Linus ASAP.

thanks

- kumar

> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -2595,15 +2595,11 @@ static int __init serial8250_init(void)
> if (ret)
> goto out;
>
> - ret = platform_driver_register(&serial8250_isa_driver);
> - if (ret)
> - goto unreg_uart_drv;
> -
> serial8250_isa_devs = platform_device_alloc("serial8250",
> PLAT8250_DEV_LEGACY);
> if (!serial8250_isa_devs) {
> ret = -ENOMEM;
> - goto unreg_plat_drv;
> + goto unreg_uart_drv;
> }
>
> ret = platform_device_add(serial8250_isa_devs);
> @@ -2612,12 +2608,13 @@ static int __init serial8250_init(void)
>
> serial8250_register_ports(&serial8250_reg, &serial8250_isa_devs->dev);
>
> - goto out;
> + ret = platform_driver_register(&serial8250_isa_driver);
> + if (ret == 0)
> + goto out;
>
> + platform_device_del(serial8250_isa_devs);
> put_dev:
> platform_device_put(serial8250_isa_devs);
> - unreg_plat_drv:
> - platform_driver_unregister(&serial8250_isa_driver);
> unreg_uart_drv:
> uart_unregister_driver(&serial8250_reg);
> out:
>
>
>

2006-01-17 02:35:44

by Tony Lindgren

[permalink] [raw]
Subject: Re: [patch 6/6] serial8250: convert to the new platform device interface

* Kumar Gala <[email protected]> [060116 16:02]:
> On Mon, 16 Jan 2006, Russell King wrote:
>
> > On Mon, Jan 16, 2006 at 04:27:17PM -0600, Kumar Gala wrote:
> > > This patch is breaking arch/ppc & arch/powerpc usage of 8250.c. The
> > > issue appears to be with the order in which platform_driver_register
> > > () is called vs platform_device_add().
> > >
> > > arch/powerpc/kernel/legacy_serial.c registers an 8250 device on the
> > > platform bus before 8250_init() gets called.
> > >
> > > Changing the order of platform_driver_register() vs
> > > platform_device_add() fixes the issue. I'm still not sure what the
> > > correct solution to this is. Ideas? comments?
> >
> > Mea Culpa - should've spotted that - that patch is actually rather
> > broken. platform_driver_register() can't be moved from where it
> > initially was.
>
> This seems to fix my issue on arch/powerpc and arch/ppc, please push to
> Linus ASAP.

This patch fixes problems on omap too.

Tony

2006-01-17 19:36:17

by Olaf Hering

[permalink] [raw]
Subject: Re: [patch 6/6] serial8250: convert to the new platform device interface

On Mon, Jan 16, Kumar Gala wrote:

> > Mea Culpa - should've spotted that - that patch is actually rather
> > broken. platform_driver_register() can't be moved from where it
> > initially was.
>
> This seems to fix my issue on arch/powerpc and arch/ppc, please push to
> Linus ASAP.

This fixes also my pseries, p270. Too bad, the 8250 depends on
CONFIG_ISA now which is not selectable for CONFIG_PPC_PSERIES.
Is this patch the way to go for ppc64?

Index: linux-2.6.15/arch/powerpc/Kconfig
===================================================================
--- linux-2.6.15.orig/arch/powerpc/Kconfig
+++ linux-2.6.15/arch/powerpc/Kconfig
@@ -712,7 +712,7 @@ menu "Bus options"

config ISA
bool "Support for ISA-bus hardware"
- depends on PPC_PREP || PPC_CHRP
+ depends on PPC_PREP || PPC_CHRP || PPC_PSERIES
select PPC_I8259
help
Find out whether you have ISA slots on your motherboard. ISA is the


--
short story of a lazy sysadmin:
alias appserv=wotan

2006-01-17 21:41:45

by Grant Likely

[permalink] [raw]
Subject: Re: [patch 6/6] serial8250: convert to the new platform device interface

Kumar Gala wrote:
> On Mon, 16 Jan 2006, Russell King wrote:
>>On Mon, Jan 16, 2006 at 04:27:17PM -0600, Kumar Gala wrote:
>>
>>Mea Culpa - should've spotted that - that patch is actually rather
>>broken. platform_driver_register() can't be moved from where it
>>initially was.
>
>
> This seems to fix my issue on arch/powerpc and arch/ppc, please push to
> Linus ASAP.

Ditto for ML403 (Virtex), thanks

g.

>
>
>>diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
>>--- a/drivers/serial/8250.c
>>+++ b/drivers/serial/8250.c
>>@@ -2595,15 +2595,11 @@ static int __init serial8250_init(void)
>> if (ret)
>> goto out;
>>
>>- ret = platform_driver_register(&serial8250_isa_driver);
>>- if (ret)
>>- goto unreg_uart_drv;
>>-
>> serial8250_isa_devs = platform_device_alloc("serial8250",
>> PLAT8250_DEV_LEGACY);
>> if (!serial8250_isa_devs) {
>> ret = -ENOMEM;
>>- goto unreg_plat_drv;
>>+ goto unreg_uart_drv;
>> }
>>
>> ret = platform_device_add(serial8250_isa_devs);
>>@@ -2612,12 +2608,13 @@ static int __init serial8250_init(void)
>>
>> serial8250_register_ports(&serial8250_reg, &serial8250_isa_devs->dev);
>>
>>- goto out;
>>+ ret = platform_driver_register(&serial8250_isa_driver);
>>+ if (ret == 0)
>>+ goto out;
>>
>>+ platform_device_del(serial8250_isa_devs);
>> put_dev:
>> platform_device_put(serial8250_isa_devs);
>>- unreg_plat_drv:
>>- platform_driver_unregister(&serial8250_isa_driver);
>> unreg_uart_drv:
>> uart_unregister_driver(&serial8250_reg);
>> out:
>>
>>
>>
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>
>

2006-01-18 09:57:04

by Russell King

[permalink] [raw]
Subject: Re: [patch 6/6] serial8250: convert to the new platform device interface

On Tue, Jan 17, 2006 at 08:36:04PM +0100, Olaf Hering wrote:
> On Mon, Jan 16, Kumar Gala wrote:
> > > Mea Culpa - should've spotted that - that patch is actually rather
> > > broken. platform_driver_register() can't be moved from where it
> > > initially was.
> >
> > This seems to fix my issue on arch/powerpc and arch/ppc, please push to
> > Linus ASAP.
>
> This fixes also my pseries, p270. Too bad, the 8250 depends on
> CONFIG_ISA now which is not selectable for CONFIG_PPC_PSERIES.
> Is this patch the way to go for ppc64?

8250 does not depend on ISA - at least not in mainline. If it did depend
on ISA, that would be completely wrong.

ISA should be set if you have ISA slots on your motherboard (as the help
says). The presence of 8250 devices does not depend on whether you have
ISA slots or not.

Patch rejected.

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

2006-01-18 10:10:55

by Olaf Hering

[permalink] [raw]
Subject: Re: [patch 6/6] serial8250: convert to the new platform device interface

On Wed, Jan 18, Russell King wrote:

> 8250 does not depend on ISA - at least not in mainline. If it did depend
> on ISA, that would be completely wrong.

Maybe something else is busted, cant get serial output without such
change.

--
short story of a lazy sysadmin:
alias appserv=wotan

2006-01-18 11:03:14

by Olaf Hering

[permalink] [raw]
Subject: Re: [patch 6/6] serial8250: convert to the new platform device interface

On Tue, Jan 17, Olaf Hering wrote:

> On Mon, Jan 16, Kumar Gala wrote:
>
> > > Mea Culpa - should've spotted that - that patch is actually rather
> > > broken. platform_driver_register() can't be moved from where it
> > > initially was.
> >
> > This seems to fix my issue on arch/powerpc and arch/ppc, please push to
> > Linus ASAP.


older pSeries systems with serial ports dont get any console output
after recent changes. CONFIG_ISA does not make sense for CONFIG_PPC_PSERIES
because it enables lots of old drivers.
Instead, remove the dependency on CONFIG_ISA from the serial port
discovery code.

Signed-off-by: Olaf Hering <[email protected]>

arch/powerpc/kernel/legacy_serial.c | 4 ----
1 files changed, 4 deletions(-)

Index: linux-2.6.16-rc1-olh/arch/powerpc/kernel/legacy_serial.c
===================================================================
--- linux-2.6.16-rc1-olh.orig/arch/powerpc/kernel/legacy_serial.c
+++ linux-2.6.16-rc1-olh/arch/powerpc/kernel/legacy_serial.c
@@ -134,7 +134,6 @@ static int __init add_legacy_soc_port(st
return add_legacy_port(np, -1, UPIO_MEM, addr, addr, NO_IRQ, flags);
}

-#ifdef CONFIG_ISA
static int __init add_legacy_isa_port(struct device_node *np,
struct device_node *isa_brg)
{
@@ -168,7 +167,6 @@ static int __init add_legacy_isa_port(st
return add_legacy_port(np, index, UPIO_PORT, reg[1], taddr, NO_IRQ, UPF_BOOT_AUTOCONF);

}
-#endif

#ifdef CONFIG_PCI
static int __init add_legacy_pci_port(struct device_node *np,
@@ -276,7 +274,6 @@ void __init find_legacy_serial_ports(voi
of_node_put(soc);
}

-#ifdef CONFIG_ISA
/* First fill our array with ISA ports */
for (np = NULL; (np = of_find_node_by_type(np, "serial"));) {
struct device_node *isa = of_get_parent(np);
@@ -287,7 +284,6 @@ void __init find_legacy_serial_ports(voi
}
of_node_put(isa);
}
-#endif

#ifdef CONFIG_PCI
/* Next, try to locate PCI ports */

--
short story of a lazy sysadmin:
alias appserv=wotan