Re: [PATCH V2 5/5] ARM: dts: exynos4412: Rename OPP nodes as opp@

[Viresh Kumar]

On 05-11-15, 10:51, Krzysztof Kozlowski wrote:
> I see this patch does not depend on the rest of patchset so I presume
> this can co through samsung-soc?

Yeah, I wouldn't mind that. But I would wait for a confirmation from
Rafael for the bindings first, for an unlikely case where he doesn't
like the fourth patch ;)

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 5/5] ARM: dts: exynos4412: Rename OPP nodes as opp@

[Viresh Kumar]

OPP bindings got updated to name OPP nodes this way, make changes
according to that.

Signed-off-by: Viresh Kumar 
---
 arch/arm/boot/dts/exynos4412.dtsi | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4412.dtsi 
b/arch/arm/boot/dts/exynos4412.dtsi
index 294cfe40388d..40beede46e55 100644
--- a/arch/arm/boot/dts/exynos4412.dtsi
+++ b/arch/arm/boot/dts/exynos4412.dtsi
@@ -64,73 +64,73 @@
compatible = "operating-points-v2";
opp-shared;
 
-   opp00 {
+   opp@2 {
opp-hz = /bits/ 64 <2>;
opp-microvolt = <90>;
clock-latency-ns = <20>;
};
-   opp01 {
+   opp@3 {
opp-hz = /bits/ 64 <3>;
opp-microvolt = <90>;
clock-latency-ns = <20>;
};
-   opp02 {
+   opp@4 {
opp-hz = /bits/ 64 <4>;
opp-microvolt = <925000>;
clock-latency-ns = <20>;
};
-   opp03 {
+   opp@5 {
opp-hz = /bits/ 64 <5>;
opp-microvolt = <95>;
clock-latency-ns = <20>;
};
-   opp04 {
+   opp@6 {
opp-hz = /bits/ 64 <6>;
opp-microvolt = <975000>;
clock-latency-ns = <20>;
};
-   opp05 {
+   opp@7 {
opp-hz = /bits/ 64 <7>;
opp-microvolt = <987500>;
clock-latency-ns = <20>;
};
-   opp06 {
+   opp@8 {
opp-hz = /bits/ 64 <8>;
opp-microvolt = <100>;
clock-latency-ns = <20>;
opp-suspend;
};
-   opp07 {
+   opp@9 {
opp-hz = /bits/ 64 <9>;
opp-microvolt = <1037500>;
clock-latency-ns = <20>;
};
-   opp08 {
+   opp@10 {
opp-hz = /bits/ 64 <10>;
opp-microvolt = <1087500>;
clock-latency-ns = <20>;
};
-   opp09 {
+   opp@11 {
opp-hz = /bits/ 64 <11>;
opp-microvolt = <1137500>;
clock-latency-ns = <20>;
};
-   opp10 {
+   opp@12 {
opp-hz = /bits/ 64 <12>;
opp-microvolt = <1187500>;
clock-latency-ns = <20>;
};
-   opp11 {
+   opp@13 {
opp-hz = /bits/ 64 <13>;
opp-microvolt = <125>;
clock-latency-ns = <20>;
};
-   opp12 {
+   opp@14 {
opp-hz = /bits/ 64 <14>;
opp-microvolt = <1287500>;
clock-latency-ns = <20>;
};
-   opp13 {
+   opp@15 {
opp-hz = /bits/ 64 <15>;
opp-microvolt = <135>;
clock-latency-ns = <20>;
-- 
2.6.2.198.g614a2ac

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/7] samsung: pmu: split up SoC specific PMU data

[Pankaj Dubey]

Hi Krzysztof,

On Tuesday 03 November 2015 07:36 AM, Krzysztof Kozlowski wrote:

On 26.10.2015 21:55, Pankaj Dubey wrote:

This patch series is a part of continuation work from following series
[1] and [2].

1: exynos: Move pmu driver to driver/soc folder and add exynos7 support
http://www.spinics.net/lists/linux-samsung-soc/msg39797.html from Amit 
Daniel Kacchap
2: soc: samsung: pmu: split up SoC specific PMU data
https://lkml.org/lkml/2015/1/7/12 from me



+Cc Bartlomiej,

There were some concerns for previous versions of this patchset. I
cannot find all of them (e.g. Bartlomiej's are not present on lkml.org
anymore) so I am not sure if they were addressed properly.



Yes. If I recall correctly he has following main concerns:
1: To convert exynos-pmu to a proper platform driver before moving out 
of arch/arm/mach-exynos. This is already addressed.


2: Do we really need common driver for both ARM and ARM64? I feel yes, 
as at least I can see that driver's basic structure will be reused. As 
in case of PMU driver most of lines of code is data part (register 
offset and its values in different mode), that part will be kept in 
separate file e.g. exynos7-pmu.c or exynos-pmu.c.

There has been already one attempt of submission for exynos7 PMU driver at:

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/305418.html

3: He had a concern that in case of ARM64 build most of ARM related code 
will be present in binary which will be dead code, that can be addressed 
by excluding ARM based SoC exynos-pmu.c file in Makefile.

It has been taken care in v3 7/7 patch in drivers/soc/samsung/Makefile.


I like the approach, it helps in reviewing the patch.


Thanks. This was a review comment from Kukjin, to separate data part of 
all SoC and move them to respective files, same as we do for clock files 
of each SoC.




I wonder - after adding this for ARM64 how much of duplicated code this
approach reduce?



It will reduce in duplication driver structure, which will be same.


Best regards,
Krzysztof


Here is another attempt for the same, in this series I am splitting up SoC
specific PMU configuration data into mach-exynos folder itself, before moving
all of them under drivers/soc/samsung/. Also instead of making all changes in
single patch it has been broken into SoC specific patches to avoid large size
of patch. With this approach there will not be unwanted big churns just after
adding exynos-pmu under drivers/soc/samsung.

All these patches are just refactoring to keep minimal changes while moving
exynos-pmu driver under drivers/soc/samsung/. Support for exynos7 PMU can be 
added
on top of it, in such a manner that for ARM64 build, ARM related SoC's PMU will 
not
get compiled and thus unnecessary increasing kernel image size.

I have tested on Peach-Pi (Exynos5880) based chromebook for boot
and S2R functionality.

These patches have been prepared on top of Kukjin Kim's for-next

Changes since v2:
  - Removed Amit's Samsung id as it's no more valid.
  - Rebased on latest kgene tree.
  - Removed redundant code from regs-pmu.h


Pankaj Dubey (7):
   ARM: EXYNOS: removing redundant code from regs-pmu.h
   ARM: EXYNOS: Move pmu specific headers under "linux/soc/samsung"
   ARCH: EXYNOS: split up exynos3250 SoC specific PMU data
   ARCH: EXYNOS: split up exynos4 SoC specific PMU data
   ARCH: EXYNOS: split up exynos5250 SoC specific PMU data
   ARCH: EXYNOS: split up exynos5420 SoC specific PMU data
   drivers: soc: Add support for Exynos PMU driver

  arch/arm/mach-exynos/Kconfig   |1 +
  arch/arm/mach-exynos/Makefile  |2 +-
  arch/arm/mach-exynos/exynos.c  |2 +-
  arch/arm/mach-exynos/mcpm-exynos.c |2 +-
  arch/arm/mach-exynos/platsmp.c |2 +-
  arch/arm/mach-exynos/pm.c  |4 +-
  arch/arm/mach-exynos/pmu.c | 1004 
  arch/arm/mach-exynos/suspend.c |4 +-
  drivers/soc/samsung/Kconfig|4 +
  drivers/soc/samsung/Makefile   |4 +
  drivers/soc/samsung/exynos-pmu.c   |  168 
  drivers/soc/samsung/exynos-pmu.h   |   52 +
  drivers/soc/samsung/exynos3250-pmu.c   |  175 
  drivers/soc/samsung/exynos4-pmu.c  |  223 +
  drivers/soc/samsung/exynos5250-pmu.c   |  196 
  drivers/soc/samsung/exynos5420-pmu.c   |  280 ++
  .../linux/soc/samsung}/exynos-pmu.h|2 +-
  .../linux/soc/samsung/exynos-regs-pmu.h|   17 +-
  18 files changed, 1116 insertions(+), 1026 deletions(-)
  delete mode 100644 arch/arm/mach-exynos/pmu.c
  create mode 100644 drivers/soc/samsung/exynos-pmu.c
  create mode 100644 drivers/soc/samsung/exynos-pmu.h
  create mode 100644 drivers/soc/samsung/exynos3250-pmu.c
  create mode 100644 

Re: [PATCH v3 5/7] ARCH: EXYNOS: split up exynos5250 SoC specific PMU data

[Pankaj Dubey]

On Tuesday 03 November 2015 07:37 AM, Krzysztof Kozlowski wrote:

On 26.10.2015 21:55, Pankaj Dubey wrote:

This patch splits up mach-exynos/pmu.c file, and moves exynos5250,
PMU configuration data and functions handing data into exynos5250
SoC specific PMU file mach-exynos/exynos5250-pmu.c.

Signed-off-by: Pankaj Dubey 
---
  arch/arm/mach-exynos/Makefile |   4 +-
  arch/arm/mach-exynos/exynos-pmu.h |   1 +
  arch/arm/mach-exynos/exynos5250-pmu.c | 196 ++
  arch/arm/mach-exynos/pmu.c| 180 ---
  4 files changed, 200 insertions(+), 181 deletions(-)
  create mode 100644 arch/arm/mach-exynos/exynos5250-pmu.c



Reviewed-by: Krzysztof Kozlowski 



Thanks.

Pankaj Dubey


Best regards,
Krzysztof


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/7] ARCH: EXYNOS: split up exynos4 SoC specific PMU data

[Pankaj Dubey]

On Tuesday 03 November 2015 07:26 AM, Krzysztof Kozlowski wrote:

On 26.10.2015 21:55, Pankaj Dubey wrote:

This patch splits up mach-exynos/pmu.c file, and moves exynos4210,
exynos4412 and exynos4212 PMU configuration data and functions handing
data into a common exynos4 SoC specific PMU file mach-exynos/exynos4-pmu.c.

Signed-off-by: Pankaj Dubey 
---
  arch/arm/mach-exynos/Makefile  |   2 +-
  arch/arm/mach-exynos/exynos-pmu.h  |   3 +
  arch/arm/mach-exynos/exynos4-pmu.c | 223 +
  arch/arm/mach-exynos/pmu.c | 207 --
  4 files changed, 227 insertions(+), 208 deletions(-)
  create mode 100644 arch/arm/mach-exynos/exynos4-pmu.c




Reviewed-by: Krzysztof Kozlowski 



Thanks,

Pankaj Dubey


Best regards,
Krzysztof



--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node

[Krzysztof Kozlowski]

On 05.11.2015 14:37, Alim Akhtar wrote:
> Hi Krzysztof
> 
> On 11/02/2015 07:22 PM, Krzysztof Kozlowski wrote:
>> 2015-11-02 22:01 GMT+09:00 Alim Akhtar :
>
>arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349
> +++
>1 file changed, 349 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> index 838a3626dac1..8ce04a0ec928 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> @@ -53,6 +53,355 @@
>   status = "okay";
>};
>
> +_4 {
> +   samsung,i2c-sda-delay = <100>;
> +   samsung,i2c-max-bus-freq = <20>;
> +   status = "okay";
> +
> +   s2mps15_pmic@66 {
> +   compatible = "samsung,s2mps15-pmic";
> +   reg = <0x66>;
> +   interrupts = <2 0>;
> +   interrupt-parent = <>;
> +   pinctrl-names = "default";
> +   pinctrl-0 = <_irq>;
> +   wakeup-source;
> +
> +   s2mps15_osc: clocks {
> +   compatible = "samsung,s2mps13-clk";
> +   #clock-cells = <1>;
> +   clock-output-names = "s2mps13_ap",
> "s2mps13_cp",
> +   "s2mps13_bt";
> +   };


 Don't you want to use one of these clocks for s3c-rtc ( node)?

>>> yes, you are right, rtc on this board is currently broken, mainly
>>> because of
>>> the introduction of rtc_src clock in the s3c-rtc driver.
>>> That is on my do list next. will take a look.
>>>
>>> Are you suggesting to remove this -clk node now and add along with rtc
>>> changes? I feel this should go in along with this patch.
>>
>> Just add it in consecutive patch in this series. You added here some
>> providers (clock and regulators) without consumers. This of course
>> looks good as a way of providing full description of the board but:
>> 1. For regulators always on: may be meaningless for kernel. Kernel
>> does not use it. Existence of regulator subnode will fulfill driver's
>> needs for probe.
>> 2. For clocks: actually will disable these clocks because of lack of
>> consumers... which is fine but probably not what you wanted.
>>
>> The standard approach is to add such providers when they are needed -
>> there are some consumers using them.
>>
> OK. for now will keep the pmic clock added as clock will be in disabled
> state, so it wont harm.
> - will keep system related regulator like supply to arm,mif,int etc ..
> will remove supplies to other peripherals IPs. Hope thats fine.

You don't have to remove other regulators (these without consumers).
Describe in DT all of regulators but:
1. Add to some of them consumers;
2. Disable these which are not needed (by not marking always-enabled).

Just like for all other boards.

Best regards,
Krzysztof

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/7] ARM: EXYNOS: removing redundant code from regs-pmu.h

[Pankaj Dubey]

On Tuesday 03 November 2015 07:07 AM, Krzysztof Kozlowski wrote:

On 26.10.2015 21:55, Pankaj Dubey wrote:

commit 6ec4f8d0d91f ("ARM: EXYNOS: add generic function to calculate
cpu number") introduced exynos_pmu_cpunr to be used by multi-cluster SoC's
e.g Exynos5420, but it's no more used in the codebase and hence removing
this part of code.

Signed-off-by: Pankaj Dubey 
---
  arch/arm/mach-exynos/pmu.c  | 1 +
  arch/arm/mach-exynos/regs-pmu.h | 9 -
  2 files changed, 1 insertion(+), 9 deletions(-)



Reviewed-by: Krzysztof Kozlowski 



Thanks for review.

Pankaj

Best regards,
Krzysztof




--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/7] ARM: EXYNOS: Move pmu specific headers under "linux/soc/samsung"

[Pankaj Dubey]

On Tuesday 03 November 2015 07:16 AM, Krzysztof Kozlowski wrote:

On 26.10.2015 21:55, Pankaj Dubey wrote:

Moving Exynos PMU specific header file into "include/linux/soc/samsung"
thus updated affected files under "mach-exynos" to use new location of
these header files.

Signed-off-by: Amit Daniel Kachhap 
Signed-off-by: Pankaj Dubey 
---
  arch/arm/mach-exynos/exynos.c | 2 +-
  arch/arm/mach-exynos/mcpm-exynos.c| 2 +-
  arch/arm/mach-exynos/platsmp.c| 2 +-
  arch/arm/mach-exynos/pm.c | 4 ++--
  arch/arm/mach-exynos/pmu.c| 6 +++---
  arch/arm/mach-exynos/suspend.c| 4 ++--
  {arch/arm/mach-exynos => include/linux/soc/samsung}/exynos-pmu.h  | 2 +-
  .../regs-pmu.h => include/linux/soc/samsung/exynos-regs-pmu.h | 8 
  8 files changed, 15 insertions(+), 15 deletions(-)
  rename {arch/arm/mach-exynos => include/linux/soc/samsung}/exynos-pmu.h (90%)
  rename arch/arm/mach-exynos/regs-pmu.h => 
include/linux/soc/samsung/exynos-regs-pmu.h (99%)
diff --git a/arch/arm/mach-exynos/exynos-pmu.h 
b/include/linux/soc/samsung/exynos-pmu.h
similarity index 90%
rename from arch/arm/mach-exynos/exynos-pmu.h
rename to include/linux/soc/samsung/exynos-pmu.h
index a2ab0d5..50dd0aa 100644
--- a/arch/arm/mach-exynos/exynos-pmu.h
+++ b/include/linux/soc/samsung/exynos-pmu.h
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd.


No changes in file, don't touch the copyright date.

Instead please update the header-inclusion guard:
__SOC_EXYNOS_PMU_H
or maybe even (because you added exynos-pmu.h in next patch):
__LINUX_SOC_EXYNOS_PMU_H



OK.




   *http://www.samsung.com
   *
   * Header for EXYNOS PMU Driver support
diff --git a/arch/arm/mach-exynos/regs-pmu.h 
b/include/linux/soc/samsung/exynos-regs-pmu.h
similarity index 99%
rename from arch/arm/mach-exynos/regs-pmu.h
rename to include/linux/soc/samsung/exynos-regs-pmu.h
index 5e4f4c2..3a5b7ff 100644
--- a/arch/arm/mach-exynos/regs-pmu.h
+++ b/include/linux/soc/samsung/exynos-regs-pmu.h
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2010-2012 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd.
   *http://www.samsung.com
   *
   * EXYNOS - Power management unit definition
@@ -9,8 +9,8 @@
   * published by the Free Software Foundation.
  */

-#ifndef __ASM_ARCH_REGS_PMU_H
-#define __ASM_ARCH_REGS_PMU_H __FILE__
+#ifndef __EXYNOS_REGS_PMU_H
+#define __EXYNOS_REGS_PMU_H __FILE__

  #define S5P_CENTRAL_SEQ_CONFIGURATION 0x0200

@@ -690,4 +690,4 @@
 | EXYNOS5420_KFC_USE_STANDBY_WFI2  \
 | EXYNOS5420_KFC_USE_STANDBY_WFI3)

-#endif /* __ASM_ARCH_REGS_PMU_H */
+#endif /* __EXYNOS_REGS_PMU_H */



ditto, copyright may stay but please add SOC prefix to define.



OK. Thanks for review.

Thanks,
Pankaj Dubey

Best regards,
Krzysztof



--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] arm64: dts: exynos7: Add pmic s2mps15 device tree node

[Alim Akhtar]

Hi Krzysztof

On 11/02/2015 07:22 PM, Krzysztof Kozlowski wrote:

2015-11-02 22:01 GMT+09:00 Alim Akhtar :


   arch/arm64/boot/dts/exynos/exynos7-espresso.dts |  349
+++
   1 file changed, 349 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
index 838a3626dac1..8ce04a0ec928 100644
--- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
+++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
@@ -53,6 +53,355 @@
  status = "okay";
   };

+_4 {
+   samsung,i2c-sda-delay = <100>;
+   samsung,i2c-max-bus-freq = <20>;
+   status = "okay";
+
+   s2mps15_pmic@66 {
+   compatible = "samsung,s2mps15-pmic";
+   reg = <0x66>;
+   interrupts = <2 0>;
+   interrupt-parent = <>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_irq>;
+   wakeup-source;
+
+   s2mps15_osc: clocks {
+   compatible = "samsung,s2mps13-clk";
+   #clock-cells = <1>;
+   clock-output-names = "s2mps13_ap", "s2mps13_cp",
+   "s2mps13_bt";
+   };



Don't you want to use one of these clocks for s3c-rtc ( node)?


yes, you are right, rtc on this board is currently broken, mainly because of
the introduction of rtc_src clock in the s3c-rtc driver.
That is on my do list next. will take a look.

Are you suggesting to remove this -clk node now and add along with rtc
changes? I feel this should go in along with this patch.


Just add it in consecutive patch in this series. You added here some
providers (clock and regulators) without consumers. This of course
looks good as a way of providing full description of the board but:
1. For regulators always on: may be meaningless for kernel. Kernel
does not use it. Existence of regulator subnode will fulfill driver's
needs for probe.
2. For clocks: actually will disable these clocks because of lack of
consumers... which is fine but probably not what you wanted.

The standard approach is to add such providers when they are needed -
there are some consumers using them.

OK. for now will keep the pmic clock added as clock will be in disabled 
state, so it wont harm.

- will keep system related regulator like supply to arm,mif,int etc ..
will remove supplies to other peripherals IPs. Hope thats fine.


+
+   regulators {
+   ldo1_reg: LDO1 {
+   regulator-name = "vdd_ldo1";
+   regulator-min-microvolt = <50>;
+   regulator-max-microvolt = <90>;
+   regulator-always-on;
+   regulator-enable-ramp-delay = <125>;
+   };


(...)


+
+   buck10_reg: BUCK10 {
+   regulator-name = "vdd_buck10";
+   regulator-min-microvolt = <100>;
+   regulator-max-microvolt = <300>;
+   regulator-boot-on;
+   regulator-always-on;
+   regulator-ramp-delay = <25000>;
+   regulator-enable-ramp-delay = <250>;
+   };



All of these ldo3 and bucks in vendor tree for Espresso board have
ramp delay of 12000. Also they don't have enable-ramp-delay set and
voltages sometimes differ. I don't have S2MPS15 datasheet so I don't
know what is the true value... I'll leave it up to you but it looks
suspicious.


These values generally comes from our board design team, so I cann't really
comment on that, it may vary from board revision etc.
I will check if we have any updated version of recommended value and update
accordingly.


Okay, just pointing the difference. I cannot verify them.




+   };
+   };
+};



What will be the benefit of defining all of these regulators if they
are always on and without consumers? No one will disable them, no one
will change the voltage. Please provide some consumers.


As many drivers are not yet enabled in arm64 defconfig, that is one of the
reason why we are not seeing many consumer for these nodes.


That is not a problem. Please send a patch changing the defconfig.
Usually defconfig (for armv7 this would be exynos and multi_v7) should
provide bootable and working environment for all of our supported
boards.


This is the ground work being done for enabling those. If you insist will
try to reduce what is being used now. Moreover this was used to verify
functionality of pmic driver as well.


Actually as a verification I think even adding simple node
"regulators" is sufficient - driver will add all regulators anyway.
However seeing all regulators described for particular board is
good... but lack of consumers is disturbing because 

Re: [PATCH v3 7/7] drivers: soc: Add support for Exynos PMU driver

[Pankaj Dubey]

Hi Krzysztof,

On Tuesday 03 November 2015 07:52 AM, Krzysztof Kozlowski wrote:

On 26.10.2015 21:55, Pankaj Dubey wrote:

This patch moves Exynos PMU driver implementation from "arm/mach-exynos"
to "drivers/soc/samsung". This driver is mainly used for setting misc
bits of register from PMU IP of Exynos SoC which will be required to
configure before Suspend/Resume. Currently all these settings are done
in "arch/arm/mach-exynos/pmu.c" but moving ahead for ARM64 based SoC
support, there is a need of this PMU driver in driver/* folder.

This driver uses existing DT binding information and there should
be no functionality change in the supported platforms.

Signed-off-by: Amit Daniel Kachhap 
Signed-off-by: Pankaj Dubey 
---
  arch/arm/mach-exynos/Kconfig   | 1 +
  arch/arm/mach-exynos/Makefile  | 4 +---
  drivers/soc/samsung/Kconfig| 4 
  drivers/soc/samsung/Makefile   | 4 
  arch/arm/mach-exynos/pmu.c => drivers/soc/samsung/exynos-pmu.c | 0
  {arch/arm/mach-exynos => drivers/soc/samsung}/exynos-pmu.h | 0
  {arch/arm/mach-exynos => drivers/soc/samsung}/exynos3250-pmu.c | 0
  {arch/arm/mach-exynos => drivers/soc/samsung}/exynos4-pmu.c| 0
  {arch/arm/mach-exynos => drivers/soc/samsung}/exynos5250-pmu.c | 0
  {arch/arm/mach-exynos => drivers/soc/samsung}/exynos5420-pmu.c | 0
  10 files changed, 10 insertions(+), 3 deletions(-)
  rename arch/arm/mach-exynos/pmu.c => drivers/soc/samsung/exynos-pmu.c (100%)
  rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos-pmu.h (100%)
  rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos3250-pmu.c (100%)
  rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos4-pmu.c (100%)
  rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos5250-pmu.c (100%)
  rename {arch/arm/mach-exynos => drivers/soc/samsung}/exynos5420-pmu.c (100%)





1. Please reorder the exynos_sys_powerdown_conf() to be after the
statics. I am thinking also about adding EXPORT_SYMBOL... but maybe this
would be over-thinking.



I could not understand your point of reordering, will you please explain 
this.



2. I think the proper location of everything is drivers/power/reset/.
Although I don't have strong opinion.



There has been discussion about the proper location for this driver, 
initial attempt was done in "drivers/mfd" folder but then we realized 
that this driver is not exactly fitting in MFD category.
There was suggestion from Catalin Marinas [1], [2] to move it to 
"drivers/power" or a more suitable place other than mfd. As I received 
comments from Bartlomiej [3] and other members also (sorry I could not 
produce all links as it was quite more than a year back), I feel driver 
is very much SoC specific and hence decided to move it here.


1: https://lkml.org/lkml/2014/4/28/879
2: 
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/252018.html
3: 
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-April/244690.html



3. Please cc linux-pm and arm-soc guys (Arnd, Olof, Kevin) on next
iteration.



Ok will keep them in CC in next revision.

Thanks,
Pankaj Dubey


Best regards,
Krzysztof


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 6/7] ARCH: EXYNOS: split up exynos5420 SoC specific PMU data

[Pankaj Dubey]

Hi Krzysztof,

On Tuesday 03 November 2015 07:40 AM, Krzysztof Kozlowski wrote:

On 26.10.2015 21:55, Pankaj Dubey wrote:

This patch splits up mach-exynos/pmu.c file, and moves exynos5420,
PMU configuration data and functions handing data into exynos5420
SoC specific PMU file mach-exynos/exynos5420-pmu.c.

Signed-off-by: Pankaj Dubey 
---
  arch/arm/mach-exynos/Makefile |   2 +-
  arch/arm/mach-exynos/exynos-pmu.h |   1 +
  arch/arm/mach-exynos/exynos5420-pmu.c | 280 ++
  arch/arm/mach-exynos/pmu.c| 263 ---
  4 files changed, 282 insertions(+), 264 deletions(-)
  create mode 100644 arch/arm/mach-exynos/exynos5420-pmu.c



This should be rebased on:
ARM: EXYNOS: Constify local exynos_pmu_data structure
https://lkml.org/lkml/2015/10/28/917

After merge window I can provide you a tag for that.



OK will do this. Or I will cherry pick this patch from patchwork, and 
will rebase next version on top of it.






diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index bfb23a5..2d58063 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -11,7 +11,7 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) += 
-I$(srctree)/$(src)/include -I$(srctree)

  obj-$(CONFIG_ARCH_EXYNOS) += exynos.o pmu.o exynos-smc.o firmware.o \
exynos3250-pmu.o exynos4-pmu.o \
-   exynos5250-pmu.o
+   exynos5250-pmu.o exynos5420-pmu.o

  obj-$(CONFIG_EXYNOS_CPU_SUSPEND) += pm.o sleep.o
  obj-$(CONFIG_PM_SLEEP)+= suspend.o
diff --git a/arch/arm/mach-exynos/exynos-pmu.h 
b/arch/arm/mach-exynos/exynos-pmu.h
index 98c6bf3..4d53b68 100644
--- a/arch/arm/mach-exynos/exynos-pmu.h
+++ b/arch/arm/mach-exynos/exynos-pmu.h
@@ -48,4 +48,5 @@ extern const struct exynos_pmu_data exynos4210_pmu_data;
  extern const struct exynos_pmu_data exynos4212_pmu_data;
  extern const struct exynos_pmu_data exynos4412_pmu_data;
  extern const struct exynos_pmu_data exynos5250_pmu_data;
+extern const struct exynos_pmu_data exynos5420_pmu_data;
  #endif /* __EXYNOSPMU_H */
diff --git a/arch/arm/mach-exynos/exynos5420-pmu.c 
b/arch/arm/mach-exynos/exynos5420-pmu.c
new file mode 100644
index 000..5810afe
--- /dev/null
+++ b/arch/arm/mach-exynos/exynos5420-pmu.c
@@ -0,0 +1,280 @@
+/*
+ * Copyright (c) 2011-2015 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com/
+ *
+ * EXYNOS5420 - CPU PMU (Power Management Unit) support
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+
+#include "exynos-pmu.h"
+
+static struct exynos_pmu_conf exynos5420_pmu_config[] = {
+   /* { .offset = offset, .val = { AFTR, LPA, SLEEP } */
+   { EXYNOS5_ARM_CORE0_SYS_PWR_REG,{ 0x0, 0x0, 0x0} },
+   { EXYNOS5_DIS_IRQ_ARM_CORE0_LOCAL_SYS_PWR_REG,  { 0x0, 0x0, 0x0} },
+   { EXYNOS5_DIS_IRQ_ARM_CORE0_CENTRAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5_ARM_CORE1_SYS_PWR_REG,{ 0x0, 0x0, 0x0} },
+   { EXYNOS5_DIS_IRQ_ARM_CORE1_LOCAL_SYS_PWR_REG,  { 0x0, 0x0, 0x0} },
+   { EXYNOS5_DIS_IRQ_ARM_CORE1_CENTRAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_ARM_CORE2_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_DIS_IRQ_ARM_CORE2_LOCAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_DIS_IRQ_ARM_CORE2_CENTRAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_ARM_CORE3_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_DIS_IRQ_ARM_CORE3_LOCAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_DIS_IRQ_ARM_CORE3_CENTRAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_KFC_CORE0_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_DIS_IRQ_KFC_CORE0_LOCAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_DIS_IRQ_KFC_CORE0_CENTRAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_KFC_CORE1_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_DIS_IRQ_KFC_CORE1_LOCAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_DIS_IRQ_KFC_CORE1_CENTRAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_KFC_CORE2_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_DIS_IRQ_KFC_CORE2_LOCAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_DIS_IRQ_KFC_CORE2_CENTRAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_KFC_CORE3_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_DIS_IRQ_KFC_CORE3_LOCAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5420_DIS_IRQ_KFC_CORE3_CENTRAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
+   { EXYNOS5_ISP_ARM_SYS_PWR_REG,  { 0x1, 0x0, 0x0} },
+   { EXYNOS5_DIS_IRQ_ISP_ARM_LOCAL_SYS_PWR_REG,{ 

Re: [PATCH v3 3/7] ARCH: EXYNOS: split up exynos3250 SoC specific PMU data

[Pankaj Dubey]

Hi Krzysztof,

On Tuesday 03 November 2015 07:25 AM, Krzysztof Kozlowski wrote:

On 26.10.2015 21:55, Pankaj Dubey wrote:

This patch splits up mach-exynos/pmu.c file, and moves exynos3250 PMU
configuration data and functions handing those data into exynos3250
SoC specific PMU file mach-exynos/exynos3250-pmu.c.

Signed-off-by: Pankaj Dubey 
---
  arch/arm/mach-exynos/Makefile |   2 +-
  arch/arm/mach-exynos/exynos-pmu.h |  47 +
  arch/arm/mach-exynos/exynos3250-pmu.c | 175 +++
  arch/arm/mach-exynos/pmu.c| 189 +-
  4 files changed, 224 insertions(+), 189 deletions(-)
  create mode 100644 arch/arm/mach-exynos/exynos-pmu.h
  create mode 100644 arch/arm/mach-exynos/exynos3250-pmu.c

diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index 2f30676..e869f86 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -9,7 +9,7 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) += 
-I$(srctree)/$(src)/include -I$(srctree)

  # Core

-obj-$(CONFIG_ARCH_EXYNOS)  += exynos.o pmu.o exynos-smc.o firmware.o
+obj-$(CONFIG_ARCH_EXYNOS)  += exynos.o pmu.o exynos-smc.o firmware.o 
exynos3250-pmu.o

  obj-$(CONFIG_EXYNOS_CPU_SUSPEND) += pm.o sleep.o
  obj-$(CONFIG_PM_SLEEP)+= suspend.o
diff --git a/arch/arm/mach-exynos/exynos-pmu.h 
b/arch/arm/mach-exynos/exynos-pmu.h
new file mode 100644
index 000..2da4964
--- /dev/null
+++ b/arch/arm/mach-exynos/exynos-pmu.h
@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2015 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * Header for EXYNOS PMU Driver support
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __EXYNOSPMU_H
+#define __EXYNOSPMU_H


__EXYNOS_PMU_H
(and rename it in last patch)



OK.


+
+#include 
+
+#define PMU_TABLE_END  (-1U)
+
+extern void __iomem *pmu_base_addr;
+
+struct exynos_pmu_conf {
+   unsigned int offset;
+   u8 val[NUM_SYS_POWERDOWN];
+};
+
+struct exynos_pmu_data {
+   const struct exynos_pmu_conf *pmu_config;
+   const struct exynos_pmu_conf *pmu_config_extra;
+
+   void (*pmu_init)(void);
+   void (*powerdown_conf)(enum sys_powerdown);
+   void (*powerdown_conf_extra)(enum sys_powerdown);
+};
+
+static inline void pmu_raw_writel(u32 val, u32 offset)
+{
+   writel_relaxed(val, pmu_base_addr + offset);
+}
+
+static inline u32 pmu_raw_readl(u32 offset)
+{
+   return readl_relaxed(pmu_base_addr + offset);
+}


These shouldn't be static inlines in header because you will duplicate
it in each compiled object. Leave optimizations to compiler.



OK. Thanks for review.

Thanks,
Pankaj Dubey

Rest looks good,
Krzysztof



--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/7] drm/exynos: add pm runtime support

[Inki Dae]

2015년 11월 04일 19:13에 Andrzej Hajda 이(가) 쓴 글:
> On 11/04/2015 08:56 AM, Inki Dae wrote:
>>
>> 2015년 11월 04일 16:24에 Andrzej Hajda 이(가) 쓴 글:
>>> On 11/03/2015 04:38 PM, Inki Dae wrote:
  
 2015-11-03 22:24 GMT+09:00 Andrzej Hajda >:
> Hi Inki,
>
> On 11/03/2015 11:47 AM, Inki Dae wrote:
>> This patch series adds pm runtime support for Exynos drm.
>>
>> Originally, this patch was posted by Gustavo but there was no any
>> answer about some comments. So I rebased this patch series on top of
>> exynos-drm-next, removed unnecessary patches and modified wrong macro.
> I have sent comment to original patchset[1], but for some strange reasons
> it went only to mailing lists.
> My concerns were as follows:
> - exynos_drm has already pm_runtime support via exynos_drm_drv pm ops,
>   why should we add per component support?
  
 For this, I think I already mentioned enough,
 http://www.spinics.net/lists/linux-samsung-soc/msg47695.html

 In brief, exynos_drm_drv doesn't control each power domain of each kms 
 device.
 It means that we cannot power off fully without pm runtime interface of 
 each
 kms device - just they can only control their clock not power domain. So
 question. How we could control power domain of each kms device without 
 runtime
 pm interface?
>>> Hmm, but to enable pm_runtime in components it is enough to use
>>> pm_runtime_enable/disable and pm_runtime_get/put(_sync), there is no need 
>>> to add
>>> pm callbacks.
>> That is this patch series does, and pm callback is implemented in 
>> exynos_drm_drv not in component drivers.
> 
> But this patchset adds runtime_pm ops to each component. Runtime_pm support 
> does
> not require
> implementing runtime_pm ops, they just increases complexity of the code, and I
> see no gain in splitting
> power off/on sequence between drm enable/disable callbacks and suspend/resume
> callbacks.

Seems reasonable but if there is no any implemention of runtime pm ops and
only runtime pm api is called in enable or disable callbacks of each compoment
driver, then we wouldn't know which component driver runtime pm request failed
at - we could check if the power domain of each compoment device is enabled
or disabled correctly by checking each compoment's runtime pm suspend/resume 
call.

So I think it'd be better to implement component's runtime pm ops to make sure
to check if the runtime pm call successed, and I think also to fulfill
the use of runtime pm api correctly.

Anyway, I'd like to open this patchset for more review for a while before going 
to -next.
Welcome to any opinions.

Thanks,
Inki Dae

> 
> Regards
> Andrzej
> 
>>
>>> In fact most of the components already supports runtime pm,  but quick grep
>>> shows that it is not implemented in:
>>> exynos_drm_dsi.c, exynos_dp_core.c, exynos_drm_mic.c.
>> exynos_dp_core already has runtime pm interface with this patch series. For 
>> dsi and mic, we need to add the runtime pm interfaces.
>>
>>> So I guess adding pm_runtime support by adding calls 
>>> pm_runtime_enable/disable
>>> and pm_runtime_get/put(_sync) in appropriate places should be enough.
>>>
> - component suspend sequence is non deterministic, but in case of
>   video pipelines, specification often requires fixed order,
 Can your show me an example? I think component suspend and resume are 
 invoked
 by only drm dpms interface, and the dpms interface has a fixed order - when
 dpms goes to off, encoder first, and when dpms goes to on, crtc first.
>>> Now as I understand you do not want to remove exynos_drm_drv pm callbacks so
>>> they will disable devices properly, after that clock disabling order should 
>>> not
>>> matter, so the whole point is not valid.
>> In this case, the dpms actually is invoked by SLEEP PM of Linux power 
>> management framework. DRM KMS has a interface to control power of kms 
>> device, which is really different from SLEEP PM.
>> So this request can be done by userspace in runtime - just Display power 
>> off/on.
>>
 My only concern is that runtime pm interface of each kms driver is called
 within pm sleep context of Exynos drm driver. So I will look into this no 
 problem.

> - the patchset adds implicit dependency on PM_SLEEP.
>
>
> Current solution should work correctly and it was OK last time I have 
> tested it.
> I am not sure about atomic requirements, are there special ones?
 Not related to atomic feature.

> There are other issues with current solution, rather easy to solve:
> - it assumes that exynos-drm device will be suspended first - it should 
> be true,
>  as it is created at the end and suspend order is reverse to creation 
> order, but
>  I am not sure if we can rely on it - some solution is to add pm 
> callbacks to
>  all components, and from 

Re: [PATCH v2] mfd: sec-core: Rename MFD and regulator names differently

[Mark Brown]

On Mon, Nov 02, 2015 at 09:36:09AM +0530, Alim Akhtar wrote:
> Currently S2MPSXX multifunction device is named as *-pmic,
> and these MFDs also supports regulator as a one of its MFD cell which
> has the same name, because current name is confusing and we want to
> sort it out.

Acked-by: Mark Brown 


signature.asc
Description: PGP signature

Re: [PATCH v5 1/4] Documentation: dt-bindings: Describe SROMc configuration

[Krzysztof Kozlowski]

On 03.11.2015 18:16, Pavel Fedin wrote:
> Add documentation for new subnode properties, allowing bank configuration.
> Based on u-boot implementation, but heavily reworked.

Please mention also fixing the size of mapped memory in  in the
example.

> 
> Signed-off-by: Pavel Fedin 
> ---
>  .../bindings/arm/samsung/exynos-srom.txt   | 68 
> +-
>  1 file changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt 
> b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt
> index 33886d5..ee378ca 100644
> --- a/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt
> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-srom.txt
> @@ -5,8 +5,72 @@ Required properties:
>  
>  - reg: offset and length of the register set
>  
> -Example:
> +- #address-cells: Must be set to 2 to allow memory address translation
> +
> +- #size-cells: Must be set to 1 to allow CS address passing
> +
> +- ranges: Must be set up to reflect the memory layout with four integer 
> values
> +   per bank:
> +  0  
> +

These are mentioned as required but actually they shouldn't be, if there
are no peripherals.

By making them required you would be breaking ABI. I see that actually
the ABI is not broken by the driver so maybe just add a separate
optional section for subdevices.
In that section you would described required attributes and subnodes.

> +Sub-nodes:
> +The SROM controller can be used to attach external peripherials. In this case

s/peripherials/peripherals/

> +device nodes should be added as subnodes to the SROMc node. These subnodes,
> +except regular device specification, should contain the following properties,
> +describing configuration of the relevant SROM bank:
> +
> +Required properties:
> +- reg: bank number, base address (relative to start of the bank) and size of
> +   the memory mapped for the device. Note that base address will be
> +   typically 0 as this is the start of the bank.
> +
> +- samsung,srom-timing : array of 6 integers, specifying bank timings in the
> +following order: Tacp, Tcah, Tcoh, Tacc, Tcos, Tacs.
> +Each value is specified in cycles (0 - 15) and has 
> the
> +following meaning:
> +Tacp : Page mode access cycle at Page mode
> +Tcah : Address holding time after CSn
> +Tcoh : Chip selection hold on OEn
> +Tacc : Access cycle

This one has values 1-32 clock cycles

> +Tcos : Chip selection set-up before OEn
> +Tacs : Address set-up before CSn
> +
> +Optional properties:
> +- reg-io-width : data width in bytes (1 or 2). If omitted, default of 1 is 
> used.
> +
> +- samsung,srom-page-mode : page mode configuration for the bank:
> +0 - normal (one data)
> +1 - four data
> +If omitted, default of 0 is used.
> +
> +Example: basic definition, no banks are configured
> + sromc@1257 {
> + compatible = "samsung,exynos-srom";
> + reg = <0x1257 0x14>;
> + };
> +
> +Example: SROMc with smsc 911x ethernet chip on bank 3

Maybe:
s/smsc 911x ethernet/SMSC911x Ethernet/

Best regards,
Krzysztof

>   sromc@1257 {
> + #address-cells = <2>;
> + #size-cells = <1>;
> + ranges = <0 0 0x0400 0x2   // Bank0
> +   1 0 0x0500 0x2   // Bank1
> +   2 0 0x0600 0x2   // Bank2
> +   3 0 0x0700 0x2>; // Bank3
> +
>   compatible = "samsung,exynos-srom";
> - reg = <0x1257 0x10>;
> + reg = <0x1257 0x14>;
> +
> + ethernet@3 {
> + compatible = "smsc,lan9115";
> + reg = <3 0 0x1>;   // Bank 3, offset = 0
> + phy-mode = "mii";
> + interrupt-parent = <>;
> + interrupts = <5 8>;
> + reg-io-width = <2>;
> + smsc,irq-push-pull;
> + smsc,force-internal-phy;
> +
> + samsung,srom-config = <1 9 12 1 9 1 1>;
> + };
>   };
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 4/4] ARM: dts: Add Ethernet chip to SMDK5410

[Krzysztof Kozlowski]

On 03.11.2015 18:16, Pavel Fedin wrote:
> The chip is smsc9115, connected via SROMc bank 3. Additionally, some GPIO
> initialization is required.
> 
> Signed-off-by: Pavel Fedin 
> ---
>  arch/arm/boot/dts/exynos5410-smdk5410.dts | 40 
> +++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos5410-smdk5410.dts 
> b/arch/arm/boot/dts/exynos5410-smdk5410.dts
> index cebeaab..f41952f 100644
> --- a/arch/arm/boot/dts/exynos5410-smdk5410.dts
> +++ b/arch/arm/boot/dts/exynos5410-smdk5410.dts
> @@ -61,6 +61,27 @@
>   disable-wp;
>  };
>  
> +_0 {
> + srom_ctl: srom-ctl {
> + samsung,pins = "gpy0-3", "gpy0-4", "gpy0-5",
> +"gpy1-0", "gpy1-1", "gpy1-2", "gpy1-3";
> + samsung,pin-function = <2>;
> + samsung,pin-drv = <0>;
> + };
> +
> + srom_ebi: srom-ebi {
> + samsung,pins = "gpy3-0", "gpy3-1", "gpy3-2", "gpy3-3",
> +"gpy3-4", "gpy3-5", "gpy3-6", "gpy3-7",
> +"gpy5-0", "gpy5-1", "gpy5-2", "gpy5-3",
> +"gpy5-4", "gpy5-5", "gpy5-6", "gpy5-7",
> +"gpy6-0", "gpy6-1", "gpy6-2", "gpy6-3",
> +"gpy6-4", "gpy6-5", "gpy6-6", "gpy6-7";
> + samsung,pin-function = <2>;
> + samsung,pin-pud = <3>;
> + samsung,pin-drv = <0>;
> + };
> +};
> +
>   {
>   status = "okay";
>  };
> @@ -72,3 +93,22 @@
>   {
>   status = "okay";
>  };
> +
> + {

Put sromc in alphabetical order.

> + pinctrl-names = "default";
> + pinctrl-0 = <_ctl>, <_ebi>;
> +
> + ethernet@3 {
> + compatible = "smsc,lan9115";
> + reg = <3 0 0x1>;
> + phy-mode = "mii";
> + interrupt-parent = <>;
> + interrupts = <5 8>;

s/8/IRQ_TYPE_LEVEL_LOW/
(is this really level low interrupt?)

> + reg-io-width = <2>;
> + smsc,irq-push-pull;
> + smsc,force-internal-phy;
> +
> + samsung,srom-page-mode = <1>;
> + samsung,srom-timing = <9 12 1 9 1 1>;

Some other DTS include regulators: vddvario-supply and vdd33a-supply. It
seems that they are not described in SMSC911x bindings but in
GPMC-eth... but the smsc911x driver is requesting them. Could you
investigate that? I think these regulators should be provided (and
SMSC911x bindings should be updated).

> + };
> +};

I don't have the board schematics so I couldn't verify the GPIOs.
Overall looks good.

Best regards,
Krzysztof

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 3/4] drivers: exynos-srom: Add support for bank configuration

[Krzysztof Kozlowski]

On 03.11.2015 18:16, Pavel Fedin wrote:
> Implement handling properties in subnodes and adding child devices to the
> system. Child devices will not be added if configuration fails.
> 
> Since the driver now does more than suspend-resume support, dependency on
> CONFIG_PM is removed.
> 
> Signed-off-by: Pavel Fedin 
> ---
>  arch/arm/mach-exynos/Kconfig  |  2 +-
>  drivers/soc/samsung/Kconfig   |  2 +-
>  drivers/soc/samsung/exynos-srom.c | 60 
> +--
>  3 files changed, 60 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
> index 83c85f5..c22dc42 100644
> --- a/arch/arm/mach-exynos/Kconfig
> +++ b/arch/arm/mach-exynos/Kconfig
> @@ -16,7 +16,7 @@ menuconfig ARCH_EXYNOS
>   select ARM_GIC
>   select COMMON_CLK_SAMSUNG
>   select EXYNOS_THERMAL
> - select EXYNOS_SROM if PM
> + select EXYNOS_SROM
>   select HAVE_ARM_SCU if SMP
>   select HAVE_S3C2410_I2C if I2C
>   select HAVE_S3C2410_WATCHDOG if WATCHDOG
> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
> index 2833b5b..ea4bc2a 100644
> --- a/drivers/soc/samsung/Kconfig
> +++ b/drivers/soc/samsung/Kconfig
> @@ -8,6 +8,6 @@ config SOC_SAMSUNG
>  
>  config EXYNOS_SROM
>   bool
> - depends on ARM && ARCH_EXYNOS && PM
> + depends on ARM && ARCH_EXYNOS
>  
>  endmenu
> diff --git a/drivers/soc/samsung/exynos-srom.c 
> b/drivers/soc/samsung/exynos-srom.c
> index 57a232d..49b5c9e 100644
> --- a/drivers/soc/samsung/exynos-srom.c
> +++ b/drivers/soc/samsung/exynos-srom.c
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -67,11 +68,50 @@ static struct exynos_srom_reg_dump 
> *exynos_srom_alloc_reg_dump(
>   return rd;
>  }
>  
> +static int decode_sromc(struct exynos_srom *srom, struct device_node *np)

I missed that one previously: add prefix and more descriptive name, like:
exynos_srom_parse_child()

> +{
> + u32 bank, width, pmc;
> + u32 timing[6];
> + u32 cs, bw;
> +
> + if (of_property_read_u32(np, "reg", ))
> + return -EINVAL;
> + if (of_property_read_u32(np, "reg-io-width", ))
> + width = 1;
> + if (of_property_read_u32(np, "samsung,srom-page-mode", ))
> + pmc = 0;
> + if (of_property_read_u32_array(np, "samsung,srom-timing", timing,
> +ARRAY_SIZE(timing)))
> + return -EINVAL;
> +
> + bank *= 4; /* Convert bank into shift/offset */
> +
> + cs = 1 << EXYNOS_SROM_BW__BYTEENABLE__SHIFT;
> + if (width == 2)
> + cs |= 1 << EXYNOS_SROM_BW__DATAWIDTH__SHIFT;
> +
> + bw = __raw_readl(srom->reg_base + EXYNOS_SROM_BW);
> + bw = (bw & ~(EXYNOS_SROM_BW__CS_MASK << bank)) | (cs << bank);
> + __raw_writel(bw, srom->reg_base + EXYNOS_SROM_BW);
> +
> + __raw_writel((pmc << EXYNOS_SROM_BCX__PMC__SHIFT) |
> + (timing[0] << EXYNOS_SROM_BCX__TACP__SHIFT) |
> + (timing[1] << EXYNOS_SROM_BCX__TCAH__SHIFT) |
> + (timing[2] << EXYNOS_SROM_BCX__TCOH__SHIFT) |
> + (timing[3] << EXYNOS_SROM_BCX__TACC__SHIFT) |
> + (timing[4] << EXYNOS_SROM_BCX__TCOS__SHIFT) |
> + (timing[5] << EXYNOS_SROM_BCX__TACS__SHIFT),
> + srom->reg_base + EXYNOS_SROM_BC0 + bank);
> +
> + return 0;
> +}
> +
>  static int exynos_srom_probe(struct platform_device *pdev)
>  {
> - struct device_node *np;
> + struct device_node *np, *child;
>   struct exynos_srom *srom;
>   struct device *dev = >dev;
> + bool error = false;

The 'error' name is misleading - like error for entire probe which is
not true.

Instead split it to separate function like:

+static int exynos_srom_parse_children() {
+   int ret = 0;
+
+   for_each_child_of_node(np, child) {
+   ret = exynos_srom_parse_child(srom, child);
+   if (ret) {
+   dev_err(dev,
+   "Could not decode bank configuration for %s: 
%d\n",
+   child->name, ret);
+   break;
+   }
+   }
+
+   return ret;
+}

Best regards,
Krzysztof

>  
>   np = dev->of_node;
>   if (!np) {
> @@ -100,7 +140,23 @@ static int exynos_srom_probe(struct platform_device 
> *pdev)
>   return -ENOMEM;
>   }
>  
> - return 0;
> + for_each_child_of_node(np, child) {
> + if (decode_sromc(srom, child)) {
> + dev_err(dev,
> + "Could not decode bank configuration for %s\n",
> + child->name);
> + error = true;
> + }
> + }
> +
> + /*
> +  * If any bank failed to configure, we still provide suspend/resume,
> +  * but do not probe child devices
> +  */
> + if 

Re: [PATCH v5 2/4] ARM: dts: Add SROMc to Exynos 5410

[Krzysztof Kozlowski]

On 03.11.2015 18:16, Pavel Fedin wrote:
> This machine uses own SoC device tree file, add missing part.
> We insert the complete description, with ranges, because we are going to
> connect devices to it. Values in ranges are SoC-specific, so they go here
> in order not to duplicate them for every machine.
> 
> Signed-off-by: Pavel Fedin 
> ---
>  arch/arm/boot/dts/exynos5410.dtsi | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos5410.dtsi 
> b/arch/arm/boot/dts/exynos5410.dtsi
> index 4603356..9cfb814 100644
> --- a/arch/arm/boot/dts/exynos5410.dtsi
> +++ b/arch/arm/boot/dts/exynos5410.dtsi
> @@ -101,6 +101,17 @@
>   reg = <0x1000 0x100>;
>   };
>  
> + sromc: sromc@1225 {
> + compatible = "samsung,exynos-srom";
> + reg = <0x1225 0x14>;
> + #address-cells = <2>;
> + #size-cells = <1>;
> + ranges = <0 0 0x0400 0x2
> +   1 0 0x0500 0x2
> +   2 0 0x0600 0x2
> +   3 0 0x0700 0x2>;

Following my comments for bindings documentation - I think it is better
to add the address-cells, size-cells and ranges in 4th patch.

Because actually in this patch you are adding just basic support for
SROM controller: for saving and restoring registers. It could be merged
even without the rest of patchset.

Best regards,
Krzysztof

> + };
> +
>   pmu_system_controller: system-controller@1004 {
>   compatible = "samsung,exynos5410-pmu", "syscon";
>   reg = <0x1004 0x5000>;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/7] drm/exynos: add pm runtime support

[Andrzej Hajda]

On 11/04/2015 08:56 AM, Inki Dae wrote:
>
> 2015년 11월 04일 16:24에 Andrzej Hajda 이(가) 쓴 글:
>> On 11/03/2015 04:38 PM, Inki Dae wrote:
>>>  
>>> 2015-11-03 22:24 GMT+09:00 Andrzej Hajda >> >:
 Hi Inki,

 On 11/03/2015 11:47 AM, Inki Dae wrote:
> This patch series adds pm runtime support for Exynos drm.
>
> Originally, this patch was posted by Gustavo but there was no any
> answer about some comments. So I rebased this patch series on top of
> exynos-drm-next, removed unnecessary patches and modified wrong macro.
 I have sent comment to original patchset[1], but for some strange reasons
 it went only to mailing lists.
 My concerns were as follows:
 - exynos_drm has already pm_runtime support via exynos_drm_drv pm ops,
   why should we add per component support?
>>>  
>>> For this, I think I already mentioned enough,
>>> http://www.spinics.net/lists/linux-samsung-soc/msg47695.html
>>>
>>> In brief, exynos_drm_drv doesn't control each power domain of each kms 
>>> device.
>>> It means that we cannot power off fully without pm runtime interface of each
>>> kms device - just they can only control their clock not power domain. So
>>> question. How we could control power domain of each kms device without 
>>> runtime
>>> pm interface?
>> Hmm, but to enable pm_runtime in components it is enough to use
>> pm_runtime_enable/disable and pm_runtime_get/put(_sync), there is no need to 
>> add
>> pm callbacks.
> That is this patch series does, and pm callback is implemented in 
> exynos_drm_drv not in component drivers.

But this patchset adds runtime_pm ops to each component. Runtime_pm support does
not require
implementing runtime_pm ops, they just increases complexity of the code, and I
see no gain in splitting
power off/on sequence between drm enable/disable callbacks and suspend/resume
callbacks.

Regards
Andrzej

>
>> In fact most of the components already supports runtime pm,  but quick grep
>> shows that it is not implemented in:
>> exynos_drm_dsi.c, exynos_dp_core.c, exynos_drm_mic.c.
> exynos_dp_core already has runtime pm interface with this patch series. For 
> dsi and mic, we need to add the runtime pm interfaces.
>
>> So I guess adding pm_runtime support by adding calls 
>> pm_runtime_enable/disable
>> and pm_runtime_get/put(_sync) in appropriate places should be enough.
>>
 - component suspend sequence is non deterministic, but in case of
   video pipelines, specification often requires fixed order,
>>> Can your show me an example? I think component suspend and resume are 
>>> invoked
>>> by only drm dpms interface, and the dpms interface has a fixed order - when
>>> dpms goes to off, encoder first, and when dpms goes to on, crtc first.
>> Now as I understand you do not want to remove exynos_drm_drv pm callbacks so
>> they will disable devices properly, after that clock disabling order should 
>> not
>> matter, so the whole point is not valid.
> In this case, the dpms actually is invoked by SLEEP PM of Linux power 
> management framework. DRM KMS has a interface to control power of kms device, 
> which is really different from SLEEP PM.
> So this request can be done by userspace in runtime - just Display power 
> off/on.
>
>>> My only concern is that runtime pm interface of each kms driver is called
>>> within pm sleep context of Exynos drm driver. So I will look into this no 
>>> problem.
>>>
 - the patchset adds implicit dependency on PM_SLEEP.


 Current solution should work correctly and it was OK last time I have 
 tested it.
 I am not sure about atomic requirements, are there special ones?
>>> Not related to atomic feature.
>>>
 There are other issues with current solution, rather easy to solve:
 - it assumes that exynos-drm device will be suspended first - it should be 
 true,
  as it is created at the end and suspend order is reverse to creation 
 order, but
  I am not sure if we can rely on it - some solution is to add pm callbacks 
 to
  all components, and from those callbacks call one centralized pm routine,
>>> You mean pm callbacks are pm sleep interface? And you mean let's add sleep 
>>> pm
>>> interface to each kms driver?  If so, I'm not agree with you because sleep 
>>> pm
>>> should be controlled by only drm top like single driver does.
>> Lets look at an example:
>> Assume we have present only fimd and dsi components.
>> In such case kernel creates two platform devices for fimd and dsi in early 
>> stage
>> of parsing device tree blob.
>> During exynos_drm_drv initialization exynos-drm platform device is created, 
>> the
>> important thing it is created after fimd and dsi.
>> Now when platform enters sleep mode suspend callbacks are called, the 
>> important
>> thing here is
>> in which order. If I remember correctly PM guarantees only that children 
>> will be
>> handled before parents.
>> Since we have no 

Re: [PATCH 4/5] thermal: exynos: Remove unneeded semicolon

[Lukasz Majewski]

Hi Krzysztof,

> Remove semicolons after switch statement.
> 
> Signed-off-by: Krzysztof Kozlowski 
> ---
>  drivers/thermal/samsung/exynos_tmu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> b/drivers/thermal/samsung/exynos_tmu.c index
> eac6aebf82f3..1af7ea8dda71 100644 ---
> a/drivers/thermal/samsung/exynos_tmu.c +++
> b/drivers/thermal/samsung/exynos_tmu.c @@ -548,7 +548,7 @@ static int
> exynos5433_tmu_initialize(struct platform_device *pdev) default:
>   pdata->cal_type = TYPE_ONE_POINT_TRIMMING;
>   break;
> - };
> + }
>  
>   dev_info(>dev, "Calibration type is %d-point
> calibration\n", cal_type ?  2 : 1);
> @@ -1356,7 +1356,7 @@ static int exynos_tmu_probe(struct
> platform_device *pdev) break;
>   default:
>   break;
> - };
> + }
>  
>   /*
>* data->tzd must be registered before calling
> exynos_tmu_initialize(),

Acked-by: Lukasz Majewski 
Tested-by: Lukasz Majewski 

-- 
Best regards,

Lukasz Majewski

Samsung R Institute Poland (SRPOL) | Linux Platform Group
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] thermal: exynos: Fix unbalanced regulator disable on probe failure

[Lukasz Majewski]

Hi Alim,

> Hello,
> 
> On Fri, Oct 9, 2015 at 4:28 PM, Krzysztof Kozlowski
>  wrote:
> > W dniu 09.10.2015 o 01:45, Alim Akhtar pisze:
> >> Hello,
> >>
> >> On Thu, Oct 8, 2015 at 11:04 AM, Krzysztof Kozlowski
> >>  wrote:
> >>> During probe if the regulator could not be enabled, the error
> >>> exit path would still disable it. This could lead to unbalanced
> >>> counter of regulator enable/disable.
> >>>
> >> Do you see a regulator unbalanced reported here during boot? You
> >> may want to add that to commit message.
> >
> > I did not see the warning/error message about unbalanced disable. It
> > would happen in certain condition only - no other enables of
> > regulator and count going below 0.
> >
> > I would have to simulate this error to get the warning message. I
> > don't think it is worth the effort.
> >
> Ok, looking at code, it does looks to be calling regulator disable in
> case regulator enable fails.
> Feel free to add
> Reviewed-by: Alim Akhtar 
> Thanks!!
> 
> > Best regards,
> > Krzysztof
> >
> >>
> >>> The patch moves code for getting and enabling the regulator from
> >>> exynos_map_dt_data() to probe function because it is really not a
> >>> part of getting Device Tree properties.
> >>>
> >>> Signed-off-by: Krzysztof Kozlowski 
> >>> Fixes: 5f09a5cbd14a ("thermal: exynos: Disable the regulator on
> >>> probe failure") Cc: 
> >>> ---
> >>>  drivers/thermal/samsung/exynos_tmu.c | 34
> >>> +- 1 file changed, 17
> >>> insertions(+), 17 deletions(-)
> >>>
> >>> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> >>> b/drivers/thermal/samsung/exynos_tmu.c index
> >>> 0bae8cc6c23a..23f4320f8ef7 100644 ---
> >>> a/drivers/thermal/samsung/exynos_tmu.c +++
> >>> b/drivers/thermal/samsung/exynos_tmu.c @@ -1168,27 +1168,10 @@
> >>> static int exynos_map_dt_data(struct platform_device *pdev)
> >>> struct exynos_tmu_data *data = platform_get_drvdata(pdev); struct
> >>> exynos_tmu_platform_data *pdata; struct resource res;
> >>> -   int ret;
> >>>
> >>> if (!data || !pdev->dev.of_node)
> >>> return -ENODEV;
> >>>
> >>> -   /*
> >>> -* Try enabling the regulator if found
> >>> -* TODO: Add regulator as an SOC feature, so that
> >>> regulator enable
> >>> -* is a compulsory call.
> >>> -*/
> >>> -   data->regulator = devm_regulator_get(>dev, "vtmu");
> >>> -   if (!IS_ERR(data->regulator)) {
> >>> -   ret = regulator_enable(data->regulator);
> >>> -   if (ret) {
> >>> -   dev_err(>dev, "failed to enable
> >>> vtmu\n");
> >>> -   return ret;
> >>> -   }
> >>> -   } else {
> >>> -   dev_info(>dev, "Regulator node (vtmu) not
> >>> found\n");
> >>> -   }
> >>> -
> >>> data->id = of_alias_get_id(pdev->dev.of_node, "tmuctrl");
> >>> if (data->id < 0)
> >>> data->id = 0;
> >>> @@ -1312,6 +1295,23 @@ static int exynos_tmu_probe(struct
> >>> platform_device *pdev) pr_err("thermal: tz: %p ERROR\n",
> >>> data->tzd); return PTR_ERR(data->tzd);
> >>> }
> >>> +
> >>> +   /*
> >>> +* Try enabling the regulator if found
> >>> +* TODO: Add regulator as an SOC feature, so that
> >>> regulator enable
> >>> +* is a compulsory call.
> >>> +*/
> >>> +   data->regulator = devm_regulator_get(>dev, "vtmu");
> >>> +   if (!IS_ERR(data->regulator)) {
> >>> +   ret = regulator_enable(data->regulator);
> >>> +   if (ret) {
> >>> +   dev_err(>dev, "failed to enable
> >>> vtmu\n");
> >>> +   return ret;
> >>> +   }
> >>> +   } else {
> >>> +   dev_info(>dev, "Regulator node (vtmu) not
> >>> found\n");
> >>> +   }
> >>> +
> >>> ret = exynos_map_dt_data(pdev);
> >>> if (ret)
> >>> goto err_sensor;
> >>> --
> >>> 1.9.1
> >>>
> >>>
> >>> ___
> >>> linux-arm-kernel mailing list
> >>> linux-arm-ker...@lists.infradead.org
> >>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >>
> >>
> >>
> >
> 
> 
> 

Acked-by: Lukasz Majewski 
Tested-by: Lukasz Majewski 

-- 
Best regards,

Lukasz Majewski

Samsung R Institute Poland (SRPOL) | Linux Platform Group
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] thermal: exynos: Directly return 0 instead of using local ret variable

[Lukasz Majewski]

Hi Alim,

> Hello,
> 
> On Thu, Oct 8, 2015 at 11:04 AM, Krzysztof Kozlowski
>  wrote:
> > The 'ret' variable in exynos5440_tmu_initialize() is initialized to
> > 0 and returned as is. Replace it with direct return statement. This
> > also fixes coccinelle warning:
> > drivers/thermal/samsung/exynos_tmu.c:611:5-8: Unneeded variable:
> > "ret". Return "0" on line 654
> >
> > Signed-off-by: Krzysztof Kozlowski 
> > ---
> Reviewed-by: Alim Akhtar 
> 
> >  drivers/thermal/samsung/exynos_tmu.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c
> > b/drivers/thermal/samsung/exynos_tmu.c index
> > 1af7ea8dda71..f340e6edcb49 100644 ---
> > a/drivers/thermal/samsung/exynos_tmu.c +++
> > b/drivers/thermal/samsung/exynos_tmu.c @@ -608,7 +608,7 @@ static
> > int exynos5440_tmu_initialize(struct platform_device *pdev) {
> > struct exynos_tmu_data *data = platform_get_drvdata(pdev);
> > unsigned int trim_info = 0, con, rising_threshold;
> > -   int ret = 0, threshold_code;
> > +   int threshold_code;
> > int crit_temp = 0;
> >
> > /*
> > @@ -651,7 +651,8 @@ static int exynos5440_tmu_initialize(struct
> > platform_device *pdev) /* Clear the PMIN in the common TMU register
> > */ if (!data->id)
> > writel(0, data->base_second + EXYNOS5440_TMU_PMIN);
> > -   return ret;
> > +
> > +   return 0;
> >  }
> >
> >  static int exynos7_tmu_initialize(struct platform_device *pdev)
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-samsung-soc" in the body of a message to
> > majord...@vger.kernel.org More majordomo info at
> > http://vger.kernel.org/majordomo-info.html
> 
> 
> 

Acked-by: Lukasz Majewski 
Tested-by: Lukasz Majewski 

Test HW: Odroid XU3 - Exynos5433

-- 
Best regards,

Lukasz Majewski

Samsung R Institute Poland (SRPOL) | Linux Platform Group
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] thermal: exynos: Use IS_ERR() because regulator cannot be NULL

[Lukasz Majewski]

Hi Alim,

> Hello,
> 
> On Thu, Oct 8, 2015 at 11:04 AM, Krzysztof Kozlowski
>  wrote:
> > The NULL check in probe's error path is not needed because in that
> > time the regulator cannot be NULL (regulator_get() returns valid
> > pointer or ERR_PTR).
> >
> > Signed-off-by: Krzysztof Kozlowski 
> > ---
> Reviewed-by: Alim Akhtar 
> 
> >  drivers/thermal/samsung/exynos_tmu.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c
> > b/drivers/thermal/samsung/exynos_tmu.c index
> > bc71a61f0c4a..eac6aebf82f3 100644 ---
> > a/drivers/thermal/samsung/exynos_tmu.c +++
> > b/drivers/thermal/samsung/exynos_tmu.c @@ -1396,7 +1396,7 @@
> > err_clk_sec: if (!IS_ERR(data->clk_sec))
> > clk_unprepare(data->clk_sec);
> >  err_sensor:
> > -   if (!IS_ERR_OR_NULL(data->regulator))
> > +   if (!IS_ERR(data->regulator))
> > regulator_disable(data->regulator);
> >
> > return ret;
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-samsung-soc" in the body of a message to
> > majord...@vger.kernel.org More majordomo info at
> > http://vger.kernel.org/majordomo-info.html
> 
> 
> 

Acked-by: Lukasz Majewski 
Tested-by: Lukasz Majewski 

-- 
Best regards,

Lukasz Majewski

Samsung R Institute Poland (SRPOL) | Linux Platform Group
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] thermal: exynos: Fix first temperature read after registering sensor

[Lukasz Majewski]

Hi Alim,

> Hello,
> 
> On Thu, Oct 8, 2015 at 11:04 AM, Krzysztof Kozlowski
>  wrote:
> > Thermal core could not read the temperature after registering the
> > thermal sensor with thermal_zone_of_sensor_register() because the
> > driver was not yet initialized.
> >
> > The call trace looked like:
> > exynos_tmu_probe()
> > thermal_zone_of_sensor_register()
> > of_thermal_set_mode()
> > thermal_zone_device_update()
> > exynos_get_temp()
> > if (!data->tmu_read) return -EINVAL;
> > exynos_map_dt_data()
> > data->tmu_read = ...
> >
> > This produced an error in dmesg:
> > thermal thermal_zone0: failed to read out thermal zone (-22)
> >
> > Register the thermal_zone_device later, after parsing Device Tree
> > and enabling necessary clocks, but before calling
> > exynos_tmu_initialize() which uses the registered
> > thermal_zone_device.
> >
> > Signed-off-by: Krzysztof Kozlowski 
> > Fixes: 3b6a1a805f34 ("thermal: samsung: core: Exynos TMU rework to
> > use device tree for configuration") ---
> Patch looks good to me.
> 
> Before this patch I was getting below on exynos5800 board:
> [2.648629] thermal thermal_zone0: failed to read out thermal zone
> (-22) [2.653906] 1006.tmu supply vtmu not found, using dummy
> regulator [2.660521] thermal thermal_zone1: failed to read out
> thermal zone (-22) [2.666985] 10064000.tmu supply vtmu not found,
> using dummy regulator [2.673658] thermal thermal_zone2: failed to
> read out thermal zone (-22) [2.680093] 10068000.tmu supply vtmu
> not found, using dummy regulator [2.686901] thermal
> thermal_zone3: failed to read out thermal zone (-22) [2.693187]
> 1006c000.tmu supply vtmu not found, using dummy regulator
> [2.699914] thermal thermal_zone4: failed to read out thermal zone
> (-22) [2.706332] 100a.tmu supply vtmu not found, using dummy
> regulator
> 
> 
> Reviewed-by: Alim Akhtar 
> 
> Tested on exynos5800 peach board, so
> Tested-by: Alim Akhtar 
> 
> 
> >  drivers/thermal/samsung/exynos_tmu.c | 27 +
> --
> >  1 file changed, 17 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/thermal/samsung/exynos_tmu.c
> > b/drivers/thermal/samsung/exynos_tmu.c index
> > 23f4320f8ef7..bc71a61f0c4a 100644 ---
> > a/drivers/thermal/samsung/exynos_tmu.c +++
> > b/drivers/thermal/samsung/exynos_tmu.c @@ -1289,13 +1289,6 @@
> > static int exynos_tmu_probe(struct platform_device *pdev)
> > platform_set_drvdata(pdev, data); mutex_init(>lock);
> >
> > -   data->tzd = thermal_zone_of_sensor_register(>dev, 0,
> > data,
> > -
> > _sensor_ops);
> > -   if (IS_ERR(data->tzd)) {
> > -   pr_err("thermal: tz: %p ERROR\n", data->tzd);
> > -   return PTR_ERR(data->tzd);
> > -   }
> > -
> > /*
> >  * Try enabling the regulator if found
> >  * TODO: Add regulator as an SOC feature, so that regulator
> > enable @@ -1365,21 +1358,36 @@ static int exynos_tmu_probe(struct
> > platform_device *pdev) break;
> > };
> >
> > +   /*
> > +* data->tzd must be registered before calling
> > exynos_tmu_initialize(),
> > +* requesting irq and calling exynos_tmu_control().
> > +*/
> > +   data->tzd = thermal_zone_of_sensor_register(>dev, 0,
> > data,
> > +
> > _sensor_ops);
> > +   if (IS_ERR(data->tzd)) {
> > +   ret = PTR_ERR(data->tzd);
> > +   dev_err(>dev, "Failed to register sensor:
> > %d\n", ret);
> > +   goto err_sclk;
> > +   }
> > +
> > ret = exynos_tmu_initialize(pdev);
> > if (ret) {
> > dev_err(>dev, "Failed to initialize TMU\n");
> > -   goto err_sclk;
> > +   goto err_thermal;
> > }
> >
> > ret = devm_request_irq(>dev, data->irq,
> > exynos_tmu_irq, IRQF_TRIGGER_RISING | IRQF_SHARED,
> > dev_name(>dev), data); if (ret) {
> > dev_err(>dev, "Failed to request irq: %d\n",
> > data->irq);
> > -   goto err_sclk;
> > +   goto err_thermal;
> > }
> >
> > exynos_tmu_control(pdev, true);
> > return 0;
> > +
> > +err_thermal:
> > +   thermal_zone_of_sensor_unregister(>dev, data->tzd);
> >  err_sclk:
> > clk_disable_unprepare(data->sclk);
> >  err_clk:
> > @@ -1390,7 +1398,6 @@ err_clk_sec:
> >  err_sensor:
> > if (!IS_ERR_OR_NULL(data->regulator))
> > regulator_disable(data->regulator);
> > -   thermal_zone_of_sensor_unregister(>dev, data->tzd);
> >
> > return ret;
> >  }
> > --
> > 1.9.1
> >
> >
> > ___
> > linux-arm-kernel mailing list
> > linux-arm-ker...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 
> 

Acked-by: