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

From: Paul Walmsley
Date: Sun Sep 27 2015 - 03:02:32 EST


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@xxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx/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