Hi Tomasz,
On 12/07/2013 01:57 AM, Tomasz Figa wrote:
On Friday 06 of December 2013 18:22:32 Sylwester Nawrocki wrote:
[...]
I wouldn't really nitpick on such things, but if we end up needing another
respin, here's what I think.
Yeah, I didn't really realize the iteration index for this series was
already
a 2 digit number and some people could have already gotten upset or
seriously
impatient. Started to regret I even bothered with posting any comments
to that.
:) Anyway, I didn't request any corrections, just pointed out what could be
improved. People are free to accept it or ignore as they see fit. And having
seen patch series that got merged after periods like 2.5 year V12 is not
something unusual anyway. :)
On 12/06/2013 06:47 AM, Leela Krishna Amudala wrote:
This patch adds pmusysreg node to exynos5250 and exynos5420 dtsi files to
s/pmusysreg/PMU sysreg ? Similarly I would capitalize it in the subject
line as well.
Well, since this is supposed to be a human readable description, I would
go even further and write "...device tree node of Power Management Unit
to...".
Agreed, it sounds better.
handle PMU register accesses in a centralized way using syscon driver
Signed-off-by: Leela Krishna Amudala<l.krishna@xxxxxxxxxxx>
Reviewed-by: Tomasz Figa<t.figa@xxxxxxxxxxx>
Reviewed-by: Doug Anderson<dianders@xxxxxxxxxxxx>
Tested-by: Doug Anderson<dianders@xxxxxxxxxxxx>
---
Documentation/devicetree/bindings/arm/samsung/pmu.txt | 15 +++++++++++++++
arch/arm/boot/dts/exynos5250.dtsi | 5 +++++
arch/arm/boot/dts/exynos5420.dtsi | 5 +++++
3 files changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/samsung/pmu.txt
diff --git a/Documentation/devicetree/bindings/arm/samsung/pmu.txt b/Documentation/devicetree/bindings/arm/samsung/pmu.txt
new file mode 100644
index 0000000..f1f1552
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/samsung/pmu.txt
I might be easy to confuse this with ARM Performance Monitoring Unit.
So perhaps we should rename this file to, e.g. power-management-unit.txt ?
Considering location of this file, which is arm/samsung, I think this
name is pretty much clear. ARM PMU is a generic thing, so it couldn't
be placed here.
It's up to the patch author if they want to change it or not, I still think
it's better to not use short acronyms like this, if we can easily make it
unambiguous.
@@ -0,0 +1,15 @@
+SAMSUNG Exynos SoC series PMU Registers
s/PMU/Power Management Unit ?
s/PMU Registers/Power Management Unit/
Yeah, that's more accurate.
+Properties:
+ - compatible : should contain two values. First value must be one from following list:
+ - "samsung,exynos5250-pmu" - for Exynos5250 SoC,
+ - "samsung,exynos5420-pmu" - for Exynos5420 SoC.
s/./; ?
+ second value must be always "syscon".
It might be more safe to specify it as the last value, so something along
the lines of:
The last value should be "syscon".
+
+ - reg : offset and length of the register set.
+
+Example :
+pmu_system_controller: system-controller@10040000 {
Might be more sensible to use 'power_management_unit' for the label.
That's quite a lot of text for a label. pmu_syscon as in previous version
of this patch would look more sensible to me.
I don't think it hurts to have this long labels, what I proposed is exactly
of same length as the original one. Anyway pmu_syscon sounds good to me too.
+ compatible = "samsung,exynos5250-pmu", "syscon";
+ reg =<0x10040000 0x5000>;
+};
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index b98ffc3..62f9e36 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -163,6 +163,11 @@
interrupts =<0 47 0>;
};
+ pmu_system_controller: system-controller@10040000 {
s/pmu_system_controller/power_management_unit ? So it describes the
subsystem better in terms used in the SoCs User Manual ?
See above.
I can't see anything wrong in such long labels, but this discussion is
really
just a bike-shedding, so let's end it right now not to risk annoying
even more
people! :)
--
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
