[PATCH] custom PM support for 8250

[PATCH] custom PM support for 8250

[Vitaly Wool]

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

Greetings,
please find the patch that allows passing the pointer to custom power 
management routine (via platform_device) to 8250 serial driver.
Please note that the interface to the outer world (i. e. exported 
functions) remained the same.

Best regards,
   Vitaly

[-- Attachment #2: 8250.diff --]
[-- Type: text/x-patch, Size: 3863 bytes --]

Currently 8250 serial driver doesn't have means to register custom
(i. e. board-specific) power management functions.
This patch modifies the driver to provide such facility.

 drivers/serial/8250.c       |   47 +++++++++++++++++++++++++++++++++++++------- include/linux/serial_8250.h |    4 +++
 2 files changed, 44 insertions(+), 7 deletions(-)
 
Signed-off-by: Vitaly Wool <vitalhome@rbcmail.ru>

Index: linux-2.6.10/drivers/serial/8250.c
===================================================================
--- linux-2.6.10.orig/drivers/serial/8250.c
+++ linux-2.6.10/drivers/serial/8250.c
@@ -2360,6 +2360,8 @@
 	uart_resume_port(&serial8250_reg, &serial8250_ports[line].port);
 }
 
+static struct uart_8250_port *uart_8250_register_port(struct uart_port *port, int *line);
+
 /*
  * Register a set of serial devices attached to a platform device.  The
  * list is terminated with a zero flags entry, which means we expect
@@ -2369,6 +2371,8 @@
 {
 	struct plat_serial8250_port *p = dev->platform_data;
 	struct uart_port port;
+	struct uart_8250_port *uart;
+	int line;
 
 	memset(&port, 0, sizeof(struct uart_port));
 
@@ -2386,7 +2390,9 @@
 		if (share_irqs)
 			port.flags |= UPF_SHARE_IRQ;
 
-		serial8250_register_port(&port);
+		uart = uart_8250_register_port(&port, &line);
+		if (!IS_ERR(uart))
+			uart->pm = p->pm;
 	}
 	return 0;
 }
@@ -2516,8 +2522,9 @@
 }
 
 /**
- *	serial8250_register_port - register a serial port
+ *	uart_8250_register_port - register a serial port
  *	@port: serial port template
+ *	@line: returned (on success) value of the line number
  *
  *	Configure the serial port specified by the request. If the
  *	port exists and is in use, it is hung up and unregistered
@@ -2526,19 +2533,20 @@
  *	The port is then probed and if necessary the IRQ is autodetected
  *	If this fails an error is returned.
  *
- *	On success the port is ready to use and the line number is returned.
+ *	On success the port is ready to use and the pointer to the 
+ *	corresponding struct uart_8250_port is returned.
  */
-int serial8250_register_port(struct uart_port *port)
+static struct uart_8250_port *uart_8250_register_port(struct uart_port *port, int *line)
 {
 	struct uart_8250_port *uart;
 	int ret = -ENOSPC;
 
 	if (port->uartclk == 0)
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
 	/* Avoid re-registering platform based ports if KGDB active */
 	if (port->line == kgdb8250_ttyS)
-		return -EBUSY;
+		return ERR_PTR(-EBUSY);
 
 	down(&serial_sem);
 
@@ -2560,10 +2568,35 @@
 
 		ret = uart_add_one_port(&serial8250_reg, &uart->port);
 		if (ret == 0)
-			ret = uart->port.line;
+			*line = uart->port.line;
 	}
 	up(&serial_sem);
 
+	return ret == 0 ? uart : ERR_PTR(ret);
+
+}
+
+/**
+ *	serial8250_register_port - register a serial port
+ *	@port: serial port template
+ *
+ *	Configure the serial port specified by the request. If the
+ *	port exists and is in use, it is hung up and unregistered
+ *	first.
+ *
+ *	The port is then probed and if necessary the IRQ is autodetected
+ *	If this fails an error is returned.
+ *
+ *	On success the port is ready to use and the line number is returned.
+ */
+int serial8250_register_port(struct uart_port *port)
+{
+	int ret = 0;
+	struct uart_8250_port *uart = uart_8250_register_port(port, &ret);
+
+	if (IS_ERR(uart))
+		ret = PTR_ERR(uart);
+
 	return ret;
 }
 EXPORT_SYMBOL(serial8250_register_port);
Index: linux-2.6.10/include/linux/serial_8250.h
===================================================================
--- linux-2.6.10.orig/include/linux/serial_8250.h
+++ linux-2.6.10/include/linux/serial_8250.h
@@ -24,6 +24,10 @@
 	unsigned char	iotype;		/* UPIO_* */
 	unsigned int	flags;		/* UPF_* flags */
 	unsigned int	line;		/* uart # */
+
+	/* per-port pm hook */
+	void			(*pm)(struct uart_port *port,
+				      unsigned int state, unsigned int old);
 };
 
 #endif

Re: [PATCH] custom PM support for 8250

[Vitaly Wool]

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

Sorry for format. Attached is the better one :)

Vitaly Wool wrote:

> Greetings,
> please find the patch that allows passing the pointer to custom power 
> management routine (via platform_device) to 8250 serial driver.
> Please note that the interface to the outer world (i. e. exported 
> functions) remained the same.
>
> Best regards,
>   Vitaly



[-- Attachment #2: 8250.diff --]
[-- Type: text/x-patch, Size: 3864 bytes --]

Currently 8250 serial driver doesn't have means to register custom
(i. e. board-specific) power management functions.
This patch modifies the driver to provide such facility.

 drivers/serial/8250.c       |   47 +++++++++++++++++++++++++++++++++++++-------
 include/linux/serial_8250.h |    4 +++
 2 files changed, 44 insertions(+), 7 deletions(-)
 
Signed-off-by: Vitaly Wool <vitalhome@rbcmail.ru>

Index: linux-2.6.10/drivers/serial/8250.c
===================================================================
--- linux-2.6.10.orig/drivers/serial/8250.c
+++ linux-2.6.10/drivers/serial/8250.c
@@ -2360,6 +2360,8 @@
 	uart_resume_port(&serial8250_reg, &serial8250_ports[line].port);
 }
 
+static struct uart_8250_port *uart_8250_register_port(struct uart_port *port, int *line);
+
 /*
  * Register a set of serial devices attached to a platform device.  The
  * list is terminated with a zero flags entry, which means we expect
@@ -2369,6 +2371,8 @@
 {
 	struct plat_serial8250_port *p = dev->platform_data;
 	struct uart_port port;
+	struct uart_8250_port *uart;
+	int line;
 
 	memset(&port, 0, sizeof(struct uart_port));
 
@@ -2386,7 +2390,9 @@
 		if (share_irqs)
 			port.flags |= UPF_SHARE_IRQ;
 
-		serial8250_register_port(&port);
+		uart = uart_8250_register_port(&port, &line);
+		if (!IS_ERR(uart))
+			uart->pm = p->pm;
 	}
 	return 0;
 }
@@ -2516,8 +2522,9 @@
 }
 
 /**
- *	serial8250_register_port - register a serial port
+ *	uart_8250_register_port - register a serial port
  *	@port: serial port template
+ *	@line: returned (on success) value of the line number
  *
  *	Configure the serial port specified by the request. If the
  *	port exists and is in use, it is hung up and unregistered
@@ -2526,19 +2533,20 @@
  *	The port is then probed and if necessary the IRQ is autodetected
  *	If this fails an error is returned.
  *
- *	On success the port is ready to use and the line number is returned.
+ *	On success the port is ready to use and the pointer to the 
+ *	corresponding struct uart_8250_port is returned.
  */
-int serial8250_register_port(struct uart_port *port)
+static struct uart_8250_port *uart_8250_register_port(struct uart_port *port, int *line)
 {
 	struct uart_8250_port *uart;
 	int ret = -ENOSPC;
 
 	if (port->uartclk == 0)
-		return -EINVAL;
+		return PTR_ERR(-EINVAL);
 
 	/* Avoid re-registering platform based ports if KGDB active */
 	if (port->line == kgdb8250_ttyS)
-		return -EBUSY;
+		return PTR_ERR(-EBUSY);
 
 	down(&serial_sem);
 
@@ -2560,10 +2568,35 @@
 
 		ret = uart_add_one_port(&serial8250_reg, &uart->port);
 		if (ret == 0)
-			ret = uart->port.line;
+			*line = uart->port.line;
 	}
 	up(&serial_sem);
 
+	return ret == 0 ? uart : PTR_ERR(ret);
+
+}
+
+/**
+ *	serial8250_register_port - register a serial port
+ *	@port: serial port template
+ *
+ *	Configure the serial port specified by the request. If the
+ *	port exists and is in use, it is hung up and unregistered
+ *	first.
+ *
+ *	The port is then probed and if necessary the IRQ is autodetected
+ *	If this fails an error is returned.
+ *
+ *	On success the port is ready to use and the line number is returned.
+ */
+int serial8250_register_port(struct uart_port *port)
+{
+	int ret = 0;
+	struct uart_8250_port *uart = uart_8250_register_port(port, &ret);
+
+	if (IS_ERR(uart))
+		ret = PTR_ERR(uart);
+
 	return ret;
 }
 EXPORT_SYMBOL(serial8250_register_port);
Index: linux-2.6.10/include/linux/serial_8250.h
===================================================================
--- linux-2.6.10.orig/include/linux/serial_8250.h
+++ linux-2.6.10/include/linux/serial_8250.h
@@ -24,6 +24,10 @@
 	unsigned char	iotype;		/* UPIO_* */
 	unsigned int	flags;		/* UPF_* flags */
 	unsigned int	line;		/* uart # */
+
+	/* per-port pm hook */
+	void			(*pm)(struct uart_port *port,
+				      unsigned int state, unsigned int old);
 };
 
 #endif

Re: [PATCH] custom PM support for 8250

[Russell King]

On Wed, Aug 31, 2005 at 03:10:09PM +0400, Vitaly Wool wrote:
> please find the patch that allows passing the pointer to custom power 
> management routine (via platform_device) to 8250 serial driver.
> Please note that the interface to the outer world (i. e. exported 
> functions) remained the same.

I'd rather change the structure passed via the platform device to
something like:

struct platform_serial_data {
	void	(*pm)(struct uart_port *port, unsigned int state, unsigned int old);
	int	nr_ports;
	struct plat_serial8250_port *ports;
};

which also eliminates the empty plat_serial8250_port terminator from
all the serial8250 platform devices (which appears to have caused some
folk problems.)

It does mean that a set of 8250 ports (grouped by each platform device)
have a common power management method - which seems a logical restriction.

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

Re: [PATCH] custom PM support for 8250

[Vitaly Wool]

Yes, that's also something I was thinking of, but I wasn't sure enough 
in this radical change :)
Anyway, I do agree that this way looks better than the current one.
So, if you don't object against other changes (say, the suggested 
approach to set uart->pm), I can proceed with the changes you suggest 
and work it out to the new patch. ;)

Best regards,
   Vitaly

Russell King wrote:

>On Wed, Aug 31, 2005 at 03:10:09PM +0400, Vitaly Wool wrote:
>  
>
>>please find the patch that allows passing the pointer to custom power 
>>management routine (via platform_device) to 8250 serial driver.
>>Please note that the interface to the outer world (i. e. exported 
>>functions) remained the same.
>>    
>>
>
>I'd rather change the structure passed via the platform device to
>something like:
>
>struct platform_serial_data {
>	void	(*pm)(struct uart_port *port, unsigned int state, unsigned int old);
>	int	nr_ports;
>	struct plat_serial8250_port *ports;
>};
>
>which also eliminates the empty plat_serial8250_port terminator from
>all the serial8250 platform devices (which appears to have caused some
>folk problems.)
>
>It does mean that a set of 8250 ports (grouped by each platform device)
>have a common power management method - which seems a logical restriction.
>
>  
>

