[PATCH 1/2] serial: xilinx_uartps: Fix warnings in the driver

[PATCH 1/2] serial: xilinx_uartps: Fix warnings in the driver

[Michal Simek]

From: Nava kishore Manne <nava.manne@xilinx.com>

This patch fixes the below warning

        -->Symbolic permissions 'S_IRUGO' are not preferred.
           Consider using octal permissions '0444'.
        -->macros should not use a trailing semicolon.
        -->line over 80 characters.
        -->void function return statements are not generally useful.
        -->Prefer 'unsigned int' to bare use of 'unsigned'.

Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Happy to split it if needed.
---
 drivers/tty/serial/xilinx_uartps.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 605354fd60b1..6af2886f5ba7 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -35,12 +35,12 @@
 
 /* Rx Trigger level */
 static int rx_trigger_level = 56;
-module_param(rx_trigger_level, uint, S_IRUGO);
+module_param(rx_trigger_level, uint, 0444);
 MODULE_PARM_DESC(rx_trigger_level, "Rx trigger level, 1-63 bytes");
 
 /* Rx Timeout */
 static int rx_timeout = 10;
-module_param(rx_timeout, uint, S_IRUGO);
+module_param(rx_timeout, uint, 0444);
 MODULE_PARM_DESC(rx_timeout, "Rx timeout, 1-255");
 
 /* Register offsets for the UART. */
@@ -199,7 +199,7 @@ struct cdns_platform_data {
 	u32 quirks;
 };
 #define to_cdns_uart(_nb) container_of(_nb, struct cdns_uart, \
-		clk_rate_change_nb);
+		clk_rate_change_nb)
 
 /**
  * cdns_uart_handle_rx - Handle the received bytes along with Rx errors.
@@ -312,7 +312,8 @@ static void cdns_uart_handle_tx(void *dev_id)
 	} else {
 		numbytes = port->fifosize;
 		while (numbytes && !uart_circ_empty(&port->state->xmit) &&
-		       !(readl(port->membase + CDNS_UART_SR) & CDNS_UART_SR_TXFULL)) {
+		       !(readl(port->membase + CDNS_UART_SR) &
+						CDNS_UART_SR_TXFULL)) {
 			/*
 			 * Get the data from the UART circular buffer
 			 * and write it to the cdns_uart's TX_FIFO
@@ -1073,8 +1074,6 @@ static void cdns_uart_poll_put_char(struct uart_port *port, unsigned char c)
 		cpu_relax();
 
 	spin_unlock_irqrestore(&port->lock, flags);
-
-	return;
 }
 #endif
 
-- 
2.17.1

[PATCH 2/2] serial: uartps: Use the same dynamin major number for all ports

[Michal Simek]

From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>

Let kernel to find out major number dynamically for the first device and
then reuse it for other instances.
This fixes the issue that each uart is registered with a
different major number.

After the patch:
crw-------    1 root     root      253,   0 Jun 10 08:31 /dev/ttyPS0
crw--w----    1 root     root      253,   1 Jan  1  1970 /dev/ttyPS1

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/tty/serial/xilinx_uartps.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 6af2886f5ba7..a0b51c60faeb 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -29,12 +29,12 @@
 
 #define CDNS_UART_TTY_NAME	"ttyPS"
 #define CDNS_UART_NAME		"xuartps"
-#define CDNS_UART_MAJOR		0	/* use dynamic node allocation */
 #define CDNS_UART_FIFO_SIZE	64	/* FIFO size */
 #define CDNS_UART_REGISTER_SPACE	0x1000
 
 /* Rx Trigger level */
 static int rx_trigger_level = 56;
+static int uartps_major;
 module_param(rx_trigger_level, uint, 0444);
 MODULE_PARM_DESC(rx_trigger_level, "Rx trigger level, 1-63 bytes");
 
@@ -1516,7 +1516,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	cdns_uart_uart_driver->owner = THIS_MODULE;
 	cdns_uart_uart_driver->driver_name = driver_name;
 	cdns_uart_uart_driver->dev_name	= CDNS_UART_TTY_NAME;
-	cdns_uart_uart_driver->major = CDNS_UART_MAJOR;
+	cdns_uart_uart_driver->major = uartps_major;
 	cdns_uart_uart_driver->minor = cdns_uart_data->id;
 	cdns_uart_uart_driver->nr = 1;
 
