2019-07-23 21:29:10

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 0/8] ti-sysc related warning fixes for v5.3-rc cycle

Hi all,

I noticed that with recent ti-sysc driver changes some new warnings
have crept in. Mostly they are caused by having different configuration
in the dts compared to the legacy platform data. Let's fix these first
before we continue dropping the legacy platform data.

I also noticed we need two fixes for the ti-sysc driver while looking
at the warnings.

Regards,

Tony

Tony Lindgren (8):
ARM: OMAP2+: Fix missing SYSC_HAS_RESET_STATUS for dra7 epwmss
ARM: OMAP2+: Remove unconfigured midlemode for am3 lcdc
bus: ti-sysc: Fix handling of forced idle
bus: ti-sysc: Fix using configured sysc mask value
ARM: dts: Drop bogus ahclkr clocks for dra7 mcasp 3 to 8
ARM: dts: Fix flags for gpio7
ARM: dts: Fix incorrect dcan register mapping for am3, am4 and dra7
ARM: dts: Fix lcdc sysc flags for am3

arch/arm/boot/dts/am33xx-l4.dtsi | 6 +++-
arch/arm/boot/dts/am437x-l4.dtsi | 4 +++
.../boot/dts/am57xx-beagle-x15-common.dtsi | 2 +-
arch/arm/boot/dts/dra7-evm.dts | 2 +-
arch/arm/boot/dts/dra7-l4.dtsi | 31 ++++++++-----------
arch/arm/mach-omap2/omap_hwmod_33xx_data.c | 2 +-
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 3 +-
drivers/bus/ti-sysc.c | 10 +++---
8 files changed, 31 insertions(+), 29 deletions(-)

--
2.21.0


2019-07-23 21:30:34

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 2/8] ARM: OMAP2+: Remove unconfigured midlemode for am3 lcdc

We currently get a warning for lcdc because of a difference
with dts provided configuration compared to the legacy platform
data. This is because lcdc has SYSC_HAS_MIDLEMODE configured in
the platform data without configuring the modes.

Let's fix the warning by removing SYSC_HAS_MIDLEMODE. Note that
the am335x TRM lists SYSC_HAS_MIDLEMODE, but it is unused.

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

diff --git a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
--- a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
@@ -231,7 +231,7 @@ static struct omap_hwmod am33xx_control_hwmod = {
static struct omap_hwmod_class_sysconfig lcdc_sysc = {
.rev_offs = 0x0,
.sysc_offs = 0x54,
- .sysc_flags = (SYSC_HAS_SIDLEMODE | SYSC_HAS_MIDLEMODE),
+ .sysc_flags = SYSC_HAS_SIDLEMODE,
.idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
.sysc_fields = &omap_hwmod_sysc_type2,
};
--
2.21.0

2019-07-23 21:31:40

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 4/8] bus: ti-sysc: Fix using configured sysc mask value

We have cases where there are no softreset bits like with am335x lcdc.
In that case ti,sysc-mask = <0> needs to be handled properly.

Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/bus/ti-sysc.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -1692,10 +1692,7 @@ static int sysc_init_sysc_mask(struct sysc *ddata)
if (error)
return 0;

- if (val)
- ddata->cfg.sysc_val = val & ddata->cap->sysc_mask;
- else
- ddata->cfg.sysc_val = ddata->cap->sysc_mask;
+ ddata->cfg.sysc_val = val & ddata->cap->sysc_mask;

return 0;
}
--
2.21.0

2019-07-23 21:35:05

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 8/8] ARM: dts: Fix lcdc sysc flags for am3

The LCDC controller on am335x has a sysconfig register without a
SOFTRESET bit. Let's configure that by setting ti,sysc-mask = <0>
as otherwise we get the following warning:

ti-sysc 4830e000.target-module: idlemodes 00000087 != 00000007

And the legacy platform data has LCDC midle unconfigured so let's
remove that property.

Signed-off-by: Tony Lindgren <[email protected]>
---
arch/arm/boot/dts/am33xx-l4.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/am33xx-l4.dtsi b/arch/arm/boot/dts/am33xx-l4.dtsi
--- a/arch/arm/boot/dts/am33xx-l4.dtsi
+++ b/arch/arm/boot/dts/am33xx-l4.dtsi
@@ -2038,7 +2038,7 @@
reg = <0xe000 0x4>,
<0xe054 0x4>;
reg-names = "rev", "sysc";
- ti,sysc-midle ;
+ ti,sysc-mask = <0>;
ti,sysc-sidle = <SYSC_IDLE_FORCE>,
<SYSC_IDLE_NO>,
<SYSC_IDLE_SMART>;
--
2.21.0

2019-07-23 21:52:55

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 1/8] ARM: OMAP2+: Fix missing SYSC_HAS_RESET_STATUS for dra7 epwmss

TRM says PWMSS_SYSCONFIG bit for SOFTRESET changes to zero when
reset is completed. Let's configure it as otherwise we get warnings
on boot when we check the data against dts provided data. Eventually
the legacy platform data will be just dropped, but let's fix the
warning first.

Signed-off-by: Tony Lindgren <[email protected]>
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -379,7 +379,8 @@ static struct omap_hwmod dra7xx_dcan2_hwmod = {
static struct omap_hwmod_class_sysconfig dra7xx_epwmss_sysc = {
.rev_offs = 0x0,
.sysc_offs = 0x4,
- .sysc_flags = SYSC_HAS_SIDLEMODE | SYSC_HAS_SOFTRESET,
+ .sysc_flags = SYSC_HAS_SIDLEMODE | SYSC_HAS_SOFTRESET |
+ SYSC_HAS_RESET_STATUS,
.idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
.sysc_fields = &omap_hwmod_sysc_type2,
};
--
2.21.0

2019-07-23 21:53:29

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 3/8] bus: ti-sysc: Fix handling of forced idle

For some devices we can get the following warning on boot:

ti-sysc 48485200.target-module: sysc_disable_module: invalid midlemode

Fix this by treating SYSC_IDLE_FORCE like we do for the other bits
for idlemodes mask.

Fixes: d59b60564cbf ("bus: ti-sysc: Add generic enable/disable functions")
Cc: Roger Quadros <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/bus/ti-sysc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -949,7 +949,7 @@ static int sysc_best_idle_mode(u32 idlemodes, u32 *best_mode)
*best_mode = SYSC_IDLE_SMART_WKUP;
else if (idlemodes & BIT(SYSC_IDLE_SMART))
*best_mode = SYSC_IDLE_SMART;
- else if (idlemodes & SYSC_IDLE_FORCE)
+ else if (idlemodes & BIT(SYSC_IDLE_FORCE))
*best_mode = SYSC_IDLE_FORCE;
else
return -EINVAL;
--
2.21.0

2019-07-23 21:55:31

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 5/8] ARM: dts: Drop bogus ahclkr clocks for dra7 mcasp 3 to 8

The ahclkr clkctrl clock bit 28 only exists for mcasp 1 and 2 on dra7.
Otherwise we get the following warning on beagle-x15:

ti-sysc 48468000.target-module: could not add child clock ahclkr: -19

Fixes: 5241ccbf2819 ("ARM: dts: Add missing ranges for dra7 mcasp l3 ports")
Signed-off-by: Tony Lindgren <[email protected]>
---
arch/arm/boot/dts/dra7-l4.dtsi | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi
--- a/arch/arm/boot/dts/dra7-l4.dtsi
+++ b/arch/arm/boot/dts/dra7-l4.dtsi
@@ -2818,9 +2818,8 @@
<SYSC_IDLE_SMART>;
/* Domains (P, C): l4per_pwrdm, l4per2_clkdm */
clocks = <&l4per2_clkctrl DRA7_L4PER2_MCASP3_CLKCTRL 0>,
- <&l4per2_clkctrl DRA7_L4PER2_MCASP3_CLKCTRL 24>,
- <&l4per2_clkctrl DRA7_L4PER2_MCASP3_CLKCTRL 28>;
- clock-names = "fck", "ahclkx", "ahclkr";
+ <&l4per2_clkctrl DRA7_L4PER2_MCASP3_CLKCTRL 24>;
+ clock-names = "fck", "ahclkx";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x68000 0x2000>,
@@ -2854,9 +2853,8 @@
<SYSC_IDLE_SMART>;
/* Domains (P, C): l4per_pwrdm, l4per2_clkdm */
clocks = <&l4per2_clkctrl DRA7_L4PER2_MCASP4_CLKCTRL 0>,
- <&l4per2_clkctrl DRA7_L4PER2_MCASP4_CLKCTRL 24>,
- <&l4per2_clkctrl DRA7_L4PER2_MCASP4_CLKCTRL 28>;
- clock-names = "fck", "ahclkx", "ahclkr";
+ <&l4per2_clkctrl DRA7_L4PER2_MCASP4_CLKCTRL 24>;
+ clock-names = "fck", "ahclkx";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x6c000 0x2000>,
@@ -2890,9 +2888,8 @@
<SYSC_IDLE_SMART>;
/* Domains (P, C): l4per_pwrdm, l4per2_clkdm */
clocks = <&l4per2_clkctrl DRA7_L4PER2_MCASP5_CLKCTRL 0>,
- <&l4per2_clkctrl DRA7_L4PER2_MCASP5_CLKCTRL 24>,
- <&l4per2_clkctrl DRA7_L4PER2_MCASP5_CLKCTRL 28>;
- clock-names = "fck", "ahclkx", "ahclkr";
+ <&l4per2_clkctrl DRA7_L4PER2_MCASP5_CLKCTRL 24>;
+ clock-names = "fck", "ahclkx";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x70000 0x2000>,
@@ -2926,9 +2923,8 @@
<SYSC_IDLE_SMART>;
/* Domains (P, C): l4per_pwrdm, l4per2_clkdm */
clocks = <&l4per2_clkctrl DRA7_L4PER2_MCASP6_CLKCTRL 0>,
- <&l4per2_clkctrl DRA7_L4PER2_MCASP6_CLKCTRL 24>,
- <&l4per2_clkctrl DRA7_L4PER2_MCASP6_CLKCTRL 28>;
- clock-names = "fck", "ahclkx", "ahclkr";
+ <&l4per2_clkctrl DRA7_L4PER2_MCASP6_CLKCTRL 24>;
+ clock-names = "fck", "ahclkx";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x74000 0x2000>,
@@ -2962,9 +2958,8 @@
<SYSC_IDLE_SMART>;
/* Domains (P, C): l4per_pwrdm, l4per2_clkdm */
clocks = <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 0>,
- <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 24>,
- <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 28>;
- clock-names = "fck", "ahclkx", "ahclkr";
+ <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 24>;
+ clock-names = "fck", "ahclkx";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0x0 0x78000 0x2000>,
--
2.21.0

2019-07-23 22:12:24

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 6/8] ARM: dts: Fix flags for gpio7

The ti,no-idle-on-init and ti,no-reset-on-init flags need to be at
the interconnect target module level for the modules that have it
defined. Otherwise we get the following warnings:

dts flag should be at module level for ti,no-idle-on-init
dts flag should be at module level for ti,no-reset-on-init

Signed-off-by: Tony Lindgren <[email protected]>
---
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi | 2 +-
arch/arm/boot/dts/dra7-evm.dts | 2 +-
arch/arm/boot/dts/dra7-l4.dtsi | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi b/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
--- a/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
+++ b/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
@@ -379,7 +379,7 @@
};
};

-&gpio7 {
+&gpio7_target {
ti,no-reset-on-init;
ti,no-idle-on-init;
};
diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
--- a/arch/arm/boot/dts/dra7-evm.dts
+++ b/arch/arm/boot/dts/dra7-evm.dts
@@ -498,7 +498,7 @@
phy-supply = <&ldousb_reg>;
};

-&gpio7 {
+&gpio7_target {
ti,no-reset-on-init;
ti,no-idle-on-init;
};
diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi
--- a/arch/arm/boot/dts/dra7-l4.dtsi
+++ b/arch/arm/boot/dts/dra7-l4.dtsi
@@ -1261,7 +1261,7 @@
};
};

- target-module@51000 { /* 0x48051000, ap 45 2e.0 */
+ gpio7_target: target-module@51000 { /* 0x48051000, ap 45 2e.0 */
compatible = "ti,sysc-omap2", "ti,sysc";
ti,hwmods = "gpio7";
reg = <0x51000 0x4>,
--
2.21.0

2019-07-23 22:12:28

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 7/8] ARM: dts: Fix incorrect dcan register mapping for am3, am4 and dra7

We are currently using a wrong register for dcan revision. Although
this is currently only used for detecting the dcan module, let's
fix it to avoid confusion.

Signed-off-by: Tony Lindgren <[email protected]>
---
arch/arm/boot/dts/am33xx-l4.dtsi | 4 ++++
arch/arm/boot/dts/am437x-l4.dtsi | 4 ++++
arch/arm/boot/dts/dra7-l4.dtsi | 4 ++--
drivers/bus/ti-sysc.c | 3 ++-
4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/am33xx-l4.dtsi b/arch/arm/boot/dts/am33xx-l4.dtsi
--- a/arch/arm/boot/dts/am33xx-l4.dtsi
+++ b/arch/arm/boot/dts/am33xx-l4.dtsi
@@ -1758,6 +1758,8 @@

target-module@cc000 { /* 0x481cc000, ap 60 46.0 */
compatible = "ti,sysc-omap4", "ti,sysc";
+ reg = <0xcc020 0x4>;
+ reg-names = "rev";
ti,hwmods = "d_can0";
/* Domains (P, C): per_pwrdm, l4ls_clkdm */
clocks = <&l4ls_clkctrl AM3_L4LS_D_CAN0_CLKCTRL 0>,
@@ -1780,6 +1782,8 @@

target-module@d0000 { /* 0x481d0000, ap 62 42.0 */
compatible = "ti,sysc-omap4", "ti,sysc";
+ reg = <0xd0020 0x4>;
+ reg-names = "rev";
ti,hwmods = "d_can1";
/* Domains (P, C): per_pwrdm, l4ls_clkdm */
clocks = <&l4ls_clkctrl AM3_L4LS_D_CAN1_CLKCTRL 0>,
diff --git a/arch/arm/boot/dts/am437x-l4.dtsi b/arch/arm/boot/dts/am437x-l4.dtsi
--- a/arch/arm/boot/dts/am437x-l4.dtsi
+++ b/arch/arm/boot/dts/am437x-l4.dtsi
@@ -1574,6 +1574,8 @@

target-module@cc000 { /* 0x481cc000, ap 50 46.0 */
compatible = "ti,sysc-omap4", "ti,sysc";
+ reg = <0xcc020 0x4>;
+ reg-names = "rev";
ti,hwmods = "d_can0";
/* Domains (P, C): per_pwrdm, l4ls_clkdm */
clocks = <&l4ls_clkctrl AM4_L4LS_D_CAN0_CLKCTRL 0>;
@@ -1593,6 +1595,8 @@

target-module@d0000 { /* 0x481d0000, ap 52 3a.0 */
compatible = "ti,sysc-omap4", "ti,sysc";
+ reg = <0xd0020 0x4>;
+ reg-names = "rev";
ti,hwmods = "d_can1";
/* Domains (P, C): per_pwrdm, l4ls_clkdm */
clocks = <&l4ls_clkctrl AM4_L4LS_D_CAN1_CLKCTRL 0>;
diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi
--- a/arch/arm/boot/dts/dra7-l4.dtsi
+++ b/arch/arm/boot/dts/dra7-l4.dtsi
@@ -3020,7 +3020,7 @@

target-module@80000 { /* 0x48480000, ap 31 16.0 */
compatible = "ti,sysc-omap4", "ti,sysc";
- reg = <0x80000 0x4>;
+ reg = <0x80020 0x4>;
reg-names = "rev";
clocks = <&l4per2_clkctrl DRA7_L4PER2_DCAN2_CLKCTRL 0>;
clock-names = "fck";
@@ -4572,7 +4572,7 @@

target-module@c000 { /* 0x4ae3c000, ap 30 04.0 */
compatible = "ti,sysc-omap4", "ti,sysc";
- reg = <0xc000 0x4>;
+ reg = <0xc020 0x4>;
reg-names = "rev";
clocks = <&wkupaon_clkctrl DRA7_WKUPAON_DCAN1_CLKCTRL 0>;
clock-names = "fck";
diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -1267,7 +1267,8 @@ static const struct sysc_revision_quirk sysc_revision_quirks[] = {
SYSC_QUIRK("control", 0, 0, 0x10, -1, 0x40000900, 0xffffffff, 0),
SYSC_QUIRK("cpgmac", 0, 0x1200, 0x1208, 0x1204, 0x4edb1902,
0xffff00f0, 0),
- SYSC_QUIRK("dcan", 0, 0, -1, -1, 0xffffffff, 0xffffffff, 0),
+ SYSC_QUIRK("dcan", 0, 0x20, -1, -1, 0xa3170504, 0xffffffff, 0),
+ SYSC_QUIRK("dcan", 0, 0x20, -1, -1, 0x4edb1902, 0xffffffff, 0),
SYSC_QUIRK("dmic", 0, 0, 0x10, -1, 0x50010000, 0xffffffff, 0),
SYSC_QUIRK("dwc3", 0, 0, 0x10, -1, 0x500a0200, 0xffffffff, 0),
SYSC_QUIRK("epwmss", 0, 0, 0x4, -1, 0x47400001, 0xffffffff, 0),
--
2.21.0

2019-07-24 02:03:43

by Suman Anna

[permalink] [raw]
Subject: Re: [PATCH 1/8] ARM: OMAP2+: Fix missing SYSC_HAS_RESET_STATUS for dra7 epwmss

On 7/23/19 6:28 AM, Tony Lindgren wrote:
> TRM says PWMSS_SYSCONFIG bit for SOFTRESET changes to zero when
> reset is completed. Let's configure it as otherwise we get warnings
> on boot when we check the data against dts provided data. Eventually
> the legacy platform data will be just dropped, but let's fix the
> warning first.
>
> Signed-off-by: Tony Lindgren <[email protected]>

Reviewed-by: Suman Anna <[email protected]>

regards
Suman

> ---
> arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -379,7 +379,8 @@ static struct omap_hwmod dra7xx_dcan2_hwmod = {
> static struct omap_hwmod_class_sysconfig dra7xx_epwmss_sysc = {
> .rev_offs = 0x0,
> .sysc_offs = 0x4,
> - .sysc_flags = SYSC_HAS_SIDLEMODE | SYSC_HAS_SOFTRESET,
> + .sysc_flags = SYSC_HAS_SIDLEMODE | SYSC_HAS_SOFTRESET |
> + SYSC_HAS_RESET_STATUS,
> .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
> .sysc_fields = &omap_hwmod_sysc_type2,
> };
>

2019-07-24 02:11:59

by Suman Anna

[permalink] [raw]
Subject: Re: [PATCH 3/8] bus: ti-sysc: Fix handling of forced idle

On 7/23/19 6:28 AM, Tony Lindgren wrote:
> For some devices we can get the following warning on boot:
>
> ti-sysc 48485200.target-module: sysc_disable_module: invalid midlemode
>
> Fix this by treating SYSC_IDLE_FORCE like we do for the other bits
> for idlemodes mask.
>
> Fixes: d59b60564cbf ("bus: ti-sysc: Add generic enable/disable functions")
> Cc: Roger Quadros <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>

Reviewed-by: Suman Anna <[email protected]>

regards
Suman

> ---
> drivers/bus/ti-sysc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -949,7 +949,7 @@ static int sysc_best_idle_mode(u32 idlemodes, u32 *best_mode)
> *best_mode = SYSC_IDLE_SMART_WKUP;
> else if (idlemodes & BIT(SYSC_IDLE_SMART))
> *best_mode = SYSC_IDLE_SMART;
> - else if (idlemodes & SYSC_IDLE_FORCE)
> + else if (idlemodes & BIT(SYSC_IDLE_FORCE))
> *best_mode = SYSC_IDLE_FORCE;
> else
> return -EINVAL;
>

2019-07-24 02:28:39

by Suman Anna

[permalink] [raw]
Subject: Re: [PATCH 2/8] ARM: OMAP2+: Remove unconfigured midlemode for am3 lcdc

+ Jyri

On 7/23/19 6:28 AM, Tony Lindgren wrote:
> We currently get a warning for lcdc because of a difference
> with dts provided configuration compared to the legacy platform
> data. This is because lcdc has SYSC_HAS_MIDLEMODE configured in
> the platform data without configuring the modes.

Hi Tony,
While I understand that you are trying to match the DT data with the
existing legacy data, do you know if there was a reason why this was
omitted in the first place? Should we be really adding the MSTANDBY_
flags and fix up the DTS node accordingly? I tried looking through the
git log, and the initial commit itself didn't add the MSTANDBY_ flags
but used the SYSC_HAS_MIDLEMODE.

Jyri,
Do you know the history?

regards
Suman

>
> Let's fix the warning by removing SYSC_HAS_MIDLEMODE. Note that
> the am335x TRM lists SYSC_HAS_MIDLEMODE, but it is unused.



>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
> arch/arm/mach-omap2/omap_hwmod_33xx_data.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
> --- a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
> @@ -231,7 +231,7 @@ static struct omap_hwmod am33xx_control_hwmod = {
> static struct omap_hwmod_class_sysconfig lcdc_sysc = {
> .rev_offs = 0x0,
> .sysc_offs = 0x54,
> - .sysc_flags = (SYSC_HAS_SIDLEMODE | SYSC_HAS_MIDLEMODE),
> + .sysc_flags = SYSC_HAS_SIDLEMODE,
> .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
> .sysc_fields = &omap_hwmod_sysc_type2,
> };
>

2019-07-24 02:31:35

by Suman Anna

[permalink] [raw]
Subject: Re: [PATCH 5/8] ARM: dts: Drop bogus ahclkr clocks for dra7 mcasp 3 to 8

Hi Tony,

On 7/23/19 6:28 AM, Tony Lindgren wrote:
> The ahclkr clkctrl clock bit 28 only exists for mcasp 1 and 2 on dra7.
> Otherwise we get the following warning on beagle-x15:
>
> ti-sysc 48468000.target-module: could not add child clock ahclkr: -19
>
> Fixes: 5241ccbf2819 ("ARM: dts: Add missing ranges for dra7 mcasp l3 ports")
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
> arch/arm/boot/dts/dra7-l4.dtsi | 25 ++++++++++---------------
> 1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi
> --- a/arch/arm/boot/dts/dra7-l4.dtsi
> +++ b/arch/arm/boot/dts/dra7-l4.dtsi
> @@ -2818,9 +2818,8 @@
> <SYSC_IDLE_SMART>;
> /* Domains (P, C): l4per_pwrdm, l4per2_clkdm */
> clocks = <&l4per2_clkctrl DRA7_L4PER2_MCASP3_CLKCTRL 0>,
> - <&l4per2_clkctrl DRA7_L4PER2_MCASP3_CLKCTRL 24>,
> - <&l4per2_clkctrl DRA7_L4PER2_MCASP3_CLKCTRL 28>;
> - clock-names = "fck", "ahclkx", "ahclkr";
> + <&l4per2_clkctrl DRA7_L4PER2_MCASP3_CLKCTRL 24>;
> + clock-names = "fck", "ahclkx";
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x0 0x68000 0x2000>,
> @@ -2854,9 +2853,8 @@
> <SYSC_IDLE_SMART>;
> /* Domains (P, C): l4per_pwrdm, l4per2_clkdm */
> clocks = <&l4per2_clkctrl DRA7_L4PER2_MCASP4_CLKCTRL 0>,
> - <&l4per2_clkctrl DRA7_L4PER2_MCASP4_CLKCTRL 24>,
> - <&l4per2_clkctrl DRA7_L4PER2_MCASP4_CLKCTRL 28>;
> - clock-names = "fck", "ahclkx", "ahclkr";
> + <&l4per2_clkctrl DRA7_L4PER2_MCASP4_CLKCTRL 24>;
> + clock-names = "fck", "ahclkx";
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x0 0x6c000 0x2000>,
> @@ -2890,9 +2888,8 @@
> <SYSC_IDLE_SMART>;
> /* Domains (P, C): l4per_pwrdm, l4per2_clkdm */
> clocks = <&l4per2_clkctrl DRA7_L4PER2_MCASP5_CLKCTRL 0>,
> - <&l4per2_clkctrl DRA7_L4PER2_MCASP5_CLKCTRL 24>,
> - <&l4per2_clkctrl DRA7_L4PER2_MCASP5_CLKCTRL 28>;
> - clock-names = "fck", "ahclkx", "ahclkr";
> + <&l4per2_clkctrl DRA7_L4PER2_MCASP5_CLKCTRL 24>;
> + clock-names = "fck", "ahclkx";
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x0 0x70000 0x2000>,
> @@ -2926,9 +2923,8 @@
> <SYSC_IDLE_SMART>;
> /* Domains (P, C): l4per_pwrdm, l4per2_clkdm */
> clocks = <&l4per2_clkctrl DRA7_L4PER2_MCASP6_CLKCTRL 0>,
> - <&l4per2_clkctrl DRA7_L4PER2_MCASP6_CLKCTRL 24>,
> - <&l4per2_clkctrl DRA7_L4PER2_MCASP6_CLKCTRL 28>;
> - clock-names = "fck", "ahclkx", "ahclkr";
> + <&l4per2_clkctrl DRA7_L4PER2_MCASP6_CLKCTRL 24>;
> + clock-names = "fck", "ahclkx";
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x0 0x74000 0x2000>,
> @@ -2962,9 +2958,8 @@
> <SYSC_IDLE_SMART>;
> /* Domains (P, C): l4per_pwrdm, l4per2_clkdm */
> clocks = <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 0>,
> - <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 24>,
> - <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 28>;
> - clock-names = "fck", "ahclkx", "ahclkr";
> + <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 24>;
> + clock-names = "fck", "ahclkx";

The equivalent change to MCASP8 is missing.

regards
Suman

> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0x0 0x78000 0x2000>,
>

2019-07-24 02:34:16

by Suman Anna

[permalink] [raw]
Subject: Re: [PATCH 6/8] ARM: dts: Fix flags for gpio7

Hi Tony,

On 7/23/19 6:28 AM, Tony Lindgren wrote:
> The ti,no-idle-on-init and ti,no-reset-on-init flags need to be at
> the interconnect target module level for the modules that have it
> defined. Otherwise we get the following warnings:
>
> dts flag should be at module level for ti,no-idle-on-init
> dts flag should be at module level for ti,no-reset-on-init
>
> Signed-off-by: Tony Lindgren <[email protected]>

There's a similar one within the am335x-icev2.dts file for gpio0
that can also use this fix.

Reviewed-by: Suman Anna <[email protected]>

regards
Suman

> ---
> arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi | 2 +-
> arch/arm/boot/dts/dra7-evm.dts | 2 +-
> arch/arm/boot/dts/dra7-l4.dtsi | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi b/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
> --- a/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
> +++ b/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
> @@ -379,7 +379,7 @@
> };
> };
>
> -&gpio7 {
> +&gpio7_target {
> ti,no-reset-on-init;
> ti,no-idle-on-init;
> };
> diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts
> --- a/arch/arm/boot/dts/dra7-evm.dts
> +++ b/arch/arm/boot/dts/dra7-evm.dts
> @@ -498,7 +498,7 @@
> phy-supply = <&ldousb_reg>;
> };
>
> -&gpio7 {
> +&gpio7_target {
> ti,no-reset-on-init;
> ti,no-idle-on-init;
> };
> diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi
> --- a/arch/arm/boot/dts/dra7-l4.dtsi
> +++ b/arch/arm/boot/dts/dra7-l4.dtsi
> @@ -1261,7 +1261,7 @@
> };
> };
>
> - target-module@51000 { /* 0x48051000, ap 45 2e.0 */
> + gpio7_target: target-module@51000 { /* 0x48051000, ap 45 2e.0 */
> compatible = "ti,sysc-omap2", "ti,sysc";
> ti,hwmods = "gpio7";
> reg = <0x51000 0x4>,
>

2019-07-24 05:51:00

by Keerthy

[permalink] [raw]
Subject: Re: [PATCH 2/8] ARM: OMAP2+: Remove unconfigured midlemode for am3 lcdc



On 24/07/19 12:33 AM, Suman Anna wrote:
> + Jyri
>
> On 7/23/19 6:28 AM, Tony Lindgren wrote:
>> We currently get a warning for lcdc because of a difference
>> with dts provided configuration compared to the legacy platform
>> data. This is because lcdc has SYSC_HAS_MIDLEMODE configured in
>> the platform data without configuring the modes.
>
> Hi Tony,
> While I understand that you are trying to match the DT data with the
> existing legacy data, do you know if there was a reason why this was
> omitted in the first place? Should we be really adding the MSTANDBY_
> flags and fix up the DTS node accordingly? I tried looking through the
> git log, and the initial commit itself didn't add the MSTANDBY_ flags
> but used the SYSC_HAS_MIDLEMODE.
>
> Jyri,
> Do you know the history?

Tony/Suman,

This patch breaks DS0 on am3.

- Keerthy

>
> regards
> Suman
>
>>
>> Let's fix the warning by removing SYSC_HAS_MIDLEMODE. Note that
>> the am335x TRM lists SYSC_HAS_MIDLEMODE, but it is unused.
>
>
>
>>
>> Signed-off-by: Tony Lindgren <[email protected]>
>> ---
>> arch/arm/mach-omap2/omap_hwmod_33xx_data.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
>> --- a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
>> @@ -231,7 +231,7 @@ static struct omap_hwmod am33xx_control_hwmod = {
>> static struct omap_hwmod_class_sysconfig lcdc_sysc = {
>> .rev_offs = 0x0,
>> .sysc_offs = 0x54,
>> - .sysc_flags = (SYSC_HAS_SIDLEMODE | SYSC_HAS_MIDLEMODE),
>> + .sysc_flags = SYSC_HAS_SIDLEMODE,
>> .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
>> .sysc_fields = &omap_hwmod_sysc_type2,
>> };
>>
>

2019-07-24 05:53:16

by Keerthy

[permalink] [raw]
Subject: Re: [PATCH 0/8] ti-sysc related warning fixes for v5.3-rc cycle



On 23/07/19 4:58 PM, Tony Lindgren wrote:
> Hi all,
>
> I noticed that with recent ti-sysc driver changes some new warnings
> have crept in. Mostly they are caused by having different configuration
> in the dts compared to the legacy platform data. Let's fix these first
> before we continue dropping the legacy platform data.
>
> I also noticed we need two fixes for the ti-sysc driver while looking
> at the warnings.

Tony,

Apart from Patch 2(breaks DS0 on AM3). Rest all work fine.

Tested for DS0/RTC+ddr on AM4, DS0 on AM3 Boneblack.

You can add my:

Tested-by: Keerthy <[email protected]>

For all the 7 patches except Patch 2.

Regards,
Keerthy

>
> Regards,
>
> Tony
>
> Tony Lindgren (8):
> ARM: OMAP2+: Fix missing SYSC_HAS_RESET_STATUS for dra7 epwmss
> ARM: OMAP2+: Remove unconfigured midlemode for am3 lcdc
> bus: ti-sysc: Fix handling of forced idle
> bus: ti-sysc: Fix using configured sysc mask value
> ARM: dts: Drop bogus ahclkr clocks for dra7 mcasp 3 to 8
> ARM: dts: Fix flags for gpio7
> ARM: dts: Fix incorrect dcan register mapping for am3, am4 and dra7
> ARM: dts: Fix lcdc sysc flags for am3
>
> arch/arm/boot/dts/am33xx-l4.dtsi | 6 +++-
> arch/arm/boot/dts/am437x-l4.dtsi | 4 +++
> .../boot/dts/am57xx-beagle-x15-common.dtsi | 2 +-
> arch/arm/boot/dts/dra7-evm.dts | 2 +-
> arch/arm/boot/dts/dra7-l4.dtsi | 31 ++++++++-----------
> arch/arm/mach-omap2/omap_hwmod_33xx_data.c | 2 +-
> arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 3 +-
> drivers/bus/ti-sysc.c | 10 +++---
> 8 files changed, 31 insertions(+), 29 deletions(-)
>

2019-07-24 06:32:04

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 2/8] ARM: OMAP2+: Remove unconfigured midlemode for am3 lcdc

* Keerthy <[email protected]> [190724 05:50]:
>
> On 24/07/19 12:33 AM, Suman Anna wrote:
> > + Jyri
> >
> > On 7/23/19 6:28 AM, Tony Lindgren wrote:
> > > We currently get a warning for lcdc because of a difference
> > > with dts provided configuration compared to the legacy platform
> > > data. This is because lcdc has SYSC_HAS_MIDLEMODE configured in
> > > the platform data without configuring the modes.
> >
> > Hi Tony,
> > While I understand that you are trying to match the DT data with the
> > existing legacy data, do you know if there was a reason why this was
> > omitted in the first place? Should we be really adding the MSTANDBY_
> > flags and fix up the DTS node accordingly? I tried looking through the
> > git log, and the initial commit itself didn't add the MSTANDBY_ flags
> > but used the SYSC_HAS_MIDLEMODE.

Yes the goal is to get rid of all errors and warnings in dmesg output
so we can spot the real issues.

> > Jyri,
> > Do you know the history?
>
> Tony/Suman,
>
> This patch breaks DS0 on am3.

OK thanks for testing. Let's drop this for now, sounds like there is
some midlemode configuration happening even with no flags set.

Probably the right fix is to configure the usable midlemodes instead
both for platform data and dts data and then drop the platform data.

Regards,

Tony



> > > Let's fix the warning by removing SYSC_HAS_MIDLEMODE. Note that
> > > the am335x TRM lists SYSC_HAS_MIDLEMODE, but it is unused.
> >
> >
> >
> > >
> > > Signed-off-by: Tony Lindgren <[email protected]>
> > > ---
> > > arch/arm/mach-omap2/omap_hwmod_33xx_data.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
> > > --- a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
> > > +++ b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
> > > @@ -231,7 +231,7 @@ static struct omap_hwmod am33xx_control_hwmod = {
> > > static struct omap_hwmod_class_sysconfig lcdc_sysc = {
> > > .rev_offs = 0x0,
> > > .sysc_offs = 0x54,
> > > - .sysc_flags = (SYSC_HAS_SIDLEMODE | SYSC_HAS_MIDLEMODE),
> > > + .sysc_flags = SYSC_HAS_SIDLEMODE,
> > > .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
> > > .sysc_fields = &omap_hwmod_sysc_type2,
> > > };
> > >
> >

2019-07-24 06:49:51

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 5/8] ARM: dts: Drop bogus ahclkr clocks for dra7 mcasp 3 to 8

* Suman Anna <[email protected]> [190723 21:02]:
> Hi Tony,
>
> On 7/23/19 6:28 AM, Tony Lindgren wrote:
> > The ahclkr clkctrl clock bit 28 only exists for mcasp 1 and 2 on dra7.
> > Otherwise we get the following warning on beagle-x15:
...
> > @@ -2962,9 +2958,8 @@
> > <SYSC_IDLE_SMART>;
> > /* Domains (P, C): l4per_pwrdm, l4per2_clkdm */
> > clocks = <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 0>,
> > - <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 24>,
> > - <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 28>;
> > - clock-names = "fck", "ahclkx", "ahclkr";
> > + <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 24>;
> > + clock-names = "fck", "ahclkx";
>
> The equivalent change to MCASP8 is missing.

Thanks for spotting it, probably should be set up the same way as
MCASP4 too looking at the TRM.

Tero, care to check the dra7 mcasp clocks we have defined?

$ grep MCASP drivers/clk/ti/clk-7xx.c
{ DRA7_IPU_MCASP1_CLKCTRL, dra7_mcasp1_bit_data, CLKF_SW_SUP, "ipu-clkctrl:0000:22" },
{ DRA7_L4PER2_MCASP2_CLKCTRL, dra7_mcasp2_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:0154:22" },
{ DRA7_L4PER2_MCASP3_CLKCTRL, dra7_mcasp3_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:015c:22" },
{ DRA7_L4PER2_MCASP5_CLKCTRL, dra7_mcasp5_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:016c:22" },
{ DRA7_L4PER2_MCASP8_CLKCTRL, dra7_mcasp8_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:0184:24" },
{ DRA7_L4PER2_MCASP4_CLKCTRL, dra7_mcasp4_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:018c:22" },
{ DRA7_L4PER2_MCASP6_CLKCTRL, dra7_mcasp6_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:01f8:22" },
{ DRA7_L4PER2_MCASP7_CLKCTRL, dra7_mcasp7_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:01fc:22" },

Is bit 24 above correct for MCASP8 or should it too be 22 like
adjacent MCASP4 in the TRM?

Regards,

Tony

2019-07-24 21:50:07

by Suman Anna

[permalink] [raw]
Subject: Re: [PATCH 2/8] ARM: OMAP2+: Remove unconfigured midlemode for am3 lcdc

On 7/24/19 1:31 AM, Tony Lindgren wrote:
> * Keerthy <[email protected]> [190724 05:50]:
>>
>> On 24/07/19 12:33 AM, Suman Anna wrote:
>>> + Jyri
>>>
>>> On 7/23/19 6:28 AM, Tony Lindgren wrote:
>>>> We currently get a warning for lcdc because of a difference
>>>> with dts provided configuration compared to the legacy platform
>>>> data. This is because lcdc has SYSC_HAS_MIDLEMODE configured in
>>>> the platform data without configuring the modes.
>>>
>>> Hi Tony,
>>> While I understand that you are trying to match the DT data with the
>>> existing legacy data, do you know if there was a reason why this was
>>> omitted in the first place? Should we be really adding the MSTANDBY_
>>> flags and fix up the DTS node accordingly? I tried looking through the
>>> git log, and the initial commit itself didn't add the MSTANDBY_ flags
>>> but used the SYSC_HAS_MIDLEMODE.
>
> Yes the goal is to get rid of all errors and warnings in dmesg output
> so we can spot the real issues.
>
>>> Jyri,
>>> Do you know the history?
>>
>> Tony/Suman,
>>
>> This patch breaks DS0 on am3.
>
> OK thanks for testing. Let's drop this for now, sounds like there is
> some midlemode configuration happening even with no flags set.

You were dropping the ti,sysc-midle property in patch 8, is that still
ok without this patch?

>
> Probably the right fix is to configure the usable midlemodes instead
> both for platform data and dts data and then drop the platform data.

Yeah, that's probably better and more accurate unless we actually have
some h/w issues that require them to be dropped.

regards
Suman

>
> Regards,
>
> Tony
>
>
>
>>>> Let's fix the warning by removing SYSC_HAS_MIDLEMODE. Note that
>>>> the am335x TRM lists SYSC_HAS_MIDLEMODE, but it is unused.
>>>
>>>
>>>
>>>>
>>>> Signed-off-by: Tony Lindgren <[email protected]>
>>>> ---
>>>> arch/arm/mach-omap2/omap_hwmod_33xx_data.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
>>>> --- a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
>>>> +++ b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
>>>> @@ -231,7 +231,7 @@ static struct omap_hwmod am33xx_control_hwmod = {
>>>> static struct omap_hwmod_class_sysconfig lcdc_sysc = {
>>>> .rev_offs = 0x0,
>>>> .sysc_offs = 0x54,
>>>> - .sysc_flags = (SYSC_HAS_SIDLEMODE | SYSC_HAS_MIDLEMODE),
>>>> + .sysc_flags = SYSC_HAS_SIDLEMODE,
>>>> .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
>>>> .sysc_fields = &omap_hwmod_sysc_type2,
>>>> };
>>>>
>>>

2019-08-13 11:05:17

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 2/8] ARM: OMAP2+: Remove unconfigured midlemode for am3 lcdc

* Suman Anna <[email protected]> [190724 18:29]:
> On 7/24/19 1:31 AM, Tony Lindgren wrote:
> > OK thanks for testing. Let's drop this for now, sounds like there is
> > some midlemode configuration happening even with no flags set.
>
> You were dropping the ti,sysc-midle property in patch 8, is that still
> ok without this patch?

Yeah let's wait on that one too until we hear back from Jyri.

Regards,

Tony

2019-08-13 11:27:54

by Jyri Sarha

[permalink] [raw]
Subject: Re: [PATCH 2/8] ARM: OMAP2+: Remove unconfigured midlemode for am3 lcdc

On 23/07/2019 22:03, Suman Anna wrote:
> + Jyri
>
> On 7/23/19 6:28 AM, Tony Lindgren wrote:
>> We currently get a warning for lcdc because of a difference
>> with dts provided configuration compared to the legacy platform
>> data. This is because lcdc has SYSC_HAS_MIDLEMODE configured in
>> the platform data without configuring the modes.
>
> Hi Tony,
> While I understand that you are trying to match the DT data with the
> existing legacy data, do you know if there was a reason why this was
> omitted in the first place? Should we be really adding the MSTANDBY_
> flags and fix up the DTS node accordingly? I tried looking through the
> git log, and the initial commit itself didn't add the MSTANDBY_ flags
> but used the SYSC_HAS_MIDLEMODE.
>
> Jyri,
> Do you know the history?
>

Sorry. This all has happened before my time. This is all new to me.


BR,
Jyri



> regards
> Suman
>
>>
>> Let's fix the warning by removing SYSC_HAS_MIDLEMODE. Note that
>> the am335x TRM lists SYSC_HAS_MIDLEMODE, but it is unused.
>
>
>
>>
>> Signed-off-by: Tony Lindgren <[email protected]>
>> ---
>> arch/arm/mach-omap2/omap_hwmod_33xx_data.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
>> --- a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
>> @@ -231,7 +231,7 @@ static struct omap_hwmod am33xx_control_hwmod = {
>> static struct omap_hwmod_class_sysconfig lcdc_sysc = {
>> .rev_offs = 0x0,
>> .sysc_offs = 0x54,
>> - .sysc_flags = (SYSC_HAS_SIDLEMODE | SYSC_HAS_MIDLEMODE),
>> + .sysc_flags = SYSC_HAS_SIDLEMODE,
>> .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
>> .sysc_fields = &omap_hwmod_sysc_type2,
>> };
>>
>


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2019-08-13 11:31:41

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 2/8] ARM: OMAP2+: Remove unconfigured midlemode for am3 lcdc

* Jyri Sarha <[email protected]> [190813 11:05]:
> On 23/07/2019 22:03, Suman Anna wrote:
> > + Jyri
> >
> > On 7/23/19 6:28 AM, Tony Lindgren wrote:
> >> We currently get a warning for lcdc because of a difference
> >> with dts provided configuration compared to the legacy platform
> >> data. This is because lcdc has SYSC_HAS_MIDLEMODE configured in
> >> the platform data without configuring the modes.
> >
> > Hi Tony,
> > While I understand that you are trying to match the DT data with the
> > existing legacy data, do you know if there was a reason why this was
> > omitted in the first place? Should we be really adding the MSTANDBY_
> > flags and fix up the DTS node accordingly? I tried looking through the
> > git log, and the initial commit itself didn't add the MSTANDBY_ flags
> > but used the SYSC_HAS_MIDLEMODE.
> >
> > Jyri,
> > Do you know the history?
> >
>
> Sorry. This all has happened before my time. This is all new to me.

Oh OK. Well if possible, could you please check if the following patch
behaves with v5.2 for you for LCDC? I only have rack access to a
beaglebone with hdmi right now.

TRM "Table 13-34. SYSCONFIG Register Field Descriptions" lists both
standbymode and idlemode that should be just the sidle and midle
registers where midle is currently unconfigured.

Regards,

Tony

8< ---------------------
diff --git a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
--- a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
@@ -255,8 +255,9 @@ static struct omap_hwmod am33xx_gpio0_hwmod = {
static struct omap_hwmod_class_sysconfig lcdc_sysc = {
.rev_offs = 0x0,
.sysc_offs = 0x54,
- .sysc_flags = (SYSC_HAS_SIDLEMODE | SYSC_HAS_MIDLEMODE),
- .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+ .sysc_flags = SYSC_HAS_SIDLEMODE | SYSC_HAS_MIDLEMODE,
+ .idlemodes = SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
+ MSTANDBY_FORCE | MSTANDBY_NO | MSTANDBY_SMART,
.sysc_fields = &omap_hwmod_sysc_type2,
};

2019-09-17 09:43:53

by Tero Kristo

[permalink] [raw]
Subject: Re: [PATCH 5/8] ARM: dts: Drop bogus ahclkr clocks for dra7 mcasp 3 to 8

On 24/07/2019 09:47, Tony Lindgren wrote:
> * Suman Anna <[email protected]> [190723 21:02]:
>> Hi Tony,
>>
>> On 7/23/19 6:28 AM, Tony Lindgren wrote:
>>> The ahclkr clkctrl clock bit 28 only exists for mcasp 1 and 2 on dra7.
>>> Otherwise we get the following warning on beagle-x15:
> ...
>>> @@ -2962,9 +2958,8 @@
>>> <SYSC_IDLE_SMART>;
>>> /* Domains (P, C): l4per_pwrdm, l4per2_clkdm */
>>> clocks = <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 0>,
>>> - <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 24>,
>>> - <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 28>;
>>> - clock-names = "fck", "ahclkx", "ahclkr";
>>> + <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 24>;
>>> + clock-names = "fck", "ahclkx";
>>
>> The equivalent change to MCASP8 is missing.
>
> Thanks for spotting it, probably should be set up the same way as
> MCASP4 too looking at the TRM.
>
> Tero, care to check the dra7 mcasp clocks we have defined?

Sorry, missed this earlier.

>
> $ grep MCASP drivers/clk/ti/clk-7xx.c
> { DRA7_IPU_MCASP1_CLKCTRL, dra7_mcasp1_bit_data, CLKF_SW_SUP, "ipu-clkctrl:0000:22" },
> { DRA7_L4PER2_MCASP2_CLKCTRL, dra7_mcasp2_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:0154:22" },
> { DRA7_L4PER2_MCASP3_CLKCTRL, dra7_mcasp3_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:015c:22" },
> { DRA7_L4PER2_MCASP5_CLKCTRL, dra7_mcasp5_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:016c:22" },
> { DRA7_L4PER2_MCASP8_CLKCTRL, dra7_mcasp8_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:0184:24" },
> { DRA7_L4PER2_MCASP4_CLKCTRL, dra7_mcasp4_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:018c:22" },
> { DRA7_L4PER2_MCASP6_CLKCTRL, dra7_mcasp6_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:01f8:22" },
> { DRA7_L4PER2_MCASP7_CLKCTRL, dra7_mcasp7_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:01fc:22" },
>
> Is bit 24 above correct for MCASP8 or should it too be 22 like
> adjacent MCASP4 in the TRM?

So yeah, mcasp8 is wrong here, should be 22 as rest of them. I did fix
mcasp8 clocks partially when doing the conversion but missed the
parenting here; it was completely broken before.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2019-09-18 17:37:46

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 5/8] ARM: dts: Drop bogus ahclkr clocks for dra7 mcasp 3 to 8

* Tero Kristo <[email protected]> [190917 07:22]:
> On 24/07/2019 09:47, Tony Lindgren wrote:
> > * Suman Anna <[email protected]> [190723 21:02]:
> > > Hi Tony,
> > >
> > > On 7/23/19 6:28 AM, Tony Lindgren wrote:
> > > > The ahclkr clkctrl clock bit 28 only exists for mcasp 1 and 2 on dra7.
> > > > Otherwise we get the following warning on beagle-x15:
> > ...
> > > > @@ -2962,9 +2958,8 @@
> > > > <SYSC_IDLE_SMART>;
> > > > /* Domains (P, C): l4per_pwrdm, l4per2_clkdm */
> > > > clocks = <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 0>,
> > > > - <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 24>,
> > > > - <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 28>;
> > > > - clock-names = "fck", "ahclkx", "ahclkr";
> > > > + <&l4per2_clkctrl DRA7_L4PER2_MCASP7_CLKCTRL 24>;
> > > > + clock-names = "fck", "ahclkx";
> > >
> > > The equivalent change to MCASP8 is missing.
> >
> > Thanks for spotting it, probably should be set up the same way as
> > MCASP4 too looking at the TRM.
> >
> > Tero, care to check the dra7 mcasp clocks we have defined?
>
> Sorry, missed this earlier.
>
> >
> > $ grep MCASP drivers/clk/ti/clk-7xx.c
> > { DRA7_IPU_MCASP1_CLKCTRL, dra7_mcasp1_bit_data, CLKF_SW_SUP, "ipu-clkctrl:0000:22" },
> > { DRA7_L4PER2_MCASP2_CLKCTRL, dra7_mcasp2_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:0154:22" },
> > { DRA7_L4PER2_MCASP3_CLKCTRL, dra7_mcasp3_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:015c:22" },
> > { DRA7_L4PER2_MCASP5_CLKCTRL, dra7_mcasp5_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:016c:22" },
> > { DRA7_L4PER2_MCASP8_CLKCTRL, dra7_mcasp8_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:0184:24" },
> > { DRA7_L4PER2_MCASP4_CLKCTRL, dra7_mcasp4_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:018c:22" },
> > { DRA7_L4PER2_MCASP6_CLKCTRL, dra7_mcasp6_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:01f8:22" },
> > { DRA7_L4PER2_MCASP7_CLKCTRL, dra7_mcasp7_bit_data, CLKF_SW_SUP, "l4per2-clkctrl:01fc:22" },
> >
> > Is bit 24 above correct for MCASP8 or should it too be 22 like
> > adjacent MCASP4 in the TRM?
>
> So yeah, mcasp8 is wrong here, should be 22 as rest of them. I did fix
> mcasp8 clocks partially when doing the conversion but missed the parenting
> here; it was completely broken before.

OK thanks, I'll post a patch to fix that and an updated mcasp dts fix.

Regards,

Tony