Hi,
On Fri, Jul 13, 2018 at 04:17:35PM -0600, Karthikeyan Ramasubramanian wrote:
> From: Girish Mahadevan <girishm@xxxxxxxxxxxxxx>
>
> Add support for flow control functionality in the GENI serial driver
> and also support for non-console higher baud rate(upto 4Mbps) usecases.
It seems this patch conflates three unrelated features: flow control,
higher baudrates and loopback mode. This should probably be a series
of 3 patches.
> Signed-off-by: Girish Mahadevan <girishm@xxxxxxxxxxxxxx>
> Signed-off-by: Mohammed Khajapasha <mkhaja@xxxxxxxxxxxxxx>
> Signed-off-by: Karthikeyan Ramasubramanian <kramasub@xxxxxxxxxxxxxx>
> ---
> .../devicetree/bindings/soc/qcom/qcom,geni-se.txt | 2 +-
> drivers/tty/serial/qcom_geni_serial.c | 261 ++++++++++++++++++---
> 2 files changed, 231 insertions(+), 32 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> index d330c73..a9eeca2 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> @@ -46,7 +46,7 @@ Child nodes should conform to I2C bus binding as described in i2c.txt.
> Qualcomm Technologies Inc. GENI Serial Engine based UART Controller
>
> Required properties:
> -- compatible: Must be "qcom,geni-debug-uart".
> +- compatible: Must be "qcom,geni-debug-uart" or "qcom,geni-uart".
> - reg: Must contain UART register location and length.
> - interrupts: Must contain UART core interrupts.
> - clock-names: Must contain "se".
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index c62e17c..29ec343 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
>
> ...
>
> +static struct qcom_geni_serial_port qcom_geni_uart_ports[GENI_UART_PORTS] = {
> + [0] = {
> + .uport = {
> + .iotype = UPIO_MEM,
> + .ops = &qcom_geni_uart_pops,
> + .flags = UPF_BOOT_AUTOCONF,
> + .line = 0,
> + },
> + },
> + [1] = {
> + .uport = {
> + .iotype = UPIO_MEM,
> + .ops = &qcom_geni_uart_pops,
> + .flags = UPF_BOOT_AUTOCONF,
> + .line = 1,
> + },
> + },
> + [2] = {
> + .uport = {
> + .iotype = UPIO_MEM,
> + .ops = &qcom_geni_uart_pops,
> + .flags = UPF_BOOT_AUTOCONF,
> + .line = 2,
> + },
> + },
> +};
Is there any particular reason to limit this to 3 ports instead of
initializing things dynamically?
> +static ssize_t loopback_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct qcom_geni_serial_port *port = platform_get_drvdata(pdev);
> +
> + return snprintf(buf, sizeof(u32), "%d\n", port->loopback);
Why sizeof(u32), i.e. 4? The max value of an u32 is 4294967295, however
.store() limts the max to MAX_LOOPBACK_CFG (aka 3), so a length of 2
should do.
> static int qcom_geni_serial_probe(struct platform_device *pdev)
> @@ -1091,13 +1257,23 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> struct uart_port *uport;
> struct resource *res;
> int irq;
> + bool console = false;
> + struct uart_driver *drv;
>
> - if (pdev->dev.of_node)
> - line = of_alias_get_id(pdev->dev.of_node, "serial");
> + if (of_device_is_compatible(pdev->dev.of_node, "qcom,geni-debug-uart"))
> + console = true;
>
> - if (line < 0 || line >= GENI_UART_CONS_PORTS)
> - return -ENXIO;
> - port = get_port_from_line(line);
> + if (pdev->dev.of_node) {
Is this check needed? Can initialization be driven by anything other
than the DT?
Cheers
Matthias
--
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
