* [PATCH 6/8] serial: uapi: Add support for bus termination
From: Jan Kiszka @ 2017-05-13 7:29 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <cover.1494660546.git.jan.kiszka@siemens.com>
The Siemens IOT2040 comes with a RS485 interface that allows to enable
or disable bus termination via software. Add a bit to the flags field of
serial_rs485 that applications can set in order to request this feature
from the hardware. This seems generic enough to add it for everyone.
Existing driver will simply ignore it when set.
Signed-off-by: Sascha Weisenberger <sascha.weisenberger@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
include/uapi/linux/serial.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
index 5d59c3ebf459..d2667ecd54ac 100644
--- a/include/uapi/linux/serial.h
+++ b/include/uapi/linux/serial.h
@@ -122,6 +122,9 @@ struct serial_rs485 {
#define SER_RS485_RTS_AFTER_SEND (1 << 2) /* Logical level for
RTS pin after sent*/
#define SER_RS485_RX_DURING_TX (1 << 4)
+#define SER_RS485_TERMINATE_BUS (1 << 5) /* Enable bus
+ termination
+ (if supported) */
__u32 delay_rts_before_send; /* Delay before send (milliseconds) */
__u32 delay_rts_after_send; /* Delay after send (milliseconds) */
__u32 padding[5]; /* Memory is cheap, new structs
--
2.12.0
^ permalink raw reply related
* [PATCH 5/8] gpio: exar: Fix reading of directions and values
From: Jan Kiszka @ 2017-05-13 7:29 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <cover.1494660546.git.jan.kiszka@siemens.com>
First, the logic for translating a register bit to the return code of
exar_get_direction and exar_get_value were wrong. And second, there was
a flip regarding the register bank in exar_get_direction.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
drivers/gpio/gpio-exar.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index d6ffb3d89b9c..98bd3eb1290e 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -68,7 +68,7 @@ static int exar_get(struct gpio_chip *chip, unsigned int reg)
value = readb(exar_gpio->regs + reg);
mutex_unlock(&exar_gpio->lock);
- return !!value;
+ return value;
}
static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
@@ -80,7 +80,7 @@ static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
val = exar_get(chip, addr) >> (offset % 8);
- return !!val;
+ return val & 1;
}
static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
@@ -89,10 +89,10 @@ static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
unsigned int addr;
int val;
- addr = bank ? EXAR_OFFSET_MPIOLVL_LO : EXAR_OFFSET_MPIOLVL_HI;
+ addr = bank ? EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
val = exar_get(chip, addr) >> (offset % 8);
- return !!val;
+ return val & 1;
}
static void exar_set_value(struct gpio_chip *chip, unsigned int offset,
--
2.12.0
^ permalink raw reply related
* [PATCH 4/8] gpio: exar: Fix iomap request
From: Jan Kiszka @ 2017-05-13 7:29 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <cover.1494660546.git.jan.kiszka@siemens.com>
The UART driver already maps the resource for us. Trying to do this here
only fails and leaves us with a non-working device.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
drivers/gpio/gpio-exar.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index 9138ee087c5d..d6ffb3d89b9c 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -128,14 +128,10 @@ static int gpio_exar_probe(struct platform_device *pdev)
return -ENODEV;
/*
- * Map the pci device to get the register addresses.
- * We will need to read and write those registers to control
- * the GPIO pins.
- * Using managed functions will save us from unmaping on exit.
- * As the device is enabled using managed functions by the
- * UART driver we can also use managed functions here.
+ * The UART driver must have mapped region 0 prior to registering this
+ * device - use it.
*/
- p = pcim_iomap(pcidev, 0, 0);
+ p = pcim_iomap_table(pcidev)[0];
if (!p)
return -ENOMEM;
--
2.12.0
^ permalink raw reply related
* [PATCH 3/8] gpio: exar: Allocate resources on behalf of the platform device
From: Jan Kiszka @ 2017-05-13 7:29 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <cover.1494660546.git.jan.kiszka@siemens.com>
Do not allocate resources on behalf of the parent device but on our own.
Otherwise, cleanup does not properly work if gpio-exar is removed but
not the parent device.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
drivers/gpio/gpio-exar.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index 0a2085faf271..9138ee087c5d 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -139,7 +139,7 @@ static int gpio_exar_probe(struct platform_device *pdev)
if (!p)
return -ENOMEM;
- exar_gpio = devm_kzalloc(&pcidev->dev, sizeof(*exar_gpio), GFP_KERNEL);
+ exar_gpio = devm_kzalloc(&pdev->dev, sizeof(*exar_gpio), GFP_KERNEL);
if (!exar_gpio)
return -ENOMEM;
@@ -160,7 +160,7 @@ static int gpio_exar_probe(struct platform_device *pdev)
exar_gpio->regs = p;
exar_gpio->index = index;
- ret = devm_gpiochip_add_data(&pcidev->dev,
+ ret = devm_gpiochip_add_data(&pdev->dev,
&exar_gpio->gpio_chip, exar_gpio);
if (ret)
goto err_destroy;
--
2.12.0
^ permalink raw reply related
* [PATCH 2/8] gpio: exar: Fix passing in of parent PCI device
From: Jan Kiszka @ 2017-05-13 7:29 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <cover.1494660546.git.jan.kiszka@siemens.com>
This fixes reloading of the driver for the same device: First of all,
the driver sets drvdata to its own value during probing and does not
restore the original value on exit. But this won't help anyway as the
core clears drvdata after the driver left.
Use stable platform_data instead.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
drivers/gpio/gpio-exar.c | 2 +-
drivers/tty/serial/8250/8250_exar.c | 8 ++++++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
index 081076771217..0a2085faf271 100644
--- a/drivers/gpio/gpio-exar.c
+++ b/drivers/gpio/gpio-exar.c
@@ -119,7 +119,7 @@ static int exar_direction_input(struct gpio_chip *chip, unsigned int offset)
static int gpio_exar_probe(struct platform_device *pdev)
{
- struct pci_dev *pcidev = platform_get_drvdata(pdev);
+ struct pci_dev *pcidev = *(struct pci_dev **)pdev->dev.platform_data;
struct exar_gpio_chip *exar_gpio;
void __iomem *p;
int index, ret;
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index b4fa585156c7..2d056d1eeca3 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -196,8 +196,12 @@ xr17v35x_register_gpio(struct pci_dev *pcidev)
if (!pdev)
return NULL;
- platform_set_drvdata(pdev, pcidev);
- if (platform_device_add(pdev) < 0) {
+ /*
+ * platform_device_add_data kmemdups the data, therefore we can safely
+ * pass a stack reference.
+ */
+ if (platform_device_add_data(pdev, &pcidev, sizeof(pcidev)) < 0 ||
+ platform_device_add(pdev) < 0) {
platform_device_put(pdev);
return NULL;
}
--
2.12.0
^ permalink raw reply related
* [PATCH 1/8] serial: exar: Preconfigure xr17v35x MPIOs as output
From: Jan Kiszka @ 2017-05-13 7:28 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
In-Reply-To: <cover.1494660546.git.jan.kiszka@siemens.com>
This is the safe default for GPIOs with unknown external wiring.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
drivers/tty/serial/8250/8250_exar.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 1270ff163f63..b4fa585156c7 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -177,13 +177,13 @@ static void setup_gpio(u8 __iomem *p)
writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
- writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
+ writeb(0xff, p + UART_EXAR_MPIOSEL_7_0);
writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
writeb(0x00, p + UART_EXAR_MPIOINT_15_8);
writeb(0x00, p + UART_EXAR_MPIOLVL_15_8);
writeb(0x00, p + UART_EXAR_MPIO3T_15_8);
writeb(0x00, p + UART_EXAR_MPIOINV_15_8);
- writeb(0x00, p + UART_EXAR_MPIOSEL_15_8);
+ writeb(0xff, p + UART_EXAR_MPIOSEL_15_8);
writeb(0x00, p + UART_EXAR_MPIOOD_15_8);
}
--
2.12.0
^ permalink raw reply related
* [PATCH 0/8] serial/gpio: exar: Fixes and support for IOT2000
From: Jan Kiszka @ 2017-05-13 7:28 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexandre Courbot
Cc: Linux Kernel Mailing List, linux-serial, linux-gpio,
Sudip Mukherjee, Andy Shevchenko, Sascha Weisenberger
This makes the gpio-exar driver usable, which was prevented by a number
of fatal bugs, and adds support for the SIMATIC IOT2040 to the 8250-exar
driver and, indirectly, to gpio-exar as well. It's a cross-subsystem
series, so I'm also cross-posting to the serial and gpio lists.
Jan
Jan Kiszka (8):
serial: exar: Preconfigure xr17v35x MPIOs as output
gpio: exar: Fix passing in of parent PCI device
gpio: exar: Allocate resources on behalf of the platform device
gpio: exar: Fix iomap request
gpio: exar: Fix reading of directions and values
serial: uapi: Add support for bus termination
gpio-exar/8250-exar: Make set of exported GPIOs configurable
serial: exar: Add support for IOT2040 device
drivers/gpio/gpio-exar.c | 55 ++++++++------
drivers/tty/serial/8250/8250_exar.c | 145 +++++++++++++++++++++++++++++++++---
include/uapi/linux/serial.h | 3 +
3 files changed, 169 insertions(+), 34 deletions(-)
--
2.12.0
^ permalink raw reply
* Re: [PATCH v2] serial: exar: Fix stuck MSIs
From: Jan Kiszka @ 2017-05-13 7:09 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Andy Shevchenko, Linux Kernel Mailing List, linux-serial,
Sudip Mukherjee
In-Reply-To: <1493035647.24567.165.camel@linux.intel.com>
On 2017-04-24 14:07, Andy Shevchenko wrote:
> On Mon, 2017-04-24 at 12:30 +0200, Jan Kiszka wrote:
>> After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
>> without further interrupts because of the special wake-up event these
>> chips send. They are only cleared by reading INT0. As we fail to do so
>> during startup and shutdown, we can leave the interrupt line asserted,
>> which is fatal with edge-triggered MSIs.
>>
>> Add the required reading of INT0 to startup and shutdown. Also account
>> for the fact that a pending wake-up interrupt means we have to return
>> 1
>> from exar_handle_irq. Drop the unneeded reading of INT1..3 along with
>> this - those never reset anything.
>>
>> An alternative approach would have been disabling the wake-up
>> interrupt.
>> Unfortunately, this feature (REGB[17] = 1) is not available on the
>> XR17D15X.
>
> FWIW:
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
Ping. Needs to go into stable 4.11 now as well.
Jan
>>
>> Fixes: 172c33cb61da ("serial: exar: Enable MSI support")
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> Changes in v2:
>> - mention removal of INT1..3 reading in commit log [Andy]
>> (no functional changes)
>>
>> drivers/tty/serial/8250/8250_port.c | 19 ++++++++++---------
>> 1 file changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_port.c
>> b/drivers/tty/serial/8250/8250_port.c
>> index 6119516ef5fc..3a3667880fcf 100644
>> --- a/drivers/tty/serial/8250/8250_port.c
>> +++ b/drivers/tty/serial/8250/8250_port.c
>> @@ -47,6 +47,7 @@
>> /*
>> * These are definitions for the Exar XR17V35X and XR17(C|D)15X
>> */
>> +#define UART_EXAR_INT0 0x80
>> #define UART_EXAR_SLEEP 0x8b /* Sleep mode */
>> #define UART_EXAR_DVID 0x8d /* Device
>> identification */
>>
>> @@ -1869,17 +1870,13 @@ static int
>> serial8250_default_handle_irq(struct uart_port *port)
>> static int exar_handle_irq(struct uart_port *port)
>> {
>> unsigned int iir = serial_port_in(port, UART_IIR);
>> - int ret;
>> + int ret = 0;
>>
>> - ret = serial8250_handle_irq(port, iir);
>> + if (((port->type == PORT_XR17V35X) || (port->type ==
>> PORT_XR17D15X)) &&
>> + serial_port_in(port, UART_EXAR_INT0) != 0)
>> + ret = 1;
>>
>> - if ((port->type == PORT_XR17V35X) ||
>> - (port->type == PORT_XR17D15X)) {
>> - serial_port_in(port, 0x80);
>> - serial_port_in(port, 0x81);
>> - serial_port_in(port, 0x82);
>> - serial_port_in(port, 0x83);
>> - }
>> + ret |= serial8250_handle_irq(port, iir);
>>
>> return ret;
>> }
>> @@ -2177,6 +2174,8 @@ int serial8250_do_startup(struct uart_port
>> *port)
>> serial_port_in(port, UART_RX);
>> serial_port_in(port, UART_IIR);
>> serial_port_in(port, UART_MSR);
>> + if ((port->type == PORT_XR17V35X) || (port->type ==
>> PORT_XR17D15X))
>> + serial_port_in(port, UART_EXAR_INT0);
>>
>> /*
>> * At this point, there's no way the LSR could still be 0xff;
>> @@ -2335,6 +2334,8 @@ int serial8250_do_startup(struct uart_port
>> *port)
>> serial_port_in(port, UART_RX);
>> serial_port_in(port, UART_IIR);
>> serial_port_in(port, UART_MSR);
>> + if ((port->type == PORT_XR17V35X) || (port->type ==
>> PORT_XR17D15X))
>> + serial_port_in(port, UART_EXAR_INT0);
>> up->lsr_saved_flags = 0;
>> up->msr_saved_flags = 0;
>>
>
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
^ permalink raw reply
* Re: [PATCH v5 14/17] dt-bindings: slave-device: add current-speed property
From: Rob Herring @ 2017-05-13 0:05 UTC (permalink / raw)
To: Stefan Wahren
Cc: David S. Miller, Mark Rutland, Greg Kroah-Hartman, Jiri Slaby,
linux-serial, linux-kernel, netdev, devicetree
In-Reply-To: <1494406408-31760-15-git-send-email-stefan.wahren@i2se.com>
On Wed, May 10, 2017 at 10:53:25AM +0200, Stefan Wahren wrote:
> This adds a new DT property to define the current baud rate of the
> slave device.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> Documentation/devicetree/bindings/serial/slave-device.txt | 9 +++++++++
> 1 file changed, 9 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH] serial: altera_jtaguart: adding iounmap()
From: Alexey Khoroshilov @ 2017-05-12 21:36 UTC (permalink / raw)
To: Tobias Klauser, Greg Kroah-Hartman
Cc: Alexey Khoroshilov, Jiri Slaby, linux-serial, nios2-dev,
linux-kernel, ldv-project
The driver does ioremap(port->mapbase, ALTERA_JTAGUART_SIZE),
but there is no any iounmap().
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/tty/serial/altera_jtaguart.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/serial/altera_jtaguart.c b/drivers/tty/serial/altera_jtaguart.c
index 18e3f8342b85..0475f5d261ce 100644
--- a/drivers/tty/serial/altera_jtaguart.c
+++ b/drivers/tty/serial/altera_jtaguart.c
@@ -478,6 +478,7 @@ static int altera_jtaguart_remove(struct platform_device *pdev)
port = &altera_jtaguart_ports[i].port;
uart_remove_one_port(&altera_jtaguart_driver, port);
+ iounmap(port->membase);
return 0;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH V2] serial: efm32: Fix parity management in 'efm32_uart_console_get_options()'
From: Uwe Kleine-König @ 2017-05-12 20:17 UTC (permalink / raw)
To: Christophe JAILLET
Cc: gregkh, jslaby, kernel, kernel-janitors, linux-arm-kernel,
linux-serial, linux-kernel
In-Reply-To: <20170512143545.15394-1-christophe.jaillet@wanadoo.fr>
On Fri, May 12, 2017 at 04:35:45PM +0200, Christophe JAILLET wrote:
> UARTn_FRAME_PARITY_ODD is 0x0300
> UARTn_FRAME_PARITY_EVEN is 0x0200
> So if the UART is configured for EVEN parity, it would be reported as ODD.
> Fix it by correctly testing if the 2 bits are set.
>
> Fixes: 3afbd89c9639 ("serial/efm32: add new driver")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Thanks for picking this topic up again.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH 3/6] dt-bindings: serial: fsl-lpuart: add i.MX7ULP support
From: Rob Herring @ 2017-05-12 20:12 UTC (permalink / raw)
To: Dong Aisheng
Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, jslaby-IBi9RG/b67k,
fugang.duan-3arQi8VN3Tc, stefan-XLVq0VzYD2Y,
Mingkai.Hu-3arQi8VN3Tc, yangbo.lu-3arQi8VN3Tc,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1494316248-24052-4-git-send-email-aisheng.dong-3arQi8VN3Tc@public.gmane.org>
On Tue, May 09, 2017 at 03:50:45PM +0800, Dong Aisheng wrote:
> The lpuart of imx7ulp is basically the same as ls1021a. It's also
> 32 bit width register, but unlike ls1021a, it's little endian.
> Besides that, imx7ulp lpuart has a minor different register layout
> from ls1021a.
>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> Cc: Jiri Slaby <jslaby-IBi9RG/b67k@public.gmane.org>
> Cc: Fugang Duan <fugang.duan-3arQi8VN3Tc@public.gmane.org>
> Cc: Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>
> Cc: Mingkai Hu <Mingkai.Hu-3arQi8VN3Tc@public.gmane.org>
> Cc: Yangbo Lu <yangbo.lu-3arQi8VN3Tc@public.gmane.org>
> Signed-off-by: Dong Aisheng <aisheng.dong-3arQi8VN3Tc@public.gmane.org>
> ---
> Documentation/devicetree/bindings/serial/fsl-lpuart.txt | 2 ++
> 1 file changed, 2 insertions(+)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH V2] serial: efm32: Fix parity management in 'efm32_uart_console_get_options()'
From: Christophe JAILLET @ 2017-05-12 14:35 UTC (permalink / raw)
To: gregkh, jslaby, kernel
Cc: linux-serial, linux-arm-kernel, linux-kernel, kernel-janitors,
Christophe JAILLET
UARTn_FRAME_PARITY_ODD is 0x0300
UARTn_FRAME_PARITY_EVEN is 0x0200
So if the UART is configured for EVEN parity, it would be reported as ODD.
Fix it by correctly testing if the 2 bits are set.
Fixes: 3afbd89c9639 ("serial/efm32: add new driver")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
v2: Add UARTn_FRAME_PARITY__MASK
Test the 2 parity bits in the EVEN case
v1 and related comment are quite old and can be found at:
http://marc.info/?t=148676095800005&r=1&w=4
I also added a __MASK to the proposed constant name to be consistent with
UARTn_FRAME_DATABITS__MASK
---
drivers/tty/serial/efm32-uart.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/efm32-uart.c b/drivers/tty/serial/efm32-uart.c
index ebd8569f9ad5..9fff25be87f9 100644
--- a/drivers/tty/serial/efm32-uart.c
+++ b/drivers/tty/serial/efm32-uart.c
@@ -27,6 +27,7 @@
#define UARTn_FRAME 0x04
#define UARTn_FRAME_DATABITS__MASK 0x000f
#define UARTn_FRAME_DATABITS(n) ((n) - 3)
+#define UARTn_FRAME_PARITY__MASK 0x0300
#define UARTn_FRAME_PARITY_NONE 0x0000
#define UARTn_FRAME_PARITY_EVEN 0x0200
#define UARTn_FRAME_PARITY_ODD 0x0300
@@ -572,12 +573,16 @@ static void efm32_uart_console_get_options(struct efm32_uart_port *efm_port,
16 * (4 + (clkdiv >> 6)));
frame = efm32_uart_read32(efm_port, UARTn_FRAME);
- if (frame & UARTn_FRAME_PARITY_ODD)
+ switch (frame & UARTn_FRAME_PARITY__MASK) {
+ case UARTn_FRAME_PARITY_ODD:
*parity = 'o';
- else if (frame & UARTn_FRAME_PARITY_EVEN)
+ break;
+ case UARTn_FRAME_PARITY_EVEN:
*parity = 'e';
- else
+ break;
+ default:
*parity = 'n';
+ }
*bits = (frame & UARTn_FRAME_DATABITS__MASK) -
UARTn_FRAME_DATABITS(4) + 4;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Petr Mladek @ 2017-05-12 13:46 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Aleksey Makarov, Sabrina Dubroca, linux-serial, linux-kernel,
Sudeep Holla, Greg Kroah-Hartman, Peter Hurley, Jiri Slaby,
Robin Murphy, Steven Rostedt, Nair, Jayachandran,
Sergey Senozhatsky
In-Reply-To: <20170512125729.GO3452@pathway.suse.cz>
On Fri 2017-05-12 14:57:29, Petr Mladek wrote:
> On Thu 2017-05-11 17:41:58, Sergey Senozhatsky wrote:
> > On (05/11/17 17:24), Sergey Senozhatsky wrote:
> > > On (05/09/17 10:29), Sabrina Dubroca wrote:
> > > [..]
> > > > That's caused a change of behavior in my qemu setup, with this cmdline
> > > >
> > > > root=/dev/sda1 console=ttyS1 console=ttyS0
> > > >
> > > > Before, the kernel logs appeared on ttyS1, and I logged in with ttyS0
> > > > (with my setup, ttyS1 is a file and ttyS0 is unix socket). Now, the
> > > > kernel logs go to ttyS0. I need to swap the two console= parameters to
> > > > restore behavior.
Do you actually need to define console=ttyS0 on the cmdline?
IMHO, if register_console() was called for the unix socket, it
would make logs appear on both ttyS0 and ttyS1. It seems
that register_console() is called only for the console that
stores logs into the file.
> > > > adding
> > > > console=tty0 anywhere on that cmdline makes the logs appear on both
> > > > tty0 and one ttyS* (but only one of them, and the ordering of the
> > > > ttyS* matters).
I guess that it worked this way before. I mean that the logs appeared
on both tty0 and one of ttyS*. The only difference should be that
the patch changed the selected ttyS*. So this is still the same problem.
> Hmm, I have no idea how to fix this. This is the case where
> a registered console matches more entries from the command line.
> The fix that caused this regression fixed exactly this situation
> and we wanted to make the preferred console first.
>
> I am not sure if we broke some backward compatibility or actually made
> it more predictable in the long term.
I think that we actually fixed a very old bug. The last mentioned
console= should be the preferred one and the logs are finally
printed there. Or do I miss anything?
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH 4/6] tty: serial: lpuart: add imx7ulp support
From: Dong Aisheng @ 2017-05-12 13:28 UTC (permalink / raw)
To: Stefan Agner
Cc: Dong Aisheng, linux-serial, linux-kernel, linux-arm-kernel,
gregkh, jslaby, Fugang Duan, Mingkai.Hu, yangbo.lu
In-Reply-To: <434c613240c36181a828ea090938b8c1@agner.ch>
On Wed, May 10, 2017 at 01:37:07PM -0700, Stefan Agner wrote:
> On 2017-05-09 23:14, Dong Aisheng wrote:
> > Hi Stefan,
> >
> > On Wed, May 10, 2017 at 12:10 PM, Stefan Agner <stefan@agner.ch> wrote:
> >> On 2017-05-09 00:50, Dong Aisheng wrote:
> >>> The lpuart of imx7ulp is basically the same as ls1021a. It's also
> >>> 32 bit width register, but unlike ls1021a, it's little endian.
> >>> Besides that, imx7ulp lpuart has a minor different register layout
> >>> from ls1021a that it has four extra registers (verid, param, global,
> >>> pincfg) located at the beginning of register map, which are currently
> >>> not used by the driver and less to be used later.
> >>>
> >>> To ease the register difference handling, we add a reg_off member
> >>> in lpuart_soc_data structure to represent if the normal
> >>> lpuart32_{read|write} requires plus a offset to hide the issue.
> >>>
> >>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>> Cc: Jiri Slaby <jslaby@suse.com>
> >>> Cc: Fugang Duan <fugang.duan@nxp.com>
> >>> Cc: Stefan Agner <stefan@agner.ch>
> >>> Cc: Mingkai Hu <Mingkai.Hu@nxp.com>
> >>> Cc: Yangbo Lu <yangbo.lu@nxp.com>
> >>> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> >>> ---
> >>> drivers/tty/serial/fsl_lpuart.c | 21 ++++++++++++++++++---
> >>> 1 file changed, 18 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> >>> index bddd041..1cdb3f9 100644
> >>> --- a/drivers/tty/serial/fsl_lpuart.c
> >>> +++ b/drivers/tty/serial/fsl_lpuart.c
> >>> @@ -231,7 +231,11 @@
> >>> #define DEV_NAME "ttyLP"
> >>> #define UART_NR 6
> >>>
> >>> +/* IMX lpuart has four extra unused regs located at the beginning */
> >>> +#define IMX_REG_OFF 0x10
> >>> +
> >>> static bool lpuart_is_be;
> >>> +static u8 lpuart_reg_off;
> >>
> >> Global variables? That hardly works once you have two UARTs...
> >>
> >
> > lpuart_reg_off is SoC specific and there's no two UART
> > with two different reg offset.
> >
> >> Instead of adding a fixed offset to any write you could just add the
> >> offset to sport->port.membase...
> >>
> >
> > That's intended as i don't want the changes to be too intrusive.
> > If adding offset in sport->port.xxx, then we have to change the basic
> > lpuart32_read/write API which is called through the whole driver.
>
> Every lpuart32_write/read call passes port.membase, so if you offset
> port.membase when it is assigned in probe you should be fine not?
>
Got your point now and that do seem better.
Thanks for the suggestion.
Will change in V2.
Regards
Dong Aisheng
^ permalink raw reply
* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Petr Mladek @ 2017-05-12 12:57 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Aleksey Makarov, Sabrina Dubroca, linux-serial, linux-kernel,
Sudeep Holla, Greg Kroah-Hartman, Peter Hurley, Jiri Slaby,
Robin Murphy, Steven Rostedt, Nair, Jayachandran,
Sergey Senozhatsky
In-Reply-To: <20170511084158.GB421@jagdpanzerIV.localdomain>
On Thu 2017-05-11 17:41:58, Sergey Senozhatsky wrote:
> On (05/11/17 17:24), Sergey Senozhatsky wrote:
> > On (05/09/17 10:29), Sabrina Dubroca wrote:
> > [..]
> > > That's caused a change of behavior in my qemu setup, with this cmdline
> > >
> > > root=/dev/sda1 console=ttyS1 console=ttyS0
> > >
> > > Before, the kernel logs appeared on ttyS1, and I logged in with ttyS0
> > > (with my setup, ttyS1 is a file and ttyS0 is unix socket). Now, the
> > > kernel logs go to ttyS0. I need to swap the two console= parameters to
> > > restore behavior.
> > >
> > > There might be some other problem (in qemu?) though, because adding
> > > console=tty0 anywhere on that cmdline makes the logs appear on both
> > > tty0 and one ttyS* (but only one of them, and the ordering of the
> > > ttyS* matters).
> >
> > thanks for the report.
> >
> > so we have ttyS1 first and ttyS0 last.
> > after commit in question, register_console() iterates console_cmdline
> > in reverse order so we see ttyS0 first, then we hit `if (newcon->index < 0)'
> > condition, set newcon to ttyS0, because we iterate in reverse order now, and
> > break out. so we enable ttyS0, instead of ttyS1.
> >
> > previously, we iterated console_cmdline from index 0 and saw ttyS1 first.
> > so the same `if (newcon->index < 0)' condition would set newcone to ttyS1,
> > and, thus, we would enable ttyS1, not ttyS0.
>
> Alexey,
> can we have preferred console at offset 0 (not at console_cmdline_cnt - 1)
> and restore the previous register_console() iteration order?
This will not help. ttyS0 is the last console defined on the command
line. Therefore it is the preferred one. It means that it will be
moved to offset 0 and hit first.
I have tried to reproduce the problem and started kernel with
console=ttyS1 console=ttyS0 in qemu. It created:
console_cmdline = {{
.name = "ttyS";
.index = 1; // from ttyS1
},{
.name = "ttyS"
.index = 0; // from ttyS0
}};
preferred_console = 1; // ttyuS0;
Then register_console() is called twice here. First time
from con_init() that registers:
static struct console vt_console_driver = {
.name = "tty",
.write = vt_console_print,
.device = vt_console_device,
.unblank = unblank_screen,
.flags = CON_PRINTBUFFER,
.index = -1,
};
It does not match and it is not enabled here.
2nd times from univ8250_console_init() that registers:
static struct console univ8250_console = {
.name = "ttyS",
.write = univ8250_console_write,
.device = uart_console_device,
.setup = univ8250_console_setup,
.match = univ8250_console_match,
.flags = CON_PRINTBUFFER | CON_ANYTIME,
.index = -1,
.data = &serial8250_reg,
};
It matches both console_cmdline entries because index = -1.
The first tested is selected.
Hmm, I have no idea how to fix this. This is the case where
a registered console matches more entries from the command line.
The fix that caused this regression fixed exactly this situation
and we wanted to make the preferred console first.
In fact, it always was kind of random because both init calls are
defined as
console_initcall(con_init);
console_initcall(univ8250_console_init);
They are put into special elf section and called from console_init()
the following way:
call = __con_initcall_start;
while (call < __con_initcall_end) {
(*call)();
call++;
}
By other words, the order depends on the linking order which is
kind of weak order enforcement.
I am not sure if we broke some backward compatibility or actually made
it more predictable in the long term.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v5 15/17] dt-bindings: qca7000: append UART interface to binding
From: Jakub Kicinski @ 2017-05-12 6:43 UTC (permalink / raw)
To: Michael Heimpold
Cc: Rob Herring, Stefan Wahren, Mark Rutland, Greg Kroah-Hartman,
Jiri Slaby, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170512061552.Horde.ggjCMHnIsr8LeADwTQwhnx7-O4EMSK59OscLBGDc3sTiBZZV94DADvEd@public.gmane.org>
On Fri, 12 May 2017 06:15:52 +0000, Michael Heimpold wrote:
> Hi,
>
> Zitat von Jakub Kicinski <kubakici-5tc4TXWwyLM@public.gmane.org>:
>
> > On Thu, 11 May 2017 21:12:22 +0200, Michael Heimpold wrote:
> >> Am Mittwoch, 10. Mai 2017, 10:53:26 CEST schrieb Stefan Wahren:
> >> > This merges the serdev binding for the QCA7000 UART driver (Ethernet over
> >> > UART) into the existing document.
> >> >
> >> > Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> >> > ---
> >> > .../devicetree/bindings/net/qca-qca7000.txt | 32
> >> > ++++++++++++++++++++++ 1 file changed, 32 insertions(+)
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/net/qca-qca7000.txt
> >> > b/Documentation/devicetree/bindings/net/qca-qca7000.txt index
> >> > a37f656..08364c3 100644
> >> > --- a/Documentation/devicetree/bindings/net/qca-qca7000.txt
> >> > +++ b/Documentation/devicetree/bindings/net/qca-qca7000.txt
> >> > @@ -54,3 +54,35 @@ ssp2: spi@80014000 {
> >> > local-mac-address = [ A0 B0 C0 D0 E0 F0 ];
> >> > };
> >> > };
> >> > +
> >> > +(b) Ethernet over UART
> >> > +
> >> > +In order to use the QCA7000 as UART slave it must be defined as
> >> a child of
> >> > a +UART master in the device tree. It is possible to preconfigure the UART
> >> > +settings of the QCA7000 firmware, but it's not possible to change them
> >> > during +runtime.
> >> > +
> >> > +Required properties:
> >> > +- compatible : Should be "qca,qca7000-uart"
> >>
> >> I already discussed this with Stefan off-list a little bit, but I would like
> >> to bring this to a broader audience: I'm not sure whether the compatible
> >> should contain the "-uart" suffix, because the hardware chip is the
> >> very same
> >> QCA7000 chip which can also be used with SPI protocol.
> >> The only difference is the loaded firmware within the chip which can either
> >> speak SPI or UART protocol (but not both at the same time - due to shared
> >> pins). So the hardware design decides which interface type is used.
> >>
> >> At the moment, this patch series adds a dedicated driver for the UART
> >> protocol, in parallel to the existing SPI driver. So a different compatible
> >> string is needed here to match against the new driver.
> >>
> >> An alternative approach would be to re-use the existing compatible string
> >> "qca,qca7000" for both, the SPI and UART protocol, because a "smarter"
> >> (combined) driver would detect which protocol to use. For example the driver
> >> could check for spi-cpha and/or spi-cpol which are required for SPI
> >> protocol:
> >> if these exists the driver could assume that SPI must be used, if both are
> >> missing then UART protocol should be used.
> >> (This way it would not be necessary to check whether the node is a child of
> >> a SPI or UART master node - but maybe this is even easier - I don't know)
> >>
> >> Or in shorter words: my concern is that while "qca7000-uart" describes the
> >> hardware, it's too closely coupled to the driver implementation. Having
> >> some feedback of the experts would be nice :-)
> >
> > I'm no expert, but devices which can do both I2C and SPI are quite
> > common, and they usually have the same compatible string for both
> > buses.
>
> do you have an example driver at hand? I only found GPIO mcp23s08 driver,
> which can handle both I2C and SPI chips, but there are different compatible
> strings used to distinguish several chip models.
I think drivers/tty/serial/sc16is7xx.c has the same strings, and some
Kconfig magic to work when either bus is enabled in .config.
Quick grep shows there are couple more potential ones to look at:
$ find . -name Kconfig | xargs grep -n 'SPI_MASTER.* I2C'
./drivers/tty/serial/Kconfig:1208: depends on (SPI_MASTER && !I2C) || I2C
./drivers/mfd/Kconfig:327: depends on (SPI_MASTER || I2C)
./drivers/iio/dac/Kconfig:10: depends on (SPI_MASTER && I2C!=m) || I2C
./drivers/iio/dac/Kconfig:34: depends on (SPI_MASTER && I2C!=m) || I2C
./drivers/iio/dac/Kconfig:57: depends on (SPI_MASTER && I2C!=m) || I2C
./drivers/gpio/Kconfig:1231: depends on (SPI_MASTER && !I2C) || I2C
$ find . -name Kconfig | xargs grep -n 'I2C.*||.*SPI_MASTER'
./drivers/mfd/Kconfig:1094: depends on (I2C=y || SPI_MASTER=y)
./drivers/iio/gyro/Kconfig:55: depends on (I2C || SPI_MASTER)
./drivers/iio/gyro/Kconfig:107: depends on (I2C || SPI_MASTER) && SYSFS
./drivers/iio/accel/Kconfig:153: depends on (I2C || SPI_MASTER) && SYSFS
./drivers/iio/pressure/Kconfig:20: depends on (I2C || SPI_MASTER)
./drivers/iio/pressure/Kconfig:161: depends on (I2C || SPI_MASTER) && SYSFS
./drivers/iio/magnetometer/Kconfig:118: depends on (I2C || SPI_MASTER) && SYSFS
drivers/mfd/mc13xxx-*.c seems to have the same strings. The iio/dac drivers
don't support DT but do share names. The MCP GPIO chip you mention indeed has
different product names based on the bus it's made for (0 vs s in the middle
of the name), so I gather less relevant case? drivers/iio/pressure/bmp280-*.c
has the same strings, if I'm looking correctly... I didn't look at the others.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 15/17] dt-bindings: qca7000: append UART interface to binding
From: Michael Heimpold @ 2017-05-12 6:15 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Rob Herring, Stefan Wahren, Mark Rutland, Greg Kroah-Hartman,
Jiri Slaby, linux-serial, linux-kernel, netdev, devicetree
In-Reply-To: <20170511194541.4e58cb7d@cakuba.netronome.com>
Hi,
Zitat von Jakub Kicinski <kubakici@wp.pl>:
> On Thu, 11 May 2017 21:12:22 +0200, Michael Heimpold wrote:
>> Am Mittwoch, 10. Mai 2017, 10:53:26 CEST schrieb Stefan Wahren:
>> > This merges the serdev binding for the QCA7000 UART driver (Ethernet over
>> > UART) into the existing document.
>> >
>> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>> > ---
>> > .../devicetree/bindings/net/qca-qca7000.txt | 32
>> > ++++++++++++++++++++++ 1 file changed, 32 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/net/qca-qca7000.txt
>> > b/Documentation/devicetree/bindings/net/qca-qca7000.txt index
>> > a37f656..08364c3 100644
>> > --- a/Documentation/devicetree/bindings/net/qca-qca7000.txt
>> > +++ b/Documentation/devicetree/bindings/net/qca-qca7000.txt
>> > @@ -54,3 +54,35 @@ ssp2: spi@80014000 {
>> > local-mac-address = [ A0 B0 C0 D0 E0 F0 ];
>> > };
>> > };
>> > +
>> > +(b) Ethernet over UART
>> > +
>> > +In order to use the QCA7000 as UART slave it must be defined as
>> a child of
>> > a +UART master in the device tree. It is possible to preconfigure the UART
>> > +settings of the QCA7000 firmware, but it's not possible to change them
>> > during +runtime.
>> > +
>> > +Required properties:
>> > +- compatible : Should be "qca,qca7000-uart"
>>
>> I already discussed this with Stefan off-list a little bit, but I would like
>> to bring this to a broader audience: I'm not sure whether the compatible
>> should contain the "-uart" suffix, because the hardware chip is the
>> very same
>> QCA7000 chip which can also be used with SPI protocol.
>> The only difference is the loaded firmware within the chip which can either
>> speak SPI or UART protocol (but not both at the same time - due to shared
>> pins). So the hardware design decides which interface type is used.
>>
>> At the moment, this patch series adds a dedicated driver for the UART
>> protocol, in parallel to the existing SPI driver. So a different compatible
>> string is needed here to match against the new driver.
>>
>> An alternative approach would be to re-use the existing compatible string
>> "qca,qca7000" for both, the SPI and UART protocol, because a "smarter"
>> (combined) driver would detect which protocol to use. For example the driver
>> could check for spi-cpha and/or spi-cpol which are required for SPI
>> protocol:
>> if these exists the driver could assume that SPI must be used, if both are
>> missing then UART protocol should be used.
>> (This way it would not be necessary to check whether the node is a child of
>> a SPI or UART master node - but maybe this is even easier - I don't know)
>>
>> Or in shorter words: my concern is that while "qca7000-uart" describes the
>> hardware, it's too closely coupled to the driver implementation. Having
>> some feedback of the experts would be nice :-)
>
> I'm no expert, but devices which can do both I2C and SPI are quite
> common, and they usually have the same compatible string for both
> buses.
do you have an example driver at hand? I only found GPIO mcp23s08 driver,
which can handle both I2C and SPI chips, but there are different compatible
strings used to distinguish several chip models.
Regards,
Michael
^ permalink raw reply
* Re: [PATCH v5 15/17] dt-bindings: qca7000: append UART interface to binding
From: Jakub Kicinski @ 2017-05-12 2:45 UTC (permalink / raw)
To: Michael Heimpold
Cc: Rob Herring, Stefan Wahren, Mark Rutland, Greg Kroah-Hartman,
Jiri Slaby, linux-serial, linux-kernel, netdev, devicetree
In-Reply-To: <1702237.66ccflAQAJ@kerker>
On Thu, 11 May 2017 21:12:22 +0200, Michael Heimpold wrote:
> Am Mittwoch, 10. Mai 2017, 10:53:26 CEST schrieb Stefan Wahren:
> > This merges the serdev binding for the QCA7000 UART driver (Ethernet over
> > UART) into the existing document.
> >
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > ---
> > .../devicetree/bindings/net/qca-qca7000.txt | 32
> > ++++++++++++++++++++++ 1 file changed, 32 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/qca-qca7000.txt
> > b/Documentation/devicetree/bindings/net/qca-qca7000.txt index
> > a37f656..08364c3 100644
> > --- a/Documentation/devicetree/bindings/net/qca-qca7000.txt
> > +++ b/Documentation/devicetree/bindings/net/qca-qca7000.txt
> > @@ -54,3 +54,35 @@ ssp2: spi@80014000 {
> > local-mac-address = [ A0 B0 C0 D0 E0 F0 ];
> > };
> > };
> > +
> > +(b) Ethernet over UART
> > +
> > +In order to use the QCA7000 as UART slave it must be defined as a child of
> > a +UART master in the device tree. It is possible to preconfigure the UART
> > +settings of the QCA7000 firmware, but it's not possible to change them
> > during +runtime.
> > +
> > +Required properties:
> > +- compatible : Should be "qca,qca7000-uart"
>
> I already discussed this with Stefan off-list a little bit, but I would like
> to bring this to a broader audience: I'm not sure whether the compatible
> should contain the "-uart" suffix, because the hardware chip is the very same
> QCA7000 chip which can also be used with SPI protocol.
> The only difference is the loaded firmware within the chip which can either
> speak SPI or UART protocol (but not both at the same time - due to shared
> pins). So the hardware design decides which interface type is used.
>
> At the moment, this patch series adds a dedicated driver for the UART
> protocol, in parallel to the existing SPI driver. So a different compatible
> string is needed here to match against the new driver.
>
> An alternative approach would be to re-use the existing compatible string
> "qca,qca7000" for both, the SPI and UART protocol, because a "smarter"
> (combined) driver would detect which protocol to use. For example the driver
> could check for spi-cpha and/or spi-cpol which are required for SPI protocol:
> if these exists the driver could assume that SPI must be used, if both are
> missing then UART protocol should be used.
> (This way it would not be necessary to check whether the node is a child of
> a SPI or UART master node - but maybe this is even easier - I don't know)
>
> Or in shorter words: my concern is that while "qca7000-uart" describes the
> hardware, it's too closely coupled to the driver implementation. Having
> some feedback of the experts would be nice :-)
I'm no expert, but devices which can do both I2C and SPI are quite
common, and they usually have the same compatible string for both
buses.
^ permalink raw reply
* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Sergey Senozhatsky @ 2017-05-12 1:11 UTC (permalink / raw)
To: Aleksey Makarov
Cc: Sergey Senozhatsky, Sabrina Dubroca, linux-serial, linux-kernel,
Sudeep Holla, Greg Kroah-Hartman, Peter Hurley, Jiri Slaby,
Robin Murphy, Steven Rostedt, Nair, Jayachandran,
Sergey Senozhatsky, Petr Mladek
In-Reply-To: <e757047d-d40a-582b-7c50-b8d8f93a076e@linaro.org>
On (05/12/17 00:17), Aleksey Makarov wrote:
> On 05/11/2017 02:32 PM, Sergey Senozhatsky wrote:
> > On (05/11/17 17:41), Sergey Senozhatsky wrote:
> > [..]
> > > Alexey,
> > > can we have preferred console at offset 0 (not at console_cmdline_cnt - 1)
> > > and restore the previous register_console() iteration order?
> >
> > btw, what if someone has configured the system as
> > console= non-braille non-braille braille non-braille?
> > "The last non-braille console is always the preferred one"
> > is not true in this case.
>
> I don't quite follow what you think is problem here.
please ignore it, just a long day at the office. sorry.
-ss
^ permalink raw reply
* Re: [PATCH v5 17/17] net: qualcomm: add QCA7000 UART driver
From: Lino Sanfilippo @ 2017-05-11 21:58 UTC (permalink / raw)
To: Stefan Wahren, Rob Herring, David S. Miller
Cc: Mark Rutland, Greg Kroah-Hartman, Jiri Slaby,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1494406408-31760-18-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
Hi,
On 10.05.2017 10:53, Stefan Wahren wrote:
> +static int qcauart_netdev_init(struct net_device *dev)
> +{
> + struct qcauart *qca = netdev_priv(dev);
> + size_t len;
> +
> + /* Finish setting up the device info. */
> + dev->mtu = QCAFRM_MAX_MTU;
> + dev->type = ARPHRD_ETHER;
> +
> + qca->rx_skb = netdev_alloc_skb_ip_align(qca->net_dev,
> + qca->net_dev->mtu +
> + VLAN_ETH_HLEN);
> + if (!qca->rx_skb)
> + return -ENOBUFS;
> +
> + len = QCAFRM_HEADER_LEN + QCAFRM_MAX_LEN + QCAFRM_FOOTER_LEN;
> + qca->tx_buffer = kmalloc(len, GFP_KERNEL);
> + if (!qca->tx_buffer)
> + return -ENOBUFS;
Freeing the rx_skb is missing here.
> +
> +static void qcauart_netdev_setup(struct net_device *dev)
> +{
> + struct qcauart *qca;
> +
> + dev->netdev_ops = &qcauart_netdev_ops;
> + dev->watchdog_timeo = QCAUART_TX_TIMEOUT;
> + dev->priv_flags &= ~IFF_TX_SKB_SHARING;
> + dev->tx_queue_len = 100;
> +
> + /* MTU range: 46 - 1500 */
> + dev->min_mtu = QCAFRM_MIN_MTU;
> + dev->max_mtu = QCAFRM_MAX_MTU;
> +
> + qca = netdev_priv(dev);
> + memset(qca, 0, sizeof(struct qcauart));
Zeroing the private data is not necessary since it is already done
at device allocation.
> +
> +static int qca_uart_probe(struct serdev_device *serdev)
> +{
> + struct net_device *qcauart_dev = alloc_etherdev(sizeof(struct qcauart));
> + struct qcauart *qca;
> + const char *mac;
> + u32 speed = 115200;
> + int ret;
> +
> + if (!qcauart_dev)
> + return -ENOMEM;
> +
> + qcauart_netdev_setup(qcauart_dev);
> +
> + qca = netdev_priv(qcauart_dev);
> + if (!qca) {
> + pr_err("qca_uart: Fail to retrieve private structure\n");
> + ret = -ENOMEM;
> + goto free;
> + }
> + qca->net_dev = qcauart_dev;
> + qca->serdev = serdev;
> + qcafrm_fsm_init_uart(&qca->frm_handle);
> +
> + spin_lock_init(&qca->lock);
> + INIT_WORK(&qca->tx_work, qcauart_transmit);
> +
> + of_property_read_u32(serdev->dev.of_node, "current-speed", &speed);
> +
> + mac = of_get_mac_address(serdev->dev.of_node);
> +
> + if (mac)
> + ether_addr_copy(qca->net_dev->dev_addr, mac);
> +
> + if (!is_valid_ether_addr(qca->net_dev->dev_addr)) {
> + eth_hw_addr_random(qca->net_dev);
> + dev_info(&serdev->dev, "Using random MAC address: %pM\n",
> + qca->net_dev->dev_addr);
> + }
> +
> + netif_carrier_on(qca->net_dev);
> + serdev_device_set_drvdata(serdev, qca);
> + serdev_device_set_client_ops(serdev, &qca_serdev_ops);
> +
> + ret = serdev_device_open(serdev);
> + if (ret) {
> + dev_err(&serdev->dev, "Unable to open device %s\n",
> + qcauart_dev->name);
> + goto free;
> + }
> +
> + speed = serdev_device_set_baudrate(serdev, speed);
> + dev_info(&serdev->dev, "Using baudrate: %u\n", speed);
> +
> + serdev_device_set_flow_control(serdev, false);
> +
> + ret = register_netdev(qcauart_dev);
> + if (ret) {
> + dev_err(&serdev->dev, "Unable to register net device %s\n",
> + qcauart_dev->name);
serdev_device_close() ?
> + goto free;
> + }
> +
> + return 0;
> +
> +free:
> + free_netdev(qcauart_dev);
> + return ret;
> +}
> +
> +static void qca_uart_remove(struct serdev_device *serdev)
> +{
> + struct qcauart *qca = serdev_device_get_drvdata(serdev);
Here you should make sure that the tx_work is not running/pending any
more (e.g by calling cancel_delayed_work_sync()).
> + /* Flush any pending characters in the driver. */
> + serdev_device_close(serdev);
> +
> + netif_carrier_off(qca->net_dev);
> + unregister_netdev(qca->net_dev);
> + free_netdev(qca->net_dev);
> +}
Regards,
Lino
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Aleksey Makarov @ 2017-05-11 21:17 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Sabrina Dubroca, linux-serial, linux-kernel, Sudeep Holla,
Greg Kroah-Hartman, Peter Hurley, Jiri Slaby, Robin Murphy,
Steven Rostedt, Nair, Jayachandran, Sergey Senozhatsky,
Petr Mladek
In-Reply-To: <20170511113220.GA11507@jagdpanzerIV.localdomain>
On 05/11/2017 02:32 PM, Sergey Senozhatsky wrote:
> On (05/11/17 17:41), Sergey Senozhatsky wrote:
> [..]
>> Alexey,
>> can we have preferred console at offset 0 (not at console_cmdline_cnt - 1)
>> and restore the previous register_console() iteration order?
>
> btw, what if someone has configured the system as
> console= non-braille non-braille braille non-braille?
> "The last non-braille console is always the preferred one"
> is not true in this case.
I don't quite follow what you think is problem here.
The invariant keeps here, the code makes it always true.
In this case the last console is non-braille and it is preferred.
What is the problem?
Thank you
Aleksey Makarov
^ permalink raw reply
* Re: [PATCH v9 3/3] printk: fix double printing with earlycon
From: Aleksey Makarov @ 2017-05-11 21:13 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Sabrina Dubroca, linux-serial, linux-kernel, Sudeep Holla,
Greg Kroah-Hartman, Peter Hurley, Jiri Slaby, Robin Murphy,
Steven Rostedt, Nair, Jayachandran, Sergey Senozhatsky,
Petr Mladek
In-Reply-To: <20170511084158.GB421@jagdpanzerIV.localdomain>
On 05/11/2017 11:41 AM, Sergey Senozhatsky wrote:
> On (05/11/17 17:24), Sergey Senozhatsky wrote:
>> On (05/09/17 10:29), Sabrina Dubroca wrote:
>> [..]
>>> That's caused a change of behavior in my qemu setup, with this cmdline
>>>
>>> root=/dev/sda1 console=ttyS1 console=ttyS0
>>>
>>> Before, the kernel logs appeared on ttyS1, and I logged in with ttyS0
>>> (with my setup, ttyS1 is a file and ttyS0 is unix socket). Now, the
>>> kernel logs go to ttyS0. I need to swap the two console= parameters to
>>> restore behavior.
>>>
>>> There might be some other problem (in qemu?) though, because adding
>>> console=tty0 anywhere on that cmdline makes the logs appear on both
>>> tty0 and one ttyS* (but only one of them, and the ordering of the
>>> ttyS* matters).
>>
>> thanks for the report.
>>
>> so we have ttyS1 first and ttyS0 last.
>> after commit in question, register_console() iterates console_cmdline
>> in reverse order so we see ttyS0 first, then we hit `if (newcon->index < 0)'
>> condition, set newcon to ttyS0, because we iterate in reverse order now, and
>> break out. so we enable ttyS0, instead of ttyS1.
>>
>> previously, we iterated console_cmdline from index 0 and saw ttyS1 first.
>> so the same `if (newcon->index < 0)' condition would set newcone to ttyS1,
>> and, thus, we would enable ttyS1, not ttyS0.
>
> Alexey,
> can we have preferred console at offset 0 (not at console_cmdline_cnt - 1)
> and restore the previous register_console() iteration order?
I don't quite understand what is the problem. Give me more time please.
I hope I will be able to look at this on the weekend.
Thank you
Aleksey Makarov
^ permalink raw reply
* Re: [PATCH v5 13/17] dt-bindings: qca7000: rename binding
From: Michael Heimpold @ 2017-05-11 19:48 UTC (permalink / raw)
To: Stefan Wahren
Cc: Rob Herring, David S. Miller, Mark Rutland, Greg Kroah-Hartman,
Jiri Slaby, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1494406408-31760-14-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
Hi,
Am Mittwoch, 10. Mai 2017, 10:53:24 CEST schrieb Stefan Wahren:
> Before we can merge the QCA7000 UART binding the document needs to be
> renamed.
>
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
> .../devicetree/bindings/net/{qca-qca7000-spi.txt => qca-qca7000.txt} |
> 0 1 file changed, 0 insertions(+), 0 deletions(-)
> rename Documentation/devicetree/bindings/net/{qca-qca7000-spi.txt =>
> qca-qca7000.txt} (100%)
>
> diff --git a/Documentation/devicetree/bindings/net/qca-qca7000-spi.txt
> b/Documentation/devicetree/bindings/net/qca-qca7000.txt similarity index
> 100%
> rename from Documentation/devicetree/bindings/net/qca-qca7000-spi.txt
> rename to Documentation/devicetree/bindings/net/qca-qca7000.txt
I was told here [1], that it is prefered when the filename matchs the
compatible string, so using "qca,qca7000.txt" should be better.
Regards,
Michael
[1]
https://www.spinics.net/lists/devicetree/msg124088.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 15/17] dt-bindings: qca7000: append UART interface to binding
From: Michael Heimpold @ 2017-05-11 19:12 UTC (permalink / raw)
To: Rob Herring
Cc: Stefan Wahren, David S. Miller, Mark Rutland, Greg Kroah-Hartman,
Jiri Slaby, linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1494406408-31760-16-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
Hi,
Am Mittwoch, 10. Mai 2017, 10:53:26 CEST schrieb Stefan Wahren:
> This merges the serdev binding for the QCA7000 UART driver (Ethernet over
> UART) into the existing document.
>
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
> .../devicetree/bindings/net/qca-qca7000.txt | 32
> ++++++++++++++++++++++ 1 file changed, 32 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/qca-qca7000.txt
> b/Documentation/devicetree/bindings/net/qca-qca7000.txt index
> a37f656..08364c3 100644
> --- a/Documentation/devicetree/bindings/net/qca-qca7000.txt
> +++ b/Documentation/devicetree/bindings/net/qca-qca7000.txt
> @@ -54,3 +54,35 @@ ssp2: spi@80014000 {
> local-mac-address = [ A0 B0 C0 D0 E0 F0 ];
> };
> };
> +
> +(b) Ethernet over UART
> +
> +In order to use the QCA7000 as UART slave it must be defined as a child of
> a +UART master in the device tree. It is possible to preconfigure the UART
> +settings of the QCA7000 firmware, but it's not possible to change them
> during +runtime.
> +
> +Required properties:
> +- compatible : Should be "qca,qca7000-uart"
I already discussed this with Stefan off-list a little bit, but I would like
to bring this to a broader audience: I'm not sure whether the compatible
should contain the "-uart" suffix, because the hardware chip is the very same
QCA7000 chip which can also be used with SPI protocol.
The only difference is the loaded firmware within the chip which can either
speak SPI or UART protocol (but not both at the same time - due to shared
pins). So the hardware design decides which interface type is used.
At the moment, this patch series adds a dedicated driver for the UART
protocol, in parallel to the existing SPI driver. So a different compatible
string is needed here to match against the new driver.
An alternative approach would be to re-use the existing compatible string
"qca,qca7000" for both, the SPI and UART protocol, because a "smarter"
(combined) driver would detect which protocol to use. For example the driver
could check for spi-cpha and/or spi-cpol which are required for SPI protocol:
if these exists the driver could assume that SPI must be used, if both are
missing then UART protocol should be used.
(This way it would not be necessary to check whether the node is a child of
a SPI or UART master node - but maybe this is even easier - I don't know)
Or in shorter words: my concern is that while "qca7000-uart" describes the
hardware, it's too closely coupled to the driver implementation. Having
some feedback of the experts would be nice :-)
Thanks,
Michael
> +
> +Optional properties:
> +- local-mac-address : see ./ethernet.txt
> +- current-speed : current baud rate of QCA7000 which defaults to 115200
> + if absent, see also ../serial/slave-device.txt
> +
> +UART Example:
> +
> +/* Freescale i.MX28 UART */
> +auart0: serial@8006a000 {
> + compatible = "fsl,imx28-auart", "fsl,imx23-auart";
> + reg = <0x8006a000 0x2000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&auart0_2pins_a>;
> + status = "okay";
> +
> + qca7000: ethernet {
> + compatible = "qca,qca7000-uart";
> + local-mac-address = [ A0 B0 C0 D0 E0 F0 ];
> + current-speed = <38400>;
> + };
> +};
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox