| Message ID | 1456439796-28546-3-git-send-email-fcooper@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi guys On Thu, 25 Feb 2016, Franklin S Cooper Jr wrote: > From: Vignesh R <vigneshr@ti.com> > > 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> So I still don't understand one thing about this patch, and I apologize if this has been covered already and I've just forgotten it. Why are EQEP, ECAP, EHRPWM listed as hwmods? It looks, based on this data, that they don't have any of the Highlander integration. Shouldn't these just be listed in a DT 'simple-bus' type of arrangement under epwmss0, epwmss1? Or am I missing something? - Paul > --- > Version 3 changes: > Switch from SYSC_HAS_RESET_STATUS to SYSC_HAS_SOFTRESET which is the > correct bitfield for that register. > > 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 848356e..4b2d68b 100644 > --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c > +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c > @@ -383,6 +383,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_SOFTRESET, > + .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 > * > @@ -2676,6 +2819,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, > @@ -3452,6 +3679,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.7.0 > - 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
* Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]: > From: Vignesh R <vigneshr@ti.com> > > 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 Looks OK to me, assuming Paul will pick this one or ack it. FYI, the URL above is outdated, looks like there's spruhz7a.pdf available. Not sure if that's been corrected for the divider? Regards, Tony -- 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 Tue, 1 Mar 2016, Tony Lindgren wrote: > * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]: > > From: Vignesh R <vigneshr@ti.com> > > > > 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 > > Looks OK to me, assuming Paul will pick this one or ack it. Well I've already sent comments on it, it doesn't look quite ready for me yet. I would hold off on the whole series because the hwmod comments also impact the DT files. - 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
* Paul Walmsley <paul@pwsan.com> [160301 10:59]: > On Tue, 1 Mar 2016, Tony Lindgren wrote: > > > * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]: > > > From: Vignesh R <vigneshr@ti.com> > > > > > > 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 > > > > Looks OK to me, assuming Paul will pick this one or ack it. > > Well I've already sent comments on it, it doesn't look quite ready for me > yet. I would hold off on the whole series because the hwmod comments also > impact the DT files. OK will drop the dt related patches then. Regards, Tony -- 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 03/01/2016 02:50 PM, Tony Lindgren wrote: > * Paul Walmsley <paul@pwsan.com> [160301 10:59]: >> On Tue, 1 Mar 2016, Tony Lindgren wrote: >> >>> * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]: >>>> From: Vignesh R <vigneshr@ti.com> >>>> >>>> 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 >>> Looks OK to me, assuming Paul will pick this one or ack it. >> Well I've already sent comments on it, it doesn't look quite ready for me >> yet. I would hold off on the whole series because the hwmod comments also >> impact the DT files. > OK will drop the dt related patches then. Sorry you previously asked this question about why hwmod is used for eCap, ePWM and eQEP before and it wasn't addressed. I'll take a look at this and I will get back to you. > > Regards, > > Tony -- 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 03/02/2016 10:22 AM, Franklin S Cooper Jr. wrote: > Hi Paul > > On 03/01/2016 02:50 PM, Tony Lindgren wrote: >> * Paul Walmsley <paul@pwsan.com> [160301 10:59]: >>> On Tue, 1 Mar 2016, Tony Lindgren wrote: >>> >>>> * Franklin S Cooper Jr <fcooper@ti.com> [160225 14:37]: >>>>> From: Vignesh R <vigneshr@ti.com> >>>>> >>>>> 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 >>>> Looks OK to me, assuming Paul will pick this one or ack it. >>> Well I've already sent comments on it, it doesn't look quite ready for me >>> yet. I would hold off on the whole series because the hwmod comments also >>> impact the DT files. >> OK will drop the dt related patches then. > Sorry you previously asked this question about why hwmod is > used for eCap, ePWM and eQEP before and it wasn't addressed. > I'll take a look at this and I will get back to you. So I looked into this more and verified that the eCAP and ePWM doesn't have their own unique clock. The PWMSS receives a clock L4PER2_L3_GICLK/2 which is passed through to its sub-devices (ePWM, eCAP and eQEP). The PWMSS is responsible for handling its clock internally while the subdevices have no role in managing this clock. So this explains why we have hwmod entries for PWMSS and why we are planning on removing it from the various subdevices. Since ePWM, eCAP and eQEP are subdevices of PWMSS they shouldn't have their own concept of their "own" clock. The ePWM , eCAP and eQEP clocks are all shared and managed by their parent PWMSS. Once the PWMSS is enabled and has its clock running then ePWM, eCAP and eQEP from their main clock perspective have everything they need. So my plan is to strip all references of clocks (including hwmod entries) for ePWM, eCAP and eQEP. The devm_clk_get calls made in the ePWM and eCAP will simply point to their parent's dev (PWMSS). I did a couple of quick test using this approach and it works. I have more testing to do but if that checks out are you ok with the above approach? Also I'm not sure how simple-bus fits in this picture. The eCAP, eQEP and ePWM are all separate devices. The only thing that they share is a single clock from their parent. So it doesn't seem like the right approach. I'm basing this on the info in this thread https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg27979.html that talks about the usage of simple-bus. So if its outdated or I"m misinterpreting it incorrectly please let me know. > >> Regards, >> >> Tony -- 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 Thu, 3 Mar 2016, Franklin S Cooper Jr. wrote: > So I looked into this more and verified that the eCAP and > ePWM doesn't have their own unique clock. The PWMSS receives > a clock L4PER2_L3_GICLK/2 which is passed through to its > sub-devices (ePWM, eCAP and eQEP). The PWMSS is responsible > for handling its clock internally while the subdevices have > no role in managing this clock. So this explains why we have > hwmod entries for PWMSS and why we are planning on removing > it from the various subdevices. It's not whether they have their own unique clock, but whether the submodules have OCP integration registers, speak the idle/standby protocols, have direct L3/L4 ports, etc. > Since ePWM, eCAP and eQEP are subdevices of PWMSS they > shouldn't have their own concept of their "own" clock. The > ePWM , eCAP and eQEP clocks are all shared and managed by > their parent PWMSS. Once the PWMSS is enabled and has its > clock running then ePWM, eCAP and eQEP from their main clock > perspective have everything they need. > > So my plan is to strip all references of clocks (including > hwmod entries) for ePWM, eCAP and eQEP. The devm_clk_get > calls made in the ePWM and eCAP will simply point to their > parent's dev (PWMSS). I did a couple of quick test using > this approach and it works. I have more testing to do but if > that checks out are you ok with the above approach? I don't know if that should be done or not. I haven't stared at the code yet, but based on your description, it sounds to me that it probably shouldn't be done. In any case, it's not what I meant... > Also I'm not sure how simple-bus fits in this picture. The > eCAP, eQEP and ePWM are all separate devices. The only thing > that they share is a single clock from their parent. So it > doesn't seem like the right approach. I'm basing this on the > info in this thread > https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg27979.html > that talks about the usage of simple-bus. So if its outdated > or I"m misinterpreting it incorrectly please let me know. What I meant is that the ECAP*, EQEP*, EHRPWM* devices don't need to be registered through the hwmod code, due to the fact that they don't have the integration mentioned above. Instead, I think those three subdevices should be listed as child nodes of epwmss* in the DT. Looking at the DT data from Vignesh, it looks like he's already got ehrpwm1 as a child node of the epwmss1: + epwmss1: epwmss@48440000 { + compatible = "ti,dra7xx-pwmss", "ti,am33xx-pwmss"; + reg = <0x48440000 0x30>; + ti,hwmods = "epwmss1"; + #address-cells = <1>; + #size-cells = <1>; + status = "disabled"; + ranges = <0x48440100 0x48440100 0x80 /* ECAP */ + 0x48440180 0x48440180 0x80 /* EQEP */ + 0x48440200 0x48440200 0x80>; /* EHRPWM */ + + ehrpwm1: ehrpwm@48440200 { + compatible = "ti,dra7xx-ehrpwm", + "ti,am33xx-ehrpwm"; + #pwm-cells = <3>; + reg = <0x48440200 0x80>; + ti,hwmods = "ehrpwm1"; So, drop the above line, since the subdevices don't have corresponding hwmods any more. + status = "disabled"; + }; Then, here, you'd add nodes similar to ehrpwm1 for ecap1 and eqep1. I can't remember at the moment if adding "simple-bus" to the epwmss1 string would be sufficient to register the subdevices after the epwmss1 is probed. If so, maybe that's all you need. + }; Then repeat for epwmss0, epwmss2. - 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 03/04/2016 12:25 AM, Paul Walmsley wrote: > On Thu, 3 Mar 2016, Franklin S Cooper Jr. wrote: > >> So I looked into this more and verified that the eCAP and >> ePWM doesn't have their own unique clock. The PWMSS receives >> a clock L4PER2_L3_GICLK/2 which is passed through to its >> sub-devices (ePWM, eCAP and eQEP). The PWMSS is responsible >> for handling its clock internally while the subdevices have >> no role in managing this clock. So this explains why we have >> hwmod entries for PWMSS and why we are planning on removing >> it from the various subdevices. > It's not whether they have their own unique clock, but whether the > submodules have OCP integration registers, speak the idle/standby > protocols, have direct L3/L4 ports, etc. Sorry from a hwmod perspective your right. I initially thought my response about the clocks were related to hwmod but I see why I was off after reading a bit more on hwmod. >> Since ePWM, eCAP and eQEP are subdevices of PWMSS they >> shouldn't have their own concept of their "own" clock. The >> ePWM , eCAP and eQEP clocks are all shared and managed by >> their parent PWMSS. Once the PWMSS is enabled and has its >> clock running then ePWM, eCAP and eQEP from their main clock >> perspective have everything they need. >> >> So my plan is to strip all references of clocks (including >> hwmod entries) for ePWM, eCAP and eQEP. The devm_clk_get >> calls made in the ePWM and eCAP will simply point to their >> parent's dev (PWMSS). I did a couple of quick test using >> this approach and it works. I have more testing to do but if >> that checks out are you ok with the above approach? > I don't know if that should be done or not. I haven't stared at the code > yet, but based on your description, it sounds to me that it probably > shouldn't be done. In any case, it's not what I meant... Your right my response did miss your initial point. But what I'm proposing sounds like exactly what you were suggesting. I think we both agree that hwmod entries for ePWM, eCAP and eQEP should be removed. Not just for dra7 but also for am335x and am437x as separate patches. My comment about devm_clk_get was based on the fact that I don't believe eCAP, ePWM and eQEP should have their own clock defined even within DT. Instead they can just reference the clk provided by pwmss (its parent) which in hardware is the clock that is directly passed to the sub-devices. The only reason those drivers need a reference to that clk is to get its rate once during probe. I plan on doing a bit more testing and I can shoot an updated v4. I believe it will address all of your comments and you will hopefully get a better understanding of the minor change I plan on making. > >> Also I'm not sure how simple-bus fits in this picture. The >> eCAP, eQEP and ePWM are all separate devices. The only thing >> that they share is a single clock from their parent. So it >> doesn't seem like the right approach. I'm basing this on the >> info in this thread >> https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg27979.html >> that talks about the usage of simple-bus. So if its outdated >> or I"m misinterpreting it incorrectly please let me know. > What I meant is that the ECAP*, EQEP*, EHRPWM* devices don't need to be > registered through the hwmod code, due to the fact that they don't have > the integration mentioned above. Instead, I think those three subdevices > should be listed as child nodes of epwmss* in the DT. > > Looking at the DT data from Vignesh, it looks like he's already got > ehrpwm1 as a child node of the epwmss1: > > + epwmss1: epwmss@48440000 { > + compatible = "ti,dra7xx-pwmss", "ti,am33xx-pwmss"; > + reg = <0x48440000 0x30>; > + ti,hwmods = "epwmss1"; > + #address-cells = <1>; > + #size-cells = <1>; > + status = "disabled"; > + ranges = <0x48440100 0x48440100 0x80 /* ECAP */ > + 0x48440180 0x48440180 0x80 /* EQEP */ > + 0x48440200 0x48440200 0x80>; /* EHRPWM */ > + > + ehrpwm1: ehrpwm@48440200 { > + compatible = "ti,dra7xx-ehrpwm", > + "ti,am33xx-ehrpwm"; > + #pwm-cells = <3>; > + reg = <0x48440200 0x80>; > + ti,hwmods = "ehrpwm1"; > > So, drop the above line, since the subdevices don't have corresponding > hwmods any more. Right > > + status = "disabled"; > + }; > > Then, here, you'd add nodes similar to ehrpwm1 for ecap1 and eqep1. I > can't remember at the moment if adding "simple-bus" to the epwmss1 string > would be sufficient to register the subdevices after the epwmss1 is > probed. If so, maybe that's all you need. > > + }; > > Then repeat for epwmss0, epwmss2. As you mentioned pwmss is the parent node in the DT while ecap and epwm are child nodes. These child nodes must be probed only after their parent node has been probed and the parent clock is running. The pwm-tipwmss.c already calls of_platform_populate(node, NULL, NULL, &pdev->dev); as the very last step of its probe call. So what you want to happen is already happening in the driver. So I don't believe simple-bus is needed. > > > - 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 Fri, 4 Mar 2016, Franklin S Cooper Jr. wrote: > So what you want to happen > is already happening in the driver. So I don't believe > simple-bus is needed. OK great, even better! - 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 848356e..4b2d68b 100644 --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c @@ -383,6 +383,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_SOFTRESET, + .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 * @@ -2676,6 +2819,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, @@ -3452,6 +3679,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, };