lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a06e4806-8b5a-4073-96d5-2a37103e572f@yoseli.org>
Date: Wed, 4 Dec 2024 11:58:05 +0100
From: Jean-Michel Hautbois <jeanmichel.hautbois@...eli.org>
To: Greg Ungerer <gerg@...ux-m68k.org>,
 Geert Uytterhoeven <geert@...ux-m68k.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Jiri Slaby <jirislaby@...nel.org>
Cc: linux-m68k@...ts.linux-m68k.org, linux-kernel@...r.kernel.org,
 linux-serial@...r.kernel.org
Subject: Re: [PATCH] m68k: coldfire: Support resources for UART

Hi Greg,

On 04/12/2024 11:54, Greg Ungerer wrote:
> Hi JM,
> 
> 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@...eli.org>
>> ---
>>   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 ?

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,


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