Re: [RESEND PATCH] leds: ledtrig-pattern: Use last_repeat when applying hw pattern
On Tue, Jul 19, 2022 at 11:30:33PM +0200, Marijn Suijten wrote: > `last_repeat` holds the actual value requested by the user whereas > `repeat` is a software iteration variable that is unused in hardware > patterns. > > Furthermore `last_repeat` is the field returned to the user when reading > the `repeat` sysfs property. This field is initialized to `-1` which is > - together with `1` - the only valid value in the upcoming Qualcomm LPG > driver. It is thus unexpected when `repeat` with an initialization > value of `0` is passed into the the driver, when the sysfs property > clearly presents a value of `-1`. > > Signed-off-by: Marijn Suijten > Reviewed-by: Bjorn Andersson > Tested-by: Bjorn Andersson Looks correct to me: Reviewed-by: Johan Hovold
Re: [PATCH v4 2/2] leds: qcom-lpg: Add PM660L configuration and compatible
On Tue, Dec 19, 2023 at 11:06:08AM +0100, Marijn Suijten wrote: > On 2023-12-19 10:33:25, Johan Hovold wrote: > > On Tue, Dec 19, 2023 at 10:17:16AM +0100, Marijn Suijten wrote: > > > > > Note that I have one more unmerged leds patch around, that hasn't been > > > looked > > > at either. Would it help to send this once again, perhaps with more > > > reviewers/ > > > testing (Johan, would you mind taking a look too)? > > > > > > https://lore.kernel.org/linux-leds/20220719213034.1664056-1-marijn.suij...@somainline.org/ > > > > Yes, I suggest you resend that one too so that it ends up in Lee's > > inbox. > > I will rebase, test and resend it too. Just asking if you notice any glaring > issues with this patch, as it won't be the first time it has been resent after > not being looked at for some time. I haven't look at this code before and only skimmed it now, but your patch looks correct to me. Johan
Re: [PATCH v4 2/2] leds: qcom-lpg: Add PM660L configuration and compatible
On Tue, Dec 19, 2023 at 10:17:16AM +0100, Marijn Suijten wrote: > Note that I have one more unmerged leds patch around, that hasn't been looked > at either. Would it help to send this once again, perhaps with more > reviewers/ > testing (Johan, would you mind taking a look too)? > > https://lore.kernel.org/linux-leds/20220719213034.1664056-1-marijn.suij...@somainline.org/ Yes, I suggest you resend that one too so that it ends up in Lee's inbox. Johan
Re: [PATCH v4 2/2] leds: qcom-lpg: Add PM660L configuration and compatible
Hi Marijn and Lee,
On Tue, Jul 19, 2022 at 11:18:48PM +0200, Marijn Suijten wrote:
> Inherit PM660L PMIC LPG/triled block configuration from downstream
> drivers and DT sources, consisting of a triled block with automatic
> trickle charge control and source selection, three colored led channels
> belonging to the synchronized triled block and one loose PWM channel.
>
> Signed-off-by: Marijn Suijten
> Reviewed-by: Bjorn Andersson
> ---
>
> Changes since v3:
> - Rebased on -next;
> - (series) dropped DTS patches that have been applied through the
> Qualcomm DTS tree, leaving only leds changes (driver and
> accompanying dt-bindings).
> diff --git a/drivers/leds/rgb/leds-qcom-lpg.c
> b/drivers/leds/rgb/leds-qcom-lpg.c
> index 02f51cc61837..102ab0c33887 100644
> --- a/drivers/leds/rgb/leds-qcom-lpg.c
> +++ b/drivers/leds/rgb/leds-qcom-lpg.c
> @@ -1304,6 +1304,23 @@ static int lpg_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct lpg_data pm660l_lpg_data = {
> + .lut_base = 0xb000,
> + .lut_size = 49,
> +
> + .triled_base = 0xd000,
> + .triled_has_atc_ctl = true,
> + .triled_has_src_sel = true,
> +
> + .num_channels = 4,
> + .channels = (const struct lpg_channel_data[]) {
> + { .base = 0xb100, .triled_mask = BIT(5) },
> + { .base = 0xb200, .triled_mask = BIT(6) },
> + { .base = 0xb300, .triled_mask = BIT(7) },
> + { .base = 0xb400 },
> + },
> +};
> +
> static const struct lpg_data pm8916_pwm_data = {
> .num_channels = 1,
> .channels = (const struct lpg_channel_data[]) {
> @@ -1424,6 +1441,7 @@ static const struct lpg_data pm8350c_pwm_data = {
> };
>
> static const struct of_device_id lpg_of_table[] = {
> + { .compatible = "qcom,pm660l-lpg", .data = _lpg_data },
> { .compatible = "qcom,pm8150b-lpg", .data = _lpg_data },
> { .compatible = "qcom,pm8150l-lpg", .data = _lpg_data },
> { .compatible = "qcom,pm8350c-pwm", .data = _pwm_data },
When reviewing the Qualcomm SPMI PMIC bindings I noticed that this patch
was never picked up by the LEDs maintainer, while the binding and dtsi
changes made it in:
https://lore.kernel.org/r/20220719211848.1653920-2-marijn.suij...@somainline.org
Looks like it may still apply cleanly, but otherwise, would you mind
rebasing and resending so that Lee can pick this one up?
Johan
Re: [PATCH v9] USB: serial: cp210x: Add support for GPIOs on CP2108
On Thu, Apr 08, 2021 at 05:36:07PM +0700, Pho Tran wrote:
> From: Pho Tran
>
> Similar to other CP210x devices, GPIO interfaces (gpiochip) should be
> supported for CP2108.
>
> CP2108 has 4 serial interfaces but only 1 set of GPIO pins are shared
> to all of those interfaces. So, just need to initialize GPIOs of CP2108
> with only one interface (I use interface 0). It means just only 1 gpiochip
> device file will be created for CP2108.
>
> CP2108 has 16 GPIOs, So data types of several variables need to be is u16
> instead of u8(in struct cp210x_serial_private). This doesn't affect other
> CP210x devices.
>
> Because CP2108 has 16 GPIO pins, the parameter passed by cp210x functions
> will be different from other CP210x devices. So need to check part number
> of the device to use correct data format before sending commands to
> devices.
>
> Like CP2104, CP2108 have GPIO pins with configurable options. Therefore,
> should be mask all pins which are not in GPIO mode in cp2108_gpio_init()
> function.
>
> Signed-off-by: Pho Tran
> ---
>
> 04/08/2021: Patch v8 Fixed build warning reported by kernel test robot
> with ARCH=i386
> 04/05/2021: Patch v7 Modified commit message follow Greg's comment.
> 04/05/2021: Patch v6 Fixed build warning reported by kernel test robot
> with ARCH=x86_64
> 03/15/2021: Patch v5 Modified code according to comment of Johan:
> 1. Unified the handling of CP2108 and other types and
> take care about endianness.
> 2. Used suitable types data for variable.
> 3. Fixed cp2108_gpio_init and add more detail on
> commit message and comment.
> 4. Dropped some of the ones that don't add any value.
You left out a few changes here. You changed how the altfunctions were
detected and fixed the gpio_init error handling. Please include all
relevant changes in your changelogs.
> 03/12/2021: Patch v4 used git send-mail instead of send patch by manual
> follow the instructions of Johan Hovold .
> 03/05/2021: Patch v3 modified format and contents of changelog follow feedback
> from Johan Hovold .
> 03/04/2021: Patch v2 modified format patch as comment from
> Johan Hovold :
> 1. Break commit message lines at 80 cols
> 2. Use kernel u8 and u16 instead of the c99 ones.
> 03/01/2021: Initialed submission of patch "Make the CP210x driver work with
> GPIOs of CP2108.".
>
> drivers/usb/serial/cp210x.c | 254 +++-
> 1 file changed, 220 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index 7bec1e730b20..3812aac2b015 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -245,9 +245,9 @@ struct cp210x_serial_private {
> #ifdef CONFIG_GPIOLIB
> struct gpio_chipgc;
> boolgpio_registered;
> - u8 gpio_pushpull;
> - u8 gpio_altfunc;
> - u8 gpio_input;
> + u16 gpio_pushpull;
> + u16 gpio_altfunc;
> + u16 gpio_input;
> #endif
> u8 partnum;
> speed_t min_speed;
> @@ -399,6 +399,18 @@ static struct usb_serial_driver * const serial_drivers[]
> = {
> #define CP210X_PARTNUM_CP2102N_QFN20 0x22
> #define CP210X_PARTNUM_UNKNOWN 0xFF
>
> +/*
> + * CP2108 Define bit locations for EnhancedFxn_IFCx
> + * Refer to
> https://www.silabs.com/documents/public/application-notes/an978-cp210x-usb-to-uart-api-specification.pdf
> + * for more information.
Thanks for the reference, but no need to refer to this one more than
once; the quad-port-config comment below should be enough.
> + */
> +#define EF_IFC_GPIO_TXLED0x01
> +#define EF_IFC_GPIO_RXLED0x02
> +#define EF_IFC_GPIO_RS4850x04
> +#define EF_IFC_GPIO_RS485_LOGIC 0x08
> +#define EF_IFC_GPIO_CLOCK0x10
> +#define EF_IFC_DYNAMIC_SUSPEND 0x40
> +
These should go after the quad-port-config structure with the other port
config defines.
And shouldn't they have a CP2108_ prefix?
> /* CP210X_GET_COMM_STATUS returns these 0x13 bytes */
> struct cp210x_comm_status {
> __le32 ulErrors;
> @@ -500,6 +512,45 @@ struct cp210x_single_port_config {
> u8 device_cfg;
> } __packed;
>
> +/*
> + * Quad Port Config definitions
> + * Refer to
> https://www.silabs.com/documents/public/application-notes/an978-cp210x-usb-to-uart-api-specification.pdf
> + * for more information.
> + * CP210X_VENDOR_SPECIFIC, CP210X_GET_PORTCONFIG call reads these 0x49 bytes
> + * on a CP2108 chip.
> + *
Re: [PATCH 2/3] serial: stm32: fix threaded interrupt handling
On Fri, Apr 16, 2021 at 10:35:25PM +0800, dillon min wrote:
> Hi Johan
>
> Thanks for share your patch.
>
> Johan Hovold 于2021年4月16日 周五22:11写道：
>
> > When DMA is enabled the receive handler runs in a threaded handler, but
> > the primary handler up until very recently neither disabled interrupts
> > in the device or used IRQF_ONESHOT. This would lead to a deadlock if an
> > interrupt comes in while the threaded receive handler is running under
> > the port lock.
> >
> Greg told me there was a patch fixed this case. In case hard irq &
> threaded_fn both offered. The local_irq_save() will be executed before call
> driver’s threaded handler.
>
> Post the original mail from Greg
>
> Please see 81e2073c175b ("genirq: Disable interrupts for force threaded
> handlers") for when threaded irq handlers have irqs disabled, isn't that
> the case you are trying to "protect" from here?
>
> Why is the "threaded" flag used at all? The driver should not care.
>
> Also see 9baedb7baeda ("serial: imx: drop workaround for forced irq
> threading") in linux-next for an example of how this was fixed up in a
> serial driver.
Neither of these commits are (directly) related to the problem this
patch addresses (they are about force-threaded handlers, this is about a
normal threaded handler which run with interrupts enabled).
Johan
Re: [PATCH 2/3] serial: stm32: fix threaded interrupt handling
On Fri, Apr 16, 2021 at 04:05:56PM +0200, Johan Hovold wrote:
> When DMA is enabled the receive handler runs in a threaded handler, but
> the primary handler up until very recently neither disabled interrupts
Scratch the "up until very recently" bit here since the driver still
doesn't disable interrupt in the device (it just disables all interrupts
in the threaded handler). The rest stands as is.
> in the device or used IRQF_ONESHOT. This would lead to a deadlock if an
> interrupt comes in while the threaded receive handler is running under
> the port lock.
>
> Commit ad7676812437 ("serial: stm32: fix a deadlock condition with
> wakeup event") claimed to fix an unrelated deadlock, but unfortunately
> also disabled interrupts in the threaded handler. While this prevents
> the deadlock mentioned in the previous paragraph it also defeats the
> purpose of using a threaded handler in the first place.
>
> Fix this by making the interrupt one-shot and not disabling interrupts
> in the threaded handler.
>
> Note that (receive) DMA must not be used for a console port as the
> threaded handler could be interrupted while holding the port lock,
> something which could lead to a deadlock in case an interrupt handler
> ends up calling printk.
Johan
[PATCH 2/3] serial: stm32: fix threaded interrupt handling
When DMA is enabled the receive handler runs in a threaded handler, but
the primary handler up until very recently neither disabled interrupts
in the device or used IRQF_ONESHOT. This would lead to a deadlock if an
interrupt comes in while the threaded receive handler is running under
the port lock.
Commit ad7676812437 ("serial: stm32: fix a deadlock condition with
wakeup event") claimed to fix an unrelated deadlock, but unfortunately
also disabled interrupts in the threaded handler. While this prevents
the deadlock mentioned in the previous paragraph it also defeats the
purpose of using a threaded handler in the first place.
Fix this by making the interrupt one-shot and not disabling interrupts
in the threaded handler.
Note that (receive) DMA must not be used for a console port as the
threaded handler could be interrupted while holding the port lock,
something which could lead to a deadlock in case an interrupt handler
ends up calling printk.
Fixes: ad7676812437 ("serial: stm32: fix a deadlock condition with wakeup
event")
Fixes: 3489187204eb ("serial: stm32: adding dma support")
Cc: sta...@vger.kernel.org # 4.9
Cc: Alexandre TORGUE
Cc: Gerald Baeza
Signed-off-by: Johan Hovold
---
drivers/tty/serial/stm32-usart.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index 4d277804c63e..3524ed2c0c73 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -214,14 +214,11 @@ static void stm32_usart_receive_chars(struct uart_port
*port, bool threaded)
struct tty_port *tport = >state->port;
struct stm32_port *stm32_port = to_stm32_port(port);
const struct stm32_usart_offsets *ofs = _port->info->ofs;
- unsigned long c, flags;
+ unsigned long c;
u32 sr;
char flag;
- if (threaded)
- spin_lock_irqsave(>lock, flags);
- else
- spin_lock(>lock);
+ spin_lock(>lock);
while (stm32_usart_pending_rx(port, , _port->last_res,
threaded)) {
@@ -278,10 +275,7 @@ static void stm32_usart_receive_chars(struct uart_port
*port, bool threaded)
uart_insert_char(port, sr, USART_SR_ORE, c, flag);
}
- if (threaded)
- spin_unlock_irqrestore(>lock, flags);
- else
- spin_unlock(>lock);
+ spin_unlock(>lock);
tty_flip_buffer_push(tport);
}
@@ -667,7 +661,8 @@ static int stm32_usart_startup(struct uart_port *port)
ret = request_threaded_irq(port->irq, stm32_usart_interrupt,
stm32_usart_threaded_interrupt,
- IRQF_NO_SUSPEND, name, port);
+ IRQF_ONESHOT | IRQF_NO_SUSPEND,
+ name, port);
if (ret)
return ret;
@@ -1156,6 +1151,13 @@ static int stm32_usart_of_dma_rx_probe(struct stm32_port
*stm32port,
struct dma_async_tx_descriptor *desc = NULL;
int ret;
+ /*
+* Using DMA and threaded handler for the console could lead to
+* deadlocks.
+*/
+ if (uart_console(port))
+ return -ENODEV;
+
/* Request DMA RX channel */
stm32port->rx_ch = dma_request_slave_channel(dev, "rx");
if (!stm32port->rx_ch) {
--
2.26.3
[PATCH 1/3] serial: do not restore interrupt state in sysrq helper
The uart_unlock_and_check_sysrq() helper can be used to defer processing
of sysrq until the interrupt handler has released the port lock and is
about to return.
Since commit 81e2073c175b ("genirq: Disable interrupts for force
threaded handlers") interrupt handlers that are not explicitly requested
as threaded are always called with interrupts disabled and there is no
need to save the interrupt state when taking the port lock.
Instead of adding another sysrq helper for when the interrupt state has
not needlessly been saved, drop the state parameter from
uart_unlock_and_check_sysrq() and update its callers to no longer
explicitly disable interrupts in their interrupt handlers.
Cc: Joel Stanley
Cc: Andrew Jeffery
Cc: Andy Gross
Cc: Bjorn Andersson
Signed-off-by: Johan Hovold
---
drivers/tty/serial/8250/8250_aspeed_vuart.c | 5 ++---
drivers/tty/serial/8250/8250_fsl.c | 11 ++-
drivers/tty/serial/8250/8250_omap.c | 6 +++---
drivers/tty/serial/8250/8250_port.c | 6 +++---
drivers/tty/serial/qcom_geni_serial.c | 6 +++---
include/linux/serial_core.h | 10 +-
6 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index 3c239d98747f..61550f24a2d3 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -320,7 +320,6 @@ static int aspeed_vuart_handle_irq(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
unsigned int iir, lsr;
- unsigned long flags;
int space, count;
iir = serial_port_in(port, UART_IIR);
@@ -328,7 +327,7 @@ static int aspeed_vuart_handle_irq(struct uart_port *port)
if (iir & UART_IIR_NO_INT)
return 0;
- spin_lock_irqsave(>lock, flags);
+ spin_lock(>lock);
lsr = serial_port_in(port, UART_LSR);
@@ -364,7 +363,7 @@ static int aspeed_vuart_handle_irq(struct uart_port *port)
if (lsr & UART_LSR_THRE)
serial8250_tx_chars(up);
- uart_unlock_and_check_sysrq(port, flags);
+ uart_unlock_and_check_sysrq(port);
return 1;
}
diff --git a/drivers/tty/serial/8250/8250_fsl.c
b/drivers/tty/serial/8250/8250_fsl.c
index cd19400b65ae..4e75d2e4f87c 100644
--- a/drivers/tty/serial/8250/8250_fsl.c
+++ b/drivers/tty/serial/8250/8250_fsl.c
@@ -30,15 +30,14 @@ struct fsl8250_data {
int fsl8250_handle_irq(struct uart_port *port)
{
unsigned char lsr, orig_lsr;
- unsigned long flags;
unsigned int iir;
struct uart_8250_port *up = up_to_u8250p(port);
- spin_lock_irqsave(>port.lock, flags);
+ spin_lock(>port.lock);
iir = port->serial_in(port, UART_IIR);
if (iir & UART_IIR_NO_INT) {
- spin_unlock_irqrestore(>port.lock, flags);
+ spin_unlock(>port.lock);
return 0;
}
@@ -46,7 +45,7 @@ int fsl8250_handle_irq(struct uart_port *port)
if (unlikely(up->lsr_saved_flags & UART_LSR_BI)) {
up->lsr_saved_flags &= ~UART_LSR_BI;
port->serial_in(port, UART_RX);
- spin_unlock_irqrestore(>port.lock, flags);
+ spin_unlock(>port.lock);
return 1;
}
@@ -82,7 +81,9 @@ int fsl8250_handle_irq(struct uart_port *port)
serial8250_tx_chars(up);
up->lsr_saved_flags = orig_lsr;
- uart_unlock_and_check_sysrq(>port, flags);
+
+ uart_unlock_and_check_sysrq(>port);
+
return 1;
}
EXPORT_SYMBOL_GPL(fsl8250_handle_irq);
diff --git a/drivers/tty/serial/8250/8250_omap.c
b/drivers/tty/serial/8250/8250_omap.c
index 23e0decde33e..8ac11eaeca51 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1143,7 +1143,6 @@ static int omap_8250_dma_handle_irq(struct uart_port
*port)
struct uart_8250_port *up = up_to_u8250p(port);
struct omap8250_priv *priv = up->port.private_data;
unsigned char status;
- unsigned long flags;
u8 iir;
serial8250_rpm_get(up);
@@ -1154,7 +1153,7 @@ static int omap_8250_dma_handle_irq(struct uart_port
*port)
return IRQ_HANDLED;
}
- spin_lock_irqsave(>lock, flags);
+ spin_lock(>lock);
status = serial_port_in(port, UART_LSR);
@@ -1179,7 +1178,8 @@ static int omap_8250_dma_handle_irq(struct uart_port
*port)
}
}
- uart_unlock_and_check_sysrq(port, flags);
+ uart_unlock_and_check_sysrq(port);
+
serial8250_rpm_put(up);
return 1;
}
diff --git a/drivers/tty/serial/8250/8250_port.c
b/drivers/tty/serial/8250/8250_port.c
index 9019f8f626bb..d45dab1ab316 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@
[PATCH 3/3] serial: stm32: defer sysrq processing
Use the uart_unlock_and_check_sysrq() helper to defer sysrq processing until receive processing is done and the port lock has been released. This allows cleaning up the console_write() implementation by not having to work around the recursive sysrq case (by dropping locking completely) and also makes the console code work with PREEMPT_RT by no longer relying on local_irq_save(). Signed-off-by: Johan Hovold --- drivers/tty/serial/stm32-usart.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c index 3524ed2c0c73..24a1dfe7058b 100644 --- a/drivers/tty/serial/stm32-usart.c +++ b/drivers/tty/serial/stm32-usart.c @@ -270,12 +270,12 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded) } } - if (uart_handle_sysrq_char(port, c)) + if (uart_prepare_sysrq_char(port, c)) continue; uart_insert_char(port, sr, USART_SR_ORE, c, flag); } - spin_unlock(>lock); + uart_unlock_and_check_sysrq(port); tty_flip_buffer_push(tport); } @@ -1430,13 +1430,10 @@ static void stm32_usart_console_write(struct console *co, const char *s, u32 old_cr1, new_cr1; int locked = 1; - local_irq_save(flags); - if (port->sysrq) - locked = 0; - else if (oops_in_progress) - locked = spin_trylock(>lock); + if (oops_in_progress) + locked = spin_trylock_irqsave(>lock, flags); else - spin_lock(>lock); + spin_lock_irqsave(>lock, flags); /* Save and disable interrupts, enable the transmitter */ old_cr1 = readl_relaxed(port->membase + ofs->cr1); @@ -1450,8 +1447,7 @@ static void stm32_usart_console_write(struct console *co, const char *s, writel_relaxed(old_cr1, port->membase + ofs->cr1); if (locked) - spin_unlock(>lock); - local_irq_restore(flags); + spin_unlock_irqrestore(>lock, flags); } static int stm32_usart_console_setup(struct console *co, char *options) -- 2.26.3
[PATCH 0/3] serial: sysrq cleanup and stm32 fixes
The first patch cleans up the interrupt handlers that rely on deferred sysrq processing by not needlessly saving the interrupt state. The second fixes the threaded interrupt handling of the stm32 driver and properly fixes a couple of deadlocks that were incidentally worked around by a recent commit. The third patch cleans up the stm32 console implementation by switching to deferred sysrq processing, thereby making the console code more robust and allowing it to be used with PREEMPT_RT. This series is against tty-next and and have only been compile tested. Johan Johan Hovold (3): serial: do not restore interrupt state in sysrq helper serial: stm32: fix threaded interrupt handling serial: stm32: defer sysrq processing drivers/tty/serial/8250/8250_aspeed_vuart.c | 5 ++- drivers/tty/serial/8250/8250_fsl.c | 11 --- drivers/tty/serial/8250/8250_omap.c | 6 ++-- drivers/tty/serial/8250/8250_port.c | 6 ++-- drivers/tty/serial/qcom_geni_serial.c | 6 ++-- drivers/tty/serial/stm32-usart.c| 36 ++--- include/linux/serial_core.h | 10 +++--- 7 files changed, 39 insertions(+), 41 deletions(-) -- 2.26.3
Re: [PATCH v3] serial: stm32: optimize spin lock usage
On Fri, Apr 16, 2021 at 06:10:41PM +0800, dillon.min...@gmail.com wrote:
> From: dillon min
>
> This patch aims to fix two potential bug:
> - no lock to protect uart register in this case
>
> stm32_usart_threaded_interrupt()
> spin_lock(>lock);
> ...
> stm32_usart_receive_chars()
>uart_handle_sysrq_char();
>sysrq_function();
>printk();
> stm32_usart_console_write();
>locked = 0; //since port->sysrq is not zero,
> no lock to protect forward register
> access.
>
> - if add spin_trylock_irqsave() to protect uart register for sysrq = 1 case,
> that might got recursive locking under UP.
> So, use uart_prepare_sysrq_char(), uart_unlock_and_check_sysrq()
> move sysrq handler position to irq/thread_d handler, just record
> sysrq_ch in stm32_usart_receive_chars() by uart_prepare_sysrq_char()
> delay the sysrq process to next interrupt handler.
>
> new flow:
>
> stm32_usart_threaded_interrupt()/stm32_usart_interrupt()
> spin_lock_irqsave(>lock);
> ...
> uart_unlock_and_check_sysrq();
> spin_unlock_irqrestore();
> handle_sysrq(sysrq_ch);
> stm32_usart_threaded_interrupt()//stm32_usart_interrupt() return
>
> Cc: Johan Hovold
> Cc: Alexandre Torgue
> Cc: Maxime Coquelin
> Cc: Gerald Baeza
> Cc: Erwan Le Ray
> Reported-by: kernel test robot
> Signed-off-by: dillon min
> ---
> v3: add uart_prepare_sysrq_char(), uart_unlock_and_check_sysrq() to move
> sysrq handler inside interrupt routinei to avoid recursive locking,
> according to Johan Hovold suggestion, thanks.
>
> drivers/tty/serial/stm32-usart.c | 24 +++-
> 1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/tty/serial/stm32-usart.c
> b/drivers/tty/serial/stm32-usart.c
> index b3675cf25a69..981f50ec784e 100644
> --- a/drivers/tty/serial/stm32-usart.c
> +++ b/drivers/tty/serial/stm32-usart.c
> @@ -271,7 +271,7 @@ static void stm32_usart_receive_chars(struct uart_port
> *port, bool threaded)
> }
> }
>
> - if (uart_handle_sysrq_char(port, c))
> + if (uart_prepare_sysrq_char(port, c))
> continue;
> uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> }
> @@ -457,9 +457,10 @@ static irqreturn_t stm32_usart_interrupt(int irq, void
> *ptr)
> struct uart_port *port = ptr;
> struct stm32_port *stm32_port = to_stm32_port(port);
> const struct stm32_usart_offsets *ofs = _port->info->ofs;
> + unsigned long flags;
> u32 sr;
>
> - spin_lock(>lock);
> + spin_lock_irqsave(>lock, flags);
>
> sr = readl_relaxed(port->membase + ofs->isr);
>
> @@ -477,7 +478,7 @@ static irqreturn_t stm32_usart_interrupt(int irq, void
> *ptr)
> if ((sr & USART_SR_TXE) && !(stm32_port->tx_ch))
> stm32_usart_transmit_chars(port);
>
> - spin_unlock(>lock);
> + uart_unlock_and_check_sysrq(port, flags);
>
> if (stm32_port->rx_ch)
> return IRQ_WAKE_THREAD;
> @@ -489,13 +490,14 @@ static irqreturn_t stm32_usart_threaded_interrupt(int
> irq, void *ptr)
> {
> struct uart_port *port = ptr;
> struct stm32_port *stm32_port = to_stm32_port(port);
> + unsigned long flags;
>
> - spin_lock(>lock);
> + spin_lock_irqsave(>lock, flags);
This essentially turns the threaded handler into a non-threaded one,
which is a bad idea.
> if (stm32_port->rx_ch)
> stm32_usart_receive_chars(port, true);
>
> - spin_unlock(>lock);
> + uart_unlock_and_check_sysrq(port, flags);
>
> return IRQ_HANDLED;
> }
You also didn't base this patch on tty-next, which has a number of
updates to this driver. Before noting that myself, I had fixed a couple
of deadlocks in this driver which turned out to have been incidentally
fixed by an unrelated path in -next.
I'll be posting a series that should fix up all of this.
Johan
Re: [PATCH v2] serial: stm32: optimize spin lock usage
[ Please avoid top-posting. ] On Thu, Apr 15, 2021 at 07:09:14PM +0200, Erwan LE RAY wrote: > Hi Dillon, > > STM32MP151 is mono-core, but both STM32MP153 and STM32MP157 are > dual-core (see > https://www.st.com/content/st_com/en/products/microcontrollers-microprocessors/stm32-arm-cortex-mpus.html). > So your point is fully relevant, thanks. > > ST already fixed the same issue in st-asc.c driver in the past (see > ef49ffd8), because a systematic deadlock was detected with RT kernel. That's not the same issue. The above mentioned commit fixed an issue on *RT* where local_irq_save() should be avoided. > You proposed a first implementation in your patch, and a second one in > the discussion. It seems that your initial proposal (ie your V2 patch) > is the most standard one (implemented in 6 drivers). The second > implementation is implemented by only 1 company. > > It looks that the solution is to avoid locking in the sysrq case and > trylock in the oops_in_progress case (see detailed analysis in > 677fe555cbfb1). > > So your initial patch looks to the right proposal, but it would be safer > if Greg could confirm it. That would only fix the RT issue (and by making the sysrq one slightly worse). Using uart_unlock_and_check_sysrq() would address both issues. Johan
Re: [PATCH v2] serial: stm32: optimize spin lock usage
On Tue, Apr 13, 2021 at 07:44:39AM +0800, dillon min wrote:
> Hi Johan, Erwan
>
> It seems still a bit of a problem in the current version, not deadlock
> but access register at the same time.
>
> For driver , we should consider it running under smp, let's think
> about it for this case:
>
> static void stm32_usart_console_write(struct console *co, const char *s,
> unsigned int cnt)
> {
> .
> local_irq_save(flags);
> if (port->sysrq)
> locked = 0;
> .
> access register cr1, tdr, isr
> .
>
> local_irq_restore(flags);
> }
>
> if port->sysrq is 1, stm32_usart_console_write() just disable local
> irq response by local_irq_save(), at the time of access register cr1,
> tdr, isr. an TXE interrupt raised, for other cores(I know stm32
> mpu/mcu do not have multi cores, just assume it has), it still has a
> chance to handle interrupt. Then there is no lock to protect the uart
> register.
Right, the sysrq handling is a bit of a hack.
> changes to below, should be more safe:
>
> .
> if (port->sysrq || oops_in_progress)
> locked = spin_trylock_irqsave(>lock, flags);
Except that the lock debugging code would detect the attempt at
recursive locking here and complain loudly on UP.
If you really want to fix this, we have uart_unlock_and_check_sysrq()
which can be used to defer sysrq processing until the interrupt handler
has released the lock.
> else
> spin_lock_irqsave(>lock, flags);
>
>
>
> if (locked)
> spin_unlock_irqrestore(>lock, flags);
Johan
Re: [PATCH 00/13] tty.h cleanups
On Thu, Apr 15, 2021 at 10:21:54AM +0200, Greg Kroah-Hartman wrote: > On Fri, Apr 09, 2021 at 09:32:45AM +0200, Johan Hovold wrote: > > On Thu, Apr 08, 2021 at 08:01:08PM +0200, Greg Kroah-Hartman wrote: > > > On Thu, Apr 08, 2021 at 04:25:22PM +0200, Johan Hovold wrote: > > > > On Thu, Apr 08, 2021 at 02:51:21PM +0200, Greg Kroah-Hartman wrote: > > > > > Turns out there is a lot of tty-internal stuff in include/linux/tty.h > > > > > that do not belong there. Create a internal-to-the-tty-layer .h file > > > > > for these types of things and move function prototypes to it instead > > > > > of > > > > > being in the system-wide header file. > > > > > > > > > > Along the way clean up the use of some old tty-only debugging macros > > > > > and > > > > > use the in-kernel dev_*() calls instead. > > > > > > > > I'm afraid that's not a good idea since not all ttys have a > > > > corresponding class device. Notable exception include pseudo terminals > > > > and serdev. > > > > > > > > While dev_printk() can handle a NULL device argument without crashing, > > > > we'll actually lose log information by removing the tty printk helpers. > > > > > > I think the same info will be printed here as before, just some NULL > > > information at the beginning, right? And the benifits overall (for real > > > tty devices), should outweigh the few devices that do not have this > > > information. > > > > No, you'll only be losing information (tty driver and tty name). Here's > > a pty example, where the first line in each pair use dev_info() and the > > second tty_info(): > > > > [ 10.235331] (NULL device *): tty_get_device > > [ 10.235441] ptm ptm0: tty_get_device > > > > [ 10.235586] (NULL device *): tty_get_device > > [ 10.235674] pts pts0: tty_get_device > > > > and similar for serdev, which is becoming more and more common. > > Ok, good point, I'll go apply only the first 2 patches in this series > (moving the macros out of tty.h and removing the unused one) and then > will redo this set of patches again. Perhaps no harm in leaving the tty_info() on in there for consistency. We have users of the _ratelimited() flavour of it (even if there's no dependency). > I think a better tty_msg() macro is warrented so that we can provide > dev_*() output if we have a device, otherwise fall back to the old > style to preserve functionality. Possibly, but the dev_printk() for the tty class devices wouldn't provide any more info than what's already there (i.e. driver name + tty name). (And associating ttys with other devices and drivers (e.g. a serdev client and its driver) might not be what we want since you lose the connection to the underlying tty driver.) Johan
Re: [PATCH v2] USB: Add LPM quirk for Lenovo ThinkPad USB-C Dock Gen2 Ethernet
On Mon, Apr 12, 2021 at 09:05:20PM +0800, Kai-Heng Feng wrote:
> This is another branded 8153 device that doesn't work well with LPM
> enabled:
> [ 400.597506] r8152 5-1.1:1.0 enx482ae3a2a6f0: Tx status -71
>
> So disable LPM to resolve the issue.
>
> BugLink: https://bugs.launchpad.net/bugs/1922651
> Signed-off-by: Kai-Heng Feng
> ---
> v2:
> - Put the quirk in the right order.
Seriously... You sent the exact same patch again. Still not ordered.
>
> drivers/usb/core/quirks.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> index 76ac5d6555ae..dfedb51cf832 100644
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -434,6 +434,9 @@ static const struct usb_device_id usb_quirk_list[] = {
> { USB_DEVICE(0x1532, 0x0116), .driver_info =
> USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL },
>
> + /* Lenovo ThinkPad USB-C Dock Gen2 Ethernet (RTL8153 GigE) */
> + { USB_DEVICE(0x17ef, 0xa387), .driver_info = USB_QUIRK_NO_LPM },
> +
> /* Lenovo ThinkCenter A630Z TI024Gen3 usb-audio */
> { USB_DEVICE(0x17ef, 0xa012), .driver_info =
> USB_QUIRK_DISCONNECT_SUSPEND },
Johan
Re: [PATCH v2] serial: stm32: optimize spin lock usage
On Mon, Apr 12, 2021 at 05:31:38PM +0800, dillon.min...@gmail.com wrote: > From: dillon min > > To avoid potential deadlock in spin_lock usage, use spin_lock_irqsave, > spin_trylock_irqsave(), spin_unlock_irqrestore() in process context. This doesn't make much sense as console_write can be called in any context. And where's the deadlock you claim to be fixing here? > remove unused local_irq_save/restore call. > > Cc: Alexandre Torgue > Cc: Maxime Coquelin > Cc: Gerald Baeza > Cc: Erwan Le Ray > Reported-by: kernel test robot > Signed-off-by: dillon min > --- > v2: remove unused code from stm32_usart_threaded_interrupt() according from > Greg's review. > > drivers/tty/serial/stm32-usart.c | 8 +++- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/tty/serial/stm32-usart.c > b/drivers/tty/serial/stm32-usart.c > index b3675cf25a69..b1ba5e36e36e 100644 > --- a/drivers/tty/serial/stm32-usart.c > +++ b/drivers/tty/serial/stm32-usart.c > @@ -1354,13 +1354,12 @@ static void stm32_usart_console_write(struct console > *co, const char *s, > u32 old_cr1, new_cr1; > int locked = 1; > > - local_irq_save(flags); > if (port->sysrq) > locked = 0; > else if (oops_in_progress) > - locked = spin_trylock(>lock); > + locked = spin_trylock_irqsave(>lock, flags); > else > - spin_lock(>lock); > + spin_lock_irqsave(>lock, flags); > > /* Save and disable interrupts, enable the transmitter */ > old_cr1 = readl_relaxed(port->membase + ofs->cr1); > @@ -1374,8 +1373,7 @@ static void stm32_usart_console_write(struct console > *co, const char *s, > writel_relaxed(old_cr1, port->membase + ofs->cr1); > > if (locked) > - spin_unlock(>lock); > - local_irq_restore(flags); > + spin_unlock_irqrestore(>lock, flags); > } > > static int stm32_usart_console_setup(struct console *co, char *options) Johan
Re: [PATCH] USB: Add LPM quirk for Lenovo ThinkPad USB-C Dock Gen2 Ethernet
On Mon, Apr 12, 2021 at 08:37:52PM +0800, Kai-Heng Feng wrote:
> This is another branded 8153 device that doesn't work well with LPM
> enabled:
> [ 400.597506] r8152 5-1.1:1.0 enx482ae3a2a6f0: Tx status -71
>
> So disable LPM to resolve the issue.
> BugLink: https://bugs.launchpad.net/bugs/1922651
>
> Signed-off-by: Kai-Heng Feng
> ---
> drivers/usb/core/quirks.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> index 76ac5d6555ae..dfedb51cf832 100644
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -434,6 +434,9 @@ static const struct usb_device_id usb_quirk_list[] = {
> { USB_DEVICE(0x1532, 0x0116), .driver_info =
> USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL },
>
> + /* Lenovo ThinkPad USB-C Dock Gen2 Ethernet (RTL8153 GigE) */
> + { USB_DEVICE(0x17ef, 0xa387), .driver_info = USB_QUIRK_NO_LPM },
> +
> /* Lenovo ThinkCenter A630Z TI024Gen3 usb-audio */
> { USB_DEVICE(0x17ef, 0xa012), .driver_info =
> USB_QUIRK_DISCONNECT_SUSPEND },
Please keep the entries sorted by VID/PID.
Johan
Re: [PATCH 0/2] USB: serial: cp210x: provide gpio valid mask
On Fri, Apr 09, 2021 at 07:23:11PM +0300, Andy Shevchenko wrote: > On Fri, Apr 9, 2021 at 6:52 PM Johan Hovold wrote: > > > > Use the new GPIO valid-mask feature to inform gpiolib which pins are > > available for use instead of handling that in a request callback. > > > > This also allows user space to figure out which pins are available > > through the chardev interface without having to request each pin in > > turn. > > Thanks! I like the series. > Independently on reaction on my comments: > Reviewed-by: Andy Shevchenko Thanks for reviewing. Now applied. Johan
[PATCH 11/12] USB: serial: xr: add copyright notice
Add another copyright notice for the work done in 2021. Signed-off-by: Johan Hovold --- drivers/usb/serial/xr_serial.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c index 1b7b3c70a9b3..6853cd56d8dc 100644 --- a/drivers/usb/serial/xr_serial.c +++ b/drivers/usb/serial/xr_serial.c @@ -3,6 +3,7 @@ * MaxLinear/Exar USB to Serial driver * * Copyright (c) 2020 Manivannan Sadhasivam + * Copyright (c) 2021 Johan Hovold * * Based on the initial driver written by Patong Yang: * -- 2.26.3
[PATCH 12/12] USB: cdc-acm: add more Maxlinear/Exar models to ignore list
From: Mauro Carvalho Chehab
Now that the xr_serial got support for other models, add their USB IDs
as well.
The Maxlinear/Exar USB UARTs can be used in either ACM mode using the
cdc-acm driver or in "custom driver" mode in which further features such
as hardware and software flow control, GPIO control and in-band
line-status reporting are available.
In ACM mode the device always enables RTS/CTS flow control, something
which could prevent transmission in case the CTS input isn't wired up
correctly.
Ensure that cdc_acm will not bind to these devices if the custom
USB-serial driver is enabled.
Signed-off-by: Mauro Carvalho Chehab
Link:
https://lore.kernel.org/r/5155887a764cbc11f8da0217fe08a24a77d120b4.1616571453.git.mchehab+hua...@kernel.org
[ johan: rewrite commit message, clean up entries ]
Cc: Oliver Neukum
Signed-off-by: Johan Hovold
---
drivers/usb/class/cdc-acm.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 337ffced9c40..434539b13a70 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1903,9 +1903,17 @@ static const struct usb_device_id acm_ids[] = {
#endif
#if IS_ENABLED(CONFIG_USB_SERIAL_XR)
- { USB_DEVICE(0x04e2, 0x1410), /* Ignore XR21V141X USB to Serial
converter */
- .driver_info = IGNORE_DEVICE,
- },
+ { USB_DEVICE(0x04e2, 0x1400), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1401), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1402), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1403), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1410), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1411), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1412), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1414), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1420), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1422), .driver_info = IGNORE_DEVICE },
+ { USB_DEVICE(0x04e2, 0x1424), .driver_info = IGNORE_DEVICE },
#endif
/*Samsung phone in firmware update mode */
--
2.26.3
[PATCH 10/12] USB: serial: xr: reset FIFOs on open
Reset the transmit and receive FIFOs before enabling the UARTs as part
of open() in order to flush any stale data.
Note that the XR21V141X needs a type-specific implementation due to its
UART Manager registers.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/xr_serial.c | 51 ++
1 file changed, 51 insertions(+)
diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 14dbda13ab4d..1b7b3c70a9b3 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -95,10 +95,13 @@ struct xr_txrx_clk_mask {
#define XR_GPIO_MODE_TX_TOGGLE 0x100
#define XR_GPIO_MODE_RX_TOGGLE 0x200
+#define XR_FIFO_RESET 0x1
+
#define XR_CUSTOM_DRIVER_ACTIVE0x1
static int xr21v141x_uart_enable(struct usb_serial_port *port);
static int xr21v141x_uart_disable(struct usb_serial_port *port);
+static int xr21v141x_fifo_reset(struct usb_serial_port *port);
static void xr21v141x_set_line_settings(struct tty_struct *tty,
struct usb_serial_port *port, struct ktermios *old_termios);
@@ -118,6 +121,8 @@ struct xr_type {
u16 gpio_set;
u16 gpio_clear;
u16 gpio_status;
+ u16 tx_fifo_reset;
+ u16 rx_fifo_reset;
u16 custom_driver;
bool have_5_6_bit_mode;
@@ -125,6 +130,7 @@ struct xr_type {
int (*enable)(struct usb_serial_port *port);
int (*disable)(struct usb_serial_port *port);
+ int (*fifo_reset)(struct usb_serial_port *port);
void (*set_line_settings)(struct tty_struct *tty,
struct usb_serial_port *port,
struct ktermios *old_termios);
@@ -158,6 +164,7 @@ static const struct xr_type xr_types[] = {
.enable = xr21v141x_uart_enable,
.disable= xr21v141x_uart_disable,
+ .fifo_reset = xr21v141x_fifo_reset,
.set_line_settings = xr21v141x_set_line_settings,
},
[XR21B142X] = {
@@ -176,6 +183,8 @@ static const struct xr_type xr_types[] = {
.gpio_set = 0x0e,
.gpio_clear = 0x0f,
.gpio_status= 0x10,
+ .tx_fifo_reset = 0x40,
+ .rx_fifo_reset = 0x43,
.custom_driver = 0x60,
.have_5_6_bit_mode = true,
@@ -197,6 +206,8 @@ static const struct xr_type xr_types[] = {
.gpio_set = 0xc0e,
.gpio_clear = 0xc0f,
.gpio_status= 0xc10,
+ .tx_fifo_reset = 0xc80,
+ .rx_fifo_reset = 0xcc0,
.custom_driver = 0x20d,
},
[XR2280X] = {
@@ -215,6 +226,8 @@ static const struct xr_type xr_types[] = {
.gpio_set = 0x4e,
.gpio_clear = 0x4f,
.gpio_status= 0x50,
+ .tx_fifo_reset = 0x60,
+ .rx_fifo_reset = 0x63,
.custom_driver = 0x81,
},
};
@@ -384,6 +397,40 @@ static int xr_uart_disable(struct usb_serial_port *port)
return __xr_uart_disable(port);
}
+static int xr21v141x_fifo_reset(struct usb_serial_port *port)
+{
+ int ret;
+
+ ret = xr_set_reg_um(port, XR21V141X_UM_TX_FIFO_RESET, XR_FIFO_RESET);
+ if (ret)
+ return ret;
+
+ ret = xr_set_reg_um(port, XR21V141X_UM_RX_FIFO_RESET, XR_FIFO_RESET);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int xr_fifo_reset(struct usb_serial_port *port)
+{
+ struct xr_data *data = usb_get_serial_port_data(port);
+ int ret;
+
+ if (data->type->fifo_reset)
+ return data->type->fifo_reset(port);
+
+ ret = xr_set_reg_uart(port, data->type->tx_fifo_reset, XR_FIFO_RESET);
+ if (ret)
+ return ret;
+
+ ret = xr_set_reg_uart(port, data->type->rx_fifo_reset, XR_FIFO_RESET);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
static int xr_tiocmget(struct tty_struct *tty)
{
struct usb_serial_port *port = tty->driver_data;
@@ -781,6 +828,10 @@ static int xr_open(struct tty_struct *tty, struct
usb_serial_port *port)
{
int ret;
+ ret = xr_fifo_reset(port);
+ if (ret)
+ return ret;
+
ret = xr_uart_enable(port);
if (ret) {
dev_err(>dev, "Failed to enable UART\n");
--
2.26.3
[PATCH 09/12] USB: serial: xr: add support for XR22801, XR22802, XR22804
The XR22801, XR22802 and XR22804 are compound devices with an embedded
hub and up to seven downstream USB devices including one, two or four
UARTs respectively.
The UART function is similar to XR21B142X but most registers are offset
by 0x40, the register requests are different and are directed at the
device rather than interface, and 5 and 6-bit words are not supported.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/xr_serial.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 46e5e1b2f3c0..14dbda13ab4d 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -134,6 +134,7 @@ enum xr_type_id {
XR21V141X,
XR21B142X,
XR21B1411,
+ XR2280X,
XR_TYPE_COUNT,
};
@@ -198,6 +199,24 @@ static const struct xr_type xr_types[] = {
.gpio_status= 0xc10,
.custom_driver = 0x20d,
},
+ [XR2280X] = {
+ .reg_width = 16,
+ .reg_recipient = USB_RECIP_DEVICE,
+ .set_reg= 0x05,
+ .get_reg= 0x05,
+
+ .uart_enable= 0x40,
+ .flow_control = 0x46,
+ .xon_char = 0x47,
+ .xoff_char = 0x48,
+ .tx_break = 0x4a,
+ .gpio_mode = 0x4c,
+ .gpio_direction = 0x4d,
+ .gpio_set = 0x4e,
+ .gpio_clear = 0x4f,
+ .gpio_status= 0x50,
+ .custom_driver = 0x81,
+ },
};
struct xr_data {
@@ -906,6 +925,10 @@ static void xr_port_remove(struct usb_serial_port *port)
.driver_info = (type)
static const struct usb_device_id id_table[] = {
+ { XR_DEVICE(0x04e2, 0x1400, XR2280X) },
+ { XR_DEVICE(0x04e2, 0x1401, XR2280X) },
+ { XR_DEVICE(0x04e2, 0x1402, XR2280X) },
+ { XR_DEVICE(0x04e2, 0x1403, XR2280X) },
{ XR_DEVICE(0x04e2, 0x1410, XR21V141X) },
{ XR_DEVICE(0x04e2, 0x1411, XR21B1411) },
{ XR_DEVICE(0x04e2, 0x1412, XR21V141X) },
--
2.26.3
[PATCH 01/12] USB: serial: xr: add support for XR21V1412 and XR21V1414
Add support for the two- and four-port variants of XR21V1410.
Use the interface number of each control interface (e.g. 0, 2, 4, 6) to
derive the zero-based channel index:
XR21V1410 0
XR21V1412 0, 1
XR21V1414 0, 1, 2, 3
Note that the UART registers reside in separate blocks per channel,
while the UART Manager functionality is implemented using per-channel
registers.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/xr_serial.c | 55 +++---
1 file changed, 51 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 88c73f88cb26..64bc9d7b948b 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -109,6 +109,10 @@ struct xr_txrx_clk_mask {
#define XR21V141X_REG_GPIO_CLR 0x1e
#define XR21V141X_REG_GPIO_STATUS 0x1f
+struct xr_data {
+ u8 channel; /* zero-based index */
+};
+
static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
{
struct usb_serial *serial = port->serial;
@@ -160,16 +164,31 @@ static int xr_get_reg(struct usb_serial_port *port, u8
block, u8 reg, u8 *val)
static int xr_set_reg_uart(struct usb_serial_port *port, u8 reg, u8 val)
{
- return xr_set_reg(port, XR21V141X_UART_REG_BLOCK, reg, val);
+ struct xr_data *data = usb_get_serial_port_data(port);
+ u8 block;
+
+ block = XR21V141X_UART_REG_BLOCK + data->channel;
+
+ return xr_set_reg(port, block, reg, val);
}
static int xr_get_reg_uart(struct usb_serial_port *port, u8 reg, u8 *val)
{
- return xr_get_reg(port, XR21V141X_UART_REG_BLOCK, reg, val);
+ struct xr_data *data = usb_get_serial_port_data(port);
+ u8 block;
+
+ block = XR21V141X_UART_REG_BLOCK + data->channel;
+
+ return xr_get_reg(port, block, reg, val);
}
-static int xr_set_reg_um(struct usb_serial_port *port, u8 reg, u8 val)
+static int xr_set_reg_um(struct usb_serial_port *port, u8 reg_base, u8 val)
{
+ struct xr_data *data = usb_get_serial_port_data(port);
+ u8 reg;
+
+ reg = reg_base + data->channel;
+
return xr_set_reg(port, XR21V141X_UM_REG_BLOCK, reg, val);
}
@@ -577,8 +596,34 @@ static int xr_probe(struct usb_serial *serial, const
struct usb_device_id *id)
return 0;
}
+static int xr_port_probe(struct usb_serial_port *port)
+{
+ struct usb_interface_descriptor *desc;
+ struct xr_data *data;
+
+ data = kzalloc(sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ desc = >serial->interface->cur_altsetting->desc;
+ data->channel = desc->bInterfaceNumber / 2;
+
+ usb_set_serial_port_data(port, data);
+
+ return 0;
+}
+
+static void xr_port_remove(struct usb_serial_port *port)
+{
+ struct xr_data *data = usb_get_serial_port_data(port);
+
+ kfree(data);
+}
+
static const struct usb_device_id id_table[] = {
- { USB_DEVICE_INTERFACE_CLASS(0x04e2, 0x1410, USB_CLASS_COMM) }, /*
XR21V141X */
+ { USB_DEVICE_INTERFACE_CLASS(0x04e2, 0x1410, USB_CLASS_COMM) }, /*
XR21V1410 */
+ { USB_DEVICE_INTERFACE_CLASS(0x04e2, 0x1412, USB_CLASS_COMM) }, /*
XR21V1412 */
+ { USB_DEVICE_INTERFACE_CLASS(0x04e2, 0x1414, USB_CLASS_COMM) }, /*
XR21V1414 */
{ }
};
MODULE_DEVICE_TABLE(usb, id_table);
@@ -591,6 +636,8 @@ static struct usb_serial_driver xr_device = {
.id_table = id_table,
.num_ports = 1,
.probe = xr_probe,
+ .port_probe = xr_port_probe,
+ .port_remove= xr_port_remove,
.open = xr_open,
.close = xr_close,
.break_ctl = xr_break_ctl,
--
2.26.3
[PATCH 06/12] USB: serial: xr: add type abstraction
There are at least four types of Maxlinear/Exar USB UARTs which differ
in various ways such as in their register layouts:
XR21V141X
XR21B142X
XR21B1411
XR22804
It is not clear whether the device type can be inferred from the
descriptors so encode it in the device-id table for now.
Add a type structure that can be used to abstract the register layout
and other features, and use it when accessing the XR21V141X UART
registers that are shared by all types.
Note that the currently supported XR21V141X type is the only type that
has a set of UART Manager registers and that these will need to be
handled specifically.
Similarly, XR21V141X is the only type which has the divisor registers
and that needs to use the format register when configuring the line
settings.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/xr_serial.c | 128 ++---
1 file changed, 85 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index bbfe92fcabc0..003aa1e04c85 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -29,10 +29,16 @@ struct xr_txrx_clk_mask {
#define XR21V141X_MIN_SPEED46U
#define XR21V141X_MAX_SPEEDXR_INT_OSC_HZ
-/* USB Requests */
+/* USB requests */
#define XR21V141X_SET_REQ 0
#define XR21V141X_GET_REQ 1
+/* XR21V141X register blocks */
+#define XR21V141X_UART_REG_BLOCK 0
+#define XR21V141X_UM_REG_BLOCK 4
+#define XR21V141X_UART_CUSTOM_BLOCK0x66
+
+/* XR21V141X UART registers */
#define XR21V141X_CLOCK_DIVISOR_0 0x04
#define XR21V141X_CLOCK_DIVISOR_1 0x05
#define XR21V141X_CLOCK_DIVISOR_2 0x06
@@ -40,13 +46,9 @@ struct xr_txrx_clk_mask {
#define XR21V141X_TX_CLOCK_MASK_1 0x08
#define XR21V141X_RX_CLOCK_MASK_0 0x09
#define XR21V141X_RX_CLOCK_MASK_1 0x0a
+#define XR21V141X_REG_FORMAT 0x0b
-/* XR21V141X register blocks */
-#define XR21V141X_UART_REG_BLOCK 0
-#define XR21V141X_UM_REG_BLOCK 4
-#define XR21V141X_UART_CUSTOM_BLOCK0x66
-
-/* XR21V141X UART Manager Registers */
+/* XR21V141X UART Manager registers */
#define XR21V141X_UM_FIFO_ENABLE_REG 0x10
#define XR21V141X_UM_ENABLE_TX_FIFO0x01
#define XR21V141X_UM_ENABLE_RX_FIFO0x02
@@ -94,23 +96,42 @@ struct xr_txrx_clk_mask {
#define XR_GPIO_MODE_RS485 0x3
#define XR_GPIO_MODE_RS485_ADDR0x4
-#define XR21V141X_REG_ENABLE 0x03
-#define XR21V141X_REG_FORMAT 0x0b
-#define XR21V141X_REG_FLOW_CTRL0x0c
-#define XR21V141X_REG_XON_CHAR 0x10
-#define XR21V141X_REG_XOFF_CHAR0x11
-#define XR21V141X_REG_LOOPBACK 0x12
-#define XR21V141X_REG_TX_BREAK 0x14
-#define XR21V141X_REG_RS845_DELAY 0x15
-#define XR21V141X_REG_GPIO_MODE0x1a
-#define XR21V141X_REG_GPIO_DIR 0x1b
-#define XR21V141X_REG_GPIO_INT_MASK0x1c
-#define XR21V141X_REG_GPIO_SET 0x1d
-#define XR21V141X_REG_GPIO_CLR 0x1e
-#define XR21V141X_REG_GPIO_STATUS 0x1f
+struct xr_type {
+ u8 uart_enable;
+ u8 flow_control;
+ u8 xon_char;
+ u8 xoff_char;
+ u8 tx_break;
+ u8 gpio_mode;
+ u8 gpio_direction;
+ u8 gpio_set;
+ u8 gpio_clear;
+ u8 gpio_status;
+};
+
+enum xr_type_id {
+ XR21V141X,
+ XR_TYPE_COUNT,
+};
+
+static const struct xr_type xr_types[] = {
+ [XR21V141X] = {
+ .uart_enable= 0x03,
+ .flow_control = 0x0c,
+ .xon_char = 0x10,
+ .xoff_char = 0x11,
+ .tx_break = 0x14,
+ .gpio_mode = 0x1a,
+ .gpio_direction = 0x1b,
+ .gpio_set = 0x1d,
+ .gpio_clear = 0x1e,
+ .gpio_status= 0x1f,
+ },
+};
struct xr_data {
- u8 channel; /* zero-based index */
+ const struct xr_type *type;
+ u8 channel; /* zero-based index */
};
static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
@@ -202,6 +223,7 @@ static int xr_set_reg_um(struct usb_serial_port *port, u8
reg_base, u8 val)
*/
static int xr_uart_enable(struct usb_serial_port *port)
{
+ struct xr_data *data = usb_get_serial_port_data(port);
int ret;
ret = xr_set_reg_um(port, XR21V141X_UM_FIFO_ENABLE_REG,
@@ -209,25 +231,25 @@ static int xr_uart_enable(struct usb_serial_port *port)
if (ret)
return ret;
- ret = xr_set_reg_uart(port, XR21V141X_REG_ENABLE,
+ ret = xr_set_reg_uart(port, data->type->uart_enable,
XR_UART_ENABLE_TX | XR_UART_ENABLE_RX);
if (ret)
return ret;
ret = xr_set_reg_um(port, XR21V141X_UM_FIFO_ENAB
[PATCH 02/12] USB: serial: xr: rename GPIO-mode defines
Rename the GPIO mode defines so that they reflect the datasheet and how
they are used.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/xr_serial.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 64bc9d7b948b..a600448c6016 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -88,11 +88,11 @@ struct xr_txrx_clk_mask {
#define XR21V141X_UART_FLOW_MODE_HW0x1
#define XR21V141X_UART_FLOW_MODE_SW0x2
-#define XR21V141X_UART_MODE_GPIO_MASK GENMASK(2, 0)
-#define XR21V141X_UART_MODE_RTS_CTS0x1
-#define XR21V141X_UART_MODE_DTR_DSR0x2
-#define XR21V141X_UART_MODE_RS485 0x3
-#define XR21V141X_UART_MODE_RS485_ADDR 0x4
+#define XR21V141X_GPIO_MODE_MASK GENMASK(2, 0)
+#define XR21V141X_GPIO_MODE_RTS_CTS0x1
+#define XR21V141X_GPIO_MODE_DTR_DSR0x2
+#define XR21V141X_GPIO_MODE_RS485 0x3
+#define XR21V141X_GPIO_MODE_RS485_ADDR 0x4
#define XR21V141X_REG_ENABLE 0x03
#define XR21V141X_REG_FORMAT 0x0b
@@ -433,11 +433,11 @@ static void xr_set_flow_mode(struct tty_struct *tty,
return;
/* Set GPIO mode for controlling the pins manually by default. */
- gpio_mode &= ~XR21V141X_UART_MODE_GPIO_MASK;
+ gpio_mode &= ~XR21V141X_GPIO_MODE_MASK;
if (C_CRTSCTS(tty) && C_BAUD(tty) != B0) {
dev_dbg(>dev, "Enabling hardware flow ctrl\n");
- gpio_mode |= XR21V141X_UART_MODE_RTS_CTS;
+ gpio_mode |= XR21V141X_GPIO_MODE_RTS_CTS;
flow = XR21V141X_UART_FLOW_MODE_HW;
} else if (I_IXON(tty)) {
u8 start_char = START_CHAR(tty);
--
2.26.3
[PATCH 07/12] USB: serial: xr: add support for XR21B1421, XR21B1422 and XR21B1424
The XR21B1421, XR21B1422 and XR21B1424 are the one-, two- and four-port
models of a second XR21B142X type of the Maxlinear/Exar USB UARTs.
The XR21B142X type differs from XR21V141X in several ways, including:
- register layout
- register width (16-bit instead of 8-bit)
- vendor register requests
- UART enable/disable sequence
- custom-driver mode flag
- three additional GPIOs (9 instead of 6)
As for XR21V141X, the XR21B142X vendor requests encode the channel index
in the MSB of wIndex, but it lacks the UART Manager registers which
have been replaced by regular UART registers. The new type also uses the
interface number of the control interface (0, 2, 4, 6) as channel index
instead of the channel number (0, 1, 2, 3).
The XR21B142X lacks the divisor and format registers used by XR21V141X
and instead uses the CDC SET_LINE_CONTROL request to configure the line
settings.
Note that the currently supported XR21V141X type lacks the custom-driver
mode flag that prevents the device from entering CDC-ACM mode when a CDC
requests is received. This specifically means that the SET_LINE_CONTROL
request cannot be used with XR21V141X even though it is otherwise
supported.
The UART enable sequence for XR21B142X does not involve explicitly
enabling the FIFOs, but according to datasheet the UART must be disabled
when writing any register but GPIO_SET, GPIO_CLEAR, TX_BREAK and
ERROR_STATUS.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/xr_serial.c | 338 +
1 file changed, 262 insertions(+), 76 deletions(-)
diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 003aa1e04c85..32055c763147 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -29,10 +29,6 @@ struct xr_txrx_clk_mask {
#define XR21V141X_MIN_SPEED46U
#define XR21V141X_MAX_SPEEDXR_INT_OSC_HZ
-/* USB requests */
-#define XR21V141X_SET_REQ 0
-#define XR21V141X_GET_REQ 1
-
/* XR21V141X register blocks */
#define XR21V141X_UART_REG_BLOCK 0
#define XR21V141X_UM_REG_BLOCK 4
@@ -65,9 +61,10 @@ struct xr_txrx_clk_mask {
#define XR_GPIO_DTRBIT(3)
#define XR_GPIO_CTSBIT(4)
#define XR_GPIO_RTSBIT(5)
-
-#define XR21V141X_UART_BREAK_ON0xff
-#define XR21V141X_UART_BREAK_OFF 0
+#define XR_GPIO_CLKBIT(6)
+#define XR_GPIO_XENBIT(7)
+#define XR_GPIO_TXTBIT(8)
+#define XR_GPIO_RXTBIT(9)
#define XR_UART_DATA_MASK GENMASK(3, 0)
#define XR_UART_DATA_7 0x7
@@ -90,13 +87,27 @@ struct xr_txrx_clk_mask {
#define XR_UART_FLOW_MODE_HW 0x1
#define XR_UART_FLOW_MODE_SW 0x2
-#define XR_GPIO_MODE_MASK GENMASK(2, 0)
-#define XR_GPIO_MODE_RTS_CTS 0x1
-#define XR_GPIO_MODE_DTR_DSR 0x2
-#define XR_GPIO_MODE_RS485 0x3
-#define XR_GPIO_MODE_RS485_ADDR0x4
+#define XR_GPIO_MODE_SEL_MASK GENMASK(2, 0)
+#define XR_GPIO_MODE_SEL_RTS_CTS 0x1
+#define XR_GPIO_MODE_SEL_DTR_DSR 0x2
+#define XR_GPIO_MODE_SEL_RS485 0x3
+#define XR_GPIO_MODE_SEL_RS485_ADDR0x4
+#define XR_GPIO_MODE_TX_TOGGLE 0x100
+#define XR_GPIO_MODE_RX_TOGGLE 0x200
+
+#define XR_CUSTOM_DRIVER_ACTIVE0x1
+
+static int xr21v141x_uart_enable(struct usb_serial_port *port);
+static int xr21v141x_uart_disable(struct usb_serial_port *port);
+static void xr21v141x_set_line_settings(struct tty_struct *tty,
+ struct usb_serial_port *port, struct ktermios *old_termios);
struct xr_type {
+ int reg_width;
+ u8 reg_recipient;
+ u8 set_reg;
+ u8 get_reg;
+
u8 uart_enable;
u8 flow_control;
u8 xon_char;
@@ -107,15 +118,30 @@ struct xr_type {
u8 gpio_set;
u8 gpio_clear;
u8 gpio_status;
+ u8 custom_driver;
+
+ bool have_xmit_toggle;
+
+ int (*enable)(struct usb_serial_port *port);
+ int (*disable)(struct usb_serial_port *port);
+ void (*set_line_settings)(struct tty_struct *tty,
+ struct usb_serial_port *port,
+ struct ktermios *old_termios);
};
enum xr_type_id {
XR21V141X,
+ XR21B142X,
XR_TYPE_COUNT,
};
static const struct xr_type xr_types[] = {
[XR21V141X] = {
+ .reg_width = 8,
+ .reg_recipient = USB_RECIP_DEVICE,
+ .set_reg= 0x00,
+ .get_reg= 0x01,
+
.uart_enable= 0x03,
.flow_control = 0x0c,
.xon_char = 0x10,
@@ -126,25 +152,50 @@ static const struct xr_type xr_types[] = {
.gpio_set = 0x1d,
.gpio_clear = 0x1e
[PATCH 08/12] USB: serial: xr: add support for XR21B1411
The single-port XR21B1411 is similar to the XR21B142X type but uses
12-bit registers and 16-bit register addresses, the register requests
are different and are directed at the device rather than interface, and
5 and 6-bit words are not supported.
The register layout is very similar to XR21B142X except that most
registers are offset by 0xc00 (corresponding to a channel index of 12 in
the MSB of wIndex). As the device is single-port so that the derived
channel index is 0, the current register accessors can be reused after
simply changing the address width.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/xr_serial.c | 64 +-
1 file changed, 48 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 32055c763147..46e5e1b2f3c0 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -108,18 +108,19 @@ struct xr_type {
u8 set_reg;
u8 get_reg;
- u8 uart_enable;
- u8 flow_control;
- u8 xon_char;
- u8 xoff_char;
- u8 tx_break;
- u8 gpio_mode;
- u8 gpio_direction;
- u8 gpio_set;
- u8 gpio_clear;
- u8 gpio_status;
- u8 custom_driver;
-
+ u16 uart_enable;
+ u16 flow_control;
+ u16 xon_char;
+ u16 xoff_char;
+ u16 tx_break;
+ u16 gpio_mode;
+ u16 gpio_direction;
+ u16 gpio_set;
+ u16 gpio_clear;
+ u16 gpio_status;
+ u16 custom_driver;
+
+ bool have_5_6_bit_mode;
bool have_xmit_toggle;
int (*enable)(struct usb_serial_port *port);
@@ -132,6 +133,7 @@ struct xr_type {
enum xr_type_id {
XR21V141X,
XR21B142X,
+ XR21B1411,
XR_TYPE_COUNT,
};
@@ -175,8 +177,27 @@ static const struct xr_type xr_types[] = {
.gpio_status= 0x10,
.custom_driver = 0x60,
+ .have_5_6_bit_mode = true,
.have_xmit_toggle = true,
},
+ [XR21B1411] = {
+ .reg_width = 12,
+ .reg_recipient = USB_RECIP_DEVICE,
+ .set_reg= 0x00,
+ .get_reg= 0x01,
+
+ .uart_enable= 0xc00,
+ .flow_control = 0xc06,
+ .xon_char = 0xc07,
+ .xoff_char = 0xc08,
+ .tx_break = 0xc0a,
+ .gpio_mode = 0xc0c,
+ .gpio_direction = 0xc0d,
+ .gpio_set = 0xc0e,
+ .gpio_clear = 0xc0f,
+ .gpio_status= 0xc10,
+ .custom_driver = 0x20d,
+ },
};
struct xr_data {
@@ -184,7 +205,7 @@ struct xr_data {
u8 channel; /* zero-based index or interface number
*/
};
-static int xr_set_reg(struct usb_serial_port *port, u8 channel, u8 reg, u16
val)
+static int xr_set_reg(struct usb_serial_port *port, u8 channel, u16 reg, u16
val)
{
struct xr_data *data = usb_get_serial_port_data(port);
const struct xr_type *type = data->type;
@@ -204,7 +225,7 @@ static int xr_set_reg(struct usb_serial_port *port, u8
channel, u8 reg, u16 val)
return 0;
}
-static int xr_get_reg(struct usb_serial_port *port, u8 channel, u8 reg, u16
*val)
+static int xr_get_reg(struct usb_serial_port *port, u8 channel, u16 reg, u16
*val)
{
struct xr_data *data = usb_get_serial_port_data(port);
const struct xr_type *type = data->type;
@@ -243,14 +264,14 @@ static int xr_get_reg(struct usb_serial_port *port, u8
channel, u8 reg, u16 *val
return ret;
}
-static int xr_set_reg_uart(struct usb_serial_port *port, u8 reg, u16 val)
+static int xr_set_reg_uart(struct usb_serial_port *port, u16 reg, u16 val)
{
struct xr_data *data = usb_get_serial_port_data(port);
return xr_set_reg(port, data->channel, reg, val);
}
-static int xr_get_reg_uart(struct usb_serial_port *port, u8 reg, u16 *val)
+static int xr_get_reg_uart(struct usb_serial_port *port, u16 reg, u16 *val)
{
struct xr_data *data = usb_get_serial_port_data(port);
@@ -646,6 +667,7 @@ static void xr21v141x_set_line_settings(struct tty_struct
*tty,
static void xr_cdc_set_line_coding(struct tty_struct *tty,
struct usb_serial_port *port, struct ktermios *old_termios)
{
+ struct xr_data *data = usb_get_serial_port_data(port);
struct usb_host_interface *alt =
port->serial->interface->cur_altsetting;
struct usb_device *udev = port->serial->dev;
struct usb_cdc_line_coding *lc;
@@ -683,6 +705,15 @@ static void xr_cdc_set_line_coding(struct tty_struct *tty,
lc->bParityType = USB_CDC_NO_PARITY;
}
+ if (!data->type->have_5_6_bit_mode &&
+ (C_CSIZE(tty) == CS5 || C_CSIZE(tty) == CS6)) {
+ tty->termios.c_cflag &= ~CSIZE;
+
[PATCH 04/12] USB: serial: xr: move pin configuration to probe
There's no need to configure the pins on every open and judging from the
vendor driver and datasheet it can be done before enabling the UART.
Move pin configuration from open() to port probe and make sure to
deassert DTR and RTS after configuring all pins as GPIO.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/xr_serial.c | 45 --
1 file changed, 37 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index f5087a8b6c86..542c1dc060cc 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -532,7 +532,6 @@ static void xr_set_termios(struct tty_struct *tty,
static int xr_open(struct tty_struct *tty, struct usb_serial_port *port)
{
- u8 gpio_dir;
int ret;
ret = xr_uart_enable(port);
@@ -541,13 +540,6 @@ static int xr_open(struct tty_struct *tty, struct
usb_serial_port *port)
return ret;
}
- /*
-* Configure DTR and RTS as outputs and RI, CD, DSR and CTS as
-* inputs.
-*/
- gpio_dir = XR21V141X_GPIO_DTR | XR21V141X_GPIO_RTS;
- xr_set_reg_uart(port, XR21V141X_REG_GPIO_DIR, gpio_dir);
-
/* Setup termios */
if (tty)
xr_set_termios(tty, port, NULL);
@@ -596,10 +588,38 @@ static int xr_probe(struct usb_serial *serial, const
struct usb_device_id *id)
return 0;
}
+static int xr_gpio_init(struct usb_serial_port *port)
+{
+ u8 mask, mode;
+ int ret;
+
+ /* Configure all pins as GPIO. */
+ mode = 0;
+ ret = xr_set_reg_uart(port, XR21V141X_REG_GPIO_MODE, mode);
+ if (ret)
+ return ret;
+
+ /*
+* Configure DTR and RTS as outputs and make sure they are deasserted
+* (active low), and configure RI, CD, DSR and CTS as inputs.
+*/
+ mask = XR21V141X_GPIO_DTR | XR21V141X_GPIO_RTS;
+ ret = xr_set_reg_uart(port, XR21V141X_REG_GPIO_DIR, mask);
+ if (ret)
+ return ret;
+
+ ret = xr_set_reg_uart(port, XR21V141X_REG_GPIO_SET, mask);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
static int xr_port_probe(struct usb_serial_port *port)
{
struct usb_interface_descriptor *desc;
struct xr_data *data;
+ int ret;
data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data)
@@ -610,7 +630,16 @@ static int xr_port_probe(struct usb_serial_port *port)
usb_set_serial_port_data(port, data);
+ ret = xr_gpio_init(port);
+ if (ret)
+ goto err_free;
+
return 0;
+
+err_free:
+ kfree(data);
+
+ return ret;
}
static void xr_port_remove(struct usb_serial_port *port)
--
2.26.3
[PATCH RESEND 00/12] USB: serial: xr: add support for more device types
[ I apparently missed the last three patches of the series when posting on March 30th so resending the whole series. ] This series adds support for another nine models of Maxlinerar/Exar USB UARTs to the xr driver. The various models can be divided into four types: XR21V141X XR21B142X XR21B1411 XR22804 with different register layouts and features. All types can be used in CDC-ACM mode but further features such as hardware and software flow control and in-band line status are available in a second "custom driver" mode. The fact that hardware flow control is enabled by default in CDC-ACM mode also prevents using the standard CDC-ACM driver in cases where the hardware engineers have failed to properly connect the CTS input. The currently supported XR21V141X type stands out from the other three by not being able to accept CDC requests without always entering CDC-ACM mode, requiring a different enable/disable sequence and by using a distinct register layout for certain functionality. Expect for the above, most differences can be handled by simply using different set of register addresses. Note that this series depends on the recently posted multi-interface-function series. Johan Johan Hovold (11): USB: serial: xr: add support for XR21V1412 and XR21V1414 USB: serial: xr: rename GPIO-mode defines USB: serial: xr: rename GPIO-pin defines USB: serial: xr: move pin configuration to probe USB: serial: xr: drop type prefix from shared defines USB: serial: xr: add type abstraction USB: serial: xr: add support for XR21B1421, XR21B1422 and XR21B1424 USB: serial: xr: add support for XR21B1411 USB: serial: xr: add support for XR22801, XR22802, XR22804 USB: serial: xr: reset FIFOs on open USB: serial: xr: add copyright notice Mauro Carvalho Chehab (1): USB: cdc-acm: add more Maxlinear/Exar models to ignore list drivers/usb/class/cdc-acm.c| 14 +- drivers/usb/serial/xr_serial.c | 727 ++--- 2 files changed, 580 insertions(+), 161 deletions(-) -- 2.26.3
[PATCH 05/12] USB: serial: xr: drop type prefix from shared defines
In preparation for adding support for further types, drop the type
prefix from defines that are not specific to XR21V141X.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/xr_serial.c | 122 -
1 file changed, 61 insertions(+), 61 deletions(-)
diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 542c1dc060cc..bbfe92fcabc0 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -54,45 +54,45 @@ struct xr_txrx_clk_mask {
#define XR21V141X_UM_RX_FIFO_RESET 0x18
#define XR21V141X_UM_TX_FIFO_RESET 0x1c
-#define XR21V141X_UART_ENABLE_TX 0x1
-#define XR21V141X_UART_ENABLE_RX 0x2
+#define XR_UART_ENABLE_TX 0x1
+#define XR_UART_ENABLE_RX 0x2
-#define XR21V141X_GPIO_RI BIT(0)
-#define XR21V141X_GPIO_CD BIT(1)
-#define XR21V141X_GPIO_DSR BIT(2)
-#define XR21V141X_GPIO_DTR BIT(3)
-#define XR21V141X_GPIO_CTS BIT(4)
-#define XR21V141X_GPIO_RTS BIT(5)
+#define XR_GPIO_RI BIT(0)
+#define XR_GPIO_CD BIT(1)
+#define XR_GPIO_DSRBIT(2)
+#define XR_GPIO_DTRBIT(3)
+#define XR_GPIO_CTSBIT(4)
+#define XR_GPIO_RTSBIT(5)
#define XR21V141X_UART_BREAK_ON0xff
#define XR21V141X_UART_BREAK_OFF 0
-#define XR21V141X_UART_DATA_MASK GENMASK(3, 0)
-#define XR21V141X_UART_DATA_7 0x7
-#define XR21V141X_UART_DATA_8 0x8
-
-#define XR21V141X_UART_PARITY_MASK GENMASK(6, 4)
-#define XR21V141X_UART_PARITY_SHIFT4
-#define XR21V141X_UART_PARITY_NONE (0x0 << XR21V141X_UART_PARITY_SHIFT)
-#define XR21V141X_UART_PARITY_ODD (0x1 << XR21V141X_UART_PARITY_SHIFT)
-#define XR21V141X_UART_PARITY_EVEN (0x2 << XR21V141X_UART_PARITY_SHIFT)
-#define XR21V141X_UART_PARITY_MARK (0x3 << XR21V141X_UART_PARITY_SHIFT)
-#define XR21V141X_UART_PARITY_SPACE(0x4 << XR21V141X_UART_PARITY_SHIFT)
-
-#define XR21V141X_UART_STOP_MASK BIT(7)
-#define XR21V141X_UART_STOP_SHIFT 7
-#define XR21V141X_UART_STOP_1 (0x0 << XR21V141X_UART_STOP_SHIFT)
-#define XR21V141X_UART_STOP_2 (0x1 << XR21V141X_UART_STOP_SHIFT)
-
-#define XR21V141X_UART_FLOW_MODE_NONE 0x0
-#define XR21V141X_UART_FLOW_MODE_HW0x1
-#define XR21V141X_UART_FLOW_MODE_SW0x2
-
-#define XR21V141X_GPIO_MODE_MASK GENMASK(2, 0)
-#define XR21V141X_GPIO_MODE_RTS_CTS0x1
-#define XR21V141X_GPIO_MODE_DTR_DSR0x2
-#define XR21V141X_GPIO_MODE_RS485 0x3
-#define XR21V141X_GPIO_MODE_RS485_ADDR 0x4
+#define XR_UART_DATA_MASK GENMASK(3, 0)
+#define XR_UART_DATA_7 0x7
+#define XR_UART_DATA_8 0x8
+
+#define XR_UART_PARITY_MASKGENMASK(6, 4)
+#define XR_UART_PARITY_SHIFT 4
+#define XR_UART_PARITY_NONE(0x0 << XR_UART_PARITY_SHIFT)
+#define XR_UART_PARITY_ODD (0x1 << XR_UART_PARITY_SHIFT)
+#define XR_UART_PARITY_EVEN(0x2 << XR_UART_PARITY_SHIFT)
+#define XR_UART_PARITY_MARK(0x3 << XR_UART_PARITY_SHIFT)
+#define XR_UART_PARITY_SPACE (0x4 << XR_UART_PARITY_SHIFT)
+
+#define XR_UART_STOP_MASK BIT(7)
+#define XR_UART_STOP_SHIFT 7
+#define XR_UART_STOP_1 (0x0 << XR_UART_STOP_SHIFT)
+#define XR_UART_STOP_2 (0x1 << XR_UART_STOP_SHIFT)
+
+#define XR_UART_FLOW_MODE_NONE 0x0
+#define XR_UART_FLOW_MODE_HW 0x1
+#define XR_UART_FLOW_MODE_SW 0x2
+
+#define XR_GPIO_MODE_MASK GENMASK(2, 0)
+#define XR_GPIO_MODE_RTS_CTS 0x1
+#define XR_GPIO_MODE_DTR_DSR 0x2
+#define XR_GPIO_MODE_RS485 0x3
+#define XR_GPIO_MODE_RS485_ADDR0x4
#define XR21V141X_REG_ENABLE 0x03
#define XR21V141X_REG_FORMAT 0x0b
@@ -210,7 +210,7 @@ static int xr_uart_enable(struct usb_serial_port *port)
return ret;
ret = xr_set_reg_uart(port, XR21V141X_REG_ENABLE,
- XR21V141X_UART_ENABLE_TX |
XR21V141X_UART_ENABLE_RX);
+ XR_UART_ENABLE_TX | XR_UART_ENABLE_RX);
if (ret)
return ret;
@@ -250,12 +250,12 @@ static int xr_tiocmget(struct tty_struct *tty)
* Modem control pins are active low, so reading '0' means it is active
* and '1' means not active.
*/
- ret = ((status & XR21V141X_GPIO_DTR) ? 0 : TIOCM_DTR) |
- ((status & XR21V141X_GPIO_RTS) ? 0 : TIOCM_RTS) |
- ((status & XR21V141X_GPIO_CTS) ? 0 : TIOCM_CTS) |
- ((status & XR21V141X_GPIO_DSR) ? 0 : TIOCM_DSR) |
- ((status & XR21V141X_GPIO_RI) ? 0 : TIOCM_RI) |
- ((status & XR21V14
[PATCH 03/12] USB: serial: xr: rename GPIO-pin defines
Rename the GPIO-pin defines so that they reflect how they are used.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/xr_serial.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index a600448c6016..f5087a8b6c86 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -57,12 +57,12 @@ struct xr_txrx_clk_mask {
#define XR21V141X_UART_ENABLE_TX 0x1
#define XR21V141X_UART_ENABLE_RX 0x2
-#define XR21V141X_UART_MODE_RI BIT(0)
-#define XR21V141X_UART_MODE_CD BIT(1)
-#define XR21V141X_UART_MODE_DSRBIT(2)
-#define XR21V141X_UART_MODE_DTRBIT(3)
-#define XR21V141X_UART_MODE_CTSBIT(4)
-#define XR21V141X_UART_MODE_RTSBIT(5)
+#define XR21V141X_GPIO_RI BIT(0)
+#define XR21V141X_GPIO_CD BIT(1)
+#define XR21V141X_GPIO_DSR BIT(2)
+#define XR21V141X_GPIO_DTR BIT(3)
+#define XR21V141X_GPIO_CTS BIT(4)
+#define XR21V141X_GPIO_RTS BIT(5)
#define XR21V141X_UART_BREAK_ON0xff
#define XR21V141X_UART_BREAK_OFF 0
@@ -250,12 +250,12 @@ static int xr_tiocmget(struct tty_struct *tty)
* Modem control pins are active low, so reading '0' means it is active
* and '1' means not active.
*/
- ret = ((status & XR21V141X_UART_MODE_DTR) ? 0 : TIOCM_DTR) |
- ((status & XR21V141X_UART_MODE_RTS) ? 0 : TIOCM_RTS) |
- ((status & XR21V141X_UART_MODE_CTS) ? 0 : TIOCM_CTS) |
- ((status & XR21V141X_UART_MODE_DSR) ? 0 : TIOCM_DSR) |
- ((status & XR21V141X_UART_MODE_RI) ? 0 : TIOCM_RI) |
- ((status & XR21V141X_UART_MODE_CD) ? 0 : TIOCM_CD);
+ ret = ((status & XR21V141X_GPIO_DTR) ? 0 : TIOCM_DTR) |
+ ((status & XR21V141X_GPIO_RTS) ? 0 : TIOCM_RTS) |
+ ((status & XR21V141X_GPIO_CTS) ? 0 : TIOCM_CTS) |
+ ((status & XR21V141X_GPIO_DSR) ? 0 : TIOCM_DSR) |
+ ((status & XR21V141X_GPIO_RI) ? 0 : TIOCM_RI) |
+ ((status & XR21V141X_GPIO_CD) ? 0 : TIOCM_CD);
return ret;
}
@@ -269,13 +269,13 @@ static int xr_tiocmset_port(struct usb_serial_port *port,
/* Modem control pins are active low, so set & clr are swapped */
if (set & TIOCM_RTS)
- gpio_clr |= XR21V141X_UART_MODE_RTS;
+ gpio_clr |= XR21V141X_GPIO_RTS;
if (set & TIOCM_DTR)
- gpio_clr |= XR21V141X_UART_MODE_DTR;
+ gpio_clr |= XR21V141X_GPIO_DTR;
if (clear & TIOCM_RTS)
- gpio_set |= XR21V141X_UART_MODE_RTS;
+ gpio_set |= XR21V141X_GPIO_RTS;
if (clear & TIOCM_DTR)
- gpio_set |= XR21V141X_UART_MODE_DTR;
+ gpio_set |= XR21V141X_GPIO_DTR;
/* Writing '0' to gpio_{set/clr} bits has no effect, so no need to do */
if (gpio_clr)
@@ -545,7 +545,7 @@ static int xr_open(struct tty_struct *tty, struct
usb_serial_port *port)
* Configure DTR and RTS as outputs and RI, CD, DSR and CTS as
* inputs.
*/
- gpio_dir = XR21V141X_UART_MODE_DTR | XR21V141X_UART_MODE_RTS;
+ gpio_dir = XR21V141X_GPIO_DTR | XR21V141X_GPIO_RTS;
xr_set_reg_uart(port, XR21V141X_REG_GPIO_DIR, gpio_dir);
/* Setup termios */
--
2.26.3
[PATCH 3/4] USB: serial: ti_usb_3410_5052: reduce drain delay to one char
The three-character drain delay was added by commit f1175daa5312 ("USB:
ti_usb_3410_5052: kill custom closing_wait") when removing the custom
closing-wait implementation, which used a fixed 20 ms poll period and
drain delay.
This was likely a bit too conservative as a one-character timeout (e.g.
33 ms at 300 bps) should be enough to compensate for the lack of a
transmitter empty bit in the TUSB5052 line-status register.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ti_usb_3410_5052.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/serial/ti_usb_3410_5052.c
b/drivers/usb/serial/ti_usb_3410_5052.c
index 03839289d6c0..8ed64115987f 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -610,7 +610,11 @@ static int ti_port_probe(struct usb_serial_port *port)
usb_set_serial_port_data(port, tport);
- port->port.drain_delay = 3;
+ /*
+* The TUSB5052 LSR does not tell when the transmitter shift register
+* has emptied so add a one-character drain delay.
+*/
+ port->port.drain_delay = 1;
return 0;
}
--
2.26.3
[PATCH 0/4] USB: serial: closing-wait fixes and cleanups
The port drain_delay parameter is used to add a time-based delay when closing the port in order to allow the transmit FIFO to drain in cases where we don't know how to tell if the FIFO is empty. This series removes a redundant time-based delay which is no longer needed, and documents the reason for two other uses where such a delay is needed to let the transmitter shift register clear. As it turns out, this is really only needed for one of the two device types handled by the ti_usb_3410_5052 driver. Johan Johan Hovold (4): USB: serial: f81232: drop time-based drain delay USB: serial: io_ti: document reason for drain delay USB: serial: ti_usb_3410_5052: reduce drain delay to one char USB: serial: ti_usb_3410_5052: drop drain delay for 3410 drivers/usb/serial/f81232.c | 1 - drivers/usb/serial/io_ti.c| 4 drivers/usb/serial/ti_usb_3410_5052.c | 21 ++--- 3 files changed, 22 insertions(+), 4 deletions(-) -- 2.26.3
[PATCH 1/4] USB: serial: f81232: drop time-based drain delay
The f81232 driver now waits for the transmit FIFO to drain during close
so there is no need to keep the time-based drain delay, which would add
up to two seconds on every close for low line speeds.
Fixes: 98405f81036d ("USB: serial: f81232: add tx_empty function")
Signed-off-by: Johan Hovold
---
drivers/usb/serial/f81232.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index b4b847dce4bc..a7a7af8d05bf 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -948,7 +948,6 @@ static int f81232_port_probe(struct usb_serial_port *port)
usb_set_serial_port_data(port, priv);
- port->port.drain_delay = 256;
priv->port = port;
return 0;
--
2.26.3
[PATCH 4/4] USB: serial: ti_usb_3410_5052: drop drain delay for 3410
Unlike the TUSB5052, the TUSB3410 has an LSR TEMT bit to tell if both
the transmitter data and shift registers are empty.
Make sure to check also the shift register on TUSB3410 when waiting for
the transmit buffer to drain during close and drop the time-based
one-char delay which is otherwise needed (e.g. 90 ms at 110 bps).
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ti_usb_3410_5052.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/serial/ti_usb_3410_5052.c
b/drivers/usb/serial/ti_usb_3410_5052.c
index 8ed64115987f..d9bffb2de8bf 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -121,6 +121,7 @@
#define TI_LSR_ERROR 0x0F
#define TI_LSR_RX_FULL 0x10
#define TI_LSR_TX_EMPTY0x20
+#define TI_LSR_TX_EMPTY_BOTH 0x40
/* Line control */
#define TI_LCR_BREAK 0x40
@@ -614,7 +615,8 @@ static int ti_port_probe(struct usb_serial_port *port)
* The TUSB5052 LSR does not tell when the transmitter shift register
* has emptied so add a one-character drain delay.
*/
- port->port.drain_delay = 1;
+ if (!tport->tp_tdev->td_is_3410)
+ port->port.drain_delay = 1;
return 0;
}
@@ -851,11 +853,20 @@ static int ti_chars_in_buffer(struct tty_struct *tty)
static bool ti_tx_empty(struct usb_serial_port *port)
{
struct ti_port *tport = usb_get_serial_port_data(port);
+ u8 lsr, mask;
int ret;
- u8 lsr;
+
+ /*
+* TUSB5052 does not have the TEMT bit to tell if the shift register
+* is empty.
+*/
+ if (tport->tp_tdev->td_is_3410)
+ mask = TI_LSR_TX_EMPTY_BOTH;
+ else
+ mask = TI_LSR_TX_EMPTY;
ret = ti_get_lsr(tport, );
- if (!ret && !(lsr & TI_LSR_TX_EMPTY))
+ if (!ret && !(lsr & mask))
return false;
return true;
--
2.26.3
[PATCH 2/4] USB: serial: io_ti: document reason for drain delay
Document that the device line-status register doesn't tell when the transmitter shift register has emptied and that this is why the one-character drain delay is needed. Signed-off-by: Johan Hovold --- drivers/usb/serial/io_ti.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c index 75325c2b295e..17720670e06c 100644 --- a/drivers/usb/serial/io_ti.c +++ b/drivers/usb/serial/io_ti.c @@ -2590,6 +2590,10 @@ static int edge_port_probe(struct usb_serial_port *port) if (ret) goto err; + /* +* The LSR does not tell when the transmitter shift register has +* emptied so add a one-character drain delay. +*/ port->port.drain_delay = 1; return 0; -- 2.26.3
Re: [PATCH 2/2] USB: serial: cp210x: add gpio-configuration debug printk
On Fri, Apr 09, 2021 at 07:22:26PM +0300, Andy Shevchenko wrote:
> On Fri, Apr 9, 2021 at 6:52 PM Johan Hovold wrote:
> >
> > Add a debug printk to dump the GPIO configuration stored in EEPROM
> > during probe.
> >
> > Signed-off-by: Johan Hovold
> > ---
> > drivers/usb/serial/cp210x.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> > index ceb3a656a075..ee595d1bea0a 100644
> > --- a/drivers/usb/serial/cp210x.c
> > +++ b/drivers/usb/serial/cp210x.c
> > @@ -1543,10 +1543,16 @@ static int cp210x_gpio_init_valid_mask(struct
> > gpio_chip *gc,
> > {
> > struct usb_serial *serial = gpiochip_get_data(gc);
> > struct cp210x_serial_private *priv = usb_get_serial_data(serial);
> > + struct device *dev = >interface->dev;
> > unsigned long altfunc_mask = priv->gpio_altfunc;
> >
> > bitmap_complement(valid_mask, _mask, ngpios);
> >
> > + if (bitmap_empty(valid_mask, ngpios))
> > + dev_dbg(dev, "no pin configured for GPIO\n");
>
> Shouldn't we drop the GPIO device completely in such a case?
I considered it when we first added support for GPIOs to this driver but
decided not to. The reason being that we want to to tell user-space that
the device has gpio capability even if the GPIOs are currently muxed (in
EEPROM) for other functionality.
> Bart, wouldn't it be a good idea for GPIO library to do something like
> this on driver's behalf?
I'd say this is mostly useful for hotpluggable devices with EEPROM
configuration and is probably best handled by the drivers.
> > + else
> > + dev_dbg(dev, "GPIO.%*pbl configured for GPIO\n", ngpios,
> > + valid_mask);
>
> A nit-pick:
> I would change GPIO -> pin in the second message in the first occurrence.
"GPIO.n" are the actual pin names from the datasheet (cf. ftdi_sio which
use "CBUSn" here). It's just a debug statement anyway.
Johan
[PATCH 0/2] USB: serial: cp210x: provide gpio valid mask
Use the new GPIO valid-mask feature to inform gpiolib which pins are available for use instead of handling that in a request callback. This also allows user space to figure out which pins are available through the chardev interface without having to request each pin in turn. Johan Johan Hovold (2): USB: serial: cp210x: provide gpio valid mask USB: serial: cp210x: add gpio-configuration debug printk drivers/usb/serial/cp210x.c | 31 +++ 1 file changed, 19 insertions(+), 12 deletions(-) -- 2.26.3
[PATCH 2/2] USB: serial: cp210x: add gpio-configuration debug printk
Add a debug printk to dump the GPIO configuration stored in EEPROM
during probe.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/cp210x.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index ceb3a656a075..ee595d1bea0a 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -1543,10 +1543,16 @@ static int cp210x_gpio_init_valid_mask(struct gpio_chip
*gc,
{
struct usb_serial *serial = gpiochip_get_data(gc);
struct cp210x_serial_private *priv = usb_get_serial_data(serial);
+ struct device *dev = >interface->dev;
unsigned long altfunc_mask = priv->gpio_altfunc;
bitmap_complement(valid_mask, _mask, ngpios);
+ if (bitmap_empty(valid_mask, ngpios))
+ dev_dbg(dev, "no pin configured for GPIO\n");
+ else
+ dev_dbg(dev, "GPIO.%*pbl configured for GPIO\n", ngpios,
+ valid_mask);
return 0;
}
--
2.26.3
[PATCH 1/2] USB: serial: cp210x: provide gpio valid mask
Use the new GPIO valid-mask feature to inform gpiolib which pins are
available for use instead of handling that in a request callback.
This also allows user space to figure out which pins are available
through the chardev interface without having to request each pin in
turn.
Note that the return value when requesting an unavailable pin will now
be -EINVAL instead of -ENODEV.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/cp210x.c | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index a373cd63b3a4..ceb3a656a075 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -1410,17 +1410,6 @@ static void cp210x_break_ctl(struct tty_struct *tty, int
break_state)
}
#ifdef CONFIG_GPIOLIB
-static int cp210x_gpio_request(struct gpio_chip *gc, unsigned int offset)
-{
- struct usb_serial *serial = gpiochip_get_data(gc);
- struct cp210x_serial_private *priv = usb_get_serial_data(serial);
-
- if (priv->gpio_altfunc & BIT(offset))
- return -ENODEV;
-
- return 0;
-}
-
static int cp210x_gpio_get(struct gpio_chip *gc, unsigned int gpio)
{
struct usb_serial *serial = gpiochip_get_data(gc);
@@ -1549,6 +1538,18 @@ static int cp210x_gpio_set_config(struct gpio_chip *gc,
unsigned int gpio,
return -ENOTSUPP;
}
+static int cp210x_gpio_init_valid_mask(struct gpio_chip *gc,
+ unsigned long *valid_mask, unsigned int ngpios)
+{
+ struct usb_serial *serial = gpiochip_get_data(gc);
+ struct cp210x_serial_private *priv = usb_get_serial_data(serial);
+ unsigned long altfunc_mask = priv->gpio_altfunc;
+
+ bitmap_complement(valid_mask, _mask, ngpios);
+
+ return 0;
+}
+
/*
* This function is for configuring GPIO using shared pins, where other signals
* are made unavailable by configuring the use of GPIO. This is believed to be
@@ -1786,13 +1787,13 @@ static int cp210x_gpio_init(struct usb_serial *serial)
return result;
priv->gc.label = "cp210x";
- priv->gc.request = cp210x_gpio_request;
priv->gc.get_direction = cp210x_gpio_direction_get;
priv->gc.direction_input = cp210x_gpio_direction_input;
priv->gc.direction_output = cp210x_gpio_direction_output;
priv->gc.get = cp210x_gpio_get;
priv->gc.set = cp210x_gpio_set;
priv->gc.set_config = cp210x_gpio_set_config;
+ priv->gc.init_valid_mask = cp210x_gpio_init_valid_mask;
priv->gc.owner = THIS_MODULE;
priv->gc.parent = >interface->dev;
priv->gc.base = -1;
--
2.26.3
Re: [PATCH v8] USB: serial: cp210x: Add support for GPIOs on CP2108
On Wed, Apr 07, 2021 at 01:25:00AM +0300, Andy Shevchenko wrote: > On Tuesday, April 6, 2021, Pho Tran wrote: > > Because CP2108 has 16 GPIO pins, the parameter passed by cp210x functions > > will be different from other CP210x devices. So need to check part number > > of the device to use correct data format before sending commands to > > devices. > > > > Like CP2104, CP2108 have GPIO pins with configurable options. Therefore, > > should be mask all pins which are not in GPIO mode in cp2108_gpio_init() > > function. > This I didn’t get. If you are talking about usage pin as GPIO, perhaps you > should use valid_mask in GPIO chip structure. Otherwise you probably need > to implement a proper pinmux ops for this (and register a pin controller > which the code below also suggests). Neither is needed here. Using a valid mask is a new feature and isn't a prerequisite for adding support for the GPIOs on cp2108. I've been meaning to implement that since we started using it for ftdi_sio, and I'll be posting that shortly. The cp2108 pin configuration can't be changed at runtime, but even if it was it's not clear what the pinctrl subsystem would buy us for a hotpluggable USB device currently (even if devicetree could be used for static topologies). I'll look at the rest of this thread and the latest version of this patch next week. Johan
[PATCH] tty: clarify that not all ttys have a class device
Commit 30004ac9c090 ("tty: add tty_struct->dev pointer to corresponding
device instance") added a struct device pointer field to struct
tty_struct which was populated with the corresponding tty class device
during initialisation.
Unfortunately, not all ttys have a class device (e.g. pseudoterminals
and serdev) in which case the device pointer will be set to NULL,
something which have bit driver authors over the years.
In retrospect perhaps this field should never have been added, but let's
at least document the current behaviour.
Signed-off-by: Johan Hovold
---
include/linux/tty.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 95fc2f100f12..178d8a3b18f2 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -284,7 +284,7 @@ struct tty_operations;
struct tty_struct {
int magic;
struct kref kref;
- struct device *dev;
+ struct device *dev; /* class device or NULL (e.g. ptys, serdev) */
struct tty_driver *driver;
const struct tty_operations *ops;
int index;
--
2.26.3
Re: [PATCH 00/13] tty.h cleanups
On Thu, Apr 08, 2021 at 08:01:08PM +0200, Greg Kroah-Hartman wrote: > On Thu, Apr 08, 2021 at 04:25:22PM +0200, Johan Hovold wrote: > > On Thu, Apr 08, 2021 at 02:51:21PM +0200, Greg Kroah-Hartman wrote: > > > Turns out there is a lot of tty-internal stuff in include/linux/tty.h > > > that do not belong there. Create a internal-to-the-tty-layer .h file > > > for these types of things and move function prototypes to it instead of > > > being in the system-wide header file. > > > > > > Along the way clean up the use of some old tty-only debugging macros and > > > use the in-kernel dev_*() calls instead. > > > > I'm afraid that's not a good idea since not all ttys have a > > corresponding class device. Notable exception include pseudo terminals > > and serdev. > > > > While dev_printk() can handle a NULL device argument without crashing, > > we'll actually lose log information by removing the tty printk helpers. > > I think the same info will be printed here as before, just some NULL > information at the beginning, right? And the benifits overall (for real > tty devices), should outweigh the few devices that do not have this > information. No, you'll only be losing information (tty driver and tty name). Here's a pty example, where the first line in each pair use dev_info() and the second tty_info(): [ 10.235331] (NULL device *): tty_get_device [ 10.235441] ptm ptm0: tty_get_device [ 10.235586] (NULL device *): tty_get_device [ 10.235674] pts pts0: tty_get_device and similar for serdev, which is becoming more and more common. Johan
[PATCH] USB: serial: do not use tty class device for debugging
Use the port struct device rather than tty class device for debugging.
Note that while USB serial doesn't support serdev yet (due to serdev not
handling hotplugging), serdev ttys do not have a corresponding class
device and would have been logged using a "(NULL device *):" prefix.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/metro-usb.c | 4 ++--
drivers/usb/serial/upd78f0730.c | 7 +++
drivers/usb/serial/usb-serial.c | 32
3 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/serial/metro-usb.c b/drivers/usb/serial/metro-usb.c
index 0bfe4459c37f..f9ce9e7b9b80 100644
--- a/drivers/usb/serial/metro-usb.c
+++ b/drivers/usb/serial/metro-usb.c
@@ -299,7 +299,7 @@ static int metrousb_tiocmset(struct tty_struct *tty,
unsigned long flags = 0;
unsigned long control_state = 0;
- dev_dbg(tty->dev, "%s - set=%d, clear=%d\n", __func__, set, clear);
+ dev_dbg(>dev, "%s - set=%d, clear=%d\n", __func__, set, clear);
spin_lock_irqsave(_priv->lock, flags);
control_state = metro_priv->control_state;
@@ -334,7 +334,7 @@ static void metrousb_unthrottle(struct tty_struct *tty)
/* Submit the urb to read from the port. */
result = usb_submit_urb(port->interrupt_in_urb, GFP_ATOMIC);
if (result)
- dev_err(tty->dev,
+ dev_err(>dev,
"failed submitting interrupt in urb error code=%d\n",
result);
}
diff --git a/drivers/usb/serial/upd78f0730.c b/drivers/usb/serial/upd78f0730.c
index 26d7b003b7e3..63d4a784ae45 100644
--- a/drivers/usb/serial/upd78f0730.c
+++ b/drivers/usb/serial/upd78f0730.c
@@ -182,7 +182,6 @@ static void upd78f0730_port_remove(struct usb_serial_port
*port)
static int upd78f0730_tiocmget(struct tty_struct *tty)
{
- struct device *dev = tty->dev;
struct upd78f0730_port_private *private;
struct usb_serial_port *port = tty->driver_data;
int signals;
@@ -197,7 +196,7 @@ static int upd78f0730_tiocmget(struct tty_struct *tty)
res = ((signals & UPD78F0730_DTR) ? TIOCM_DTR : 0) |
((signals & UPD78F0730_RTS) ? TIOCM_RTS : 0);
- dev_dbg(dev, "%s - res = %x\n", __func__, res);
+ dev_dbg(>dev, "%s - res = %x\n", __func__, res);
return res;
}
@@ -205,10 +204,10 @@ static int upd78f0730_tiocmget(struct tty_struct *tty)
static int upd78f0730_tiocmset(struct tty_struct *tty,
unsigned int set, unsigned int clear)
{
- struct device *dev = tty->dev;
struct usb_serial_port *port = tty->driver_data;
struct upd78f0730_port_private *private;
struct upd78f0730_set_dtr_rts request;
+ struct device *dev = >dev;
int res;
private = usb_get_serial_port_data(port);
@@ -241,10 +240,10 @@ static int upd78f0730_tiocmset(struct tty_struct *tty,
static void upd78f0730_break_ctl(struct tty_struct *tty, int break_state)
{
- struct device *dev = tty->dev;
struct upd78f0730_port_private *private;
struct usb_serial_port *port = tty->driver_data;
struct upd78f0730_set_dtr_rts request;
+ struct device *dev = >dev;
private = usb_get_serial_port_data(port);
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 255f562ef1a0..98b33b1b5357 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -281,7 +281,7 @@ static int serial_open(struct tty_struct *tty, struct file
*filp)
{
struct usb_serial_port *port = tty->driver_data;
- dev_dbg(tty->dev, "%s\n", __func__);
+ dev_dbg(>dev, "%s\n", __func__);
return tty_port_open(>port, tty, filp);
}
@@ -310,7 +310,7 @@ static void serial_hangup(struct tty_struct *tty)
{
struct usb_serial_port *port = tty->driver_data;
- dev_dbg(tty->dev, "%s\n", __func__);
+ dev_dbg(>dev, "%s\n", __func__);
tty_port_hangup(>port);
}
@@ -319,7 +319,7 @@ static void serial_close(struct tty_struct *tty, struct
file *filp)
{
struct usb_serial_port *port = tty->driver_data;
- dev_dbg(tty->dev, "%s\n", __func__);
+ dev_dbg(>dev, "%s\n", __func__);
tty_port_close(>port, tty, filp);
}
@@ -339,7 +339,7 @@ static void serial_cleanup(struct tty_struct *tty)
struct usb_serial *serial;
struct module *owner;
- dev_dbg(tty->dev, "%s\n", __func__);
+ dev_dbg(>dev, "%s\n", __func__);
/* The console is magical. Do not hang up the console hardware
* or there will be tears.
@@ -367,7 +367,7 @@ static int serial_write(struct tty_struct *tty, const
unsigned char *buf,
if (port-
Re: [PATCH 00/13] tty.h cleanups
On Thu, Apr 08, 2021 at 02:51:21PM +0200, Greg Kroah-Hartman wrote: > Turns out there is a lot of tty-internal stuff in include/linux/tty.h > that do not belong there. Create a internal-to-the-tty-layer .h file > for these types of things and move function prototypes to it instead of > being in the system-wide header file. > > Along the way clean up the use of some old tty-only debugging macros and > use the in-kernel dev_*() calls instead. I'm afraid that's not a good idea since not all ttys have a corresponding class device. Notable exception include pseudo terminals and serdev. While dev_printk() can handle a NULL device argument without crashing, we'll actually lose log information by removing the tty printk helpers. Johan
[PATCH v2 1/3] Revert "USB: cdc-acm: fix rounding error in TIOCSSERIAL"
This reverts commit b401f8c4f492cbf74f3f59c9141e5be3071071bb.
The offending commit claimed that trying to set the values reported back
by TIOCGSERIAL as a regular user could result in an -EPERM error when HZ
is 250, but that was never the case.
With HZ=250, the default 0.5 second value of close_delay is converted to
125 jiffies when set and is converted back to 50 centiseconds by
TIOCGSERIAL as expected (not 12 cs as was claimed, even if that was the
case before an earlier fix).
Comparing the internal current and new jiffies values is just fine to
determine if the value is about to change so drop the bogus workaround
(which was also backported to stable).
For completeness: With different default values for these parameters or
with a HZ value not divisible by two, the lack of rounding when setting
the default values in tty_port_init() could result in an -EPERM being
returned, but this is hardly something we need to worry about.
Cc: Anthony Mallet
Cc: sta...@vger.kernel.org
Signed-off-by: Johan Hovold
---
drivers/usb/class/cdc-acm.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 3fda1ec961d7..96e221803fa6 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -942,7 +942,6 @@ static int set_serial_info(struct tty_struct *tty, struct
serial_struct *ss)
{
struct acm *acm = tty->driver_data;
unsigned int closing_wait, close_delay;
- unsigned int old_closing_wait, old_close_delay;
int retval = 0;
close_delay = msecs_to_jiffies(ss->close_delay * 10);
@@ -950,17 +949,11 @@ static int set_serial_info(struct tty_struct *tty, struct
serial_struct *ss)
ASYNC_CLOSING_WAIT_NONE :
msecs_to_jiffies(ss->closing_wait * 10);
- /* we must redo the rounding here, so that the values match */
- old_close_delay = jiffies_to_msecs(acm->port.close_delay) / 10;
- old_closing_wait = acm->port.closing_wait == ASYNC_CLOSING_WAIT_NONE ?
- ASYNC_CLOSING_WAIT_NONE :
- jiffies_to_msecs(acm->port.closing_wait) / 10;
-
mutex_lock(>port.mutex);
if (!capable(CAP_SYS_ADMIN)) {
- if ((ss->close_delay != old_close_delay) ||
- (ss->closing_wait != old_closing_wait))
+ if ((close_delay != acm->port.close_delay) ||
+ (closing_wait != acm->port.closing_wait))
retval = -EPERM;
else
retval = -EOPNOTSUPP;
--
2.26.3
[PATCH v2 0/3] TIOCSSERIAL fixes
This series fixes up a few issues with cdc-acm TIOCSSERIAL implementation. Johan Changes in v2 - amend commit message to clarify that the 12 cs close_delay bug had already been fixed by an earlier patch (1/3) - amend commit message to clarify that the base clock rate isn't known for CDC and that the current line speed can still be retrieved through the standard termios interfaces (3/3) Johan Hovold (3): Revert "USB: cdc-acm: fix rounding error in TIOCSSERIAL" USB: cdc-acm: fix unprivileged TIOCCSERIAL USB: cdc-acm: fix TIOCGSERIAL implementation drivers/usb/class/cdc-acm.c | 16 +++- 1 file changed, 3 insertions(+), 13 deletions(-) -- 2.26.3
[PATCH v2 2/3] USB: cdc-acm: fix unprivileged TIOCCSERIAL
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
A non-privileged user has only ever been able to set the since long
deprecated ASYNC_SPD flags and trying to change any other *supported*
feature should result in -EPERM being returned. Setting the current
values for any supported features should return success.
Fix the cdc-acm implementation which instead indicated that the
TIOCSSERIAL ioctl was not even implemented when a non-privileged user
set the current values.
Fixes: ba2d8ce9db0a ("cdc-acm: implement TIOCSSERIAL to avoid blocking
close(2)")
Signed-off-by: Johan Hovold
---
drivers/usb/class/cdc-acm.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 96e221803fa6..43e31dad4831 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -955,8 +955,6 @@ static int set_serial_info(struct tty_struct *tty, struct
serial_struct *ss)
if ((close_delay != acm->port.close_delay) ||
(closing_wait != acm->port.closing_wait))
retval = -EPERM;
- else
- retval = -EOPNOTSUPP;
} else {
acm->port.close_delay = close_delay;
acm->port.closing_wait = closing_wait;
--
2.26.3
[PATCH v2 3/3] USB: cdc-acm: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The xmit_fifo_size parameter could be used to set the hardware transmit
fifo size of a legacy UART when it could not be detected, but the
interface is limited to eight bits and should be left unset when it is
not used.
Similarly, baud_base could be used to set the UART base clock when it
could not be detected, but might as well be left unset when it is not
known (which is the case for CDC).
Fix the cdc-acm TIOCGSERIAL implementation by dropping its custom
interpretation of the unused xmit_fifo_size and baud_base fields, which
overflowed the former with the URB buffer size and set the latter to the
current line speed. Also return the port line number, which is the only
other value used besides the close parameters.
Note that the current line speed can still be retrieved through the
standard termios interfaces.
Fixes: 18c75720e667 ("USB: allow users to run setserial with cdc-acm")
Signed-off-by: Johan Hovold
---
drivers/usb/class/cdc-acm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 43e31dad4831..b74713518b3a 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -929,8 +929,7 @@ static int get_serial_info(struct tty_struct *tty, struct
serial_struct *ss)
{
struct acm *acm = tty->driver_data;
- ss->xmit_fifo_size = acm->writesize;
- ss->baud_base = le32_to_cpu(acm->line.dwDTERate);
+ ss->line = acm->minor;
ss->close_delay = jiffies_to_msecs(acm->port.close_delay) / 10;
ss->closing_wait = acm->port.closing_wait == ASYNC_CLOSING_WAIT_NONE ?
ASYNC_CLOSING_WAIT_NONE :
--
2.26.3
Re: [PATCH 3/3] USB: cdc-acm: fix TIOCGSERIAL implementation
On Thu, Apr 08, 2021 at 01:34:12PM +0200, Oliver Neukum wrote: > Am Donnerstag, den 08.04.2021, 11:48 +0200 schrieb Johan Hovold: > > On Thu, Apr 08, 2021 at 10:36:46AM +0200, Oliver Neukum wrote: > > > Am Mittwoch, den 07.04.2021, 12:28 +0200 schrieb Johan Hovold: > > > > Well, the devices report it. It is part of the standard. > > > > No, the standard doesn't include anything about a baud-base clock > > AFAICT. > > Unfortunately it does. > dwDTERate - chapter 6.3.11 - table 17 That's not the base clock rate, that's just the currently configured line speed which you can read from termios. > If we does this wrongly, we should certainly fix it, but just removing > the reporting doesn't look right to me. The driver got its interpretation of baud_base wrong, and CDC doesn't even have a concept of base clock rate so removing it is the right thing to do. Again, baud_base is really only relevant with legacy UARTs and when using the deprecated ASYNC_SPD_CUST. And if the user wants to knows the current line speed we have a different interface for that. Johan
Re: [PATCH 3/3] USB: cdc-acm: fix TIOCGSERIAL implementation
On Thu, Apr 08, 2021 at 10:36:46AM +0200, Oliver Neukum wrote:
> Am Mittwoch, den 07.04.2021, 12:28 +0200 schrieb Johan Hovold:
> > TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
> > serial devices is only useful for setting the close_delay and
> > closing_wait parameters.
> >
> > The xmit_fifo_size parameter could be used to set the hardware transmit
> > fifo size of a legacy UART when it could not be detected, but the
> > interface is limited to eight bits and should be left unset when it is
> > not used.
>
> OK.
>
> > Similarly, baud_base could be used to set the uart base clock when it
> > could not be detected, but might as well be left unset when it is not
> > known.
>
> Well, the devices report it. It is part of the standard.
No, the standard doesn't include anything about a baud-base clock
AFAICT.
> > Fix the cdc-acm TIOCGSERIAL implementation by dropping its custom
> > interpretation of the unused xmit_fifo_size and baud_base fields, which
> > overflowed the former with the URB buffer size and set the latter to the
> > current line speed. Also return the port line number, which is the only
> > other value used besides the close parameters.
> >
> > Fixes: 18c75720e667 ("USB: allow users to run setserial with cdc-acm")
> > Signed-off-by: Johan Hovold
> > ---
> > drivers/usb/class/cdc-acm.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> > index 43e31dad4831..b74713518b3a 100644
> > --- a/drivers/usb/class/cdc-acm.c
> > +++ b/drivers/usb/class/cdc-acm.c
> > @@ -929,8 +929,7 @@ static int get_serial_info(struct tty_struct *tty,
> > struct serial_struct *ss)
> > {
> > struct acm *acm = tty->driver_data;
> >
> > - ss->xmit_fifo_size = acm->writesize;
> > - ss->baud_base = le32_to_cpu(acm->line.dwDTERate);
>
> If we do this, we have a value that can be set, but is not reported.
> That looks a bit strange to me.
Again, no, the baud_base cannot be set and it is unknown and unused.
The only reason to report back baud_base is to support the deprecated
ASYNC_SPD_CUST interface used to set a custom divisor. cdc-acm has never
supported that for obvious reasons.
Johan
Re: [PATCH 2/3] USB: cdc-acm: fix unprivileged TIOCCSERIAL
On Thu, Apr 08, 2021 at 09:48:38AM +0200, Oliver Neukum wrote: > Am Mittwoch, den 07.04.2021, 12:28 +0200 schrieb Johan Hovold: > > TIOCSSERIAL is a horrid, underspecified, legacy interface which for most > > serial devices is only useful for setting the close_delay and > > closing_wait parameters. > > > > A non-privileged user has only ever been able to set the since long > > deprecated ASYNC_SPD flags and trying to change any other *supported* > > feature should result in -EPERM being returned. Setting the current > > values for any supported features should return success. > > > > Fix the cdc-acm implementation which instead indicated that the > > TIOCSSERIAL ioctl was not even implemented when a non-privileged user > > set the current values. > > Hi, > > the idea here was that you are setting something else, if you are > not changing a parameter that can be changed. That conclusion is > dubious, but at the same time, this implementation can change > only these two parameters. So can the test really be dropped > as opposed to be modified? The de-facto standard for how to handle change requests for non-supported features (e.g. changing the I/O port or IRQ) is to simply ignore them and return 0. For most (non-legacy) serial devices the only relevant parameters are close_delay and closing_wait. And as we need to return -EPERM when a non-privileged user tries to change these, we cannot drop the test. (And returning -EOPNOTSUPP was never correct as the ioctl is indeed supported.) Johan
Re: [PATCH 0/4] USB: serial: closing-wait cleanups
On Wed, Apr 07, 2021 at 05:24:52PM +0200, Greg Kroah-Hartman wrote: > On Wed, Apr 07, 2021 at 12:45:25PM +0200, Johan Hovold wrote: > > Now that all USB serial drivers supports changing the closing_wait > > parameter through TIOCSSERIAL (setserial), we can remove the related > > driver-specific module parameters and settings. > > > > These depend on the recently posted TIOCSSERIAL series. > > Yes! Getting rid of the module parameter is so good... > > Reviewed-by: Greg Kroah-Hartman Thanks for reviewing these. All three sets now applied. Johan
Re: [PATCH 1/3] Revert "USB: cdc-acm: fix rounding error in TIOCSSERIAL"
On Wed, Apr 07, 2021 at 01:04:31PM +0200, Anthony Mallet wrote: > On Wednesday 7 Apr 2021, at 12:28, Johan Hovold wrote: > > With HZ=250, the default 0.5 second value of close_delay is converted to > > 125 jiffies when set and is converted back to 50 centiseconds by > > TIOCGSERIAL as expected (not 12 cs as was claimed). > > It was "12" (instead of 50) because the conversion gor TIOCGSERIAL was > initially broken, and that was fixed in the previous commit > 633e2b2ded739a34bd0fb1d8b5b871f7e489ea29 Right, so this patch is still just broken. The missing jiffies conversion had already been added. > > For completeness: With different default values for these parameters or > > with a HZ value not divisible by two, the lack of rounding when setting > > the default values in tty_port_init() could result in an -EPERM being > > returned, but this is hardly something we need to worry about. > > The -EPERM is harmful when a regular user wants to update other > members of serial_struct without changing the close delays, > e.g. ASYNC_LOW_LATENCY, which is granted to regular users. You're missing the point; -EPERM will *not* be returned -- and this patch was never needed. Johan
[PATCH net-next] net: wan: z85230: drop unused async state
According to the changelog, asynchronous mode was dropped sometime before v2.2. Let's get rid of the unused driver-specific async state as well so that it doesn't show up when doing tree-wide tty work. Signed-off-by: Johan Hovold --- drivers/net/wan/z85230.h | 39 --- 1 file changed, 39 deletions(-) diff --git a/drivers/net/wan/z85230.h b/drivers/net/wan/z85230.h index 1081d171e477..462cb620bc5d 100644 --- a/drivers/net/wan/z85230.h +++ b/drivers/net/wan/z85230.h @@ -327,45 +327,6 @@ struct z8530_channel void*private; /* For our owner */ struct net_device *netdevice; /* Network layer device */ - /* -* Async features -*/ - - struct tty_struct *tty; /* Attached terminal */ - int line; /* Minor number */ - wait_queue_head_t open_wait; /* Tasks waiting to open */ - wait_queue_head_t close_wait; /* and for close to end */ - unsigned long event; /* Pending events */ - int fdcount;/* # of fd on device */ - int blocked_open; /* # of blocked opens */ - int x_char; /* XON/XOF char */ - unsigned char *xmit_buf; /* Transmit pointer */ - int xmit_head; /* Transmit ring */ - int xmit_tail; - int xmit_cnt; - int flags; - int timeout; - int xmit_fifo_size; /* Transmit FIFO info */ - - int close_delay;/* Do we wait for drain on close ? */ - unsigned short closing_wait; - - /* We need to know the current clock divisor -* to read the bps rate the chip has currently -* loaded. -*/ - - unsigned char clk_divisor; /* May be 1, 16, 32, or 64 */ - int zs_baud; - - int magic; - int baud_base; /* Baud parameters */ - int custom_divisor; - - - unsigned char tx_active; /* character is being xmitted */ - unsigned char tx_stopped; /* output is suspended */ - spinlock_t *lock;/* Device lock */ }; -- 2.26.3
[PATCH 0/4] USB: serial: closing-wait cleanups
Now that all USB serial drivers supports changing the closing_wait parameter through TIOCSSERIAL (setserial), we can remove the related driver-specific module parameters and settings. These depend on the recently posted TIOCSSERIAL series. Johan Johan Hovold (4): USB: serial: io_ti: drop closing_wait module parameter USB: serial: io_ti: switch to 30-second closing wait USB: serial: ti_usb_3410_5052: drop closing_wait module parameter USB: serial: ti_usb_3410_5052: switch to 30-second closing wait drivers/usb/serial/io_ti.c| 7 --- drivers/usb/serial/ti_usb_3410_5052.c | 9 - 2 files changed, 16 deletions(-) -- 2.26.3
[PATCH 4/4] USB: serial: ti_usb_3410_5052: switch to 30-second closing wait
Switch to using the system-wide default 30-second closing-wait timeout
instead of the driver specific 40-second timeout.
The timeout can be changed per port using TIOCSSERIAL (setserial) if
needed.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ti_usb_3410_5052.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/usb/serial/ti_usb_3410_5052.c
b/drivers/usb/serial/ti_usb_3410_5052.c
index 35cc1be738ef..03839289d6c0 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -270,8 +270,6 @@ struct ti_firmware_header {
#define TI_TRANSFER_TIMEOUT2
-#define TI_DEFAULT_CLOSING_WAIT4000/* in .01 secs */
-
/* read urb states */
#define TI_READ_URB_RUNNING0
#define TI_READ_URB_STOPPING 1
@@ -602,7 +600,6 @@ static int ti_port_probe(struct usb_serial_port *port)
tport->tp_uart_base_addr = TI_UART1_BASE_ADDR;
else
tport->tp_uart_base_addr = TI_UART2_BASE_ADDR;
- port->port.closing_wait = msecs_to_jiffies(10 *
TI_DEFAULT_CLOSING_WAIT);
tport->tp_port = port;
tport->tp_tdev = usb_get_serial_data(port->serial);
--
2.26.3
[PATCH 3/4] USB: serial: ti_usb_3410_5052: drop closing_wait module parameter
The ti_usb_3410_5052 has supported changing the closing_wait parameter
through TIOCSSERIAL (setserial) for about a decade and commit
f1175daa5312 ("USB: ti_usb_3410_5052: kill custom closing_wait").
It's time to drop the corresponding driver-specific module parameter.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ti_usb_3410_5052.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/usb/serial/ti_usb_3410_5052.c
b/drivers/usb/serial/ti_usb_3410_5052.c
index c312d0cce5fb..35cc1be738ef 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -344,8 +344,6 @@ static int ti_write_byte(struct usb_serial_port *port,
struct ti_device *tdev,
static int ti_download_firmware(struct ti_device *tdev);
-static int closing_wait = TI_DEFAULT_CLOSING_WAIT;
-
static const struct usb_device_id ti_id_table_3410[] = {
{ USB_DEVICE(TI_VENDOR_ID, TI_3410_PRODUCT_ID) },
{ USB_DEVICE(TI_VENDOR_ID, TI_3410_EZ430_ID) },
@@ -497,10 +495,6 @@ MODULE_FIRMWARE("moxa/moxa-1131.fw");
MODULE_FIRMWARE("moxa/moxa-1150.fw");
MODULE_FIRMWARE("moxa/moxa-1151.fw");
-module_param(closing_wait, int, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(closing_wait,
-"Maximum wait for data to drain in close, in .01 secs, default is 4000");
-
MODULE_DEVICE_TABLE(usb, ti_id_table_combined);
module_usb_serial_driver(serial_drivers, ti_id_table_combined);
@@ -608,7 +602,7 @@ static int ti_port_probe(struct usb_serial_port *port)
tport->tp_uart_base_addr = TI_UART1_BASE_ADDR;
else
tport->tp_uart_base_addr = TI_UART2_BASE_ADDR;
- port->port.closing_wait = msecs_to_jiffies(10 * closing_wait);
+ port->port.closing_wait = msecs_to_jiffies(10 *
TI_DEFAULT_CLOSING_WAIT);
tport->tp_port = port;
tport->tp_tdev = usb_get_serial_data(port->serial);
--
2.26.3
[PATCH 1/4] USB: serial: io_ti: drop closing_wait module parameter
Now that all USB serial drivers supports setting the closing_wait
parameter through TIOCSSERIAL (setserial) it's time to drop the
corresponding io_ti module parameter.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/io_ti.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index f548cdbf0a51..6eff0e5a7545 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -211,7 +211,6 @@ static const struct usb_device_id id_table_combined[] = {
MODULE_DEVICE_TABLE(usb, id_table_combined);
-static int closing_wait = EDGE_CLOSING_WAIT;
static bool ignore_cpu_rev;
static int default_uart_mode; /* RS232 */
@@ -2593,7 +2592,7 @@ static int edge_port_probe(struct usb_serial_port *port)
if (ret)
goto err;
- port->port.closing_wait = msecs_to_jiffies(closing_wait * 10);
+ port->port.closing_wait = msecs_to_jiffies(EDGE_CLOSING_WAIT * 10);
port->port.drain_delay = 1;
return 0;
@@ -2759,9 +2758,6 @@ MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE("GPL");
MODULE_FIRMWARE("edgeport/down3.bin");
-module_param(closing_wait, int, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(closing_wait, "Maximum wait for data to drain, in .01 secs");
-
module_param(ignore_cpu_rev, bool, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(ignore_cpu_rev,
"Ignore the cpu revision when connecting to a device");
--
2.26.3
[PATCH 2/4] USB: serial: io_ti: switch to 30-second closing wait
Switch to using the system-wide default 30-second closing-wait timeout
instead of the driver specific 40-second timeout.
The timeout can be changed per port using TIOCSSERIAL (setserial) if
needed.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/io_ti.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index 6eff0e5a7545..75325c2b295e 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -60,8 +60,6 @@
#define EDGE_READ_URB_STOPPING 1
#define EDGE_READ_URB_STOPPED 2
-#define EDGE_CLOSING_WAIT 4000/* in .01 sec */
-
/* Product information read from the Edgeport */
struct product_info {
@@ -2592,7 +2590,6 @@ static int edge_port_probe(struct usb_serial_port *port)
if (ret)
goto err;
- port->port.closing_wait = msecs_to_jiffies(EDGE_CLOSING_WAIT * 10);
port->port.drain_delay = 1;
return 0;
--
2.26.3
[PATCH 21/24] USB: serial: stop reporting legacy UART types
The TIOCGSERIAL ioctl can be used to set and retrieve the UART type for
legacy UARTs, but some USB serial drivers have been reporting back
random types in order to "make user-space happy".
Some applications have historically expected TIOCGSERIAL to be
implemented, but judging from the Debian sources, the port type not
being PORT_UNKNOWN is only used to check for the existence of legacy
serial ports (ttySn).
Drivers like ftdi_sio have been using PORT_UNKNOWN for twenty years (and
option for 10 years) without anyone complaining so let's stop reporting
back anything else.
In the unlikely event that this do cause problems, this should be fixed
tree-wide anyway (e.g. for all USB serial drivers and also CDC-ACM).
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ark3116.c | 7 ---
drivers/usb/serial/f81232.c | 1 -
drivers/usb/serial/f81534.c | 1 -
drivers/usb/serial/io_edgeport.c | 10 --
drivers/usb/serial/io_ti.c| 7 ---
drivers/usb/serial/mos7720.c | 6 --
drivers/usb/serial/mos7840.c | 11 ---
drivers/usb/serial/opticon.c | 7 ---
drivers/usb/serial/pl2303.c | 6 --
drivers/usb/serial/ti_usb_3410_5052.c | 1 -
drivers/usb/serial/whiteheat.c| 1 -
11 files changed, 58 deletions(-)
diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index c0cf60e9273d..5dd710e9fe7d 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -385,12 +385,6 @@ static int ark3116_open(struct tty_struct *tty, struct
usb_serial_port *port)
return result;
}
-static void ark3116_get_serial_info(struct tty_struct *tty,
- struct serial_struct *ss)
-{
- ss->type = PORT_16654;
-}
-
static int ark3116_tiocmget(struct tty_struct *tty)
{
struct usb_serial_port *port = tty->driver_data;
@@ -627,7 +621,6 @@ static struct usb_serial_driver ark3116_device = {
.port_probe = ark3116_port_probe,
.port_remove = ark3116_port_remove,
.set_termios = ark3116_set_termios,
- .get_serial = ark3116_get_serial_info,
.tiocmget = ark3116_tiocmget,
.tiocmset = ark3116_tiocmset,
.tiocmiwait = usb_serial_generic_tiocmiwait,
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 5e34b364d94d..b4b847dce4bc 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -825,7 +825,6 @@ static void f81232_get_serial(struct tty_struct *tty,
struct serial_struct *ss)
struct usb_serial_port *port = tty->driver_data;
struct f81232_private *priv = usb_get_serial_port_data(port);
- ss->type = PORT_16550A;
ss->baud_base = priv->baud_base;
}
diff --git a/drivers/usb/serial/f81534.c b/drivers/usb/serial/f81534.c
index 633de52feaad..c0bca52ef92a 100644
--- a/drivers/usb/serial/f81534.c
+++ b/drivers/usb/serial/f81534.c
@@ -1147,7 +1147,6 @@ static void f81534_get_serial_info(struct tty_struct
*tty, struct serial_struct
port_priv = usb_get_serial_port_data(port);
- ss->type = PORT_16550A;
ss->baud_base = port_priv->baud_base;
}
diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c
index f669b065dc42..4f9fb1d96380 100644
--- a/drivers/usb/serial/io_edgeport.c
+++ b/drivers/usb/serial/io_edgeport.c
@@ -1637,12 +1637,6 @@ static int edge_tiocmget(struct tty_struct *tty)
return result;
}
-static void get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
-{
- ss->type = PORT_16550A;
-}
-
-
/*
* SerialIoctl
* this function handles any ioctl calls to the driver
@@ -3104,7 +3098,6 @@ static struct usb_serial_driver edgeport_2port_device = {
.set_termios= edge_set_termios,
.tiocmget = edge_tiocmget,
.tiocmset = edge_tiocmset,
- .get_serial = get_serial_info,
.tiocmiwait = usb_serial_generic_tiocmiwait,
.get_icount = usb_serial_generic_get_icount,
.write = edge_write,
@@ -3140,7 +3133,6 @@ static struct usb_serial_driver edgeport_4port_device = {
.set_termios= edge_set_termios,
.tiocmget = edge_tiocmget,
.tiocmset = edge_tiocmset,
- .get_serial = get_serial_info,
.tiocmiwait = usb_serial_generic_tiocmiwait,
.get_icount = usb_serial_generic_get_icount,
.write = edge_write,
@@ -3176,7 +3168,6 @@ static struct usb_serial_driver edgeport_8port_device = {
.set_termios= edge_set_termios,
.tiocmget = edge_tiocmget,
[PATCH 19/24] USB: serial: fix return value for unsupported ioctls
Drivers should return -ENOTTY ("Inappropriate I/O control operation")
when an ioctl isn't supported, while -EINVAL is used for invalid
arguments.
Fix up the TIOCMGET, TIOCMSET and TIOCGICOUNT helpers which returned
-EINVAL when a USB serial driver did not implement the corresponding
methods.
Note that the TIOCMGET and TIOCMSET helpers predate git and do not get a
corresponding Fixes tag below.
Fixes: d281da7ff6f7 ("tty: Make tiocgicount a handler")
Signed-off-by: Johan Hovold
---
drivers/usb/serial/usb-serial.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 27e3bb58c872..c311cc4fabee 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -502,7 +502,7 @@ static int serial_tiocmget(struct tty_struct *tty)
if (port->serial->type->tiocmget)
return port->serial->type->tiocmget(tty);
- return -EINVAL;
+ return -ENOTTY;
}
static int serial_tiocmset(struct tty_struct *tty,
@@ -514,7 +514,7 @@ static int serial_tiocmset(struct tty_struct *tty,
if (port->serial->type->tiocmset)
return port->serial->type->tiocmset(tty, set, clear);
- return -EINVAL;
+ return -ENOTTY;
}
static int serial_get_icount(struct tty_struct *tty,
@@ -526,7 +526,7 @@ static int serial_get_icount(struct tty_struct *tty,
if (port->serial->type->get_icount)
return port->serial->type->get_icount(tty, icount);
- return -EINVAL;
+ return -ENOTTY;
}
/*
--
2.26.3
[PATCH 20/24] USB: serial: add generic support for TIOCSSERIAL
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The closing_wait parameter determines how long to wait for the transfer
buffers to drain during close and the default timeout of 30 seconds may
not be sufficient at low line speeds. In other cases, when for example
flow is stopped, the default timeout may instead be too long.
Add generic support for TIOCSSERIAL and TIOCGSERIAL with handling of the
three common parameters close_delay, closing_wait and line for the
benefit of all USB serial drivers while still allowing drivers to
implement further functionality through the existing callbacks.
This currently includes a few drivers that report their base baud clock
rate even if that is really only of interest when setting custom
divisors through the deprecated ASYNC_SPD_CUST interface; an interface
which only the FTDI driver actually implements.
Some drivers have also been reporting back a fake UART type, something
which should no longer be needed and will be dropped by a follow-on
patch.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ark3116.c | 9 +
drivers/usb/serial/f81232.c | 12 ++
drivers/usb/serial/f81534.c | 8 +---
drivers/usb/serial/ftdi_sio.c | 11 +-
drivers/usb/serial/io_edgeport.c | 12 +-
drivers/usb/serial/io_ti.c| 17 +
drivers/usb/serial/mos7720.c | 12 +-
drivers/usb/serial/mos7840.c | 10 +
drivers/usb/serial/opticon.c | 12 +-
drivers/usb/serial/option.c | 2 -
drivers/usb/serial/pl2303.c | 10 +
drivers/usb/serial/quatech2.c | 13 ---
drivers/usb/serial/ssu100.c | 13 ---
drivers/usb/serial/ti_usb_3410_5052.c | 42 +
drivers/usb/serial/usb-serial.c | 53 ---
drivers/usb/serial/usb-wwan.h | 4 --
drivers/usb/serial/usb_wwan.c | 42 -
drivers/usb/serial/whiteheat.c| 12 +-
include/linux/usb/serial.h| 2 +-
19 files changed, 70 insertions(+), 226 deletions(-)
diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index 957cdd694b1f..c0cf60e9273d 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -385,17 +385,10 @@ static int ark3116_open(struct tty_struct *tty, struct
usb_serial_port *port)
return result;
}
-static int ark3116_get_serial_info(struct tty_struct *tty,
+static void ark3116_get_serial_info(struct tty_struct *tty,
struct serial_struct *ss)
{
- struct usb_serial_port *port = tty->driver_data;
-
ss->type = PORT_16654;
- ss->line = port->minor;
- ss->close_delay = 50;
- ss->closing_wait = 3000;
-
- return 0;
}
static int ark3116_tiocmget(struct tty_struct *tty)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index af0fe2a82eb2..5e34b364d94d 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -820,19 +820,13 @@ static int f81232_carrier_raised(struct usb_serial_port
*port)
return 0;
}
-static int f81232_get_serial_info(struct tty_struct *tty,
- struct serial_struct *ss)
+static void f81232_get_serial(struct tty_struct *tty, struct serial_struct *ss)
{
struct usb_serial_port *port = tty->driver_data;
struct f81232_private *priv = usb_get_serial_port_data(port);
ss->type = PORT_16550A;
- ss->line = port->minor;
ss->baud_base = priv->baud_base;
- ss->close_delay = 50;
- ss->closing_wait = 3000;
-
- return 0;
}
static void f81232_interrupt_work(struct work_struct *work)
@@ -1023,7 +1017,7 @@ static struct usb_serial_driver f81232_device = {
.close =f81232_close,
.dtr_rts = f81232_dtr_rts,
.carrier_raised = f81232_carrier_raised,
- .get_serial = f81232_get_serial_info,
+ .get_serial = f81232_get_serial,
.break_ctl =f81232_break_ctl,
.set_termios = f81232_set_termios,
.tiocmget = f81232_tiocmget,
@@ -1048,7 +1042,7 @@ static struct usb_serial_driver f81534a_device = {
.close =f81232_close,
.dtr_rts = f81232_dtr_rts,
.carrier_raised = f81232_carrier_raised,
- .get_serial = f81232_get_serial_info,
+ .get_serial = f81232_get_serial,
.break_ctl =f81232_break_ctl,
.set_termios = f81232_set_termios,
.tiocmget = f81232_tiocmget,
diff --git a/drivers/usb/serial/f81534.c b/drivers/usb/serial/f81534.c
index c9f90d437e3a..633de52feaad 100644
--- a/drivers/usb/serial/f81534.c
+++ b/drivers/usb/serial/
[PATCH 24/24] USB: serial: ftdi_sio: clean up TIOCSSERIAL
The TIOCSSERIAL implementation needs to compare the old flag and divisor
settings with the new to detect ASYNC_SPD changes, but there's no need
to copy all driver state to the stack for that.
While at it, unbreak the function parameter list.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ftdi_sio.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 9228e56a91c0..6f2659e59b2e 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1486,15 +1486,13 @@ static void get_serial_info(struct tty_struct *tty,
struct serial_struct *ss)
ss->custom_divisor = priv->custom_divisor;
}
-static int set_serial_info(struct tty_struct *tty,
- struct serial_struct *ss)
+static int set_serial_info(struct tty_struct *tty, struct serial_struct *ss)
{
struct usb_serial_port *port = tty->driver_data;
struct ftdi_private *priv = usb_get_serial_port_data(port);
- struct ftdi_private old_priv;
+ int old_flags, old_divisor;
mutex_lock(>cfg_lock);
- old_priv = *priv;
if (!capable(CAP_SYS_ADMIN)) {
if ((ss->flags ^ priv->flags) & ~ASYNC_USR_MASK) {
@@ -1503,14 +1501,17 @@ static int set_serial_info(struct tty_struct *tty,
}
}
+ old_flags = priv->flags;
+ old_divisor = priv->custom_divisor;
+
priv->flags = ss->flags & ASYNC_FLAGS;
priv->custom_divisor = ss->custom_divisor;
write_latency_timer(port);
- if ((priv->flags ^ old_priv.flags) & ASYNC_SPD_MASK ||
+ if ((priv->flags ^ old_flags) & ASYNC_SPD_MASK ||
((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST &&
-priv->custom_divisor != old_priv.custom_divisor)) {
+priv->custom_divisor != old_divisor)) {
/* warn about deprecation unless clearing */
if (priv->flags & ASYNC_SPD_MASK)
--
2.26.3
[PATCH 22/24] USB: serial: ftdi_sio: ignore baud_base changes
The TIOCSSERIAL error handling is inconsistent at best, but drivers tend
to ignore requests to change parameters which cannot be changed rather
than return an error.
The FTDI driver ignores change requests for all immutable parameters but
baud_base so return success also in this case for consistency.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ftdi_sio.c | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 16d3e50487e6..3fd7875200b9 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1509,10 +1509,6 @@ static int set_serial_info(struct tty_struct *tty,
goto check_and_exit;
}
- if (ss->baud_base != priv->baud_base) {
- mutex_unlock(>cfg_lock);
- return -EINVAL;
- }
/* Make the changes - these are privileged changes! */
--
2.26.3
[PATCH 23/24] USB: serial: ftdi_sio: simplify TIOCGSERIAL permission check
Changing the deprecated custom_divisor field is an unprivileged
operation so after verifying that flag field does not contain any
privileged changes both updates can be carried out by any user.
Combine the two branches and drop the erroneous comment.
Note that private flags field is only used for ASYNC flags so there's no
need to try to retain any other bits when updating the flags.
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ftdi_sio.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 3fd7875200b9..9228e56a91c0 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1496,27 +1496,16 @@ static int set_serial_info(struct tty_struct *tty,
mutex_lock(>cfg_lock);
old_priv = *priv;
- /* Do error checking and permission checking */
-
if (!capable(CAP_SYS_ADMIN)) {
if ((ss->flags ^ priv->flags) & ~ASYNC_USR_MASK) {
mutex_unlock(>cfg_lock);
return -EPERM;
}
- priv->flags = ((priv->flags & ~ASYNC_USR_MASK) |
- (ss->flags & ASYNC_USR_MASK));
- priv->custom_divisor = ss->custom_divisor;
- goto check_and_exit;
}
-
- /* Make the changes - these are privileged changes! */
-
- priv->flags = ((priv->flags & ~ASYNC_FLAGS) |
- (ss->flags & ASYNC_FLAGS));
+ priv->flags = ss->flags & ASYNC_FLAGS;
priv->custom_divisor = ss->custom_divisor;
-check_and_exit:
write_latency_timer(port);
if ((priv->flags ^ old_priv.flags) & ASYNC_SPD_MASK ||
--
2.26.3
[PATCH 11/24] USB: serial: quatech2: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The xmit_fifo_size parameter could be used to set the hardware transmit
fifo size of a legacy UART when it could not be detected, but the
interface is limited to eight bits and should be left unset when not
used.
Similarly, baud_base could be used to set the UART base clock when it
could not be detected but might as well be left unset when it is not
known.
The close_delay and closing_wait parameters returned by TIOCGSERIAL are
specified in centiseconds (not jiffies). The driver does not yet support
changing these, but let's report back the default values actually used
(0.5 and 30 seconds, respectively).
Fixes: f7a33e608d9a ("USB: serial: add quatech2 usb to serial driver")
Signed-off-by: Johan Hovold
---
drivers/usb/serial/quatech2.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/serial/quatech2.c b/drivers/usb/serial/quatech2.c
index 599dcb2e374d..0d23e565e0d2 100644
--- a/drivers/usb/serial/quatech2.c
+++ b/drivers/usb/serial/quatech2.c
@@ -459,12 +459,9 @@ static int get_serial_info(struct tty_struct *tty,
struct usb_serial_port *port = tty->driver_data;
ss->line= port->minor;
- ss->port= 0;
- ss->irq = 0;
- ss->xmit_fifo_size = port->bulk_out_size;
- ss->baud_base = 9600;
- ss->close_delay = 5*HZ;
- ss->closing_wait= 30*HZ;
+ ss->close_delay = 50;
+ ss->closing_wait= 3000;
+
return 0;
}
--
2.26.3
[PATCH 18/24] USB: serial: whiteheat: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters. The port parameter is used to set the I/O port and does not make any sense to use for USB serial devices. The xmit_fifo_size parameter could be used to set the hardware transmit fifo size of a legacy UART when it could not be detected, but the interface is limited to eight bits and should be left unset when not used. The close_delay and closing_wait parameters returned by TIOCGSERIAL are specified in centiseconds (not jiffies). The driver does not yet support changing these, but let's report back the default values actually used (0.5 and 30 seconds, respectively). Signed-off-by: Johan Hovold --- drivers/usb/serial/whiteheat.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/usb/serial/whiteheat.c b/drivers/usb/serial/whiteheat.c index ccfd5ed652cd..c8b10faa2ff8 100644 --- a/drivers/usb/serial/whiteheat.c +++ b/drivers/usb/serial/whiteheat.c @@ -170,7 +170,6 @@ static int firm_report_tx_done(struct usb_serial_port *port); #define COMMAND_PORT 4 #define COMMAND_TIMEOUT(2*HZ) /* 2 second timeout for a command */ #defineCOMMAND_TIMEOUT_MS 2000 -#define CLOSING_DELAY (30 * HZ) /* @@ -447,12 +446,9 @@ static int whiteheat_get_serial(struct tty_struct *tty, ss->type = PORT_16654; ss->line = port->minor; - ss->port = port->port_number; - ss->xmit_fifo_size = kfifo_size(>write_fifo); - ss->custom_divisor = 0; ss->baud_base = 460800; - ss->close_delay = CLOSING_DELAY; - ss->closing_wait = CLOSING_DELAY; + ss->close_delay = 50; + ss->closing_wait = 3000; return 0; } -- 2.26.3
[PATCH 15/24] USB: serial: usb_wwan: fix TIOCSSERIAL jiffies conversions
The port close_delay and closing_wait parameters set by TIOCSSERIAL are
specified in jiffies and not milliseconds.
Add the missing conversions so that the TIOCSSERIAL works as expected
also when HZ is not 1000.
Fixes: 02303f73373a ("usb-wwan: implement TIOCGSERIAL and TIOCSSERIAL to avoid
blocking close(2)")
Cc: sta...@vger.kernel.org # 2.6.38
Signed-off-by: Johan Hovold
---
drivers/usb/serial/usb_wwan.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
index 46d46a4f99c9..4e9c994a972a 100644
--- a/drivers/usb/serial/usb_wwan.c
+++ b/drivers/usb/serial/usb_wwan.c
@@ -140,10 +140,10 @@ int usb_wwan_get_serial_info(struct tty_struct *tty,
ss->line= port->minor;
ss->port= port->port_number;
ss->baud_base = tty_get_baud_rate(port->port.tty);
- ss->close_delay = port->port.close_delay / 10;
+ ss->close_delay = jiffies_to_msecs(port->port.close_delay) / 10;
ss->closing_wait= port->port.closing_wait ==
ASYNC_CLOSING_WAIT_NONE ?
ASYNC_CLOSING_WAIT_NONE :
-port->port.closing_wait / 10;
+jiffies_to_msecs(port->port.closing_wait) / 10;
return 0;
}
EXPORT_SYMBOL(usb_wwan_get_serial_info);
@@ -155,9 +155,10 @@ int usb_wwan_set_serial_info(struct tty_struct *tty,
unsigned int closing_wait, close_delay;
int retval = 0;
- close_delay = ss->close_delay * 10;
+ close_delay = msecs_to_jiffies(ss->close_delay * 10);
closing_wait = ss->closing_wait == ASYNC_CLOSING_WAIT_NONE ?
- ASYNC_CLOSING_WAIT_NONE : ss->closing_wait * 10;
+ ASYNC_CLOSING_WAIT_NONE :
+ msecs_to_jiffies(ss->closing_wait * 10);
mutex_lock(>port.mutex);
--
2.26.3
[PATCH 16/24] USB: serial: usb_wwan: fix unprivileged TIOCCSERIAL
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
A non-privileged user has only ever been able to set the since long
deprecated ASYNC_SPD flags and trying to change any other *supported*
feature should result in -EPERM being returned. Setting the current
values for any supported features should return success.
Fix the usb_wwan implementation which instead indicated that the
TIOCSSERIAL ioctl was not even implemented when a non-privileged user
set the current values.
Fixes: 02303f73373a ("usb-wwan: implement TIOCGSERIAL and TIOCSSERIAL to avoid
blocking close(2)")
Signed-off-by: Johan Hovold
---
drivers/usb/serial/usb_wwan.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
index 4e9c994a972a..e71c828682f5 100644
--- a/drivers/usb/serial/usb_wwan.c
+++ b/drivers/usb/serial/usb_wwan.c
@@ -166,8 +166,6 @@ int usb_wwan_set_serial_info(struct tty_struct *tty,
if ((close_delay != port->port.close_delay) ||
(closing_wait != port->port.closing_wait))
retval = -EPERM;
- else
- retval = -EOPNOTSUPP;
} else {
port->port.close_delay = close_delay;
port->port.closing_wait = closing_wait;
--
2.26.3
[PATCH 17/24] USB: serial: usb_wwan: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The port parameter is used to set the I/O port and does not make any
sense to use for USB serial devices.
The baud_base parameter could be used to set the UART base clock when it
could not be detected but might as well be left unset when it is not
known.
Fix the usb_wwan TIOCGSERIAL implementation by dropping its custom
interpretation of the unused port and baud_base fields, which were set
to the port index and current line speed, respectively.
Fixes: 02303f73373a ("usb-wwan: implement TIOCGSERIAL and TIOCSSERIAL to avoid
blocking close(2)")
Signed-off-by: Johan Hovold
---
drivers/usb/serial/usb_wwan.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
index e71c828682f5..4ea315e5e69b 100644
--- a/drivers/usb/serial/usb_wwan.c
+++ b/drivers/usb/serial/usb_wwan.c
@@ -138,8 +138,6 @@ int usb_wwan_get_serial_info(struct tty_struct *tty,
struct usb_serial_port *port = tty->driver_data;
ss->line= port->minor;
- ss->port= port->port_number;
- ss->baud_base = tty_get_baud_rate(port->port.tty);
ss->close_delay = jiffies_to_msecs(port->port.close_delay) / 10;
ss->closing_wait= port->port.closing_wait ==
ASYNC_CLOSING_WAIT_NONE ?
ASYNC_CLOSING_WAIT_NONE :
--
2.26.3
[PATCH 12/24] USB: serial: ssu100: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The xmit_fifo_size parameter could be used to set the hardware transmit
fifo size of a legacy UART when it could not be detected, but the
interface is limited to eight bits and should be left unset when not
used.
Similarly, baud_base could be used to set the UART base clock when it
could not be detected but might as well be left unset when it is not
known.
The close_delay and closing_wait parameters returned by TIOCGSERIAL are
specified in centiseconds (not jiffies). The driver does not yet support
changing these, but let's report back the default values actually used
(0.5 and 30 seconds, respectively).
Fixes: 52af95459939 ("USB: add USB serial ssu100 driver")
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ssu100.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/serial/ssu100.c b/drivers/usb/serial/ssu100.c
index 89fdc5c19285..c4616c37f33f 100644
--- a/drivers/usb/serial/ssu100.c
+++ b/drivers/usb/serial/ssu100.c
@@ -337,12 +337,9 @@ static int get_serial_info(struct tty_struct *tty,
struct usb_serial_port *port = tty->driver_data;
ss->line= port->minor;
- ss->port= 0;
- ss->irq = 0;
- ss->xmit_fifo_size = port->bulk_out_size;
- ss->baud_base = 9600;
- ss->close_delay = 5*HZ;
- ss->closing_wait= 30*HZ;
+ ss->close_delay = 50;
+ ss->closing_wait= 3000;
+
return 0;
}
--
2.26.3
[PATCH 14/24] USB: serial: ti_usb_3410_5052: fix TIOCSSERIAL permission check
Changing the port closing-wait parameter is a privileged operation so
make sure to return -EPERM if a regular user tries to change it.
Cc: sta...@vger.kernel.org
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ti_usb_3410_5052.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/ti_usb_3410_5052.c
b/drivers/usb/serial/ti_usb_3410_5052.c
index 4b497c1e850b..bb50098a0ce6 100644
--- a/drivers/usb/serial/ti_usb_3410_5052.c
+++ b/drivers/usb/serial/ti_usb_3410_5052.c
@@ -1418,14 +1418,19 @@ static int ti_set_serial_info(struct tty_struct *tty,
struct serial_struct *ss)
{
struct usb_serial_port *port = tty->driver_data;
- struct ti_port *tport = usb_get_serial_port_data(port);
+ struct tty_port *tport = >port;
unsigned cwait;
cwait = ss->closing_wait;
if (cwait != ASYNC_CLOSING_WAIT_NONE)
cwait = msecs_to_jiffies(10 * ss->closing_wait);
- tport->tp_port->port.closing_wait = cwait;
+ if (!capable(CAP_SYS_ADMIN)) {
+ if (cwait != tport->closing_wait)
+ return -EPERM;
+ }
+
+ tport->closing_wait = cwait;
return 0;
}
--
2.26.3
[PATCH 13/24] USB: serial: ti_usb_3410_5052: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters. The port parameter is used to set the I/O port and does not make any sense to use for USB serial devices. The xmit_fifo_size parameter could be used to set the hardware transmit fifo size of a legacy UART when it could not be detected, but the interface is limited to eight bits and should be left unset when not used. The close_delay and closing_wait parameters returned by TIOCGSERIAL are specified in centiseconds. The driver does not yet support changing close_delay, but let's report back the default value actually used (0.5 seconds). Signed-off-by: Johan Hovold --- drivers/usb/serial/ti_usb_3410_5052.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/ti_usb_3410_5052.c b/drivers/usb/serial/ti_usb_3410_5052.c index 7252b0ce75a6..4b497c1e850b 100644 --- a/drivers/usb/serial/ti_usb_3410_5052.c +++ b/drivers/usb/serial/ti_usb_3410_5052.c @@ -1406,10 +1406,10 @@ static int ti_get_serial_info(struct tty_struct *tty, ss->type = PORT_16550A; ss->line = port->minor; - ss->port = port->port_number; - ss->xmit_fifo_size = kfifo_size(>write_fifo); ss->baud_base = tport->tp_tdev->td_is_3410 ? 921600 : 460800; + ss->close_delay = 50; ss->closing_wait = cwait; + return 0; } -- 2.26.3
[PATCH 08/24] USB: serial: mos7840: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The port parameter is used to set the I/O port and does not make any
sense to use for USB serial devices.
The xmit_fifo_size parameter could be used to set the hardware transmit
fifo size of a legacy UART when it could not be detected, but the
interface is limited to eight bits and should be left unset when not
used.
Similarly, baud_base could be used to set the UART base clock when it
could not be detected but might as well be left unset when it is not
known.
The close_delay and closing_wait parameters returned by TIOCGSERIAL are
specified in centiseconds (not jiffies). The driver does not yet support
changing these, but let's report back the default values actually used
(0.5 and 30 seconds, respectively).
Fixes: 3f5429746d91 ("USB: Moschip 7840 USB-Serial Driver")
Signed-off-by: Johan Hovold
---
drivers/usb/serial/mos7840.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 1bf0d066f55a..77cbe18a1629 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -1392,16 +1392,12 @@ static int mos7840_get_serial_info(struct tty_struct
*tty,
struct serial_struct *ss)
{
struct usb_serial_port *port = tty->driver_data;
- struct moschip_port *mos7840_port = usb_get_serial_port_data(port);
ss->type = PORT_16550A;
- ss->line = mos7840_port->port->minor;
- ss->port = mos7840_port->port->port_number;
- ss->irq = 0;
- ss->xmit_fifo_size = NUM_URBS * URB_TRANSFER_BUFFER_SIZE;
- ss->baud_base = 9600;
- ss->close_delay = 5 * HZ;
- ss->closing_wait = 30 * HZ;
+ ss->line = port->minor;
+ ss->close_delay = 50;
+ ss->closing_wait = 3000;
+
return 0;
}
--
2.26.3
[PATCH 04/24] USB: serial: ftdi_sio: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters. The FTDI driver is the only USB serial driver supporting the deprecated ASYNC_SPD flags, which are reported back as they should by TIOCGSERIAL, but the returned parameters did not include the line number. The close_delay and closing_wait parameters returned by TIOCGSERIAL are specified in centiseconds. The driver does not yet support changing these, but let's report back the default values actually used (0.5 and 30 seconds, respectively). Signed-off-by: Johan Hovold --- drivers/usb/serial/ftdi_sio.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c index c867592477c9..f8a0911f90ea 100644 --- a/drivers/usb/serial/ftdi_sio.c +++ b/drivers/usb/serial/ftdi_sio.c @@ -1483,9 +1483,13 @@ static int get_serial_info(struct tty_struct *tty, struct usb_serial_port *port = tty->driver_data; struct ftdi_private *priv = usb_get_serial_port_data(port); + ss->line = port->minor; ss->flags = priv->flags; ss->baud_base = priv->baud_base; ss->custom_divisor = priv->custom_divisor; + ss->close_delay = 50; + ss->closing_wait = 3000; + return 0; } -- 2.26.3
[PATCH 09/24] USB: serial: opticon: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The xmit_fifo_size parameter could be used to set the hardware transmit
fifo size of a legacy UART when it could not be detected, but the
interface is limited to eight bits and should be left unset when not
used.
Similarly, baud_base could be used to set the UART base clock when it
could not be detected but might as well be left unset when it is not
known.
The close_delay and closing_wait parameters returned by TIOCGSERIAL are
specified in centiseconds (not jiffies). The driver does not yet support
changing these, but let's report back the default values actually used
(0.5 and 30 seconds, respectively).
Fixes: faac64ad9c7b ("USB: serial: opticon: add serial line ioctls")
Signed-off-by: Johan Hovold
---
drivers/usb/serial/opticon.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/serial/opticon.c b/drivers/usb/serial/opticon.c
index eecb72aef83e..1c7e5dc2c272 100644
--- a/drivers/usb/serial/opticon.c
+++ b/drivers/usb/serial/opticon.c
@@ -360,12 +360,9 @@ static int get_serial_info(struct tty_struct *tty,
/* fake emulate a 16550 uart to make userspace code happy */
ss->type= PORT_16550A;
ss->line= port->minor;
- ss->port= 0;
- ss->irq = 0;
- ss->xmit_fifo_size = 1024;
- ss->baud_base = 9600;
- ss->close_delay = 5*HZ;
- ss->closing_wait= 30*HZ;
+ ss->close_delay = 50;
+ ss->closing_wait= 3000;
+
return 0;
}
--
2.26.3
[PATCH 03/24] USB: serial: f81534: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The port parameter is used to set the I/O port and does not make any
sense to use for USB serial devices.
The close_delay and closing_wait parameters returned by TIOCGSERIAL are
specified in centiseconds. The driver does not yet support changing
these, but let's report back the default values actually used (0.5 and
30 seconds, respectively).
Fixes: aac1fc386fa1 ("USB: serial: add Fintek F81232 usb to serial driver")
Signed-off-by: Johan Hovold
---
drivers/usb/serial/f81534.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/serial/f81534.c b/drivers/usb/serial/f81534.c
index a763b362f081..c9f90d437e3a 100644
--- a/drivers/usb/serial/f81534.c
+++ b/drivers/usb/serial/f81534.c
@@ -1149,9 +1149,11 @@ static int f81534_get_serial_info(struct tty_struct *tty,
port_priv = usb_get_serial_port_data(port);
ss->type = PORT_16550A;
- ss->port = port->port_number;
ss->line = port->minor;
ss->baud_base = port_priv->baud_base;
+ ss->close_delay = 50;
+ ss->closing_wait = 3000;
+
return 0;
}
--
2.26.3
[PATCH 10/24] USB: serial: pl2303: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters. The port parameter is used to set the I/O port and does not make any sense to use for USB serial devices. The baud_base parameter could be used to set the UART base clock when it could not be detected but might as well be left unset when it is not known. The close_delay and closing_wait parameters returned by TIOCGSERIAL are specified in centiseconds. The driver does not yet support changing these, but let's report back the default values actually used (0.5 and 30 seconds, respectively). Signed-off-by: Johan Hovold --- drivers/usb/serial/pl2303.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c index eed9acd1ae08..0455add8593a 100644 --- a/drivers/usb/serial/pl2303.c +++ b/drivers/usb/serial/pl2303.c @@ -946,8 +946,9 @@ static int pl2303_get_serial(struct tty_struct *tty, ss->type = PORT_16654; ss->line = port->minor; - ss->port = port->port_number; - ss->baud_base = 460800; + ss->close_delay = 50; + ss->closing_wait = 3000; + return 0; } -- 2.26.3
[PATCH 05/24] USB: serial: io_edgeport: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The port parameter is used to set the I/O port and does not make any
sense to use for USB serial devices.
The xmit_fifo_size parameter could be used to set the hardware transmit
fifo size of a legacy UART when it could not be detected, but the
interface is limited to eight bits and should be left unset when not
used.
Similarly, baud_base could be used to set the uart base clock when it
could not be detected, but might as well be left unset when it is not
known.
The close_delay and closing_wait parameters returned by TIOCGSERIAL are
specified in centiseconds (not jiffies). The driver does not yet support
changing these, but let's report back the default values actually used
(0.5 and 30 seconds, respectively).
Signed-off-by: Johan Hovold
---
drivers/usb/serial/io_edgeport.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c
index 68401adcffde..471a1a04c9c3 100644
--- a/drivers/usb/serial/io_edgeport.c
+++ b/drivers/usb/serial/io_edgeport.c
@@ -1641,16 +1641,12 @@ static int get_serial_info(struct tty_struct *tty,
struct serial_struct *ss)
{
struct usb_serial_port *port = tty->driver_data;
- struct edgeport_port *edge_port = usb_get_serial_port_data(port);
ss->type= PORT_16550A;
- ss->line= edge_port->port->minor;
- ss->port= edge_port->port->port_number;
- ss->irq = 0;
- ss->xmit_fifo_size = edge_port->maxTxCredits;
- ss->baud_base = 9600;
- ss->close_delay = 5*HZ;
- ss->closing_wait= 30*HZ;
+ ss->line= port->minor;
+ ss->close_delay = 50;
+ ss->closing_wait= 3000;
+
return 0;
}
--
2.26.3
[PATCH 01/24] USB: serial: ark3116: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The port parameter is used to set the I/O port and does not make any
sense to use for USB serial devices.
The baud_base parameter could be used to set the UART base clock when it
could not be detected but might as well be left unset when it is not
known.
The close_delay and closing_wait parameters returned by TIOCGSERIAL are
specified in centiseconds. The driver does not yet support changing
these, but let's report back the default values actually used (0.5 and
30 seconds, respectively).
Fixes: 2f430b4bbae7 ("USB: ark3116: Add TIOCGSERIAL and TIOCSSERIAL ioctl
calls.")
Signed-off-by: Johan Hovold
---
drivers/usb/serial/ark3116.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index b9bedfe9bd09..957cdd694b1f 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -392,8 +392,9 @@ static int ark3116_get_serial_info(struct tty_struct *tty,
ss->type = PORT_16654;
ss->line = port->minor;
- ss->port = port->port_number;
- ss->baud_base = 460800;
+ ss->close_delay = 50;
+ ss->closing_wait = 3000;
+
return 0;
}
--
2.26.3
[PATCH 06/24] USB: serial: io_ti: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The port parameter is used to set the I/O port and does not make any
sense to use for USB serial devices.
The xmit_fifo_size parameter could be used to set the hardware transmit
fifo size of a legacy UART when it could not be detected, but the
interface is limited to eight bits and should be left unset when not
used.
Similarly, baud_base could be used to set the UART base clock when it
could not be detected but might as well be left unset when it is not
known.
The close_delay and closing_wait parameters returned by TIOCGSERIAL are
specified in centiseconds (not jiffies). The driver does not yet support
changing close_delay, but let's report back the default value actually
used (0.5 seconds).
Signed-off-by: Johan Hovold
---
drivers/usb/serial/io_ti.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index e800547be9e0..f5aab570fd05 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -2437,21 +2437,17 @@ static int get_serial_info(struct tty_struct *tty,
struct serial_struct *ss)
{
struct usb_serial_port *port = tty->driver_data;
- struct edgeport_port *edge_port = usb_get_serial_port_data(port);
unsigned cwait;
- cwait = edge_port->port->port.closing_wait;
+ cwait = port->port.closing_wait;
if (cwait != ASYNC_CLOSING_WAIT_NONE)
cwait = jiffies_to_msecs(cwait) / 10;
ss->type= PORT_16550A;
- ss->line= edge_port->port->minor;
- ss->port= edge_port->port->port_number;
- ss->irq = 0;
- ss->xmit_fifo_size = edge_port->port->bulk_out_size;
- ss->baud_base = 9600;
- ss->close_delay = 5*HZ;
+ ss->line= port->minor;
+ ss->close_delay = 50;
ss->closing_wait= cwait;
+
return 0;
}
--
2.26.3
[PATCH 02/24] USB: serial: f81232: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The port parameter is used to set the I/O port and does not make any
sense to use for USB serial devices.
The close_delay and closing_wait parameters returned by TIOCGSERIAL are
specified in centiseconds. The driver does not yet support changing
these, but let's report back the default values actually used (0.5 and
30 seconds, respectively).
Fixes: aac1fc386fa1 ("USB: serial: add Fintek F81232 usb to serial driver")
Signed-off-by: Johan Hovold
---
drivers/usb/serial/f81232.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 6a8f39147d8e..af0fe2a82eb2 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -828,8 +828,10 @@ static int f81232_get_serial_info(struct tty_struct *tty,
ss->type = PORT_16550A;
ss->line = port->minor;
- ss->port = port->port_number;
ss->baud_base = priv->baud_base;
+ ss->close_delay = 50;
+ ss->closing_wait = 3000;
+
return 0;
}
--
2.26.3
[PATCH 00/24] USB: serial: TIOCSSERIAL fixes and generic support
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters. This series fixes up the various USB serial driver implementations, before adding generic support to core for the benefit of all USB serial drivers. Johan Johan Hovold (24): USB: serial: ark3116: fix TIOCGSERIAL implementation USB: serial: f81232: fix TIOCGSERIAL implementation USB: serial: f81534: fix TIOCGSERIAL implementation USB: serial: ftdi_sio: fix TIOCGSERIAL implementation USB: serial: io_edgeport: fix TIOCGSERIAL implementation USB: serial: io_ti: fix TIOCGSERIAL implementation USB: serial: mos7720: fix TIOCGSERIAL implementation USB: serial: mos7840: fix TIOCGSERIAL implementation USB: serial: opticon: fix TIOCGSERIAL implementation USB: serial: pl2303: fix TIOCGSERIAL implementation USB: serial: quatech2: fix TIOCGSERIAL implementation USB: serial: ssu100: fix TIOCGSERIAL implementation USB: serial: ti_usb_3410_5052: fix TIOCGSERIAL implementation USB: serial: ti_usb_3410_5052: fix TIOCSSERIAL permission check USB: serial: usb_wwan: fix TIOCSSERIAL jiffies conversions USB: serial: usb_wwan: fix unprivileged TIOCCSERIAL USB: serial: usb_wwan: fix TIOCGSERIAL implementation USB: serial: whiteheat: fix TIOCGSERIAL implementation USB: serial: fix return value for unsupported ioctls USB: serial: add generic support for TIOCSSERIAL USB: serial: stop reporting legacy UART types USB: serial: ftdi_sio: ignore baud_base changes USB: serial: ftdi_sio: simplify TIOCGSERIAL permission check USB: serial: ftdi_sio: clean up TIOCSSERIAL drivers/usb/serial/ark3116.c | 13 -- drivers/usb/serial/f81232.c | 11 ++--- drivers/usb/serial/f81534.c | 7 +--- drivers/usb/serial/ftdi_sio.c | 35 drivers/usb/serial/io_edgeport.c | 22 -- drivers/usb/serial/io_ti.c| 24 --- drivers/usb/serial/mos7720.c | 18 drivers/usb/serial/mos7840.c | 23 --- drivers/usb/serial/opticon.c | 18 drivers/usb/serial/option.c | 2 - drivers/usb/serial/pl2303.c | 13 -- drivers/usb/serial/quatech2.c | 16 drivers/usb/serial/ssu100.c | 16 drivers/usb/serial/ti_usb_3410_5052.c | 38 + drivers/usb/serial/usb-serial.c | 59 +++ drivers/usb/serial/usb-wwan.h | 4 -- drivers/usb/serial/usb_wwan.c | 45 drivers/usb/serial/whiteheat.c| 17 +--- include/linux/usb/serial.h| 2 +- 19 files changed, 69 insertions(+), 314 deletions(-) -- 2.26.3
[PATCH 07/24] USB: serial: mos7720: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The port parameter is used to set the I/O port and does not make any
sense to use for USB serial devices.
The xmit_fifo_size parameter could be used to set the hardware transmit
fifo size of a legacy UART when it could not be detected, but the
interface is limited to eight bits and should be left unset when not
used.
Similarly, baud_base could be used to set the UART base clock when it
could not be detected but might as well be left unset when it is not
known.
The close_delay and closing_wait parameters returned by TIOCGSERIAL are
specified in centiseconds (not jiffies). The driver does not yet support
changing these, but let's report back the default values actually used
(0.5 and 30 seconds, respectively).
Fixes: 0f64478cbc7a ("USB: add USB serial mos7720 driver")
Signed-off-by: Johan Hovold
---
drivers/usb/serial/mos7720.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
index bb3d39307d93..db90ce560d42 100644
--- a/drivers/usb/serial/mos7720.c
+++ b/drivers/usb/serial/mos7720.c
@@ -1638,16 +1638,12 @@ static int get_serial_info(struct tty_struct *tty,
struct serial_struct *ss)
{
struct usb_serial_port *port = tty->driver_data;
- struct moschip_port *mos7720_port = usb_get_serial_port_data(port);
ss->type= PORT_16550A;
- ss->line= mos7720_port->port->minor;
- ss->port= mos7720_port->port->port_number;
- ss->irq = 0;
- ss->xmit_fifo_size = NUM_URBS * URB_TRANSFER_BUFFER_SIZE;
- ss->baud_base = 9600;
- ss->close_delay = 5*HZ;
- ss->closing_wait= 30*HZ;
+ ss->line= port->minor;
+ ss->close_delay = 50;
+ ss->closing_wait= 3000;
+
return 0;
}
--
2.26.3
[PATCH 0/3] USB: cdc-acm: TIOCSSERIAL fixes
This series fixes up a few issues with cdc-acm TIOCSSERIAL implementation. Johan Johan Hovold (3): Revert "USB: cdc-acm: fix rounding error in TIOCSSERIAL" USB: cdc-acm: fix unprivileged TIOCCSERIAL USB: cdc-acm: fix TIOCGSERIAL implementation drivers/usb/class/cdc-acm.c | 16 +++- 1 file changed, 3 insertions(+), 13 deletions(-) -- 2.26.3
[PATCH 2/3] USB: cdc-acm: fix unprivileged TIOCCSERIAL
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
A non-privileged user has only ever been able to set the since long
deprecated ASYNC_SPD flags and trying to change any other *supported*
feature should result in -EPERM being returned. Setting the current
values for any supported features should return success.
Fix the cdc-acm implementation which instead indicated that the
TIOCSSERIAL ioctl was not even implemented when a non-privileged user
set the current values.
Fixes: ba2d8ce9db0a ("cdc-acm: implement TIOCSSERIAL to avoid blocking
close(2)")
Signed-off-by: Johan Hovold
---
drivers/usb/class/cdc-acm.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 96e221803fa6..43e31dad4831 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -955,8 +955,6 @@ static int set_serial_info(struct tty_struct *tty, struct
serial_struct *ss)
if ((close_delay != acm->port.close_delay) ||
(closing_wait != acm->port.closing_wait))
retval = -EPERM;
- else
- retval = -EOPNOTSUPP;
} else {
acm->port.close_delay = close_delay;
acm->port.closing_wait = closing_wait;
--
2.26.3
[PATCH 1/3] Revert "USB: cdc-acm: fix rounding error in TIOCSSERIAL"
This reverts commit b401f8c4f492cbf74f3f59c9141e5be3071071bb.
The offending commit claimed that trying to set the values reported back
by TIOCGSERIAL as a regular user could result in an -EPERM error when HZ
is 250, but that was never the case.
With HZ=250, the default 0.5 second value of close_delay is converted to
125 jiffies when set and is converted back to 50 centiseconds by
TIOCGSERIAL as expected (not 12 cs as was claimed).
Comparing the internal current and new jiffies values is just fine to
determine if the value is about to change so drop the bogus workaround
(which was also backported to stable).
For completeness: With different default values for these parameters or
with a HZ value not divisible by two, the lack of rounding when setting
the default values in tty_port_init() could result in an -EPERM being
returned, but this is hardly something we need to worry about.
Cc: Anthony Mallet
Cc: sta...@vger.kernel.org
Signed-off-by: Johan Hovold
---
drivers/usb/class/cdc-acm.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 3fda1ec961d7..96e221803fa6 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -942,7 +942,6 @@ static int set_serial_info(struct tty_struct *tty, struct
serial_struct *ss)
{
struct acm *acm = tty->driver_data;
unsigned int closing_wait, close_delay;
- unsigned int old_closing_wait, old_close_delay;
int retval = 0;
close_delay = msecs_to_jiffies(ss->close_delay * 10);
@@ -950,17 +949,11 @@ static int set_serial_info(struct tty_struct *tty, struct
serial_struct *ss)
ASYNC_CLOSING_WAIT_NONE :
msecs_to_jiffies(ss->closing_wait * 10);
- /* we must redo the rounding here, so that the values match */
- old_close_delay = jiffies_to_msecs(acm->port.close_delay) / 10;
- old_closing_wait = acm->port.closing_wait == ASYNC_CLOSING_WAIT_NONE ?
- ASYNC_CLOSING_WAIT_NONE :
- jiffies_to_msecs(acm->port.closing_wait) / 10;
-
mutex_lock(>port.mutex);
if (!capable(CAP_SYS_ADMIN)) {
- if ((ss->close_delay != old_close_delay) ||
- (ss->closing_wait != old_closing_wait))
+ if ((close_delay != acm->port.close_delay) ||
+ (closing_wait != acm->port.closing_wait))
retval = -EPERM;
else
retval = -EOPNOTSUPP;
--
2.26.3
[PATCH 3/3] USB: cdc-acm: fix TIOCGSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The xmit_fifo_size parameter could be used to set the hardware transmit
fifo size of a legacy UART when it could not be detected, but the
interface is limited to eight bits and should be left unset when it is
not used.
Similarly, baud_base could be used to set the uart base clock when it
could not be detected, but might as well be left unset when it is not
known.
Fix the cdc-acm TIOCGSERIAL implementation by dropping its custom
interpretation of the unused xmit_fifo_size and baud_base fields, which
overflowed the former with the URB buffer size and set the latter to the
current line speed. Also return the port line number, which is the only
other value used besides the close parameters.
Fixes: 18c75720e667 ("USB: allow users to run setserial with cdc-acm")
Signed-off-by: Johan Hovold
---
drivers/usb/class/cdc-acm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 43e31dad4831..b74713518b3a 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -929,8 +929,7 @@ static int get_serial_info(struct tty_struct *tty, struct
serial_struct *ss)
{
struct acm *acm = tty->driver_data;
- ss->xmit_fifo_size = acm->writesize;
- ss->baud_base = le32_to_cpu(acm->line.dwDTERate);
+ ss->line = acm->minor;
ss->close_delay = jiffies_to_msecs(acm->port.close_delay) / 10;
ss->closing_wait = acm->port.closing_wait == ASYNC_CLOSING_WAIT_NONE ?
ASYNC_CLOSING_WAIT_NONE :
--
2.26.3
[PATCH 16/16] tty: synclink_gt: drop redundant tty-port initialisation
The port close_delay and closing_wait parameters have already been by tty_port_init() so drop the redundant driver initialisation to the default values. Signed-off-by: Johan Hovold --- drivers/tty/synclink_gt.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/tty/synclink_gt.c b/drivers/tty/synclink_gt.c index 1db908f62fde..994618618466 100644 --- a/drivers/tty/synclink_gt.c +++ b/drivers/tty/synclink_gt.c @@ -3511,8 +3511,6 @@ static struct slgt_info *alloc_dev(int adapter_num, int port_num, struct pci_dev info->max_frame_size = 4096; info->base_clock = 14745600; info->rbuf_fill_level = DMABUFSIZE; - info->port.close_delay = 5*HZ/10; - info->port.closing_wait = 30*HZ; init_waitqueue_head(>status_event_wait_q); init_waitqueue_head(>event_wait_q); spin_lock_init(>netlock); -- 2.26.3
[PATCH 15/16] pcmcia: synclink_cs: drop redundant tty-port initialisation
The port close_delay and closing_wait parameters have already been by tty_port_init() so drop the redundant driver initialisation to the default values. Signed-off-by: Johan Hovold --- drivers/char/pcmcia/synclink_cs.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/char/pcmcia/synclink_cs.c b/drivers/char/pcmcia/synclink_cs.c index 2be8d9a8eec5..3287a7627ed0 100644 --- a/drivers/char/pcmcia/synclink_cs.c +++ b/drivers/char/pcmcia/synclink_cs.c @@ -530,8 +530,6 @@ static int mgslpc_probe(struct pcmcia_device *link) info->port.ops = _port_ops; INIT_WORK(>task, bh_handler); info->max_frame_size = 4096; - info->port.close_delay = 5*HZ/10; - info->port.closing_wait = 30*HZ; init_waitqueue_head(>status_event_wait_q); init_waitqueue_head(>event_wait_q); spin_lock_init(>lock); -- 2.26.3
[PATCH 14/16] tty: mxser: fix TIOCSSERIAL permission check
Changing the port type and closing_wait parameter are privileged
operations so make sure to return -EPERM if a regular user tries to
change them.
Note that the closing_wait parameter would not actually have been
changed but the return value did not indicate that.
Cc: sta...@vger.kernel.org
Signed-off-by: Johan Hovold
---
drivers/tty/mxser.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index 914b23071961..2d8e76263a25 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -1270,6 +1270,7 @@ static int mxser_set_serial_info(struct tty_struct *tty,
if (!capable(CAP_SYS_ADMIN)) {
if ((ss->baud_base != info->baud_base) ||
(close_delay != info->port.close_delay) ||
+ (closing_wait != info->port.closing_wait) ||
((ss->flags & ~ASYNC_USR_MASK) !=
(info->port.flags & ~ASYNC_USR_MASK))) {
mutex_unlock(>mutex);
return -EPERM;
@@ -1296,11 +1297,11 @@ static int mxser_set_serial_info(struct tty_struct *tty,
baud = ss->baud_base / ss->custom_divisor;
tty_encode_baud_rate(tty, baud, baud);
}
- }
- info->type = ss->type;
+ info->type = ss->type;
- process_txrx_fifo(info);
+ process_txrx_fifo(info);
+ }
if (tty_port_initialized(port)) {
if (flags != (port->flags & ASYNC_SPD_MASK)) {
--
2.26.3
[PATCH 13/16] tty: mxser: fix TIOCSSERIAL jiffies conversions
The port close_delay and closing wait parameters set by TIOCSSERIAL are
specified in jiffies, while the values returned by TIOCGSERIAL are
specified in centiseconds.
Add the missing conversions so that TIOCSSERIAL works as expected also
when HZ is not 100.
Cc: sta...@vger.kernel.org
Signed-off-by: Johan Hovold
---
drivers/tty/mxser.c | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index 4203b64bccdb..914b23071961 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -1208,19 +1208,26 @@ static int mxser_get_serial_info(struct tty_struct *tty,
{
struct mxser_port *info = tty->driver_data;
struct tty_port *port = >port;
+ unsigned int closing_wait, close_delay;
if (tty->index == MXSER_PORTS)
return -ENOTTY;
mutex_lock(>mutex);
+
+ close_delay = jiffies_to_msecs(info->port.close_delay) / 10;
+ closing_wait = info->port.closing_wait;
+ if (closing_wait != ASYNC_CLOSING_WAIT_NONE)
+ closing_wait = jiffies_to_msecs(closing_wait) / 10;
+
ss->type = info->type,
ss->line = tty->index,
ss->port = info->ioaddr,
ss->irq = info->board->irq,
ss->flags = info->port.flags,
ss->baud_base = info->baud_base,
- ss->close_delay = info->port.close_delay,
- ss->closing_wait = info->port.closing_wait,
+ ss->close_delay = close_delay;
+ ss->closing_wait = closing_wait;
ss->custom_divisor = info->custom_divisor,
mutex_unlock(>mutex);
return 0;
@@ -1233,7 +1240,7 @@ static int mxser_set_serial_info(struct tty_struct *tty,
struct tty_port *port = >port;
speed_t baud;
unsigned long sl_flags;
- unsigned int flags;
+ unsigned int flags, close_delay, closing_wait;
int retval = 0;
if (tty->index == MXSER_PORTS)
@@ -1255,9 +1262,14 @@ static int mxser_set_serial_info(struct tty_struct *tty,
flags = port->flags & ASYNC_SPD_MASK;
+ close_delay = msecs_to_jiffies(ss->close_delay * 10);
+ closing_wait = ss->closing_wait;
+ if (closing_wait != ASYNC_CLOSING_WAIT_NONE)
+ closing_wait = msecs_to_jiffies(closing_wait * 10);
+
if (!capable(CAP_SYS_ADMIN)) {
if ((ss->baud_base != info->baud_base) ||
- (ss->close_delay != info->port.close_delay) ||
+ (close_delay != info->port.close_delay) ||
((ss->flags & ~ASYNC_USR_MASK) !=
(info->port.flags & ~ASYNC_USR_MASK))) {
mutex_unlock(>mutex);
return -EPERM;
@@ -1271,8 +1283,8 @@ static int mxser_set_serial_info(struct tty_struct *tty,
*/
port->flags = ((port->flags & ~ASYNC_FLAGS) |
(ss->flags & ASYNC_FLAGS));
- port->close_delay = ss->close_delay * HZ / 100;
- port->closing_wait = ss->closing_wait * HZ / 100;
+ port->close_delay = close_delay;
+ port->closing_wait = closing_wait;
if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST &&
(ss->baud_base != info->baud_base ||
ss->custom_divisor !=
--
2.26.3
[PATCH 11/16] tty: moxa: fix TIOCSSERIAL permission check
Changing the port close delay or type are privileged operations so make
sure to return -EPERM if a regular user tries to change them.
Cc: sta...@vger.kernel.org
Signed-off-by: Johan Hovold
---
drivers/tty/moxa.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/moxa.c b/drivers/tty/moxa.c
index 5b7bc7af8b1e..63e440d900ff 100644
--- a/drivers/tty/moxa.c
+++ b/drivers/tty/moxa.c
@@ -2048,6 +2048,7 @@ static int moxa_set_serial_info(struct tty_struct *tty,
struct serial_struct *ss)
{
struct moxa_port *info = tty->driver_data;
+ unsigned int close_delay;
if (tty->index == MAX_PORTS)
return -EINVAL;
@@ -2059,19 +2060,24 @@ static int moxa_set_serial_info(struct tty_struct *tty,
ss->baud_base != 921600)
return -EPERM;
+ close_delay = msecs_to_jiffies(ss->close_delay * 10);
+
mutex_lock(>port.mutex);
if (!capable(CAP_SYS_ADMIN)) {
- if (((ss->flags & ~ASYNC_USR_MASK) !=
+ if (close_delay != info->port.close_delay ||
+ ss->type != info->type ||
+ ((ss->flags & ~ASYNC_USR_MASK) !=
(info->port.flags & ~ASYNC_USR_MASK))) {
mutex_unlock(>port.mutex);
return -EPERM;
}
- }
- info->port.close_delay = msecs_to_jiffies(ss->close_delay * 10);
+ } else {
+ info->port.close_delay = close_delay;
- MoxaSetFifo(info, ss->type == PORT_16550A);
+ MoxaSetFifo(info, ss->type == PORT_16550A);
- info->type = ss->type;
+ info->type = ss->type;
+ }
mutex_unlock(>port.mutex);
return 0;
}
--
2.26.3
[PATCH 12/16] tty: moxa: fix TIOCSSERIAL implementation
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most serial devices is only useful for setting the close_delay and closing_wait parameters. A non-privileged user has only ever been able to set the since long deprecated ASYNC_SPD flags and trying to change any other *supported* feature should result in -EPERM being returned. Setting the current values for any supported features should return success. Fix the moxa implementation which was returning -EPERM also for a privileged user when trying to change certain unsupported parameters and instead return success consistently. Signed-off-by: Johan Hovold --- drivers/tty/moxa.c | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/tty/moxa.c b/drivers/tty/moxa.c index 63e440d900ff..4d4f15b5cd29 100644 --- a/drivers/tty/moxa.c +++ b/drivers/tty/moxa.c @@ -2055,11 +2055,6 @@ static int moxa_set_serial_info(struct tty_struct *tty, if (!info) return -ENODEV; - if (ss->irq != 0 || ss->port != 0 || - ss->custom_divisor != 0 || - ss->baud_base != 921600) - return -EPERM; - close_delay = msecs_to_jiffies(ss->close_delay * 10); mutex_lock(>port.mutex); -- 2.26.3
[PATCH 09/16] tty: amiserial: add missing TIOCSSERIAL jiffies conversions
The tty-port close_delay and closing_wait parameters set by TIOCSSERIAL
are specified in jiffies, while the values returned by TIOCGSERIAL are
specified in centiseconds.
Add the missing conversions so that TIOCSSERIAL works as expected also
if this code is ever reused on a system where HZ is not 100.
Signed-off-by: Johan Hovold
---
drivers/tty/amiserial.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
index ec6802ba2bf8..ca48ce5a0862 100644
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -937,15 +937,21 @@ static void rs_unthrottle(struct tty_struct * tty)
static int get_serial_info(struct tty_struct *tty, struct serial_struct *ss)
{
struct serial_state *state = tty->driver_data;
+ unsigned int close_delay, closing_wait;
tty_lock(tty);
+ close_delay = jiffies_to_msecs(state->tport.close_delay) / 10;
+ closing_wait = state->tport.closing_wait;
+ if (closing_wait != ASYNC_CLOSING_WAIT_NONE)
+ closing_wait = jiffies_to_msecs(closing_wait) / 10;
+
ss->line = tty->index;
ss->port = state->port;
ss->flags = state->tport.flags;
ss->xmit_fifo_size = state->xmit_fifo_size;
ss->baud_base = state->baud_base;
- ss->close_delay = state->tport.close_delay;
- ss->closing_wait = state->tport.closing_wait;
+ ss->close_delay = close_delay;
+ ss->closing_wait = closing_wait;
ss->custom_divisor = state->custom_divisor;
tty_unlock(tty);
return 0;
@@ -957,6 +963,7 @@ static int set_serial_info(struct tty_struct *tty, struct
serial_struct *ss)
struct tty_port *port = >tport;
bool change_spd;
int retval = 0;
+ unsigned int close_delay, closing_wait;
tty_lock(tty);
change_spd = ((ss->flags ^ port->flags) & ASYNC_SPD_MASK) ||
@@ -966,11 +973,16 @@ static int set_serial_info(struct tty_struct *tty, struct
serial_struct *ss)
tty_unlock(tty);
return -EINVAL;
}
-
+
+ close_delay = msecs_to_jiffies(ss->close_delay * 10);
+ closing_wait = ss->closing_wait;
+ if (closing_wait != ASYNC_CLOSING_WAIT_NONE)
+ closing_wait = msecs_to_jiffies(closing_wait * 10);
+
if (!serial_isroot()) {
if ((ss->baud_base != state->baud_base) ||
- (ss->close_delay != port->close_delay) ||
- (ss->closing_wait != port->closing_wait) ||
+ (close_delay != port->close_delay) ||
+ (closing_wait != port->closing_wait) ||
(ss->xmit_fifo_size != state->xmit_fifo_size) ||
((ss->flags & ~ASYNC_USR_MASK) !=
(port->flags & ~ASYNC_USR_MASK))) {
@@ -997,8 +1009,8 @@ static int set_serial_info(struct tty_struct *tty, struct
serial_struct *ss)
port->flags = ((port->flags & ~ASYNC_FLAGS) |
(ss->flags & ASYNC_FLAGS));
state->custom_divisor = ss->custom_divisor;
- port->close_delay = ss->close_delay * HZ/100;
- port->closing_wait = ss->closing_wait * HZ/100;
+ port->close_delay = close_delay;
+ port->closing_wait = closing_wait;
check_and_exit:
if (tty_port_initialized(port)) {
--
2.26.3
[PATCH 08/16] tty: amiserial: fix TIOCSSERIAL permission check
Changing the port closing_wait parameter is a privileged operation.
Add the missing check to TIOCSSERIAL so that -EPERM is returned in case
an unprivileged user tries to change the closing-wait setting.
Cc: sta...@vger.kernel.org
Signed-off-by: Johan Hovold
---
drivers/tty/amiserial.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
index 0c8157fab17f..ec6802ba2bf8 100644
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -970,6 +970,7 @@ static int set_serial_info(struct tty_struct *tty, struct
serial_struct *ss)
if (!serial_isroot()) {
if ((ss->baud_base != state->baud_base) ||
(ss->close_delay != port->close_delay) ||
+ (ss->closing_wait != port->closing_wait) ||
(ss->xmit_fifo_size != state->xmit_fifo_size) ||
((ss->flags & ~ASYNC_USR_MASK) !=
(port->flags & ~ASYNC_USR_MASK))) {
--
2.26.3
[PATCH 07/16] staging: greybus: uart: clean up TIOCGSERIAL
TIOCSSERIAL is a horrid, underspecified, legacy interface which for most
serial devices is only useful for setting the close_delay and
closing_wait parameters.
The xmit_fifo_size parameter could be used to set the hardware transmit
fifo size of a legacy UART when it could not be detected, but the
interface is limited to eight bits and should be left unset when not
used.
Similarly, baud_base could be used to set the UART base clock when it
could not be detected but might as well be left unset when it is not
known.
The type parameter could be used to set the UART type, but is
better left unspecified (type unknown) when it isn't used.
Note that some applications have historically expected TIOCGSERIAL to be
implemented, but judging from the Debian sources, the port type not
being PORT_UNKNOWN is only used to check for the existence of legacy
serial ports (ttySn). Notably USB serial drivers like ftdi_sio have been
using PORT_UNKNOWN for twenty years without any problems.
Drop the bogus values provided by the greybus implementation.
Signed-off-by: Johan Hovold
---
drivers/staging/greybus/uart.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
index a520f7f213db..b1e63f7798b0 100644
--- a/drivers/staging/greybus/uart.c
+++ b/drivers/staging/greybus/uart.c
@@ -610,10 +610,7 @@ static int get_serial_info(struct tty_struct *tty,
{
struct gb_tty *gb_tty = tty->driver_data;
- ss->type = PORT_16550A;
ss->line = gb_tty->minor;
- ss->xmit_fifo_size = 16;
- ss->baud_base = 9600;
ss->close_delay = jiffies_to_msecs(gb_tty->port.close_delay) / 10;
ss->closing_wait =
gb_tty->port.closing_wait == ASYNC_CLOSING_WAIT_NONE ?
--
2.26.3
[PATCH 10/16] tty: moxa: fix TIOCSSERIAL jiffies conversions
The port close_delay parameter set by TIOCSSERIAL is specified in jiffies, while the value returned by TIOCGSERIAL is specified in centiseconds. Add the missing conversions so that TIOCGSERIAL works as expected also when HZ is not 100. Cc: sta...@vger.kernel.org Signed-off-by: Johan Hovold --- drivers/tty/moxa.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/moxa.c b/drivers/tty/moxa.c index 32eb6b5e510f..5b7bc7af8b1e 100644 --- a/drivers/tty/moxa.c +++ b/drivers/tty/moxa.c @@ -2038,7 +2038,7 @@ static int moxa_get_serial_info(struct tty_struct *tty, ss->line = info->port.tty->index, ss->flags = info->port.flags, ss->baud_base = 921600, - ss->close_delay = info->port.close_delay; + ss->close_delay = jiffies_to_msecs(info->port.close_delay) / 10; mutex_unlock(>port.mutex); return 0; } @@ -2067,7 +2067,7 @@ static int moxa_set_serial_info(struct tty_struct *tty, return -EPERM; } } - info->port.close_delay = ss->close_delay * HZ / 100; + info->port.close_delay = msecs_to_jiffies(ss->close_delay * 10); MoxaSetFifo(info, ss->type == PORT_16550A); -- 2.26.3
