RE: Change to serial_core.c .... causing my serial driver problem Help Request

RE: Change to serial_core.c .... causing my serial driver problem Help Request

[Guennadi Liakhovetski]

On Tue, 15 Apr 2008, vcgandhi1@aol.com wrote:

> Will do,

I added linux-serial to cc.

> But it seems like the code that was check in has issues.

Given the number and names of signed-off's under this patch, it is pretty 
unlikely.

> If you look in 
> the directory no serial driver properly sets port->dev to be pointing to 
> the parent. It looks like they all point to themselves. Can you tell me 
> which driver you tested this change on. I can look to see how they set 
> port->dev.

8250_pci.c, and I do think it sets it properly in pciserial_init_ports().

Thanks
Guennadi

> Vipul?
> ?
> 
> -----Original Message-----
> From: Guennadi Liakhovetski [mailto:g.liakhovetski@gmx.de] 
> Sent: Tuesday, April 15, 2008 2:50 PM
> To: Gandhi, Vipul
> Subject: Re: Change to serial_core.c .... causing my serial driver problem Help Request
> 
> ?
> 
> On Tue, 15 Apr 2008, Gandhi, Vipul wrote:
> 
> ?
> 
> > For my driver the following call is returning a NULL .????????? 
> 
> > 
> 
> >? 
> 
> > 
> 
> > tty_dev = device_find_child(port->dev, &match, serial_match_port);
> 
> > 
> 
> >? 
> 
> > 
> 
> > Any idea? why, previously this was not in serial_core.c but was recently
> 
> > added by you I believe. Any help would be appreciated.
> 
> ?
> 
> No, not without seeing the source-code of your driver. Also, please, 
> 
> direct such questions to the linux-serial or linux-kernel mailing list, 
> 
> you may then additionally CC the person, who you think can help you best.
> 
> ?
> 
> Thanks
> 
> Guennadi
> 
> ---
> 
> Guennadi Liakhovetski
> 

---
Guennadi Liakhovetski

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[vcgandhi1]

Guennadi,

I think the issue is when the serial port is embedded and not connected 
through the bus. So I am talking about the drivers in the 
kernel/drivers/serial directory. A few of them use suspend and resume 
and I think they may not be compatible with your change. 

Do you know if you anybody tested any serial ports that are located in 
that directory and use  the suspend and resume calls. Ports not 
connected through the pci  bus.

Vipul



-----Original Message-----
From: vcgandhi1@aol.com
To: g.liakhovetski@gmx.de
Cc: linux-serial@vger.kernel.org
Sent: Tue, 15 Apr 2008 4:30 pm
Subject: Re: Change to serial_core.c .... causing my serial driver 
problem Help Request


Guennadi,

I think the issue is when the serial port is embedded and not connected 
through the bus. So I am talking about the drivers in the 
kernel/drivers/serial directory. A few of them use suspend and resume 
and I think they may not be compatible with your change. I not sure who 
signed off on in but I am having problems with the change and would 
like to like to get it resolved.  

Do you know if you anybody tested any serial ports that are located in 
that directory and use  the suspend and resume calls. Ports not 
connected through a bus.

Vipul




-----Original Message-----
From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
To: vcgandhi1@aol.com
Cc: linux-serial@vger.kernel.org
Sent: Tue, 15 Apr 2008 3:49 pm
Subject: RE: Change to serial_core.c .... causing my serial driver 
problem Help Request



On Tue, 15 Apr 2008, vcgandhi1@aol.com wrote:

> Will do,

I added linux-serial to cc.

> But it seems like the code that was check in has issues.

Given the number and names of signed-off's under this patch, it is 
pretty
unlikely.

> If you look in
> the directory no serial driver properly sets port->dev to be pointing 
to
> the parent. It looks like they all point to themselves. Can you tell 
me
> which driver you tested this change on. I can look to see how they 
set
> port->dev.

8250_pci.c, and I do think it sets it properly in 
pciserial_init_ports().

Thanks
Guennadi

