2015-06-03 12:29:24

by Vignesh Raghavendra

[permalink] [raw]
Subject: [PATCH v2 0/5] Add support for PWMSS on DRA7


Hi,

This patch series adds support for PWMSS on DRA7. The IP is same as that
present in AM33XX and AM43XX.
The first patch changes clock domain in which PWMSS is present
(l4per2_7xx_clkdm) to SW_WKUP. This is because legacy IPs like PWM
does'nt support HW_AUTO prorperly. Hence, switch clock domain to
SW_WKUP. This is based on the input from the hardware team.
The rest of the patches add hwmod and dt entries and enable PWMSS on
DRA7 based SoCs.

Vignesh R (5):
ARM: OMAP2+: DRA7: clockdomain: change l4per2_7xx_clkdm to SW_WKUP
ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS
ARM: dts: DRA7: Add TBCLK for PWMSS
clk: ti: DRA7: Add tbclk data for ehrpwm
ARM: dts: DRA7: Add dt nodes for PWMSS

.../devicetree/bindings/pwm/pwm-tiehrpwm.txt | 8 +
.../devicetree/bindings/pwm/pwm-tipwmss.txt | 17 +-
arch/arm/boot/dts/dra7.dtsi | 69 ++++++
arch/arm/boot/dts/dra7xx-clocks.dtsi | 26 +++
arch/arm/mach-omap2/clockdomains7xx_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 239 +++++++++++++++++++
drivers/clk/ti/clk-7xx.c | 3 +
7 files changed, 362 insertions(+), 2 deletions(-)

--
2.4.1


2015-06-03 12:29:20

by Vignesh Raghavendra

[permalink] [raw]
Subject: [PATCH v2 1/5] ARM: OMAP2+: DRA7: clockdomain: change l4per2_7xx_clkdm to SW_WKUP

Legacy IPs like PWMSS, present under l4per2_7xx_clkdm, cannot support
smart-idle when its clock domain is in HW_AUTO on DRA7 SoCs. Hence,
program clock domain to SW_WKUP.

Signed-off-by: Vignesh R <[email protected]>
---
arch/arm/mach-omap2/clockdomains7xx_data.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/clockdomains7xx_data.c b/arch/arm/mach-omap2/clockdomains7xx_data.c
index 57d5df0c1fbd..7581e036bda6 100644
--- a/arch/arm/mach-omap2/clockdomains7xx_data.c
+++ b/arch/arm/mach-omap2/clockdomains7xx_data.c
@@ -331,7 +331,7 @@ static struct clockdomain l4per2_7xx_clkdm = {
.dep_bit = DRA7XX_L4PER2_STATDEP_SHIFT,
.wkdep_srcs = l4per2_wkup_sleep_deps,
.sleepdep_srcs = l4per2_wkup_sleep_deps,
- .flags = CLKDM_CAN_HWSUP_SWSUP,
+ .flags = CLKDM_CAN_SWSUP,
};

static struct clockdomain mpu0_7xx_clkdm = {
--
2.4.1

2015-06-03 12:28:46

by Vignesh Raghavendra

[permalink] [raw]
Subject: [PATCH v2 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS

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] http://www.ti.com/lit/ug/spruhz6/spruhz6.pdf

Signed-off-by: Vignesh R <[email protected]>
---

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,
+ .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,
};

--
2.4.1

2015-06-03 12:26:26

by Vignesh Raghavendra

[permalink] [raw]
Subject: [PATCH v2 3/5] ARM: dts: DRA7: Add TBCLK for PWMSS

tbclk is used by ehrpwm to generate PWM waveform on DRA7 SoC. Add Linux
clock to control ehrpwm tbclk.
The TRM says, tbclk is derived from SYSCLKOUT. SYSCLKOUT is nothing but
ehrpwm functional clock derived from the gateable interface and
functional clock of PWMSS(l4_root_clk_div).
Refer AM57x TRM SPRUHZ6[1], October 2014, Table 29-4 and Section 29.2.2.1,
Table 29-19 and the NOTE at the end of the table.

[1] http://www.ti.com/lit/ug/spruhz6/spruhz6.pdf

Signed-off-by: Vignesh R <[email protected]>
---

v2:
* add TRM references.

arch/arm/boot/dts/dra7.dtsi | 5 +++++
arch/arm/boot/dts/dra7xx-clocks.dtsi | 26 ++++++++++++++++++++++++++
2 files changed, 31 insertions(+)

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index f03a091cd076..387c76ca41f9 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -131,6 +131,11 @@
regulator-max-microvolt = <3000000>;
};
};
+
+ scm_conf_clocks: clocks {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
};

dra7_pmx_core: pinmux@1400 {
diff --git a/arch/arm/boot/dts/dra7xx-clocks.dtsi b/arch/arm/boot/dts/dra7xx-clocks.dtsi
index 3b933f74d000..92452d61cf58 100644
--- a/arch/arm/boot/dts/dra7xx-clocks.dtsi
+++ b/arch/arm/boot/dts/dra7xx-clocks.dtsi
@@ -2136,3 +2136,29 @@
clocks = <&dpll_usb_ck>;
};
};
+
+&scm_conf_clocks {
+ ehrpwm0_tbclk: ehrpwm0_tbclk {
+ #clock-cells = <0>;
+ compatible = "ti,gate-clock";
+ clocks = <&l4_root_clk_div>;
+ ti,bit-shift = <20>;
+ reg = <0x0558>;
+ };
+
+ ehrpwm1_tbclk: ehrpwm1_tbclk {
+ #clock-cells = <0>;
+ compatible = "ti,gate-clock";
+ clocks = <&l4_root_clk_div>;
+ ti,bit-shift = <21>;
+ reg = <0x0558>;
+ };
+
+ ehrpwm2_tbclk: ehrpwm2_tbclk {
+ #clock-cells = <0>;
+ compatible = "ti,gate-clock";
+ clocks = <&l4_root_clk_div>;
+ ti,bit-shift = <22>;
+ reg = <0x0558>;
+ };
+};
--
2.4.1

2015-06-03 12:26:36

by Vignesh Raghavendra

[permalink] [raw]
Subject: [PATCH v2 4/5] clk: ti: DRA7: Add tbclk data for ehrpwm

tbclk is needed by ehrpwm to generate pwm waveforms. Hence, register
the required clock information.

Signed-off-by: Vignesh R <[email protected]>
---
drivers/clk/ti/clk-7xx.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
index 5d2217ae4478..366be43d15fe 100644
--- a/drivers/clk/ti/clk-7xx.c
+++ b/drivers/clk/ti/clk-7xx.c
@@ -304,6 +304,9 @@ static struct ti_dt_clk dra7xx_clks[] = {
DT_CLK("4882a000.timer", "timer_sys_ck", "timer_sys_clk_div"),
DT_CLK("4882c000.timer", "timer_sys_ck", "timer_sys_clk_div"),
DT_CLK("4882e000.timer", "timer_sys_ck", "timer_sys_clk_div"),
+ DT_CLK("4843e200.ehrpwm", "tbclk", "ehrpwm0_tbclk"),
+ DT_CLK("48440200.ehrpwm", "tbclk", "ehrpwm1_tbclk"),
+ DT_CLK("48442200.ehrpwm", "tbclk", "ehrpwm2_tbclk"),
DT_CLK(NULL, "sys_clkin", "sys_clkin1"),
{ .node_name = NULL },
};
--
2.4.1

2015-06-03 12:26:41

by Vignesh Raghavendra

[permalink] [raw]
Subject: [PATCH v2 5/5] ARM: dts: DRA7: Add dt nodes for PWMSS

Add PWMSS device tree nodes for DRA7 SoC family and add documentation
for dt bindings.

Signed-off-by: Vignesh R <[email protected]>
---
.../devicetree/bindings/pwm/pwm-tiehrpwm.txt | 8 +++
.../devicetree/bindings/pwm/pwm-tipwmss.txt | 17 +++++-
arch/arm/boot/dts/dra7.dtsi | 64 ++++++++++++++++++++++
3 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
index 9c100b2c5b23..25d91ae57de5 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-tiehrpwm.txt
@@ -4,6 +4,7 @@ Required properties:
- compatible: Must be "ti,<soc>-ehrpwm".
for am33xx - compatible = "ti,am33xx-ehrpwm";
for da850 - compatible = "ti,da850-ehrpwm", "ti,am33xx-ehrpwm";
+ for dra7xx - compatible = "ti,dra7xx-ehrpwm", "ti,am33xx-ehrpwm";
- #pwm-cells: should be 3. See pwm.txt in this directory for a description of
the cells format. The only third cell flag supported by this binding is
PWM_POLARITY_INVERTED.
@@ -27,3 +28,10 @@ ehrpwm0: ehrpwm@0 { /* EHRPWM on da850 */
#pwm-cells = <3>;
reg = <0x300000 0x2000>;
};
+
+ehrpwm0: ehrpwm@0 { /* EHRPWM on dra7xx */
+ compatible = "ti,dra7xx-ehrpwm", "ti,am33xx-ehrpwm";
+ #pwm-cells = <3>;
+ reg = <0x48440200 0x80>;
+ ti,hwmods = "ehrpwm0";
+};
diff --git a/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt b/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
index f7eae77f8354..9270ce6b2da2 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-tipwmss.txt
@@ -1,7 +1,9 @@
TI SOC based PWM Subsystem

Required properties:
-- compatible: Must be "ti,am33xx-pwmss";
+- compatible: Must be "ti,<soc>-pwmss".
+ for am33xx - compatible = "ti,am33xx-pwmss"
+ for dra7xx - compatible = "ti,dra7xx-pwmss", "ti,am33xx-pwmss"
- reg: physical base address and size of the registers map.
- address-cells: Specify the number of u32 entries needed in child nodes.
Should set to 1.
@@ -29,3 +31,16 @@ pwmss0: pwmss@48300000 {

/* child nodes go here */
};
+
+epwmss0: epwmss@4843e000 { /* On DRA7xx */
+ compatible = "ti,dra7xx-pwmss", "ti,am33xx-pwmss";
+ reg = <0x4843e000 0x30>;
+ ti,hwmods = "epwmss0";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0x4843e100 0x4843e100 0x80 /* ECAP */
+ 0x4843e180 0x4843e180 0x80 /* EQEP */
+ 0x4843e200 0x4843e200 0x80>; /* EHRPWM */
+
+ /* child nodes go here */
+};
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 387c76ca41f9..98a9203d6992 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -1474,6 +1474,70 @@
clocks = <&sys_clkin1>;
status = "disabled";
};
+
+ epwmss0: epwmss@4843e000 {
+ compatible = "ti,dra7xx-pwmss", "ti,am33xx-pwmss";
+ reg = <0x4843e000 0x30>;
+ ti,hwmods = "epwmss0";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ status = "disabled";
+ ranges = <0x4843e100 0x4843e100 0x80 /* ECAP */
+ 0x4843e180 0x4843e180 0x80 /* EQEP */
+ 0x4843e200 0x4843e200 0x80>;/* EHRPWM */
+
+ ehrpwm0: ehrpwm@4843e200 {
+ compatible = "ti,dra7xx-ehrpwm",
+ "ti,am33xx-ehrpwm";
+ #pwm-cells = <3>;
+ reg = <0x4843e200 0x80>;
+ ti,hwmods = "ehrpwm0";
+ status = "disabled";
+ };
+ };
+
+ 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";
+ status = "disabled";
+ };
+ };
+
+ epwmss2: epwmss@48442000 {
+ compatible = "ti,dra7xx-pwmss", "ti,am33xx-pwmss";
+ reg = <0x48442000 0x30>;
+ ti,hwmods = "epwmss2";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ status = "disabled";
+
+ ranges = <0x48442100 0x48442100 0x80 /* ECAP */
+ 0x48442180 0x48442180 0x80 /* EQEP */
+ 0x48442200 0x48442200 0x80>; /* EHRPWM */
+
+ ehrpwm2: ehrpwm@48442200 {
+ compatible = "ti,dra7xx-ehrpwm",
+ "ti,am33xx-ehrpwm";
+ #pwm-cells = <3>;
+ reg = <0x48442200 0x80>;
+ ti,hwmods = "ehrpwm2";
+ status = "disabled";
+ };
+ };
};

thermal_zones: thermal-zones {
--
2.4.1

2015-06-18 22:39:59

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] clk: ti: DRA7: Add tbclk data for ehrpwm

Quoting Vignesh R (2015-06-03 04:51:23)
> tbclk is needed by ehrpwm to generate pwm waveforms. Hence, register
> the required clock information.
>
> Signed-off-by: Vignesh R <[email protected]>

Looks good to me. Please feel free to add:

Acked-by: Michael Turquette <[email protected]>

Regards,
Mike

> ---
> drivers/clk/ti/clk-7xx.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/clk/ti/clk-7xx.c b/drivers/clk/ti/clk-7xx.c
> index 5d2217ae4478..366be43d15fe 100644
> --- a/drivers/clk/ti/clk-7xx.c
> +++ b/drivers/clk/ti/clk-7xx.c
> @@ -304,6 +304,9 @@ static struct ti_dt_clk dra7xx_clks[] = {
> DT_CLK("4882a000.timer", "timer_sys_ck", "timer_sys_clk_div"),
> DT_CLK("4882c000.timer", "timer_sys_ck", "timer_sys_clk_div"),
> DT_CLK("4882e000.timer", "timer_sys_ck", "timer_sys_clk_div"),
> + DT_CLK("4843e200.ehrpwm", "tbclk", "ehrpwm0_tbclk"),
> + DT_CLK("48440200.ehrpwm", "tbclk", "ehrpwm1_tbclk"),
> + DT_CLK("48442200.ehrpwm", "tbclk", "ehrpwm2_tbclk"),
> DT_CLK(NULL, "sys_clkin", "sys_clkin1"),
> { .node_name = NULL },
> };
> --
> 2.4.1
>

2015-07-06 06:12:54

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] Add support for PWMSS on DRA7



On Wednesday 03 June 2015 05:21 PM, Vignesh R wrote:
>
> Hi,
>
> This patch series adds support for PWMSS on DRA7. The IP is same as that
> present in AM33XX and AM43XX.
> The first patch changes clock domain in which PWMSS is present
> (l4per2_7xx_clkdm) to SW_WKUP. This is because legacy IPs like PWM
> does'nt support HW_AUTO prorperly. Hence, switch clock domain to
> SW_WKUP. This is based on the input from the hardware team.
> The rest of the patches add hwmod and dt entries and enable PWMSS on
> DRA7 based SoCs.

Gentle ping...


--
Regards
Vignesh

2015-07-07 12:50:18

by Tero Kristo

[permalink] [raw]
Subject: Re: [PATCH v2 0/5] Add support for PWMSS on DRA7

On 07/06/2015 09:11 AM, Vignesh R wrote:
>
>
> On Wednesday 03 June 2015 05:21 PM, Vignesh R wrote:
>>
>> Hi,
>>
>> This patch series adds support for PWMSS on DRA7. The IP is same as that
>> present in AM33XX and AM43XX.
>> The first patch changes clock domain in which PWMSS is present
>> (l4per2_7xx_clkdm) to SW_WKUP. This is because legacy IPs like PWM
>> does'nt support HW_AUTO prorperly. Hence, switch clock domain to
>> SW_WKUP. This is based on the input from the hardware team.
>> The rest of the patches add hwmod and dt entries and enable PWMSS on
>> DRA7 based SoCs.
>
> Gentle ping...
>
>

Patches 1-4 look good to me, not going to try to review patch 5 as I
have no clue about PWM driver itself. So, for 1-4:

Acked-by: Tero Kristo <[email protected]>

Some of the patches cause trivial merge conflicts with 4.2-rc1 though.

-Tero

2015-07-15 19:56:28

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] ARM: OMAP2+: DRA7: clockdomain: change l4per2_7xx_clkdm to SW_WKUP

On Wed, 3 Jun 2015, Vignesh R wrote:

