| Message ID | 20250114070449.34226-2-andre.werner@systec-electronic.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v7,1/2] dt-bindings: serial: sc16is7xx: Add description for polling mode | expand |
On Tue, Jan 14, 2025 at 08:04:49AM +0100, 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). ... > 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 Please, rebase your series on top of the recent Linux Next.
Dear Andy, On Tue, 14 Jan 2025, Andy Shevchenko wrote: > On Tue, Jan 14, 2025 at 08:04:49AM +0100, 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). > > ... > > > 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 > > Please, rebase your series on top of the recent Linux Next. I performed the rebase and get a single new commit to fix the merge conflicts. How do you expect the submission? Is it a new commit or should I mark it as a fixup? Shall it have new version number v8 or is it a full new commit series? > > -- > With Best Regards, > Andy Shevchenko > > > Thanks and best regards, André
On Wed, Jan 15, 2025 at 7:23 AM Andre Werner <andre.werner@systec-electronic.com> wrote: > On Tue, 14 Jan 2025, Andy Shevchenko wrote: > > On Tue, Jan 14, 2025 at 08:04:49AM +0100, Andre Werner wrote: ... > > > 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 > > > > Please, rebase your series on top of the recent Linux Next. > > I performed the rebase and get a single new commit to fix the merge > conflicts. How do you expect the submission? Is it a new commit or > should I mark it as a fixup? Shall it have new version number v8 or > is it a full new commit series? When you rebase you should not get _merge_ conflicts. What is expected is that the first (DT binding) commit will be gone from your local branch, followed by the shrunken / conflicting second one.
On Thu, Jan 16, 2025 at 09:34:47AM +0100, 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). The commit message should be changed to reflect the code. I.e. "Fix the IRQ check to treat the negative values as No IRQ." > Signed-off-by: Andre Werner <andre.werner@systec-electronic.com> > Link: https://lore.kernel.org/r/20250110073104.1029633-2-andre.werner@systec-electronic.com This should not be here. > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> It's not signed by Greg. > V7: > - Try to improve and unify comments as requested. > - Fix typo in commit message: pull -> poll Version should be 1 and Subject has to be changed accordingly. ... > - s->polling = !!irq; > + s->polling = (irq <= 0); This is the only line has to be changed in the patch.
On Thu, Jan 16, 2025 at 09:34:47AM +0100, 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@systec-electronic.com> > Link: https://lore.kernel.org/r/20250110073104.1029633-2-andre.werner@systec-electronic.com > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> This whole commit is already in my tree, why is it submitted again? confused, greg k-h
Dear Greg, > On Thu, Jan 16, 2025 at 09:34:47AM +0100, 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@systec-electronic.com> > > Link: https://lore.kernel.org/r/20250110073104.1029633-2-andre.werner@systec-electronic.com > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > This whole commit is already in my tree, why is it submitted again? This is my fault. I misunderstood a comment from a reviewer. There is a patch that i still discussed in this thread: https://lkml.org/lkml/2025/1/16/398 Also the thread has a weird structure. Sorry! > > confused, > > greg k-h > Regards, André
On Fri, Jan 17, 2025 at 01:37:54PM +0100, Andre Werner wrote: > Dear Greg, > > > On Thu, Jan 16, 2025 at 09:34:47AM +0100, 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@systec-electronic.com> > > > Link: https://lore.kernel.org/r/20250110073104.1029633-2-andre.werner@systec-electronic.com > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > This whole commit is already in my tree, why is it submitted again? > > This is my fault. I misunderstood a comment from a reviewer. > There is a patch that i still discussed in this thread: > > https://lkml.org/lkml/2025/1/16/398 > > Also the thread has a weird structure. Sorry! Yes, please send a new version so that it can be considered to be applied properly, I can't dig it out of that thread easily, could you if you had to do it? :) thanks, greg k-h
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index a3093e09309f..348ddc3103cd 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_MS 10 #define SC16IS7XX_RECONF_MD BIT(0) #define SC16IS7XX_RECONF_IER BIT(1) #define SC16IS7XX_RECONF_RS485 BIT(2) @@ -348,6 +349,8 @@ struct sc16is7xx_port { u8 mctrl_mask; struct kthread_worker kworker; struct task_struct *kworker_task; + struct kthread_delayed_work poll_work; + bool polling; struct sc16is7xx_one p[]; }; @@ -861,6 +864,20 @@ static irqreturn_t sc16is7xx_irq(int irq, void *dev_id) return IRQ_HANDLED; } +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 and set to zero. + */ + sc16is7xx_irq(0, s); + + kthread_queue_delayed_work(&s->kworker, &s->poll_work, + msecs_to_jiffies(SC16IS7XX_POLL_PERIOD_MS)); +} + 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; @@ -1211,6 +1229,10 @@ static int sc16is7xx_startup(struct uart_port *port) sc16is7xx_enable_ms(port); uart_port_unlock_irqrestore(port, flags); + if (s->polling) + kthread_queue_delayed_work(&s->kworker, &s->poll_work, + msecs_to_jiffies(SC16IS7XX_POLL_PERIOD_MS)); + return 0; } @@ -1232,6 +1254,9 @@ static void sc16is7xx_shutdown(struct uart_port *port) sc16is7xx_power(port, 0); + if (s->polling) + kthread_cancel_delayed_work_sync(&s->poll_work); + kthread_flush_worker(&s->kworker); } @@ -1538,6 +1563,11 @@ 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 <= 0); + if (s->polling) + dev_dbg(dev, + "No interrupt pin definition, falling back to polling mode\n"); + s->clk = devm_clk_get_optional(dev, NULL); if (IS_ERR(s->clk)) return PTR_ERR(s->clk); @@ -1665,6 +1695,12 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype, goto out_ports; #endif + if (s->polling) { + /* Initialize a kthread work struct that is dedicated to polling */ + kthread_init_delayed_work(&s->poll_work, sc16is7xx_poll_proc); + return 0; + } + /* * Setup interrupt. We first try to acquire the IRQ line as level IRQ. * If that succeeds, we can allow sharing the interrupt as well. @@ -1724,6 +1760,9 @@ 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);
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@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 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 | 39 ++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)