2011-02-11 14:39:13

by Subhasish Ghosh

[permalink] [raw]
Subject: [PATCH v2 11/13] da850: pruss SUART board specific additions.

This patch adds the pruss SUART pin mux and registers the device
with the pruss mfd driver.

Signed-off-by: Subhasish Ghosh <[email protected]>
---
arch/arm/mach-davinci/board-da850-evm.c | 36 +++++++++++++++++++++++++++++++
1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
index f9c38f8..3858516 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -1060,6 +1060,25 @@ const short da850_evm_pruss_can_pins[] = {
-1
};

+const short da850_evm_pruss_suart_pins[] = {
+ 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,
+ -1
+};
+
+static int __init da850_evm_setup_pruss_suart(void)
+{
+ int ret;
+
+ ret = davinci_cfg_reg_list(da850_evm_pruss_suart_pins);
+ if (ret)
+ pr_warning("%s: da850_evm_pruss_suart_pins "
+ "mux setup failed: %d\n", __func__, ret);
+ return ret;
+}
+
static int __init da850_evm_setup_pruss_can(void)
{
int ret, val = 0;
@@ -1085,6 +1104,17 @@ static int __init da850_evm_setup_pruss_can(void)
return ret;
}

+static struct da850_evm_pruss_suart_data suart_data = {
+ .version = 1,
+ .resource = {
+ .name = "da8xx_mcasp0_iomem",
+ .start = DAVINCI_DA8XX_MCASP0_REG_BASE,
+ .end = DAVINCI_DA8XX_MCASP0_REG_BASE +
+ (SZ_1K * 12) - 1,
+ .flags = IORESOURCE_MEM,
+ },
+};
+
static struct da8xx_pruss_can_data can_data = {
.version = 1,
};
@@ -1094,6 +1124,12 @@ static struct da8xx_pruss_devices pruss_devices[] = {
.dev_name = "da8xx_pruss_can",
.pdata = &can_data,
.pdata_size = sizeof(can_data),
+ .setup = da850_evm_setup_pruss_suart,
+ },
+ {
+ .dev_name = "da8xx_pruss_uart",
+ .pdata = &suart_data,
+ .pdata_size = sizeof(suart_data),
.setup = da850_evm_setup_pruss_can,
},
{
--
1.7.2.3


2011-02-11 15:26:38

by Michael Williamson

[permalink] [raw]
Subject: Re: [PATCH v2 11/13] da850: pruss SUART board specific additions.

Hi Subhasish,

On 2/11/2011 9:51 AM, Subhasish Ghosh wrote:

> This patch adds the pruss SUART pin mux and registers the device
> with the pruss mfd driver.
>
> Signed-off-by: Subhasish Ghosh <[email protected]>
> ---
> arch/arm/mach-davinci/board-da850-evm.c | 36 +++++++++++++++++++++++++++++++
> 1 files changed, 36 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
> index f9c38f8..3858516 100644
> --- a/arch/arm/mach-davinci/board-da850-evm.c
> +++ b/arch/arm/mach-davinci/board-da850-evm.c
> @@ -1060,6 +1060,25 @@ const short da850_evm_pruss_can_pins[] = {
> -1
> };
>
> +const short da850_evm_pruss_suart_pins[] = {
> + 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,
> + -1
> +};
> +


Shouldn't this pins select PRU[0,1]_XXX type functions and not McASP functions?
E.G.: PRU0_R31[17] instead of AHCLKX, PRU0_R31[18] instead of AHCLKR, etc.

> +static int __init da850_evm_setup_pruss_suart(void)
> +{
> + int ret;
> +
> + ret = davinci_cfg_reg_list(da850_evm_pruss_suart_pins);
> + if (ret)
> + pr_warning("%s: da850_evm_pruss_suart_pins "
> + "mux setup failed: %d\n", __func__, ret);
> + return ret;
> +}
> +
> static int __init da850_evm_setup_pruss_can(void)
> {
> int ret, val = 0;
> @@ -1085,6 +1104,17 @@ static int __init da850_evm_setup_pruss_can(void)
> return ret;
> }
>
> +static struct da850_evm_pruss_suart_data suart_data = {
> + .version = 1,
> + .resource = {
> + .name = "da8xx_mcasp0_iomem",
> + .start = DAVINCI_DA8XX_MCASP0_REG_BASE,
> + .end = DAVINCI_DA8XX_MCASP0_REG_BASE +
> + (SZ_1K * 12) - 1,
> + .flags = IORESOURCE_MEM,
> + },
> +};
> +
> static struct da8xx_pruss_can_data can_data = {
> .version = 1,
> };
> @@ -1094,6 +1124,12 @@ static struct da8xx_pruss_devices pruss_devices[] = {
> .dev_name = "da8xx_pruss_can",
> .pdata = &can_data,
> .pdata_size = sizeof(can_data),
> + .setup = da850_evm_setup_pruss_suart,


Should this be da850_evm_setup_pruss_can instead?

> + },
> + {
> + .dev_name = "da8xx_pruss_uart",
> + .pdata = &suart_data,
> + .pdata_size = sizeof(suart_data),
> .setup = da850_evm_setup_pruss_can,


Should this be da850_evm_setup_pruss_suart instead?

> },
> {


-Mike

2011-02-11 18:51:49

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH v2 11/13] da850: pruss SUART board specific additions.

Subhasish Ghosh wrote:

> This patch adds the pruss SUART pin mux and registers the device
> with the pruss mfd driver.

> Signed-off-by: Subhasish Ghosh <[email protected]>
> ---
> arch/arm/mach-davinci/board-da850-evm.c | 36 +++++++++++++++++++++++++++++++
> 1 files changed, 36 insertions(+), 0 deletions(-)

> diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
> index f9c38f8..3858516 100644
> --- a/arch/arm/mach-davinci/board-da850-evm.c
> +++ b/arch/arm/mach-davinci/board-da850-evm.c
[...]
> @@ -1085,6 +1104,17 @@ static int __init da850_evm_setup_pruss_can(void)
> return ret;
> }
>
> +static struct da850_evm_pruss_suart_data suart_data = {
> + .version = 1,
> + .resource = {
> + .name = "da8xx_mcasp0_iomem",
> + .start = DAVINCI_DA8XX_MCASP0_REG_BASE,
> + .end = DAVINCI_DA8XX_MCASP0_REG_BASE +
> + (SZ_1K * 12) - 1,
> + .flags = IORESOURCE_MEM,
> + },
> +};
> +

I don't think passing a resource thru platform data is good idea...
Resources should be bound to the platform device.

WBR, Sergei

2011-02-18 07:12:31

by Subhasish Ghosh

[permalink] [raw]
Subject: Re: [PATCH v2 11/13] da850: pruss SUART board specific additions.

--------------------------------------------------
From: "Michael Williamson" <[email protected]>
Sent: Friday, February 11, 2011 8:56 PM
To: "Subhasish Ghosh" <[email protected]>
Cc: <[email protected]>;
<[email protected]>; "Russell King" <[email protected]>;
"Kevin Hilman" <[email protected]>; "open list"
<[email protected]>; <[email protected]>;
<[email protected]>
Subject: Re: [PATCH v2 11/13] da850: pruss SUART board specific additions.

> Hi Subhasish,
>
> On 2/11/2011 9:51 AM, Subhasish Ghosh wrote:
>
>> This patch adds the pruss SUART pin mux and registers the device
>> with the pruss mfd driver.
>>
>> Signed-off-by: Subhasish Ghosh <[email protected]>
>> ---
>> arch/arm/mach-davinci/board-da850-evm.c | 36
>> +++++++++++++++++++++++++++++++
>> 1 files changed, 36 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/board-da850-evm.c
>> b/arch/arm/mach-davinci/board-da850-evm.c
>> index f9c38f8..3858516 100644
>> --- a/arch/arm/mach-davinci/board-da850-evm.c
>> +++ b/arch/arm/mach-davinci/board-da850-evm.c
>> @@ -1060,6 +1060,25 @@ const short da850_evm_pruss_can_pins[] = {
>> -1
>> };
>>
>> +const short da850_evm_pruss_suart_pins[] = {
>> + 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,
>> + -1
>> +};
>> +
>
>
> Shouldn't this pins select PRU[0,1]_XXX type functions and not McASP
> functions?
> E.G.: PRU0_R31[17] instead of AHCLKX, PRU0_R31[18] instead of AHCLKR, etc.
>

