[PATCH] serial: samsung: Remove hard-coded major/minor numbers

[PATCH] serial: samsung: Remove hard-coded major/minor numbers

[Tushar Behera]

The hard-coded values clash with the values set for amba-pl011 serial
driver. Because of this there is no serial output on Samsung boards
if amba-pl011 is enabled alongwith samsung-serial driver.

Remove the hardcoded values and let the framework decide on
appropriate major/minor number. This is required for multi-platform
development work on Exynos platform.

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

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index c1af04d..9c20543 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -56,8 +56,6 @@
 /* UART name and device definitions */
 
 #define S3C24XX_SERIAL_NAME	"ttySAC"
-#define S3C24XX_SERIAL_MAJOR	204
-#define S3C24XX_SERIAL_MINOR	64
 
 /* macros to change one thing to another */
 
@@ -951,8 +949,6 @@ static struct uart_driver s3c24xx_uart_drv = {
 	.nr		= CONFIG_SERIAL_SAMSUNG_UARTS,
 	.cons		= S3C24XX_SERIAL_CONSOLE,
 	.dev_name	= S3C24XX_SERIAL_NAME,
-	.major		= S3C24XX_SERIAL_MAJOR,
-	.minor		= S3C24XX_SERIAL_MINOR,
 };
 
 static struct s3c24xx_uart_port s3c24xx_serial_ports[CONFIG_SERIAL_SAMSUNG_UARTS] = {
-- 
1.7.9.5

Re: [PATCH] serial: samsung: Remove hard-coded major/minor numbers

[Greg KH]

On Fri, Dec 27, 2013 at 10:37:28AM +0530, Tushar Behera wrote:
> The hard-coded values clash with the values set for amba-pl011 serial
> driver. Because of this there is no serial output on Samsung boards
> if amba-pl011 is enabled alongwith samsung-serial driver.
> 
> Remove the hardcoded values and let the framework decide on
> appropriate major/minor number. This is required for multi-platform
> development work on Exynos platform.
> 
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---
>  drivers/tty/serial/samsung.c |    4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
> index c1af04d..9c20543 100644
> --- a/drivers/tty/serial/samsung.c
> +++ b/drivers/tty/serial/samsung.c
> @@ -56,8 +56,6 @@
>  /* UART name and device definitions */
>  
>  #define S3C24XX_SERIAL_NAME	"ttySAC"
> -#define S3C24XX_SERIAL_MAJOR	204
> -#define S3C24XX_SERIAL_MINOR	64
>  
>  /* macros to change one thing to another */
>  
> @@ -951,8 +949,6 @@ static struct uart_driver s3c24xx_uart_drv = {
>  	.nr		= CONFIG_SERIAL_SAMSUNG_UARTS,
>  	.cons		= S3C24XX_SERIAL_CONSOLE,
>  	.dev_name	= S3C24XX_SERIAL_NAME,
> -	.major		= S3C24XX_SERIAL_MAJOR,
> -	.minor		= S3C24XX_SERIAL_MINOR,

Doesn't this break existing systems and configurations that are
expecting 204:64 as the location of this serial port?

Why change this one and not the amba-pl011 driver?

greg k-h

Re: [PATCH] serial: samsung: Remove hard-coded major/minor numbers

[Tushar Behera]

On 27 December 2013 10:48, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Dec 27, 2013 at 10:37:28AM +0530, Tushar Behera wrote:
>> The hard-coded values clash with the values set for amba-pl011 serial
>> driver. Because of this there is no serial output on Samsung boards
>> if amba-pl011 is enabled alongwith samsung-serial driver.
>>
>> Remove the hardcoded values and let the framework decide on
>> appropriate major/minor number. This is required for multi-platform
>> development work on Exynos platform.
>>
>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>> ---
>>  drivers/tty/serial/samsung.c |    4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
>> index c1af04d..9c20543 100644
>> --- a/drivers/tty/serial/samsung.c
>> +++ b/drivers/tty/serial/samsung.c
>> @@ -56,8 +56,6 @@
>>  /* UART name and device definitions */
>>
>>  #define S3C24XX_SERIAL_NAME  "ttySAC"
>> -#define S3C24XX_SERIAL_MAJOR 204
>> -#define S3C24XX_SERIAL_MINOR 64
>>
>>  /* macros to change one thing to another */
>>
>> @@ -951,8 +949,6 @@ static struct uart_driver s3c24xx_uart_drv = {
>>       .nr             = CONFIG_SERIAL_SAMSUNG_UARTS,
>>       .cons           = S3C24XX_SERIAL_CONSOLE,
>>       .dev_name       = S3C24XX_SERIAL_NAME,
>> -     .major          = S3C24XX_SERIAL_MAJOR,
>> -     .minor          = S3C24XX_SERIAL_MINOR,
>
> Doesn't this break existing systems and configurations that are
> expecting 204:64 as the location of this serial port?
>

I tested this on Exynos4210-Origen, Exynos5250-Arndale board, it works
fine there. I haven't tested on any older boards.

> Why change this one and not the amba-pl011 driver?
>

I could only test this driver, so thought of changing this rather than
modifying amba-pl011 driver. I don't have any other reason.

> greg k-h


Thanks for reviewing.
-- 
Tushar Behera

Re: [PATCH] serial: samsung: Remove hard-coded major/minor numbers

[Greg KH]

On Fri, Dec 27, 2013 at 12:00:20PM +0530, Tushar Behera wrote:
> On 27 December 2013 10:48, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Fri, Dec 27, 2013 at 10:37:28AM +0530, Tushar Behera wrote:
> >> The hard-coded values clash with the values set for amba-pl011 serial
> >> driver. Because of this there is no serial output on Samsung boards
> >> if amba-pl011 is enabled alongwith samsung-serial driver.
> >>
> >> Remove the hardcoded values and let the framework decide on
> >> appropriate major/minor number. This is required for multi-platform
> >> development work on Exynos platform.
> >>
> >> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> >> ---
> >>  drivers/tty/serial/samsung.c |    4 ----
> >>  1 file changed, 4 deletions(-)
> >>
> >> diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
> >> index c1af04d..9c20543 100644
> >> --- a/drivers/tty/serial/samsung.c
> >> +++ b/drivers/tty/serial/samsung.c
> >> @@ -56,8 +56,6 @@
> >>  /* UART name and device definitions */
> >>
> >>  #define S3C24XX_SERIAL_NAME  "ttySAC"
> >> -#define S3C24XX_SERIAL_MAJOR 204
> >> -#define S3C24XX_SERIAL_MINOR 64
> >>
> >>  /* macros to change one thing to another */
> >>
> >> @@ -951,8 +949,6 @@ static struct uart_driver s3c24xx_uart_drv = {
> >>       .nr             = CONFIG_SERIAL_SAMSUNG_UARTS,
> >>       .cons           = S3C24XX_SERIAL_CONSOLE,
> >>       .dev_name       = S3C24XX_SERIAL_NAME,
> >> -     .major          = S3C24XX_SERIAL_MAJOR,
> >> -     .minor          = S3C24XX_SERIAL_MINOR,
> >
> > Doesn't this break existing systems and configurations that are
> > expecting 204:64 as the location of this serial port?
> >
> 
> I tested this on Exynos4210-Origen, Exynos5250-Arndale board, it works
> fine there. I haven't tested on any older boards.

How did it work?  You are relying on some userspace tools to do this
properly, right?  What about systems without those specific tools?

> > Why change this one and not the amba-pl011 driver?
> >
> 
> I could only test this driver, so thought of changing this rather than
> modifying amba-pl011 driver. I don't have any other reason.

Please get the samsung driver maintainer to agree with this and sign off
on it before trying to get it merged again.

greg k-h

Re: [PATCH] serial: samsung: Remove hard-coded major/minor numbers

[Alexander Shiyan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 1904 bytes --]

Hello.
> On Fri, Dec 27, 2013 at 12:00:20PM +0530, Tushar Behera wrote:
> > On 27 December 2013 10:48, Greg KH <gregkh@linuxfoundation.org> wrote:
> > > On Fri, Dec 27, 2013 at 10:37:28AM +0530, Tushar Behera wrote:
> > >> The hard-coded values clash with the values set for amba-pl011 serial
> > >> driver. Because of this there is no serial output on Samsung boards
> > >> if amba-pl011 is enabled alongwith samsung-serial driver.
> > >>
> > >> Remove the hardcoded values and let the framework decide on
> > >> appropriate major/minor number. This is required for multi-platform
> > >> development work on Exynos platform.
> > >>
> > >> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
...
> > >>  #define S3C24XX_SERIAL_NAME  "ttySAC"
> > >> -#define S3C24XX_SERIAL_MAJOR 204
> > >> -#define S3C24XX_SERIAL_MINOR 64
> > >>
> > >>  /* macros to change one thing to another */
> > >>
> > >> @@ -951,8 +949,6 @@ static struct uart_driver s3c24xx_uart_drv = {
> > >>       .nr             = CONFIG_SERIAL_SAMSUNG_UARTS,
> > >>       .cons           = S3C24XX_SERIAL_CONSOLE,
> > >>       .dev_name       = S3C24XX_SERIAL_NAME,
> > >> -     .major          = S3C24XX_SERIAL_MAJOR,
> > >> -     .minor          = S3C24XX_SERIAL_MINOR,
> > >
> > > Doesn't this break existing systems and configurations that are
> > > expecting 204:64 as the location of this serial port?
> > >
> > 
> > I tested this on Exynos4210-Origen, Exynos5250-Arndale board, it works
> > fine there. I haven't tested on any older boards.
> 
> How did it work?  You are relying on some userspace tools to do this
> properly, right?  What about systems without those specific tools?

Can this issue be resolved by using MODULE_ALIAS_CHARDEV()
in the driver code?

---
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] serial: samsung: Remove hard-coded major/minor numbers

[Greg KH]

On Fri, Dec 27, 2013 at 10:43:23AM +0400, Alexander Shiyan wrote:
> Hello.
> > On Fri, Dec 27, 2013 at 12:00:20PM +0530, Tushar Behera wrote:
> > > On 27 December 2013 10:48, Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > On Fri, Dec 27, 2013 at 10:37:28AM +0530, Tushar Behera wrote:
> > > >> The hard-coded values clash with the values set for amba-pl011 serial
> > > >> driver. Because of this there is no serial output on Samsung boards
> > > >> if amba-pl011 is enabled alongwith samsung-serial driver.
> > > >>
> > > >> Remove the hardcoded values and let the framework decide on
> > > >> appropriate major/minor number. This is required for multi-platform
> > > >> development work on Exynos platform.
> > > >>
> > > >> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ...
> > > >>  #define S3C24XX_SERIAL_NAME  "ttySAC"
> > > >> -#define S3C24XX_SERIAL_MAJOR 204
> > > >> -#define S3C24XX_SERIAL_MINOR 64
> > > >>
> > > >>  /* macros to change one thing to another */
> > > >>
> > > >> @@ -951,8 +949,6 @@ static struct uart_driver s3c24xx_uart_drv = {
> > > >>       .nr             = CONFIG_SERIAL_SAMSUNG_UARTS,
> > > >>       .cons           = S3C24XX_SERIAL_CONSOLE,
> > > >>       .dev_name       = S3C24XX_SERIAL_NAME,
> > > >> -     .major          = S3C24XX_SERIAL_MAJOR,
> > > >> -     .minor          = S3C24XX_SERIAL_MINOR,
> > > >
> > > > Doesn't this break existing systems and configurations that are
> > > > expecting 204:64 as the location of this serial port?
> > > >
> > > 
> > > I tested this on Exynos4210-Origen, Exynos5250-Arndale board, it works
> > > fine there. I haven't tested on any older boards.
> > 
> > How did it work?  You are relying on some userspace tools to do this
> > properly, right?  What about systems without those specific tools?
> 
> Can this issue be resolved by using MODULE_ALIAS_CHARDEV()
> in the driver code?

How exactly would that work?

Re: [PATCH] serial: samsung: Remove hard-coded major/minor numbers

[Tushar Behera]

On 27 December 2013 12:08, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Dec 27, 2013 at 12:00:20PM +0530, Tushar Behera wrote:
>> On 27 December 2013 10:48, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Fri, Dec 27, 2013 at 10:37:28AM +0530, Tushar Behera wrote:

[ ... ]

>> >> @@ -951,8 +949,6 @@ static struct uart_driver s3c24xx_uart_drv = {
>> >>       .nr             = CONFIG_SERIAL_SAMSUNG_UARTS,
>> >>       .cons           = S3C24XX_SERIAL_CONSOLE,
>> >>       .dev_name       = S3C24XX_SERIAL_NAME,
>> >> -     .major          = S3C24XX_SERIAL_MAJOR,
>> >> -     .minor          = S3C24XX_SERIAL_MINOR,
>> >
>> > Doesn't this break existing systems and configurations that are
>> > expecting 204:64 as the location of this serial port?
>> >
>>
>> I tested this on Exynos4210-Origen, Exynos5250-Arndale board, it works
>> fine there. I haven't tested on any older boards.
>
> How did it work?  You are relying on some userspace tools to do this
> properly, right?  What about systems without those specific tools?
>

Enabling CONFIG_DEVTMPFS, all the /dev/ttySAC<n> nodes are generated
and the appropriate console is specified through command line
argument.

>> > Why change this one and not the amba-pl011 driver?
>> >
>>
>> I could only test this driver, so thought of changing this rather than
>> modifying amba-pl011 driver. I don't have any other reason.
>
> Please get the samsung driver maintainer to agree with this and sign off
> on it before trying to get it merged again.
>
> greg k-h

Adding a few other developers who have contributed to this file or are
working on different Samsung platforms to get an ack/sign-off.

-- 
Tushar Behera

Re: [PATCH] serial: samsung: Remove hard-coded major/minor numbers

[Greg KH]

On Fri, Dec 27, 2013 at 03:47:31PM +0530, Tushar Behera wrote:
> On 27 December 2013 12:08, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Fri, Dec 27, 2013 at 12:00:20PM +0530, Tushar Behera wrote:
> >> On 27 December 2013 10:48, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> > On Fri, Dec 27, 2013 at 10:37:28AM +0530, Tushar Behera wrote:
> 
> [ ... ]
> 
> >> >> @@ -951,8 +949,6 @@ static struct uart_driver s3c24xx_uart_drv = {
> >> >>       .nr             = CONFIG_SERIAL_SAMSUNG_UARTS,
> >> >>       .cons           = S3C24XX_SERIAL_CONSOLE,
> >> >>       .dev_name       = S3C24XX_SERIAL_NAME,
> >> >> -     .major          = S3C24XX_SERIAL_MAJOR,
> >> >> -     .minor          = S3C24XX_SERIAL_MINOR,
> >> >
> >> > Doesn't this break existing systems and configurations that are
> >> > expecting 204:64 as the location of this serial port?
> >> >
> >>
> >> I tested this on Exynos4210-Origen, Exynos5250-Arndale board, it works
> >> fine there. I haven't tested on any older boards.
> >
> > How did it work?  You are relying on some userspace tools to do this
> > properly, right?  What about systems without those specific tools?
> >
> 
> Enabling CONFIG_DEVTMPFS, all the /dev/ttySAC<n> nodes are generated
> and the appropriate console is specified through command line
> argument.

But what about systems that rely on a hard-coded /dev?

Look, I'm all for making everyone use devtmpfs, but just changing
major:minor numbers for drivers isn't ok, as you are changing the
userspace ABI for the device.

Please realize what you are asking for here, I really don't think you
grasp it given that you didn't ask any of the maintainers of this driver
about the change in the first place.

Please get approval for this patch from others within Linaro before
sending it out again.  Linaro has a process in place for this type of
thing, please use it, otherwise it makes people like me really grumpy
and upset and causes me to yell at people at their conferences.

greg k-h

Re: [PATCH] serial: samsung: Remove hard-coded major/minor numbers

[Mark Brown]

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

On Fri, Dec 27, 2013 at 10:44:24AM -0800, Greg KH wrote:

> Please get approval for this patch from others within Linaro before
> sending it out again.  Linaro has a process in place for this type of
> thing, please use it, otherwise it makes people like me really grumpy
> and upset and causes me to yell at people at their conferences.

This isn't really anything to do with Linaro - it's a Samsung driver so
it's their call.  A good proportion of the Samsung guys and their list
are in the CCs so hopefully they'll chime in.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]