All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Exynos4210: fix power domain for MDMA1 device
@ 2015-11-25 12:55 ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-25 12:55 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel
  Cc: Marek Szyprowski, Russell King - ARM Linux, Kukjin Kim,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz

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(+)

-- 
1.9.2

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 0/2] Exynos4210: fix power domain for MDMA1 device
@ 2015-11-25 12:55 ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-25 12:55 UTC (permalink / raw)
  To: linux-arm-kernel

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(+)

-- 
1.9.2

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH] ARM: dts: exynos4210: Add power domain to G2D device
  2015-11-25 12:55 ` Marek Szyprowski
@ 2015-11-25 12:55   ` Marek Szyprowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-25 12:55 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel
  Cc: Marek Szyprowski, Russell King - ARM Linux, Kukjin Kim,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz

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 <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos4210.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index aac0f17..d254200 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -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";
 	};
-- 
1.9.2

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH] ARM: dts: exynos4210: Add power domain to G2D device
@ 2015-11-25 12:55   ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-25 12:55 UTC (permalink / raw)
  To: linux-arm-kernel

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 <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos4210.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index aac0f17..d254200 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -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";
 	};
-- 
1.9.2

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 2/2] ARM: dts: exynos4210: MDMA1 device belongs to LCD0 power domain
  2015-11-25 12:55 ` Marek Szyprowski
@ 2015-11-25 12:55   ` Marek Szyprowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-25 12:55 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel
  Cc: Marek Szyprowski, Russell King - ARM Linux, Kukjin Kim,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz

On 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.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos4210.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index b7474cf2..aac0f17 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -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";
-- 
1.9.2

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 2/2] ARM: dts: exynos4210: MDMA1 device belongs to LCD0 power domain
@ 2015-11-25 12:55   ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-25 12:55 UTC (permalink / raw)
  To: linux-arm-kernel

On 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.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/boot/dts/exynos4210.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index b7474cf2..aac0f17 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -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";
-- 
1.9.2

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 1/2] drivers: amba: properly handle devices with power domains
  2015-11-25 12:55 ` Marek Szyprowski
@ 2015-11-25 12:58   ` Marek Szyprowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-25 12:58 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel
  Cc: Marek Szyprowski, Russell King - ARM Linux, Kukjin Kim,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz

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 <m.szyprowski@samsung.com>
---
 drivers/amba/bus.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index f009936..25715cb 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -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;
-- 
1.9.2

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 1/2] drivers: amba: properly handle devices with power domains
@ 2015-11-25 12:58   ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-25 12:58 UTC (permalink / raw)
  To: linux-arm-kernel

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 <m.szyprowski@samsung.com>
---
 drivers/amba/bus.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index f009936..25715cb 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -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;
-- 
1.9.2

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/2] drivers: amba: properly handle devices with power domains
  2015-11-25 12:58   ` Marek Szyprowski
@ 2015-11-25 13:24     ` Russell King - ARM Linux
  -1 siblings, 0 replies; 36+ messages in thread
From: Russell King - ARM Linux @ 2015-11-25 13:24 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: linux-samsung-soc, linux-arm-kernel, Kukjin Kim,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz

On Wed, Nov 25, 2015 at 01:58:09PM +0100, Marek Szyprowski wrote:
> 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 <m.szyprowski@samsung.com>
> ---
>  drivers/amba/bus.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
> index f009936..25715cb 100644
> --- a/drivers/amba/bus.c
> +++ b/drivers/amba/bus.c
> @@ -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.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/2] drivers: amba: properly handle devices with power domains
@ 2015-11-25 13:24     ` Russell King - ARM Linux
  0 siblings, 0 replies; 36+ messages in thread
From: Russell King - ARM Linux @ 2015-11-25 13:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Nov 25, 2015 at 01:58:09PM +0100, Marek Szyprowski wrote:
> 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 <m.szyprowski@samsung.com>
> ---
>  drivers/amba/bus.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
> index f009936..25715cb 100644
> --- a/drivers/amba/bus.c
> +++ b/drivers/amba/bus.c
> @@ -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.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/2] drivers: amba: properly handle devices with power domains
  2015-11-25 13:24     ` Russell King - ARM Linux
