| Message ID | 1433332284-10766-3-git-send-email-vigneshr@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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? > [1] www.ti.com/lit/ug/spruhz6/spruhz6.pdf > > Signed-off-by: Vignesh R <vigneshr@ti.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? > + .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. > +}; > + > +/* eqep0 */ > +struct omap_hwmod dra7xx_eqep0_hwmod = { > + .name = "eqep0", > + .class = &dra7xx_eqep_hwmod_class, > + .clkdm_name = "l4per2_clkdm", > + .main_clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > +}; > + > +/* ehrpwm0 */ > +struct omap_hwmod dra7xx_ehrpwm0_hwmod = { > + .name = "ehrpwm0", > + .class = &dra7xx_ehrpwm_hwmod_class, > + .clkdm_name = "l4per2_clkdm", > + .main_clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > +}; > + > +/* epwmss1 */ > +struct omap_hwmod dra7xx_epwmss1_hwmod = { > + .name = "epwmss1", > + .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_PWMSS2_CLKCTRL_OFFSET, > + .context_offs = DRA7XX_RM_L4PER2_PWMSS2_CONTEXT_OFFSET, > + }, > + }, Same flags comment as for epwmss0. > +}; > + > +/* ecap1 */ > +struct omap_hwmod dra7xx_ecap1_hwmod = { > + .name = "ecap1", > + .class = &dra7xx_ecap_hwmod_class, > + .clkdm_name = "l4per2_clkdm", > + .main_clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > +}; > + > +/* eqep1 */ > +struct omap_hwmod dra7xx_eqep1_hwmod = { > + .name = "eqep1", > + .class = &dra7xx_eqep_hwmod_class, > + .clkdm_name = "l4per2_clkdm", > + .main_clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > +}; > + > +/* ehrpwm1 */ > +struct omap_hwmod dra7xx_ehrpwm1_hwmod = { > + .name = "ehrpwm1", > + .class = &dra7xx_ehrpwm_hwmod_class, > + .clkdm_name = "l4per2_clkdm", > + .main_clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > +}; > + > +/* epwmss2 */ > +struct omap_hwmod dra7xx_epwmss2_hwmod = { > + .name = "epwmss2", > + .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_PWMSS3_CLKCTRL_OFFSET, > + .context_offs = DRA7XX_RM_L4PER2_PWMSS3_CONTEXT_OFFSET, > + }, > + }, Same flags comment as for epwmss0. > +}; > + > +/* ecap2 */ > +struct omap_hwmod dra7xx_ecap2_hwmod = { > + .name = "ecap2", > + .class = &dra7xx_ecap_hwmod_class, > + .clkdm_name = "l4per2_clkdm", > + .main_clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > +}; > + > +/* eqep2 */ > +struct omap_hwmod dra7xx_eqep2_hwmod = { > + .name = "eqep2", > + .class = &dra7xx_eqep_hwmod_class, > + .clkdm_name = "l4per2_clkdm", > + .main_clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > +}; > + > +/* ehrpwm2 */ > +struct omap_hwmod dra7xx_ehrpwm2_hwmod = { > + .name = "ehrpwm2", > + .class = &dra7xx_ehrpwm_hwmod_class, > + .clkdm_name = "l4per2_clkdm", > + .main_clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > +}; > + > /* > * 'dma' class > * > @@ -2601,6 +2744,90 @@ static struct omap_hwmod_ocp_if dra7xx_l4_per1__gpio6 = { > .user = OCP_USER_MPU | OCP_USER_SDMA, > }; > > +struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss0 = { > + .master = &dra7xx_l4_per2_hwmod, > + .slave = &dra7xx_epwmss0_hwmod, > + .clk = "l4_root_clk_div", > + .user = OCP_USER_MPU, > +}; > + > +struct omap_hwmod_ocp_if dra7xx_epwmss0__ecap0 = { > + .master = &dra7xx_epwmss0_hwmod, > + .slave = &dra7xx_ecap0_hwmod, > + .clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > + .user = OCP_USER_MPU, > +}; > + > +struct omap_hwmod_ocp_if dra7xx_epwmss0__eqep0 = { > + .master = &dra7xx_epwmss0_hwmod, > + .slave = &dra7xx_eqep0_hwmod, > + .clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > + .user = OCP_USER_MPU, > +}; > + > +struct omap_hwmod_ocp_if dra7xx_epwmss0__ehrpwm0 = { > + .master = &dra7xx_epwmss0_hwmod, > + .slave = &dra7xx_ehrpwm0_hwmod, > + .clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > + .user = OCP_USER_MPU, > +}; > + > +struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss1 = { > + .master = &dra7xx_l4_per2_hwmod, > + .slave = &dra7xx_epwmss1_hwmod, > + .clk = "l4_root_clk_div", > + .user = OCP_USER_MPU, > +}; > + > +struct omap_hwmod_ocp_if dra7xx_epwmss1__ecap1 = { > + .master = &dra7xx_epwmss1_hwmod, > + .slave = &dra7xx_ecap1_hwmod, > + .clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > + .user = OCP_USER_MPU, > +}; > + > +struct omap_hwmod_ocp_if dra7xx_epwmss1__eqep1 = { > + .master = &dra7xx_epwmss1_hwmod, > + .slave = &dra7xx_eqep1_hwmod, > + .clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > + .user = OCP_USER_MPU, > +}; > + > +struct omap_hwmod_ocp_if dra7xx_epwmss1__ehrpwm1 = { > + .master = &dra7xx_epwmss1_hwmod, > + .slave = &dra7xx_ehrpwm1_hwmod, > + .clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > + .user = OCP_USER_MPU, > +}; > + > +struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss2 = { > + .master = &dra7xx_l4_per2_hwmod, > + .slave = &dra7xx_epwmss2_hwmod, > + .clk = "l4_root_clk_div", > + .user = OCP_USER_MPU, > +}; > + > +struct omap_hwmod_ocp_if dra7xx_epwmss2__ecap2 = { > + .master = &dra7xx_epwmss2_hwmod, > + .slave = &dra7xx_ecap2_hwmod, > + .clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > + .user = OCP_USER_MPU, > +}; > + > +struct omap_hwmod_ocp_if dra7xx_epwmss2__eqep2 = { > + .master = &dra7xx_epwmss2_hwmod, > + .slave = &dra7xx_eqep2_hwmod, > + .clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > + .user = OCP_USER_MPU, > +}; > + > +struct omap_hwmod_ocp_if dra7xx_epwmss2__ehrpwm2 = { > + .master = &dra7xx_epwmss2_hwmod, > + .slave = &dra7xx_ehrpwm2_hwmod, > + .clk = "l4_root_clk_div", Same main_clk comments as for ecap0. > + .user = OCP_USER_MPU, > +}; > + > /* l4_per1 -> gpio7 */ > static struct omap_hwmod_ocp_if dra7xx_l4_per1__gpio7 = { > .master = &dra7xx_l4_per1_hwmod, > @@ -3394,6 +3621,18 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = { > &dra7xx_l3_main_1__vcp2, > &dra7xx_l4_per2__vcp2, > &dra7xx_l4_wkup__wd_timer2, > + &dra7xx_l4_per2__epwmss0, > + &dra7xx_epwmss0__ecap0, > + &dra7xx_epwmss0__eqep0, > + &dra7xx_epwmss0__ehrpwm0, > + &dra7xx_l4_per2__epwmss1, > + &dra7xx_epwmss1__ecap1, > + &dra7xx_epwmss1__eqep1, > + &dra7xx_epwmss1__ehrpwm1, > + &dra7xx_l4_per2__epwmss2, > + &dra7xx_epwmss2__ecap2, > + &dra7xx_epwmss2__eqep2, > + &dra7xx_epwmss2__ehrpwm2, > NULL, > }; > > -- > 2.4.1 > - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@ti.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?
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@ti.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-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/23/2015 09:05 PM, R, Vignesh wrote: > > > 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@ti.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... > I really want PWMSS support to go into v4.3... I see two options: 1. Drop pwm-tipwmss.c that is currently managing "PWMSS Modules Local Clock Gating" and change am335x, am437x and dra7 to use new main_clk and update clocks.dtsi (Breaks DT backward compatibility for am335x, am437x) 2. Or stick with $subject approach to maintain consistency. I prefer second option, Shall I send a v3 fixing other comments for this patch?
Hi On Thu, 16 Jul 2015, R, Vignesh wrote: > On 07/16/2015 03:24 AM, Paul Walmsley wrote: > > > > 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. OK > >> --- 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. OK > >> +/* 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? It's certainly possible that we as maintainers didn't look closely enough at the AM33xx data for the PWMSS when we merged it. But if that's incorrect too, then now is the time to fix this. Otherwise it will never get fixed, since each new group of people patching this code will keep punting it off to the indeterminate future. In terms of hwmod data: based on the register maps in sections 29.4.3, 29.3.3, and 29.2.3 of SPRUHZ6, none of these subdevices are hwmod devices. They don't support the Highlander OCP registers, they have no individual PRCM registers or register bitfields, and all of the idle and status reporting is to the PWMSS top-level IP block itself. So it looks to me like the eCAP, eQEP, and ePWM modules should be registered via DT, rather than via hwmod. It looks like you can get away with using the "simple-bus" abstraction, but you might ultimately have to define something custom here. However, the PWMSS top level subsystem, described in section 29.1, does have the OCP registers, sideband signals, etc., and thus should remain a hwmod-registered device (via DT). In terms of the clock data: based on section 29.1.4, section 29.1.5.2, figure 29-3, and table 29-4, there are several clock gating control bits. These should be modeled as clock nodes in the Linux common clock framework. Furthermore these registers are located inside the PWMSS subsystem itself, and are only accessible when the PWMSS IP block is functional in a PM runtime point of view: i.e., when the block is mapped into memory space, clocked and out of reset, etc. So the clock types for the PWMSS_CFG IP block should most likely be implemented in the PWMSS driver. I think you'll still be able to define the clock node data itself in DT, but this will probably need a closer look. Ideally, since the clock node register address data is the same for all three subsystem instances, one would be able to simply include a DT include file three times; but the DT binding data format may ultimately make this impractical. In terms of the transition from the old approach to this approach, it ooks to me like the first thing to do would be to convert drivers/pwm/pwm-tipwmss.c to define some clk_ops and register some clock nodes with the CCF. You've got the meat of the clock gating control code there already. Then the next thing to do would be to to get rid of pwmss_submodule_state_change() and use clk_{prepare,enable,disable,unprepare}*() in the drivers/pwm/pwm-ti*.c subdrivers instead. All of that looks like it should be possible to implement in a backwards-compatible way. Then you'd convert the eCAP, eQEP, and ePWM code to probe as platform_devices, driven from a simple-bus or pwmss-bus or whatever in the DT data. Naturally, the AM33xx/43xx data should be fixed also. - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/31/2015 10:51 AM, Paul Walmsley wrote: > Hi > > On Thu, 16 Jul 2015, R, Vignesh wrote: > >> On 07/16/2015 03:24 AM, Paul Walmsley wrote: >>> 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. > OK > >>>> --- 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. > OK > >>>> +/* 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? > It's certainly possible that we as maintainers didn't look closely enough > at the AM33xx data for the PWMSS when we merged it. But if that's > incorrect too, then now is the time to fix this. Otherwise it will never > get fixed, since each new group of people patching this code will keep > punting it off to the indeterminate future. > > In terms of hwmod data: based on the register maps in sections 29.4.3, > 29.3.3, and 29.2.3 of SPRUHZ6, none of these subdevices are hwmod devices. > They don't support the Highlander OCP registers, they have no individual > PRCM registers or register bitfields, and all of the idle and status > reporting is to the PWMSS top-level IP block itself. So it looks to me > like the eCAP, eQEP, and ePWM modules should be registered via DT, rather > than via hwmod. It looks like you can get away with using the > "simple-bus" abstraction, but you might ultimately have to define > something custom here. However, the PWMSS top level subsystem, described > in section 29.1, does have the OCP registers, sideband signals, etc., and > thus should remain a hwmod-registered device (via DT). > > In terms of the clock data: based on section 29.1.4, section 29.1.5.2, > figure 29-3, and table 29-4, there are several clock gating control bits. > These should be modeled as clock nodes in the Linux common clock > framework. Furthermore these registers are located inside the PWMSS > subsystem itself, and are only accessible when the PWMSS IP block is > functional in a PM runtime point of view: i.e., when the block is mapped > into memory space, clocked and out of reset, etc. So the clock types for > the PWMSS_CFG IP block should most likely be implemented in the PWMSS > driver. I think you'll still be able to define the clock node data itself > in DT, but this will probably need a closer look. Ideally, since the > clock node register address data is the same for all three subsystem > instances, one would be able to simply include a DT include file three > times; but the DT binding data format may ultimately make this > impractical. > > In terms of the transition from the old approach to this approach, it ooks > to me like the first thing to do would be to convert > drivers/pwm/pwm-tipwmss.c to define some clk_ops and register some clock > nodes with the CCF. You've got the meat of the clock gating control code > there already. Then the next thing to do would be to to get rid of > pwmss_submodule_state_change() and use > clk_{prepare,enable,disable,unprepare}*() in the drivers/pwm/pwm-ti*.c > subdrivers instead. All of that looks like it should be possible to > implement in a backwards-compatible way. Then you'd convert the eCAP, > eQEP, and ePWM code to probe as platform_devices, driven from a simple-bus > or pwmss-bus or whatever in the DT data. Naturally, the AM33xx/43xx data > should be fixed also. Hi Paul, Sorry for bringing up this old thread again. I am working on updating this patchset on behalf of Vignesh based on your suggestion. During the process of converting the pwm-tipwmss.c to be a clock provider I discovered that there is an issue with the PWMSS local clock gates. For example on AM437x if you run the below commands you will get a " Custom Error: MASTER M2 (64-bit) TARGET L4_PER_0 (Read): Data Access in User mode during Functional access" error. rmmod pwm_tiecap rmmod pwm_bl rmmod pwm_tiecap modprobe pwm_tiecap modprobe pwm_bl Full Log: http://pastebin.com/sEyy52HV On device powerup, the various PWMSS local clock gates are "unlocked". The remove call in the ecap driver gates the local PWMSS clock gate for the ecap. The probe of the ecap driver "unlocks" the clock gate for the ecap. However, once you try to access any of the ecap registers which is indirectly accessed by the pwm_bl driver you will get an error. This is because the ecap clock is still being gated. This has been verified by hardware folks on our side. Its been determined these registers are not needed since they were designed for devices that didn't have a PRCM. Therefore, the current plan is to update the trms of AM335x,AM437x,DRA7 and AM57x to make it clear that these registers shouldn't be touched. I plan on sending a patch for the pwm-tipwmss.c driver essentially removing anything that touches the PWMSS_CLKCONFIG and PWMSS_CLKSTATUS registers. I will also be sending a v3 version of this patch since I believe the struct omap_hwmod dra7xx_ecap0_hwmod and similar entries you had comments about before are ok to leave as is based on the above findings. > > > - Paul > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Franklin On Wed, 17 Feb 2016, Franklin S Cooper Jr. wrote: > On 08/31/2015 10:51 AM, Paul Walmsley wrote: > > On Thu, 16 Jul 2015, R, Vignesh wrote: > >> On 07/16/2015 03:24 AM, Paul Walmsley wrote: > >>> 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. > > OK > > > >>>> --- 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. > > OK > > > >>>> +/* 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? > > It's certainly possible that we as maintainers didn't look closely enough > > at the AM33xx data for the PWMSS when we merged it. But if that's > > incorrect too, then now is the time to fix this. Otherwise it will never > > get fixed, since each new group of people patching this code will keep > > punting it off to the indeterminate future. > > > > In terms of hwmod data: based on the register maps in sections 29.4.3, > > 29.3.3, and 29.2.3 of SPRUHZ6, none of these subdevices are hwmod devices. > > They don't support the Highlander OCP registers, they have no individual > > PRCM registers or register bitfields, and all of the idle and status > > reporting is to the PWMSS top-level IP block itself. So it looks to me > > like the eCAP, eQEP, and ePWM modules should be registered via DT, rather > > than via hwmod. It looks like you can get away with using the > > "simple-bus" abstraction, but you might ultimately have to define > > something custom here. However, the PWMSS top level subsystem, described > > in section 29.1, does have the OCP registers, sideband signals, etc., and > > thus should remain a hwmod-registered device (via DT). > > > > In terms of the clock data: based on section 29.1.4, section 29.1.5.2, > > figure 29-3, and table 29-4, there are several clock gating control bits. > > These should be modeled as clock nodes in the Linux common clock > > framework. Furthermore these registers are located inside the PWMSS > > subsystem itself, and are only accessible when the PWMSS IP block is > > functional in a PM runtime point of view: i.e., when the block is mapped > > into memory space, clocked and out of reset, etc. So the clock types for > > the PWMSS_CFG IP block should most likely be implemented in the PWMSS > > driver. I think you'll still be able to define the clock node data itself > > in DT, but this will probably need a closer look. Ideally, since the > > clock node register address data is the same for all three subsystem > > instances, one would be able to simply include a DT include file three > > times; but the DT binding data format may ultimately make this > > impractical. > > > > In terms of the transition from the old approach to this approach, it ooks > > to me like the first thing to do would be to convert > > drivers/pwm/pwm-tipwmss.c to define some clk_ops and register some clock > > nodes with the CCF. You've got the meat of the clock gating control code > > there already. Then the next thing to do would be to to get rid of > > pwmss_submodule_state_change() and use > > clk_{prepare,enable,disable,unprepare}*() in the drivers/pwm/pwm-ti*.c > > subdrivers instead. All of that looks like it should be possible to > > implement in a backwards-compatible way. Then you'd convert the eCAP, > > eQEP, and ePWM code to probe as platform_devices, driven from a simple-bus > > or pwmss-bus or whatever in the DT data. Naturally, the AM33xx/43xx data > > should be fixed also. > > I am working on updating this patchset on behalf of Vignesh > based on your suggestion. During the process of converting > the pwm-tipwmss.c to be a clock provider I discovered that > there is an issue with the PWMSS local clock gates. For > example on AM437x if you run the below commands you will get > a " Custom Error: MASTER M2 (64-bit) TARGET L4_PER_0 (Read): > Data Access in User mode during Functional access" error. > > rmmod pwm_tiecap > rmmod pwm_bl > rmmod pwm_tiecap > modprobe pwm_tiecap > modprobe pwm_bl > > Full Log: http://pastebin.com/sEyy52HV > > On device powerup, the various PWMSS local clock gates are > "unlocked". The remove call in the ecap driver gates the > local PWMSS clock gate for the ecap. The probe of the ecap > driver "unlocks" the clock gate for the ecap. However, once > you try to access any of the ecap registers which is > indirectly accessed by the pwm_bl driver you will get an > error. This is because the ecap clock is still being gated. OK I'm not sure I understand what's going on, particularly the part about locking and unlocking. Are you saying that the pwm_bl driver calls into the pwm_tiecap module to write to the PWMSS_CLKCONFIG registers to ungate the ECAP clock, and then the hardware silently ignores the write? If that's the case, shouldn't we be seeing some warning messages from a failure to ungate the clock from a subsequent PWMSS_CLKSTATUS poll? Or am I misunderstanding what's going on here? > This has been verified by hardware folks on our side. Its > been determined these registers are not needed since they > were designed for devices that didn't have a PRCM. > Therefore, the current plan is to update the trms of > AM335x,AM437x,DRA7 and AM57x to make it clear that these > registers shouldn't be touched. OK > I plan on sending a patch for the pwm-tipwmss.c driver > essentially removing anything that touches the > PWMSS_CLKCONFIG and PWMSS_CLKSTATUS registers. I will also > be sending a v3 version of this patch since I believe the > struct omap_hwmod dra7xx_ecap0_hwmod and similar entries you > had comments about before are ok to leave as is based on the > above findings. Including the comments regarding dra7xx_epwmss_sysc ? - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Paul On 02/18/2016 12:58 AM, Paul Walmsley wrote: > Hi Franklin > > On Wed, 17 Feb 2016, Franklin S Cooper Jr. wrote: > >> On 08/31/2015 10:51 AM, Paul Walmsley wrote: >>> On Thu, 16 Jul 2015, R, Vignesh wrote: >>>> On 07/16/2015 03:24 AM, Paul Walmsley wrote: >>>>> 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. >>> OK >>> >>>>>> --- 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. >>> OK >>> >>>>>> +/* 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? >>> It's certainly possible that we as maintainers didn't look closely enough >>> at the AM33xx data for the PWMSS when we merged it. But if that's >>> incorrect too, then now is the time to fix this. Otherwise it will never >>> get fixed, since each new group of people patching this code will keep >>> punting it off to the indeterminate future. >>> >>> In terms of hwmod data: based on the register maps in sections 29.4.3, >>> 29.3.3, and 29.2.3 of SPRUHZ6, none of these subdevices are hwmod devices. >>> They don't support the Highlander OCP registers, they have no individual >>> PRCM registers or register bitfields, and all of the idle and status >>> reporting is to the PWMSS top-level IP block itself. So it looks to me >>> like the eCAP, eQEP, and ePWM modules should be registered via DT, rather >>> than via hwmod. It looks like you can get away with using the >>> "simple-bus" abstraction, but you might ultimately have to define >>> something custom here. However, the PWMSS top level subsystem, described >>> in section 29.1, does have the OCP registers, sideband signals, etc., and >>> thus should remain a hwmod-registered device (via DT). >>> >>> In terms of the clock data: based on section 29.1.4, section 29.1.5.2, >>> figure 29-3, and table 29-4, there are several clock gating control bits. >>> These should be modeled as clock nodes in the Linux common clock >>> framework. Furthermore these registers are located inside the PWMSS >>> subsystem itself, and are only accessible when the PWMSS IP block is >>> functional in a PM runtime point of view: i.e., when the block is mapped >>> into memory space, clocked and out of reset, etc. So the clock types for >>> the PWMSS_CFG IP block should most likely be implemented in the PWMSS >>> driver. I think you'll still be able to define the clock node data itself >>> in DT, but this will probably need a closer look. Ideally, since the >>> clock node register address data is the same for all three subsystem >>> instances, one would be able to simply include a DT include file three >>> times; but the DT binding data format may ultimately make this >>> impractical. >>> >>> In terms of the transition from the old approach to this approach, it ooks >>> to me like the first thing to do would be to convert >>> drivers/pwm/pwm-tipwmss.c to define some clk_ops and register some clock >>> nodes with the CCF. You've got the meat of the clock gating control code >>> there already. Then the next thing to do would be to to get rid of >>> pwmss_submodule_state_change() and use >>> clk_{prepare,enable,disable,unprepare}*() in the drivers/pwm/pwm-ti*.c >>> subdrivers instead. All of that looks like it should be possible to >>> implement in a backwards-compatible way. Then you'd convert the eCAP, >>> eQEP, and ePWM code to probe as platform_devices, driven from a simple-bus >>> or pwmss-bus or whatever in the DT data. Naturally, the AM33xx/43xx data >>> should be fixed also. >> I am working on updating this patchset on behalf of Vignesh >> based on your suggestion. During the process of converting >> the pwm-tipwmss.c to be a clock provider I discovered that >> there is an issue with the PWMSS local clock gates. For >> example on AM437x if you run the below commands you will get >> a " Custom Error: MASTER M2 (64-bit) TARGET L4_PER_0 (Read): >> Data Access in User mode during Functional access" error. >> >> rmmod pwm_tiecap >> rmmod pwm_bl >> rmmod pwm_tiecap >> modprobe pwm_tiecap >> modprobe pwm_bl >> >> Full Log: http://pastebin.com/sEyy52HV >> >> On device powerup, the various PWMSS local clock gates are >> "unlocked". The remove call in the ecap driver gates the >> local PWMSS clock gate for the ecap. The probe of the ecap >> driver "unlocks" the clock gate for the ecap. However, once >> you try to access any of the ecap registers which is >> indirectly accessed by the pwm_bl driver you will get an >> error. This is because the ecap clock is still being gated. > OK I'm not sure I understand what's going on, particularly the part about > locking and unlocking. Are you saying that the pwm_bl driver calls into > the pwm_tiecap module to write to the PWMSS_CLKCONFIG registers to ungate > the ECAP clock, and then the hardware silently ignores the write? If > that's the case, shouldn't we be seeing some warning messages from a > failure to ungate the clock from a subsequent PWMSS_CLKSTATUS poll? Or am > I misunderstanding what's going on here? Pwm-tipwmss.c exports a function pwmss_submodule_state_change which interacts with PWMSS_CLKCONFIG and PWMSS_CLKSTATUS registers. Both pwm-tiehrpwm.c and pwm-tiecap.c calls this exported function to unlock the gate at probe time and lock the gate when the driver is removed. So when the gate fails to be unlocked after it previously is locked there isn't a way to know this via the PWMSS_CLKSTATUS registers. So the driver "believes" that the gate is unlocked and when pwm_bl runs it calls the set_polarity function. Set_polarity in the case of AM437x gp evm maps to the ecap's ecap_pwm_set_polarity function call. This call then attempts to write to the ecap registers which results in the external abort since the clock to the ecap is still gated. When the ecap and tihrpwm driver request to unlock the clock gate it already checks the XXX_CLK_EN_ACK bitfields within CLKSTATUS and it shows that the clocks "should" be unlocked. So there is an issue with the IP. >> This has been verified by hardware folks on our side. Its >> been determined these registers are not needed since they >> were designed for devices that didn't have a PRCM. >> Therefore, the current plan is to update the trms of >> AM335x,AM437x,DRA7 and AM57x to make it clear that these >> registers shouldn't be touched. > OK > >> I plan on sending a patch for the pwm-tipwmss.c driver >> essentially removing anything that touches the >> PWMSS_CLKCONFIG and PWMSS_CLKSTATUS registers. I will also >> be sending a v3 version of this patch since I believe the >> struct omap_hwmod dra7xx_ecap0_hwmod and similar entries you >> had comments about before are ok to leave as is based on the >> above findings. > Including the comments regarding dra7xx_epwmss_sysc ? Yes, I plan on addressing the change Vignesh acknowledged regarding changing RESETSTATUS to SOFTRESET. For your other comments I believe he already explained why it has to be set in that particular way. If there is anything that I missed or something that isn't clear please let me know. > > > - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Franklin, On Thu, 18 Feb 2016, Franklin S Cooper Jr. wrote: > On 02/18/2016 12:58 AM, Paul Walmsley wrote: > > > OK I'm not sure I understand what's going on, particularly the part about > > locking and unlocking. Are you saying that the pwm_bl driver calls into > > the pwm_tiecap module to write to the PWMSS_CLKCONFIG registers to ungate > > the ECAP clock, and then the hardware silently ignores the write? If > > that's the case, shouldn't we be seeing some warning messages from a > > failure to ungate the clock from a subsequent PWMSS_CLKSTATUS poll? Or am > > I misunderstanding what's going on here? > Pwm-tipwmss.c exports a function > pwmss_submodule_state_change which interacts with > PWMSS_CLKCONFIG and PWMSS_CLKSTATUS registers. Both > pwm-tiehrpwm.c and pwm-tiecap.c calls this exported function > to unlock the gate at probe time and lock the gate when the > driver is removed. > > So when the gate fails to be unlocked after it previously is > locked there isn't a way to know this via the > PWMSS_CLKSTATUS registers. OK I understand now. Sounds like the gating -> ungating transition is effectively a one-time operation, and once the ungating -> gating transition is done, there's no way to reverse it. Please correct me if I've got it wrong. > So the driver "believes" that the gate is unlocked and when pwm_bl runs > it calls the set_polarity function. Set_polarity in the case of AM437x > gp evm maps to the ecap's ecap_pwm_set_polarity function call. This call > then attempts to write to the ecap registers which results in the > external abort since the clock to the ecap is still gated. > > When the ecap and tihrpwm driver request to unlock the clock > gate it already checks the XXX_CLK_EN_ACK bitfields within > CLKSTATUS and it shows that the clocks "should" be unlocked. > So there is an issue with the IP. OK > Yes, I plan on addressing the change Vignesh acknowledged > regarding changing RESETSTATUS to SOFTRESET. For your other > comments I believe he already explained why it has to be set > in that particular way. If there is anything that I missed > or something that isn't clear please let me know. Perfect, thanks for the good description and the followup. - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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, + .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, + }, + }, +}; + +/* ecap0 */ +struct omap_hwmod dra7xx_ecap0_hwmod = { + .name = "ecap0", + .class = &dra7xx_ecap_hwmod_class, + .clkdm_name = "l4per2_clkdm", + .main_clk = "l4_root_clk_div", +}; + +/* eqep0 */ +struct omap_hwmod dra7xx_eqep0_hwmod = { + .name = "eqep0", + .class = &dra7xx_eqep_hwmod_class, + .clkdm_name = "l4per2_clkdm", + .main_clk = "l4_root_clk_div", +}; + +/* ehrpwm0 */ +struct omap_hwmod dra7xx_ehrpwm0_hwmod = { + .name = "ehrpwm0", + .class = &dra7xx_ehrpwm_hwmod_class, + .clkdm_name = "l4per2_clkdm", + .main_clk = "l4_root_clk_div", +}; + +/* epwmss1 */ +struct omap_hwmod dra7xx_epwmss1_hwmod = { + .name = "epwmss1", + .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_PWMSS2_CLKCTRL_OFFSET, + .context_offs = DRA7XX_RM_L4PER2_PWMSS2_CONTEXT_OFFSET, + }, + }, +}; + +/* ecap1 */ +struct omap_hwmod dra7xx_ecap1_hwmod = { + .name = "ecap1", + .class = &dra7xx_ecap_hwmod_class, + .clkdm_name = "l4per2_clkdm", + .main_clk = "l4_root_clk_div", +}; + +/* eqep1 */ +struct omap_hwmod dra7xx_eqep1_hwmod = { + .name = "eqep1", + .class = &dra7xx_eqep_hwmod_class, + .clkdm_name = "l4per2_clkdm", + .main_clk = "l4_root_clk_div", +}; + +/* ehrpwm1 */ +struct omap_hwmod dra7xx_ehrpwm1_hwmod = { + .name = "ehrpwm1", + .class = &dra7xx_ehrpwm_hwmod_class, + .clkdm_name = "l4per2_clkdm", + .main_clk = "l4_root_clk_div", +}; + +/* epwmss2 */ +struct omap_hwmod dra7xx_epwmss2_hwmod = { + .name = "epwmss2", + .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_PWMSS3_CLKCTRL_OFFSET, + .context_offs = DRA7XX_RM_L4PER2_PWMSS3_CONTEXT_OFFSET, + }, + }, +}; + +/* ecap2 */ +struct omap_hwmod dra7xx_ecap2_hwmod = { + .name = "ecap2", + .class = &dra7xx_ecap_hwmod_class, + .clkdm_name = "l4per2_clkdm", + .main_clk = "l4_root_clk_div", +}; + +/* eqep2 */ +struct omap_hwmod dra7xx_eqep2_hwmod = { + .name = "eqep2", + .class = &dra7xx_eqep_hwmod_class, + .clkdm_name = "l4per2_clkdm", + .main_clk = "l4_root_clk_div", +}; + +/* ehrpwm2 */ +struct omap_hwmod dra7xx_ehrpwm2_hwmod = { + .name = "ehrpwm2", + .class = &dra7xx_ehrpwm_hwmod_class, + .clkdm_name = "l4per2_clkdm", + .main_clk = "l4_root_clk_div", +}; + /* * 'dma' class * @@ -2601,6 +2744,90 @@ static struct omap_hwmod_ocp_if dra7xx_l4_per1__gpio6 = { .user = OCP_USER_MPU | OCP_USER_SDMA, }; +struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss0 = { + .master = &dra7xx_l4_per2_hwmod, + .slave = &dra7xx_epwmss0_hwmod, + .clk = "l4_root_clk_div", + .user = OCP_USER_MPU, +}; + +struct omap_hwmod_ocp_if dra7xx_epwmss0__ecap0 = { + .master = &dra7xx_epwmss0_hwmod, + .slave = &dra7xx_ecap0_hwmod, + .clk = "l4_root_clk_div", + .user = OCP_USER_MPU, +}; + +struct omap_hwmod_ocp_if dra7xx_epwmss0__eqep0 = { + .master = &dra7xx_epwmss0_hwmod, + .slave = &dra7xx_eqep0_hwmod, + .clk = "l4_root_clk_div", + .user = OCP_USER_MPU, +}; + +struct omap_hwmod_ocp_if dra7xx_epwmss0__ehrpwm0 = { + .master = &dra7xx_epwmss0_hwmod, + .slave = &dra7xx_ehrpwm0_hwmod, + .clk = "l4_root_clk_div", + .user = OCP_USER_MPU, +}; + +struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss1 = { + .master = &dra7xx_l4_per2_hwmod, + .slave = &dra7xx_epwmss1_hwmod, + .clk = "l4_root_clk_div", + .user = OCP_USER_MPU, +}; + +struct omap_hwmod_ocp_if dra7xx_epwmss1__ecap1 = { + .master = &dra7xx_epwmss1_hwmod, + .slave = &dra7xx_ecap1_hwmod, + .clk = "l4_root_clk_div", + .user = OCP_USER_MPU, +}; + +struct omap_hwmod_ocp_if dra7xx_epwmss1__eqep1 = { + .master = &dra7xx_epwmss1_hwmod, + .slave = &dra7xx_eqep1_hwmod, + .clk = "l4_root_clk_div", + .user = OCP_USER_MPU, +}; + +struct omap_hwmod_ocp_if dra7xx_epwmss1__ehrpwm1 = { + .master = &dra7xx_epwmss1_hwmod, + .slave = &dra7xx_ehrpwm1_hwmod, + .clk = "l4_root_clk_div", + .user = OCP_USER_MPU, +}; + +struct omap_hwmod_ocp_if dra7xx_l4_per2__epwmss2 = { + .master = &dra7xx_l4_per2_hwmod, + .slave = &dra7xx_epwmss2_hwmod, + .clk = "l4_root_clk_div", + .user = OCP_USER_MPU, +}; + +struct omap_hwmod_ocp_if dra7xx_epwmss2__ecap2 = { + .master = &dra7xx_epwmss2_hwmod, + .slave = &dra7xx_ecap2_hwmod, + .clk = "l4_root_clk_div", + .user = OCP_USER_MPU, +}; + +struct omap_hwmod_ocp_if dra7xx_epwmss2__eqep2 = { + .master = &dra7xx_epwmss2_hwmod, + .slave = &dra7xx_eqep2_hwmod, + .clk = "l4_root_clk_div", + .user = OCP_USER_MPU, +}; + +struct omap_hwmod_ocp_if dra7xx_epwmss2__ehrpwm2 = { + .master = &dra7xx_epwmss2_hwmod, + .slave = &dra7xx_ehrpwm2_hwmod, + .clk = "l4_root_clk_div", + .user = OCP_USER_MPU, +}; + /* l4_per1 -> gpio7 */ static struct omap_hwmod_ocp_if dra7xx_l4_per1__gpio7 = { .master = &dra7xx_l4_per1_hwmod, @@ -3394,6 +3621,18 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = { &dra7xx_l3_main_1__vcp2, &dra7xx_l4_per2__vcp2, &dra7xx_l4_wkup__wd_timer2, + &dra7xx_l4_per2__epwmss0, + &dra7xx_epwmss0__ecap0, + &dra7xx_epwmss0__eqep0, + &dra7xx_epwmss0__ehrpwm0, + &dra7xx_l4_per2__epwmss1, + &dra7xx_epwmss1__ecap1, + &dra7xx_epwmss1__eqep1, + &dra7xx_epwmss1__ehrpwm1, + &dra7xx_l4_per2__epwmss2, + &dra7xx_epwmss2__ecap2, + &dra7xx_epwmss2__eqep2, + &dra7xx_epwmss2__ehrpwm2, NULL, };
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. [1] www.ti.com/lit/ug/spruhz6/spruhz6.pdf Signed-off-by: Vignesh R <vigneshr@ti.com> --- v2: * add TRM references. arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 239 ++++++++++++++++++++++++++++++ 1 file changed, 239 insertions(+)