On 28 June 2015 at 23:29, Jakub Kiciński <moorray3@xxxxx> wrote:
> Michael, can you try this please?
>
> ---->8--------------
>
> From: Jakub Kicinski <kubakici@xxxxx>
> Date: Thu, 25 Jun 2015 18:17:58 +0200
> Subject: [PATCH] sc16is7xx: support multiple devices
>
> We currently register the uart driver during device probe
> which makes it hard to support more than one chip.
> Move the driver registration to module init/exit time and
> preallocate space for up to 8 lines (4-8 chips).
>
> Reported-by: Michael Allwright <michael.allwright@xxxxxx>
> Signed-off-by: Jakub Kicinski <kubakici@xxxxx>
> ---
> drivers/tty/serial/sc16is7xx.c | 73 ++++++++++++++++++++++++++++--------------
> 1 file changed, 49 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index 5ccc698cbbfa..84fb3e8c3f80 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -11,6 +11,8 @@
> *
> */
>
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> #include <linux/bitops.h>
> #include <linux/clk.h>
> #include <linux/delay.h>
> @@ -29,6 +31,7 @@
> #include <linux/uaccess.h>
>
> #define SC16IS7XX_NAME "sc16is7xx"
> +#define SC16IS7XX_MAX_DEVS 8
>
> /* SC16IS7XX register definitions */
> #define SC16IS7XX_RHR_REG (0x00) /* RX FIFO */
> @@ -318,7 +321,6 @@ struct sc16is7xx_one {
> };
>
> struct sc16is7xx_port {
> - struct uart_driver uart;
> struct sc16is7xx_devtype *devtype;
> struct regmap *regmap;
> struct clk *clk;
> @@ -332,6 +334,14 @@ struct sc16is7xx_port {
> struct sc16is7xx_one p[0];
> };
>
> +static unsigned long sc16is7xx_lines;
> +
> +static struct uart_driver sc16is7xx_uart = {
> + .owner = THIS_MODULE,
> + .dev_name = "ttySC",
> + .nr = SC16IS7XX_MAX_DEVS,
> +};
> +
> #define to_sc16is7xx_port(p,e) ((container_of((p), struct sc16is7xx_port, e)))
> #define to_sc16is7xx_one(p,e) ((container_of((p), struct sc16is7xx_one, e)))
>
> @@ -384,6 +394,18 @@ static void sc16is7xx_port_update(struct uart_port *port, u8 reg,
> mask, val);
> }
>
> +static int sc16is7xx_alloc_line(void)
> +{
> + int i;
> +
> + BUILD_BUG_ON(SC16IS7XX_MAX_DEVS > BITS_PER_LONG);
> +
> + for (i = 0; i < SC16IS7XX_MAX_DEVS; i++)
> + if (!test_and_set_bit(i, &sc16is7xx_lines))
> + break;
> +
> + return i;
> +}
>
> static void sc16is7xx_power(struct uart_port *port, int on)
> {
> @@ -661,7 +683,7 @@ static void sc16is7xx_ist(struct kthread_work *ws)
> struct sc16is7xx_port *s = to_sc16is7xx_port(ws, irq_work);
> int i;
>
> - for (i = 0; i < s->uart.nr; ++i)
> + for (i = 0; i < s->devtype->nr_uart; ++i)
> sc16is7xx_port_irq(s, i);
> }
>
> @@ -1134,23 +1156,13 @@ static int sc16is7xx_probe(struct device *dev,
> s->devtype = devtype;
> dev_set_drvdata(dev, s);
>
> - /* Register UART driver */
> - s->uart.owner = THIS_MODULE;
> - s->uart.dev_name = "ttySC";
> - s->uart.nr = devtype->nr_uart;
> - ret = uart_register_driver(&s->uart);
> - if (ret) {
> - dev_err(dev, "Registering UART driver failed\n");
> - goto out_clk;
> - }
> -
> init_kthread_worker(&s->kworker);
> init_kthread_work(&s->irq_work, sc16is7xx_ist);
> s->kworker_task = kthread_run(kthread_worker_fn, &s->kworker,
> "sc16is7xx");
> if (IS_ERR(s->kworker_task)) {
> ret = PTR_ERR(s->kworker_task);
> - goto out_uart;
> + goto out_clk;
> }
> sched_setscheduler(s->kworker_task, SCHED_FIFO, &sched_param);
>
> @@ -1175,7 +1187,6 @@ static int sc16is7xx_probe(struct device *dev,
>
> for (i = 0; i < devtype->nr_uart; ++i) {
> /* Initialize port data */
> - s->p[i].port.line = i;
> s->p[i].port.dev = dev;
> s->p[i].port.irq = irq;
> s->p[i].port.type = PORT_SC16IS7XX;
> @@ -1185,6 +1196,12 @@ static int sc16is7xx_probe(struct device *dev,
> s->p[i].port.uartclk = freq;
> s->p[i].port.rs485_config = sc16is7xx_config_rs485;
> s->p[i].port.ops = &sc16is7xx_ops;
> + s->p[i].port.line = sc16is7xx_alloc_line();
> + if (s->p[i].port.line >= SC16IS7XX_MAX_DEVS) {
> + ret = -ENOMEM;
> + goto out_ports;
> + }
> +
> /* Disable all interrupts */
> sc16is7xx_port_write(&s->p[i].port, SC16IS7XX_IER_REG, 0);
> /* Disable TX/RX */
> @@ -1195,7 +1212,7 @@ static int sc16is7xx_probe(struct device *dev,
> init_kthread_work(&s->p[i].tx_work, sc16is7xx_tx_proc);
> init_kthread_work(&s->p[i].reg_work, sc16is7xx_reg_proc);
> /* Register port */
> - uart_add_one_port(&s->uart, &s->p[i].port);
> + uart_add_one_port(&sc16is7xx_uart, &s->p[i].port);
> /* Go to suspend mode */
> sc16is7xx_power(&s->p[i].port, 0);
> }
> @@ -1206,8 +1223,11 @@ static int sc16is7xx_probe(struct device *dev,
> if (!ret)
> return 0;
>
> - for (i = 0; i < s->uart.nr; i++)
> - uart_remove_one_port(&s->uart, &s->p[i].port);
> +out_ports:
> + for (i--; i >= 0; i--) {
> + uart_remove_one_port(&sc16is7xx_uart, &s->p[i].port);
> + clear_bit(s->p[i].port.line, &sc16is7xx_lines);
> + }
>
> #ifdef CONFIG_GPIOLIB
> if (devtype->nr_gpio)
> @@ -1217,9 +1237,6 @@ out_thread:
> #endif
> kthread_stop(s->kworker_task);
>
> -out_uart:
> - uart_unregister_driver(&s->uart);
> -
> out_clk:
> if (!IS_ERR(s->clk))
> clk_disable_unprepare(s->clk);
> @@ -1237,15 +1254,15 @@ static int sc16is7xx_remove(struct device *dev)
> gpiochip_remove(&s->gpio);
> #endif
>
> - for (i = 0; i < s->uart.nr; i++) {
> - uart_remove_one_port(&s->uart, &s->p[i].port);
> + for (i = 0; i < s->devtype->nr_uart; i++) {
> + uart_remove_one_port(&sc16is7xx_uart, &s->p[i].port);
> + clear_bit(s->p[i].port.line, &sc16is7xx_lines);
> sc16is7xx_power(&s->p[i].port, 0);
> }
>
> flush_kthread_worker(&s->kworker);
> kthread_stop(s->kworker_task);
>
> - uart_unregister_driver(&s->uart);
> if (!IS_ERR(s->clk))
> clk_disable_unprepare(s->clk);
>
> @@ -1398,7 +1415,14 @@ MODULE_ALIAS("i2c:sc16is7xx");
>
> static int __init sc16is7xx_init(void)
> {
> - int ret = 0;
> + int ret;
> +
> + ret = uart_register_driver(&sc16is7xx_uart);
> + if (ret) {
> + pr_err("Registering UART driver failed\n");
> + return ret;
> + }
> +
> #ifdef CONFIG_SERIAL_SC16IS7XX_I2C
> ret = i2c_add_driver(&sc16is7xx_i2c_uart_driver);
> if (ret < 0) {
> @@ -1427,6 +1451,7 @@ static void __exit sc16is7xx_exit(void)
> #ifdef CONFIG_SERIAL_SC16IS7XX_SPI
> spi_unregister_driver(&sc16is7xx_spi_uart_driver);
> #endif
> + uart_unregister_driver(&sc16is7xx_uart);
> }
> module_exit(sc16is7xx_exit);
>
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Jakub,
I tried your patch today and although it is a cleaner implementation
than what I had, I'm still getting the same results as I did on
Friday. When both devices are configured, the ttySC2,3 shows up in
/dev but I can not get any RX or TX using picocom. I use the device
tree to instantiate the devices and when only one device is present
there are no issues (that is after the "fix FIFO address of secondary
UART" from Bo is applied -
http://www.spinics.net/lists/linux-serial/msg17820.html).
Here is the relevant parts of my device tree that are declared under
the second I2C bus, and would stress the point, that if I comment out
one of either "uart56: sc16is762@48" or "uart78: sc16is762@49", the
other device works as expected, so this is definitely a software
problem and not a hardware / device configuration problem.
I have also attached the output from sysfs of each relevant which
perhaps contains some useful information...
uart56: sc16is762@48 {
compatible = "nxp,sc16is762";
reg = <0x48>;
clocks = <&uart56_clk>;
interrupt-parent = <&gpio2>;
interrupts = <12 IRQ_TYPE_EDGE_FALLING>; /* gpmc_a20.gpio_44 */
uart56_osc: uart56_osc {
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <1843200>;
};
uart56_clk: uart56_clk {
compatible = "gpio-gate-clock";
clocks = <&uart56_osc>;
#clock-cells = <0>;
enable-gpios = <&gpio3 17 GPIO_ACTIVE_HIGH>; /*
cam_shutter.gpio_81: uart56_clk_en */
};
};
uart78: sc16is762@49 {
compatible = "nxp,sc16is762";
reg = <0x49>;
clocks = <&uart78_clk>;
interrupt-parent = <&gpio2>;
interrupts = <16 IRQ_TYPE_EDGE_FALLING>; /* gpmc_a24.gpio_48 */
uart78_osc: uart78_osc {
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <1843200>;
};
uart78_clk: uart78_clk {
compatible = "gpio-gate-clock";
clocks = <&uart78_osc>;
#clock-cells = <0>;
enable-gpios = <&gpio3 18 GPIO_ACTIVE_HIGH>; /*
cam_strobe.gpio_82: uart78_clk_en */
};
};
Regarding not working on tty-next, I'm not actually sure what the
correct workflow for resolving these sort of issues with the community
is. Since I'm also working with linux-media who also have their own
tree, I have been pulling the changes back and forth into my the
mainline kernel for my application as patches. Perhaps there is a
better way?
All the best,
Michael
/sys/class/tty/ttySC0/dev
251:0
/sys/class/tty/ttySC0/irq
71
/sys/class/tty/ttySC0/line
0
/sys/class/tty/ttySC0/port
0x0
/sys/class/tty/ttySC0/type
108
/sys/class/tty/ttySC0/flags
0x8002000
/sys/class/tty/ttySC0/power/control
auto
/sys/class/tty/ttySC0/power/wakeup_abort_count
/sys/class/tty/ttySC0/power/wakeup_active
/sys/class/tty/ttySC0/power/wakeup_total_time_ms
/sys/class/tty/ttySC0/power/wakeup_active_count
/sys/class/tty/ttySC0/power/runtime_active_time
0
/sys/class/tty/ttySC0/power/wakeup_max_time_ms
/sys/class/tty/ttySC0/power/wakeup_count
/sys/class/tty/ttySC0/power/wakeup_last_time_ms
/sys/class/tty/ttySC0/power/wakeup
disabled
/sys/class/tty/ttySC0/power/autosuspend_delay_ms
/sys/class/tty/ttySC0/power/runtime_status
unsupported
/sys/class/tty/ttySC0/power/wakeup_expire_count
/sys/class/tty/ttySC0/power/runtime_suspended_time
0
/sys/class/tty/ttySC0/iomem_base
0x0
/sys/class/tty/ttySC0/custom_divisor
0
/sys/class/tty/ttySC0/iomem_reg_shift
0
/sys/class/tty/ttySC0/uartclk
1843200
/sys/class/tty/ttySC0/xmit_fifo_size
64
/sys/class/tty/ttySC0/close_delay
50
/sys/class/tty/ttySC0/closing_wait
3000
/sys/class/tty/ttySC0/uevent
MAJOR=251
MINOR=0
DEVNAME=ttySC0
/sys/class/tty/ttySC0/io_type
0
/sys/class/tty/ttySC1/dev
251:1
/sys/class/tty/ttySC1/irq
71
/sys/class/tty/ttySC1/line
1
/sys/class/tty/ttySC1/port
0x0
/sys/class/tty/ttySC1/type
108
/sys/class/tty/ttySC1/flags
0x8002000
/sys/class/tty/ttySC1/power/control
auto
/sys/class/tty/ttySC1/power/wakeup_abort_count
/sys/class/tty/ttySC1/power/wakeup_active
/sys/class/tty/ttySC1/power/wakeup_total_time_ms
/sys/class/tty/ttySC1/power/wakeup_active_count
/sys/class/tty/ttySC1/power/runtime_active_time
0
/sys/class/tty/ttySC1/power/wakeup_max_time_ms
/sys/class/tty/ttySC1/power/wakeup_count
/sys/class/tty/ttySC1/power/wakeup_last_time_ms
/sys/class/tty/ttySC1/power/wakeup
disabled
/sys/class/tty/ttySC1/power/autosuspend_delay_ms
/sys/class/tty/ttySC1/power/runtime_status
unsupported
/sys/class/tty/ttySC1/power/wakeup_expire_count
/sys/class/tty/ttySC1/power/runtime_suspended_time
0
/sys/class/tty/ttySC1/iomem_base
0x0
/sys/class/tty/ttySC1/custom_divisor
0
/sys/class/tty/ttySC1/iomem_reg_shift
0
/sys/class/tty/ttySC1/uartclk
1843200
/sys/class/tty/ttySC1/xmit_fifo_size
64
/sys/class/tty/ttySC1/close_delay
50
/sys/class/tty/ttySC1/closing_wait
3000
/sys/class/tty/ttySC1/uevent
MAJOR=251
MINOR=1
DEVNAME=ttySC1
/sys/class/tty/ttySC1/io_type
0
/sys/class/tty/ttySC2/dev
251:2
/sys/class/tty/ttySC2/irq
75
/sys/class/tty/ttySC2/line
2
/sys/class/tty/ttySC2/port
0x0
/sys/class/tty/ttySC2/type
108
/sys/class/tty/ttySC2/flags
0x8002000
/sys/class/tty/ttySC2/power/control
auto
/sys/class/tty/ttySC2/power/wakeup_abort_count
/sys/class/tty/ttySC2/power/wakeup_active
/sys/class/tty/ttySC2/power/wakeup_total_time_ms
/sys/class/tty/ttySC2/power/wakeup_active_count
/sys/class/tty/ttySC2/power/runtime_active_time
0
/sys/class/tty/ttySC2/power/wakeup_max_time_ms
/sys/class/tty/ttySC2/power/wakeup_count
/sys/class/tty/ttySC2/power/wakeup_last_time_ms
/sys/class/tty/ttySC2/power/wakeup
disabled
/sys/class/tty/ttySC2/power/autosuspend_delay_ms
/sys/class/tty/ttySC2/power/runtime_status
unsupported
/sys/class/tty/ttySC2/power/wakeup_expire_count
/sys/class/tty/ttySC2/power/runtime_suspended_time
0
/sys/class/tty/ttySC2/iomem_base
0x0
/sys/class/tty/ttySC2/custom_divisor
0
/sys/class/tty/ttySC2/iomem_reg_shift
0
/sys/class/tty/ttySC2/uartclk
1843200
/sys/class/tty/ttySC2/xmit_fifo_size
64
/sys/class/tty/ttySC2/close_delay
50
/sys/class/tty/ttySC2/closing_wait
3000
/sys/class/tty/ttySC2/uevent
MAJOR=251
MINOR=2
DEVNAME=ttySC2
/sys/class/tty/ttySC2/io_type
0
/sys/class/tty/ttySC3/dev
251:3
/sys/class/tty/ttySC3/irq
75
/sys/class/tty/ttySC3/line
3
/sys/class/tty/ttySC3/port
0x0
/sys/class/tty/ttySC3/type
108
/sys/class/tty/ttySC3/flags
0x8002000
/sys/class/tty/ttySC3/power/control
auto
/sys/class/tty/ttySC3/power/wakeup_abort_count
/sys/class/tty/ttySC3/power/wakeup_active
/sys/class/tty/ttySC3/power/wakeup_total_time_ms
/sys/class/tty/ttySC3/power/wakeup_active_count
/sys/class/tty/ttySC3/power/runtime_active_time
0
/sys/class/tty/ttySC3/power/wakeup_max_time_ms
/sys/class/tty/ttySC3/power/wakeup_count
/sys/class/tty/ttySC3/power/wakeup_last_time_ms
/sys/class/tty/ttySC3/power/wakeup
disabled
/sys/class/tty/ttySC3/power/autosuspend_delay_ms
/sys/class/tty/ttySC3/power/runtime_status
unsupported
/sys/class/tty/ttySC3/power/wakeup_expire_count
/sys/class/tty/ttySC3/power/runtime_suspended_time
0
/sys/class/tty/ttySC3/iomem_base
0x0
/sys/class/tty/ttySC3/custom_divisor
0
/sys/class/tty/ttySC3/iomem_reg_shift
0
/sys/class/tty/ttySC3/uartclk
1843200
/sys/class/tty/ttySC3/xmit_fifo_size
64
/sys/class/tty/ttySC3/close_delay
50
/sys/class/tty/ttySC3/closing_wait
3000
/sys/class/tty/ttySC3/uevent
MAJOR=251
MINOR=3
DEVNAME=ttySC3
/sys/class/tty/ttySC3/io_type
0