@ 2015-11-25 13:34       ` Marek Szyprowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-25 13:34 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: linux-samsung-soc, linux-arm-kernel, Kukjin Kim,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz

Hello,

On 2015-11-25 14:24, Russell King - ARM Linux wrote:
> On Wed, Nov 25, 2015 at 01:58:09PM +0100, Marek Szyprowski wrote:
>> 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 <m.szyprowski@samsung.com>
>> ---
>>   drivers/amba/bus.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
>> index f009936..25715cb 100644
>> --- a/drivers/amba/bus.c
>> +++ b/drivers/amba/bus.c
>> @@ -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.

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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/2] drivers: amba: properly handle devices with power domains
@ 2015-11-25 13:34       ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-25 13:34 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On 2015-11-25 14:24, Russell King - ARM Linux wrote:
> On Wed, Nov 25, 2015 at 01:58:09PM +0100, Marek Szyprowski wrote:
>> 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 <m.szyprowski@samsung.com>
>> ---
>>   drivers/amba/bus.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
>> index f009936..25715cb 100644
>> --- a/drivers/amba/bus.c
>> +++ b/drivers/amba/bus.c
>> @@ -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.

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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/2] drivers: amba: properly handle devices with power domains
  2015-11-25 13:34       ` Marek Szyprowski
@ 2015-11-25 13:56         ` Ulf Hansson
  -1 siblings, 0 replies; 36+ messages in thread
From: Ulf Hansson @ 2015-11-25 13:56 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Russell King - ARM Linux, Krzysztof Kozlowski, linux-samsung-soc,
	Kukjin Kim, linux-arm-kernel, Bartlomiej Zolnierkiewicz

On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> Hello,
>
> On 2015-11-25 14:24, Russell King - ARM Linux wrote:
>>
>> On Wed, Nov 25, 2015 at 01:58:09PM +0100, Marek Szyprowski wrote:
>>>
>>> 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 <m.szyprowski@samsung.com>
>>> ---
>>>   drivers/amba/bus.c | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
>>> index f009936..25715cb 100644
>>> --- a/drivers/amba/bus.c
>>> +++ b/drivers/amba/bus.c
>>> @@ -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.

>
>
> 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.

>
> 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

[1]
http://comments.gmane.org/gmane.linux.kernel.mmc/32587

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/2] drivers: amba: properly handle devices with power domains
@ 2015-11-25 13:56         ` Ulf Hansson
  0 siblings, 0 replies; 36+ messages in thread
From: Ulf Hansson @ 2015-11-25 13:56 UTC (permalink / raw)
  To: linux-arm-kernel

On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> Hello,
>
> On 2015-11-25 14:24, Russell King - ARM Linux wrote:
>>
>> On Wed, Nov 25, 2015 at 01:58:09PM +0100, Marek Szyprowski wrote:
>>>
>>> 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 <m.szyprowski@samsung.com>
>>> ---
>>>   drivers/amba/bus.c | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
>>> index f009936..25715cb 100644
>>> --- a/drivers/amba/bus.c
>>> +++ b/drivers/amba/bus.c
>>> @@ -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.

>
>
> 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.

>
> 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

[1]
http://comments.gmane.org/gmane.linux.kernel.mmc/32587

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/2] drivers: amba: properly handle devices with power domains
  2015-11-25 13:56         ` Ulf Hansson
@ 2015-11-25 14:43           ` Marek Szyprowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-25 14:43 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Russell King - ARM Linux, Krzysztof Kozlowski, linux-samsung-soc,
	Kukjin Kim, linux-arm-kernel, Bartlomiej Zolnierkiewicz

Hello,

On 2015-11-25 14:56, Ulf Hansson wrote:
> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> On 2015-11-25 14:24, Russell King - ARM Linux wrote:
>>> On Wed, Nov 25, 2015 at 01:58:09PM +0100, Marek Szyprowski wrote:
>>>> 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 <m.szyprowski@samsung.com>
>>>> ---
>>>>    drivers/amba/bus.c | 7 +++++++
>>>>    1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
>>>> index f009936..25715cb 100644
>>>> --- a/drivers/amba/bus.c
>>>> +++ b/drivers/amba/bus.c
>>>> @@ -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.
>
>>
>> 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.

>> 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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/2] drivers: amba: properly handle devices with power domains
@ 2015-11-25 14:43           ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-25 14:43 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On 2015-11-25 14:56, Ulf Hansson wrote:
> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> On 2015-11-25 14:24, Russell King - ARM Linux wrote:
>>> On Wed, Nov 25, 2015 at 01:58:09PM +0100, Marek Szyprowski wrote:
>>>> 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 <m.szyprowski@samsung.com>
>>>> ---
>>>>    drivers/amba/bus.c | 7 +++++++
>>>>    1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
>>>> index f009936..25715cb 100644
>>>> --- a/drivers/amba/bus.c
>>>> +++ b/drivers/amba/bus.c
>>>> @@ -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.
>
>>
>> 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.

>> 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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/2] drivers: amba: properly handle devices with power domains
  2015-11-25 13:56         ` Ulf Hansson
@ 2015-11-25 18:09           ` Russell King - ARM Linux
  -1 siblings, 0 replies; 36+ messages in thread
From: Russell King - ARM Linux @ 2015-11-25 18:09 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Marek Szyprowski, Krzysztof Kozlowski, linux-samsung-soc,
	Kukjin Kim, linux-arm-kernel, Bartlomiej Zolnierkiewicz

On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > 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.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/2] drivers: amba: properly handle devices with power domains
@ 2015-11-25 18:09           ` Russell King - ARM Linux
  0 siblings, 0 replies; 36+ messages in thread
From: Russell King - ARM Linux @ 2015-11-25 18:09 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > 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.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] ARM: dts: exynos4210: Add power domain to G2D device
  2015-11-25 12:55   ` Marek Szyprowski
@ 2015-11-25 23:55     ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Krzysztof Kozlowski @ 2015-11-25 23:55 UTC (permalink / raw)
  To: Marek Szyprowski, linux-samsung-soc, linux-arm-kernel
  Cc: Russell King - ARM Linux, Kukjin Kim, Bartlomiej Zolnierkiewicz

On 25.11.2015 21:55, Marek Szyprowski wrote:
> 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 <m.szyprowski@samsung.com>
> ---
>  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

> 
> diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
> index aac0f17..d254200 100644
> --- a/arch/arm/boot/dts/exynos4210.dtsi
> +++ b/arch/arm/boot/dts/exynos4210.dtsi
> @@ -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";
>  	};
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH] ARM: dts: exynos4210: Add power domain to G2D device
@ 2015-11-25 23:55     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 36+ messages in thread
From: Krzysztof Kozlowski @ 2015-11-25 23:55 UTC (permalink / raw)
  To: linux-arm-kernel

On 25.11.2015 21:55, Marek Szyprowski wrote:
> 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 <m.szyprowski@samsung.com>
> ---
>  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

> 
> diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
> index aac0f17..d254200 100644
> --- a/arch/arm/boot/dts/exynos4210.dtsi
> +++ b/arch/arm/boot/dts/exynos4210.dtsi
> @@ -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";
>  	};
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] ARM: dts: exynos4210: Add power domain to G2D device
  2015-11-25 12:55   ` Marek Szyprowski
@ 2015-11-26  7:35     ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Krzysztof Kozlowski @ 2015-11-26  7:35 UTC (permalink / raw)
  To: Marek Szyprowski, linux-samsung-soc, linux-arm-kernel
  Cc: Russell King - ARM Linux, Kukjin Kim, Bartlomiej Zolnierkiewicz

On 25.11.2015 21:55, Marek Szyprowski wrote:
> 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 <m.szyprowski@samsung.com>
> ---
>  arch/arm/boot/dts/exynos4210.dtsi | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH] ARM: dts: exynos4210: Add power domain to G2D device
@ 2015-11-26  7:35     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 36+ messages in thread
From: Krzysztof Kozlowski @ 2015-11-26  7:35 UTC (permalink / raw)
  To: linux-arm-kernel

On 25.11.2015 21:55, Marek Szyprowski wrote:
> 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 <m.szyprowski@samsung.com>
> ---
>  arch/arm/boot/dts/exynos4210.dtsi | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/2] drivers: amba: properly handle devices with power domains
  2015-11-25 18:09           ` Russell King - ARM Linux