SG - The Soft-UART implementation uses the McASP as shift registers to push
out the data sequentially.
Hence, we configure the McASP PINS and not the PRU PINS.

>> +static int __init da850_evm_setup_pruss_suart(void)
>> +{
>> + int ret;
>> +
>> + ret = davinci_cfg_reg_list(da850_evm_pruss_suart_pins);
>> + if (ret)
>> + pr_warning("%s: da850_evm_pruss_suart_pins "
>> + "mux setup failed: %d\n", __func__, ret);
>> + return ret;
>> +}
>> +
>> static int __init da850_evm_setup_pruss_can(void)
>> {
>> int ret, val = 0;
>> @@ -1085,6 +1104,17 @@ static int __init da850_evm_setup_pruss_can(void)
>> return ret;
>> }
>>
>> +static struct da850_evm_pruss_suart_data suart_data = {
>> + .version = 1,
>> + .resource = {
>> + .name = "da8xx_mcasp0_iomem",
>> + .start = DAVINCI_DA8XX_MCASP0_REG_BASE,
>> + .end = DAVINCI_DA8XX_MCASP0_REG_BASE +
>> + (SZ_1K * 12) - 1,
>> + .flags = IORESOURCE_MEM,
>> + },
>> +};
>> +
>> static struct da8xx_pruss_can_data can_data = {
>> .version = 1,
>> };
>> @@ -1094,6 +1124,12 @@ static struct da8xx_pruss_devices pruss_devices[]
>> = {
>> .dev_name = "da8xx_pruss_can",
>> .pdata = &can_data,
>> .pdata_size = sizeof(can_data),
>> + .setup = da850_evm_setup_pruss_suart,
>
>
> Should this be da850_evm_setup_pruss_can instead?

SG - This just the way the patch is displayed. In the code the order is
correct.

>
>> + },
>> + {
>> + .dev_name = "da8xx_pruss_uart",
>> + .pdata = &suart_data,
>> + .pdata_size = sizeof(suart_data),
>> .setup = da850_evm_setup_pruss_can,
>
>
> Should this be da850_evm_setup_pruss_suart instead?
SG - Ditto
>
>> },
>> {
>
>
> -Mike
>

2011-02-22 06:26:20

by Subhasish Ghosh

[permalink] [raw]
Subject: Re: [PATCH v2 11/13] da850: pruss SUART board specific additions.

Hello,

Have modified this as shown below:
Please let me know if this looks ok.

diff --git a/arch/arm/mach-davinci/board-da850-evm.c
b/arch/arm/mach-davinci/board-da850-evm.c
+static struct da850_evm_pruss_can_data can_data = {
.version = 1,
};

+static struct resource da850_evm_suart_resource[] =
------------------------->>> Added a separate resource structure for
suart.
+ {
+ .name = "da8xx_mcasp0_iomem",
+ .start = DAVINCI_DA8XX_MCASP0_REG_BASE,
+ .end = DAVINCI_DA8XX_MCASP0_REG_BASE +
+ (SZ_1K * 12) - 1,
+ .flags = IORESOURCE_MEM,
+ },
+};
+
static struct da8xx_pruss_devices pruss_devices[] = {
{
.dev_name = "da8xx_pruss_can",
.pdata = &can_data,
.pdata_size = sizeof(can_data),
.setup = da850_evm_setup_pruss_can,
+ .num_resources = 0,
+ .resources = NULL,
},
{
.dev_name = "da8xx_pruss_uart",
.pdata = &suart_data,
.pdata_size = sizeof(suart_data),
.setup = da850_evm_setup_pruss_suart,
+ .num_resources = ARRAY_SIZE(da850_evm_suart_resource),
+ .resources =
a850_evm_suart_resource, ---------------------------- >>>>> Initialized it
here
},
{
.dev_name = NULL,


diff --git a/drivers/mfd/da8xx_pru.c b/drivers/mfd/da8xx_pru.c
index f7868a4..140bf14 100644
--- a/drivers/mfd/da8xx_pru.c
+++ b/drivers/mfd/da8xx_pru.c
@@ -332,6 +332,8 @@ static int pruss_mfd_add_devices(struct platform_device
*pdev)
cell.name = (dev_data + count)->dev_name;
cell.platform_data = (dev_data + count)->pdata;
cell.data_size = (dev_data + count)->pdata_size;
+ cell.resources = (dev_data +
ount)->resources; ---------------------->>>Modified the MFD driver to add
the resources.
+ cell.num_resources = (dev_data + count)->num_resources;


diff --git a/drivers/tty/serial/da8xx_pruss/pruss_suart.c
b/drivers/tty/serial/da8xx_pruss/pruss_suart.c

+ res = platform_get_resource(pdev, IORESOURCE_MEM,
0); ------------------>> Used platform infrastructure to get the data.
+ if (!res) {
+ dev_err(&pdev->dev, "Failed to get resource");
+ return -ENOMEM;
+ }
+
+ if (!request_mem_region(res->start,
+ resource_size(res),



--------------------------------------------------
From: "Sergei Shtylyov" <[email protected]>
Sent: Saturday, February 12, 2011 12:20 AM
To: "Subhasish Ghosh" <[email protected]>
Cc: <[email protected]>;
<[email protected]>; "Russell King" <[email protected]>;
"Kevin Hilman" <[email protected]>; <[email protected]>; "open list"
<[email protected]>; <[email protected]>;
<[email protected]>
Subject: Re: [PATCH v2 11/13] da850: pruss SUART board specific additions.

> Subhasish Ghosh wrote:
>
>> This patch adds the pruss SUART pin mux and registers the device
>> with the pruss mfd driver.
>
>> Signed-off-by: Subhasish Ghosh <[email protected]>
>> ---
>> arch/arm/mach-davinci/board-da850-evm.c | 36
>> +++++++++++++++++++++++++++++++
>> 1 files changed, 36 insertions(+), 0 deletions(-)
>
>> diff --git a/arch/arm/mach-davinci/board-da850-evm.c
>> b/arch/arm/mach-davinci/board-da850-evm.c
>> index f9c38f8..3858516 100644
>> --- a/arch/arm/mach-davinci/board-da850-evm.c
>> +++ b/arch/arm/mach-davinci/board-da850-evm.c
> [...]
>> @@ -1085,6 +1104,17 @@ static int __init da850_evm_setup_pruss_can(void)
>> return ret;
>> }
>> +static struct da850_evm_pruss_suart_data suart_data = {
>> + .version = 1,
>> + .resource = {
>> + .name = "da8xx_mcasp0_iomem",
>> + .start = DAVINCI_DA8XX_MCASP0_REG_BASE,
>> + .end = DAVINCI_DA8XX_MCASP0_REG_BASE +
>> + (SZ_1K * 12) - 1,
>> + .flags = IORESOURCE_MEM,
>> + },
>> +};
>> +
>
> I don't think passing a resource thru platform data is good idea...
> Resources should be bound to the platform device.
>
> WBR, Sergei