linux-kernel - Re: [PATCH v7] serial: sc16is7xx: Add polling mode if no IRQ pin is available

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <58638faa-ddc2-bc64-5467-d62ea6a222cd@systec-electronic.com>
Date: Thu, 16 Jan 2025 09:42:27 +0100 (CET)
From: Andre Werner <andre.werner@...tec-electronic.com>
To: Andre Werner <andre.werner@...tec-electronic.com>
cc: gregkh@...uxfoundation.org, jirislaby@...nel.org, hvilleneuve@...onoff.com, 
    andy@...nel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
    linux-serial@...r.kernel.org, lech.perczak@...lingroup.com, 
    krzk+dt@...nel.org, conor+dt@...nel.org, robh@...nel.org
Subject: Re: [PATCH v7] serial: sc16is7xx: Add polling mode if no IRQ pin is
 available

Dear Andy,

I hope this is what you expected and is what I have understood so far.
I performed the rebase to
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
branch. This is a commit that has been created due to the rebase. The
commit message title is the same as from the previous commit already in
linux-next and the commit message body differs, because there was a typo
in the message and the description was still including the "interrupts"
property from dt to use to select polling mode that is not directly used
anymore in the latest version of the patch.

Sorry if I still did something wrong, but this is what I thought I had
to do.

Best regards,
André


On Thu, 16 Jan 2025, Andre Werner wrote:

> Fall back to polling mode if no interrupt is configured because there
> is no possibility to connect the interrupt pin.
>
> If no interrupt pin is available the driver uses a delayed worker to
> poll the state of interrupt status registers (IIR).
>
> Signed-off-by: Andre Werner <andre.werner@...tec-electronic.com>
> Link: https://lore.kernel.org/r/20250110073104.1029633-2-andre.werner@systec-electronic.com
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> ---
> 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
> V3:
> - Use suffix with units and drop a comment SC16IS7XX_POLL_PERIOD_MS. Sorry for that miss.
> - Make Kernel lowercase.
> V4:
> - Reword commit messages for better understanding.
> - Remove 'shutdown' property for canceling delayed worker.
> - Rename worker function: sc16is7xx_transmission_poll -> sc16is7xx_poll_proc
> - Unify argument for worker functions: kthread_work *work -> kthread_work *ws
> V5:
> - Replace of_property check with IRQ number check to set polling
>   property. This will add support for usage without device tree
>   definitions. Thanks for that advice.
> - Add blank line es requested.
> V6:
> - Use polling mode for IRQ numbers <= 0 which encounter no valid IRQ
>   were found/defined.
> V7:
> - Try to improve and unify comments as requested.
> - Fix typo in commit message: pull -> poll
> ---
>  drivers/tty/serial/sc16is7xx.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index 7b51cdc274fd..348ddc3103cd 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -868,10 +868,12 @@ static void sc16is7xx_poll_proc(struct kthread_work *ws)
>  {
>  	struct sc16is7xx_port *s = container_of(ws, struct sc16is7xx_port, poll_work.work);
>
> -	/* Reuse standard IRQ handler. Interrupt ID is unused in this context. */
> +	/*
> +	 * Reuse standard IRQ handler. Interrupt ID is unused in this
> +	 * context and set to zero.
> +	 */
>  	sc16is7xx_irq(0, s);
>
> -	/* Setup delay based on SC16IS7XX_POLL_PERIOD_MS */
>  	kthread_queue_delayed_work(&s->kworker, &s->poll_work,
>  				   msecs_to_jiffies(SC16IS7XX_POLL_PERIOD_MS));
>  }
> @@ -1561,7 +1563,7 @@ 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 = !!irq;
> +	s->polling = (irq <= 0);
>  	if (s->polling)
>  		dev_dbg(dev,
>  			"No interrupt pin definition, falling back to polling mode\n");
> @@ -1694,7 +1696,7 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
>  #endif
>
>  	if (s->polling) {
> -		/* Initialize kernel thread for polling */
> +		/* Initialize a kthread work struct that is dedicated to polling */
>  		kthread_init_delayed_work(&s->poll_work, sc16is7xx_poll_proc);
>  		return 0;
>  	}
> --
> 2.48.0
>
>