> Vipul?
> ?
>
> -----Original Message-----
> From: Guennadi Liakhovetski [mailto:g.liakhovetski@gmx.de]
> Sent: Tuesday, April 15, 2008 2:50 PM
> To: Gandhi, Vipul
> Subject: Re: Change to serial_core.c .... causing my serial driver 
problem
Help Request
>
> ?
>
> On Tue, 15 Apr 2008, Gandhi, Vipul wrote:
>
> ?
>
> > For my driver the following call is returning a NULL .?????????
>
> >
>
> >?
>
> >
>
> > tty_dev = device_find_child(port->dev, &match, serial_match_port);
>
> >
>
> >?
>
> >
>
> > Any idea? why, previously this was not in serial_core.c but was 
recently
>
> > added by you I believe. Any help would be appreciated.
>
> ?
>
> No, not without seeing the source-code of your driver. Also, please,
>
> direct such questions to the linux-serial or linux-kernel mailing 
list,
>
> you may then additionally CC the person, who you think can help you 
best.
>
> ?
>
> Thanks
>
> Guennadi
>
> ---
>
> Guennadi Liakhovetski
>

---
Guennadi Liakhovetski
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[Guennadi Liakhovetski]

On Tue, 15 Apr 2008, vcgandhi1@aol.com wrote:

> I think the issue is when the serial port is embedded and not connected
> through the bus. So I am talking about the drivers in the
> kernel/drivers/serial directory. A few of them use suspend and resume and I
> think they may not be compatible with your change. 

Which specific driver do you see the problem with? I cannot check all 
drivers. I took as an example mpc52xx_uart.c. And I see there

	port->dev	= &dev->dev;

in its ->probe, so, it shouldn't have a problem either.

> Do you know if you anybody tested any serial ports that are located in that
> directory and use  the suspend and resume calls. Ports not connected through
> the pci  bus.

No, I don't know.

Thanks
Guennadi
---
Guennadi Liakhovetski
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[vcgandhi1]

Hi,

I do the same

port->dev   = &dev->dev;

But I am not sure &dev->dev points to the parent. I think it points to 
the device it's self, but I may be wrong.  I have tried port->dev = 
&dev->dev as well as port->dev = dev->dev.parent. One of these found 
something when the port was TTYS0 but did not work when the port was 
TTYS2.

Did you test with a non zero port->line. I working on ttyS2.


Just trying to narrow down what could be wrong. It just that    

tty_dev = device_find_child(port->dev, &match, serial_match_port);

is returning NULL for me.

Thank You for your help.
Vipul

PS. One quick suggestion you may want to check the return value and 
verify it is non-NULL before using it. Makes it easier to debug the 
code. ...


-----Original Message-----
From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
To: vcgandhi1@aol.com
Cc: linux-serial@vger.kernel.org
Sent: Wed, 16 Apr 2008 4:11 pm
Subject: Re: Change to serial_core.c .... causing my serial driver 
problem Help Request



On Tue, 15 Apr 2008, vcgandhi1@aol.com wrote:

> I think the issue is when the serial port is embedded and not 
connected
> through the bus. So I am talking about the drivers in the
> kernel/drivers/serial directory. A few of them use suspend and resume 
and I
> think they may not be compatible with your change. 

Which specific driver do you see the problem with? I cannot check all
drivers. I took as an example mpc52xx_uart.c. And I see there

    port->dev   = &dev->dev;

in its ->probe, so, it shouldn't have a problem either.

> Do you know if you anybody tested any serial ports that are located 
in that
> directory and use  the suspend and resume calls. Ports not connected 
through
> the pci  bus.

No, I don't know.

Thanks
Guennadi
---
Guennadi Liakhovetski

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[Guennadi Liakhovetski]

On Wed, 16 Apr 2008, vcgandhi1@aol.com wrote:

> I do the same
> 
> port->dev   = &dev->dev;
> 
> But I am not sure &dev->dev points to the parent. I think it points to the
> device it's self, but I may be wrong.  I have tried port->dev = &dev->dev as
> well as port->dev = dev->dev.parent.

It depends on what your "dev" is, and this is driver-specific. So, 
unfortunately, I don't think I'll be able to help you any further without 
seeing the sources.

> One of these found something when the
> port was TTYS0 but did not work when the port was TTYS2.
> 
> Did you test with a non zero port->line. I working on ttyS2.

Yes, I tested it with ttyS0 and ttyS1.

> Just trying to narrow down what could be wrong. It just that    
> tty_dev = device_find_child(port->dev, &match, serial_match_port);
> 
> is returning NULL for me.
> 
> Thank You for your help.
> Vipul
> 
> PS. One quick suggestion you may want to check the return value and verify it
> is non-NULL before using it. Makes it easier to debug the code. ...

Feel free to submit a patch.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[vcgandhi1]

Guennadi,

I think the problem is with dynamic Major and Minor Addresses.


This is the code from serial_core.c ...

"
static int serial_match_port(struct device *dev, void *data)
{
    struct uart_match *match = data;
    dev_t devt = MKDEV(match->driver->major, match->driver->minor) + 
match->port->line;

    return dev->devt == devt; /* Actually, only one tty per port */
}
"

Well when you uses Dynamic Major and Minor number you set it equal to 0 
in the driver structure of the UART.

Example.

"
static struct uart_driver uart_driver = {
    .owner                  = THIS_MODULE,
    .driver_name            = "msm_serial",
    .dev_name               = "ttyS0",
    .major                  = 0, /* tty layer will assign the major id */
    .minor                  = 0,
    .nr                     = UART_MAX_COUNT,
    .cons                   = uart_console,
};

"

You match function does not work. At least this is my thoughts.  What 
do you think.

Can you try to use  Dynamic Major and Minor address and see if you 
driver works.


Thank You
Vipul







-----Original Message-----
From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
To: vcgandhi1@aol.com
Cc: linux-serial@vger.kernel.org
Sent: Thu, 17 Apr 2008 7:55 am
Subject: Re: Change to serial_core.c .... causing my serial driver 
problem Help Request



On Wed, 16 Apr 2008, vcgandhi1@aol.com wrote:

> I do the same
>
> port->dev   = &dev->dev;
>
> But I am not sure &dev->dev points to the parent. I think it points 
to the
> device it's self, but I may be wrong.  I have tried port->dev = 
&dev->dev as
> well as port->dev = dev->dev.parent.

It depends on what your "dev" is, and this is driver-specific. So,
unfortunately, I don't think I'll be able to help you any further 
without
seeing the sources.

> One of these found something when the
> port was TTYS0 but did not work when the port was TTYS2.
>
> Did you test with a non zero port->line. I working on ttyS2.

Yes, I tested it with ttyS0 and ttyS1.

> Just trying to narrow down what could be wrong. It just that
> tty_dev = device_find_child(port->dev, &match, serial_match_port);
>
> is returning NULL for me.
>
> Thank You for your help.
> Vipul
>
> PS. One quick suggestion you may want to check the return value and 
verify it
> is non-NULL before using it. Makes it easier to debug the code. ...

Feel free to submit a patch.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[vcgandhi1]

dev_t devt = MKDEV(match->driver->major, match->driver->minor) + 
match->port->line;

One more thing, I use dynamic major and minor number could that be the 
problem. Does the driver you test use  dynamic Major and Minor Numbers?


Vipul
-----Original Message-----
From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
To: vcgandhi1@aol.com
Cc: linux-serial@vger.kernel.org
Sent: Wed, 16 Apr 2008 4:11 pm
Subject: Re: Change to serial_core.c .... causing my serial driver 
problem Help Request



On Tue, 15 Apr 2008, vcgandhi1@aol.com wrote:

> I think the issue is when the serial port is embedded and not 
connected
> through the bus. So I am talking about the drivers in the
> kernel/drivers/serial directory. A few of them use suspend and resume 
and I
> think they may not be compatible with your change. 

Which specific driver do you see the problem with? I cannot check all
drivers. I took as an example mpc52xx_uart.c. And I see there

    port->dev   = &dev->dev;

in its ->probe, so, it shouldn't have a problem either.

> Do you know if you anybody tested any serial ports that are located 
in that
> directory and use  the suspend and resume calls. Ports not connected 
through
> the pci  bus.

No, I don't know.

Thanks
Guennadi
---
Guennadi Liakhovetski

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[Guennadi Liakhovetski]

On Wed, 16 Apr 2008, vcgandhi1@aol.com wrote:

> dev_t devt = MKDEV(match->driver->major, match->driver->minor) +
> match->port->line;
> 
> One more thing, I use dynamic major and minor number could that be the
> problem. Does the driver you test use  dynamic Major and Minor Numbers?

Doesn't look like 8250.c uses dynamic major and minor numbers.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[vcgandhi1]

Guennadi,

When I switch to static Major and Minor numbers, the code started to 
work. So I believe the issue is with dynamic Major and Minor number.

I switch to static Major and Minor number till this gets fix or if it 
ever gets fixed. Let me know what your thoughts are maybe I doing 
something wrong which is causing Dynamic Major and Minor numbers not to 
work.

Thank You
Vipul



-----Original Message-----
From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
To: vcgandhi1@aol.com
Cc: linux-serial@vger.kernel.org
Sent: Thu, 17 Apr 2008 7:58 am
Subject: Re: Change to serial_core.c .... causing my serial driver 
problem Help Request



On Wed, 16 Apr 2008, vcgandhi1@aol.com wrote:

> dev_t devt = MKDEV(match->driver->major, match->driver->minor) +
> match->port->line;
>
> One more thing, I use dynamic major and minor number could that be the
> problem. Does the driver you test use  dynamic Major and Minor 
Numbers?

Doesn't look like 8250.c uses dynamic major and minor numbers.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[Guennadi Liakhovetski]

On Thu, 17 Apr 2008, vcgandhi1@aol.com wrote:

> When I switch to static Major and Minor numbers, the code started to work. So
> I believe the issue is with dynamic Major and Minor number.

I still don't understand. drivers/serial/jsm/jsm_driver.c does the same. 
But as it calls in its jsm_init_module:

uart_register_driver
tty_register_driver

where

	if (!driver->major) {
		error = alloc_chrdev_region(&dev, driver->minor_start,
						driver->num, driver->name);
		if (!error) {
			driver->major = MAJOR(dev);
			driver->minor_start = MINOR(dev);
		}

then it does get non-zero major and minor numbers. Doesn't this work for 
you?

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[vcgandhi1]

Guennadi,

My driver does the same as jsm_init_module. I would be very surprised 
if jsm actually worked with your change. You need to test it under 
suspend and resume condition.

My driver does the same uart_register_driver/tty_register_driver, but 
it looks like tty_register_driver sets the major address for the tty 
structure, but that does not get set back in the uart_driver structure. 
So uart_driver structure is still at zero.

To try to simplify this a little.

tty_register_driver will fill in the major and minor address in the 
following structure. struct tty_driver *driver

uart_register_driver input is struct uart_driver *drv. The value never 
gets set back into uart_driver and that is where you are looking to do 
the match.

To me this is either a bug in uart_register_driver for not propagating 
the value into uart_driver structure. But again it really did not 
matter since no one was looking at it till you modified the code.

Or your change should look at the tty_driver structure not the 
uart_driver structure, easy enough change. I do not think it is good 
for the lower level driver to fill in the value. It should not have to 
look at the tty structure it is suppose to be abstracted out.


Vipul

My recommendation is change you test driver to use 0 as the major 
address and do a suspend. It should crash.






-----Original Message-----
From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
To: vcgandhi1@aol.com
Cc: linux-serial@vger.kernel.org
Sent: Thu, 17 Apr 2008 10:19 am
Subject: Re: Change to serial_core.c .... causing my serial driver 
problem Help Request



On Thu, 17 Apr 2008, vcgandhi1@aol.com wrote:

> When I switch to static Major and Minor numbers, the code started to 
work. So
> I believe the issue is with dynamic Major and Minor number.

I still don't understand. drivers/serial/jsm/jsm_driver.c does the 
same.
But as it calls in its jsm_init_module:

uart_register_driver
tty_register_driver

where

if (!driver->major) {
error = alloc_chrdev_region(&dev, driver->minor_start,
driver->num, driver->name);
if (!error) {
driver->major = MAJOR(dev);
driver->minor_start = MINOR(dev);
}

then it does get non-zero major and minor numbers. Doesn't this work 
for
you?

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[Guennadi Liakhovetski]

On Thu, 17 Apr 2008, vcgandhi1@aol.com wrote:

> My driver does the same as jsm_init_module. I would be very surprised if jsm
> actually worked with your change. You need to test it under suspend and resume
> condition.
> 
> My driver does the same uart_register_driver/tty_register_driver, but it looks
> like tty_register_driver sets the major address for the tty structure, but
> that does not get set back in the uart_driver structure. So uart_driver
> structure is still at zero.

Indeed, you're right. One possible solution would be to pull the just 
obtained numbers into struct uart_driver in the specific serial driver 
like

diff --git a/drivers/serial/jsm/jsm_driver.c b/drivers/serial/jsm/jsm_driver.c
index 6767ee3..3afebab 100644
--- a/drivers/serial/jsm/jsm_driver.c
+++ b/drivers/serial/jsm/jsm_driver.c
@@ -225,6 +225,8 @@ static int __init jsm_init_module(void)
 
 	rc = uart_register_driver(&jsm_uart_driver);
 	if (!rc) {
+		jsm_uart_driver.major = jsm_uart_driver.tty_driver->major;
+		jsm_uart_driver.minor = jsm_uart_driver.tty_driver->minor_start;
 		rc = pci_register_driver(&jsm_driver);
 		if (rc)
 			uart_unregister_driver(&jsm_uart_driver);

You could do the same in your driver and see if this fixes your problem. 
However, maybe it is really better to fix it once for all drivers like

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 0f5a179..11e96bc 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2274,6 +2274,11 @@ int uart_register_driver(struct uart_driver *drv)
 	if (retval < 0) {
 		put_tty_driver(normal);
 		kfree(drv->state);
+	} else if (!drv->major) {
+		/* Driver uses dynamic major and minor numbers,
+		 * propagate the numbers, we just obtained, back */
+		drv->major = normal->major;
+		drv->minor = normal->minor_start;
 	}
 	return retval;
 }

Could you test this one too, please (without the previous one, of course)?

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[vcgandhi1]

FYI,

I have not had a chance to test it today, Sorry. The patches should 
solve the problem, but I will confirm on the hardware this weekend.

Vipul



-----Original Message-----
From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
To: vcgandhi1@aol.com
Cc: linux-serial@vger.kernel.org
Sent: Fri, 18 Apr 2008 8:04 am
Subject: Re: Change to serial_core.c .... causing my serial driver 
problem Help Request



On Thu, 17 Apr 2008, vcgandhi1@aol.com wrote:

> My driver does the same as jsm_init_module. I would be very surprised 
if jsm
> actually worked with your change. You need to test it under suspend 
and resume
> condition.
>
> My driver does the same uart_register_driver/tty_register_driver, but 
it looks
> like tty_register_driver sets the major address for the tty 
structure, but
> that does not get set back in the uart_driver structure. So 
uart_driver
> structure is still at zero.

Indeed, you're right. One possible solution would be to pull the just
obtained numbers into struct uart_driver in the specific serial driver
like

diff --git a/drivers/serial/jsm/jsm_driver.c 
b/drivers/serial/jsm/jsm_driver.c
index 6767ee3..3afebab 100644
--- a/drivers/serial/jsm/jsm_driver.c
+++ b/drivers/serial/jsm/jsm_driver.c
@@ -225,6 +225,8 @@ static int __init jsm_init_module(void)

    rc = uart_register_driver(&jsm_uart_driver);
    if (!rc) {
+       jsm_uart_driver.major = jsm_uart_driver.tty_driver->major;
+       jsm_uart_driver.minor = jsm_uart_driver.tty_driver->minor_start;
        rc = pci_register_driver(&jsm_driver);
        if (rc)
            uart_unregister_driver(&jsm_uart_driver);

You could do the same in your driver and see if this fixes your 
problem.
However, maybe it is really better to fix it once for all drivers like

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 0f5a179..11e96bc 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -2274,6 +2274,11 @@ int uart_register_driver(struct uart_driver *drv)
    if (retval < 0) {
        put_tty_driver(normal);
        kfree(drv->state);
+   } else if (!drv->major) {
+       /* Driver uses dynamic major and minor numbers,
+        * propagate the numbers, we just obtained, back */
+       drv->major = normal->major;
+       drv->minor = normal->minor_start;
    }
    return retval;
 }

Could you test this one too, please (without the previous one, of 
course)?

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[Guennadi Liakhovetski]

On Sat, 19 Apr 2008, vcgandhi1@aol.com wrote:

> I have not had a chance to test it today, Sorry. The patches should solve the
> problem, but I will confirm on the hardware this weekend.

Have you been able to test the patches? We need to know it if we want to 
fix it in the mainline.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[vcgandhi1]

Guennadi,

Yes that fixes the problem. Sorry for taking some time to test it, but 
been busy buying a house. The process is making me exhausted.

Please do try to get the patch into the mainline, and thank you for 
your time.

Vipul


-----Original Message-----
From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
To: vcgandhi1@aol.com
Cc: linux-serial@vger.kernel.org
Sent: Wed, 23 Apr 2008 2:09 am
Subject: Re: Change to serial_core.c .... causing my serial driver 
problem Help Request



On Sat, 19 Apr 2008, vcgandhi1@aol.com wrote:

> I have not had a chance to test it today, Sorry. The patches should 
solve the
> problem, but I will confirm on the hardware this weekend.

Have you been able to test the patches? We need to know it if we want 
to
fix it in the mainline.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[Guennadi Liakhovetski]

On Wed, 23 Apr 2008, vcgandhi1@aol.com wrote:

> Yes that fixes the problem. Sorry for taking some time to test it, but been
> busy buying a house. The process is making me exhausted.
> 
> Please do try to get the patch into the mainline, and thank you for your time.

Thanks for testing, I presume, you tested the generic patch, modifying 
drivers/serial/serial_core.c, not setting major and minor numbers in your 
driver. However, on a second thought, I think, you probably were right, 
suggesting to use major and minor numbers from the tty device in 
serial_match_port. So, if you just could confirm that the patch below also 
works for you, I'll submit it. Sorry for asking you again, I just cannot 
easily test it myself ATM. And if it does work, please add your 
"Tested-by:" to it.

Thanks
Guennadi
---
Guennadi Liakhovetski

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 0f5a179..593ae85 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1949,7 +1949,9 @@ struct uart_match {
 static int serial_match_port(struct device *dev, void *data)
 {
 	struct uart_match *match = data;
-	dev_t devt = MKDEV(match->driver->major, match->driver->minor) + match->port->line;
+	struct tty_driver *tty_drv = match->driver->tty_driver;
+	dev_t devt = MKDEV(tty_drv->major, tty_drv->minor_start) +
+		match->port->line;
 
 	return dev->devt == devt; /* Actually, only one tty per port */
 }

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[vcgandhi1]

FYI,

I will get the patch tested tomorrow.

Vipul


-----Original Message-----
From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
To: vcgandhi1@aol.com
Cc: linux-serial@vger.kernel.org
Sent: Thu, 24 Apr 2008 7:02 am
Subject: Re: Change to serial_core.c .... causing my serial driver 
problem Help Request



On Wed, 23 Apr 2008, vcgandhi1@aol.com wrote:

> Yes that fixes the problem. Sorry for taking some time to test it, 
but been
> busy buying a house. The process is making me exhausted.
>
> Please do try to get the patch into the mainline, and thank you for 
your time.

Thanks for testing, I presume, you tested the generic patch, modifying
drivers/serial/serial_core.c, not setting major and minor numbers in 
your
driver. However, on a second thought, I think, you probably were right,
suggesting to use major and minor numbers from the tty device in
serial_match_port. So, if you just could confirm that the patch below 
also
works for you, I'll submit it. Sorry for asking you again, I just 
cannot
easily test it myself ATM. And if it does work, please add your
"Tested-by:" to it.

Thanks
Guennadi
---
Guennadi Liakhovetski

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 0f5a179..593ae85 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1949,7 +1949,9 @@ struct uart_match {
 static int serial_match_port(struct device *dev, void *data)
 {
    struct uart_match *match = data;
-   dev_t devt = MKDEV(match->driver->major, match->driver->minor) +
match->port->line;
+   struct tty_driver *tty_drv = match->driver->tty_driver;
+   dev_t devt = MKDEV(tty_drv->major, tty_drv->minor_start) +
+       match->port->line;

    return dev->devt == devt; /* Actually, only one tty per port */
 }

Re: Change to serial_core.c .... causing my serial driver problem Help Request

[vcgandhi1]

This patch worked, sorry for the delay.


Tested-by:vcgandhi1@aol.com


Vipul Gandhi

-----Original Message-----
From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
To: vcgandhi1@aol.com
Cc: linux-serial@vger.kernel.org
Sent: Thu, 24 Apr 2008 7:02 am
Subject: Re: Change to serial_core.c .... causing my serial driver 
problem Help Request



On Wed, 23 Apr 2008, vcgandhi1@aol.com wrote:

> Yes that fixes the problem. Sorry for taking some time to test it, 
but been
> busy buying a house. The process is making me exhausted.
>
> Please do try to get the patch into the mainline, and thank you for 
your time.

Thanks for testing, I presume, you tested the generic patch, modifying
drivers/serial/serial_core.c, not setting major and minor numbers in 
your
driver. However, on a second thought, I think, you probably were right,
suggesting to use major and minor numbers from the tty device in
serial_match_port. So, if you just could confirm that the patch below 
also
works for you, I'll submit it. Sorry for asking you again, I just 
cannot
easily test it myself ATM. And if it does work, please add your
"Tested-by:" to it.

Thanks
Guennadi
---
Guennadi Liakhovetski

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 0f5a179..593ae85 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1949,7 +1949,9 @@ struct uart_match {
 static int serial_match_port(struct device *dev, void *data)
 {
    struct uart_match *match = data;
-   dev_t devt = MKDEV(match->driver->major, match->driver->minor) +
match->port->line;
+   struct tty_driver *tty_drv = match->driver->tty_driver;
+   dev_t devt = MKDEV(tty_drv->major, tty_drv->minor_start) +
+       match->port->line;

    return dev->devt == devt; /* Actually, only one tty per port */
 }

[PATCH] Fix serial_match_port() for dynamic major tty-device numbers

[Guennadi Liakhovetski]

As reported by Vipul Gandhi, the current serial_match_port() doesn't work 
for tty-devices using dynamic major number allocation. Fix it.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Tested-by: Vipul Gandhi <vcgandhi1@aol.com>

---

diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
index 0f5a179..593ae85 100644
--- a/drivers/serial/serial_core.c
+++ b/drivers/serial/serial_core.c
@@ -1949,7 +1949,9 @@ struct uart_match {
 static int serial_match_port(struct device *dev, void *data)
 {
 	struct uart_match *match = data;
-	dev_t devt = MKDEV(match->driver->major, match->driver->minor) + match->port->line;
+	struct tty_driver *tty_drv = match->driver->tty_driver;
+	dev_t devt = MKDEV(tty_drv->major, tty_drv->minor_start) +
+		match->port->line;
 
 	return dev->devt == devt; /* Actually, only one tty per port */
 }

Re: [PATCH] Fix serial_match_port() for dynamic major tty-device numbers

[Andrew Morton]

On Sun, 22 Jun 2008 00:45:25 +0200 (CEST)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

> As reported by Vipul Gandhi, the current serial_match_port() doesn't work 
> for tty-devices using dynamic major number allocation. Fix it.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Tested-by: Vipul Gandhi <vcgandhi1@aol.com>
> 
> ---
> 
> diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
> index 0f5a179..593ae85 100644
> --- a/drivers/serial/serial_core.c
> +++ b/drivers/serial/serial_core.c
> @@ -1949,7 +1949,9 @@ struct uart_match {
>  static int serial_match_port(struct device *dev, void *data)
>  {
>  	struct uart_match *match = data;
> -	dev_t devt = MKDEV(match->driver->major, match->driver->minor) + match->port->line;
> +	struct tty_driver *tty_drv = match->driver->tty_driver;
> +	dev_t devt = MKDEV(tty_drv->major, tty_drv->minor_start) +
> +		match->port->line;
>  
>  	return dev->devt == devt; /* Actually, only one tty per port */

Well that sounds bad.  We need to work out whether this fix is needed
in 2.6.25 and possibly eariler.

What are the consequences of this error?

Re: [PATCH] Fix serial_match_port() for dynamic major tty-device numbers

[Guennadi Liakhovetski]

On Tue, 24 Jun 2008, Andrew Morton wrote:

> On Sun, 22 Jun 2008 00:45:25 +0200 (CEST)
> Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> 
> > As reported by Vipul Gandhi, the current serial_match_port() doesn't work 
> > for tty-devices using dynamic major number allocation. Fix it.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Tested-by: Vipul Gandhi <vcgandhi1@aol.com>
> > 
> > ---
> > 
> > diff --git a/drivers/serial/serial_core.c b/drivers/serial/serial_core.c
> > index 0f5a179..593ae85 100644
> > --- a/drivers/serial/serial_core.c
> > +++ b/drivers/serial/serial_core.c
> > @@ -1949,7 +1949,9 @@ struct uart_match {
> >  static int serial_match_port(struct device *dev, void *data)
> >  {
> >  	struct uart_match *match = data;
> > -	dev_t devt = MKDEV(match->driver->major, match->driver->minor) + match->port->line;
> > +	struct tty_driver *tty_drv = match->driver->tty_driver;
> > +	dev_t devt = MKDEV(tty_drv->major, tty_drv->minor_start) +
> > +		match->port->line;
> >  
> >  	return dev->devt == devt; /* Actually, only one tty per port */
> 
> Well that sounds bad.  We need to work out whether this fix is needed
> in 2.6.25 and possibly eariler.

Not earlier, this code is only there since 2.6.25.

> What are the consequences of this error?

It Oopses, if you suspend a serial port with _dynamic_ major number. ATM, 
I think, there's only the drivers/serial/jsm/jsm_driver.c driver, that 
does it in-tree.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer