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

From: Peter Ujfalusi
Date: Fri Oct 02 2015 - 08:49:17 EST


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