diff mbox

[1/2] serial: samsung: Move uart_register_driver call to device probe

Message ID 1390208555-27770-2-git-send-email-tushar.behera@linaro.org
State Accepted
Commit 6f134c3c770355b7e930d3ffc558864668f13055
Headers show

Commit Message

Tushar Behera Jan. 20, 2014, 9:02 a.m. UTC
uart_register_driver call binds the driver to a specific device
node through tty_register_driver call. This should typically happen
during device probe call.

In a multiplatform scenario, it is possible that multiple serial
drivers are part of the kernel. Currently the driver registration fails
if multiple serial drivers with same default major/minor numbers are
included in the kernel.

A typical case is observed with amba-pl011 and samsung-uart drivers.

Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
 drivers/tty/serial/samsung.c |   40 +++++++++++-----------------------------
 1 file changed, 11 insertions(+), 29 deletions(-)

Comments

Tushar Behera Jan. 20, 2014, 11:53 a.m. UTC | #1
On 20 January 2014 15:35, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jan 20, 2014 at 02:32:34PM +0530, Tushar Behera wrote:
>> uart_register_driver call binds the driver to a specific device
>> node through tty_register_driver call. This should typically happen
>> during device probe call.
>>
>> In a multiplatform scenario, it is possible that multiple serial
>> drivers are part of the kernel. Currently the driver registration fails
>> if multiple serial drivers with same default major/minor numbers are
>> included in the kernel.
>>
>> A typical case is observed with amba-pl011 and samsung-uart drivers.
>
> The samsung-uart driver is at fault here - the major/minor numbers were
> officially registered to amba-pl011.  Samsung needs to be fixed properly.
>

I had earlier submitted a patch [1] to remove the hard coded
major/minor number for Samsung UART driver, but that was rejected
because of userspace breakage. Without this patch, Samsung UART driver
can't bind to the hard-coded device node. Changing the default
major/minor will also not help to fix the objections raised in [1].

Would you please suggest a way forward?

[1] https://lkml.org/lkml/2013/12/27/2
Nicolas Pitre Jan. 27, 2014, 4:30 a.m. UTC | #2
On Sun, 26 Jan 2014, Russell King - ARM Linux wrote:

> On Tue, Jan 21, 2014 at 12:45:05AM +0000, Alan Cox wrote:
> > Peter handed it on. Try using git log on Documentation/devices.txt. It
> > still gets updates.
> > 
> > Perhaps you'd care to stick to reality and fix the tree instead of trying
> > to excuse the mess ?
> 
> Perhaps returning to reality might be advantageous rather than trying
> to repeat statements which can't have any bearing on this - especially
> as the git history which you're referring to only goes back to 2.6.12-rc2,
> and this predates 2.6.12-rc2 by a long shot.
> 
> > More importantly certain folks need to stop abusing static numbers
> > allocated properly. Repeating it having made a total hash of it before
> > is dismal.
> 
> And if you continue these stupid accusations which have no basis at all,
> we're going to get into a real big argument, because you are soo _wrong_
> on that point.  I was always the one arguing /against/ the re-use of
> existing major/minor numbers.  I was the one arguing /against/ Nicolas'
> patches to make every serial port appear in the 4,64 ttyS namespace.

If you remember correctly, that was my attempt at making serial port 
minor assignment to be dynamic... just like everything else is today.  
And it seemed to me that you thought this was a good idea.

But that was objected by the X86 folks who insisted on always having 
COM1 == ttyS0 and COM2 == ttyS1, whether or not COM1 was present.

Nowdays serial ports have pretty much disappeared from the X86 world.  
But the problem they've created is still with us.

Either everything is dynamic, or everything follows proper minor 
assignment process.


Nicolas
--
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
Nicolas Pitre Jan. 27, 2014, 3:03 p.m. UTC | #3
On Mon, 27 Jan 2014, Russell King - ARM Linux wrote:

> On Sun, Jan 26, 2014 at 11:30:00PM -0500, Nicolas Pitre wrote:
> > On Sun, 26 Jan 2014, Russell King - ARM Linux wrote:
> > 
> > > On Tue, Jan 21, 2014 at 12:45:05AM +0000, Alan Cox wrote:
> > > > Peter handed it on. Try using git log on Documentation/devices.txt. It
> > > > still gets updates.
> > > > 
> > > > Perhaps you'd care to stick to reality and fix the tree instead of trying
> > > > to excuse the mess ?
> > > 
> > > Perhaps returning to reality might be advantageous rather than trying
> > > to repeat statements which can't have any bearing on this - especially
> > > as the git history which you're referring to only goes back to 2.6.12-rc2,
> > > and this predates 2.6.12-rc2 by a long shot.
> > > 
> > > > More importantly certain folks need to stop abusing static numbers
> > > > allocated properly. Repeating it having made a total hash of it before
> > > > is dismal.
> > > 
> > > And if you continue these stupid accusations which have no basis at all,
> > > we're going to get into a real big argument, because you are soo _wrong_
> > > on that point.  I was always the one arguing /against/ the re-use of
> > > existing major/minor numbers.  I was the one arguing /against/ Nicolas'
> > > patches to make every serial port appear in the 4,64 ttyS namespace.
> > 
> > If you remember correctly, that was my attempt at making serial port 
> > minor assignment to be dynamic... just like everything else is today.  
> > And it seemed to me that you thought this was a good idea.
> 
> I may have thought that a dynamic space for serial devices was a good
> idea, but what I was referring to above was specifically the
> implementation.

Oh I do not dispute the fact that the implementation at the time was not 
up to needed standards.  That would have been fixed eventually though, 
if it hadn't been for the policy based objection we received.  At which 
point this effort was simply dropped.


Nicolas
--
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
diff mbox

Patch

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index c1af04d..f8f41dc 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -1283,6 +1283,14 @@  static int s3c24xx_serial_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto probe_err;
 
+	if (!s3c24xx_uart_drv.state) {
+		ret = uart_register_driver(&s3c24xx_uart_drv);
+		if (ret < 0) {
+			pr_err("Failed to register Samsung UART driver\n");
+			return ret;
+		}
+	}
+
 	dbg("%s: adding port\n", __func__);
 	uart_add_one_port(&s3c24xx_uart_drv, &ourport->port);
 	platform_set_drvdata(pdev, &ourport->port);
@@ -1315,6 +1323,8 @@  static int s3c24xx_serial_remove(struct platform_device *dev)
 		uart_remove_one_port(&s3c24xx_uart_drv, port);
 	}
 
+	uart_unregister_driver(&s3c24xx_uart_drv);
+
 	return 0;
 }
 
@@ -1814,35 +1824,7 @@  static struct platform_driver samsung_serial_driver = {
 	},
 };
 
-/* module initialisation code */
-
-static int __init s3c24xx_serial_modinit(void)
-{
-	int ret;
-
-	ret = uart_register_driver(&s3c24xx_uart_drv);
-	if (ret < 0) {
-		pr_err("Failed to register Samsung UART driver\n");
-		return ret;
-	}
-
-	ret = platform_driver_register(&samsung_serial_driver);
-	if (ret < 0) {
-		pr_err("Failed to register platform driver\n");
-		uart_unregister_driver(&s3c24xx_uart_drv);
-	}
-
-	return ret;
-}
-
-static void __exit s3c24xx_serial_modexit(void)
-{
-	platform_driver_unregister(&samsung_serial_driver);
-	uart_unregister_driver(&s3c24xx_uart_drv);
-}
-
-module_init(s3c24xx_serial_modinit);
-module_exit(s3c24xx_serial_modexit);
+module_platform_driver(samsung_serial_driver);
 
 MODULE_ALIAS("platform:samsung-uart");
 MODULE_DESCRIPTION("Samsung SoC Serial port driver");