Re: [PATCH] m68k: coldfire: Support resources for UART

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Greg,

On 04/12/2024 12:15, Greg Ungerer wrote:
Hi JM,

On 4/12/24 20:58, Jean-Michel Hautbois wrote:
On 04/12/2024 11:54, Greg Ungerer wrote:
On 2/12/24 20:34, Jean-Michel Hautbois wrote:
In order to use the eDMA channels for UART, the mcf_platform_uart needs
to be changed. Instead of adding another custom member for the
structure, use a resource tree in a platform_device per UART. It then
makes it possible to have a device named like "mcfuart.N" with N the
UART number.

Later, adding the dma channel in the mcf tty driver will also be more
straightfoward.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@xxxxxxxxxx>
---
  arch/m68k/coldfire/device.c | 96 +++++++++++++ +-------------------------------
  drivers/tty/serial/mcf.c    | 69 +++++++++++++++++++-------------
  2 files changed, 70 insertions(+), 95 deletions(-)

diff --git a/arch/m68k/coldfire/device.c b/arch/m68k/coldfire/device.c
index b6958ec2a220cf91a78a14fc7fa18749451412f7..fd7d0b0ce7eb2970cb8ffe33589fe8d7e88c268d 100644
--- a/arch/m68k/coldfire/device.c
+++ b/arch/m68k/coldfire/device.c
@@ -24,73 +24,35 @@
  #include <linux/platform_data/dma-mcf-edma.h>
  #include <linux/platform_data/mmc-esdhc-mcf.h>
-/*
- *    All current ColdFire parts contain from 2, 3, 4 or 10 UARTS.
- */
-static struct mcf_platform_uart mcf_uart_platform_data[] = {
-    {
-        .mapbase    = MCFUART_BASE0,
-        .irq        = MCF_IRQ_UART0,
-    },
-    {
-        .mapbase    = MCFUART_BASE1,
-        .irq        = MCF_IRQ_UART1,
-    },
-#ifdef MCFUART_BASE2
-    {
-        .mapbase    = MCFUART_BASE2,
-        .irq        = MCF_IRQ_UART2,
-    },
-#endif
-#ifdef MCFUART_BASE3
-    {
-        .mapbase    = MCFUART_BASE3,
-        .irq        = MCF_IRQ_UART3,
-    },
-#endif
-#ifdef MCFUART_BASE4
-    {
-        .mapbase    = MCFUART_BASE4,
-        .irq        = MCF_IRQ_UART4,
-    },
-#endif
-#ifdef MCFUART_BASE5
-    {
-        .mapbase    = MCFUART_BASE5,
-        .irq        = MCF_IRQ_UART5,
-    },
-#endif
-#ifdef MCFUART_BASE6
-    {
-        .mapbase    = MCFUART_BASE6,
-        .irq        = MCF_IRQ_UART6,
-    },
-#endif
-#ifdef MCFUART_BASE7
-    {
-        .mapbase    = MCFUART_BASE7,
-        .irq        = MCF_IRQ_UART7,
+static u64 mcf_uart_mask = DMA_BIT_MASK(32);
+
+static struct resource mcf_uart0_resource[] = {
+    [0] = {
+        .start = MCFUART_BASE0,
+        .end   = MCFUART_BASE0 + 0x3fff,
+        .flags = IORESOURCE_MEM,
      },
-#endif
-#ifdef MCFUART_BASE8
-    {
-        .mapbase    = MCFUART_BASE8,
-        .irq        = MCF_IRQ_UART8,
+    [1] = {
+        .start = 2,
+        .end   = 3,
+        .flags = IORESOURCE_DMA,
      },
-#endif
-#ifdef MCFUART_BASE9
-    {
-        .mapbase    = MCFUART_BASE9,
-        .irq        = MCF_IRQ_UART9,
+    [2] = {
+        .start = MCF_IRQ_UART0,
+        .end   = MCF_IRQ_UART0,
+        .flags = IORESOURCE_IRQ,
      },
-#endif
-    { },
  };
-static struct platform_device mcf_uart = {
+static struct platform_device mcf_uart0 = {
      .name            = "mcfuart",
      .id            = 0,
-    .dev.platform_data    = mcf_uart_platform_data,
+    .num_resources = ARRAY_SIZE(mcf_uart0_resource),
+    .resource = mcf_uart0_resource,
+    .dev = {
+        .dma_mask = &mcf_uart_mask,
+        .coherent_dma_mask = DMA_BIT_MASK(32),
+    },
  };
  #ifdef MCFFEC_BASE0
@@ -485,12 +447,12 @@ static struct platform_device mcf_i2c5 = {
  static const struct dma_slave_map mcf_edma_map[] = {
      { "dreq0", "rx-tx", MCF_EDMA_FILTER_PARAM(0) },
      { "dreq1", "rx-tx", MCF_EDMA_FILTER_PARAM(1) },
-    { "uart.0", "rx", MCF_EDMA_FILTER_PARAM(2) },
-    { "uart.0", "tx", MCF_EDMA_FILTER_PARAM(3) },
-    { "uart.1", "rx", MCF_EDMA_FILTER_PARAM(4) },
-    { "uart.1", "tx", MCF_EDMA_FILTER_PARAM(5) },
-    { "uart.2", "rx", MCF_EDMA_FILTER_PARAM(6) },
-    { "uart.2", "tx", MCF_EDMA_FILTER_PARAM(7) },
+    { "mcfuart.0", "rx", MCF_EDMA_FILTER_PARAM(2) },
+    { "mcfuart.0", "tx", MCF_EDMA_FILTER_PARAM(3) },
+    { "mcfuart.1", "rx", MCF_EDMA_FILTER_PARAM(4) },
+    { "mcfuart.1", "tx", MCF_EDMA_FILTER_PARAM(5) },
+    { "mcfuart.2", "rx", MCF_EDMA_FILTER_PARAM(6) },
+    { "mcfuart.2", "tx", MCF_EDMA_FILTER_PARAM(7) },
      { "timer0", "rx-tx", MCF_EDMA_FILTER_PARAM(8) },
      { "timer1", "rx-tx", MCF_EDMA_FILTER_PARAM(9) },
      { "timer2", "rx-tx", MCF_EDMA_FILTER_PARAM(10) },
@@ -623,7 +585,7 @@ static struct platform_device mcf_flexcan0 = {
  #endif /* MCFFLEXCAN_SIZE */
  static struct platform_device *mcf_devices[] __initdata = {
-    &mcf_uart,
+    &mcf_uart0,
  #ifdef MCFFEC_BASE0
      &mcf_fec0,
  #endif
diff --git a/drivers/tty/serial/mcf.c b/drivers/tty/serial/mcf.c
index 93e7dda4d39acd23daf8c0d4c29ac8d666f263c5..07b8decfdb6005f0265dd130765e45c3fd1715eb 100644
--- a/drivers/tty/serial/mcf.c
+++ b/drivers/tty/serial/mcf.c
@@ -570,31 +570,46 @@ static struct uart_driver mcf_driver = {
  static int mcf_probe(struct platform_device *pdev)
  {
-    struct mcf_platform_uart *platp = dev_get_platdata(&pdev->dev);
      struct uart_port *port;
-    int i;
-
-    for (i = 0; ((i < MCF_MAXPORTS) && (platp[i].mapbase)); i++) {
-        port = &mcf_ports[i].port;
-
-        port->line = i;
-        port->type = PORT_MCF;
-        port->mapbase = platp[i].mapbase;
-        port->membase = (platp[i].membase) ? platp[i].membase :
-            (unsigned char __iomem *) platp[i].mapbase;
-        port->dev = &pdev->dev;
-        port->iotype = SERIAL_IO_MEM;
-        port->irq = platp[i].irq;
-        port->uartclk = MCF_BUSCLK;
-        port->ops = &mcf_uart_ops;
-        port->flags = UPF_BOOT_AUTOCONF;
-        port->rs485_config = mcf_config_rs485;
-        port->rs485_supported = mcf_rs485_supported;
-        port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_MCF_CONSOLE);
-
-        uart_add_one_port(&mcf_driver, port);
+    struct mcf_uart *pp;
+    struct resource *res;
+    void __iomem *base;
+    int id = pdev->id;
+
+    if (id == -1 || id >= MCF_MAXPORTS) {
+        dev_err(&pdev->dev, "uart%d out of range\n",
+            id);
+        return -EINVAL;
      }
+    port = &mcf_ports[id].port;
+    port->line = id;
+
+    base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+    if (IS_ERR(base))
+        return PTR_ERR(base);
+
+    port->mapbase = res->start;
+    port->membase = base;
+
+    port->irq = platform_get_irq(pdev, 0);
+    if (port->irq < 0)
+        return port->irq;
+
+    port->type = PORT_MCF;
+    port->dev = &pdev->dev;
+    port->iotype = SERIAL_IO_MEM;
+    port->uartclk = MCF_BUSCLK;
+    port->ops = &mcf_uart_ops;
+    port->flags = UPF_BOOT_AUTOCONF;
+    port->rs485_config = mcf_config_rs485;
+    port->rs485_supported = mcf_rs485_supported;
+    port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_MCF_CONSOLE);
+
+    pp = container_of(port, struct mcf_uart, port);
+
+    uart_add_one_port(&mcf_driver, port);
+

This breaks platforms with more than one UART - which is quite a few of
the ColdFire platforms. Numerous boards bring and use more than one UART.

I don't get why, as I have two uarts here, and each is detected properly when declaring those in my platform ? I get that it breaks existing detection (we are parsing all uarts even when only one or two is used) but it does not prevent it to work ?

Building and testing on an M5208EVB platform.
With original un-modified code boot console shows:

...
[    0.110000] romfs: ROMFS MTD (C) 2007 Red Hat, Inc.
[    0.110000] ColdFire internal UART serial driver
[    0.110000] mcfuart.0: ttyS0 at MMIO 0xfc060000 (irq = 90, base_baud = 5208333) is a ColdFire UART
[    0.120000] printk: legacy console [ttyS0] enabled
[    0.120000] mcfuart.0: ttyS1 at MMIO 0xfc064000 (irq = 91, base_baud = 5208333) is a ColdFire UART [    0.120000] mcfuart.0: ttyS2 at MMIO 0xfc068000 (irq = 92, base_baud = 5208333) is a ColdFire UART
[    0.130000] brd: module loaded
...


But with this change applied only the first port is probed:

...
[    0.120000] romfs: ROMFS MTD (C) 2007 Red Hat, Inc.
[    0.120000] ColdFire internal UART serial driver
[    0.130000] mcfuart.0: ttyS0 at MMIO 0xfc060000 (irq = 90, base_baud = 5208333) is a ColdFire UART
[    0.130000] printk: legacy console [ttyS0] enabled
[    0.130000] brd: module loaded
...

OK, I see what you mean. Let me try to explain why I did it :-).

The idea is to avoid probing a UART device which may exist as such on the core, but not be used as UART at all (on my board, for instance, I have uart2 and uart6, I don't need any other UART to be probed).

So, based on what I think is the dts philosophy, you declare the devices you really need to probe ?

I can add all the uarts as resources with the ifdefs like before, but on the M54418 it will always probe 10 devices, which sounds like a bit overkill ?

Thanks !
JM


Regards
Greg



static struct resource mcf_uart2_resource[] = {
     [0] = {
         .start = MCFUART_BASE2,
         .end   = MCFUART_BASE2 + 0x3fff,
         .flags = IORESOURCE_MEM,
     },
     [1] = {
         .start = 6,
         .end   = 7,
         .flags = IORESOURCE_DMA,
     },
     [2] = {
         .start = MCF_IRQ_UART2,
         .end   = MCF_IRQ_UART2,
         .flags = IORESOURCE_IRQ,
     },
};

static struct platform_device mcf_uart2 = {
     .name            = "mcfuart",
     .id            = 2,
     .num_resources = ARRAY_SIZE(mcf_uart2_resource),
     .resource = mcf_uart2_resource,
     .dev = {
         .dma_mask = &mcf_uart_mask,
         .coherent_dma_mask = DMA_BIT_MASK(32),
     },
};

static struct resource mcf_uart6_resource[] = {
     [0] = {
         .start = MCFUART_BASE6,
         .end   = MCFUART_BASE6 + 0x3fff,
         .flags = IORESOURCE_MEM,
     },
     [1] = {
         .start = 22,
         .end   = 23,
         .flags = IORESOURCE_DMA,
     },
     [2] = {
         .start = MCF_IRQ_UART6,
         .end   = MCF_IRQ_UART6,
         .flags = IORESOURCE_IRQ,
     },
};

static struct platform_device mcf_uart6 = {
     .name            = "mcfuart",
     .id            = 6,
     .num_resources = ARRAY_SIZE(mcf_uart6_resource),
     .resource = mcf_uart6_resource,
     .dev = {
         .dma_mask = &mcf_uart_mask,
         .coherent_dma_mask = DMA_BIT_MASK(32),
     },
};

JM


Regards
Greg



      return 0;
  }
@@ -603,13 +618,11 @@ static int mcf_probe(struct platform_device *pdev)
  static void mcf_remove(struct platform_device *pdev)
  {
      struct uart_port *port;
-    int i;
+    int id = pdev->id;
-    for (i = 0; (i < MCF_MAXPORTS); i++) {
-        port = &mcf_ports[i].port;
-        if (port)
-            uart_remove_one_port(&mcf_driver, port);
-    }
+    port = &mcf_ports[id].port;
+    if (port)
+        uart_remove_one_port(&mcf_driver, port);
  }
  / ****************************************************************************/

---
base-commit: e457f18d7f25288d143c1fe024a620d0b15caec1
change-id: 20241202-m5441x_uart_resource-729b30c15363

Best regards,






[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux