[PATCH v2 1/2] dt-bindings: serial: sc16is7xx: Add description for polling mode

[PATCH v2 1/2] dt-bindings: serial: sc16is7xx: Add description for polling mode

[Andre Werner]

Polling mode is enabled if the "interrupts" property is missing.
Thus, this commit deletes "interrupts" entry from "required" section
and adds a description for the fallback to polling mode at the
"interrupts" entry.

Signed-off-by: Andre Werner <andre.werner@systec-electronic.com>
---
 Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
index 88871480018e..ab39b95dae40 100644
--- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
+++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.yaml
@@ -23,6 +23,8 @@ properties:
     maxItems: 1
 
   interrupts:
+    description:
+      When missing, device driver uses polling instead.
     maxItems: 1
 
   clocks:
@@ -76,7 +78,6 @@ properties:
 required:
   - compatible
   - reg
-  - interrupts
 
 allOf:
   - $ref: /schemas/spi/spi-peripheral-props.yaml#
-- 
2.47.1

[PATCH v2 2/2] serial: sc16is7xx: Add polling feature if no IRQ usage possible

[Andre Werner]

Fall back to polling mode if no interrupt is configured because not
possible. If "interrupts" property is missing in devicetree the driver
uses a delayed worker to pull state of interrupt status registers.

Signed-off-by: Andre Werner <andre.werner@systec-electronic.com>
---
V2:
- Change warning for polling mode to debug log entry
- Correct typo: Resuse -> Reuse
- Format define with missing tabs for SC16IS7XX_POLL_PERIOD
- Format struct declaration sc16is7xx_one_config with missing tabs for polling and shutdown
- Adapt dtbinding with new polling feature
---
---
 drivers/tty/serial/sc16is7xx.c | 40 ++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index a3093e09309f..cfe63fe8a729 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -314,6 +314,7 @@
 #define SC16IS7XX_FIFO_SIZE		(64)
 #define SC16IS7XX_GPIOS_PER_BANK	4
 
+#define SC16IS7XX_POLL_PERIOD		10 /*ms*/
 #define SC16IS7XX_RECONF_MD		BIT(0)
 #define SC16IS7XX_RECONF_IER		BIT(1)
 #define SC16IS7XX_RECONF_RS485		BIT(2)
@@ -348,6 +349,9 @@ struct sc16is7xx_port {
 	u8				mctrl_mask;
 	struct kthread_worker		kworker;
 	struct task_struct		*kworker_task;
+	struct kthread_delayed_work	poll_work;
+	bool				polling;
+	bool				shutdown;
 	struct sc16is7xx_one		p[];
 };
 
