| 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
| ||
|
Message-ID: <55B109C1.80901@ti.com>
Date: Thu, 23 Jul 2015 21:05:29 +0530
From: "R, Vignesh" <vigneshr@...com>
To: Paul Walmsley <paul@...an.com>
CC: Tero Kristo <t-kristo@...com>,
Thierry Reding <thierry.reding@...il.com>,
Rob Herring <robh+dt@...nel.org>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Ian Campbell <ijc+devicetree@...lion.org.uk>,
Kumar Gala <galak@...eaurora.org>,
Benoit Cousson <bcousson@...libre.com>,
Tony Lindgren <tony@...mide.com>,
Russell King <linux@....linux.org.uk>,
Mike Turquette <mturquette@...aro.org>,
Stephen Boyd <sboyd@...eaurora.org>,
<linux-pwm@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-omap@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <linux-clk@...r.kernel.org>
Subject: Re: [PATCH v2 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS
On 7/16/2015 9:01 PM, R, Vignesh wrote:
> Hi,
>
> On 07/16/2015 03:24 AM, Paul Walmsley wrote:
>> Hi,
>>
>> some comments.
>>
>> On Wed, 3 Jun 2015, Vignesh R wrote:
>>
>>> Add hwmod entries for the PWMSS on DRA7.
>>>
>>> Set l4_root_clk_div as the main_clk of PWMSS. It is fixed-factored clock
>>> equal to L4PER2_L3_GICLK/2(l3_iclk_div/2).
>>> As per AM57x TRM SPRUHZ6[1], October 2014, Section 29.1.3 Table 29-4,
>>> clock source to PWMSS is L4PER2_L3_GICLK. But it is actually
>>> L4PER2_L3_GICLK/2. The TRM does not show the division by 2.
>>
>> Is the divide-by-two coming from PWMSS_EPWM.EPWM_TBCTL[HSPCLKDIV]? Or is
>> HSPCLKDIV a separate divider after the divide-by-2 you mention above?
>
> No, it not related to HSPCLKDIV. The TRM wrongly states L4PER2_L3_GICLK
> as clock input for PWMSS. But actually L4PER2_L4_GICLK(=L3_GICLK/2) is
> the clock input for PWMSS. This will be updated in TRM soon.
>
>>
>>> [1] www.ti.com/lit/ug/spruhz6/spruhz6.pdf
>>>
>>> Signed-off-by: Vignesh R <vigneshr@...com>
>>> ---
>>>
>>> v2:
>>> * add TRM references.
>>>
>>> arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 239 ++++++++++++++++++++++++++++++
>>> 1 file changed, 239 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> index 0e64c2fac0b5..86a7ac9a3138 100644
>>> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
>>> @@ -362,6 +362,149 @@ static struct omap_hwmod dra7xx_dcan2_hwmod = {
>>> },
>>> };
>>>
>>> +/* pwmss */
>>> +static struct omap_hwmod_class_sysconfig dra7xx_epwmss_sysc = {
>>> + .rev_offs = 0x0,
>>> + .sysc_offs = 0x4,
>>> + .sysc_flags = SYSC_HAS_SIDLEMODE | SYSC_HAS_RESET_STATUS,
>>
>> This doesn't match SPRUHZ6 Table 29-13 "PWMSS_SYSCONFIG". There's no
>> RESETSTATUS bit. There is a SOFTRESET bit.
>>
>> Could you please confirm whether this is intentional?
>
> sorry my bad... I will change this in v3.
>
>>
>>> + .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
>>> + .sysc_fields = &omap_hwmod_sysc_type2,
>>> +};
>>> +
>>> +struct omap_hwmod_class dra7xx_epwmss_hwmod_class = {
>>> + .name = "epwmss",
>>> + .sysc = &dra7xx_epwmss_sysc,
>>> +};
>>> +
>>> +static struct omap_hwmod_class dra7xx_ecap_hwmod_class = {
>>> + .name = "ecap",
>>> +};
>>> +
>>> +static struct omap_hwmod_class dra7xx_eqep_hwmod_class = {
>>> + .name = "eqep",
>>> +};
>>> +
>>> +struct omap_hwmod_class dra7xx_ehrpwm_hwmod_class = {
>>> + .name = "ehrpwm",
>>> +};
>>> +
>>> +/* epwmss0 */
>>> +struct omap_hwmod dra7xx_epwmss0_hwmod = {
>>> + .name = "epwmss0",
>>> + .class = &dra7xx_epwmss_hwmod_class,
>>> + .clkdm_name = "l4per2_clkdm",
>>> + .main_clk = "l4_root_clk_div",
>>> + .prcm = {
>>> + .omap4 = {
>>> + .modulemode = MODULEMODE_SWCTRL,
>>> + .clkctrl_offs = DRA7XX_CM_L4PER2_PWMSS1_CLKCTRL_OFFSET,
>>> + .context_offs = DRA7XX_RM_L4PER2_PWMSS1_CONTEXT_OFFSET,
>>> + },
>>> + },
>>
>> Per my comment on the previous patch, sounds like it might be better to
>> mark this as HWMOD_SWSUP_SIDLE? Then again, see the next comment below
>> re: main_clk.
>>
>>> +};
>>> +
>>> +/* ecap0 */
>>> +struct omap_hwmod dra7xx_ecap0_hwmod = {
>>> + .name = "ecap0",
>>> + .class = &dra7xx_ecap_hwmod_class,
>>> + .clkdm_name = "l4per2_clkdm",
>>> + .main_clk = "l4_root_clk_div",
>>
>> Looking at SPRUHZ6 Section 29.1.4.2 "PWMSS Modules Local Clock Gating",
>> there appears to be a local "mini-PRCM" for the PWMSS which implements
>> clock gating and reports back on the status of what I'd guess is the local
>> clock gating FSM.
>>
>> So from that point of view, you should probably create a clock driver that
>> manages both the clock gate request bit and the FSM status bit. It should
>> be something that can be reused for the other PWMSS IP blocks. Then
>> you'd create per-IP block clock nodes and set the main_clk to point to
>> that node.
>>
>
> Since, this register is within the config space of PWMSS, the individual
> gating and reporting for the modules within PWMSS
> (PWMSS_CLKCONFIG) is currently being taken care by pwm-tipwmss.c(almost
> the sole function this driver is doing). It has been the same since
> am335x. Adding new clock nodes will result in driver changes and also
> changes to am335x, am437x (and other platforms) hwmod files. It also
> involves adding new nodes to clocks.dtsi and it will be difficult to
> maintain backward compatibility for older platforms. Is it not better to
> keep this as is, in order to maintain consistency (with am335x, am437x
> etc) and also that these clock bits are within IP's config space?
>
ping...
Regards
Vignesh
--
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/
Powered by blists - more mailing lists