@ 2015-11-26  8:39             ` Marek Szyprowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-26  8:39 UTC (permalink / raw)
  To: Russell King - ARM Linux, Ulf Hansson
  Cc: Krzysztof Kozlowski, linux-samsung-soc, Kukjin Kim,
	linux-arm-kernel, Bartlomiej Zolnierkiewicz

Hello,

On 2015-11-25 19:09, Russell King - ARM Linux wrote:
> On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
>> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>> 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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/2] drivers: amba: properly handle devices with power domains
@ 2015-11-26  8:39             ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-26  8:39 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On 2015-11-25 19:09, Russell King - ARM Linux wrote:
> On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
>> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>> 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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/2] drivers: amba: properly handle devices with power domains
  2015-11-26  8:39             ` Marek Szyprowski
@ 2015-11-26 10:24               ` Ulf Hansson
  -1 siblings, 0 replies; 36+ messages in thread
From: Ulf Hansson @ 2015-11-26 10:24 UTC (permalink / raw)
  To: Marek Szyprowski, Russell King - ARM Linux
  Cc: Krzysztof Kozlowski, linux-samsung-soc, Kukjin Kim,
	linux-arm-kernel, Bartlomiej Zolnierkiewicz

On 26 November 2015 at 09:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> Hello,
>
> On 2015-11-25 19:09, Russell King - ARM Linux wrote:
>>
>> On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
>>>
>>> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com>
>>> wrote:
>>>>
>>>> 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).

Russell, what do you think?

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/2] drivers: amba: properly handle devices with power domains
@ 2015-11-26 10:24               ` Ulf Hansson
  0 siblings, 0 replies; 36+ messages in thread
From: Ulf Hansson @ 2015-11-26 10:24 UTC (permalink / raw)
  To: linux-arm-kernel

On 26 November 2015 at 09:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> Hello,
>
> On 2015-11-25 19:09, Russell King - ARM Linux wrote:
>>
>> On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
>>>
>>> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com>
>>> wrote:
>>>>
>>>> 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).

Russell, what do you think?

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/2] drivers: amba: properly handle devices with power domains
  2015-11-26 10:24               ` Ulf Hansson
@ 2015-11-26 10:59                 ` Russell King - ARM Linux
  -1 siblings, 0 replies; 36+ messages in thread
From: Russell King - ARM Linux @ 2015-11-26 10:59 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Marek Szyprowski, Krzysztof Kozlowski, linux-samsung-soc,
	Kukjin Kim, linux-arm-kernel, Bartlomiej Zolnierkiewicz

On Thu, Nov 26, 2015 at 11:24:50AM +0100, Ulf Hansson wrote:
> On 26 November 2015 at 09:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > Hello,
> >
> > On 2015-11-25 19:09, Russell King - ARM Linux wrote:
> >>
> >> On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
> >>>
> >>> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com>
> >>> wrote:
> >>>>
> >>>> 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).
> 
> Russell, what do you think?

I thought someone had a patch to read the ID during the match callback?

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/2] drivers: amba: properly handle devices with power domains
@ 2015-11-26 10:59                 ` Russell King - ARM Linux
  0 siblings, 0 replies; 36+ messages in thread
From: Russell King - ARM Linux @ 2015-11-26 10:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Nov 26, 2015 at 11:24:50AM +0100, Ulf Hansson wrote:
> On 26 November 2015 at 09:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> > Hello,
> >
> > On 2015-11-25 19:09, Russell King - ARM Linux wrote:
> >>
> >> On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
> >>>
> >>> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com>
> >>> wrote:
> >>>>
> >>>> 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).
> 
> Russell, what do you think?

I thought someone had a patch to read the ID during the match callback?

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/2] drivers: amba: properly handle devices with power domains
  2015-11-26 10:59                 ` Russell King - ARM Linux
