[PATCH 0/2] serial: 8250_dw: fall back to poll if there's no interrupt

[PATCH 0/2] serial: 8250_dw: fall back to poll if there's no interrupt

[Jisheng Zhang]

When there's no irq(this can be due to various reasons, for example,
no irq from HW support, or we just want to use poll solution, and so
on), falling back to poll is still better than no support at all.

patch1 makes the interrupt property in dt-binding optional
patch2 falls back to poll if there's no interrupt

Jisheng Zhang (2):
  dt-bindings: serial: snps-dw-apb-uart: make interrupt optional
  serial: 8250_dw: fall back to poll if there's no interrupt

 .../devicetree/bindings/serial/snps-dw-apb-uart.yaml      | 1 -
 drivers/tty/serial/8250/8250_dw.c                         | 8 ++++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

-- 
2.40.1

[PATCH 1/2] dt-bindings: serial: snps-dw-apb-uart: make interrupt optional

[Jisheng Zhang]

The driver fall back to poll style when there's no irq. "poll" still
looks better than no support.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
index 3862411c77b5..17c553123f96 100644
--- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
@@ -117,7 +117,6 @@ properties:
 required:
   - compatible
   - reg
-  - interrupts
 
 unevaluatedProperties: false
 
-- 
2.40.1

[PATCH 2/2] serial: 8250_dw: fall back to poll if there's no interrupt

[Jisheng Zhang]

When there's no irq(this can be due to various reasons, for example,
no irq from HW support, or we just want to use poll solution, and so
on), falling back to poll is still better than no support at all.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/tty/serial/8250/8250_dw.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 7db51781289e..39db768517eb 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -524,8 +524,12 @@ static int dw8250_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, -EINVAL, "no registers defined\n");
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
+	if (irq < 0) {
+		if (irq != -ENXIO)
+			return irq;
+		/* no interrupt -> fall back to polling */
+		irq = 0;
+	}
 
 	spin_lock_init(&p->lock);
 	p->mapbase	= regs->start;
-- 
2.40.1

Re: [PATCH 1/2] dt-bindings: serial: snps-dw-apb-uart: make interrupt optional

[Conor Dooley]

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

On Wed, Aug 02, 2023 at 11:05:44PM +0800, Jisheng Zhang wrote:
> The driver fall back to poll style when there's no irq. "poll" still
> looks better than no support.

What is the user for this where the interrupt is not actually wired up
in the hardware?

> 
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>  Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> index 3862411c77b5..17c553123f96 100644
> --- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
> @@ -117,7 +117,6 @@ properties:
>  required:
>    - compatible
>    - reg
> -  - interrupts
>  
>  unevaluatedProperties: false
>  
> -- 
> 2.40.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/2] serial: 8250_dw: fall back to poll if there's no interrupt

[Andy Shevchenko]

On Wed, Aug 02, 2023 at 11:05:45PM +0800, Jisheng Zhang wrote:
> When there's no irq(this can be due to various reasons, for example,
> no irq from HW support, or we just want to use poll solution, and so
> on), falling back to poll is still better than no support at all.

...

>  	irq = platform_get_irq(pdev, 0);

You will still have an error message. Perhaps you need to replace it with

	irq = platform_get_irq_optional(pdev, 0);

> -	if (irq < 0)
> -		return irq;
> +	if (irq < 0) {
> +		if (irq != -ENXIO)
> +			return irq;
> +		/* no interrupt -> fall back to polling */
> +		irq = 0;
> +	}

This can be slightly modified:

	/* no interrupt -> fall back to polling */
	if (irq == -ENXIO)
		irq = 0;
	if (irq < 0)
		return irq;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] dt-bindings: serial: snps-dw-apb-uart: make interrupt optional

[Andy Shevchenko]

On Wed, Aug 02, 2023 at 04:43:48PM +0100, Conor Dooley wrote:
> On Wed, Aug 02, 2023 at 11:05:44PM +0800, Jisheng Zhang wrote:
> > The driver fall back to poll style when there's no irq. "poll" still
> > looks better than no support.
> 
> What is the user for this where the interrupt is not actually wired up
> in the hardware?

FYI: kernel console doesn't use interrupts, so for example it might be
the debug port. Note, I have no idea of the Zhang's case, just my assumption.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] dt-bindings: serial: snps-dw-apb-uart: make interrupt optional

[Conor Dooley]

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

On Thu, Aug 03, 2023 at 12:57:44AM +0300, Andy Shevchenko wrote:
> On Wed, Aug 02, 2023 at 04:43:48PM +0100, Conor Dooley wrote:
> > On Wed, Aug 02, 2023 at 11:05:44PM +0800, Jisheng Zhang wrote:
> > > The driver fall back to poll style when there's no irq. "poll" still
> > > looks better than no support.
> > 
> > What is the user for this where the interrupt is not actually wired up
> > in the hardware?
> 
> FYI: kernel console doesn't use interrupts, so for example it might be
> the debug port. Note, I have no idea of the Zhang's case, just my assumption.

I'm less interested in what the software is doing, it's what the device 
that has not connected the interrupt is that I am curious about.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] dt-bindings: serial: snps-dw-apb-uart: make interrupt optional

[Andy Shevchenko]

On Wed, Aug 02, 2023 at 11:53:59PM +0100, Conor Dooley wrote:
> On Thu, Aug 03, 2023 at 12:57:44AM +0300, Andy Shevchenko wrote:
> > On Wed, Aug 02, 2023 at 04:43:48PM +0100, Conor Dooley wrote:
> > > On Wed, Aug 02, 2023 at 11:05:44PM +0800, Jisheng Zhang wrote:
> > > > The driver fall back to poll style when there's no irq. "poll" still
> > > > looks better than no support.
> > > 
> > > What is the user for this where the interrupt is not actually wired up
> > > in the hardware?
> > 
> > FYI: kernel console doesn't use interrupts, so for example it might be
> > the debug port. Note, I have no idea of the Zhang's case, just my assumption.
> 
> I'm less interested in what the software is doing, it's what the device 
> that has not connected the interrupt is that I am curious about.

As I said. The hw may be purely for debugging purposes (and yes, I have heard
about such a hardware).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] dt-bindings: serial: snps-dw-apb-uart: make interrupt optional

[Jisheng Zhang]

On Thu, Aug 03, 2023 at 02:40:05PM +0300, Andy Shevchenko wrote:
> On Wed, Aug 02, 2023 at 11:53:59PM +0100, Conor Dooley wrote:
> > On Thu, Aug 03, 2023 at 12:57:44AM +0300, Andy Shevchenko wrote:
> > > On Wed, Aug 02, 2023 at 04:43:48PM +0100, Conor Dooley wrote:
> > > > On Wed, Aug 02, 2023 at 11:05:44PM +0800, Jisheng Zhang wrote:
> > > > > The driver fall back to poll style when there's no irq. "poll" still
> > > > > looks better than no support.
> > > > 
> > > > What is the user for this where the interrupt is not actually wired up
> > > > in the hardware?
> > > 
> > > FYI: kernel console doesn't use interrupts, so for example it might be
> > > the debug port. Note, I have no idea of the Zhang's case, just my assumption.

Hi Conor, Andy,

Andy's assumption about the uart dedicated for debug purpose is true, since
it's for log only, so no interrupt need at all. We can also see such no
irq support in litex uart driver(liteuart.c) and even a 8250
variant(8250_ioc3.c).

Thanks
> > 
> > I'm less interested in what the software is doing, it's what the device 
> > that has not connected the interrupt is that I am curious about.
> 
> As I said. The hw may be purely for debugging purposes (and yes, I have heard
> about such a hardware).
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>