[PATCH 0/3] Add PMU node for Exynos4210, Exynos4412, Exynos4212

[PATCH 0/3] Add PMU node for Exynos4210, Exynos4412, Exynos4212

[Pankaj Dubey]

This patch updates Exynos4210, Exynos4412 and Exynos4212 dtsi files
for adding PMU (Power Management Unit) reg node. Also updated binding
document for the same. 
These patches required for using PMU base address via DT for these SoC.
Patches for using PMU base address via DT are posted here-

http://www.mail-archive.com/linux-samsung-soc at vger.kernel.org/msg27875.html

Pankaj Dubey (3):
  ARM: dts: Add PMU reg node to exynos4210
  ARM: dts: Add PMU reg node to exynos4212 and exynos4412
  Documentation: update samsung pmu binding information for
    Exynos4210/4212/4412

 Documentation/devicetree/bindings/arm/samsung/pmu.txt |    3 +++
 arch/arm/boot/dts/exynos4210.dtsi                     |    5 +++++
 arch/arm/boot/dts/exynos4212.dtsi                     |    5 +++++
 arch/arm/boot/dts/exynos4412.dtsi                     |    5 +++++
 4 files changed, 18 insertions(+)

-- 
1.7.10.4

[PATCH 1/3] ARM: dts: Add PMU reg node to exynos4210

[Pankaj Dubey]

This patch adds pmu regnode to exynos4210 dtsi to handle
PMU register access via DT.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/boot/dts/exynos4210.dtsi |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index cacf614..0a2c0fe 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -81,6 +81,11 @@
 		interrupts = <2 2>, <3 2>;
 	};
 
+	pmu_system_controller: system-controller at 10020000 {
+		compatible = "samsung,exynos4210-pmu", "syscon";
+		reg = <0x10020000 0x5000>;
+	};
+
 	pinctrl_0: pinctrl at 11400000 {
 		compatible = "samsung,exynos4210-pinctrl";
 		reg = <0x11400000 0x1000>;
-- 
1.7.10.4

[PATCH 2/3] ARM: dts: Add PMU reg node to exynos4212 and exynos4412

[Pankaj Dubey]

This patch adds pmu regnode to exynos4212 and exynos4412 dtsi to
handle PMU register access via DT.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/boot/dts/exynos4212.dtsi |    5 +++++
 arch/arm/boot/dts/exynos4412.dtsi |    5 +++++
 2 files changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
index 3c00e6e..4661ffa 100644
--- a/arch/arm/boot/dts/exynos4212.dtsi
+++ b/arch/arm/boot/dts/exynos4212.dtsi
@@ -29,4 +29,9 @@
 	gic: interrupt-controller at 10490000 {
 		cpu-offset = <0x8000>;
 	};
+
+	pmu_system_controller: system-controller at 10020000 {
+		compatible = "samsung,exynos4212-pmu", "syscon";
+		reg = <0x10020000 0x5000>;
+	};
 };
diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
index 15d3c0a..6f4cb22 100644
--- a/arch/arm/boot/dts/exynos4412.dtsi
+++ b/arch/arm/boot/dts/exynos4412.dtsi
@@ -29,4 +29,9 @@
 	gic: interrupt-controller at 10490000 {
 		cpu-offset = <0x4000>;
 	};
+
+	pmu_system_controller: system-controller at 10020000 {
+		compatible = "samsung,exynos4412-pmu", "syscon";
+		reg = <0x10020000 0x5000>;
+	};
 };
-- 
1.7.10.4

[PATCH 3/3] Documentation: update samsung pmu binding information for Exynos4210/4212/4412

[Pankaj Dubey]

This patch updates samsung pmu (power management unit) binding information
for Exynos4210, Exynos4212 and Exynos4412.

CC: Leela Krishna Amudala <l.krishna@samsung.com>
CC: Tomasz Figa <t.figa@samsung.com>
CC: Guenter Roeck <linux@roeck-us.net>
CC: Rob Landley <rob@landley.net>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 Documentation/devicetree/bindings/arm/samsung/pmu.txt |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/samsung/pmu.txt b/Documentation/devicetree/bindings/arm/samsung/pmu.txt
index f1f1552..b562634 100644
--- a/Documentation/devicetree/bindings/arm/samsung/pmu.txt
+++ b/Documentation/devicetree/bindings/arm/samsung/pmu.txt
@@ -2,6 +2,9 @@ SAMSUNG Exynos SoC series PMU Registers
 
 Properties:
  - compatible : should contain two values. First value must be one from following list:
+		   - "samsung,exynos4210-pmu" - for Exynos4210 SoC,
+		   - "samsung,exynos4212-pmu" - for Exynos4212 SoC,
+		   - "samsung,exynos4412-pmu" - for Exynos4412 SoC,
 		   - "samsung,exynos5250-pmu" - for Exynos5250 SoC,
 		   - "samsung,exynos5420-pmu" - for Exynos5420 SoC.
 		second value must be always "syscon".
-- 
1.7.10.4

[PATCH 1/3] ARM: dts: Add PMU reg node to exynos4210

[Vikas Sajjan]

+Tomasz


On Wed, Apr 2, 2014 at 1:32 PM, Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
> This patch adds pmu regnode to exynos4210 dtsi to handle
> PMU register access via DT.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
>  arch/arm/boot/dts/exynos4210.dtsi |    5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
> index cacf614..0a2c0fe 100644
> --- a/arch/arm/boot/dts/exynos4210.dtsi
> +++ b/arch/arm/boot/dts/exynos4210.dtsi
> @@ -81,6 +81,11 @@
>                 interrupts = <2 2>, <3 2>;
>         };
>
> +       pmu_system_controller: system-controller at 10020000 {
> +               compatible = "samsung,exynos4210-pmu", "syscon";
> +               reg = <0x10020000 0x5000>;

Can we have 2 strings as "compatible" and these 2 strings are used by
2 different driver?

Because once syscon driver gets probed, exynos-pmu.c [1] driver will
never be probed.

The new PMU driver [1] completely relies on this. With just
exynos_defconfig, you will not notice this issue, since syscon is NOT
enabled by exynos_defconfig. When I enabled "System Controller
Register R/W Based on Regmap" in menuconfig, I could see PMU driver
[1] is NEVER probed.

Let me know, if I am missing something.

 [1] http://www.spinics.net/lists/arm-kernel/msg319750.html


> +       };
> +
>         pinctrl_0: pinctrl at 11400000 {
>                 compatible = "samsung,exynos4210-pinctrl";
>                 reg = <0x11400000 0x1000>;
> --
> 1.7.10.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] ARM: dts: Add PMU reg node to exynos4210

[Pankaj Dubey]

Hi Vikas,

On 04/24/2014 08:59 PM, Vikas Sajjan wrote:
> +Tomasz
>
>
> On Wed, Apr 2, 2014 at 1:32 PM, Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
>> This patch adds pmu regnode to exynos4210 dtsi to handle
>> PMU register access via DT.
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>>   arch/arm/boot/dts/exynos4210.dtsi |    5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
>> index cacf614..0a2c0fe 100644
>> --- a/arch/arm/boot/dts/exynos4210.dtsi
>> +++ b/arch/arm/boot/dts/exynos4210.dtsi
>> @@ -81,6 +81,11 @@
>>                  interrupts = <2 2>, <3 2>;
>>          };
>>
>> +       pmu_system_controller: system-controller at 10020000 {
>> +               compatible = "samsung,exynos4210-pmu", "syscon";
>> +               reg = <0x10020000 0x5000>;
> Can we have 2 strings as "compatible" and these 2 strings are used by
> 2 different driver?
>
> Because once syscon driver gets probed, exynos-pmu.c [1] driver will
> never be probed.
>
> The new PMU driver [1] completely relies on this. With just
> exynos_defconfig, you will not notice this issue, since syscon is NOT
> enabled by exynos_defconfig. When I enabled "System Controller
> Register R/W Based on Regmap" in menuconfig, I could see PMU driver
> [1] is NEVER probed.
>
> Let me know, if I am missing something.
>
>   [1] http://www.spinics.net/lists/arm-kernel/msg319750.html

You are correct. We also missed this because "SYSCON" option was
not enabled in exynos_defconfig.
I noticed this during preparation of V2 when I planned to use
"early_syscon_init" [1] as suggested by Sylwester here [2].

In V2 I will take care of this. I hope soon I will be able to post second
version of this series, just waiting for testing and internal code review.

[1] https://lkml.org/lkml/2014/4/8/239
[2] https://lkml.org/lkml/2014/4/2/171
>
>
>> +       };
>> +
>>          pinctrl_0: pinctrl at 11400000 {
>>                  compatible = "samsung,exynos4210-pinctrl";
>>                  reg = <0x11400000 0x1000>;
>> --
>> 1.7.10.4
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


-- 
Best Regards,
Pankaj Dubey

[PATCH 1/3] ARM: dts: Add PMU reg node to exynos4210

[Vikas Sajjan]

Hi Pankaj,

On Thu, Apr 24, 2014 at 6:27 PM, Pankaj Dubey <pankaj.dubey@samsung.com> wrote:
> Hi Vikas,
>
>
> On 04/24/2014 08:59 PM, Vikas Sajjan wrote:
>>
>> +Tomasz
>>
>>
>> On Wed, Apr 2, 2014 at 1:32 PM, Pankaj Dubey <pankaj.dubey@samsung.com>
>> wrote:
>>>
>>> This patch adds pmu regnode to exynos4210 dtsi to handle
>>> PMU register access via DT.
>>>
>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>> ---
>>>   arch/arm/boot/dts/exynos4210.dtsi |    5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/exynos4210.dtsi
>>> b/arch/arm/boot/dts/exynos4210.dtsi
>>> index cacf614..0a2c0fe 100644
>>> --- a/arch/arm/boot/dts/exynos4210.dtsi
>>> +++ b/arch/arm/boot/dts/exynos4210.dtsi
>>> @@ -81,6 +81,11 @@
>>>                  interrupts = <2 2>, <3 2>;
>>>          };
>>>
>>> +       pmu_system_controller: system-controller at 10020000 {
>>> +               compatible = "samsung,exynos4210-pmu", "syscon";
>>> +               reg = <0x10020000 0x5000>;
>>
>> Can we have 2 strings as "compatible" and these 2 strings are used by
>> 2 different driver?
>>
>> Because once syscon driver gets probed, exynos-pmu.c [1] driver will
>> never be probed.
>>
>> The new PMU driver [1] completely relies on this. With just
>> exynos_defconfig, you will not notice this issue, since syscon is NOT
>> enabled by exynos_defconfig. When I enabled "System Controller
>> Register R/W Based on Regmap" in menuconfig, I could see PMU driver
>> [1] is NEVER probed.
>>
>> Let me know, if I am missing something.
>>
>>   [1] http://www.spinics.net/lists/arm-kernel/msg319750.html
>
>
> You are correct. We also missed this because "SYSCON" option was
> not enabled in exynos_defconfig.
> I noticed this during preparation of V2 when I planned to use
> "early_syscon_init" [1] as suggested by Sylwester here [2].
>
OK.

> In V2 I will take care of this. I hope soon I will be able to post second

So in your V2, will you be adding new DT node altogether for PMU?
Because with existing DT node "pmu_system_controller"  (which is
already present in mailine for exynos 5250) and SYSCON driver enable,
exynos PMU driver will NEVER probed.

> version of this series, just waiting for testing and internal code review.
>
> [1] https://lkml.org/lkml/2014/4/8/239
> [2] https://lkml.org/lkml/2014/4/2/171
>
>>
>>
>>> +       };
>>> +
>>>          pinctrl_0: pinctrl at 11400000 {
>>>                  compatible = "samsung,exynos4210-pinctrl";
>>>                  reg = <0x11400000 0x1000>;
>>> --
>>> 1.7.10.4
>>>
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
>
> --
> Best Regards,
> Pankaj Dubey
>

[PATCH v2 0/5] Add PMU node for Exynos SoCs

[Pankaj Dubey]

This patch series adds new property in samsung PMU binding as
"samsung,syscon-phandle" to get PMU regmap handle in PMU driver
implementation and "mach-exynos/exynos.c".
This patch also updates Exynos4210, Exynos4412 and Exynos4212 dtsi files
for adding PMU (Power Management Unit) reg node.
Updated binding document for the same.

These patches are required for converting PMU implementation free from
static mapping of PMU registers as well as it's dependency from machine
header files, thus allowing us to converting it into DT based.

Changes since v1:
 - Added new property "samsung,syscon-phandle" to get access to PMU regmap handle.
 - Updated Exynos5250, Exynos5420, Exynos4210/4412/4212 dtsi files.
 - Rebased on Kukjin Kim's for-next branch.

Pankaj Dubey (5):
  ARM: dts: Add syscon handle in pmu node for exynos5250
  ARM: dts: Add PMU node to exynos4210
  ARM: dts: Add PMU node to exynos4212 and exynos4412
  arm: dts: Add syscon handle in pmu node for exynos5420
  Documentation: update samsung pmu binding information

 Documentation/devicetree/bindings/arm/samsung/pmu.txt |    7 +++++++
 arch/arm/boot/dts/exynos4210.dtsi                     |    6 ++++++
 arch/arm/boot/dts/exynos4212.dtsi                     |    6 ++++++
 arch/arm/boot/dts/exynos4412.dtsi                     |    6 ++++++
 arch/arm/boot/dts/exynos5250.dtsi                     |    1 +
 arch/arm/boot/dts/exynos5420.dtsi                     |    1 +
 6 files changed, 27 insertions(+)

-- 
1.7.10.4

[PATCH v2 1/5] ARM: dts: Add syscon handle in pmu node for exynos5250

[Pankaj Dubey]

Add "samsung,syscon-phandle" property pointing to PMU node
to access PMU register via PMU regmap handle.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/boot/dts/exynos5250.dtsi |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 3742331..52801af 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -173,6 +173,7 @@
 	pmu_system_controller: system-controller at 10040000 {
 		compatible = "samsung,exynos5250-pmu", "syscon";
 		reg = <0x10040000 0x5000>;
+		samsung,syscon-phandle = <&pmu_system_controller>;
 	};
 
 	watchdog at 101D0000 {
-- 
1.7.10.4

[PATCH v2 2/5] ARM: dts: Add PMU node to exynos4210

[Pankaj Dubey]

This patch adds pmu regnode to exynos4210 dtsi to handle
PMU register access via DT.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/boot/dts/exynos4210.dtsi |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index cacf614..042b6d9 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -81,6 +81,12 @@
 		interrupts = <2 2>, <3 2>;
 	};
 
+	pmu_system_controller: system-controller at 10020000 {
+		compatible = "samsung,exynos4210-pmu", "syscon";
+		reg = <0x10020000 0x5000>;
+		samsung,syscon-phandle = <&pmu_system_controller>;
+	};
+
 	pinctrl_0: pinctrl at 11400000 {
 		compatible = "samsung,exynos4210-pinctrl";
 		reg = <0x11400000 0x1000>;
-- 
1.7.10.4

[PATCH v2 3/5] ARM: dts: Add PMU node to exynos4212 and exynos4412

[Pankaj Dubey]

This patch adds pmu regnode to exynos4212 and exynos4412 dtsi to
handle PMU register access via DT.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/boot/dts/exynos4212.dtsi |    6 ++++++
 arch/arm/boot/dts/exynos4412.dtsi |    6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
index 3c00e6e..b1d4768 100644
--- a/arch/arm/boot/dts/exynos4212.dtsi
+++ b/arch/arm/boot/dts/exynos4212.dtsi
@@ -29,4 +29,10 @@
 	gic: interrupt-controller at 10490000 {
 		cpu-offset = <0x8000>;
 	};
+
+	pmu_system_controller: system-controller at 10020000 {
+		compatible = "samsung,exynos4212-pmu", "syscon";
+		reg = <0x10020000 0x5000>;
+		samsung,syscon-phandle = <&pmu_system_controller>;
+	};
 };
diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
index 15d3c0a..6152e70 100644
--- a/arch/arm/boot/dts/exynos4412.dtsi
+++ b/arch/arm/boot/dts/exynos4412.dtsi
@@ -29,4 +29,10 @@
 	gic: interrupt-controller at 10490000 {
 		cpu-offset = <0x4000>;
 	};
+
+	pmu_system_controller: system-controller at 10020000 {
+		compatible = "samsung,exynos4412-pmu", "syscon";
+		reg = <0x10020000 0x5000>;
+		samsung,syscon-phandle = <&pmu_system_controller>;
+	};
 };
-- 
1.7.10.4

[PATCH v2 4/5] arm: dts: Add syscon handle in pmu node for exynos5420

[Pankaj Dubey]

Add "samsung,syscon-phandle" property pointing to PMU node
to access PMU register via PMU regmap handle.

Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 arch/arm/boot/dts/exynos5420.dtsi |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index c3a9a66..c6e449d 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -673,6 +673,7 @@
 	pmu_system_controller: system-controller at 10040000 {
 		compatible = "samsung,exynos5420-pmu", "syscon";
 		reg = <0x10040000 0x5000>;
+		samsung,syscon-phandle = <&pmu_system_controller>;
 	};
 
 	tmu_cpu0: tmu at 10060000 {
-- 
1.7.10.4

[PATCH v2 5/5] Documentation: update samsung pmu binding information

[Pankaj Dubey]

This patch updates samsung pmu (power management unit) binding information.

CC: Leela Krishna Amudala <l.krishna@samsung.com>
CC: Guenter Roeck <linux@roeck-us.net>
CC: Rob Landley <rob@landley.net>
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
 Documentation/devicetree/bindings/arm/samsung/pmu.txt |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/samsung/pmu.txt b/Documentation/devicetree/bindings/arm/samsung/pmu.txt
index f1f1552..ac9b45c 100644
--- a/Documentation/devicetree/bindings/arm/samsung/pmu.txt
+++ b/Documentation/devicetree/bindings/arm/samsung/pmu.txt
@@ -2,14 +2,21 @@ SAMSUNG Exynos SoC series PMU Registers
 
 Properties:
  - compatible : should contain two values. First value must be one from following list:
+		   - "samsung,exynos4210-pmu" - for Exynos4210 SoC,
+		   - "samsung,exynos4212-pmu" - for Exynos4212 SoC,
+		   - "samsung,exynos4412-pmu" - for Exynos4412 SoC,
 		   - "samsung,exynos5250-pmu" - for Exynos5250 SoC,
 		   - "samsung,exynos5420-pmu" - for Exynos5420 SoC.
 		second value must be always "syscon".
 
  - reg : offset and length of the register set.
 
+ - samsung,syscon-phandle : reference to syscon node. This property points to syscon node
+		holding the PMU base address.
+
 Example :
 pmu_system_controller: system-controller at 10040000 {
 	compatible = "samsung,exynos5250-pmu", "syscon";
 	reg = <0x10040000 0x5000>;
+	samsung,syscon-phandle = <&pmu_system_controller>;
 };
-- 
1.7.10.4

[PATCH v2 1/5] ARM: dts: Add syscon handle in pmu node for exynos5250

[Tomasz Figa]

Hi Pankaj,

On 25.04.2014 13:54, Pankaj Dubey wrote:
> Add "samsung,syscon-phandle" property pointing to PMU node
> to access PMU register via PMU regmap handle.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
>   arch/arm/boot/dts/exynos5250.dtsi |    1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> index 3742331..52801af 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -173,6 +173,7 @@
>   	pmu_system_controller: system-controller at 10040000 {
>   		compatible = "samsung,exynos5250-pmu", "syscon";
>   		reg = <0x10040000 0x5000>;
> +		samsung,syscon-phandle = <&pmu_system_controller>;
>   	};
>
>   	watchdog at 101D0000 {
>

This looks strange. A node that refers back to itself. If I understand 
correctly, you are relying on the fact that if you don't use 
platform_driver model for the PMU driver, it won't bind to this node and 
instead the generic syscon driver will. I'm afraid this is incorrect, 
because the PMU driver should normally use the driver model.

I see two possible options to solve this problem:

1) Instead, the PMU driver should bind to this node and become a syscon 
provider. This will require a small change in the syscon API, which 
would be reasonable anyway:

  a) instead of using driver_find_device() and dev_get_drvdata() in 
syscon_node_to_regmap(), a local list of syscon structs should be kept 
in this driver and then the look-up would just iterate over this list, 
that could contain external syscon implementations as well,

  b) syscon_register() function should be provided to register an 
external syscon provider from other drivers, like Exynos PMU driver.

Another solution would be:

2) Register a static platform device from Exynos PMU driver, with 
.of_node set to PMU node and .name to "syscon" to instantiate the syscon 
device and create a syscon provider, even though the PMU driver would be 
bound to the node.

The change mentioned in point 1.a) should be implemented regardless of 
which solution is chosen, as iterating over all devices in the system 
and relying on their driver_data is rather a poor practice. A local list 
would be faster - all syscons to iterate over, instead of all devices in 
the system, and more flexible - a single device could be a provider of 
multiple resources.

As for the solution for Exynos PMU itself, I tend to prefer 2), as it 
wouldn't require additional functions exported from the syscon driver.

Best regards,
Tomasz

[PATCH v2 1/5] ARM: dts: Add syscon handle in pmu node for exynos5250

[Pankaj Dubey]

Hi Tomasz,

On 04/26/2014 05:55 AM, Tomasz Figa wrote:
> Hi Pankaj,
>
> On 25.04.2014 13:54, Pankaj Dubey wrote:
>> Add "samsung,syscon-phandle" property pointing to PMU node
>> to access PMU register via PMU regmap handle.
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>>   arch/arm/boot/dts/exynos5250.dtsi |    1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
>> b/arch/arm/boot/dts/exynos5250.dtsi
>> index 3742331..52801af 100644
>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>> @@ -173,6 +173,7 @@
>>       pmu_system_controller: system-controller at 10040000 {
>>           compatible = "samsung,exynos5250-pmu", "syscon";
>>           reg = <0x10040000 0x5000>;
>> +        samsung,syscon-phandle = <&pmu_system_controller>;
>>       };
>>
>>       watchdog at 101D0000 {
>>
>
> This looks strange. A node that refers back to itself. If I understand 
> correctly, you are relying on the fact that if you don't use platform_driver 
> model for the PMU driver, it won't bind to this node and instead the generic 
> syscon driver will. I'm afraid this is incorrect, because the PMU driver 
> should normally use the driver model.

No, let me explain you, property "samsung, syscon-phandle" referring back to 
it's own node
and I am not using platform_driver model for PMU driver are not related with 
each other.

We added "samsung, syscon-phandle" property to PMU node, as current syscon API's 
to get regmap
such as "syscon_regmap_lookup_by_phandle" expects "property_name" to lookup for 
proper device_node
which has regmap. So if we are passing PMU device_node to such API we should 
have that property under
PMU device node. So even though it's looking strange I do not think it's wrong.

But still since, you pointed out I checked syscon APIs carefully and found that 
we can avoid such property
which refers back to itself if we modify syscon APIs and allow them to accept 
"property" parameter as NULL,
assuming that device_node passed itself has regmap handle, following 
modification worked for me and if
it is acceptable I do not need to add such property in PMU device node.

==========
struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np,
                      const char *property)
  {
      struct device_node *syscon_np;
      struct regmap *regmap;
-
-    syscon_np = of_parse_phandle(np, property, 0);
-    if (!syscon_np)
-        return ERR_PTR(-ENODEV);
-
-    regmap = syscon_node_to_regmap(syscon_np);
-    of_node_put(syscon_np);
-
+
+    if (property) {
+        syscon_np = of_parse_phandle(np, property, 0);
+        if (!syscon_np)
+            return ERR_PTR(-ENODEV);
+
+        regmap = syscon_node_to_regmap(syscon_np);
+        of_node_put(syscon_np);
+    } else {
+        regmap = syscon_node_to_regmap(np);
+    }
      return regmap;
  }
