Sergei Shtylyov-2 wrote:
>
> Hello.
>
> On 29-11-2010 14:06, Subhasish Ghosh wrote:
>
>> The patch adds support for emulated UART controllers
>> on the programmable realtime unit (PRU) available on OMAPL138.
>> This defines the system resource requirements such as pin mux,
>> clock, iomem, interrupt etc and registers the platform device
>> as per the Linux driver model.
>
>> Signed-off-by: Subhasish Ghosh<subhas...@mistralsolutions.com>
> [...]
>
>> diff --git a/arch/arm/mach-davinci/board-da850-evm.c
>> b/arch/arm/mach-davinci/board-da850-evm.c
>> index f89b0b7..4ee09ed 100644
>> --- a/arch/arm/mach-davinci/board-da850-evm.c
>> +++ b/arch/arm/mach-davinci/board-da850-evm.c
>
> The board file should be in a patch of its own.
>
>> diff --git a/arch/arm/mach-davinci/da850.c
>> b/arch/arm/mach-davinci/da850.c
>> index 63916b9..a5eeb4f 100644
>> --- a/arch/arm/mach-davinci/da850.c
>> +++ b/arch/arm/mach-davinci/da850.c
>> @@ -238,6 +238,13 @@ static struct clk tptc2_clk = {
>> .flags = ALWAYS_ENABLED,
>> };
>>
>> +static struct clk pru_clk = {
>> + .name = "pru_ck",
>> + .parent = &pll0_sysclk2,
>> + .lpsc = DA8XX_LPSC0_DMAX,
>> + .flags = ALWAYS_ENABLED,
>
> Why it's always enabled?
> [SG] - As per the L138 Datasheet, this falls under "Always On" domain.
>
>> @@ -392,9 +409,13 @@ static struct clk_lookup da850_clks[] = {
>>
>> /*
>> * Device specific mux setup
>> - *
>> - * soc description mux mode mode mux dbg
>> - * reg offset mask mode
>> + * soc -> DA850
>> + * desc -> Pin name, which evaluates to soc##_##desc.
>> + * muxreg -> Pin Multiplexing Control n (PINMUXn) Register
>> number.
>> + * mode_offset -> Bit offset in the register PINMUXn.
>> + * mode_mask -> Number of bits for Pin Multiplexing Control n.
>> + * mux_mode -> Multiplexing mode to set.
>> + * dbg -> debug on/off
>
> Why change this at all? And doesn't this intersect with the patch
> you've
> just posted?
>
>> @@ -557,6 +578,14 @@ const short da850_uart0_pins[] __initdata = {
>> -1
>> };
>>
>> +const short da850_pru_suart_pins[] __initdata = {
>> + DA850_AHCLKX, DA850_ACLKX, DA850_AFSX,
>> + DA850_AHCLKR, DA850_ACLKR, DA850_AFSR,
>> + DA850_AXR_13, DA850_AXR_9, DA850_AXR_7,
>> + DA850_AXR_14, DA850_AXR_10, DA850_AXR_8,
>
> Don't these pins belong to McASP?
> [SG] -- Yes, To emulate the Soft UART we use the McASP as serializers.
>
>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c
>> b/arch/arm/mach-davinci/devices-da8xx.c
>> index 9eec630..3ae9c3e 100644
>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>> @@ -85,7 +85,107 @@ struct platform_device da8xx_serial_device = {
>> },
>> };
>>
>> -static const s8 da8xx_queue_tc_mapping[][2] = {
>> +
>> +#define OMAPL138_PRU_MEM_BASE 0x01C30000
>
> Isn't these already #define'd in your first patch?
>
>> +static struct resource omapl138_pru_suart_resources[] = {
>> + {
>> + .name = "omapl_pru_suart",
>> + .start = OMAPL138_PRU_MEM_BASE,
>> + .end = OMAPL138_PRU_MEM_BASE + 0xFFFF,
>> + .flags = IORESOURCE_MEM,
>> + },
>> + {
>> + .start = DAVINCI_DA8XX_MCASP0_REG_BASE,
>> + .end = DAVINCI_DA8XX_MCASP0_REG_BASE + (SZ_1K * 12) - 1,
>> + .flags = IORESOURCE_MEM,
>> + },
>> + {
>> + .start = DA8XX_PSC0_BASE,
>> + .end = DA8XX_PSC0_BASE + (SZ_1K * 3) - 1,
>> + .flags = IORESOURCE_MEM,
>> + },
>> + {
>> + .start = DA8XX_PSC1_BASE,
>> + .end = DA8XX_PSC1_BASE + (SZ_1K * 3) - 1,
>> + .flags = IORESOURCE_MEM,
>
> Why do you need PSCs?
> [SG] - These are required by the PRU firmware to enable McASPs.
>
>> + },
>> + {
>> + .start = DA8XX_SHARED_RAM_BASE,
>> + .end = DA8XX_SHARED_RAM_BASE + (SZ_1K * 8) - 1,
>> + .flags = IORESOURCE_MEM,
>> + },
>> + {
>> + .start = OMAPL138_INT_PRU_SUART_1,
>> + .end = OMAPL138_INT_PRU_SUART_1,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + {
>> + .start = OMAPL138_INT_PRU_SUART_2,
>> + .end = OMAPL138_INT_PRU_SUART_2,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + {
>> + .start = OMAPL138_INT_PRU_SUART_3,
>> + .end = OMAPL138_INT_PRU_SUART_3,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + {
>> + .start = OMAPL138_INT_PRU_SUART_4,
>> + .end = OMAPL138_INT_PRU_SUART_4,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + {
>> + .start = OMAPL138_INT_PRU_SUART_5,
>> + .end = OMAPL138_INT_PRU_SUART_5,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + {
>> + .start = OMAPL138_INT_PRU_SUART_6,
>> + .end = OMAPL138_INT_PRU_SUART_6,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + {
>> + .start = OMAPL138_INT_PRU_SUART_7,
>> + .end = OMAPL138_INT_PRU_SUART_7,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + {
>> + .start = OMAPL138_INT_PRU_SUART_8,
>> + .end = OMAPL138_INT_PRU_SUART_8,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> +};
>
> What kind of device is this, with so many subdevices and IRQs?
> [SG] -- We are emulating eight UARTs on the two PRUs available in OMAPL.
> The interrupts are for each of the eight UARTs. For the application,
> the visibility is a single controller emulating eight UARTs.
>
>> +struct platform_device omapl_pru_suart_device = {
>> + .name = "davinci_pru_suart",
>> + .id = 1,
>
> Why 1? Is there another device? I don't see it...
>
>> + .num_resources = ARRAY_SIZE(omapl138_pru_suart_resources),
>> + .resource = omapl138_pru_suart_resources,
>
> Please align the initialzers properly.
>
>> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
>> index 212eb4c..b831ebc 100644
>> --- a/include/linux/serial_core.h
>> +++ b/include/linux/serial_core.h
>> @@ -199,6 +199,9 @@
>> /* TI OMAP-UART */
>> #define PORT_OMAP 96
>>
>> +/* omapl pru uart emulation */
>> +#define OMAPL_PRU_SUART 97
>
> Shouldn't these constants have PORT_ as a prefix?
>
>> diff --git a/include/linux/ti_omapl_pru_suart.h
>> b/include/linux/ti_omapl_pru_suart.h
>> new file mode 100644
>> index 0000000..9179f2c
>> --- /dev/null
>> +++ b/include/linux/ti_omapl_pru_suart.h
>
> I think this file belongs with the driver patch. Also, you could place
> it
> into arch/arm/mach-davinci/include/mach...
> [SG] -- This header file is included in devices-da8xx.c.
>
>> @@ -0,0 +1,38 @@
>> +/*
>> + * linux/include/linux/ti_omapl_pru_suart.h
>
> File names in comments have long been frowned upon.
>
>> + */
>> +#ifndef _LINUX_SUART_OMAPL_PRU_H
>> +#define _LINUX_SUART_OMAPL_PRU_H
>> +
>> +#include<linux/serial_core.h>
>> +#include<linux/platform_device.h>
>> +
>> +/*
>> + * TI OMAPL PRU SUART Emulation device driver
>> + *
>> + * This driver supports TI's PRU SUART Emulation and the
>> + *
>> + * Copyright (C) 2009 Texas Instruments Incorporated -
>> http://www.ti.com/
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + *
>> + * This program is distributed as is WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>
> Copyright should precede the #include's, not follow them.
>
> WBR, Sergei
> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source@linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
>
>
--
View this message in context:
http://davinci-linux-open-source.1494791.n2.nabble.com/RFC-PATCH-1-1-da850-evm-Support-for-TI-s-PRU-SoftUART-Emulation-tp5783929p5784351.html
Sent from the davinci-linux-open-source mailing list archive at Nabble.com.
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