@@ -1545,6 +1545,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
 		goto err_out_id;
 	}
 
+	uartps_major = cdns_uart_uart_driver->tty_driver->major;
 	cdns_uart_data->cdns_uart_driver = cdns_uart_uart_driver;
 
 	/*
-- 
2.17.1

Re: [PATCH 1/2] serial: xilinx_uartps: Fix warnings in the driver

[Greg KH]

On Mon, Jun 10, 2019 at 10:44:55AM +0200, Michal Simek wrote:
> From: Nava kishore Manne <nava.manne@xilinx.com>
> 
> This patch fixes the below warning
> 
>         -->Symbolic permissions 'S_IRUGO' are not preferred.
>            Consider using octal permissions '0444'.
>         -->macros should not use a trailing semicolon.
>         -->line over 80 characters.
>         -->void function return statements are not generally useful.
>         -->Prefer 'unsigned int' to bare use of 'unsigned'.
> 
> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
> Happy to split it if needed.

Please split.  Do not do more than one "logical thing" per patch.

And the subject is not correct, there are no general "warnings", these
are all checkpatch warnings, not a build issue.

thanks,

greg k-h

Re: [PATCH 1/2] serial: xilinx_uartps: Fix warnings in the driver

[Michal Simek]

On 10. 06. 19 16:44, Greg KH wrote:
> On Mon, Jun 10, 2019 at 10:44:55AM +0200, Michal Simek wrote:
>> From: Nava kishore Manne <nava.manne@xilinx.com>
>>
>> This patch fixes the below warning
>>
>>         -->Symbolic permissions 'S_IRUGO' are not preferred.
>>            Consider using octal permissions '0444'.
>>         -->macros should not use a trailing semicolon.
>>         -->line over 80 characters.
>>         -->void function return statements are not generally useful.
>>         -->Prefer 'unsigned int' to bare use of 'unsigned'.
>>
>> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> Happy to split it if needed.
> 
> Please split.  Do not do more than one "logical thing" per patch.
> 
> And the subject is not correct, there are no general "warnings", these
> are all checkpatch warnings, not a build issue.

ok. Will do. Any issue with second patch?

Thanks,
Michal

Re: [PATCH 1/2] serial: xilinx_uartps: Fix warnings in the driver

[Greg KH]

On Mon, Jun 10, 2019 at 05:06:57PM +0200, Michal Simek wrote:
> On 10. 06. 19 16:44, Greg KH wrote:
> > On Mon, Jun 10, 2019 at 10:44:55AM +0200, Michal Simek wrote:
> >> From: Nava kishore Manne <nava.manne@xilinx.com>
> >>
> >> This patch fixes the below warning
> >>
> >>         -->Symbolic permissions 'S_IRUGO' are not preferred.
> >>            Consider using octal permissions '0444'.
> >>         -->macros should not use a trailing semicolon.
> >>         -->line over 80 characters.
> >>         -->void function return statements are not generally useful.
> >>         -->Prefer 'unsigned int' to bare use of 'unsigned'.
> >>
> >> Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >> ---
> >>
> >> Happy to split it if needed.
> > 
> > Please split.  Do not do more than one "logical thing" per patch.
> > 
> > And the subject is not correct, there are no general "warnings", these
> > are all checkpatch warnings, not a build issue.
> 
> ok. Will do. Any issue with second patch?

It will change your user/kernel api but hey, it's your call, it looks
like a nice fix to me :)

thanks,

greg k-h

Re: [PATCH 2/2] serial: uartps: Use the same dynamin major number for all ports

[Greg KH]

On Mon, Jun 10, 2019 at 10:44:56AM +0200, Michal Simek wrote:
> From: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> 
> Let kernel to find out major number dynamically for the first device and
> then reuse it for other instances.
> This fixes the issue that each uart is registered with a
> different major number.
> 
> After the patch:
> crw-------    1 root     root      253,   0 Jun 10 08:31 /dev/ttyPS0
> crw--w----    1 root     root      253,   1 Jan  1  1970 /dev/ttyPS1
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
>  drivers/tty/serial/xilinx_uartps.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Does not apply without patch 1/2, so dropping it from my queue.

Which is ALWAYS a good reason why you should submit bug fixes or other
things before code cleanup patches.

thanks,

greg k-h