Re: [RFC PATCH 0/3] clocksource: exynos_mct: allow mct to use 64-bit counter from coprocessor
On Mon, Jul 27, 2015 at 10:28:27PM +0100, Alexey Klimov wrote: Hi all, year(s) ago it was discovered that MCT timer and ARM architectured timer are the same hardware with different interface. Here [1]. Are they actually interfaces to the same timer, or are they just fed by the same system counter? I followed mail-list discussions about removing MCT and using arch timer for Exynos5-based SoCs but things aren't moving at least latest upstream kernel on odroid-xu3 will use MCT as default timers. Maybe the reason are some power-management related things that very specific to Samsung. I don't know. Idea of this draft patchset comes from Doug patches when he tried to optimize read of 64-bit counter located in mmio. [2] Why not using cp15 counter instead if possible? If they're the same device, then you can simply use the architected timer, and gain all the benefits without additional code. That could give you additional benefits (e.g. VDSO support). Previous numbers for 100 gettimeofday() calls from userspace are about 1 ms. With this patches we have 0.5 ms or even better. So twice as fast. Just as matter of interest i tried to perform 200 sched_yield() calls from userspace. I see around 20% speedup with patches applied. I tried to use hackbench but it's hard to feel the difference, maybe speedup is 0.5% but with bad fluctuation. Everything is tested on odroid-xu3. Looks like Cortex-A9 based Samsung SoCs (odroid-u2, odroid-x) don't have arch timer. [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014 -May/256943.html [2] https://lkml.org/lkml/2014/6/20/431 Current code created with such assumptions: -- don't want to insert huge refactoring in MCT code; -- target platform only ARMv7 Exynos5-based SoC that works in 32-bit mode so i don't want to build described functionality on ARM64. For arm64 the generic timer is mandatory, and I can't imagine a case where we wouldn't use it in preference. Currently i'm trying to patch odroid-xu3 DT only. -- firmware on odroid-xu3 is broken and secondary cores start in SVC mode instead of HYP mode. Maybe i can remove check for hyp mode; -- in addition instead of DT property I may parse PFR regs and find Generic Timer Extension there. ID_PFR1.GenTimer only tells you whether or not the generic timer is implemented in the CPU, not how it's wired up (e.g. whether it shares the system counter with the MCT), so I don't think it tells you anything useful. Thanks, Mark. -- 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 v7 1/9] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
Hi Chanwoo, Could you please reply to the below? Without an answer I'm going to have to ask for the patch to be unqueued for the moment, and I'd prefer that we came to a solution instead. Thanks, Mark. On Tue, Apr 07, 2015 at 11:25:27AM +0100, Mark Rutland wrote: I'm very worried about adding a DT that's known broken, especially when we have no idea as to if/when the FW will be fixed judging from prior replies. As I replied, I can not fix the FW because I don't have any code of FW. Surely you are able to contact those who do? I don't have any solution to fix it on Linux kernel level. So, If you agree, I can add the comment of CPU0 hotplug issue on DT file. I disagree. I do not want to add a DT that is known to be broken; especially when we have no idea how to fix it. It creates long-term maintenance pain for everyone, and marginal gain for few. A comment does nothing to aid the end-user. So NAK to the PSCI node and PSCI enable method in this dts until we either have a working firmware, or a reasonable mechanism to handle the deficiency. There is only CPU0 hotplug issue. Excpet CPU{1-7} are well working. I understand that, but the issue with CPU0 is still a blocker from my PoV. To fix this issue, we must need the help of firmware developer. But, We never get the any help. Previously you said that you did not have access to the source code rather than not having help from the relevant firmware engineers. I take it you have informed them of the issue with CPU0? Also, as I mentioned on previous mail, all Exynos SoCs can not turn off the CPU0. I've never seen Exynos SoC that CP0 hotplug is possible. While that may be the case, PSCI is a more generic standard, and it is used on systems where CPU0 can be hot unplugged. So Exynos-specific details cannot dictate how the kernel PSCI driver should behave. Is there a particular reason that CPU0 cannot be hotplugged? In PSCI 0.2 and later it's possible to determine whether a trusted OS prohibits a core from being hotplugged, but this mechanism doesn't exist in earlier versions. I am not averse to adding a property to PSCI 0.1 to mark a CPU as not being hotpluggable if there is a fundamental reason (i.e. not simply a bug) for this being the case. Thanks, Mark. -- 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 -- 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 v7 1/9] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
So NAK to the PSCI node and PSCI enable method in this dts until we either have a working firmware, or a reasonable mechanism to handle the deficiency. There is only CPU0 hotplug issue. Excpet CPU{1-7} are well working. I understand that, but the issue with CPU0 is still a blocker from my PoV. To fix this issue, we must need the help of firmware developer. But, We never get the any help. Previously you said that you did not have access to the source code rather than not having help from the relevant firmware engineers. I take it you have informed them of the issue with CPU0? I didn't ask any help to firmware engineer because I didn't know who firmware engineer and also didn't access the source code. If I knew the engineer and can access them, I would have asked some help to them or inquired the reason about CPU0 not hotplugged. You must have acquired the firmware (or board(s) with the firmware preloaded) from somewhere. Surely you can work backwards from there to file a bug report and/or inquire as to who you need to speak to... Mark. -- 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 v7 1/9] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
I'm very worried about adding a DT that's known broken, especially when we have no idea as to if/when the FW will be fixed judging from prior replies. As I replied, I can not fix the FW because I don't have any code of FW. Surely you are able to contact those who do? I don't have any solution to fix it on Linux kernel level. So, If you agree, I can add the comment of CPU0 hotplug issue on DT file. I disagree. I do not want to add a DT that is known to be broken; especially when we have no idea how to fix it. It creates long-term maintenance pain for everyone, and marginal gain for few. A comment does nothing to aid the end-user. So NAK to the PSCI node and PSCI enable method in this dts until we either have a working firmware, or a reasonable mechanism to handle the deficiency. There is only CPU0 hotplug issue. Excpet CPU{1-7} are well working. I understand that, but the issue with CPU0 is still a blocker from my PoV. To fix this issue, we must need the help of firmware developer. But, We never get the any help. Previously you said that you did not have access to the source code rather than not having help from the relevant firmware engineers. I take it you have informed them of the issue with CPU0? Also, as I mentioned on previous mail, all Exynos SoCs can not turn off the CPU0. I've never seen Exynos SoC that CP0 hotplug is possible. While that may be the case, PSCI is a more generic standard, and it is used on systems where CPU0 can be hot unplugged. So Exynos-specific details cannot dictate how the kernel PSCI driver should behave. Is there a particular reason that CPU0 cannot be hotplugged? In PSCI 0.2 and later it's possible to determine whether a trusted OS prohibits a core from being hotplugged, but this mechanism doesn't exist in earlier versions. I am not averse to adding a property to PSCI 0.1 to mark a CPU as not being hotpluggable if there is a fundamental reason (i.e. not simply a bug) for this being the case. Thanks, Mark. -- 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 v7 1/9] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
On Tue, Mar 31, 2015 at 12:56:38AM +0100, Chanwoo Choi wrote: Hi Mark, On 03/31/2015 01:09 AM, Mark Rutland wrote: Hi, On Wed, Mar 18, 2015 at 12:17:28AM +, Chanwoo Choi wrote: This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53). And Exynos5433 supports PSCI (Power State Coordination Interface) v0.1. This patch includes following dt node to support Exynos5433 SoC: 1. Octa core for big.LITTLE architecture - Cortex-A53 LITTLE Quad-core - Cortex-A57 big Quad-core - Support PSCI v0.1 Is CPU0 hotplug still broken, or has the FW been fixed? CPU0 hotplug is still not working. I'm very worried about adding a DT that's known broken, especially when we have no idea as to if/when the FW will be fixed judging from prior replies. As I replied, I can not fix the FW because I don't have any code of FW. Surely you are able to contact those who do? I don't have any solution to fix it on Linux kernel level. So, If you agree, I can add the comment of CPU0 hotplug issue on DT file. I disagree. I do not want to add a DT that is known to be broken; especially when we have no idea how to fix it. It creates long-term maintenance pain for everyone, and marginal gain for few. A comment does nothing to aid the end-user. So NAK to the PSCI node and PSCI enable method in this dts until we either have a working firmware, or a reasonable mechanism to handle the deficiency. Mark. -- 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 v7 1/9] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
Hi, On Wed, Mar 18, 2015 at 12:17:28AM +, Chanwoo Choi wrote: This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53). And Exynos5433 supports PSCI (Power State Coordination Interface) v0.1. This patch includes following dt node to support Exynos5433 SoC: 1. Octa core for big.LITTLE architecture - Cortex-A53 LITTLE Quad-core - Cortex-A57 big Quad-core - Support PSCI v0.1 Is CPU0 hotplug still broken, or has the FW been fixed? I'm very worried about adding a DT that's known broken, especially when we have no idea as to if/when the FW will be fixed judging from prior replies. Mark. -- 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 1/9] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
[...] Do CPUs enter the kernel at EL2 or at EL1? Could you give me a tip how to check the kernel at EL2 or EL1? Hmm... I thought we logged this but it looks like we don't. You could hack in a check of is_hyp_mode_available() and is_hyp_mode_mismatched(). That will tell you if EL2/hyp is available, and whether all CPUs enter at the same mode (mandatory per the boot protocol). OK, I'll try it. I check the return value of is_hyp_mode_available() to catch whether EL1 or EL2. The is_hyp_mode_available() returns 'false' during kernel booting. - __boot_cpu_mode[0]: 0xe11 (BOOT_CPU_MODE_EL1) - __boot_cpu_mode[1]: 0x0 Thanks for taking a look. It's unfortunate that CPUs aren't booted at EL2 (especially given that booting them at EL1N means the FW is doing more work to be less helpful to the OS), but at least they seem to be booted in consistent modes. Thanks, Mark. -- 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 1/9] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
CPU0 (boot CPU) is only well working for CPU_OFF. But when I try to turn on the CPU0 after CPU_OFF, I failed it. That's rather worrying. Can you look into what's going on here? I'd rather not have dts describing things which are known to be broken. The board dts don't include any node for CPU_ON/OFF. I don't understand. The CPU_ON and CPU_OFF IDs are in the psci node quoted above, and all the CPUs had enable-method = psci. I mean that there are not additional dt node except for 'cpu' and 'psci' node. The psci node and cpu enable-method are sufficient. No other nodes should be relevant. When I try to turn on the CPU0 (boot CPU), fail to turn on and lockup happen. After lockup happen, I cannot use the console. That sounds like a pretty major bug. Are you able to investigate with a hardware debugger? I can't do because there are not any jtag connector. That is very unfortunate. Which PSCI implementation are you using? Surely whoever developed it has access to debug. Surely they should have tested this? Do other CPUs eventually log errors regarding the lockup? Or is the machine completely dead from this point on? I tested CPU0 on/off. When I turn on the CPU0, I fail it. But, kernel just show the error log without lockup. I gave you wrong infromation about CPU0 off. Ok. However that's still a major bug. [...] I take it CPUs boot at EL2? Do the CPUs boot at EL1 or EL2? Unfortunately, I cannot check the secure firmware for Exynos5433 SoC. I think that a few SoC provider probably would know it. I guess I asked the wrong question. Do CPUs enter the kernel at EL2 or at EL1? Could you give me a tip how to check the kernel at EL2 or EL1? Hmm... I thought we logged this but it looks like we don't. You could hack in a check of is_hyp_mode_available() and is_hyp_mode_mismatched(). That will tell you if EL2/hyp is available, and whether all CPUs enter at the same mode (mandatory per the boot protocol). Thanks, Mark. -- 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 1/9] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
Hi, [...] + psci { + compatible = arm,psci; + method = smc; + cpu_off = 0x8402; + cpu_on = 0xC403; + }; Back at v2 you mentioned that CPU_OFF wasn't working [1]. Do both CPU_ON and CPU_OFF work for all CPUs, including the boot CPU? The CPU1 ~ CPU7 are well woking about CPU_ON/OFF. CPU0 (boot CPU) is only well working for CPU_OFF. But when I try to turn on the CPU0 after CPU_OFF, I failed it. That's rather worrying. Can you look into what's going on here? I'd rather not have dts describing things which are known to be broken. I take it CPUs boot at EL2? Do the CPUs boot at EL1 or EL2? [...] The timer node should be moved under the root node. It doesn't live on the bus; it's a component of the CPU. OK. I'll move it according to your comment. Thanks. Mark. -- 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 1/9] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
Hi, + psci { + compatible = arm,psci; + method = smc; + cpu_off = 0x8402; + cpu_on = 0xC403; + }; Back at v2 you mentioned that CPU_OFF wasn't working [1]. Do both CPU_ON and CPU_OFF work for all CPUs, including the boot CPU? The CPU1 ~ CPU7 are well woking about CPU_ON/OFF. CPU0 (boot CPU) is only well working for CPU_OFF. But when I try to turn on the CPU0 after CPU_OFF, I failed it. That's rather worrying. Can you look into what's going on here? I'd rather not have dts describing things which are known to be broken. The board dts don't include any node for CPU_ON/OFF. I don't understand. The CPU_ON and CPU_OFF IDs are in the psci node quoted above, and all the CPUs had enable-method = psci. When I try to turn on the CPU0 (boot CPU), fail to turn on and lockup happen. After lockup happen, I cannot use the console. That sounds like a pretty major bug. Are you able to investigate with a hardware debugger? Do other CPUs eventually log errors regarding the lockup? Or is the machine completely dead from this point on? I take it CPUs boot at EL2? Do the CPUs boot at EL1 or EL2? Unfortunately, I cannot check the secure firmware for Exynos5433 SoC. I think that a few SoC provider probably would know it. I guess I asked the wrong question. Do CPUs enter the kernel at EL2 or at EL1? Thanks, Mark. -- 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 1/9] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
On Thu, Mar 05, 2015 at 05:38:23AM +, Chanwoo Choi wrote: This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53). And Exynos5433 supports PSCI (Power State Coordination Interface) v0.1. This patch includes following dt node to support Exynos5433 SoC: 1. Octa core for big.LITTLE architecture - Cortex-A53 LITTLE Quad-core - Cortex-A57 big Quad-core - Support PSCI v0.1 [...] + psci { + compatible = arm,psci; + method = smc; + cpu_off = 0x8402; + cpu_on = 0xC403; + }; Back at v2 you mentioned that CPU_OFF wasn't working [1]. Do both CPU_ON and CPU_OFF work for all CPUs, including the boot CPU? I take it CPUs boot at EL2? [...] + timer { + compatible = arm,armv8-timer; + interrupts = 1 13 0xff04, +1 14 0xff04, +1 11 0xff04, +1 10 0xff04; + }; The timer node should be moved under the root node. It doesn't live on the bus; it's a component of the CPU. Thanks, Mark. [1] https://lkml.org/lkml/2014/12/2/413 -- 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: [RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC
On Wed, Jan 14, 2015 at 11:57:00PM +, Chanwoo Choi wrote: Hi Kukjin, On 01/15/2015 01:02 AM, Daniel Lezcano wrote: On 01/14/2015 04:51 PM, Kukjin Kim wrote: On 01/14/15 14:33, Chanwoo Choi wrote: Hi, + Doug, Olof This patch adds the support for Exynos 64bit SoC. The delay_timer is only used for Exynos 32bit SoC. Yes, the Exynos MCT(Multi-Core Timer) is 64bit timer and it is available on 64bit exynos SoC such as exynos7. But basically ARMv8 architecture is including ARM ARCH timer (ARM Generic Timer) and exynos7 also has implemented it and additionally its access is faster than using memory mapped register called SFR for MCT...so Doug submitted patch to use MCT on 32bit exynos SoCs before. I know arch_timer. As you comment, ARCH timer would be used for system timer for ARMv8. But, Exynos5433/Exynos7 (ARMv8) include MCT (Multi-Core Timer) IP. I checked it on Exynos5433/EXynos7 User-manaual and tested it. I think that exynos_mct.c should support the Exynos 64-bit SoC because Exynos5433/Exynos7 include already MCT (Multi-Core Timer) IP. Also, I have a problem to verify ARCH timer on Exynos SoC. Exynos User-manual never includes the detailed information about for ARCH timer(e.g, clock for ARCH timer). I knew that I can get the document of ARCH timer for ARM official site but I think it is insufficient to implement ARCH timer on Exynos SoC. What do you mean by insufficient to implement ARCH timer? The architected timer is mandatory in ARMv8, and required by the arm64 kernel. Additional timers may be requried if you want to put all CPUs into low power states where the timer logic may be disabled and/or lose state, but regardless the architected timers are necessary. Thanks, Mark. -- 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: [RESEND PATCH v3] clocksource: exynos_mct: Add the support for Exynos 64bit SoC
On Thu, Jan 15, 2015 at 12:52:38PM +, Chanwoo Choi wrote: On Thu, Jan 15, 2015 at 9:46 PM, Chanwoo Choi cwcho...@gmail.com wrote: Hi Mark, On Thu, Jan 15, 2015 at 8:29 PM, Mark Rutland mark.rutl...@arm.com wrote: On Wed, Jan 14, 2015 at 11:57:00PM +, Chanwoo Choi wrote: Hi Kukjin, On 01/15/2015 01:02 AM, Daniel Lezcano wrote: On 01/14/2015 04:51 PM, Kukjin Kim wrote: On 01/14/15 14:33, Chanwoo Choi wrote: Hi, + Doug, Olof This patch adds the support for Exynos 64bit SoC. The delay_timer is only used for Exynos 32bit SoC. Yes, the Exynos MCT(Multi-Core Timer) is 64bit timer and it is available on 64bit exynos SoC such as exynos7. But basically ARMv8 architecture is including ARM ARCH timer (ARM Generic Timer) and exynos7 also has implemented it and additionally its access is faster than using memory mapped register called SFR for MCT...so Doug submitted patch to use MCT on 32bit exynos SoCs before. I know arch_timer. As you comment, ARCH timer would be used for system timer for ARMv8. But, Exynos5433/Exynos7 (ARMv8) include MCT (Multi-Core Timer) IP. I checked it on Exynos5433/EXynos7 User-manaual and tested it. I think that exynos_mct.c should support the Exynos 64-bit SoC because Exynos5433/Exynos7 include already MCT (Multi-Core Timer) IP. Also, I have a problem to verify ARCH timer on Exynos SoC. Exynos User-manual never includes the detailed information about for ARCH timer(e.g, clock for ARCH timer). I knew that I can get the document of ARCH timer for ARM official site but I think it is insufficient to implement ARCH timer on Exynos SoC. What do you mean by insufficient to implement ARCH timer? As I knew, timer must need the source clock. The clock for ARCH timer has dependency on Exynos SoC, But I cannot find I'm so sorry about this mistake. I pressed the send button before completing reply. As I knew, timer must need the source clock. The clock for ARCH timer has dependency on Exynos SoC, But I cannot find the clock information for ARCH timer on Exynos SoC user-manual. When I tried to use ARCH timer on Exynos3250, It is not working. We can check this ARCH timer issue of Exynos3250 on following patch[1]: [1] http://www.spinics.net/lists/arm-kernel/msg322943.html Hmm. That is annoying. Your boot code should have been initialising this already. The architected timer is mandatory in ARMv8, and required by the arm64 kernel. Additional timers may be requried if you want to put all CPUs into low power states where the timer logic may be disabled and/or lose state, but regardless the architected timers are necessary. I agree that ARCH timer is mandatory. I just think that existing exynos-mct.c driver should support the Exynos5/7 SoC because released Exynos5/7 SoC includes already MCT IP for system timer. I'm not opposed to the MCT. My only concern is that a configured and enabled architected timer is mandated by the boot protocol, and is a prerequisite for a functioning kernel. Your initial response made it sound like you expected the MCT alone to be sufficient. Thanks, Mark. -- 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: [RFC 1/3] devfreq: dt-bindings: Document Exynos3250 devfreq driver
On Fri, Dec 05, 2014 at 04:46:26PM +, Krzysztof Kozlowski wrote: Add documentation for bindings used by Exynos3250 devfreq driver. Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com --- .../bindings/arm/samsung/exynos3250-devfreq.txt| 66 ++ 1 file changed, 66 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/samsung/exynos3250-devfreq.txt diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos3250-devfreq.txt b/Documentation/devicetree/bindings/arm/samsung/exynos3250-devfreq.txt new file mode 100644 index ..047955e9e371 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/samsung/exynos3250-devfreq.txt @@ -0,0 +1,66 @@ +Samsung Exynos3250 devfreq driver The binding should describe the hardware, not a particular driver of that hardware. Please write the binding (and its documentation) with that in mind, and drop references to the driver and busfreq. With an adequate binding we can probe the device and forward the information to relevant driver(s) as necessary. += + +The driver support changing frequencies and voltage for: + - memory controller and bus, + - peripheral buses (left and right). What do left and right mean in this context? + +Memory controller and bus += +Required properties: + - compatible : should be samsung,exynos3250-busfreq-mif + - reg : two sets (offset and length of the register) for PPMU registers + used by this devfreq driver + - clock-names : one clock of name dmc to manage frequency + - clocks : phandle and specifier for clock listed in clock-names property + - vdd_mif-supply : phandle to MIF voltage regulator s/_/-/ in property names please. + +Peripheral buses + +Required properties: + - compatible : should be samsung,exynos3250-busfreq-int What does int mean here? + - reg : two sets (offset and length of the register) for PPMU registers + used by this devfreq driver + - clock-names : names for PPMU clocks and bus clocks to manage frequencies; + All following clock names (and corresponding phandles) must be + provided: + - ppmu_left, ppmu_right, + - aclk_400, aclk_266, aclk_200, aclk_160, aclk_gdl, aclk_gdr, mfc; + - clocks : phandles and specifiers for clocks listed in clock-names property + - vdd_mif-supply : phandle to INT voltage regulator s/_/-/ here too. Thanks, Mark. + +Example +=== + busfreq_mif: busfreq@106A { + compatible = samsung,exynos3250-busfreq-mif; + reg = 0x106A 0x2000, 0x106B 0x2000; + clocks = cmu_dmc CLK_DIV_DMC; + clock-names = dmc; + vdd_mif-supply = buck1_reg; + status = okay; + }; + + busfreq_int: busfreq@116A { + compatible = samsung,exynos3250-busfreq-int; + reg = 0x116A 0x2000, 0x112A 0x2000; + clocks = cmu CLK_PPMULEFT, + cmu CLK_PPMURIGHT, + cmu CLK_DIV_ACLK_400_MCUISP, + cmu CLK_DIV_ACLK_266, + cmu CLK_DIV_ACLK_200, + cmu CLK_DIV_ACLK_160, + cmu CLK_DIV_GDL, + cmu CLK_DIV_GDR, + cmu CLK_DIV_MFC; + clock-names = ppmuleft, + ppmuright, + aclk_400, + aclk_266, + aclk_200, + aclk_160, + aclk_gdl, + aclk_gdr, + mfc; + vdd_int-supply = buck3_reg; + status = okay; + }; -- 1.9.1 -- 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 -- 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 16/19] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
On Mon, Dec 01, 2014 at 02:21:46AM +, Chanwoo Choi wrote: Dear Mark, On 11/28/2014 11:00 PM, Mark Rutland wrote: On Fri, Nov 28, 2014 at 01:18:25PM +, Chanwoo Choi wrote: Dear Mark, On 11/27/2014 08:18 PM, Mark Rutland wrote: On Thu, Nov 27, 2014 at 07:35:13AM +, Chanwoo Choi wrote: This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53). Cc: Kukjin Kim kgene@samsung.com Cc: Mark Rutland mark.rutl...@arm.com Cc: Arnd Bergmann a...@arndb.de Cc: Olof Johansson o...@lixom.net Cc: Catalin Marinas catalin.mari...@arm.com Cc: Will Deacon will.dea...@arm.com Signed-off-by: Chanwoo Choi cw00.c...@samsung.com Acked-by: Inki Dae inki@samsung.com Acked-by: Geunsik Lim geunsik@samsung.com --- arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi | 698 + arch/arm64/boot/dts/exynos/exynos5433.dtsi | 523 +++ 2 files changed, 1221 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi [...] + cpus { + #address-cells = 2; + #size-cells = 0; + + cpu0: cpu@100 { + device_type = cpu; + compatible = arm,cortex-a53, arm,armv8; + enable-method = psci; While the CPU nodes have enable-methods, I didn't spot a PSCI node anywhere, so this dts cannot possibly have been used to bring up an SMP system. How has this dts been tested? What PSCI revision have you implemented? Have have you tested it? My mistake, Exynos5433 supports PSCI v0.1. I'll add following PSCI nodes: I tested the boot of secondary cpu. psci { compatible = arm,psci; method = smc; cpu_off = 0x8402; cpu_on = 0xC403; }; Ok. I take it _any_ CPU may be hotplugged (including CPU0), given that you don't have MIGRATE_INFO_TYPE from PSCI 0.2 to tell you that this is not possible? If not, attempting to hotplug CPU0 will result in a BUG() and the kernel will explode. Has that been tested? I just tested secondary CPU on during kernel booting after added 'psci' dt node. So, I got the ON state of Octa CPUs. Maybe I need more time to implement CPU0 and secondary cpu hotplugged dynamically on runtime. So currently PSCI CPU_OFF is not implemented at all? Do all CPUs enter the kernel at EL2? I didn't consider EL2 for hypervisor mode. First role of this job, I'll implement CPU on/off and suspend by using PSCI. Is there any reason not to enter the kernel at EL2? PSCI 0.2 mandates entering at EL2 if present (and not under a hypervisor), and it gives the kernel a lot more flexibility to fix things up (and there's less for FW to restore) even when a hypervisor is not in use. Implementing all that to EL2 is _simpler_ than implementing it to EL1. The kernel will restore what it needs to. Thanks, Mark. -- 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 14/19] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
Hi, On Tue, Dec 02, 2014 at 08:49:51AM +, Chanwoo Choi wrote: This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53). And Exynos5433 supports PSCI (Power State Coordination Interface) v0.1. Cc: Kukjin Kim kgene@samsung.com Cc: Mark Rutland mark.rutl...@arm.com Cc: Marc Zyngier marc.zyng...@arm.com Cc: Arnd Bergmann a...@arndb.de Cc: Olof Johansson o...@lixom.net Cc: Catalin Marinas catalin.mari...@arm.com Cc: Will Deacon will.dea...@arm.com Signed-off-by: Chanwoo Choi cw00.c...@samsung.com Acked-by: Inki Dae inki@samsung.com Acked-by: Geunsik Lim geunsik@samsung.com --- arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi | 698 + arch/arm64/boot/dts/exynos/exynos5433.dtsi | 515 +++ 2 files changed, 1213 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi [...] + cpus { + #address-cells = 1; + #size-cells = 0; + + cpu0: cpu@100 { + device_type = cpu; + compatible = arm,cortex-a53, arm,armv8; + enable-method = psci; + reg = 0x100; + }; + + cpu1: cpu@101 { + device_type = cpu; + compatible = arm,cortex-a53, arm,armv8; + enable-method = psci; + reg = 0x101; + }; + + cpu2: cpu@102 { + device_type = cpu; + compatible = arm,cortex-a53, arm,armv8; + enable-method = psci; + reg = 0x0 0x102; + }; + + cpu3: cpu@103 { + device_type = cpu; + compatible = arm,cortex-a53, arm,armv8; + enable-method = psci; + reg = 0x103; + }; + + cpu4: cpu@0 { + device_type = cpu; + compatible = arm,cortex-a57, arm,armv8; + enable-method = psci; + reg = 0x0; + }; + + cpu5: cpu@1 { + device_type = cpu; + compatible = arm,cortex-a57, arm,armv8; + enable-method = psci; + reg = 0x1; + }; + + cpu6: cpu@2 { + device_type = cpu; + compatible = arm,cortex-a57, arm,armv8; + enable-method = psci; + reg = 0x2; + }; + + cpu7: cpu@3 { + device_type = cpu; + compatible = arm,cortex-a57, arm,armv8; + enable-method = psci; + reg = 0x3; + }; + }; + + psci { + compatible = arm,psci; + method = smc; + cpu_off = 0x8402; + cpu_on = 0xC403; + }; Given your comments on the latest posting, has CPU_OFF been tested, and does it work for _all_ CPUs (including CPU0)? + + soc: soc { + compatible = simple-bus; + #address-cells = 1; + #size-cells = 1; + ranges; Is that valid when changing the number of cells? The address spaces aren't strictly identical in that case, and I'd expect a translation something like: ranges = 0x0 0x0 0x0 0xff00; Where the final cell is a sufficiently large value to cover all addresses in the soc node. [...] + gic:interrupt-controller@11001000 { + compatible = arm,gic-400; + #interrupt-cells = 3; + interrupt-controller; + reg = 0x11001000 0x1000, + 0x11002000 0x1000, + 0x11004000 0x2000, + 0x11006000 0x2000; + interrupts = 1 9 0xf04; + }; The GICC needs to be 0x2000 long to map the GICC_DIR, which is at 0x1000-0x1003. [...] + pinctrl_alive: pinctrl@1058 { + compatible = samsung,exynos5433-pinctrl; + reg = 0x1058 0x1000; + + wakeup-interrupt-controller { + compatible = samsung,exynos7-wakeup-eint; + interrupts = 0 16 0; + }; + }; How exactly does the wakeup interrupt controller interact with the GIC? Surely the relationship between the two should be described? Is it a subcomponent
Re: [PATCH 14/19] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
Hi, + psci { + compatible = arm,psci; + method = smc; + cpu_off = 0x8402; + cpu_on = 0xC403; + }; Given your comments on the latest posting, has CPU_OFF been tested, and does it work for _all_ CPUs (including CPU0)? At current version, CPU_OFF of Exynos5433 is not working. I'm now working to find the cause of CPU_OFF fail. (I got CPU_ON of Exynos5433 all cores.) CPU_OFF should not be described in the DT unless it works. [...] + soc: soc { + compatible = simple-bus; + #address-cells = 1; + #size-cells = 1; + ranges; Is that valid when changing the number of cells? The address spaces aren't strictly identical in that case, and I'd expect a translation something like: ranges = 0x0 0x0 0x0 0xff00; I'll fix it after checking correct spec. Thanks. [...] + gic:interrupt-controller@11001000 { + compatible = arm,gic-400; + #interrupt-cells = 3; + interrupt-controller; + reg = 0x11001000 0x1000, + 0x11002000 0x1000, + 0x11004000 0x2000, + 0x11006000 0x2000; + interrupts = 1 9 0xf04; + }; The GICC needs to be 0x2000 long to map the GICC_DIR, which is at 0x1000-0x1003. Do you mean that following dt node is right for gic-400? reg = 0x11001000 0x1000, 0x11002000 0x2000,- I changed the the range of GICC. 0x11004000 0x2000, 0x11006000 0x2000; Yes. [...] + pinctrl_alive: pinctrl@1058 { + compatible = samsung,exynos5433-pinctrl; + reg = 0x1058 0x1000; + + wakeup-interrupt-controller { + compatible = samsung,exynos7-wakeup-eint; + interrupts = 0 16 0; + }; + }; How exactly does the wakeup interrupt controller interact with the GIC? Surely the relationship between the two should be described? The pinctrl_alive contains the alive part of GPIO PAD (gpa0~gpa3). The each GPA0/GPA1 of pinctrl_alive pad did map to unique SPI number of GIC amd GPA2/GPA3 use only one interrupt (SPI[16]) as following: +pinctrl_alive { + gpa0: gpa0 { + gpio-controller; + #gpio-cells = 2; + + interrupt-controller; + interrupt-parent = gic; + interrupts = 0 0 0, 0 1 0, 0 2 0, 0 3 0, +0 4 0, 0 5 0, 0 6 0, 0 7 0; + #interrupt-cells = 2; + }; + + gpa1: gpa1 { + gpio-controller; + #gpio-cells = 2; + + interrupt-controller; + interrupt-parent = gic; + interrupts = 0 8 0, 0 9 0, 0 10 0, 0 11 0, +0 12 0, 0 13 0, 0 14 0, 0 15 0; + #interrupt-cells = 2; + }; gpa0-0 - SPI[0] gpa0-1 - SPI[1] gpa0-2 - SPI[2] gpa0-3 - SPI[3] gpa0-4 - SPI[4] gpa0-5 - SPI[5] gpa0-6 - SPI[6] gpa0-7 - SPI[7] gpa1-0 - SPI[8] gpa1-1 - SPI[9] gpa1-2 - SPI[10] gpa1-3 - SPI[11] gpa1-4 - SPI[12] gpa1-5 - SPI[13] gpa1-6 - SPI[14] gpa1-7 - SPI[15] GPA2/GPA3 use only one interrupt (SPI[16]). The pinctrl-exynos.c driver initialized external wakeup interrupt (e.g., GPA0/GPA1/GPA2/GPA3 of Exynos5433) in exynos_eint_wkup_init() function. Following patch[1] adds the control for Exynos5433 wakeup irq.The exynos5433_pin_ctrl structure includes '.eint_wkup_init = exynos_eint_wkup_init;' fields to handle wakeup interrupt of Exynos SoC. [PATCHv2] pinctrl: exynos: Add support for Exynos543 - https://lkml.org/lkml/2014/12/2/207 +struct samsung_pin_ctrl exynos5433_pin_ctrl[] = { + { + /* pin-controller instance 0 data */ + .pin_banks = exynos5433_pin_banks0, + .nr_banks = ARRAY_SIZE(exynos5433_pin_banks0), + .eint_wkup_init = exynos_eint_wkup_init, + .suspend= exynos_pinctrl_suspend, + .resume = exynos_pinctrl_resume, + .label = exynos5433-gpio-ctrl0, + }, { And, 'struct exynos_irq_chip
Re: [PATCH 16/19] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
On Fri, Nov 28, 2014 at 01:18:25PM +, Chanwoo Choi wrote: Dear Mark, On 11/27/2014 08:18 PM, Mark Rutland wrote: On Thu, Nov 27, 2014 at 07:35:13AM +, Chanwoo Choi wrote: This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53). Cc: Kukjin Kim kgene@samsung.com Cc: Mark Rutland mark.rutl...@arm.com Cc: Arnd Bergmann a...@arndb.de Cc: Olof Johansson o...@lixom.net Cc: Catalin Marinas catalin.mari...@arm.com Cc: Will Deacon will.dea...@arm.com Signed-off-by: Chanwoo Choi cw00.c...@samsung.com Acked-by: Inki Dae inki@samsung.com Acked-by: Geunsik Lim geunsik@samsung.com --- arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi | 698 + arch/arm64/boot/dts/exynos/exynos5433.dtsi | 523 +++ 2 files changed, 1221 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi [...] + cpus { + #address-cells = 2; + #size-cells = 0; + + cpu0: cpu@100 { + device_type = cpu; + compatible = arm,cortex-a53, arm,armv8; + enable-method = psci; While the CPU nodes have enable-methods, I didn't spot a PSCI node anywhere, so this dts cannot possibly have been used to bring up an SMP system. How has this dts been tested? What PSCI revision have you implemented? Have have you tested it? My mistake, Exynos5433 supports PSCI v0.1. I'll add following PSCI nodes: I tested the boot of secondary cpu. psci { compatible = arm,psci; method = smc; cpu_off = 0x8402; cpu_on = 0xC403; }; Ok. I take it _any_ CPU may be hotplugged (including CPU0), given that you don't have MIGRATE_INFO_TYPE from PSCI 0.2 to tell you that this is not possible? If not, attempting to hotplug CPU0 will result in a BUG() and the kernel will explode. Has that been tested? Do all CPUs enter the kernel at EL2? + soc: soc { + compatible = simple-bus; + #address-cells = 1; + #size-cells = 1; + ranges; + + fixed-rate-clocks { + #address-cells = 1; + #size-cells = 0; + + xusbxti: clock@0 { + compatible = fixed-clock; + clock-output-names = xusbxti; + #clock-cells = 0; + }; + }; Get rid of the fixed-rate-clocks container node. It's pointless and messy. Given you only have one there's no need for the bogus unit-address either. OK, I'll remove unneeded code and will add following dt node for fin_pll. fin_pll: xxti { compatible = fixed-clock; clock-output-names = fin_pll; #clock-cells = 0; }; That looks fine to me. [...] + mct@101c { + compatible = samsung,exynos4210-mct; + reg = 0x101c 0x800; + interrupts = 0 102 0, 0 103 0, 0 104 0, 0 105 0, + 0 106 0, 0 107 0, 0 108 0, 0 109, + 0 110 0, 0 111 0, 0 112 0, 0 113 0; + clocks = cmu_top CLK_FIN_PLL, cmu_peris CLK_PCLK_MCT; + clock-names = fin_pll, mct; + }; Hase this block had no changes whatsoever since its use in Exynos4210? Do we not need a samsung,exynos5433-mct comaptible string too? The type of Exynos5433's MCT(Multi-Core Timer) IP is the same with the type of Exynos4210 MCT. Just Exynos5433 have eight local timer for Octa cores. So samsung,exynos4210-mct should appear in the list. I'm just wondering if it's worth having: compatible = samsung,exynos5433-mct, samsung,exynos4210-mct; Just in case we need to special-case the 5433 MCT for some reason later. CPU0 CPU1 CPU2 CPU3 CPU4 CPU5 CPU6 CPU7 134: 0 0 0 0 0 0 0 0 GIC 134 mct_comp_irq 138: 3189 0 0 0 0 0 0 0 GIC 138 mct_tick0 139: 0 2670 0 0 0 0 0 0 GIC 139 mct_tick1 140: 0 0 2763 0 0 0 0 0 GIC 140 mct_tick2 141: 0 0 0 2732 0 0 0 0 GIC 141 mct_tick3 142: 0 0
Re: [PATCH 16/19] arm64: dts: exynos: Add dts files for 64-bit Exynos5433 SoC
On Thu, Nov 27, 2014 at 07:35:13AM +, Chanwoo Choi wrote: This patch adds new Exynos5433 dtsi to support 64-bit Exynos5433 SoC based on Octal core CPUs (quad Cortex-A57 and quad Cortex-A53). Cc: Kukjin Kim kgene@samsung.com Cc: Mark Rutland mark.rutl...@arm.com Cc: Arnd Bergmann a...@arndb.de Cc: Olof Johansson o...@lixom.net Cc: Catalin Marinas catalin.mari...@arm.com Cc: Will Deacon will.dea...@arm.com Signed-off-by: Chanwoo Choi cw00.c...@samsung.com Acked-by: Inki Dae inki@samsung.com Acked-by: Geunsik Lim geunsik@samsung.com --- arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi | 698 + arch/arm64/boot/dts/exynos/exynos5433.dtsi | 523 +++ 2 files changed, 1221 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-pinctrl.dtsi create mode 100644 arch/arm64/boot/dts/exynos/exynos5433.dtsi [...] diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi new file mode 100644 index 000..3d8b576 --- /dev/null +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi @@ -0,0 +1,523 @@ +/* + * Samsung's Exynos5433 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Samsung's Exynos5433 SoC device nodes are listed in this file. Exynos5433 + * based board files can include this file and provide values for board specfic + * bindings. + * + * Note: This file does not include device nodes for all the controllers in + * Exynos5433 SoC. As device tree coverage for Exynos5433 increases, additional + * nodes can be added to this file. + * + * 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 skeleton.dtsi +#include dt-bindings/clock/exynos5433.h + Just to check: no memory reservations required for any reason? There also don't appear to be any memory nodes. Typically if that's filled in by the bootloader/FW we'd have an empty node (or one with a zero size entry) and a comment regarding the FW. +/ { + compatible = samsung,exynos5433; + #address-cells = 1; + #size-cells = 1; Not two, on both counts? The CPUs can address more than 32 bits. Is there nothing in the physical address space above 0x? [...] + cpus { + #address-cells = 2; + #size-cells = 0; + + cpu0: cpu@100 { + device_type = cpu; + compatible = arm,cortex-a53, arm,armv8; + enable-method = psci; While the CPU nodes have enable-methods, I didn't spot a PSCI node anywhere, so this dts cannot possibly have been used to bring up an SMP system. How has this dts been tested? What PSCI revision have you implemented? Have have you tested it? I take it from the presence of GICH/GICV in the gic node that CPUs enter the kernel at EL2? + reg = 0x0 0x100; + clock-frequency = 105000; What uses this? + }; [...] + soc: soc { + compatible = simple-bus; + #address-cells = 1; + #size-cells = 1; + ranges; + + fixed-rate-clocks { + #address-cells = 1; + #size-cells = 0; + + xusbxti: clock@0 { + compatible = fixed-clock; + clock-output-names = xusbxti; + #clock-cells = 0; + }; + }; Get rid of the fixed-rate-clocks container node. It's pointless and messy. Given you only have one there's no need for the bogus unit-address either. + + cmu_top: clock-controller@0x1003{ s/@0x/@/ -- a unit-address should not have the leading '0x'. Please apply that to the rest of the file. + compatible = samsung,exynos5433-cmu-top; + reg = 0x1003 0x0c04; + #clock-cells = 1; + }; [...] + mct@101c { + compatible = samsung,exynos4210-mct; + reg = 0x101c 0x800; + interrupts = 0 102 0, 0 103 0, 0 104 0, 0 105 0, + 0 106 0, 0 107 0, 0 108 0, 0 109, + 0 110 0, 0 111 0, 0 112 0, 0 113 0; + clocks = cmu_top CLK_FIN_PLL, cmu_peris CLK_PCLK_MCT; + clock-names = fin_pll, mct; + }; Hase this block had no changes whatsoever since its use in Exynos4210? Do we not need a samsung,exynos5433-mct comaptible string too? + + gic:interrupt-controller@11001000
Re: [PATCH 02/19] clk: samsung: Add binding documentation for Exynos5433 clock controller
On Thu, Nov 27, 2014 at 07:34:59AM +, Chanwoo Choi wrote: This patch add binding documentation for Exynos5433 clock controller. Exynos5433 has various clock domains So, this documentation explains the detailed clock domains ans usage guide. Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Tomasz Figa tomasz.f...@gmail.com Signed-off-by: Chanwoo Choi cw00.c...@samsung.com Acked-by: Inki Dae inki@samsung.com Acked-by: Geunsik Lim geunsik@samsung.com --- .../devicetree/bindings/clock/exynos5433-clock.txt | 106 + 1 file changed, 106 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/exynos5433-clock.txt diff --git a/Documentation/devicetree/bindings/clock/exynos5433-clock.txt b/Documentation/devicetree/bindings/clock/exynos5433-clock.txt new file mode 100644 index 000..72cd0ba --- /dev/null +++ b/Documentation/devicetree/bindings/clock/exynos5433-clock.txt @@ -0,0 +1,106 @@ +* Samsung Exynos5433 CMU (Clock Management Units) + +The Exynos5433 clock controller generates and supplies clock to various +controllers within the Exynos5433 SoC. + +Required Properties: + +- compatible: should be one of the following. + - samsung,exynos5433-cmu-top - clock controller compatible for CMU_TOP +which generates clocks for IMEM/FSYS/G3D/GSCL/HEVC/MSCL/G2D/MFC/PERIC/PERIS +domains and bus clocks. + - samsung,exynos5433-cmu-cpif - clock controller compatible for CMU_CPIF +which generates clocks for LLI (Low Latency Interface) IP. + - samsung,exynos5433-cmu-mif - clock controller compatible for CMU_MIF +which generates clocks for DRAM Memory Controller domain. + - samsung,exynos5433-cmu-peric - clock controller compatible for CMU_PERIC +which generates clocks for UART/I2C/SPI/I2S/PCM/SPDIF/PWM/SLIMBUS IPs. + - samsung,exynos5433-cmu-peris - clock controller compatible for CMU_PERIS +which generates clocks for PMU/TMU/MCT/WDT/RTC/SECKEY/TZPC IPs. + - samsung,exynos5433-cmu-fsys - clock controller compatible for CMU_FSYS +which generates clocks for USB/UFS/SDMMC/TSI/PDMA IPs. + +- 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 can use this identifier +to specify the clock which they consume. + +All available clocks are defined as preprocessor macros in +dt-bindings/clock/exynos5433.h header and can be used in device +tree sources. That header should be added as part of this patch, otehrwise this is incomplete. Mark. -- 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] regulator: of: Add regulator-initial-mode parse support
On Thu, Oct 09, 2014 at 04:19:47PM +0100, Javier Martinez Canillas wrote: Hello Mark, On 10/09/2014 12:27 PM, Mark Rutland wrote: Well, is not fairly obvious to me. One can also say the opposite, why the kernel is documenting a DT binding that is not (currently) implemented? Checkpatch will complain regarding undocumented bindings, so from a pragmatic point of view the binding must come first. Personally, when I read a patch series I do an initial pass in-order, and having the binding first makes things clearer. I might have some questions regarding the binding that the driver answers later, and it makes it easier to spot undocumented properties or conventions used by the driver. Doing so the other way around usually leaves me with more questions at the end. Thanks a lot for the explanation, it certainly makes sense then to have the DT binding before. I'll propose a patch to add that information to Documentation/devicetree/bindings/submitting-patches.txt so people (like me) who didn't find it obvious can know what the convention is. That sounds like a good idea; yes please. Thanks, Mark. -- 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] ARM: dts: add board dts file for Exynos3250-based Monk board
On Thu, Oct 02, 2014 at 01:50:25AM +0100, YoungJun Cho wrote: From: Youngjun Cho yj44@samsung.com This patch adds new board dts file to support Samsung Monk board which is based on Exynos3250 SoC and has different H/W configuration from Rinato. This patch is based on linux-samsung.git for-next branch and depends on [PATCH 0/2] ARM: dts: Add new board dts file for Exynos3250-based Rinato board[1] This dts file support following features: - eMMC - Main PMIC (Samsung S2MPS14) - Interface PMIC (Maxim MAX77836, MUIC, fuel-gauge, charger) - RTC of Exynos3250 - ADC of Exynos3250 with NTC thermistor - I2S of Exynos3250 - TMU of Exynos3250 - Secure firmware for Exynos3250 secondary cpu boot - Serial ports of Exynos3250 - gpio-key for power key [1] http://www.spinics.net/lists/arm-kernel/msg366849.html Signed-off-by: Youngjun Cho yj44@samsung.com Signed-off-by: Chanwoo Choi cw00.c...@samsung.com Signed-off-by: Inki Dae inki@samsung.com Signed-off-by: Seung-Woo Kim sw0312@samsung.com Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- arch/arm/boot/dts/Makefile| 3 +- arch/arm/boot/dts/exynos3250-monk.dts | 592 ++ 2 files changed, 594 insertions(+), 1 deletion(-) create mode 100644 arch/arm/boot/dts/exynos3250-monk.dts diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 5728918..0c8ae64 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -64,7 +64,8 @@ dtb-$(CONFIG_ARCH_BRCMSTB) += \ dtb-$(CONFIG_ARCH_DAVINCI) += da850-enbw-cmc.dtb \ da850-evm.dtb dtb-$(CONFIG_ARCH_EFM32) += efm32gg-dk3750.dtb -dtb-$(CONFIG_ARCH_EXYNOS) += exynos3250-rinato.dtb \ +dtb-$(CONFIG_ARCH_EXYNOS) += exynos3250-monk.dtb \ + exynos3250-rinato.dtb \ exynos4210-origen.dtb \ exynos4210-smdkv310.dtb \ exynos4210-trats.dtb \ diff --git a/arch/arm/boot/dts/exynos3250-monk.dts b/arch/arm/boot/dts/exynos3250-monk.dts new file mode 100644 index 000..9e94294 --- /dev/null +++ b/arch/arm/boot/dts/exynos3250-monk.dts @@ -0,0 +1,592 @@ +/* + * Samsung's Exynos3250 based Monk board device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Device tree source file for Samsung's Monk board which is based on + * Samsung Exynos3250 SoC. + * + * 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. + */ + +/dts-v1/; +#include exynos3250.dtsi +#include dt-bindings/input/input.h + +/ { + model = Samsung Monk board; + compatible = samsung,monk, samsung,exynos3250, samsung,exynos3; + + aliases { + i2c7 = i2c_max77836; + }; + + memory { + reg = 0x4000 0x0800 + 0x4800 0x0800 + 0x5000 0x0800 + 0x5800 0x07F0; Nit: please bracket list entries individually. That said, it looks like this could be coalesced into: memory@4000 { reg = 0x4000 0x1ff0; }; + }; + + chosen { + bootargs = console=ttySAC1,115200N8 root=/dev/mmcblk0p15 rootwait earlyprintk panic=5; + }; + + firmware@0205F000 { + compatible = samsung,secure-firmware; + reg = 0x0205F000 0x1000; + } ; Nit: useless space before the ';'. + + gpio_keys { + compatible = gpio-keys; + + power_key { + interrupt-parent = gpx2; + interrupts = 7 0; + gpios = gpx2 7 1; + linux,code = KEY_POWER; + label = power key; + debounce-interval = 10; + gpio-key,wakeup; + }; + }; + + regulators { + compatible = simple-bus; + #address-cells = 1; + #size-cells = 0; + + vemmc_reg: voltage-regulator-0 { + compatible = regulator-fixed; + regulator-name = V_EMMC_2.8V-fixed; + regulator-min-microvolt = 280; + regulator-max-microvolt = 280; + gpio = gpk0 2 0; + enable-active-high; + }; + }; Why not put this directly under the root node? There's no reason to group regulators simply because they're regulators, and there's only one anyhow. Mark. -- 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
Re: [PATCH v5 4/7] ARM: l2c: Add support for overriding prefetch settings
On Wed, Sep 24, 2014 at 12:05:38PM +0100, Marek Szyprowski wrote: From: Tomasz Figa t.f...@samsung.com Firmware on certain boards (e.g. ODROID-U3) can leave incorrect L2C prefetch settings configured in registers leading to crashes if L2C is enabled without overriding them. This patch introduces bindings to enable prefetch settings to be specified from DT and necessary support in the driver. Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com --- Documentation/devicetree/bindings/arm/l2cc.txt | 10 +++ arch/arm/mm/cache-l2x0.c | 39 ++ 2 files changed, 49 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt index af527ee111c2..3443d2d76788 100644 --- a/Documentation/devicetree/bindings/arm/l2cc.txt +++ b/Documentation/devicetree/bindings/arm/l2cc.txt @@ -47,6 +47,16 @@ Optional properties: - cache-id-part: cache id part number to be used if it is not present on hardware - wt-override: If present then L2 is forced to Write through mode +- arm,double-linefill : Override double linefill enable setting. Enable if + non-zero, disable if zero. +- arm,double-linefill-incr : Override double linefill on INCR read. Enable + if non-zero, disable if zero. +- arm,double-linefill-wrap : Override double linefill on WRAP read. Enable + if non-zero, disable if zero. +- arm,prefetch-drop : Override prefetch drop enable setting. Enable if non-zero, + disable if zero. I'm not too keen on tristate properties. Is this level of flexibility really required? What exact overrides do you need for boards you know of? Why do these cause crashes if not overridden? Mark. +- arm,prefetch-offset : Override prefetch offset value. Valid values are + 0-7, 15, 23, and 31. Example: diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c index 84c6c55ab896..af90a6ff6b49 100644 --- a/arch/arm/mm/cache-l2x0.c +++ b/arch/arm/mm/cache-l2x0.c @@ -1059,6 +1059,8 @@ static void __init l2c310_of_parse(const struct device_node *np, u32 data[3] = { 0, 0, 0 }; u32 tag[3] = { 0, 0, 0 }; u32 filter[2] = { 0, 0 }; + u32 prefetch; + u32 val; of_property_read_u32_array(np, arm,tag-latency, tag, ARRAY_SIZE(tag)); if (tag[0] tag[1] tag[2]) @@ -1083,6 +1085,43 @@ static void __init l2c310_of_parse(const struct device_node *np, l2x0_saved_regs.filter_start = (filter[0] ~(SZ_1M - 1)) | L310_ADDR_FILTER_EN; } + + prefetch = l2x0_saved_regs.prefetch_ctrl; + + if (!of_property_read_u32(np, arm,double-linefill, val)) { + if (val) + prefetch |= L310_PREFETCH_CTRL_DBL_LINEFILL; + else + prefetch = ~L310_PREFETCH_CTRL_DBL_LINEFILL; + } + + if (!of_property_read_u32(np, arm,double-linefill-incr, val)) { + if (val) + prefetch |= L310_PREFETCH_CTRL_DBL_LINEFILL_INCR; + else + prefetch = ~L310_PREFETCH_CTRL_DBL_LINEFILL_INCR; + } + + if (!of_property_read_u32(np, arm,double-linefill-wrap, val)) { + if (!val) + prefetch |= L310_PREFETCH_CTRL_DBL_LINEFILL_WRAP; + else + prefetch = ~L310_PREFETCH_CTRL_DBL_LINEFILL_WRAP; + } + + if (!of_property_read_u32(np, arm,prefetch-drop, val)) { + if (val) + prefetch |= L310_PREFETCH_CTRL_PREFETCH_DROP; + else + prefetch = ~L310_PREFETCH_CTRL_PREFETCH_DROP; + } + + if (!of_property_read_u32(np, arm,prefetch-offset, val)) { + prefetch = ~L310_PREFETCH_CTRL_OFFSET_MASK; + prefetch |= val L310_PREFETCH_CTRL_OFFSET_MASK; + } + + l2x0_saved_regs.prefetch_ctrl = prefetch; } static const struct l2c_init_data of_l2c310_data __initconst = { -- 1.9.2 ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- 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/7] ARM: l2c: Add support for overriding prefetch settings
On Wed, Sep 24, 2014 at 12:19:45PM +0100, Tomasz Figa wrote: On 24.09.2014 13:14, Mark Rutland wrote: On Wed, Sep 24, 2014 at 12:05:38PM +0100, Marek Szyprowski wrote: From: Tomasz Figa t.f...@samsung.com Firmware on certain boards (e.g. ODROID-U3) can leave incorrect L2C prefetch settings configured in registers leading to crashes if L2C is enabled without overriding them. This patch introduces bindings to enable prefetch settings to be specified from DT and necessary support in the driver. Signed-off-by: Tomasz Figa t.f...@samsung.com Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com --- Documentation/devicetree/bindings/arm/l2cc.txt | 10 +++ arch/arm/mm/cache-l2x0.c | 39 ++ 2 files changed, 49 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt index af527ee111c2..3443d2d76788 100644 --- a/Documentation/devicetree/bindings/arm/l2cc.txt +++ b/Documentation/devicetree/bindings/arm/l2cc.txt @@ -47,6 +47,16 @@ Optional properties: - cache-id-part: cache id part number to be used if it is not present on hardware - wt-override: If present then L2 is forced to Write through mode +- arm,double-linefill : Override double linefill enable setting. Enable if + non-zero, disable if zero. +- arm,double-linefill-incr : Override double linefill on INCR read. Enable + if non-zero, disable if zero. +- arm,double-linefill-wrap : Override double linefill on WRAP read. Enable + if non-zero, disable if zero. +- arm,prefetch-drop : Override prefetch drop enable setting. Enable if non-zero, + disable if zero. I'm not too keen on tristate properties. Is this level of flexibility really required? What exact overrides do you need for boards you know of? Why do these cause crashes if not overridden? Well, this is all thanks to broken firmware, which doesn't initialize those values properly on certain systems and they must be overridden. On the other side, there are systems with firmware that does it correctly and for those the boot defaults should be preferred. I don't see any other reasonable option of handling this. With the lack of warnings for present but empty properties, I can forsee people placing empty properties (as the names make them sound like booleans which enable features). Surely for enabling/disabling options we should only need to override one-way, disabling a feature that causes breakage for some reason? Otherwise we can keep the reset value (which might be sub-optimal). Perhaps a simple warning is sufficient if the property exists but is empty. As for why they cause crashes, I'm not an expert if it is about L2C internals, so I can't really tell, but apparently the cache can work correctly only on certain settings. Likewise, I'm no expert on the l2x0 family. It would be nice to know if this justs masks an issue elsewhere in Linux, is required for some reason by the interconnect, etc. I guess we don't have enough information to figure that out. Mark. -- 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] usb: dwc3: exynos: Add support for SCLK present on Exynos7
On Tue, Sep 02, 2014 at 11:39:08AM +0100, Vivek Gautam wrote: Hi, On Fri, Aug 29, 2014 at 12:18 AM, Mark Rutland mark.rutl...@arm.com wrote: On Thu, Aug 28, 2014 at 09:01:56AM +0100, Vivek Gautam wrote: Exynos7 also has a separate special gate clock going to the IP apart from the usual AHB clock. So add support for the same. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/dwc3/dwc3-exynos.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c index f9fb8ad..bab6395 100644 --- a/drivers/usb/dwc3/dwc3-exynos.c +++ b/drivers/usb/dwc3/dwc3-exynos.c @@ -35,6 +35,7 @@ struct dwc3_exynos { struct device *dev; struct clk *clk; + struct clk *sclk; struct regulator*vdd33; struct regulator*vdd10; }; @@ -141,10 +142,17 @@ static int dwc3_exynos_probe(struct platform_device *pdev) return -EINVAL; } + /* Exynos7 has a special gate clock going to this IP */ + exynos-sclk = devm_clk_get(dev, usbdrd30_sclk); + if (IS_ERR(exynos-sclk)) + dev_warn(dev, couldn't get sclk\n); Doesn't this introduce a pointless warning for Exynos SoCs other than Exynos7? True, it will introduce an unnecessary warning for non-Exynos7 systems. I initially thought of introducing a compatible check for Exynos7-dwc3, but that way we may end up adding such checks for future SoCs which have similar controller but have some clock difference or some other small change, no ? Perhaps. I don't know what your future hardware will look like. Is the usbdrd30_sclk input unique to Exynos7, or was it previously there but just without an input? If the latter you could just reduce this to: dev_info(dev, no sclk specified); Mark. -- 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 1/2] ARM: dts: Add DT changes for display on snow
On Thu, Aug 28, 2014 at 06:34:33AM +0100, Ajay kumar wrote: On Wed, Aug 27, 2014 at 8:31 PM, Mark Rutland mark.rutl...@arm.com wrote: On Wed, Aug 27, 2014 at 03:48:27PM +0100, Ajay Kumar wrote: Add DT nodes for ptn3460 bridge chip and panel. Add backlight enable pin and backlight power supply for pwm-backlight. Also add bridge phandle needed by dp to enable display on snow. Note that, snow doesn't support chunghwa,claa101wb01 panel, but still we choose to reuse the binding since chunghwa,claa101wb01 has similar LCD timings. What does it actually have? It's fine to have chunghwa,claa101wb01 as a fallback but we should have an identifier for the actual display, too. Actual display used is AUO,B116XTN0, for which no data currently exists in panel file. Instead of adding a new panel_desc for B116XTN0, why not we reuse existing ones? what is the identifier you are talking about? The compatible string. So in the dts we'd have compatible = au0,b116xtn0, chunghwa,claa101wb01; That way if we need specific data we can add it later. What vendor is AU0? Can we add a vendor-prefix in parallel? Thanks, Mark. Ajay Thanks, Mark. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- Changes since V1: -- Remove simple-panel compatible string. -- Use GPIO_ACTIVE_HIGH instead of 0. -- Change panel node naming from panel-simple to panel. arch/arm/boot/dts/exynos5250-snow.dts | 41 ++--- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts index f2b8c41..1ac9709 100644 --- a/arch/arm/boot/dts/exynos5250-snow.dts +++ b/arch/arm/boot/dts/exynos5250-snow.dts @@ -9,6 +9,7 @@ */ /dts-v1/; +#include dt-bindings/gpio/gpio.h #include exynos5250.dtsi #include exynos5250-cros-common.dtsi @@ -181,7 +182,7 @@ dcdc3 { ti,enable-ext-control; }; - fet1 { + fet1: fet1 { regulator-name = vcd_led; ti,overcurrent-wait = 3; }; @@ -204,7 +205,7 @@ regulator-always-on; ti,overcurrent-wait = 3; }; - fet6 { + fet6: fet6 { regulator-name = lcd_vdd; ti,overcurrent-wait = 3; }; @@ -253,6 +254,15 @@ pinctrl-0 = max98095_en; pinctrl-names = default; }; + + ptn3460: lvds-bridge@20 { + compatible = nxp,ptn3460; + reg = 0x20; + powerdown-gpios = gpy2 5 GPIO_ACTIVE_HIGH; + reset-gpios = gpx1 5 GPIO_ACTIVE_HIGH; + edid-emulation = 5; + panel = panel; + }; }; i2s0: i2s@0383 { @@ -300,11 +310,13 @@ vdd_pll-supply = ldo8_reg; }; - backlight { + backlight: backlight { compatible = pwm-backlight; pwms = pwm 0 100 0; brightness-levels = 0 100 500 1000 1500 2000 2500 2800; default-brightness-level = 7; + enable-gpios = gpx3 0 GPIO_ACTIVE_HIGH; + power-supply = fet1; pinctrl-0 = pwm0_out; pinctrl-names = default; }; @@ -314,6 +326,12 @@ samsung,invert-vclk; }; + panel: panel { + compatible = chunghwa,claa101wb01; + power-supply = fet6; + backlight = backlight; + }; + dp-controller@145B { status = okay; pinctrl-names = default; @@ -325,22 +343,7 @@ samsung,link-rate = 0x0a; samsung,lane-count = 2; samsung,hpd-gpio = gpx0 7 0; - - display-timings { - native-mode = timing1; - - timing1: timing@1 { - clock-frequency = 70589280; - hactive = 1366; - vactive = 768; - hfront-porch = 40; - hback-porch = 40; - hsync-len = 32; - vback-porch = 10; - vfront-porch = 12
Re: [PATCH 11/14] arm64: dts: Add initial device tree support for EXYNOS7
Hi, + cpus { + #address-cells = 2; + #size-cells = 0; Why size-cells=2? Can you not fit a cpuid in 32 bits? As of commit 72aea393a2e7 (arm64: smp: honour #address-size when parsing CPU reg property) Linux can handle single-cell cpu node reg entries where /cpus/#address-cells = 1. I can't make any guarantees about other code (e.g. bootloaders) which might try to do things with cpu nodes, YMMV. [...] + hsi2c_2: hsi2c@14E6 { I much prefer lowercase hex in unit addresses (and reg entries) below. I know 32-bit uses uppercase, but let's switch going forward here. My preference also; I'm happy to enforce that on new dts. [...] + timer { + compatible = arm,armv8-timer; + interrupts = 1 13 0xff01, +1 14 0xff01, +1 11 0xff01, +1 10 0xff01; + clock-frequency = 2400; + use-clocksource-only; + use-physical-timer; These two properties are not standard, and I would expect any 64-bit platform to come with PSCI such that you have a way to initialize the virtual timers. Likewise with clock-frequency. It's not a full workaround, and it's not hard to initialise CNTFRQ on each CPU. Mark. -- 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 1/2] ARM: dts: Add DT changes for display on snow
On Thu, Aug 28, 2014 at 10:34:32AM +0100, Ajay kumar wrote: On Thu, Aug 28, 2014 at 2:45 PM, Mark Rutland mark.rutl...@arm.com wrote: On Thu, Aug 28, 2014 at 06:34:33AM +0100, Ajay kumar wrote: On Wed, Aug 27, 2014 at 8:31 PM, Mark Rutland mark.rutl...@arm.com wrote: On Wed, Aug 27, 2014 at 03:48:27PM +0100, Ajay Kumar wrote: Add DT nodes for ptn3460 bridge chip and panel. Add backlight enable pin and backlight power supply for pwm-backlight. Also add bridge phandle needed by dp to enable display on snow. Note that, snow doesn't support chunghwa,claa101wb01 panel, but still we choose to reuse the binding since chunghwa,claa101wb01 has similar LCD timings. What does it actually have? It's fine to have chunghwa,claa101wb01 as a fallback but we should have an identifier for the actual display, too. Actual display used is AUO,B116XTN0, for which no data currently exists in panel file. Instead of adding a new panel_desc for B116XTN0, why not we reuse existing ones? what is the identifier you are talking about? The compatible string. So in the dts we'd have compatible = au0,b116xtn0, chunghwa,claa101wb01; Sorry, hardware engineer says actual LCD name is auo,b116xw03. Ok. That way if we need specific data we can add it later. I think we can add a new panel_desc for auo,b116xw03. Because its actual LCD dimension is 256x144, but chungwa, claa101wb01 has LCD dimension 223x125. What is Thierry's opinion on adding this new LCD? What vendor is AU0? Can we add a vendor-prefix in parallel? its auo (already present: AU Optronics Corporation) Ah, whoops. Mistook 'O' for '0'. Thanks, Mark. -- 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 1/2] ARM: dts: Add DT changes for display on snow
On Thu, Aug 28, 2014 at 02:10:18PM +0100, Thierry Reding wrote: On Thu, Aug 28, 2014 at 03:04:32PM +0530, Ajay kumar wrote: On Thu, Aug 28, 2014 at 2:45 PM, Mark Rutland mark.rutl...@arm.com wrote: On Thu, Aug 28, 2014 at 06:34:33AM +0100, Ajay kumar wrote: On Wed, Aug 27, 2014 at 8:31 PM, Mark Rutland mark.rutl...@arm.com wrote: On Wed, Aug 27, 2014 at 03:48:27PM +0100, Ajay Kumar wrote: Add DT nodes for ptn3460 bridge chip and panel. Add backlight enable pin and backlight power supply for pwm-backlight. Also add bridge phandle needed by dp to enable display on snow. Note that, snow doesn't support chunghwa,claa101wb01 panel, but still we choose to reuse the binding since chunghwa,claa101wb01 has similar LCD timings. What does it actually have? It's fine to have chunghwa,claa101wb01 as a fallback but we should have an identifier for the actual display, too. Actual display used is AUO,B116XTN0, for which no data currently exists in panel file. Instead of adding a new panel_desc for B116XTN0, why not we reuse existing ones? what is the identifier you are talking about? The compatible string. So in the dts we'd have compatible = au0,b116xtn0, chunghwa,claa101wb01; Sorry, hardware engineer says actual LCD name is auo,b116xw03. That way if we need specific data we can add it later. I think we can add a new panel_desc for auo,b116xw03. Because its actual LCD dimension is 256x144, but chungwa, claa101wb01 has LCD dimension 223x125. What is Thierry's opinion on adding this new LCD? I think you should simply add a new panel_desc for the panel. It may use similar timings, but it's likely not compatible in the way required by device tree. As you say, dimensions are different and those may have an influence on the DPI setting. There's really no reason why this should share the panel_desc with another panel. Thierry This sounds sane to me. Mark. -- 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 11/14] arm64: dts: Add initial device tree support for EXYNOS7
On Thu, Aug 28, 2014 at 05:28:22PM +0100, Olof Johansson wrote: On Thu, Aug 28, 2014 at 2:48 AM, Mark Rutland mark.rutl...@arm.com wrote: Hi, + cpus { + #address-cells = 2; + #size-cells = 0; Why size-cells=2? Can you not fit a cpuid in 32 bits? As of commit 72aea393a2e7 (arm64: smp: honour #address-size when parsing CPU reg property) Linux can handle single-cell cpu node reg entries where /cpus/#address-cells = 1. I can't make any guarantees about other code (e.g. bootloaders) which might try to do things with cpu nodes, YMMV. Ok. If address-cells is kept at 2 the unit address needs to be changed to 0,0. So one or the other has to be changed. I'm happy either way. I'm not sure the rest of the tree had 0, prefixes on all of the unit-addresses for 64-bit addresses that were under 4GB, and I'm not sure that existing dts consistently do that either. Do we want to enforce that for all 64-bit unit-addresses? [...] + hsi2c_2: hsi2c@14E6 { I much prefer lowercase hex in unit addresses (and reg entries) below. I know 32-bit uses uppercase, but let's switch going forward here. My preference also; I'm happy to enforce that on new dts. [...] + timer { + compatible = arm,armv8-timer; + interrupts = 1 13 0xff01, +1 14 0xff01, +1 11 0xff01, +1 10 0xff01; + clock-frequency = 2400; + use-clocksource-only; + use-physical-timer; These two properties are not standard, and I would expect any 64-bit platform to come with PSCI such that you have a way to initialize the virtual timers. Likewise with clock-frequency. It's not a full workaround, and it's not hard to initialise CNTFRQ on each CPU. Technically clock-frequency is documented, but not recommended to be used unless needed for working around firmware that doesn't setup the register value. :) True. In this case it's likely a cargo cult carry over from 5250 where the CNTFRQ requirement happened around the same time as we were working on it so that generation firmware lacked support for it -- it should since then have been fixed properly. It's probably unhelpful that the documentation isn't explicit about that. On that front, how about the patch below? Mark. 8 From 67104ad5a56e4c18f9c41f06af028b7561740afd Mon Sep 17 00:00:00 2001 From: Mark Rutland mark.rutl...@arm.com Date: Thu, 28 Aug 2014 17:41:03 +0100 Subject: [PATCH] Doc: dt: arch_timer: discourage clock-frequency use The ARM Generic Timer (AKA the architected timer, arm_arch_timer) features a CPU register (CNTFRQ) which firmware is intended to initialize, and non-secure software can read to determine the frequency of the timer. On CPUs with secure state, this register cannot be written from non-secure states. The firmware of early SoCs featuring the timer did not correctly initialize CNTFRQ correctly on all CPUs, requiring the frequency to be described in DT as a workaround. This workaround is not complete however as CNTFRQ is exposed to all software in a privileged non-secure mode, including KVM guests. The firmware and DTs for recent SoCs have followed the example set by these early SoCs. This patch updates the arch timer binding documentation to make it clearer that the use of the clock-frequency property is a poor work-around. The MMIO generic timer binding is similarly updated, though this is less of a concern as there is generally no need to expose the MMIO timers to guest OSs. Signed-off-by: Mark Rutland mark.rutl...@arm.com Cc: Marc Zyngier marc.zyng...@arm.com --- Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt index 37b2caf..5ca3f95 100644 --- a/Documentation/devicetree/bindings/arm/arch_timer.txt +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt @@ -17,7 +17,10 @@ to deliver its interrupts via SPIs. - interrupts : Interrupt list for secure, non-secure, virtual and hypervisor timers, in that order. -- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency of the main counter, in Hz. Should be present + only where necessary to work around BROKEN firmware which does not configure + CNTFRQ on all CPUs to a uniform correct value. Use of this property is + STRONGLY DISCOURAGED; fix your firmware unless absolutely impossible. - always-on : a boolean property. If present, the timer is powered through an always-on power domain, therefore it never loses context. @@ -38,7 +41,8 @@ Example: - compatible : Should at least contain arm,armv7-timer-mem. -- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency of the main
Re: [PATCH 11/14] arm64: dts: Add initial device tree support for EXYNOS7
On Thu, Aug 28, 2014 at 06:27:04PM +0100, Marc Zyngier wrote: On 28/08/14 18:03, Mark Rutland wrote: From 67104ad5a56e4c18f9c41f06af028b7561740afd Mon Sep 17 00:00:00 2001 From: Mark Rutland mark.rutl...@arm.com Date: Thu, 28 Aug 2014 17:41:03 +0100 Subject: [PATCH] Doc: dt: arch_timer: discourage clock-frequency use The ARM Generic Timer (AKA the architected timer, arm_arch_timer) features a CPU register (CNTFRQ) which firmware is intended to initialize, and non-secure software can read to determine the frequency of the timer. On CPUs with secure state, this register cannot be written from non-secure states. The firmware of early SoCs featuring the timer did not correctly initialize CNTFRQ correctly on all CPUs, requiring the frequency to be described in DT as a workaround. This workaround is not complete however as CNTFRQ is exposed to all software in a privileged non-secure mode, including KVM guests. The firmware and DTs for recent SoCs have followed I believe Xen is also affected by this. True. s/KVM/KVM\/Xen/, then? the example set by these early SoCs. This patch updates the arch timer binding documentation to make it clearer that the use of the clock-frequency property is a poor work-around. The MMIO generic timer binding is similarly updated, though this is less of a concern as there is generally no need to expose the MMIO timers to guest OSs. Signed-off-by: Mark Rutland mark.rutl...@arm.com Cc: Marc Zyngier marc.zyng...@arm.com Short of more explicit threats: Acked-by: Marc Zyngier marc.zyng...@arm.com Cheers. Mark. --- Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt index 37b2caf..5ca3f95 100644 --- a/Documentation/devicetree/bindings/arm/arch_timer.txt +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt @@ -17,7 +17,10 @@ to deliver its interrupts via SPIs. - interrupts : Interrupt list for secure, non-secure, virtual and hypervisor timers, in that order. -- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency of the main counter, in Hz. Should be present + only where necessary to work around BROKEN firmware which does not configure + CNTFRQ on all CPUs to a uniform correct value. Use of this property is + STRONGLY DISCOURAGED; fix your firmware unless absolutely impossible. - always-on : a boolean property. If present, the timer is powered through an always-on power domain, therefore it never loses context. @@ -38,7 +41,8 @@ Example: - compatible : Should at least contain arm,armv7-timer-mem. -- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency of the main counter, in Hz. Should be present + only when firmware has not configured the MMIO CNTFRQ registers. - reg : The control frame base address. -- Jazz is not dead. It just smells funny... -- 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 11/14] arm64: dts: Add initial device tree support for EXYNOS7
On Thu, Aug 28, 2014 at 06:19:00PM +0100, Olof Johansson wrote: On Thu, Aug 28, 2014 at 10:03 AM, Mark Rutland mark.rutl...@arm.com wrote: On Thu, Aug 28, 2014 at 05:28:22PM +0100, Olof Johansson wrote: On Thu, Aug 28, 2014 at 2:48 AM, Mark Rutland mark.rutl...@arm.com wrote: Hi, + cpus { + #address-cells = 2; + #size-cells = 0; Why size-cells=2? Can you not fit a cpuid in 32 bits? As of commit 72aea393a2e7 (arm64: smp: honour #address-size when parsing CPU reg property) Linux can handle single-cell cpu node reg entries where /cpus/#address-cells = 1. I can't make any guarantees about other code (e.g. bootloaders) which might try to do things with cpu nodes, YMMV. Ok. If address-cells is kept at 2 the unit address needs to be changed to 0,0. So one or the other has to be changed. I'm happy either way. I'm not sure the rest of the tree had 0, prefixes on all of the unit-addresses for 64-bit addresses that were under 4GB, and I'm not sure that existing dts consistently do that either. Do we want to enforce that for all 64-bit unit-addresses? Yeah, I believe that's the only valid format for a 2-address-cell unit address. Fair enough. I didn't spot this explicitly mentioned anywhere in ePAPR, but the examples match. I should probably re-jig that checkpatch test I had for unit-addresses. [...] + hsi2c_2: hsi2c@14E6 { I much prefer lowercase hex in unit addresses (and reg entries) below. I know 32-bit uses uppercase, but let's switch going forward here. My preference also; I'm happy to enforce that on new dts. [...] + timer { + compatible = arm,armv8-timer; + interrupts = 1 13 0xff01, +1 14 0xff01, +1 11 0xff01, +1 10 0xff01; + clock-frequency = 2400; + use-clocksource-only; + use-physical-timer; These two properties are not standard, and I would expect any 64-bit platform to come with PSCI such that you have a way to initialize the virtual timers. Likewise with clock-frequency. It's not a full workaround, and it's not hard to initialise CNTFRQ on each CPU. Technically clock-frequency is documented, but not recommended to be used unless needed for working around firmware that doesn't setup the register value. :) True. In this case it's likely a cargo cult carry over from 5250 where the CNTFRQ requirement happened around the same time as we were working on it so that generation firmware lacked support for it -- it should since then have been fixed properly. It's probably unhelpful that the documentation isn't explicit about that. On that front, how about the patch below? Mark. 8 From 67104ad5a56e4c18f9c41f06af028b7561740afd Mon Sep 17 00:00:00 2001 From: Mark Rutland mark.rutl...@arm.com Date: Thu, 28 Aug 2014 17:41:03 +0100 Subject: [PATCH] Doc: dt: arch_timer: discourage clock-frequency use The ARM Generic Timer (AKA the architected timer, arm_arch_timer) features a CPU register (CNTFRQ) which firmware is intended to initialize, and non-secure software can read to determine the frequency of the timer. On CPUs with secure state, this register cannot be written from non-secure states. The firmware of early SoCs featuring the timer did not correctly initialize CNTFRQ correctly on all CPUs, requiring the frequency to be described in DT as a workaround. This workaround is not complete however as CNTFRQ is exposed to all software in a privileged non-secure mode, including KVM guests. The firmware and DTs for recent SoCs have followed the example set by these early SoCs. This patch updates the arch timer binding documentation to make it clearer that the use of the clock-frequency property is a poor work-around. The MMIO generic timer binding is similarly updated, though this is less of a concern as there is generally no need to expose the MMIO timers to guest OSs. Signed-off-by: Mark Rutland mark.rutl...@arm.com Cc: Marc Zyngier marc.zyng...@arm.com With caps fixed: Acked-by: Olof Johansson o...@lixom.net Cheers. --- Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt index 37b2caf..5ca3f95 100644 --- a/Documentation/devicetree/bindings/arm/arch_timer.txt +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt @@ -17,7 +17,10 @@ to deliver its interrupts via SPIs. - interrupts : Interrupt list for secure, non-secure, virtual and hypervisor timers, in that order. -- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency
Re: [PATCH 11/14] arm64: dts: Add initial device tree support for EXYNOS7
On Thu, Aug 28, 2014 at 06:33:13PM +0100, Rob Herring wrote: On Thu, Aug 28, 2014 at 12:27 PM, Marc Zyngier marc.zyng...@arm.com wrote: On 28/08/14 18:03, Mark Rutland wrote: From 67104ad5a56e4c18f9c41f06af028b7561740afd Mon Sep 17 00:00:00 2001 From: Mark Rutland mark.rutl...@arm.com Date: Thu, 28 Aug 2014 17:41:03 +0100 Subject: [PATCH] Doc: dt: arch_timer: discourage clock-frequency use The ARM Generic Timer (AKA the architected timer, arm_arch_timer) features a CPU register (CNTFRQ) which firmware is intended to initialize, and non-secure software can read to determine the frequency of the timer. On CPUs with secure state, this register cannot be written from non-secure states. The firmware of early SoCs featuring the timer did not correctly initialize CNTFRQ correctly on all CPUs, requiring the frequency to be described in DT as a workaround. This workaround is not complete however as CNTFRQ is exposed to all software in a privileged non-secure mode, including KVM guests. The firmware and DTs for recent SoCs have followed I believe Xen is also affected by this. the example set by these early SoCs. This patch updates the arch timer binding documentation to make it clearer that the use of the clock-frequency property is a poor work-around. The MMIO generic timer binding is similarly updated, though this is less of a concern as there is generally no need to expose the MMIO timers to guest OSs. Signed-off-by: Mark Rutland mark.rutl...@arm.com Cc: Marc Zyngier marc.zyng...@arm.com Short of more explicit threats: Why not also add WARNs (and mark for stable). People tend to notice and fix those. If not the vendors, those pesky customers always complain (the same ones that get concerned about BogoMIPS values being too low). I had a go a while back but it was a bit painful becuase the MMIO and cpu timer code is intertwined, and a clock-frequency property on the MMIO timers isn't all that problematic (though admittedly still a workaround for FW not initialising something it was intended to). I was hoping I'd have the chance to split the driver in two first, so as to sidestep that. I'll take another look. Mark. -- 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 11/14] arm64: dts: Add initial device tree support for EXYNOS7
On Thu, Aug 28, 2014 at 06:37:19PM +0100, Marc Zyngier wrote: On 28/08/14 18:30, Mark Rutland wrote: On Thu, Aug 28, 2014 at 06:27:04PM +0100, Marc Zyngier wrote: On 28/08/14 18:03, Mark Rutland wrote: From 67104ad5a56e4c18f9c41f06af028b7561740afd Mon Sep 17 00:00:00 2001 From: Mark Rutland mark.rutl...@arm.com Date: Thu, 28 Aug 2014 17:41:03 +0100 Subject: [PATCH] Doc: dt: arch_timer: discourage clock-frequency use The ARM Generic Timer (AKA the architected timer, arm_arch_timer) features a CPU register (CNTFRQ) which firmware is intended to initialize, and non-secure software can read to determine the frequency of the timer. On CPUs with secure state, this register cannot be written from non-secure states. The firmware of early SoCs featuring the timer did not correctly initialize CNTFRQ correctly on all CPUs, requiring the frequency to be described in DT as a workaround. This workaround is not complete however as CNTFRQ is exposed to all software in a privileged non-secure mode, including KVM guests. The firmware and DTs for recent SoCs have followed I believe Xen is also affected by this. True. s/KVM/KVM\/Xen/, then? Yup. Or including guests running under a hypervisor Ah, that sounds better. I'll use that for the next posting. I expect this to be such a fundamental problem that all hypervisors will trip over on that one (Jailhouse definitely does). Yeah, this is a generic problem. Mark. -- 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 11/14] arm64: dts: Add initial device tree support for EXYNOS7
On Thu, Aug 28, 2014 at 06:47:00PM +0100, Geert Uytterhoeven wrote: Hi Mark, On Thu, Aug 28, 2014 at 7:39 PM, Mark Rutland mark.rutl...@arm.com wrote: Ok. If address-cells is kept at 2 the unit address needs to be changed to 0,0. So one or the other has to be changed. I'm happy either way. I'm not sure the rest of the tree had 0, prefixes on all of the unit-addresses for 64-bit addresses that were under 4GB, and I'm not sure that existing dts consistently do that either. Do we want to enforce that for all 64-bit unit-addresses? Yeah, I believe that's the only valid format for a 2-address-cell unit address. Fair enough. I didn't spot this explicitly mentioned anywhere in ePAPR, but the examples match. I couldn't find much about how the unit-addresses should really look like. Power_ePAPR_APPROVED_v1.1.pdf: The unit-address component of the name is specific to the bus type on which the node sits. It consists of one or more ASCII characters from the set of characters in Table 2-1. The unit-address must match the first address specified in the reg property of the node. If the node has no reg property, the @ and unit-address must be omitted and the node-name alone differentiates the node from other nodes at the same level in the tree. The binding for a particular bus may specify additional, more specific requirements for the format of reg and the unit-address. Table 2.1 contains lot of characters, definitely not limited to hex numbers. Also nothing about (not) needing a 0x prefix. This is unfortunate. I guess this was assumed to be implied by way of the examples. :/ I should probably re-jig that checkpatch test I had for unit-addresses. It would be great if dtc started complaining about unit-addresses not matching the first reg property. Agreed. When I last tried I thought that required more complex parsing than could be done with a regex. That said, I'd forgotten that properties must come before child nodes, so I though I had to at least balance '{' and '}' for children. I guess all we need to do is find a line beginning with '\s*reg\s*=\s*' before the next '{' or '}'. Maybe this will be easier than previously thought. :) Mark. -- 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] phy: exynos5-usbdrd: Add pipe-clk and utmi-clk support
On Thu, Aug 28, 2014 at 09:01:57AM +0100, Vivek Gautam wrote: Exynos7 SoC has now separate gate control for 125MHz pipe3 phy clock, as well as 60MHz utmi phy clock. So get the same and control in the phy-exynos5-usbdrd driver. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- .../devicetree/bindings/phy/samsung-phy.txt|4 drivers/phy/phy-exynos5-usbdrd.c | 24 2 files changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt index 7a6feea..b64d616 100644 --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt @@ -135,6 +135,10 @@ Required properties: PHY operations, associated by phy name. It is used to determine bit values for clock settings register. For Exynos5420 this is given as 'sclk_usbphy30' in CMU. + - optional clocks: Next gen Exynos SoCs have following additional It's not going to be 'Next gen' for long... +gate clocks available: +- phy_pipe: for PIPE3 phy +- phy_utmi: for UTMI+ phy - samsung,pmu-syscon: phandle for PMU system controller interface, used to control pmu registers for power isolation. - #phy-cells : from the generic PHY bindings, must be 1; diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c index b05302b..685c108 100644 --- a/drivers/phy/phy-exynos5-usbdrd.c +++ b/drivers/phy/phy-exynos5-usbdrd.c @@ -148,6 +148,8 @@ struct exynos5_usbdrd_phy_drvdata { * @dev: pointer to device instance of this platform device * @reg_phy: usb phy controller register memory base * @clk: phy clock for register access + * @pipeclk: clock for pipe3 phy + * @utmiclk: clock for utmi+ phy * @drv_data: pointer to SoC level driver data structure * @phys[]: array for 'EXYNOS5_DRDPHYS_NUM' number of PHY * instances each with its 'phy' and 'phy_cfg'. @@ -161,6 +163,8 @@ struct exynos5_usbdrd_phy { struct device *dev; void __iomem *reg_phy; struct clk *clk; + struct clk *pipeclk; + struct clk *utmiclk; const struct exynos5_usbdrd_phy_drvdata *drv_data; struct phy_usb_instance { struct phy *phy; @@ -446,6 +450,10 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy) dev_dbg(phy_drd-dev, Request to power_on usbdrd_phy phy\n); + if (!IS_ERR(phy_drd-utmiclk)) + clk_prepare_enable(phy_drd-utmiclk); + if (!IS_ERR(phy_drd-pipeclk)) + clk_prepare_enable(phy_drd-pipeclk); clk_prepare_enable(phy_drd-ref_clk); /* Enable VBUS supply */ @@ -464,6 +472,10 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy) fail_vbus: clk_disable_unprepare(phy_drd-ref_clk); + if (!IS_ERR(phy_drd-pipeclk)) + clk_disable_unprepare(phy_drd-pipeclk); + if (!IS_ERR(phy_drd-utmiclk)) + clk_disable_unprepare(phy_drd-utmiclk); return ret; } @@ -483,6 +495,10 @@ static int exynos5_usbdrd_phy_power_off(struct phy *phy) regulator_disable(phy_drd-vbus); clk_disable_unprepare(phy_drd-ref_clk); + if (!IS_ERR(phy_drd-pipeclk)) + clk_disable_unprepare(phy_drd-pipeclk); + if (!IS_ERR(phy_drd-utmiclk)) + clk_disable_unprepare(phy_drd-utmiclk); return 0; } @@ -581,6 +597,14 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev) return PTR_ERR(phy_drd-clk); } + phy_drd-pipeclk = devm_clk_get(dev, phy_pipe); + if (IS_ERR(phy_drd-pipeclk)) + dev_warn(dev, Failed to get pipe3 phy operational clock\n); + + phy_drd-utmiclk = devm_clk_get(dev, phy_utmi); + if (IS_ERR(phy_drd-utmiclk)) + dev_warn(dev, Failed to get utmi phy operational clock\n); + Pointless warnings for !Exynos7? Would it not be better to set these to NULL and not litter the code with IS_ERR checks? Mark. -- 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] usb: dwc3: exynos: Add support for SCLK present on Exynos7
On Thu, Aug 28, 2014 at 09:01:56AM +0100, Vivek Gautam wrote: Exynos7 also has a separate special gate clock going to the IP apart from the usual AHB clock. So add support for the same. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/usb/dwc3/dwc3-exynos.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c index f9fb8ad..bab6395 100644 --- a/drivers/usb/dwc3/dwc3-exynos.c +++ b/drivers/usb/dwc3/dwc3-exynos.c @@ -35,6 +35,7 @@ struct dwc3_exynos { struct device *dev; struct clk *clk; + struct clk *sclk; struct regulator*vdd33; struct regulator*vdd10; }; @@ -141,10 +142,17 @@ static int dwc3_exynos_probe(struct platform_device *pdev) return -EINVAL; } + /* Exynos7 has a special gate clock going to this IP */ + exynos-sclk = devm_clk_get(dev, usbdrd30_sclk); + if (IS_ERR(exynos-sclk)) + dev_warn(dev, couldn't get sclk\n); Doesn't this introduce a pointless warning for Exynos SoCs other than Exynos7? + exynos-dev = dev; exynos-clk = clk; clk_prepare_enable(exynos-clk); + if (!IS_ERR(exynos-sclk)) + clk_prepare_enable(exynos-sclk); If you replaced the returned err value with NULL you could avoid these IS_ERR cases. Mark. exynos-vdd33 = devm_regulator_get(dev, vdd33); if (IS_ERR(exynos-vdd33)) { @@ -187,6 +195,8 @@ err4: err3: regulator_disable(exynos-vdd33); err2: + if (!IS_ERR(exynos-sclk)) + clk_disable_unprepare(exynos-sclk); clk_disable_unprepare(clk); return ret; } @@ -199,6 +209,8 @@ static int dwc3_exynos_remove(struct platform_device *pdev) platform_device_unregister(exynos-usb2_phy); platform_device_unregister(exynos-usb3_phy); + if (!IS_ERR(exynos-sclk)) + clk_disable_unprepare(exynos-sclk); clk_disable_unprepare(exynos-clk); regulator_disable(exynos-vdd33); @@ -220,6 +232,8 @@ static int dwc3_exynos_suspend(struct device *dev) { struct dwc3_exynos *exynos = dev_get_drvdata(dev); + if (!IS_ERR(exynos-sclk)) + clk_disable(exynos-sclk); clk_disable(exynos-clk); regulator_disable(exynos-vdd33); @@ -245,6 +259,8 @@ static int dwc3_exynos_resume(struct device *dev) } clk_enable(exynos-clk); + if (!IS_ERR(exynos-sclk)) + clk_enable(exynos-sclk); /* runtime set active to reflect active state. */ pm_runtime_disable(dev); -- 1.7.10.4 -- 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 -- 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 11/14] arm64: dts: Add initial device tree support for EXYNOS7
Hi Naveen, On Wed, Aug 27, 2014 at 10:44:18AM +0100, Naveen Krishna Chatradhi wrote: Add initial device tree nodes for EXYNOS7 SoC. Also, includes the dt-binding definitions for clock ids. Fallout from a rebase? That latter part doesn't seem to be relevant. Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com Cc: Thomas Abraham thomas...@samsung.com Cc: Rob Herring r...@kernel.org Cc: Catalin Marinas catalin.mari...@arm.com --- arch/arm64/boot/dts/exynos7.dtsi | 553 ++ 1 file changed, 553 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos7.dtsi diff --git a/arch/arm64/boot/dts/exynos7.dtsi b/arch/arm64/boot/dts/exynos7.dtsi new file mode 100644 index 000..6b9eaf4 --- /dev/null +++ b/arch/arm64/boot/dts/exynos7.dtsi @@ -0,0 +1,553 @@ +/* + * SAMSUNG EXYNOS7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * SAMSUNG EXYNOS7 SoC device nodes are listed in this file. + * EXYNOS7 based board files can include this file and provide + * values for board specfic bindings. + * + * 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 dt-bindings/clock/exynos7-clk.h + +/ { + compatible = samsung,exynos7; + interrupt-parent = gic; + #address-cells = 1; + #size-cells = 1; Can we guarantee everything going to live within 0x0 - 0x for all boards using the SoC? I suspect that we can't, so the addresses and sizes at the top level should be two cells. At some point there is bound to be something above 4GB that we'll need to map, so to save us from a painful dts refactoring we should have the dts organised to support that from the outside. [...] + cpus { + #address-cells = 2; + #size-cells = 0; + + cpu@0 { + device_type = cpu; + compatible = arm,cortex-a57, arm,armv8; + reg = 0x0 0x0; + }; + }; Only UP? [...] + gic: interrupt-controller@11001000 { + compatible = arm,gic-400; + #interrupt-cells = 3; + #address-cells = 0; + interrupt-controller; + reg = 0x11001000 0x1000, + 0x11002000 0x1000, + 0x11004000 0x2000, + 0x11006000 0x2000; + }; Nice to see GICV and GICH. [...] + mct@101C { + compatible = samsung,exynos4210-mct; + reg = 0x101C 0x800; + interrupt-controller; + #interrupt-cells = 1; + interrupt-parent = mct_map; + interrupts =0, 1, 2, 3, + 4, 5, 6, 7, + 8, 9, 10, 11; + clocks = fin_pll, clock_peris PCLK_MCT; + clock-names = fin_pll, mct; + + mct_map: mct-map { + #interrupt-cells = 1; + #address-cells = 0; + #size-cells = 0; + interrupt-map = 0 gic 0 112 0, + 1 gic 0 113 0, + 2 gic 0 114 0, + 3 gic 0 115 0, + 4 gic 0 116 0, + 5 gic 0 117 0, + 6 gic 0 118 0, + 7 gic 0 119 0, + 8 gic 0 120 0, + 9 gic 0 121 0, + 10 gic 0 122 0, + 11 gic 0 123 0; + }; + }; I don't see why need the map here. Why can't we describe the GIC interrupts directly? [...] + timer { + compatible = arm,armv8-timer; + interrupts = 1 13 0xff01, +1 14 0xff01, +1 11 0xff01, +1 10 0xff01; + clock-frequency = 2400; Your firmware/bootloader should configure CNTFRQ, and this shouldn't be necessary. The clock-frequency property is an incomplete workaround for broken firmware that in an ideal world we could kill off. + use-clocksource-only; + use-physical-timer; Neither of these properties were introduced by this series, and no rationale was given. What are these properties for, and why do you believe they are necessary? Thanks, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More
Re: [PATCH 13/14] arm64: exynos7: Enable ARMv8 based Exynos7 (SoC) support
Hi, On Wed, Aug 27, 2014 at 10:44:20AM +0100, Naveen Krishna Chatradhi wrote: From: Alim Akhtar alim.akh...@samsung.com This patch adds the necessary Kconfig entries to enable support for the ARMv8 based Exynos7 SoC. Signed-off-by: Alim Akhtar alim.akh...@samsung.com Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com Cc: Rob Herring r...@kernel.org Cc: Catalin Marinas catalin.mari...@arm.com --- arch/arm64/Kconfig | 17 + 1 file changed, 17 insertions(+) diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index fd4e81a..d58 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -134,6 +134,23 @@ source kernel/Kconfig.freezer menu Platform selection +config ARCH_EXYNOS7 + bool ARMv8 based SAMSUNG EXYNOS7 + select HAVE_S3C2410_WATCHDOG if WATCHDOG + select CLKSRC_OF This seems to be implied by ARM_ARCH_TIMER in the core arm64 Kconfig, so I'm not sure this is necessary. + select COMMON_CLK_SAMSUNG + select GPIOLIB You select ARCH_REQUIRE_GPIOLIB below, so is this necessary? + select PINCTRL + select PINCTRL_EXYNOS + select RTC_CLASS + select HAVE_S3C_RTC + select GENERIC_GPIO + select ARCH_REQUIRE_GPIOLIB + select HAVE_CLK Isn't this selected already through the core arm64 Kconfig? It looks like we have COMMON_CLK, which selects CLKDEV_LOOKUP, which selects HAVE_CLK. + select HAVE_SMP I may have missed something, but I didn't see any SMP support in this series. Thanks, Mark. -- 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 1/2] ARM: dts: Add DT changes for display on snow
On Wed, Aug 27, 2014 at 03:48:27PM +0100, Ajay Kumar wrote: Add DT nodes for ptn3460 bridge chip and panel. Add backlight enable pin and backlight power supply for pwm-backlight. Also add bridge phandle needed by dp to enable display on snow. Note that, snow doesn't support chunghwa,claa101wb01 panel, but still we choose to reuse the binding since chunghwa,claa101wb01 has similar LCD timings. What does it actually have? It's fine to have chunghwa,claa101wb01 as a fallback but we should have an identifier for the actual display, too. Thanks, Mark. Signed-off-by: Ajay Kumar ajaykumar...@samsung.com --- Changes since V1: -- Remove simple-panel compatible string. -- Use GPIO_ACTIVE_HIGH instead of 0. -- Change panel node naming from panel-simple to panel. arch/arm/boot/dts/exynos5250-snow.dts | 41 ++--- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts index f2b8c41..1ac9709 100644 --- a/arch/arm/boot/dts/exynos5250-snow.dts +++ b/arch/arm/boot/dts/exynos5250-snow.dts @@ -9,6 +9,7 @@ */ /dts-v1/; +#include dt-bindings/gpio/gpio.h #include exynos5250.dtsi #include exynos5250-cros-common.dtsi @@ -181,7 +182,7 @@ dcdc3 { ti,enable-ext-control; }; - fet1 { + fet1: fet1 { regulator-name = vcd_led; ti,overcurrent-wait = 3; }; @@ -204,7 +205,7 @@ regulator-always-on; ti,overcurrent-wait = 3; }; - fet6 { + fet6: fet6 { regulator-name = lcd_vdd; ti,overcurrent-wait = 3; }; @@ -253,6 +254,15 @@ pinctrl-0 = max98095_en; pinctrl-names = default; }; + + ptn3460: lvds-bridge@20 { + compatible = nxp,ptn3460; + reg = 0x20; + powerdown-gpios = gpy2 5 GPIO_ACTIVE_HIGH; + reset-gpios = gpx1 5 GPIO_ACTIVE_HIGH; + edid-emulation = 5; + panel = panel; + }; }; i2s0: i2s@0383 { @@ -300,11 +310,13 @@ vdd_pll-supply = ldo8_reg; }; - backlight { + backlight: backlight { compatible = pwm-backlight; pwms = pwm 0 100 0; brightness-levels = 0 100 500 1000 1500 2000 2500 2800; default-brightness-level = 7; + enable-gpios = gpx3 0 GPIO_ACTIVE_HIGH; + power-supply = fet1; pinctrl-0 = pwm0_out; pinctrl-names = default; }; @@ -314,6 +326,12 @@ samsung,invert-vclk; }; + panel: panel { + compatible = chunghwa,claa101wb01; + power-supply = fet6; + backlight = backlight; + }; + dp-controller@145B { status = okay; pinctrl-names = default; @@ -325,22 +343,7 @@ samsung,link-rate = 0x0a; samsung,lane-count = 2; samsung,hpd-gpio = gpx0 7 0; - - display-timings { - native-mode = timing1; - - timing1: timing@1 { - clock-frequency = 70589280; - hactive = 1366; - vactive = 768; - hfront-porch = 40; - hback-porch = 40; - hsync-len = 32; - vback-porch = 10; - vfront-porch = 12; - vsync-len = 6; - }; - }; + bridge = ptn3460; }; }; -- 1.7.9.5 -- 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 -- 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] ARM: dts: add CPU nodes for Exynos4 SoCs
On Fri, Jul 18, 2014 at 05:00:02PM +0100, Bartlomiej Zolnierkiewicz wrote: Recent patch by Tomasz Figa (irqchip: gic: Fix core ID calculation when topology is read from DT) fixed GIC driver to filter cluster ID from values returned by cpu_logical_map() for SoCs having registers mapped without per-CPU banking making it is possible to add CPU nodes for Exynos4 SoCs. In case of Exynos SoCs these CPU nodes are also required by future changes adding initialization of cpuidle states in Exynos cpuidle driver through DT. Tested on Origen board (Exynos4210 SoC) and Trats2 (Exynos4412 SoC). Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com --- Based on next-20140717 branch of linux-next tree + - [PATCH 2/6] ARM: EXYNOS: Fix core ID used by platsmp and hotplug code http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg32811.html - [PATCH] irqchip: gic: Fix core ID calculation when topology is read from DT http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg34277.html arch/arm/boot/dts/exynos4210.dtsi | 17 + arch/arm/boot/dts/exynos4212.dtsi | 17 + arch/arm/boot/dts/exynos4412.dtsi | 29 + 3 files changed, 63 insertions(+) diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi index ee3001f..b99fc83 100644 --- a/arch/arm/boot/dts/exynos4210.dtsi +++ b/arch/arm/boot/dts/exynos4210.dtsi @@ -31,6 +31,23 @@ pinctrl2 = pinctrl_2; }; + cpus { + #address-cells = 1; + #size-cells = 0; + + cpu@0 { + device_type = cpu; + compatible = arm,cortex-a9; + reg = 0x900; + }; Please match the unit-address with the reg (e.g. this should be cpu@900). Please do this for all the other CPU nodes. Thanks, Mark. + + cpu@1 { + device_type = cpu; + compatible = arm,cortex-a9; + reg = 0x901; + }; + }; + sysram@0202 { compatible = mmio-sram; reg = 0x0202 0x2; diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi index 3c00e6e..484a2da 100644 --- a/arch/arm/boot/dts/exynos4212.dtsi +++ b/arch/arm/boot/dts/exynos4212.dtsi @@ -22,6 +22,23 @@ / { compatible = samsung,exynos4212, samsung,exynos4; + cpus { + #address-cells = 1; + #size-cells = 0; + + cpu@0 { + device_type = cpu; + compatible = arm,cortex-a9; + reg = 0xA00; + }; + + cpu@1 { + device_type = cpu; + compatible = arm,cortex-a9; + reg = 0xA01; + }; + }; + combiner: interrupt-controller@1044 { samsung,combiner-nr = 18; }; diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi index c42a3e1..89f4743 100644 --- a/arch/arm/boot/dts/exynos4412.dtsi +++ b/arch/arm/boot/dts/exynos4412.dtsi @@ -22,6 +22,35 @@ / { compatible = samsung,exynos4412, samsung,exynos4; + cpus { + #address-cells = 1; + #size-cells = 0; + + cpu@0 { + device_type = cpu; + compatible = arm,cortex-a9; + reg = 0xA00; + }; + + cpu@1 { + device_type = cpu; + compatible = arm,cortex-a9; + reg = 0xA01; + }; + + cpu@2 { + device_type = cpu; + compatible = arm,cortex-a9; + reg = 0xA02; + }; + + cpu@3 { + device_type = cpu; + compatible = arm,cortex-a9; + reg = 0xA03; + }; + }; + combiner: interrupt-controller@1044 { samsung,combiner-nr = 20; }; -- 1.8.2.3 ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- 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] drm/exynos: Support DP CLKCON register in FIMD driver
On Fri, Jun 27, 2014 at 11:24:58AM +0100, Inki Dae wrote: + DT mailing list Thanks for the Cc. Can we get the rest of the series? Judging a series based on its diffstat alone is a little hard... Or is my mailbox filtering hiding the rest of these from me? Any reason for not Ccing lakml? Mark. On 2014년 06월 27일 19:12, Ajay Kumar wrote: This series is based on exynos-drm-next branch of Inki Dae's tree at: git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git Changes since V2: Add DT property to know the type of FIMD output interface as per Inki's suggestion. Add samsung,output-type DT property in FIMD node for all boards supporting DP interface. Ajay Kumar (7): [PATCH V3 1/7] drm/exynos: Support DP CLKCON register in FIMD driver [PATCH V3 2/7] ARM: dts: Add FIMD output property for snow [PATCH V3 3/7] ARM: dts: Add FIMD output property for peach_pit [PATCH V3 4/7] ARM: dts: Add FIMD output property for peach_pi [PATCH V3 5/7] ARM: dts: Add FIMD output property for smdk5250 [PATCH V3 6/7] ARM: dts: Add FIMD output property for smdk5420 [PATCH V3 7/7] ARM: dts: Add FIMD output property for arndale .../devicetree/bindings/video/samsung-fimd.txt |1 + arch/arm/boot/dts/exynos5250-arndale.dts |1 + arch/arm/boot/dts/exynos5250-smdk5250.dts |1 + arch/arm/boot/dts/exynos5250-snow.dts |1 + arch/arm/boot/dts/exynos5420-peach-pit.dts |1 + arch/arm/boot/dts/exynos5420-smdk5420.dts |1 + arch/arm/boot/dts/exynos5800-peach-pi.dts |1 + drivers/gpu/drm/exynos/exynos_drm_fimd.c | 23 include/video/samsung_fimd.h |4 9 files changed, 34 insertions(+) -- 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 -- 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/4] Documentation: devicetree: Fix tps65090 typos
On Mon, Jun 23, 2014 at 06:27:04PM +0100, Doug Anderson wrote: Andreas, On Sun, Jun 22, 2014 at 6:21 PM, Andreas Färber afaer...@suse.de wrote: It's vsys-l{1,2}-supply, not vsys_l{1,2}-supply. Signed-off-by: Andreas Färber afaer...@suse.de --- Documentation/devicetree/bindings/regulator/tps65090.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/regulator/tps65090.txt b/Documentation/devicetree/bindings/regulator/tps65090.txt index 34098023..ca69f5e 100644 --- a/Documentation/devicetree/bindings/regulator/tps65090.txt +++ b/Documentation/devicetree/bindings/regulator/tps65090.txt @@ -45,8 +45,8 @@ Example: infet5-supply = some_reg; infet6-supply = some_reg; infet7-supply = some_reg; - vsys_l1-supply = some_reg; - vsys_l2-supply = some_reg; + vsys-l1-supply = some_reg; + vsys-l2-supply = some_reg; Your change matches the code and all existing device trees in the Linux kernel. Could this fact please be mentioned in the commit message? Given that: Acked-by: Mark Rutland mark.rutl...@arm.com I also see plenty of other bindings with dashes, so this seems reasonable. Dashes rather than underscores are preferred/correct for property names and compatible strings. Given no-one can possibly be using the bad/incorrect form with underscores, fixing the documentation to use dashes makes sense. Thanks, Mark. -- 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/4] devicetree: bindings: Document murata vendor prefix
On Tue, Jun 24, 2014 at 01:19:13PM +0100, Naveen Krishna Chatradhi wrote: Add Murata Manufacturing Co., Ltd. to the list of device tree vendor prefixes. Murata manufactures NTC (Negative Temperature Coefficient) based Thermistors for small scale applications like Mobiles and PDAs. Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com Cc: Guenter Roeck li...@roeck-us.net --- This changes is needed for the following reasons 1. The vendor prefix ntc is not defined in vendor-prefixes.txt Thus, giving an error when checked with scripts/checkpatch.pl 2. Murata Manufacturing Co., Ltd. Is the vendor for the NTC (Negative Temperature Coefficient) based thermistors. Hence, murata is the right vendor-prefix, Not ntc. 3. NTC is a technology used, But the prefix ntc is wrongly and heavily used in the driver and the documentation. .../devicetree/bindings/vendor-prefixes.txt|1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index 4d7f375..a39b7d7 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -84,6 +84,7 @@ moxaMoxa mpl MPL AG mundoreader Mundo Reader S.L. mxicyMacronix International Co., Ltd. +murata Murata Manufacturing Co., Ltd. Nit: please keep this list in alphabetical order. Otherwise, this looks fine to me. With the order fixed: Acked-by: Mark Rutland mark.rutl...@arm.com Mark. national National Semiconductor neonode Neonode Inc. netgear NETGEAR -- 1.7.9.5 -- 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 -- 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/4] hwmon: ntc_thermistor: Use the manufacturer name properly
On Tue, Jun 24, 2014 at 01:19:14PM +0100, Naveen Krishna Chatradhi wrote: Murata Manufacturing Co., Ltd is the vendor for NTC (Negative Temperature coefficient) based Thermistors. But, the driver extensively uses NTC as the vendor name. This patch corrects the vendor name also updates the compatibility strings according to the vendor-prefix.txt Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com Cc: Guenter Roeck li...@roeck-us.net --- This changes is needed for the following reasons 1. The vendor prefix ntc is not defined in vendor-prefixes.txt Thus, giving an error when checked with scripts/checkpatch.pl 2. Murata Manufacturing Co., Ltd. Is the vendor for the NTC (Negative Temperature Coefficient) based thermistors. Hence, murata is the right vendor-prefix, Not ntc. 3. NTC is a technology used, But the prefix ntc is wrongly and heavily used in the driver and the documentation. .../devicetree/bindings/arm/samsung/exynos-adc.txt |2 +- .../devicetree/bindings/hwmon/ntc_thermistor.txt | 12 ++-- Documentation/hwmon/ntc_thermistor |8 drivers/hwmon/Kconfig |5 +++-- drivers/hwmon/ntc_thermistor.c | 12 ++-- 5 files changed, 20 insertions(+), 19 deletions(-) diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt index 5d49f2b..832fe8c 100644 --- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt @@ -48,7 +48,7 @@ adc@12D1 { /* NTC thermistor is a hwmon device */ ncp15wb473@0 { - compatible = ntc,ncp15wb473; + compatible = murata,ncp15wb473; pullup-uv = 180; pullup-ohm = 47000; pulldown-ohm = 0; diff --git a/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt b/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt index c6f6667..4e9f344 100644 --- a/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt +++ b/Documentation/devicetree/bindings/hwmon/ntc_thermistor.txt @@ -3,11 +3,11 @@ NTC Thermistor hwmon sensors Requires node properties: - compatible value : one of - ntc,ncp15wb473 - ntc,ncp18wb473 - ntc,ncp21wb473 - ntc,ncp03wb473 - ntc,ncp15wl333 + murata,ncp15wb473 + murata,ncp18wb473 + murata,ncp21wb473 + murata,ncp03wb473 + murata,ncp15wl333 So we're outright changing these rather than deprecating the existing forms? In general we've pushed back on changes like this, and requested that the old strings are kept in both documentation and code as deprecated forms. Can you guarantee that changing this is not going to stop someone's board worknig properly? I suspect not. Mark. - pullup-uvPull up voltage in micro volts - pullup-ohm Pull up resistor value in ohms - pulldown-ohm Pull down resistor value in ohms @@ -21,7 +21,7 @@ Read more about iio bindings at Example: ncp15wb473@0 { - compatible = ntc,ncp15wb473; + compatible = murata,ncp15wb473; pullup-uv = 180; pullup-ohm = 47000; pulldown-ohm = 0; diff --git a/Documentation/hwmon/ntc_thermistor b/Documentation/hwmon/ntc_thermistor index 3bfda94..057b770 100644 --- a/Documentation/hwmon/ntc_thermistor +++ b/Documentation/hwmon/ntc_thermistor @@ -1,7 +1,7 @@ Kernel driver ntc_thermistor = -Supported thermistors: +Supported thermistors from Murata: * Murata NTC Thermistors NCP15WB473, NCP18WB473, NCP21WB473, NCP03WB473, NCP15WL333 Prefixes: 'ncp15wb473', 'ncp18wb473', 'ncp21wb473', 'ncp03wb473', 'ncp15wl333' Datasheet: Publicly available at Murata @@ -15,9 +15,9 @@ Authors: Description --- -The NTC thermistor is a simple thermistor that requires users to provide the -resistance and lookup the corresponding compensation table to get the -temperature input. +The NTC (Negative Temperature Coefficient) thermistor is a simple thermistor +that requires users to provide the resistance and lookup the corresponding +compensation table to get the temperature input. The NTC driver provides lookup tables with a linear approximation function and four circuit models with an option not to use any of the four models. diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 08531a1..154851b 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1052,7 +1052,7 @@ config SENSORS_PC87427 will be called pc87427. config SENSORS_NTC_THERMISTOR - tristate NTC thermistor support + tristate NTC based thermistor support from Murata depends on !OF || IIO=n || IIO help This driver supports NTC thermistors sensor reading and its @@
Re: [PATCH 3/4] ARM: DTS: use new compatible string as per the documentation
On Tue, Jun 24, 2014 at 01:19:15PM +0100, Naveen Krishna Chatradhi wrote: As Murata is the Manufactures the NTC thermistors. The vendor name in the compatibility is preposed to change to murata, ncpXXX This patch uses the new compatibility string in exynos4412 based Trats2 board. Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com Cc: Chanwoo Choi cw00.c...@samsung.com --- 1. Updates the thermistor node in Exynos4412 based Trats2 dts using the new DT bindings for NTC thermistors updated with vendor-prefix arch/arm/boot/dts/exynos4412-trats2.dts |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts index 11967f4..d35755a 100644 --- a/arch/arm/boot/dts/exynos4412-trats2.dts +++ b/arch/arm/boot/dts/exynos4412-trats2.dts @@ -771,7 +771,7 @@ }; thermistor-ap@0 { - compatible = ntc,ncp15wb473; + compatible = murata,ncp15wb473; So the previous patch woul have broken this board? Mark. pullup-uv = 180; /* VCC_1.8V_AP */ pullup-ohm = 10; /* 100K */ pulldown-ohm = 10; /* 100K */ @@ -779,7 +779,7 @@ }; thermistor-battery@1 { - compatible = ntc,ncp15wb473; + compatible = murata,ncp15wb473; pullup-uv = 180; /* VCC_1.8V_AP */ pullup-ohm = 10; /* 100K */ pulldown-ohm = 10; /* 100K */ -- 1.7.9.5 -- 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 -- 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: [PATCHv3 3/4] mmc: dw_mmc: remove the supports-highspeed property.
On Fri, May 30, 2014 at 09:01:16AM +0100, Ulf Hansson wrote: On 28 May 2014 07:35, Jaehoon Chung jh80.ch...@samsung.com wrote: Removed the parser for supports-highspeed. It can be parsed with cap-mmc-highsped or cap-sd-highspeed at mmc_of_parse(). Signed-off-by: Jaehoon Chung jh80.ch...@samsung.com --- drivers/mmc/host/dw_mmc.c |3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 3285bdd..34b5210 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -2281,9 +2281,6 @@ static struct dw_mci_board *dw_mci_parse_dt(struct dw_mci *host) return ERR_PTR(ret); } - if (of_find_property(np, supports-highspeed, NULL)) - pdata-caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED; - According to DT guys, normally we shouldn't remove DT bindings. Thus, you need to keep this, unless you can get some of the DT guys to ack it. In general, yes. Unless there's a compelling reason to drop a binding, and all users are happy with it being dropped, then there's not much point in removing it. It's usually not possible to get agreement because it's usually nto possible to know the full set of users, so in general we can't drop or change stuff. Though, you still want to move the DTs to use common mmc bindings. And you could mark the documentation of the above binding as deprecated. Deprecated bindings can be supported even if discouraged. As far as I can see, keeping the existing binding along side the new one looks relatively simple in this case. Thanks, Mark. -- 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 v6 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
Hi, Apologies for being somewhat late w.r.t. review on this. On Fri, May 30, 2014 at 10:01:17AM +0100, Thomas Abraham wrote: From: Thomas Abraham thomas...@samsung.com Add a new optional boost-frequency binding for specifying the frequencies usable in boost mode. Cc: Rob Herring robh...@kernel.org Cc: Pawel Moll pawel.m...@arm.com Cc: Mark Rutland mark.rutl...@arm.com Cc: Ian Campbell ijc+devicet...@hellion.org.uk Cc: Kumar Gala ga...@codeaurora.org Signed-off-by: Thomas Abraham thomas...@samsung.com Acked-by: Viresh Kumar viresh.ku...@linaro.org Acked-by: Nishanth Menon n...@ti.com Acked-by: Lukasz Majewski l.majew...@samsung.com --- .../devicetree/bindings/cpufreq/cpufreq-boost.txt | 38 1 file changed, 38 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file mode 100644 index 000..63ed0fc --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt @@ -0,0 +1,38 @@ +* Device tree binding for CPU boost frequency (aka over-clocking) + +Certain CPU's can be operated in optional 'boost' mode (or sometimes referred as Nit: CPUs (we're not greengrocers [1]) +overclocking) in which the CPU can operate at frequencies which are not +specified by the manufacturer as CPU's operating frequency. + +Optional Properties: +- boost-frequencies: list of frequencies in KHz to be used only in boost mode. + This list should be a subset of frequencies listed in operating-points + property. Refer to Documentation/devicetree/bindings/power/opp.txt for + details about operating-points property. What is 'boost-mode'? What are the limitations on boost frequencies? When is a CPU expected to go to these frequencies and for now long? When should I as a dt author place elements in boost-frequencies? Why are these in both operating-points and boost-frequencies? It'll be really easy to accidentally forget to mark something as a boost-frequency this way. Why not have a boost-points instead? + +Example: + + cpus { + #address-cells = 1; + #size-cells = 0; + cpu@0 { + device_type = cpu; + compatible = arm,cortex-a9; + reg = 0; + + operating-points = + 150 135 + 140 1287500 + 130 125 + 120 1187500 + 110 1137500 + 100 1087500 + ; + boost-frequencies = 150 140; This is more of a general issue, but I hate the whole cpufreq-cpu0 way of assuming that all CPUs mirror CPU0. It would be nicer if either this were dropped in /cpus or repeated per-cpu. Cheers, Mark. + }; + cpu@1 { + device_type = cpu; + compatible = arm,cortex-a9; + reg = 1; + }; + }; -- 1.7.9.5 [1] http://en.wikipedia.org/wiki/Apostrophe#Superfluous_apostrophes_.28.22greengrocers.27_apostrophes.22.29 -- 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 v6 2/2] Documentation: devicetree: Add boost-frequency binding to list boost mode frequency
On Fri, May 30, 2014 at 07:05:43PM +0100, Thomas Abraham wrote: Hi Mark, On Fri, May 30, 2014 at 6:38 PM, Mark Rutland mark.rutl...@arm.com wrote: Hi, Apologies for being somewhat late w.r.t. review on this. On Fri, May 30, 2014 at 10:01:17AM +0100, Thomas Abraham wrote: From: Thomas Abraham thomas...@samsung.com Add a new optional boost-frequency binding for specifying the frequencies usable in boost mode. Cc: Rob Herring robh...@kernel.org Cc: Pawel Moll pawel.m...@arm.com Cc: Mark Rutland mark.rutl...@arm.com Cc: Ian Campbell ijc+devicet...@hellion.org.uk Cc: Kumar Gala ga...@codeaurora.org Signed-off-by: Thomas Abraham thomas...@samsung.com Acked-by: Viresh Kumar viresh.ku...@linaro.org Acked-by: Nishanth Menon n...@ti.com Acked-by: Lukasz Majewski l.majew...@samsung.com --- .../devicetree/bindings/cpufreq/cpufreq-boost.txt | 38 1 file changed, 38 insertions(+) create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt new file mode 100644 index 000..63ed0fc --- /dev/null +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-boost.txt @@ -0,0 +1,38 @@ +* Device tree binding for CPU boost frequency (aka over-clocking) + +Certain CPU's can be operated in optional 'boost' mode (or sometimes referred as Nit: CPUs (we're not greengrocers [1]) +overclocking) in which the CPU can operate at frequencies which are not +specified by the manufacturer as CPU's operating frequency. + +Optional Properties: +- boost-frequencies: list of frequencies in KHz to be used only in boost mode. + This list should be a subset of frequencies listed in operating-points + property. Refer to Documentation/devicetree/bindings/power/opp.txt for + details about operating-points property. What is 'boost-mode'? boost-mode activates additional one or more cpu clock speeds (which are not specified as operating frequency of the cpu by the manufacturer). The cpu is allowed to operate in these boost mode speeds when the boost mode is active. The boost mode speeds are usually undocumented. Some of the chip samples could be clocked in boost mode speeds and only such samples can be safely operated in boost mode. The mechanism of entry into and exit out of boost mode is outside the scope of this documentation. What are the limitations on boost frequencies? When is a CPU expected to go to these frequencies and for now long? When should I as a dt author place elements in boost-frequencies? I will add these details in the next revision of this patch. Cheers. Why are these in both operating-points and boost-frequencies? It'll be really easy to accidentally forget to mark something as a boost-frequency this way. Why not have a boost-points instead? Does boost-points mean a set of clock speeds which are not listed as part of operating-points property? If yes, that also is a possible implementation (it was implemented in one of the earlier version of this series). Could you confirm that you want the boost mode speeds to be exclusive of the speeds listed in operating-points? If these boost mode operating points are not generally advisable for use as the other operating-points are, then they should IMO been in an entirely separate property exclusive of (but in the same format as) the operating-points property, e.g. operating points = A B, C D; boost-points = E F; Otherwise, without boost-mode support we have to parse the boost mode table to figure out which points to avoid. Or if someone typos a value in either table we might go into a boost mode when we didn't want to! Cheers, Mark. -- 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 07/27] irqchip: Declare cortex-a7's irqchip to initialize gic from dt
On Thu, Apr 10, 2014 at 11:04:59AM +0100, Marc Zyngier wrote: On Thu, Apr 10 2014 at 10:28:24 am BST, Chanwoo Choi cw00.c...@samsung.com wrote: This patch declare coretex-a7's irqchip to initialze gic from dt with arm,cortex-a7-gic data. Cc: Thomas Gleixner t...@linutronix.de Signed-off-by: Chanwoo Choi cw00.c...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/irqchip/irq-gic.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 4300b66..8e906e4 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -1069,6 +1069,7 @@ gic_of_init(struct device_node *node, struct device_node *parent) } IRQCHIP_DECLARE(cortex_a15_gic, arm,cortex-a15-gic, gic_of_init); IRQCHIP_DECLARE(cortex_a9_gic, arm,cortex-a9-gic, gic_of_init); +IRQCHIP_DECLARE(cortex_a7_gic, arm,cortex-a7-gic, gic_of_init); IRQCHIP_DECLARE(msm_8660_qgic, qcom,msm-8660-qgic, gic_of_init); IRQCHIP_DECLARE(msm_qgic2, qcom,msm-qgic2, gic_of_init); Frankly, this patch adds no value. Are we going to add arm,cortex-a12-gic, arm,cortex-a17-gic, arm,cortex-a53-gic, arm,cortex-a57-gic? And that's just to mention the ARM Ltd cores... Instead, how about defining a generic arm,gic property, and mandate that new DT files are using that? We can always use a more precise compatible for quirks. Nit: s/property/compatible/ As mentioned elsewhere, arm,gic-v2 would seem to fit the bill (and we can have arm,gic or arm,gic-v1 for v1). Mark, what do you think? I think this has been discussed in the past already. It's been repeatedly brought up and agreed on, it's just that no-one's implemented it. I agree that having an arm,gic-v2 binding that people can place in their compatible list is a sensible thing to do, and I'd be happy to Ack patches implementing it. Cheers, Mark. -- 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 V4 1/8] sxgbe: Add device-tree binding support document
On Wed, Mar 19, 2014 at 10:32:48PM +, Byungho An wrote: Mark Rutland mark.rutl...@arm.com : On Tue, Mar 18, 2014 at 04:27:46PM +, Byungho An wrote: Mark Rutland mark.rutl...@arm.com : Hi, As a general note it's helpful for devicetree to be Cc'd on the entire series (though the binding document should be a separate patch) as it provides useful context for reviewing the binding. OK. On Tue, Mar 18, 2014 at 06:47:13AM +, Byungho An wrote: From: Siva Reddy siva.kal...@samsung.com This patch adds binding document for SXGBE ethernet driver via device-tree. Signed-off-by: Siva Reddy Kallam siva.kal...@samsung.com Signed-off-by: Byungho An bh74...@samsung.com --- .../devicetree/bindings/net/samsung-sxgbe.txt | 53 1 file changed, 53 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/samsung-sxgbe.txt diff --git a/Documentation/devicetree/bindings/net/samsung-sxgbe.txt b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt new file mode 100644 index 000..ca27947 --- /dev/null +++ b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt @@ -0,0 +1,53 @@ +* Samsung 10G Ethernet driver (SXGBE) + +Required properties: +- compatible: Should be samsung,sxgbe-v2.0a +- reg: Address and length of the register set for the device +- interrupt-parent: Should be the phandle for the interrupt +controller + that services interrupts for this device +- interrupts: Should contain the SXGBE interrupts + These interrupts are ordered by fixed and follows variable + trasmit DMA interrupts, receive DMA interrupts and lpi interrupt. + index 0 - this is fixed common interrupt of SXGBE and it is +always + available. + index 1 to 25 - 8 variable trasmit interrupts, variable 16 +receive interrupts + and 1 optional lpi interrupt. +- phy-mode: String, operation mode of the PHY interface. + Supported values are: xaui, gmii. +- samsung,pbl: Integer, Programmable Burst Length. + Supported values are 1, 2, 4, 8, 16, or 32. There's no need to abbreviate to pbl. Is this a property of the hardware, or configuration that the kernel will program in? If the latter, why can the kernel not choose? Yes, this is hardware property Ok. +- samsung,fixed-burst: Boolean, Program the DMA to use the fixed +burst mode +- samsung,burst-map: Integer, Program the possible bursts +supported by sxgbe + This is an interger and represents allowable DMA bursts when +fixed burst. + Allowable range is 0x00-0x3F. This field is valid only when +fixed burst is + enabled, otherwise ignored. If that's the case, why not have just this property and have it imply the use of fixed burst mode? OK. It will be implemented in next patch set. When is it necessary to use fixed burst mode? This is the configurable mode of DMA an used internally by hardware to fetch data from platform bus Sure, but that doesn't describe when it is necessary. Is this the way the DMA was configured at integration time, or the way the kernel should configure it? It is needed when fixed length of burst is needed. if it is not configured, the length of burst will be variable(not fixed). Anyway, I'll add description more for it. And when is it necessary to have a fixed length of burst? Is this a property of the system, or the conenction of the system to another? If the latter, is it absolutely necessary for correctness to use fixed-burst mode? Or is it just always sensible to use it if available? It is not absolutely necessary, as I mentioned above. The description above does not make this clear. What does the driver do if fixed burst mode is not available? Would this work in the presence of fixed-burt mode? Fixed burst mode is always available so driver doesn't need to care about it. I'm not arguing to remove these properties. I'd just like to understand if all you're describing is the presence of a feature or that the use of the feature is absolutely necessary for correctness. OK I'm perfectly happy for Linux to always decide to use these features if available. +- samsung,adv-addr-mode: Boolean, Program the DMA to use Enhanced +address mode. When would this be selected, and why can the kernel not choose? Kernel doesn't have the provision to find out a way to select this. So, need to pass from DT Likewise, it is always absolutely necessary, or just always sensible to use enhanced address mode if present? Not always necessary. When extended address mode is needed, it can be set in the DT. When is this needed? Cheers, Mark
Re: [PATCH V8 1/7] sxgbe: Add device-tree binding support document
+Example: + + aliases { + ethernet0 = sxgbe0; + }; + + sxgbe0: ethernet@1a04 { + compatible = samsung,sxgbe-v2.0a; + reg = 0 0x1a04 0 0x1; + interrupt-parent = gic; + interrupts = 0 209 4, 0 185 4, 0 186 4, 0 187 4, + 0 188 4, 0 189 4, 0 190 4, 0 191 4, + 0 192 4, 0 193 4, 0 194 4, 0 195 4, + 0 196 4, 0 197 4, 0 198 4, 0 199 4, + 0 200 4, 0 201 4, 0 202 4, 0 203 4, + 0 204 4, 0 205 4, 0 206 4, 0 207 4, + 0 208 4, 0 210 4; + samsung,pbl = 0x08 + samsung,burst-map = 0x20 + samsung,adv-addr-mode = true + samsung,force-sf-dma-mode = true You don't need these strings for boolean properties. Their presence of the property alone (even in the absende of a value) is sufficient. These should simply be: samsung,adv-addr-map; samsung,force-sf-dma-mode; It is still an open question as to whether these properties are necessary. Cheers, Mark. -- 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 V8 4/7] net: sxgbe: add EEE(Energy Efficient Ethernet) for Samsung sxgbe
On Thu, Mar 20, 2014 at 05:26:06PM +, Byungho An wrote: From: Girish K S ks.g...@samsung.com Added support for the EEE(Energy Efficient Ethernet) in 10G ethernet driver. Signed-off-by: Girish K S ks.g...@samsung.com Neatening-by: Joe Perches j...@perches.com Signed-off-by: Byungho An bh74...@samsung.com --- drivers/net/ethernet/samsung/sxgbe/sxgbe_common.h | 53 +++ drivers/net/ethernet/samsung/sxgbe/sxgbe_core.c| 86 +- drivers/net/ethernet/samsung/sxgbe/sxgbe_ethtool.c | 47 ++ drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c| 165 +++- .../net/ethernet/samsung/sxgbe/sxgbe_platform.c|4 + drivers/net/ethernet/samsung/sxgbe/sxgbe_reg.h |5 + 6 files changed, 358 insertions(+), 2 deletions(-) [...] diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c b/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c index e410d84..b23ec57 100644 --- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c +++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c @@ -160,6 +160,10 @@ static int sxgbe_platform_probe(struct platform_device *pdev) } } + priv-lpi_irq = irq_of_parse_and_map(dev-of_node, loop++); You can use platform_get_irq(pdev, loop++) here. The interrupts have already been parsed elsewhere. Cheers, Mark. -- 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/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board
Hi, On Mon, Mar 17, 2014 at 10:14:35PM +, Kukjin Kim wrote: Signed-off-by: Kukjin Kim kgene@samsung.com Reviewed-by: Thomas Abraham thomas...@samsung.com Cc: Catalin Marinas catalin.mari...@arm.com --- arch/arm64/boot/dts/samsung-gh7.dtsi | 134 +++ arch/arm64/boot/dts/samsung-ssdk-gh7.dts | 26 ++ 2 files changed, 160 insertions(+) create mode 100644 arch/arm64/boot/dts/samsung-gh7.dtsi create mode 100644 arch/arm64/boot/dts/samsung-ssdk-gh7.dts diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi b/arch/arm64/boot/dts/samsung-gh7.dtsi new file mode 100644 index 000..d3ab914 --- /dev/null +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi @@ -0,0 +1,134 @@ +/* + * SAMSUNG GH7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * 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. +*/ + +/memreserve/ 0xFEC0 0x140; /* EL3 monitor, secure intepreter */ As I've mentioned, I'm concerned that this is even in the non-secure address space that the kernel can access. Why is this not hidden from the kernel entirely? Why is it expected to be mapped in and reserved? Additionally, the memory the used by the spin-table (0x0 0x8000fff8) has not been reserved, and thus the kernel is free to clobber it. [...] + gic: interrupt-controller@1C00 { + compatible = arm,cortex-a15-gic; For targeting any future workarounds I would very much prefer a more specific string. [...] + pmu { + compatible = samsung,gh7-pmu, armv8-pmuv3; + interrupts = 0 294 0, + 0 295 0, + 0 296 0, + 0 297 0, + 0 298 0, + 0 299 0, + 0 300 0, + 0 301 0; + }; These are all missing a trigger type (thus making them unusable), and as GH7 is the SoC name rather than the CPU name, the compatible string is somewhat bad. + + amba { + compatible = arm,amba-bus; + #address-cells = 2; + #size-cells = 2; + ranges; + + serial@12c0 { + compatible = arm,pl011, arm,primecell; + reg = 0 0x12c0 0 0x1; + interrupts = 0 418 0; + }; + + serial@12c2 { + compatible = arm,pl011, arm,primecell; + reg = 0 0x12c2 0 0x1; + interrupts = 0 420 0; + }; While the primecell bindings and PL011 bindings state that clocks are optional, the primecell bus code requires a clock named apb_pclk, and the pl011 driver requires a clock (which it expects to be UARTCLK) to acquire the frequency from. As neither are provided I do not see how this DT could possibly be used to boot a usable system. Additionally the interrupt trigger types are missing. Given that these are the only IO devices described in the dtsi/dts combination, and they do not appear to be usable, what is the point in merging this? Cheers, Mark. -- 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 V4 1/8] sxgbe: Add device-tree binding support document
On Tue, Mar 18, 2014 at 04:27:46PM +, Byungho An wrote: Mark Rutland mark.rutl...@arm.com : Hi, As a general note it's helpful for devicetree to be Cc'd on the entire series (though the binding document should be a separate patch) as it provides useful context for reviewing the binding. OK. On Tue, Mar 18, 2014 at 06:47:13AM +, Byungho An wrote: From: Siva Reddy siva.kal...@samsung.com This patch adds binding document for SXGBE ethernet driver via device-tree. Signed-off-by: Siva Reddy Kallam siva.kal...@samsung.com Signed-off-by: Byungho An bh74...@samsung.com --- .../devicetree/bindings/net/samsung-sxgbe.txt | 53 1 file changed, 53 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/samsung-sxgbe.txt diff --git a/Documentation/devicetree/bindings/net/samsung-sxgbe.txt b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt new file mode 100644 index 000..ca27947 --- /dev/null +++ b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt @@ -0,0 +1,53 @@ +* Samsung 10G Ethernet driver (SXGBE) + +Required properties: +- compatible: Should be samsung,sxgbe-v2.0a +- reg: Address and length of the register set for the device +- interrupt-parent: Should be the phandle for the interrupt +controller + that services interrupts for this device +- interrupts: Should contain the SXGBE interrupts + These interrupts are ordered by fixed and follows variable + trasmit DMA interrupts, receive DMA interrupts and lpi interrupt. + index 0 - this is fixed common interrupt of SXGBE and it is always + available. + index 1 to 25 - 8 variable trasmit interrupts, variable 16 receive interrupts + and 1 optional lpi interrupt. +- phy-mode: String, operation mode of the PHY interface. + Supported values are: xaui, gmii. +- samsung,pbl: Integer, Programmable Burst Length. + Supported values are 1, 2, 4, 8, 16, or 32. There's no need to abbreviate to pbl. Is this a property of the hardware, or configuration that the kernel will program in? If the latter, why can the kernel not choose? Yes, this is hardware property Ok. +- samsung,fixed-burst: Boolean, Program the DMA to use the fixed +burst mode +- samsung,burst-map: Integer, Program the possible bursts supported +by sxgbe + This is an interger and represents allowable DMA bursts when fixed burst. + Allowable range is 0x00-0x3F. This field is valid only when fixed +burst is + enabled, otherwise ignored. If that's the case, why not have just this property and have it imply the use of fixed burst mode? OK. It will be implemented in next patch set. When is it necessary to use fixed burst mode? This is the configurable mode of DMA an used internally by hardware to fetch data from platform bus Sure, but that doesn't describe when it is necessary. Is this the way the DMA was configured at integration time, or the way the kernel should configure it? If the latter, is it absolutely necessary for correctness to use fixed-burst mode? Or is it just always sensible to use it if available? What does the driver do if fixed burst mode is not available? Would this work in the presence of fixed-burt mode? I'm not arguing to remove these properties. I'd just like to understand if all you're describing is the presence of a feature or that the use of the feature is absolutely necessary for correctness. I'm perfectly happy for Linux to always decide to use these features if available. +- samsung,adv-addr-mode: Boolean, Program the DMA to use Enhanced +address mode. When would this be selected, and why can the kernel not choose? Kernel doesn't have the provision to find out a way to select this. So, need to pass from DT Likewise, it is always absolutely necessary, or just always sensible to use enhanced address mode if present? +- samsung,force_thresh_dma_mode: Boolean, Force DMA to use the +threshold mode + for both tx and rx s/_/-/ in property names. OK. Likewise, why can the kernel not choose. SXGBE h/w registers can't provide information to select this. So, need to pass from DT depend on Platform Similarly. +- samsung,force_sf_dma_mode: Boolean, Force DMA to use the Store and +Forward + mode for both tx and rx. This flag is ignored if +force_thresh_dma_mode is set. Likewise for both points. Same above. And again. Cheers, Mark. -- 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 V4 1/8] sxgbe: Add device-tree binding support document
Hi, As a general note it's helpful for devicetree to be Cc'd on the entire series (though the binding document should be a separate patch) as it provides useful context for reviewing the binding. On Tue, Mar 18, 2014 at 06:47:13AM +, Byungho An wrote: From: Siva Reddy siva.kal...@samsung.com This patch adds binding document for SXGBE ethernet driver via device-tree. Signed-off-by: Siva Reddy Kallam siva.kal...@samsung.com Signed-off-by: Byungho An bh74...@samsung.com --- .../devicetree/bindings/net/samsung-sxgbe.txt | 53 1 file changed, 53 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/samsung-sxgbe.txt diff --git a/Documentation/devicetree/bindings/net/samsung-sxgbe.txt b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt new file mode 100644 index 000..ca27947 --- /dev/null +++ b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt @@ -0,0 +1,53 @@ +* Samsung 10G Ethernet driver (SXGBE) + +Required properties: +- compatible: Should be samsung,sxgbe-v2.0a +- reg: Address and length of the register set for the device +- interrupt-parent: Should be the phandle for the interrupt controller + that services interrupts for this device +- interrupts: Should contain the SXGBE interrupts + These interrupts are ordered by fixed and follows variable + trasmit DMA interrupts, receive DMA interrupts and lpi interrupt. + index 0 - this is fixed common interrupt of SXGBE and it is always + available. + index 1 to 25 - 8 variable trasmit interrupts, variable 16 receive interrupts + and 1 optional lpi interrupt. +- phy-mode: String, operation mode of the PHY interface. + Supported values are: xaui, gmii. +- samsung,pbl: Integer, Programmable Burst Length. + Supported values are 1, 2, 4, 8, 16, or 32. There's no need to abbreviate to pbl. Is this a property of the hardware, or configuration that the kernel will program in? If the latter, why can the kernel not choose? +- samsung,fixed-burst: Boolean, Program the DMA to use the fixed burst mode +- samsung,burst-map: Integer, Program the possible bursts supported by sxgbe + This is an interger and represents allowable DMA bursts when fixed burst. + Allowable range is 0x00-0x3F. This field is valid only when fixed burst is + enabled, otherwise ignored. If that's the case, why not have just this property and have it imply the use of fixed burst mode? When is it necessary to use fixed burst mode? +- samsung,adv-addr-mode: Boolean, Program the DMA to use Enhanced address mode. When would this be selected, and why can the kernel not choose? +- samsung,force_thresh_dma_mode: Boolean, Force DMA to use the threshold mode + for both tx and rx s/_/-/ in property names. Likewise, why can the kernel not choose. +- samsung,force_sf_dma_mode: Boolean, Force DMA to use the Store and Forward + mode for both tx and rx. This flag is ignored if force_thresh_dma_mode is set. Likewise for both points. +- samsung,phy-addr: Integer, Address of the PHY attached with SXGBE. Some of these properties appear to be missing from the example. Are they required or optional? Thanks, Mark. + +Optional properties: +- mac-address: 6 bytes, mac address + +Example: + + aliases { + ethernet0 = sxgbe0; + }; + + sxgbe0: ethernet@1a04 { + compatible = samsung,sxgbe-v2.0a; + reg = 0 0x1a04 0 0x1; + interrupt-parent = gic; + interrupts = 0 209 4, 0 185 4, 0 186 4, 0 187 4, + 0 188 4, 0 189 4, 0 190 4, 0 191 4, + 0 192 4, 0 193 4, 0 194 4, 0 195 4, + 0 196 4, 0 197 4, 0 198 4, 0 199 4, + 0 200 4, 0 201 4, 0 202 4, 0 203 4, + 0 204 4, 0 205 4, 0 206 4, 0 207 4, + 0 208 4, 0 210 4; + mac-address = []; /* Filled in by U-Boot */ + phy-mode = xaui; + }; -- 1.7.10.4 -- 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 -- 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 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board
On Fri, Mar 14, 2014 at 01:26:31AM +, Kukjin Kim wrote: + cpu@000 { + device_type = cpu; + compatible = arm,armv8; The arm,armv8 should be a fallback rather than the only entry. The precise core should be first (see arch/arm64/boot/dts/apm-storm.dtsi for an example). Well, do I really need another name if GH7 has just 'ARMv8 core' exactly? Yes please. You presumably don't have just 'ARMv8 core', but rather a specific ARMv8 implementation. OK, so in my understanding if it is really _just_ 'ARMv8 core' from S/W point of view, arm,armv8 should be fine. I will make sure. It will work, sure. But the compatible list should also list the specific CPU. From the S/W point of view there are likely implementation-defined registers which mean that no real CPU will be just an ARMv8 core. We have enough trouble with DTBs for 32-bit SoCs not listing things they should. I see no reason to be lax here. + gic: interrupt-controller@1C00 { + compatible = arm,cortex-a15-gic; We should have a more specific compatible string early in the list here too. Well, I think more specific compatible string is not required for gic, there were discussion about using another compatible string for gic-v2. And cortex-a15-gic should be fine at this moment and if another name is required as per previous discussion, we will then. While it's probably ok to have arm,cortex-a15-gic in the compatible list, it would be good to also have a more specific string, so we can identify the GIC implementation precisely in future (in case we need to perform any workarounds, or there's some kind of optimisation we could perform for some implementations). Hmm... let's use just it for now then add another specific string later ;-) Why not do this from the start? Then we can actually rely on the DTB rather than it being useless. + pmu { + compatible = arm,armv8-pmuv3; This is missing a specific compatible string. I don't know why I need another specific compatible string here because I thought the 'armv8-pmuv3' is enough. Each CPU microarchitecture has different PMU events and quirks. While the CPU PMU might be compatible with ARMv8's PMUv3, it will also have implementation-specific events, and may have quirks we need to work around in future. As with 32-bit, we'd expect these to be of the form ${implementor},${cpu}-pmu. OK. + compatible = samsung,gh7-pmu, armv8-pmuv3; Is GH7 an SoC or a CPU? + serial@12c2 { + compatible = arm,pl011, arm,primecell; + reg = 0 0x12c2 0 0x1; + interrupts = 0 420 0; + }; These are both missing required clocks. Yes right. So you'll add these? To be honest, still I'm not sure how it should be handled because regarding clocks do not need to handle in the kernel for GH7. And for it, I needed to use some HACKs to change clock handling in the kernel. I will provide proper solution soon. Are the clocks always-on, fixed-rate clocks, or does some firmware manage them dynamically? We have bindings for the former. There's not much point having a DTS upstream that relies on hacks which are not. Cheers, Mark. -- 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 1/7] net: sxgbe: add basic framework for Samsung 10Gb ethernet driver
On Mon, Mar 17, 2014 at 05:51:21AM +, Byungho An wrote: Mark Rutland mark.rutl...@arm.com : On Wed, Mar 12, 2014 at 01:28:00PM +, Byungho An wrote: From: Siva Reddy siva.kal...@samsung.com This patch adds support for Samsung 10Gb ethernet driver(sxgbe). - sxgbe core initialization - Tx and Rx support - MDIO support - ISRs for Tx and Rx - ifconfig support to driver Signed-off-by: Siva Reddy Kallam siva.kal...@samsung.com Signed-off-by: Vipul Pandya vipul.pan...@samsung.com Signed-off-by: Girish K S ks.g...@samsung.com Neatening-by: Joe Perches j...@perches.com Signed-off-by: Byungho An bh74...@samsung.com [...] +static int sxgbe_probe_config_dt(struct platform_device *pdev, +struct sxgbe_plat_data *plat, +const char **mac) { + struct device_node *np = pdev-dev.of_node; + struct sxgbe_dma_cfg *dma_cfg; + u32 phy_addr; + + if (!np) + return -ENODEV; + + *mac = of_get_mac_address(np); I see that of_get_mac_address returns a *void rather than *char, but it's always a string of hex digits. Would it make sense to change the of_get_mac_address prototype to return a char* ? This implementation is of_ specific, our driver only uses it. Sure, this was a general observation that wasn't specific to this driver. + plat-interface = of_get_phy_mode(np); + + plat-bus_id = of_alias_get_id(np, ethernet); + if (plat-bus_id 0) + plat-bus_id = 0; This wasn't mentioned in the binding. It doesn't need to be in the binding document, because we get the alias id by passing the node name. Here ethernet is not a property it is the dt node name e.g. ethernet@x { }; Huh? The acquisition of the alias ID is not performed based on the node name, but rather the node and stem of the entry in the /alias node that points to it. Consider this dts fragment: / { ... alias { ethernet5 = /bus/arbitrarily_named_sxgbe@deadbeef; }; bus { ... arbtirarily_named_sxgbe@0xdeadbeef { compatible = samsung,sxgbe-v2.0a; reg = 0xdeadbeef 0, ... ; ... }; }; } The above call to of_alias_get_id(np, ethernet) would return 5 for the sxgbe node, despite the node name being completely arbitrary. While the node should probably be named ethernet, this doesn't matter for the purpose of alias id handling. If your binding has a particular form of alias, please document it so DT authors can be aware of it and make use of it. That said we probably need better guidance on the use of aliases. The usage model is undocumented beyond aliases being informational, which is somewhat vague. Unfortunately the description in ePAPR also glosses over the expected usage. Grant, would you be able to elaborate on the expected usage of aliases? Should alias names always be from a standard set (e.g. ethernet, serial, pci per ePAPR), or are binding-specific names (e.g. the Exynos S-Scalar's gsc) something we're happy with? Are aliases allowed to be required, or are they always supposed to be an optional hint to the OS? Cheers, Mark. -- 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: [PATCHv2 8/8] devfreq: exynos4: Add busfreq driver for exynos4210/exynos4x12
On Fri, Mar 14, 2014 at 07:14:37AM +, Chanwoo Choi wrote: Hi Mark, On 03/14/2014 02:53 AM, Mark Rutland wrote: On Thu, Mar 13, 2014 at 08:17:29AM +, Chanwoo Choi wrote: This patch add busfreq driver for Exynos4210/Exynos4x12 memory interface and bus to support DVFS(Dynamic Voltage Frequency Scaling) according to PPMU counters. PPMU (Performance Profiling Monitorings Units) of Exynos4 SoC provides PPMU counters for DMC(Dynamic Memory Controller) to check memory bus utilization and then busfreq driver adjusts dynamically the operating frequency/voltage by using DEVFREQ Subsystem. Signed-off-by: Chanwoo Choi cw00.c...@samsung.com --- .../devicetree/bindings/devfreq/exynos4_bus.txt| 49 ++ 1 file changed, 49 insertions(+) create mode 100644 Documentation/devicetree/bindings/devfreq/exynos4_bus.txt diff --git a/Documentation/devicetree/bindings/devfreq/exynos4_bus.txt b/Documentation/devicetree/bindings/devfreq/exynos4_bus.txt new file mode 100644 index 000..2a83fcc --- /dev/null +++ b/Documentation/devicetree/bindings/devfreq/exynos4_bus.txt @@ -0,0 +1,49 @@ + +Exynos4210/4x12 busfreq driver +- + +Exynos4210/4x12 Soc busfreq driver with devfreq for Memory bus frequency/voltage +scaling according to PPMU counters of memory controllers + +Required properties: +- compatible : should contain Exynos4 SoC type as follwoing: +- samsung,exynos4x12-busfreq for Exynos4x12 +- samsung,exynos4210-busfreq for Exynos4210 Is there a device called busfreq? What device does this binding describe? I'll add detailed description of busfreq as following: busfreq(bus frequendcy) driver means that busfreq driver control dynamically memory bus frequency/voltage by checking memory bus utilization to optimize power-consumption. When checking memeory bus utilization, exynos4_busfreq driver would use PPMU(Performance Profiling Monitoring Units). This still sounds like a description of the _driver_, not the _device_. The binding should describe the hardware, now the high level abstraction that software is going to build atop of it. It sounds like this is a binding for the DMC PPMU? Is the PPMU a component of the DMC, or is it bolted on the side? +- reg : offset and length of the ppmudmc0/1 +- PPMU (Performance Profiling Monitoring Units) You seem to require a particular order here. It would be good to be explicit about it. OK, I'll modify it as following: the offset and length of the PPMU_DMC0 / PPMU_DMC1 PPMU_DMC0/PPMU_DMC1 show memory buy utilization to exynos4_bus driver. +: It is to profile performance event of DMC(Dynamic Memory +Controller) So, exynos4_bus.c can check memory bus utilization +by using PPMU of Exynos4 SoC. This is superfluous, and Linux-specific. The binding document shouldn't need to refer to drivers. I'll remove this description. +- clocks : clock number of ppmudmc0/1 +- clock-names : clock name of ppmudmc0/1 Are these two clocks, or one clock with a slash in the name? Please list each name separately. I'll expalin clocks as following: ppmudmc0, ppmudmc1 +- vdd_int-supply: regulator for interface block of Exynos4 How does the interface block relate to the DMC / PPMU? Cheers, Mark. -- 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 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board
On Wed, Mar 12, 2014 at 04:31:56AM +, Kukjin Kim wrote: Mark Rutland wrote: Hi Mark, On Mon, Mar 10, 2014 at 10:51:17PM +, Kukjin Kim wrote: Signed-off-by: Kukjin Kim kgene@samsung.com Reviewed-by: Thomas Abraham thomas...@samsung.com Cc: Catalin Marinas catalin.mari...@arm.com --- arch/arm64/boot/dts/samsung-gh7.dtsi | 106 +++ arch/arm64/boot/dts/samsung-ssdk-gh7.dts | 26 2 files changed, 132 insertions(+) create mode 100644 arch/arm64/boot/dts/samsung-gh7.dtsi create mode 100644 arch/arm64/boot/dts/samsung-ssdk-gh7.dts diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi b/arch/arm64/boot/dts/samsung-gh7.dtsi new file mode 100644 index 000..b5e8488 --- /dev/null +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi @@ -0,0 +1,106 @@ +/* + * SAMSUNG GH7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * 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. +*/ + +/memreserve/ 0x8000 0x0C40; Please have a comment as to what this memreserve is intended to protect. This is very large, and we don't want pointless memreserves. Well, you mean I need to comment about each reserved memory area? Please have a comment about what each /memreserve/ is intended to protect. We need the reserved memory for EL3 monitor, UEFI services, secure interpreter, hypervisor and Scan-chain for debug...BTW isn't it depending on each hardware platform and usage model? If it depends on each particular model then it doesn't belong in the shared dtsi, and each dts should have the /memreserve/ for that platform. Why would the hypervisor or secure OS memory ever be accessible to the kernel such that it would require a memreserve? That sounds fundamentally broken. I was under the impression that if using UEFI we can identify and reserve the UEFI services by walking to UEFI memory map. If we don't use them, they don't need to be protected. As such, I don't see why they need a memreserve for them. Just note, I will change the reserve memory area and size. From at 0xf800 and size is 128MB(0x800). Ok. What address does the kernel end up getting loaded at on this board? We load the kernel image from at 0x8008 after changing the reserve memory area. Ok. +/ { + interrupt-parent = gic; + #address-cells = 2; + #size-cells = 2; + + aliases { + serial0 = /amba/uart@12c0; + }; + + cpus { + #address-cells = 2; + #size-cells = 0; + + cpu@000 { + device_type = cpu; + compatible = arm,armv8; The arm,armv8 should be a fallback rather than the only entry. The precise core should be first (see arch/arm64/boot/dts/apm-storm.dtsi for an example). Well, do I really need another name if GH7 has just 'ARMv8 core' exactly? Yes please. You presumably don't have just 'ARMv8 core', but rather a specific ARMv8 implementation. + reg = 0x0 0x000; Any reason for padding these to three hexadecimal digits (in both the reg and unit-address)? Yes, I already commented on Olof's question before, other cores will be added and it should be separated from this. Ok. + enable-method = spin-table; + cpu-release-addr = 0x0 0x8000fff8; + }; + cpu@001 { + device_type = cpu; + compatible = arm,armv8; + reg = 0x0 0x001; + enable-method = spin-table; + cpu-release-addr = 0x0 0x8000fff8; + }; + cpu@002 { + device_type = cpu; + compatible = arm,armv8; + reg = 0x0 0x002; + enable-method = spin-table; + cpu-release-addr = 0x0 0x8000fff8; + }; + cpu@003 { + device_type = cpu; + compatible = arm,armv8; + reg = 0x0 0x003; + enable-method = spin-table; + cpu-release-addr = 0x0 0x8000fff8; + }; + }; + + gic: interrupt-controller@1C00 { + compatible = arm,cortex-a15-gic; We should have a more specific compatible string early in the list here too. Well, I think more specific compatible string is not required for gic, there were discussion about using another compatible string for gic-v2. And cortex-a15-gic should be fine at this moment and if another name is required as per previous discussion, we will then. While it's probably ok to have arm,cortex-a15-gic in the compatible list, it would be good to also
Re: [PATCHv2 8/8] devfreq: exynos4: Add busfreq driver for exynos4210/exynos4x12
On Thu, Mar 13, 2014 at 08:17:29AM +, Chanwoo Choi wrote: This patch add busfreq driver for Exynos4210/Exynos4x12 memory interface and bus to support DVFS(Dynamic Voltage Frequency Scaling) according to PPMU counters. PPMU (Performance Profiling Monitorings Units) of Exynos4 SoC provides PPMU counters for DMC(Dynamic Memory Controller) to check memory bus utilization and then busfreq driver adjusts dynamically the operating frequency/voltage by using DEVFREQ Subsystem. Signed-off-by: Chanwoo Choi cw00.c...@samsung.com --- .../devicetree/bindings/devfreq/exynos4_bus.txt| 49 ++ 1 file changed, 49 insertions(+) create mode 100644 Documentation/devicetree/bindings/devfreq/exynos4_bus.txt diff --git a/Documentation/devicetree/bindings/devfreq/exynos4_bus.txt b/Documentation/devicetree/bindings/devfreq/exynos4_bus.txt new file mode 100644 index 000..2a83fcc --- /dev/null +++ b/Documentation/devicetree/bindings/devfreq/exynos4_bus.txt @@ -0,0 +1,49 @@ + +Exynos4210/4x12 busfreq driver +- + +Exynos4210/4x12 Soc busfreq driver with devfreq for Memory bus frequency/voltage +scaling according to PPMU counters of memory controllers + +Required properties: +- compatible : should contain Exynos4 SoC type as follwoing: + - samsung,exynos4x12-busfreq for Exynos4x12 + - samsung,exynos4210-busfreq for Exynos4210 Is there a device called busfreq? What device does this binding describe? +- reg: offset and length of the ppmudmc0/1 + - PPMU (Performance Profiling Monitoring Units) You seem to require a particular order here. It would be good to be explicit about it. + : It is to profile performance event of DMC(Dynamic Memory + Controller) So, exynos4_bus.c can check memory bus utilization + by using PPMU of Exynos4 SoC. This is superfluous, and Linux-specific. The binding document shouldn't need to refer to drivers. +- clocks : clock number of ppmudmc0/1 +- clock-names: clock name of ppmudmc0/1 Are these two clocks, or one clock with a slash in the name? Please list each name separately. +- vdd_int-supply: regulator for interface block of Exynos4 + +Optional properties: +- vdd_mif-supply: regulator for DMC block of Exynos4x12 if Exynos4x12 Soc +- regs-name : register name of ppmudmc0/1 This is completely nonstandard. Did you mean reg-names? Please be explicit about the names you expect. Write them in full, quoted, and describe the relationship to the reg property. + +All the required listed above must be defined under code busfreq with devfreq I'm having some difficulty figuring out what exactly this is intended to mean. Thanks, Mark. -- 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 1/7] net: sxgbe: add basic framework for Samsung 10Gb ethernet driver
On Wed, Mar 12, 2014 at 01:28:00PM +, Byungho An wrote: From: Siva Reddy siva.kal...@samsung.com This patch adds support for Samsung 10Gb ethernet driver(sxgbe). - sxgbe core initialization - Tx and Rx support - MDIO support - ISRs for Tx and Rx - ifconfig support to driver Signed-off-by: Siva Reddy Kallam siva.kal...@samsung.com Signed-off-by: Vipul Pandya vipul.pan...@samsung.com Signed-off-by: Girish K S ks.g...@samsung.com Neatening-by: Joe Perches j...@perches.com Signed-off-by: Byungho An bh74...@samsung.com --- [...] diff --git a/Documentation/devicetree/bindings/net/samsung-sxgbe.txt b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt new file mode 100644 index 000..f2abf65 --- /dev/null +++ b/Documentation/devicetree/bindings/net/samsung-sxgbe.txt Please split the binding into a separate patch from the code (it makes it far easier for us DT guys to find that portions relevant to use). Is there any public documentation? @@ -0,0 +1,39 @@ +* Samsung 10G Ethernet driver (SXGBE) + +Required properties: +- compatible: Should be samsung,sxgbe-v2.0a +- reg: Address and length of the register set for the device +- interrupt-parent: Should be the phandle for the interrupt controller + that services interrupts for this device +- interrupts: Should contain the SXGBE interrupts How many, in which order, what are each of them for? +- samsung,burst-mapProgram the possible bursts supported by sxgbe +- samsung,fixed-burst Program the DMA to use the fixed burst mode +- samsung,adv-addr-modeprogram the DMA to use Enhanced address mode What are valid values? Units/types? Please describe what these actually do. +- samsung,force_thresh_dma_modeForce DMA to use the threshold mode for + both tx and rx Odd formatting here. s/_/-/ in property names please. When and why should this property be set in a dt? +- samsung,force_sf_dma_modeForce DMA to use the Store and Forward + mode for both tx and rx. This flag is + ignored if force_thresh_dma_mode is set. Likewise, for all points. [...] +/* Clean the tx descriptor as soon as the tx irq is received */ +static void sxgbe_release_tx_desc(struct sxgbe_tx_norm_desc *p) +{ + memset(p, 0, sizeof(struct sxgbe_tx_norm_desc)); You can use sizeof(*p) here. [...] +static int sxgbe_probe_config_dt(struct platform_device *pdev, +struct sxgbe_plat_data *plat, +const char **mac) +{ + struct device_node *np = pdev-dev.of_node; + struct sxgbe_dma_cfg *dma_cfg; + u32 phy_addr; + + if (!np) + return -ENODEV; + + *mac = of_get_mac_address(np); I see that of_get_mac_address returns a *void rather than *char, but it's always a string of hex digits. Would it make sense to change the of_get_mac_address prototype to return a char* ? + plat-interface = of_get_phy_mode(np); + + plat-bus_id = of_alias_get_id(np, ethernet); + if (plat-bus_id 0) + plat-bus_id = 0; This wasn't mentioned in the binding. + + of_property_read_u32(np, samsung,phy-addr, plat-phy_addr); Neither was this. + + plat-mdio_bus_data = devm_kzalloc(pdev-dev, + sizeof(struct sxgbe_mdio_bus_data), + GFP_KERNEL); + + if (of_device_is_compatible(np, samsung,sxgbe-v2.0a)) + plat-pmt = 1; Only one compatible string is documented. When would this _not_ be the case? [...] +static int sxgbe_platform_probe(struct platform_device *pdev) +{ + int ret = 0; + int loop = 0; + int index1, index2; + struct resource *res; + struct device *dev = pdev-dev; + void __iomem *addr = NULL; + struct sxgbe_priv_data *priv = NULL; + struct sxgbe_plat_data *plat_dat = NULL; + const char *mac = NULL; + int total_dma_channels = SXGBE_TX_QUEUES + SXGBE_RX_QUEUES; + + /* Get memory resource */ + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -ENODEV; + + addr = devm_ioremap_resource(dev, res); + if (IS_ERR(addr)) + return PTR_ERR(addr); + + plat_dat = pdev-dev.platform_data; This is a new dt-driven driver. Why would you need additional platform data? Why can this information not come from dt? + if (pdev-dev.of_node) { + if (!plat_dat) + plat_dat = devm_kzalloc(pdev-dev, + sizeof(struct sxgbe_plat_data), + GFP_KERNEL); + if (!plat_dat) + return -ENOMEM; + + ret = sxgbe_probe_config_dt(pdev, plat_dat, mac); +
Re: [PATCH v2 1/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board
On Mon, Mar 10, 2014 at 10:51:17PM +, Kukjin Kim wrote: Signed-off-by: Kukjin Kim kgene@samsung.com Reviewed-by: Thomas Abraham thomas...@samsung.com Cc: Catalin Marinas catalin.mari...@arm.com --- arch/arm64/boot/dts/samsung-gh7.dtsi | 106 +++ arch/arm64/boot/dts/samsung-ssdk-gh7.dts | 26 2 files changed, 132 insertions(+) create mode 100644 arch/arm64/boot/dts/samsung-gh7.dtsi create mode 100644 arch/arm64/boot/dts/samsung-ssdk-gh7.dts diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi b/arch/arm64/boot/dts/samsung-gh7.dtsi new file mode 100644 index 000..b5e8488 --- /dev/null +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi @@ -0,0 +1,106 @@ +/* + * SAMSUNG GH7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * 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. +*/ + +/memreserve/ 0x8000 0x0C40; Please have a comment as to what this memreserve is intended to protect. This is very large, and we don't want pointless memreserves. What address does the kernel end up getting loaded at on this board? + +/ { + interrupt-parent = gic; + #address-cells = 2; + #size-cells = 2; + + aliases { + serial0 = /amba/uart@12c0; + }; + + cpus { + #address-cells = 2; + #size-cells = 0; + + cpu@000 { + device_type = cpu; + compatible = arm,armv8; The arm,armv8 should be a fallback rather than the only entry. The precise core should be first (see arch/arm64/boot/dts/apm-storm.dtsi for an example). + reg = 0x0 0x000; Any reason for padding these to three hexadecimal digits (in both the reg and unit-address)? + enable-method = spin-table; + cpu-release-addr = 0x0 0x8000fff8; + }; + cpu@001 { + device_type = cpu; + compatible = arm,armv8; + reg = 0x0 0x001; + enable-method = spin-table; + cpu-release-addr = 0x0 0x8000fff8; + }; + cpu@002 { + device_type = cpu; + compatible = arm,armv8; + reg = 0x0 0x002; + enable-method = spin-table; + cpu-release-addr = 0x0 0x8000fff8; + }; + cpu@003 { + device_type = cpu; + compatible = arm,armv8; + reg = 0x0 0x003; + enable-method = spin-table; + cpu-release-addr = 0x0 0x8000fff8; + }; + }; + + gic: interrupt-controller@1C00 { + compatible = arm,cortex-a15-gic; We should have a more specific compatible string early in the list here too. + #interrupt-cells = 3; + interrupt-controller; + reg = 0x0 0x1C001000 0 0x1000,/* GIC Dist */ + 0x0 0x1C002000 0 0x1000,/* GIC CPU */ + 0x0 0x1C004000 0 0x2000,/* GIC VCPU Control */ + 0x0 0x1C006000 0 0x2000;/* GIC VCPU */ + interrupts = 1 9 0xf04; /* GIC Maintenence IRQ */ + }; + + timer { + compatible = arm,armv8-timer; + interrupts = 1 13 0xff01, /* Secure Phys IRQ */ + 1 14 0xff01, /* Non-secure Phys IRQ */ + 1 11 0xff01, /* Virt IRQ */ + 1 10 0xff01; /* Hyp IRQ */ + }; + + pmu { + compatible = arm,armv8-pmuv3; This is missing a specific compatible string. + interrupts = 0 294 0, + 0 295 0, + 0 296 0, + 0 297 0, + 0 298 0, + 0 299 0, + 0 300 0, + 0 301 0; + }; There are four CPUs described, yet eight interrupts here. There should be one per CPU. + + amba { + compatible = arm,amba-bus; + #address-cells = 2; + #size-cells = 2; + ranges; + + serial@12c0 { + compatible = arm,pl011, arm,primecell; + reg = 0 0x12c0 0 0x1; + interrupts = 0 418 0; + }; + + serial@12c2 { + compatible = arm,pl011, arm,primecell; + reg = 0 0x12c2 0 0x1; + interrupts = 0 420 0; + }; These are both missing
Re: [PATCH v4 03/10] Documentation: devicetree: Update Samsung FIMC DT binding
On Thu, Feb 20, 2014 at 07:40:30PM +, Sylwester Nawrocki wrote: This patch documents following updates of the Exynos4 SoC camera subsystem devicetree binding: - addition of #clock-cells property to 'camera' node - the #clock-cells property is needed when the sensor sub-devices use clock provided by the camera host interface; - addition of an optional clock-output-names property; - change of the clock-frequency at image sensor node from mandatory to an optional property - there should be no need to require this property by the camera host device binding, a default frequency value can ofen be used; - addition of a requirement of specific order of values in clocks/ clock-names properties, so the first two entry in the clock-names property can be used as parent clock names for the camera master clock provider. It happens all in-kernel dts files list the clock in such order, thus there should be no regression as far as in-kernel dts files are concerned. I'm not sure I follow the reasoning here. Why does this matter? Why can child nodes not get these by name if they have to? Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Acked-by: Kyungmin Park kyungmin.p...@samsung.com --- .../devicetree/bindings/media/samsung-fimc.txt | 36 +++- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/media/samsung-fimc.txt b/Documentation/devicetree/bindings/media/samsung-fimc.txt index 96312f6..1a5820d 100644 --- a/Documentation/devicetree/bindings/media/samsung-fimc.txt +++ b/Documentation/devicetree/bindings/media/samsung-fimc.txt @@ -20,6 +20,7 @@ Required properties: the clock-names property; - clock-names: must contain sclk_cam0, sclk_cam1, pxl_async0, pxl_async1 entries, matching entries in the clocks property. + First two entries must be sclk_cam0, sclk_cam1. I don't think this is a good idea. The pinctrl bindings defined in ../pinctrl/pinctrl-bindings.txt must be used to define a required pinctrl state named default and optional pinctrl states: @@ -32,6 +33,22 @@ way around. The 'camera' node must include at least one 'fimc' child node. +Optional properties (*: Is that a smiley face? + +- #clock-cells: from the common clock bindings (../clock/clock-bindings.txt), + must be 1. A clock provider is associated with the 'camera' node and it should + be referenced by external sensors that use clocks provided by the SoC on + CAM_*_CLKOUT pins. The clock specifier cell stores an index of a clock. + The indices are 0, 1 for CAM_A_CLKOUT, CAM_B_CLKOUT clocks respectively. + +- clock-output-names: from the common clock bindings, should contain names of + clocks registered by the camera subsystem corresponding to CAM_A_CLKOUT, + CAM_B_CLKOUT output clocks, in this order. Parent clock of these clocks are + specified be first two entries of the clock-names property. Do you need this? That's not how clock-names is supposed to work. The clock-names property is for the names of the _input_ clock lines on the device, not the output names on whichever parent clock they came from. Any clock-names property description should define absolutely the set of names. As this does not, NAK. + +(* #clock-cells and clock-output-names are mandatory properties if external +image sensor devices reference 'camera' device node as a clock provider. s/(*/Note:/ + 'fimc' device nodes --- @@ -97,8 +114,8 @@ Image sensor nodes The sensor device nodes should be added to their control bus controller (e.g. I2C0) nodes and linked to a port node in the csis or the parallel-ports node, using the common video interfaces bindings, defined in video-interfaces.txt. -The implementation of this bindings requires clock-frequency property to be -present in the sensor device nodes. +An optional clock-frequency property needs to be present in the sensor device +nodes. Default value when this property is not present is 24 MHz. s/needs to/should/ ? What is this the frequency of? Thanks, Mark. -- 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 v4 02/10] Documentation: dt: Add DT binding documentation for S5C73M3 camera
; + ... + }; +}; Otherwise I don't see anything problematic about the binding. Acked-by: Mark Rutland mark.rutl...@arm.com Cheers, Mark. -- 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 v4 07/10] exynos4-is: Add clock provider for the SCLK_CAM clock outputs
On Thu, Feb 20, 2014 at 07:40:34PM +, Sylwester Nawrocki wrote: This patch adds clock provider so the the SCLK_CAM0/1 output clocks can be accessed by image sensor devices through the clk API. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Acked-by: Kyungmin Park kyungmin.p...@samsung.com --- Changes since v3: - use clock-output-names DT property instead of hard coding names of registered clocks in the driver; first two entries of the clock-names property value are used to specify parent clocks of cam_{a,b}_clkout clocks, rather than hard coding it to sclk_cam{0,1} in the driver. - addressed issues pointed out in review by Mauro. Changes since v2: - use 'camera' DT node drirectly as clock provider node, rather than creating additional clock-controller child node. - DT binding documentation moved to a separate patch. --- drivers/media/platform/exynos4-is/media-dev.c | 110 + drivers/media/platform/exynos4-is/media-dev.h | 19 - 2 files changed, 128 insertions(+), 1 deletion(-) [...] +static int fimc_md_register_clk_provider(struct fimc_md *fmd) +{ + struct cam_clk_provider *cp = fmd-clk_provider; + struct device *dev = fmd-pdev-dev; + int i, ret; + + for (i = 0; i ARRAY_SIZE(cp-clks); i++) { + struct cam_clk *camclk = cp-camclk[i]; + struct clk_init_data init; + + ret = of_property_read_string_index(dev-of_node, + clock-output-names, i, init.name); Are there not well-defined names for the clock outputs of the block? + if (ret 0) + break; + + ret = of_property_read_string_index(dev-of_node, + clock-names, i, init.parent_names); This shouldn't be a parent name. It should be the input line name. I don't think this makes sense. Why do you need the name of the parent clock? + if (ret 0) + break; + + init.num_parents = 1; + init.ops = cam_clk_ops; + init.flags = CLK_SET_RATE_PARENT; + camclk-hw.init = init; + camclk-fmd = fmd; + + cp-clks[i] = clk_register(NULL, camclk-hw); + if (IS_ERR(cp-clks[i])) { + dev_err(dev, failed to register clock: %s (%ld)\n, + init.name, PTR_ERR(cp-clks[i])); + ret = PTR_ERR(cp-clks[i]); + goto err; + } + cp-num_clocks++; + } + + if (cp-num_clocks == 0) { + dev_warn(dev, clk provider not registered\n); + return 0; + } + + cp-clk_data.clks = cp-clks; + cp-clk_data.clk_num = cp-num_clocks; + cp-of_node = dev-of_node; + ret = of_clk_add_provider(dev-of_node, of_clk_src_onecell_get, + cp-clk_data); Are _all_ of the input clock lines available to children in hardware? The binding and commit message(s) implied only two clocks were, so what's the point in exporting clocks which aren't available? Thanks, Mark. -- 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] V4L: s5k6a3: Add DT binding documentation
On Tue, Feb 18, 2014 at 03:43:11PM +, Sylwester Nawrocki wrote: On 22/12/13 22:27, Sylwester Nawrocki wrote: This patch adds DT binding documentation for the Samsung S5K6A3(YX) raw image sensor. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- This patch adds missing documentation [1] for the samsung,s5k6a3 compatible. Rob, can you please merge it through your tree if it looks OK ? Anyone cares to Ack this patch so it can be merged through the media tree ? It looks fine to me: Acked-by: Mark Rutland mark.rutl...@arm.com Thanks, Mark. Thanks, Sylwester [1] http://www.spinics.net/lists/devicetree/msg10693.html Changes since v3 (https://linuxtv.org/patch/20429): - rephrased 'clocks' and 'clock-names' properties' description. --- .../devicetree/bindings/media/samsung-s5k6a3.txt | 33 1 files changed, 33 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k6a3.txt diff --git a/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt b/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt new file mode 100644 index 000..cce01e8 --- /dev/null +++ b/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt @@ -0,0 +1,33 @@ +Samsung S5K6A3(YX) raw image sensor +- + +S5K6A3(YX) is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces +and CCI (I2C compatible) control bus. + +Required properties: + +- compatible : samsung,s5k6a3; +- reg : I2C slave address of the sensor; +- svdda-supply : core voltage supply; +- svddio-supply: I/O voltage supply; +- afvdd-supply : AF (actuator) voltage supply; +- gpios: specifier of a GPIO connected to the RESET pin; +- clocks : should contain list of phandle and clock specifier pairs + according to common clock bindings for the clocks described + in the clock-names property; +- clock-names : should contain extclk entry for the sensor's EXTCLK clock; + +Optional properties: + +- clock-frequency : the frequency at which the extclk clock should be + configured to operate, in Hz; if this property is not + specified default 24 MHz value will be used. + +The common video interfaces bindings (see video-interfaces.txt) should be +used to specify link to the image data receiver. The S5K6A3(YX) device +node should contain one 'port' child node with an 'endpoint' subnode. + +Following properties are valid for the endpoint node: + +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in + video-interfaces.txt. The sensor supports only one data lane. -- 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/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board
+ gic: interrupt-controller@1C00 { + compatible = arm,cortex-a15-gic, arm,cortex-a9-gic; This looks incorrect -- you should at the very least have a more specific one than a15-gic? Marc? arm,cortex-a9-gic is definitely wrong (the A9 GIC doesn't have the virt extensions). This binding matches what the A15 GIC has, so arm,cortex-a15-gic is probably fine. Main issue here is that the GICv2 driver has no compatible string for anything else. Should we define something more generic (like arm,gic-v2)? Or carry on adding more compatible strings? It's been proposed repeatedly, and it probably makes sense to add the generic versions to the driver, and allow for more specific ones in the binding which DTs can use. That way we don't get an explosion of strings in the driver, but if we need to handle any particular GIC specially in future we can do so. I guess for Linux we'd want to add arm,gic-v1 and arm,gic-v2 to the driver. We could just add arm,gic-v1 and expect it later in the compatible list if v2 is a strict superset of v1; I think it is but I'm not a GIC expert. Thanks, Mark. -- 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/3] arm64: dts: add initial dts for Samsung GH7 SoC and SSDK-GH7 board
On Tue, Feb 11, 2014 at 06:29:41AM +, Kukjin Kim wrote: Signed-off-by: Kukjin Kim kgene@samsung.com Reviewed-by: Thomas Abraham thomas...@samsung.com Cc: Catalin Marinas catalin.mari...@arm.com --- arch/arm64/boot/dts/samsung-gh7.dtsi | 108 ++ arch/arm64/boot/dts/samsung-ssdk-gh7.dts | 26 +++ 2 files changed, 134 insertions(+) create mode 100644 arch/arm64/boot/dts/samsung-gh7.dtsi create mode 100644 arch/arm64/boot/dts/samsung-ssdk-gh7.dts diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi b/arch/arm64/boot/dts/samsung-gh7.dtsi new file mode 100644 index 000..5b8785c --- /dev/null +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi @@ -0,0 +1,108 @@ +/* + * SAMSUNG GH7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * 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. +*/ + +/dts-v1/; + +/memreserve/ 0x8000 0x0C40; That looks _very_ large. What is this for? [...] + timer { + compatible = arm,armv8-timer; + interrupts = 1 13 0xff01, /* Secure Phys IRQ */ + 1 14 0xff01, /* Non-secure Phys IRQ */ + 1 11 0xff01, /* Virt IRQ */ + 1 10 0xff01; /* Hyp IRQ */ + clock-frequency = 1; Please, get your bootloader to set CNTFREQ. The clock frequency property for the timer node is a horrible hack for buggy firmware. [...] + amba { + compatible = arm,amba-bus; + #address-cells = 1; + #size-cells = 1; + ranges; + + serial@12c0 { + compatible = arm,pl011, arm,primecell; + reg = 0x12c0 0x1; + interrupts = 418; + }; + + serial@12c2 { + compatible = arm,pl011, arm,primecell; + reg = 0x12c2 0x1; + interrupts = 420; + }; Don't these need clocks? [...] + memory@8000 { + device_type = memory; + reg = 0x 0x8000 0 0x8000; Minor nit, but it would be nice for the 0 values to be consistently padded. Thanks, Mark. -- 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] thermal: exynos: handle gate clock for misplaced TRIMINFO register
On Thu, Nov 07, 2013 at 12:42:34PM +, Naveen Krishna Chatradhi wrote: On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second should be acompanied by enabling the respective clock. This patch which allow for a clk_sec clock to be specified in the device-tree which will be ungated when accessing the TRIMINFO register. Missing binding document update? Or was clk_sec originally in the binding but unused? The code seems to expect tmu_apbif_sec as the clock name in the DT, but this isn't mentioned in the commit message. I grepped Documentation/devicetree in mainline, but found no reference of either. Thanks, Mark. Signed-off-by: Andrew Bresticker abres...@chromium.org Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com --- drivers/thermal/samsung/exynos_tmu.c | 24 +++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index b54825a..33edd1a 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -47,6 +47,7 @@ * @irq_work: pointer to the irq work structure. * @lock: lock to implement synchronization. * @clk: pointer to the clock structure. + * @clk_sec: pointer to the clock structure for accessing the base_second. * @temp_error1: fused value of the first point trim. * @temp_error2: fused value of the second point trim. * @regulator: pointer to the TMU regulator structure. @@ -61,7 +62,7 @@ struct exynos_tmu_data { enum soc_type soc; struct work_struct irq_work; struct mutex lock; - struct clk *clk; + struct clk *clk, *clk_sec; u8 temp_error1, temp_error2; struct regulator *regulator; struct thermal_sensor_conf *reg_conf; @@ -152,6 +153,8 @@ static int exynos_tmu_initialize(struct platform_device *pdev) mutex_lock(data-lock); clk_enable(data-clk); + if (!IS_ERR(data-clk_sec)) + clk_enable(data-clk_sec); if (TMU_SUPPORTS(pdata, READY_STATUS)) { status = readb(data-base + reg-tmu_status); @@ -306,6 +309,8 @@ skip_calib_data: out: clk_disable(data-clk); mutex_unlock(data-lock); + if (!IS_ERR(data-clk_sec)) + clk_disable(data-clk_sec); return ret; } @@ -457,12 +462,16 @@ static void exynos_tmu_work(struct work_struct *work) const struct exynos_tmu_registers *reg = pdata-registers; unsigned int val_irq, val_type; + if (!IS_ERR(data-clk_sec)) + clk_enable(data-clk_sec); /* Find which sensor generated this interrupt */ if (reg-tmu_irqstatus) { val_type = readl(data-base_second + reg-tmu_irqstatus); if (!((val_type data-id) 0x1)) goto out; } + if (!IS_ERR(data-clk_sec)) + clk_disable(data-clk_sec); exynos_report_trigger(data-reg_conf); mutex_lock(data-lock); @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev) if (ret) return ret; + data-clk_sec = devm_clk_get(pdev-dev, tmu_apbif_sec); + if (!IS_ERR(data-clk_sec)) { + ret = clk_prepare(data-clk_sec); + if (ret) { + dev_err(pdev-dev, Failed to get clock\n); + return PTR_ERR(data-clk_sec); + } + } + if (pdata-type == SOC_ARCH_EXYNOS4210 || pdata-type == SOC_ARCH_EXYNOS4412 || pdata-type == SOC_ARCH_EXYNOS5250 || @@ -713,6 +731,8 @@ static int exynos_tmu_probe(struct platform_device *pdev) return 0; err_clk: clk_unprepare(data-clk); + if (!IS_ERR(data-clk_sec)) + clk_unprepare(data-clk_sec); return ret; } @@ -725,6 +745,8 @@ static int exynos_tmu_remove(struct platform_device *pdev) exynos_unregister_thermal(data-reg_conf); clk_unprepare(data-clk); + if (!IS_ERR(data-clk_sec)) + clk_unprepare(data-clk_sec); if (!IS_ERR(data-regulator)) regulator_disable(data-regulator); -- 1.7.10.4 -- 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 -- 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] thermal: exynos: handle gate clock for misplaced TRIMINFO register
On Mon, Feb 10, 2014 at 10:50:01AM +, Naveen Krishna Ch wrote: Hello Mark, On 10 February 2014 16:03, Mark Rutland mark.rutl...@arm.com wrote: On Thu, Nov 07, 2013 at 12:42:34PM +, Naveen Krishna Chatradhi wrote: On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second should be acompanied by enabling the respective clock. This patch which allow for a clk_sec clock to be specified in the device-tree which will be ungated when accessing the TRIMINFO register. Missing binding document update? Or was clk_sec originally in the binding but unused? The code seems to expect tmu_apbif_sec as the clock name in the DT, but this isn't mentioned in the commit message. I grepped Documentation/devicetree in mainline, but found no reference of either. Thanks, Mark. This CL is to be abandoned. Ok. As mentioned in the previous replies to this patch. The changes in this patched were merged with http://www.spinics.net/lists/devicetree/msg15165.html The latest patch set can be found at. 1. http://www.spinics.net/lists/devicetree/msg15163.html 2. http://www.spinics.net/lists/devicetree/msg15164.html 3. http://www.spinics.net/lists/devicetree/msg15165.html 4. http://www.spinics.net/lists/devicetree/msg15165.html I responded here because of your ping message on 2014-02-07. The latest patches seem to have been posted before that. Was the ping misplaced or have I misunderstood? Thanks, Mark -- 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 v10 1/2] [media] exynos5-is: Adds DT binding documentation
On Mon, Feb 03, 2014 at 10:13:55AM +, Arun Kumar K wrote: Hi Mark, Hi Arun, This patch and hence a full series of 13 patches is waiting for a long time now due to your missing ack on this DT binding patch. I have addressed your review comments given on earlier version - http://www.spinics.net/lists/devicetree/msg11550.html Please check this and give an ack if it is fine to be merged. Apologies for the delay. As far as I can tell this looks ok: Acked-by: Mark Rutland mark.rutl...@arm.com Regards Arun On Fri, Dec 13, 2013 at 10:42 AM, Arun Kumar K arun...@samsung.com wrote: From: Shaik Ameer Basha shaik.am...@samsung.com The patch adds the DT binding doc for exynos5 SoC camera subsystem. Signed-off-by: Shaik Ameer Basha shaik.am...@samsung.com Signed-off-by: Arun Kumar K arun...@samsung.com --- .../bindings/media/exynos5250-camera.txt | 136 1 file changed, 136 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/exynos5250-camera.txt diff --git a/Documentation/devicetree/bindings/media/exynos5250-camera.txt b/Documentation/devicetree/bindings/media/exynos5250-camera.txt new file mode 100644 index 000..0c36bc4 --- /dev/null +++ b/Documentation/devicetree/bindings/media/exynos5250-camera.txt @@ -0,0 +1,136 @@ +Samsung EXYNOS5 SoC Camera Subsystem + + +The Exynos5 SoC Camera subsystem comprises of multiple sub-devices +represented by separate device tree nodes. Currently this includes: FIMC-LITE, +MIPI CSIS and FIMC-IS. + +The sub-device nodes are referenced using phandles in the common 'camera' node +which also includes common properties of the whole subsystem not really +specific to any single sub-device, like common camera port pins or the common +camera bus clocks. + +Common 'camera' node + + +Required properties: + +- compatible : must be samsung,exynos5250-fimc +- clocks : list of phandles and clock specifiers, corresponding + to entries in the clock-names property +- clock-names : must contain sclk_bayer entry +- samsung,csis : list of phandles to the mipi-csis device nodes +- samsung,fimc-lite: list of phandles to the fimc-lite device nodes +- samsung,fimc-is : phandle to the fimc-is device node + +The pinctrl bindings defined in ../pinctrl/pinctrl-bindings.txt must be used +to define a required pinctrl state named default. + +'parallel-ports' node +- + +This node should contain child 'port' nodes specifying active parallel video +input ports. It includes camera A, camera B and RGB bay inputs. +'reg' property in the port nodes specifies the input type: + 1 - parallel camport A + 2 - parallel camport B + 5 - RGB camera bay + +3, 4 are for MIPI CSI-2 bus and are already described in samsung-mipi-csis.txt + +Required properties: + +For describing the input type in the child nodes, the following properties +have to be present in the parallel-ports node: +- #address-cells: Must be 1 +- #size-cells: Must be 0 + +Image sensor nodes +-- + +The sensor device nodes should be added to their control bus controller (e.g. +I2C0) nodes and linked to a port node in the csis or the parallel-ports node, +using the common video interfaces bindings, defined in video-interfaces.txt. + +Example: + + aliases { + fimc-lite0 = fimc_lite_0 + }; + + /* Parallel bus IF sensor */ + i2c_0: i2c@1386 { + s5k6aa: sensor@3c { + compatible = samsung,s5k6aafx; + reg = 0x3c; + vddio-supply = ...; + + clock-frequency = 2400; + clocks = ...; + clock-names = mclk; + + port { + s5k6aa_ep: endpoint { + remote-endpoint = fimc0_ep; + bus-width = 8; + hsync-active = 0; + vsync-active = 1; + pclk-sample = 1; + }; + }; + }; + }; + + /* MIPI CSI-2 bus IF sensor */ + s5c73m3: sensor@1a { + compatible = samsung,s5c73m3; + reg = 0x1a; + vddio-supply = ...; + + clock-frequency = 2400; + clocks = ...; + clock-names = mclk; + + port { + s5c73m3_1: endpoint { + data-lanes = 1 2 3 4
Re: [PATCH 2/8 v4] crypto:s5p-sss: Add device tree support
On Thu, Jan 23, 2014 at 05:47:25PM +, Sylwester Nawrocki wrote: On 23/01/14 18:41, Mark Rutland wrote: diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c index 93cddeb..2da5617 100644 --- a/drivers/crypto/s5p-sss.c +++ b/drivers/crypto/s5p-sss.c @@ -22,6 +22,7 @@ #include linux/scatterlist.h #include linux/dma-mapping.h #include linux/io.h +#include linux/of.h #include linux/crypto.h #include linux/interrupt.h @@ -177,6 +178,12 @@ struct s5p_aes_dev { static struct s5p_aes_dev *s5p_dev; +static const struct of_device_id s5p_sss_dt_match[] = { + { .compatible = samsung,s5pv210-secss, }, nit: the first semicolon could be omitted. I assume you mean comma ratehr than semicolon? Indeed...and it's actually the second one :-/ Also, I meant rather rather than ratehr. At least we figured it out in the end. :) Thanks, Mark. -- 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 v9] s5k5baf: add camera sensor driver
On Wed, Dec 04, 2013 at 11:17:43PM +, Sylwester Nawrocki wrote: On 10/31/2013 04:29 PM, Andrzej Hajda wrote: Driver for Samsung S5K5BAF UXGA 1/5 2M CMOS Image Sensor with embedded SoC ISP. The driver exposes the sensor as two V4L2 subdevices: - S5K5BAF-CIS - pure CMOS Image Sensor, fixed 1600x1200 format, no controls. - S5K5BAF-ISP - Image Signal Processor, formats up to 1600x1200, pre/post ISP cropping, downscaling via selection API, controls. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Andrzej Hajdaa.ha...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- Hi, This is the 9th iteration of the patch. In this iteration 'binary' blobs from source file have been moved to separate firmware file. Firmware file will be uploaded to appropriate repository. [...] v9 - patch, ccm and cis configuration blobs moved to firmware set files, - minor improvements of bindings, - cosmetic changes Hi Mark, Hi Sylwester, What do you think about this DT binding now, could we have your Ack ? Other than a minor nit below, the binding looks fine to me. With the fixed, for the binding: Acked-by: Mark Rutland mark.rutl...@arm.com I think we still need to move the DT binding into a separate patch. If you're going to post the patch again, then please do split the binding into a separate patch. [...] diff --git a/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt new file mode 100644 index 000..23ebe0f --- /dev/null +++ b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt @@ -0,0 +1,57 @@ +Samsung S5K5BAF UXGA 1/5 2M CMOS Image Sensor with embedded SoC ISP + + +Required properties: + +- compatible : samsung,s5k5baf; +- reg : I2C slave address of the sensor; +- vdda-supply : analog power supply 2.8V (2.6V to 3.0V); +- vddreg-supply: regulator input power supply 1.8V (1.7V to 1.9V) + or 2.8V (2.6V to 3.0); +- vddio-supply : I/O power supply 1.8V (1.65V to 1.95V) + or 2.8V (2.5V to 3.1V); +- stbyn-gpios : GPIO connected to STDBYN pin; +- rstn-gpios : GPIO connected to RSTN pin; +- clocks : clock-specifiers (per the common clock bindings) for the + clocks described in clock-names; Clocks are referred to by phandle + clock-specifier pairs rather than just clock-specifiers, it would be nice to fix up the terminology here. Thanks, Mark. -- 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/1] ARM: dts: Add missing clock names for exynos4412 dwmmc node
On Sun, Dec 01, 2013 at 10:11:47AM +, Alex Ling wrote: This patch adds biu and ciu clock names for exynos4412 dwmmc node. Without this patch, dwmmc host driver will skip enabling the two clocks and it will break dwmmc host function on exynos4412. Tested on FriendlyARM TINY4412 board. The patch description is a little off; this patch adds the clocks as well as the clock-names. The patch itself looks fine to me. Mark. Signed-off-by: Alex Ling kasiml...@gmail.com --- arch/arm/boot/dts/exynos4412.dtsi |2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi index e743e67..67bef99 100644 --- a/arch/arm/boot/dts/exynos4412.dtsi +++ b/arch/arm/boot/dts/exynos4412.dtsi @@ -67,5 +67,7 @@ interrupts = 0 77 0; #address-cells = 1; #size-cells = 0; + clocks = clock 301, clock 149; + clock-names = biu, ciu; }; }; -- 1.7.9.5 -- 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/4 v9] thermal: samsung: Add TMU support for Exynos5420 SoCs
On Mon, Nov 18, 2013 at 03:22:57AM +, Naveen Krishna Ch wrote: Hello All, On 12 November 2013 12:07, Naveen Krishna Chatradhi ch.nav...@samsung.com wrote: Exynos5420 has 5 TMU channels, the TRIMINFO register is misplaced for TMU channels 2, 3 and 4 TRIMINFO at 0x1006c000 contains data for TMU channel 3 TRIMINFO at 0x100a contains data for TMU channel 4 TRIMINFO at 0x10068000 contains data for TMU channel 2 This patch 1 Adds the neccessary register changes and arch information to support Exynos5420 SoCs. 2. Handles the gate clock for misplaced TRIMINFO register 3. Updates the Documentation at Documentation/devicetree/bindings/thermal/exynos-thermal.txt Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com Signed-off-by: Andrew Bresticker abres...@chromium.org --- Changes since v8: 1. rewrote the Documentation for device tree bindings 2. Merged the https://lkml.org/lkml/2013/11/7/262 (as this is a fix) 3. introduces samsung,exynos5420-tmu-triminfo and samsung,exynos5420-tmu-triminfo-clk to handle the TMU channels on Exynos5420 more appropriately .../devicetree/bindings/thermal/exynos-thermal.txt | 45 + drivers/thermal/samsung/exynos_tmu.c | 58 ++- drivers/thermal/samsung/exynos_tmu.h |2 + drivers/thermal/samsung/exynos_tmu_data.c | 106 drivers/thermal/samsung/exynos_tmu_data.h |8 ++ 5 files changed, 215 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt index 116cca0..5055b31 100644 --- a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt +++ b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt @@ -6,6 +6,11 @@ samsung,exynos4412-tmu samsung,exynos4210-tmu samsung,exynos5250-tmu + samsung,exynos5420-tmu for TMU channel 0, 1 on Exynos5420 + samsung,exynos5420-tmu-triminfo for TMU channel 2 Exynos5420 + (Must pass triminfo base) + samsung,exynos5420-tmu-triminfo-clk for TMU channel 3 and 4 + Exynos5420 (Must pass triminfo base and triminfo clock) While this mentions what the differences the binding expects to, what is fundamentally different about these blocks? samsung,exynos5440-tmu - interrupt-parent : The phandle for the interrupt controller - reg : Address range of the thermal registers. For soc's which has multiple @@ -13,6 +18,18 @@ interrupt related then 2 set of register has to supplied. First set belongs to each instance of TMU and second set belongs to second set of common TMU registers. + + NOTE: On Exynos5420, the TRIMINFO register is misplaced for TMU + channels 2, 3 and 4 + Use samsung,exynos5420-tmu-triminfo in cases, there is a misplaced + register but no need of another clock to access that base. + Use samsung,exynos5420-tmu-triminfo-clk in cases where there is a misplaced + register and we need another clock to access that base. This would have made more sense to have next to the compatible strings. Instead of this, you could have a boolean property (samsung,misplaced-triminfo) to describe the misplaced triminfo register(s), and the presence of an additional clock would imlpy that it's required for accessing triminfo register(s). + + TRIMINFO at 0x1006c000 contains data for TMU channel 3 + TRIMINFO at 0x100a contains data for TMU channel 4 + TRIMINFO at 0x10068000 contains data for TMU channel 2 + - interrupts : Should contain interrupt for thermal system - clocks : The main clock for TMU device - clock-names : Thermal system clock name @@ -43,6 +60,34 @@ Example 2): clock-names = tmu_apbif; }; I'd have expected the clock-names to have an addition. The names of the clock inputs are presumably well known, or you should be able to come up with some sensible names... +Example 3): (In case of Exynos5420 with misplaced TRIMINFO register) + /* tmu for CPU2 */ + tmu@10068000 { + compatible = samsung,exynos5420-tmu-triminfo; + reg = 0x10068000 0x100, 0x1006c000 0x4; + interrupts = 0 184 0; + clocks = clock 318; + clock-names = tmu_apbif; + }; + + /* tmu for CPU3 */ + tmu@1006c000 { + compatible = samsung,exynos5420-tmu-triminfo-clk; + reg = 0x1006c000 0x100, 0x100a 0x4; + interrupts = 0 185 0; + clocks = clock 318; + clock-names = tmu_apbif, tmu_triminfo_apbif; Each clock-names entry should correspond to an entry in clocks.
Re: [PATCH v9 01/13] [media] exynos5-is: Adding media device driver for exynos5
On Fri, Sep 27, 2013 at 11:59:06AM +0100, Arun Kumar K wrote: From: Shaik Ameer Basha shaik.am...@samsung.com This patch adds support for media device for EXYNOS5 SoCs. The current media device supports the following ips to connect through the media controller framework. [...] diff --git a/Documentation/devicetree/bindings/media/exynos5250-camera.txt b/Documentation/devicetree/bindings/media/exynos5250-camera.txt new file mode 100644 index 000..09420ba --- /dev/null +++ b/Documentation/devicetree/bindings/media/exynos5250-camera.txt @@ -0,0 +1,126 @@ +Samsung EXYNOS5 SoC Camera Subsystem + + +The Exynos5 SoC Camera subsystem comprises of multiple sub-devices +represented by separate device tree nodes. Currently this includes: FIMC-LITE, +MIPI CSIS and FIMC-IS. + +The sub-device nodes are referenced using phandles in the common 'camera' node +which also includes common properties of the whole subsystem not really +specific to any single sub-device, like common camera port pins or the common +camera bus clocks. + +Common 'camera' node + + +Required properties: + +- compatible : must be samsung,exynos5250-fimc +- clocks : list of clock specifiers, corresponding to entries in + the clock-names property Minor nit: clocks are references by phandle + clock-specifier pairs, as the clock-specifier is separate from the phandle to the clock. +- clock-names : must contain sclk_bayer entry +- samsung,csis : list of phandles to the mipi-csis device nodes +- samsung,fimc-lite: list of phandles to the fimc-lite device nodes +- samsung,fimc-is : phandle to the fimc-is device node + +The pinctrl bindings defined in ../pinctrl/pinctrl-bindings.txt must be used +to define a required pinctrl state named default. + +'parallel-ports' node +- + +This node should contain child 'port' nodes specifying active parallel video +input ports. It includes camera A, camera B and RGB bay inputs. +'reg' property in the port nodes specifies the input type: + 1 - parallel camport A + 2 - parallel camport B + 5 - RGB camera bay + +3, 4 are for MIPI CSI-2 bus and are already described in samsung-mipi-csis.txt I believe the parallel ports node must have #address-cells and #size-cells defined for the child nodes' reg properties to be meaningful. Judging by the examples and code, it seems you expect #address-cells = 1 and #size-cells = 0. It would be nice to have that described. Cheers, Mark. -- 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 v9 02/13] [media] exynos5-fimc-is: Add Exynos5 FIMC-IS device tree bindings documentation
On Fri, Sep 27, 2013 at 11:59:07AM +0100, Arun Kumar K wrote: The patch adds the DT binding documentation for Samsung Exynos5 SoC series imaging subsystem (FIMC-IS). Signed-off-by: Arun Kumar K arun...@samsung.com Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com --- .../devicetree/bindings/media/exynos5-fimc-is.txt | 84 1 file changed, 84 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/exynos5-fimc-is.txt diff --git a/Documentation/devicetree/bindings/media/exynos5-fimc-is.txt b/Documentation/devicetree/bindings/media/exynos5-fimc-is.txt new file mode 100644 index 000..0525417 --- /dev/null +++ b/Documentation/devicetree/bindings/media/exynos5-fimc-is.txt @@ -0,0 +1,84 @@ +Samsung EXYNOS5 SoC series Imaging Subsystem (FIMC-IS) +-- + +The camera subsystem on Samsung Exynos5 SoC has some changes relative +to previous SoC versions. Exynos5 has almost similar MIPI-CSIS and +FIMC-LITE IPs but has a much improved version of FIMC-IS which can +handle sensor controls and camera post-processing operations. The +Exynos5 FIMC-IS has a dedicated ARM Cortex A5 processor, many +post-processing blocks (ISP, DRC, FD, ODC, DIS, 3DNR) and two +dedicated scalers (SCC and SCP). + +fimc-is node + + +Required properties: + +- compatible: must be samsung,exynos5250-fimc-is s/must be/should contain/ +- reg : physical base address and size of the memory mapped + registers +- interrupt-parent : parent interrupt controller Is this actually necessary? Is it not implicit in most cases? +- interrupts: fimc-is interrupt to the parent interrupt controller - interrupts: interrupt-specifier for the sole interrupt generated by the device. +- clocks: list of clock specifiers, corresponding to entries in + clock-names property - clocks: A list of phandle + clock-specifier pairs corresponding to entries in clock-names +- clock-names : must contain isp, mcu_isp, isp_div0, isp_div1, + isp_divmpwm, mcu_isp_div0, mcu_isp_div1 entries, + matching entries in the clocks property +- samsung,pmu : phandle to the Power Management Unit (PMU) node + It may be worth point out that #address-cells, #size-cells, and ranges need to be present as approriate for mapping sub-nodes. +i2c-isp (ISP I2C bus controller) nodes +-- +The i2c-isp node is defined as the child node of fimc-is. + +Required properties: + +- compatible : should be samsung,exynos4212-i2c-isp for Exynos4212, + Exynos4412 and Exynos5250 SoCs s/should be/should contain/ +- reg: physical base address and length of the registers set +- clocks : must contain gate clock specifier for this controller +- clock-names: must contain i2c_isp entry Please reword clocks to be in terms of clock-names, as above. e.g. - clocks: A list of phandle + clock-specifier pairs corresponding to entries in clock-names - clock-names: Should contain i2c_isp fot the gate clock Otherwise, I think this looks ok. Cheers, Mark. -- 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] Documentation/watchdog: Update node name in samsung-wdt example
On Mon, Oct 28, 2013 at 06:24:17AM +, Sachin Kamat wrote: Update the name as per DT naming convention. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Acked-by: Mark Rutland mark.rutl...@arm.com --- .../devicetree/bindings/watchdog/samsung-wdt.txt |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt index 4c798e3..0642fb8 100644 --- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt +++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt @@ -17,7 +17,7 @@ Optional properties: Example: -watchdog { +watchdog@101D { compatible = samsung,s3c2410-wdt; reg = 0x101D 0x100, 0x10040408 0x4, 0x1004040c 0x4; interrupts = 0 42 0; -- 1.7.9.5 -- 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 -- 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 v10 01/12] exynos5-fimc-is: Add Exynos5 FIMC-IS device tree bindings documentation
Hi, Apologies for the late reply. I have a few comments, but nothing major. On Fri, Oct 18, 2013 at 06:37:28AM +0100, Arun Kumar K wrote: The patch adds the DT binding documentation for Samsung Exynos5 SoC series imaging subsystem (FIMC-IS). Signed-off-by: Arun Kumar K arun...@samsung.com Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com --- .../devicetree/bindings/media/exynos5-fimc-is.txt | 84 1 file changed, 84 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/exynos5-fimc-is.txt diff --git a/Documentation/devicetree/bindings/media/exynos5-fimc-is.txt b/Documentation/devicetree/bindings/media/exynos5-fimc-is.txt new file mode 100644 index 000..0525417 --- /dev/null +++ b/Documentation/devicetree/bindings/media/exynos5-fimc-is.txt @@ -0,0 +1,84 @@ +Samsung EXYNOS5 SoC series Imaging Subsystem (FIMC-IS) +-- + +The camera subsystem on Samsung Exynos5 SoC has some changes relative +to previous SoC versions. Exynos5 has almost similar MIPI-CSIS and +FIMC-LITE IPs but has a much improved version of FIMC-IS which can +handle sensor controls and camera post-processing operations. The +Exynos5 FIMC-IS has a dedicated ARM Cortex A5 processor, many +post-processing blocks (ISP, DRC, FD, ODC, DIS, 3DNR) and two +dedicated scalers (SCC and SCP). + +fimc-is node + + +Required properties: + +- compatible: must be samsung,exynos5250-fimc-is s/must be/should contain/ +- reg : physical base address and size of the memory mapped + registers +- interrupt-parent : parent interrupt controller I don't think this is actually required in all cases. It's a standard property that people can add if it happens to be required in a particular instance. +- interrupts: fimc-is interrupt to the parent interrupt controller Is this the only interrupt the device generates? If so: - interrupts: interrupt-specifier for the fimc-is interrupt. +- clocks: list of clock specifiers, corresponding to entries in + clock-names property +- clock-names : must contain isp, mcu_isp, isp_div0, isp_div1, + isp_divmpwm, mcu_isp_div0, mcu_isp_div1 entries, + matching entries in the clocks property +- samsung,pmu : phandle to the Power Management Unit (PMU) node + +i2c-isp (ISP I2C bus controller) nodes +-- +The i2c-isp node is defined as the child node of fimc-is. There are multiple of these, so how about the following instead: The i2c-isp nodes should be children of the fimc-is node. It might be worth pointing out that ranges, #address-cells, and #size-cells should be present as appropriate. + +Required properties: + +- compatible : should be samsung,exynos4212-i2c-isp for Exynos4212, + Exynos4412 and Exynos5250 SoCs Similarly, s/should be/must contain/ +- reg: physical base address and length of the registers set +- clocks : must contain gate clock specifier for this controller +- clock-names: must contain i2c_isp entry I'd prefer clocks to be described as for the simc-is, with clock names being something like: - clock-names: should contain i2c_isp for the gate clock. + +For the i2c-isp node, it is required to specify a pinctrl state named default, +according to the pinctrl bindings defined in ../pinctrl/pinctrl-bindings.txt. I'd prefer a mention of pinctrl-0 and pinctrl-names, as that's what most other bindings do. Also, as this is described as required it should be in the example. + +Device tree nodes of the image sensors controlled directly by the FIMC-IS +firmware must be child nodes of their corresponding ISP I2C bus controller node. +The data link of these image sensors must be specified using the common video +interfaces bindings, defined in video-interfaces.txt. These don't seem to be in the example and probably should be. Otherwise, this looks fine. With those points fixed up: Acked-by: Mark Rutland mark.rutl...@arm.com Thanks, Mark. -- 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 v10 11/12] V4L: Add DT binding doc for s5k4e5 image sensor
On Fri, Oct 18, 2013 at 06:37:38AM +0100, Arun Kumar K wrote: S5K4E5 is a Samsung raw image sensor controlled via I2C. This patch adds the DT binding documentation for the same. Signed-off-by: Arun Kumar K arun...@samsung.com Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com --- .../devicetree/bindings/media/samsung-s5k4e5.txt | 45 1 file changed, 45 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k4e5.txt diff --git a/Documentation/devicetree/bindings/media/samsung-s5k4e5.txt b/Documentation/devicetree/bindings/media/samsung-s5k4e5.txt new file mode 100644 index 000..0fca087 --- /dev/null +++ b/Documentation/devicetree/bindings/media/samsung-s5k4e5.txt @@ -0,0 +1,45 @@ +* Samsung S5K4E5 Raw Image Sensor + +S5K4E5 is a raw image sensor with maximum resolution of 2560x1920 +pixels. Data transfer is carried out via MIPI CSI-2 port and controls +via I2C bus. + +Required Properties: +- compatible : must be samsung,s5k4e5 s/must be/should contain/ +- reg: I2C device address +- reset-gpios: specifier of a GPIO connected to the RESET pin +- clocks : should contain the sensor's EXTCLK clock specifier, from + the common clock bindings I would reword this to reference clock-names so as to make the ordering relationship explicit. With that, as everything else looks sane: Acked-by: Mark Rutland mark.rutl...@arm.com Thanks Mark. -- 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] ARM: dts: Update arch timer node with clock frequency
On Tue, Oct 08, 2013 at 11:15:47PM +0100, Olof Johansson wrote: [Adding Tony, who reported a mainline booting issue, and Sean who helped me track this down] On Mon, Sep 23, 2013 at 7:15 AM, Mark Rutland mark.rutl...@arm.com wrote: On Sat, Sep 21, 2013 at 04:24:59PM +0100, Tomasz Figa wrote: Hi Yuvaraj, On Wednesday 18 of September 2013 15:41:53 Yuvaraj Kumar C D wrote: Without the clock-frequency property in arch timer node, could able to see the below crash dump. [snip] diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index 7d7cc77..668ce5d 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -96,6 +96,7 @@ 1 14 0xf08, 1 11 0xf08, 1 10 0xf08; + clock-frequency = 2400; Shouldn't it rather come from some clock provided by some clock controller instead? The frequency would be then retrieved using clk_get_rate() on a clock received by clk_get(), specified in device tree using generic clock bindings. If the bootloader has initialised the generic timer correctly, the CNTFRQ register should contain the clock frequency, independent of any external clock. So, we just sat here to bisect a problem on the Samsung Chromebook where we hit exactly this problem. The read-only firmware on the device does not set CNTFRQ at boot, so this fails. Ouch. That's a shame. A chained bootloader (like the KVM guys are using) should be able to set CNTFRQ, so as long as any chained loader actually does that this won't cause that to blow up... Apparantly the u-boot that comes with Arndale sets it, so I haven't seen this error on that platform. Having the bootloader set CNTFRQ is by far the preferable solution, it is architected for this purpose. Unfortunately there is now real hardware out there that needs this due to firmware bugs / missing features, so there's little other choice. :( Indeed :( I'll pick this patch up in the fixes branch for 3.12, unless someone complains loudly. Could you please add a note to the dts regarding why we actually need this? It would be nice to maintain the impression that this is not the preferred way of doing things... Cheers, Mark. -- 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] ARM: dts: Update arch timer node with clock frequency
On Wed, Oct 09, 2013 at 08:46:06PM +0100, Olof Johansson wrote: On Wed, Oct 9, 2013 at 1:25 AM, Mark Rutland mark.rutl...@arm.com wrote: On Tue, Oct 08, 2013 at 11:15:47PM +0100, Olof Johansson wrote: [Adding Tony, who reported a mainline booting issue, and Sean who helped me track this down] On Mon, Sep 23, 2013 at 7:15 AM, Mark Rutland mark.rutl...@arm.com wrote: On Sat, Sep 21, 2013 at 04:24:59PM +0100, Tomasz Figa wrote: Hi Yuvaraj, On Wednesday 18 of September 2013 15:41:53 Yuvaraj Kumar C D wrote: Without the clock-frequency property in arch timer node, could able to see the below crash dump. [snip] diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index 7d7cc77..668ce5d 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -96,6 +96,7 @@ 1 14 0xf08, 1 11 0xf08, 1 10 0xf08; + clock-frequency = 2400; Shouldn't it rather come from some clock provided by some clock controller instead? The frequency would be then retrieved using clk_get_rate() on a clock received by clk_get(), specified in device tree using generic clock bindings. If the bootloader has initialised the generic timer correctly, the CNTFRQ register should contain the clock frequency, independent of any external clock. So, we just sat here to bisect a problem on the Samsung Chromebook where we hit exactly this problem. The read-only firmware on the device does not set CNTFRQ at boot, so this fails. Ouch. That's a shame. A chained bootloader (like the KVM guys are using) should be able to set CNTFRQ, so as long as any chained loader actually does that this won't cause that to blow up... Yes, but we have cases where we want to be able to boot without a chained u-boot as well. Sorry, that was poorly worded on my behalf. I meant that in the main case I'm aware of where the having the clock-frequency property wasn't good enough (because it doesn't get propagated to guests), we have another workaround. Apparantly the u-boot that comes with Arndale sets it, so I haven't seen this error on that platform. Having the bootloader set CNTFRQ is by far the preferable solution, it is architected for this purpose. Unfortunately there is now real hardware out there that needs this due to firmware bugs / missing features, so there's little other choice. :( Indeed :( I'll pick this patch up in the fixes branch for 3.12, unless someone complains loudly. Could you please add a note to the dts regarding why we actually need this? It would be nice to maintain the impression that this is not the preferred way of doing things... s/dts/dtsi/, yes I can do that. Hopefully with that we won't get too much automated copy and paste to other platforms. Cheers! Thanks, Mark. -- 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 v8] s5k5baf: add camera sensor driver
diff --git a/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt new file mode 100644 index 000..7704a1e --- /dev/null +++ b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt @@ -0,0 +1,58 @@ +Samsung S5K5BAF UXGA 1/5 2M CMOS Image Sensor with embedded SoC ISP + + +Required properties: + +- compatible : samsung,s5k5baf; +- reg: I2C slave address of the sensor; +- vdda-supply: analog power supply 2.8V (2.6V to 3.0V); +- vddreg-supply : regulator input power supply 1.8V (1.7V to 1.9V) + or 2.8V (2.6V to 3.0); +- vddio-supply : I/O power supply 1.8V (1.65V to 1.95V) + or 2.8V (2.5V to 3.1V); +- stbyn-gpios: GPIO connected to STDBYN pin; +- rstn-gpios : GPIO connected to RSTN pin; +- clocks : the sensor's master clock specifier (from the common + clock bindings); +- clock-names: must be mclk; I'd reword this slightly: - clocks: clock-specifiers (per the common clock bindings) for the clocks described in clock-names OK - clock-names: should include mclk for the sensor's master clock IMHO it suggests there could be more than one clock, is it OK? In general I'd prefer things to be described in this way so as to not describe any requirements on the order of clocks in the binding (and to suggest that clocks should be requested by name in code). Future hardware variants could have multiple clocks that we need to add to bindings, or it's possible we miss some clocks in the initial version of a binding. In either case it's easier to extend the binding and code if clocks are described by name and requested by name. + +Optional properties: + +- clock-frequency : the frequency at which the mclk clock should be + configured to operate, in Hz; if this property is not + specified default 24 MHz value will be used. + +The device node should contain one 'port' child node with one child 'endpoint' +node, according to the bindings defined in Documentation/devicetree/bindings/ +media/video-interfaces.txt. The following are properties specific to those +nodes. + +endpoint node +- + +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in + video-interfaces.txt. This sensor doesn't support data lane remapping + and physical lane indexes in subsequent elements of the array should + have consecutive values. Do these need to start at 1, or may they have any initial value? After re-checking I have found some inconsistency in the specs, regarding lanes. Final conclusion is that sensor supports only one lane without re-mapping. I would then change description to: - data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in video-interfaces.txt. If present it should be 1 - the device supports only one data lane without re-mapping. OK. That sounds reasonable to me. + +Example: + +s5k5bafx@2d { + compatible = samsung,s5k5baf; + reg = 0x2d; + vdda-supply = cam_io_en_reg; + vddreg-supply = vt_core_15v_reg; + vddio-supply = vtcam_reg; + stbyn-gpios = gpl2 0 1; + rstn-gpios = gpl2 1 1; + clock-names = mclk; + clocks = clock_cam 0; + clock-frequency = 2400; + + port { + s5k5bafx_ep: endpoint { + remote-endpoint = csis1_ep; + data-lanes = 1; + }; + }; +}; Otherwise, I think the binding looks fine. I took a quick skim over the driver and I have a few other comments. [...] +enum s5k5baf_gpio_id { + STBY, + RST, + GPIO_NUM, I'd expect this to be NUM_GPIOS or MAX_GPIOS, as GPIO_NUM sounds like the index for a GPIO, rather than how many GPIOs there are. OK +}; + +#define PAD_CIS 0 +#define PAD_OUT 1 +#define CIS_PAD_NUM 1 +#define ISP_PAD_NUM 2 Similarly here, I think NUM_*S or MAX_*S is preferable. OK [...] +static void s5k5baf_hw_patch(struct s5k5baf *state) +{ + static const u16 nseq_patch[] = { + NSEQ(0x1668, + 0xb5fe, 0x0007, 0x683c, 0x687e, 0x1da5, 0x88a0, 0x2800, 0xd00b, + 0x88a8, 0x2800, 0xd008, 0x8820, 0x8829, 0x4288, 0xd301, 0x1a40, + 0xe000, 0x1a08, 0x9001, 0xe001, 0x2019, 0x9001, 0x4916, 0x466b, + 0x8a48, 0x8118, 0x8a88, 0x8158, 0x4814, 0x8940, 0x0040, 0x2103, + 0xf000, 0xf826, 0x88a1, 0x4288, 0xd908, 0x8828, 0x8030, 0x8868, + 0x8070, 0x88a8, 0x6038, 0xbcfe, 0xbc08, 0x4718, 0x88a9, 0x4288, + 0xd906, 0x8820, 0x8030, 0x8860, 0x8070, 0x88a0, 0x6038, 0xe7f2, +
Re: [PATCH v2] MTD: Onenand: Add device tree support for samsung onenand
On Tue, Sep 24, 2013 at 02:00:01PM +0100, Tomasz Figa wrote: Hi Mateusz, Mark, Hi, On Monday 23 of September 2013 15:08:23 Mark Rutland wrote: On Mon, Sep 23, 2013 at 02:06:48PM +0100, Mateusz Krawczuk wrote: + +Required properties: + - compatible: value should be either of the following. + (a) samsung,s3c6400-onenand, + for onenand controller compatible with s3c6400. + (b) samsung,s3c6410-onenand, + for onenand controller compatible with s3c6410. + (c) samsung,s5pc100-onenand, + for onenand controller compatible with s5pc100. + (d) samsung,s5pc110-onenand, + for s5pc100-like onenand controller used on s5pc110 which supports DMA. + As I asked on the last posting, what are the differences between these implementations? (d) is a completely different controller than (a), (b) and (c). They have completely different register layouts. Also see below. Ok. +Required properties for s5pc110: + + - reg: the offset and length of the control registers. First region describes + OneNAND interface, second control registers. Also, the complete reg description is a bit confusing. How about something like: - reg: two register specifiers: [0] - The OneNAND interface [1] - The control registers This looks much better, although I wouldn't be afraid of using words instead of symbols to write this as follows instead: - reg: two memory mapped register regions: - first entry: memory mapped OneNAND chip, - second entry: control registers. I'm also happy with that. But... The controller in theory supports more than one OneNAND chip, each mapped at different, so I'd suggest reordering the entries: - reg: memory mapped register regions: - first entry: control registers. - second entry: memory mapped OneNAND chip 0, - ... - Nth entry: memory mapped OneNAND chip (N-2). I agree on the reordering, hence the questions I had below. Do we expect future OneNAND devices which may require more reg entries to describe? Nope. In age of eMMC I wouldn't even expect any new OneNAND controller to be handled by these bindings. Ok. + - interrupt-parent, interrupts Onenand memory interrupts As it's a common property, I don't think interrupt-parent needs to be described. Is there more than one interrupt? What's it called on the manual? There is one interrupt per OneNAND chip, so this is what should be represented in the binding. Ah. That should be mentioned explicitly, with the required ordering. + +Required properties for others: + + - reg: the offset and length of the control registers. First region describes + control registers, second OneNAND interface. Why does the s5pc110 OneNAND binding take its registers in the opposite order to the rest of these? Can we not fix up the driver first to make it consistent? Then we only need to describe this once and it's going to be far less of a headache to support. Both areas in both major hardware variants have completely different meaning: - in case of SoCs earlier or equal to S5PC100, first entry represents controller registers that are used to control various operating parameters, while second entry is a command/data interface, which is used to trigger read/write/etc. operations in the controller and push/pull data through it. In this variant there is no direct mapping of OneNAND chip in CPU address space. This variant supports only one memory chip per controller instance, - in case of S5PC110, each OneNAND chip is directly mapped into CPU address space. In addition there is a block of control registers that are used to configure the interface and control internal DMA engine (which is not present in = S5PC100 variants). This variant supports multiple memory chips per controller instance (2 in case of S5PV210, but current driver handles only one AFAIK). Ok. Even with that, I think the suggestion to reorder the reg entries above still makes this more consistent, with the controller registers first, then an entry per-chip (which is either the chip itself or the related command/data interface depending on the particular controller). + +Clocks: + - gate - clock which output is supplied to external OneNAND flash memory. How about the following (without the Clocks header): - clocks: clock-specifiers for the clocks named in clock-names, per the common clock bindings. - clock-names: should contain gate for the clock to the OneNAND flash memory. Is this the only clock that might be necessary on all platforms? S3C64xx SoCs have only one OneNAND gate clock per controller. S5PC100 SoC has one IP gate clock and one memory chip gate clock. S5PC110 SoC has one gate that gates both IP and all memory chips. Good to know
Re: [PATCH v2] MTD: Onenand: Add device tree support for samsung onenand
On Mon, Sep 23, 2013 at 02:06:48PM +0100, Mateusz Krawczuk wrote: This patch add clk and device tree nodes for samsung onenand driver. since v1: Updated Documentation according to Mark Rutland notes. Signed-off-by: Mateusz Krawczuk m.krawc...@partner.samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- .../devicetree/bindings/mtd/samsung-onenand.txt| 44 ++ drivers/mtd/onenand/samsung.c | 37 +- 2 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/mtd/samsung-onenand.txt diff --git a/Documentation/devicetree/bindings/mtd/samsung-onenand.txt b/Documentation/devicetree/bindings/mtd/samsung-onenand.txt new file mode 100644 index 000..5bf931c --- /dev/null +++ b/Documentation/devicetree/bindings/mtd/samsung-onenand.txt @@ -0,0 +1,44 @@ +Device tree bindings for Samsung Onenand + +Required properties: + - compatible: value should be either of the following. + (a) samsung,s3c6400-onenand, + for onenand controller compatible with s3c6400. + (b) samsung,s3c6410-onenand, + for onenand controller compatible with s3c6410. + (c) samsung,s5pc100-onenand, + for onenand controller compatible with s5pc100. + (d) samsung,s5pc110-onenand, + for s5pc100-like onenand controller used on s5pc110 which supports DMA. + As I asked on the last posting, what are the differences between these implementations? +Required properties for s5pc110: + + - reg: the offset and length of the control registers. First region describes + OneNAND interface, second control registers. Also, the complete reg description is a bit confusing. How about something like: - reg: two register specifiers: [0] - The OneNAND interface [1] - The control registers Do we expect future OneNAND devices which may require more reg entries to describe? + - interrupt-parent, interrupts Onenand memory interrupts As it's a common property, I don't think interrupt-parent needs to be described. Is there more than one interrupt? What's it called on the manual? + +Required properties for others: + + - reg: the offset and length of the control registers. First region describes + control registers, second OneNAND interface. Why does the s5pc110 OneNAND binding take its registers in the opposite order to the rest of these? Can we not fix up the driver first to make it consistent? Then we only need to describe this once and it's going to be far less of a headache to support. + +Clocks: + - gate - clock which output is supplied to external OneNAND flash memory. How about the following (without the Clocks header): - clocks: clock-specifiers for the clocks named in clock-names, per the common clock bindings. - clock-names: should contain gate for the clock to the OneNAND flash memory. Is this the only clock that might be necessary on all platforms? + + +For partiton table parsing (optional) please refer to: + [1] Documentation/devicetree/bindings/mtd/partition.txt + +Example for an s5pv210 board: + + onenand@b000 { + compatible = samsung,s5pc110-onenand; + reg = 0xb000 0x2, 0xb060 0x2000; + interrupt-parent = vic1; + interrupts = 31; + #address-cells = 1; + #size-cells = 1; + clocks = clocks NANDXL; + clock-names = gate; + status = okay; + }; Cheers, Mark. -- 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] ARM: dts: Update arch timer node with clock frequency
On Sat, Sep 21, 2013 at 04:24:59PM +0100, Tomasz Figa wrote: Hi Yuvaraj, On Wednesday 18 of September 2013 15:41:53 Yuvaraj Kumar C D wrote: Without the clock-frequency property in arch timer node, could able to see the below crash dump. [snip] diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index 7d7cc77..668ce5d 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -96,6 +96,7 @@ 1 14 0xf08, 1 11 0xf08, 1 10 0xf08; + clock-frequency = 2400; Shouldn't it rather come from some clock provided by some clock controller instead? The frequency would be then retrieved using clk_get_rate() on a clock received by clk_get(), specified in device tree using generic clock bindings. If the bootloader has initialised the generic timer correctly, the CNTFRQ register should contain the clock frequency, independent of any external clock. Having the bootloader set CNTFRQ is by far the preferable solution, it is architected for this purpose. Thanks, Mark. -- 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 v8] s5k5baf: add camera sensor driver
On Fri, Sep 06, 2013 at 11:31:06AM +0100, Andrzej Hajda wrote: Driver for Samsung S5K5BAF UXGA 1/5 2M CMOS Image Sensor with embedded SoC ISP. The driver exposes the sensor as two V4L2 subdevices: - S5K5BAF-CIS - pure CMOS Image Sensor, fixed 1600x1200 format, no controls. - S5K5BAF-ISP - Image Signal Processor, formats up to 1600x1200, pre/post ISP cropping, downscaling via selection API, controls. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Andrzej Hajda a.ha...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- Hi, This is the 8th iteration of the patch. I have applied suggestions from Laurent, Sylwester and Mark, thanks. One exeception, I have left static struct v4l2_rect s5k5baf_cis_rect not const due to fact its address is passed to function which could modify its arguments, of course it never modifies s5k5baf_cis_rect. Regards Andrzej v8 - improved description of data-lanes binding, - added algorithm caching, - added comments to functions, - video bus type checking moved to probe, - clk_get/put moved to probe, - moved streaming checking under mutex, - use proper functions for endian conversion, - probe returns -EPROBE_DEFER in case it cannot get clock, - v4l2_async_unregister_subdev is called on remove, - cosmetic changes v7 - changed description of 'clock-frequency' DT property v6 - endpoint node presence is now optional, - added asynchronous subdev registration support and clock handling, - use named gpios in DT bindings v5 - removed hflip/vflip device tree properties v4 - GPL changed to GPLv2, - bitfields replaced by u8, - cosmetic changes, - corrected s_stream flow, - gpio pins are no longer exported, - added I2C addresses to subdev names, - CIS subdev registration postponed after succesfull HW initialization, - added enums for pads, - selections are initialized only during probe, - default resolution changed to 1600x1200, - state-error pattern removed from few other functions, - entity link creation moved to registered callback. v3: - narrowed state-error usage to i2c and power errors, - private gain controls replaced by red/blue balance user controls, - added checks to devicetree gpio node parsing v2: - lower-cased driver name, - removed underscore from regulator names, - removed platform data code, - v4l controls grouped in anonymous structs, - added s5k5baf_clear_error function, - private controls definitions moved to uapi header file, - added v4l2-controls.h reservation for private controls, - corrected subdev registered/unregistered code, - .log_status sudbev op set to v4l2 helper, - moved entity link creation to probe routines, - added cleanup on error to probe function. --- .../devicetree/bindings/media/samsung-s5k5baf.txt | 58 + MAINTAINERS|7 + drivers/media/i2c/Kconfig |7 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/s5k5baf.c| 2050 5 files changed, 2123 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k5baf.txt create mode 100644 drivers/media/i2c/s5k5baf.c diff --git a/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt new file mode 100644 index 000..7704a1e --- /dev/null +++ b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt @@ -0,0 +1,58 @@ +Samsung S5K5BAF UXGA 1/5 2M CMOS Image Sensor with embedded SoC ISP + + +Required properties: + +- compatible : samsung,s5k5baf; +- reg: I2C slave address of the sensor; +- vdda-supply: analog power supply 2.8V (2.6V to 3.0V); +- vddreg-supply : regulator input power supply 1.8V (1.7V to 1.9V) + or 2.8V (2.6V to 3.0); +- vddio-supply : I/O power supply 1.8V (1.65V to 1.95V) + or 2.8V (2.5V to 3.1V); +- stbyn-gpios: GPIO connected to STDBYN pin; +- rstn-gpios : GPIO connected to RSTN pin; +- clocks : the sensor's master clock specifier (from the common + clock bindings); +- clock-names: must be mclk; I'd reword this slightly: - clocks: clock-specifiers (per the common clock bindings) for the clocks described in clock-names - clock-names: should include mclk for the sensor's master clock + +Optional properties: + +- clock-frequency : the frequency at which the mclk clock should be + configured to operate, in Hz; if this property is not + specified default 24 MHz value will be used. + +The device node should contain one 'port' child node with one child 'endpoint' +node, according to the bindings defined in Documentation/devicetree/bindings/
Re: [PATCH] ARM: dts: Update arch timer node with clock frequency
[adding lakml] On Wed, Sep 18, 2013 at 11:11:53AM +0100, Yuvaraj Kumar C D wrote: Without the clock-frequency property in arch timer node, could able to see the below crash dump. Why does this cause the below crash specifically? What is CNTFRQ reading as? Your firmware or bootloader should set CNTFRQ -- setting the clock-frequency is a work-around for buggy firmware/bootloaders that should be avoided as far as possible. Is it not possible to fix your firmware or bootlaoder to set CNTFRQ? Thanks, Mark. [c0014e28] (unwind_backtrace+0x0/0xf4) from [c0011808] (show_stack+0x10/0x14) [c0011808] (show_stack+0x10/0x14) from [c036ac1c] (dump_stack+0x7c/0xb0) [c036ac1c] (dump_stack+0x7c/0xb0) from [c01ab760] (Ldiv0_64+0x8/0x18) [c01ab760] (Ldiv0_64+0x8/0x18) from [c0062f60] (clockevents_config.part.2+0x1c/0x74) [c0062f60] (clockevents_config.part.2+0x1c/0x74) from [c0062fd8] (clockevents_config_and_register+0x20/0x2c) [c0062fd8] (clockevents_config_and_register+0x20/0x2c) from [c02b8e8c] (arch_timer_setup+0xa8/0x134) [c02b8e8c] (arch_timer_setup+0xa8/0x134) from [c04b47b4] (arch_timer_init+0x1f4/0x24c) [c04b47b4] (arch_timer_init+0x1f4/0x24c) from [c04b40d8] (clocksource_of_init+0x34/0x58) [c04b40d8] (clocksource_of_init+0x34/0x58) from [c049ed8c] (time_init+0x20/0x2c) [c049ed8c] (time_init+0x20/0x2c) from [c049b95c] (start_kernel+0x1e0/0x39c) Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com --- arch/arm/boot/dts/exynos5250.dtsi |1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index 7d7cc77..668ce5d 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -96,6 +96,7 @@ 1 14 0xf08, 1 11 0xf08, 1 10 0xf08; + clock-frequency = 2400; }; mct@101C { -- 1.7.9.5 -- 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] ARM: dts: Disable Exynos5250 I2S controllers by default
On Fri, Sep 06, 2013 at 07:17:15PM +0100, Mark Brown wrote: From: Mark Brown broo...@linaro.org Rather than requiring each board to explicitly disable the I2S controllers it is not using instead require boards to enable those that they are using. This is required for audio operation on Arndale, one of the unused I2S controllers is pinmuxed with the LDO enable GPIOs for the WM1811A. Signed-off-by: Mark Brown broo...@linaro.org --- This seems like a more robust approach to handling the externally visible IPs - if this is OK I can go through and do further updates for other devices. Acked-by: Mark Rutland mark.rutl...@arm.com It seems far more sensible to me to mark devices disabled by default in shared dtsi files and then okay them as needed in particular dts files. I'd be happy with more of this. arch/arm/boot/dts/exynos5250-arndale.dts | 4 arch/arm/boot/dts/exynos5250-smdk5250.dts | 8 arch/arm/boot/dts/exynos5250.dtsi | 3 +++ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts index cee55fa..4687fa0 100644 --- a/arch/arm/boot/dts/exynos5250-arndale.dts +++ b/arch/arm/boot/dts/exynos5250-arndale.dts @@ -412,6 +412,10 @@ status = disabled; }; + i2s0: i2s@0383 { + status = okay; + }; + spi_0: spi@12d2 { status = disabled; }; diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts index 2538b32..f86d567 100644 --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts @@ -231,14 +231,6 @@ status = okay; }; - i2s1: i2s@12D6 { - status = disabled; - }; - - i2s2: i2s@12D7 { - status = disabled; - }; - sound { compatible = samsung,smdk-wm8994; diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index 7d7cc77..c863113 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -417,6 +417,7 @@ i2s0: i2s@0383 { compatible = samsung,s5pv210-i2s; + status = disabled; reg = 0x0383 0x100; dmas = pdma0 10 pdma0 9 @@ -433,6 +434,7 @@ i2s1: i2s@12D6 { compatible = samsung,s3c6410-i2s; + status = disabled; reg = 0x12D6 0x100; dmas = pdma1 12 pdma1 11; @@ -445,6 +447,7 @@ i2s2: i2s@12D7 { compatible = samsung,s3c6410-i2s; + status = disabled; reg = 0x12D7 0x100; dmas = pdma0 12 pdma0 11; -- 1.8.4.rc3 -- 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 v7] s5k5baf: add camera sensor driver
On Mon, Sep 02, 2013 at 05:21:58PM +0100, Sylwester Nawrocki wrote: Hi Mark, Hi Sylwester, On 08/27/2013 11:14 AM, Mark Rutland wrote: +endpoint node +- + +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in + video-interfaces.txt. This property can be only used to specify number + of data lanes, i.e. the array's content is unused, only its length is + meaningful. When this property is not specified default value of 1 lane + will be used. Apologies for having not replied to the last posting, but having looked at the documentation I was provided last time [1], I don't think the values in the data-lanes property should be described as unused. That may be the way the Linux driver functions at present, but it's not how the generic video-interfaces binding documentation describes the property. If the CSI transmitter hardware doesn't support logical remapping of lanes, then the only valid values for data-lanes would be a contiguous list of lane IDs starting at 1, ending at 4 at most. Valid values for the property would be one of: data-lanes = 1; data-lanes = 1, 2; data-lanes = 1, 2, 3; data-lanes = 1, 2, 3, 4; We can mention the fact the hardware doesn't support remapping of lanes, and therefore the list must start with lane 1 and end with (at most) lane 4. That way a dts will match the generic binding and actually describe the hardware, and it's possible for Linux (or any other OS) to factor out the parsing of data-lanes later as desired. I don't think we should offer freedom to encode garbage in the dt when we can just as easily encourage more standard use of bindings that will make our lives easier in the long-term. I entirely agree, that's a very accurate analysis. Presumably the data-lanes property's descriptions could be improved so it is said explicitly that array elements 0...N - 1, where N = 4, correspond to logical data lanes 1...N. That makes sense to me. Then considering the values in the data-lanes property, I didn't make the description terribly specific about the fact that pool of indexes 0...4 is used for the clock lane and 4 data lanes. The values could well be H/W specific, but it seems more sensible to enforce common range. It may not match exactly with documentation of various hardware. E.g. OMAP, see page 1661, register CSI2_COMPLEXIO_CFG [1], uses indexes 1..5 to indicate position of a data lane and 0 is used to mark a lane as unused. Ah, I see. I guess this boils down to what the physical indexes referred to by the video-interfaces binding actually are. If an interface may only ever have 5 lanes, then using a logical index sounds fine. But if we ever have the case where a CSI transmitter has more than 5 lanes (so it could communicate with multiple receviers), then the value has to become hardware-specific. At that point, we'd possibly need to define remapping of the clocks too. I'm not that familiar with CSI so I'm not sure if such a device is possible. I think we should have similarly defined value 0 to indicate an unused lane. None of drivers in mainline uses this line remapping feature, so changing meaning of the array values wouldn't presumably have any bad side effects. I'm not sure if it's OK to make a change like this now. IIUC the MIPI CSI-2 standard requires consecutive data lane indexes, so valid set of data lanes could be only: 1, 1, 2, 1, 2, 3, 1, 2, 3, 4. So there seems to be no issue for MIPI CSI-2. But for future protocols the current convention might not have been flexible enough. Given that the video-interfaces binding refers to the clock being on lane 0, I'm not sure it makes sense to reserve this as an unused id -- we'd be saying the lane is the clock to say it's unused, but maybe that doesn't matter. Thanks, Mark. [1] http://www.mipi.org/specifications/camera-interface#CSI2 [2] http://www.ti.com/general/docs/lit/getliterature.tsp?literatureNumber=swpu231aofileType=pdf -- Regards, Sylwester -- 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 v7] s5k5baf: add camera sensor driver
Hi, [trimming down to relevant context] +endpoint node +- + +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in + video-interfaces.txt. This property can be only used to specify number + of data lanes, i.e. the array's content is unused, only its length is + meaningful. When this property is not specified default value of 1 lane + will be used. Apologies for having not replied to the last posting, but having looked at the documentation I was provided last time [1], I don't think the values in the data-lanes property should be described as unused. That may be the way the Linux driver functions at present, but it's not how the generic video-interfaces binding documentation describes the property. If the CSI transmitter hardware doesn't support logical remapping of lanes, then the only valid values for data-lanes would be a contiguous list of lane IDs starting at 1, ending at 4 at most. Valid values for the property would be one of: data-lanes = 1; data-lanes = 1, 2; data-lanes = 1, 2, 3; data-lanes = 1, 2, 3, 4; We can mention the fact the hardware doesn't support remapping of lanes, and therefore the list must start with lane 1 and end with (at most) lane 4. That way a dts will match the generic binding and actually describe the hardware, and it's possible for Linux (or any other OS) to factor out the parsing of data-lanes later as desired. I don't think we should offer freedom to encode garbage in the dt when we can just as easily encourage more standard use of bindings that will make our lives easier in the long-term. Thanks, Mark. [1] http://www.mipi.org/specifications/camera-interface#CSI2 -- 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 V4] ARM: dts: Add dwmmc DT nodes for exynos5420 SOC
On Tue, Aug 27, 2013 at 10:22:31AM +0100, Yuvaraj Kumar C D wrote: This patch adds the device tree node entries for exynos5420 SOC. Exynos5420 has a different version of DWMMC controller,so a new compatible string is used to distinguish it from the prior SOC's. This patch depends on mmc: dw_mmc: exynos: Add a new compatible string for exynos5420 changes since V3: 1.change fifo-depth size from 0x80 to 0x40 2.Move the below properties a.card-detect-delay b.samsung,dw-mshc-ciu-div c.samsung,dw-mshc-sdr-timing d.samsung,dw-mshc-ddr-timing from SOC dts to board dts file as suggested by Doug Anderson changes since V2: 1.dropped num-slots property from node as its not required if number of card slots available is 1. 2.Move the below properties a.fifo-depth b.card-detect-delay c.samsung,dw-mshc-ciu-div d.samsung,dw-mshc-sdr-timing e.samsung,dw-mshc-ddr-timing from board dts to SOC dts,as these are not board specific properties. 3.Updated the binding document exynos-dw-mshc.txt. changes since V1: 1.disable node by status = disabled in SOC file 2.enable node by status = okay in board specific file Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com --- .../devicetree/bindings/mmc/exynos-dw-mshc.txt |4 ++ arch/arm/boot/dts/exynos5420-smdk5420.dts | 34 + arch/arm/boot/dts/exynos5420.dtsi | 39 3 files changed, 77 insertions(+) diff --git a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt index 6d1c098..25368e8 100644 --- a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt +++ b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt @@ -16,6 +16,8 @@ Required Properties: specific extensions. - samsung,exynos5250-dw-mshc: for controllers with Samsung Exynos5250 specific extensions. + - samsung,exynos5420-dw-mshc: for controllers with Samsung Exynos5420 + specific extensions. * samsung,dw-mshc-ciu-div: Specifies the divider value for the card interface unit (ciu) clock. This property is applicable only for Exynos5 SoC's and @@ -31,6 +33,8 @@ Required Properties: data rate mode operation. Refer notes below for the order of the cells and the valid values. +* bypass-smu: Bypass Security Management Unit of eMMC channel 0 and channel 1. + Could you elaborate on why this is needed? Is the SMU broken or not present in some hardware revisions? Notes for the sdr-timing and ddr-timing values: The order of the cells should be On an unrelated note, I see the binding in mainline defines a gpios property that doesn't seem to be used anywhere. Am I missing something, or do we just not have support for that part of the binding? Thanks, Mark. diff --git a/arch/arm/boot/dts/exynos5420-smdk5420.dts b/arch/arm/boot/dts/exynos5420-smdk5420.dts index bafba25..576066c 100644 --- a/arch/arm/boot/dts/exynos5420-smdk5420.dts +++ b/arch/arm/boot/dts/exynos5420-smdk5420.dts @@ -31,6 +31,40 @@ }; }; + dwmmc0@1220 { + status = okay; + broken-cd; + bypass-smu; + supports-highspeed; + card-detect-delay = 200; + samsung,dw-mshc-ciu-div = 3; + samsung,dw-mshc-sdr-timing = 0 4; + samsung,dw-mshc-ddr-timing = 0 2; + pinctrl-names = default; + pinctrl-0 = sd0_clk sd0_cmd sd0_bus4 sd0_bus8; + + slot@0 { + reg = 0; + bus-width = 8; + }; + }; + + dwmmc2@1222 { + status = okay; + supports-highspeed; + card-detect-delay = 200; + samsung,dw-mshc-ciu-div = 3; + samsung,dw-mshc-sdr-timing = 2 3; + samsung,dw-mshc-ddr-timing = 1 2; + pinctrl-names = default; + pinctrl-0 = sd2_clk sd2_cmd sd2_cd sd2_bus4; + + slot@0 { + reg = 0; + bus-width = 4; + }; + }; + dp-controller@145B { pinctrl-names = default; pinctrl-0 = dp_hpd; diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi index d537cd7..3893f45 100644 --- a/arch/arm/boot/dts/exynos5420.dtsi +++ b/arch/arm/boot/dts/exynos5420.dtsi @@ -22,6 +22,9 @@ compatible = samsung,exynos5420; aliases { + mshc0 = dwmmc_0; + mshc1 = dwmmc_1; + mshc2 = dwmmc_2; pinctrl0 = pinctrl_0; pinctrl1 = pinctrl_1; pinctrl2 = pinctrl_2; @@ -84,6 +87,42 @@
Re: [PATCH V4] ARM: dts: Add dwmmc DT nodes for exynos5420 SOC
On Tue, Aug 27, 2013 at 01:02:52PM +0100, Yuvaraj Kumar wrote: On Tue, Aug 27, 2013 at 4:31 PM, Mark Rutland mark.rutl...@arm.com wrote: On Tue, Aug 27, 2013 at 10:22:31AM +0100, Yuvaraj Kumar C D wrote: This patch adds the device tree node entries for exynos5420 SOC. Exynos5420 has a different version of DWMMC controller,so a new compatible string is used to distinguish it from the prior SOC's. This patch depends on mmc: dw_mmc: exynos: Add a new compatible string for exynos5420 changes since V3: 1.change fifo-depth size from 0x80 to 0x40 2.Move the below properties a.card-detect-delay b.samsung,dw-mshc-ciu-div c.samsung,dw-mshc-sdr-timing d.samsung,dw-mshc-ddr-timing from SOC dts to board dts file as suggested by Doug Anderson changes since V2: 1.dropped num-slots property from node as its not required if number of card slots available is 1. 2.Move the below properties a.fifo-depth b.card-detect-delay c.samsung,dw-mshc-ciu-div d.samsung,dw-mshc-sdr-timing e.samsung,dw-mshc-ddr-timing from board dts to SOC dts,as these are not board specific properties. 3.Updated the binding document exynos-dw-mshc.txt. changes since V1: 1.disable node by status = disabled in SOC file 2.enable node by status = okay in board specific file Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com --- .../devicetree/bindings/mmc/exynos-dw-mshc.txt |4 ++ arch/arm/boot/dts/exynos5420-smdk5420.dts | 34 + arch/arm/boot/dts/exynos5420.dtsi | 39 3 files changed, 77 insertions(+) diff --git a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt index 6d1c098..25368e8 100644 --- a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt +++ b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt @@ -16,6 +16,8 @@ Required Properties: specific extensions. - samsung,exynos5250-dw-mshc: for controllers with Samsung Exynos5250 specific extensions. + - samsung,exynos5420-dw-mshc: for controllers with Samsung Exynos5420 + specific extensions. * samsung,dw-mshc-ciu-div: Specifies the divider value for the card interface unit (ciu) clock. This property is applicable only for Exynos5 SoC's and @@ -31,6 +33,8 @@ Required Properties: data rate mode operation. Refer notes below for the order of the cells and the valid values. +* bypass-smu: Bypass Security Management Unit of eMMC channel 0 and channel 1. + Could you elaborate on why this is needed? Exynos5420 Mobile Storage Host controller has a Security Management Unit (SMU) for channel 0 and channel 1 (mainly for eMMC). This binding property requires to add a quirk to bypass SMU as it is not being used yet. When you say it's not being used _yet_, what do you mean? That the driver doesn't have support for it, but will in future? Can the driver not just choose to bypass it entirely for now, regardless of what's in the dt? Do we need to bypass it in future for some reason? I don't see why this needs to be in the dt. Is the SMU broken or not present in some hardware revisions? SMU is only present in channel 0 and channel 1,but not in channel 2.So to distinguish this, bypass-smu property has been added as quirks in the channel. Notes for the sdr-timing and ddr-timing values: The order of the cells should be On an unrelated note, I see the binding in mainline defines a gpios property that doesn't seem to be used anywhere. Am I missing something, or do we just not have support for that part of the binding? Yes,This document has been little outdated.I will update and post a separate patch. Cheers! Mark. -- 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 RFC v5] s5k5baf: add camera sensor driver
On Mon, Aug 19, 2013 at 02:18:27PM +0100, Andrzej Hajda wrote: Driver for Samsung S5K5BAF UXGA 1/5 2M CMOS Image Sensor with embedded SoC ISP. The driver exposes the sensor as two V4L2 subdevices: - S5K5BAF-CIS - pure CMOS Image Sensor, fixed 1600x1200 format, no controls. - S5K5BAF-ISP - Image Signal Processor, formats up to 1600x1200, pre/post ISP cropping, downscaling via selection API, controls. Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Andrzej Hajda a.ha...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- v5 - removed non-standard samsung hflip/vflip device tree bindings v4 - GPL changed to GPLv2, - bitfields replaced by u8, - cosmetic changes, - corrected s_stream flow, - gpio pins are no longer exported, - added I2C addresses to subdev names, - CIS subdev registration postponed after succesfull HW initialization, - added enums for pads, - selections are initialized only during probe, - default resolution changed to 1600x1200, - state-error pattern removed from few other functions, - entity link creation moved to registered callback. v3: - narrowed state-error usage to i2c and power errors, - private gain controls replaced by red/blue balance user controls, - added checks to devicetree gpio node parsing v2: - lower-cased driver name, - removed underscore from regulator names, - removed platform data code, - v4l controls grouped in anonymous structs, - added s5k5baf_clear_error function, - private controls definitions moved to uapi header file, - added v4l2-controls.h reservation for private controls, - corrected subdev registered/unregistered code, - .log_status sudbev op set to v4l2 helper, - moved entity link creation to probe routines, - added cleanup on error to probe function. --- .../devicetree/bindings/media/samsung-s5k5baf.txt | 51 + MAINTAINERS|7 + drivers/media/i2c/Kconfig |7 + drivers/media/i2c/Makefile |1 + drivers/media/i2c/s5k5baf.c| 1980 5 files changed, 2046 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k5baf.txt create mode 100644 drivers/media/i2c/s5k5baf.c diff --git a/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt new file mode 100644 index 000..b1f2fde --- /dev/null +++ b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt @@ -0,0 +1,51 @@ +Samsung S5K5BAF UXGA 1/5 2M CMOS Image Sensor with embedded SoC ISP +- + +Required properties: + +- compatible : samsung,s5k5baf; +- reg: I2C slave address of the sensor; +- vdda-supply: analog power supply 2.8V (2.6V to 3.0V); +- vddreg-supply : regulator input power supply 1.8V (1.7V to 1.9V) +or 2.8V (2.6V to 3.0); +- vddio-supply : I/O power supply 1.8V (1.65V to 1.95V) +or 2.8V (2.5V to 3.1V); +- gpios : GPIOs connected to STDBYN and RSTN pins, +in order: STBYN, RSTN; +- clock-frequency : master clock frequency in Hz; Why is this necessary? Could you not just require having a clocks property? You could then get equivalent functionality to the clock-frequency property by having a fixed-clock node if you don't ahve a real clock specifier to wire up. + +Optional properties: + +- clocks : contains the sensor's master clock specifier; +- clock-names: contains mclk entry; + +The device node should contain one 'port' child node with one child 'endpoint' +node, according to the bindings defined in Documentation/devicetree/bindings/ +media/video-interfaces.txt. The following are properties specific to those nodes. + +endpoint node +- + +- data-lanes : (optional) an array specifying active physical MIPI-CSI2 + data output lanes and their mapping to logical lanes; the + array's content is unused, only its length is meaningful; Is that a property of the driver, or does the design of the hardware mean that this can never encode useful information? What does the length of the property imply? + +Example: + +s5k5bafx@2d { + compatible = samsung,s5k5baf; + reg = 0x2d; + vdda-supply = cam_io_en_reg; + vddreg-supply = vt_core_15v_reg; + vddio-supply = vtcam_reg; + gpios = gpl2 0 1, + gpl2 1 1; + clock-frequency = 2400; + + port { + s5k5bafx_ep: endpoint { + remote-endpoint = csis1_ep; + data-lanes = 1; + }; + }; +}; Thanks, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
On Wed, Aug 07, 2013 at 06:06:05PM +0100, Julius Werner wrote: This breaks compatibility, both for an old kernel and a new dt and a new kernel with an old dt. Is anyone using these bindings? They only affect Samsung SoCs and have only been upstream for half a year, so I doubt it's heavily used. I'm not sure everyone will be happy with that line. Why are we describing fewer registers now? Are they described elsewhere? The dt should describe the device, not only the portion of it Linux wants to use right now. This only ever described a small section of the huge set of PMU registers anyway. Before it described up to three registers controlling different PHYs (using hardcoded offsets in the code to later find the right one)... with my patch every PHY's DT entry only describes the one register concerning itself, which makes more sense in my opinion. It will also prevent the register descriptions in different DT entries from overlapping. I'm not sure I understand. The old documentation referred to the USBDEVICE_PHY_CONTROL and USBHOST_PHY_CONTROL registers for a phy, and your new version only refers to (usb device) PHY_CONTROL. Regardless of multiple phys, you're suggesting that we describe less of each phy. That seems like taking away usable information. Unless I've misunderstood? Ideally, we'd describe the whole set of registers and linkages to phys, even if Linux doesn't ahppen to use that information right now. Thanks, Mark. -- 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] ARM: EXYNOS: add power domains support for EXYNOS5440
On Tue, Aug 06, 2013 at 05:16:56PM +0100, Bartlomiej Zolnierkiewicz wrote: Hi, On Saturday, August 03, 2013 09:14:11 PM Mark Rutland wrote: On Fri, Aug 02, 2013 at 10:01:52PM +0100, Stephen Warren wrote: (CCing the other DT maintainers, hence quoting binding in full) On 08/02/2013 07:23 AM, Bartlomiej Zolnierkiewicz wrote: On EXYNOS5440 power domains are handled in a different way than on the previous EXYNOS SoCs. Add support for them to EXYNOS pm_domains driver. Then add device tree nodes for PCIe (controlling power for PCIe host controller) and Conn2 (controlling power for Ethernet, SATA and USB controllers) power domains to exynos5440.dtsi. Currently if runtime Power Management is enabled and the driver for devices under power domain is disabled then the power domain will be disabled by EXYNOS pm_domains driver. To make more active use of power domains (dynamically enable and disabled them as needed) it is required to add runtime PM support to pci-exynos, stmmac, ahci_platform, ohci-exynos and ehci-s5p drivers (to be done later). Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com Signed-off-by: Myungjoo Ham myungjoo@samsung.com Cc: Tomasz Figa t.f...@samsung.com Cc: Stephen Warren swar...@wwwdotorg.org --- Documentation/devicetree/bindings/arm/exynos/power_domain.txt | 33 ++ arch/arm/boot/dts/exynos5440.dtsi | 23 + arch/arm/mach-exynos/Kconfig |1 arch/arm/mach-exynos/common.c |4 arch/arm/mach-exynos/pm_domains.c | 138 +- 5 files changed, 190 insertions(+), 9 deletions(-) Index: b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt === --- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt 2013-08-02 14:45:53.551392396 +0200 +++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt 2013-08-02 14:46:29.799391845 +0200 @@ -5,20 +5,47 @@ to gate power to one or more peripherals Required Properties: - compatible: should be one of the following. -* samsung,exynos4210-pd - for exynos4210 type power domain. +* samsung,exynos4210-pd - for Exynos4210 type power domain. +* samsung,exynos5440-pd - for Exynos5440 type power domain. - reg: physical base address of the controller and length of memory mapped -region. +region (Exynos4210 type power domain) or bit offset in the control +register (Exynos5440 type power domain). + +Additional parent node must be created for Exynos5440 power domains with +the following required properties: +- compatible: samsung,exynos5440-power-domains, simple-bus +- reg: physical base address of the XMU controller and length of memory +mapped region It's a little odd to describe the child nodes first. Given the 4210 and 5440 bindings work so differently, I'd suggest making separate binding files for the two; samsung,exynos4210-pd.txt and samsung,exynos5440-pd.txt. OK, I'll fix it. The node being describe here clearly is not a simple-bus; the child nodes appear to have a specific need that their parent be compatible = samsung,exynos5440-power-domains, hence they aren't the independent devices that simple-bus would usually contain. +1. This is most definitely not a simple-bus, the child nodes reg properties cdon't represent the same address space as the parent's reg property. What does in mean in the practice? What should I do instead? The driver for the parent nodes should probe for the child nodes via some mechanism. The simple-bus compatible property should be removed from the parent node as it's simply not true. Simple-bus should only be used where the child nodes make sense and are useable on their own, regardless of the parent node. How much does the association of bit-offsets to power domains vary? On EXYNOS5440 power domains are controlled by control and status registers which are shared among all power domains. On other EXYNOS SoCs there are separate control/status registers for each power domain. Ok. Note that I only briefly reviewed the low-level structural aspects of the binding that I mentioned above; I haven't thought about the binding at a higher level of e.g. does this binding conceptually make sense. This is unfortunately difficult to do :( Node of a device using power domains must have a samsung,power-domain property defined with a phandle to respective power domain. -Example: +Example for Exynos4210 compatible power domain: lcd0: power-domain-lcd0 { compatible = samsung,exynos4210-pd
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
On Tue, Aug 06, 2013 at 07:00:17PM +0100, Julius Werner wrote: This patch simplifies the way the phy-samsung-usb code finds the correct power management register to enable PHY clock gating. Previously, the code would calculate the register address from a device tree supplied base address and add an offset based on the PHY type. Since every PHY has its own device tree entry and needs only one register, we can just encode the address itself in the device tree and remove the diffentiation in the code. The bitmask needed to specify the bit within that register stays in place, allowing support for platforms like s3c64xx that use different bits within the same register. This breaks compatibility, both for an old kernel and a new dt and a new kernel with an old dt. Is anyone using these bindings? Signed-off-by: Julius Werner jwer...@chromium.org --- .../devicetree/bindings/usb/samsung-usbphy.txt | 26 +- arch/arm/boot/dts/exynos5250.dtsi | 4 ++-- drivers/usb/phy/phy-samsung-usb.c | 18 --- drivers/usb/phy/phy-samsung-usb.h | 23 +-- drivers/usb/phy/phy-samsung-usb2.c | 11 - drivers/usb/phy/phy-samsung-usb3.c | 2 +- 6 files changed, 22 insertions(+), 62 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt index 33fd354..1cf9b68 100644 --- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt @@ -34,14 +34,7 @@ Optional properties: - The child node 'usbphy-sys' to the node 'usbphy' is for the system controller interface for usb-phy. It should provide the following information required by usb-phy controller to control phy. - - reg : base physical address of PHY_CONTROL registers. - The size of this register is the total sum of size of all PHY_CONTROL - registers that the SoC has. For example, the size will be - '0x4' in case we have only one PHY_CONTROL register (e.g. - OTHERS register in S3C64XX or USB_PHY_CONTROL register in S5PV210) - and, '0x8' in case we have two PHY_CONTROL registers (e.g. - USBDEVICE_PHY_CONTROL and USBHOST_PHY_CONTROL registers in exynos4x). - and so on. + - reg : address of PHY_CONTROL register for this PHY. Example: - Exynos4210 @@ -57,8 +50,8 @@ Example: clock-names = xusbxti, otg; usbphy-sys { - /* USB device and host PHY_CONTROL registers */ - reg = 0x10020704 0x8; + /* USB device PHY_CONTROL register */ + reg = 0x10020704 0x4; }; }; Why are we describing fewer registers now? Are they described elsewhere? The dt should describe the device, not only the portion of it Linux wants to use right now. Thanks, Mark. -- 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/3] Initial skeleton of VFIO support for Device Tree based devices
[adding DT maintainers to Cc] On Mon, Aug 05, 2013 at 02:17:11PM +0100, Antonios Motakis wrote: Platform devices in the Linux kernel are usually managed by the DT interface. This patch forms the base to support these kind of devices with VFIO. Signed-off-by: Antonios Motakis a.mota...@virtualopensystems.com --- drivers/vfio/Kconfig | 10 +++ drivers/vfio/Makefile | 1 + drivers/vfio/vfio_dt.c| 187 ++ include/uapi/linux/vfio.h | 1 + 4 files changed, 199 insertions(+) create mode 100644 drivers/vfio/vfio_dt.c diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index 1f84eda..a77a2e4 100644 --- a/drivers/vfio/Kconfig +++ b/drivers/vfio/Kconfig @@ -13,4 +13,14 @@ menuconfig VFIO If you don't know what to do here, say N. +config VFIO_DT + tristate VFIO support for Device Tree devices + depends on VFIO EVENTFD + help + Support for the VFIO Device Tree driver. This is required to make + use of platform devices present on Device Tree nodes using the VFIO + framework. + + If you don't know what to do here, say N. + source drivers/vfio/pci/Kconfig diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile index 2398d4a..d599a67 100644 --- a/drivers/vfio/Makefile +++ b/drivers/vfio/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_VFIO) += vfio.o obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o obj-$(CONFIG_VFIO_PCI) += pci/ +obj-$(CONFIG_VFIO_DT) += vfio_dt.o diff --git a/drivers/vfio/vfio_dt.c b/drivers/vfio/vfio_dt.c new file mode 100644 index 000..ad4d31d --- /dev/null +++ b/drivers/vfio/vfio_dt.c @@ -0,0 +1,187 @@ +/* + * Copyright (C) 2013 - Virtual Open Systems + * Author: Antonios Motakis a.mota...@virtualopensystems.com + * + * 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. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +#include linux/device.h +#include linux/eventfd.h +#include linux/interrupt.h +#include linux/iommu.h +#include linux/module.h +#include linux/mutex.h +#include linux/notifier.h +#include linux/pm_runtime.h +#include linux/slab.h +#include linux/types.h +#include linux/uaccess.h +#include linux/vfio.h + +#define DRIVER_VERSION 0.1 +#define DRIVER_AUTHOR Antonios Motakis a.mota...@virtualopensystems.com +#define DRIVER_DESC VFIO Device Tree devices - User Level meta-driver + +struct vfio_dt_device { + struct platform_device *pdev; +}; + +static void vfio_dt_release(void *device_data) +{ + module_put(THIS_MODULE); +} + +static int vfio_dt_open(void *device_data) +{ + if (!try_module_get(THIS_MODULE)) + return -ENODEV; + + return 0; +} + +static long vfio_dt_ioctl(void *device_data, +unsigned int cmd, unsigned long arg) +{ + struct vfio_dt_device *vdev = device_data; + unsigned long minsz; + + if (cmd == VFIO_DEVICE_GET_INFO) { + struct vfio_device_info info; + + minsz = offsetofend(struct vfio_device_info, num_irqs); + + if (copy_from_user(info, (void __user *)arg, minsz)) + return -EFAULT; + + if (info.argsz minsz) + return -EINVAL; + + info.flags = VFIO_DEVICE_FLAGS_DT; + info.num_regions = 0; + info.num_irqs = 0; + + return copy_to_user((void __user *)arg, info, minsz); + + } else if (cmd == VFIO_DEVICE_GET_REGION_INFO) + return -EINVAL; + + else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) + return -EINVAL; + + else if (cmd == VFIO_DEVICE_SET_IRQS) + return -EINVAL; + + else if (cmd == VFIO_DEVICE_RESET) + return -EINVAL; + + return -ENOTTY; +} + +static ssize_t vfio_dt_read(void *device_data, char __user *buf, + size_t count, loff_t *ppos) +{ + return 0; +} + +static ssize_t vfio_dt_write(void *device_data, const char __user *buf, + size_t count, loff_t *ppos) +{ + return 0; +} + +static int vfio_dt_mmap(void *device_data, struct vm_area_struct *vma) +{ + return -EINVAL; +} + +static const struct vfio_device_ops vfio_dt_ops = { + .name = vfio-dts, + .open = vfio_dt_open, + .release=
Re: [PATCH] ARM: EXYNOS: add power domains support for EXYNOS5440
On Fri, Aug 02, 2013 at 10:01:52PM +0100, Stephen Warren wrote: (CCing the other DT maintainers, hence quoting binding in full) On 08/02/2013 07:23 AM, Bartlomiej Zolnierkiewicz wrote: On EXYNOS5440 power domains are handled in a different way than on the previous EXYNOS SoCs. Add support for them to EXYNOS pm_domains driver. Then add device tree nodes for PCIe (controlling power for PCIe host controller) and Conn2 (controlling power for Ethernet, SATA and USB controllers) power domains to exynos5440.dtsi. Currently if runtime Power Management is enabled and the driver for devices under power domain is disabled then the power domain will be disabled by EXYNOS pm_domains driver. To make more active use of power domains (dynamically enable and disabled them as needed) it is required to add runtime PM support to pci-exynos, stmmac, ahci_platform, ohci-exynos and ehci-s5p drivers (to be done later). Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com Signed-off-by: Myungjoo Ham myungjoo@samsung.com Cc: Tomasz Figa t.f...@samsung.com Cc: Stephen Warren swar...@wwwdotorg.org --- Documentation/devicetree/bindings/arm/exynos/power_domain.txt | 33 ++ arch/arm/boot/dts/exynos5440.dtsi | 23 + arch/arm/mach-exynos/Kconfig |1 arch/arm/mach-exynos/common.c |4 arch/arm/mach-exynos/pm_domains.c | 138 +- 5 files changed, 190 insertions(+), 9 deletions(-) Index: b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt === --- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt 2013-08-02 14:45:53.551392396 +0200 +++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt 2013-08-02 14:46:29.799391845 +0200 @@ -5,20 +5,47 @@ to gate power to one or more peripherals Required Properties: - compatible: should be one of the following. -* samsung,exynos4210-pd - for exynos4210 type power domain. +* samsung,exynos4210-pd - for Exynos4210 type power domain. +* samsung,exynos5440-pd - for Exynos5440 type power domain. - reg: physical base address of the controller and length of memory mapped -region. +region (Exynos4210 type power domain) or bit offset in the control +register (Exynos5440 type power domain). + +Additional parent node must be created for Exynos5440 power domains with +the following required properties: +- compatible: samsung,exynos5440-power-domains, simple-bus +- reg: physical base address of the XMU controller and length of memory +mapped region It's a little odd to describe the child nodes first. Given the 4210 and 5440 bindings work so differently, I'd suggest making separate binding files for the two; samsung,exynos4210-pd.txt and samsung,exynos5440-pd.txt. The node being describe here clearly is not a simple-bus; the child nodes appear to have a specific need that their parent be compatible = samsung,exynos5440-power-domains, hence they aren't the independent devices that simple-bus would usually contain. +1. This is most definitely not a simple-bus, the child nodes reg properties cdon't represent the same address space as the parent's reg property. How much does the association of bit-offsets to power domains vary? Note that I only briefly reviewed the low-level structural aspects of the binding that I mentioned above; I haven't thought about the binding at a higher level of e.g. does this binding conceptually make sense. This is unfortunately difficult to do :( Node of a device using power domains must have a samsung,power-domain property defined with a phandle to respective power domain. -Example: +Example for Exynos4210 compatible power domain: lcd0: power-domain-lcd0 { compatible = samsung,exynos4210-pd; reg = 0x10023C00 0x10; }; +Example for Exynos5440 compatible power domains: + + power-domains@0016 { + compatible = samsung,exynos5440-power-domains, simple-bus; + reg = 0x0016 0x1000; + #address-cells = 1; + #size-cells = 0; + + pd_pcie: pcie-power-domain@6 { + compatible = samsung,exynos5440-pd; + reg = 6; + }; + + pd_conn2: conn2-power-domain@7 { + compatible = samsung,exynos5440-pd; + reg = 7; + }; + }; + Example of the node using power domain: node { Index: b/arch/arm/boot/dts/exynos5440.dtsi === --- a/arch/arm/boot/dts/exynos5440.dtsi 2013-08-02 14:45:53.599392397 +0200 +++ b/arch/arm/boot/dts/exynos5440.dtsi 2013-08-02