[PATCH 0/7] tty: xilinx_uartps: fixes and improvements

[PATCH 0/7] tty: xilinx_uartps: fixes and improvements

[Shubhrajyoti Datta]

Many of the stuff is reported by static analysis
changes are 

- Update the copyright text
- Check the return valuesof runtime and clock enable calls
- Check the ignore_status in the isr 
- Prevent writing to the fifo when controller is disabled.

Michal Simek (1):
  xilinx: Update copyright text to correct format

Shubhrajyoti Datta (6):
  tty: xilinx_uartps: Check the clk_enable return value
  tty: xilinx_uartps: Add check for runtime_get_sync calls
  tty: xilinx_uartps: Check clk_enable return type
  tty: xilinx_uartps: Make the timeout unsigned
  serial: uartps: Fix the ignore_status
  serial: uartps: Prevent writes when the controller is disabled

 drivers/tty/serial/xilinx_uartps.c | 45 ++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH 1/7] xilinx: Update copyright text to correct format

[Shubhrajyoti Datta]

From: Michal Simek <michal.simek@xilinx.com>

Based on recommended guidance Copyright term should be also present in
front of (c). That's why aligned drivers to match this pattern.
It helps automated tools with source code scanning.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 250a1d888eeb..fa82f88844a1 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -2,7 +2,7 @@
 /*
  * Cadence UART driver (found in Xilinx Zynq)
  *
- * 2011 - 2014 (C) Xilinx Inc.
+ * Copyright (c) 2011 - 2014 Xilinx, Inc.
  *
  * This driver has originally been pushed by Xilinx using a Zynq-branding. This
  * still shows in the naming of this file, the kconfig symbols and some symbols
-- 
2.25.1

[PATCH 2/7] tty: xilinx_uartps: Check the clk_enable return value

[Shubhrajyoti Datta]

Check the clk_enable return value.

Addresses-Coverity: Event check_return.
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index fa82f88844a1..8f15fe24a0eb 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1397,9 +1397,17 @@ static int __maybe_unused cdns_runtime_resume(struct device *dev)
 {
 	struct uart_port *port = dev_get_drvdata(dev);
 	struct cdns_uart *cdns_uart = port->private_data;
+	int ret;
+
+	ret = clk_enable(cdns_uart->pclk);
+	if (ret)
+		return ret;
 
-	clk_enable(cdns_uart->pclk);
-	clk_enable(cdns_uart->uartclk);
+	ret = clk_enable(cdns_uart->uartclk);
+	if (ret) {
+		clk_disable(cdns_uart->pclk);
+		return ret;
+	}
 	return 0;
 };
 
-- 
2.25.1

[PATCH 3/7] tty: xilinx_uartps: Add check for runtime_get_sync calls

[Shubhrajyoti Datta]

Add a check for the return value of runtime get_sync calls.

Addresses-Coverity: Event check_return.
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 8f15fe24a0eb..868f4e587263 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1100,13 +1100,17 @@ static void cdns_uart_poll_put_char(struct uart_port *port, unsigned char c)
 static void cdns_uart_pm(struct uart_port *port, unsigned int state,
 		   unsigned int oldstate)
 {
+	int ret;
+
 	switch (state) {
 	case UART_PM_STATE_OFF:
 		pm_runtime_mark_last_busy(port->dev);
 		pm_runtime_put_autosuspend(port->dev);
 		break;
 	default:
-		pm_runtime_get_sync(port->dev);
+		ret = pm_runtime_get_sync(port->dev);
+		if (ret < 0)
+			dev_err(port->dev, "Failed to enable clocks\n");
 		break;
 	}
 }
-- 
2.25.1

[PATCH 4/7] tty: xilinx_uartps: Check clk_enable return value

[Shubhrajyoti Datta]

Check for the clock enable return value.

Addresses-Coverity: Event check_return.
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 868f4e587263..bf0415f0a194 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1347,12 +1347,18 @@ static int cdns_uart_resume(struct device *device)
 	unsigned long flags;
 	u32 ctrl_reg;
 	int may_wake;
+	int ret;
 
 	may_wake = device_may_wakeup(device);
 
 	if (console_suspend_enabled && uart_console(port) && !may_wake) {
-		clk_enable(cdns_uart->pclk);
-		clk_enable(cdns_uart->uartclk);
+		ret = clk_enable(cdns_uart->pclk);
+		if (ret)
+			return ret;
+
+		ret = clk_enable(cdns_uart->uartclk);
+		if (ret)
+			return ret;
 
 		spin_lock_irqsave(&port->lock, flags);
 
-- 
2.25.1

[PATCH 5/7] tty: xilinx_uartps: Make the timeout unsigned

[Shubhrajyoti Datta]

The timeout cannot be negative make it unsigned.
Also the same for the trigger level.

Addresses-Coverity: Event incompatible_param.
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index bf0415f0a194..289d70914956 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -34,12 +34,12 @@
 #define TX_TIMEOUT		500000
 
 /* Rx Trigger level */
-static int rx_trigger_level = 56;
+static uint rx_trigger_level = 56;
 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;
+static uint rx_timeout = 10;
 module_param(rx_timeout, uint, 0444);
 MODULE_PARM_DESC(rx_timeout, "Rx timeout, 1-255");
 
-- 
2.25.1

[PATCH 6/7] serial: uartps: Fix the ignore_status

[Shubhrajyoti Datta]

Currently the ignore_status is not considered in the isr.
Also the ignore_status is not updated in the set_termios.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 289d70914956..81ba69c57716 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -375,6 +375,8 @@ static irqreturn_t cdns_uart_isr(int irq, void *dev_id)
 		isrstatus &= ~CDNS_UART_IXR_TXEMPTY;
 	}
 
+	isrstatus &= port->read_status_mask;
+	isrstatus &= ~port->ignore_status_mask;
 	/*
 	 * Skip RX processing if RX is disabled as RXEMPTY will never be set
 	 * as read bytes will not be removed from the FIFO.
@@ -1583,6 +1585,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
 	port->dev = &pdev->dev;
 	port->uartclk = clk_get_rate(cdns_uart_data->uartclk);
 	port->private_data = cdns_uart_data;
+	port->read_status_mask = CDNS_UART_IXR_TXEMPTY | CDNS_UART_IXR_RXTRIG |
+			CDNS_UART_IXR_OVERRUN | CDNS_UART_IXR_TOUT;
 	cdns_uart_data->port = port;
 	platform_set_drvdata(pdev, port);
 
-- 
2.25.1

[PATCH 7/7] serial: uartps: Prevent writes when the controller is disabled

[Shubhrajyoti Datta]

Prevent writing to the fifo if the controller is disabled.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 81ba69c57716..f629c4ca940f 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1150,6 +1150,13 @@ static struct uart_driver cdns_uart_uart_driver;
  */
 static void cdns_uart_console_putchar(struct uart_port *port, unsigned char ch)
 {
+	unsigned int ctrl_reg;
+
+	ctrl_reg = readl(port->membase + CDNS_UART_CR);
+	while (ctrl_reg & CDNS_UART_CR_TX_DIS) {
+		ctrl_reg = readl(port->membase + CDNS_UART_CR);
+		cpu_relax();
+	}
 	while (readl(port->membase + CDNS_UART_SR) & CDNS_UART_SR_TXFULL)
 		cpu_relax();
 	writel(ch, port->membase + CDNS_UART_FIFO);
-- 
2.25.1

Re: [PATCH 0/7] tty: xilinx_uartps: fixes and improvements

[Greg KH]

On Fri, Apr 29, 2022 at 01:44:15PM +0530, Shubhrajyoti Datta wrote:
> Many of the stuff is reported by static analysis
> changes are 
> 
> - Update the copyright text
> - Check the return valuesof runtime and clock enable calls
> - Check the ignore_status in the isr 
> - Prevent writing to the fifo when controller is disabled.
> 
> Michal Simek (1):
>   xilinx: Update copyright text to correct format
> 
> Shubhrajyoti Datta (6):
>   tty: xilinx_uartps: Check the clk_enable return value
>   tty: xilinx_uartps: Add check for runtime_get_sync calls
>   tty: xilinx_uartps: Check clk_enable return type
>   tty: xilinx_uartps: Make the timeout unsigned
>   serial: uartps: Fix the ignore_status
>   serial: uartps: Prevent writes when the controller is disabled

Nit, why is the subject line prefix not unified here?  Shouldn't they
all be "tty: xilinx_uartps:" like 4 of them are?

Can you fix up and resend?

thanks,

greg k-h

Re: [PATCH 2/7] tty: xilinx_uartps: Check the clk_enable return value

[Greg KH]

On Fri, Apr 29, 2022 at 01:44:17PM +0530, Shubhrajyoti Datta wrote:
> Check the clk_enable return value.

That says what, but not why.

> 
> Addresses-Coverity: Event check_return.

Shouldn't this be a covertity id?

thanks,

greg k-h

Re: [PATCH 3/7] tty: xilinx_uartps: Add check for runtime_get_sync calls

[Greg KH]

On Fri, Apr 29, 2022 at 01:44:18PM +0530, Shubhrajyoti Datta wrote:
> Add a check for the return value of runtime get_sync calls.
> 
> Addresses-Coverity: Event check_return.
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
>  drivers/tty/serial/xilinx_uartps.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index 8f15fe24a0eb..868f4e587263 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -1100,13 +1100,17 @@ static void cdns_uart_poll_put_char(struct uart_port *port, unsigned char c)
>  static void cdns_uart_pm(struct uart_port *port, unsigned int state,
>  		   unsigned int oldstate)
>  {
> +	int ret;
> +
>  	switch (state) {
>  	case UART_PM_STATE_OFF:
>  		pm_runtime_mark_last_busy(port->dev);
>  		pm_runtime_put_autosuspend(port->dev);
>  		break;
>  	default:
> -		pm_runtime_get_sync(port->dev);
> +		ret = pm_runtime_get_sync(port->dev);
> +		if (ret < 0)
> +			dev_err(port->dev, "Failed to enable clocks\n");

So you just ignore the error?  SHouldn't you propagate it back upward?

thanks,

greg k-h

Re: [PATCH 4/7] tty: xilinx_uartps: Check clk_enable return value

[Greg KH]

On Fri, Apr 29, 2022 at 01:44:19PM +0530, Shubhrajyoti Datta wrote:
> Check for the clock enable return value.
> 
> Addresses-Coverity: Event check_return.
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
>  drivers/tty/serial/xilinx_uartps.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index 868f4e587263..bf0415f0a194 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -1347,12 +1347,18 @@ static int cdns_uart_resume(struct device *device)
>  	unsigned long flags;
>  	u32 ctrl_reg;
>  	int may_wake;
> +	int ret;
>  
>  	may_wake = device_may_wakeup(device);
>  
>  	if (console_suspend_enabled && uart_console(port) && !may_wake) {
> -		clk_enable(cdns_uart->pclk);
> -		clk_enable(cdns_uart->uartclk);
> +		ret = clk_enable(cdns_uart->pclk);
> +		if (ret)
> +			return ret;
> +
> +		ret = clk_enable(cdns_uart->uartclk);
> +		if (ret)
> +			return ret;

Shouldn't you disable the clock you enabled if this fails here?

thanks,

greg k-h

Re: [PATCH 5/7] tty: xilinx_uartps: Make the timeout unsigned

[Greg KH]

On Fri, Apr 29, 2022 at 01:44:20PM +0530, Shubhrajyoti Datta wrote:
> The timeout cannot be negative make it unsigned.
> Also the same for the trigger level.
> 
> Addresses-Coverity: Event incompatible_param.

What does this mean?

> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
>  drivers/tty/serial/xilinx_uartps.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index bf0415f0a194..289d70914956 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -34,12 +34,12 @@
>  #define TX_TIMEOUT		500000
>  
>  /* Rx Trigger level */
> -static int rx_trigger_level = 56;
> +static uint rx_trigger_level = 56;
>  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;
> +static uint rx_timeout = 10;
>  module_param(rx_timeout, uint, 0444);
>  MODULE_PARM_DESC(rx_timeout, "Rx timeout, 1-255");

As this is bounded (right?) why is this an issue?

Shouldn't it be a "byte" instead?

thanks,

greg k-h

Re: [PATCH 6/7] serial: uartps: Fix the ignore_status

[Greg KH]

On Fri, Apr 29, 2022 at 01:44:21PM +0530, Shubhrajyoti Datta wrote:
> Currently the ignore_status is not considered in the isr.
> Also the ignore_status is not updated in the set_termios.

When you say "also" that's a huge hint that it should be a separate
patch.

> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
>  drivers/tty/serial/xilinx_uartps.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index 289d70914956..81ba69c57716 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -375,6 +375,8 @@ static irqreturn_t cdns_uart_isr(int irq, void *dev_id)
>  		isrstatus &= ~CDNS_UART_IXR_TXEMPTY;
>  	}
>  
> +	isrstatus &= port->read_status_mask;
> +	isrstatus &= ~port->ignore_status_mask;
>  	/*
>  	 * Skip RX processing if RX is disabled as RXEMPTY will never be set
>  	 * as read bytes will not be removed from the FIFO.
> @@ -1583,6 +1585,8 @@ static int cdns_uart_probe(struct platform_device *pdev)
>  	port->dev = &pdev->dev;
>  	port->uartclk = clk_get_rate(cdns_uart_data->uartclk);
>  	port->private_data = cdns_uart_data;
> +	port->read_status_mask = CDNS_UART_IXR_TXEMPTY | CDNS_UART_IXR_RXTRIG |
> +			CDNS_UART_IXR_OVERRUN | CDNS_UART_IXR_TOUT;

You are doing two different things here, please make this two different
patches.

Also, what commit id does this fix?  Does it need to be backported?

thanks,

greg k-h

Re: [PATCH 7/7] serial: uartps: Prevent writes when the controller is disabled

[Greg KH]

On Fri, Apr 29, 2022 at 01:44:22PM +0530, Shubhrajyoti Datta wrote:
> Prevent writing to the fifo if the controller is disabled.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
>  drivers/tty/serial/xilinx_uartps.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index 81ba69c57716..f629c4ca940f 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -1150,6 +1150,13 @@ static struct uart_driver cdns_uart_uart_driver;
>   */
>  static void cdns_uart_console_putchar(struct uart_port *port, unsigned char ch)
>  {
> +	unsigned int ctrl_reg;
> +
> +	ctrl_reg = readl(port->membase + CDNS_UART_CR);
> +	while (ctrl_reg & CDNS_UART_CR_TX_DIS) {
> +		ctrl_reg = readl(port->membase + CDNS_UART_CR);
> +		cpu_relax();

You are spinning for forever?  With no timeout possible?

That's not ok :(


> +	}
>  	while (readl(port->membase + CDNS_UART_SR) & CDNS_UART_SR_TXFULL)
>  		cpu_relax();

Ick, same here, you better hope your hardware works...

thanks,

greg k-h

RE: [PATCH 3/7] tty: xilinx_uartps: Add check for runtime_get_sync calls

[Shubhrajyoti Datta]

> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Friday, May 6, 2022 2:21 AM
> To: Shubhrajyoti Datta <shubhraj@xilinx.com>
> Cc: linux-serial@vger.kernel.org; Michal Simek <michals@xilinx.com>;
> jirislaby@kernel.org; git <git@xilinx.com>; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 3/7] tty: xilinx_uartps: Add check for runtime_get_sync calls
> 
> On Fri, Apr 29, 2022 at 01:44:18PM +0530, Shubhrajyoti Datta wrote:
> > Add a check for the return value of runtime get_sync calls.
> >
> > Addresses-Coverity: Event check_return.
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > ---
> >  drivers/tty/serial/xilinx_uartps.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/xilinx_uartps.c
> > b/drivers/tty/serial/xilinx_uartps.c
> > index 8f15fe24a0eb..868f4e587263 100644
> > --- a/drivers/tty/serial/xilinx_uartps.c
> > +++ b/drivers/tty/serial/xilinx_uartps.c
> > @@ -1100,13 +1100,17 @@ static void cdns_uart_poll_put_char(struct
> > uart_port *port, unsigned char c)  static void cdns_uart_pm(struct uart_port
> *port, unsigned int state,
> >  		   unsigned int oldstate)
> >  {
> > +	int ret;
> > +
> >  	switch (state) {
> >  	case UART_PM_STATE_OFF:
> >  		pm_runtime_mark_last_busy(port->dev);
> >  		pm_runtime_put_autosuspend(port->dev);
> >  		break;
> >  	default:
> > -		pm_runtime_get_sync(port->dev);
> > +		ret = pm_runtime_get_sync(port->dev);
> > +		if (ret < 0)
> > +			dev_err(port->dev, "Failed to enable clocks\n");
> 
> So you just ignore the error?  SHouldn't you propagate it back upward?

The cdns_uart_pm is void so we cannot propagate it upward.

> 
> thanks,
> 
> greg k-h

RE: [PATCH 0/7] tty: xilinx_uartps: fixes and improvements

[Shubhrajyoti Datta]

Hi Greg,

Thanks for the review.

> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Friday, May 6, 2022 2:19 AM
> To: Shubhrajyoti Datta <shubhraj@xilinx.com>
> Cc: linux-serial@vger.kernel.org; Michal Simek <michals@xilinx.com>;
> jirislaby@kernel.org; git <git@xilinx.com>; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 0/7] tty: xilinx_uartps: fixes and improvements
> 
> On Fri, Apr 29, 2022 at 01:44:15PM +0530, Shubhrajyoti Datta wrote:
> > Many of the stuff is reported by static analysis changes are
> >
> > - Update the copyright text
> > - Check the return valuesof runtime and clock enable calls
> > - Check the ignore_status in the isr
> > - Prevent writing to the fifo when controller is disabled.
> >
> > Michal Simek (1):
> >   xilinx: Update copyright text to correct format
> >
> > Shubhrajyoti Datta (6):
> >   tty: xilinx_uartps: Check the clk_enable return value
> >   tty: xilinx_uartps: Add check for runtime_get_sync calls
> >   tty: xilinx_uartps: Check clk_enable return type
> >   tty: xilinx_uartps: Make the timeout unsigned
> >   serial: uartps: Fix the ignore_status
> >   serial: uartps: Prevent writes when the controller is disabled
> 
> Nit, why is the subject line prefix not unified here?  Shouldn't they all be "tty:
> xilinx_uartps:" like 4 of them are?
> 
> Can you fix up and resend?

I will fix up and resend in v2
> 
> thanks,
> 
> greg k-h

RE: [PATCH 2/7] tty: xilinx_uartps: Check the clk_enable return value

[Datta, Shubhrajyoti]

[AMD Official Use Only - General]

Hi Greg,

> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Friday, May 6, 2022 2:20 AM
> To: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> Cc: linux-serial@vger.kernel.org; michal.simek@xilinx.com;
> jirislaby@kernel.org; git@xilinx.com; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/7] tty: xilinx_uartps: Check the clk_enable return value
> 
> On Fri, Apr 29, 2022 at 01:44:17PM +0530, Shubhrajyoti Datta wrote:
> > Check the clk_enable return value.
> 
> That says what, but not why.
> 
Will fix v2.
> >
> > Addresses-Coverity: Event check_return.
> 
> Shouldn't this be a covertity id?

I could not find the warning in the Coverity that is run on the linux kernel. 
Somehow was seeing int when I was running  locally.

> 
> thanks,
> 
> greg k-h

Re: [PATCH 2/7] tty: xilinx_uartps: Check the clk_enable return value

[Greg KH]

On Thu, Jul 28, 2022 at 11:09:10AM +0000, Datta, Shubhrajyoti wrote:
> [AMD Official Use Only - General]
> 
> Hi Greg,
> 
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: Friday, May 6, 2022 2:20 AM
> > To: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> > Cc: linux-serial@vger.kernel.org; michal.simek@xilinx.com;
> > jirislaby@kernel.org; git@xilinx.com; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH 2/7] tty: xilinx_uartps: Check the clk_enable return value
> > 
> > On Fri, Apr 29, 2022 at 01:44:17PM +0530, Shubhrajyoti Datta wrote:
> > > Check the clk_enable return value.
> > 
> > That says what, but not why.
> > 
> Will fix v2.
> > >
> > > Addresses-Coverity: Event check_return.
> > 
> > Shouldn't this be a covertity id?
> 
> I could not find the warning in the Coverity that is run on the linux kernel. 
> Somehow was seeing int when I was running  locally.

Ok, then there's no need to reference coverity at all then :)

thanks,

greg k-h