=============

Other problem as you are seeing that current patch does not uses platform_driver 
model for PMU
is because PMU device node has two compatibility string and "syscon" driver is 
getting registered as
core_initcall, much before PMU registration. So only "syscon" driver's probe is 
getting called. So it does
not depend whether we have this property in PMU device node or we do not have.

>
> I see two possible options to solve this problem:
>
> 1) Instead, the PMU driver should bind to this node and become a syscon 
> provider. This will require a small change in the syscon API, which would be 
> reasonable anyway:
>
>  a) instead of using driver_find_device() and dev_get_drvdata() in 
> syscon_node_to_regmap(), a local list of syscon structs should be kept in this 
> driver and then the look-up would just iterate over this list, that could 
> contain external syscon implementations as well,
>
>  b) syscon_register() function should be provided to register an external 
> syscon provider from other drivers, like Exynos PMU driver.
>
> Another solution would be:
>
> 2) Register a static platform device from Exynos PMU driver, with .of_node set 
> to PMU node and .name to "syscon" to instantiate the syscon device and create 
> a syscon provider, even though the PMU driver would be bound to the node.

I do not understand you here. Will you please explain a bit more here?

How come two driver with same ".name" can be registered? I can see if "syscon" 
driver
registration comes first, static registration from PMU driver, with same .name 
will fail.
Please correct me if I am wrong.


>
> The change mentioned in point 1.a) should be implemented regardless of which 
> solution is chosen, as iterating over all devices in the system and relying on 
> their driver_data is rather a poor practice. A local list would be faster - 
> all syscons to iterate over, instead of all devices in the system, and more 
> flexible - a single device could be a provider of multiple resources.

Your suggestion 1.a) worked well and I modified and tested it also.
So probably I can send that patch separately. Even though I can't see this 
helping me to convert PMU
as platform_driver model.

>
> As for the solution for Exynos PMU itself, I tend to prefer 2), as it wouldn't 
> require additional functions exported from the syscon driver.
>
> Best regards,
> Tomasz
>


-- 
Best Regards,
Pankaj Dubey