Re: [PATCH v2 3/6] clk: hi6220: Document devicetree bindings for hi6220 clock
Hello Arnd, 2015-04-14 21:20 GMT+08:00 Arnd Bergmann a...@arndb.de: On Tuesday 14 April 2015 20:37:11 Bintian wrote: It's really a smart fix! For the four system controllers, how about change the following strings: + - hisilicon,aoctrl + - hisilicon,sysctrl + - hisilicon,mediactrl + - hisilicon,pmctrl to + - hisilicon,hi6220-aoctrl + - hisilicon,hi6220-sysctrl + - hisilicon,hi6220-mediactrl + - hisilicon,hi6220-pmctrl Yes, that works. Thanks, will fix in next version. and I also use hisilicon,hi6220- for hi6220 clk driver directly ? Seems fine to me, though I'd have to look at the bigger picture to see any potential problems. At the moment, I don't see any. Sure, please also help to review the 6/6 patch of this patch set, many thanks! BR, Bintian Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe devicetree 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/3] arm64: dts: Add dts files for Hisilicon Hi6220 SoC
Hello Mark,
2015-02-06 16:42 GMT+08:00 Brent Wang wangbint...@gmail.com:
[...]
+ gic: interrupt-controller@f680 {
+ compatible = arm,gic-400, arm,cortex-a15-gic;
Surely there's no need for the arm,cortex-a15-gic fallback entry? What
am I missing?
Remove it in next version.
After remove arm,cortex-a15-gic, I get the following error during
kernel booting:
-
kvm [1]: Using HYP init bounce page @396d9000
kvm [1]: error: no compatible GIC node found
kvm [1]: error initializing Hyp mode: -19
-
Check code virt/kvm/arm/vgic.c, gicv2 only cortex-a15-gic and
gicv3 support kvm now,
so I think we should keep it, how about your idea?
Thanks,
Bintian
+ reg = 0x0 0xf6801000 0x0 0x1000, /* GICD */
This doesn't match the unit-address.
Do you mean change to 0x0 0xf6801000 0 0x1000 ?
+ 0x0 0xf6802000 0x0 0x2000, /* GICC */
+ 0x0 0xf6804000 0x0 0x2000, /* GICH */
+ 0x0 0xf6806000 0x0 0x2000; /* GICV */
I guess no-one's bothered to consider 64k pages?
Given GICH and GICV, I hope that this platform is booted at EL2?
Transfer from EL3 to EL1 directly, keep these two just for future use.
+ #interrupt-cells = 3;
+ #address-cells = 0;
+ interrupt-controller;
+ };
+
+
[...]
--
To unsubscribe from this list: send the line unsubscribe devicetree 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/3] arm64: dts: Add dts files for Hisilicon Hi6220 SoC
Hello Marc,
2015-04-12 18:57 GMT+08:00 Marc Zyngier marc.zyng...@arm.com:
On 2015-04-12 07:40, Brent Wang wrote:
Hello Mark,
2015-02-06 16:42 GMT+08:00 Brent Wang wangbint...@gmail.com:
[...]
+ gic: interrupt-controller@f680 {
+ compatible = arm,gic-400, arm,cortex-a15-gic;
Surely there's no need for the arm,cortex-a15-gic fallback entry? What
am I missing?
Remove it in next version.
After remove arm,cortex-a15-gic, I get the following error during
kernel booting:
-
kvm [1]: Using HYP init bounce page @396d9000
kvm [1]: error: no compatible GIC node found
kvm [1]: error initializing Hyp mode: -19
-
Check code virt/kvm/arm/vgic.c, gicv2 only cortex-a15-gic and
gicv3 support kvm now,
so I think we should keep it, how about your idea?
Please look at commit 0f37247574b3 that is queued for merge in 4.1.
It adds the required compatibility strings to KVM, so Mark is perfectly
right to ask you to drop this cortex-a15-gic from your DT.
Thanks for the information.
This DT won't be merged before 4.1 anyway, so there is no point trying
to make it work with older kernels.
Thanks,
M.
--
Fast, cheap, reliable. Pick two.
--
Best Regards,
Bintian
===
Don't be nervous, just be happy!
--
To unsubscribe from this list: send the line unsubscribe devicetree 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/3] arm64: dts: Add dts files for Hisilicon Hi6220 SoC
Hello Mark,
2015-02-10 21:37 GMT+08:00 Mark Rutland mark.rutl...@arm.com:
On Fri, Feb 06, 2015 at 03:37:52PM +, Brent Wang wrote:
Hello Mark,
2015-02-06 18:44 GMT+08:00 Mark Rutland mark.rutl...@arm.com:
On Fri, Feb 06, 2015 at 08:42:22AM +, Brent Wang wrote:
Hello Mark,
2015-02-06 3:30 GMT+08:00 Mark Rutland mark.rutl...@arm.com:
On Thu, Feb 05, 2015 at 09:24:37AM +, Bintian Wang wrote:
Add initial dtsi file to support Hisilicon Hi6220 SoC with
support of Octal core CPUs in two clusters and each cluster
has quard Cortex-A53.
We now use the spin-table method for SMP, and it will be
changed to PSCI later.
If that's the case, please don't place the enable-method and related
properties in this file. Get your bootloader to add the appropriate
properties for its boot protocol.
When is PSCI likely to appear?
PSCI is under development.
Sure. Do you have an estimate as to when it will appear?
Another team will do the job, I can not give my estimation now.
Ok.
What are you using for your PSCI implementation? The ARM Trusted
Firmware?
Yes, ATF.
How are you testing it?
I think cpu hotplug can test it.
Are we certain of the split between components the PSCI implementation
must touch and those the kernel must touch?
Also add dts file to support HiKey development board which
based on Hi6220 SoC and document the devicetree bindings.
These dts files will be changed later and more nodes will be
added to describe other devices.
How is this going to be changed other than the addition of nodes?
Will this DTB continue to work in future? Or do you intend to make
changes that will break support?
My original idea is: use spin-table for SMP now, when firmware is OK to
support PSCI, we submit another patch to replace the spin-table with PSCI.
For any users who have not updated their FW, this will break booting.
This is why I suggest having hte bootloader/FW fill this in as it should
know what enable-method the FW supports.
If DTB should continue to work in the future, we really need to remove
the spin-table
from current dts file, how about just enable one core now?
Which one do you favor or any other suggestion?
If spin-table is just for testing while you await PSCI, drop spin-table
from the dts for now.
So, just booting one core may be the right choice now, right?
Without an enable-method for secondary CPUs, that will be all that's
possible. If your FW/bootlaoder injects the appropriate enable-method,
then you could gain spin-table based SMP bringup while awaiting PSCI,
without there being a DTB compatibility issue.
For DTB compatibility issue, how about just describe one core in dts
file? when PSCI
is OK, we can add the CPU topology to the dts file again.
[...]
+ pm_ctrl: pm_ctrl {
+ compatible = hisilicon,pmctrl, syscon;
+ #address-cells = 1;
+ #size-cells = 1;
+ reg = 0x0 0xf7032000 0x0 0x1000;
+ ranges = 0 0x0 0xf7032000 0x1000;
+
+ clock_power: clock3@0 {
+ compatible =
hisilicon,hi6220-clock-power;
+ reg = 0 0x1000;
+ #clock-cells = 1;
+ };
+ };
I really doesn't see the point in having a sub-device that covers the
entirely of the register space of the containing node, and that being
the case I am extremely concerned that the containers are marked as
syscon compatible.
The SoC clocks are designed and placed under different system controllers,
so I define corresponding nodes under different controllers for clock
operation.
What I'm concerned wit hhere is that the pm_ctrl node and the clock3@0
sub-node have the _exact_ same register space.
Given this should mean that the clock3@0 node owns that register space,
having the container node export this as syscon does not make sense. And
the split between pm_ctrl and clock3@) doesn't seem to make sense given
they cover the same space.
I understand your worry and will find the max offset of those clocks
under this controller.
As I asked before, why is pm_ctrl marked as a syscon, and what's the
point of the separate sub-node?
There is no big difference between pm_ctrl and other controller, they
are all designed as
the base address to control some functions of other modules (certainly
include some clock gates).
Are they just different instances of the same IP block, or are there
fundamental differences between them?
You can understand it as a different instance of the same IP block,
there is no fundamental
differences between them.
Maybe only one node is enough, not one node plus one sub-node ?
At least in the case above, I cannot see a reason to have more than a
single node
Re: [PATCH 3/3] arm64: dts: Add dts files for Hisilicon Hi6220 SoC
Hello Mark,
2015-02-10 23:27 GMT+08:00 Mark Rutland mark.rutl...@arm.com:
[...]
+ pm_ctrl: pm_ctrl {
+ compatible = hisilicon,pmctrl, syscon;
+ #address-cells = 1;
+ #size-cells = 1;
+ reg = 0x0 0xf7032000 0x0 0x1000;
+ ranges = 0 0x0 0xf7032000 0x1000;
+
+ clock_power: clock3@0 {
+ compatible =
hisilicon,hi6220-clock-power;
+ reg = 0 0x1000;
+ #clock-cells = 1;
+ };
+ };
I really doesn't see the point in having a sub-device that covers the
entirely of the register space of the containing node, and that being
the case I am extremely concerned that the containers are marked as
syscon compatible.
The SoC clocks are designed and placed under different system
controllers,
so I define corresponding nodes under different controllers for clock
operation.
What I'm concerned wit hhere is that the pm_ctrl node and the clock3@0
sub-node have the _exact_ same register space.
Given this should mean that the clock3@0 node owns that register space,
having the container node export this as syscon does not make sense. And
the split between pm_ctrl and clock3@) doesn't seem to make sense given
they cover the same space.
I understand your worry and will find the max offset of those clocks
under this controller.
As I asked before, why is pm_ctrl marked as a syscon, and what's the
point of the separate sub-node?
There is no big difference between pm_ctrl and other controller, they
are all designed as
the base address to control some functions of other modules (certainly
include some clock gates).
Are they just different instances of the same IP block, or are there
fundamental differences between them?
You can understand it as a different instance of the same IP block,
there is no fundamental
differences between them.
Ok. If that's the case each should have the same compatible string.
Although they have same function, they control different domain, I
should use different compatible string to distinguish different
domains.
Thanks,
Bintian
Thanks,
Mark.
--
Best Regards,
Bintian
===
Don't be nervous, just be happy!
--
To unsubscribe from this list: send the line unsubscribe devicetree 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/3] arm64: dts: Add dts files for Hisilicon Hi6220 SoC
Hello Marc,
2015-02-06 17:07 GMT+08:00 Marc Zyngier marc.zyng...@arm.com:
On 06/02/15 08:42, Brent Wang wrote:
[...]
+ 0x0 0xf6802000 0x0 0x2000, /* GICC */
+ 0x0 0xf6804000 0x0 0x2000, /* GICH */
+ 0x0 0xf6806000 0x0 0x2000; /* GICV */
I guess no-one's bothered to consider 64k pages?
Given GICH and GICV, I hope that this platform is booted at EL2?
Transfer from EL3 to EL1 directly, keep these two just for future use.
That's a real shame, as it keeps users away from some key aspects of the
ARMv8 architecture.
+ #interrupt-cells = 3;
+ #address-cells = 0;
+ interrupt-controller;
And if you're keeping GICH/GICV, where is the maintenance interrupt?
+ };
+
+
+ timer {
+ compatible = arm,armv8-timer;
+ interrupt-parent = gic;
+ interrupts = 1 13 0xff08,
+ 1 14 0xff08,
+ 1 11 0xff08,
+ 1 10 0xff08;
+ clock-frequency = 120;
+ };
NAK. Fix your firmware to configure CNTFRQ, on all CPUs.
Fix in next version, maybe it will take some time to change firmware.
While you're at it, make sure CNTVOFF_EL2 is set to zero on all CPUs
before dropping to EL1. This tends to be overlooked.
Thank you for reminding me, I will keep that in mind.
Thanks,
Thanks,
M.
--
Jazz is not dead. It just smells funny...
--
Best Regards,
Bintian
===
Don't be nervous, just be happy!
--
To unsubscribe from this list: send the line unsubscribe devicetree 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/3] clk: hi6220: Clock driver support for Hisilicon hi6220 SoC
Hello Tyler, Thank you very much for helping test this patchset! 2015-02-07 2:10 GMT+08:00 Tyler Baker tyler.ba...@linaro.org: Hi Bintian, This patch applied to next-20150204 is producing build failures on various ARM defconfigs[1]. I received the following error in all cases: drivers/built-in.o: In function `hi6220_clk_register_divider': :(.init.text+0x1a84c): undefined reference to `hi6220_register_clkdiv' Makefile:925: recipe for target 'vmlinux' failed make: *** [vmlinux] Error 1 It's my fault, I just test on ARM64, The following patch can fix this error: = diff --git a/drivers/clk/hisilicon/Makefile b/drivers/clk/hisilicon/Makefile index bbf0539..48f0116 100644 --- a/drivers/clk/hisilicon/Makefile +++ b/drivers/clk/hisilicon/Makefile @@ -2,9 +2,9 @@ # Hisilicon Clock specific Makefile # -obj-y += clk.o clkgate-separated.o +obj-y += clk.o clkgate-separated.o clkdivider-hi6220.o obj-$(CONFIG_ARCH_HI3xxx) += clk-hi3620.o obj-$(CONFIG_ARCH_HIP04) += clk-hip04.o obj-$(CONFIG_ARCH_HIX5HD2) += clk-hix5hd2.o -obj-$(CONFIG_COMMON_CLK_HI6220)+= clkdivider-hi6220.o clk-hi6220.o +obj-$(CONFIG_COMMON_CLK_HI6220)+= clk-hi6220.o == I will fix this problem in next version. Thanks, Bintian On 5 February 2015 at 01:24, Bintian Wang bintian.w...@huawei.com wrote: Add clock drivers for hi6220 SoC, this driver controls the SoC registers to supply different clocks to different IPs in the SoC. We add one divider clock for hi6220 because the divider in hi6220 also has a mask bit but it doesnot obey the rule defined by flag CLK_DIVIDER_HIWORD_MASK, we can not get index of the mask bit by left shift fixed bits (e.g. 16 bits), so we add this divider clock to handle it. This patch also enables this clock driver for ARCH_HISI and document devicetree bindings. Signed-off-by: Bintian Wang bintian.w...@huawei.com Reviewed-by: Haojian Zhuang haojian.zhu...@linaro.org Reviewed-by: Zhangfei Gao zhangfei@linaro.org --- .../devicetree/bindings/clock/hi6220-clock.txt | 30 +++ arch/arm64/Kconfig |1 + drivers/clk/Kconfig|2 + drivers/clk/Makefile |4 +- drivers/clk/hisilicon/Kconfig |5 + drivers/clk/hisilicon/Makefile |1 + drivers/clk/hisilicon/clk-hi6220.c | 284 drivers/clk/hisilicon/clk.c| 29 ++ drivers/clk/hisilicon/clk.h| 17 ++ drivers/clk/hisilicon/clkdivider-hi6220.c | 273 +++ include/dt-bindings/clock/hi6220-clock.h | 172 11 files changed, 815 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/hi6220-clock.txt create mode 100644 drivers/clk/hisilicon/Kconfig create mode 100644 drivers/clk/hisilicon/clk-hi6220.c create mode 100644 drivers/clk/hisilicon/clkdivider-hi6220.c create mode 100644 include/dt-bindings/clock/hi6220-clock.h diff --git a/Documentation/devicetree/bindings/clock/hi6220-clock.txt b/Documentation/devicetree/bindings/clock/hi6220-clock.txt new file mode 100644 index 000..a3ddda1 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/hi6220-clock.txt @@ -0,0 +1,30 @@ +* Hisilicon Hi6220 Clock Controller + +The hi6220 clock controller generates and supplies clock to various +controllers within the hi6220 SoC. + +Required Properties: + +- compatible: should be one of the following: + - hisilicon,hi6220-clock-ao - controller for those clocks under SoC + power always on(AO) domain, it is the sub node of SoC power AO + controller in dts file. + - hisilicon,hi6220-clock-sys - controller for those clocks under SoC + system control domain, it is the sub node of SoC system controller + in dts file. + - hisilicon,hi6220-clock-media - controller for those clocks under + SoC media control domain, it is the sub node of SoC media controller + in dts file. + - hisilicon,hi6220-clock-power - controller for those clocks under + SoC power control domain, it is the sub node of SoC power controller + in dts file. + +- reg: physical base address of the controller and length of memory mapped + region. + +- #clock-cells: should be 1. + +Each clock is assigned an identifier and client nodes use this identifier +to specify the clock which they consume. + +All these identifier could be found in dt-bindings/clock/hi6220-clock.h. diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 78cd41c8..6efc3e3 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -175,6 +175,7 @@ config ARCH_XGENE config ARCH_HISI bool Hisilicon SoC Family + select COMMON_CLK_HI6220 help This enables support for Hisilicon ARMv8 Family of SoCs.
Re: [PATCH 3/3] arm64: dts: Add dts files for Hisilicon Hi6220 SoC
Hello Mark, 2015-02-06 3:30 GMT+08:00 Mark Rutland mark.rutl...@arm.com: On Thu, Feb 05, 2015 at 09:24:37AM +, Bintian Wang wrote: Add initial dtsi file to support Hisilicon Hi6220 SoC with support of Octal core CPUs in two clusters and each cluster has quard Cortex-A53. We now use the spin-table method for SMP, and it will be changed to PSCI later. If that's the case, please don't place the enable-method and related properties in this file. Get your bootloader to add the appropriate properties for its boot protocol. When is PSCI likely to appear? PSCI is under development. Are we certain of the split between components the PSCI implementation must touch and those the kernel must touch? Also add dts file to support HiKey development board which based on Hi6220 SoC and document the devicetree bindings. These dts files will be changed later and more nodes will be added to describe other devices. How is this going to be changed other than the addition of nodes? Will this DTB continue to work in future? Or do you intend to make changes that will break support? My original idea is: use spin-table for SMP now, when firmware is OK to support PSCI, we submit another patch to replace the spin-table with PSCI. If DTB should continue to work in the future, we really need to remove the spin-table from current dts file, how about just enable one core now? Which one do you favor or any other suggestion? Signed-off-by: Bintian Wang bintian.w...@huawei.com Reviewed-by: Haojian Zhuang haojian.zhu...@linaro.org Reviewed-by: Yiping Xu xuyip...@hisilicon.com --- .../bindings/arm/hisilicon/hisilicon.txt | 33 arch/arm64/boot/dts/Makefile |1 + arch/arm64/boot/dts/hisilicon/Makefile |5 + arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 31 +++ arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 204 5 files changed, 274 insertions(+) create mode 100644 arch/arm64/boot/dts/hisilicon/Makefile create mode 100644 arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts create mode 100644 arch/arm64/boot/dts/hisilicon/hi6220.dtsi diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt index f717c7b..5eb6b41 100644 --- a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt +++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt @@ -9,6 +9,9 @@ HiP04 D01 Board Required root node properties: - compatible = hisilicon,hip04-d01; +HiKey Board +Required root node properties: + - compatible = hisilicon,hi6220-hikey; Hisilicon system controller @@ -62,6 +65,36 @@ Example: }; --- +Hisilicon Power Always ON domain controller + +Required properties: +- compatible : hisilicon,aoctrl +- reg : Register address and size + +Some clock registers are defined in power always on system controller, +especially in Hi6220 SoC which is used for mobile platform. + +--- +Hisilicon Media domain controller + +Required properties: +- compatible : hisilicon,mediactrl +- reg : Register address and size + +Some clock registers of media module are defined in media system +controller, especially in Hi6220 SoC which is used for mobile platform. + +--- +Hisilicon Power Management domain controller + +Required properties: +- compatible : hisilicon,pmctrl +- reg : Register address and size + +Some clock registers and PMU registers are defined in power management +controller, especially in Hin6220 SoC which is used for mobile platform. + +--- Looking at the dts below, none of these binding docs are sufficient. Define _all_ the properties and what they mean. In fact, Hisilicon designs several system controllers for different function domains, we can control different functions(e.g. clk gate on or off /IP reset) based on the base address of controller + offset, I will give more description in next version. Please also split documentation into earlier patches. OK, separate document and code in next version. Fabric: Required Properties: diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile index c62b0f4..bffd6b7 100644 --- a/arch/arm64/boot/dts/Makefile +++ b/arch/arm64/boot/dts/Makefile @@ -2,5 +2,6 @@ dts-dirs += amd dts-dirs += apm dts-dirs += arm dts-dirs += cavium +dts-dirs += hisilicon subdir-y := $(dts-dirs) diff --git a/arch/arm64/boot/dts/hisilicon/Makefile b/arch/arm64/boot/dts/hisilicon/Makefile new file mode 100644 index 000..fa81a6e --- /dev/null +++ b/arch/arm64/boot/dts/hisilicon/Makefile @@ -0,0 +1,5 @@
Re: [PATCH 3/3] arm64: dts: Add dts files for Hisilicon Hi6220 SoC
Hello Mark,
2015-02-06 18:44 GMT+08:00 Mark Rutland mark.rutl...@arm.com:
On Fri, Feb 06, 2015 at 08:42:22AM +, Brent Wang wrote:
Hello Mark,
2015-02-06 3:30 GMT+08:00 Mark Rutland mark.rutl...@arm.com:
On Thu, Feb 05, 2015 at 09:24:37AM +, Bintian Wang wrote:
Add initial dtsi file to support Hisilicon Hi6220 SoC with
support of Octal core CPUs in two clusters and each cluster
has quard Cortex-A53.
We now use the spin-table method for SMP, and it will be
changed to PSCI later.
If that's the case, please don't place the enable-method and related
properties in this file. Get your bootloader to add the appropriate
properties for its boot protocol.
When is PSCI likely to appear?
PSCI is under development.
Sure. Do you have an estimate as to when it will appear?
Another team will do the job, I can not give my estimation now.
What are you using for your PSCI implementation? The ARM Trusted
Firmware?
Yes, ATF.
How are you testing it?
I think cpu hotplug can test it.
Are we certain of the split between components the PSCI implementation
must touch and those the kernel must touch?
Also add dts file to support HiKey development board which
based on Hi6220 SoC and document the devicetree bindings.
These dts files will be changed later and more nodes will be
added to describe other devices.
How is this going to be changed other than the addition of nodes?
Will this DTB continue to work in future? Or do you intend to make
changes that will break support?
My original idea is: use spin-table for SMP now, when firmware is OK to
support PSCI, we submit another patch to replace the spin-table with PSCI.
For any users who have not updated their FW, this will break booting.
This is why I suggest having hte bootloader/FW fill this in as it should
know what enable-method the FW supports.
If DTB should continue to work in the future, we really need to remove
the spin-table
from current dts file, how about just enable one core now?
Which one do you favor or any other suggestion?
If spin-table is just for testing while you await PSCI, drop spin-table
from the dts for now.
So, just booting one core may be the right choice now, right?
[...]
+ timer {
+ compatible = arm,armv8-timer;
+ interrupt-parent = gic;
+ interrupts = 1 13 0xff08,
+ 1 14 0xff08,
+ 1 11 0xff08,
+ 1 10 0xff08;
+ clock-frequency = 120;
+ };
NAK. Fix your firmware to configure CNTFRQ, on all CPUs.
Fix in next version, maybe it will take some time to change firmware.
Thanks. This _must_ be fixed.
[...]
+ pm_ctrl: pm_ctrl {
+ compatible = hisilicon,pmctrl, syscon;
+ #address-cells = 1;
+ #size-cells = 1;
+ reg = 0x0 0xf7032000 0x0 0x1000;
+ ranges = 0 0x0 0xf7032000 0x1000;
+
+ clock_power: clock3@0 {
+ compatible = hisilicon,hi6220-clock-power;
+ reg = 0 0x1000;
+ #clock-cells = 1;
+ };
+ };
I really doesn't see the point in having a sub-device that covers the
entirely of the register space of the containing node, and that being
the case I am extremely concerned that the containers are marked as
syscon compatible.
The SoC clocks are designed and placed under different system controllers,
so I define corresponding nodes under different controllers for clock
operation.
What I'm concerned wit hhere is that the pm_ctrl node and the clock3@0
sub-node have the _exact_ same register space.
Given this should mean that the clock3@0 node owns that register space,
having the container node export this as syscon does not make sense. And
the split between pm_ctrl and clock3@) doesn't seem to make sense given
they cover the same space.
I understand your worry and will find the max offset of those clocks
under this controller.
As I asked before, why is pm_ctrl marked as a syscon, and what's the
point of the separate sub-node?
There is no big difference between pm_ctrl and other controller, they
are all designed as
the base address to control some functions of other modules (certainly
include some clock gates).
Maybe only one node is enough, not one node plus one sub-node ?
Thanks,
Mark.
--
Best Regards,
Bintian
===
Don't be nervous, just be happy!
--
To unsubscribe from this list: send the line unsubscribe devicetree 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/3] clk: hi6220: Clock driver support for Hisilicon hi6220 SoC
Hi Mark, 2015-02-06 3:25 GMT+08:00 Mark Rutland mark.rutl...@arm.com: On Thu, Feb 05, 2015 at 09:24:36AM +, Bintian Wang wrote: Add clock drivers for hi6220 SoC, this driver controls the SoC registers to supply different clocks to different IPs in the SoC. We add one divider clock for hi6220 because the divider in hi6220 also has a mask bit but it doesnot obey the rule defined by flag CLK_DIVIDER_HIWORD_MASK, we can not get index of the mask bit by left shift fixed bits (e.g. 16 bits), so we add this divider clock to handle it. This patch also enables this clock driver for ARCH_HISI and document devicetree bindings. Signed-off-by: Bintian Wang bintian.w...@huawei.com Reviewed-by: Haojian Zhuang haojian.zhu...@linaro.org Reviewed-by: Zhangfei Gao zhangfei@linaro.org --- .../devicetree/bindings/clock/hi6220-clock.txt | 30 +++ arch/arm64/Kconfig |1 + drivers/clk/Kconfig|2 + drivers/clk/Makefile |4 +- drivers/clk/hisilicon/Kconfig |5 + drivers/clk/hisilicon/Makefile |1 + drivers/clk/hisilicon/clk-hi6220.c | 284 drivers/clk/hisilicon/clk.c| 29 ++ drivers/clk/hisilicon/clk.h| 17 ++ drivers/clk/hisilicon/clkdivider-hi6220.c | 273 +++ include/dt-bindings/clock/hi6220-clock.h | 172 11 files changed, 815 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/hi6220-clock.txt create mode 100644 drivers/clk/hisilicon/Kconfig create mode 100644 drivers/clk/hisilicon/clk-hi6220.c create mode 100644 drivers/clk/hisilicon/clkdivider-hi6220.c create mode 100644 include/dt-bindings/clock/hi6220-clock.h diff --git a/Documentation/devicetree/bindings/clock/hi6220-clock.txt b/Documentation/devicetree/bindings/clock/hi6220-clock.txt new file mode 100644 index 000..a3ddda1 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/hi6220-clock.txt @@ -0,0 +1,30 @@ +* Hisilicon Hi6220 Clock Controller + +The hi6220 clock controller generates and supplies clock to various +controllers within the hi6220 SoC. + +Required Properties: + +- compatible: should be one of the following: + - hisilicon,hi6220-clock-ao - controller for those clocks under SoC + power always on(AO) domain, it is the sub node of SoC power AO + controller in dts file. + - hisilicon,hi6220-clock-sys - controller for those clocks under SoC + system control domain, it is the sub node of SoC system controller + in dts file. + - hisilicon,hi6220-clock-media - controller for those clocks under + SoC media control domain, it is the sub node of SoC media controller + in dts file. + - hisilicon,hi6220-clock-power - controller for those clocks under + SoC power control domain, it is the sub node of SoC power controller + in dts file. These all refer to things which aren't documented (yet). Please sort out your patches so that any documentation you depend on comes earlier. Please also separate documentation from code. Note that dt includes are _bindings_ and should be added with the relevant documentation. Thanks for help to review, I also think separate documentation from code is better way, it will be fixed in next version. Thanks, Bintian Thanks, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] arm64,hi6220: Enable Hisilicon Hi6220 SoC
Hello Olof, 2015-02-06 14:18 GMT+08:00 Olof Johansson o...@lixom.net: On Thu, Feb 5, 2015 at 8:21 PM, Brent Wang wangbint...@gmail.com wrote: Hello Olof and Tyler, 2015-02-06 7:52 GMT+08:00 Tyler Baker tyler.ba...@linaro.org: On 5 February 2015 at 11:02, Olof Johansson o...@lixom.net wrote: On Thu, Feb 5, 2015 at 10:46 AM, Tyler Baker tyler.ba...@linaro.org wrote: Hi Bintian, On 5 February 2015 at 01:24, Bintian Wang bintian.w...@huawei.com wrote: Hello, Hi6220 is one mobile solution of Hisilicon, this patchset contains initial support for Hi6220 SoC and HiKey development board, which is based on ARM Cortex A53 architecture. Initial support is minimal and includes just the arch configuration, clock driver, device tree configuration. Many peripheral drivers will be submitted later. Any comments will be appreciated! Thanks, Bintian Wang (3): arm64: Enable Hisilicon ARMv8 SoC family in Kconfig and defconfig clk: hi6220: Clock driver support for Hisilicon hi6220 SoC arm64: dts: Add dts files for Hisilicon Hi6220 SoC Thanks for posting these! I've applied this series on top of next-20150204, however there was some fuzz that needed to be cleaned up on 3/3 [1]. I've confirmed the platform is booting to a basic user space without issue. From ramdisk only, right? Correct, ramdisk only. Given the timing of the posting of this patch set, I'm not going to merge it for 3.20. Hopefully for 3.21 there will be some more peripheral support as well -- at least some sort of storage device. Seem fair to me. I also hope to see more patches posted shortly. Yes, the mmc and sd drivers will be submitted soon, should they be included in this patchset? I have thought submitting this patch first for review, if there is no problem for this patchset and then submit other drivers, you know, other drivers will depend on this patchset. The drivers should ideally not depend on the SoC patchset -- the driver can go in independently. The DTS updates to specify the hardware should go in through arm-soc even if the driver itself (and the binding document update) should go in through the driver subsystem instead. So, you can choose if you want to post everything as a long series, and cc different maintainers on the various parts of the series -- or you can post each driver or subsystem as a patchset on its own and let that get merged through respective maintainer. The latter is most common these days. I think the latter is a better way, thank you! -Olof -- Best Regards, Bintian === Don't be nervous, just be happy! -- To unsubscribe from this list: send the line unsubscribe devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