> Legacy IPs like PWMSS, present under l4per2_7xx_clkdm, cannot support
> smart-idle when its clock domain is in HW_AUTO on DRA7 SoCs. Hence,
> program clock domain to SW_WKUP.
>
> Signed-off-by: Vignesh R <[email protected]>
> ---
> arch/arm/mach-omap2/clockdomains7xx_data.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/clockdomains7xx_data.c b/arch/arm/mach-omap2/clockdomains7xx_data.c
> index 57d5df0c1fbd..7581e036bda6 100644
> --- a/arch/arm/mach-omap2/clockdomains7xx_data.c
> +++ b/arch/arm/mach-omap2/clockdomains7xx_data.c
> @@ -331,7 +331,7 @@ static struct clockdomain l4per2_7xx_clkdm = {
> .dep_bit = DRA7XX_L4PER2_STATDEP_SHIFT,
> .wkdep_srcs = l4per2_wkup_sleep_deps,
> .sleepdep_srcs = l4per2_wkup_sleep_deps,
> - .flags = CLKDM_CAN_HWSUP_SWSUP,
> + .flags = CLKDM_CAN_SWSUP,
> };
>
> static struct clockdomain mpu0_7xx_clkdm = {

Thanks, queued for v4.2-rc fixes. Note that I cannot test this, since I
don't have a DRA7xx board.

- Paul

2015-07-15 20:27:19

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] ARM: OMAP2+: DRA7: clockdomain: change l4per2_7xx_clkdm to SW_WKUP

On Wed, 15 Jul 2015, Paul Walmsley wrote:

> On Wed, 3 Jun 2015, Vignesh R wrote:
>
> > Legacy IPs like PWMSS, present under l4per2_7xx_clkdm, cannot support
> > smart-idle when its clock domain is in HW_AUTO on DRA7 SoCs. Hence,
> > program clock domain to SW_WKUP.
> >
> > Signed-off-by: Vignesh R <[email protected]>
> > ---
> > arch/arm/mach-omap2/clockdomains7xx_data.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-omap2/clockdomains7xx_data.c b/arch/arm/mach-omap2/clockdomains7xx_data.c
> > index 57d5df0c1fbd..7581e036bda6 100644
> > --- a/arch/arm/mach-omap2/clockdomains7xx_data.c
> > +++ b/arch/arm/mach-omap2/clockdomains7xx_data.c
> > @@ -331,7 +331,7 @@ static struct clockdomain l4per2_7xx_clkdm = {
> > .dep_bit = DRA7XX_L4PER2_STATDEP_SHIFT,
> > .wkdep_srcs = l4per2_wkup_sleep_deps,
> > .sleepdep_srcs = l4per2_wkup_sleep_deps,
> > - .flags = CLKDM_CAN_HWSUP_SWSUP,
> > + .flags = CLKDM_CAN_SWSUP,
> > };
> >
> > static struct clockdomain mpu0_7xx_clkdm = {
>
> Thanks, queued for v4.2-rc fixes. Note that I cannot test this, since I
> don't have a DRA7xx board.

You know, upon further thought, this doesn't make sense. If the bug
is with the PWMSS IP block specifically, why not just set
HWMOD_SWSUP_SIDLE on all the IP blocks where the hardware folks didn't
implement hardware smart-idle? At least that way, if those legacy IP
blocks aren't in use, the clockdomain can still enter hardware-supervised
idle?

This patch basically disables hardware-supervised/smart idle for all of
the IP blocks in that clockdomain?


- Paul

2015-07-15 21:54:25

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS

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] http://www.ti.com/lit/ug/spruhz6/spruhz6.pdf
>
> Signed-off-by: Vignesh R <[email protected]>
> ---
>
> 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

2015-07-16 15:28:28

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] ARM: OMAP2+: DRA7: clockdomain: change l4per2_7xx_clkdm to SW_WKUP

Hi,

