soc: samsung: Fix Exynos4412 ISP CMU reset in AFTR mode

Message ID 20190328135153.7641-1-m.szyprowski@samsung.com
State New
Headers show
Series
  • soc: samsung: Fix Exynos4412 ISP CMU reset in AFTR mode
Related show

Commit Message

Marek Szyprowski March 28, 2019, 1:51 p.m.
When CPUidle enters C1 (AFTR) mode on Exynos4412, the operation of ISP
power domain fails. This can be observed by the following messages:

Power domain ISP disable failed

in the kernel log after using any of the ISP related device. This is
caused by enforcing low power state on the ISP CMU, which is a part of
the ISP power domain by the PMU driver during entering AFTR state.
This however fails, because the ISP power domain may be already turned
off by the Exynos power domain driver. To fix this disable forcing
low-power mode of the ISP domain from the PMU driver.

Fixes: bfce552d0b10 ("drivers: soc: Add support for Exynos PMU driver")
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

---
 drivers/soc/samsung/exynos4-pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.17.1

Comments

Krzysztof Kozlowski March 30, 2019, 6:52 a.m. | #1
On Thu, Mar 28, 2019 at 02:51:53PM +0100, Marek Szyprowski wrote:
> When CPUidle enters C1 (AFTR) mode on Exynos4412, the operation of ISP

> power domain fails.


Hi Marek,

I have trouble understanding the reasoning.

The sentence means that operation fails when you enter AFTR. During the
process of entering AFTR. But maybe you wanted to say, that once SoC
entered AFTR and woke up, disabling power domain will fail?

When exactly this log appears?

> This can be observed by the following messages:

> 

> Power domain ISP disable failed

> 

> in the kernel log after using any of the ISP related device. This is

> caused by enforcing low power state on the ISP CMU, which is a part of

> the ISP power domain by the PMU driver during entering AFTR state.

> This however fails, because the ISP power domain may be already turned

> off by the Exynos power domain driver.


So entering AFTR fails because SP is already off? That's really odd...

> To fix this disable forcing

> low-power mode of the ISP domain from the PMU driver.


Value of 0x1 is for power-on mode so effectively you are rather enforcing
low-power mode (like in LPA or SLEEP), not disabling forcing.
Additionally, this is only the CMU_RESET not the CLKSTOP register which
is left untouched.

I really miss the point what you want to achieve here.

Best regards,
Krzysztof

> 

> Fixes: bfce552d0b10 ("drivers: soc: Add support for Exynos PMU driver")

> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---

>  drivers/soc/samsung/exynos4-pmu.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/drivers/soc/samsung/exynos4-pmu.c b/drivers/soc/samsung/exynos4-pmu.c

> index a7cdbf1aac0c..62dcc68ada59 100644

> --- a/drivers/soc/samsung/exynos4-pmu.c

> +++ b/drivers/soc/samsung/exynos4-pmu.c

> @@ -131,7 +131,7 @@ static const struct exynos_pmu_conf exynos4412_pmu_config[] = {

>  	{ S5P_CMU_RESET_MFC_LOWPWR,		{ 0x1, 0x0, 0x0 } },

>  	{ S5P_CMU_RESET_G3D_LOWPWR,		{ 0x1, 0x0, 0x0 } },

>  	{ S5P_CMU_RESET_LCD0_LOWPWR,		{ 0x1, 0x0, 0x0 } },

> -	{ S5P_CMU_RESET_ISP_LOWPWR,		{ 0x1, 0x0, 0x0 } },

> +	{ S5P_CMU_RESET_ISP_LOWPWR,		{ 0x0, 0x0, 0x0 } },

>  	{ S5P_CMU_RESET_MAUDIO_LOWPWR,		{ 0x1, 0x1, 0x0 } },

>  	{ S5P_CMU_RESET_GPS_LOWPWR,		{ 0x1, 0x0, 0x0 } },

>  	{ S5P_TOP_BUS_LOWPWR,			{ 0x3, 0x0, 0x0 } },

> -- 

> 2.17.1

>

Patch

diff --git a/drivers/soc/samsung/exynos4-pmu.c b/drivers/soc/samsung/exynos4-pmu.c
index a7cdbf1aac0c..62dcc68ada59 100644
--- a/drivers/soc/samsung/exynos4-pmu.c
+++ b/drivers/soc/samsung/exynos4-pmu.c
@@ -131,7 +131,7 @@  static const struct exynos_pmu_conf exynos4412_pmu_config[] = {
 	{ S5P_CMU_RESET_MFC_LOWPWR,		{ 0x1, 0x0, 0x0 } },
 	{ S5P_CMU_RESET_G3D_LOWPWR,		{ 0x1, 0x0, 0x0 } },
 	{ S5P_CMU_RESET_LCD0_LOWPWR,		{ 0x1, 0x0, 0x0 } },
-	{ S5P_CMU_RESET_ISP_LOWPWR,		{ 0x1, 0x0, 0x0 } },
+	{ S5P_CMU_RESET_ISP_LOWPWR,		{ 0x0, 0x0, 0x0 } },
 	{ S5P_CMU_RESET_MAUDIO_LOWPWR,		{ 0x1, 0x1, 0x0 } },
 	{ S5P_CMU_RESET_GPS_LOWPWR,		{ 0x1, 0x0, 0x0 } },
 	{ S5P_TOP_BUS_LOWPWR,			{ 0x3, 0x0, 0x0 } },