Re: [PATCH] custom PM support for 8250

[Pavel Machek]

Hi!

> Greetings,
> please find the patch that allows passing the pointer to custom power 
> management routine (via platform_device) to 8250 serial driver.
> Please note that the interface to the outer world (i. e. exported 
> functions) remained the same.

No. Current state needs to be pm_message_t, not int.
				Pavel


-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         

Re: [PATCH] custom PM support for 8250

[Grigory Tolstolytkin]

Hi Russel,

I tried the patch provided by Vitaly Wool. And it works correctly. And 
now I'm successful with the PM support for my own serial8250 driver. Are 
you planning to commit the Vitlaly's changes into the mainstream? I 
guess it'll be helpful for the other people too. What I want is to know 
whether this changes will be supported by the Community or not. It's 
important for the project I'm worin on, cause I'm planning to push it 
into Open Source ;)

Thanks,
Grigory.

Russell King wrote:

>On Wed, Aug 31, 2005 at 03:10:09PM +0400, Vitaly Wool wrote:
>  
>
>>please find the patch that allows passing the pointer to custom power 
>>management routine (via platform_device) to 8250 serial driver.
>>Please note that the interface to the outer world (i. e. exported 
>>functions) remained the same.
>>    
>>
>
>I'd rather change the structure passed via the platform device to
>something like:
>
>struct platform_serial_data {
>	void	(*pm)(struct uart_port *port, unsigned int state, unsigned int old);
>	int	nr_ports;
>	struct plat_serial8250_port *ports;
>};
>
>which also eliminates the empty plat_serial8250_port terminator from
>all the serial8250 platform devices (which appears to have caused some
>folk problems.)
>
>It does mean that a set of 8250 ports (grouped by each platform device)
>have a common power management method - which seems a logical restriction.
>
>  
>

Re: [PATCH] custom PM support for 8250

[Vitaly Wool]

Russell,

what I'd suggest is to separate the changes that allow to provide PM 
callbacks and generic changes in the interface/structures as the latter 
will result in changing a lot of code that uses 8250 serial driver, 
otherwise that code will stop working.
Namely, is it posible to have the first patch I've sent for 8250 in 
place first?
I'll continue to work on 8250 PM/platform_serial_data changes then.

Vitaly

Grigory Tolstolytkin wrote:

> Hi Russel,
>
> I tried the patch provided by Vitaly Wool. And it works correctly. And 
> now I'm successful with the PM support for my own serial8250 driver. 
> Are you planning to commit the Vitlaly's changes into the mainstream? 
> I guess it'll be helpful for the other people too. What I want is to 
> know whether this changes will be supported by the Community or not. 
> It's important for the project I'm worin on, cause I'm planning to 
> push it into Open Source ;)
>
> Thanks,
> Grigory.
>
> Russell King wrote:
>
>> On Wed, Aug 31, 2005 at 03:10:09PM +0400, Vitaly Wool wrote:
>>  
>>
>>> please find the patch that allows passing the pointer to custom 
>>> power management routine (via platform_device) to 8250 serial driver.
>>> Please note that the interface to the outer world (i. e. exported 
>>> functions) remained the same.
>>>   
>>
>>
>> I'd rather change the structure passed via the platform device to
>> something like:
>>
>> struct platform_serial_data {
>>     void    (*pm)(struct uart_port *port, unsigned int state, 
>> unsigned int old);
>>     int    nr_ports;
>>     struct plat_serial8250_port *ports;
>> };
>>
>> which also eliminates the empty plat_serial8250_port terminator from
>> all the serial8250 platform devices (which appears to have caused some
>> folk problems.)
>>
>> It does mean that a set of 8250 ports (grouped by each platform device)
>> have a common power management method - which seems a logical 
>> restriction.
>>
>>  
>>
>