@ 2015-11-26 12:33                   ` Marek Szyprowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-26 12:33 UTC (permalink / raw)
  To: Russell King - ARM Linux, Ulf Hansson
  Cc: Krzysztof Kozlowski, linux-samsung-soc, Kukjin Kim,
	linux-arm-kernel, Bartlomiej Zolnierkiewicz

Hello,

On 2015-11-26 11:59, Russell King - ARM Linux wrote:
> On Thu, Nov 26, 2015 at 11:24:50AM +0100, Ulf Hansson wrote:
>> On 26 November 2015 at 09:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>> Hello,
>>>
>>> On 2015-11-25 19:09, Russell King - ARM Linux wrote:
>>>> On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
>>>>> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com>
>>>>> wrote:
>>>>>> 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).
>>
>> 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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/2] drivers: amba: properly handle devices with power domains
@ 2015-11-26 12:33                   ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-26 12:33 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On 2015-11-26 11:59, Russell King - ARM Linux wrote:
> On Thu, Nov 26, 2015 at 11:24:50AM +0100, Ulf Hansson wrote:
>> On 26 November 2015 at 09:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>> Hello,
>>>
>>> On 2015-11-25 19:09, Russell King - ARM Linux wrote:
>>>> On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
>>>>> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com>
>>>>> wrote:
>>>>>> 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).
>>
>> 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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/2] drivers: amba: properly handle devices with power domains
  2015-11-26 10:24               ` Ulf Hansson
@ 2015-11-26 12:55                 ` Marek Szyprowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-26 12:55 UTC (permalink / raw)
  To: Ulf Hansson, Russell King - ARM Linux
  Cc: Krzysztof Kozlowski, linux-samsung-soc, Kukjin Kim,
	linux-arm-kernel, Bartlomiej Zolnierkiewicz

Hello,

On 2015-11-26 11:24, Ulf Hansson wrote:
> On 26 November 2015 at 09:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> On 2015-11-25 19:09, Russell King - ARM Linux wrote:
>>> On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
>>>> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com>
>>>> wrote:
>>>>> 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).
>
> 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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/2] drivers: amba: properly handle devices with power domains
@ 2015-11-26 12:55                 ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-26 12:55 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On 2015-11-26 11:24, Ulf Hansson wrote:
> On 26 November 2015 at 09:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> On 2015-11-25 19:09, Russell King - ARM Linux wrote:
>>> On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
>>>> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com>
>>>> wrote:
>>>>> 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).
>
> 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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/2] drivers: amba: properly handle devices with power domains
  2015-11-26 10:24               ` Ulf Hansson
@ 2015-11-26 15:04                 ` Mathieu Poirier
  -1 siblings, 0 replies; 36+ messages in thread
From: Mathieu Poirier @ 2015-11-26 15:04 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Marek Szyprowski, Russell King - ARM Linux, Krzysztof Kozlowski,
	linux-samsung-soc, Kukjin Kim, linux-arm-kernel,
	Bartlomiej Zolnierkiewicz

On 26 November 2015 at 03:24, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 26 November 2015 at 09:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> Hello,
>>
>> On 2015-11-25 19:09, Russell King - ARM Linux wrote:
>>>
>>> On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
>>>>
>>>> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com>
>>>> wrote:
>>>>>
>>>>> 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).
>
> 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.

>
> Kind regards
> Uffe
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/2] drivers: amba: properly handle devices with power domains
@ 2015-11-26 15:04                 ` Mathieu Poirier
  0 siblings, 0 replies; 36+ messages in thread
From: Mathieu Poirier @ 2015-11-26 15:04 UTC (permalink / raw)
  To: linux-arm-kernel

On 26 November 2015 at 03:24, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 26 November 2015 at 09:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> Hello,
>>
>> On 2015-11-25 19:09, Russell King - ARM Linux wrote:
>>>
>>> On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
>>>>
>>>> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com>
>>>> wrote:
>>>>>
>>>>> 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).
>
> 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.

>
> Kind regards
> Uffe
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 1/2] drivers: amba: properly handle devices with power domains
  2015-11-26 15:04                 ` Mathieu Poirier
@ 2015-11-27  7:42                   ` Marek Szyprowski
  -1 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-27  7:42 UTC (permalink / raw)
  To: Mathieu Poirier, Ulf Hansson
  Cc: Russell King - ARM Linux, Krzysztof Kozlowski, linux-samsung-soc,
	Kukjin Kim, linux-arm-kernel, Bartlomiej Zolnierkiewicz

Hello,

On 2015-11-26 16:04, Mathieu Poirier wrote:
> On 26 November 2015 at 03:24, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 26 November 2015 at 09:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>> On 2015-11-25 19:09, Russell King - ARM Linux wrote:
>>>> On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
>>>>> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com>
>>>>> wrote:
>>>>>> 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).
>>
>> 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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 1/2] drivers: amba: properly handle devices with power domains
@ 2015-11-27  7:42                   ` Marek Szyprowski
  0 siblings, 0 replies; 36+ messages in thread
From: Marek Szyprowski @ 2015-11-27  7:42 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On 2015-11-26 16:04, Mathieu Poirier wrote:
> On 26 November 2015 at 03:24, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 26 November 2015 at 09:39, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>> On 2015-11-25 19:09, Russell King - ARM Linux wrote:
>>>> On Wed, Nov 25, 2015 at 02:56:10PM +0100, Ulf Hansson wrote:
>>>>> On 25 November 2015 at 14:34, Marek Szyprowski <m.szyprowski@samsung.com>
>>>>> wrote:
>>>>>> 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).
>>
>> 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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2015-11-27  7:43 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-25 12:55 [PATCH 0/2] Exynos4210: fix power domain for MDMA1 device Marek Szyprowski
2015-11-25 12:55 ` Marek Szyprowski
2015-11-25 12:55 ` [PATCH] ARM: dts: exynos4210: Add power domain to G2D device Marek Szyprowski
2015-11-25 12:55   ` Marek Szyprowski
2015-11-25 23:55   ` Krzysztof Kozlowski
2015-11-25 23:55     ` Krzysztof Kozlowski
2015-11-26  7:35   ` Krzysztof Kozlowski
2015-11-26  7:35     ` Krzysztof Kozlowski
2015-11-25 12:55 ` [PATCH 2/2] ARM: dts: exynos4210: MDMA1 device belongs to LCD0 power domain Marek Szyprowski
2015-11-25 12:55   ` Marek Szyprowski
2015-11-25 12:58 ` [PATCH 1/2] drivers: amba: properly handle devices with power domains Marek Szyprowski
2015-11-25 12:58   ` Marek Szyprowski
2015-11-25 13:24   ` Russell King - ARM Linux
2015-11-25 13:24     ` Russell King - ARM Linux
2015-11-25 13:34     ` Marek Szyprowski
2015-11-25 13:34       ` Marek Szyprowski
2015-11-25 13:56       ` Ulf Hansson
2015-11-25 13:56         ` Ulf Hansson
2015-11-25 14:43         ` Marek Szyprowski
2015-11-25 14:43           ` Marek Szyprowski
2015-11-25 18:09         ` Russell King - ARM Linux
2015-11-25 18:09           ` Russell King - ARM Linux
2015-11-26  8:39           ` Marek Szyprowski
2015-11-26  8:39             ` Marek Szyprowski
2015-11-26 10:24             ` Ulf Hansson
2015-11-26 10:24               ` Ulf Hansson
2015-11-26 10:59               ` Russell King - ARM Linux
2015-11-26 10:59                 ` Russell King - ARM Linux
2015-11-26 12:33                 ` Marek Szyprowski
2015-11-26 12:33                   ` Marek Szyprowski
2015-11-26 12:55               ` Marek Szyprowski
2015-11-26 12:55                 ` Marek Szyprowski
2015-11-26 15:04               ` Mathieu Poirier
2015-11-26 15:04                 ` Mathieu Poirier
2015-11-27  7:42                 ` Marek Szyprowski
2015-11-27  7:42                   ` Marek Szyprowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.