@@ -861,6 +865,19 @@ static irqreturn_t sc16is7xx_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static void sc16is7xx_transmission_poll(struct kthread_work *work)
+{
+	struct sc16is7xx_port *s = container_of(work, struct sc16is7xx_port, poll_work.work);
+
+	/* Reuse standard IRQ handler. Interrupt ID is unused in this context. */
+	sc16is7xx_irq(0, s);
+
+	/* setup delay based on SC16IS7XX_POLL_PERIOD */
+	if (!s->shutdown)
+		kthread_queue_delayed_work(&s->kworker, &s->poll_work,
+					   msecs_to_jiffies(SC16IS7XX_POLL_PERIOD));
+}
+
 static void sc16is7xx_tx_proc(struct kthread_work *ws)
 {
 	struct uart_port *port = &(to_sc16is7xx_one(ws, tx_work)->port);
@@ -1149,6 +1166,7 @@ static int sc16is7xx_config_rs485(struct uart_port *port, struct ktermios *termi
 static int sc16is7xx_startup(struct uart_port *port)
 {
 	struct sc16is7xx_one *one = to_sc16is7xx_one(port, port);
+	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
 	unsigned int val;
 	unsigned long flags;
 
@@ -1210,6 +1228,11 @@ static int sc16is7xx_startup(struct uart_port *port)
 	uart_port_lock_irqsave(port, &flags);
 	sc16is7xx_enable_ms(port);
 	uart_port_unlock_irqrestore(port, flags);
+	if (s->polling) {
+		s->shutdown = false;
+		kthread_queue_delayed_work(&s->kworker, &s->poll_work,
+					   msecs_to_jiffies(SC16IS7XX_POLL_PERIOD));
+	}
 
 	return 0;
 }
@@ -1232,6 +1255,10 @@ static void sc16is7xx_shutdown(struct uart_port *port)
 
 	sc16is7xx_power(port, 0);
 
+	if (s->polling) {
+		s->shutdown = true;
+		kthread_cancel_delayed_work_sync(&s->poll_work);
+	}
 	kthread_flush_worker(&s->kworker);
 }
 
@@ -1537,7 +1564,13 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
 
 	/* Always ask for fixed clock rate from a property. */
 	device_property_read_u32(dev, "clock-frequency", &uartclk);
+	s->polling = !device_property_present(dev, "interrupts");
 
+	if (s->polling) {
+		dev_dbg(dev,
+			"No interrupt definition found. Falling back to polling mode.\n");
+		irq = 0;
+	}
 	s->clk = devm_clk_get_optional(dev, NULL);
 	if (IS_ERR(s->clk))
 		return PTR_ERR(s->clk);
@@ -1664,6 +1697,11 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
 	if (ret)
 		goto out_ports;
 #endif
+	if (s->polling) {
+		/* Initialize Kernel timer for polling */
+		kthread_init_delayed_work(&s->poll_work, sc16is7xx_transmission_poll);
+		return 0;
+	}
 
 	/*
 	 * Setup interrupt. We first try to acquire the IRQ line as level IRQ.
@@ -1724,6 +1762,8 @@ void sc16is7xx_remove(struct device *dev)
 		sc16is7xx_power(&s->p[i].port, 0);
 	}
 
+	if (s->polling)
+		kthread_cancel_delayed_work_sync(&s->poll_work);
 	kthread_flush_worker(&s->kworker);
 	kthread_stop(s->kworker_task);
 
-- 
2.47.1

Re: [PATCH v2 1/2] dt-bindings: serial: sc16is7xx: Add description for polling mode

[Krzysztof Kozlowski]

On Mon, Jan 06, 2025 at 09:50:50AM +0100, Andre Werner wrote:
> Polling mode is enabled if the "interrupts" property is missing.
> Thus, this commit deletes "interrupts" entry from "required" section
> and adds a description for the fallback to polling mode at the
> "interrupts" entry.

Can the device actually operate with interrupt line disconnected? I
skimmed through datasheet and they did not mention that as a valid
setup.

Best regards,
Krzysztof

Re: [External Email] Re: [PATCH v2 1/2] dt-bindings: serial: sc16is7xx: Add description for polling mode

[Andre Werner]

Dear Krzysztof,

On Tue, 7 Jan 2025, Krzysztof Kozlowski wrote:

> On Mon, Jan 06, 2025 at 09:50:50AM +0100, Andre Werner wrote:
> > Polling mode is enabled if the "interrupts" property is missing.
> > Thus, this commit deletes "interrupts" entry from "required" section
> > and adds a description for the fallback to polling mode at the
> > "interrupts" entry.
>
> Can the device actually operate with interrupt line disconnected? I
> skimmed through datasheet and they did not mention that as a valid
> setup.

For polling mode the datasheet said:

In Polled mode (IER[3:0] = 0000) the status of the receiver and transmitter can be
checked by polling the Line Status Register (LSR). This mode is an alternative to the FIFO
Interrupt mode of operation where the status of the receiver and transmitter is
automatically known by means of interrupts sent to the CPU.

This is what the driver does. It polls the LSR according to poll period
used in the driver. Unfortunately, the internal TX and especially RX
buffer is very small. So we need to poll in a very short period.


>
> Best regards,
> Krzysztof
>
>

Regards,

Andre

Re: [External Email] Re: [PATCH v2 1/2] dt-bindings: serial: sc16is7xx: Add description for polling mode

[Krzysztof Kozlowski]

On 07/01/2025 09:49, Andre Werner wrote:
> Dear Krzysztof,
> 
> On Tue, 7 Jan 2025, Krzysztof Kozlowski wrote:
> 
>> On Mon, Jan 06, 2025 at 09:50:50AM +0100, Andre Werner wrote:
>>> Polling mode is enabled if the "interrupts" property is missing.
>>> Thus, this commit deletes "interrupts" entry from "required" section
>>> and adds a description for the fallback to polling mode at the
>>> "interrupts" entry.
>>
>> Can the device actually operate with interrupt line disconnected? I
>> skimmed through datasheet and they did not mention that as a valid
>> setup.
> 
> For polling mode the datasheet said:
> 
> In Polled mode (IER[3:0] = 0000) the status of the receiver and transmitter can be
> checked by polling the Line Status Register (LSR). This mode is an alternative to the FIFO
> Interrupt mode of operation where the status of the receiver and transmitter is
> automatically known by means of interrupts sent to the CPU.

I guess this would mean that line can be physically disconnected.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof