Exynos4210: fix power domain for MDMA1 device
Subsystems (3)
cover Exynos4210: fix power domain for MDMA1 device
This patchset fixes mysterious boot hang on Exynos 4210 SoCs, when IOMMU is enabled. There is no direct dependency between IOMMU devices and MDMA1. However enabling IOMMU changes the device probe order, what results in LCD0 power domain being turned off for some time. During that time the registration of MDMA1 device happens, what results in system hangs, because the common bus code tries to read PID/CID registers from turned-off device.
The first patch adds support for enabling power domain during AMBA device registration process, which require access to device's registers to read PID/CID values. The second assigns MDMA1 device on Exynos 4210 to proper power domain.
Best regards
Marek Szyprowski
Samsung R&D Institute Poland
Patch summary:
Marek Szyprowski (2):
drivers: amba: properly handle devices with power domains
ARM: dts: exynos4210: MDMA1 device belongs to LCD0 power domain
arch/arm/boot/dts/exynos4210.dtsi | 4 ++++ drivers/amba/bus.c | 7 +++++++ 2 files changed, 11 insertions(+)
Signature · 1 line
1.9.2
PATCH 1/1 ARM: dts: exynos4210: Add power domain to G2D device
G2D device and it's SYSMMU belongs to LCD0 power domain on Exynos 4210, so add missing power-domains property to G2D device node (G2D's SYSMMU is already bound to LCD0 power domain).
--- arch/arm/boot/dts/exynos4210.dtsi | 1 + 1 file changed, 1 insertion(+)
arch/arm/boot/dts/exynos4210.dtsi +1 -0
@@ -185,6 +185,7 @@ interrupts = <0 89 0>; clocks = <&clock CLK_SCLK_FIMG2D>, <&clock CLK_G2D>; clock-names = "sclk_fimg2d", "fimg2d"; + power-domains = <&pd_lcd0>; iommus = <&sysmmu_g2d>; status = "disabled"; };
Signature · 1 line
1.9.2
PATCH 1/1 ARM: dts: exynos4210: Add power domain to G2D device
Quoted 8 lines · Marek Szyprowski
G2D device and it's SYSMMU belongs to LCD0 power domain on Exynos 4210, so add missing power-domains property to G2D device node (G2D's SYSMMU is already bound to LCD0 power domain).
Signed-off-by Marek Szyprowski <[email protected]>--- arch/arm/boot/dts/exynos4210.dtsi | 1 + 1 file changed, 1 insertion(+)
You sent it with "in-reply-to" cover letter but without any explanation why? Are here any dependencies on the rest of patchset? Any specific order? This is a little bit confusing.
Best regards,
Krzysztof
Quoted 14 lines
arch/arm/boot/dts/exynos4210.dtsi +1
-0@@ -185,6 +185,7 @@ interrupts = <0 89 0>; clocks = <&clock CLK_SCLK_FIMG2D>, <&clock CLK_G2D>; clock-names = "sclk_fimg2d", "fimg2d"; + power-domains = <&pd_lcd0>; iommus = <&sysmmu_g2d>; status = "disabled"; };
PATCH 1/1 ARM: dts: exynos4210: Add power domain to G2D device
In reply to Marek SzyprowskiQuoted 8 lines · Marek Szyprowski
G2D device and it's SYSMMU belongs to LCD0 power domain on Exynos 4210, so add missing power-domains property to G2D device node (G2D's SYSMMU is already bound to LCD0 power domain).
Signed-off-by Marek Szyprowski <[email protected]>--- arch/arm/boot/dts/exynos4210.dtsi | 1 + 1 file changed, 1 insertion(+)
Best regards,
Krzysztof
PATCH 1/2 drivers: amba: properly handle devices with power domains
In reply to Marek SzyprowskiTo read pid/cid registers, the probed device need to be properly turned on. When it is inside a power domain, the bus code should ensure that the given power domain is enabled before trying to access device's registers.
--- drivers/amba/bus.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/amba/bus.c +7 -0
@@ -373,6 +373,12 @@ int amba_device_add(struct amba_device *dev, struct resource *parent) goto err_release; } + ret =dev_pm_domain_attach (&dev->dev, true); +if (ret) { +iounmap (tmp); + goto err_release; + } + ret =amba_get_enable_pclk (dev);if (ret ==0 ) {u32 pid, cid;
@@ -398,6 +404,7 @@ int amba_device_add(struct amba_device *dev, struct resource *parent) }iounmap (tmp); +dev_pm_domain_detach (&dev->dev, true);if (ret) goto err_release;
Signature · 1 line
1.9.2
PATCH 1/2 drivers: amba: properly handle devices with power domains
Quoted 14 lines · Marek Szyprowski
To read pid/cid registers, the probed device need to be properly turned on. When it is inside a power domain, the bus code should ensure that the given power domain is enabled before trying to access device's registers.
Signed-off-by Marek Szyprowski <[email protected]>--- drivers/amba/bus.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/amba/bus.c +0
-0
drivers/amba/bus.c int amba_device_add(struct amba_device *dev, struct resource *parent) · line 381
Quoted 9 lines
drivers/amba/bus.c +6
-0@@ -373,6 +373,12 @@ int amba_device_add(struct amba_device *dev, struct resource *parent) goto err_release; } + ret =dev_pm_domain_attach (&dev->dev, true); +if (ret) { +iounmap (tmp); + goto err_release; + } +
NAK. If dev_pm_domain_attach() returns an error, even -EPROBE_DEFER, the result will be a missing device that has no way to be recovered. This is too fragile.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
PATCH 1/2 drivers: amba: properly handle devices with power domains
Hello,
Quoted 18 lines · Russell King - ARM Linux
Quoted 17 lines · Marek Szyprowski
To read pid/cid registers, the probed device need to be properly turned on. When it is inside a power domain, the bus code should ensure that the given power domain is enabled before trying to access device's registers.
Signed-off-by Marek Szyprowski <[email protected]>--- drivers/amba/bus.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/amba/bus.c +0
-0@@ -373,6 +373,12 @@ int amba_device_add(struct amba_device *dev, struct resource *parent) goto err_release; }
Quoted 9 lines
drivers/amba/bus.c int amba_device_add(struct amba_device *dev, struct resource *parent) · line 381
Quoted 6 lines
drivers/amba/bus.c +6
-0+ ret =dev_pm_domain_attach (&dev->dev, true); +if (ret) { +iounmap (tmp); + goto err_release; + } +NAK. If
dev_pm_domain_attach()returns an error, even -EPROBE_DEFER, the result will be a missing device that has no way to be recovered. This is too fragile.
Then how the problem of accessing registers in turned-off device should be solved?
Is ignoring dev_pm_domain_attach() return value a solution for you?
Best regards
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
PATCH 1/2 drivers: amba: properly handle devices with power domains
Quoted 34 lines · Marek Szyprowski
Hello,
Quoted 31 lines · Russell King - ARM Linux
Quoted 16 lines · Marek Szyprowski
To read pid/cid registers, the probed device need to be properly turned on.
When it is inside a power domain, the bus code should ensure that the given power domain is enabled before trying to access device's registers.Signed-off-by Marek Szyprowski <[email protected]>--- drivers/amba/bus.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/amba/bus.c +0
-0drivers/amba/bus.c int amba_device_add(struct amba_device *dev, struct · line 381
Quoted 9 lines
drivers/amba/bus.c +5
-0@@ -373,6 +373,12 @@ int amba_device_add(struct amba_device *dev, struct resource *parent) goto err_release; } + ret =dev_pm_domain_attach (&dev->dev, true); +if (ret) { +iounmap (tmp); + goto err_release; + } +NAK. If
dev_pm_domain_attach()returns an error, even -EPROBE_DEFER, the result will be a missing device that has no way to be recovered. This is too fragile.
I agree with Russell here, this isn't going to work.
Quoted 4 lines
Then how the problem of accessing registers in turned-off device should be solved?
The long term solution has been discussed [1], please have a look and feel free to hack on it if you like. If not, I will sooner or later find time to pick up the work I have started, but can't give you any promises when ready.
Quoted 2 lines
Is ignoring
dev_pm_domain_attach()return value a solution for you?
That's probably better than nothing, but I wonder if it in practice will have any effect? It requires the PM domain to be initialized (and having a DT provider for it) before you register the AMBA device, is that so in your case?
Kind regards
Uffe
PATCH 1/2 drivers: amba: properly handle devices with power domains
Hello,
Quoted 33 lines · Ulf Hansson
Quoted 21 lines · Marek Szyprowski
Quoted 20 lines · Russell King - ARM Linux
Quoted 19 lines · Marek Szyprowski
To read pid/cid registers, the probed device need to be properly turned on.
When it is inside a power domain, the bus code should ensure that the given power domain is enabled before trying to access device's registers.Signed-off-by Marek Szyprowski <[email protected]>--- drivers/amba/bus.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/amba/bus.c +0
-0@@ -373,6 +373,12 @@ int amba_device_add(struct amba_device *dev, struct resource *parent) goto err_release; } + ret =dev_pm_domain_attach (&dev->dev, true);Quoted 8 lines
Quoted 8 lines
drivers/amba/bus.c int amba_device_add(struct amba_device *dev, struct resource *parent) · line 381
Quoted 5 lines
drivers/amba/bus.c +5
-0+if (ret) { +iounmap (tmp); + goto err_release; + } +NAK. If
dev_pm_domain_attach()returns an error, even -EPROBE_DEFER, the result will be a missing device that has no way to be recovered. This is too fragile.I agree with Russell here, this isn't going to work.
Quoted 1 line
Quoted 6 lines
Quoted 2 lines
Then how the problem of accessing registers in turned-off device should be solved?
The long term solution has been discussed [1], please have a look and feel free to hack on it if you like. If not, I will sooner or later find time to pick up the work I have started, but can't give you any promises when ready.
Frankly, I still don't get how do you want to solve the problem of incomplete/not-yet-probed pm domain during the amba_device_add() and reading pid/cid identifiers, so I will leave it for you.
Quoted 5 lines
Quoted 1 line
Is ignoring
dev_pm_domain_attach()return value a solution for you?That's probably better than nothing, but I wonder if it in practice will have any effect? It requires the PM domain to be initialized (and having a DT provider for it) before you register the AMBA device, is that so in your case?
Yes, this will work for me, as power domain driver is registered very early on Exynos platform (this was already needed to let it working with IOMMU driver).
Best regards
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
PATCH 1/2 drivers: amba: properly handle devices with power domains
In reply to Ulf HanssonQuoted 5 lines · Ulf Hansson
Quoted 1 line · Marek Szyprowski
Is ignoring
dev_pm_domain_attach()return value a solution for you?That's probably better than nothing, but I wonder if it in practice will have any effect?
If the PM domain is down, then trying to tread the ID is likely to oops the kernel.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
PATCH 1/2 drivers: amba: properly handle devices with power domains
Hello,
Quoted 7 lines · Russell King - ARM Linux
Quoted 4 lines · Ulf Hansson
Quoted 1 line · Marek Szyprowski
Is ignoring
dev_pm_domain_attach()return value a solution for you?That's probably better than nothing, but I wonder if it in practice will have any effect?
If the PM domain is down, then trying to tread the ID is likely to oops the kernel.
In my case kernel simply hangs (no single message, even when earlyprintk is enabled)
instead of oopsing, that's why I've submitted this patch.
Best regards
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
PATCH 1/2 drivers: amba: properly handle devices with power domains
Quoted 14 lines · Marek Szyprowski
Hello,
Quoted 11 lines · Russell King - ARM Linux
Quoted 8 lines · Ulf Hansson
Quoted 2 lines · Marek Szyprowski
Is ignoring
dev_pm_domain_attach()return value a solution for you?That's probably better than nothing, but I wonder if it in practice will have any effect?
Quoted 8 lines
Quoted 2 lines
If the PM domain is down, then trying to tread the ID is likely to oops the kernel.
In my case kernel simply hangs (no single message, even when earlyprintk is enabled)
instead of oopsing, that's why I've submitted this patch.
Okay.
I suggest we go ahead and try that approach (ignoring the return value). It's "quick fix", but the easiest way forward to solve the problem.
My only concern is if such change impacts the boot time. We should do some tests to see if the change is negligible, if not we should probably think of something else (like keeping the PM domain powered until late_init).
Russell, what do you think?
Kind regards
Uffe
PATCH 1/2 drivers: amba: properly handle devices with power domains
Quoted 30 lines · Ulf Hansson
Quoted 14 lines · Marek Szyprowski
Hello,
Quoted 11 lines · Russell King - ARM Linux
Quoted 8 lines · Ulf Hansson
Quoted 2 lines · Marek Szyprowski
Is ignoring
dev_pm_domain_attach()return value a solution for you?That's probably better than nothing, but I wonder if it in practice will have any effect?
Quoted 8 lines
Quoted 2 lines
If the PM domain is down, then trying to tread the ID is likely to oops the kernel.
In my case kernel simply hangs (no single message, even when earlyprintk is enabled)
instead of oopsing, that's why I've submitted this patch.Okay.
I suggest we go ahead and try that approach (ignoring the return value). It's "quick fix", but the easiest way forward to solve the problem.
Quoted 6 lines
My only concern is if such change impacts the boot time. We should do some tests to see if the change is negligible, if not we should probably think of something else (like keeping the PM domain powered until
late_init).Russell, what do you think?
I thought someone had a patch to read the ID during the match callback?
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
PATCH 1/2 drivers: amba: properly handle devices with power domains
Hello,
Quoted 24 lines · Russell King - ARM Linux
Quoted 23 lines · Ulf Hansson
Quoted 7 lines · Marek Szyprowski
Hello,
Quoted 4 lines · Russell King - ARM Linux
Quoted 3 lines · Ulf Hansson
Quoted 1 line · Marek Szyprowski
Is ignoring
dev_pm_domain_attach()return value a solution for you?Quoted 9 lines
Quoted 4 lines
Quoted 2 lines
That's probably better than nothing, but I wonder if it in practice will have any effect?
If the PM domain is down, then trying to tread the ID is likely to oops the kernel.
In my case kernel simply hangs (no single message, even when earlyprintk is enabled)
instead of oopsing, that's why I've submitted this patch.Okay.
I suggest we go ahead and try that approach (ignoring the return value). It's "quick fix", but the easiest way forward to solve the problem.
Quoted 7 lines
Quoted 6 lines
My only concern is if such change impacts the boot time. We should do some tests to see if the change is negligible, if not we should probably think of something else (like keeping the PM domain powered until
late_init).Russell, what do you think?
I thought someone had a patch to read the ID during the match callback?
Okay, I've reused that patch and it seems to solve mthisproblem. I will send v2
in a few minutes.
Best regards
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
PATCH 1/2 drivers: amba: properly handle devices with power domains
In reply to Ulf HanssonHello,
Quoted 21 lines · Ulf Hansson
Quoted 5 lines · Marek Szyprowski
Quoted 4 lines · Russell King - ARM Linux
Quoted 3 lines · Ulf Hansson
Quoted 1 line · Marek Szyprowski
Is ignoring
dev_pm_domain_attach()return value a solution for you?Quoted 9 lines
Quoted 4 lines
Quoted 2 lines
That's probably better than nothing, but I wonder if it in practice will have any effect?
If the PM domain is down, then trying to tread the ID is likely to oops the kernel.
In my case kernel simply hangs (no single message, even when earlyprintk is enabled)
instead of oopsing, that's why I've submitted this patch.Okay.
I suggest we go ahead and try that approach (ignoring the return value). It's "quick fix", but the easiest way forward to solve the problem.
Quoted 6 lines
My only concern is if such change impacts the boot time. We should do some tests to see if the change is negligible, if not we should probably think of something else (like keeping the PM domain powered until
late_init).Russell, what do you think?
Keeping PM domains powered until late_init would be a good idea regardless of
the problem discussed here, because I often see that pm domains are turned on
and off a dozen of times during typical boot of Exynos based systems.
Best regards
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
PATCH 1/2 drivers: amba: properly handle devices with power domains
In reply to Ulf HanssonQuoted 30 lines · Ulf Hansson
Quoted 14 lines · Marek Szyprowski
Hello,
Quoted 11 lines · Russell King - ARM Linux
Quoted 8 lines · Ulf Hansson
Quoted 2 lines · Marek Szyprowski
Is ignoring
dev_pm_domain_attach()return value a solution for you?That's probably better than nothing, but I wonder if it in practice will have any effect?
Quoted 8 lines
Quoted 2 lines
If the PM domain is down, then trying to tread the ID is likely to oops the kernel.
In my case kernel simply hangs (no single message, even when earlyprintk is enabled)
instead of oopsing, that's why I've submitted this patch.Okay.
I suggest we go ahead and try that approach (ignoring the return value). It's "quick fix", but the easiest way forward to solve the problem.
Quoted 6 lines
My only concern is if such change impacts the boot time. We should do some tests to see if the change is negligible, if not we should probably think of something else (like keeping the PM domain powered until
late_init).Russell, what do you think?
That's the approach I took for coresight on Juno - simply initialise the power domain very early on and keep it on for things like amba probing to be successful. When the boot process is done unused genPDs are switched off automatically. That way we don't need to add extra code to amba.
Quoted 8 lines
Kind regards
Uffe_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/2 drivers: amba: properly handle devices with power domains
Hello,
Quoted 26 lines · Mathieu Poirier
Quoted 25 lines · Ulf Hansson
Quoted 4 lines · Marek Szyprowski
Quoted 3 lines · Russell King - ARM Linux
Quoted 2 lines · Ulf Hansson
On 25 November 2015 at 14:34, Marek Szyprowski <[email protected]> wrote:
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Is ignoring
dev_pm_domain_attach()return value a solution for you?That's probably better than nothing, but I wonder if it in practice will have any effect?
If the PM domain is down, then trying to tread the ID is likely to oops the kernel.
In my case kernel simply hangs (no single message, even when earlyprintk is enabled)
instead of oopsing, that's why I've submitted this patch.Okay.
I suggest we go ahead and try that approach (ignoring the return value). It's "quick fix", but the easiest way forward to solve the problem.
My only concern is if such change impacts the boot time. We should do some tests to see if the change is negligible, if not we should probably think of something else (like keeping the PM domain powered until
late_init).
Quoted 6 lines
Quoted 1 line
Russell, what do you think?
That's the approach I took for coresight on Juno - simply initialise the power domain very early on and keep it on for things like amba probing to be successful. When the boot process is done unused genPDs are switched off automatically. That way we don't need to add extra code to amba.
Frankly I don't get the point that 'you don't need to add extra code to amba'.
Turning power domains on very early on boot is rationale, but then gen_pd core
often turns them off and on, depending on the sequence of the registered devices
and their runtime pm status. Amba bus should support proper registration of devices in both cases, when power domain for the registered device is turn on
and off.
In my opinion it should be a gen_pd core responsibility to keep power domain turned on until late init. Right now it turns it off when all devices, which have been registered so far, have been turned off. Usually a moment later another device get registered to the given domain and this requires the domain
to get enabled again. This problem can be observed only when there are more than on device in a power domain.
Best regards
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
PATCH 2/2 ARM: dts: exynos4210: MDMA1 device belongs to LCD0 power domain
In reply to Marek SzyprowskiOn Exynos 4210 MDMA1 device belongs to LCD0 power domain, so add proper power-domains property. On Exynos 4x12, it belongs to TOP power domain, which is always enabled, thus require no assignment in exynos4x12.dtsi.
--- arch/arm/boot/dts/exynos4210.dtsi | 4 ++++ 1 file changed, 4 insertions(+)
arch/arm/boot/dts/exynos4210.dtsi +4 -0
@@ -271,6 +271,10 @@ <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>; }; +&mdma1 { + power-domains = <&pd_lcd0>; +}; + &pmu_system_controller { clock-names = "clkout0", "clkout1", "clkout2", "clkout3", "clkout4", "clkout8", "clkout9";
Signature · 1 line
1.9.2