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

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <560E7094.408@ti.com>
Date:	Fri, 2 Oct 2015 14:55:00 +0300
From:	Peter Ujfalusi <peter.ujfalusi@...com>
To:	Paul Walmsley <paul@...an.com>
CC:	<tony@...mide.com>, <linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>,
	<linux-omap@...r.kernel.org>, <t-kristo@...com>
Subject: Re: [PATCH 01/11 RESEND] ARM: OMAP: DRA7: hwmod: Add data for McASP3

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/