On 07/16/2015 01:57 AM, Paul Walmsley wrote:
> On Wed, 15 Jul 2015, Paul Walmsley wrote:
>
>> On Wed, 3 Jun 2015, Vignesh R wrote:
>>
>>> Legacy IPs like PWMSS, present under l4per2_7xx_clkdm, cannot support
>>> smart-idle when its clock domain is in HW_AUTO on DRA7 SoCs. Hence,
>>> program clock domain to SW_WKUP.
>>>
>>> Signed-off-by: Vignesh R <[email protected]>
>>> ---
>>> arch/arm/mach-omap2/clockdomains7xx_data.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/clockdomains7xx_data.c b/arch/arm/mach-omap2/clockdomains7xx_data.c
>>> index 57d5df0c1fbd..7581e036bda6 100644
>>> --- a/arch/arm/mach-omap2/clockdomains7xx_data.c
>>> +++ b/arch/arm/mach-omap2/clockdomains7xx_data.c
>>> @@ -331,7 +331,7 @@ static struct clockdomain l4per2_7xx_clkdm = {
>>> .dep_bit = DRA7XX_L4PER2_STATDEP_SHIFT,
>>> .wkdep_srcs = l4per2_wkup_sleep_deps,
>>> .sleepdep_srcs = l4per2_wkup_sleep_deps,
>>> - .flags = CLKDM_CAN_HWSUP_SWSUP,
>>> + .flags = CLKDM_CAN_SWSUP,
>>> };
>>>
>>> static struct clockdomain mpu0_7xx_clkdm = {
>>
>> Thanks, queued for v4.2-rc fixes. Note that I cannot test this, since I
>> don't have a DRA7xx board.
>
> You know, upon further thought, this doesn't make sense. If the bug
> is with the PWMSS IP block specifically, why not just set
> HWMOD_SWSUP_SIDLE on all the IP blocks where the hardware folks didn't
> implement hardware smart-idle? At least that way, if those legacy IP
> blocks aren't in use, the clockdomain can still enter hardware-supervised
> idle?

According to hardware folks, HW_AUTO (for clockdomain) and
HWMOD_SWSUP_SIDLE (PWMSS in NO-IDLE) for PWMSS *is not* a good
combination. If clockdomain is in HW_AUTO and PWMSS is put in NO-IDLE,
then IDLEST of PWMSSx_CLKCTRL reads "stuck in wakeup/ sleep transition"
which is not a consistent state (this is because of problems with PWM
IP). Hence, it is recommended to program the clock domain to SW_WKUP and
leave the PWMSS idlemode as smart-idle.

--
Regards
Vignesh

2015-07-16 15:33:01

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS

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] http://www.ti.com/lit/ug/spruhz6/spruhz6.pdf
>>
>> Signed-off-by: Vignesh R <[email protected]>
>> ---
>>
>> 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?



--
Regards
Vignesh

2015-07-23 15:36:34

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS



On 7/16/2015 9:01 PM, R, Vignesh wrote:
> Hi,
>
> On 07/16/2015 03:24 AM, Paul Walmsley wrote:
>> Hi,
>>
>> some comments.
>>
>> On Wed, 3 Jun 2015, Vignesh R wrote:
>>
>>> Add hwmod entries for the PWMSS on DRA7.
>>>
>>> Set l4_root_clk_div as the main_clk of PWMSS. It is fixed-factored clock
>>> equal to L4PER2_L3_GICLK/2(l3_iclk_div/2).
>>> As per AM57x TRM SPRUHZ6[1], October 2014, Section 29.1.3 Table 29-4,
>>> clock source to PWMSS is L4PER2_L3_GICLK. But it is actually
>>> L4PER2_L3_GICLK/2. The TRM does not show the division by 2.
>>
>> Is the divide-by-two coming from PWMSS_EPWM.EPWM_TBCTL[HSPCLKDIV]? Or is
>> HSPCLKDIV a separate divider after the divide-by-2 you mention above?
>
> No, it not related to HSPCLKDIV. The TRM wrongly states L4PER2_L3_GICLK
> as clock input for PWMSS. But actually L4PER2_L4_GICLK(=L3_GICLK/2) is
> the clock input for PWMSS. This will be updated in TRM soon.
>
>>
>>> [1] http://www.ti.com/lit/ug/spruhz6/spruhz6.pdf
>>>
>>> Signed-off-by: Vignesh R <[email protected]>
>>> ---
>>>
>>> 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

2015-07-29 06:33:51

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS



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] http://www.ti.com/lit/ug/spruhz6/spruhz6.pdf
>>>>
>>>> Signed-off-by: Vignesh R <[email protected]>
>>>> ---
>>>>
>>>> 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?

--
Regards
Vignesh

2015-08-31 15:51:10

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] ARM: OMAP2+: DRA7: Add hwmod entries for PWMSS

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