[PATCH 01/11 RESEND] ARM: OMAP: DRA7: hwmod: Add data for McASP3

[PATCH 01/11 RESEND] ARM: OMAP: DRA7: hwmod: Add data for McASP3

[Peter Ujfalusi]

McASP3 is used by default on DRA7x based boards for audio.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
Hi Paul,

this patch is part of my earlier series and as Tony suggested I'll resend the
hwmod patch for you to review since I missed you from the TO in the series.

The original series:
https://www.mail-archive.com/linux-omap at vger.kernel.org/msg119319.html

Regards,
Peter

 arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 41 +++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index 562247bced49..c38b7fa30c27 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -1298,6 +1298,38 @@ static struct omap_hwmod dra7xx_mcspi4_hwmod = {
 };
 
 /*
+ * 'mcasp' class
+ *
+ */
+static struct omap_hwmod_class_sysconfig dra7xx_mcasp_sysc = {
+	.sysc_offs	= 0x0004,
+	.sysc_flags	= SYSC_HAS_SIDLEMODE,
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type3,
+};
+
+static struct omap_hwmod_class dra7xx_mcasp_hwmod_class = {
+	.name	= "mcasp",
+	.sysc	= &dra7xx_mcasp_sysc,
+};
+
+/* mcasp3 */
+static struct omap_hwmod dra7xx_mcasp3_hwmod = {
+	.name		= "mcasp3",
+	.class		= &dra7xx_mcasp_hwmod_class,
+	.clkdm_name	= "l4per2_clkdm",
+	.main_clk	= "mcasp3_ahclkx_mux",
+	.flags		= HWMOD_SWSUP_SIDLE,
+	.prcm = {
+		.omap4 = {
+			.clkctrl_offs = DRA7XX_CM_L4PER2_MCASP3_CLKCTRL_OFFSET,
+			.context_offs = DRA7XX_RM_L4PER2_MCASP3_CONTEXT_OFFSET,
+			.modulemode   = MODULEMODE_SWCTRL,
+		},
+	},
+};
+
+/*
  * 'mmc' class
  *
  */
@@ -2566,6 +2598,14 @@ static struct omap_hwmod_ocp_if dra7xx_l3_main_1__hdmi = {
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
+/* l4_per2 -> mcasp3 */
+static struct omap_hwmod_ocp_if dra7xx_l4_per2__mcasp3 = {
+	.master		= &dra7xx_l4_per2_hwmod,
+	.slave		= &dra7xx_mcasp3_hwmod,
+	.clk		= "l3_iclk_div",
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
 static struct omap_hwmod_addr_space dra7xx_elm_addrs[] = {
 	{
 		.pa_start	= 0x48078000,
@@ -3338,6 +3378,7 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = {
 	&dra7xx_l4_wkup__dcan1,
 	&dra7xx_l4_per2__dcan2,
 	&dra7xx_l4_per2__cpgmac0,
+	&dra7xx_l4_per2__mcasp3,
 	&dra7xx_gmac__mdio,
 	&dra7xx_l4_cfg__dma_system,
 	&dra7xx_l3_main_1__dss,
-- 
2.5.0

[PATCH 01/11 RESEND] ARM: OMAP: DRA7: hwmod: Add data for McASP3

[Peter Ujfalusi]

Paul,

On 09/15/2015 05:10 PM, Peter Ujfalusi wrote:
> McASP3 is used by default on DRA7x based boards for audio.

Can you take a look at this patch? It would be great to have this one reviewed
so the audio support for the dra7 family could be applied by Tony.

Thanks,
P?ter

> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
> Hi Paul,
> 
> this patch is part of my earlier series and as Tony suggested I'll resend the
> hwmod patch for you to review since I missed you from the TO in the series.
> 
> The original series:
> https://www.mail-archive.com/linux-omap at vger.kernel.org/msg119319.html
> 
> Regards,
> Peter
> 
>  arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 41 +++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index 562247bced49..c38b7fa30c27 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1298,6 +1298,38 @@ static struct omap_hwmod dra7xx_mcspi4_hwmod = {
>  };
>  
>  /*
> + * 'mcasp' class
> + *
> + */
> +static struct omap_hwmod_class_sysconfig dra7xx_mcasp_sysc = {
> +	.sysc_offs	= 0x0004,
> +	.sysc_flags	= SYSC_HAS_SIDLEMODE,
> +	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
> +	.sysc_fields	= &omap_hwmod_sysc_type3,
> +};
> +
> +static struct omap_hwmod_class dra7xx_mcasp_hwmod_class = {
> +	.name	= "mcasp",
> +	.sysc	= &dra7xx_mcasp_sysc,
> +};
> +
> +/* mcasp3 */
> +static struct omap_hwmod dra7xx_mcasp3_hwmod = {
> +	.name		= "mcasp3",
> +	.class		= &dra7xx_mcasp_hwmod_class,
> +	.clkdm_name	= "l4per2_clkdm",
> +	.main_clk	= "mcasp3_ahclkx_mux",
> +	.flags		= HWMOD_SWSUP_SIDLE,
> +	.prcm = {
> +		.omap4 = {
> +			.clkctrl_offs = DRA7XX_CM_L4PER2_MCASP3_CLKCTRL_OFFSET,
> +			.context_offs = DRA7XX_RM_L4PER2_MCASP3_CONTEXT_OFFSET,
> +			.modulemode   = MODULEMODE_SWCTRL,
> +		},
> +	},
> +};
> +
> +/*
>   * 'mmc' class
>   *
>   */
> @@ -2566,6 +2598,14 @@ static struct omap_hwmod_ocp_if dra7xx_l3_main_1__hdmi = {
>  	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>  };
>  
> +/* l4_per2 -> mcasp3 */
> +static struct omap_hwmod_ocp_if dra7xx_l4_per2__mcasp3 = {
> +	.master		= &dra7xx_l4_per2_hwmod,
> +	.slave		= &dra7xx_mcasp3_hwmod,
> +	.clk		= "l3_iclk_div",
> +	.user		= OCP_USER_MPU | OCP_USER_SDMA,
> +};
> +
>  static struct omap_hwmod_addr_space dra7xx_elm_addrs[] = {
>  	{
>  		.pa_start	= 0x48078000,
> @@ -3338,6 +3378,7 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = {
>  	&dra7xx_l4_wkup__dcan1,
>  	&dra7xx_l4_per2__dcan2,
>  	&dra7xx_l4_per2__cpgmac0,
> +	&dra7xx_l4_per2__mcasp3,
>  	&dra7xx_gmac__mdio,
>  	&dra7xx_l4_cfg__dma_system,
>  	&dra7xx_l3_main_1__dss,
> 

[PATCH 01/11 RESEND] ARM: OMAP: DRA7: hwmod: Add data for McASP3

[Paul Walmsley]

Hi P?ter,

a few comments:

On Tue, 15 Sep 2015, Peter Ujfalusi wrote:

> McASP3 is used by default on DRA7x based boards for audio.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
> Hi Paul,
> 
> this patch is part of my earlier series and as Tony suggested I'll resend the
> hwmod patch for you to review since I missed you from the TO in the series.
> 
> The original series:
> https://www.mail-archive.com/linux-omap at vger.kernel.org/msg119319.html
> 
> Regards,
> Peter
> 
>  arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 41 +++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index 562247bced49..c38b7fa30c27 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1298,6 +1298,38 @@ static struct omap_hwmod dra7xx_mcspi4_hwmod = {
>  };
>  
>  /*
> + * 'mcasp' class
> + *
> + */
> +static struct omap_hwmod_class_sysconfig dra7xx_mcasp_sysc = {
> +	.sysc_offs	= 0x0004,
> +	.sysc_flags	= SYSC_HAS_SIDLEMODE,
> +	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
> +	.sysc_fields	= &omap_hwmod_sysc_type3,
> +};
> +
> +static struct omap_hwmod_class dra7xx_mcasp_hwmod_class = {
> +	.name	= "mcasp",
> +	.sysc	= &dra7xx_mcasp_sysc,
> +};
> +
> +/* mcasp3 */
> +static struct omap_hwmod dra7xx_mcasp3_hwmod = {
> +	.name		= "mcasp3",
> +	.class		= &dra7xx_mcasp_hwmod_class,
> +	.clkdm_name	= "l4per2_clkdm",
> +	.main_clk	= "mcasp3_ahclkx_mux",

I'd expect this clock to be something derived from mcasp3_aux_gfclk, 
according to Table 24-408 "Clocks and Resets" of SPRUHZ6.  Could you 
please doublecheck this?

> +	.flags		= HWMOD_SWSUP_SIDLE,

Is this needed?  If it is, please add a brief comment describing the issue 
or bug that it's working around.

> +	.prcm = {
> +		.omap4 = {
> +			.clkctrl_offs = DRA7XX_CM_L4PER2_MCASP3_CLKCTRL_OFFSET,
> +			.context_offs = DRA7XX_RM_L4PER2_MCASP3_CONTEXT_OFFSET,
> +			.modulemode   = MODULEMODE_SWCTRL,
> +		},
> +	},
> +};
> +
> +/*
>   * 'mmc' class
>   *
>   */
> @@ -2566,6 +2598,14 @@ static struct omap_hwmod_ocp_if dra7xx_l3_main_1__hdmi = {
>  	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>  };
>  
> +/* l4_per2 -> mcasp3 */
> +static struct omap_hwmod_ocp_if dra7xx_l4_per2__mcasp3 = {
> +	.master		= &dra7xx_l4_per2_hwmod,
> +	.slave		= &dra7xx_mcasp3_hwmod,

So this is the low-speed control/register access port, where the MPU 
writes to the McASP3 config registers...

> +	.clk		= "l3_iclk_div",

... and thus this interface clock doesn't look right for this port, since 
it's most likely generated from the L4PER2, where this port is connected.  
So it should probably be "l4_iclk_div".
 
> +	.user		= OCP_USER_MPU | OCP_USER_SDMA,
> +};

There's another struct omap_hwmod_ocp_if record missing: the high-speed 
bus-master port that the McASP3 uses to DMA audio data.  This port should 
most likely be clocked with "l3_iclk_div" per Table 24-408 "Clocks and 
Resets".  This port is also where the registers described in Table 24-555 
"MCASP_DAT Register Summary 3" L3_MAIN column are exposed.  You've got 
that address map range blocked out in your DT data reg property, and 
associated with this device, right? 0x46000000?

> +
>  static struct omap_hwmod_addr_space dra7xx_elm_addrs[] = {
>  	{
>  		.pa_start	= 0x48078000,
> @@ -3338,6 +3378,7 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = {
>  	&dra7xx_l4_wkup__dcan1,
>  	&dra7xx_l4_per2__dcan2,
>  	&dra7xx_l4_per2__cpgmac0,
> +	&dra7xx_l4_per2__mcasp3,
>  	&dra7xx_gmac__mdio,
>  	&dra7xx_l4_cfg__dma_system,
>  	&dra7xx_l3_main_1__dss,
> -- 
> 2.5.0
> 


- Paul

[PATCH 01/11 RESEND] ARM: OMAP: DRA7: hwmod: Add data for McASP3

[Peter Ujfalusi]

Paul,

On 09/27/2015 10:02 AM, Paul Walmsley wrote:
>>  /*
>> + * 'mcasp' class
>> + *
>> + */
>> +static struct omap_hwmod_class_sysconfig dra7xx_mcasp_sysc = {
>> +	.sysc_offs	= 0x0004,
>> +	.sysc_flags	= SYSC_HAS_SIDLEMODE,
>> +	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
>> +	.sysc_fields	= &omap_hwmod_sysc_type3,
>> +};
>> +
>> +static struct omap_hwmod_class dra7xx_mcasp_hwmod_class = {
>> +	.name	= "mcasp",
>> +	.sysc	= &dra7xx_mcasp_sysc,
>> +};
>> +
>> +/* mcasp3 */
>> +static struct omap_hwmod dra7xx_mcasp3_hwmod = {
>> +	.name		= "mcasp3",
>> +	.class		= &dra7xx_mcasp_hwmod_class,
>> +	.clkdm_name	= "l4per2_clkdm",
>> +	.main_clk	= "mcasp3_ahclkx_mux",
> 
> I'd expect this clock to be something derived from mcasp3_aux_gfclk, 
> according to Table 24-408 "Clocks and Resets" of SPRUHZ6.  Could you 
> please doublecheck this?

I can not explain this. If I change the main_clk to "mcasp3_aux_gfclk_mux"
then I can not access to McASP3 register at all.
I don't see anything popping out in the clock data, nor in other places.

>> +	.flags		= HWMOD_SWSUP_SIDLE,

Not sure why this has been added, I can not find any pointers regarding to
this and everything is working w/o this flag. Will remove it in v2.


> Is this needed?  If it is, please add a brief comment describing the issue 
> or bug that it's working around.
> 
>> +	.prcm = {
>> +		.omap4 = {
>> +			.clkctrl_offs = DRA7XX_CM_L4PER2_MCASP3_CLKCTRL_OFFSET,
>> +			.context_offs = DRA7XX_RM_L4PER2_MCASP3_CONTEXT_OFFSET,
>> +			.modulemode   = MODULEMODE_SWCTRL,
>> +		},
>> +	},
>> +};
>> +
>> +/*
>>   * 'mmc' class
>>   *
>>   */
>> @@ -2566,6 +2598,14 @@ static struct omap_hwmod_ocp_if dra7xx_l3_main_1__hdmi = {
>>  	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>>  };
>>  
>> +/* l4_per2 -> mcasp3 */
>> +static struct omap_hwmod_ocp_if dra7xx_l4_per2__mcasp3 = {
>> +	.master		= &dra7xx_l4_per2_hwmod,
>> +	.slave		= &dra7xx_mcasp3_hwmod,
> 
> So this is the low-speed control/register access port, where the MPU 
> writes to the McASP3 config registers...
> 
>> +	.clk		= "l3_iclk_div",
> 
> ... and thus this interface clock doesn't look right for this port, since 
> it's most likely generated from the L4PER2, where this port is connected.  
> So it should probably be "l4_iclk_div".

There is no "l4_iclk_div" for dra7xx. Looking around the file all other script
generated data uses "l3_iclk_div" for IPs under dra7xx_l4_per2_hwmod.

Tero: do you know the reason for this?

>  
>> +	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>> +};
> 
> There's another struct omap_hwmod_ocp_if record missing: the high-speed 
> bus-master port that the McASP3 uses to DMA audio data.  This port should 
> most likely be clocked with "l3_iclk_div" per Table 24-408 "Clocks and 
> Resets".  This port is also where the registers described in Table 24-555 
> "MCASP_DAT Register Summary 3" L3_MAIN column are exposed.  You've got 
> that address map range blocked out in your DT data reg property, and 
> associated with this device, right? 0x46000000?

Yes, the McASP3-dat port is not used ATM. This is over the L3 interconnect and
due to a feature we can not use it with sDMA (constant addressing is not
supported through L3 interconnect for DMAs).
We could use eDMA, but there are complications regarding to that.
At the moment we are using the sDMA through the L4 interconnect address space.

> 
>> +
>>  static struct omap_hwmod_addr_space dra7xx_elm_addrs[] = {
>>  	{
>>  		.pa_start	= 0x48078000,
>> @@ -3338,6 +3378,7 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = {
>>  	&dra7xx_l4_wkup__dcan1,
>>  	&dra7xx_l4_per2__dcan2,
>>  	&dra7xx_l4_per2__cpgmac0,
>> +	&dra7xx_l4_per2__mcasp3,
>>  	&dra7xx_gmac__mdio,
>>  	&dra7xx_l4_cfg__dma_system,
>>  	&dra7xx_l3_main_1__dss,
>> -- 
>> 2.5.0
>>
> 
> 
> - Paul
> 


-- 
P?ter

[PATCH 01/11 RESEND] ARM: OMAP: DRA7: hwmod: Add data for McASP3

[Tero Kristo]

On 09/30/2015 01:06 PM, Peter Ujfalusi wrote:
> Paul,
>
> On 09/27/2015 10:02 AM, Paul Walmsley wrote:
>>>   /*
>>> + * 'mcasp' class
>>> + *
>>> + */
>>> +static struct omap_hwmod_class_sysconfig dra7xx_mcasp_sysc = {
>>> +	.sysc_offs	= 0x0004,
>>> +	.sysc_flags	= SYSC_HAS_SIDLEMODE,
>>> +	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
>>> +	.sysc_fields	= &omap_hwmod_sysc_type3,
>>> +};
>>> +
>>> +static struct omap_hwmod_class dra7xx_mcasp_hwmod_class = {
>>> +	.name	= "mcasp",
>>> +	.sysc	= &dra7xx_mcasp_sysc,
>>> +};
>>> +
>>> +/* mcasp3 */
>>> +static struct omap_hwmod dra7xx_mcasp3_hwmod = {
>>> +	.name		= "mcasp3",
>>> +	.class		= &dra7xx_mcasp_hwmod_class,
>>> +	.clkdm_name	= "l4per2_clkdm",
>>> +	.main_clk	= "mcasp3_ahclkx_mux",
>>
>> I'd expect this clock to be something derived from mcasp3_aux_gfclk,
>> according to Table 24-408 "Clocks and Resets" of SPRUHZ6.  Could you
>> please doublecheck this?
>
> I can not explain this. If I change the main_clk to "mcasp3_aux_gfclk_mux"
> then I can not access to McASP3 register at all.
> I don't see anything popping out in the clock data, nor in other places.
>
>>> +	.flags		= HWMOD_SWSUP_SIDLE,
>
> Not sure why this has been added, I can not find any pointers regarding to
> this and everything is working w/o this flag. Will remove it in v2.
>
>
>> Is this needed?  If it is, please add a brief comment describing the issue
>> or bug that it's working around.
>>
>>> +	.prcm = {
>>> +		.omap4 = {
>>> +			.clkctrl_offs = DRA7XX_CM_L4PER2_MCASP3_CLKCTRL_OFFSET,
>>> +			.context_offs = DRA7XX_RM_L4PER2_MCASP3_CONTEXT_OFFSET,
>>> +			.modulemode   = MODULEMODE_SWCTRL,
>>> +		},
>>> +	},
>>> +};
>>> +
>>> +/*
>>>    * 'mmc' class
>>>    *
>>>    */
>>> @@ -2566,6 +2598,14 @@ static struct omap_hwmod_ocp_if dra7xx_l3_main_1__hdmi = {
>>>   	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>>>   };
>>>
>>> +/* l4_per2 -> mcasp3 */
>>> +static struct omap_hwmod_ocp_if dra7xx_l4_per2__mcasp3 = {
>>> +	.master		= &dra7xx_l4_per2_hwmod,
>>> +	.slave		= &dra7xx_mcasp3_hwmod,
>>
>> So this is the low-speed control/register access port, where the MPU
>> writes to the McASP3 config registers...
>>
>>> +	.clk		= "l3_iclk_div",
>>
>> ... and thus this interface clock doesn't look right for this port, since
>> it's most likely generated from the L4PER2, where this port is connected.
>> So it should probably be "l4_iclk_div".
>
> There is no "l4_iclk_div" for dra7xx. Looking around the file all other script
> generated data uses "l3_iclk_div" for IPs under dra7xx_l4_per2_hwmod.
>
> Tero: do you know the reason for this?

This comes from the autogen generated data. Looking at the hwdb data for 
dra7, it seems l3 clock is defined as the OCP input clock for most of 
the modules.

Looking at TRM, we also have L3 ICK defined as the interface clock for 
GPIO modules for example, and also mcasp modules.

I think this is just a documentation issue and we are missing a divide 
by 2 from all interface clocks, the interface clocks are coming from l4 
interconnects and the interconnect chapter still clearly states that the 
l4 clock is l3 clock / 2.

-Tero

>
>>
>>> +	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>>> +};
>>
>> There's another struct omap_hwmod_ocp_if record missing: the high-speed
>> bus-master port that the McASP3 uses to DMA audio data.  This port should
>> most likely be clocked with "l3_iclk_div" per Table 24-408 "Clocks and
>> Resets".  This port is also where the registers described in Table 24-555
>> "MCASP_DAT Register Summary 3" L3_MAIN column are exposed.  You've got
>> that address map range blocked out in your DT data reg property, and
>> associated with this device, right? 0x46000000?
>
> Yes, the McASP3-dat port is not used ATM. This is over the L3 interconnect and
> due to a feature we can not use it with sDMA (constant addressing is not
> supported through L3 interconnect for DMAs).
> We could use eDMA, but there are complications regarding to that.
> At the moment we are using the sDMA through the L4 interconnect address space.
>
>>
>>> +
>>>   static struct omap_hwmod_addr_space dra7xx_elm_addrs[] = {
>>>   	{
>>>   		.pa_start	= 0x48078000,
>>> @@ -3338,6 +3378,7 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = {
>>>   	&dra7xx_l4_wkup__dcan1,
>>>   	&dra7xx_l4_per2__dcan2,
>>>   	&dra7xx_l4_per2__cpgmac0,
>>> +	&dra7xx_l4_per2__mcasp3,
>>>   	&dra7xx_gmac__mdio,
>>>   	&dra7xx_l4_cfg__dma_system,
>>>   	&dra7xx_l3_main_1__dss,
>>> --
>>> 2.5.0
>>>
>>
>>
>> - Paul
>>
>
>

[PATCH 01/11 RESEND] ARM: OMAP: DRA7: hwmod: Add data for McASP3

[Peter Ujfalusi]

On 09/30/2015 04:00 PM, Tero Kristo wrote:
>>>> +/* l4_per2 -> mcasp3 */
>>>> +static struct omap_hwmod_ocp_if dra7xx_l4_per2__mcasp3 = {
>>>> +    .master        = &dra7xx_l4_per2_hwmod,
>>>> +    .slave        = &dra7xx_mcasp3_hwmod,
>>>
>>> So this is the low-speed control/register access port, where the MPU
>>> writes to the McASP3 config registers...
>>>
>>>> +    .clk        = "l3_iclk_div",
>>>
>>> ... and thus this interface clock doesn't look right for this port, since
>>> it's most likely generated from the L4PER2, where this port is connected.
>>> So it should probably be "l4_iclk_div".
>>
>> There is no "l4_iclk_div" for dra7xx. Looking around the file all other script
>> generated data uses "l3_iclk_div" for IPs under dra7xx_l4_per2_hwmod.
>>
>> Tero: do you know the reason for this?
> 
> This comes from the autogen generated data. Looking at the hwdb data for dra7,
> it seems l3 clock is defined as the OCP input clock for most of the modules.
> 
> Looking at TRM, we also have L3 ICK defined as the interface clock for GPIO
> modules for example, and also mcasp modules.
> 
> I think this is just a documentation issue and we are missing a divide by 2
> from all interface clocks, the interface clocks are coming from l4
> interconnects and the interconnect chapter still clearly states that the l4
> clock is l3 clock / 2.

It seams that we have the clock node for the l4_iclk_div, but it is called as
l4_root_clk_div.

I will use this in the mcasp3 hwmod patch.

-- 
P?ter

[PATCH 01/11 RESEND] ARM: OMAP: DRA7: hwmod: Add data for McASP3

[Paul Walmsley]

Hello P?ter,

On Wed, 30 Sep 2015, Peter Ujfalusi wrote:

> On 09/27/2015 10:02 AM, Paul Walmsley wrote:
> >>  /*
> >> + * 'mcasp' class
> >> + *
> >> + */
> >> +static struct omap_hwmod_class_sysconfig dra7xx_mcasp_sysc = {
> >> +	.sysc_offs	= 0x0004,
> >> +	.sysc_flags	= SYSC_HAS_SIDLEMODE,
> >> +	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
> >> +	.sysc_fields	= &omap_hwmod_sysc_type3,
> >> +};
> >> +
> >> +static struct omap_hwmod_class dra7xx_mcasp_hwmod_class = {
> >> +	.name	= "mcasp",
> >> +	.sysc	= &dra7xx_mcasp_sysc,
> >> +};
> >> +
> >> +/* mcasp3 */
> >> +static struct omap_hwmod dra7xx_mcasp3_hwmod = {
> >> +	.name		= "mcasp3",
> >> +	.class		= &dra7xx_mcasp_hwmod_class,
> >> +	.clkdm_name	= "l4per2_clkdm",
> >> +	.main_clk	= "mcasp3_ahclkx_mux",
> > 
> > I'd expect this clock to be something derived from mcasp3_aux_gfclk, 
> > according to Table 24-408 "Clocks and Resets" of SPRUHZ6.  Could you 
> > please doublecheck this?
> 
> I can not explain this. If I change the main_clk to "mcasp3_aux_gfclk_mux"
> then I can not access to McASP3 register at all.
> I don't see anything popping out in the clock data, nor in other places.

OK thank you for testing that.  Maybe just add a brief comment along those 
lines in the hwmod data, above the structure record declaration?

> >> +	.flags		= HWMOD_SWSUP_SIDLE,
> 
> Not sure why this has been added, I can not find any pointers regarding to
> this and everything is working w/o this flag. Will remove it in v2.

OK thanks.  

> >> @@ -2566,6 +2598,14 @@ static struct omap_hwmod_ocp_if dra7xx_l3_main_1__hdmi = {
> >>  	.user		= OCP_USER_MPU | OCP_USER_SDMA,
> >>  };
> >>  
> >> +/* l4_per2 -> mcasp3 */
> >> +static struct omap_hwmod_ocp_if dra7xx_l4_per2__mcasp3 = {
> >> +	.master		= &dra7xx_l4_per2_hwmod,
> >> +	.slave		= &dra7xx_mcasp3_hwmod,
> > 
> > So this is the low-speed control/register access port, where the MPU 
> > writes to the McASP3 config registers...
> > 
> >> +	.clk		= "l3_iclk_div",
> > 
> > ... and thus this interface clock doesn't look right for this port, since 
> > it's most likely generated from the L4PER2, where this port is connected.  
> > So it should probably be "l4_iclk_div".
> 
> There is no "l4_iclk_div" for dra7xx. Looking around the file all other script
> generated data uses "l3_iclk_div" for IPs under dra7xx_l4_per2_hwmod.
> 
> Tero: do you know the reason for this?

Sounds like from your followup E-mail that the clock name to use in the 
kernel is "l4_root_clk_div", which sounds fine to me.  (Haven't looked 
closely at the clock data, though.)

> >> +	.user		= OCP_USER_MPU | OCP_USER_SDMA,
> >> +};
> > 
> > There's another struct omap_hwmod_ocp_if record missing: the high-speed 
> > bus-master port that the McASP3 uses to DMA audio data.  This port should 
> > most likely be clocked with "l3_iclk_div" per Table 24-408 "Clocks and 
> > Resets".  This port is also where the registers described in Table 24-555 
> > "MCASP_DAT Register Summary 3" L3_MAIN column are exposed.  You've got 
> > that address map range blocked out in your DT data reg property, and 
> > associated with this device, right? 0x46000000?
> 
> Yes, the McASP3-dat port is not used ATM. This is over the L3 interconnect and
> due to a feature we can not use it with sDMA (constant addressing is not
> supported through L3 interconnect for DMAs).
> We could use eDMA, but there are complications regarding to that.
> At the moment we are using the sDMA through the L4 interconnect address space.

OK thanks for the explanation.  The hwmod code prevents links from being 
registered if no initiator 'users' are listed, so sounds like we should 
leave it out for now.  Could you add a brief comment, similar to your 
paragraph above, in the data, in case others wonder about the L3 link?

After those changes are made, feel free to add my ack.


K?sz?n?m,

- Paul

[PATCH 01/11 RESEND] ARM: OMAP: DRA7: hwmod: Add data for McASP3

[Peter Ujfalusi]

Paul,

On 10/02/2015 12:07 PM, Paul Walmsley wrote:
> Hello P?ter,
> 
> On Wed, 30 Sep 2015, Peter Ujfalusi wrote:
> 
>> On 09/27/2015 10:02 AM, Paul Walmsley wrote:
>>>>  /*
>>>> + * 'mcasp' class
>>>> + *
>>>> + */
>>>> +static struct omap_hwmod_class_sysconfig dra7xx_mcasp_sysc = {
>>>> +	.sysc_offs	= 0x0004,
>>>> +	.sysc_flags	= SYSC_HAS_SIDLEMODE,
>>>> +	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
>>>> +	.sysc_fields	= &omap_hwmod_sysc_type3,
>>>> +};
>>>> +
>>>> +static struct omap_hwmod_class dra7xx_mcasp_hwmod_class = {
>>>> +	.name	= "mcasp",
>>>> +	.sysc	= &dra7xx_mcasp_sysc,
>>>> +};
>>>> +
>>>> +/* mcasp3 */
>>>> +static struct omap_hwmod dra7xx_mcasp3_hwmod = {
>>>> +	.name		= "mcasp3",
>>>> +	.class		= &dra7xx_mcasp_hwmod_class,
>>>> +	.clkdm_name	= "l4per2_clkdm",
>>>> +	.main_clk	= "mcasp3_ahclkx_mux",
>>>
>>> I'd expect this clock to be something derived from mcasp3_aux_gfclk, 
>>> according to Table 24-408 "Clocks and Resets" of SPRUHZ6.  Could you 
>>> please doublecheck this?
>>
>> I can not explain this. If I change the main_clk to "mcasp3_aux_gfclk_mux"
>> then I can not access to McASP3 register at all.
>> I don't see anything popping out in the clock data, nor in other places.
> 
> OK thank you for testing that.  Maybe just add a brief comment along those 
> lines in the hwmod data, above the structure record declaration?

Now I more or less figured out the root of the issue. According to the
documentations the McASP in dra7xx family has following clocks:
ICLK - interface clock
GFCLK - functional clock
AHCLKX - Transmit high-frequency master clock
AHCLKR - Receive high-frequency master clock (on selected instances)

In order to access registers all of these clock lines must have valid clock
signal. No other SoC with McASP has this setup.
As for why things seams to work when mcasp3_ahclkx_mux is set as main_clk and
mcasp3_aux_gfclk_mux is not handled at all?
The mcasp3_aux_gfclk_mux by default is from PER_ABE_X1GFCLK we never change
this. On dra7/72 evm the AHCLKX is reparented to ATL2 clock.
When with pm_runtime we enable the device, the mcasp3_ahclkx_mux will be
enabled by the SW (and ATL as well) _and_ the mcasp3_aux_gfclk_mux path will
be enabled by HW w/o SW.
The reason why I have had crash when I switched the main_clk to
mcasp3_aux_gfclk_mux is that even the path for the mcasp3_ahclkx_mux is
enabled by the HW, the ATL itself was not enabled, so it was not generating
the needed clocks. Same goes backwards for the gfclk: if I reparent it to
let's say HDMI clock - which is not present, and handle the ahclkx clock I
have similar crash.
All in all: the McASP3 needs ICLK and both FCLK and AHCLKX to be running in
order to be able to access registers.

This current setup works fine, the only issue I see is that the refcounts for
the mcasp3_aux_gfclk_mux path is not reflecting the reality.

With Tero we looked at different angles of this and how to solve it w/o
considering it as a hack.
Either we go with the hwmod data with main_clk set to mcasp3_ahclkx_mux and
document it in a comment, or:
Add new flag HWMOD_OPT_CLKS_NEEDED to hwmods to use to tell that the optional
clocks are not really optional as they are needed to be enabled in order to
access to the IP. In omap_hwmod.c's _enable_clocks() and _disable_clocks() we
call _enable_optional_clocks()/_disable_optional_clocks() if the flag is set
for the hwmod and add the ahclkx_mux as optional clock for McASP3.

>>>> +	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>>>> +};
>>>
>>> There's another struct omap_hwmod_ocp_if record missing: the high-speed 
>>> bus-master port that the McASP3 uses to DMA audio data.  This port should 
>>> most likely be clocked with "l3_iclk_div" per Table 24-408 "Clocks and 
>>> Resets".  This port is also where the registers described in Table 24-555 
>>> "MCASP_DAT Register Summary 3" L3_MAIN column are exposed.  You've got 
>>> that address map range blocked out in your DT data reg property, and 
>>> associated with this device, right? 0x46000000?
>>
>> Yes, the McASP3-dat port is not used ATM. This is over the L3 interconnect and
>> due to a feature we can not use it with sDMA (constant addressing is not
>> supported through L3 interconnect for DMAs).
>> We could use eDMA, but there are complications regarding to that.
>> At the moment we are using the sDMA through the L4 interconnect address space.
> 
> OK thanks for the explanation.  The hwmod code prevents links from being 
> registered if no initiator 'users' are listed, so sounds like we should 
> leave it out for now.  Could you add a brief comment, similar to your 
> paragraph above, in the data, in case others wonder about the L3 link?

Sure, I'll do that.

> 
> After those changes are made, feel free to add my ack.

I'll wait for your comment regarding to the multiple main_clk need for the
McASP3 before I'll send the next version.

> K?sz?n?m,

:D

> 
> - Paul
> 

-- 
P?ter

[PATCH 01/11 RESEND] ARM: OMAP: DRA7: hwmod: Add data for McASP3

[Peter Ujfalusi]

On 10/02/2015 02:22 PM, Peter Ujfalusi wrote:
> Paul,
> 
> On 10/02/2015 12:07 PM, Paul Walmsley wrote:
>> Hello P?ter,
>>
>> On Wed, 30 Sep 2015, Peter Ujfalusi wrote:
>>
>>> On 09/27/2015 10:02 AM, Paul Walmsley wrote:
>>>>>  /*
>>>>> + * 'mcasp' class
>>>>> + *
>>>>> + */
>>>>> +static struct omap_hwmod_class_sysconfig dra7xx_mcasp_sysc = {
>>>>> +	.sysc_offs	= 0x0004,
>>>>> +	.sysc_flags	= SYSC_HAS_SIDLEMODE,
>>>>> +	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
>>>>> +	.sysc_fields	= &omap_hwmod_sysc_type3,
>>>>> +};
>>>>> +
>>>>> +static struct omap_hwmod_class dra7xx_mcasp_hwmod_class = {
>>>>> +	.name	= "mcasp",
>>>>> +	.sysc	= &dra7xx_mcasp_sysc,
>>>>> +};
>>>>> +
>>>>> +/* mcasp3 */
>>>>> +static struct omap_hwmod dra7xx_mcasp3_hwmod = {
>>>>> +	.name		= "mcasp3",
>>>>> +	.class		= &dra7xx_mcasp_hwmod_class,
>>>>> +	.clkdm_name	= "l4per2_clkdm",
>>>>> +	.main_clk	= "mcasp3_ahclkx_mux",
>>>>
>>>> I'd expect this clock to be something derived from mcasp3_aux_gfclk, 
>>>> according to Table 24-408 "Clocks and Resets" of SPRUHZ6.  Could you 
>>>> please doublecheck this?
>>>
>>> I can not explain this. If I change the main_clk to "mcasp3_aux_gfclk_mux"
>>> then I can not access to McASP3 register at all.
>>> I don't see anything popping out in the clock data, nor in other places.
>>
>> OK thank you for testing that.  Maybe just add a brief comment along those 
>> lines in the hwmod data, above the structure record declaration?
> 
> Now I more or less figured out the root of the issue. According to the
> documentations the McASP in dra7xx family has following clocks:
> ICLK - interface clock
> GFCLK - functional clock
> AHCLKX - Transmit high-frequency master clock
> AHCLKR - Receive high-frequency master clock (on selected instances)
> 
> In order to access registers all of these clock lines must have valid clock
> signal. No other SoC with McASP has this setup.
> As for why things seams to work when mcasp3_ahclkx_mux is set as main_clk and
> mcasp3_aux_gfclk_mux is not handled at all?
> The mcasp3_aux_gfclk_mux by default is from PER_ABE_X1GFCLK we never change
> this. On dra7/72 evm the AHCLKX is reparented to ATL2 clock.
> When with pm_runtime we enable the device, the mcasp3_ahclkx_mux will be
> enabled by the SW (and ATL as well) _and_ the mcasp3_aux_gfclk_mux path will
> be enabled by HW w/o SW.
> The reason why I have had crash when I switched the main_clk to
> mcasp3_aux_gfclk_mux is that even the path for the mcasp3_ahclkx_mux is
> enabled by the HW, the ATL itself was not enabled, so it was not generating
> the needed clocks. Same goes backwards for the gfclk: if I reparent it to
> let's say HDMI clock - which is not present, and handle the ahclkx clock I
> have similar crash.
> All in all: the McASP3 needs ICLK and both FCLK and AHCLKX to be running in
> order to be able to access registers.
> 
> This current setup works fine, the only issue I see is that the refcounts for
> the mcasp3_aux_gfclk_mux path is not reflecting the reality.
> 
> With Tero we looked at different angles of this and how to solve it w/o
> considering it as a hack.
> Either we go with the hwmod data with main_clk set to mcasp3_ahclkx_mux and
> document it in a comment, or:
> Add new flag HWMOD_OPT_CLKS_NEEDED to hwmods to use to tell that the optional
> clocks are not really optional as they are needed to be enabled in order to
> access to the IP. In omap_hwmod.c's _enable_clocks() and _disable_clocks() we
> call _enable_optional_clocks()/_disable_optional_clocks() if the flag is set
> for the hwmod and add the ahclkx_mux as optional clock for McASP3.

This might be awkward to say that the optional clocks are not optional,
probably would be better to add:
struct omap_hwmod {
	...
	struct omap_hwmod_opt_clk	*needed_clks;
	...
	u8				needed_clks_cnt;
	...
};

and use the needed_clks in _init_main_clk()/_enable_clocks()/_disable_clocks() ?

-- 
P?ter

[PATCH 01/11 RESEND] ARM: OMAP: DRA7: hwmod: Add data for McASP3

[Peter Ujfalusi]

On 10/02/2015 02:55 PM, Peter Ujfalusi wrote:
> On 10/02/2015 02:22 PM, Peter Ujfalusi wrote:
>> Paul,
>>
>> On 10/02/2015 12:07 PM, Paul Walmsley wrote:
>>> Hello P?ter,
>>>
>>> On Wed, 30 Sep 2015, Peter Ujfalusi wrote:
>>>
>>>> On 09/27/2015 10:02 AM, Paul Walmsley wrote:
>>>>>>  /*
>>>>>> + * 'mcasp' class
>>>>>> + *
>>>>>> + */
>>>>>> +static struct omap_hwmod_class_sysconfig dra7xx_mcasp_sysc = {
>>>>>> +	.sysc_offs	= 0x0004,
>>>>>> +	.sysc_flags	= SYSC_HAS_SIDLEMODE,
>>>>>> +	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
>>>>>> +	.sysc_fields	= &omap_hwmod_sysc_type3,
>>>>>> +};
>>>>>> +
>>>>>> +static struct omap_hwmod_class dra7xx_mcasp_hwmod_class = {
>>>>>> +	.name	= "mcasp",
>>>>>> +	.sysc	= &dra7xx_mcasp_sysc,
>>>>>> +};
>>>>>> +
>>>>>> +/* mcasp3 */
>>>>>> +static struct omap_hwmod dra7xx_mcasp3_hwmod = {
>>>>>> +	.name		= "mcasp3",
>>>>>> +	.class		= &dra7xx_mcasp_hwmod_class,
>>>>>> +	.clkdm_name	= "l4per2_clkdm",
>>>>>> +	.main_clk	= "mcasp3_ahclkx_mux",
>>>>>
>>>>> I'd expect this clock to be something derived from mcasp3_aux_gfclk, 
>>>>> according to Table 24-408 "Clocks and Resets" of SPRUHZ6.  Could you 
>>>>> please doublecheck this?
>>>>
>>>> I can not explain this. If I change the main_clk to "mcasp3_aux_gfclk_mux"
>>>> then I can not access to McASP3 register at all.
>>>> I don't see anything popping out in the clock data, nor in other places.
>>>
>>> OK thank you for testing that.  Maybe just add a brief comment along those 
>>> lines in the hwmod data, above the structure record declaration?
>>
>> Now I more or less figured out the root of the issue. According to the
>> documentations the McASP in dra7xx family has following clocks:
>> ICLK - interface clock
>> GFCLK - functional clock
>> AHCLKX - Transmit high-frequency master clock
>> AHCLKR - Receive high-frequency master clock (on selected instances)
>>
>> In order to access registers all of these clock lines must have valid clock
>> signal. No other SoC with McASP has this setup.
>> As for why things seams to work when mcasp3_ahclkx_mux is set as main_clk and
>> mcasp3_aux_gfclk_mux is not handled at all?
>> The mcasp3_aux_gfclk_mux by default is from PER_ABE_X1GFCLK we never change
>> this. On dra7/72 evm the AHCLKX is reparented to ATL2 clock.
>> When with pm_runtime we enable the device, the mcasp3_ahclkx_mux will be
>> enabled by the SW (and ATL as well) _and_ the mcasp3_aux_gfclk_mux path will
>> be enabled by HW w/o SW.
>> The reason why I have had crash when I switched the main_clk to
>> mcasp3_aux_gfclk_mux is that even the path for the mcasp3_ahclkx_mux is
>> enabled by the HW, the ATL itself was not enabled, so it was not generating
>> the needed clocks. Same goes backwards for the gfclk: if I reparent it to
>> let's say HDMI clock - which is not present, and handle the ahclkx clock I
>> have similar crash.
>> All in all: the McASP3 needs ICLK and both FCLK and AHCLKX to be running in
>> order to be able to access registers.
>>
>> This current setup works fine, the only issue I see is that the refcounts for
>> the mcasp3_aux_gfclk_mux path is not reflecting the reality.
>>
>> With Tero we looked at different angles of this and how to solve it w/o
>> considering it as a hack.
>> Either we go with the hwmod data with main_clk set to mcasp3_ahclkx_mux and
>> document it in a comment, or:
>> Add new flag HWMOD_OPT_CLKS_NEEDED to hwmods to use to tell that the optional
>> clocks are not really optional as they are needed to be enabled in order to
>> access to the IP. In omap_hwmod.c's _enable_clocks() and _disable_clocks() we
>> call _enable_optional_clocks()/_disable_optional_clocks() if the flag is set
>> for the hwmod and add the ahclkx_mux as optional clock for McASP3.
> 
> This might be awkward to say that the optional clocks are not optional,
> probably would be better to add:
> struct omap_hwmod {
> 	...
> 	struct omap_hwmod_opt_clk	*needed_clks;
> 	...
> 	u8				needed_clks_cnt;
> 	...
> };
> 
> and use the needed_clks in _init_main_clk()/_enable_clocks()/_disable_clocks() ?

Arrgh, but how to deal with this via DT bindings? It will be better to mark
the optional clocks as needed.

-- 
P?ter